ACP executor modes and scoped catalog lifecycle

[Sun-GLiang]

Summary

Add optional opaque ACP mode selection across the executor catalog, Host, Desktop, and retained Session continuity. Scope catalog probes to the workspace directory, with bounded reuse and explicit invalidation. Confirm idle model and mode changes with the Agent and roll back combined changes on failure. On macOS, select an extracted ACP directory because the native picker disables the Mach-O server executable.

Refs #5103

Verification, 2026-10-01

Latest review follow-up at 6c4cd2d3e fixes Composer send gating during mode confirmation. Mode and model changes share the pending gate; drafts remain intact, failed changes never submit automatically, and selector unmount releases the gate. Five new regression cases cover success, rejection, existing model behavior, and late-response cleanup. On this head, all workspace suites passed: 14,155 tests, 14,117 passed, 38 skipped, zero failures or cancellations. Full workspace build, typecheck, lint, format, Desktop/UI Knip, renderer architecture, locale hygiene, ASF headers, and diff checks passed locally. Hosted CI for this head is pending. The preceding 3263e483c follow-up fixed model-dependent mode rollback and covered stale/invalid modes, durable selection preservation, and same-Session prompt retry. All substantive follow-ups include Generated-by: Codex; the earlier six trailer repairs preserve the original commit trees and merge topology.

The branch integrates main at 1e80e3b885 and uses compatibility epoch 204. Build, typecheck, lint, format, renderer architecture, locale hygiene, ASF headers, diff checks and the current-base protocol guard passed locally. The serial workspace suite reported 14,149 tests: 14,111 passed, 38 skipped, zero failures or cancellations. After the final epoch-only adjustment, the full build and 89 protocol tests passed again. Historical local typecheck failures are superseded by this run. CI for each push is tracked separately. Hosted CI for the pre-follow-up head a0f624dcc completed successfully, including Runtime Host tests, Desktop e2e, Storybook smoke, transcript geometry invariants, and installed CLI release-candidate validation (run).

Controlled tests cover fresh-Session model-dependent modes, idle drift, rollback, directory cache isolation, late notifications and retained Session restoration. Official Agent 1.2.1 production-executor probes separately verified 11 model IDs, three modes, A-only refresh with B cache identity retained, confirmed selections and same-Session restoration.

Real macOS arm64 Desktop acceptance then completed all nine steps in one isolated profile using the rebuilt code ccd3c195e (record-only head 9580ea7c0). Both toy projects were added through the native folder picker. B → A → B switching, A's explicit UI refresh, model/mode selection, and both real prompts passed: B used gemini-3.7-flash-high / default, A used gemini-3.8-flash-high / auto_edit, and each returned exactly ACK. Task metadata and committed continuity records confirmed the intended directories and selections.

A's idle change to gemini-3.6-flash-high / default was confirmed. After normal Cmd-Q/reopen and explicit Restore, a prompt omitting the original code received exactly A-5826-2648. A second normal restart/Restore retained the same external Session, committed phase, two prompts and confirmed configuration. Only fingerprint equality was logged. No HTTP 403 recurred, and no account or network settings were changed. The temporary native automation blocker resolved after the user foregrounded the visible app.

This was an ad-hoc signed Maka Dev development build; signed distributable qualification remains separate. Exact results and evidence scope are recorded in docs/archive/antigravity-acp-pr4-acceptance.md.

Acceptance procedure

The repeatable acceptance procedure now specifies prerequisites, automated commands, nine Desktop steps with pass criteria, evidence requirements, and the blocked-run handoff. The completed run records both project prompts and both restarts in the same profile, separately from historical runs.

Acceptance

• Verify real mode candidates and confirmed model/mode changes with a signed-in official Agent (1.2.1).
• Capture Desktop evidence for selection, refresh, idle change, successful prompts, and restart continuation with the same external Session.
• Switch B → A → B in one packaged Desktop profile and refresh A. Both projects showed the real Gemini and mode choices; B remained available without manual refresh. Production AcpExecutor probes separately confirmed that A refresh left B's cache entry unchanged.
• Complete successful prompt execution in both toy projects in one Desktop profile. Both returned ACK with confirmed directories/model/mode; idle change, restart recall, and second same-Session restoration also passed. The earlier eligibility HTTP 403 did not recur.

AI use

• No generative tool made a substantive contribution
• Generative tooling made a substantive contribution

Tool(s) and scope: Codex implemented the code, tests, and acceptance record. The commit includes Generated-by: Codex.

