test(sdk): deterministic host steer response handshake - #4643
Merged
Conversation
Dev CI shard-6 (run 32059470791, job 95479277528) failed both sdk-host-steer-integration tests at the harness transport's fixed 100 x Bun.sleep(1) poll with "host did not respond", while the same head passed on rerun (job 95485461325): a wall-clock poll bound, not a host response regression from #4631/#4613. Replace the poll loop with a per-emission promise handshake resolved by the response frame correlated to the live request id, fenced against stale/late frames (recorded, never resolving a later await), with a bounded safety timeout instead of a hang. Transport now also supports an injectable response delay for race-contract tests: delayed response beyond the old 100ms bound, timeout then fenced late response, and sequential control isolation. Durable replay/restart, no-redispatch, and undefined-session-file state-root contracts are preserved verbatim. Sandbox-only unrelated evidence recorded in the PR: tools/lsp-regressions C#-LSP/tlaplus tests fail under the run-bun-test-files sandboxed HOME at this head both with and without this patch (environment-sensitive, not absorbed in this lane). Closes #4641 Lore-id: 4641-steer-sync Constraint: no blind sleep inflation; handshake must be tied to actual host response Constraint: keep synchronous delivery semantics of the memory transport Rejected: raising the poll budget to e.g. 500 iterations | still wall-clock dependent under fresh-process load Rejected: unbounded await on response promise | a lost response would hang the file to the harness timeout Tested: focused file 3x (4/0), 3x fresh-process sandboxed (4/0), adjacent SDK suites 105/0, package check green Not-tested: none; CI shard-6 rerun on the PR head Confidence: high Scope-risk: narrow Reversibility: trivial
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Owner
Author
Signed exact-head evidence — #4643
Root cause (not a product regression)
Fix — causal handshake, not a bigger sleep
Verification at exact head
Unrelated shard attribution (explicitly not absorbed)
Reviewers requested: @probepark @snowykr (independent review on this exact head; verdict stays — |
This was referenced Aug 18, 2026
pull Bot
pushed a commit
to nenyatech-mirror/gajae-code
that referenced
this pull request
Aug 18, 2026
…eachan-Heo#4643) Dev CI shard-6 (run 32059470791, job 95479277528) failed both sdk-host-steer-integration tests at the harness transport's fixed 100 x Bun.sleep(1) poll with "host did not respond", while the same head passed on rerun (job 95485461325): a wall-clock poll bound, not a host response regression from Yeachan-Heo#4631/Yeachan-Heo#4613. Replace the poll loop with a per-emission promise handshake resolved by the response frame correlated to the live request id, fenced against stale/late frames (recorded, never resolving a later await), with a bounded safety timeout instead of a hang. Transport now also supports an injectable response delay for race-contract tests: delayed response beyond the old 100ms bound, timeout then fenced late response, and sequential control isolation. Durable replay/restart, no-redispatch, and undefined-session-file state-root contracts are preserved verbatim. Sandbox-only unrelated evidence recorded in the PR: tools/lsp-regressions C#-LSP/tlaplus tests fail under the run-bun-test-files sandboxed HOME at this head both with and without this patch (environment-sensitive, not absorbed in this lane). Closes Yeachan-Heo#4641 Lore-id: 4641-steer-sync Constraint: no blind sleep inflation; handshake must be tied to actual host response Constraint: keep synchronous delivery semantics of the memory transport Rejected: raising the poll budget to e.g. 500 iterations | still wall-clock dependent under fresh-process load Rejected: unbounded await on response promise | a lost response would hang the file to the harness timeout Tested: focused file 3x (4/0), 3x fresh-process sandboxed (4/0), adjacent SDK suites 105/0, package check green Not-tested: none; CI shard-6 rerun on the PR head Confidence: high Scope-risk: narrow Reversibility: trivial Co-authored-by: Yeachan Heo <yeachan.heo@gmail.com> Co-authored-by: probe <re2rar@gmail.com> (cherry picked from commit b72f125)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4641
Root cause
Dev CI run
32059470791(job95479277528, shard 6-of-8) failed both tests inpackages/coding-agent/test/sdk-host-steer-integration.test.tsat the harness transport's fixed 100 ×Bun.sleep(1)poll →host did not respond. The same run/head passed on rerun (job95485461325). Reproduced locally at exact head6696988b6dd757f898ab7b33984ba830b6344109only under fresh-process load; both merge parents (80a0c0974d#4631,6696988b6d#4613) carry the same poll loop. Not a product regression — an invalid wall-clock bound on the test transport: the host answers asynchronously, so any scheduling stall > ~100ms starves the poll while the response is still in flight.Fix (causal contract, not a bigger sleep)
Per-emission promise handshake resolved by the response frame itself, correlated by request id:
sendFrameresolves the live emission only whenframe.id === pending.id; anything else is fenced intolateResponses(a stale frame can never satisfy a later emission).RESPONSE_TIMEOUT_MS = 5000) fails a lost response instead of hanging to the harness timeout; dangling sleep timer does not delay process exit.stop()clears pending delayed-delivery timers.responseDelayMsfor deterministic race-contract tests.New race-contract tests
lateResponses === [].Preserved verbatim: durable replay correlation, restart
process_restart_uncertain, no redispatch (dispatchescounts), undefined-session-file state-root persistence.Evidence (exact head
d0a1397472, parent6696988b6dd757f898ab7b33984ba830b6344109)bun test×3HOME/XDG_*/TMPDIR/GJC_HOME+test-preload.ts) ×3session-runtime,sdk-surface-parity,sdk-broker-restart,sdk-reconciliation-recovery,sdk-q26-prompt-status,sdk-session-isolation)bun --cwd=packages/coding-agent run check(biome + tsc)check-visible-definitionsUnrelated shard evidence (not absorbed in this lane)
Shard-6-of-8 full run locally fails
packages/coding-agent/test/tools/lsp-regressions.test.ts(3 tests: tlaplus detection, csharp-ls preferred, omnisharp fallback) only under the sandboxedHOMEused byrun-bun-test-files.ts; verified at exact dev head6696988b6dd757f898ab7b33984ba830b6344109with this patch stashed (clean tree) → identical failures. Unsandboxed: 24/24 pass. Environment-sensitivepath-trust/$whichinteraction with the synthetic~/.gjc-lsp-*binaries — pre-existing, out of #4641 scope, needs its own issue.GJC verdict
—
[repo owner's gaebal-gajae (clawdbot) 🦞]