fix: automatically start Crosscheck reviews on PR registration - #373
Merged
Conversation
…thoritative documentation pointer
ruby-dlee
force-pushed
the
fm/crosscheck-autostart-g4
branch
from
August 27, 2026 04:53
27c88c0 to
f1165f8
Compare
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.
Intent
Fix the review-ready merge flow so the normal PR registration command automatically starts the required exact-head independent Crosscheck review rather than only arming a merge poll and leaving the gate blocked until a separate manual command. Registration must return promptly while the review runs asynchronously. Repeated starts for the same task, PR URL, and head must reuse a live matching review, skip a matching CLEAR result, and allow retry after a dead or failed launcher; a changed PR head must start a fresh review. Reviews must be isolated per task/head with no global serialization, preserving Azure lane capacity and cost admission. Missing, unsafe, or incomplete private Azure configuration must not fail PR registration or block unrelated review starts: record an actionable per-task failure, keep the merge gate uncleared, and allow the same command to retry after repair. The launcher must load the authoritative operator-private fleet configuration through an owned safe mechanism without copying secrets into tracked files, prompts, logs, or durable records. Crosscheck must never merge, approve, or weaken exact-current-head CLEAR gating. Add executable behavior regressions for prompt return, matching-run deduplication, launcher-failure retry, new-head restart, and concurrent unrelated starts, avoiding source-substring assertions. Update only the owning docs and script headers, not AGENTS.md. Preserve the concrete single-operator design constraint: non-security launcher failures are visible and retryable rather than globally blocking, while credential custody and spending admission remain fail-closed.
What Changed
Risk Assessment
🚨 High: Captain, the source repairs appear consistent with the requested invariants, but the required semantic-repair proof remains explicitly unverified and contradictory.
Testing
The supplied baseline and focused tests passed. Executable checks covered asynchronous registration, deduplication, configuration and dead-worker retries, changed heads, unrelated concurrent starts, all three race regressions, generated poll consumption and merged detection, plus moved-head refusal before spending. Captured CLI and state evidence uses controlled GitHub/reviewer fixtures; no live Azure review was performed. No UI changed.
Evidence: Registration CLI output, review calls, and persisted coordinator states from controlled fixtures
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
🔧 **Rebase** - 1 issue found → auto-fixed ✅
AGENTS.md- merge conflict rebasing onto origin/main🔧 Fix applied.
✅ Re-checked - no issues remain.
bin/fm-crosscheck-autostart.py:574- A queued head can lose its coordinator: worker A reads the unchanged request at line 574 and decides to exit; registration B then fails to acquire A's still-held lock, writes a new head, and returns success; A closes its lock without reading B's request. No review starts until another registration. This contradicts “a changed PR head must start a fresh review.” Make request publication and coordinator retirement an atomic handoff, and cover this interleaving through the executable interface.bin/fm-pr-check.sh:112- A persisted launcher failure now prevents the check from ever querying GitHub. If the operator subsequently completes Crosscheck through the supported manual command and merges the PR, the autostart record remains failed, so every poll reports the old failure instead of 'merged'. This suppresses the watcher's merged-PR cleanup path. Preserve live merge detection independently of launcher diagnostics; it does not grant merge authorization.🔧 Fix: Fix Crosscheck retirement handoff and live merge detection
4 issues (2 errors, 2 warnings) still open:
bin/fm-crosscheck-autostart.py:642- Concurrent registrations can replace a newer head with an older one. Registration A resolves head H1 and pauses before start; B resolves H2 and starts its coordinator; A resumes and unconditionally publishes H1 here. The coordinator then attempts H1, refuses because GitHub is at H2, and retires without reviewing H2. This contradicts “a changed PR head must start a fresh review.” Preserve registration ordering across head capture and request publication, and add an executable regression for this interleaving.bin/fm-crosscheck-autostart.py:762- Status can overwrite successful completion with a permanent false failure: it reads a running record, the worker writes clear and releases its lock, then status observes the unlocked coordinator and writes failed using its stale record. Subsequent polls keep reporting failure until registration is repeated. Serialize status reconciliation with coordinator retirement and re-read the record before changing it.tests/fm-pr-crosscheck-autostart.test.sh:440- The supplied repair evidence does not establish the required fail-before/pass-after result: both executions exited zero, and repair_complete is false. The retirement regression exists, but its execution against the pre-fix implementation has not been demonstrated. Have the outer executor run the new regression against both implementations and resolve the unverified proof before accepting the repair.bin/fm-crosscheck-autostart.py- The fixer could not prove the semantic repair with a public/executable fail-before/pass-after regression and relevant integration or consumer compatibility evidence; primary-agent handoff is required.🔧 Fix: Preserve Crosscheck registration order and completion state
2 issues (1 error, 1 warning) still open:
tests/fm-pr-crosscheck-autostart.test.sh:440- The repair proof remains contradictory: executor evidence reports repair_complete=false, proof_status=unverified, and exit=0 for both before and after, while state/review-proof logs show targeted failures and passes without recording their implementation revisions or exit codes. Have the outer executor reconcile and verify the targeted fail-before/pass-after evidence before accepting the semantic repair; the generated polls are correctly confined to test-local state.bin/fm-crosscheck-autostart.py- The fixer could not prove the semantic repair with a public/executable fail-before/pass-after regression and relevant integration or consumer compatibility evidence; primary-agent handoff is required.✅ **Test** - passed
✅ No issues found.
if [ "${FM_AZURE_VALIDATION_CELL:-0}" = 1 ]; then exec "$FM_AZURE_VALIDATION_SHARD_BRIDGE" behavior --count "${FM_AZURE_VALIDATION_SHARD_COUNT:-8}"; else exec bin/fm-no-mistakes-test-command.sh; fiConfigured baseline command previously passed, as supplied by the outer executor.TMPDIR="$PWD/state/local-test-tmp" tests/run.sh tests/fm-pr-crosscheck-autostart.test.shTMPDIR="$PWD/state/local-test-tmp" FM_TEST_CASE=test_registered_expected_head_refuses_a_moved_head_before_spend tests/run.sh tests/fm-crosscheck.test.shCaptured generated registration output, review invocation records, and persisted coordinator state during execution.git status --shortconfirmed a clean worktree after cleanup.✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.