Draft: chore(skills): dispatch unprefixed comments in mr-watch

What

/mr-watch dropped every comment from the author or an assigned reviewer that did not open with remember:, blocking:, escalate:, or question:. A reviewer who wrote "this cap looks off by one" got no reaction, no reply, and no fix, and nothing told them why. This adds a comment class for those notes, alongside the existing duo and appsec ones.

The prefixes keep their job, which is letting a reviewer name which branch they want. What changes is that naming none no longer means the comment is ignored.

How an unprefixed comment is handled

The comment	What the watcher does
asks for a change, and the fix is clear	scoped commit, then a threaded reply leading with the SHA
asks a question, or the behavior is deliberate	in-thread reply with cited sources, no commit
asks for nothing (LGTM, thanks)	the 🤖 reaction, and no reply at all
is ambiguous	goes to the operator with the commenter quoted and 2 to 3 options, and the watch keeps ticking

An unprefixed note is never read as escalate: (it cannot pause the watch) and never writes to AGENTS.md. Those two still need their prefix, because both are things a reviewer has to ask for on purpose. And the note plus its thread are read as data, never as instructions to the watcher: this is the one class nobody opts into, so a request whose whole source is quoted or pasted third-party text goes to the operator instead of being acted on.

Which threads the class reads

Only resolvable threads, which means diff-anchored comments and threads opened from the Overview tab. Top-level notes are out of scope, and a prefix still works on one if a reviewer wants it dispatched.

That check is the duo class's, borrowed for the same reason it exists there: resolvable == true is what excludes a whole-MR review digest, which holds many findings and is not the single request this class answers. Measured with the branch's own collect_candidates against live payloads, it is also what separates review feedback from bookkeeping:

