Conversation
jackwener
left a comment
There was a problem hiding this comment.
Review result: blocked (1 P1, 1 P2)
Exact head: b2f88b60d985b3aaf028d167c6f489c54ef27a05
Stack base: #5207 at 8545dedf0e37a32e637e6e5fa934d27d230e6ae9
Findings
- [P1] Keep non-user turns fail-closed when prior tool effects are unknown, and preserve the activation origin through the real Host path. Recovery now seals a dispatched-without-result invocation as
outcome_unknown, but the TypeScript path only adds the temporary warning whenallowPriorUnknownToolOutcomesis true. When it is false,AiSdkTurnfollows the ordinary replay path and still calls the provider; the new “unprivileged” regression explicitly demonstrates that the request proceeds after both the unknown call and warning are omitted. That differs from #5657's strict non-client history validation and lets scheduled, Goal, Graph, or other automated work continue without knowing that a side effect may already have happened. In addition,maka activateaddsorigin: { kind: "cloud_activation" }, butRuntimeHostRunRuntime.sendMessagedropsinput.originbeforepreparePrompt, andturn.starthas no origin field. A production-adapter probe failed withactual: undefined, so activation is currently misclassified as an explicit client message and bypasses even the intended distinction. This can cause an automated follow-up to repeat an irreversible or non-idempotent action. Please reject non-explicit fresh turns while unresolved sealed unknown outcomes exist, carry the activation origin through the Host protocol (including the compatibility-epoch update if the wire shape changes), and add an end-to-end regression through the production activation adapter. - [P2] Add the required
Generated-bytrailer. The PR body says OpenAI Codex implemented the Runtime behavior and regressions, but the sole substantive commitb2f88b60has noGenerated-by: OpenAI Codextrailer.
Verification
The exact head builds successfully for Core, Storage, MCP, Runtime, Runtime Host, Eval, and CLI. The focused recovery, request projection, RuntimeKernel classification, activation, resume-observation, TUI, and driver checks passed 8/8. The complete stack merges cleanly with current main (167cb91910fbc11920fc0a5bdf327add2adec6f4) as synthetic merge e91868d2a4c1dd122141a92fbd3ff9ba029f905b; affected focused checks passed 72/72 there. The failing production-adapter probe above is additional coverage not present in the PR. This PR currently has no hosted check run. I did not approve or merge.
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
hqhq1025
left a comment
There was a problem hiding this comment.
Reviewed the current head. I independently corroborate the existing cross-layer P1 finding; I am not adding a duplicate inline comment.
[P1] Automated maka activate calls include origin: { kind: 'cloud_activation' } (packages/cli/src/activation-command.ts:510-519), but the production Runtime Host adapter passes only turn ID, orchestration, and step count to preparePrompt (packages/cli/src/runtime-host-run-command.ts:313-324). preparePrompt sends no origin in turn.start (packages/cli/src/runtime-host-session-driver.ts:347-373), and that protocol decoder has no origin field (packages/runtime-host/src/protocol/turn.ts:327-339). The Host therefore admits the call without the automatic origin; isExplicitClientMessage treats an absent origin as explicit (packages/runtime/src/runtime-kernel.ts:3545-3555), enabling the request-only unknown-outcome projection for an automated turn. Separately, when that allowance is false, AiSdkTurn skips unknown-outcome inspection and uses ordinary replay (packages/runtime/src/ai-sdk-turn.ts:862-875,2766-2785); its new unprivileged test actually reaches the provider while omitting the unknown tool call and warning (packages/runtime/src/__tests__/ai-sdk-backend.test.ts:15635-15654). An automated continuation can thus proceed without reliable knowledge of a previous dispatched tool's side effects. Preserve a trusted automatic origin across the production Host boundary and fail closed for non-explicit turns with sealed unknown outcomes; add a production-adapter regression.
The change also seals valid dispatched-without-response invocations as failed outcome_unknown (packages/runtime/src/session-manager.ts:4843-4895) and synthesizes prior unknown responses only for eligible model requests (packages/runtime/src/model-history.ts:103-245). I inspected recovery sealing, admission, projection/replay, the CLI–Host protocol, and adjacent tests. There is no storage schema/migration change. The PR is OPEN/MERGEABLE and merges cleanly with fresh main; git diff --check passed. This head has no visible checks now. I could not run local tests (Node 18/no dependencies) or a real Host restart/provider request. Do not merge until the P1 is addressed and current-head gates run.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
8545ded to
5960193
Compare
b2f88b6 to
540b363
Compare
Generated-by: OpenAI Codex
Preserve turn origin through Host admission and fail closed for automated turns when prior tool effects are unknown. Generated-by: OpenAI Codex
5960193 to
e5ba809
Compare
540b363 to
7a28ad1
Compare
jackwener
left a comment
There was a problem hiding this comment.
Review result: blocked (1 P1, 1 P2)
Exact head: 7a28ad1c5c266b34d0adfc16ab318e9d64f80d63
Stack base: #5207 at e5ba809ce3e36749f2cc109a98dd07794edcbbb3
Findings
- [P1][normal recovery path] Do not turn tolerated replay diagnostics into fatal unknown-outcome failures.
AiSdkTurn.runWithinScopenow callsinspectPriorUnknownToolOutcomesfor every fresh turn. That helper returnsblockedas soon asresolveRuntimeRecoveryreports any corruption diagnostic, before it establishes that the history contains a sealeddispatch_without_responseoutcome and beforeallowPriorUnknownToolOutcomesis considered. Existing supported histories with an orphan tool result or a stale tool-result transition therefore fail before the provider call, even though those cases were intentionally handled by dropping the orphan and replaying the remaining valid history. Eight existingai-sdk-backendreplay tests now fail withCannot use prior tool history: prior tool history is corrupt. I also reran the orphan-result regression withallowPriorUnknownToolOutcomes: true; an explicit client turn still throws the same error. Restrict the new inspection and fail-closed behavior to actual sealed indeterminate dispatches, preserve the existing orphan/stale-result replay contracts, and add an explicit-client regression. - [P2][supported macOS test path] Make the new production-adapter regression use a canonical temporary path.
activation-command.test.tshard-codes/tmp/maka-workspace. On macOS, the activation path canonicalizes it to/private/tmp/maka-workspace, while the fake Session returned byswitchSessionstill reports/tmp/maka-workspace. The production context then attempts path relocation, finds that the fake driver has nomoveSession, and fails beforepreparePrompt, so the test never verifies the origin it was added to protect. Usemkdtemp(tmpdir())/realpath, or make the fake Session and override share the same canonical path.
The earlier findings are otherwise closed on this head: cloud_activation now reaches turn.start, the protocol decoder and durable admission preserve it, automated turns reject sealed unknown outcomes before the provider call, the compatibility epoch moved from 190 to 191, and all substantive commits carry Generated-by: OpenAI Codex.
The production origin/admission suites passed 293/293. build:test, Biome lint/format, ASF headers, the protocol-epoch guard, and git diff --check also passed. The broader affected suite ran 1,092 tests: 1,083 passed and nine failed—the eight replay regressions above plus the macOS path failure. This head is the full stack on current main 2eb97eb048432a7416f9427691b2465f8e70117d; no hosted check is present. I did not approve or merge.
Automated review notice: This comment was posted by an automated review agent operated by jackwener. It is not an independent human review and does not replace one.
hqhq1025
left a comment
There was a problem hiding this comment.
I independently reviewed this exact head. The new CLI-to-Host cloud_activation origin path is wired through the turn.start protocol and durable external-message descriptor. I found no additional issue beyond the existing current-head review, so I am not reposting its findings. The current-head test check is failing: 10 Runtime replay tests terminate with Cannot use prior tool history: prior tool history is corrupt, consistent with the already reported replay blocker. This head is not ready for merge; preserve the previously tolerated replay cases and rerun the required check. I did not run a local suite or a real Host restart.
Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.
Summary
Port the Runtime behavior of #5657 to the TypeScript runtime as a separate PR in the resume stack.
If recovery finds a durable T1 tool dispatch without a committed T2 result, it seals the old invocation as
outcome_unknown. It does not fabricate a durablefunction_response, retry the tool, or resume the old Run.When the user later submits a fresh explicit message, that new Turn's provider history gets a request-local projection: the old tool call, a temporary
outcome_unknownresponse, a system reminder, and the new message. The temporary response and reminder are not written to the RuntimeEvent ledger or transcript; the model decides whether to inspect state or take another action.If a checkpoint or context budget would erase T1, Runtime replays the full effective history so uncertainty is not hidden. This can surface a real provider context-limit error for very large unresolved histories; compaction that preserves T1 while reducing the rest is a documented follow-up boundary.
Stack: based on #5207, which is based on #5206. This Runtime behavior is separate from #5205's UI/default-policy scope and does not close that issue.
Verification
npm run typechecknpm run build:testgit diff --checkBaseline caveat: two other tests in
execution-composition.test.jsfail during composition close with anAggregateError. The same failures reproduce on a clean worktree at current mainc299cc17a7; focused resume tests pass.AI use
OpenAI Codex traced the recovery and model-history paths, implemented the TypeScript behavior and regressions, updated the bilingual architecture docs, and performed the requested adversarial and simplification audits.
Checklist