Skip to content

fix(acp): persist model catalogs and scope discovery per agent - #6197

Open
Sun-GLiang wants to merge 14 commits into
apache:mainfrom
Sun-GLiang:codex/fix-acp-catalog-cache
Open

Sun-GLiang wants to merge 14 commits into
apache:mainfrom
Sun-GLiang:codex/fix-acp-catalog-cache

Conversation

@Sun-GLiang

@Sun-GLiang Sun-GLiang commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary

Opening a new draft after more than 60 seconds or a Host/application restart previously launched another ACP discovery process. Successful neutral-discovery catalogs now persist across drafts and restarts.

  • Keep public catalogs separate from conversation-specific models, selections, and modes. Project A cannot overwrite project B's public choices; real sessions validate their own configuration.
  • Bind snapshots to configuration and executable/helper metadata. Version 2 rejects potentially session-contaminated version 1 snapshots. Every successful neutral probe saves current metadata, including when the model list is unchanged.
  • Scope discovery, loading, invalidation, and retry per agent. Transient refresh failures retain previous choices; cold failures have a 60-second retry cooldown. Explicit refresh and setup/login invalidation discover changes without a periodic timer.
  • Preserve serialized probes, invalidation fencing, writes, and cleanup ownership while removing redundant bookkeeping and persistence caches.
  • Synchronize the Astryx surface inventory for the model picker's Spinner.

Performance benefits and trade-offs

This change primarily improves model-picker readiness and reduces discovery subprocess churn. It does not accelerate model inference, and real conversation initialization still validates its own model and mode.

Optimization Benefit Trade-off / current limitation
Reuse successful catalogs without a 60-second TTL, including across Host restarts Reopening a draft can use memory or a persisted snapshot instead of launching and disposing another ACP process. Warm memory reads do no storage I/O. Catalog freshness becomes event/manual driven. Remote model changes and account switches performed outside Maka can leave stale choices until refresh or invalidation. Executable/helper metadata is checked on snapshot load and successful probe persistence, not on every memory hit; an in-place installation change is not detected immediately by a running cached executor.
Load and refresh catalogs per Agent A fast Agent can become selectable before a slow Agent finishes. Targeted retries and setup invalidation avoid re-probing unrelated Agents. A draft now requests an inventory followed by one request per Agent: roughly N + 1 requests instead of one batched request. This can cost more when every catalog is warm, especially over a remote Host connection. Discovery still shares a two-probe FIFO budget, so cold Agents can still queue; this does not increase probe concurrency.
Preserve the last good catalog on transient refresh failure; cool down cold failures Existing choices remain usable during temporary discovery failures, and repeated draft reads do not continually restart a failing Agent. Cached choices are advisory and may later fail real Session validation. A cold failure can delay an automatic retry for up to 60 seconds; expiry only permits a later read to retry, with no scheduled retry. If a good catalog was retained after a failed refresh, ordinary reads keep returning it with refreshFailed; another explicit refresh or invalidation is needed to re-probe. Explicit retry bypasses the cooldown.
Keep neutral catalogs separate from real Session capabilities Project-specific models, defaults and modes cannot overwrite the public choices shown in other drafts. A real Session may expose different choices and require reselection. Even if a task has already initialized successfully, the first uncached draft still needs its own neutral probe. Draft discovery does not warm or replace the real conversation process.
Persist versioned snapshots bound to configuration and executable/helper metadata Compatible restarts reuse validated snapshots; missing, corrupt or incompatible records fall back to discovery. The first load adds a storage read and metadata checks, and successful probes add persistence work. Derived labels/groups/defaults depend on a manually maintained CATALOG_SNAPSHOT_VERSION; adapter or normalization changes require a version bump. Binary/configuration fingerprints do not automatically detect those code-only changes.
Revalidate ready Session pickers silently Periodic inspection and Session events no longer flash the loading indicator for an already usable picker. The existing 3-second Session polling remains. This improves presentation stability, not polling traffic or inference latency.

A controlled local comparison of ff74d21c8 and 0bcb095a used the repository's real stdio test Agent, ACP SDK and subprocess lifecycle on an Apple M4 Pro with Node 24.18.1 (3 warmup rounds, 30 measured rounds). Median catalog reads after the old TTL elapsed changed from 97.98 ms to 0.006 ms; executor recreation with a valid file-backed snapshot changed from 98.61 ms to 0.52 ms. Both scenarios went from 30 probe process starts to zero across the measured rounds. First uncached discovery and forced refresh remained about 98 ms.

These are discovery-layer measurements, not end-to-end application speedups: the TTL boundary was simulated by advancing the clock, restart was executor recreation rather than a full Host/app restart, and persistence used a JSON file rather than the Host database. Electron IPC/UI and authenticated official Agents were not measured. The remote-Host cost of the additional per-Agent requests remains unquantified.

Verification

  • Synced with main (f7fd46469) and advanced the protocol epoch to 228, preserving main's epoch-227 ShellRun event-wait changes.
  • Passed the protocol epoch guard against current main and all 17 guard regression tests.
  • Passed full build:test, full workspace typecheck, repository-wide lint and formatting checks, and the staged ASF header check.
  • Passed 260 focused catalog, persistence, executor, protocol, goal-protocol, Desktop IPC, selection, and model-picker regression tests.
  • Passed Astryx surface inventory and Windows test inventory checks (including 3 inventory tests).
  • The earlier Storybook assertion fixes passed CI on 0bcb095a; the synchronized head triggers a fresh CI run.
  • Full workspace suites and official-binary acceptance were not rerun locally. An earlier full ACP attempt stalled at a real stdio checkpoint test and was stopped. Historical official Antigravity inference returned a location-eligibility HTTP 403, so full question/answer execution remains unverified.

Compatibility

Runtime Host compatibility epoch increases from 227 to 228 for executor inventory/filter queries; update Desktop and Host together. Missing, corrupt, or incompatible snapshots fall back to neutral discovery.

AI use

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

OpenAI Codex implemented the changes, regression tests, documentation, and local verification. Follow-up commits include a Generated-by: Codex trailer.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and affected tests pass within the verification scope above

Does this PR entail a change in behavior?

  • Yes — described above
  • No

@github-actions github-actions Bot added the effort/XL Under 2500 readable lines label Oct 20, 2026
@Sun-GLiang
Sun-GLiang force-pushed the codex/fix-acp-catalog-cache branch from 6754a58 to e16eb1c Compare October 21, 2026 03:05
@Sun-GLiang
Sun-GLiang marked this pull request as ready for review October 21, 2026 05:58

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

I reviewed head e16eb1c9c536088aa896e8ca38d97b3b103f5525. This change keeps ACP model discovery in a neutral session, persists the resulting catalog with executable/configuration fingerprints, and replaces repeated discovery with explicit per-agent refresh and process-free executor inventory.

P2 — Advance the compatibility epoch beyond current main. packages/runtime-host/src/protocol/index.ts:111 still uses epoch 226, which current main (85e942724c02e7149c4a56f44a0ad2f1350563be) already uses. The new executorId and listOnly catalog-query fields are observable protocol changes: the actual main decoder accepts the legacy query but rejects either new field, while this head accepts both. Both peers would advertise epoch 226. The protocol guard against current main also fails. Rebase, resolve the conflicts, and choose the next unused epoch (currently 227 or later); these strict input changes cannot be justified as wire-invisible.

The catalog behavior passed an independent probe using actual ACP stdio subprocesses and separate Node processes sharing a disk-backed cache. Two project sessions advertised different private models and sent later configuration updates; neither contaminated the public neutral catalog. An aged successful cache entry and a newly constructed executor reused the catalog without another agent process. Explicit refresh observed a changed upstream model, persisted it for the next process, and invalidation forced discovery again. Substituting the previous implementation caused the aged-cache process-count assertion to fail, as expected. This probe uses the real executor/agent transport with a file-backed storage seam, rather than the complete Host storage service.

Validation passed: a fresh dependency install and normal root build:test; 249 focused tests; five additional real-process discovery/cleanup tests; Desktop, ACP and Antigravity type checks; the normal renderer build; 168 architecture tests; surface inventory, license headers, locale hygiene, Windows test inventory and formatting checks. I also inspected persistence revision fencing, retained-session cleanup, Host query routing, and picker loading/retry state. I found no additional confirmed P0–P2 runtime defect in those checks.

Merge status: GitHub reports successful checks for this head, but the local protocol guard fails against current main. A trial merge also conflicts in packages/runtime-host/src/protocol/index.ts and apps/desktop/src/main/__tests__/runtime-host-session-catalog-ipc-main.test.ts; I have not built a resolved merge. I did not validate native Electron interaction, authenticated external ACP agents, or external account/model changes without an explicit refresh. This is a feature change requiring maintainer judgment after these integration issues are resolved.

