Fix exact-head readiness when status rows omit SHA

[justin808]

Summary

• accept GitHub commit-status rows that omit a row-level SHA when the combined-status response is bound to the exact PR head
• fail closed when an optional row SHA contradicts the exact-head envelope
• add a deterministic replay of the shakacode/hichee-data#431 incident and negative controls

Closes #303.

Validation

• ruby skills/pr-batch/bin/pr-ci-readiness-test.rb — 107 runs, 607 assertions, 0 failures
• focused incident replay — 3 runs, 32 assertions, 0 failures
• HiChee Data #431-shaped missing-row-SHA replay — READY
• wrong combined-status ref — UNKNOWN
• contradictory row SHA — UNKNOWN
• ruby -c on both changed files — Syntax OK
• git diff --check — passed
• bin/validate — all cohorts through solution docs and review-finding schema passed; the first run hit one unrelated timing-fixture error that passed immediately in isolation, and the clean rerun passed that drift cohort before the installer harness deadlocked alongside concurrent installer validators. Exact-head hosted CI remains required before merge.

QA

The independent aw-merge-qa lane is required and pending. It will replay the incident and negative controls against this exact head before merge.

Codex Decision Log

• Non-blocking: Whether to treat a present empty sha field as equivalent to an omitted field.
  • Decision: Only an omitted row-level sha is accepted; a present value must equal the exact-head envelope.
  • Why: This preserves fail-closed contradiction handling while matching GitHub's observed omission behavior.
  • Review later: None.

Confidence note: High for the scoped status-inventory fix and deterministic controls; merge remains blocked on exact-head CI, independent current-head review, QA replay, unresolved-thread inventory, autonomous eligibility, and merge assurance.

Summary by CodeRabbit

• Bug Fixes

  • Improved commit-status validation to accept status rows without individual commit identifiers when the overall response confirms the requested commit.
  • Continued rejecting mismatched, missing, or contradictory commit references to prevent inaccurate readiness results.

• Tests

  • Added regression coverage for valid and invalid commit-status response scenarios.
  • Added verification for normalized CI evidence from supported integrations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 5825552c-f1c6-4e05-9df5-ff2afd9c2480

📥 Commits

Reviewing files that changed from the base of the PR and between 96a6d1d and 2bc274d.

📒 Files selected for processing (2)

• skills/pr-batch/bin/pr-ci-readiness
• skills/pr-batch/bin/pr-ci-readiness-test.rb

---

Walkthrough

The readiness validator now accepts status rows without sha when the combined-status response binds evidence to the requested head. It rejects conflicting SHAs. Regression tests replay exact-head ready, wrong-ref, contradictory-SHA, and matching-SHA cases.

Changes

Exact-head status validation

Layer / File(s)	Summary
Optional status-row SHA validation skills/pr-batch/bin/pr-ci-readiness	Status rows may omit sha. Provided values must match the requested head.
Exact-head regression coverage skills/pr-batch/bin/pr-ci-readiness-test.rb	Tests replay shakacode/hichee-data#431 and validate missing, wrong, contradictory, and matching head bindings.
Estimated code review effort: 2 (Simple)	~10 minutes

Possibly related PRs

• shakacode/agent-workflows#279: Extends the same exact-head CI evidence validation.
• shakacode/agent-workflows#285: Covers combined-status exact-head validation related to this change.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the primary change: accepting exact-head readiness when status rows omit SHA.
Linked Issues check	✅ Passed	The implementation and regression tests satisfy issue #303 by binding omitted-SHA status rows to the exact head while preserving fail-closed controls.
Out of Scope Changes check	✅ Passed	The changes are limited to exact-head status validation and focused regression coverage required by issue #303.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch jg-codex/issue-303-status-row-sha

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Review

Overview

