Where should the engine read in-ticket commands from?

[xqliu]

/milestone (#1290, PR #1293) is the first in-ticket command, and #1294 lifts its
parsing, permission and receipt machinery into a reusable layer. Neither touches the
question this thread is for: which tickets should the engine read commands from?

Today the answer is the narrowest possible one, and that was the right call for a
first command. But it decides which commands can exist at all, so it deserves a
decision rather than a default.

Where commands are read today

The only call site is nested inside the idle path:

# src/orbi/milestone.py, advance_active_milestone_on_idle
if not auto_next_milestone:                      # only when auto-advance is off
    issue_number = _pending_milestone_issue(...) # only the confirmation ticket
    if issue_number is not None:
        process_milestone_commands(...)          # only then are comments read

Three conditions stack: the idle tick, auto-advance disabled, and a ticket the engine
itself opened. Outside that intersection, a /command comment is never seen.

This is not an oversight — #1290 states it plainly:

空闲 tick 目前不读任何 comment——steering 的轮询只在 Pi 会话存活期间运行

Why it constrains the command set

The current shape serves exactly one interaction: the engine asks a question on a
ticket it opened, and a human answers there. /milestone fits perfectly.

Commands that act on a delivery ticket do not fit at all, because those tickets are
written by humans and the engine never opens a confirmation ticket for them:

• /approve — clear a human-review gate without the label dance
• /retry — requeue a delivery that ended ai-blocked
• /cancel — stop an in-flight delivery
• /model <name> — override the engine for one delivery

None of these has anywhere to land right now.

Option A — keep the narrow scope

Commands live only on engine-opened tickets. New interactions must follow the same
shape: the engine opens a ticket asking something, the human answers there.

For

• Zero additional API calls per tick
• Idempotency is simple: the receipt marker dedupes against one ticket's comment list
• No cursor state — the whole comment list is small and bounded
• Abuse surface is tiny: only tickets the engine itself created

Against

• The four commands above cannot be built
• "Engine opens a ticket to ask" is a heavy ceremony for a lightweight answer
• A tenant who wants to retry a blocked delivery still has to relabel by hand, which
  is exactly the CLI-shaped gap 里程碑推进只接受 CLI：托管租户收到提问却给不出引擎认的答案，且建 milestone 与开 release 票仍需人工 #1290 set out to close

Option B — scan ai-* tickets

The idle tick scans open issues carrying an ai- label and reads their comments.

For

• The four commands become possible
• Matches where people already comment, no extra ticket

Against

• N extra API calls per tick, plus pagination and rate-limit handling
• Needs a "last scanned" cursor per ticket, or every tick re-parses full comment
  history
• Idempotency gets harder: receipts currently dedupe within one ticket; across many
  tickets that needs rethinking
• Wider abuse surface — write permission is already required, but the reachable set
  grows from "tickets we opened" to "every delivery ticket"

Not to be confused with steering

steering_poll_seconds (src/orbi/config.py:144) polls while a Pi session is
alive, inside the delivery loop (src/orbi/runner.py:3148). In-ticket commands run
on the idle tick. Different mechanism, different lifetime — they should not share
a poller, and merging them would couple command availability to whether a delivery
happens to be running.

What would settle this

Option B's cost is only justified by a command that needs it. So the useful question
is not "which option is better" but:

Which of /approve, /retry, /cancel, /model do we actually want — and is any
of them worth a per-tick scan?

If the answer is none of them for now, A stands and this thread can rest. If one of
them is wanted, it should be specified first, and the scan scope decided as part of
building it — not before.

Related: #1290 (the first command), #1293 (its implementation), #1294 (extracting the
reusable layer, deliberately scope-neutral).

[xqliu]

Read Prow and bors-ng at HEAD to see how mature implementations handle this. One
finding bears directly on the scope question here, and one on a command we might want.

Prow chose the widest possible scope, and pays for it structurally

Prow scans every comment on every configured repo, then broadcasts it: for each
comment it spawns a goroutine per enabled plugin, and each plugin decides for itself
whether the comment concerns it (pkg/hook/events.go:82-100). N comments × M plugins
regex matches per event.

The cost shows up as a patch on the missing router — the trigger plugin had to add
an early bail (pkg/plugins/trigger/generic-comment.go:67, commentMatchesTrigger)
because it was doing real work on comments that were never meant for it.

This is the price of "read everywhere": without a central router, every handler must
carry its own guard, and the guards drift. It argues that if we ever widen the scope,
the dispatch must stay a central route, not a broadcast — one parse, one lookup,
one handler.

/retry is more dangerous than it looks

Option B lists /retry as a motivating command. bors has it, and it is the source of
their one real privilege-escalation path (lib/web/command.ex:513-516):

def run(c, :retry) do
  {commenter, cmd} = Logging.most_recent_cmd(c.patch)
  run(%{c | commenter: commenter}, cmd)
end

:retry requires only :member (command.ex:376), but it replays the last command
under the original commenter's identity, entering through run/2 and so skipping
the permission gate that lives in run/1 (command.ex:337-354).

So a member can replay a reviewer-level r+. The lesson is not "don't build
/retry" — it is that a replay command must re-authorise the current caller for
the replayed command's level, and that there must be exactly one entry point to a
handler. #1294 now requires the single-entry-point property for this reason.

Does this change the A/B calculus?

Not decisively, but it sharpens the costs already listed under B:

• "N extra API calls per tick" understates it — Prow's experience is that the
  dispatch also needs to stay centralised, or every command grows its own filter
• /retry and /cancel both act on in-flight deliveries, so both need the
  re-authorisation rule above. That is real design work, not a thin handler

Still no reason to decide now. The question in the opening post stands: which of
/approve, /retry, /cancel, /model do we actually want? If one of them is
wanted, it gets specified first and the scope is decided as part of building it.

Sources are kubernetes-sigs/prow and bors-ng/bors-ng at HEAD; every path and line
number above was verified by grep against a fresh clone.