Repository navigation
Conversation
Rebase of #6209 onto dev after #6210 squash-landed the PR's first commits (8941685..feb58dc). Re-applies the remaining delta (feb58dc..7c8b162): explicit Codex terminal veto, separate bare server_is_overloaded vs server_error/internal_error admission, managed outcome terminal veto, and their regression tests. Conflict resolution: keep dev's deterministicVeto retryMaxAttempts:1 for typed codes that are not explicit terminal vetoes; explicit terminal vetoes drop transport facts (PR behaviour). Tested: bun test packages/ai/test/openai-codex-stream.test.ts (106 pass) Tested: bun test packages/coding-agent/test/agent-session-resilient-retry.test.ts (136 pass) Tested: bun --cwd=packages/ai run check; bun --cwd=packages/coding-agent run check
Nested Codex terminal veto messages can lose transport facts and bypass the bare-default gate. Classify explicit veto codes and messages before retry policy admission so configured and managed paths retain the one-request ceiling.
…osed The default fixture persisted about 234 MiB of transcript, above the product's 128 MiB eager-resume ceiling, so every default run died with the oversized-transcript recovery message before it could report a measurement. AC-1 gated only the append steady state, so the ~128 MiB the read path retains after the forced GC turn stayed invisible, --force-memory-only refused to start unless the operator pre-set GJC_CODING_AGENT_DIR, and a failed verdict still exited 0. The default fixture now stays inside the imported ceiling and an override that would exceed it fails closed with the limit and the fix; the harness always runs against its own throwaway agent dir, so an ambient GJC_CODING_AGENT_DIR no longer redirects a benchmark into the operator's ~/.gjc; AC-1 gates the read-path residual as a multiple of the fixture transcript plus an explicit allocator reclaim floor; and a fail verdict exits non-zero. Tested: bun packages/coding-agent/scripts/resident-memory-bench.ts --mode rss --runs 1 (default 500x48KiB: exit 0, verdict pass, steady 62.6 MiB, read-path residual 126.2 MiB against a 197.2 MiB ceiling); --force-memory-only (exit 0, forced-memory-fallback); --mode put-latency --puts 64 (exit 0); --mode read-churn --entries 8 (exit 0); --entries 5000 (exit 1 with the ceiling diagnostic); ratio temporarily lowered to 4 (verdict fail, exit 1); operator ~/.gjc/agent/resident-cache mtime unchanged across all runs Not-tested: CI variance of the new ratio ceiling; the fresh-open diagnostic still reports ~248 MiB, which stays documented evidence Lore-id: 7c1f4a92 Confidence: high Scope-risk: narrow
Loading cli-main pulled ./commands/quick-lane, ./exec/bash-shell-guardian, ./exec/bash-shell-supervisor, ./exec/bash-shell-worker, ./exec/isolated-shell, ./tools/browser/tab-worker-smoke and ./cli/fixture-report into every invocation that reaches the ordinary dispatcher, including ones that only fail on an unknown option. Each of those modules is needed by exactly one path, and the file already used lazy `await import()` for its other per-path modules. Moving all seven to their call sites (and quick-lane to the registry's own `load:` thunk) keeps observable behaviour identical and lowers the light invocation footprints. Tested: /usr/bin/time -l maximum resident set size on the rebuilt binary vs the pre-change binary -- --version 32,423,936 -> 27,639,808 B; --help 32,718,840 -> 28,131,328 B; `--doctor` (unknown option, exit 2) 239,239,168 -> 238,125,056 B. stdout, stderr and exit codes are byte-identical for all three paths; `gjc --smoke-test` still reports ok, which exercises the now-lazy tab-worker and isolated-shell modules; `bun scripts/verify-rss-checkpoints.ts --scenario S1` reports stable-tree RSS median 43.13 MiB with no regressions against the 42.79 MiB b4a8559 baseline. Not-tested: the 238 MB unknown-option footprint stays dominated by routing an unrecognised root token into `launch`, which loads the agent runtime before flag validation; that is not an import-order problem and is left as follow-up. Lore-id: 3d90b6f1 Confidence: high Scope-risk: narrow Reversibility: clean
The header still described the old 5,000-entry default and prefixed every example with GJC_CODING_AGENT_DIR="$(mktemp -d)", which the harness now ignores because it always runs against its own throwaway agent dir. The read-path ceiling also gained an absolute floor. A pure ratio against the fixture transcript is not scale-invariant: the tiny documented smoke fixtures retain a few MiB of ordinary runtime overhead after forced GC, so 8x an 11 KiB transcript flagged them as failures and the documented smoke commands exited 1. The ceiling is now max(32 MiB, 8x transcript), which keeps the ratio meaningful at working scales and stops the floor from false-positiving on smoke fixtures. Tested: --mode rss --runs 1 (default) exit 0 verdict pass, steady 70,615,040 B, read-path residual 123,158,528 B against a 197,248,000 B ceiling; --mode rss --entries 8 --bytes-per-entry 4096 --runs 1 exit 0 verdict pass (residual 4,210,688 B against the 33,554,432 B floor); --mode put-latency --puts 64 exit 0; --mode read-churn --entries 8 exit 0; --entries 5000 exit 1 with the eager-resume ceiling diagnostic; floor 0 + ratio 4 temporarily produced verdict fail with exit 1, then both constants were restored; bun --cwd=packages/coding-agent run check exit 0 Lore-id: 5b7ea3c8 Confidence: high Scope-risk: narrow
The AC-1 verdict ordering let `reclaimProof` win over the read-path ceiling: when the post-reclaim delta stayed inside the append limit, the run was relabeled documented-evidence-with-reclaim-proof and the process still exited 0 even though `readPathResidual.passes` was false. That is the same defect as the earlier "a failed verdict exits 0" report, just reachable through the read-path ceiling instead of the steady-state one, so a downstream CI gate reading the exit code could not see a read-path retention regression. The reclaim proof now excuses the steady-state ceiling only; the read-path ceiling and the reclaim contract stay hard gates. The documented-evidence label is unchanged for the case it was written for, and the header prose says which gates a proof can excuse. Tested: reproduced the masking on this tree by pinning the proof true with the ratio at 4 and the floor at 0 -- pre-fix verdict documented-evidence-with-reclaim-proof, exit 0, readPathResidual.passes false (residual 120,406,016 B over a 98,624,000 B ceiling); post-fix verdict fail, exit 1, residual 122,322,944 B. Default run exit 0 verdict pass (steady 52,002,816 B, residual 121,700,352 B vs 197,248,000 B); tiny smoke fixture exit 0 verdict pass (residual 4,227,072 B vs the 33,554,432 B floor); --mode put-latency --puts 64 exit 0; --entries 5000 exit 1 with the eager-resume ceiling diagnostic; bun --cwd=packages/coding-agent run check exit 0; the patched file was restored byte-exact (sha256 6012695ff4b04238e4b7494d955aa83cbee954513da945f1e3d605439dbfae55) Lore-id: 7c41d9e6 Confidence: high Scope-risk: narrow
Runs the existing verify-rss-checkpoints.ts harness on S1,S2,S3 (cheap startup/version/interactive subset; heavy S4/S5/S7 stay in the release rss_checkpoint job) against the compiled gjc binary and compares it with the previous published stable release using --compare without --advisory, so a real regression fails the job. When no published stable release exists there is no comparable baseline, and the job fails with an actionable diagnostic instead of passing silently. The job triggers only through the existing affected-plan changed-path machinery when the binary build path, native addon, session manager, or the harness itself changes, and inherits the metadata-edit no-op and dispatch conventions of its sibling jobs. The trigger list also covers the compiled binary's boot entrypoints (cli.ts, cli-main.ts, cli-ordinary.ts, cli-commands.ts). S1/S2/S3 measure startup cost, and the module graph of exactly those files decides what those processes load: moving a module out of the eager graph moved `--version` peak RSS by 14.8% without touching the build path or the session manager, so a path list without them would have skipped the gate for the very change class it exists to catch. It stays path-scoped rather than matching all of packages/coding-agent/src, so ordinary PRs still do not schedule it. Tested: bun scripts/check-workflow-yaml.ts (5 workflows parsed); bun scripts/check-workflow-permissions.ts; bun test scripts/dev-ci-guard-topology.test.ts; local reproduction of the compare path's fail-closed exit (bun scripts/verify-rss-checkpoints.ts --scenario S1 --compare -> exit 1, BaselineDefaultMissing) Not-tested: the job in GitHub Actions (needs a published stable release and a Linux x64 runner; the baseline download path is unchanged from the release rss_checkpoint job) Lore-id: b3d95e71 Confidence: high Scope-risk: narrow Reversibility: easy
A complete salvaged call without both source identifiers cannot be replayed alongside later tool output. Require reconstructable source identity before changing response state.
Keep reserved wire names and output ordering when a complete function call is recovered after a transient stream close.
…retry fix(session): retry content-free Codex server error events
…ssions Follow-up to #6209: the bare-default list still said only server_is_overloaded was admitted for Codex, and did not state that explicit Codex terminal vetoes are never retried.
ACP Router attachment retirement previously revoked the SDK adapter without notifying active prompt waiters, leaving them behind the inactivity watchdog. Settle the prompt with the existing abandoned error and preserve its plan snapshot when the host is removed.
…ver-error docs(retry): document Codex server_error/internal_error bare-default admissions
… not EEXIST (#6261) - detached-idle host revokes its own <id>.lifecycle.ready.json on graceful exit (exact-identity unlink) - launch retires the same id leftover ready/marker pair when the recorded owner is proven exited - reapDeadLifecycleMarkers counts only *.lifecycle.json toward inspected
An acknowledged abort remains authoritative when the SDK host closes during the cancellation grace window. Host-close errors now require a valid session attachment before retrying.
The startup RSS reduction in 4d5972b is user-visible, so it needs a release note fragment under the repository's per-change fragment convention instead of a line in CHANGELOG.md, which every PR would conflict on. This commit is docs-only and postdates the frozen boundary review digest (sha256:3851a82c54ada37403c129be63975d0aca7577474997c7827887ac61cd875542); the verified program text is unchanged -- packages/coding-agent/src/cli-main.ts is still md5 45cb9d314a66c9225b6a724617258bfd, the exact bytes the cohort lanes and the terminal critic inspected. Tested: bun --cwd=packages/coding-agent run check exit 0 Lore-id: c2a0be47 Confidence: high Scope-risk: narrow
Same-generation Router attachment replacement is transport rotation, not host retirement. Keep the active prompt owned while clearing foreground busy state on actual host-close settlement.
fix(bench): make the resident-memory RSS harness runnable and fail closed
perf(cli): defer the per-path modules out of the eager cli-main graph
…labeling Address review on #6268: docs/perf-profiling-corpus.md requires every RSS threshold to start advisory until variance is characterized and a human approves enforcement, matching the release rss_checkpoint job. The baseline is the previous stable release, so summary output labels deltas as release-to-head drift. Missing baselines and harness errors still fail.
A stalled session-index heartbeat checkpoint could block discovery publication long enough for peers to treat a live broker as stale. Run the checkpoint independently after each successful discovery heartbeat so broker liveness remains observable. Confidence: high Scope-risk: focused Reversibility: easy Tested: stalled checkpoint heartbeat and concurrent broker startup regression
ci(dev): add advisory RSS checkpoint compare for RSS-relevant changes
fix(acp): settle prompts when session host exits
fix(sdk): decouple checkpoint from broker heartbeat
Modern MCP advertised form support without registering an input handler, so servers requesting user input failed before any UI could appear. Roots-only input also incorrectly depended on an interactive handler. Register lazy session-owned input surfaces and reject unsupported schemas without auto-approval or transport replay. Preserve decline and cancellation. Lore-id: 7d734e9c Constraint: never auto-approve server input or mutate inherited managers Tested: 89 focused MCP tests, package types, Biome, source CLI smoke Tested: independent baseline reproduces roots-only and session registration failures Not-tested: live Supabase mutation; installed binary unchanged Not-tested: compiled binary blocked by native provenance mismatch and missing Cargo Confidence: high Scope-risk: narrow Reversibility: easy
Untrusted form labels must retain server attribution and cannot use bidi controls to disguise the question. Expired or invalid remote receipts must not turn an uncommitted selection into server approval. Lore-id: 890d5e12 Constraint: no acceptance without an actual committed remote decision Tested: 93 focused tests, package types, Biome, late receipt abort settlement Confidence: high Scope-risk: narrow Reversibility: easy
fix(ai): salvage complete Codex calls after reasoning opens
fix(mcp): wire owned-session form input handling
Cutoff cleanup can run before a session endpoint is published. Treat an authoritative ENOENT as completed cleanup so the host does not spend the retry window sleeping before disposal.
Startup and native loading can consume the fixed observation window before the shipped host sees its cutoff marker. Start the prompt-exit assertion when the cutoff receipt is published so Linux CI measures the lifecycle contract directly.
Collaborator
Author
|
Closing as a duplicate of #6265. This PR's head (6e89aab) is identical to the #6265 head: the cutoff-receipt test commit is already on the #6265 branch, so review and CI continue there. The base branch here (fix-6261-stale-ready) is behind dev, so the diff shown on this PR includes unrelated dev commits and has no unique changes. |
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.
Agent
codex/gpt-6.1-sol via paseo on tank (worktree fix-6261-ci-cutoff)
Task
목표: PR #6265의 Dev CI 실패를 고쳐
shipped session host exits promptly after publishing a cutoff receipt테스트를 Linux CI에서 안정적으로 통과시키고, 기존 AC-1 stale-ready 수정과 real-host e2e 결과를 보존한다.수정 위치: packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts 주변 line 821 cutoff receipt assertion, 그리고 해당 테스트가 의존하는 session-host lifecycle 구현만 필요한 경우. 먼저 현재 PR head와 CI 로그를 확인하고 원인을 재현한다.
금지: unrelated files/features 수정 금지; AC-1 ready/endpoint cleanup 의미를 되돌리지 말 것; e2e·브라우저·외부 서비스·클러스터 테스트 금지; main/dev 직접 push 금지; 질문하지 말고 합리적 가정을 최종 보고의 Assumptions에 적을 것.
참고 코드: PR #6265, current head b4367df. Failed CI run 37127870634, job 111218750002: sdk-broker-lifecycle-e2e has 149 pass/1 fail. Failure at packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts:825:
expect(outcome).toBeDefined()afterPromise.race([child.exited..., Bun.sleep(1_500)...]); the shipped host did not exit within 1.5s in Linux CI. Existing real-host e2e passed on tank for both direct SIGTERM and broker session.close: PR comment 5969948035.완료 확인: run the repository's prescribed local unit/integration test and lint commands for the affected path; at
Local 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)
-lifecycle-e2e.test.ts --test-name-pattern 'shipped session host exits promptly'
[Shell] PORT_BASE=57680 COMPOSE_PROJECT_NAME=t_4ad8772a bun test packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts
[Shell] git status --short; biome=$(command -v biome || echo node_modules/.bin/biome); $biome check packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts packages/coding-agent/src/commands/sdk.ts
[Shell] git checkout -- packages/natives/native/diagnostic-artifact.json packages/natives/native/index.d.ts; python3 - <<'PY' p='packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts'; s=open(p).read...
[Shell] ./node_modules/.bin/biome check packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts packages/coding-agent/src/commands/sdk.ts; git diff --stat; git diff -- packages/coding-agent/test/sdk-br...
[Shell] python3 - <<'PY' p='packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts'; s=open(p).read(); s=s.replace('await waitFor(async () => (await Bun.file(failurePath).exists()) ? true : undefined,...
[Shell] pgrep -af 'bun test packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts' | head
[Shell] ps -p 1177504 -o etime=,stat=,cmd=; sleep 5; ps -p 1177504 -o etime=,stat=,cmd=
[Shell] sleep 20; ps -p 1177504 -o etime=,stat= || true
[Shell] git add packages/coding-agent/test/sdk-broker-lifecycle-e2e.test.ts && git commit -m "test(sdk-broker): measure cutoff exit after receipt" -m "Startup and native loading can consume the fixed obser...
[Shell] "gh pr view 6265 --json body --jq .body > /tmp/pr6265.body; cat >> /tmp/pr6265.body <<'EOF' ## CI cutoff follow-up (6e89aab) The shipped-host prompt-exit assertion now starts its 1.5s observation...
[Shell] git status --short --branch; gh pr view 6265 --json headRefOid --jq .headRefOid
수정 완료했습니다.
6e89aab1ded4bc0759cf67f99be93ae13d3dbee5fix-6261-stale-ready-clean에 push 완료검증:
sdk-broker-lifecycle-e2e.test.ts: 통과Assumptions: Linux CI의 1.5초 창에는 Bun/native startup 시간이 포함되므로, receipt 발행 이후 lifecycle 종료 시간을 측정하는 것이 테스트 계약에 부합한다고 판단했습니다.