Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
848 changes: 848 additions & 0 deletions bin/fm-crosscheck-autostart.py

Large diffs are not rendered by default.

32 changes: 29 additions & 3 deletions bin/fm-crosscheck.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
#!/usr/bin/env python3
"""Fail-closed independent review ledger bound to an exact pull-request head."""
"""Fail-closed independent review ledger bound to an exact pull-request head.

The public `run TASK URL` surface resolves the live head itself.
The PR-registration coordinator additionally passes `--expected-head SHA` so a head change between registration and launch refuses before reviewer or Azure spending.
"""

from __future__ import annotations

Expand Down Expand Up @@ -6954,7 +6958,13 @@ def write_ledger(path: Path, ledger: dict[str, Any]) -> None:
atomic_write(path, encoded)


def run_crosscheck(root: Path, home: Path, task_id: str, url: str) -> int:
def run_crosscheck(
root: Path,
home: Path,
task_id: str,
url: str,
expected_head: str | None = None,
) -> int:
# C1 (docs/azure-requirements.md): the invocation's clock starts here, so
# the recorded `total` covers everything the caller waits for, including
# the unattributed gaps between the named phases.
Expand Down Expand Up @@ -6985,6 +6995,11 @@ def run_crosscheck(root: Path, home: Path, task_id: str, url: str) -> int:
snapshot_value = github_snapshot(root, url)
except CrosscheckError as exc:
tool_fail(f"GitHub snapshot preflight failed: {exc}")
if expected_head is not None and snapshot_value["head_sha"] != expected_head:
tool_fail(
"registered PR head changed before Crosscheck launch: expected "
f"{expected_head}, observed {snapshot_value['head_sha']}"
)
with timer.phase("ledger"):
try:
ledger = load_ledger(ledger_path, task_id, url)
Expand Down Expand Up @@ -7796,6 +7811,8 @@ def build_parser() -> argparse.ArgumentParser:
command = subparsers.add_parser(name)
command.add_argument("task_id")
command.add_argument("pr_url")
if name == "run":
command.add_argument("--expected-head")
timings = subparsers.add_parser("timings")
timings.add_argument("task_id")
economics = subparsers.add_parser("economics")
Expand Down Expand Up @@ -7904,7 +7921,16 @@ def main() -> int:
except BlockingIOError:
tool_fail("another crosscheck operation already owns this task")
if args.command == "run":
return run_crosscheck(root, home, args.task_id, args.pr_url)
expected_head = args.expected_head
if expected_head is not None and SHA_RE.fullmatch(expected_head) is None:
tool_fail("expected registered PR head must be one 40-hex SHA")
return run_crosscheck(
root,
home,
args.task_id,
args.pr_url,
expected_head,
)
if args.command == "verify":
return verify_crosscheck(root, home, args.task_id, args.pr_url)
return merge_crosschecked(
Expand Down
6 changes: 4 additions & 2 deletions bin/fm-crosscheck.sh
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,17 @@
# Run or verify the independent exact-head crosscheck ledger for a task PR.
#
# Usage:
# fm-crosscheck.sh run <task-id> <full GitHub PR URL>
# fm-crosscheck.sh run <task-id> <full GitHub PR URL> [--expected-head <SHA>]
# fm-crosscheck.sh verify <task-id> <full GitHub PR URL>
# fm-crosscheck.sh status
# fm-crosscheck.sh timings <task-id>
# fm-crosscheck.sh economics <task-id>
# fm-crosscheck.sh merge <task-id> <full GitHub PR URL> <reviewed SHA> <method> [--allow-queue]
#
# `run` is intentionally independent of no-mistakes so both reviews can be in
# flight together once a PR exists. `verify` is the merge-gate operation: it
# flight together once a PR exists. The task-local PR-registration coordinator
# uses `--expected-head` to refuse a moved head before reviewer or Azure spend.
# `verify` is the merge-gate operation: it
# re-reads live GitHub state, requires the latest attempt for that exact head
# and claims document to be clear, and prints only the reviewed SHA.
# `timings` is the read-only C1 breakdown: it prints the per-phase duration
Expand Down
79 changes: 66 additions & 13 deletions bin/fm-pr-check.sh
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
#!/usr/bin/env bash
# Record a PR-ready task: appends pr=<url> and GitHub's pr_head=<sha> to
# state/<id>.meta when available, then arms the watcher's merge poll by writing
# state/<id>.check.sh, which prints one line when the PR is merged or its lookup
# fails (the watcher's check contract: output = wake, silence = keep sleeping).
# With central Slack config installed, then binds the live PR head to the signed
# launch record created before the task agent started. Issuance failure exits
# nonzero after poll setup.
# Register a PR-ready task: append pr=<url> and GitHub's pr_head=<sha> to
# state/<id>.meta, arm the watcher's merge poll, and asynchronously start the
# independent exact-head Crosscheck review. Registration returns after the
# task-local coordinator is requested; review latency never parks the caller.
# Matching active or CLEAR heads deduplicate, failed/dead coordinators remain
# visible and retryable, and unrelated tasks never share a launcher lock.
# A task-local registration lock orders head capture and publication without
# holding the account metadata lock across the remote lookup.
# With central Slack config installed, binds the live PR head to the signed
# launch record created before the task agent started before requesting review.
# Issuance failure exits nonzero after poll setup.
# Usage: fm-pr-check.sh <task-id> <pr-url>
set -eu

Expand All @@ -26,6 +30,32 @@ META="$STATE/$ID.meta"
LOOKUP_WT=
LOOKUP_GENERATION=
PR_HEAD=
CROSSCHECK_AUTOSTART="$SCRIPT_DIR/fm-crosscheck-autostart.py"
CROSSCHECK_AUTOSTART_ENABLED=1
case "${FM_CROSSCHECK_AUTOSTART_TEST_DISABLE:-}" in
'') ;;
firstmate-pr-check-nonautostart-test-v1)
[ "${FM_TEST_RUNNER_ACTIVE:-}" = firstmate-test-runner-v1 ] || {
echo "error: the Crosscheck autostart test bypass is available only inside the sealed behavior-test runner" >&2
exit 1
}
CROSSCHECK_AUTOSTART_ENABLED=0
;;
*)
echo "error: invalid FM_CROSSCHECK_AUTOSTART_TEST_DISABLE value" >&2
exit 1
;;
esac
REGISTRATION_LOCK=$(fm_account_lock_acquire "$STATE" "$ID" pr-registration \
"PR registration" "${FM_ACCOUNT_META_LOCK_WAIT_SECONDS:-10}") || exit 1
META_LOCK=
release_meta_lock() {
if [ -n "$META_LOCK" ]; then
fm_account_meta_lock_release "$META_LOCK" >/dev/null 2>&1 || true
fi
fm_account_meta_lock_release "$REGISTRATION_LOCK" >/dev/null 2>&1 || true
}
trap release_meta_lock EXIT
META_LOCK=$(fm_account_meta_lock_acquire "$STATE" "$ID") || exit 1
if [ ! -f "$META" ]; then
fm_account_meta_lock_release "$META_LOCK"
Expand Down Expand Up @@ -54,18 +84,17 @@ if [ -z "$LOOKUP_GENERATION" ]; then
exit 1
fi
fi
# Serialize head capture/publication only against other registrations, not
# account-session updates or task retirement during a remote lookup.
fm_account_meta_lock_release "$META_LOCK"
META_LOCK=
if ! PR_HEAD_LOOKUP=$("$FM_ROOT/bin/fm-github-pr.py" head "$URL" 2>&1); then
PR_HEAD_DIAGNOSTIC=$(printf '%s' "$PR_HEAD_LOOKUP" | tr '\r\n' ' ')
printf 'UNREVIEWED: PR head lookup failed: %.500s\n' "$PR_HEAD_DIAGNOSTIC" >&2
exit 1
fi
PR_HEAD=$PR_HEAD_LOOKUP
META_LOCK=$(fm_account_meta_lock_acquire "$STATE" "$ID") || exit 1
release_meta_lock() {
fm_account_meta_lock_release "$META_LOCK" >/dev/null 2>&1 || true
}
trap release_meta_lock EXIT
if [ -f "$META" ]; then
CURRENT_WT=$(fm_account_meta_value "$META" worktree)
CURRENT_GENERATION=$(fm_account_meta_value "$META" generation_id)
Expand All @@ -85,13 +114,26 @@ else
fi
CHECK_TMP=$(mktemp "$STATE/.$ID.check.XXXXXX") || exit 1
printf -v PR_ADAPTER_Q '%q' "$FM_ROOT/bin/fm-github-pr.py"
printf -v CROSSCHECK_AUTOSTART_Q '%q' "$CROSSCHECK_AUTOSTART"
printf -v ID_Q '%q' "$ID"
printf -v URL_Q '%q' "$URL"
printf -v PR_HEAD_Q '%q' "$PR_HEAD"
printf -v GENERATION_Q '%q' "$LOOKUP_GENERATION"
cat > "$CHECK_TMP" <<EOF
if ! state=\$($PR_ADAPTER_Q state $URL_Q 2>&1); then
diagnostic=\$(printf '%s' "\$state" | tr '\r\n' ' ')
printf 'UNREVIEWED: PR state lookup failed: %.500s\n' "\$diagnostic"
exit 0
fi
if [ "\$state" = MERGED ]; then
echo "merged"
exit 0
fi
if ! crosscheck_state=\$($CROSSCHECK_AUTOSTART_Q status $ID_Q $URL_Q $PR_HEAD_Q $GENERATION_Q 2>&1); then
diagnostic=\$(printf '%s' "\$crosscheck_state" | tr '\r\n' ' ')
printf 'UNREVIEWED: Crosscheck autostart failed: %.500s\n' "\$diagnostic"
exit 0
fi
case "\$state" in
OPEN) ;;
MERGED) echo "merged" ;;
Expand All @@ -100,8 +142,6 @@ esac
EOF
chmod +x "$CHECK_TMP"
mv "$CHECK_TMP" "$STATE/$ID.check.sh"
fm_account_meta_lock_release "$META_LOCK"
trap - EXIT
SLACK_CONFIG=${FM_CROSSCHECK_SLACK_CONFIG:-$FM_HOME/config/crosscheck-slack.json}
if [ -f "$SLACK_CONFIG" ]; then
"$FM_ROOT/bin/fm-crosscheck-slack.sh" attest-task \
Expand All @@ -111,3 +151,16 @@ if [ -f "$SLACK_CONFIG" ]; then
}
fi
echo "armed: state/$ID.check.sh polls $URL"
if [ "$CROSSCHECK_AUTOSTART_ENABLED" = 1 ]; then
if CROSSCHECK_AUTOSTART_OUT=$("$CROSSCHECK_AUTOSTART" start \
"$ID" "$URL" "$PR_HEAD" "$LOOKUP_GENERATION" 2>&1); then
printf '%s\n' "$CROSSCHECK_AUTOSTART_OUT"
else
CROSSCHECK_AUTOSTART_DIAGNOSTIC=$(printf '%s' "$CROSSCHECK_AUTOSTART_OUT" | tr '\r\n' ' ')
printf 'UNREVIEWED: Crosscheck autostart launcher failed: %.500s\n' \
"$CROSSCHECK_AUTOSTART_DIAGNOSTIC" >&2
fi
fi
fm_account_meta_lock_release "$META_LOCK"
fm_account_meta_lock_release "$REGISTRATION_LOCK"
trap - EXIT
49 changes: 36 additions & 13 deletions docs/crosscheck.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,40 @@
# Crosscheck

