Repository navigation
Make v1 the default sub-agent surface and ask before base or v2 #4462
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
252fb88
67667a5
d0d808c
f89c00e
bb69d28
d080d7a
3d1c078
57fad86
01ef3e0
550bf4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # Sub-agent surface: v1 as the install default, with an approval guard on base and v2 | ||
|
|
||
| ## Why this unit exists | ||
|
|
||
| The v2 collaboration surface cannot deliver a task from a ChatGPT-native parent to a | ||
| routed child. Codex mints that child task as backend `encrypted_content`, and only the | ||
| ChatGPT backend holds the key, so an external provider receives ciphertext it cannot | ||
| read. OpenCodex already fails closed on it (`unreadable_encrypted_agent_task`) and | ||
| documents the limitation as issue #92, but the product still ships `base` as the install | ||
| default, and `base` pins Sol and Terra to v2. A new user who delegates from Sol to Grok | ||
| therefore meets the failure on their first attempt, with no warning that the mode they | ||
| never chose is the reason. | ||
|
|
||
| This unit makes v1 the default, and makes base and v2 a choice the operator confirms | ||
| after reading what it costs. | ||
|
|
||
| ## Locked decisions | ||
|
|
||
| **D1 — an absent `multiAgentMode` key still means base.** Roughly fifteen call sites read | ||
| `config.multiAgentMode ?? "default"`, and the management API deletes the key when the | ||
| operator selects base. Re-pointing that fallback to `"v1"` would make the key's absence | ||
| ambiguous: it could no longer distinguish "never configured" from "explicitly base". So | ||
| what changes is the written default — `getDefaultConfig()` now emits an explicit | ||
| `multiAgentMode: "v1"`, exactly the shape `subagentModels` already uses. The resolver is | ||
| untouched, and the merge that could push a default into an existing document runs only | ||
| on the schema-repair path. | ||
|
|
||
| **D2 — an existing operator is asked, not flipped.** Rewriting a stored | ||
| `multiAgentMode: "v2"` during an update would change a setting its owner deliberately | ||
| made, on a machine they are not looking at. Instead the runtime computes an advisory, | ||
| the GUI raises it once, and only the operator's answer writes. That is also what makes | ||
| the two buttons meaningful: 계속하기 keeps the current mode, v1으로 바꾸기 applies v1. | ||
|
|
||
| **D3 — the advisory is a version counter, not a boolean.** `multiAgentSurfaceAdvisoryVersion` | ||
| mirrors `subagentModelsVersion`. When the encryption limitation is fixed upstream, or when | ||
| a later release needs to say something different about the same setting, bumping the | ||
| constant re-raises the notice for everyone without adding another key. | ||
|
|
||
| **D4 — one dialog component, two triggers.** The approval dialog shown when the operator | ||
| selects base or v2 and the post-update advisory differ only in their heading and body | ||
| text. They offer the same two actions and link to the same guide, so they are one | ||
| component with a `reason` discriminator rather than two that drift apart. | ||
|
|
||
| **D5 — the guide is a real published page, not an anchor.** The dialog links to | ||
| `https://opencodex.me/guides/subagent-v1-default/`, a new docs-site page whose job is to | ||
| make the failure legible: an inline SVG showing the same delegation succeeding under v1 | ||
| and dying at the provider boundary under v2. | ||
|
|
||
| ## Work phases | ||
|
|
||
| | Phase | Outcome | | ||
| | --- | --- | | ||
| | wp1 | This roadmap, with the decisions above locked before any code moves. | | ||
| | wp2 | Runtime: `getDefaultConfig()` emits v1, the advisory state and its version constant exist, and `GET/PUT /api/v2` carry them. | | ||
| | wp3 | GUI: the approval dialog, both triggers wired, and translations for all nine locales. | | ||
| | wp4 | The docs-site guide and its SVG, plus the link from the dialog and the existing surface guide. | | ||
| | wp5 | Focused tests, typecheck, PR to `dev` with a UI screenshot, exact-head CI, merge. | | ||
|
|
||
| ## Out of scope | ||
|
|
||
| Fixing the encryption limitation itself. `agentTaskRecovery` and `plaintextV2AgentMessages` | ||
| remain experimental and default-off; this unit changes which surface an operator lands | ||
| on by default, not what v2 can carry. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| # wp2 — Runtime: v1 default and the advisory state | ||
|
|
||
| ## What changes | ||
|
|
||
| `src/config/multi-agent-surface.ts` (new) owns three things: | ||
|
|
||
| - `MULTI_AGENT_SURFACE_ADVISORY_VERSION = 1`. | ||
| - `resolveMultiAgentMode(config)`, the single reader that turns a stored value into | ||
| `"v1" | "default" | "v2"`. It keeps the existing rule that an absent key means | ||
| `"default"`; it exists so the advisory and the catalog agree on one answer. | ||
| - `multiAgentSurfaceAdvisoryRequired(config)`, true when the resolved mode is not `"v1"` | ||
| and the stored `multiAgentSurfaceAdvisoryVersion` is below the constant. | ||
|
|
||
| `getDefaultConfig()` gains `multiAgentMode: "v1"` and | ||
| `multiAgentSurfaceAdvisoryVersion: MULTI_AGENT_SURFACE_ADVISORY_VERSION`. A fresh install | ||
| has nothing to advise about, so it starts acknowledged. | ||
|
|
||
| `src/types/config.ts` gains the optional `multiAgentSurfaceAdvisoryVersion?: number`, and | ||
| `configSchema` in `src/config.ts` accepts it as a positive integer with `.catch(undefined)` | ||
| so a hand-edited value cannot fail the parse. | ||
|
|
||
| ## Management API | ||
|
|
||
| `GET /api/v2` adds one response-only object: | ||
|
|
||
| ```json | ||
| { | ||
| "multiAgentSurfaceAdvisory": { | ||
| "required": true, | ||
| "mode": "v2", | ||
| "recommended": "v1", | ||
| "version": 1, | ||
| "docsUrl": "https://opencodex.me/guides/subagent-v1-default/" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| `PUT /api/v2` accepts `multiAgentSurfaceAdvisoryAcknowledged: true`, which stores the | ||
| current constant and is idempotent. It composes with an existing `multiAgentMode` write in | ||
| the same body, because `v1으로 바꾸기` sends both in one request and must not leave the | ||
| advisory raised if the mode write succeeded. | ||
|
|
||
| It stays a boolean in, number out. A client that could post an arbitrary version could | ||
| silence a future advisory it has never seen. | ||
|
|
||
| ## What deliberately does not change | ||
|
|
||
| The catalog stamping in `src/codex/catalog/sync.ts`, the `keepNativeChatGptOnV1` hybrid, | ||
| and every `?? "default"` fallback. This phase moves the written default and adds one | ||
| piece of state; it does not renegotiate what the three modes mean. | ||
|
|
||
| ## Tests | ||
|
|
||
| New tests go into existing files. `tests/test-layout.test.ts` requires every new test file to | ||
| be registered in both `scripts/test-layout/layout.json` and | ||
| `tests/fixtures/test-layout-expected.json`, and a file named after this route does not resolve | ||
| through the regex seeds. Extending files that already own these subsystems is the smaller | ||
| change and keeps the guard quiet. | ||
|
|
||
| `tests/server/config.test.ts` — a fresh config resolves to v1 and starts acknowledged. | ||
| `tests/codex-integration/codex-v2-gate.test.ts`, in its existing management-API parity | ||
| describe block — the advisory is required for a v2 config and for a config with no key, not | ||
| required after acknowledgement, and a combined mode plus acknowledgement write persists both. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| # wp3 — GUI: the approval dialog and its two triggers | ||
|
|
||
| ## Component | ||
|
|
||
| `gui/src/components/SubagentSurfaceWarningModal.tsx`, modelled on | ||
| `OAuthTosWarningModal.tsx`: a native `<dialog>` opened with `showModal()` so focus | ||
| trapping, the backdrop and Escape come from the platform. | ||
|
|
||
| Props: | ||
|
|
||
| | Prop | Meaning | | ||
| | --- | --- | | ||
| | `reason` | `"selection"` when the operator just clicked base or v2, `"advisory"` for the one-time post-update notice. | | ||
| | `mode` | The mode being selected, or the mode currently stored, depending on `reason`. | | ||
| | `docsUrl` | The published guide. | | ||
| | `onContinue` | Keep or apply the non-v1 mode. | | ||
| | `onChooseV1` | Apply v1. | | ||
|
|
||
| Both actions are real buttons in `.modal-actions`. `v1으로 바꾸기` is the primary, because | ||
| it is the recommended answer; `계속하기` is the ghost. Escape and the backdrop resolve to | ||
| the same outcome as `계속하기` for the advisory (the stored mode is kept) and to a no-op | ||
| for a selection (the click is abandoned and the segmented control does not move). | ||
|
|
||
| ## Triggers | ||
|
|
||
| **Selection.** `Models.tsx` `setMultiAgentMode` and `use-dashboard-data.ts` `switchMaMode` | ||
| stop writing directly for `"default"` and `"v2"`. They stage the pending mode, render the | ||
| dialog, and write only from `onContinue`. Selecting `"v1"` stays immediate: confirming a | ||
| move toward the safe default would be noise. | ||
|
Comment on lines
+26
to
+29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Document the third mode-switch entry point. This section names only 🤖 Prompt for AI Agents |
||
|
|
||
| **Advisory.** The dashboard reads `multiAgentSurfaceAdvisory.required` from the `/api/v2` | ||
| poll it already runs and opens the dialog once per load while it is required. `계속하기` | ||
| sends the acknowledgement alone; `v1으로 바꾸기` sends the mode and the acknowledgement | ||
| together. | ||
|
|
||
| ## Translations | ||
|
|
||
| Nine locales live in `gui/src/i18n/`: en, ko, ja, zh, zh-TW, fr, de, ru, tr. Keys: | ||
|
|
||
| - `subagentSurface.selectionTitle`, `subagentSurface.selectionBody` | ||
| - `subagentSurface.advisoryTitle`, `subagentSurface.advisoryBody` | ||
| - `subagentSurface.continue`, `subagentSurface.switchToV1`, `subagentSurface.learnMore` | ||
|
|
||
| Korean carries the literal `계속하기` and `v1으로 바꾸기` the user asked for; the other | ||
| locales carry their natural equivalents. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| # wp4 — The guide and its visual explanation | ||
|
|
||
| `docs-site/src/content/docs/guides/subagent-v1-default.md` answers one question: why does | ||
| a v2 delegation from a ChatGPT model to Grok or Claude fail, and why is v1 the default | ||
| until it does not. | ||
|
|
||
| Structure, answer first: | ||
|
|
||
| 1. What you will see — the exact `unreadable_encrypted_agent_task` failure and the HTTP 400. | ||
| 2. Why it happens — an inline SVG, described below. | ||
| 3. What each mode does now, and why v1 is the shipped default. | ||
| 4. What to do if you want v2 anyway, including `keepNativeChatGptOnV1`. | ||
| 5. When this page goes away — the condition under which the default moves back. | ||
|
|
||
| ## The SVG | ||
|
|
||
| Two stacked lanes over the same delegation, so the reader compares one variable. | ||
|
|
||
| The v1 lane: parent (ChatGPT model) emits a plaintext `spawn_agent` task, OpenCodex routes | ||
| it, the routed child reads it, and the lane ends green. | ||
|
|
||
| The v2 lane: the parent emits `encrypted_content` minted by the ChatGPT backend. The lane | ||
| crosses the provider boundary, drawn as a vertical rule, and the routed child holds | ||
| ciphertext with no key. The lane ends at a stop marker labelled with the real error code. | ||
| A short caption states the asymmetry in words: the key never leaves the backend that | ||
| minted it, so the failure is structural rather than a misconfiguration. | ||
|
|
||
| Accessibility is part of the artifact, not a later pass: `role="img"`, a `<title>` and a | ||
| `<desc>`, meaning carried by labels rather than colour alone, and text that stays legible | ||
| at the width Starlight gives a guide body. The prose immediately below repeats what the | ||
| diagram says, so the page still works with images off. | ||
|
|
||
| ## Wiring | ||
|
|
||
| The page needs an explicit sidebar entry in `docs-site/astro.config.mjs`, next to the | ||
| `guides/sub-agent-surface` entry. Registration is per-page there; nothing is autogenerated | ||
| from the directory, so a page without an entry ships invisible. | ||
|
|
||
| `guides/sub-agent-surface.md` gains a pointer at the mode table and its `Not sure?` tip | ||
| changes to recommend v1. The GUI dialog links to the published URL. The i18n mirrors under | ||
| `docs-site/src/content/docs/<locale>/` are not translated in this unit; the English guide | ||
| is the source and the existing locales keep their current surface page. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| # wp5 — Verification and delivery | ||
|
|
||
| ## Local checks | ||
|
|
||
| `bun run typecheck`, `bun run lint:gui`, and the focused files touched by wp2 and wp3. | ||
| The full suite is the hosted gate, not a local one: CI runs typecheck and the whole suite | ||
| on Linux, Windows and macOS, and that run is the evidence the merge decision uses. | ||
|
|
||
| ## Screenshot | ||
|
|
||
| `enforce-target` rejects a PR that mentions `gui` without a screenshot of the UI change. | ||
| That is a real gate and this PR is squarely a GUI change, so the dialog is captured from a | ||
| running dashboard, not mocked in a drawing tool, and attached to the description. | ||
|
|
||
| ## PR | ||
|
|
||
| Target `dev`. Fill Summary, Verification and Checklist from | ||
| `.github/PULL_REQUEST_TEMPLATE.md`. Reference issue #92 as context rather than `Closes`, | ||
| because this unit does not fix the encryption limitation. | ||
|
|
||
| ## Merge | ||
|
|
||
| Maintainer integration through the PR, under the `dev` policy in `MAINTAINERS.md`: record | ||
| the decision and CI evidence at the exact final head. A green aggregate on an earlier | ||
| commit is not evidence for the head being merged. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # Evidence: is the v2 encrypted-task limitation fixed anywhere? | ||
|
|
||
| Collected 2026-09-13 with the Aside browser agent against public GitHub, to check that | ||
| this unit is not shipping a warning about something already solved. | ||
|
|
||
| ## Upstream (openai/codex) | ||
|
|
||
| | Item | State | What it actually covers | | ||
| | --- | --- | --- | | ||
| | #35845 Support plaintext collaboration tool messages | merged, 0.147.0-alpha.1 | The receiving half only. It handles plaintext that was already produced, gated on the `encrypted_function_args: []` marker. It does not make an OpenAI parent emit plaintext. | | ||
| | #36892 Support leaf models in multi-agent v2 | merged | Fixed the Sol/Terra plus Luna case. Does not touch cross-provider delivery. | | ||
| | #36376 OpenAI parent still sends an encrypted v2 task to a non-OpenAI child | open | Reproduced across CLI 0.146 to 0.151 on Windows, macOS and Linux. | | ||
| | #37197 Plaintext support does not complete cross-provider delivery | open | The clearest statement that a sending-side policy is the missing piece; the reporter built and validated a local `message_delivery = "plaintext"` policy. | | ||
| | #32031 spawn_agent hides model overrides | open | Adjacent; no maintainer response. | | ||
|
|
||
| None of the open issues has an official maintainer response or an ETA. So the sending side | ||
| is unfixed, and it is the side that matters here. | ||
|
|
||
| ## Why a proxy cannot paper over it | ||
|
|
||
| With an OpenAI parent the value in `encrypted_content` is real Fernet-shaped ciphertext | ||
| (`gAAAA...`), not a plaintext assignment behind a marker. A proxy that rewrites the field | ||
| has nothing to rewrite it from. The fix has to come from the parent side emitting the | ||
| plaintext marker, which is exactly the path #35845 does not open for OpenAI parents. | ||
|
|
||
| ## Scope of the failure | ||
|
|
||
| Affected: v2 with a ChatGPT-native parent and a routed child. Observed with Sol to Grok, | ||
| Astra to Grok, Luna and Astra to Claude, DeepSeek, GLM and others. It is not model | ||
| specific; no routed child can read backend ciphertext. | ||
|
|
||
| Unaffected: v1 in any topology, routed parent to routed child under v2, and native parent | ||
| to native child under v2 (the backend can decrypt what it minted). | ||
|
|
||
| Already handled here: the mid-thread native-to-routed model switch, fixed by #4135, and | ||
| the opt-in `agentTaskRecovery` path, which recovers most fresh spawns but still loses | ||
| message-type follow-ups and multipart envelopes (#3661). | ||
|
|
||
| ## Conclusion for this unit | ||
|
|
||
| The warning is accurate and the default change is justified. v1 is the surface maintainers | ||
| and users both name as the reliable one for cross-provider delegation, and there is no | ||
| upstream commitment that would make this advisory short-lived by accident. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| # Audit follow-ups | ||
|
|
||
| An independent reviewer audited the roadmap against the branch tip before any code moved. | ||
| Verdict: near-pass. No factual claim in D1 through D5 failed; three cautions are resolved | ||
| here. | ||
|
|
||
| ## 1. The advisory fires for most existing installs, and that is the point | ||
|
|
||
| The reviewer noted that a config written before this change usually has no `multiAgentMode` | ||
| key at all, so the advisory raises for the majority of upgrading users rather than a rare | ||
| corner. That is intended. Those are precisely the operators running base, which pins Sol and | ||
| Terra to v2, and who have never been told what that costs them when they delegate to a | ||
| routed model. It fires once per install and both answers end it. | ||
|
|
||
| ## 2. New test files would trip the layout guard | ||
|
|
||
| Folded into 020: the new assertions extend `tests/server/config.test.ts` and the existing | ||
| management-API describe block in `tests/codex-integration/codex-v2-gate.test.ts` rather than | ||
| creating files that would need registering in two manifests. | ||
|
|
||
| ## 3. The guide needs a sidebar entry | ||
|
|
||
| Folded into 040. | ||
|
|
||
| ## Carried into the build | ||
|
|
||
| `src/config/multi-agent-surface.ts` lands in an existing documented directory, so | ||
| `structure:check` has nothing new to claim. `structure/gui-and-management-api.md` documents | ||
| the `/api/v2` contract and must be updated in the same change, as must the mode table in | ||
| `structure/subagents.md`, which still calls base the install default. | ||
|
|
||
| GUI locale parity is enforced by `Record<TKey, string>` at GUI build time, so a missing key | ||
| in any of the nine catalogs fails `bun run build:gui` rather than slipping through. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| # Outcome | ||
|
|
||
| Landed as [#4462](https://github.com/lidge-jun/opencodex/pull/4462), squashed onto `dev`. | ||
|
|
||
| ## What shipped | ||
|
|
||
| All five work phases, as planned. v1 is the written install default; an absent `multiAgentMode` | ||
| still means base; existing installs are asked once instead of being rewritten; the approval dialog | ||
| guards all three places the mode switch is rendered; and the guide is published with a diagram. | ||
|
|
||
| ## What the plan got wrong | ||
|
|
||
| Two things, both caught by review rather than by writing the plan more carefully. | ||
|
|
||
| **The repair path.** D1 argued that `getDefaultConfig()` reaches an existing config only on the | ||
| schema-repair path, and treated that as safe. It is not: a config missing an unrelated field — | ||
| `defaultProvider` is the common one — takes that path, and the defaults were spread *underneath* | ||
| the stored document. A base operator would have been repaired into v1 with the advisory | ||
| pre-answered, which is exactly the silent flip D2 forbids. Both merges now pin the two keys to the | ||
| stored document. The roadmap audit missed this; the implementation audit found it by enumerating | ||
| the merge paths instead of reasoning about them. | ||
|
|
||
| **The third switch.** 030 described two mode switches. There are three — the Subagents page renders | ||
| one too, and it wrote straight through, which made the dialog decoration on that page. The lesson is | ||
| cheap to state and easy to miss: when a control is duplicated, count the copies before gating one. | ||
|
|
||
| ## Review findings, by who found what | ||
|
|
||
| | Reviewer | Finding | Severity | | ||
| | --- | --- | --- | | ||
| | Implementation audit | Repair path flips an existing operator's surface | blocking | | ||
| | Implementation audit | Ordering comment claimed a transaction the code does not have | blocking | | ||
| | GUI audit (second pass) | Subagents page bypasses the dialog | blocking | | ||
| | GUI audit (second pass) | Continuing to base or v2 left the notice raised, so the poll re-asked | real | | ||
| | GUI audit (second pass) | Locale test grepped source, so a key in a comment would pass | real | | ||
| | Codex | Acknowledgement gated on a projection that goes stale | P2 | | ||
| | CodeRabbit | Staged selections not scoped to the endpoint that staged them | major | | ||
| | CodeRabbit | Escape and backdrop dismissible while a write is in flight | minor | | ||
| | CodeRabbit | Unsupported stored mode echoed raw from the API | minor | | ||
| | CodeRabbit | Warning text overstated the failure on base; Korean, French and Russian copy | minor | | ||
|
|
||
| ## Still true after this lands | ||
|
|
||
| The upstream limitation is unfixed. When a ChatGPT-native parent can emit a routed child's task as | ||
| plaintext, bump `MULTI_AGENT_SURFACE_ADVISORY_VERSION`, move the default back to base, and the same | ||
| machinery tells everyone who was asked the first time. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # Evidence: the dialog as the operator sees it | ||
|
|
||
| Captured from a real dashboard on 2026-09-13. A throwaway OpenCodex home with | ||
| `multiAgentMode: "v2"` and no stored acknowledgement, served on a spare port, read with a | ||
| headless browser. Not a mockup. | ||
|
|
||
| `dashboard-advisory-en.png` — the one-time advisory an existing v2 install raises after | ||
| updating. The sub-agent switch behind it still reads v2, because nothing was changed for | ||
| that operator: the notice is asking, not reporting. | ||
|
|
||
| `dashboard-advisory-ko.png` — the same dialog in Korean, showing the two answers the | ||
| request specified: 계속하기 and v1으로 바꾸기. | ||
|
|
||
| The selection dialog is the same component with `reason="selection"`; its heading and body | ||
| differ and its two answers do not. `gui/tests/subagent-surface-warning.test.tsx` covers that | ||
| variant, the backdrop dismissal, and the gate on all three mode switches. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🔎 Supported by static analysis
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 11569
🏁 Script executed:
Repository: lidge-jun/opencodex
Length of output: 17079
Update the integration-test reference.
The advisory coverage is in
tests/codex-integration/multi-agent-keep-native-v1.test.ts:448-528. The parity block intests/codex-integration/codex-v2-gate.test.ts:1336-1513does not contain these advisory assertions. Update lines 61-63 to referencemulti-agent-keep-native-v1.test.ts.🤖 Prompt for AI Agents