Fixes exact-head readiness so combined-status rows that omit a row-level sha (as GitHub does in the wild, per the shakacode/hichee-data#431 incident) are accepted, while a row that does include a sha contradicting the exact-head envelope still fails closed. Adds a deterministic replay of the incident plus two negative-control tests.

Code quality

• The fix is small, surgical, and consistent with the existing ensure_exact_head! fail-closed idiom used elsewhere in this file (fetch_exact_head_check_runs, envelope-level sha binding in fetch_exact_head_statuses).
• The new comment above fetch_exact_head_statuses clearly documents why the check is conditional on row.key?("sha") rather than requiring it — good, since the surrounding code otherwise treats missing identity fields as a hard failure.
• Logic check: row.key?("sha") && row["sha"].to_s != head_sha correctly implements "absent → accept, present-and-equal → accept, present-and-different → reject." Confirmed by tracing fetch_paginated_collection → evidence_scope → inventory_scopes, where other_rows merges check_runs + statuses; the test's expected row shape (status/conclusion keys for check runs, state for statuses) matches normalize_actions_row/the inline status hash exactly.

Tests

• test_hichee_data_431_status_row_without_sha_replays_ready is a solid deterministic replay of the real incident (Azure check-runs + sha-less CodeRabbit status → READY).
• test_hichee_data_431_wrong_combined_status_ref_remains_unknown and test_hichee_data_431_status_row_with_contradictory_sha_remains_unknown cover the envelope-level and row-level negative controls respectively — good to see both fail-closed paths distinguished.
• Left one inline nit: there's no positive-control test for a row whose sha is present and matches head_sha (the "accept" side of the new conditional when the key does exist). Cheap to add and would guard the branch from silent regressions.

Risk / security

• No security-relevant surface here — pure JSON field validation on GitHub API responses, no shell/eval/injection paths introduced.
• Behavior change is appropriately conservative: it only widens acceptance for the specific documented omission case, and preserves (arguably strengthens, via the new contradiction check) fail-closed behavior everywhere else.

Nit

• The error message's actual.empty? ? 'missing' : actual collapses an explicit empty-string sha and a truly absent field into the same "missing" wording. This mirrors the existing ensure_exact_head! helper's phrasing, so it's consistent with the codebase rather than a new issue — just flagging as a pre-existing minor ambiguity, not blocking.

Overall: well-scoped bug fix with good regression coverage tied to a real incident. No blocking issues found.

[justin808]

Address-review pass for PR #307.

Mattered

• No must-fix or discuss-before-merge findings were present in the completed review cohort.

Optional

• Added the requested permanent positive control for a status row whose SHA matches the exact PR head in commit 858d3d99b604686d3f5e714910a5c765a33cf855.
• Replayed the focused incident/negative/control set: 4 runs, 37 assertions, 0 failures.
• Replayed the full helper suite: 108 runs, 612 assertions, 0 failures.
• Replied on and resolved Fix exact-head readiness when status rows omit SHA #307 (comment).

Skipped

• CodeRabbit's rate-limit notice was a terminal reviewer-capacity status, not an actionable code finding.
• Claude's general positive review contained no additional actionable finding.

Current-head CI and independent QA are being rerun because the optional-test commit changed the exact head.

[claude]

Review: Fix exact-head readiness when status rows omit SHA

Overview

The PR fixes fetch_exact_head_statuses in pr-ci-readiness so a commit-status row that omits a row-level sha is treated as acceptable when the combined-status envelope already asserts the exact head (via its top-level sha), rather than being (mis)treated as missing/invalid. If a row does include a sha, it must now match the exact-head envelope or the whole scope fails closed with UNKNOWN. This directly addresses the shakacode/hichee-data#431 incident where a CodeRabbit status row without sha incorrectly caused an UNKNOWN verdict.

Code quality

• The fix is small, surgical, and well-scoped — it only changes the row validation branch inside fetch_exact_head_statuses; no unrelated refactors.
• The added code comment above the method (skills/pr-batch/bin/pr-ci-readiness:700-701) clearly documents the "omitted is fine, present-and-wrong is not" contract, which is important context that isn't obvious from the code alone.
• Naming and style match the surrounding code (raise Error, "..." pattern, .to_s normalization, empty? ? 'missing' : actual phrasing) consistent with ensure_exact_head!.

Correctness

• The core logic is sound: row.key?("sha") && row["sha"].to_s != head_sha correctly distinguishes "key absent" (allowed) from "key present with a different value" (rejected), which is exactly GitHub's observed behavior for the combined-status endpoint's per-row shape.
• The combined-envelope-level sha check (ensure_exact_head! with sha_key: "sha") is untouched and remains the primary binding assertion — the new row-level check is additive/defense-in-depth, not a replacement.
• Left one minor inline nit: when a row's sha key is present but explicitly null/empty, the error message says "found missing," which is slightly confusing given the key was technically present. Not a bug — fail-closed behavior is still correct — just a minor message-wording nit inherited from the existing ensure_exact_head! convention.

Test coverage

• Good coverage of the new behavior:
  • test_hichee_data_431_status_row_without_sha_replays_ready — deterministic replay of the actual incident (Azure check runs + sha-less CodeRabbit status) resulting in READY.
  • test_hichee_data_431_status_row_with_contradictory_sha_remains_unknown — negative control proving a mismatched row sha still fails closed with a descriptive error.
  • test_hichee_data_431_status_row_with_matching_sha_remains_ready — positive control proving a row sha that matches the head still passes.
  • test_hichee_data_431_wrong_combined_status_ref_remains_unknown (renamed/updated from the old "for another commit" test) and test_combined_commit_status_without_sha_fails_closed continue to validate the envelope-level assertion is untouched.
• The renamed tests (e.g., test_commit_status_bearing_head_yields_complete_other_scope_and_known_verdict → test_hichee_data_431_status_row_without_sha_replays_ready) are more descriptive and tie directly to the incident being replayed — a nice improvement in test discoverability. I confirmed no stale references to the old test names remain.

Security / performance

• No security concerns — this is a validation-tightening change (adds a new fail-closed check), which if anything improves the security posture by preventing a false READY verdict from being silently accepted when status data is internally contradictory.
• Performance impact is negligible: one extra key lookup and string comparison per status row, inside an existing O(n) loop.

Risk

• Low risk. The change only affects the other (commit-status) scope's validation path, is backed by both positive and negative deterministic tests, and doesn't alter the Actions/check-run scopes at all.

Overall: solid, well-tested fix with good incident-driven regression coverage. No blocking issues found.

[justin808]

Final-candidate address-review pass for PR #307 at 858d3d99b604686d3f5e714910a5c765a33cf855.

Mattered

• No must-fix or discuss-before-merge findings.

Optional

• No further code changes. The new current-head review independently confirmed the missing, matching, and contradictory row-SHA behavior and found the change low risk.

Skipped

• Kept the explicit-empty-SHA error wording unchanged because it follows the existing ensure_exact_head! convention and still fails closed. Replied with that rationale and resolved Fix exact-head readiness when status rows omit SHA #307 (comment).
• CodeRabbit's rate-limit notice remains a terminal reviewer-capacity status, not an actionable code finding.

All current review threads are resolved. Hosted validation and the independent QA replay remain separate merge gates.

[justin808]

QA Evidence

• QA lane: /root/aw_merge_qa; fresh gpt-5.6-sol/xhigh checker in a clean detached exact-head worktree
• Tested head: 858d3d99b604686d3f5e714910a5c765a33cf855
• Scope: exact-head combined-status envelope/row identity, omitted/matching/contradictory row SHA, incident replay, pagination/deduplication, malformed input, portability, and final current-head review/thread state
• Automated checks: Ruby syntax; focused four-case replay 4 runs / 37 assertions; full helper suite 108 runs / 612 assertions; git diff --check
• Manual checks: adversarial source/diff inspection and current-head review/thread inspection
• Findings: none; the only wording nit was correctly classified as non-actionable and resolved
• Release-blocking QA status: clear
• Hosted validate remains a separate merge gate

[claude]

Review: Fix exact-head readiness when status rows omit SHA

Overview

This PR fixes fetch_exact_head_statuses in skills/pr-batch/bin/pr-ci-readiness so it stops requiring a per-row sha on commit-status rows (which GitHub's combined-status endpoint normally omits) while still failing closed if a row does carry a sha that contradicts the already-verified exact-head envelope. It closes #303, and adds a deterministic replay of the hichee-data#431 incident plus negative controls (wrong combined-status ref, contradictory row sha, matching row sha).

Code quality

• The fix is minimal and correctly scoped: the exact-head guarantee still comes from the combined-status envelope's top-level sha (verified earlier via ensure_exact_head!); the new check only rejects a row-level sha that contradicts that already-verified envelope, rather than requiring one.
• The added comment above fetch_exact_head_statuses clearly documents the reasoning and complements the existing method-level comment.
• Error message reuses the same "(found missing/actual)" idiom as ensure_exact_head!, keeping style consistent.

Correctness

• Logic matches the documented "Codex Decision Log" entry: only an omitted sha key is accepted; if the key is present (including an explicit null, since row.key?("sha") is true and nil.to_s == ""), it must equal head_sha or the request fails closed. This is a deliberate, documented choice and looks correct for the stated fail-closed goal.
• Good test coverage for the new branch: omitted-sha (READY), wrong top-level ref (UNKNOWN, pre-existing), contradictory row sha (UNKNOWN, new), and matching row sha (READY, new).

Minor notes (non-blocking)

• Several previously generic regression tests (e.g. test_commit_status_bearing_head_yields_complete_other_scope_and_known_verdict, test_combined_commit_status_for_another_commit_fails_closed) were renamed/reworked into incident-specific tests (test_hichee_data_431_*) with more elaborate fixtures (Azure check-runs, pr_identity). This ties the tests tightly to one incident's shape; consider whether a lighter-weight generic case alongside the incident replay would keep the original regression intent easy to find, though this is a style preference rather than a defect.
• The "found missing" wording when a row supplies an explicit empty/null sha is slightly misleading (it's technically present, just empty) — matches existing convention from ensure_exact_head! though, so it's consistent rather than a new issue.

Security / performance

• No security or performance concerns — this only tightens/loosens a validation check on data already fetched; no new external calls or trust boundaries introduced.

Overall: a well-scoped, well-tested fix with clear rationale. No blocking issues found.

[justin808]

Final-candidate address-review pass for PR #307 at 2bc274da6bb553b71e320de7acd6ec5415c7be13.

Mattered

• No must-fix or discuss-before-merge findings.
• Hosted CI's two RuboCop findings were fixed in the exact current head; targeted RuboCop now reports no offenses.

Optional

• No further code changes. Claude and CodeRabbit both reviewed the current head and reported no actionable findings.

Skipped

• The incident-specific test naming preference and explicit-empty-SHA wording preference are non-defects. The four-case fixture retains generic missing, matching, wrong-envelope, and contradictory controls while tying the reproduction to the real incident.

All current review threads are resolved. Hosted validation and current-head independent QA remain separate merge gates.

[justin808]

QA Evidence

• QA lane: /root/aw_merge_qa; fresh gpt-5.6-sol/xhigh checker in a clean detached exact-head worktree
• Tested head: 2bc274da6bb553b71e320de7acd6ec5415c7be13
• Scope: exact-head status identity, four incident/control cases, behavior-neutral RuboCop repair, malformed input, pagination/deduplication, portability, and final review/thread state
• Automated checks: direct RuboCop, Ruby syntax, focused 4/37, full suite 108/612, and git diff --check
• Manual checks: adversarial full diff and lint-only delta inspection
• Findings: none
• Release-blocking QA status: clear
• Hosted validate remains a separate merge gate

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2bc274da6b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Replace the consumer-specific replay fixture

This shared-skill test hardcodes the hichee-data consumer branch (upgrade-rails), and the replay also embeds that repository's Azure/CodeRabbit labels. Convert these to generic fixture identifiers so consumer-specific policy does not become part of the portable workflow pack.

AGENTS.md reference: AGENTS.md:L17-L18

Useful? React with 👍 / 👎.