[scuttletoby]

Plan row 14, part 2 of 4: the queue's merge path, fork-ci/actions/pr-queue/pr_queue.py. One run brings the oldest armed pull request that is BEHIND its base up to date. It turns auto-merge off and runs gh pr update-branch. Then it turns auto-merge back on (with --match-head-commit) only for a new head that holds exactly the change the review read, merged with the base. Anything else leaves auto-merge off with a comment that names the way out. Usage, exit codes and STATE: README, "The queue". This is the tier-3 part: it merges and comments.

Stack (each PR's base branch is the one before it; review and merge them in order):

1. fork-ci/pr-queue-lens → main: the lens review rules and a strict config
2. fork-ci/pr-queue-core → 1/4: the queue's merge path (pr_queue.py)
3. fork-ci/pr-queue-sweep → 2/4: armed pull requests that cannot merge are told and passed over (SkyRPG [Feature]: escape button to go back to previous thread from settings page pingdotgg/t3code#1350)
4. fork-ci/pr-queue-workflow → 3/4: "red only because the base was red" (SkyRPG Fix plan composer layout by preventing flex overflow in chat/sidebar pingdotgg/t3code#706 / feat: Codex OpenAI API key and base URL overrides from Settings pingdotgg/t3code#1174), action.yml, tested-base/ and the reusable workflow .github/workflows/fork-pr-queue.yml

Once all four are open, a comment links them.

Why this approach

• Disarm before the update. GitHub's auto-merge request records no head, and GitHub keeps auto-merge on after a push by anyone with write access. An update left armed would merge whatever the new head holds, read or not.
• Re-arm only for the read change. The queue merges the head the review read with the base (git merge-tree --write-tree) and compares the result, tree for tree, with the new head. It then re-arms pinned to that head (--match-head-commit). A force-push, a web conflict fix or an applied suggestion is a different tree, so it stays off with a comment.
• Only the project's own people pick the read head. The repository may be public, and anyone can comment there. Only comments and reviews from lens_author_associations (1/4) count, and only when shown and never edited. Anyone else's comment is ignored. It neither vouches for a head nor disarms a pull request.
• The plan is on disk before the disarm. --state-file holds {repo, pr, old, base, read}, written and synced before the first GitHub write. A run that stops half-way is finished by the next run first: against the review as it is then, and before any other pull request is touched. A pull request is never left off and silent.
• One pull request per run. Updating every behind pull request at once rebuilds them all after each merge, on the same runners.
• Its own git and files. It fetches into the queue's own bare repository (--cache), with credentials from gh auth git-credential only, never a helper the machine configured. Every git call runs with no hooks, no fsmonitor and no replace refs, because the token is in git's environment. The cache is used only as the queue made it: mode 0700, this user's, its own config. STATE is written 0600 and read only if no one else could have written it. The runner must run no pull_request job and no other repository's job (README, "The queue"). It refuses to start (exit 2, touching nothing) when the runner is GitHub-hosted, a path is relative or inside the job's workspace, --repo is not owner/name, the config is bad, or git is older than 2.38.

Rejected alternatives

• GitHub's merge queue (merge_group): it needs an organization-owned repository, and both the fork and SkyRPG are user-owned.
• Keeping auto-merge on through the update: see above. The updated head would merge unread.
• Keeping the plan in GitHub (a label or comment): that needs a GitHub write before the disarm, which can fail the same way the re-arm can. A local file is written before any GitHub write.
• Re-arming on "the checks are green": green checks say nothing about whether a lens read the change.
• Splitting pr_queue.py: see Size.

What it replaces in SkyRPG (the switch-over is row 15, not this PR)

At dd7244bd7e, scripts/skyrpg-dev/pr_queue.py:

• :64-224 (state paths, gh/git, the own clone, fetch, same_change, holds), :317-543 (view through recover), :565-594 (main, dry_run) and run()'s update path (:595-627, without the red and stuck branches, which are 3/4) become pr_queue.py.
• LOCK :48, STATE :51 and CACHE :54 (paths fixed for one machine) become --lock-file, --state-file and --cache.
• Dropped: adopt_old_state (:456-471), which adopted an older queue's state file. That migration is SkyRPG's.
• Stays in SkyRPG: start() and cancel_superseded (:544-563). They cancel SkyRPG's own queued ci.yml runs to free its build runners, so they belong to that project's runners, not the queue.
• Tests: scripts/skyrpg-dev/tests/test_pr_queue.py Update :54, Main :203, ReviewedHead :267, QueueKeepsTheReadChange :304, Recover :333, plus the queue cases of test_lens542_r2_di1.py and test_lens542_r3_di1.py (AStopHalfWayLeavesATrace, WhitespaceIsPartOfTheChange, AMovedHunkIsAnotherChange, PrefixesOfOneShaAreOneHead, AFailedRecoverIsNotDropped). All are ported under the same names in pr_queue.test.py, except PrefixesOfOneShaAreOneHead. It became ShortReadsNameNoHead, because a read must now be the full sha (1/4).

What the port changed: everything project-specific comes from the config; STATE names its repository; exit 3 (was 2) means "may be off and silent", so that 2 can mean "refused"; and the queue's own advice keeps its backticks and <sha> placeholder (lens.no_lens_text(keep_markdown=True)), which the source stripped from comments. After the security review, the port also differs from the source in these ways: only write-access lens comments count, git runs with no hooks, the cache and STATE must be this user's alone, and recover uses the base GitHub shows rather than the stored one.

State: what happens when this runs again, stops half-way, or meets existing/old/empty state?

The writes are: the lock, STATE, the cache, auto-merge off, update-branch, auto-merge on, and a comment.

Case	What happens	Tests
Two runs at once (a push run and the timer)	The second finds the lock held, exits 0 and touches nothing. 4/4's workflow also has a per-repository concurrency group that is never cancelled	Main.test_a_held_lock_runs_nothing
A stop between the update and the re-arm (killed at update, poll or arm)	STATE was written before the disarm. The next run re-arms at a checked head, or leaves it off with a comment, and touches no other PR until then	AStopHalfWayLeavesATrace.test_killed_after_the_disarm_then_the_next_run_re_arms_or_says_it_is_off, Recover.test_an_unfinished_recover_ends_the_run, Recover.test_the_plan_survives_a_new_process_and_an_emptied_lock_folder
A stop before the disarm, or any rerun	The PR is as before (armed, behind), and the rerun retries	Update.test_a_failed_disarm_means_no_update, Update.test_rerun_after_any_stop_touches_nothing
The PR force-pushed mid-run	The new head is not the read change merged with the base, so it stays off with a comment	Update.test_a_new_head_with_other_content_stays_off
An empty queue	queue empty, exit 0, nothing written	Main.test_an_empty_queue_does_nothing
gh pr update-branch fails, or the re-arm (gh pr merge --auto --match-head-commit) fails	It stays off with a comment that names the way out. STATE is removed once the comment is posted	Update.test_failed_update_or_arm_stays_off
No new head within wait_seconds	STATE stays, and the next run checks the head GitHub shows then	Update.test_no_new_head_in_time_keeps_the_plan_for_the_next_run
A lens comment posted after the stop	recover reads the review as it is now, not the stored read	Recover.test_a_lens_posted_after_the_stop_counts
A lens comment edited after posting (before or after the stop)	It reads nothing until that lens posts again, so the PR stays off with a comment	Recover.test_a_lens_comment_edited_after_the_stop_reads_nothing_until_that_lens_posts_again, WhoseCommentsCount.test_an_edited_lens_comment_does_not_count
A lens comment by someone without write access (any association outside lens_author_associations)	Ignored: it is neither a read nor a lens that read nothing, so it can neither re-arm an unread head nor disarm a reviewed PR. The run log names it	WhoseCommentsCount.test_a_strangers_lens_comment_never_moves_the_read_head, WhoseCommentsCount.test_a_strangers_junk_comment_cannot_disarm_a_reviewed_pr, WhoseCommentsCount.test_a_lens_comment_whose_author_does_not_count_is_named_in_the_run_log
A hidden lens comment or override (old rounds hidden as Outdated)	A hidden lens comment reads nothing until that lens posts again, so the PR stays off with a comment; a hidden override counts nothing	WhoseCommentsCount.test_a_hidden_lens_comment_reads_nothing, Sec2.test_all_lens_comments_hidden_as_outdated_does_not_vouch_for_an_unread_push, Sec2.test_a_hidden_override_does_not_count, Sec1R2.test_a_hidden_override_vouches_for_nothing
A short read (7 to 39 hex)	No head. It stays off with a comment: re-post with the full sha	ShortReadsNameNoHead.test_a_short_read_of_the_head_is_left_off_with_the_way_out
The PR retargeted to another base after the stop	recover checks against the base GitHub shows now, not the stored one	Recover.test_the_base_comes_from_github_not_from_the_plan
The comment fails after the disarm	Exit 3. STATE stays and the next run says it, once	Update.test_a_failed_comment_keeps_the_state, Recover.test_a_gh_error_keeps_the_state_and_says_so_once, Recover.test_a_fetch_error_that_does_not_heal_says_so_once
An old STATE (the source's format, no repo)	Finished as this repository's	Recover.test_a_plan_without_repo_from_the_source_queue_is_finished
Another repository's STATE (two queues on one file)	Exit 1, kept as it was	Recover.test_a_plan_of_another_repository_is_refused_and_touches_nothing
An unreadable STATE	Kept as STATE.bad. The run fails, and every later run warns	Recover.test_an_unreadable_state_is_kept_aside_and_never_silent
A STATE another user could have written (another owner, or writable by group or others; also an older queue's file with a looser mode)	Treated like an unreadable one: kept as STATE.bad, the run fails, nothing is re-armed. A new STATE is 0600	Recover.test_a_plan_another_user_could_write_is_kept_aside_and_not_followed, Recover.test_the_plan_is_this_users_alone
A cache half made (a killed git init or fetch)	A stale ref lock is removed; a half-made cache is set aside and rebuilt; leftovers go on the next rebuild	Recover.test_a_stale_ref_lock_in_the_cache_is_removed, Recover.test_a_half_made_cache_with_an_undeletable_file_is_set_aside, Recover.test_leftovers_are_removed_by_the_next_rebuild
A cache someone else changed (a planted hook, a config the queue did not write, a mode open to others, info/grafts, shallow or alternates)	No hook ever runs. Any other cache is set aside and rebuilt 0700 with the queue's config and no template hooks	OwnCache.test_a_planted_hook_never_runs, OwnCache.test_a_cache_with_another_config_or_open_to_others_is_rebuilt, OwnCache.test_a_cache_with_grafts_shallow_or_alternates_is_rebuilt, OwnCache.test_a_new_cache_is_this_users_alone_with_the_queues_config
A dry run	No lock, no STATE, no cache, no comment	Update.test_dry_run_changes_nothing, Update.test_dry_run_leaves_an_existing_cache_as_it_was, Main.test_a_dry_run_takes_no_lock_and_shows_a_stopped_plan

Round 2 (security reviews SEC-1 and SEC-2, round 2)

Rebased onto main after #15 and #16 merged (they changed the same test runner): the runner is main's (presentTests), .github/fork-ci-tests.txt lists the tests of both, and this stack's end-to-end runner test stays beside main's. The lens fixes are in 1/4. Here:

Finding	Fix	Test (red at the old head 2e661e1 and at 4/4's c366408, green now)
Major: every lens comment hidden made the queue vouch for an unread push	1/4's rule; the queue's comment names the way out (that lens posts again, or the override, also for a lens no longer used)	Sec2.test_all_lens_comments_hidden_as_outdated_does_not_vouch_for_an_unread_push
Minor: a hidden override re-armed its head	1/4's rule	Sec2.test_a_hidden_override_does_not_count, Sec1R2.test_a_hidden_override_vouches_for_nothing
Minor: ignored lens-looking posts left no log line	1/4's log line reaches the queue's run log	WhoseCommentsCount.test_a_lens_comment_whose_author_does_not_count_is_named_in_the_run_log
Note: cache_problem did not check info/grafts, shallow or objects/info/alternates	A cache that holds any of them (or objects/info/http-alternates) is set aside and rebuilt. The README still says a same-user job can change the cache after the check: only the runner rule stops that	OwnCache.test_a_cache_with_grafts_shallow_or_alternates_is_rebuilt

What happens when this runs again, stops half-way, or meets existing/old/empty state? No new write step. The cache rebuild is the existing set-aside-and-rebuild path with three more reasons: a rerun finds the new cache clean and uses it; a rebuild stopped half-way leaves CACHE.new, which the next run sets aside like before; an old cache from an earlier queue version holds none of these files and is kept. A PR left off because of a hidden lens comment gets one comment, and the next run reads the review as it is then (a new comment of that lens, or an override, re-arms it through the same-change check).

Round 3 (SEC-2 round 3, #19 (comment))

r3 finding	Fix	Test
Major (in #18's lens.py): a stranger's huge comment stalled every run	fixed in #18 (author first, linear patterns)	in #18
Minor: a planted objects/info/commit-graph(s) could change the parents git reports	core.commitGraph=false joins GIT_SAFE (every git call)	OwnCache argv test pins it

General vs project

• General (here): one update per run, disarm/update/re-arm with the same-change check, the STATE plan and its recovery, the cache and its repair, the lock, the refusals and the exit codes.
• Project (config and CLI): the repository, the required checks, the commands and doc path quoted in comments, the merge method, where STATE, the cache and the lock live, and the runner.

Tests

• pr_queue.test.py: 98 tests on real commits in a temporary world. A local bare repository stands in for GitHub, and gh is a fake. A guard fails the whole run if anything starts the real gh. lens.test.py: 33.
• node .github/scripts/fork-ci-tests.cjs exits 0 at this commit, run from a clean copy with GH_TOKEN/GITHUB_TOKEN unset and an empty GH_CONFIG_DIR.

Size

pr_queue.py is 555 lines; lens.py changes by 18. That is above the ~400-line rule, and there is no split, for these reasons:

• The disarm path and the recovery are one state machine. A PR with only the update would turn auto-merge off with no code to finish a stopped run. A PR with only the recovery would have nothing to recover.
• The cache and fetch helpers (about 100 lines) serve only the same-change check.
• The port keeps the source's order and names, so it can be read side by side with pr_queue.py.

There are also 1292 test lines.

Round 4 (last round) and merge

r4 lenses: correctness SAFE (#21 (comment)), security SAFE (#21 (comment)), at heads 1f3382b/efae48e8/5480bf0a/11e9ca28. Then rebased on main after #27 merged: the only conflict was the sorted test list .github/fork-ci-tests.txt (both sides added lines; kept both). All suites pass at each new head.

Minors left at merge (follow-up row with the other stack minors):

• pr_queue.py:64: the cache-rebuild file list lacks info/attributes (a planted * merge=union turns a conflict clean; only a same-user job could plant it, which the runner rule excludes).

🤖 Generated with Claude Code

[scuttletoby]

Stack links (review and merge in order):

1. fork-ci pr-queue 1/4: lens review rules and a strict config #18: lens review rules and a strict config (base main)
2. fork-ci pr-queue 2/4: the queue's merge path (pr_queue.py) #19: the merge path, pr_queue.py (base fork-ci pr-queue 1/4: lens review rules and a strict config #18)
3. fork-ci pr-queue 3/4: armed pull requests that cannot merge are told and passed over #20: armed PRs that cannot merge are told and passed over, SkyRPG [Feature]: escape button to go back to previous thread from settings page pingdotgg/t3code#1350 (base fork-ci pr-queue 2/4: the queue's merge path (pr_queue.py) #19)
4. fork-ci pr-queue 4/4: update a PR that was red only because its base was, and the reusable workflow #21: the base-was-red rule (SkyRPG Fix plan composer layout by preventing flex overflow in chat/sidebar pingdotgg/t3code#706 / feat: Codex OpenAI API key and base URL overrides from Settings pingdotgg/t3code#1174), action.yml, tested-base/ and the reusable workflow (base fork-ci pr-queue 3/4: armed pull requests that cannot merge are told and passed over #20)

#18 repeats the fork-ci/ test-runner change of #16. Whichever merges second drops it when rebasing.

[scuttletoby]

lens: security SEC-2 · reviewer: opus high (t3lens-stack-sec2) · read at: 2d1c5b4
covered: lens.py (all, at #18 and #19, incl. the #19 no_lens_text change), config.py (all), pr_queue.py (all: holds, fetch, same_change, update, update_off, leave_off, arm, recover, tell_once, main, run, cli); the #19 failure table; pr_queue.test.py Admin/OnlyWhatALensReadIsReArmed and the test helpers; .github/workflows/fork-pr-queue.yml and action.yml at #21 (skimmed)
checks: ran pr_queue.test.py (82 OK) and lens.test.py (20 OK) with GH_TOKEN/GITHUB_TOKEN unset and an empty GH_CONFIG_DIR. Traced who counts as a lens author: lens_posts drops only bot logins, and lens_entries/reviewed_head never look at authorAssociation (only override_sha does). Wrote a test in the suite's own style (Admin world): the owner's lenses data-integrity and correctness read P; a write-access push H1 adds code; then an account with authorAssociation NONE posts lens: data-integrity ... read at: <H1 7-hex> and the same for correctness. reviewed_head returns H1, update_off runs update-branch, holds(merge(H1, main), H1) is true and arm() re-arms: "#7: updated to 45563a741d53, exactly the change read at 820f5a38dedf merged with main: auto-merge back on". Still the same at the #21 head. The same comment as a direct lens.Review check gives (True, H1, 'every lens read it'). Also checked: 7/12/40-hex reads of one commit are one head (full_sha), and a prefix that two known shas share is None (fail closed); a force-push or conflict fix between read and arm is a different tree (holds), and a push between same_change and arm is refused by --match-head-commit; an override counts only from the configured write-access association and only unedited; an override-looking first line from a stranger counts as nothing; the queue's own comments carry no read at/lens: (no_lens_text), so they never become a read, even when posted with the owner's token. State question: a rerun after any stop, or a stop between the disarm and the re-arm, is finished from STATE by recover(). recover() reads the review as it is NOW, so forged comments posted while STATE is waiting are trusted there too (same root cause, same fix). An empty review (no lens comments) means R = the current head: re-armed unread by design (test_control_a_pr_without_lens_comments_is_still_re_armed), which is no worse than GitHub's own auto-merge.
finding: blocker fork-ci/actions/pr-queue/lens.py:249 (lens_posts; read by lens_entries :251 and reviewed_head :270, used at pr_queue.py:265 and :381). Any GitHub account can comment on a public repository (this fork is public, and the action is meant for other projects). Inputs: a PR armed and BEHIND, a push after the lenses (GitHub keeps auto-merge on after a write-access push), then a comment from an author with no write access that gives each existing lens name a read at: of the new head. Wrong result: the queue updates the branch and re-arms it, so code no lens read gets merged. SkyRPG's private repo hides this because only collaborators can comment there. Fix: count lens comments (and lens-looking comments) only from a configured set of write-access associations, the way override_sha does, and ignore everyone else's comments, not fail closed (see the next finding). (high) [test: Forged.test_a_strangers_lens_comments_do_not_vouch_for_a_push, as above: fails now and must pass]
finding: major fork-ci/actions/pr-queue/lens.py:260 (lens_entries). The same root cause gives a denial of service. A comment from any outsider that only looks like a lens report (for example looks fine, read at: deadbee1 or safe to merge) adds an entry with no sha, so reviewed_head gives sha None. update_off (pr_queue.py:291) then turns auto-merge OFF on that PR and comments. Also, a stranger who writes a full sha that shares the 7-hex prefix of an owner's short read makes full_sha ambiguous, so it returns None. With a public repo, anyone can disarm any queued PR again and again. (high) [test: a stranger's comment read at: deadbee1 on an armed, behind PR whose lenses all read its head must leave it re-armed]
finding: major #19 PR body, failure table. Two write steps have no row: gh pr update-branch failing, and the re-arm (gh pr merge --auto --match-head-commit) failing. Both are handled and tested (Update.test_failed_update_or_arm_stays_off), but the table does not list them. There is also no row for a lens comment by someone without write access, which is the blocker above. (high)
finding: minor fork-ci/actions/pr-queue/lens.py:251 (lens_entries). An edited lens comment counts as it reads now, and is ordered by its createdAt. Unlike the override, includesCreatedEdit is never checked, so a read at: edited to a newer head without a new read looks like a read, and it sorts before any override posted in between. Only accounts with write access can edit other people's comments, so this is no escalation, but it is not in the README's rules. (medium)
finding: minor fork-ci/actions/pr-queue/pr_queue.py:21. With head_check null (the config allows it, and README:48 says it "only changes the advice"), a write-access push to an armed PR that is already up to date merges at the new head without any lens read. The queue never touches such a PR. So the docstring's "Auto-merge only ever merges a head someone checked" holds only when a head_check required check exists. The README should say so, or the config should require head_check. (medium)

[scuttletoby]

lens: security SEC-1 · reviewer: opus high (t3lens-stack-sec1) · read at: 2d1c5b4
covered: #18 lens.py (all), config.py (all); #19 pr_queue.py (all: holds, fetch, same_change, own_cache, update, update_off, leave_off, arm, recover, tell_once, main, run, cli) and the lens.py diff; skimmed the #20/#21 pr_queue.py diff (red_checks, stuck, tell_red, tested_base, red_from_old_base, run), #21 fork-pr-queue.yml and action.yml; tests read where they back a claim.
checks: traced every path that arms (update_off and recover): R from reviewed_head, exact tree compare in holds (parent on base, depth 3), --match-head-commit with the head from the last view (a push between view and arm is refused by GitHub; a force-push mid-run stays off). Override: association + unedited + prefix at the start, quoted or bot copies ignored. Header only as the first plain line; code blocks, quotes, indents, fences and two headers fail closed; inline review comments are not fetched, so they never count (the safe direction). Workflow: only push/schedule/dispatch, permissions: {} + contents read, toolkit from job.workflow_sha, config path checked, token must differ from GITHUB_TOKEN, no pull_request_target; a contents/PR/issues-write token cannot disable Actions (that needs Actions or Administration write). Hardest case: who may write the comments that pick R. State question: rerun = lock, then recover from STATE first; stop half-way = STATE on disk before the disarm, recover re-reads the review; legacy STATE without repo is finished; empty history (no lens comments) only re-arms the head someone else armed. lens.test.py and pr_queue.test.py green at this head (GH_TOKEN/GITHUB_TOKEN unset, empty GH_CONFIG_DIR). My three tests below (a Base subclass of pr_queue.test.py, real commits) fail at this head and at #21's.
finding: blocker fork-ci/actions/pr-queue/lens.py:245 lens_posts keeps every non-bot comment and review; unlike override_sha, nothing checks authorAssociation or includesCreatedEdit. PR armed after its lenses read R; a write-access push H1 (auto-merge stays on, as this PR's "Why" says); then any GitHub user (public repo) posts a newer comment with the same lens names naming H1 -> reviewed_head = H1 -> the queue updates and re-arms merge(H1, base): unread H1 merges. Hidden (minimized) comments count too. Fix: only comments and reviews from a configured association count at all (a stranger's post is neither a read nor "a lens that read nothing"). (high) [test: Sec1.test_a_strangers_lens_comment_never_moves_the_read_head]
finding: major fork-ci/actions/pr-queue/lens.py:245 the same rule from the edit side: a lens comment edited after posting counts as it reads now, so its author can move it to an unread head after the review; Recover.test_a_lens_comment_edited_after_the_stop_counts_as_it_reads_now and the table row endorse this. includesCreatedEdit says it was edited, not by whom or what changed: an edited header should be a lens that read nothing until re-posted. (high) [test: Sec1.test_an_edited_lens_comment_does_not_count]
finding: major fork-ci/actions/pr-queue/lens.py:148 with :281 a short read resolves only against the PR's CURRENT commits. Lens names abc1234 (commit A); a force-push drops A and pushes B with the same 7-hex prefix (about 2^28 commit-message tries, minutes on a CPU) -> B is the one match -> R = B, re-armed. Shown: reviewed_head with head B, commits [B] and that header returns (True, B); with A still listed it is ambiguous (None), so only the force-push case is open. Fix: count only 40 hex, or add the PR's force-push history (HeadRefForcePushedEvent beforeCommit) to the candidates so a collision is ambiguous. 12 hex (2^48) is harder but not out of reach. (high on the logic, medium on how often short reads are used)
finding: major fork-ci/actions/pr-queue/pr_queue.py:157 own_cache reuses whatever bare repo it finds, and git() (:149) runs it with its hooks and config while GH_TOKEN (a contents + PR write token) is in the environment. Anything that can write the runner's disk (a pull_request job on the same self-hosted runner, a fork PR's job on a public repo) plants a hook and gets the token; I checked that a reference-transaction hook in a bare repo runs on git fetch and sees GH_TOKEN (git 2.43). A planted STATE read is also trusted by recover when the PR has no lens comments. Fix: -c core.hooksPath=/dev/null, no trust in the cache's own config (check owner/mode or rebuild), and "Use it" must require a runner (group) that runs no pull_request job, with the state/cache folder writable only by the queue's user. (medium)
finding: major fork-ci/actions/pr-queue/lens.py:245 (via lens_problem) denial of service: any GitHub user turns every armed BEHIND PR off by posting a comment saying "safe to merge" (it becomes a lens that read nothing -> R unknown -> disarmed and left off), and can repeat it after each re-arm. Same fix as the blocker. (high) [test: Sec1.test_a_strangers_junk_comment_cannot_disarm_a_reviewed_pr]
finding: major (PR body) the #19 state table has no row for gh pr update-branch failing, the re-arm (arm) failing, or no new head within wait_seconds; tests exist (Update.test_failed_update_or_arm_stays_off, Update.test_no_new_head_in_time_keeps_the_plan_for_the_next_run) but the rows are missing. (high)
finding: minor fork-ci/actions/pr-queue/pr_queue.py (#21, tell_red) the once-per-head mark matches a comment by anyone, so a stranger who posts the mark first silences the queue's notice; and the pr_queue tested base annotation can be written (or doubled) by the PR's own job code. Both only decide whether a red PR is told or updated; the re-arm still needs holds and the lenses, so no unread merge. Check the comment's author for the mark. (high)
finding: minor fork-ci/actions/pr-queue/README.md (#21, head_check) with head_check null, a push to an armed, up-to-date PR keeps auto-merge on and the queue never compares that head with the review again; the README says the key "only changes the advice". Say that a project without a required head check is not protected against a push after arming. (medium)
Verdict: not safe to merge until the blocker and the edit/short-read majors are fixed with tests that fail without the fix.

[scuttletoby]

lens: security SEC-1 · reviewer: opus high (t3lens-stack-sec1) · read at: 2e661e1
covered: round 2, whole files again at the new heads, not only the fix diffs: #18 lens.py (all) and config.py (all); #19 pr_queue.py (all: holds, write_text, cache_problem, own_cache, fetch, same_change, view, new_head, leave_off, read_advice, arm, update, update_off, write_state/drop_state, tell_once, recover, main, dry_run, run, cli); #20 tell_red (own-login mark) and #21 tested_base/red_from_old_base, fork-pr-queue.yml, action.yml, README "The queue", "The base was red" and "Use it"; the #19 state table; WhoseCommentsCount, OwnCache, Recover, ShortReadsNameNoHead and the own()/note() helpers in pr_queue.test.py.
checks: Suites run myself (GH_TOKEN/GITHUB_TOKEN unset, empty GH_CONFIG_DIR): #19 lens 30 OK, pr_queue 93 OK; #21 lens 30 OK, pr_queue 149 OK, tested_base 8 OK; #18 lens 28 OK. My three round-1 attack tests (stranger's read of an unread head, edited lens comment, stranger's junk disarming) failed at the old heads and now pass at #19 and #21.
Round-1 findings, each at its root: (1) stranger's comment moves the reviewed head: fixed in Review.counts, which drops a post outside lens_author_associations before lens_problem, so it is neither a read nor a lens that read nothing (also fixes (4), the disarm DoS); config allows only OWNER/MEMBER/COLLABORATOR, never CONTRIBUTOR/NONE. Probed directly: a NONE review with a newer read, a MEMBER comment when MEMBER is not listed, a post with no authorAssociation, a bot login with OWNER association: all ignored. (2) edited lens comment: the newest entry per lens with includesCreatedEdit not false reads nothing (EDITED); a review without the field fails closed; an edited older comment under a fresh newer one does not block. I checked that gh 2.102 really returns authorAssociation, includesCreatedEdit, isMinimized and viewerDidAuthor for comments and authorAssociation/includesCreatedEdit for reviews. (3) short read after a force-push: gone at the root, only a full 40-hex read names a head (FULL_SHA.fullmatch), 41 hex names none, upper case is lowered, and a short read is left off with "re-post with the full sha". (5) cache: hooks off on every git call (core.hooksPath=/dev/null, fsmonitor off, no replace refs), the cache is used only when lstat says a 0700 dir of this uid with exactly the queue's config, rebuilt otherwise, init --template= so no sample hooks; STATE is written 0600 with O_NOFOLLOW and followed only when this uid owns it and no group/other write bit is set. A swap of the cache dir after the check by another user is also refused by git's own dubious-ownership check. (6) the state table now has rows for update-branch failing, the re-arm failing and no new head in time. (7) the told mark counts only with viewerDidAuthor and as the comment's start (RedChecks.test_only_the_queues_own_comment_counts_as_told). (8) head_check is now required and must be one of required_checks.
New code attacked: recover now takes the base GitHub shows; changing a PR's base needs write access, and holds still demands exactly the read change on top of that base, so no unread code; a non-ref base name fails fetch (left off with a comment). Override: only override.author_association, unedited, prefix at the body start, 40 hex; a collaborator's override-shaped text counts as nothing. Queue comments carry no lens-looking text (no_lens_text; advice keeps markdown only when it is clean). Workflow: only push/schedule/dispatch, permissions: {} + contents read, toolkit from job.workflow_sha, token must differ from GITHUB_TOKEN; a contents/PR/issues-write token cannot disable Actions or edit workflow files (needs actions/administration or workflows permission).
Only documented, and whether that is acceptable: (a) a same-user job on the queue's runner can read the token and write STATE/cache: no code can tell another job of the same user apart, so the README rule (own runner or group, no pull_request job, own user, own folders) is the right place; acceptable. (b) hidden reviews: GitHub gives no isMinimized for reviews, so a review counts as shown; only lens-association authors count at all and hiding needs write access, so the worst case is a trusted author's retracted review still counting; acceptable. (c) forged tested-base note: the PR's own job code can write the annotation, but that only gets a red armed PR updated or told; the update still goes through reviewed_head + holds + --match-head-commit, and two notes are "unknown"; acceptable, and the README now says so.
State question: rerun = lock, then recover first; stopped half-way = STATE on disk (synced) before the disarm, finished next run against the review and base GitHub shows then, never off and silent; old STATE without repo is finished, another repo's is refused, a STATE another user could write goes to .bad and nothing is re-armed; a half-made or foreign cache is rebuilt; empty history (no counted lens comments) re-arms only the head someone with write access armed, and head_check (now required) covers up-to-date heads. Legacy SkyRPG comments with short or edited reads now read nothing: the PR is left off with a comment that says how to fix it, never silently armed.
finding: minor fork-ci/actions/pr-queue/lens.py:287 (reviewed_head, overrides via override_sha :204) an override comment counts while hidden (minimized): counts is applied to lens comments only. Inputs: lenses read R, the owner posts <prefix> <C>: ... and then hides it to take it back -> R = C and the queue updates and re-arms C. README "Lens comments" says hidden comments are neither reads nor reports. Needs an override-association author and write access to hide, so no stranger gets in; fix: skip a minimized override as well. (high) [test: Sec1R2.test_a_hidden_override_vouches_for_nothing, a Base subclass: qt.override(self.c, ...) with isMinimized=True after lens_at('correctness', reviewed) must leave f.armed None; fails at #19 and #21 now]
Verdict: safe to merge from the security side; every round-1 blocker and major is fixed at its root with tests that fail without the fix. The minor can go in this stack or a follow-up.

[scuttletoby]

lens: security SEC-2 · reviewer: opus high (t3lens-stack-sec2) · read at: 2e661e1
covered: lens.py (all, at #18 da09388 and #19 2e661e1: counts, override_sha, lens_problem, lens_posts, lens_entries, reviewed_head), config.py (all, incl. lens_author_associations and the required head_check), pr_queue.py (all at #19: holds, cache_problem, own_cache, fetch, same_change, update, update_off, leave_off, arm, tell_once, recover, main, dry_run, run, cli); the #20/#21 additions read in full (red_checks, stuck, tell_red with the viewerDidAuthor mark, tested_base, commit_checks, red_from_old_base, run); fork-pr-queue.yml, both action.yml files; the #19 state table; the round-1 sections of the PR bodies; WhoseCommentsCount, OwnCache, Recover and ShortReadsNameNoHead tests.
checks: suites green at every new head with GH_TOKEN/GITHUB_TOKEN unset and an empty GH_CONFIG_DIR: lens.test.py at #18-#21, pr_queue.test.py at #19/#20/#21, tested_base.test.py and fork-pr-queue-workflow.test.py at #21. Round-1 findings, at the root: (1) stranger lens comments: lens_entries now skips every post whose authorAssociation is not configured (counts), so a NONE/CONTRIBUTOR/FIRST_TIME_CONTRIBUTOR/MEMBER-not-listed "read" of an unread head neither re-arms nor disarms; my round-1 scenario is now WhoseCommentsCount.test_a_strangers_lens_comment_never_moves_the_read_head and passes (PR left OFF). (2) the junk-comment DoS is gone by the same rule. (3) edited lens comments read nothing (EDITED; reviews do carry includesCreatedEdit in gh 2.102, checked on a real PR). (4) reads must be 40 hex: 7/12/39/41-hex give no head (checked directly), so the force-push prefix collision is gone. (5) the cache: GIT_SAFE (no hooks, no fsmonitor, no replace refs) on every git call, and cache_problem rebuilds a cache that is not 0700/this uid/exact config. (6) STATE is 0600 and read only if this uid wrote it and it is not group/other-writable. (7) tell_red's mark counts only with viewerDidAuthor, so a stranger's copy no longer silences it. (8) head_check is required and must be a required check. (9) the missing table rows are there. Attacked the new code: an override counts only from its association and unedited; a stranger's override-looking or lens-looking comment is skipped before lens_problem; the queue's own comments (leave_off via no_lens_text, tell_red with config names, a 12-hex sha and a branch name that cannot hold read at <hex>, lens: or safe to merge) never become a read even when the token is the owner's; recover takes the base and the review from GitHub as they are now. Hardest case: what the empty review means now that hidden comments are ignored (finding 1). State question: rerun = lock, then recover from STATE before any other PR; stop half-way = plan on disk before the disarm, recover re-reads the review (strangers ignored there too); old STATE without repo is finished; STATE of another repo or user is refused; an empty or foreign cache is rebuilt; an empty review vouches for the current head, which is the gap below when the review is only hidden. What stays only documented, and whether that is acceptable: a job on the queue's runner as the same user (it can read the token from the process anyway, and could plant info/grafts, objects/info/alternates or shallow in the cache, which cache_problem does not check): acceptable only as the README states it, a dedicated runner and user; no in-process check can beat the same uid. Hidden reviews count as shown (gh gives no isMinimized for reviews): acceptable, only configured authors can post them, so a hidden review can only keep a trusted read in play. A forged tested-base note (and a non-Actions check whose detailsUrl names another job): acceptable, it only decides update-or-skip and the told text; the re-arm still needs holds() against a counted read, so at worst one extra update per base push for a red armed PR. MEMBER/COLLABORATOR not meaning write access: acceptable as an explicit opt-in the README warns about. Workflow: unchanged posture (permissions {} + contents read, push/schedule/dispatch only, toolkit at job.workflow_sha, token must differ from GITHUB_TOKEN; it cannot disable Actions).
finding: major fork-ci/actions/pr-queue/lens.py:289 (reviewed_head if not found, with counts() at :202), used at pr_queue.py:315 and :432. Hidden comments are now ignored, so a PR whose only lens comments are hidden counts as "no lens comments", and the queue then vouches for the head it finds. Inputs: lenses read R, the PR is armed, a write-access push adds C (auto-merge stays on), and the round-1 lens comments are hidden as Outdated (routine once a new round is due) before the new lenses post. Wrong result: update() takes R = C, updates and re-arms merge(C, main): "#7: updated to 6dce31156c8f, exactly the change read at 355a0ecabe51 merged with main: auto-merge back on". Before round 2 the hidden read of R kept it OFF; edited comments already fail closed here, hidden ones fail open. Deleting every lens comment does the same, but hiding old rounds is normal hygiene, not an attack. Fix: a hidden lens comment by a counted author still makes the PR reviewed (an entry that read nothing, like an edited one), so only a PR that never had a counted lens comment takes the "current head" path. Only the head_check still stands between this and a merge. (high on the logic, medium on how often) [test: Sec2.test_all_lens_comments_hidden_as_outdated_does_not_vouch_for_an_unread_push, a Base subclass in pr_queue.test.py: one lens_at(reviewed) with isMinimized=True, head C; fails at #19 and #21]
finding: minor fork-ci/actions/pr-queue/lens.py:208 (override_sha). A hidden override still vouches: it never checks isMinimized, while README:76 says hidden comments are neither reads nor reports. An owner who hides a mistaken override instead of deleting it still gets that head re-armed (shown: a lens read of an older commit plus a hidden override of C re-arms merge(C, main)). Fix: require isMinimized false in override_sha too. (high) [test: Sec2.test_a_hidden_override_does_not_count, fails at #19 and #21]
finding: minor fork-ci/actions/pr-queue/lens.py:263 (lens_entries skips non-counted posts silently). A wrong lens_author_associations (say the lens accounts are COLLABORATOR and only OWNER is listed) makes every reviewed PR look unreviewed, so the queue vouches for each current head and nothing in the run says why. Fix: print (log only, no comment) when a skipped post holds a valid first-line lens header, so a misconfig shows in the run log. (medium)
Verdict: not safe to merge until the hidden-review major is fixed with its test; the round-1 blocker and majors are fixed at the root, and the two minors can follow.

[scuttletoby]

lens: security SEC-1 · reviewer: opus high (t3lens-stack-sec1-r3) · read at: 5e94aa7
covered: round 3, whole files again at the new heads, after the round-1/2 security comments on #18/#19: #18 lens.py (all: counts, hidden, override_sha, lens_problem, lens_posts, lens_entries, reviewed_head) and config.py (all); #19 lens.py diff and pr_queue.py (all: holds, cache_problem with UNTRUSTED_FILES, own_cache, fetch, same_change, view, new_head, leave_off, read_advice, arm, update, update_off, STATE, tell_once, recover, main, dry_run, run, cli); the #19→#21 pr_queue.py diff for new paths that read the review (none: arm is still called only from update_off and recover), fork-pr-queue.yml permissions/triggers; README "Lens comments" and "The queue"; the #18/#19 round-2 tables and state paragraphs.
checks: lens.test.py 31 OK at #18 and 33 OK at #21, run myself (GH_TOKEN/GITHUB_TOKEN unset, empty GH_CONFIG_DIR); pr_queue.test.py not re-run (machine over capacity), relied on the evidence (5 red before, 98 OK after) and the green Fork tests. Attacked the new hidden rule with direct Review probes. (1) Can hiding re-arm unread code? No: each lens's NEWEST counted post wins whether hidden or not, so hiding a newer comment never brings an older read back; a hidden newest gives no sha; every lens hidden gives (True, None), never "no lens comments". (2) Can un-hiding? Only by write access, and it restores a read a counted author really posted; no new head appears. (3) Can a non-counted author's hidden post block a PR? No: counts runs before the hidden check, so a NONE/CONTRIBUTOR post, hidden or shown, lens-shaped or "safe to merge", is skipped and only logged (ignored a lens-looking post <url>: author association NONE ..., no body text, so no workflow-command injection through the log). (4) Hidden override: refused in override_sha; an override newer than a hidden lens covers it, an older one does not. (5) A comment without isMinimized counts as shown (fail-safe direction: a trusted read stays), a missing includesCreatedEdit still fails closed. (6) Remaining ways a reviewed PR becomes "no lens comments": deleting every lens comment, removing the lens account as collaborator, or narrowing lens_author_associations (now logged): all need the owner or write access, and the required head_check still guards an up-to-date head; acceptable. (7) Cache r2 fix: grafts/shallow/alternates rebuilt; the class is wider (a same-user writer can also plant objects/info/commit-graph or swap object files), but the README now says only the runner rule stops a same-user job, which is the right boundary; acceptable as documented. Round-1/2 fixes re-checked at the root: strangers ignored, edited reads nothing, 40-hex only, hooks off, STATE 0600/own uid, viewerDidAuthor told mark, workflow permissions: {} + contents read, push/schedule/dispatch only, toolkit at job.workflow_sha. State question: rerun reads the hidden state fresh (same comments, same answer); stopped half-way, recover reads the review as it is then, so a comment hidden after the stop leaves the PR off with the hidden advice; old state (SkyRPG PRs whose old rounds are hidden) stays off until that lens posts again or an override covers it, said in one comment; empty (no counted lens comment ever) re-arms only the head found, as designed. No new write step this round; the #19 table has the hidden row with tests.
finding: minor fork-ci/actions/pr-queue/lens.py:286 (lens_entries, the NO_HEADER entry keyed per post) with pr_queue.py:298 (read_advice). A counted author's lens-looking post that is not a valid read (a header in a code block, a chat line "safe to merge") is its own entry with no sha forever: re-posting the lens correctly does not replace it, and since this round hiding it does not either. Shown: code-fenced lens: correctness · read at: <R> then a correct lens: correctness · read at: <R> gives (True, None), also with the first one hidden. The queue's advice says "re-post it that way", which leaves the PR off again every run; only an override or deleting that comment clears it. Fail closed, no unread merge, but it is the pingdotgg#1350 pattern (off with advice that cannot work), so it matters for the lens gate and merge-queue batching: make the advice (and the lens_entries docstring at :266) say "delete it (hiding is not enough), or post the override". (high) [test: a fenced header, then the same lens re-posted correctly: the queue comment must name delete/override, not only re-post]
finding: minor #18 PR body, "Why" bullet 3 still says lens comments count "only while they are shown (not hidden)"; the rule is now that a hidden one is a lens that read nothing. Doc only. (high)
Verdict: safe to merge from the security side. The round-2 major (all hidden = unreviewed) is fixed at the root with tests red before and green after, the two round-2 minors are fixed, and the new rule cannot be used to re-arm unread code or by a non-counted author to block a PR. Both minors can go in the merge list.

[scuttletoby]

lens: design · reviewer: opus high (t3lens-stack-design) · read at: 5e94aa7
covered: whole stack #18-#21 as one design, full report: #21 (comment)
checks: as in the full report (own view first, every write step against the failure tables, state question, the four suites run at 029c7ac)
finding: minor fork-ci/actions/pr-queue/pr_queue.py:528 the reason for rejecting a plan kept in GitHub does not hold (a failed marker write before the disarm means no disarm). The disk plan is what forces the dedicated self-hosted runner. Matters for the batching work, not for this PR. (medium)
Verdict #19: safe to merge.

[scuttletoby]

lens: correctness (#19) · reviewer: opus high (t3lens-18-19-corr) · read at: 5e94aa7
covered: full diff of #19 against #18's head a7c8b7e: fork-ci/actions/pr-queue/pr_queue.py (all functions: holds, set_aside, remove_old, write_text, sync_dir, git_problem, Queue.cache_problem/own_cache/has/fetch/same_change/view/new_head/leave_off/read_advice/arm/update/update_off/write_state/drop_state/tell_once/recover/main/dry_run/run, cli), lens.py changes (no_lens_text keep_markdown, SHORT), lens.test.py additions, README.md, .github/fork-ci-tests.txt; pr_queue.test.py read by test names and the cases cited below.
checks: pr_queue.test.py 98/98 OK, lens.test.py 33/33 OK at this head (git 2.43, GH_TOKEN/GITHUB_TOKEN unset, empty GH_CONFIG_DIR). Traced every stop point in update(). (a) Killed before the disarm: PR still armed, recover sees it is not open_off and drops STATE. (b) Disarm error that GitHub applied anyway: SILENT, STATE kept (tested). (c) Killed after update-branch: recover re-checks the current head against the current review and base. (d) update-branch a no-op: no new head, exit 3, and recover arms at old only if old holds the read change. (e) Arm OK but killed before drop_state: not open_off, so dropped. (f) Comment fails: SILENT, recover says it later. holds(): a later manual clean merge of the base and a second queue update are both accepted through the recursion. A force-push, a conflict fix or a moved hunk is a different tree and stays off. A base that moved between update and fetch still has the merge parent as an ancestor. Races: a push between view and disarm, or after update-branch, only changes the head that holds() rejects. The arm is pinned with --match-head-commit. The queue's own comments: why goes through no_lens_text, and the advice keeps markdown only when cleaning would not change it. So with a counted (PAT) author, its comment is neither a lens nor a look-alike. State question: answered by the failure table. Every write step (lock, STATE, cache, disarm, update-branch, re-arm, comment, tell_once's told, STATE.bad) has a row and a test. A rerun touches nothing new. Old STATE without repo is finished; another repository's STATE is refused. Empty queue: exit 0, nothing written.
finding: minor fork-ci/actions/pr-queue/pr_queue.py:298-301 read_advice for NO_HEADER tells people to "re-post it that way". A re-post does not clear the old malformed comment (#18's lens.py:286 keys it by its URL), so someone who follows the advice and runs ready gets the same "left OFF" comment again. Only deleting it, editing its first line to not a lens:, or the override (also in the advice) works. Ported from the source (skyrpg pr_queue.py:391). Fails closed. Matters for the lens gate. (high) [test: test_a_headerless_lens_comment_says_how_to_repost pins the wrong text; add one where a re-post alone still leaves it off]
finding: minor fork-ci/actions/pr-queue/pr_queue.py:438-455 recover() re-arms any open PR whose auto-merge is off, at a head that holds the read change. That includes a PR a person turned off on purpose after the stop. STATE can live for several runs (no new head in time, or a gh/fetch error with tell_once), so the window is not one run long. It never re-arms an unread head, but it overrides a person's disarm without saying so. Matters for merge-queue batching: a multi-PR plan in STATE widens this. (high) [test: none]
finding: minor fork-ci/actions/pr-queue/pr_queue.py:518-521 a PR in UNKNOWN merge state is skipped like DIRTY. GitHub often shows UNKNOWN for a few seconds after a push to the base, which is exactly when the push-triggered run starts. A younger behind PR can then be updated ahead of an older one (out of order, and the older PR's CI runs again later). The timer bounds it, and it is inherited from the source. Matters for batching and order. (medium) [test: none]
Verdict for #19: safe to merge. All three findings are minors for the merge list. No blocker or major found.

[scuttletoby]

lens: security SEC-2 · reviewer: opus high (t3lens-stack-sec2-r3) · read at: 5e94aa7
covered: round 3, whole files again at the new heads: #18 lens.py (all: clean_inline, norm_line, lens_header, Review.counts, hidden, override_sha, lens_problem, lens_posts, lens_entries, reviewed_head) and config.py (all); #19 pr_queue.py (all: holds, write_text, cache_problem with UNTRUSTED_FILES, own_cache, fetch, same_change, view, new_head, leave_off, read_advice incl. the new HIDDEN advice, arm, update, update_off, write_state/drop_state, tell_once, recover, main, dry_run, run, cli); #21 fork-pr-queue.yml (timeout, concurrency, permissions) and the places reviewed_head is called at #21 (update, recover only); README "Lens comments"; the #19 state table and the round-2 fix table; the new tests test_a_hidden_lens_comment_reads_nothing_until_that_lens_posts_again, test_hidden_posts_that_never_counted_stay_ignored, test_a_hidden_override_does_not_count, test_a_lens_comment_whose_author_does_not_count_is_logged (lens.test.py), Sec2, Sec1R2, WhoseCommentsCount hidden/log tests and OwnCache.test_a_cache_with_grafts_shallow_or_alternates_is_rebuilt (pr_queue.test.py).
checks: Suites run myself with GH_TOKEN/GITHUB_TOKEN unset and an empty GH_CONFIG_DIR: lens.test.py OK at #18, #19 and #21; pr_queue.test.py OK at #19 and #21; tested_base.test.py OK at #21. My round-2 major and minors are fixed at the root: a hidden post of a counted author is now an entry that reads nothing (HIDDEN), so "every lens comment hidden" no longer takes the "no lens comments -> current head" path (Sec2.test_all_lens_comments_hidden_as_outdated_does_not_vouch_for_an_unread_push, red at the old heads per the fix table); override_sha refuses a hidden override; a non-counted lens-looking post is logged once per post with only its URL/login and association (no comment text, so no log injection: logins and URLs hold no newline). Attacked the new hidden rule: (1) hiding the newest read of a lens fails closed (no older read comes back); hiding an older read under a newer one changes nothing; (2) un-hiding brings back a read of the sha that lens really read, and holds() still demands exactly that change on top of the base, so un-hiding cannot re-arm unread code; (3) a stranger's hidden post (or any non-counted post) is dropped by counts() before it can become an entry, so it cannot block a PR (test_hidden_posts_that_never_counted_stay_ignored); (4) a hidden junk post of a counted author now blocks until an override or deletion (before, hiding cleared it): fail closed, and read_advice names the override. Hardest case: what the round-3 reordering in lens_entries costs, since lens_problem now runs on every stranger's post (finding 1, measured). State question: rerun = lock, then recover first, which re-reads the review as it is now (a hide/un-hide between runs is taken as it is then); stopped half-way = STATE on disk before the disarm; old/empty state: an empty review vouches for the current head only when no counted author ever posted a lens-looking comment (hidden ones now count as reviewed); old STATE without repo finished; foreign STATE/cache refused or rebuilt. The state table has a row for every write step (disarm, update-branch, re-arm, comment, STATE) with tests. The one state the table promises but finding 1 breaks: "never OFF and silent" when every recover run is killed by the job timeout before it reaches a comment. Documented-only and acceptable: a same-user job on the queue's runner (README rule); a hidden review counts as shown (GitHub gives no isMinimized for reviews, only counted authors post them); authorAssociation is the author's current one, so removing a lens author's collaborator access or narrowing lens_author_associations drops their reads and can make a reviewed PR look unreviewed (an admin action, and the new log line now names each dropped post).
finding: major fork-ci/actions/pr-queue/lens.py:274 (lens_entries runs lens_problem before counts() at :276; slow regexes in clean_inline :58-59). Round 3 moved lens_problem ahead of the author check to log ignored posts, so every stranger's comment or review body now goes through clean_inline, whose !?\[([^\]]*)\]\([^)]*\) and <[A-Za-z][^>]*> are quadratic on unclosed brackets. Inputs: anyone posts one 65536-char comment of [ on the armed PR at the front of the queue. Measured: reviewed_head takes 0.00 s at the round-2 lens.py and 143-173 s at this head for that one comment; about nine such comments pass the workflow's timeout-minutes: 20 (fork-pr-queue.yml:45 at #21). Wrong result: update() (pr_queue.py:324) dies before write_state on every run, and the oldest armed BEHIND PR is always picked first, so no PR in the repository is ever updated again; if a STATE is pending, recover() (pr_queue.py:441) dies every run before leave_off/tell_once, so that PR stays OFF and silent, against the state table. No unread code merges (the gate stays closed); it is a stranger-reachable stall that round 2 did not have. Fix: call counts() first and run only a cheap test for the log line on non-counted posts (say lens in the first 300 chars of the body), and make clean_inline linear: </?[A-Za-z][^<>]*> and !?\[([^\[\]]*)\]\([^()]*\) keep every current output I tried and bring that comment to 0.01 s (the rest of lens_problem then stays under 1 s on every 64 KB shape I tried: <a, [, [](, - , 1. , # , fences, read at, lens:, spaces). (high) [test: Sec2R3.test_a_strangers_huge_comment_costs_no_time in lens.test.py: reviewed_head(pr(stranger('[' * 65536))) returns (False, None, 'no lens comments') in under 2 s; fails at this head, passes with either fix]
finding: minor fork-ci/actions/pr-queue/pr_queue.py:55 (UNTRUSTED_FILES). The list says it holds every file that makes the cache's history other than fetched, but objects/info/commit-graph and objects/info/commit-graphs/ also give git the parents that holds() reads (rev-list --parents, merge-base --is-ancestor), and the queue never writes them either (fetch does not write one with the default fetch.writeCommitGraph=false). Only a same-user job can plant one, which the README already leaves to the runner rule, so this is completeness, not a new hole. Fix: add -c core.commitGraph=false to GIT_SAFE (one place, covers both paths). (medium) [test: extend OwnCache.test_a_cache_with_grafts_shallow_or_alternates_is_rebuilt or assert the flag in GIT_SAFE]
Which minors matter later: the pattern behind the major (author check before any text parsing, linear cleanup) matters for the later lens gate and merge-queue batching work, since both read the same strangers' comments through these functions; the commit-graph minor does not.
Verdict: #19 not safe to merge until the major is fixed with its test (small, local fix); every round-1 and round-2 security finding is fixed at its root, and the hidden-comment rule holds against hide/un-hide and against non-counted authors.

[scuttletoby]

lens: tests · reviewer: opus high (t3lens-stack-tests) · read at: 5e94aa7
covered: lens.test.py and pr_queue.test.py in full, plus the code under test (lens.py, pr_queue.py, config.py). The full report (all four PRs) is on #21: #21 (comment)
checks: Green at this head: lens 33, pr_queue 98. Mutation runs at this head catch: hooksPath dropped, any cache config, UNTRUSTED_FILES cut down (grafts only, or no http-alternates), recover's owner/0o022 check, the base taken from the plan, and a repo mismatch. Every write step in the failure table has a row with an existing test: disarm, update-branch, re-arm, comment, the STATE write/drop and the cache rebuild. State question: a rerun, a stop at update/poll/arm, old or foreign plans and an empty queue are tested. The gaps are below.
finding: minor fork-ci/actions/pr-queue/lens.py:260 No test puts a post in reviews. Dropping the reviews line keeps every test green. (high) [test: none]
finding: minor fork-ci/actions/pr-queue/pr_queue.py:175 The cache check 0o077 passes the tests when weakened to 0o022: only 0o770 is tested. A cache that others can read (0755) would be kept. (high) [test: OwnCache.test_a_cache_with_another_config_or_open_to_others_is_rebuilt]
finding: minor fork-ci/actions/pr-queue/pr_queue.py:118 Three lines can each be removed with every test green: fchmod 0600 (:119), O_NOFOLLOW on the .tmp (:118) and O_NOFOLLOW in recover (:413). A leftover 0644 .tmp, or a planted symlink, is never tested. (high) [test: none]
finding: minor fork-ci/actions/pr-queue/pr_queue.test.py:270 The assertOff check that a queue comment never counts as a lens uses a post with no authorAssociation, so its reviewed_head assertion cannot fail. The queue's own comments come from a write-access account. (high) [test: Update.assertOff]

[scuttletoby]

lens: correctness (#19) · reviewer: opus high (t3lens-stack-r4-corr) · read at: efae48e
covered: round 4 of the stack (only the three new commits changed; every other commit is identical to round 3 by range-diff). This PR: efae48e (core.commitGraph=false in GIT_SAFE, used by every git call). Full report: #21 (comment)
checks: as in the full report (all four suites at #21's head and lens.test.py at #18's head OK, GH_TOKEN/GITHUB_TOKEN unset, empty GH_CONFIG_DIR; old versus new on the whole corpus; the state question for each change)
finding: none for this PR.
Verdict #19: safe to merge.

[scuttletoby]

lens: security · reviewer: opus high (t3lens-stack-r4-sec) · read at: efae48e
covered: round 4 re-check of efae48e (core.commitGraph=false in GIT_SAFE) and every git call in the cache, full report: #21 (comment)
checks: as in the full report (range-diff, the four suites at 11e9ca2, a planted-file test on git 2.43)
finding: minor fork-ci/actions/pr-queue/pr_queue.py:57 UNTRUSTED_FILES lacks info/attributes: a planted * merge=union makes merge-tree --write-tree (:77) merge a conflict cleanly, so holds can accept a tree the real merge refuses. Same-user only (the runner rule), same class as the commit-graph minor. Fix: add the one entry. (high)
Verdict #19: safe to merge (the commit-graph minor is fixed).