Crosscheck is an on-demand, exact-head PR reviewer. It is independent of
Firstmate task orchestration: any agent or operator that can run the supported
wrapper and read the configured Firstmate home can use it.
Crosscheck is an exact-head PR reviewer that starts automatically when Firstmate registers a PR-ready task and remains available on demand.
It is independent of Firstmate task orchestration: any agent or operator that can run the supported wrapper and read the configured Firstmate home can use it.

Crosscheck does one job. It reviews the current PR head, returns `CLEAR` or
`BLOCKING`, and records cited findings and suspicions against that exact SHA.
Crosscheck does one job.
It reviews the current PR head, returns `CLEAR` or `BLOCKING`, and records cited findings and suspicions against that exact SHA.
It does not rerun CI, manufacture proof scripts, or launch verifier VMs.

## Run it

Use a unique task ID and the full public GitHub PR URL:
The normal Firstmate path is PR-ready registration:

```sh
FM_HOME=/Users/dongkeun/firstmate-home \
bin/fm-pr-check.sh <task-id> \
https://github.com/OWNER/REPO/pull/NUMBER
```

Registration records the live PR head, arms the merge poll, durably requests Crosscheck, starts one task-local coordinator, and returns without waiting for the review.
A matching active request is reused, and a matching exact-head and exact-claims `CLEAR` result is verified without another review.
Registering a new head replaces the queued request so the coordinator reviews that head next.
Registration holds the task metadata lock from head capture through poll emission and request publication, so an older capture cannot replace a newer registration.
A short task-local handoff lock couples request publication, status reconciliation, and coordinator retirement; it is never held during review execution.
A dead or failed coordinator releases its task-local lock and retries when the same registration command runs again.
The merge poll observes live GitHub merge state before reporting launcher failures, so manual completion and merge still trigger cleanup without granting merge authorization.
Unrelated task coordinators share no launcher lock, so the Azure lane-capacity and cost-admission controls remain the only review spending authority.

Before launching a review, the coordinator loads the authoritative operator-private fleet environment from `~/.fm-azure/fleet.env` by default.
`FM_CROSSCHECK_FLEET_ENV` may select another absolute file.
The launcher opens that file without following symlinks and requires a current-operator-owned regular file that is not group or world writable.
It sources the already-open file only inside the Crosscheck child, suppresses output from the source operation, and never copies environment values into argv, prompts, logs, repository files, or launcher records.

Missing, unsafe, or incomplete fleet configuration does not undo or fail PR registration.
The task remains honestly uncleared, the actionable failure is recorded in `state/<task-id>.crosscheck-autostart.json` and `state/<task-id>.crosscheck-autostart.log`, and the task check surfaces it for repair and retry.

For an explicit on-demand run, use a unique task ID and the full public GitHub PR URL:

```sh
set -a
Expand All @@ -21,15 +45,14 @@ FM_HOME=/Users/dongkeun/firstmate-home \
https://github.com/OWNER/REPO/pull/NUMBER
```

The fleet environment is operator-private Azure configuration. Load it into the
process environment; never paste its values into a prompt or command.
The fleet environment is operator-private Azure configuration.
Load it into the process environment; never paste its values into a prompt or command.

A new task ID needs no pre-created metadata file. Existing state must match the
same task and PR identity or the run fails closed.
A new task ID needs no pre-created metadata file.
Existing state must match the same task and PR identity or the run fails closed.

The command exits zero only for a valid `CLEAR` verdict on the live head. A
finding, unresolved suspicion, stale head, provider failure, malformed verdict,
or infrastructure failure exits nonzero and is never presented as clearance.
The command exits zero only for a valid `CLEAR` verdict on the live head.
A finding, unresolved suspicion, stale head, provider failure, malformed verdict, or infrastructure failure exits nonzero and is never presented as clearance.

Results are written to:

Expand Down
2 changes: 1 addition & 1 deletion docs/scripts.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ The shared no-mistakes gate refusal used by every directly invocable mutating co
| `fm-crosscheck-slack.sh` | Run, preflight, selftest, or issue exact-head provenance for the Slack Crosscheck lane |
| `fm-crosscheck-slack.py` | Serve allowlisted, metered, exact-head Slack reviews through the shared core lanes |
| `fm-crosscheck-slack-service.sh` | Install and operate the credential-free macOS launchd wrapper for the central listener |
| `fm-pr-check.sh` | Record `pr=` and `pr_head=` for a PR-ready task, then arm the watcher's merge poll |
| `fm-pr-check.sh` | Register a PR-ready task; see [Crosscheck operator flow](crosscheck.md#run-it) |
| `fm-pr-merge.sh` | Require exact-head crosscheck, record PR metadata, and atomically merge or enqueue the reviewed SHA |
| `fm-promote.sh` | Promote a scout task in place to a protected ship task |
| `fm-report-contract-lib.sh` | Render the shared ship completion-report contract inserted into briefs and continuation prompts |
Expand Down
1 change: 1 addition & 0 deletions tests/behavior-test-durations.tsv
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
4000 tests/fm-pi-refresh.test.sh
3750 tests/fm-pi-retry-continuity.test.sh
3170 tests/fm-pi-watch-extension.test.sh
14000 tests/fm-pr-crosscheck-autostart.test.sh
1574 tests/fm-pr-merge.test.sh
6030 tests/fm-process-tree.test.sh
802 tests/fm-prompt-exec.test.sh
Expand Down
32 changes: 32 additions & 0 deletions tests/fm-crosscheck-slack.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1276,6 +1276,38 @@ with Path({str(launch_log)!r}).open('a') as handle:
sys.exit(1 if sys.argv[1] == 'print' else 0)
""")
launchctl.chmod(0o755)
# Model the macOS plist editor on every host; the emitted plist is parsed below
# and its actual service command is executed with the emitted environment.
plutil = bin_dir / "plutil"
plutil.write_text(f"#!{sys.executable}\n" + """
import plistlib
from pathlib import Path
import sys
args = sys.argv[1:]
path = Path(args[-1])
if args[0] == '-create':
value = {}
else:
value = plistlib.loads(path.read_bytes())
parts = args[1].split('.')
parent = value
for part in parts[:-1]:
parent = parent[int(part)] if isinstance(parent, list) else parent[part]
kind = args[2]
item = {'-array': [], '-dictionary': {}}.get(kind)
if kind == '-string':
item = args[3]
elif kind == '-bool':
item = args[3] == 'true'
elif kind == '-integer':
item = int(args[3])
if isinstance(parent, list):
parent.insert(int(parts[-1]), item)
else:
parent[parts[-1]] = item
path.write_bytes(plistlib.dumps(value))
""")
plutil.chmod(0o755)
environment = dict(os.environ, HOME=str(home), FM_HOME=str(fm_home), FM_ROOT_OVERRIDE=str(fixture), FM_CROSSCHECK_SLACK_CONFIG=str(config_path), FM_CROSSCHECK_PYTHON=sys.executable, PATH=str(bin_dir) + os.pathsep + os.environ['PATH'])
for name in ('app_token_env', 'bot_token_env', 'github_token_env'):
environment[config[name]] = 'fixture-inherited-secret'
Expand Down
26 changes: 26 additions & 0 deletions tests/fm-crosscheck.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -3462,6 +3462,30 @@ assert run["state"] == "clear"
pass "a pipeline-updated PR is reviewed at its exact remote head while the author worktree remains behind"
}

test_registered_expected_head_refuses_a_moved_head_before_spend() {
local record case_dir base head expected rc
record=$(make_case registered-head-moved)
IFS=$'\t' read -r case_dir base head <<< "$record"
expected=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
[ "$expected" != "$head" ] || fail "expected-head fixture did not move"
set +e
run_case "$case_dir" "$base" "$head" clear run --expected-head "$expected" \
> "$case_dir/out" 2> "$case_dir/err"
rc=$?
set -e
expect_code 1 "$rc" "moved registered head"
assert_grep "registered PR head changed before Crosscheck launch: expected $expected, observed $head" \
"$case_dir/err" \
"moved registered head did not produce the exact pre-spend diagnostic"
assert_absent "$case_dir/codex.log" \
"reviewer launched after the registered exact head changed"
assert_absent "$case_dir/pi.log" \
"Pi reviewer launched after the registered exact head changed"
assert_absent "$case_dir/data/task-x1/crosscheck-ledger.json" \
"head mismatch fabricated a durable review attempt"
pass "a moved registered head refuses before reviewer or Azure spending"
}

test_missing_pr_head_ref_fails_closed() {
local record case_dir base head rc
record=$(make_case missing-pr-head-ref)
Expand Down Expand Up @@ -6691,6 +6715,7 @@ if [ -n "${FM_TEST_CASE:-}" ]; then
test_missing_metadata_for_existing_task_fails_closed|\
test_existing_task_metadata_identity_collision_fails_closed|\
test_review_fetches_exact_pr_head_when_author_worktree_is_behind|\
test_registered_expected_head_refuses_a_moved_head_before_spend|\
test_missing_pr_head_ref_fails_closed|\
test_codex_reviewer_requires_bound_auth_and_clears_ambient_credentials|\
test_launcher_requires_supported_python|\
Expand Down Expand Up @@ -6845,6 +6870,7 @@ test_mismatched_state_without_metadata_fails_closed
test_missing_metadata_for_existing_task_fails_closed
test_existing_task_metadata_identity_collision_fails_closed
test_review_fetches_exact_pr_head_when_author_worktree_is_behind
test_registered_expected_head_refuses_a_moved_head_before_spend
test_missing_pr_head_ref_fails_closed
test_codex_reviewer_requires_bound_auth_and_clears_ambient_credentials
test_null_ledger_fails_without_normalization
Expand Down
Loading
Loading