Creatidy/scarcity-router
5
0
Fork 0
Code Issues 13 Pull requests Projects Releases Packages Wiki Activity Actions

E. Codex client tool-call round trip (security-critical) #137

New issue
Closed
opened 2026-09-26 21:43:46 +00:00 by kilo · 10 comments
kilo commented 2026-09-26 21:43:46 +00:00
Member
Copy link

Child E of #132. Security-critical. The primary interoperability blocker: client-owned tool calls on the Codex subscription source. Investigation first; implementation ONLY if semantics can be preserved.

Required semantic boundary

OpenAI tools[] → Codex external/dynamic tool declaration
  → Codex requests a CLIENT tool
  → Scarcity Router returns an OpenAI tool_call to the CLIENT
  → ZCode executes it under ZCode's own permission model
  → ZCode sends the tool result back
  → Scarcity Router maps the result into the Codex continuation
  → Codex continues generation

Hard invariants

  • Scarcity Router NEVER executes ZCode's client-owned tools; the Precision worker NEVER executes Bash/Read/Edit/etc as worker-local tools.
  • Codex internal commandExecution/fileChange tools are never confused with client-owned OpenAI function tools.
  • Controlled CODEX_HOME isolation, no unapproved MCP/plugins/apps, sandbox/network restrictions, approval-cancellation behavior: unchanged.
  • Client tool_call ids and result association preserved; pending continuation state bound to the authenticated inference client + exact execution target; bounded by count/bytes/TTL; cancellation, disconnects, worker loss, server restart handled explicitly; no cross-client leakage; no fabricated tool results; no replay of ambiguously consumed external actions.

Investigation (before any coding)

  1. Establish the CURRENT Codex binary/App Server version on the controlled source and its generated protocol/schema. Do NOT assume the 2026-09-20 rust-v0.155.1 evidence still describes today's runtime.
  2. Determine whether dynamicTools / item/tool/call (or successor mechanisms) can express: exact client tool declarations per turn; Codex pausing for a client tool; the client result returning into the SAME turn/thread; and how cancel/interrupt interacts.
  3. Evaluate both implementation families and pick only with proof:
    • A. Stateful suspended continuation: keep the app-server/thread/turn alive while the OpenAI client executes; resume on the next request.
    • B. Stateless/reconstructed continuation: terminate/interrupt safely; reconstruct assistant tool_calls + tool results as supported items; continue in a fresh ephemeral thread/turn.
      Use the simpler design only if semantic equivalence is proved.
  4. STOP and report a well-supported NO if the current runtime cannot round-trip client tools safely without semantic loss, unsafe state retention or worker-side execution. Do not weaken the compatibility matrix to pass admission.

If viable

  • Change tool_calls/tool_results evidence cells only to the level actually established.
  • Deterministic fake-App-Server tests; full composed HTTP → worker → adapter tests; tool-call ids + multiple sequential tool turns; client cancellation while pending; stale/expired continuation; wrong client; wrong model/source/effort continuation; server/worker restart behavior.
  • Child-A tools pin flips only to the evidenced level.

Review

Independent GLM-5.3-Flash review + mandatory Daybreak (gpt-daybreak-blue-latest) security review when available; if unavailable: SECURITY_REVIEW_UNAVAILABLE, gate stays open, PR paused at the human decision point. UX assessment mandatory.

Complexity boundary

Unknown until investigation concludes; investigation itself is bounded at 90 min wall clock. Implementation, if authorized by evidence, is a separate frozen-scope amendment of this issue before coding starts.

Non-goals

No /v1/responses surface; no parallel tool-call fan-out semantics beyond evidence; no executing client tools anywhere in Scarcity Router; no matrix weakening.

Child E of #132. **Security-critical.** The primary interoperability blocker: client-owned tool calls on the Codex subscription source. Investigation first; implementation ONLY if semantics can be preserved. ## Required semantic boundary ``` OpenAI tools[] → Codex external/dynamic tool declaration → Codex requests a CLIENT tool → Scarcity Router returns an OpenAI tool_call to the CLIENT → ZCode executes it under ZCode's own permission model → ZCode sends the tool result back → Scarcity Router maps the result into the Codex continuation → Codex continues generation ``` ## Hard invariants - Scarcity Router NEVER executes ZCode's client-owned tools; the Precision worker NEVER executes Bash/Read/Edit/etc as worker-local tools. - Codex internal `commandExecution`/`fileChange` tools are never confused with client-owned OpenAI function tools. - Controlled CODEX_HOME isolation, no unapproved MCP/plugins/apps, sandbox/network restrictions, approval-cancellation behavior: unchanged. - Client tool_call ids and result association preserved; pending continuation state bound to the authenticated inference client + exact execution target; bounded by count/bytes/TTL; cancellation, disconnects, worker loss, server restart handled explicitly; no cross-client leakage; no fabricated tool results; no replay of ambiguously consumed external actions. ## Investigation (before any coding) 1. Establish the CURRENT Codex binary/App Server version on the controlled source and its generated protocol/schema. Do NOT assume the 2026-09-20 rust-v0.155.1 evidence still describes today's runtime. 2. Determine whether `dynamicTools` / `item/tool/call` (or successor mechanisms) can express: exact client tool declarations per turn; Codex pausing for a client tool; the client result returning into the SAME turn/thread; and how cancel/interrupt interacts. 3. Evaluate both implementation families and pick only with proof: - **A. Stateful suspended continuation:** keep the app-server/thread/turn alive while the OpenAI client executes; resume on the next request. - **B. Stateless/reconstructed continuation:** terminate/interrupt safely; reconstruct assistant tool_calls + tool results as supported items; continue in a fresh ephemeral thread/turn. Use the simpler design only if semantic equivalence is proved. 4. **STOP and report a well-supported NO** if the current runtime cannot round-trip client tools safely without semantic loss, unsafe state retention or worker-side execution. Do not weaken the compatibility matrix to pass admission. ## If viable - Change `tool_calls`/`tool_results` evidence cells only to the level actually established. - Deterministic fake-App-Server tests; full composed HTTP → worker → adapter tests; tool-call ids + multiple sequential tool turns; client cancellation while pending; stale/expired continuation; wrong client; wrong model/source/effort continuation; server/worker restart behavior. - Child-A tools pin flips only to the evidenced level. ## Review Independent GLM-5.3-Flash review + **mandatory Daybreak (`gpt-daybreak-blue-latest`) security review** when available; if unavailable: `SECURITY_REVIEW_UNAVAILABLE`, gate stays open, PR paused at the human decision point. UX assessment mandatory. ## Complexity boundary Unknown until investigation concludes; investigation itself is bounded at 90 min wall clock. Implementation, if authorized by evidence, is a separate frozen-scope amendment of this issue before coding starts. ## Non-goals No `/v1/responses` surface; no parallel tool-call fan-out semantics beyond evidence; no executing client tools anywhere in Scarcity Router; no matrix weakening.
kilo commented 2026-09-27 21:01:48 +00:00
Author
Member
Copy link

Additive scope note (D-056, PR #147 — frozen scope text above unchanged):

D-056 pt 5 reframes what this child investigates — read "ZCode" in the frozen scope above as "the harness" throughout:

  • The question is whether the Codex execution source can satisfy the GENERIC harness-owned tool-call + tool-result semantic contract (the D-056 lifecycle) safely and without semantic loss. ZCode is a representative workload only; whichever harness is on the other end executes its own client-owned tools under its own permission model.
  • This is not a global Scarcity Router tool deficiency: the generic contract is already implemented on evidence-backed server-direct execution paths (tool_calls/tool_results PASS/PARTIAL cells). The gap is specifically the Codex worker source, whose evidence records both cells UNSUPPORTED.
  • A well-supported TOOL_BRIDGE_NO remains a fully valid outcome — "a limitation of that source, never of the architecture" (D-056 pt 5). Do not weaken compatibility evidence to manufacture support; the frozen investigation/STOP discipline above is unchanged.

Authoritative text: D-056 pt 5 in docs/decisions.md; "Harness compatibility contract (D-056)" in docs/execution-surface.md.

Additive scope note (D-056, PR #147 — frozen scope text above unchanged): D-056 pt 5 reframes what this child investigates — read "ZCode" in the frozen scope above as "the harness" throughout: - The question is whether the Codex execution source can satisfy the GENERIC harness-owned tool-call + tool-result semantic contract (the D-056 lifecycle) safely and without semantic loss. ZCode is a representative workload only; whichever harness is on the other end executes its own client-owned tools under its own permission model. - This is not a global Scarcity Router tool deficiency: the generic contract is already implemented on evidence-backed server-direct execution paths (`tool_calls`/`tool_results` PASS/PARTIAL cells). The gap is specifically the Codex worker source, whose evidence records both cells UNSUPPORTED. - A well-supported **TOOL_BRIDGE_NO** remains a fully valid outcome — "a limitation of that source, never of the architecture" (D-056 pt 5). Do not weaken compatibility evidence to manufacture support; the frozen investigation/STOP discipline above is unchanged. Authoritative text: D-056 pt 5 in `docs/decisions.md`; "Harness compatibility contract (D-056)" in `docs/execution-surface.md`.
kilo commented 2026-09-27 23:15:51 +00:00
Author
Member
Copy link

Tool-bridge investigation report — CURRENT runtime re-evidence (2026-09-28, UTC)

Verdict: TOOL_BRIDGE_VIABLE (conditional — constraints and unverified halves recorded below; implementation remains gated on a separate frozen-scope amendment of this issue before any coding, per the frozen scope).

Investigation only; no repository code, matrix or evidence cells were changed by this investigation. All probes were bounded, structure-only, on a fresh controlled CODEX_HOME under /tmp (never the user home), with synthetic strings only: no account/auth method was sent, no turn was executed, no inference was possible (the probe home is unsigned-in and turns fail closed at upstream 401), and no quota was consumed. Output was sanitized to structure before recording.

1. Current runtime established (do-not-assume-2026-09-20 satisfied)

  • Binary: codex-cli 0.155.0-alpha.16.3 — U-001 VS Code extension discovery layout (openai.chatgpt-26.917.62051-linux-x64, bin/linux-x86_64/codex-package.json layoutVersion: 1, variant: "codex", static musl ELF). This is NEWER lineage than both the 2026-09-20 probe (0.154.0-alpha.6.2) and the pinned rust-v0.155.1 schema tag.
  • Version-pinned schema generated FROM THIS BINARY (codex app-server generate-json-schema): 39 top-level + 271 v2 files. The generated bundle reflects the STABLE surface: experimental-gated fields are omitted (proved below), so schema absence alone is not evidence of removal.

2. Findings against the frozen investigation questions

F1 — The client-tool mechanism exists on the current runtime and is experimentalApi-gated (three evidence classes agree).

  • Local probe: thread/start with dynamicTools WITHOUT the capability → JSON-RPC error -32600, message thread/start.dynamicTools requires experimentalApi capability (field RECOGNIZED and gated — not an unknown field; the same probe shows genuinely unknown fields are silently ignored, i.e. parsing is lenient, so this error is positive identification). WITH capabilities.experimentalApi: true → thread/start accepted.
  • Official doc (fetched 2026-09-28; developers.openai.com/codex/app-server now redirects to learn.chatgpt.com/docs/app-server): "Dynamic tool calls (experimental)" section — dynamicTools on thread/start requires capabilities.experimentalApi = true; per-invocation flow: item/started (dynamicToolCall, status inProgress) → item/tool/call server request → client response with content items → item/completed with final status/contentItems/success. It also states Codex persists dynamic tools in thread rollout metadata and restores them on thread/resume.
  • Binary-generated schema: ServerRequest carries item/tool/call (DynamicToolCallParams: threadId, turnId, callId, tool, namespace?, arguments); DynamicToolCallResponse: {success: boolean, contentItems: [inputText | inputImage | inputAudio]}; dynamicToolCall ThreadItem: {id, type, tool, namespace?, arguments, status (inProgress|completed|failed), success, contentItems, durationMs}.

F2 — Constraint: declarations are THREAD-scoped, not per-turn. TurnStartParams has no dynamicTools member (live probe: turn/start with dynamicTools silently ignores it — consistent with the lenient parse control; the stable schema omits it too). Exact per-turn tool sets are therefore expressible only when the tool set is stable for the life of the thread; a client that changes its tools[] between the declaration and the continuation cannot be honored exactly on the open thread → must be a typed fail-closed rejection, never a silent narrowing.

F3 — Client-owned vs backend-native tools stay cleanly separated. dynamicToolCall is a distinct ThreadItem type alongside commandExecution, fileChange, mcpToolCall, collabAgentToolCall. Nothing in the mechanism executes a client tool in Codex or in the worker: the adapter would RELAY item/tool/call to the harness and return the harness's result. toolOutput on turn/start (stable) is now {name, namespace?, output} — name-keyed, no callId.

F4 — Implementation families. Family A (stateful suspended continuation) is the protocol-native design: the pause IS the pending item/tool/call JSON-RPC request on the live stdio connection; the continuation IS answering it (callId-correlated) so the SAME turn/thread completes; docs confirm tools persist and are restored on thread/resume. Family B (stateless reconstruction) has two UNVERIFIED hinges: (a) turn/interrupt behavior while a server request is pending, and (b) thread/inject_items fidelity for assistant function-call history (the 0.154-era adapter already rejected assistant-side tool history replay). Family B is NOT proved semantically equivalent, so under the frozen rule ("use the simpler design only if semantic equivalence is proved") family A is the only selectable design on this evidence.

F5 — Feature-flag surface. experimentalFeature/list (live, 147 entries) exposes NO dynamic-tools flag: gating is purely the experimentalApi capability. The D-019/U-011 generation-aware containment (parse per probed generation, safe disable on contract change) remains the real defense.

F6 — Cancel/interrupt. turn/interrupt → TurnStatus: interrupted is documented and schema-present. Its interaction with a PENDING item/tool/call is UNVERIFIED live (needs a signed-in turn). Safe resolution regardless of behavior: the worker owns the app-server process lifecycle (bounded terminate/kill/reap, already implemented) — process teardown resolves any ambiguous pending state without fabricating a tool result.

3. Hard-invariant check (all preservable)

  • Scarcity Router / worker never execute client-owned tools (relay-only; F3).
  • Codex-internal tools never presented as client tool calls (distinct item types).
  • Controlled CODEX_HOME isolation, no MCP/plugins/apps, sandbox/approval discipline: unchanged by the bridge.
  • Pending continuation state is constructible within the frozen bounds: bound to authenticated inference client + exact execution target, count/bytes/TTL bounded, explicit cancellation/disconnect/worker-loss/restart handling (worker-local process death = typed failure; the harness retries with its own history; no cross-client leakage; no fabricated results — a pending call is answered ONLY with the client's result or never).
  • No matrix weakening: cells move only to the level actually established.

4. What remains unverified (honest gaps)

Live turn-level behavior — an actual pause on item/tool/call, same-turn resume timing, interrupt-while-pending, sequential multi-tool rounds — requires a signed-in subscription home and consumes quota; it is the same EXTERNAL_ACCEPTANCE_GATE: LIVE_CODEX_SUBSCRIPTION class as the existing M10 gate and stays open until that acceptance. Everything above is mechanism-level evidence (probe + official doc + binary-pinned schema), not a live round trip.

5. Consequent evidence levels (for the future amendment, not changed now)

  • tool_calls: UNSUPPORTED (stable surface) → PARTIAL (conditional) — experimentalApi-gated dynamic tools; thread-scoped declarations (mid-round-trip tool-set changes rejected); live pause/resume pending the M10-class gate.
  • tool_results: UNSUPPORTED → PARTIAL (conditional) under the same conditions (client result returned via the item/tool/call response).
  • Child-A tools pin flips only to that evidenced level, and only by the amendment's implementation.
## Tool-bridge investigation report — CURRENT runtime re-evidence (2026-09-28, UTC) **Verdict: TOOL_BRIDGE_VIABLE** (conditional — constraints and unverified halves recorded below; implementation remains gated on a separate frozen-scope amendment of this issue before any coding, per the frozen scope). Investigation only; no repository code, matrix or evidence cells were changed by this investigation. All probes were bounded, structure-only, on a fresh controlled `CODEX_HOME` under `/tmp` (never the user home), with synthetic strings only: no account/auth method was sent, no turn was executed, no inference was possible (the probe home is unsigned-in and turns fail closed at upstream 401), and no quota was consumed. Output was sanitized to structure before recording. ### 1. Current runtime established (do-not-assume-2026-09-20 satisfied) - Binary: `codex-cli 0.155.0-alpha.16.3` — U-001 VS Code extension discovery layout (`openai.chatgpt-26.917.62051-linux-x64`, `bin/linux-x86_64/codex-package.json` `layoutVersion: 1`, `variant: "codex"`, static musl ELF). This is NEWER lineage than both the 2026-09-20 probe (`0.154.0-alpha.6.2`) and the pinned `rust-v0.155.1` schema tag. - Version-pinned schema generated FROM THIS BINARY (`codex app-server generate-json-schema`): 39 top-level + 271 v2 files. The generated bundle reflects the STABLE surface: experimental-gated fields are omitted (proved below), so schema absence alone is not evidence of removal. ### 2. Findings against the frozen investigation questions **F1 — The client-tool mechanism exists on the current runtime and is `experimentalApi`-gated (three evidence classes agree).** - Local probe: `thread/start` with `dynamicTools` WITHOUT the capability → JSON-RPC error `-32600`, message `thread/start.dynamicTools requires experimentalApi capability` (field RECOGNIZED and gated — not an unknown field; the same probe shows genuinely unknown fields are silently ignored, i.e. parsing is lenient, so this error is positive identification). WITH `capabilities.experimentalApi: true` → `thread/start` accepted. - Official doc (fetched 2026-09-28; developers.openai.com/codex/app-server now redirects to learn.chatgpt.com/docs/app-server): "Dynamic tool calls (experimental)" section — `dynamicTools` on `thread/start` requires `capabilities.experimentalApi = true`; per-invocation flow: `item/started` (`dynamicToolCall`, status `inProgress`) → `item/tool/call` server request → client response with content items → `item/completed` with final status/`contentItems`/`success`. It also states Codex persists dynamic tools in thread rollout metadata and restores them on `thread/resume`. - Binary-generated schema: `ServerRequest` carries `item/tool/call` (`DynamicToolCallParams`: `threadId`, `turnId`, `callId`, `tool`, `namespace?`, `arguments`); `DynamicToolCallResponse`: `{success: boolean, contentItems: [inputText | inputImage | inputAudio]}`; `dynamicToolCall` ThreadItem: `{id, type, tool, namespace?, arguments, status (inProgress|completed|failed), success, contentItems, durationMs}`. **F2 — Constraint: declarations are THREAD-scoped, not per-turn.** `TurnStartParams` has no `dynamicTools` member (live probe: `turn/start` with `dynamicTools` silently ignores it — consistent with the lenient parse control; the stable schema omits it too). Exact per-turn tool sets are therefore expressible only when the tool set is stable for the life of the thread; a client that changes its `tools[]` between the declaration and the continuation cannot be honored exactly on the open thread → must be a typed fail-closed rejection, never a silent narrowing. **F3 — Client-owned vs backend-native tools stay cleanly separated.** `dynamicToolCall` is a distinct ThreadItem type alongside `commandExecution`, `fileChange`, `mcpToolCall`, `collabAgentToolCall`. Nothing in the mechanism executes a client tool in Codex or in the worker: the adapter would RELAY `item/tool/call` to the harness and return the harness's result. `toolOutput` on `turn/start` (stable) is now `{name, namespace?, output}` — name-keyed, no `callId`. **F4 — Implementation families.** Family A (stateful suspended continuation) is the protocol-native design: the pause IS the pending `item/tool/call` JSON-RPC request on the live stdio connection; the continuation IS answering it (`callId`-correlated) so the SAME turn/thread completes; docs confirm tools persist and are restored on `thread/resume`. Family B (stateless reconstruction) has two UNVERIFIED hinges: (a) `turn/interrupt` behavior while a server request is pending, and (b) `thread/inject_items` fidelity for assistant function-call history (the 0.154-era adapter already rejected assistant-side tool history replay). Family B is NOT proved semantically equivalent, so under the frozen rule ("use the simpler design only if semantic equivalence is proved") family A is the only selectable design on this evidence. **F5 — Feature-flag surface.** `experimentalFeature/list` (live, 147 entries) exposes NO dynamic-tools flag: gating is purely the `experimentalApi` capability. The D-019/U-011 generation-aware containment (parse per probed generation, safe disable on contract change) remains the real defense. **F6 — Cancel/interrupt.** `turn/interrupt` → `TurnStatus: interrupted` is documented and schema-present. Its interaction with a PENDING `item/tool/call` is UNVERIFIED live (needs a signed-in turn). Safe resolution regardless of behavior: the worker owns the app-server process lifecycle (bounded terminate/kill/reap, already implemented) — process teardown resolves any ambiguous pending state without fabricating a tool result. ### 3. Hard-invariant check (all preservable) - Scarcity Router / worker never execute client-owned tools (relay-only; F3). - Codex-internal tools never presented as client tool calls (distinct item types). - Controlled `CODEX_HOME` isolation, no MCP/plugins/apps, sandbox/approval discipline: unchanged by the bridge. - Pending continuation state is constructible within the frozen bounds: bound to authenticated inference client + exact execution target, count/bytes/TTL bounded, explicit cancellation/disconnect/worker-loss/restart handling (worker-local process death = typed failure; the harness retries with its own history; no cross-client leakage; no fabricated results — a pending call is answered ONLY with the client's result or never). - No matrix weakening: cells move only to the level actually established. ### 4. What remains unverified (honest gaps) Live turn-level behavior — an actual pause on `item/tool/call`, same-turn resume timing, interrupt-while-pending, sequential multi-tool rounds — requires a signed-in subscription home and consumes quota; it is the same `EXTERNAL_ACCEPTANCE_GATE: LIVE_CODEX_SUBSCRIPTION` class as the existing M10 gate and stays open until that acceptance. Everything above is mechanism-level evidence (probe + official doc + binary-pinned schema), not a live round trip. ### 5. Consequent evidence levels (for the future amendment, not changed now) - `tool_calls`: `UNSUPPORTED (stable surface)` → `PARTIAL (conditional)` — experimentalApi-gated dynamic tools; thread-scoped declarations (mid-round-trip tool-set changes rejected); live pause/resume pending the M10-class gate. - `tool_results`: `UNSUPPORTED` → `PARTIAL (conditional)` under the same conditions (client result returned via the `item/tool/call` response). - Child-A tools pin flips only to that evidenced level, and only by the amendment's implementation.
kilo commented 2026-09-28 17:57:37 +00:00
Author
Member
Copy link

D-060 implementation note — Family A landed on gateway/codex-client-tools (READY_FOR_REVIEW pending reviews)

TOOL_BRIDGE_VIABLE accepted; the Family A suspended-turn continuation is implemented. PR opens against develop (base 6fec46c, unchanged at commit time — re-verified immediately before push). Frozen scope text above is unchanged; this note is the additive implementation record.

Implementation summary

  • Worker protocol v3 (closed, version-gated): exactly two new attempt-scoped messages, execute_tool_call (worker→server) and execute_tool_result (server→worker). Server accepts (3,2,1). v1/v2 sessions carrying v3 vocabulary = fatal; a v1/v2 worker is NEVER tool-continuation-capable; ordinary non-tool execution on old workers unchanged.
  • Family A round trip: one App Server process/thread/turn; item/tool/call is HELD (never refused inline) and relayed; the harness's ordinary role:"tool" request resumes the SAME turn. Reconstruction/replay/rerouting/restart do not exist; pre-delivery loss = 404 continuation_not_found, post-delivery loss = honest ambiguous_execution_state.
  • Continuation identity: externally visible tool_call_id = server-issued srct- + 128 random bits; Codex callId/thread/process ids never leave the worker. Exactly-once, single-subscriber claims; replay/double-delivery = 409 continuation_already_resolved (client-scoped bounded tombstone ring); foreign client = indistinguishable 404; any drift (model, effort, tools fingerprint, tool_choice, conversation-prefix fingerprint, assistant echo, extra tool messages, >4 MiB result) = 400 continuation_mismatch. Only bounded digests stored — no prompts, arguments or results.
  • One absolute lifetime: the ORIGINAL attempt's admission deadline (execution_time_limit_seconds); the worker's existing deadline timer + the gateway reaper tick cancel the suspended turn at expiry; no per-leg reset, no new client-expandable policy.
  • Narrow experimental scope: the stable surface keeps initialize with empty capabilities byte-for-byte (pinned by trace); ONLY tool-bearing requests opt into experimentalApi: true and declare dynamicTools verbatim (no renaming/truncation/weakening; bounded). tool_choice modes the surface cannot enforce (required/named/none) refuse before execution; parallel_tool_calls keeps its existing refuse-not-drop treatment; sequential rounds bounded (32/turn), one pending call at a time.
  • success mapping (recorded, D-060 pt 6): a valid role:"tool" message maps to {"success": true, "contentItems":[{"type":"inputText","text":<content>}]} — text-only v1; failure signalling stays the harness's content-level concern; nothing inferred from text, nothing fabricated.
  • Audit: suspension leg = completed + usage-free unknown call observation + suspended_for_client_tool; terminal leg = completed + the turn's SINGLE usage-bearing observation + continuation_resumed, repeating the original decision/target identity. One provider call is never reported as two.
  • Sticky vs revocable: the resume leg performs no routing/capacity/policy I/O (test-pinned by breaking every admission input — the continuation still resolves); D-059 campaigns/scarcity/quota cannot move a suspended turn. Worker/source revocation, session loss, expiry and gateway restart fail closed (in-memory state; a lost suspension says so).

Evidence/capability changes (conservative): tool_calls/tool_results UNSUPPORTED → PARTIAL (dated 2026-09-28: mechanism evidence + deterministic round trip; the live signed-in acceptance stays behind the LIVE_CODEX_SUBSCRIPTION-class gate). CODEX_SURFACE_CAPABILITIES.tool_calls → true. Tool eligibility additionally requires the live worker session to negotiate v3.

Gates (full run on the exact head 97441a6): complete test suite 2303 tests OK (incl. the new full-stack Family-A acceptance: non-streaming + streaming + negative matrix; adapter/endpoint/protocol/registry/coordinator unit suites); basedpyright 0 errors / 0 warnings / 0 notes; guardrails OK; git diff --check clean; single commit over origin/develop, no merges. #143 note: one load-sensitive test_scenario_11 cancellation flake fired once in a full run (pre-existing, documented in #143; the test passes on rerun and exercises no continuation code — not folded into this PR).

Reviews: independent exact-head review + the mandatory gpt-daybreak-blue-latest security review requested on the frozen head. If Daybreak access is unavailable: SECURITY_REVIEW_BLOCKED, gate stays open at the human decision point.

DEFERs: (1) live signed-in acceptance (#141-class gate) — PARTIAL, never PASS until then; (2) required/named tool_choice support would need an evidenced upstream enforcement surface — refused today; (3) dispatch-time (not pre-ranking) v3-worker eligibility — a v3-less selected worker fails closed with a typed backend failure; pre-ranking awareness would need live worker-protocol facts in the matrix (#106 territory); (4) continuation_ttl_seconds as a separate administrator knob — the admission deadline bounds the turn today; revisiting is cheap if a real workload needs a longer tool budget than its inference budget.

## D-060 implementation note — Family A landed on `gateway/codex-client-tools` (READY_FOR_REVIEW pending reviews) TOOL_BRIDGE_VIABLE accepted; the Family A suspended-turn continuation is implemented. PR opens against `develop` (base `6fec46c`, unchanged at commit time — re-verified immediately before push). Frozen scope text above is unchanged; this note is the additive implementation record. **Implementation summary** - **Worker protocol v3** (closed, version-gated): exactly two new attempt-scoped messages, `execute_tool_call` (worker→server) and `execute_tool_result` (server→worker). Server accepts (3,2,1). v1/v2 sessions carrying v3 vocabulary = fatal; a v1/v2 worker is NEVER tool-continuation-capable; ordinary non-tool execution on old workers unchanged. - **Family A round trip:** one App Server process/thread/turn; `item/tool/call` is HELD (never refused inline) and relayed; the harness's ordinary `role:"tool"` request resumes the SAME turn. Reconstruction/replay/rerouting/restart do not exist; pre-delivery loss = `404 continuation_not_found`, post-delivery loss = honest `ambiguous_execution_state`. - **Continuation identity:** externally visible `tool_call_id` = server-issued `srct-` + 128 random bits; Codex `callId`/thread/process ids never leave the worker. Exactly-once, single-subscriber claims; replay/double-delivery = `409 continuation_already_resolved` (client-scoped bounded tombstone ring); foreign client = indistinguishable `404`; any drift (model, effort, tools fingerprint, tool_choice, conversation-prefix fingerprint, assistant echo, extra tool messages, >4 MiB result) = `400 continuation_mismatch`. Only bounded digests stored — no prompts, arguments or results. - **One absolute lifetime:** the ORIGINAL attempt's admission deadline (`execution_time_limit_seconds`); the worker's existing deadline timer + the gateway reaper tick cancel the suspended turn at expiry; no per-leg reset, no new client-expandable policy. - **Narrow experimental scope:** the stable surface keeps `initialize` with empty capabilities byte-for-byte (pinned by trace); ONLY tool-bearing requests opt into `experimentalApi: true` and declare `dynamicTools` verbatim (no renaming/truncation/weakening; bounded). `tool_choice` modes the surface cannot enforce (`required`/named/`none`) refuse before execution; `parallel_tool_calls` keeps its existing refuse-not-drop treatment; sequential rounds bounded (32/turn), one pending call at a time. - **`success` mapping (recorded, D-060 pt 6):** a valid `role:"tool"` message maps to `{"success": true, "contentItems":[{"type":"inputText","text":<content>}]}` — text-only v1; failure signalling stays the harness's content-level concern; nothing inferred from text, nothing fabricated. - **Audit:** suspension leg = `completed` + usage-free `unknown` call observation + `suspended_for_client_tool`; terminal leg = `completed` + the turn's SINGLE usage-bearing observation + `continuation_resumed`, repeating the original decision/target identity. One provider call is never reported as two. - **Sticky vs revocable:** the resume leg performs no routing/capacity/policy I/O (test-pinned by breaking every admission input — the continuation still resolves); D-059 campaigns/scarcity/quota cannot move a suspended turn. Worker/source revocation, session loss, expiry and gateway restart fail closed (in-memory state; a lost suspension says so). **Evidence/capability changes (conservative):** `tool_calls`/`tool_results` UNSUPPORTED → **PARTIAL** (dated 2026-09-28: mechanism evidence + deterministic round trip; the live signed-in acceptance stays behind the `LIVE_CODEX_SUBSCRIPTION`-class gate). `CODEX_SURFACE_CAPABILITIES.tool_calls` → true. Tool eligibility additionally requires the live worker session to negotiate v3. **Gates (full run on the exact head `97441a6`):** complete test suite **2303 tests OK** (incl. the new full-stack Family-A acceptance: non-streaming + streaming + negative matrix; adapter/endpoint/protocol/registry/coordinator unit suites); `basedpyright` **0 errors / 0 warnings / 0 notes**; guardrails OK; `git diff --check` clean; single commit over `origin/develop`, no merges. **#143 note:** one load-sensitive `test_scenario_11` cancellation flake fired once in a full run (pre-existing, documented in #143; the test passes on rerun and exercises no continuation code — not folded into this PR). **Reviews:** independent exact-head review + the mandatory `gpt-daybreak-blue-latest` security review requested on the frozen head. If Daybreak access is unavailable: `SECURITY_REVIEW_BLOCKED`, gate stays open at the human decision point. **DEFERs:** (1) live signed-in acceptance (#141-class gate) — PARTIAL, never PASS until then; (2) `required`/named `tool_choice` support would need an evidenced upstream enforcement surface — refused today; (3) dispatch-time (not pre-ranking) v3-worker eligibility — a v3-less selected worker fails closed with a typed backend failure; pre-ranking awareness would need live worker-protocol facts in the matrix (#106 territory); (4) `continuation_ttl_seconds` as a separate administrator knob — the admission deadline bounds the turn today; revisiting is cheap if a real workload needs a longer tool budget than its inference budget.
kilo commented 2026-09-28 18:36:48 +00:00
Author
Member
Copy link

Review round 1 remediation — new exact head 8b02bce

The independent exact-head review returned CHANGES_REQUESTED with two evidenced MERGE_BLOCKERs (both reproduced by the reviewer against the repo's own fixtures). Both accepted and fixed in one remediation commit, with regression pins:

  1. Tombstone poisoning (blocker). Continuation detection is now shape-aware: only a request whose FINAL message is a role:"tool" result can trigger continuation/tombstone/srct--namespace logic. A conforming full-history harness's follow-up turns (which necessarily still carry the consumed tool result mid-history) route normally — a completed round can no longer 409-poison the conversation. Pinned in DetectionShapeTests.test_follow_up_turn_after_a_round_is_not_a_replay.
  2. Dangling srct- result silently re-routed (blocker). Within the delivery shape, a gateway-issued token with no live record and no tombstone (gateway restart, registry bound) now yields the typed 404 continuation_not_found — never a fresh model selection. Additionally, when registration hits the pending bound, the already-suspended worker turn is cancelled immediately instead of orphaned until its deadline. Pinned in test_delivering_result_to_a_lost_suspension_is_not_found.
  3. DEFER 3 (fixed): a sequential tool round closes the superseded token as terminal — precise 409 on very-late replays and the registry slot is freed as the round advances. Pinned.
  4. DEFER 4 (fixed): a failed reaper tick is skipped, not fatal (the previous early-return could silently disable worker liveness reaping AND continuation expiry for the process lifetime).

Remaining review DEFERs (left recorded, not blocking): (5) the continuation leg skips the input-context admission pre-check (bounded by the fingerprint-matched prefix + 4 MiB result cap + HTTP body limit; administrator global body/context ceilings still apply at the HTTP layer); (6) thread/start protocol errors on tool turns all map to dynamic_tools_runtime_refused (misleading-reason-only, error path); (7) a close() race can append an owner-less tombstone entry (honest 404 instead of 409 in that narrow race).

Gates on 8b02bce: full suite 2307 tests OK; basedpyright 0/0/0; git diff --check clean; origin/develop..HEAD = 2 commits (feature + one remediation), no merges. The reviewer's narrow final verification on the new exact head is requested. Daybreak security review: SECURITY_REVIEW_BLOCKED — gpt-daybreak-blue-latest is not available in this environment; per policy no substitute model closes that gate, so the PR stays paused at the human decision point. Live acceptance: LIVE_TOOL_BRIDGE_NOT_VERIFIED (cells stay PARTIAL; note: a signed-in eligible Codex source exists, but exercising it would require either copying credentials or injecting into the production worker — both forbidden — so the owner-run acceptance remains open).

## Review round 1 remediation — new exact head `8b02bce` The independent exact-head review returned **CHANGES_REQUESTED** with two evidenced MERGE_BLOCKERs (both reproduced by the reviewer against the repo's own fixtures). Both accepted and fixed in one remediation commit, with regression pins: 1. **Tombstone poisoning (blocker).** Continuation detection is now **shape-aware**: only a request whose FINAL message is a `role:"tool"` result can trigger continuation/tombstone/`srct-`-namespace logic. A conforming full-history harness's follow-up turns (which necessarily still carry the consumed tool result mid-history) route normally — a completed round can no longer 409-poison the conversation. Pinned in `DetectionShapeTests.test_follow_up_turn_after_a_round_is_not_a_replay`. 2. **Dangling `srct-` result silently re-routed (blocker).** Within the delivery shape, a gateway-issued token with no live record and no tombstone (gateway restart, registry bound) now yields the typed `404 continuation_not_found` — never a fresh model selection. Additionally, when registration hits the pending bound, the already-suspended worker turn is **cancelled immediately** instead of orphaned until its deadline. Pinned in `test_delivering_result_to_a_lost_suspension_is_not_found`. 3. **DEFER 3 (fixed):** a sequential tool round closes the superseded token as terminal — precise `409` on very-late replays and the registry slot is freed as the round advances. Pinned. 4. **DEFER 4 (fixed):** a failed reaper tick is skipped, not fatal (the previous early-return could silently disable worker liveness reaping AND continuation expiry for the process lifetime). **Remaining review DEFERs (left recorded, not blocking):** (5) the continuation leg skips the input-context admission pre-check (bounded by the fingerprint-matched prefix + 4 MiB result cap + HTTP body limit; administrator global body/context ceilings still apply at the HTTP layer); (6) `thread/start` protocol errors on tool turns all map to `dynamic_tools_runtime_refused` (misleading-reason-only, error path); (7) a `close()` race can append an owner-less tombstone entry (honest 404 instead of 409 in that narrow race). **Gates on `8b02bce`:** full suite **2307 tests OK**; `basedpyright` **0/0/0**; `git diff --check` clean; `origin/develop..HEAD` = 2 commits (feature + one remediation), no merges. The reviewer's narrow final verification on the new exact head is requested. **Daybreak security review: `SECURITY_REVIEW_BLOCKED`** — `gpt-daybreak-blue-latest` is not available in this environment; per policy no substitute model closes that gate, so the PR stays paused at the human decision point. **Live acceptance: `LIVE_TOOL_BRIDGE_NOT_VERIFIED`** (cells stay PARTIAL; note: a signed-in eligible Codex source exists, but exercising it would require either copying credentials or injecting into the production worker — both forbidden — so the owner-run acceptance remains open).
kilo commented 2026-09-28 18:48:46 +00:00
Author
Member
Copy link

Final status — head 71d0c79 (decision renumbered D-060 → D-062)

  • Renumbering: D-060 is reserved by open issue #156 and D-061 by open PR #157 while this branch was in flight; the decision is now D-062 (canonical develop carried neither number; whole-branch textual substitution, rationale recorded in the decisions.md entry). Earlier comments above that say "D-060" refer to this same decision.
  • Independent review: round 1 CHANGES_REQUESTED (2 MERGE_BLOCKERs, both reviewer-reproduced) → remediated → narrow final verification on 8b02bce: VERDICT: APPROVED, UX impact: none (all reproductions empirically confirmed fixed; exactly-once preserved; no new issues in the remediation diff; one non-blocking note: the registration-bound cancellation path was verified manually but has no committed test pin — added to the DEFER list).
  • Gates on 71d0c79: full suite 2307 tests OK; basedpyright 0/0/0; guardrails OK; git diff --check clean; origin/develop..HEAD = 3 commits (feature, one remediation, renumbering), no merges.
  • Daybreak security review: SECURITY_REVIEW_BLOCKED — unavailable in this environment; per policy the gate stays open at the human decision point; no substitute model closed it.
  • Live acceptance: LIVE_TOOL_BRIDGE_NOT_VERIFIED (cells stay PARTIAL; owner-run acceptance open).
  • PR #159 remains open, unmerged, for the human merge gate.
## Final status — head `71d0c79` (decision renumbered D-060 → **D-062**) - **Renumbering:** D-060 is reserved by open issue #156 and D-061 by open PR #157 while this branch was in flight; the decision is now **D-062** (canonical `develop` carried neither number; whole-branch textual substitution, rationale recorded in the decisions.md entry). Earlier comments above that say "D-060" refer to this same decision. - **Independent review:** round 1 CHANGES_REQUESTED (2 MERGE_BLOCKERs, both reviewer-reproduced) → remediated → **narrow final verification on `8b02bce`: VERDICT: APPROVED, `UX impact: none`** (all reproductions empirically confirmed fixed; exactly-once preserved; no new issues in the remediation diff; one non-blocking note: the registration-bound cancellation path was verified manually but has no committed test pin — added to the DEFER list). - **Gates on `71d0c79`:** full suite **2307 tests OK**; `basedpyright` **0/0/0**; guardrails OK; `git diff --check` clean; `origin/develop..HEAD` = 3 commits (feature, one remediation, renumbering), no merges. - **Daybreak security review: `SECURITY_REVIEW_BLOCKED`** — unavailable in this environment; per policy the gate stays open at the human decision point; no substitute model closed it. - **Live acceptance: `LIVE_TOOL_BRIDGE_NOT_VERIFIED`** (cells stay PARTIAL; owner-run acceptance open). - PR #159 remains open, unmerged, for the human merge gate.
kilo commented 2026-09-28 20:45:16 +00:00
Author
Member
Copy link

Review round 2 — remediation landed; one finding requires an OWNER DECISION (packet below)

All six findings were accepted. Five are fully remediated with regression pins; finding 1 hit the frozen STOP rule and is resolved by stopping the semantic sub-part — the round trip deliberately does not complete on this branch until you decide the packet below.

Evidence established for finding 1 (verified first-hand)

  • Upstream at the exact pin openai/codex @ 36650394, codex-rs/protocol/src/protocol.rs: /// Whether the tool call succeeded. / pub success: bool — a REQUIRED plain bool, no #[serde(default)], not optional. No "unknown" encoding exists.
  • The generic Chat Completions contract on this surface: role: "tool" messages are text-only with a closed field set (unknown fields rejected at ingress) — no success/failure fact.
  • The representative ZCode capture (representative_client_fixture.py) contains no continuation leg at all — no signal there either.

Conclusion: no faithful success/failure signal exists. Per the STOP rule, the previous success: true mapping and its D-062 rationale are RETRACTED (D-062 pt 6 rewritten; the earlier text is not stated as upstream fact anywhere).

Remediations (head will be frozen after gates; regressions in tests/test_gateway_continuation_coordinator.py and the adapter/e2e suites)

  1. F1 (STOP, implemented): the adapter REFUSES to fabricate the answer — a delivered result fails the attempt typed (tool_result_success_unresolved), the held item/tool/call is never answered, tool_results cell returns to UNSUPPORTED (dated), tool_calls stays PARTIAL (declarations/suspension/relay only), and the e2e pins the STOP (no tool_call_answer event ever traced).
  2. F2: PendingContinuation retains the assistant echo as a bounded DIGEST (assistant_tool_calls_digest); raw arguments/result/prompt content are gone from registry state; retention test inspects a live record for the secret markers.
  3. F3: a live per-resource availability fact (worker_continuation_unavailable) gates tool-bearing requests BEFORE ranking on the owning worker's actual protocol-v3 negotiation — regression: two identical worker routes, only B v3 ⇒ B selected and dispatched, A never chosen; empty/absent fact ⇒ 503 fail-closed; non-tool requests still use the v2 worker; the D-043 matrix key is untouched (the fact rides the availability stage via a RouteRequest live-input member, wired from the worker endpoint's dispatch-authorization path).
  4. F4: the invariant "observable ⇒ registered" now holds by construction: the adapter invokes a coordinator registration closure INSIDE the dispatch, before the streamed tool_call frame or the response is produced. A bound failure cancels the suspended backend and fails typed with NO token exposed (non-streaming and streaming regressions; ordering pinned).
  5. F5: every post-claim exit (permanent/ambiguous/timeout/lost/cancelled/failed/unexpected/disconnect) now closes the record terminal — pending_count() returns to 0 immediately on each path (tested per-case); expire_due on a RESUMING record also cancels + tombstones instead of silently dropping.
  6. F6: before delivering a result, the client's CURRENT grant is re-evaluated against the EXACT original target through the SAME M02 authorization stage admission uses (providers/channels/entitlements/blocked-resources/spend). Failure ⇒ typed 403 unauthorized_target, the record closes cancelled, and the suspended backend turn is interrupted — never rerouted. Regression: authorized → suspend → administrator blocks the resource → same client's result is NOT delivered. D-059/scarcity remain non-authoritative for live turns.
  7. Cleanups: the control-server banner now prints the real negotiated protocol version (was hardcoded "v1"); the MAX_FRAME_BYTES comment no longer claims the stale 1 MiB body default and explicitly disclaims headroom over the D-058 16 MiB body limit.

OWNER DECISION PACKET — the upstream success semantic (#137 gate)

The bridge cannot complete a round trip until you choose how success is answered. Options:

  • (a) Accept a documented compatibility rule: answer success: true for every well-formed delivered result, recorded as an EXPLICIT owner decision that consciously diverges from the upstream "tool call succeeded" semantics (rationale available: Chat Completions pipelines treat failure-signalling as content-level; the harness chose to return rather than error). Cells then return to PARTIAL and the bridge ships as-is.
  • (b) Wait for a faithful surface: keep the round trip stopped (current state) until execution surface v2 adds an optional per-result status member (a versioned contract change with its own decision), or until upstream offers an "unknown" semantic.
  • (c) Hybrid: accept (a) now with an audit reason code marking every auto-true answer, preserving the ability to migrate to (b) later.

No code change is required to choose (b) — it is the shipped state. Choosing (a)/(c) is a one-line mapping + evidence-cell flip with your provenance.

Status: all repository gates re-run on the remediation head before the freeze; then a fresh independent normal review on the exact head. Daybreak remains SECURITY_REVIEW_BLOCKED (unavailable in this environment); live acceptance remains LIVE_TOOL_BRIDGE_NOT_VERIFIED; tool_calls/tool_results remain PARTIAL/UNSUPPORTED respectively until (a)/(c) is decided and #141's layered acceptance runs.

## Review round 2 — remediation landed; one finding requires an OWNER DECISION (packet below) All six findings were accepted. Five are fully remediated with regression pins; finding 1 hit the frozen STOP rule and is resolved by **stopping the semantic sub-part** — the round trip deliberately does not complete on this branch until you decide the packet below. ### Evidence established for finding 1 (verified first-hand) - Upstream at the exact pin `openai/codex @ 36650394`, `codex-rs/protocol/src/protocol.rs`: `/// Whether the tool call succeeded.` / `pub success: bool` — a REQUIRED plain bool, no `#[serde(default)]`, not optional. No "unknown" encoding exists. - The generic Chat Completions contract on this surface: `role: "tool"` messages are text-only with a closed field set (unknown fields rejected at ingress) — no success/failure fact. - The representative ZCode capture (`representative_client_fixture.py`) contains no continuation leg at all — no signal there either. **Conclusion: no faithful success/failure signal exists.** Per the STOP rule, the previous `success: true` mapping and its D-062 rationale are RETRACTED (D-062 pt 6 rewritten; the earlier text is not stated as upstream fact anywhere). ### Remediations (head will be frozen after gates; regressions in `tests/test_gateway_continuation_coordinator.py` and the adapter/e2e suites) 1. **F1 (STOP, implemented):** the adapter REFUSES to fabricate the answer — a delivered result fails the attempt typed (`tool_result_success_unresolved`), the held `item/tool/call` is never answered, `tool_results` cell returns to UNSUPPORTED (dated), `tool_calls` stays PARTIAL (declarations/suspension/relay only), and the e2e pins the STOP (no `tool_call_answer` event ever traced). 2. **F2:** `PendingContinuation` retains the assistant echo as a bounded DIGEST (`assistant_tool_calls_digest`); raw arguments/result/prompt content are gone from registry state; retention test inspects a live record for the secret markers. 3. **F3:** a live per-resource availability fact (`worker_continuation_unavailable`) gates tool-bearing requests BEFORE ranking on the owning worker's actual protocol-v3 negotiation — regression: two identical worker routes, only B v3 ⇒ **B selected and dispatched**, A never chosen; empty/absent fact ⇒ 503 fail-closed; non-tool requests still use the v2 worker; the D-043 matrix key is untouched (the fact rides the availability stage via a `RouteRequest` live-input member, wired from the worker endpoint's dispatch-authorization path). 4. **F4:** the invariant "observable ⇒ registered" now holds by construction: the adapter invokes a coordinator registration closure INSIDE the dispatch, before the streamed `tool_call` frame or the response is produced. A bound failure cancels the suspended backend and fails typed with NO token exposed (non-streaming and streaming regressions; ordering pinned). 5. **F5:** every post-claim exit (permanent/ambiguous/timeout/lost/cancelled/failed/unexpected/disconnect) now closes the record terminal — `pending_count()` returns to 0 immediately on each path (tested per-case); `expire_due` on a RESUMING record also cancels + tombstones instead of silently dropping. 6. **F6:** before delivering a result, the client's CURRENT grant is re-evaluated against the EXACT original target through the SAME M02 authorization stage admission uses (providers/channels/entitlements/blocked-resources/spend). Failure ⇒ typed `403 unauthorized_target`, the record closes cancelled, and the suspended backend turn is interrupted — never rerouted. Regression: authorized → suspend → administrator blocks the resource → same client's result is NOT delivered. D-059/scarcity remain non-authoritative for live turns. 7. **Cleanups:** the control-server banner now prints the real negotiated protocol version (was hardcoded "v1"); the `MAX_FRAME_BYTES` comment no longer claims the stale 1 MiB body default and explicitly disclaims headroom over the D-058 16 MiB body limit. ### OWNER DECISION PACKET — the upstream `success` semantic (#137 gate) The bridge cannot complete a round trip until you choose how `success` is answered. Options: - **(a) Accept a documented compatibility rule:** answer `success: true` for every well-formed delivered result, recorded as an EXPLICIT owner decision that consciously diverges from the upstream "tool call succeeded" semantics (rationale available: Chat Completions pipelines treat failure-signalling as content-level; the harness chose to return rather than error). Cells then return to PARTIAL and the bridge ships as-is. - **(b) Wait for a faithful surface:** keep the round trip stopped (current state) until execution surface v2 adds an optional per-result status member (a versioned contract change with its own decision), or until upstream offers an "unknown" semantic. - **(c) Hybrid:** accept (a) now with an audit reason code marking every auto-true answer, preserving the ability to migrate to (b) later. No code change is required to choose (b) — it is the shipped state. Choosing (a)/(c) is a one-line mapping + evidence-cell flip with your provenance. **Status:** all repository gates re-run on the remediation head before the freeze; then a fresh independent normal review on the exact head. **Daybreak remains `SECURITY_REVIEW_BLOCKED`** (unavailable in this environment); live acceptance remains `LIVE_TOOL_BRIDGE_NOT_VERIFIED`; `tool_calls`/`tool_results` remain PARTIAL/UNSUPPORTED respectively until (a)/(c) is decided and #141's layered acceptance runs.
kilo commented 2026-09-28 21:37:01 +00:00
Author
Member
Copy link

Review round 2 — remediation pushed; branch rebased; frozen head babf3f9

All six findings accepted and resolved on the rebased branch (base is now 59538e9 — D-061/#157 merged while this branch was in flight; the rebase re-applied the D-060→D-062 renumber scoped to this decision's section only, so D-061's own reference to the D-060 reservation is preserved verbatim).

F1 — STOP implemented (no invented mapping). Upstream verified first-hand at 36650394 (protocol.rs: /// Whether the tool call succeeded. — required plain bool, no default). The generic text-only role:"tool" message and the representative capture carry no success fact. The adapter now REFUSES to fabricate the answer: a delivered result fails the attempt typed tool_result_success_unresolved, the held item/tool/call is never answered (e2e pins: zero tool_call_answer events traced), tool_results returns to UNSUPPORTED (dated), tool_calls stays PARTIAL (declarations/suspension/relay only), and D-062 pt 6 is rewritten — the earlier interpretation is retracted as an upstream-fact claim. Owner decision packet posted above (options a/b/c); option (b) is the shipped state, no code change needed.

F2 — digest-only state: PendingContinuation stores assistant_tool_calls_digest (bounded sha256); retention test inspects a live record and proves raw arguments/results/prompts are absent.

F3 — live v3 eligibility, pre-ranking: a new availability-stage fact (worker_continuation_unavailable) excludes worker_bridged candidates whose owning worker session has not negotiated v3, BEFORE ranking, driven by a RouteRequest live-input member wired from the worker endpoint's dispatch-authorization path. Regressions: two same-identity worker routes with only B v3 ⇒ B selected and executed (A excluded at availability); empty set or absent fact ⇒ 503 fail-closed; non-tool requests still use the v2 worker; the D-043 matrix key is untouched.

F4 — register-before-exposure: the adapter invokes a coordinator registration closure INSIDE the dispatch, before the streamed tool_call frame or the response body exists; the invariant "observable ⇒ registered" now holds for racing clients by construction. Bound failure cancels the suspended backend and returns a typed failure exposing no token (non-streaming + streaming regressions).

F5 — terminal cleanup on every exit: all post-claim paths (permanent, ambiguous, timeout, lost, cancelled result, failed result, unexpected, client disconnect) close the record terminal — pending_count() returns to 0 immediately on each path (per-case tests); expire_due on a RESUMING record also cancels + tombstones.

F6 — authority recheck: the client's CURRENT grant is evaluated against the EXACT original target through the SAME M02 authorization stage admission uses, before delivery. Revocation ⇒ typed 403 unauthorized_target, record closed cancelled, suspended backend interrupted; no scarcity/campaign rerun. Regression: authorized → suspend → administrator blocks the resource → same client's result NOT delivered.

Cleanups: control-server banner prints the real protocol version (was hardcoded "v1"); MAX_FRAME_BYTES comment no longer claims the stale 1 MiB default and disclaims headroom over the D-058 16 MiB body limit; _compatibility_failure now actually returns the sorted-duplicate-free codes its docstring always promised (two same-code failures previously produced a duplicate pair that TargetExclusion's grammar rejects).

Gates on babf3f9: full suite 2317 tests OK; basedpyright 0/0/0; guardrails OK; git diff --check clean; rebased onto origin/develop = 59538e9 (4 commits, no merges). Fresh independent normal review dispatched on this exact head. Daybreak remains SECURITY_REVIEW_BLOCKED (unavailable); live acceptance remains LIVE_TOOL_BRIDGE_NOT_VERIFIED.

## Review round 2 — remediation pushed; branch rebased; frozen head `babf3f9` All six findings accepted and resolved on the rebased branch (base is now `59538e9` — D-061/#157 merged while this branch was in flight; the rebase re-applied the D-060→D-062 renumber **scoped to this decision's section only**, so D-061's own reference to the D-060 reservation is preserved verbatim). **F1 — STOP implemented (no invented mapping).** Upstream verified first-hand at `36650394` (`protocol.rs`: `/// Whether the tool call succeeded.` — required plain bool, no default). The generic text-only `role:"tool"` message and the representative capture carry no success fact. The adapter now REFUSES to fabricate the answer: a delivered result fails the attempt typed `tool_result_success_unresolved`, the held `item/tool/call` is never answered (e2e pins: zero `tool_call_answer` events traced), `tool_results` returns to UNSUPPORTED (dated), `tool_calls` stays PARTIAL (declarations/suspension/relay only), and D-062 pt 6 is rewritten — the earlier interpretation is retracted as an upstream-fact claim. **Owner decision packet posted above** (options a/b/c); option (b) is the shipped state, no code change needed. **F2 — digest-only state:** `PendingContinuation` stores `assistant_tool_calls_digest` (bounded sha256); retention test inspects a live record and proves raw arguments/results/prompts are absent. **F3 — live v3 eligibility, pre-ranking:** a new availability-stage fact (`worker_continuation_unavailable`) excludes `worker_bridged` candidates whose owning worker session has not negotiated v3, BEFORE ranking, driven by a `RouteRequest` live-input member wired from the worker endpoint's dispatch-authorization path. Regressions: two same-identity worker routes with only B v3 ⇒ **B selected and executed** (A excluded at availability); empty set or absent fact ⇒ 503 fail-closed; non-tool requests still use the v2 worker; the D-043 matrix key is untouched. **F4 — register-before-exposure:** the adapter invokes a coordinator registration closure INSIDE the dispatch, before the streamed `tool_call` frame or the response body exists; the invariant "observable ⇒ registered" now holds for racing clients by construction. Bound failure cancels the suspended backend and returns a typed failure exposing no token (non-streaming + streaming regressions). **F5 — terminal cleanup on every exit:** all post-claim paths (permanent, ambiguous, timeout, lost, cancelled result, failed result, unexpected, client disconnect) close the record terminal — `pending_count()` returns to 0 immediately on each path (per-case tests); `expire_due` on a RESUMING record also cancels + tombstones. **F6 — authority recheck:** the client's CURRENT grant is evaluated against the EXACT original target through the SAME M02 authorization stage admission uses, before delivery. Revocation ⇒ typed `403 unauthorized_target`, record closed cancelled, suspended backend interrupted; no scarcity/campaign rerun. Regression: authorized → suspend → administrator blocks the resource → same client's result NOT delivered. **Cleanups:** control-server banner prints the real protocol version (was hardcoded "v1"); `MAX_FRAME_BYTES` comment no longer claims the stale 1 MiB default and disclaims headroom over the D-058 16 MiB body limit; `_compatibility_failure` now actually returns the sorted-duplicate-free codes its docstring always promised (two same-code failures previously produced a duplicate pair that TargetExclusion's grammar rejects). **Gates on `babf3f9`:** full suite **2317 tests OK**; `basedpyright` **0/0/0**; guardrails OK; `git diff --check` clean; rebased onto `origin/develop` = `59538e9` (4 commits, no merges). **Fresh independent normal review dispatched on this exact head.** Daybreak remains `SECURITY_REVIEW_BLOCKED` (unavailable); live acceptance remains `LIVE_TOOL_BRIDGE_NOT_VERIFIED`.
kilo commented 2026-09-28 22:37:45 +00:00
Author
Member
Copy link

Review round 3 — remediation pushed; new exact head e232e39

The fresh independent review of babf3f9 returned CHANGES_REQUESTED with one docs-only MERGE_BLOCKER and two defers (its summary of the implementation itself: "held up against every attack I threw at it" — state machine, exactly-once delivery, register-before-exposure, terminal cleanup, authority recheck, v3 gating, and the STOP all verified, including 27 custom adversarial probes).

MERGE BLOCKER (docs-only, fixed): three prose sites still claimed tool_results moved to PARTIAL while the authoritative cell table says UNSUPPORTED. Corrected: D-062 point 8 now states the split truthfully (tool_calls → PARTIAL, tool_results STAYS UNSUPPORTED per point 6), the evidence-module docstring matches its own table, and the stage1 addendum heading/intro record the SPLIT.

DEFER (fixed): a failed continuation admission now releases the adapter's local suspension handle on both legs — the bounded table can no longer starve under sustained registry-full conditions. Regression added: a v3-session bound failure asserts the local table is empty and the worker received the cancel.

DEFER (fixed): the duplicated D-062 numbering rationale folded into one bullet.

Gates on e232e39: full suite 2318 tests OK; basedpyright 0/0/0; guardrails OK; git diff --check clean; base origin/develop = 59538e9 (rebase re-applied the D-062 renumber scoped to this section, preserving D-061's reservation reference); 5 commits, no merges.

Standing gates: Daybreak security review SECURITY_REVIEW_BLOCKED (unavailable in this environment; no substitute may close it); live acceptance LIVE_TOOL_BRIDGE_NOT_VERIFIED; tool_results UNSUPPORTED / tool_calls PARTIAL pending the owner decision packet (options a/b/c above — option (b) "keep the STOP" is the shipped state and requires no code change).

## Review round 3 — remediation pushed; new exact head `e232e39` The fresh independent review of `babf3f9` returned **CHANGES_REQUESTED** with one docs-only MERGE_BLOCKER and two defers (its summary of the implementation itself: "held up against every attack I threw at it" — state machine, exactly-once delivery, register-before-exposure, terminal cleanup, authority recheck, v3 gating, and the STOP all verified, including 27 custom adversarial probes). **MERGE BLOCKER (docs-only, fixed):** three prose sites still claimed `tool_results` moved to PARTIAL while the authoritative cell table says UNSUPPORTED. Corrected: D-062 point 8 now states the split truthfully (`tool_calls` → PARTIAL, `tool_results` STAYS UNSUPPORTED per point 6), the evidence-module docstring matches its own table, and the stage1 addendum heading/intro record the SPLIT. **DEFER (fixed):** a failed continuation admission now releases the adapter's local suspension handle on both legs — the bounded table can no longer starve under sustained registry-full conditions. Regression added: a v3-session bound failure asserts the local table is empty and the worker received the cancel. **DEFER (fixed):** the duplicated D-062 numbering rationale folded into one bullet. **Gates on `e232e39`:** full suite **2318 tests OK**; `basedpyright` **0/0/0**; guardrails OK; `git diff --check` clean; base `origin/develop` = `59538e9` (rebase re-applied the D-062 renumber scoped to this section, preserving D-061's reservation reference); 5 commits, no merges. **Standing gates:** Daybreak security review `SECURITY_REVIEW_BLOCKED` (unavailable in this environment; no substitute may close it); live acceptance `LIVE_TOOL_BRIDGE_NOT_VERIFIED`; `tool_results` UNSUPPORTED / `tool_calls` PARTIAL pending the owner decision packet (options a/b/c above — option (b) "keep the STOP" is the shipped state and requires no code change).
kilo commented 2026-09-29 06:35:32 +00:00
Author
Member
Copy link

Owner decision implemented — exact head ffe6da2

The packet is RESOLVED per your option (a). The mapping ships as an owner-accepted LOSSY protocol translation recorded as an amendment to D-062 pt 6 (no new decision number; Reconciliation records the provenance and preserves the upstream fact "Whether the tool call succeeded" as Codex's native semantic).

Implementation: the answer-point STOP is removed; after ALL continuation validation (exact client, exact continuation, digest-validated echo, exactly-once claim, current hard-authority recheck, absolute deadline), the adapter answers the held item/tool/call with {success: true, contentItems: [{type: inputText, text: <verbatim role:tool content>}]}. Content is never inspected/parsed/reinterpreted; success: false is never inferred; no client-specific branch exists.

Evidence: tool_calls = PARTIAL, tool_results = PARTIAL (live acceptance outstanding; no PASS claim anywhere).

Stale-prose sweep (all listed terms searched across the final tree): tool_result_success_unresolved survives ONLY in D-062 pt 6 as the recorded historical supersession ("the interim STOP ... is superseded by this decision and removed"); every other STOP-era site (cells, module docstring, surface comment, execution-surface rows + section, addendum heading/intro/mapping/cell-state, pt 8, Confidence) now tells the one story: both cells PARTIAL, owner-approved compatibility mapping, live acceptance pending, no PASS. D-056's historical "UNSUPPORTED there" wording is left as history (its record is date-stamped; D-062 pt 8 documents the move).

Tests restored: the full deterministic round trip is pinned end to end for non-streaming AND streaming (tools → suspension → tool_calls response → role:"tool" → {success:true, contentItems:[inputText]} onto the ORIGINAL callId → SAME thread/turn continues → terminal answer; one thread/start + one turn/start across both legs; audit: usage carried once), plus the required verbatim error-text pin: "ERROR: lookup failed" is forwarded under the SAME mapping, success: true, never parsed.

Gates on ffe6da2: full suite 2320 tests OK; basedpyright 0/0/0; guardrails OK; git diff --check clean; base origin/develop = 59538e9 (unchanged — no rebase needed); 6 commits, no merges. Fresh independent normal exact-head review dispatched now. Daybreak status follows after that review: if gpt-daybreak-blue-latest remains unavailable, final status stays SECURITY_REVIEW_BLOCKED and the PR is not merged. Live acceptance remains LIVE_TOOL_BRIDGE_NOT_VERIFIED.

## Owner decision implemented — exact head `ffe6da2` The packet is RESOLVED per your option (a). The mapping ships as an **owner-accepted LOSSY protocol translation** recorded as an amendment to D-062 pt 6 (no new decision number; Reconciliation records the provenance and preserves the upstream fact "Whether the tool call succeeded" as Codex's native semantic). **Implementation:** the answer-point STOP is removed; after ALL continuation validation (exact client, exact continuation, digest-validated echo, exactly-once claim, current hard-authority recheck, absolute deadline), the adapter answers the held `item/tool/call` with `{success: true, contentItems: [{type: inputText, text: <verbatim role:tool content>}]}`. Content is never inspected/parsed/reinterpreted; `success: false` is never inferred; no client-specific branch exists. **Evidence:** `tool_calls` = PARTIAL, `tool_results` = PARTIAL (live acceptance outstanding; **no PASS claim anywhere**). **Stale-prose sweep (all listed terms searched across the final tree):** `tool_result_success_unresolved` survives ONLY in D-062 pt 6 as the recorded historical supersession ("the interim STOP ... is superseded by this decision and removed"); every other STOP-era site (cells, module docstring, surface comment, execution-surface rows + section, addendum heading/intro/mapping/cell-state, pt 8, Confidence) now tells the one story: both cells PARTIAL, owner-approved compatibility mapping, live acceptance pending, no PASS. D-056's historical "UNSUPPORTED there" wording is left as history (its record is date-stamped; D-062 pt 8 documents the move). **Tests restored:** the full deterministic round trip is pinned end to end for non-streaming AND streaming (tools → suspension → `tool_calls` response → `role:"tool"` → `{success:true, contentItems:[inputText]}` onto the ORIGINAL callId → SAME thread/turn continues → terminal answer; one thread/start + one turn/start across both legs; audit: usage carried once), plus the required **verbatim error-text pin**: `"ERROR: lookup failed"` is forwarded under the SAME mapping, `success: true`, never parsed. **Gates on `ffe6da2`:** full suite **2320 tests OK**; `basedpyright` **0/0/0**; guardrails OK; `git diff --check` clean; base `origin/develop` = `59538e9` (unchanged — no rebase needed); 6 commits, no merges. **Fresh independent normal exact-head review dispatched now.** Daybreak status follows after that review: if `gpt-daybreak-blue-latest` remains unavailable, final status stays **`SECURITY_REVIEW_BLOCKED`** and the PR is not merged. Live acceptance remains `LIVE_TOOL_BRIDGE_NOT_VERIFIED`.
kilo commented 2026-09-29 06:51:12 +00:00
Author
Member
Copy link

Round 4 review: APPROVED — owner decision implemented; exact head 42a4832

The fresh independent review of ffe6da2 (the owner-decision implementation) returned VERDICT: APPROVED, UX impact: none, zero MERGE_BLOCKERs. Verified by the reviewer: the mapping is emitted at exactly one site and only after the full validation chain (client equality → shape/digest echo → exactly-once claim → deadline → authority recheck); shape is exactly D-062 pt 6; content is forwarded verbatim with no inspection and no success: false inference; no client-specific branch; STOP supersession is clean (tool_result_success_unresolved survives only as the historical supersession note in pt 6); the documentation tells one consistent story (both cells PARTIAL, owner-approved lossy mapping, live gate pending, no PASS claim); all restored round-trip/replay/authority/mismatch suites green; basedpyright 0/0/0.

One DEFER fixed (42a4832): D-062's Boundary list now names routing_core.py (the live v3 availability gate, the continuation_capable_resource_ids request input, and the compatibility reason-code dedup). A second briefing note needs no repo change: the live gate is recorded in the repo under its own name LIVE_CODEX_SUBSCRIPTION (the LIVE_TOOL_BRIDGE_NOT_VERIFIED wording is the task-report label for the same outstanding gate).

Final state of this task:

  • Exact develop base: 59538e9. Exact head: 42a4832 (7 commits over develop, no merges, PR open and unmerged).
  • Owner-approved success compatibility rule: implemented, recorded as the D-062 pt 6 amendment (lossy translation; upstream fact preserved; content opaque; generic).
  • Evidence: tool_calls = PARTIAL, tool_results = PARTIAL. No PASS claim.
  • Deterministic E2E: full round trip pinned non-streaming AND streaming (usage carried once; one thread/start + one turn/start across both legs), verbatim error-text pin, plus the full negative matrix (replay 409, foreign 404, mismatch 400, authority 403, expired/not-found, v2-worker exclusion, bound-no-exposure).
  • Gates: full suite 2320 tests OK; basedpyright 0/0/0; guardrails OK; git diff --check clean.
  • Normal reviewer verdict: APPROVED (round 4).
  • Daybreak verdict: SECURITY_REVIEW_BLOCKED — gpt-daybreak-blue-latest is not available in this environment (verified against the host's configured models); per policy no substitute model closes the security gate, so the PR stays paused at the human decision point. The recorded attack surface for that review is listed in the task and remains applicable to head 42a4832.
  • Live acceptance: remains behind the recorded LIVE_CODEX_SUBSCRIPTION gate; #141 owns the final layered acceptance.
## Round 4 review: APPROVED — owner decision implemented; exact head `42a4832` The fresh independent review of `ffe6da2` (the owner-decision implementation) returned **VERDICT: APPROVED, `UX impact: none`**, zero MERGE_BLOCKERs. Verified by the reviewer: the mapping is emitted at exactly one site and only after the full validation chain (client equality → shape/digest echo → exactly-once claim → deadline → authority recheck); shape is exactly D-062 pt 6; content is forwarded verbatim with no inspection and no `success: false` inference; no client-specific branch; STOP supersession is clean (`tool_result_success_unresolved` survives only as the historical supersession note in pt 6); the documentation tells one consistent story (both cells PARTIAL, owner-approved lossy mapping, live gate pending, no PASS claim); all restored round-trip/replay/authority/mismatch suites green; basedpyright 0/0/0. **One DEFER fixed** (`42a4832`): D-062's Boundary list now names `routing_core.py` (the live v3 availability gate, the `continuation_capable_resource_ids` request input, and the compatibility reason-code dedup). A second briefing note needs no repo change: the live gate is recorded in the repo under its own name `LIVE_CODEX_SUBSCRIPTION` (the `LIVE_TOOL_BRIDGE_NOT_VERIFIED` wording is the task-report label for the same outstanding gate). **Final state of this task:** - Exact develop base: `59538e9`. Exact head: **`42a4832`** (7 commits over develop, no merges, PR open and unmerged). - Owner-approved success compatibility rule: implemented, recorded as the D-062 pt 6 amendment (lossy translation; upstream fact preserved; content opaque; generic). - Evidence: `tool_calls` = PARTIAL, `tool_results` = PARTIAL. No PASS claim. - Deterministic E2E: full round trip pinned non-streaming AND streaming (usage carried once; one thread/start + one turn/start across both legs), verbatim error-text pin, plus the full negative matrix (replay 409, foreign 404, mismatch 400, authority 403, expired/not-found, v2-worker exclusion, bound-no-exposure). - Gates: full suite 2320 tests OK; basedpyright 0/0/0; guardrails OK; git diff --check clean. - Normal reviewer verdict: **APPROVED** (round 4). - **Daybreak verdict: `SECURITY_REVIEW_BLOCKED`** — `gpt-daybreak-blue-latest` is not available in this environment (verified against the host's configured models); per policy no substitute model closes the security gate, so the PR stays paused at the human decision point. The recorded attack surface for that review is listed in the task and remains applicable to head `42a4832`. - Live acceptance: remains behind the recorded `LIVE_CODEX_SUBSCRIPTION` gate; #141 owns the final layered acceptance.
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
develop
issue-246-authorized-receipt
issue-245-mi-startup
issue-226-administrator-ux
issue-183-observer-access
issue-183-event-acceptance
issue-183-internal-events
issue-183-resource-state
issue-242-secure-bootstrap
issue-238-mi-server
issue-239-architecture-reconciliation
issue-224-certificate-lifecycle
issue-224-origin-bound-worker
issue-224-worker-ca-trust
issue-234-evidenced-resources
issue-186-installed-lifecycle
issue-186-ghcr-distribution
issue-225-cancel-paths
issue-228-backlog-disposition
issue-222-active-mi
issue-176-mi-consumer
issue-185-contract-conformance
issue-219-project-security-audit
issue-181-responses-subset
issue-214-claude-subscription
issue-205-review-result-recovery
issue-174-executable-contract
issue-215-conversational-review
issue-210-d069-review-governance
issue-212-d069a-bootstrap
issue-202-dependency-eligibility
issue-184-strict-no-payg
issue-175-kernel-requirements
issue-180-native-authority-boundary
issue-182-readable-status
issue-195-post-selection-gates
issue-179-catalog-effort
issue-178-exact-target-pin
issue-190-workflow-self-remediation
task/168-capacity-controlled-home
fix/143-scenario11-cancel-race
policy/zai-model-governance-campaign
main
No results found.
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
bug
Something isn't working

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
P1
Owner-declared highest explicit implementation priority (loop ordering rank)

  
P2
Required product and integration work

  
P3
Deferred or producer-dependent work

  
question
Further information is requested

  
wontfix
This will not be worked on

No labels
accessibility
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
P1
P2
P3
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Creatidy/scarcity-router#137
No description provided.