fix(agents): document write-path commit attribution and gate on it (BLO-21416) - #1087
Closed
allyblockcast[bot] wants to merge 1 commit into
Closed
fix(agents): document write-path commit attribution and gate on it (BLO-21416)#1087allyblockcast[bot] wants to merge 1 commit into
allyblockcast[bot] wants to merge 1 commit into
Conversation
…LO-21416) allyblockcast[bot] is one shared GitHub App credential every agent pod uses. REST commit-creation endpoints (contents/merge API, MCP create_or_update_file /push_files) default commit.author to that authenticated identity, so any agent writing via the API path gets stamped with the App instead of itself — git push is unaffected since git already reads per-agent user.name/user.email from local config. Reproduced via controlled probe and an internal control (one agent, one PR, three author identities by write path alone); see the BLO-21416 issue for the full evidence. - Document the rule and the gh api author[] workaround in AGENTS.md §9, so no future agent re-derives this or re-files it as a fresh misattribution report (it already was once, as BLO-19528). - Add scripts/check-commit-author-attribution.mjs: a local git-log mode (--no-merges over base..head, wired into pr.yml as a going-forward gate on every paperclip PR) and a --audit-merged mode (gh api across the last N merged PRs of one or more repos) for the AC's automated verifying signal. Verified against the documented Blockcast/trafficcontrol#1326 baseline (reproduces its exact 4 violations) and against Blockcast/paperclip's own live history (4 violations in the last 10 merged PRs: #1051, #1018). - Merge/squash-merge commits are excluded everywhere (legitimately App-attributed via the merge API) and the graphify-reindex bot branch is exempted from the new PR gate (its PRs are genuinely App-authored end-to-end, unlike an agent's branch merely opened via the API). Co-Authored-By: Paperclip <noreply@paperclip.ing>
Author
13 tasks
|
Superseded by #1097, which carries the exact same head under independent PR author kkroo so Ally can provide the required App approval. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thinking Path
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 pushis already correct, the MCPcreate_or_update_file/push_filestools have noauthorfield and must not be used to land commits, andgh api(the one API call site agents control) must pass an explicitauthor[name]/author[email]when a raw API write is unavoidable.scripts/check-commit-author-attribution.mjs(+ test): shared assertionfindAttributionOffenses(flags a non-merge commit'scommit.author.email == "290875700+allyblockcast[bot]@users.noreply.github.com") with two front-ends:git log --no-mergesover abase..headrange — no network, no new secret.--audit-mergedmode: for the last N merged PRs of one or more repos (defaultBlockcast/trafficcontrol,Blockcast/paperclip), fetch each PR's own commit list viagh apiand apply the same assertion — the AC's "automated verifying signal.".github/workflows/pr.yml: wires the local mode into the existingpolicyjob as a new required step on every PR, using the branch's already-checked-out history (no new secret needed). Exemptsbot/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.--audit-mergedagainst 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.git pushwith agent git config identity and confirmed viagh 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
actions/checkoutstep (fetch-depth: 0); no new secret or external call in the per-PR gate.--audit-mergedmode makes livegh apicalls (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_KEYis unprovisioned on Blockcast) — flagged as a follow-up, not done here to avoid unilaterally provisioning new CI credentials.create_or_update_file/push_filestools still have noauthorfield — they're a third-party binary (github/github-mcp-server, pulled fromghcr.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.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
Fixes: #/Closes #/Refs #OR (b) described the issue in-PR following the relevant issue template