Skip to content

fix(coding-agent): preserve surviving effort on session resume - #1596

Open
rlaope wants to merge 5 commits into
code-yeongyu:mainfrom
rlaope:fix/resume-configuration-effort-rlaope
Open

rlaope wants to merge 5 commits into
code-yeongyu:mainfrom
rlaope:fix/resume-configuration-effort-rlaope

Conversation

@rlaope

@rlaope rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Summary

Preserve a session's surviving reasoning effort on resume instead of replacing it with a remembered startup default when missing ancestry makes the original thinking selection unreachable.

  • Recover only supported effort in the existing native configuration-update scope: gpt-6-astra on openai / openai-codex.
  • Keep explicit CLI/SDK/model-suffix overrides and reachable thinking selections ahead of recovery. Persist overrides and append a final inline configuration update when needed, so local state, provider input, and subsequent resumes agree.
  • Preserve the original request reasoning baseline and append-only history. No ancestry repair, unrelated-branch scanning, new defaults, or global-settings changes.

This is separate from #1588, started from upstream 37a3a183f, and is synchronized with current main f66726040 (including upstream's independently merged partial-answer test correction). To clear the required CI blocker, it also ports only #1588's verified rpc-worker-routing.test.ts correction for the current partial-answer contract; no freeze-fix production code or branch merge is included. Fixtures are synthetic; no private session content is included. The underlying missing-parent cause and recovery of an unknown original cache baseline are outside this fix.

Verification

  • Failing-first tests reproduced low instead of surviving xhigh, stale inline effort overriding explicit/later selections, and an intact-history override reverting on the next resume.
  • 266 tests passed across 24 related files on the integrated tree, including 20 new SDK/session regressions covering both providers, per-model/global defaults, explicit/model overrides, intact/unrelated branches, later selections, invalid/unsupported effort, model scope, and repeat resume.
  • The old RPC fixture independently reproduced its question_incomplete deadline (5 passed / 1 failed). After correction, routing + question-bridge + SDK-resume tests passed 33/33 in one run. Empty submissions remain rejected; partial-answer and comment-only results are checked on all three real IPC peers, with listeners registered before responding. No sleeps, timeout increases, or skipped tests.
  • bun run check and the full bun run build passed. Scoped Biome, TypeScript compiler diagnostics, node --check, git diff --check, and the local changelog gate passed. The optional LSP server was unavailable; compiler diagnostics were used instead.
  • 7 real source CLI --session / RPC scenarios passed, each checking local effort, one localhost provider request, ordered inline updates, the request baseline, clean child exit, and a second resume without overrides. This includes the shipped Astra rule that --thinking off clamps to low.
  • Standard OpenAI Responses mock-loop QA: 4/4 passed. No live provider QA or paid model requests were used for runtime testing; real auth remained unchanged.
  • Independent read-only gate review: APPROVE, HIGH confidence, no blockers for the resume fix; a fresh incremental reviewer also APPROVED/HIGH the exact RPC test correction and its red/green evidence. Main integration retained that reviewed test file byte-for-byte, with refreshed checks, full build, 266 tests, and all seven real CLI scenarios passing. Remote CI is the remaining delivery gate.

Reproduce the real-surface QA after installing dependencies/building workspace entries:

node --import tsx .agents/skills/senpi-qa/scripts/scenarios/resume-effort-qa.mjs --self-test --evidence resume-effort

Local receipts are retained under local-ignore/qa-evidence/20260911-resume-effort/. The affected package's Unreleased changelog and exact nearest changes.md are included. This PR is intentionally left unmerged.

@rlaope

rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

CI is blocked; this PR is not ready for a green-CI handoff and must remain unmerged.

The only failing test is the unchanged test/suite/rpc-worker-routing.test.ts case broadcasts question prompts across IPC and hydrates a late attachment, reporting Error: RPC deadline. It also fails on the exact upstream base 37a3a183f, independently of this diff:

All other CI jobs passed, including static checks, the other two coding-agent shards, workspaces/scripts, Windows RPC, terminal tools, and Inspector handoff. Changelog and review-claim gates also passed. No failed-job rerun, test weakening, or unrelated RPC patch has been applied. Local resume-effort verification remains 253 passing related tests and seven passing real CLI/RPC resume scenarios; independent gate review approved the scoped change.

@rlaope

rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

The required CI correction is now included in 692d567: only the verified RPC worker-routing test patch from #1588 was ported, without merging that PR or changing production behavior. The old fixture waited for an error after a partial answer that the current contract correctly accepts.

Local red: 5 passed / 1 RPC deadline. Green: 33/33 routing, question-bridge, and resume-effort tests in one run. Both partial-answer and comment-only cases now assert resolution payloads on all three peers, with listeners registered before responding. Empty submissions and late responses remain rejected; no sleeps, timeout increases, skips, or suppressions were added. Root checks and pre-commit hooks passed. A fresh independent incremental reviewer returned APPROVE / HIGH confidence.

Fresh CI is running on the corrected tree; the PR remains unmerged.

@rlaope

rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

The corrected RPC test passed CI, along with all three coding-agent shards and the other completed jobs. The workspace/scripts job was canceled during Ubuntu apt package installation, before dependency installation, build, or tests: https://github.com/code-yeongyu/senpi/actions/runs/34592090012/job/103239470489 . No project test failed in that run.

GitHub denied the rlaope rerun request because repository admin rights are required. Commit db2648a completes the repository-required PR/contributor attribution in the Unreleased changelog and triggers a fresh run. Runtime and test code are unchanged from the independently reviewed correction. The PR remains unmerged pending green required checks.

@rlaope

rlaope commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

Upstream #1599 merged its own partial-answer test correction while CI was running, creating a conflict in the same test file. Commit 80e051a integrates main f667260 and resolves only our owned test conflict by retaining the already-reviewed two-outcome/three-peer test byte-for-byte. The new upstream diagnostic helper is retained. #1588 itself remains unmerged and was not merged into this branch.

The integrated tree passes root checks, full build, 266 tests across 24 related files, and all seven real source CLI resume/RPC scenarios including repeat resume. Fresh CI is now running on the conflict-free tree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant