Skip to content

fix: retain reviewer sessions established by replies - #143

Merged
zzxwill merged 15 commits into
masterfrom
fix/reply-session-persistence
Oct 21, 2026
Merged

zzxwill merged 15 commits into
masterfrom
fix/reply-session-persistence

Conversation

@zzxwill

@zzxwill zzxwill commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator

When a review has no resumable session ID, a reply can establish a new session but previously discarded its ID. Save and reuse session IDs from both standalone and automatic replies, preserving reviewer/judge separation and legacy events. Correlate replies with their selected event snapshot so late or abandoned replies cannot overwrite a newer review or accepted session change.

Retain known or explicitly reported IDs across failures. Accept generated IDs only after successful execution and a valid, nonempty response; reject malformed, failed and empty whole/tail responses before storing unconfirmed IDs. Regression fixtures cover repeated replies, concurrency order, missing executables, nonzero exits and synchronized timeouts.

Validation on 92e3316b5566aed5cf9069d4cbbd25b428bc8262:

  • 17 focused session/error/concurrency checks passed; fail-before evidence retained locally.
  • node scripts/ci-check.mjs test: exit 0, 302 tests passed.
  • node scripts/ci-check.mjs test:package: exit 0, 55 packaged files and installed CLI checks passed.
  • Actual Jury: exit 0, NO NEW FINDINGS, clean after one round (96.7s). The final reviewer inspected the diff and passed syntax checks; its read-only sandbox prevented write-heavy tests, which passed independently above. Codex was configured for both roles; this clean first round did not require judge triage.

Exact executed review command:

GIT_AUTHOR_NAME=zzxwill GIT_COMMITTER_NAME=zzxwill node bin/jury.js review --dir /Users/bytedance/Documents/Codex/2026-10-20/task/codejury-post-142-audit --trunk master --reviewer codex --judge codex --rounds 2 --summary 'Review the focused reply-session persistence diff against master. Generated IDs require successful execution and a valid nonempty extracted report; known or explicitly reported IDs survive failures. Empty whole and tail responses are rejected in the shared result path. Concurrency tests cover abandoned/no-ID replies, completion order, stale snapshots and newer reviews. Independent final-head checks passed 302 tests and installed-package validation. The unchanged pre-existing run.json republish race is documented and deferred; do not implement it here. Assess new correctness regressions and preserve the focused scope.' --web=false --push=false > ../reply-session-review-evidence/jury-final-92e3316.log 2>&1

The unchanged, pre-existing run.json republishing race is documented and deferred; no fix for it is included. Separate master-CI diagnostics localized a package timeout to a web-launch test and console child but did not establish its root cause; no related source changes or retries are included. Live provider-session resumption is not claimed by local fake-provider regressions.

@zzxwill
zzxwill marked this pull request as ready for review October 21, 2026 18:06
@zzxwill
zzxwill merged commit 079c29f into master Oct 21, 2026
8 checks passed
@zzxwill
zzxwill deleted the fix/reply-session-persistence branch October 21, 2026 18:07
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