-
Notifications
You must be signed in to change notification settings - Fork 1.2k
docs(devlog): audit the native control stack as one integration contract #4882
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
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,52 @@ | ||
| # L7 — native control stack: read-only audit | ||
|
|
||
| Lane R-L7. Four open pull requests that GitHub shows as four independent | ||
| branches off `dev` are, in commit terms, one four-deep stack: | ||
|
|
||
| ```text | ||
| #4782 native WebSocket steering | ||
| └─ #4858 multi-agent function-result injection | ||
| └─ #4861 typed result continuations + hosted output preservation | ||
| └─ #4864 bounded steering waits + sparse replay output | ||
| ``` | ||
|
|
||
| This lane does not implement anything. It separates what each stage actually | ||
| adds, states the four stages as one contract a reviewer can check, decides in | ||
| code whether issue #4850 gates the stack, and records what still has to happen | ||
| before either flag is turned on. Corrections that belong to an author go to that | ||
| author's pull request as a review comment; no pull request is superseded, | ||
| rebased or reimplemented here. | ||
|
|
||
| ## Units | ||
|
|
||
| - 010 — parent-relative diff of each stage. | ||
| - 020 — the four stages as one integration contract, with verdicts. | ||
| - 030 — whether #4850 (native-main read fence) is a precondition. | ||
| - 040 — stack hygiene, upstream evidence, and the activation decision. | ||
|
|
||
| ## Write scope | ||
|
|
||
| `devlog/_plan/260917_l7_native_control_stack_audit/` only. No `src/`, no | ||
| `tests/`, no `structure/`, no `docs-site/`. The four audited branches are read | ||
| through `git show` and `git diff` against fetched `refs/pull/*/head`; nothing in | ||
| this branch touches them. | ||
|
|
||
| ## Verification posture | ||
|
|
||
| Nothing is executed. No suite, no focused file, no typecheck, no build, no | ||
| proxy. Every claim below is either a source read at a named commit or a hosted | ||
| CI fact read from GitHub, and each is written so a reviewer can re-derive it | ||
| from the same command. Where a claim could not be established from source it is | ||
| recorded as unproven rather than assumed. | ||
|
|
||
| Reference points, all read on 2026-09-17: | ||
|
|
||
| | Ref | Commit | | ||
| |---|---| | ||
| | `origin/dev` | `f1dfda8e48b52a1734eb202550a0225f3e5f8ab1` | | ||
| | #4782 head | `76d7452afb38fd7cc5d9ff7fa4d573b06a9507e3` | | ||
| | #4858 head | `7a9a6d28dd8680cce890e06813e4a08796624d0a` | | ||
| | #4861 head | `59a1d6357e018d44104a50b1126350b72368c81d` | | ||
| | #4864 head | `7b548ad85e8f2a6af313198fa68a4111003cbb05` | | ||
| | #4868 head (undeclared fifth level) | `15a8e715851d53d13d3718b16c8ad4cdc8e6ec32` | | ||
| | openai/codex pinned checkout | `095da4b7e` | | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| # 010 — what each stage actually adds | ||
|
|
||
| ## Why the GitHub diff is misleading | ||
|
|
||
| All four pull requests declare base `dev`, and none of them uses the stacked-child | ||
| workflow described in `AGENTS.md`. Each therefore shows its parents' commits in | ||
| its own diff. #4858 is the extreme case: against `dev` it reads as 120 files and | ||
| `+6441/-1004`, because its branch also carries a pinned-`dev` merge | ||
| (`9c411a1048`) that drags in unrelated integration work. Its own feature delta is | ||
| 39 files. | ||
|
|
||
| The ranges below are the parent-relative deltas. Each one is reproducible: | ||
|
|
||
| ```bash | ||
| git fetch origin pull/4782/head:pr4782 pull/4858/head:pr4858 \ | ||
| pull/4861/head:pr4861 pull/4864/head:pr4864 | ||
| git diff --stat "$(git merge-base origin/dev pr4782)" pr4782 # L7.1 | ||
|
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,60p' devlog/_plan/260917_l7_native_control_stack_audit/010_stage_diffs.md
sed -n '35,52p' devlog/_plan/260917_l7_native_control_stack_audit/000_plan.md
find .. -name AGENTS.md -o -name CONTRIBUTING.md | head -20Repository: lidge-jun/opencodex Length of output: 4966 🤖 get_repo_knowledge executed:
Length of output: 14112 Pin the L7.1 base commit in the reproducibility command.
🤖 Prompt for AI Agents |
||
| git diff --stat 9c411a1048 pr4858 # L7.2 | ||
| git diff --stat de600be5f3 pr4861 # L7.3 | ||
| git diff --stat b00654b368 pr4864 # L7.4 | ||
| ``` | ||
|
|
||
| ## Per-stage delta | ||
|
|
||
| | Stage | PR | Range | Total | `src/` | `tests/` | `structure/` + `docs-site/` | | ||
| |---|---|---|---|---|---|---| | ||
| | L7.1 | #4782 | merge-base → `76d7452afb` | 40 files, +1278/-33 | 15 files, +669/-31 | 3 files, +478/-1 | 21 files, +129 | | ||
| | L7.2 | #4858 | `9c411a1048` → `7a9a6d28dd` | 39 files, +1165/-71 | 16 files, +495/-52 | 5 files, +519/-19 | 17 files, +150 | | ||
| | L7.3 | #4861 | `de600be5f3` → `59a1d6357e` | 29 files, +621/-58 | 7 files, +230/-38 | 5 files, +292/-3 | 16 files, +98/-17 | | ||
| | L7.4 | #4864 | `b00654b368` → `7b548ad85e` | 25 files, +496/-41 | 5 files, +121/-39 | 3 files, +275/-1 | 16 files, +98 | | ||
|
Comment on lines
+25
to
+30
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. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🔎 Supported by static analysis🏁 Script executed: sed -n '1,35p' devlog/_plan/260917_l7_native_control_stack_audit/010_stage_diffs.md
rg -n 'other/|Total|structure/|docs-site/' devlog/_plan/260917_l7_native_control_stack_auditRepository: lidge-jun/opencodex Length of output: 2379 🏁 Script executed: #!/bin/bash
set -e
sed -n '25,80p' devlog/_plan/260917_l7_native_control_stack_audit/010_stage_diffs.md
printf '\n--- plan references ---\n'
sed -n '1,75p' devlog/_plan/260917_l7_native_control_stack_audit/000_plan.md
printf '\n--- recorded range stats by path ---\n'
for spec in \
'$(git merge-base origin/dev pr4782) pr4782' \
'9c411a1048 7a9a6d28dd' \
'de600be5f3 59a1d6357e' \
'b00654b368 7b548ad85e'; do
echo "RANGE $spec"
eval "git diff --name-only $spec" | awk '
BEGIN { src=tests=structure=docs=other=0 }
/^src\\// { src++; next }
/^tests\\// { tests++; next }
/^structure\\// { structure++; next }
/^docs-site\\// { docs++; next }
{ other++ }
END { printf "src=%d tests=%d structure=%d docs-site=%d other=%d total=%d\\n", src, tests, structure, docs, other, src+tests+structure+docs+other }'
doneRepository: lidge-jun/opencodex Length of output: 6505 🏁 Script executed: #!/bin/bash
set -o pipefail
printf '%s\n' '--- remaining stage-diff text ---'
sed -n '70,125p' devlog/_plan/260917_l7_native_control_stack_audit/010_stage_diffs.md
printf '%s\n' '--- remote ---'
git remote -v
printf '%s\n' '--- compare path counts from GitHub API ---'
python3 - <<'PY'
import json, subprocess, urllib.request
repo = "lidge-jun/opencodex"
ranges = [
("L7.1", "f1dfda8e48b52a1734eb202550a0225f3e5f8ab1", "76d7452afb38fd7cc5d9ff7fa4d573b06a9507e3"),
("L7.2", "9c411a1048", "7a9a6d28dd8680cce890e06813e4a08796624d0a"),
("L7.3", "de600be5f3", "59a1d6357e018d44104a50b1126350b72368c81d"),
("L7.4", "b00654b368", "7b548ad85e8f2a6af313198fa68a4111003cbb05"),
]
for name, base, head in ranges:
url = f"https://api.github.com/repos/{repo}/compare/{base}...{head}"
req = urllib.request.Request(url, headers={"Accept":"application/vnd.github+json","User-Agent":"review-inspection"})
try:
with urllib.request.urlopen(req, timeout=20) as r:
data = json.load(r)
files = [x["filename"] for x in data.get("files", [])]
counts = {}
for path in files:
key = next((k for k in ("src/","tests/","structure/","docs-site/") if path.startswith(k)), "other")
counts[key] = counts.get(key, 0) + 1
print(name, "status=", data.get("status"), "ahead_by=", data.get("ahead_by"),
"files_returned=", len(files), "counts=", counts)
print("other_paths=", [p for p in files if not p.startswith(("src/","tests/","structure/","docs-site/"))])
except Exception as e:
print(name, "ERROR", repr(e))
PYRepository: lidge-jun/opencodex Length of output: 3368 🏁 Script executed: #!/bin/bash
set -e
python3 - <<'PY'
import json, subprocess
ranges = [
("L7.1", "f1dfda8e48b52a1734eb202550a0225f3e5f8ab1", "76d7452afb38fd7cc5d9ff7fa4d573b06a9507e3"),
("L7.2", "9c411a1048", "7a9a6d28dd8680cce890e06813e4a08796624d0a"),
("L7.3", "de600be5f3", "59a1d6357e018d44104a50b1126350b72368c81d"),
("L7.4", "b00654b368", "7b548ad85e8f2a6af313198fa68a4111003cbb05"),
]
for name, base, head in ranges:
url = f"https://api.github.com/repos/lidge-jun/opencodex/compare/{base}...{head}"
raw = subprocess.check_output([
"curl", "-ksS", "--fail", "-H", "Accept: application/vnd.github+json",
"-H", "User-Agent: review-inspection", url
], text=True)
data = json.loads(raw)
files = [entry["filename"] for entry in data.get("files", [])]
buckets = {"src/": [], "tests/": [], "structure/": [], "docs-site/": [], "other": []}
for path in files:
key = next((prefix for prefix in ("src/","tests/","structure/","docs-site/") if path.startswith(prefix)), "other")
buckets[key].append(path)
print(f"{name}: status={data.get('status')} files={len(files)}")
for key, paths in buckets.items():
print(f" {key}: {len(paths)}")
if key == "other":
print(" " + "\n ".join(paths))
PYRepository: lidge-jun/opencodex Length of output: 682 Account for the omitted Each range includes 🤖 Prompt for AI Agents |
||
|
|
||
| The `structure/` rows are almost entirely ownership-table lines required by | ||
| `structure:check`, not new architecture prose. | ||
|
|
||
| ## L7.1 — #4782, native WebSocket steering | ||
|
|
||
| New modules: `native-steering.ts` (the channel: envelope validation, parent/steer | ||
| bookkeeping, settings pinning by digest, chain and byte caps), | ||
| `native-steering-replay.ts` (connection-local journal where only a | ||
| `response.created` successor commits queued input), `native-steering-log.ts` | ||
| (per-response usage aggregation that never samples control frames). | ||
|
|
||
| Wiring: `codexNativeSteering` in the config schema and `OcxConfig`; the inbound | ||
| WS handler recognizes `response.steer` and routes `response.create` through | ||
| `continue()`; `ws-upstream` skips the idle-socket pool when a control channel is | ||
| present; `codex-ws-exchange` attaches the channel and owns control sends; | ||
| `passthrough-delivery` keeps the bounded upstream as the sole reader of a | ||
| multi-terminal stream; `ws-bridge` gains `untilEof` so one SSE body may carry | ||
| several response terminals. | ||
|
|
||
| One refactor rides along: `markBodyNonPersistable` moves from `responses/state.ts` | ||
| to a new `responses/state/body-policy.ts` so the dispatch path can also read it. | ||
|
|
||
| ## L7.2 — #4858, multi-agent function-result injection | ||
|
|
||
| New modules: `native-injection.ts` (a second, separate owner with a FIFO of at | ||
| most one in-flight frame, because the acknowledgement names the response rather | ||
| than the injection), `native-injection-protocol.ts`, `native-injection-replay.ts`, | ||
| `native-response-control.ts` (the `NativeResponseControl` interface, the | ||
| eligibility predicate and the mode selector). | ||
|
|
||
| This is the stage that widens the route surface. `nativeResponseControlEligible` | ||
| keeps canonical ChatGPT forwarding for both modes and additionally admits, for | ||
| injection only, an `openai-responses` provider pinned to exactly | ||
| `https://api.openai.com/v1` with `upstreamWebsocket: true` and a non-forward auth | ||
| mode; on that route `ws-upstream` appends `responses_multi_agent=v1` to | ||
| `openai-beta` without discarding configured tokens. Mode selection reads the | ||
| frame, never the model name: a request carrying `multi_agent.enabled: true` can | ||
| only obtain the injection channel, and the steering channel's constructor rejects | ||
| it outright. | ||
|
|
||
| ## L7.3 — #4861, typed result continuations and hosted output | ||
|
|
||
| New modules: `native-tool-results.ts` (the wider saved-result schema — | ||
| `custom_tool_call_output`, `mcp_approval_response`, rich content parts with | ||
| bounded image/file references, and caller provenance reduced to a digest) and | ||
| `native-response-output.ts` (merges completed `response.output_item.done` items | ||
| with a sparse terminal `output`, preserving relative order and failing on a | ||
| contradiction instead of dropping items). | ||
|
|
||
| Two later commits on this branch are corrections, and they matter to L7.4: | ||
| `4670525d48` rejects a continuation that *omits* a pinned setting — before it, | ||
| only a changed setting was caught, so dropping a key bypassed the pin — and | ||
| `59a1d6357e` refunds the exact reserved byte count of an injection batch instead | ||
| of recomputing it from a possibly different serialization. | ||
|
|
||
| ## L7.4 — #4864, bounded waits and sparse replay output | ||
|
|
||
| Replaces the steering channel's single re-armable `wait()` with absolute | ||
| deadlines. Before this stage each control re-armed a fresh 90-second window, so a | ||
| client that kept sending controls could hold the socket indefinitely, and late | ||
| wire activity could win a race against an expired-but-unfired timer. After it, | ||
| every stage carries its own absolute deadline (`nextDeadline` takes the earliest, | ||
| `armTimer` never extends one, `assertTimely` settles on arrival, `expire` settles | ||
| exactly once and reports unknown delivery rather than retrying). | ||
|
|
||
| It also extracts `native-response-json.ts` so the JSON record/fingerprint helpers | ||
| no longer live in the injection protocol module, and routes the steering replay's | ||
| terminal output through `nativeResponseOutput` — previously it took the terminal | ||
| `output` whenever non-empty and silently dropped observed items a sparse terminal | ||
| omitted. | ||
|
|
||
| ## The divergence a reviewer must see | ||
|
|
||
| #4864 is based on `b00654b368`, which is #4861's *feature* commit, not #4861's | ||
| head. The two corrections above are not in #4864's branch, so nothing has been | ||
| built or tested on the combination `dev` will actually receive. A squash merge in | ||
| #4861 → #4864 order does not revert them, because neither correction touches a | ||
| line #4864 edits, but the combined behavior is unverified. #4864 should be rebased | ||
| onto #4861's head before it is treated as the stack tip. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,141 @@ | ||
| # 020 — the four stages as one contract | ||
|
|
||
| The stages are separate pull requests but a single runtime object graph: one | ||
| downstream WebSocket turn owns one control channel, that channel owns one | ||
| physical upstream socket, and every later control frame is emitted through the | ||
| closure that opened it. The five clauses below are what that graph has to | ||
| guarantee for the stack to be safe to enable. Each verdict is a source read at | ||
| the heads listed in `000_plan.md`. | ||
|
|
||
| ## C1 — the selected account and the original physical socket are preserved | ||
|
|
||
| **Holds by construction.** | ||
|
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. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Correct the C1 verdict for the current stack. Lines 26-29 state that the injection channel's omitted-key check exists only in 🤖 Prompt for AI Agents |
||
|
|
||
| `codexWsUpstreamFetch` computes a pool-reuse identity only when no control | ||
| channel is present (`const identity = control ? null : codexWsReuseIdentity(...)`), | ||
| so an owned connection is never taken from, and never returned to, the idle | ||
| socket pool. Control frames are sent through the `ws` captured by | ||
| `codexWsExchange`, which is the same physical connection that carried the | ||
| original create. A `response.create` continuation is not re-routed: the exchange | ||
| rebuilds it from the original `frameText` and overlays only `input` and | ||
| `previous_response_id`, so a caller-supplied `previous_response_id` never reaches | ||
| the REST sanitizer or the account selector. The per-send guard receives | ||
| `new Headers(headers)` — a copy — so it can reserve quota and refuse a send but | ||
| cannot swap the credential underneath an open socket. | ||
|
|
||
| Both channels also pin the request's non-envelope settings as SHA-256 digests at | ||
| construction and reject any continuation whose settings differ. After | ||
| `4670525d48` (on #4861, not on #4864) the injection channel also rejects a | ||
| continuation that omits a pinned key. | ||
|
|
||
| Residual: preservation is the point, so the credential chosen at create time is | ||
| the credential the whole chain uses, for as long as the chain lives. See C4 for | ||
| how long that can be, and 030 for why that makes #4850 an activation | ||
| precondition. | ||
|
|
||
| ## C2 — an unsupported route does not detour to another account or to HTTP | ||
|
|
||
| **Holds, with one seam worth an explicit branch.** | ||
|
|
||
| Four independent gates have to agree before a channel is constructed or used: | ||
| `nativeResponseControlMode` requires the matching flag to be exactly `true`; | ||
| `nativeResponseControlEligible` requires canonical ChatGPT forwarding, or — for | ||
| injection only — an `openai-responses` provider pinned to exactly | ||
| `https://api.openai.com/v1` with `upstreamWebsocket: true` and a non-forward auth | ||
| mode; `preparePassthroughExchange` additionally requires `inboundTransport === | ||
| "websocket"`, no Combo attempt, and no plaintext-v2 agent-message tool rewriting; | ||
| `codexWsUpstreamFetch` re-checks `prepared.canonical` (or the exact public API | ||
| URL) and, for injection, re-parses the outgoing frame to confirm | ||
| `multi_agent.enabled`. A route that fails any of them gets `undefined`, and a | ||
| later `response.steer` or `response.inject` is answered with an explicit | ||
| `steering_not_supported` / `injection_not_supported` error frame rather than | ||
| being discarded or retried elsewhere. | ||
|
|
||
| The seam is in `codex-ws-exchange`. `nativeSteering.attach()` is called inside the | ||
| same `try` block as `ws.send(frameText)`, and that block's `catch` treats a | ||
| pre-activity failure as "the frame never left" and resolves `sseFallback(url, | ||
| init)`. For steering that is reachable but harmless, because `attach` only | ||
| refuses when the channel is already bound. For injection it is reachable and | ||
| consequential: `NativeInjectionChannel.attach` throws permanently once | ||
| `everAttached` is set, so a second physical WebSocket attempt for the same turn — | ||
| the transient-retry wrapper passes the same channel to every dispatch site — | ||
| converts a multi-agent turn into an ordinary HTTP turn while the client still | ||
| holds a channel that can never attach. No credential moves and no success is | ||
| invented; the client learns only when its first `response.inject` is refused. | ||
| An attach failure should be distinguishable from a send failure rather than | ||
| sharing the fallback path. Raised on #4858. | ||
|
|
||
| ## C3 — a steering or injection failure is never presented as success | ||
|
|
||
| **Holds.** | ||
|
|
||
| Every settle path reports uncertainty instead of inventing an outcome. | ||
| `NativeSteeringChannel.expire()` settles once, disposes the replay journal and | ||
| calls `onFailure`, which fails the client stream; the message states that | ||
| delivery is unknown and that tools and steering input must not be replayed. | ||
| `NativeInjectionChannel.fail()` does the same and never falls back to HTTP, | ||
| re-sends or re-runs a tool. An acknowledgement must match the sole in-flight | ||
| submission by response ID and strictly increasing sequence number, and a | ||
| `response.inject.failed` must carry a fingerprint of exactly the submitted | ||
| results, so a rejection cannot be attributed to a different batch. Only a | ||
| `response_already_completed` rejection is marked recoverable, keyed by a digest | ||
| of the saved result rather than a second copy of it. A response terminal does not | ||
| finish the owner while submitted results are unacknowledged. | ||
|
|
||
| In the replay journals, only a validated `response.created` successor commits | ||
| queued input, and only `response.completed` reaches shared continuation state — | ||
| a steered or failed parent's output is used solely as a successor's prefix. | ||
|
|
||
| One suppression exists and is correct: `createNativeSteeringLogObserver` does not | ||
| record a parent's `response.incomplete` with `incomplete_details.reason === | ||
| "steered"` as an upstream failure. That affects the request log only; the frame | ||
| itself is still relayed to the client unchanged. | ||
|
|
||
| ## C4 — cancellation and confirmation waits terminate finitely | ||
|
|
||
| **Holds per stage after #4864 — but the chain has no aggregate bound.** | ||
|
|
||
| After #4864 every wait is an absolute deadline rather than a re-armable window: | ||
| 90 s per unacknowledged steer, 90 s for an automatic successor after a parent | ||
| terminal, 90 s for a sent continuation, 30 minutes for a server-requested | ||
| required-input wait, and `stallTimeoutSec` (default 300 s) of idle while a | ||
| response is streaming. The injection channel uses the same 90 s acknowledgement | ||
| bound, explicitly non-extendable by unrelated stream activity, plus the same | ||
| 30-minute saved-result wait. `assertTimely()` closes the race where a late frame | ||
| arrives after a deadline passed but before its timer fired. | ||
|
|
||
| What is not bounded is their composition. A chain may run up to 128 responses on | ||
| one owned connection, and each response may legitimately consume its own idle and | ||
| required-input waits, so a single downstream turn can hold one physical socket | ||
| and one pinned credential for far longer than any ordinary turn — on the order of | ||
| tens of hours in the worst case, without any individual deadline being violated. | ||
| Nothing in the stack caps the lifetime of the owned connection itself. That is an | ||
| operational number the activation decision needs, not a correctness defect. | ||
| Raised on #4864. | ||
|
|
||
| Client disconnect and supersession are handled: a new `response.create` clears | ||
| `ws.data.nativeSteering` before admission and calls the previous turn's | ||
| `cancel()`, and the exchange's `cleanup()` runs `detachSteering`, which drops the | ||
| timers, the retained prefix and every queued submission. | ||
|
|
||
| ## C5 — the default-off boundary the original PR proposed is intact | ||
|
|
||
| **Holds.** | ||
|
|
||
| `codexNativeSteering` and `codexNativeInjection` are optional booleans in the | ||
| config schema, absent by default, and the mode selector requires `=== true`. | ||
| Neither channel can be constructed outside the inbound WebSocket create path, | ||
| which itself requires the already-opt-in `websockets: true`. Injection requires a | ||
| third, client-supplied gate: the create frame must carry | ||
| `multi_agent.enabled: true`. No model name, catalog entry or capability flag | ||
| turns any of this on, and rollback is unsetting the flag and restarting. | ||
|
|
||
| ## Summary | ||
|
|
||
| | Clause | Verdict | Follow-up | | ||
| |---|---|---| | ||
| | C1 account and socket identity | Holds | — | | ||
| | C2 no unsupported-route detour | Holds, one seam | Comment on #4858 | | ||
| | C3 no failure reported as success | Holds | — | | ||
| | C4 finite waits | Holds per stage, chain unbounded | Comment on #4864 | | ||
| | C5 default-off boundary | Holds | — | | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| # 030 — is #4850 a precondition for this stack? | ||
|
|
||
| Issue #4850 reports that a `thread_spawn` request authenticating with its own | ||
| forwardable Codex bearer still opens the operator's physical native-main | ||
| `auth.json` during request preview. The existing fence | ||
|
Comment on lines
+3
to
+5
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.
Because #4850 is explicitly unresolved, these lines publish an unfixed credential-boundary finding and identify the affected request and native-main credential path in the repository's tracked public AGENTS.md reference: AGENTS.md:L124-L131 Useful? React with 👍 / 👎. |
||
| (`nativeMainReadsForbidden`, threaded through `request-prepare.ts`, | ||
| `auth-context.ts`, `core-normalize.ts` and `subagent-model-fallback.ts`) already | ||
| covers quota priming, entitlement discovery, denial-cache validation, | ||
| reconciliation and final selection. Pool eligibility is outside it: | ||
| `isCodexAccountUsable` reaches `isMainAccountCredentialUsable()` for the main | ||
| account unless `nativeMainSelectionOnly` is set, and the preview closures at | ||
| `request-prepare.ts:550` and `:731` call `previewCodexAccountForRequest` without | ||
| that suppression. | ||
|
|
||
| ## Mechanically, no | ||
|
|
||
| The four pull requests touch nothing in that path. Taking the whole stack at | ||
| #4864's head against its included `dev`: | ||
|
|
||
| ```bash | ||
| git diff --name-only 7ef3f67452 pr4864 -- src/codex src/routing \ | ||
| src/server/responses/request-prepare.ts | ||
| # (no output) | ||
| ``` | ||
|
|
||
| Nothing under `src/codex/`, nothing under `src/routing/`, and not | ||
| `request-prepare.ts`. The stack adds a control channel below the point where | ||
| preview and selection have already run. It does not re-enter them either: a | ||
| continuation is rebuilt from the original create frame inside | ||
| `codex-ws-exchange`, so `previous_response_id` never reaches the account | ||
| selector, and the per-frame guard gets a copy of the original headers. So #4850 | ||
| is not a merge-order blocker, and none of the four PRs can fix or worsen the | ||
| read itself. | ||
|
|
||
| ## Substantively, yes — for turning the flags on | ||
|
|
||
| Two facts make it a precondition for activation rather than for merging. | ||
|
|
||
| First, the request class is the same one. #4850's reproduction is a | ||
| `thread_spawn` request carrying a caller-owned bearer, and a multi-agent | ||
| injection turn is exactly that class: every injection create traverses | ||
| `prepareResponsesRequest`, and therefore the unfenced pool-eligibility preview, | ||
| before any channel exists. Enabling `codexNativeInjection` does not introduce the | ||
| read, but it makes the affected request class the primary use of the feature. | ||
|
|
||
| Second, the stack's whole value is that the create-time decision is pinned. C1 | ||
| holds precisely because the account chosen at create time is the account the | ||
| entire chain uses, and C4 shows that chain can be long. #4850's observable | ||
| consequence — operator-main liveness, cached quota and plan state influencing a | ||
| subagent model rewrite for a request that owns its own credential — is a | ||
| one-request inconsistency today. Under this stack the same preview result governs | ||
| up to 128 responses on one pinned credential, with no re-evaluation point in | ||
| between, because there deliberately is none. | ||
|
|
||
| ## Determination | ||
|
|
||
| #4850 does not gate landing #4782, #4858, #4861 or #4864. It gates documenting | ||
| or recommending `codexNativeInjection: true`, and it should be resolved before | ||
| any operator is told to enable it. The lane that owns #4850 should know that | ||
| fixing pool eligibility inside the fence is enough for this stack; no additional | ||
| seam is needed on the native control path. | ||
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: 4096
Distinguish the audited stages from the activation scope.
000_plan.mdpresents#4782through#4864as the four audited implementation stages, but its reference table also lists#4868as a fifth child.040_activation_decision.mdincludes#4868in stack hygiene and activation because it affects the settings pin used by C1. State that#4868is not an audited implementation stage but is considered for stack hygiene and activation. Do not describe it as entirely out of scope.🤖 Prompt for AI Agents