Trials P-1, P-7, P-24 and P-28 (App push): the machine user's approval counts, and a push dismisses it

Merged
DO
2d ago
+19285-21

Five fixtures from the sitting of 2026-10-19 in the canary, built from the redacted captures alone (the trial-author App's script exchanges first, then the session's reads, dispatches, dismissal, merge or close, in order), each redacted once more with tools/redact_trial.py (nothing left to remove) and checked with python3 -m tools.check_trials. Under the working rulings on #24, this trial set-up needs no decision record.

What each trial asked, and found

FixtureAskedFoundVerdict
trials/P-1.jsonDoes a pull request authored by an App need an extra approval under the ruleset's rule for unattributed changes?On PR 29, opened by the App with its one commit, the machine user's one approval (review 5572599906 on f338e49) gives reviewDecision APPROVED and mergeStateStatus CLEAN; the Maintainer's squash merge returns 200, merged true, with no bypass actorpass
trials/P-7.maintainer-pushed.jsonThe case slice 1 meets: the Maintainer pushed last, and the machine user approves as code ownerOn PR 34, opened and pushed by the Maintainer under * @combsmith/org-leads, the machine user's approval of f03b9ec gives APPROVED and CLEAN, and still does once CodeRabbit's approval of the same head is dismissed by hand, so the machine user's is the only APPROVED reviewpass
trials/P-7.jsonDoes the machine user's approval satisfy code-owner review and the last-push rule when an App pushed last?After the App pushed 355cb4d to PR 34, the machine user's approval of that head (review 5576449423) gives APPROVED and CLEAN under require_code_owner_review and require_last_push_approval; the squash merge returns 200, merged true at acddd57pass
trials/P-24.jsonWith the Maintainer as the only code owner of a path, does the machine user's approval fail to satisfy code-owner review there?On PR 35, the App's change under p24/ (/p24/ @don-petry), the machine user's approval of 00247e0 is the only review and leaves REVIEW_REQUIRED and BLOCKED, with don-petry still requested; the pull request was closed unmergedpass
trials/P-28.app.jsonCan the actor who last pushed a pull request's head be read as a fact, when an App pushed it?In GET /repos/{repo}/activity?ref=refs/heads/trial/p-7, the entry whose after is the head 355cb4d is a push by combsmith-sf-trial-author[bot], actor.type Bot; the entry before it is the Maintainer's push of f03b9ec, type Userpass

Each App exchange's GET /installation/repositories lists the canary alone, and each script run ends with DELETE /installation/token, 204.

