home-network-k3s-impl/decisions/2026-11-07-pm-agent-pr-mentions.md

date, branch, title

date	branch	title
2026-11-07	feat/pm-agent-pr-mentions	The PM agent can be @-mentioned on a pull request — an EXPLICIT mention is the only trigger, reply only, shipped OFF

What changed

Issue #116 asked for the PM agent to react when it is @-mentioned on a pull request: read the PR, the changes and the thread, and reply in that thread. This branch implements the trigger, context assembly, dispatch and reply half. The change-authoring half (the operator's point 6 — revise an agent-owned branch versus open a fresh branch/PR against main) is deliberately absent and wants its own issue, as the PM said on the thread.

Revised 2026-11-07: continuation removed, an explicit mention is the only trigger

The first version of this branch carried a continuation rule: once a thread held a mention, every later non-bot comment in that thread triggered a run with no second mention. The operator withdrew that requirement after reading the first PR description:

Since there is no threads and the PR comments are flat, then the pm-agent should be looking for explicit mentions rather than a single mention and continued scrapping of comments within the PR in perpetuity

Read with their earlier "the 'same thread' requirement was based on my assumption that comments can be hierarchical. If not, then disregard", the requirement is withdrawn, not relocated. So the arming state is gone, in both code and fixtures:

• plan_pr_replies no longer carries an engaged/armed flag. A comment is answered iff (1) it is not the bot's own, (2) no earlier reply of the bot's marks it answered, and (3) it explicitly mentions the bot account. Nothing a comment's neighbours say can make it actionable, so a comment with no mention produces no reply and no model call.
• Applied uniformly — conversation comments, review bodies and inline review comments. Inline comments are genuinely threaded in Gitea, and keeping continuation for them alone was considered and rejected: it would be a second rule the operator did not ask for, and the brief said to stop and ask rather than invent a middle rule. Nothing turned out to be load-bearing on the arming state for the inline path (the inline path keys replies off the (path, position) group, which is a destination, not a trigger), so there was nothing to stop and ask about.
• Thread identity survives, but only as a reply destination. "What counts as the same thread" needed a policy while it was part of the trigger; now it only answers "where does this reply go", and the two answers are Gitea's own: the conversation, or the (path, position) group.
• reason on a plan is now always "mention". The field is kept because it is what the log line and PM_PLAN print as why you are running, and the selftest asserts across every fixture that no plan ever carries another value — a second value would mean a second trigger had come back.
• What a mention still batches: two unanswered mentions in one thread are one reply naming both keys, because the at-most-once-reply model is per reply, not per mention. Comments sitting between them are neither answered nor keyed.
• The PM persona now tells the PM to answer as if its reply is the last word it will see, and to ask the human to tag it again if it needs something back — under the old rule it could end a reply with a question and expect the answer to reach it. It will not.

Kept untouched, as the brief required: the in-reply idempotency marker and the at-least-once-decision / at-most-once-reply failure model; the self-authored filter as the first check; the hostile-payload fencing and the untrusted banner; PM_PR_MENTIONS: "false", PM_PLAN, PM_DRY_RUN implying plan mode and the two-step rollout; the /notifications query with its fallback and the read:notification scope note; PM_PR_DIFF_MAX_CHARS and its default. Context assembly is unchanged — "explicit mention required" governs what triggers a run and what gets replied to, not how much the model sees, so the payload still carries the PR, the changed files, the diff and the whole comment list. The only payload edit is a line telling the PM that comments which do not mention it are context, not questions put to it.

The dispatcher was not structured the way the brief guessed, and that is worth recording. The brief expected the poll loop under ci/** and/or .gitea/**. It is not there: it is infra/agent/pm-dispatch.py, a long-running Python program in the pm-agent Deployment (platform/pm-agent/). ci/** and .gitea/** are the merge gate and have nothing to do with it. So this PR touches no approval-policy sensitive path — I checked every changed path against infra/gitea/approval-policy.json by hand (see "What I verified") — and the ci/approval-policy.sh stage should pass on a green gate without a human review. The PM's own warning on the issue that the PR would sit red was an honest guess from an unverified belief about where the code lived; it was wrong, and nothing about the change needed to move to make it so.

There was a poll loop to extend, so no architecture was improvised.

infra/agent/pm-dispatch.py:

• A second, independently-guarded pass per poll — cycle_pr_mentions(), called from run() rather than from cycle(). Two reasons: the issue cycle returns early at the worker cap and a mention creates no worker, so it must not be suppressed by a cap it cannot contribute to; and a Gitea failure on one path must not silence the other.
• Discovery (discover_pr_candidates). Tries Gitea's notification list for the bot account — one bounded call, which is the "way to check for '@'s for the bot account instead of scouring the entire pull request list" the operator asked for — and falls back to listing open pull requests. Mode is a knob (auto / notifications / pulls), and which query answered is logged every cycle. A 401/403 is handled as its own case (GiteaForbidden) because on this service it is almost certainly a token scope, not a bug: /api/v1/notifications sits behind Gitea's notification scope category and the bot's token is minted read:user, write:repository, write:issue, read:package. The log names read:notification as the remedy instead of reporting "no mentions found".
• Detection (normalize_pr_entries, plan_pr_replies) — pure functions over normalized comments, so the parts that must be provable offline are provable offline. Reads all three Gitea resources: conversation comments (/issues/{n}/comments), review bodies (/pulls/{n}/reviews) and inline review comments (.../reviews/{id}/comments).
• Context assembly (render_pr_thread) — PR title and body, the changed-file summary, a bounded diff, the full thread being answered with the anchor comment marked, and the surrounding comments. Every field that came from a person goes through a single new fence() helper under an explicit untrusted banner; render_thread was refactored onto the same helper, which removes a twice-retyped .replace("```", "`` ")`.
• A narrowed decision shape. PR_ACTIONS = ("comment", "decline") and PR_DECISION_SCHEMA with no branch and no brief property at all — the PM has no field in which to ask for work. handle_pull() refuses anything outside that set (and still marks it answered, so an unusable decision costs one decision rather than repeating forever), and nothing on this path calls build_job().
• Replying in the thread. Conversation threads get an issue comment. Inline threads get a one-comment review (POST /pulls/{n}/reviews, event: COMMENT — never APPROVE), which is the only way Gitea creates an inline comment; if Gitea refuses it the reply falls back to the conversation naming the file and position, because a mention answered in the wrong place is better than one answered nowhere.
• PM_PLAN — discover, assemble, print the exact payload and the exact thread it would post into; ask no model, post nothing. Setting it also enables the path, so it is one knob to look before leaping. PM_DRY_RUN implies it (see "the decision I made" below).

infra/agent/testdata/pr-mention-fixtures.json (new) — 18 fixture cases driving the trigger logic from PM_SELFTEST=1 pm-dispatch, covering the six the brief required plus twelve edges. The three cases that asserted continuation are inverted rather than deleted, so the old behaviour cannot creep back unnoticed, and three were added for the revised rule. infra/agent/Containerfile COPYs them into the verify stage only (test data has no business in the shipped image) and the selftest fails rather than skips if it cannot find them. infra/agent/pm-persona.md gains a "When you are reading a PULL REQUEST" section, and the selftest asserts it is there — without it the PM would be answering a mention while still believing it may pick implement.

platform/pm-agent/configmap.yaml adds the knobs with PM_PR_MENTIONS: "false"; deployment.yaml bumps config-generation 5 → 6 in the same commit, as that file requires. platform/pm-agent/README.md and runbooks/pm-agent.md document the path, the token scope, the plan mode and six new log lines.

The decisions made explicitly

The trigger. Settled by the operator rather than by me: an explicit mention, and nothing else. See the revision section above. The earlier back-and-forth about what "the same thread" means for a flat comment list is moot — the question only existed because continuation needed a notion of sameness, and there is no continuation.

Idempotency ordering. The marker lives inside the reply the PM posts (<!-- pm-agent:replied-to=comment-9001,review-comment-77 -->), which makes it one atomic write with the reply and therefore impossible to get out of step with it. The failure mode chosen, deliberately, is at-least-once model call, at-most-once reply: a crash before posting costs one repeated decision; a crash after posting is a no-op because the marker is already in the thread; and a POST that errors while Gitea actually created the comment is also a no-op, because nothing is retried inside the cycle and the next cycle re-reads the list. A mention is therefore never silently dropped and a reply is never duplicated. The rejected alternative — write a marker before asking the model — buys cheapness by making a crash swallow the mention with nothing in the thread saying so. PM_PR_COOLDOWN_SECONDS (900s) bounds the cost of the repeat, which is why paying it is the cheap side of the trade.

Loop prevention. With no author gate (operator answer 3: "anyone can talk and have the agent work"), any passer-by can start a run, so a bug that let the bot's replies count as triggers is an unbounded loop anyone could light. The self-authored check is the first thing plan_pr_replies does with each comment and it is structural — a bot-authored comment can only ever clear work, never create it, so a reply of ours containing our own handle is inert. The posted reply additionally never contains @-the-bot. Fixture (c) is that case and the selftest asserts the second guard too. Dropping continuation narrows this surface further: a reply is now reachable only from a comment that spells the handle out, and the bot's own output never writes it, so the guard is doubled rather than relied upon.

Why it ships off. Not one call on this path has been made against the live Gitea, and the endpoint it most wants is expected to be refused on today's token. CLAUDE.md §2b is explicit that this repository is not evidence about anything live, and the service's own rollout precedent is a staged one (PM_DRY_RUN=true for a day, PM_REQUIRE_LABEL=agent). Shipping it on would be asserting it works. PM_DRY_RUN was also made to imply PM_PLAN, because the runbook sells PM_DRY_RUN as "stop it taking new work" and a path that kept posting replies under it would be a footgun — and on this path the reply is the action under test, and it also carries the idempotency marker, so posting it would make a dry run not dry.

No requirement ID is claimed. The brief is right that nothing in the R-* set covers agent tooling, and inventing one would be worse than the gap.

What I verified

The offline selftest, run in this container, exit 0. 95 assertions, all passing:

PM_SELFTEST=1 PM_WORKER_JOB_TEMPLATE=infra/agent/worker-job-template.yaml \
  PM_PERSONA_FILE=infra/agent/pm-persona.md python3 infra/agent/pm-dispatch.py
-> selftest OK, rc=0

PM_PERSONA_FILE has to be named because this worker container's own image is older than this branch, so the baked /etc/pm-agent/pm-persona.md is the pre-#116 copy and the "persona states" assertion fails against it. In the shipped image the Containerfile COPYs the branch's persona to that path and the override is unnecessary.

All 18 fixture cases pass, including the six the brief named: (a) a mention in a PR conversation comment is detected; (b) a mention in an inline review comment is detected as its own thread; (c) a bot-authored comment is ignored including when it contains the bot's own handle; (d) an already-marked comment id is skipped; (e) a conversation follow-up with no mention is not actionable even after the bot replied; (f) an unrelated comment on an unrelated PR triggers nothing.

The cases that pin the revised rule:

• (e), (m), (l) — inverted, not deleted. (e) a conversation follow-up after a reply, (m) an inline follow-up in a thread the bot has already replied in, and (l) an unmentioned inline comment on a pull request whose conversation was answered: all three now expect no reply at all. (m) is the deliberate one — an inline thread is genuinely threaded in Gitea and still gets no continuation.
• (i) — the burst shape the operator described. Three comments, only the first mentioning the bot: exactly one reply, keyed to the first comment only. The other two are carried into the payload as context and are not answered.
• (q) — the consequence that is now closed. A once-mentioned-and-answered pull request then gets comments from renovate and gitea-actions: no reply.
• (p) two unanswered mentions in one thread are one reply naming both keys; (r) a fresh mention after an answered one gets its own reply, keyed only to itself, so a mention is repeatable; (o) a comment made before the mention is still not pulled in.
• Three assertions over every fixture at once, so a single case drifting cannot hide the regression: every plan's reason is mention, every comment answered matches the mention pattern, and no bot-authored comment is ever answered.

Also still passing, unchanged: @pm-agent-worker and [email protected] (and @pm-agentx, mail:foo@pm-agent, @pm-agent.worker) are not mentions — which matters more now that the mention is the only trigger; a marker pasted by a non-bot author does not suppress a mention; a mention in a review body is a conversation trigger; two inline threads on one file stay separate with the newest-anchored one taken first; a position-less inline comment still lands in a stable thread.

Also asserted: the reply carries a marker for every key it answers and contains no @pm-agent; the refusal comment is marked too; PR_ACTIONS is exactly comment/decline and a subset of ACTIONS; PR_DECISION_SCHEMA has no branch/brief and additionalProperties: false; fence() breaks an embedded triple backtick rather than dropping it; the assembled payload on a hostile PR (a body and a diff that each try to close the fence and issue orders) has balanced fences, carries the banner twice, and quotes the hostile text verbatim inside a fence; truncation keeps both the head and the tail.

The dispatcher plumbing the selftest does not reach, re-exercised against a stub Gitea after the revision (a throwaway script under $TMPDIR, not committed; 26 assertions, all passing). Measured:

• discovery resolves a notification subject URL to index 120 and reports source notifications; a GiteaForbidden from /notifications falls back to open-pulls; in notifications-only mode it returns ([], 'notifications-unavailable') rather than silently widening into the scan the operator rejected;
• plan mode printed the payload and posted nothing for four fixtures;
• case (a) posted into the conversation carrying marker comment-9001, and case (b) posted inline on infra/agent/pm-dispatch.py position 42 carrying marker review-comment-7701 — so the reply still goes back where the mention was made;
• case (i) (burst, only the first mentions) produced exactly one reply whose marker names only comment-9070, and its payload still carried comments 9071 and 9072 as context;
• cases (e), (m), (l) and (q) produced no reply and no model call — the stub counted zero calls into ask_pm_pr across all four, which is the "no reply and no model call" half of the brief's done-criterion measured rather than asserted;
• case (p) produced one reply whose marker names both comment-9100 and comment-9102;
• an out-of-vocabulary action (implement) was refused with a marked explanatory comment and no Job;
• the rendered payload says why you are running: **mention** and carries the line telling the PM that unmentioned comments are context, not questions put to it.

The merge gate, in this container, re-run after the revision:

./ci/gate.sh   ->   exit 3 (UNDETERMINED)
Stages that did not run: spec-immutability gitea-settings
diff base: origin/main @ 5b484f6b7884 ... HEAD @ 2fdb95393cc0

Nothing failed. Those are the two skips CLAUDE.md §3 names as expected here — the sibling spec repo is not reachable from the cluster and SETTINGS_API_TOKEN exists only in CI — and they are the only two. decisions-log, lint, l0, render-golden, l1 and approval-policy all reported green.

That this PR touches no sensitive path, checked path by path against the sensitive_paths globs in infra/gitea/approval-policy.json: the changed files are under infra/agent/, platform/pm-agent/, runbooks/ and decisions/. infra/agent/** is not in the list (infra/gitea/** is, and is a different directory), platform/pm-agent/** is not (platform/kyverno/** is), no rbac.yaml and no *.sops.yaml is touched, and platform-config/kustomization.yaml and CLAUDE.md are untouched. The gate's own approval-policy stage is the authority on this and it only evaluates on a pull_request event, so read its verdict on the PR rather than trusting this paragraph.

What I did NOT verify

• Nothing here has run against the live Gitea, and nothing here is deployed. I held no Gitea credential, no kubeconfig and no ServiceAccount token: this container has egress to the in-cluster Gitea and the Anthropic API only, and kubectl fails. Every claim below is therefore an operator measurement, not mine.

• Every one of the five new Gitea API calls is UNEXERCISED. /notifications, /pulls/{n}, /pulls/{n}/files, /pulls/{n}.diff, /pulls/{n}/reviews, .../reviews/{id}/comments and POST /pulls/{n}/reviews were written to Gitea's published swagger for the 1.24 line. The fixtures prove this program parses those shapes; they prove nothing about whether this Gitea emits them. The exact read-only curl commands to settle it are in platform/pm-agent/README.md, and replacing the fixtures with a real capture is the cheapest way to close the gap.

• That /api/v1/notifications needs read:notification is read from Gitea's documented scope model, not measured. It is the reason the fallback and the named-remedy log line exist; if the endpoint turns out to work on today's token, the fallback simply never fires.

• Whether a notification is actually raised for a mention in an inline review comment, and whether subject.url is populated for it. If it is not, discovery via notifications will miss inline mentions and only the pulls fallback will find them. Untested; it is the single most likely gap in the cheap query.

• Whether POST /pulls/{n}/reviews with one comment lands in the existing inline thread or opens a second conversation at the same line. Gitea groups by (path, position) in its UI, which is why this should work, but I have not seen it. The fallback covers outright refusal, not mis-placement.

• Whether position from a stored review comment is still a valid new_position on the current head. It will not be after a force-push, which is exactly when the inline fallback fires.

• The image. Nothing was built. The Containerfile verify stage change (the fixture COPY) has not been run by a container engine — there is none in this container — so the only evidence the selftest passes under the fixture path resolution is the repo-checkout run above, which takes branch 2 of pr_fixture_path(). CI's images workflow is what exercises branch 3. Related and worth knowing when reading a failure: the selftest's "persona states" assertion fails in this container against the baked /etc/pm-agent/pm-persona.md, because the image predates this branch. That is an artefact of where the run happened, not a defect — in the built image the two are the same file — but it does mean the image's own selftest invocation (PM_SELFTEST=1 pm-dispatch, with no PM_PERSONA_FILE) has not been observed passing with this branch's persona.

• The two open items from the first version of this branch are closed, and that part is verified offline rather than unverified. They were "continuation has no end condition" and "another bot commenting on a once-mentioned PR would get a reply". Both are gone by construction: there is no continuation left to terminate, and fixture (q) measures that renovate and gitea-actions commenting on an answered pull request draw nothing. No ignore-list for automated authors was needed or added.

  The residual case, stated precisely. A reply is still reachable by anyone who writes the literal handle, including an automation, because the operator's answer (3) is "anyone can talk and have the agent work" and there is no author gate. Two concrete shapes: (1) a bot whose comment template happens to contain @pm-agent — e.g. somebody quoting a mention into a Renovate PR description that Renovate then echoes — gets one reply per such comment; (2) a quoted mention counts as a mention. Gitea gives this program no way to tell > @pm-agent what do you think? pasted into a Markdown quote from a fresh request, so re-quoting an earlier mention in a new comment costs one more decision and one more reply. Both are bounded — one reply per comment id, one decision per pull request per 15 minutes — but neither is prevented. Whether to ignore quoted lines, or particular authors, is a policy call for the operator and not one I should take.

• Spend. PM_PR_DIFF_MAX_CHARS defaults to 20000, so a mention costs a decision over a payload meaningfully larger than an issue triage's. No measurement of the real cost exists. Dropping continuation should reduce the number of runs substantially — the trigger rate is now how often somebody types the handle rather than how busy a once-mentioned pull request is — but that is reasoning, not a measurement.

• R-TEST-06 (spec immutability) and L2/L3 conformance were not verified by this run — the two gate skips above. And the approval-policy stage reported N/A because there was no pull_request event; its verdict on the PR is the authoritative one.