Skip to content

fix(acp): refresh direct runtime on config#1956

Merged
zerob13 merged 1 commit into
devfrom
codex/acp-runtime-refresh-fix
Jul 13, 2026
Merged

fix(acp): refresh direct runtime on config#1956
zerob13 merged 1 commit into
devfrom
codex/acp-runtime-refresh-fix

Conversation

@zerob13

@zerob13 zerob13 commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • close enabled direct ACP runtimes before the global ACP provider is disabled
  • refresh changed or removed registry-backed runtimes after registry initialization and manual refresh
  • add regression coverage for both lifecycle paths

Root cause

The compatibility provider no longer owns the shared ACP runtime. Global disable and registry refresh only updated provider/catalog state, leaving hydrated direct runtimes alive or stale.

Validation

  • pnpm run typecheck
  • pnpm run test:main (341 files, 3706 tests passed)
  • pnpm exec vitest run test/main/presenter/configPresenter/fontSizeSettings.test.ts
  • pnpm run format
  • pnpm run i18n
  • pnpm run lint

Follow-up to #1952.

Summary by CodeRabbit

  • Bug Fixes
    • Improved ACP registry synchronization by refreshing only agents whose configurations changed.
    • Ensured affected ACP agents are refreshed before ACP is disabled.
    • Improved initialization and registry refresh behavior to keep agent state up to date.

@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 11c63156-c3b7-4902-9e0c-7d2f6fd41423

📥 Commits

Reviewing files that changed from the base of the PR and between 6d6f396 and 7227452.

📒 Files selected for processing (2)
  • src/main/presenter/configPresenter/index.ts
  • test/main/presenter/configPresenter/fontSizeSettings.test.ts

📝 Walkthrough

Walkthrough

Changes

ACP registry refresh flow

Layer / File(s) Summary
Registry agent change detection
src/main/presenter/configPresenter/index.ts
Adds deep comparison of previous and updated ACP registry agents to identify changed IDs.
Registry lifecycle integration
src/main/presenter/configPresenter/index.ts, test/main/presenter/configPresenter/fontSizeSettings.test.ts
Applies selective provider refreshes during initialization and registry refresh, refreshes enabled direct agents before ACP disable cleanup, and tests both behaviors.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ConfigPresenter
  participant acpRegistryService
  participant Repository
  participant ACPProviderAgents
  ConfigPresenter->>acpRegistryService: Refresh registry
  acpRegistryService-->>ConfigPresenter: Return updated agents
  ConfigPresenter->>Repository: Sync registry state
  ConfigPresenter->>ACPProviderAgents: Refresh changed agent IDs
Loading

Possibly related PRs

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/acp-runtime-refresh-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@zerob13 zerob13 marked this pull request as ready for review July 13, 2026 07:36
@zerob13 zerob13 merged commit 28e2a0e into dev Jul 13, 2026
3 of 4 checks passed
@zhangmo8 zhangmo8 deleted the codex/acp-runtime-refresh-fix branch July 13, 2026 07:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant