Repository navigation
[qa-sweep] Eliminate final-reply checkpoint race in evaluator EOF/SIGTERM teardown - #186
Merged
Merged
Conversation
…F/… [ORB-13847] [qa-sweep] Eliminate final-reply checkpoint race in evaluator EOF/SIGTERM teardown Planned-By: claude
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
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.
Task
ORB-13847 — [qa-sweep] Eliminate final-reply checkpoint race in evaluator EOF/SIGTERM teardown
Description
QA on clean HEAD 17d8203 reproduced a final-reply/checkpoint race in the evaluator's EOF/SIGTERM shutdown fixture. The minimum-budget test failed for both baseline and graph arms in the first opt-in run; a single targeted baseline reproduction failed for graph while baseline passed, demonstrating scheduling dependence rather than a reliable deterministic teardown.
Exact reproduction from a built clean checkout:
AGENT_EVAL_TEST_TMP="$PWD/.orbit/tmp/budget-teardown-repro" AGENT_EVAL_ORBIT=~/.cargo/bin/orbit AGENT_EVAL_ORBIT_GRAPH="$PWD/target/debug/orbit-graph" python3 -B -m unittest discover -s scripts/agent-eval/tests -p test_reply_provenance.py -k test_minimum_budget_reconciles_provider_log_proofs_and_episode_cost -v
The binaries may be supplied as equivalent reviewed absolute paths. The test uses fake_codex.py and a disposable installed-plugin host; it makes no provider calls or live host installation. Do not retry to conceal a failure.
Observed error at scripts/agent-eval/tests/test_reply_provenance.py:352, via test_runner.py:127:
AssertionError: Tuples differ: ('failed', 'broker_failed') != ('ok', None)
{'code': 'broker_failed', 'message': 'the broker worker did not exit cleanly'}
Preserved graph-arm episode evidence: all six read/rg/Git calls completed; provider turn completed, no malformed lines, protocol errors or budget failures. At supervisor_signal the input pipe had EOF and the parent was alive, but checkpoint was null; worker exit was -15, cleanup signals ['SIGTERM'], survivors []. Log ordering: final git child succeeded at at_ms 82917704; supervisor_signal/checkpoint=null at 82917775; final reply seq=8/calls=6 and stdin_eof stop appeared at 82917776; broker_exit -15 at 82917817. The passing baseline has checkpoint {requests:8,calls:6}, exit 0, stderr_eof=true and no cleanup signals.
Source-backed mechanism: eval_broker.py:1089 flushes the MCP response through reply():1108-1109 before publishing reply checkpoint at :1091. fake_codex.py:144 stops the worker after reading its last response; it then closes stdin and signals the supervisor at :157-158. If SIGSTOP lands before checkpoint publication, idle_checkpoint():1253-1272 correctly returns null and supervision refuses orderly drain. This establishes the fixture/protocol ordering race; do not assume every production cancellation is harmless or simply relax the null-checkpoint refusal.
Validation impact: the README-documented opt-in evaluator suite fails with real installed-plugin binaries (17 selected tests, 2 failing subcases, 1 namespace skip). The prescribed CONTRIBUTING.md Cargo validation is unaffected: 705 nextest tests and 14 doctests passed. This is not a failure blocking make ci, but it blocks reliable qualification of the opt-in evaluator checks.
Production impact: normal graph CLI, plugin CLI/MCP conformance, and 64 hands-on CLI checks passed. The observation proves evaluator/test-fixture instability; the same flush-before-checkpoint ordering could affect a real client that tears down immediately after its final response, but that production consequence is not yet demonstrated. Preserve fail-closed cancellation and historical study outcomes.
Environment: Linux 6.8.0-142-generic x86_64, Python 3.12.3, Rust 1.96.0, Orbit 0.25.1 binary SHA256 f6ee64107973b148557a3318ae8c4bb6b4530c305160c677bd0d7b8ff318f360; orbit-graph 0.10.0/extractor24 binary SHA256 8d9453830572c0bf423ece1c4b67d531a8a0a943ae448906d94d685cfcc3e5fd. The failing fixture explicitly uses containment none, so the host's unavailable bwrap namespaces do not explain this failure.
Related landed work: lifecycle repair ORB-13793 (46009e6); pagination repair ORB-13799 (integrated cf4ea61); later MCP transport repair ORB-13802 (3d8b80c). The exact introducing revision has not been isolated. Searches of open and closed tasks for "minimum budget", "broker worker", "eof_then_sigterm" found only completed related work; no open owner covers this race.
Evidence is retained by QA task ORB-11535 under qa/evaluation-real-product.json, qa/budget-baseline-reproduction.json and qa/budget-reproduction-evidence.json. Scope a focused lifecycle/fixture ordering repair, add a deterministic interleaving regression through the real stdio entry point, and retain negative cancellation/pending-request/stalled-worker/parent-death proof. Do not modify historical captures, locks or reports, run model episodes, release, or install on live hosts.
Acceptance Criteria
Execution Summary
Click to expand
Repair: eval_broker.Broker.reply() now records the request's
replycheckpoint after wire redaction and immediately before writing/flushing the MCP response (previously flush-then-record). A client holding its final response therefore always finds a published idle checkpoint, removing the scheduling-dependent null-checkpoint/broker_failed race. Classifier (idle_checkpoint, eof_idle_signal, orderly_broker_exit, runner complete_broker_lifecycle), lifecycle contract eof-idle-at-term-observation-v2, schemas and runner/broker version strings are unchanged; null-checkpoint refusal was NOT relaxed.Criteria:
stdout.flush()for request 42; the client reads the reply, closes stdin, TERMs the supervisor, waits for the logged supervisor_signal, SIGCONTs (kernel state ordering, no sleeps/retries). On baseline HEAD 17d8203 it fails deterministically (checkpoint null, input_eof true, cleanup signals [SIGTERM], stopped=cancelled -> supervisor exit 1; evidence interleaving-evidence.log, baseline-interleaving-test.log). After repair it passes with checkpoint {requests:2,calls:1}, no cleanup signals, exit 0, orderly exit and complete ledger.stdout.flush()(old anchor line moved); it also failed on baseline at that anchor, same race.Validation:
cargo deny --locked check: FAILED environmentally (advisory DB lock on read-only ~/.cargo/advisory-dbs); alternativecargo deny --locked check bans licenses sourcesPASSED, advisories not run.cargo nextest run: NOT RUN (installed nextest 0.9.136 < required 0.9.146; no global install); alternativecargo test --workspace --lockedPASSED 719, 0 failed, 3 ignored. No Rust files changed.Provenance/deviation: following ORB-13802 precedent, BROKER_VERSION/RUNNER_VERSION/contract were not bumped (study-v2 code pins runner 5/broker 4); the change is distinguished by the recorded broker sha256/harness hashes and documented in scripts/agent-eval/README.md 'Reply checkpoint ordering' (earlier captures keep post-flush reply meaning and outcomes; freeze new hashes before use). Reviewer may prefer an explicit version bump. Semantic note: at TERM observation a reply checkpoint now means 'work complete, response published, write pending' — the drain may finish writing to a still-open pipe, equivalent to the previously accepted flushed-but-unread case; broken/full pipe or expired drain still fails. No actual-Codex/model rehearsal was run.
Reconciliation: delivery paths M scripts/agent-eval/eval_broker.py, M scripts/agent-eval/tests/test_broker.py, M scripts/agent-eval/README.md (selector added; all 5 prior selectors retained, verified by re-read). No untracked files outside ignored .orbit/tmp scratch; fake_codex.py, test_runner.py, test_reply_provenance.py unchanged. Artifacts under jrun-20261004-2219-c3/.
Validation
Branch Freshness
origin/agent-mainorbit/ORB-13847-724606ac