Attribute the measurement commit to the App, not to nobody (T-1502) - #743
Merged
Conversation
The run that got past the credential fix reported:
--- branch pushed; commit committer on the server: unknown
The push itself worked, so `persist-credentials: false` did what it was for. What did not work was attribution: the committer address used `4622872`, which is the App id. The number in a noreply address is the bot *user* id, and for this App that is `317873099` (`gh api 'users/commitlore-canonical-build[bot]'`). GitHub could not match the address to an account, so it attributed the commit to nobody and the read-back printed `unknown`.
That read-back was added one commit earlier to tell two failures apart, and it worked -- it just reported a defect in the line above it rather than in the token. Left alone it would have spoiled a successful measurement: ADR-0036 assumes the App opens the pull request, and a commit attributed to nobody makes the answer ambiguous exactly when the rest of the run finally goes green.
Limit: this fixes attribution, not access -- the same run was refused at `gh pr create` with `Resource not accessible by integration (createPullRequest)`, which is a missing `Pull requests: write` on the App installation and is not something a workflow can change
Blast: local
Undo: easy
Certainty: firm
Record-Id: r-committerid1502
Provenance: authored
Verified: `gh api 'users/commitlore-canonical-build[bot]'` returns `id=317873099 type=Bot`, and the App id in the failing line is 4622872; the two numbers are different things and the log's `unknown` is what using the wrong one looks like
CommitLore-Version: 2.0.0
CommitLore — record lintTrailers: clean — 2 commits in Trailer violations fail this check. Active constraints are informational — they are what the repository already decided, not a verdict on this PR. |
The cleanup step was gated on `steps.open.outputs.number`, which is only set after `gh pr create` returns. Both runs that failed at create failed *after* pushing the branch, so `number` was empty, the step was skipped, and the branch outlived the run that made it. That is the wrong key. The push is what creates something to clean up, so the branch name is what the cleanup should be keyed on -- it is written to the step output before anything else happens, and it exists on every path that reaches the push. The delete is followed by a reference read rather than trusted. `git push --delete` on a ref that is already gone is a failure, and `|| true` would then hide a delete that genuinely did not happen. Reading the ref back separates "already absent" from "still there", which is the same shape as the readback #735 put in the installer. Limit: this cleans up after a failed measurement; it does not stop one from failing, and a run killed between the push and this step still leaves the branch Blast: local Undo: easy Certainty: firm Record-Id: r-measurecleanup1502 Provenance: authored Verified: the two failed runs (32081602096 and its predecessor) both reached the push and stopped at `gh pr create`, which is the window where `number` is empty and `branch` is not; the successful run 32082467906 shows the close step working when a number exists, and that path is unchanged CommitLore-Version: 2.0.0
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
1 participant
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.
The run after #741 got further and printed:
Two separate things, and this PR fixes the first.
Attribution. The committer address used
4622872— the App id. The number in a noreply address is the bot user id, which for this App is317873099:GitHub could not match the address to an account, so the commit was attributed to nobody. The read-back added in dbcd73b did its job — it reported a defect in the line above it. Left alone it would have spoiled a successful measurement: ADR-0036 assumes the App opens the pull request, and a commit attributed to nobody makes the answer ambiguous exactly when the rest of the run finally goes green.
Access, which this PR does not fix.
gh pr createwas refused withResource not accessible by integration (createPullRequest). The installation hascontents: write— the push proves it — but notPull requests: write. No workflow change can grant that; it is a change to the App's declared permissions, made by the App owner, followed by accepting the updated permissions on the installation.That is worth stating plainly, because it lands on ADR-0036: the decision that a bot merge opens a pull request rests on the App being able to open one, and today it cannot. The original question — whether
on: pull_requestfires for an App-opened pull request — is still unmeasured, and the next run answers it once the permission is added.