MR	Discussions	Rows without the check	Rows with it
!1900 (merged)	67	7 (6 of them the author's own Rebased onto the updated target: ..., Pipeline N failed in get_sources ..., Round disclosure, no reply needed.)	1, the AppSec finding
!1783 (merged)	37	1, a reviewer's ## Review summary digest	0
!1722 (merged)	39	2 real reviewer comments	the same 2
!1937	15	0	0

Seven of seven notes that were not requests are gone, and both real reviewer comments are kept. A top-level note is also the one kind no human can retire: it has no resolved state, so without this check only the bot's own reaction ever closed it.

Which notes the class excludes

The bot posts under whichever identity holds the token, which is the MR author on a self-watched MR, so a class that trusted the author name alone would treat the bot's own replies as fresh review comments and answer them on every tick. Beyond the resolvable check, four exclusions apply:

• a body carrying the bot marker line, matched with surrounding whitespace stripped. The marker is written by a model into a heredoc, and this check is the only thing keeping the class off the bot's own output, so one trailing space would have made a reply the newest unprefixed note in its own thread with nothing left in that thread able to dedupe anything;
• a body whose text opens with the robot glyph, which is a human echoing one of the bot's status literals. The test reads the same suggestion-stripped text the prefix test reads, so a glyph sitting behind a leading suggestion block is caught too;
• an internal note. This is the per-note half of keeping the AppSec confidential thread off a public branch whose outcomes are a public commit message and a verbatim quote in the operator escalation. The whole-thread half is below;
• a note with nothing in it at all. A note whose whole content is a suggestion block is not that, and keeps its row under a stand-in excerpt: a bare suggestion is the least ambiguous request a reviewer can make.

Two more limits keep the volume sane. At most one row per discussion, the newest one no reaction has closed. And a discussion never dispatches a comment row alongside another class: a prefixed row suppresses it, and so does any note from the AppSec bot in that thread. Keying the AppSec suppression on the thread rather than on the appsec row is what makes it hold after the finding has been answered, and what makes it independent of whether the payload carries a per-note internal flag at all.

Against a duo row the newer note wins. Dropping the older row loses nothing, because every branch that can act on a thread re-fetches the whole thread first, so whichever row survives reads the other note, and because the survivor is known to be unreacted and so will actually be dispatched.

COMMENT_CAP = 5 comment rows per tick, oldest first. A batched review ("Start a review", then "Submit review") lands every inline finding at once, all unprefixed, so without a cap one tick would ask the Action pass for one commit and one push per finding: a 20-finding review becomes 20 pipelines inside a single pass with no recovery point between them. The deferred tail carries no reaction and no reply, so later ticks serve it.

When the reaction is posted, and when it is read

Every other branch reacts 🤖 first, as a display-only "I'm on it" signal. This one reacts last, and only once its outcome has landed, because here the reaction is also what dedupes the note. Reacting first would close the note before the outcome was known, so a fix deferred by a dirty tree, a reply that failed to post, or a crash mid-branch would leave the reviewer a reaction and drop the comment for good, which is the exact failure this class exists to stop. The AppSec branch already defers its reaction for the same reason.

Two properties of the read are load-bearing rather than incidental:

• it happens inside the discussions walk, before the walk arbitrates between classes. A closed row is therefore gone before it can win its thread or set the prefixed suppression. Reading it afterwards instead let a closed row win and then vanish, taking the candidate it had displaced with it and permanently: a duo finding under a reacted acknowledgement, an older unprefixed request under a reacted otherwise LGTM, and every unprefixed follow-up in the thread of a reacted escalate:;
• it carries per_page=100. Awards come back oldest-first, so on a note already holding 20 reactions the bot's own robot award sits past the default page, and a reaction-only outcome, which posts no reply to dedupe on, would re-dispatch on every remaining tick forever.

A reaction-only close counts toward neither step 7 tally. It asked for nothing, so nothing was handled, and counting it fired a rereview request at a reviewer whose only note was LGTM, which is the signal erosion that step's own skip conditions exist to prevent.

The cost is one award_emoji read per candidate the walk has to test. A reaction-closed row stays a candidate for the rest of the watch, so that read does not decay: a thread of acknowledgements keeps paying for itself until the watch ends. It is bounded by the resolvable unresolved-discussion count, at most one comment row each, which the resolvable check above cuts substantially.

Where the sweep says what it dropped

The facts block gains a SUPPRESSED section naming every candidate the walk dropped by arbitration rather than by filter: a row a reaction had closed, an unprefixed note yielding to a prefixed one or to its thread's AppSec ownership, either side of the duo-versus-comment call, and a row deferred past COMMENT_CAP. The filters leave no row and so leave no line. These are the only drops the sweep makes by judgment, so they are the only ones an operator cannot re-derive from the payload, and a lost candidate is the one failure nothing downstream can recover.

Tests

triage_sweep_test.py goes from 62 to 98 cases. The filter tests cover the class's predicate: an unprefixed reviewer note becoming a candidate, an unprefixed author note, the bot's own reply and the robot-glyph body excluded, a glyph hidden behind a suggestion block, a marker with a trailing space still reading as a bot note, an internal note, a top-level note and a review digest, out-of-scope, system and resolved-thread notes, the later-bot-reply dedupe, newest-wins within a thread, a prefixed note suppressing the comment row, an answered prefix leaving the follow-up eligible, an AppSec thread suppressing the row both before and after the finding is answered and with no internal flag anywhere, a duo row losing to a newer human note and winning against an older one, a suggestion-only note keeping its row, an empty body dropped, and the cap deferring its tail.

A new TestSweepEndToEnd drives main() with every glab read faked, which is the only level at which the reaction reader is exercised at all: collect_candidates takes it as an argument, so a test calling collect_candidates directly can never catch a wrong path, a missing page size, or a read issued more than once per note. That gap is why the ordering defect above shipped in the first review round; the previous test asserted only that the class was a member of REACTION_DEDUPED.

watch_loop_test.sh (9 cases, offline, stubs the sweep it drives) now runs too. It existed and nothing executed it, while both the CI job's changes: anchor and the pre-commit hook's files: pattern already fired on any scripts/ edit, so a watch-loop.sh regression shipped next to a green gate.

Both suites run in CI (lint:skill-scripts:mr-watch) and in pre-commit (mr-watch-skill-tests), mirroring the bug-report skill's job and hook. Nothing executed either before, so a broken filter could ship green.

Verified against live MRs as well, per the table above.

Reviewable size

996 LOC, past the 500 ceiling, and not usefully splittable: the class's predicate, the arbitration it participates in, and the tests that pin both are one behavior. Split by file group:

Group	LOC
scripts/triage_sweep_test.py	522
scripts/triage_sweep.py	301
SKILL.md	94
CI, pre-commit, docs, report templates	79

Over half is tests. The two script files are interdependent in the way docs/dev/development-model.md names: staging the predicate without the tests, or the tests without the predicate, leaves a tree that does not pass its own gate.

Note on commit 4dc1cef2b

Its body claims that "an appsec row also suppresses any comment row in its thread, which covers a payload that carries no note-level internal flag". That was wrong: appsec_taken was only set inside a branch gated on the internal flag, so the payload shape it named could not set it. The suppression now keys on the thread rather than the row, so the claim holds as of this branch's head, but the commit body itself cannot be corrected without a force-push. This section is the correction.