[jonahgabriel]

Summary

Ticket OMN-20708. Repo omniclaude. This PR rebases the earlier draft omniclaude#2608 onto dev and adds the wiring that draft lacked. The reusable workflow .github/workflows/branch-claim-check-reusable.yml gains a claim_source input: file (default, the existing markdown-ledger path, every store step gated on it) or work-ledger.

Behaviour

With work-ledger the job reads the check "branch-claim / work-ledger" posted on the PR head by omnimarket's node_branch_claim_check_effect (omnimarket#3547), which reads omninode_internal.work_ledger_rows on the lab. Only runs from the GitHub App onexbot-occ-writer are trusted; the latest completed one wins. The job waits up to 900 seconds (input lab_check_wait_seconds), polling every 20 seconds. The first summary line must match "branch-claim-outcome: ...". A missing check, a garbled line or a did-not-run outcome fails with "THE CHECK DID NOT RUN" in both modes. held-elsewhere and fence-behind fail only in refuse mode. unidentified warns. held-by-pusher, unclaimed and no-ticket pass. The job opens no database connection and checks out no ledger.

unclaimed stays clean on purpose, as in tests/scripts/test_branch_claim.py::test_unclaimed_is_clean and as the lab node reports it (a success conclusion). The plan unit's wording "refuses an unclaimed branch" is therefore met by the held-elsewhere and fence-behind refusal in refuse mode, not by refusing a ticket with no claim row.

Folded gate: branch-claim-gate.yml no longer exists on dev, so the earlier draft's edit to it is replaced by ci.yml wiring. The Branch Claim Gate job in ci.yml gets a named step "Branch-claim check workflow reader (OMN-20708)" running tests/scripts/test_branch_claim_check_workflow.py. Its path filter now names the caller, the test and .pre-commit-config.yaml. A check-only pre-commit hook branch-claim-check-workflow runs the same file. Branch Claim Gate is not on the CI Summary soft allowlist, so a red there fails the required CI Summary context. The test no longer skips when jq is missing; it fails. contracts/OMN-20708.yaml added with AC1 (no ledger step runs with work-ledger, and an unheld branch passes with no ledger present), AC2 (held-elsewhere and fence-behind refused in refuse mode; an absent, pending, foreign-app, garbled or did-not-run lab check never passes), AC3 (wired).

Not done here

The caller .github/workflows/branch-claim-check.yml names claim_source: file explicitly (and has checks: read). It is not flipped to work-ledger. On 2026-10-13 none of omniclaude#2708, #2710, #2711 or #2672 carried a "branch-claim / work-ledger" check, so flipping now would make every omniclaude PR wait 900 seconds and fail. The flip is the one claim_source line, to be made once a completed lab check from the writer App is visible on an omniclaude PR head. Removing the file path and its inputs is a later change. required-checks.yaml and branch protection are not touched, so the branch-claim-check context is still not a required context; that is for the operator.

Tests

tests/scripts/test_branch_claim_check_workflow.py has 38 passing cases. Named selections: -k ledger_absent_passes, -k work_ledger_source_runs_no_ledger_step, -k claimed_elsewhere_refused, -k branch_claim_check_workflow_is_wired. With the base (dev d6a31b6) versions of branch-claim-check-reusable.yml, branch-claim-check.yml, ci.yml and .pre-commit-config.yaml restored, those four selections give 2 failed and 3 errors; restored to this branch's versions all pass.

Lab: host=h200 lane=omn20708-3d38 sha=822752b04a8b90aa5fc34b858be4f776a60910f2 command=onex_lab_run.py --via bus --repo omniclaude --ref jonah/omn-20708-branch-claim-work-ledger -- uv run pytest tests/scripts/test_branch_claim_check_workflow.py tests/scripts/test_branch_claim.py tests/scripts/test_branch_claim_hook.py tests/test_ci_full_suite_ignores.py tests/ci/test_ci_summary_gate.py -q --no-cov -p no:randomly; work unit lw-6051a454648d4238; observed 300 passed in 20.80s, exit 0.

[github-actions]

✅ Hostile Reviewer — PASSED

Blocking findings (critical/error): 0
Total findings: 12
Models succeeded: correctness,security-contracts

Findings

Severity	Location	Finding
error	.github/workflows/branch-claim-check-reusable.yml, step 'Read the lab branch-claim check for this head':1	ai-reviewer:correctness:correctness Shell regex uses literal backslash instead of escaped space, causing false failures | The Bash step 'Read the lab branch-claim check for this head' uses the regex ^branch-claim-outcome:\ (held-elsewhere|fence-behind|unidentified|held-by-pusher|unclaimed|no-ticket|did-not-run)\ to validate the first line of the check summary. In Bash, \ inside a double-quoted string is interpreted as a literal backslash character, not a space. The regex therefore requires a literal backslash after 'branch-claim-outcome:
warning	.github/workflows/branch-claim-check-reusable.yml, step 'Read the lab branch-claim check for this head':1	ai-reviewer:correctness:correctness Polling loop deadline calculation uses SECONDS which is not monotonic | The polling loop uses deadline=$((SECONDS + wait_seconds)) and then checks remaining=$((deadline - SECONDS)). The SECONDS variable in Bash is the number of seconds since the shell started, not a monotonic clock. If the system clock is adjusted (NTP sync, etc.), SECONDS can jump, causing the deadline to be miscalculated. More critically, SECONDS resets if the shell is restarted, and in a long-running CI job, the value can becom
warning	.github/workflows/branch-claim-check-reusable.yml, step 'Read the lab branch-claim check for this head':1	ai-reviewer:correctness:security Check run summary is parsed without sanitization, enabling annotation injection | The step extracts summary from the check run's output.summary field and uses it directly in ::error:: and ::warning:: annotations. The summary content comes from the lab node's check run, which is published by the onexbot-occ-writer GitHub App. While the app slug is validated, the summary content is not sanitized. An attacker who compromises the lab node or the writer App could publish a check run with a summary cont
warning	.github/workflows/branch-claim-check-reusable.yml, step 'Read the lab branch-claim check for this head':1	ai-reviewer:correctness:correctness jq sort_by on completed_at with null values produces undefined behavior | The jq expression [.[].check_runs[] | select(.app.slug == $app and .status == "completed") | sort_by(.completed_at) | last // empty sorts by completed_at. If a check run has status: "completed" but completed_at is null (which can happen in edge cases or if the API returns inconsistent data), sort_by(.completed_at) will place nulls first in jq's sort order. This means a completed run with a null completed_at could be select
warning	tests/scripts/test_branch_claim_check_workflow.py, test_outcomes_follow_mode:1	ai-reviewer:correctness:testing Test fixture does not exercise the actual regex quoting bug | The test test_outcomes_follow_mode and related tests run the Bash step via subprocess.run(["bash", "-c", lab_step["run"]] with LAB_CHECK_WAIT_SECONDS set to 0. This executes the actual Bash code, including the regex. However, the test passes because the regex bug (literal backslash instead of space) causes the regex to fail, which makes the step exit 1. The test then asserts result.returncode == expected where expected is 0 for valid ou
info	tests/scripts/test_branch_claim_check_workflow.py:1	ai-reviewer:correctness:completeness No test for the case where the lab check summary is empty string | The jq expression .output.summary // "" defaults to an empty string if output.summary is null or missing. The regex validation will fail on an empty string, which is correct. However, there is no explicit test for this case. The test_garbled_first_line_fails test includes an empty string case, but it is tested via the summary parameter which sets check["output"]["summary"] to the empty string. This is covered, but the case where `o
info	.github/workflows/branch-claim-check-reusable.yml, step 'Read the lab branch-claim check for this head':1	ai-reviewer:correctness:performance Polling loop sleeps 20 seconds between API calls, which may be too coarse for CI | The polling loop sleeps for 20 seconds between API requests. With a default wait of 900 seconds, this means at most 45 API calls. This is reasonable for a long-running lab check, but for CI where the lab check may complete quickly, the 20-second sleep adds unnecessary latency. The first iteration happens immediately, which is good, but subsequent iterations wait 20 seconds. If the lab check completes in 5 seconds, the workflo
warning	.github/workflows/branch-claim-check-reusable.yml:1	ai-reviewer:security-contracts:security Lab check app slug is hardcoded to a single value | The lab_check_app_slug input defaults to onexbot-occ-writer and the test suite hardcodes this same slug. If the lab node's GitHub App is renamed, migrated, or replaced (e.g., during a credential rotation or organizational restructure), the reader will silently reject all valid check runs from the new app slug, causing every PR to fail with 'THE CHECK DID NOT RUN'. There is no mechanism to validate that the configured slug matches the actual app publish
warning	.github/workflows/branch-claim-check-reusable.yml:1	ai-reviewer:security-contracts:correctness Race condition in lab check polling loop | The polling loop uses gh api --paginate which may return partial results if the API is rate-limited or if pagination is incomplete. The loop breaks as soon as a completed check run from the trusted app is found, but if the first page of results does not contain the relevant check run (e.g., due to ordering by completed_at not being guaranteed across pages), the loop may miss it. Additionally, the sort_by(.completed_at) | last logic assumes that the latest com
info	.github/workflows/branch-claim-check-reusable.yml:1	ai-reviewer:security-contracts:performance Inefficient polling interval for lab check | The polling loop sleeps for 20 seconds between API calls, which is excessive for a check that is expected to complete within seconds. This increases the total wait time for PRs that depend on the lab check, potentially delaying CI feedback by up to 20 seconds per polling iteration. For a 900-second wait window, this results in approximately 45 API calls, which may hit rate limits for high-traffic repositories. | Evidence: sleep_for=20 if ((remaining <
info	tests/scripts/test_branch_claim_check_workflow.py:1	ai-reviewer:security-contracts:testing Test does not cover pagination edge cases | The test test_latest_completed_trusted_run_wins_across_pages only tests a two-page scenario with specific data. It does not test scenarios where the relevant check run is on a later page, where pagination is incomplete, or where the API returns an error mid-pagination. This leaves a gap in coverage for real-world API behavior. | Evidence: def test_latest_completed_trusted_run_wins_across_pages(run_lab_step) -> None: older = _check("did-not-run", completed_at
info	contracts/OMN-20708.yaml:1	ai-reviewer:security-contracts:completeness Missing documentation for lab check format | The workflow expects the lab check's output.summary to start with a specific format: branch-claim-outcome: <outcome> ticket=<id> holder=<holder> window=<start>..<end> rows=<n>. However, this format is not documented in the contract file or in any README. If the lab node changes the format, the reader will fail with 'garbled first line', but there is no clear reference for what the expected format is. | Evidence: if [[ ! "$first_line" =~ ^branch-claim-outcome:

---

Gate semantics (pilot phase)

Verdict	Meaning	Blocks merge?
passed	No critical/error findings	No
blocked	CRITICAL or ERROR findings found	Yes
degraded	Reviewer ran but every model failed mid-run	No (pilot)
infra_error	cli_review failed to run at all (install/crash) after 2 attempts	Yes
unavailable_fork_isolation	Fork PR — the LAN-only model is unreachable and the fleet is off limits	Yes

Powered by omniintelligence.review_pairing.cli_review — node-based adversarial review via HandlerLlmCliSubprocess (OMN-8524)