What trials/P-7.json also records

  • A push dismisses the earlier approval. Before the App's push (the last read of P-7.maintainer-pushed.json): the machine user's approval of f03b9ec, APPROVED, CLEAN. After it (exchanges 7 to 11 of P-7.json, before any new review): that review is DISMISSED, reviewDecision REVIEW_REQUIRED, mergeStateStatus BLOCKED.
  • The dismissed approval stays on its commit, and the pusher is the dismisser (the P-38 observation, exchange 16, the timeline): the review_dismissed event at 19:11:46Z has actor combsmith-sf-trial-author[bot], dismissed_review.review_id 5576237982, dismissal_message null and dismissal_commit_id 355cb4d; the review keeps commit_id f03b9ec with state DISMISSED. The hand dismissal of CodeRabbit's review (18:56:10Z) names don-petry, carries the message and no commit.
  • An App approved, and was dismissed by hand. CodeRabbit, a GitHub App, approved f03b9ec at 18:06:54Z (review 5575697022). It was dismissed by the Maintainer's login before the verdict read, so the pass of P-7.maintainer-pushed.json rests on a read where the machine user's is the only APPROVED review (ruling 8 on #24). Whether an App's approval alone satisfies the ruleset was not measured.
  • A read of PR 34 by the App right after its own push (exchange 5) still returned head f03b9ec: GitHub served the cached pull request for a moment. The session's read 32 seconds later had 355cb4d, with last-modified 19:11:53Z. The fake already has a_read_can_be_stale; no trial asks it, so it stays unverified.

Grounding (sim/grounding.py)

  • an_app_pushes_to_the_head and the_machine_users_approval_counts: grounded by trials/P-7.json.
  • the_pusher_is_the_last_pusher: keeps trials/P-28.json (#164, a person's push) and gains trials/P-28.app.json (an App's). A rule may now name a tuple of fixtures; tools/check_trials.py holds each to the same rules (exists, validates, records a trial that asks the rule), refuses an empty tuple, and unverified() counts a tuple-grounded rule as verified. tests/test_tools.py::test_a_rule_may_be_grounded_by_a_tuple_of_fixtures_each_held_to_the_same_rules and ::test_the_approval_trials_ground_the_rules_they_asked hold it.
  • For the Maintainer: an_approval_survives_a_push is wrong under the ruleset. The fake leaves an earlier approval in place when the head is pushed; GitHub, with dismiss_stale_reviews_on_push, dismissed the machine user's approval when the App pushed, naming the pusher as the dismisser. As the skill's step 7 and sim/AGENTS.md say, the rule is left unverified and the fake unchanged, with a comment on the rule and a paragraph under "Known to differ": changing the fake is a story of its own. Two things that story would weigh: the Kernel's reading of a dismissed approval (sim/world.py counts an approval dismissed by a non-person as still standing for the contract, decision 0025) meets a dismissal GitHub attributes to the pusher, who can be a person; and a_request_for_changes_survives_a_push was not exercised here.

Reviews and verification

  • make verify on a fresh git archive export (PATH=/usr/bin:/bin SIM_SEEDS=40): passed at de2a708, 98be8c8 and df503a5 (744 tests OK, every check ok); and at beccc29. 02e79a0 answers CodeRabbit: the comment beccc29 added held two apostrophes inside the single-quoted Python of the token mint, which ended it; reworded, and the mint block run alone on a proxied capture. The mutation run is CI's.
  • Reviews on d32a494 (the build; de2a708 reflows one paragraph of sim/AGENTS.md to the 40-line limit, nothing else), each given the diff with the five fixtures replaced by a per-exchange summary, since the full 1.2 MB exceeded the argument limit of AGY's command line and stalled Muse for 50 minutes:
    • A fresh-context Claude review with a security pass: approve. No credential in any file, every verdict borne out by the recorded bodies, tools/trial_as_app.sh read line by line, three hand-made mutants on the tuple support killed by the new test. One minor fixed (98be8c8: the timeline read of P-7.json was listed after two reads made later); two noted below.
    • AGY Argon: no findings.
    • Muse Wonder Full: one major and four minors, no blocker. Fixed in df503a5: the App script ran gh api with GH_HOST unset, so gh would send the installation token to whatever host GH_HOST or its configured default named, where the curl calls pin api.github.com; the script now pins GH_HOST=github.com. Also fixed there: a mutant that checked a tuple's first fixture alone for the trial that asks the rule survived; a case with a valid fixture of another trial second in the tuple kills it. Noted, not changed: the script mints its token with the installation's full grants (contents and pull requests write, metadata read, as exchange 1 records) rather than narrowing, and records GET /installation/repositories without refusing a second repo; the head the script prints comes from a re-read that can lag the push (exchange 5 of P-7.json); the NN.request.json copies in the scratch directory are not themselves redacted (their authorization value is the words "not recorded"; exchanges.json, which the fixture is built from, is).
    • AGY read df503a5 as well: one major and two minors, all in tools/trial_as_app.sh. Fixed in beccc29: through an HTTPS proxy, curl -i prints the proxy's own "Connection established" block before GitHub's, so the first blank line ended the headers too soon and the token went unread, unused and unrevoked; the parse now takes the last blank line, and the message says what an unread token means. AGY's claim that the token then reached the raw file unpopped is true of the file for a moment, but tools/redact_trial.py runs over it before anything is read and deletes a field named token, and the raw file is deleted. Not changed: finish() does not check the final redaction's exit status, and the head the script prints can lag the push (the stale read above).
  • Noted from the Claude review, not changed: removed names the authorization header of the gh run watch and gh run view --log exchanges too, whose recorded headers are empty, as trials/P-3.json does for its gh secret set; gh sent one. And the ruleset's settings the pass conditions cite (require_extra_approval_for_unattributed_changes, one approval, stale reviews dismissed on push) are the rulings on #24, write 5, recorded in no fixture; the dismissal by the pusher is observed, the setting behind it is inferred.
  • Step 8, what stays in the canary, as the rulings on #24 say: the ruleset, CODEOWNERS (its /p24/ line too, for batch 2), machine-user-approves.yml, the environment main-only without its one secret (the canary's copy of the machine user's token, which the Maintainer deleted on 2026-10-19 at about 20:40Z; the token itself is kept for other uses, as recorded on #27), the App's installation, the two squash merges on main (P-1's and P-7's evidence, with trial-notes/), the closed pull requests and the approval runs. Undone and recorded here: three installation tokens revoked (each script's last exchange, 204), PR 35 closed and trial/p-24 deleted, trial/p-1 and trial/p-7 deleted.

Closes #14. Closes #27: P-24 and P-28 are done here, and the canary's copy of the token was deleted and recorded there.

🤖 Generated with Claude Code