Skip to content

Add read-only PR evidence-to-head binding - #203

Open
saagpatel wants to merge 3 commits into
mainfrom
codex/github-pr-head-binder
Open

Add read-only PR evidence-to-head binding#203
saagpatel wants to merge 3 commits into
mainfrom
codex/github-pr-head-binder

Conversation

@saagpatel

@saagpatel saagpatel commented Jul 29, 2026

Copy link
Copy Markdown
Owner

What

Adds a fixture-first, read-only operator capability for determining whether supplied pull request approvals and checks apply to the supplied current head.

  • defines strict PRHeadEvidenceV1 input and deterministic PRHeadEvidenceVerdictV1 output
  • adds the token-free audit pr-evidence CLI path
  • classifies review state, dismissal, actor eligibility, commit binding, check status, conclusion, and head binding
  • derives approval and check requirements only from supplied branch-protection or ruleset data
  • returns explicit stale, blocked, pending, missing, neutral, skipped, and unknown states
  • adds semantic fixture triplets, CLI fixtures, documentation, and changelog coverage

Why

Stale approvals, comments, dismissed reviews, and successful checks from older SHAs must not appear as current evidence for a newer pull request head. Missing rules, incomplete pagination, permission gaps, and malformed snapshots must fail closed instead of silently becoming green.

Review Of What Was Built

  • a pure local evaluator with no GitHub requests or output-file writes
  • SHA binding for reviews, check runs, and check suites
  • latest-reviewable-push approval semantics
  • case-insensitive actor identity handling and explicit rejection of PR-author approvals
  • chronological review ordering across timezone offsets
  • deterministic JSON plus exit codes for current, known non-current, and unknown outcomes
  • preserved separation from portfolio scoring and generated PortfolioTruth

Cleanup Review

  • did not add live API ingestion because it is not needed to prove the capability
  • did not add GitHub mutation paths, review submission, comments, reruns, branch updates, merges, or ruleset writes
  • did not regenerate or modify portfolio truth
  • kept the feature inside the existing CLI rather than creating another command center

Verification Summary

Verification below is bound to head 08a57f2c34b8fc47a657cafc4a65a5495c0e1ea6.

  • uv run pytest -q tests/test_pr_head_evidence.py tests/test_pr_head_evidence_cli.py — 37 passed
  • uv run pytest -q — 2,988 passed, 2 skipped
  • uv run ruff check . — passed
  • changed-file ruff format --check — passed
  • changed-module mypy — passed
  • current, stale, comment-only, dismissed, incomplete, and malformed CLI smokes — expected states and exit codes
  • repeat-run JSON comparison — byte-identical
  • adversarial identity cases — case variants count once, the latest pusher cannot self-satisfy, and the PR author cannot count
  • mixed-timezone adversarial cases — latest decisive reviews and latest-push approvers are ordered by chronological instant
  • current-head GitHub checks — Python 3.11, Python 3.12, CodeQL analysis, and CodeQL summary all successful

Repository-wide format and mypy commands still expose unchanged pre-existing debt outside this diff; the feature-owned files and modules pass their scoped gates.

Shipped Summary

The branch now has a documented, versioned, deterministic way to bind supplied PR evidence to a supplied head SHA without credentials, network access, GitHub writes, or portfolio-truth changes.

Next Phase

Obtain an independent approval on the current head before merge. Any live read-only GitHub snapshot collector should remain a separate follow-up and must preserve the same completeness and claim-ceiling rules.

Remaining Roadmap

  • Optional: add a separate read-only live snapshot collector if API coverage can remain complete and permission-honest.

@saagpatel
saagpatel marked this pull request as ready for review July 29, 2026 04:25

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08a57f2c34

ℹ️ 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".

Comment thread src/pr_head_evidence.py
Comment on lines +846 to +850
if review.actor.can_count is None and review.state in {
"APPROVED",
"CHANGES_REQUESTED",
}:
unknown_eligibility = True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore unknown reviewer eligibility when it cannot affect requirements

When the supplied rules require zero approvals and no latest-push approval, any APPROVED or CHANGES_REQUESTED review with can_count=null still sets unknown_eligibility, causing an otherwise fully covered snapshot to exit 2 as unknown even though the verdict simultaneously reports github_requirement_satisfied=true. Only propagate unknown eligibility when that review could change an applicable approval requirement or blocking decision.

AGENTS.md reference: src/AGENTS.md:L17-L19

Useful? React with 👍 / 👎.

Comment thread src/pr_head_evidence.py
Comment on lines +1262 to +1266
"coverage": {
"state": "complete"
if not coverage_reasons and not rule_reasons
else "unknown",
"reasons": sorted(set(coverage_reasons + rule_reasons)),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Include freshness and eligibility gaps in coverage state

For a valid snapshot with provenance.freshness="unknown", this condition emits coverage.state="complete" with no coverage reasons while the top-level state is unknown; an eligibility gap that can affect approval requirements produces the same contradiction. This breaks the documented PRHeadEvidenceVerdictV1 coverage contract and can mislead consumers that inspect the coverage object, so these gaps must participate in the coverage state and reasons.

AGENTS.md reference: src/AGENTS.md:L17-L19

Useful? React with 👍 / 👎.

Comment on lines +25 to +27
try:
raw_text = path.read_text(encoding="utf-8")
except OSError as exc:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Emit the versioned error verdict for invalid UTF-8

If the snapshot contains invalid UTF-8 bytes, Path.read_text() raises UnicodeDecodeError, which is not an OSError, so the command leaks a traceback and emits no JSON instead of the promised deterministic malformed-input verdict with exit code 2. Catch decoding failures here and route them through invalid_snapshot_verdict just like unreadable or syntactically invalid JSON.

AGENTS.md reference: src/AGENTS.md:L17-L19

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant