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). Exempts bot/graphify-reindex (that automation is genuinely App-authored end-to-end).

Verification

• node --test scripts/check-commit-author-attribution.test.mjs — 8/8 passing, including a real local git-range fixture and a merge-commit-exclusion fixture.
• 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.

Model Used

Claude Sonnet 5 (claude-sonnet-5[1m]), Anthropic — extended reasoning, tool use (Bash, Read/Write/Edit, GitHub MCP), 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.