// interoperability. Mismatches are rejected before domain commands are admitted.
// Re-check against main before merging; sibling PRs may use the same next epoch.
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 225 as const;
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 226 as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Allocate a fresh compatibility epoch after rebasing onto main

Current main already advertises epoch 226. This PR adds executorId and listOnly to plugin.executor.query, but main's strict decoder rejects either field even though the compatibility epoch matches. I verified that the legacy query succeeds with both decoders and that only the new head accepts the two new fields. The protocol guard against current main fails as well. Rebase and use the next unused epoch (currently at least 227) so incompatible peers are rejected during admission.

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Second, independent review of e16eb1c9, written before reading the review above. It agrees on the epoch P2 and the merge conflicts. Severity note: the stale-catalog-after-Maka-update finding below was raised only in this review and has not been reproduced. The stored record already carries a manual version field (record.version !== 2 in acp-catalog-store.ts) that can be bumped when the catalog derivation changes, and users can refresh manually, so treat it as P3 (a maintenance hazard worth addressing, e.g. by bumping version whenever adapter grouping/labels change or by folding a plugin build id into the fingerprint), not a merge blocker.


First review of head e16eb1c9c536088aa896e8ca38d97b3b103f5525 (merge-base b0ed5707, 28 files, +2235/-133). I read every non-test source change: the ACP catalog store and catalog lifecycle, the executor probe/dispose changes, the per-agent executorId/listOnly query from the protocol through the Host, Desktop IPC/preload and renderer hook, and the model picker. There are no prior findings to follow up. I did not run tests locally because memory is limited on the review machine. CI on this exact head is green (test, label).

What looks right:

  • Scoping. The snapshot key is acp-catalog/<adapter.id>/<sha256(entryId)>/snapshot inside the plugin's namespaced storage, so agents and entries cannot see each other's catalogs. invalidateCatalog(executorId) now only invalidates the named agent (Antigravity on setup success). Per-agent renderer loads use [key, attempt, id] operation keys and are guarded by the attempt revision, so a stale response is discarded and cannot land in another draft.
  • Invalidation and concurrency. The binding covers executor id, config, launch spec, platform/arch, and realpath/size/mtime/ctime of the executable and helper. That covers a changed executable path (reconcile installs a new entry, so the fingerprint mismatches) and the managed installer's versioned directory (setup onSucceeded invalidates as well). Hydration is fenced by revision. Writes are serialized, so an old save cannot bring back a deleted snapshot. Dropping the blanket invalidation in applyRuntimePolicyMutationEffects looks safe, because the only policy input to the agent is executable, which reconcile turns into a new entry.
  • Failure and offline behavior. authentication_required clears both the memory and the persisted catalog. Transient failures keep the last good list, mark it refreshFailed and apply a 60-second cooldown. If the agent is uninstalled, the realpath/X_OK check makes load() fail, so it falls back to fresh discovery. Version-1 snapshots are rejected.

Findings:

P2: Protocol epoch collision. The PR sets RUNTIME_HOST_COMPATIBILITY_EPOCH to 226, but main is already at 226 (#6176, assistant.allocate-workspace). After rebase the PR would ship a wire change (executorId/listOnly on the executor catalog query) under the same epoch as main, and the protocol guard would fail. Please bump to the next unused value (227 unless another PR lands first). Also add the // 227: ... history comment, which the PR currently omits.

P3 (downgraded, see note above): The persisted snapshot is not tied to the adapter/plugin code that produced it. createAcpCatalogStore stores the fully derived ExecutorCatalogEntry, which includes displayName and the output of adapter.describeModels (Antigravity modelGroups/providerType). The binding (acp-catalog-store.ts:43) only covers config, launch and binary metadata. The in-memory TTL is gone and hydration survives restarts, so after a Maka update that changes describeAntigravityModels, the display name, or catalog normalization, users keep seeing the old derived catalog indefinitely. It only clears if they press Refresh or the agent binary changes. Suggested fixes: persist the raw model/mode option lists and re-derive with #catalogEntry on load, or include a plugin/adapter build identity in the binding (for example, the package content digest or an explicit derivation version).

P3: The snapshot is orphaned when the agent is removed. Uninstalling the Antigravity package (clearing its executable) does not delete acp-catalog/.../snapshot, just like the existing continuity records. The binding makes it harmless on reuse, but it is never cleaned up.

P3: Neither the catalog nor the snapshot binding notices an external account change (sign out or account switch done outside Maka). The persisted list stays until the first real Session reports authentication failure or the user refreshes. This is acceptable as a documented trade-off, but worth a note in the README section the PR edits.

P3: In a draft, any change to executorId or key re-runs the effect. That re-fetches the roster and marks every agent as loadingIds, so each picker briefly shows "Loading models..." even when every catalog is already warm. This is cosmetic only.

Mergeability: the branch does not merge cleanly into current main (d2818638). git merge-tree reports content conflicts in packages/runtime-host/src/protocol/index.ts (the epoch line, see above) and in apps/desktop/src/main/__tests__/runtime-host-session-catalog-ipc-main.test.ts. The test conflict is trivial: both sides append tests, so keep both. GitHub reports CONFLICTING/DIRTY. A rebase is required.

CI: test and label pass on e16eb1c9. Nothing in CI is caused by this PR.

Verdict: needs a rebase with the epoch moved to 227, and the persisted catalog should be tied to the code that derives it. The rest of the persistence, scoping and failure-handling design looks correct.

// interoperability. Mismatches are rejected before domain commands are admitted.
// Re-check against main before merging; sibling PRs may use the same next epoch.
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 225 as const;
export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 226 as const;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: main is already at epoch 226 (#6176). This wire change needs the next unused epoch (227 unless another PR lands first) plus a // 227: ... history comment. Otherwise the protocol guard fails after rebase.

// Persist only a digest, never configuration values or credentials. Do not
// fingerprint the inherited process environment: it includes per-launch
// values and would make an otherwise unchanged restart miss the cache.
const configuration = JSON.stringify(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: The binding covers config, launch and binary metadata but not the adapter/plugin code. entry stores derived data (displayName and the describeModels output: modelGroups/providerType). With no TTL and hydration across restarts, a Maka update that changes that derivation keeps serving the stale catalog until a manual refresh. Suggest persisting the raw model/mode options and re-deriving on load, or adding a plugin build/derivation version to the binding.

Reuse successful catalogs across drafts and Host restarts, update candidates from real sessions, and isolate invalidation, loading and retry state by agent. Keep stale choices on transient refresh failure and retain probe ownership until cleanup succeeds.

Generated-by: Codex
Merge persisted neutral defaults with early Session observations before saving, including during shutdown. Add ready-agent refresh controls and route inventory errors through inventory retry. Cover hydration races and picker recovery with regression tests, refresh documentation and Windows inventory, and record browser verification.

Generated-by: Codex
Keep ready executor selections visible during background polling and session updates while preserving loading feedback for initial reads and explicit retries. Cover silent polling, model updates, and failure recovery.

Generated-by: Codex
Hide draft refresh controls while a native model is selected and expose a single action for the selected external agent. Update picker coverage and catalog documentation.

Generated-by: Codex
Keep public model catalogs sourced exclusively from neutral discovery so project-specific Session candidates cannot hide models in other projects. Remove the Session-to-catalog write path and reject version 1 snapshots that may contain project-specific candidates.

Cover cold and warm cross-project discovery, Session updates, restart reuse, explicit refresh, and snapshot compatibility. Update documentation to describe the plugin-local boundary.

Generated-by: Codex
Remove redundant probe bookkeeping and persistence caches while retaining invalidation fences and serialized writes. Cover executable updates and shutdown races, and align documentation with neutral-only snapshots and measured ablation results.

Generated-by: Codex
Regenerate the Astryx inventory to include the model picker Spinner and restore the CI inventory check.

Generated-by: Codex
Keep current catalog behavior in the plugin README and architecture document; remove historical screenshots and duplicate experiment records.

Generated-by: Codex
@Sun-GLiang
Sun-GLiang force-pushed the codex/fix-acp-catalog-cache branch from e16eb1c to 1105d8d Compare October 21, 2026 08:16
Centralize the snapshot version and document when adapter presentation and catalog semantics require invalidation. Cover one-time rediscovery and restart reuse across a version change with unchanged config and executable metadata.

Generated-by: Codex
Show only the selected model or model family in the composer trigger. Keep agent names in the dropdown section headings and agent-specific actions.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Incremental review at 6baf1045b2c983cf1520316d64b76a994d5143cf, following e16eb1c9c536088aa896e8ca38d97b3b103f5525. The previous protocol P2 and merge conflicts are resolved. One new P2 remains: the model-label change breaks two required Storybook play assertions.

I compared the author commits across the rebase, checked the protocol and IPC conflict resolutions, and read the two additional commits separately from main's changes. Those commits make the catalog snapshot version's maintenance contract explicit and remove agent prefixes from the selected model label. The catalog still uses version 2; the new constant documents when maintainers must bump it rather than automatically detecting code changes.

P2 — update the Storybook assertions for the new model label. packages/ui/src/composer-model-picker.tsx:246 deliberately stops displaying the agent prefix, but packages/ui/stories/model-picker.stories.tsx:170 and :189 still expect it. I rebuilt Storybook and ran both unmodified stories in Chromium. product-model-picker--codex-and-acp fails expecting Codex · GPT-6 Astra while the actual trigger reads GPT-6 Astra; product-model-picker--unselected-unavailable-agent-narrow fails expecting Antigravity · Gemini 3.8 Flash while the actual trigger reads Gemini 3.8 Flash. An adjacent unchanged executor story passes. These are the same two failures in the exact-head CI run. The story file is byte-identical to current main, whose CI passed. Align both assertions with the intended label while retaining the agent-group and notice checks.

The protocol now advances from main's 226 to 227, and the epoch guard passes. Actual decoder checks confirm that this head accepts the new catalog fields that main rejects; peers now advertise different epochs. A static merge with main 9826db1b9dad32897be105b2c1c621becedb27ce is clean. I did not separately build the merged tree.

P3 — coordinate epoch 227 before merging. At this check, open PRs #6091 (efbe29e6), #6192 (07c7a8bb) and this PR all declare 227. The first merged protocol change consumes that epoch. The remaining PRs must choose the next unused epoch and be reviewed again; they must not all land unchanged under 227.

Validation otherwise passed: 278 focused tests, the normal clean root test build, all five Desktop TypeScript configurations, Storybook and production renderer builds, 168 architecture-checker tests plus the strict application check, Biome on 25 changed code files, and git diff --check.

Additional real-subprocess checks with disk-backed storage verified neutral catalog reuse across restarts, explicit refresh and invalidation, and isolation of project-specific models from the public snapshot. For the version contract, an actual earlier implementation wrote a v1 snapshot. The current implementation re-probed once and wrote v2 with the same executable/configuration binding; another restart reused it. Removing only the version check reproduced stale catalog reuse. This verifies the version mechanism, not automatic detection of every adapter code change.

The independent persistence probe uses a file-backed storage seam rather than a full Host database. Native Electron, Windows/macOS, authenticated external agents and the complete repository suite were not exercised. The Storybook CI failure above remains unresolved.

const selectedModel = executor?.selection?.configuration.model ?? selected?.currentModel;
const group = selected && executorModelGroup(selected, selectedModel);
const modelLabel = contributed ? contributedModel?.label ?? props.executorTarget?.model ?? copy.default : executor?.selection
const label = contributed ? contributedModel?.label ?? props.executorTarget?.model ?? copy.default : executor?.selection

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: removing the agent prefix here breaks the required Storybook smoke run because two existing play assertions still require that prefix. The unmodified product-model-picker--codex-and-acp story expects Codex · GPT-6 Astra but receives GPT-6 Astra; product-model-picker--unselected-unavailable-agent-narrow expects Antigravity · Gemini 3.8 Flash but receives Gemini 3.8 Flash. Both fail in a fresh local Chromium build and CI run 39569703472. Update packages/ui/stories/model-picker.stories.tsx:170,189 to the intentional label behavior while preserving its agent-group/notice coverage.

Match selected model labels exactly without agent prefixes while retaining agent grouping and unavailable-agent notice coverage.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Incremental review of 6baf1045..0bcb095a. The ten existing commits are unchanged (git range-diff), and one test-only commit was added (model-picker.stories.tsx, 2 lines). Our P2 is fixed; no P0–P2 remains.

Both Storybook play assertions that still expected the old agent-prefixed trigger labels now match the model-only label exactly: /^GPT-6 Astra$/ in CodexAndAcp, and /^Gemini 3\.8 Flash$/ in UnselectedUnavailableAgentNarrow. Anchoring the regex also proves the prefix is gone, so this is a stronger check than the old substring. Agent grouping and the unavailable-agent notice assertions are unchanged. CI test now passes on this head, and the PR is mergeable.

The P3 from our last round still stands. This PR, #6091 and #6192 all set RUNTIME_HOST_COMPATIBILITY_EPOCH = 227, while main is at 226. Whichever merges first keeps 227; the others need the next free epoch and a short re-review.

Merge upstream/main and preserve its ShellRun event-wait protocol changes. Advance the ACP catalog query epoch to 228 so current main's epoch-227 peers cannot accept incompatible executor queries.

Generated-by: Codex

@Astro-Han Astro-Han left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Incremental review at 795b1782c4f76c677c967a86a6e4c31bef1247c6, following 0bcb095adf404e0594413e7109205e429160dc78. No new confirmed P0–P2. The earlier protocol and Storybook-label P2 findings are resolved. There is one P3 about the new story's alignment assertion at narrow viewports, reported inline.

I separated the merge of main f7fd4646 from the two follow-up commits. Of the previous 30 author-modified paths, 25 blobs are unchanged; four other files can be reconstructed exactly by applying main's changes to the previous feature source. The remaining difference is the protocol epoch/history. The original ACP catalog implementation is unchanged by this merge. The follow-ups move refresh failures into a toast and publish the crash-test readiness marker atomically.

The toast at packages/ui/src/composer-model-picker.tsx:171 and :413 leaves the selected models and refresh action available. In Chromium, the unmodified new AppShell story passes at 1280px in English, Simplified Chinese and Traditional Chinese. At 720px its final same-row assertion fails, but independent before/after measurements show the controls were already wrapped: their center separation remains 32px and the composer height remains 162px. The toast stays outside the composer, and another refresh still works. The two previously broken model-label stories and an adjacent executor story also pass unchanged. These checks exercise the actual components with the story's simulated refresh response.

The readiness-marker fix at packages/runtime-host/src/__tests__/goal-root-authority.test.ts:3300 addresses the previous head's CI failure: the parent could read ready.json after creation but before its JSON contents were written. Both original SIGKILL recovery tests pass with a 250ms delay deliberately inserted between file creation and writing its contents. Reverting only the atomic-publication change reproduces Unexpected end of JSON input in both tests under the same delay. The recovered commands still start once. A first run alongside builds hit the fixture-readiness timeout; the unchanged tests passed when run separately.

The epoch now advances from current main's 227 to 228, preserving the ShellRun event-wait history. The guard passes; actual decoder checks confirm that this head accepts the new catalog query fields while main rejects them, and both accept the legacy query. The previous 227 collision is resolved for this head. Recheck the next unused epoch if another protocol change lands first.

Validation passed: a fresh dependency install and normal clean root build:test; 247 focused ACP/protocol/runtime/Desktop tests, 798 UI tests, the two crash-recovery tests above, five Desktop TypeScript configurations plus Host type checking, Storybook build, surface inventory, locale hygiene, Windows inventory, E2E budget and formatting checks. A separate real-ACP-subprocess probe with file-backed storage again verified neutral catalog reuse across process restarts, explicit refresh/invalidation, and isolation from project-specific session models.

At publication, this head is mergeable and its fresh CI run is still running. A static merge with main 2b0e4a25fa01e6ee6d60bc5c5d6e4f682caecfbf is clean; I did not build that merged tree. I did not rerun native Electron, Windows/macOS, authenticated external agents, the full repository suite or the PR's performance benchmarks. The persistence probe uses a file-backed storage seam rather than the complete Host database.

await expect(composer.getBoundingClientRect().height).toBe(before.height);
const buttonBox = refresh.getBoundingClientRect();
const modelBox = model.getBoundingClientRect();
await expect(Math.abs((buttonBox.y + buttonBox.height / 2) - (modelBox.y + modelBox.height / 2))).toBeLessThanOrEqual(2);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3 — scope this assertion to a single-row layout, or compare against the pre-refresh geometry. At a 720px viewport, the new story fails here in all three shipped languages: the refresh and model buttons are already on different rows before the click, with 32px between their vertical centers, and they retain exactly that separation afterward. The composer also retains its 162px height, the toast is outside it, and retry remains available. The default 1280px story passes. This is an extra-viewport test limitation, not a demonstrated toast layout regression or the cause of the previous CI failure.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

effort/XL Under 2500 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants