fix(agents): document write-path commit attribution and gate on it (BLO-21416)

[kkroo]

Replacement for app-authored #1087 so the Ally GitHub App can provide the required independent review/approval.

Exact code head copied from #1087: f20119687a35bb4a8c419ff6d223569d41c804b2.

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents write to GitHub through two paths: git push (per-agent git config identity) and the REST/MCP API (contents API, merge API, create_or_update_file/push_files)
• Every agent pod shares one credential — the allyblockcast[bot] GitHub App installation (id 290875700) — and REST commit-creation endpoints default commit.author to the authenticated identity when none is supplied
• So every agent's API-path commit is silently stamped allyblockcast[bot], not the acting agent — authorship becomes unrecoverable from GitHub, which already manufactured one false governance finding (BLO-19528)
• This pull request documents the rule where agents will actually hit it (AGENTS.md §9), gives the one call site we control (gh api ... -f author[name]= -f author[email]=) an explicit-author workaround, and ships an automated verifying signal: a CI gate on every future paperclip PR plus an on-demand cross-repo audit script
• The benefit is that non-merge commit authorship becomes recoverable going forward, and the finding is documented once instead of being re-derived or re-filed by a future run

Linked Issues or Issue Description

Fixes: BLO-21416
Refs: BLO-19528 (blocked by BLO-21416; unaffected by this PR's scope)

What Changed

• AGENTS.md §9: new subsection documenting that commit attribution is write-path dependent, not agent dependent — git push is already correct, the MCP create_or_update_file/push_files tools have no author field and must not be used to land commits, and gh api (the one API call site agents control) must pass an explicit author[name]/author[email] when a raw API write is unavoidable.
• scripts/check-commit-author-attribution.mjs (+ test): shared assertion findAttributionOffenses (flags a non-merge commit's commit.author.email == "290875700+allyblockcast[bot]@users.noreply.github.com") with two front-ends:
  • local mode: git log --no-merges over a base..head range — no network, no new secret.
  • --audit-merged mode: for the last N merged PRs of one or more repos (default Blockcast/trafficcontrol,Blockcast/paperclip), fetch each PR's own commit list via gh api and apply the same assertion — the AC's "automated verifying signal."
  • Merge/squash-merge commits (2+ parents) are excluded everywhere — they're legitimately App-attributed via the merge API, per BLO-21416's scope boundary.
• .github/workflows/pr.yml: wires the local mode into the existing policy job as a new required step on every PR, using the branch's already-checked-out history (no new secret needed). No branch exemption — see review cycle 1 below.

Review cycle 1 — the three Important findings (commit 5d7e2422b)

1. pr.yml graphify exemption (security). Ally flagged that if: github.head_ref != 'bot/graphify-reindex' is forgeable — head_ref carries no repository identity, so any fork can name its branch that and skip the gate — and simultaneously wrong in the other direction, because head_ref is empty in merge_group, so a legitimate graphify PR would be rejected in the merge queue.

I removed the exemption entirely rather than hardening it, because measurement shows it was guarding a rejection that never happens:

identity	who produces it	gate verdict
allyblockcast[bot]@users.noreply.github.com	graphify-reindex bot, via git push under its own git identity	passes — not the string this gate matches
290875700+allyblockcast[bot]@users.noreply.github.com	the REST/MCP write path (what BLO-21416 is about)	flagged

The gate's constant is the numeric App-user form; the bot's commits carry the bare form. Running the unmodified gate over the real graphify PRs #789 and #944 passes both. The exemption's stated premise ("that automation's PRs are legitimately App-authored end-to-end") was simply not true of the shipped bot.

Deleting it fixes both halves of the finding at once, adds gate coverage in the merge queue rather than special-casing it, and avoids restating isGraphifyReindexArtifactOnlyPr's factors in a second place where they could drift. (That helper stays where it is, correctly, for the PR-template gate — a different concern: graphify PRs legitimately have no template body.) A regression test pins the bot's real identity, so if it ever moves onto the REST write path the test says so — and the fix then is to move the bot back, not to re-add an exemption.

2. Audit window ordering. Superseded by review cycle 2 below — sorting alone was not enough.

3. Commit-list truncation. GET /pulls/{n}/commits hard-caps at 250 entries even with --paginate, so an offense past commit 250 read as a clean pass. The audit now detects the cap and fails closed. It reports these as INCOMPLETE, distinct from VIOLATION — an audit that could not finish is a different fact from one that finished clean, and conflating them would send someone hunting a commit the audit never saw.

Review cycle 2 — the audit window, properly

Cycle 1 dispositioned findings 1 and 3 as fixed. Finding 2 was marked still-present, correctly: over-fetch + sort makes the ordering deterministic but still cannot establish the window, and the tool went on claiming "the last N merged PRs" — a claim its retrieval could not back.

Ally offered two acceptable resolutions: relabel the output as a bounded sample, or use a retrieval strategy that can prove the window. I took the second, because a count-based window is unprovable by construction (gh pr list orders by creation, and no GitHub list or search ordering exposes merge time), so relabelling would have preserved a weak signal in a tool whose entire purpose is to be the verifying signal.

The audit now selects by merge time: --since (default 7d, also accepts YYYY-MM-DD) drives a merged:>=<date> search qualifier, and every PR merged in that window is audited. --per-repo-limit and the over-fetch factor are removed rather than kept alongside — there is one window concept, not two.

Coverage it cannot prove is now reported rather than assumed: if a repo returns AUDIT_PR_LIST_MAX (300) PRs the window did not fit, and the run fails closed with INCOMPLETE … Narrow --since, alongside the existing per-PR 250-commit INCOMPLETE. Each run prints the merge window it actually covered.

This was not a cosmetic correction. Same repo, same 2-day period, measured both ways:

retrieval	PRs examined	violations found
creation-ordered (cycle 1, --per-repo-limit 5)	5	1
merge-time window (--since 2d)	50	26 across 13 PRs

The old window was missing the large majority of what it claimed to cover. Live output: 173 non-merge commits across 50 PRs merged 2026-08-07T06:42:23Z .. 2026-08-08T23:27:25Z, no INCOMPLETE, exit 1.

Verification

• node --test scripts/check-commit-author-attribution.test.mjs — 18/18 passing (was 8/8), including a real local git-range fixture, a merge-commit-exclusion fixture, and new cases covering both review cycles.
• Negative controls on every new guard — reverting only the truncation check fails exactly the two truncation tests; reverting only the window-cap detection fails exactly the window test; reverting the merged:>= search back to a creation-ordered list fails exactly the merge-time-selection test. Each guard is confirmed to fire for its own reason, not incidentally.
• pr.yml re-parsed with ruby -ryaml after editing (10 jobs), and the step asserted to still exist with no if: key. A workflow that does not parse produces no jobs and therefore cannot fail any gate, so this is checked explicitly rather than inferred from a green run.
• Live end-to-end on the real paths:
  • the modified gate over real graphify PRs chore(graphify): refresh knowledge graphs #789 and chore(graphify): refresh knowledge graphs #944 — both exit 0, which is the whole basis for removing the exemption;
  • --audit-merged --repos Blockcast/paperclip --per-repo-limit 5 exits 1 on a genuine in-the-wild violation (#1104 bb6fefe, carrying the numeric App identity). That is a positive control worth stating plainly: on the same run, the bare graphify identity passes and the REST-path identity is caught, so the two really are distinct strings and not a distinction I inferred from the constant.
• Ran --audit-merged against real history as evidence:
  • Blockcast/trafficcontrol#1326 (last N=1, that PR only): reproduces the issue's documented baseline exactly — 4/9 commits App-attributed (78f62b7, 9db630f, 6b8b6e2, 8108887).
  • Blockcast/paperclip (last 10 merged PRs): 4 App-attributed non-merge commits found today (PR fix(github-webhook): close two Markdown escapes in owning-reference parsing (BLO-20886) #1051: df25973, bf823a4; PR fix(deps): bump undici to 7.29.0 #1018: 0fa7f4d, 6b818dd) — a live, current-state confirmation the gap is real, not just historical.
• This PR's own commit was pushed via git push with agent git config identity and confirmed via gh api repos/Blockcast/paperclip/commits/{sha}: commit.author.email = platformsreengineer@paperclip.blockcast.net, not the App — i.e., the documented correct path, demonstrated in the PR that documents it.

Risks

• Low risk: new CI step only reads local git history already fetched by the existing actions/checkout step (fetch-depth: 0); no new secret or external call in the per-PR gate.
• The --audit-merged mode makes live gh api calls (rate-limit exposure) but is not wired into any scheduled workflow yet — it's a manually-invoked audit tool for now, documented as the AC's verifying signal. Wiring it to a schedule would need a cross-repo-capable token (COMMITPERCLIP_KEY is unprovisioned on Blockcast) — flagged as a follow-up, not done here to avoid unilaterally provisioning new CI credentials.
• Residual gap (documented, not silently left): the MCP create_or_update_file/push_files tools still have no author field — they're a third-party binary (github/github-mcp-server, pulled from ghcr.io, not vendored/patchable in this repo). The fix here is a hard ban-by-policy (AGENTS.md + PR gate) on using them for commits, not a patch to the tool itself.
• Trafficcontrol repo does not yet have the equivalent CI gate — this PR only covers paperclip (the repo this session has write access to). Filed as a follow-up; the audit script already covers trafficcontrol read-only today via --audit-merged.
• Behaviour change from review cycle 1: with the if: removed, the gate now also runs on merge_group. That is deliberate added coverage (nothing can land between PR CI and the queue without being checked), and it is safe precisely because the graphify identity passes the gate — verified above on chore(graphify): refresh knowledge graphs #789/chore(graphify): refresh knowledge graphs #944. The falsification path is cheap and immediate: chore(graphify): refresh knowledge graphs #944 is open right now, so once this lands, re-running its checks exercises the gate against a live graphify PR.
• Known limitation, stated rather than papered over: the gate matches one exact email string, so it catches the accidental REST-write-path stamp it was built for, not an agent that deliberately sets a different git identity. That is the intended scope (AGENTS.md §9 is a hygiene rule, and the per-PR gate is its enforcement), but it is worth being explicit that this is not an anti-forgery control. Broadening the match to any allyblockcast[bot] address would break the graphify bot — which is exactly what the new regression test would tell you.

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m]), Anthropic — original change: extended reasoning, tool use (Bash, Read/Write/Edit, GitHub MCP), 1M context window.

Claude Opus 5 (claude-opus-5[1m]), Anthropic — review cycle 1 (commit 5d7e2422b): extended reasoning, tool use (Bash, Read/Edit, gh), 1M context window.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• I have run tests locally and they pass
• I have added or updated tests where applicable
• If this change affects the UI, I have included before/after screenshots (N/A — no UI change)
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green (pending this PR's own CI run)
• Greptile is 5/5 with no open P2s, recommendations, or follow-ups (pending review)
• I will address all Greptile and reviewer comments before requesting merge

[allyblockcast]

🔗 Paperclip issue: BLO-19528
🔗 Paperclip issue: BLO-21416

[allyblockcast]

🔗 Paperclip issue: BLO-19528
🔗 Paperclip issue: BLO-21416

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[kkroo]

@ally please review head f201196 for BLO-21416 commit-author attribution policy/gate behavior.

This replaces app-authored #1087 under independent PR author kkroo so the Ally GitHub App can approve.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: f201196

Critical Issues (0)

Important Issues (3)

• [gstack/review] .github/workflows/pr.yml:99 — The graphify exemption trusts only github.head_ref, so a fork can name its branch bot/graphify-reindex and bypass the attribution gate. The same condition fails in the opposite direction for merge_group, where github.head_ref is empty: a legitimate graphify PR that passed PR CI will run the gate in the merge queue and reject its intentionally App-authored commits.
  • Bind the exemption to the trusted App author, expected head repository/branch, and artifact-only changed paths, and carry that classification into merge-group runs instead of relying on github.head_ref alone.
• [native-codex] scripts/check-commit-author-attribution.mjs:88 — gh pr list --state merged --limit N returns merged PRs ordered by creation time, not mergedAt. The current live result already places PR #1034 (merged Aug 6) behind PR #1051 (merged Aug 5), so the advertised "last N merged PRs" audit silently checks the wrong window and can omit a recently merged long-running PR.
  • Fetch enough merged PRs to establish the window, sort by mergedAt descending locally, and only then take perRepoLimit.
• [pr-review-toolkit] scripts/check-commit-author-attribution.mjs:105 — GET /pulls/{number}/commits has a documented hard maximum of 250 commits; --paginate cannot retrieve commits beyond that cap. A violation after commit 250 is silently omitted while the audit can report a pass.
  • Detect truncation and fail closed, or traverse the PR's exact commit range through an API that can return the complete history; add a test for the over-250 case.

Suggestions (0)

Strengths

• The per-PR gate uses event-captured full SHAs and a full-history checkout, avoiding mutable branch-range ambiguity.
• Git subprocesses use argument arrays rather than shell interpolation, and the tests exercise real Git ranges plus paginated API output.
• CI is green at the reviewed head, including policy, the serialized suites, build, typecheck, and verify.

Recommended Action

1. Address the Important issues this cycle before merge.

[kkroo]

Working the review findings. Posting analysis for finding 1 before changing code, because half of it is a design decision on a security gate rather than a mechanical fix.

Finding 1 confirmed, both directions

.github/workflows/pr.yml:99 is if: github.head_ref != 'bot/graphify-reindex' — a bare branch-name compare. Verified against the workflow's own triggers (pull_request and merge_group, lines 3–10):

• Fork bypass (real). Nothing binds the exemption to the head repository or the PR author. A fork PR whose branch is named bot/graphify-reindex skips the attribution gate entirely and can land App-attributed commits unchecked.
• merge_group false rejection (real). github.head_ref is empty on merge_group, so '' != 'bot/graphify-reindex' is true and the gate runs — meaning a legitimate graphify PR that correctly skipped the gate at PR time gets rejected inside the merge queue.

The trust model already exists — pr.yml just doesn't use it

.github/scripts/run-quality-gates.mjs:25 already exports exactly the three-factor check Ally describes:

export function isGraphifyReindexArtifactOnlyPr({ author, branch, files }) {
  return author === GRAPHIFY_REINDEX_AUTHOR       // 'allyblockcast[bot]'
    && branch === GRAPHIFY_REINDEX_BRANCH          // 'bot/graphify-reindex'
    && files.length > 0
    && files.every(f => f.filename?.startsWith('server/src/graphify-out/'));
}

pr.yml duplicates a strictly weaker one-factor version of it inline. That duplication is the bug.

Recommended patch for the fork bypass (unambiguous)

if: >-
  github.event_name == 'pull_request' &&
  !(github.event.pull_request.head.repo.full_name == github.repository &&
    github.head_ref == 'bot/graphify-reindex' &&
    github.event.pull_request.user.login == 'allyblockcast[bot]')

head.repo.full_name == github.repository closes the fork hole (a fork's full_name differs), and PR author is GitHub-authenticated so it can't be spoofed by branch naming.

The part I'm not deciding unilaterally

That expression also makes merge_group never run the gate, which fixes the false rejection — and is defensible, since the gate is enforced on every PR and the queue can only contain PRs that passed it. Under merge_method: REBASE the commits are rebased but author metadata is preserved, so a re-run is deterministic and adds nothing.

But the review says "carry that classification into merge-group runs", i.e. keep the gate in the queue and make the exemption work there. Doing that means recovering PR context from the queue ref (gh-readonly-queue/master/pr-<N>-<sha>), which is brittle, or moving the exemption into the script and keying it on App-author + artifact-only paths — and that second option has a wrinkle worth naming: the trust anchor is APP_NOREPLY_EMAIL, a git author email, which is locally forgeable. Forging it today is self-defeating (it makes the gate reject you), but an exemption keyed on it would invert that into a way through the gate for anyone willing to restrict their diff to server/src/graphify-out/.

So: skip-in-merge_group (simple, no new bypass surface, loses only a redundant re-check) versus classify-in-merge_group (matches the review's wording, but risks turning a forgeable field into an exemption key). I'd take the former. Since this is a security gate I don't own, flagging the choice rather than picking it silently.

Findings 2 (merged-PR ordering by mergedAt, not creation time) and 3 (fail closed on the 250-commit /pulls/{n}/commits cap) are mechanical and I'll take them as written.

[kkroo]

All three Important findings addressed in 5d7e2422b.

Finding 1 — I did not implement the suggested fix. Flagging that explicitly.

The recommendation was to bind the exemption to the trusted App author, expected head repo/branch, and artifact-only paths, and carry that classification into merge-group runs. I removed the exemption instead, because measuring the bot showed there was nothing to exempt:

identity	produced by	this gate's verdict
allyblockcast[bot]@users.noreply.github.com	graphify-reindex bot, via git push under its own git identity	passes
290875700+allyblockcast[bot]@users.noreply.github.com	the REST/MCP write path (what BLO-21416 targets)	flagged

The gate matches the numeric App-user form. The bot's commits carry the bare form. Running the unmodified gate over the real graphify PRs #789 and #944 exits 0 on both. So the exemption's stated premise — "that automation's PRs are legitimately App-authored end-to-end" — is false of the shipped bot, and the exemption was protecting against a rejection that never occurs.

That makes deletion strictly better than hardening here: it closes the fork bypass (nothing left to forge), it turns the merge_group case from a false-reject into added coverage rather than a special case, and it avoids restating isGraphifyReindexArtifactOnlyPr's factors in YAML where they could drift from the copy in run-quality-gates.mjs. That helper stays exactly where it is for the PR-template gate — a genuinely different concern, since graphify PRs legitimately have no template body.

Two things I'd want challenged, since I'm departing from your recommendation:

1. My claim rests on the two email strings being genuinely distinct, not on me reading the constant. The positive control: one live --audit-merged run flags #1104 bb6fefe (numeric form) and exits 1, on the same run where both graphify PRs pass. If you think there's a path where the bot emits the numeric form, that's the thing to attack.
2. The gate now runs on merge_group. I argue that's correct — but it is a behaviour change, and it's only safe because of (1).

The measurement is pinned as a regression test on the bot's real identity, with a comment saying that if it fails, the fix is to move the bot back off the REST write path rather than re-add an exemption.

Finding 2 — merged-PR ordering. Implemented as recommended. selectRecentlyMergedPrs over-fetches AUDIT_OVERFETCH_FACTOR × limit, sorts by mergedAt descending, then takes the window; entries with a missing or unparseable mergedAt are dropped rather than sorted arbitrarily. The residual gap — a PR created beyond the over-fetch window but merged inside it — is documented on the constant, since no GitHub list or search ordering exposes merge time and widening the window is the only lever.

Finding 3 — 250-commit cap. Implemented as recommended, fail-closed. Truncation is reported as INCOMPLETE, deliberately distinct from VIOLATION: an audit that could not finish is a different fact from one that finished clean, and merging them would send someone hunting a commit the audit never saw. Test added for the over-250 case, plus one asserting runAudit returns passed: false with zero offenses.

Verification. 15/15 unit tests (was 8/8). Both new guards confirmed by negative control — reverting only the truncation check fails exactly the two truncation tests, reverting only the sort fails exactly the two ordering tests. pr.yml re-parsed with ruby -ryaml and the step asserted present with no if: key (an unparseable workflow emits no jobs and so cannot fail any gate — worth checking directly rather than inferring from a green run).

One limitation I want on the record rather than buried: this gate matches one exact string, so it catches the accidental REST-write-path stamp it was built for, not an agent that deliberately configures a different identity. That's the intended scope, but it is not an anti-forgery control. Broadening the match to any allyblockcast[bot] address would break the graphify bot — which is precisely what the new regression test would report.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: 5d7e242

Prior Findings Dispositioned (3)

• prior:f201196 important 1 — fixed — .github/workflows/pr.yml:110 — The gate now runs without the forgeable branch-name exemption, so fork branch naming cannot bypass it and merge_group receives the same check.
• prior:f201196 important 2 — still-present — scripts/check-commit-author-attribution.mjs:225 — Fetching only perRepoLimit * AUDIT_OVERFETCH_FACTOR PRs before sorting still omits a PR created before that bound but merged more recently. The documented residual limitation means the result is not reliably the advertised “last N merged PRs.”
• prior:f201196 important 3 — fixed — scripts/check-commit-author-attribution.mjs:256 — A 250-entry commit response is now reported as INCOMPLETE, causing the audit to fail rather than report a clean result from a truncated commit list.

Critical Issues (0)

Important Issues (1)

• [native-codex] prior:f201196 important 2 scripts/check-commit-author-attribution.mjs:225 — The audit still claims to cover the last N merged PRs, but its 5x creation-ordered candidate list cannot establish that window. A long-running PR outside that creation bound can merge last and be skipped.
  • Either label this output as a bounded sample of recently created-and-merged PRs, or use a retrieval strategy that can prove the merge-time window before retaining the “last N merged” claim.

Suggestions (0)

Strengths

• Removing the exemption fixes both its fork bypass and its merge-queue inconsistency without adding a new trust predicate.
• The exact REST-path email distinction is corroborated by live graphify commit metadata, and the 15 focused tests pass at the reviewed SHA.

Recommended Action

1. Correct the audit-window contract before merge.

[kkroo]

Cycle 2 — finding 2 fixed properly in b22a36621.

You were right to mark it still-present. Sorting made the ordering deterministic but the retrieval still could not establish the window, and the tool went on advertising "the last N merged PRs" — a claim it could not back. That is the same defect class the other two findings were, just one level up: the output described work the code hadn't done.

You offered two resolutions. I took the second (prove the window) rather than the first (relabel as a bounded sample), because a count-based window is unprovable by construction — gh pr list orders by creation and no GitHub list or search ordering exposes merge time — so relabelling would have left a knowingly weak signal inside the one tool whose entire job is to be the verifying signal.

What changed. The audit selects by merge time: --since (default 7d, also accepts YYYY-MM-DD) drives a merged:>=<date> search qualifier, and every PR merged in that window is audited. --per-repo-limit, DEFAULT_PER_REPO_LIMIT, AUDIT_OVERFETCH_FACTOR and selectRecentlyMergedPrs are deleted rather than kept alongside — one window concept, not two. Where the window still cannot be proven, it says so: hitting AUDIT_PR_LIST_MAX (300) fails closed with INCOMPLETE … Narrow --since, distinct from VIOLATION, alongside the existing per-PR 250-commit INCOMPLETE. Every run prints the merge window it actually covered.

This was not cosmetic. Same repo, same 2-day period, measured both ways:

retrieval	PRs examined	violations found
creation-ordered (cycle 1)	5	1
merge-time window (--since 2d)	50	26 across 13 PRs

The old window was missing the large majority of what it claimed to cover, so the finding was material rather than theoretical. Live run covers 2026-08-07T06:42:23Z .. 2026-08-08T23:27:25Z, 173 non-merge commits, no INCOMPLETE, exit 1.

Verification. 18/18 tests. Each new guard confirmed by negative control: forcing windowTruncated false fails exactly the window test; reverting merged:>= back to a creation-ordered list fails exactly the merge-time-selection test. node --check clean, and no dead references to the removed symbols.

---

Separately, on the previous head's CI — neither red was from this change, and both are now filed rather than retried:

• General tests (workspaces-a) was red with 3035/3035 tests passing; vitest exits 1 on 8 unhandled errors, all ReferenceError: window is not defined from React's scheduler firing after jsdom teardown in editable-sections.test.tsx (last modified 2026-06-19, byte-identical on branches where it passes) → BLO-23426.
• OpenCode Responses replay failed with exit=null, which means the signal arm fired — but that assertion prints only code, so which signal is unrecoverable. The sibling assertion 20 lines above prints signal= correctly → BLO-23423.

I could not classify the second as environmental vs real, precisely because the guard discarded its own evidence, so I have not claimed it was a flake.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b22a366

Prior Findings Dispositioned (3)

• prior:f201196 important 1 — fixed — .github/workflows/pr.yml:110 — The attribution gate has no branch-name exemption, so fork branch naming cannot bypass it and merge-queue runs execute the same gate.
• prior:f201196 important 2 — fixed — scripts/check-commit-author-attribution.mjs:262 — The audit now queries merged:>=<since> directly and reports INCOMPLETE if the merge-time window reaches the 300-PR cap; it no longer claims a count-based “last N” window.
• prior:f201196 important 3 — fixed — scripts/check-commit-author-attribution.mjs:288 — A 250-entry pull-request commit response is marked truncated, causing the audit to fail closed instead of treating an incomplete history as clean.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The gate checks the exact PR range using full history and excludes only merge commits, matching the stated policy boundary.
• The audit distinguishes confirmed attribution violations from incomplete evidence and has focused regression tests for both cap conditions.

Recommended Action

1. No blocking changes requested.

[allyblockcast]

Ally — Consolidated PR Review

Lenses: pr-review-toolkit (code, tests, comments, errors, types) + gstack/review + native-codex.
Reviewed head: b22a366

Prior Findings Dispositioned (3)

• prior:f201196 important 1 — fixed — .github/workflows/pr.yml:110 — The attribution gate has no branch-name exemption, so fork branch naming cannot bypass it and merge-queue runs execute the same gate.
• prior:f201196 important 2 — fixed — scripts/check-commit-author-attribution.mjs:262 — The audit now queries merged:>=<since> directly and reports INCOMPLETE if the merge-time window reaches the 300-PR cap; it no longer claims a count-based “last N” window.
• prior:f201196 important 3 — fixed — scripts/check-commit-author-attribution.mjs:288 — A 250-entry pull-request commit response is marked truncated, causing the audit to fail closed instead of treating an incomplete history as clean.

Critical Issues (0)

Important Issues (0)

Suggestions (0)

Strengths

• The gate checks the exact PR range using full history and excludes only merge commits, matching the stated policy boundary.
• The audit distinguishes confirmed attribution violations from incomplete evidence and has focused regression tests for both cap conditions.

Recommended Action

1. No blocking changes requested.