Design: where the last pusher and the base are read from (P-28, P-29) #163

Activity

1. don-petry commented on Oct 18, 2026

   

   don-petry

   on Oct 18, 2026

   ContributorAuthor

   More actions

   Working rulings (overturnable in review)

   Solution Architect, delegated. On main at c094a8b (#170). Read from trials/P-28.json, trials/P-29.base.json and trials/P-29.truncated.json (#11, #164). P-28 for an App's push is #27, which has not run and is waiting on the Maintainer. Every part that rests on it says so.

   The cause. The design trusted three facts that had never been read. The trials showed where each one really is.

   • The last pusher. Only GET /repos/{repo}/activity?ref=refs/heads/{head} says who pushed, and it does so for a plain push and a force push alike. Each entry has before, after, activity_type, and an actor with login and type. The entries were there 11 s after each push.
     • The REST timeline and GraphQL name an actor only for a force push.
     • Commits carry only git's author and committer, and an amend keeps both.
     • The events API showed no push at all after five minutes.
   • The base. When main moved by a plain push, the pull request's base.sha stayed on the old commit for over two minutes, and so did merge_commit_sha. Both moved only when the head was pushed. mergeable_state stayed clean. The diff against the merge base, which content_id hashes, did not change (P-29's pass).
   • Short reads.
     • A diff of more than 300 files is refused with 406 too_large.
     • The list of files stops at 3,000 entries with a 200 and an empty next page. Only changed_files (3,001) shows it short.
     • A file of 200,000 lines kept its entry in that list but lost patch.
     • A diff of 225,000 lines came back whole, 1.65 MB, with a 200.

   What the Kernel reads today: last_pusher, for I-26 and for a dependency update's exemption from the model review; mergeable; targets_protected; content_id; diff; and changed_paths. It reads no base commit. §3.3 lists head, base and merge_base, but schemas/facts.schema.json has none of them.

   1. The last pusher is read from the head branch's activity, and from nothing else. The port reads the newest entry of GET /repos/{repo}/activity?ref=refs/heads/{head.ref}, in the repo that holds the head.

   • If that entry's after is the pull request's head.sha, its actor is the last pusher, whatever its activity_type (push, force_push, branch_creation).
   • Otherwise the last pusher cannot be read, and the port gives null. That covers a head that moved since, an entry not yet there, and no entry at all.
   • Use the newest entry, not any entry whose after matches. Otherwise a head forced back to an older commit would be credited to whoever first pushed that commit.
   • The actor's kind comes from actor.type and the configuration, as an_actor_has_a_kind says, never from how the login is spelled.

   Why: P-28 found no other record that names who made a plain push. This waits on adapters/; #138's port work picks it up.

   • This waits on a trial not yet run. In P-28 a person pushed through git. Whether an entry appears, naming the App with type: Bot, when an App pushes is Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27's P-28, which is waiting on the Maintainer. Add three things to that trial:

     • an App's push through git, with an installation token;
     • an App moving the head through the API (PATCH /repos/{repo}/git/refs/...), which is how a Role may push;
     • how far back the activity list goes.

     Also record the least permission the reader App needs for the activity read, as Three Apps registered and installed, and their real grants compared with the data #26's amendment does for the queue. Until Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27 runs, the fake GitHub's the_pusher_is_the_last_pusher is grounded only for a person's push, and its text says so.

   • Not tried at all: a head in a fork. Its activity belongs to the fork, where no Factory App is installed. See "Needs the Maintainer?".

   2. A last pusher that cannot be read lets nothing through. In schemas/facts.schema.json, last_pusher becomes a non-empty name or null. In kernel/derive.py, right after the paths_unreadable guard:

       if not isinstance(facts.get("last_pusher"), str) or facts["last_pusher"] == "":
           return {"state": None, "reason": "last_pusher_unreadable", "evidence": []}

   This is read before required_model_lenses and before the approver check. So an unreadable pusher never excuses a dependency update from the model review, and never lets an approval through.

   • last_pusher_unreadable goes in NEEDS_YOU_REASONS, because it may not mend: an entry that is gone stays gone.
   • A queued item leaves the queue, as it does for every other unreadable fact.

   Why: I-13 and I-26. Today an unreadable pusher has nowhere to go but a guess.

   3. Wherever the Factory reads the base, it reads the base branch's head at that pass, never base.sha. Each reading:

   • Approval binding reads no base. An approval is bound to content_id and submitted for the head commit (decision 0007, §3.6). content_id hashes the diff GitHub computes against the merge base at the moment of the read, and P-29 showed it stable when the base moves. Nothing changes.
   • Mergeability is GitHub's own reading, mergeable. P-29 showed that GitHub does not work it out again when only the base moves. The Kernel goes on reading it as "merges cleanly, as GitHub last worked it out". It gates nothing that the merge queue does not check again. The queue builds the merge on the base's head, and a conflict found there ejects the item, an act that the act ceiling bounds. So a stale true costs acts and a stale false costs a fix Run, but neither costs safety. No rule changes.
     • This waits on a trial not yet run. Whether GitHub works mergeable out again for a pull request that a base move makes conflict, and how soon, was not tried. Add it to §6 as a new trial, batch 2, with the next free number (P-40 if Three Apps registered and installed, and their real grants compared with the data #26's P-39 is the last one taken): "When only the base branch moves, does GitHub work out mergeable and merge_commit_sha again, for a pull request that still merges cleanly and for one the move makes conflict, and how long after?" Pass: each answered and recorded.
     • Until it runs, the exploration's move "the base moves" stays as it is. It makes mergeable unknown, which explores more cases than GitHub may produce.
   • The protected paths and the Lenses' inputs. Decision 0020 builds the list from the base branch's config/hooks.json. The Controller reads that file at the base branch's head, found through GET /repos/{repo}/branches/{base.ref}, once a pass for each base branch. It never reads it at a pull request's base.sha, which may be minutes or days old. This waits on controller/.
   • What a Lens reads from the base (decision 0023) is the event's pull_request.base.sha, or merge_group.base_sha in the queue. A push to the head moves base.sha, so on a synchronize event it is main's head at that moment. On any other event it may be an older commit of main. That is still a commit of the protected branch, never of the head. The queue's run, from the base its group is built on (P-37), gives the verdict that merges. 0023 does not change.
   • In §3.3, define base as the base branch's head, read at the pass, and merge_base as the merge base GitHub gives for the head against that branch (compare's merge_base_commit). Neither is in the facts schema, and neither is added until something reads it.

   4. A short read is found by its counts, never by its status alone. For the port:

   • The diff.
     • A 406 too_large gives diff.truncated true. The Kernel holds that as too_large for a person, as it does now.
     • A diff served with a 200 is checked against the same read's changed_files, additions and deletions: its diff --git headers, its added lines and its deleted lines. If they differ, the diff is short: complete.diff is false, and the Kernel gives facts_incomplete.
     • The Kernel holds any diff over max_diff_lines (4,000) before GitHub's own limits matter.
   • The list of files. Fewer records than changed_files means the list is short. Kernel: a change to the Factory's own rules or a Lens's inputs is reviewed by a model Lens #135's check, that records equal diff.files, already gives paths_unreadable for this.
   • patch. No fact reads it. changed_paths takes only filename and previous_filename. content_id and a Task's diff come from the diff endpoint, never from patches put together. So today a missing patch makes nothing short. The rule for the port: build no read from patch, and if one ever is built, read an entry without patch as short. This answers the issue's third box. It is not a box to build: the fake GitHub has nothing to model until a fact reads patch.

   All of this waits on adapters/. Why: in P-29.truncated, both short reads that came back with a 200 could be found only against a count GitHub gives for the same pull request.

   5. Boundary tests, written first. Only ruling 2 is Kernel code.

   • In tests/test_decide.py, test_an_unreadable_last_pusher_lets_nothing_through:
     • A pull request that would converge, with last_pusher null: None / last_pusher_unreadable / defer / needs_you, and no issue_approval.
     • The same with last_pusher "": the same Decision.
     • The other side, with the approver's name: held / approver_is_author_or_last_pusher / hold. With another person's name: act / issue_approval.
     • A dependency update that changes nothing on the list, with last_pusher null: None / last_pusher_unreadable, never converged without the model review. With last_pusher equal to author: judged by the deterministic Lenses alone, as now.
     • A queued item with last_pusher null: act / dequeue.
   • examples/facts.json keeps a name. A counter-example in examples/invalid/ with last_pusher "" must be refused; add one if none exists.
   • In tests/test_transitions.py, BASES gains last_pusher_unreadable. The test that every reason has facts will ask for it.

   6. Mutants, in MUTANTS, each with text that appears once in its file:

   • "an unreadable last pusher is read": in kernel/derive.py, if not isinstance(facts.get("last_pusher"), str) or facts["last_pusher"] == "":\n becomes if False:\n. The first test kills it: the item converges and asks for an approval.
   • "an empty last pusher is read": in the same line, or facts["last_pusher"] == "" is taken out. The "" test kills it.
   • "an unreadable last pusher calls nobody": in kernel/decide.py, the "last_pusher_unreadable", entry comes out of NEEDS_YOU_REASONS. Write it so its text appears once. The attention assertion kills it.

   7. The simulator.

   • In sim/grounding.py, a_path_list_can_leave_out_a_file gains P-29 and trials/P-29.truncated.json: the 3,000-entry cap is exactly that rule. a_short_diff_says_so is already grounded by trials/P-29.truncated.json, so that box of the issue is done.
   • The text of the_pusher_is_the_last_pusher gains: "when a person pushed; an App's push waits on P-28's App case (Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27)".
   • A new rule with no trial behind it: a_push_is_missing_from_the_activity, "The push that left the head can be missing from its branch's activity for a pass". No trial asks it: P-28 saw entries within 11 s and tried nothing shorter. In sim/world.py, on seeds chosen by a constant, last_pusher reads null for a pass. The check: nothing is approved, enqueued or admitted on such a read. Every other seed replays as it does now.
   • No rule for the base. sim/world.py models no move of the base. When it does, that rule ("a move of the base leaves base.sha, the test merge and mergeable as they were until the head is pushed") is grounded by trials/P-29.base.json.

   8. Record. No new record: each ruling applies a decision already made. Boundary rows:

   • Decision 0005, whose rule is that no approval is made where the approver is the last pusher: "A last pusher that cannot be read makes no approval, admits nothing and takes a queued item out; it never excuses a dependency update from the model review". It names the test of ruling 5.
   • Decision 0026. Its row on last_pusher, waiting on adapters/, stays as it is. Add: "The port reads last_pusher from the newest entry of the head branch's activity, when its after is the head commit, and gives null otherwise; an App's push is read the same way once P-28's App case grounds it": waiting on adapters/.
   • Decision 0020: "The Controller reads the base branch's config/hooks.json at that branch's head, once a pass, never at a pull request's base.sha": waiting on controller/.
   • Decision 0002, after its row on facts that say whether they are complete: "A diff served with a 200 is checked against the same read's counts of files and lines, and a list of files against changed_files; a difference is a short read; no read is built from patch": waiting on adapters/.

   Design edits, in the same change:

   • §3.3's last_pusher row gains: "read from the head branch's activity (P-28); null, which is unreadable, when the newest entry is not the head commit".
   • §3.3's head, base and merge_base are defined as in ruling 3.
   • §3.3's mergeable row gains: "as GitHub last worked it out; a move of the base alone does not make GitHub work it out again (P-29)". §4.6's row on a merge state not yet worked out gains the same.
   • §6 gains the new trial from ruling 3.

   9. Out of scope.

   • The port itself (Port: the paths a pull request changes, with both names of a rename #138, adapters/).
   • P-28's App case (Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27).
   • Whether mergeable is worked out again when the base moves (the new trial).
   • How long GitHub keeps activity entries. That is added to Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27; until it is known, a pull request whose last push is older than the oldest kept entry reads null and waits for the Maintainer.

   Needs the Maintainer? One point, taken as recommended (option 2), overturnable in review: the last pusher of a head in a fork.

   • A fork's branch activity is in the fork. No Factory App is installed there, and no one has read it.
   • Options:
     1. Add a fork case to Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27's P-28: an account without write access forks the canary, opens a pull request and pushes, and the reader reads the fork's activity. This needs a second, outside account, which is a new identity. Trials P-24 and P-28 for an App's push; then the canary's token is revoked #27 is meant to be the last story that uses the canary token.
     2. Give null for a fork's head until a trial grounds the read. An outsider's pull request would then defer with last_pusher_unreadable for the Maintainer, even after a person admits it, and the Factory would never approve one.
     3. Read the fork's activity without a trial, as public data.
   • Taken: option 2 now, and option 1 once an outside account exists for some other reason. Outsiders' pull requests are rare in this repo and already need a person to admit them. Option 3 would rest I-26 on a read nobody has seen.
2. don-petry

   closed this as completedin #223on Oct 20, 2026
3. don-petry

   added a commit that references this issue on Oct 22, 2026

   The controller reads an issue's comments since a time, on a pull requ…

   ...

   6ef54e1