Repository navigation
refactor(protocol): each area owns its published runtime capability list - #30641
Conversation
RUNTIME_CAPABILITIES now spreads one array per area instead of a flat hand-appended list, and runtime-capability-constants.ts is split into the areas that own each constant. The published set is unchanged and pinned by an exact sorted test.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Priority: ⬇️ Low Merge Risk: 🔵 Low · up to
|
There was a problem hiding this comment.
🧹 Nitpick comments: 1
src/shared/protocol-version-published-capabilities.test.ts: 1
89-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the existing disk-write precondition to the published baseline.
files.write.disk-precondition.v1was already in the PR base'sRUNTIME_CAPABILITIES, so it is part of the frozen published set. Without it, removing the capability later can pass the preservation test.Proposed change
'files.resolve-terminal-path.cross-workspace.v1', + 'files.write.disk-precondition.v1', 'folder-workspace.path-status.v1',
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
474c9cad-b0c1-4b29-ae15-734d6a352052
📒 Files selected for processing: 9
src/main/agent-hooks/server/server-status-application.tssrc/main/agent-hooks/server/server-status-update.tssrc/main/daemon/pty-subprocess-spawn-file-foreground.test.tssrc/main/daemon/terminal-host-process-inspection.test.tssrc/main/providers/local-pty-child-process-verdict.test.tssrc/main/providers/local-pty-provider-foreground-process.test.tssrc/shared/files-runtime-capabilities.tssrc/shared/protocol-version-published-capabilities.test.tssrc/shared/protocol-version.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 5 remain after this review.
|
Current-head review refresh — 2026-10-20. Main's overnight capability additions conflicted with the removed central registry. Merged main Independent narrow review found no lost main changes or new findings: main and final publish the same 179 unique static names, preserve all 141 main exports (including original 138) apart from intended aggregate order, retain native/client lists in both sidebar modes, and keep the original 170-name baseline byte-identical with additions allowed. New item-detail and queued-message-image-edit constants are in the agent-session owner. 35 tests across three exact named files passed. The current full PR workflow passed on this exact head, including aggregate gate; every auxiliary workflow passed too. The independent reviewer confirmed a mechanical merge with no shipped behavior change beyond incorporation of main. Under the coordinator's explicit instruction, earlier real M4Air QA remains applicable and was not rerun. The screenshots and test counts in the earlier review below are historical: they show Earlier completed full review and real QA screenshotsEvery new server capability previously required editing the same central list, so unrelated feature PRs repeatedly conflicted. The list is now assembled from its owning areas. Paired clients retain the same support; Settings intentionally previews three different capability names in the new area order. Full review completed on
Deferred observation: completed Grok tab titles differed (main's prompt summary versus final's Baseline default-tab behavior: with setup Limits: live QA covered macOS desktop/headless hosts and the real Grok terminal route. Other agents, structured chat, Windows/Linux/SSH UI, mobile devices, every conditional combination and native focus were not exercised live here; source parity and CI cover their unchanged contracts. The initial frame before setup completed was not captured for the nonempty default-tab fixture. No runtime performance measurement is claimed. For Brennan's review The intended visible difference is Settings' first-three preview: main shows
Ordinary flows compared on main and finalThe three code judgment points are the one-spread-per-area aggregate, the additions-allowed frozen baseline, and the first-three Settings preview. |






















ELI5
Every feature that lets a paired Orca server tell clients "I support X" had to add a line to the same long list in one shared file. Parallel PRs kept colliding there. Now each area (agent sessions, terminal, browser, worktrees, ...) keeps its own list in its own file, and the shared file just combines them. Paired clients retain the same feature support. In Settings, the host details preview shows three capability names in the new area order; that display change is intended.
What Changed
Problem (developer-facing):
src/shared/protocol-version.tsheldRUNTIME_CAPABILITIES, one flat hand-appended array mixing about 20 areas plus 9 inline string literals, andsrc/shared/runtime-capability-constants.tsheld 79 loose constants from about 15 areas. Every new wire capability appended to both, at the same spot, so it was the most-conflicted file in recent merges (33 merged, 15 closed and 9 open PRs touched it). Earlier work (#25839, and the files that say "Split out of protocol-version.ts") moved some areas out, but left the central list hand-kept.After: each area module exports
<AREA>_RUNTIME_CAPABILITIESnext to its constants, andRUNTIME_CAPABILITIESis 20 spread lines, one per area. A new capability in an existing area edits only that area's file; the central list changes only when a new area is born.src/shared/:agent-session-,browser-,terminal-,worktree-,task-source-(Jira, Linear, task context),source-control-(GitHub/GitLab ready-for-review, commit-message model),files-,host-(status, environments, shared control, updates, managed runtimes, SSH targets),mobile-,preflight-,workspace-ports-,ai-vault-runtime-capabilities.ts.session-tabs-runtime-capabilities.ts(close-intent, inventory, and the client-only tab/proof deltas),terminal-quick-command-capabilities.ts(now owns its constant).runtime-capability-constants.tsis deleted. Its direct importers are repointed, including the two CLI imports added on merged main.protocol-version.tsstill exports every name it exported before (same values), so the ~180 files that import from it are untouched.queued-messagesflag stay out of their area's list, exactly as they stayed out of the central one.NATIVE_REMOTE_RUNTIME_CLIENT_CAPABILITIES, mobile/web/electron/desktop-renderer lists) are not changed.Wire: no change from the merged main. The published capability set is identical: 179 strings on merged main
2fc2f92a386471ccfbe8277cea3df71d2f6ec0cfand current heade3e555118fb75ba3c7471afe422948f83753418d. Main'sfiles.write.disk-precondition.v1remains in the files area, and its newproject-host-setup.host-target.v1is preserved in the worktree area; the original test baseline remains 170 names. Independently evaluatedRUNTIME_CAPABILITIES, the native client list and everyprotocol-versionexport from each Git revision: sorted published sets equal, no duplicate names, native list identical including order (9 entries), all 141 exports from merged main retained (including the original 138) and every previous value except the intentionally reordered host aggregate unchanged; 12 additional exports are the area arrays. No protocol version change.Order — intended: the array's order changes (grouped by area). Feature gating checks membership (
includes/filter). The Settings runtime host detail previews the first three capability names plus a count (runtime-environment-host-details.ts:96), so that preview shows three different names in the new order; the count is unchanged. This Settings preview-order change is intended. Raw status JSON from the CLI, local hosts and SSH hosts also exposes the grouped capability-array order; that order change is intended. Membership and counts match main for identical host conditions; runtime filters and conditional additions make the actual count host-specific.Review path: full. This refactor exceeds the light-path size limits and touches published capability negotiation across independently updated desktop, server, phone and SSH peers. It requires the full review, exact-head PR workflow and real Electron QA.
Why
The common pattern is that the code owning a feature declares its own capability and the aggregate is collected, not hand-appended. TypeScript has no link-time registry, so the aggregate is one static spread per area. A self-registering design (each module registers at import time) was rejected: the published set would then depend on which modules each bundle (main, renderer, mobile, CLI) happened to import, which is a silent wire change, and the cross-version tests read
RUNTIME_CAPABILITIESfrom released builds as a static array. Only moving the loose constants (and keeping the hand-appended central list) was rejected because the append point is what conflicts.The new test
protocol-version-published-capabilities.test.tskeeps a frozen baseline of the original 170 published names and checks that every baseline name remains published. Adding a capability requires no baseline edit, so feature additions do not recreate a central test edit hotspot. Deliberately retiring support requires an explicit baseline edit. The one-time before/after equality checks above still verify that this refactor preserves the published set, native client list order and all previous export values.Linked Issue
N/A (maintainer refactor; tech-debt audit item G, PR 1 of the capability-registry plan).
Final main integration
On 2026-10-20, merged main
2fc2f92a386471ccfbe8277cea3df71d2f6ec0cfto resolve overnight conflicts, producinge3e555118fb75ba3c7471afe422948f83753418d. Main's new item-detail and queued-message-image-edit capabilities belong to the agent-session module; other additions flow through existing area arrays. Independent checks confirm 179 identical unique static names, all 141 main exports retained, unchanged client contracts in both sidebar modes, and the unchanged original 170-name baseline. All 35 targeted tests passed. The exact-head full PR workflow and its aggregate gate passed, as did every auxiliary workflow.An independent reviewer checked every resolution difference and found the integration mechanical, with no new findings or changed shipped behavior beyond main. Under the coordinator's explicit ruling, the real QA below remains applicable; no fresh app run was performed after this merge. The screenshots depict the earlier comparison and its historical 172-static/179-runtime counts, not today's 179-static total. Hook changes to an incoming tracked document only adjust Markdown table padding.
Visual Proof
Earlier real comparative Electron QA completed on M4Air through qa-gate ticket 533, using built main
41fdd7e6d8301ff1faa956e8ecc6a8ef86cff10fand final7c32100f8f60b66a7216f5f85a451492eefc1fe6, identical fixtures and a real paired headless host. Both roles published 179 names, with equal membership and exclusions (172 static names plus the same host-specific additions).Intended Settings difference: main previews
ai-vault.chat-fork-source.v1, jira.transition-fields.v1, jira.agent-access.v1 +176; final previewsruntime.status.compat.v1, runtime.environments.v1, remote-runtime.shared-control.v1 +176. The grouped order is also visible in raw status JSON. Counts and support remain equal for the same host facts.Setup executed once on each pin; ordinary terminal, quick command, real Grok idle/working/Done, and nonempty configured default tabs all completed. The review comment includes paired screenshots for those surrounding flows. With Grok selected, the first configured Check tab adopts that startup agent on both pins, retaining Check/Shell/Setup order; this is established behavior, not a new change.
Deferred presentation observation: main's completed Grok tab showed a prompt summary while final showed
grok. The traced code is identical; main's original raw terminal title output was unavailable, so causality remains unresolved. The launcher explicitly accepted deferral with no PR regression established. Live QA covered macOS desktop/headless and real Grok terminal sessions; other agents, structured chat, Windows/Linux/SSH UI, mobile devices and native focus were not exercised live. All recorded rigs/drivers stopped and the ticket was released.Testing
Automated tests added/updated, or explained why not below
New:
src/shared/protocol-version-published-capabilities.test.ts(frozen 170-name baseline remains a subset of published capabilities; new additions need no baseline edit).Review fix: ran that exact named file through the local test gate with Node and
config/vitest.config.ts; 1 file and 1 test collected, 1 passed. The 170-name baseline is unchanged.Ran locally: that test plus
session-tabs-tab-delta,agent-status-run-capability,agent-chat-permission-host-support,terminal-image-viewer-env(51 passing). CI covers typecheck, lint and the full suite, including the cross-version wire suites.One-time before/after dump of the published set and barrel exports, as above.
Earlier main integration: the capability baseline, shared disk-precondition contract and files RPC forwarding tests collected 16 tests across 3 exact named files; all passed through the local test gate. Final conflict integration: the capability baseline, CLI project setup and shared project-host target tests collected 33 tests across 3 exact named files; all passed through the local test gate. The original 170-name baseline is byte-identical; the independent final export proof is summarized above.
Review
Integration notes for open PRs touching these lists (none edited here):
structured-agent-session-surface-capabilities.tsand client lists: not touched here; its new capability flows in through the agent-session list without editing the frozen baseline test.orchestration-runtime-capabilities.ts, which this PR does not touch.Differences from the common pattern: the aggregate is still one line per area rather than collected automatically (intended: TypeScript has no link-time registry, and module side-effect registration would make the published set import-order dependent).
Agent skill upstream boundary
Notes
Security, cross-platform, SSH, mobile, backwards compatibility and performance: capability gating is unchanged; the same strings are published to the same peers. The intended visible change is the Settings three-name preview order described above. Mobile imports one constant from its new area file.
Checklist