Checklist

• Tests cover the change and fail without it
• Lint, format, typecheck and the workspace suites pass locally after integrating current main (14,111 passed, 38 skipped; pre-follow-up hosted CI also passed; latest follow-up CI pending).

Does this PR entail a change in behavior?

• Yes — described under Summary above
• No

[jackwener]

[kabi-sol]

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

No P0–P2 found in the assigned behavior scope at 5ccbfdb355ba5e44a91dc97822326deb273cbc57.

Mounted the production PluginExecutorService with AcpExecutor and a controlled ACP connection. Model-only, mode-only and combined changes remain pending, with the old durable configuration intact, until the Agent response is released. A second-option failure restores both Agent values and the persisted configuration. Directory-specific candidates stay separate; probes verified the 16-entry cache bound, 60-second expiry, explicit invalidation and rejection of a stale in-flight probe after invalidation.

All 65 ACP-plugin/service/catalog tests passed, including retained external-Session restoration and late-notification cases. As a control, the current tests against the parent commit's plugin implementation fail on caller cancellation, clearing a mode removed by a model change, and an unsolicited idle configuration notification. The last commit therefore fixes observable failures, rather than only changing assertions. Its Host-side complete-snapshot replacement was inspected, not separately exercised here.

Core/storage/MCP/runtime/ACP-plugin builds passed; current-head CI test is successful. No authenticated official Agent, real process-restart persistence, Desktop UI, full Host integration, or full repository suite was run in this review. The connection and state store in the behavioral probes are controlled test doubles.

[jackwener]

[kabi-opus-dev] Review of the protocol (epoch, plugin-platform.ts compatibility, executor-catalog.ts decoding), Desktop IPC/preload/bridge signatures, and the architecture gate. Automated review (Claude). Same owner as the coordinating seat, so this is not independent corroboration.

Bound to 5ccbfdb355ba5e44a91dc97822326deb273cbc57; head re-checked before publishing. CI test is green on this head. That matters here: the P1 below ships through a green CI because no test sends a catalog query through the real decoder without refresh.

Conclusion: 1×P1, 1×P3 (both inline). Everything else in my scope holds.

P1 — every non-refresh executor catalog query is rejected

plugin-platform.ts:345 lists refresh in requireExactRecord, but assertExactKeys (codec.ts:53-68) requires every listed key to be present. The Desktop IPC handler (runtime-host-session-catalog-ipc-main.ts:122) sends refresh only when it is true, and the renderer's ordinary load (use-executor-selection.ts:81, force false/undefined) is exactly that case. client.request runs spec.decodeInput(input) before sending (client/connection.ts:510, client/reconnecting-connection.ts:295), so the request is rejected on the client with Invalid Executor catalog fields, and the new-task executor picker lands in its error state with an empty catalog. Details, reproduction and a verified one-line fix are inline.

① Protocol

• Epoch: 198 → 199 with a changelog line. The merge-base and current origin/main (0f98c2a48, 2 commits ahead) are both at 198, and git merge-tree --write-tree HEAD origin/main exits 0. No epoch conflict.
• plugin-platform.ts and older peers: mixed-epoch peers are rejected at the handshake, so a 198 peer never sees refresh, modes, currentMode or supportsModeChange. The compatibility story is the epoch, and it is bumped. Within epoch 199 the new optional field is not actually optional: that is the P1.
• executor-catalog.ts strictness: the mode configuration validation mirrors model (non-empty, ≤1024, no NUL/CR/LF), and the protocol test adds '', 'bad\nvalue' and 4. modes is capped at 64, ids are deduped, names are checked by isCatalogText, and the output is re-frozen with only {id, name}. One gap: a mode entry with no id is accepted (P3 inline). currentMode is not required to be a member of modes; that matches the existing currentModel / models precedent, so I am not grading it.

② Desktop IPC / preload / bridge

The signature grows by an optional refresh?: boolean consistently in bridge-contract.d.ts:1022, preload.ts:1928, conversation/ports.ts:91, and the IPC handler, which also rejects a non-boolean refresh. The only caller is use-executor-selection.ts:81. Typecheck of desktop tsconfig.{preload,main,renderer,storybook} (run individually) plus @maka/core, @maka/runtime-host, @maka/runtime, @maka/ui and @maka/acp-executor-plugin: all exit 0, so every implementer and caller is in sync at the type level. The P1 sits below the type system: the TS type says optional, and the runtime decoder says required.

③ Architecture gate

--base ae71ab319 --strict-base and --base 0f98c2a48 --strict-base (current main): both passed, fixtures 112/112. The PR does not touch renderer-architecture.json, and regenerating it with --write at head gives a byte-identical file. Knip --workspace apps/desktop and --workspace packages/ui both exit 0.

Tests run

All 8 touched test files: core 9/9, runtime-host 109/109, runtime 12/12, ui 22/22, acp-executor-plugin 44/44, desktop 6/6. They all pass alongside the P1, which is the coverage gap.

Not verified

I did not test mode selection behaviour end to end against a real ACP agent, the scoped catalog lifecycle or refresh semantics in the Host coordinator, UI rendering, E2E, or the full suite.

[zhiiw]

Bound to 5ccbfdb355ba5e44a91dc97822326deb273cbc57 (re-checked against GitHub immediately before posting, unmoved). Not draft; CI label + test green on this head. Real Windows machine, Node 24.18.1.

No P0–P2, no P3, no inline comments. Both named ablations land on exactly their pins; the path-casing question has a direct answer.

Suites on this head (Windows) and attribution

• acp-executor-plugin 44/44 · runtime plugin-executor-service 12/12 · ui executor-model-picker 22/22 · core executor-catalog 9/9 — all green.
• runtime-host session-catalog-coordinator + session-catalog-protocol + external-agent-setup-coordinator: 107/109 — both failures are the known no-Developer-Mode environment class: EPERM: operation not permitted, symlink while creating their fixtures (creation persists a canonical cwd… and Host-path relocation canonicalizes once…). Both are symlink fixtures for the canonical-cwd path; neither can create its fixture on this machine. Zero assertion failures.

Path casing / drive letters on the catalog key (asked specifically)

The cache key is await realpath(resolve(input.cwd)) (index.ts:212). Direct probe on this machine: C:/Users/wzy, c:/users/wzy, C:\Users\wzy all resolve to the identical on-disk casing C:\Users\wzy — so case variants and slash styles of one directory share one cache entry on Windows, and a symlinked spelling lands on its target's key. The two EPERM-skipped fixtures are exactly the symlink-arm of this; the casing arm is verified above without them.

Ablations (each restored, rebuilt, re-verified — full plugin file 44/44 green after)

1. Combined-change rollback removed (the restore-both-fields-and-verify loop in the catch) → exactly a failed second configuration option restores both model and mode goes red.
2. Catalog scope made global (the cache key forced to a constant instead of the canonicalized cwd) → exactly catalog reuse is scoped to cwd and refresh only replaces that scope goes red.

Read, no finding

• The provider-confirmation-as-complete-snapshot semantics (an omitted option is cleared) is consistent across the plugin (withConfirmedConfiguration), the coordinator's confirmation check (#mergeConfigurationPatch + the model/mode mismatch throw), and the fingerprint (mode included). The idle notification path refuses to adopt a notification that disagrees with the local model/mode during a mutation — the race is closed from both directions.
• The busy gate for executor configuration now also covers isTurnBusy and non-active headers — a stricter gate, matching the confirmation contract.

Not checked

The two symlink-fixture tests (environment, above), e2e, full repo suite.

UTC 2026-09-29 11:05.

[hqhq1025]

I reviewed the ACP executor catalog/mode path, the Desktop picker-to-Host request path, and the protocol boundary at commit 5ccbfdb355ba5e44a91dc97822326deb273cbc57. This is not ready to merge.

The P1 already reported on this head is reproducible: plugin-platform.ts:345 passes ['kind', 'cwd', 'refresh'] to requireExactRecord, whose assertExactKeys requires every listed key (codec.ts:63-66). The normal Desktop IPC request omits refresh unless it is true (runtime-host-session-catalog-ipc-main.ts:119-122), and the client decodes before sending (connection.ts:510). A direct call to the built decoder with {kind:'catalog',cwd:'/tmp'} throws Invalid Executor catalog fields, while refresh:true succeeds. Thus opening the executor picker without a forced refresh cannot obtain its catalog. I am not duplicating the existing inline finding. I also confirmed the separately reported P3 validation gap in executor-catalog.ts:130-133: isExecutorConfiguration({mode: undefined}) does not establish that each mode entry has a string id.

The current-head hosted test and label checks pass, and a fresh-main merge-tree and diff check are clean. Locally, core/storage/runtime/runtime-host builds and 44 Plugin Platform tests passed. I did not exercise a real ACP agent, packaged Desktop, or the complete repository suite. The protocol P1 remains a merge blocker despite those green checks.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Reviewed at 5ccbfdb3 and re-checked at exact head 9754140008d0eaba09d82b2efd55a7007ae898df. The new commit fixes the earlier refresh codec P1 and the mode.id P3. It doesn't touch session-catalog-coordinator.ts or acp-executor-plugin/src/index.ts, so the findings below, and their line numbers, still apply to this head.

P1: a model or mode change is now rejected unless the Session is active (session-catalog-coordinator.ts:857). On main, only waiting_for_user was blocked. A failed turn leaves the Session blocked, and a user stop leaves it aborted, so the model can no longer be switched after a model or quota failure. restore() for a restorable Session whose last turn failed is rejected too. Sending is already blocked while readiness isn't ready, so that Session has no way forward. Reproduced: with the compiled coordinator fixture, blocked/aborted return operation_conflict and configureExecutor is never called, while active succeeds.

P1: agent-originated config_option_update is dropped when model or mode differs (acp-executor-plugin/src/index.ts:978-985). This leaves configOptions stale, so the next prompt skips setConfigOption and runs on the agent's own configuration, while Maka reports and persists the old one. Reproduced: I changed only the fake agent so that its notification really changes its state. The existing test then shows a prompt that requested default running on fast. Modes can matter for permissions (plan vs. edit).

P2:

• On restore, a model or mode mismatch fails before any attempt to re-apply the saved configuration, so every retry fails the same way (index.ts:664-673).
• Catalog discovery now opens an agent session in the user's real workspace every 60s per directory, where it used to use a disposable temp directory (index.ts:212). This needs a trust/design sign-off.

P3: refreshing aborts the shared in-flight probe, so callers already waiting on it get unavailable. A mode-only change with no live session also resets the model to the agent default (both reproduced).

Other notes:

• This head sets RUNTIME_HOST_COMPATIBILITY_EPOCH to 199 (packages/runtime-host/src/protocol/index.ts:107). Several other open PRs also claim 199, so whichever merges later needs to renumber.
• Older builds will reject session headers that carry executorConfig.mode, so downgrading is not safe.

Checks and tests:

• git diff --check, check:asf-headers, check:app-shell-hooks and check:renderer-architecture all pass.
• After targeted builds, all 196 tests in the 7 affected test files pass.
• Not run: the desktop suite, the full workspace tests, and a real ACP agent.

Automated review notice: This comment was posted by an automated review agent (Claude) operating on behalf of @Astro-Han. It is not an independent human review and does not replace one.

[hqhq1025]

Reviewed the two commits since 5ccbfdb3 at exact head 1d5525cb3fe85eb0b43c89d5ce47b98ec375aad1, focusing on the prior protocol/configuration findings, ACP restore and notification flow, Host configuration changes, and added regressions. One new P2 is inline; I would not merge this head yet.

The ordinary catalog request now decodes without refresh, and a mode entry missing id is rejected. The Host permits idle configuration changes after blocked/aborted turns, restore re-applies saved model/mode, and a mode-only edit passes the merged configuration to the provider. The awaited probe is redirected after a refresh. The prior concern about opening an ACP agent session in the real workspace during discovery remains a trust/design question; I did not verify a real agent's workspace hooks or external history behavior. This PR's protocol epoch is 199 and must be reconciled with whichever concurrent epoch-199 change merges first.

Core, ACP plugin, and runtime-host builds passed on Node 24. All 220 focused tests across the four affected test modules passed. The current-head hosted test check passed; fresh main ec324da4 merged cleanly and git diff --check passed. I did not run a real ACP agent, packaged Desktop, or the full workspace suite.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Astro-Han]

Re-reviewed exact head 1d5525cb3fe85eb0b43c89d5ce47b98ec375aad1 against my earlier findings on 97541400.

Fixed

• Config changes on blocked/aborted Sessions: only running/waiting_for_user are rejected now, and there are tests for all four states.
• Divergent agent notifications: they now update the cache without being persisted as confirmed. The adopted test uses a fake agent that really switches.
• Restore: it re-applies the saved model/mode and fails only if the agent still disagrees.
• The partial-patch reset: the merged config is now sent.
• Refresh no longer strands callers waiting on the in-flight probe. invalidateCatalog() on auth failure still resolves them as unavailable, which is minor.

Still open

• The catalog probe still opens an agent session in the user's real workspace (every 60s per directory) instead of a disposable directory. This needs an explicit trust/design decision.
• The separate P2 from the other review on this head still applies: drift that arrives during an active prompt is persisted as confirmed at ack time.

New P3s (both reproduced with temporary tests)

• index.ts:1034 (inline): a second identical drifted notification is persisted as confirmed.
• session-catalog-coordinator.ts:872: sending the merged config means a model-only change also re-applies a saved mode. That fails with acp_config_unavailable: mode if the new model has no mode option. This is a narrow case.

Epoch
The epoch is still 199 (packages/runtime-host/src/protocol/index.ts:107), so the collision with other open PRs remains.

Checks run

• git diff --check is clean.
• The ASF header, renderer architecture and app-shell hooks checks pass.
• Targeted tests pass: acp-executor-plugin 46/46, coordinator 76/76, session-catalog-protocol 24/24, protocol 89/89, executor-catalog 9/9, plugin-executor-service 12/12.
• Not run: anything against a real agent, and the full suite.

Automated review notice: This comment was posted by an automated review agent (Claude) operating on behalf of @Astro-Han. It is not an independent human review and does not replace one.

[Sun-GLiang]

Follow-up on the latest review: 2c98f49 fixes the fresh-Session model-only case where the selected model removes the prior mode. Initialization now applies the merged selection once; a regression test confirms that the provider returns { model: fast } and clears the saved mode. The two notification-drift findings are also fixed and tested in the same commit. The workspace-probe trust assumption is documented and discussed in its inline thread. The protocol epoch is still 199; apache/main currently remains at 198, so an epoch collision depends on which concurrent PR merges first and should be reconciled against main before merge.

[Astro-Han]

Re-checked at 2c98f49b and re-bound to exact head 5b786f8d90a250a4b43a4f4bb0ce5a0bd165fd3e. The two newest commits only touch use-executor-selection.ts, its test, and the acceptance doc. acp-executor-plugin/src/index.ts and session-catalog-coordinator.ts are unchanged, so the findings below apply to this head.

Fixed since my last review: drift during an active prompt is no longer persisted as confirmed at ack time. A repeated drift notification no longer overwrites the saved selection either, since agent notifications no longer write durable state. The PR's new tests and my reproductions both keep the saved model/mode.

Still open:

• Model-only change re-applying the saved mode (session-catalog-coordinator.ts:872, P3) is only partly fixed. It still fails with acp_config_unavailable: mode for an existing Session whose mode drifted while idle. It also fails for a fresh Session whose saved mode isn't the agent default.
• The catalog probe in the real workspace is now documented in the acceptance doc (use discovery only with an agent trusted for that workspace, probes across directories are unbounded), but the code is unchanged. This still needs a maintainer design decision.
• invalidateCatalog() waiters still resolve unavailable (minor).

New P3, a regression from 2c98f49b (inline at index.ts:360): on a fresh Session, configureConversation now applies the launch config first (for Antigravity, the configured default model) and then validates the requested config against that model's options. If the launch model has no mode option, {model:'default', mode:'auto'} fails with acp_config_unavailable: mode before anything is applied, and the fresh Session is lost. I reproduced it: the same scenario succeeds on 1d5525cb and fails on 2c98f49b. The prompt path is unaffected.

Epoch: still 199 (packages/runtime-host/src/protocol/index.ts:107) vs 198 on main.

Checks (at 2c98f49b):

• git diff --check clean, check:asf-headers passes.
• Tests: plugin 48/48, coordinator 76/76, catalog protocol 24/24, core catalog 9/9, antigravity 9/9.
• Not run: the full suite, lint, and anything against a real agent.

Automated review notice: This comment was posted by an automated review agent (Claude) operating on behalf of @Astro-Han. It is not an independent human review and does not replace one.

[hqhq1025]

Reviewed exact head 5b786f8d90a250a4b43a4f4bb0ce5a0bd165fd3e, focusing on the three commits since 1d5525cb: ACP confirmation/persistence, the Desktop executor-selection hook and its regressions, and the acceptance-document update. The earlier P2 is fixed on this head: an Agent-originated configuration update changes the observed options, but neither acknowledgement nor the notification persists it as the user's confirmed selection (acp-executor-plugin/src/index.ts:813-834,1019-1027). The Desktop hook now gives saved or locally confirmed configuration precedence over observed Agent values (use-executor-selection.ts:154-174,195-215). I found no additional substantiated P0–P3 issue in these inspected paths.

This is not a merge endorsement. The separately reported fresh-Session mode availability P3 remains in the configuration path (index.ts:354-361,786-793), and discovery still opens an Agent session in the selected workspace. The new documentation describes that trust assumption, but a maintainer still needs to accept the design. The acceptance document reports an authenticated official-Agent follow-up; I did not independently run that Agent or verify its claims.

On Node 24, the ACP plugin build and 48 focused ACP tests passed. The emitted current-head Desktop executor-selection test passed all 7 cases. A full UI/Desktop TypeScript build did not pass locally because SideNavItemProps.trailingAction is missing in an unchanged UI path, so I do not count it as a build pass. Fresh main 53f566b4 merges cleanly and git diff --check passes. The current-head hosted test check was pending when this review was posted. I did not run a packaged Desktop or the full repository suite.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

Reviewed exact head e9a1b18ff2e829d699947f1250cf10491f69cff5. This commit changes the macOS “choose existing program” picker from a file to the extracted directory and appends agy_acp_server.par; other platforms retain file selection (apps/desktop/src/main/runtime-host-boot.ts:1627-1634). The selected path is saved by the settings page, and connection setup verifies the server file and adjacent localharness_external helper before launching (packages/runtime-host/src/server/acp/antigravity.ts:62-65,128-139). Locale copy and the acceptance document were updated. I found no new substantiated P0–P3 issue in this increment.

The acceptance document reports a successful macOS development-build flow, but I did not independently reproduce the signed-in Agent or packaged Desktop path. A previously reported fresh-Session mode-availability P3 remains outside this commit. The current-head test check is red in the unchanged Runtime Host gitoxide-helper-invocation-internal process-identity wait (1 failure among 2,197 tests); it must be rerun or resolved before merge. The PR merges cleanly with fetched main and passes git diff --check. Protocol epoch 199 and the real-workspace catalog-probe trust assumption still require maintainer resolution; this is not merge approval.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[Sun-GLiang]

Follow-up to @hqhq1025's review at e9a1b18: I merged current apache/main through c61cf9be9 in ff70ec9d8. The storage-usage change remains epoch 199, Agent Graph remains 200, and this PR's ACP mode/catalog wire change now advances the epoch to 201. The protocol conflict is resolved; GitHub now reports the branch as mergeable.

Validation: node scripts/protocol-epoch-check.mjs --base upstream/main passes (200 → 201), the 17 protocol-epoch guard tests passed during conflict resolution, and git diff upstream/main...HEAD --check is clean. The previous head's failed test run was rerun successfully; CI for ff70ec9d8 is still running.

The fresh-Session mode-availability P3 remains open, and I have left its review thread unresolved. The workspace-probe trust assumption remains documented for maintainer review. Already-fixed inline findings have individual replies, so I have not duplicated them.Follow-up to @hqhq1025's review at e9a1b18 I merged current apache/main through c61cf9b in ff70ec9. Storage usage remains protocol epoch 199 Agent Graph remains 200 and this PR's ACP mode/catalog change advances the epoch to 201. GitHub now reports the branch as mergeable.

The merge-result protocol epoch guard passes 200 to 201 all 17 protocol epoch guard tests passed during conflict resolution and git diff --check is clean. The previous head's failed test run was rerun successfully CI for ff70ec9 is still running.

The fresh-Session mode-availability P3 remains open and its thread remains unresolved. The workspace-probe trust assumption remains documented for maintainer review. Already-fixed inline findings have individual replies so I have not duplicated them.

[hqhq1025]

Reviewed the current head, including the model-before-mode configuration repair and the subsequent acceptance-document updates. packages/acp-executor-plugin/src/index.ts:877-939 now selects the model first, validates the dependent mode against the Agent’s returned options, and rolls back an invalid post-model selection. The added regression cases cover a fresh Session, idle drift, and rollback. The merge advances the ACP wire change to epoch 201 after main’s epoch 200 (packages/runtime-host/src/protocol/index.ts:107-112). I found no substantiated new P0–P3 issue in the inspected increment.

Node 24 ACP plugin build and all 51 plugin tests pass; the protocol epoch guard, fresh-main merge-tree, and diff check pass. The current-head hosted test is still running, so the gate is not yet green. I did not independently reproduce the documented authenticated official-Agent/Desktop runs, cross-project UI switching, or a signed distributable. Catalog probing invokes the configured Agent in the chosen workspace, so the trust decision described in the acceptance record remains for maintainers.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[hqhq1025]

The only change since the prior reviewed head is the acceptance record (docs/archive/antigravity-acp-pr4-acceptance.md:65-67,86). It now reports a B → A → B Desktop project switch, an A-only refresh, and catalog/mode availability in both projects. It also distinguishes that UI observation from the executor cache-identity check and explicitly states that new prompts in the second profile failed with HTTP 403. No product code or protocol changed in this increment; I found no substantiated new P0–P3 code issue.

The current-head hosted test check passes, and a fresh-main merge-tree and diff check are clean. I did not independently reproduce the documented official-Agent or packaged Desktop interactions. Successful cross-project prompt execution remains unverified, and maintainers still need to decide whether catalog probing a configured Agent in the selected workspace matches the intended trust boundary.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

[jackwener]

Reviewed exact head a0f624dccccc8dca6269b2ca360a6b8b2086c280 against base and current main at 1e80e3b885d963b799f7e2e9a70083ceda99943d.

Findings

P1 — restore the model before restoring its model-dependent mode

The failure rollback in packages/acp-executor-plugin/src/index.ts restores mode before model. That order fails when the two models expose different mode values. For example, start with model default and mode ask, then request model fast plus a mode that is stale or invalid for fast, where fast exposes only auto. The Agent first changes to fast and auto; validation rejects the requested mode; rollback then asks the still-fast Agent to select the old ask mode. The Agent rejects that value, so the loop never restores the original model and the catch path loses the retained Session. A normal failed configuration change therefore leaves the task in a history-only or restore-failed state instead of preserving the previous confirmed configuration.

I reproduced this with a temporary production-provider regression whose mode candidates differed by model. At this head it failed with the Agent still on fast instead of the expected default. Changing only the rollback order from ['mode', 'model'] to ['model', 'mode'] made the regression pass. Please restore the model first, then re-read and restore its dependent mode, and keep a regression with model-specific mode candidates.

P2 — add the required Generated-by: Codex trailers

The PR states that Codex implemented the code, tests, and acceptance record, but six affected non-merge commits do not carry the required trailer: 5ccbfdb3, 8784e033, 222a7e7d, 4d0b290d, e7088b07, and d8d36d71. CONTRIBUTING.md requires Generated-by: <tool> on each affected commit. Please amend these commits while preserving the trailer through the final history.

Re-review evidence

The earlier catalog-decoder P1 is fixed: the real decoder accepts catalog requests with refresh omitted, false, or true, while rejecting unknown fields and non-boolean values. The earlier mode-entry P3 is also fixed: catalog normalization now requires a string mode.id. The subsequent busy-state, restore, Agent-drift, refresh handoff, merged-configuration, and model-before-mode fixes are present and covered by the affected suites.

On Node 24.18.1, the affected Core, Storage, MCP, Runtime, ACP plugin, Runtime Host, UI, Computer Use, and Desktop test builds passed. Ten focused files passed 349/349 tests; after restoring the exact source following the diagnostic mutation, the ACP plugin suite passed 51/51. ASF headers, locale hygiene, renderer architecture 121/121, AppShell hooks, diff checking, and the protocol epoch guard passed. The exact-head hosted test check is successful. The branch already contains current main as its merge base and GitHub reports it mergeable, so there is no additional merge delta.

I did not independently rerun the authenticated official Agent or the packaged Desktop acceptance described in the PR. The old fresh-Session P3 review thread remains mechanically unresolved, although the code and regression at 8784e033 close its reported behavior.

Conclusion: BLOCKED — 1×P1 and 1×P2 remain.

---

Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.

[Sun-GLiang]

Automated follow-up by Codex on behalf of the PR author.

Both findings in the latest review are valid and addressed at 3263e483c9c1853723880e028fe8f28dbef384de.

• P1: rollback now restores the model first, then reads the Agent's returned options to restore the dependent mode. The regression uses distinct mode candidates (default: auto/ask; fast: auto) and a model change that resets the mode. Both a stale ask selection and an invalid invented selection failed against the previous implementation with the Agent still on fast. Both pass with the fix, preserving the durable selection, ready state, external Session ID, and a subsequent successful prompt on default/ask without creating another Session.
• P2: added Generated-by: Codex to all six cited non-merge commits: 5ccbfdb35 → c925d2672, 8784e033a → 364369898, 222a7e7d8 → 88a36a296, 4d0b290db → 812ef4792, e7088b07b → 82d0d3334, and d8d36d71a → da5aa4e35. Each rewritten commit retains its original tree, author/committer metadata, and merge topology. All PR non-merge commits now carry the trailer. The branch update used an explicit lease on the reviewed head a0f624dcc.

Validation: all 75 ACP plugin tests and 98 related Runtime service, Host coordinator, and Antigravity tests passed (173 total). Full workspace build, typecheck, lint, format, both Desktop/UI Knip checks, ASF headers, protocol epoch guard (202 → 204), and diff checks passed. New-head hosted CI has not completed yet; the previously green run belongs to a0f624dcc.

The existing authenticated-Agent/Desktop acceptance remains historical evidence; it was not rerun for this focused rollback fix.

[Astro-Han]

Re-review at 6c4cd2d. My last review was at 5b786f8. The branch has since been rewritten for trailers and has merged main; range-diff shows that the earlier commits are unchanged. This review covers the delta: 364369898 (validate mode after model), 3263e483c (rollback order), 6c4cd2d3e (mode-pending send gate), a85b38107 (macOS picker), the epoch move, and doc updates. All 20 non-merge commits now carry Generated-by.

Earlier findings:

• Model-only change re-asserting a saved mode the new model drops (P3): fixed. The mode is validated against the post-model options, and a mode option that disappears after a model change is skipped. My temporary repro now gives:
  • existing Session {default, auto} → {fast, auto} with fast dropping modes: returns {model: fast} on the same Session;
  • switching back to default re-applies auto.
• Fresh Session applying the launch model before validating the requested mode (P3, regression in 2c98f49): fixed. The merged {...launch.initialConfig, ...input} is applied once, and a PR test covers it.
• Rollback restoring mode before model (jackwener's P1): fixed, and covered by stale-mode and invented-mode tests with candidates that differ per model.
• Mode-pending send gate: sound. Hooks run before the early return, the gate is released on unmount, and the model picker, restore and thinking controls honour the combined pending state.
• invalidateCatalog() waiters resolving unavailable: unchanged. Still a minor P3.

Catalog probe design, still open (P2, needs maintainer sign-off): discover() still calls the Agent's session/new with cwd set to the user's real workspace, once per directory with a 60s TTL, and nothing bounds probes across distinct directories. The acceptance doc (L242) now states the trust assumption: the Agent may read workspace config, run startup hooks, or keep the probe Session in its history. Real-Agent runs confirmed the catalog contents, but nobody has checked those side effects. If the per-workspace catalog is not strictly needed, there are lower-risk options:

• probe in a neutral temp directory for the picker, and read workspace-specific options only from a retained task Session (inspectConversation already does this);
• or keep the probe Session and reuse it as the first task Session, instead of discarding it and leaving an orphan in the Agent's history;
• either way, cap concurrent probes.

Related design note: the real Agent exposes a yolo mode. Agent-side auto-approval bypasses Maka's permission prompts, so the picker copy should make clear that this is an Agent setting.

Tests run locally:

• acp-executor-plugin: 52/52
• ui executor-model-picker: 27/27
• core executor-catalog: 9/9
• runtime-host session-catalog-coordinator, session-catalog-protocol, protocol and external-agent-setup: 202/202
• runtime plugin-executor-service: 12/12
• antigravity: 9/9

CI: test passes. The PR merges cleanly.

Verdict: the code findings from earlier rounds are resolved. What remains is the probe trust-boundary decision (P2, design) and two small P3s, inline.

Automated review by Claude (Anthropic), posted on behalf of the maintainer. It is not an independent human review.

[Astro-Han]

P3: the epoch history now has two entries for this one PR, and one of them describes an epoch it never ships.

main is at 202, and this PR jumps to 204. That is fine for the guard, and it sidesteps the 203 that #5902, #5753 and #5495 claim. However:

• The 204: line ("reconcile with main's archive/removal protocol") describes a merge, not a wire change.
• The 203: line describes this PR's real change under a number that other PRs will use for something else.

Once any of those lands, the comment log will have two different 203: meanings. Please collapse these into a single // 204: Executor catalogs and Session configuration carry opaque mode IDs; catalog queries may request a provider refresh... entry.

[Astro-Han]

P3: on macOS this accepts only a directory and blindly appends agy_acp_server.par.

Nothing checks that the file exists. A user who picks the binary itself (which is possible when it is not quarantined), or a folder from a differently laid-out release, gets a saved path that points nowhere. They only find out at "check connection".

Fix: use ['openFile', 'openDirectory'] on darwin. Join the file name only when a directory was selected, and verify that it (and localharness_external, as the help copy says) exists before returning. Otherwise return undefined or an error so that the settings page can say what is missing.