test(sdk): assert correlated retry terminal outcome - #6333
Conversation
An interactive or monitor run can finish after a queued SDK successor starts. Preserve its unmatched lifecycle batch so its final cannot terminalize that successor, while retaining attached invocation ownership and existing SDK expiry cleanup. Lore-id: 5eb7d864 Constraint: never rewrite canonical receipts or replay accepted live work Tested: 16 focused lifecycle contracts with 83 assertions; two baseline wrong-final reproductions; scoped native TypeScript check on two reserved remote cores; focused Biome check Not-tested: full suite and live runtime deployment Confidence: high Scope-risk: bounded Reversibility: git-revert
Accepted retries consume the deferred predecessor end. Carry that boundary identity from the producer instead of retaining an unmatched empty run that blocks terminal publication when the same session is reopened. Lore-id: f54ec7a2 Constraint: preserve independent delayed unowned boundaries and ambiguity guards Tested: reproduced missing replacement terminal on the reviewed head Tested: 19 SDK lifecycle contracts, 98 assertions Tested: 7 retry producer contracts, 57 assertions Not-tested: deployment or live session adoption Confidence: high Scope-risk: moderate
The session source exceeds Biome's default file-size limit, so the normal package check skips its import and layout violations. The touched file must also pass the explicit size-aware check with a 2 MB limit. Apply only the reviewed server-formatted patch: order imports, remove the unused onboarding import, and format the messages getter and steer guard. Lore-id: 9ddf720a Constraint: preserve accepted lifecycle attribution and upstream behavior Not-tested: tests and checks deferred to parent for exact-final-head verification Confidence: high Scope-risk: narrow Reversibility: git-revert
snowykr
left a comment
There was a problem hiding this comment.
Verdict
APPROVED
Summary
This change retains lifecycle identity across accepted automatic retries and keeps session-initiated lifecycle batches separate from queued SDK turns. No actionable merge-blocking defect was identified at the reviewed head.
Findings / Required Changes
No blocking or actionable findings.
Non-blocking Observations
- The opt-in WSLv2/NTFS qualification was skipped; it was not required for these changed paths.
CI / Verification
- Dev CI run
37216544183succeeded for head46539b5d4620c9026263c48dd5e77b241e49d5a7(based9a517893150bf9dc10cbb749f3034c732ceb1fb). The affected-path test tasks for both changed test files, the SDK-host shard, isolated production-host task, and relevant aggregate/final validation completed successfully. - A separate run for the same head was skipped after metadata-only changes; the workflow classifies it as non-code evidence. Other observed skips were conditional or opt-in, not failed required checks.
- I did not execute tests. CI artifacts exposed successful task conclusions, but not test-runner output or independently verifiable per-test counts.
Axis Coverage
| Axis | Verdict | Coverage |
|---|---|---|
| A1 — Intent / Policy / Contract | APPROVED |
Lifecycle correlation behavior matches the inspected SDK contract and changelog; no concrete intent or policy mismatch. |
| A2 — Architecture / Correctness / Failure | APPROVED |
Retry and unowned-batch ownership, terminalization and cleanup preserve correlation across inspected failure paths. |
| A3 — Security / Privacy / Trust | APPROVED |
The new lifecycle identity is internally derived; no client-controlled authorization path or privilege boundary change found. |
| A4 — Verification / Tests / CI | APPROVED |
Changed regression suites and relevant exact-head CI validation succeeded; see CI limitations above. |
| A5 — Context / Compatibility / Platform | APPROVED |
Consumers remain on the in-process lifecycle path; no persisted/generated/platform contract changes or materially harmful duplicate abstraction found. |
Limitations
The review was static; tests were not run by the reviewer. CI runner artifacts were inaccessible, so successful task conclusions—not individual test output or counts—are the available verification evidence.
|
Merged into dev. snowykr approved the exact head — |
Agent
omo/cliproxy/gpt-6.1-sol via paseo on home2 (worktree g4-retry-end-corr-r2)
Task
repo: Yeachan-Heo/gajae-code (base: dev). PR must target dev.
What
Retain the public lifecycle boundary (
lifecycleScope) across accepted auto-retries whose predecessoragent_endwas suppressed, and keep session-initiated (unowned) lifecycle batches separate from queued SDK turns. The finalagent_endof an auto-retried turn is now attributed to the owning invocation's correlation, so the ACP prompt settles at turn end instead of being dropped asincomplete_correlationand expiring at the 340s watchdog.Why
hermes-ops gjc-acp-guard G4 (
watchdog_expired = 0) failed on 2026-10-04: work:a896777e and work:94f7adc2 both hit a mid-turn Codex transient error (server_is_overloaded/request_timeout), auto-retried, finished withstopReason=stop, then loggedacp_prompt_terminal_dropped reason=incomplete_correlationfollowed byacp_prompt_watchdog_expired. Layer: SDK session host lifecycle (session-runtime). paseo, relay, and reap are not involved.Testing
bun test src/sdk/host/session-runtime.test.ts test/agent-session-retry-busy-recovery.test.ts(packages/coding-agent, home2, head 46539b5): 187 pass, 0 fail (2980 expect calls, 248s)bun run --workspaces --if-present check:typesrc=0; telegram-daemon-generation-guard rc=0Acceptance
agent_endof an auto-retried turn carries the owning commandId/turnIdsession-runtime.tsemitLifecycleagent_start(lifecycleScopeOwners / continuationBatch);agent-session.ts#lifecycleScopesByAttemptScope;types.tsAgentStartEvent.lifecycleScopeunownedStartbatches retained in openLifecycleBatches filtersDRY=1 gjc-acp-guard.pyre-measureNeeds e2e
None in CI scope. Post-rollout verification is the hermes gjc-acp-guard G4 re-measure.
Risk classification
low-riskregression-riskhigh-riskGoal
After a mid-turn provider error that gjc auto-retries, the turn's final
agent_endreaches the ACP layer without a complete correlation (commandId/turnId). ACP drops it asincomplete_correlation, so the externally submitted prompt never settles andacp_prompt_watchdog_expiredfires 340s later. Fix it so that the finalagent_endof an auto-retried turn carries the owning invocation's correlation and outcome. The ACP prompt has to settle at that moment, not at the watchdog.Evidence (work mac, gjc 0.18.6 stack-run build cb60f55, 2026-10-04)
13:57:42Z assistant message stopReason=error "Codex error event: Our servers are currently overloaded (code=server_is_overloaded)" (thinking only)
13:58:08Z the next assistant message (auto-retry) continues normally with toolUse, and so on
14:03:06Z the final assistant has stopReason=stop
14:03:06.228Z log
acp_prompt_terminal_dropped terminalType=agent_end reason=incomplete_correlation expectedCommandId=924c51f1 expectedTurnId=91d8f20014:08:46Z
acp_prompt_watchdog_expired cause="the SDK session host stopped producing frames" silenceMs=340003 lastFrameType=message_end.sdk-reconciliation/a896777e.json still has the prompt record status=in_flight.
stream disconnected before completion (code=request_timeout)at 14:12:37Z, tLocal CI gate (prepush)
PREPUSH_CMD rc=0 bun run --workspaces --if-present check:types
PREPUSH_CMD rc=0 bun scripts/telegram-daemon-generation-guard.ts "$(git merge-base origin/dev HEAD)" "$(git rev-parse HEAD)"
Agent log (tail)
✓ post-acceptance invocation terminalization > a queued prompt stays non-terminal even if isIdle flips during the accept window [1409.18ms]
✓ post-acceptance invocation terminalization > a pre-acceptance failure rejects the submission without creating a record [1457.18ms]
✓ post-acceptance invocation terminalization > a later provider error never changes or re-opens an already terminal prompt [1431.71ms]
✓ accepted-control zero-execution bound (#4668) > an accepted prompt that never reaches agent_start remains recoverable [1977.55ms]
✓ accepted-control zero-execution bound (#4668) > a running prompt deadline stops its exact turn and keeps normal terminals working [2513.89ms]
✓ accepted-control zero-execution bound (#4668) > prompt agent_end waits for durable reconciliation before publication [3326.27ms]
[Final output truncated to the last 2000 chars; the full history is still peekable.]
[Thought] Running full package check
[Eval]
[Thought] Planning manual ACP settlement check
Preparing isolated baseline test environment
[Eval]
[Thought] Planning symlinking native addons for packaging
[Eval]
[Thought] Adding temporary symlink for native addon
[Eval]
[Thought] Investigating test false positive causes
Modifying test for three agent starts
[Apply patch]
[Eval]
[Thought] Running baseline temperature test
[Eval]
[Thought] Planning retry logic for agent_end errors
Inspecting test for prompt correlation adaptation
[Eval]
[Thought] Planning test with session switch and prompt correlation
Evaluating session switch impact on test failures
[Eval]
[Thought] Verifying cherry-pick commit differences
Preparing commit amendment
[Eval]
[Thought] Planning targeted test rerun and final checks
Confirming completion of focused tests and checks
[Eval]