PM-Agent also need to look at PR comments #116
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: Home-Network/home-network-k3s-impl#116
Reference in new issue
Block a user
Currently, the PM-Agent only looks at issues that are labeled. However, there are times where a PR needs the PM-Agent to look and action things within a PR. If the PM-Agent is '@'ed then it should receive the notification, read the PR, the changes, the comments, and the context of what is being asked to the agent. Replies should remain in the thread it was '@'ed with any changes coming as a separate PR if not already owned by the pm-agent.
Holding this one for your input rather than starting a branch: the ask is to change the machinery that decides when the PM agent runs, and I don't think I should guess at that design. Two things make me cautious. First, extending the trigger from "labelled issue" to "mentioned on a PR" widens who can cause an agent run — today it's whoever can apply the
agentlabel, afterwards it's anyone who can comment — so the authorization model needs stating, and the role/trigger boundary is described in the spec repo'sagents/roles.md/AGENTS.md, which I can't amend. If the architect is happy for that to be an implementation detail with no spec change, say so on this issue and I'll treat it as in scope; if not, it wants a requirement or ADR first. Second, the work would land inci/**and/or.gitea/**, which are approval-policy sensitive paths, so any PR will sit red on the approval stage until you review it — and since the change is the PM wiring up its own invocation path, that human review is the point rather than an inconvenience.To brief an engineer properly I need answers to these:
maininstead.Also note this thread contains two separable concerns (the trigger plumbing, and the reply/change-authoring behaviour once triggered); I'd rather they be two issues so each gets its own small PR.
Action:
comment— no work started.This change is expected to touch an approval-policy sensitive path, so the merge gate will require an approving review from a human who is not the author (
ci/approval-policy.sh). A green gate alone will not be enough.Posted by the PM agent. It opens pull requests and never merges them; every change reaches
mainonly through the merge gate.The label is just a distinguished on what issues should be assigned to the agent vs not. It is not a permissioning things at all (at least not intended). Scoping larger to include comments in the PR is therefore not widening the permissions as use of the pm-agent was always open to the entire project.
Thanks — that unblocks it. Your answer to (3) is the one that mattered: if the
agentlabel was never a permissions boundary, then triggering on a mention isn't widening authorization, and I'll treat the trigger mechanism as an implementation detail needing no spec change. Starting onfeat/pm-agent-pr-mentions; a PR will follow.I'm scoping that branch to the trigger-and-reply half only: find mentions of the bot account (conversation comments and inline review threads), assemble the PR context, dispatch a run, post the reply in-thread, with self-authored comments ignored, a per-comment-ID idempotency marker, and thread continuation after the first mention. The change-authoring half — your point 6, revise an agent-owned branch vs. open a fresh PR against
main— I'm deliberately leaving out and would like as a separate issue, so each lands as a small reviewable PR. Say the word if you'd rather have both in one and I'll fold it in.Two things to flag. This will land in
ci/**and/or.gitea/**, so the PR will sit red on the approval stage until you review it; given the change is the PM wiring its own invocation path, that review is the point. And I can't find anyR-*requirement this traces to — it's agent tooling rather than platform behaviour — so if you want it covered by a requirement, that's an architect call, not something I can add.One open question I've told the engineer to stop and ask about rather than guess: "the same thread" has no native meaning for PR conversation comments, which are a flat list, so continuation there needs a policy decision (likely: once the bot has replied on a PR, subsequent non-bot comments continue). Expect a question back on this issue if there's no existing convention in the repo.
Action:
implementon branchfeat/pm-agent-pr-mentions— workerpm-agent-task-116-1794027368started.This change is expected to touch an approval-policy sensitive path, so the merge gate will require an approving review from a human who is not the author (
ci/approval-policy.sh). A green gate alone will not be enough.Posted by the PM agent. It opens pull requests and never merges them; every change reaches
mainonly through the merge gate.Pull request #117 is open from
feat/pm-agent-pr-mentionsfor this issue.PR-mention trigger for the PM agent — built, tested offline, shipped off
The trigger-and-reply half of #116 is done: a mention of
@pm-agenton a pull request (inthe conversation or in an inline review thread) makes the PM read the PR, its changed
files, its diff and the thread, and reply in that same thread. Once a thread holds a
mention, follow-ups in it continue the conversation with no new mention. It can only
reply — the change-authoring half (your point 6) is not in here and wants its own issue.
One correction to the PM's brief, in your favour
The brief expected the dispatcher under
ci/**or.gitea/**. It isn't there — it'sinfra/agent/pm-dispatch.py, the program in the pm-agent Deployment. Those two directoriesare the merge gate and are unrelated. So this PR touches no approval-policy sensitive
path and should merge on a green gate with no human review needed. I checked every changed
path against
infra/gitea/approval-policy.jsonby hand; the gate'sapproval-policystageis the authority, so read its verdict on the PR rather than my word. Changed paths are
infra/agent/,platform/pm-agent/,runbooks/anddecisions/.There was a poll loop to extend, so nothing was improvised.
The two judgement calls you asked to be made explicitly
"The same thread" — I did not need to stop and ask. The brief said to ask only if the
repo had no existing convention. It has one: the issue path already treats a flat comment
list as one conversation ("act iff the newest thing in this thread was not written by me").
I applied that reading, but refined the arming rule to your words rather than the PM's
paraphrase. The PM guessed "once the bot has replied, follow-ups continue"; you wrote "a
thread that starts with a mention should continue". Those differ: under the PM's version,
someone who types three comments and tags the bot only in the first would have the last two
silently ignored. So continuation is armed by the mention, not by the reply, per thread.
It's still a policy, not a Gitea fact — tell me if you want it the other way, or want an
end condition; there currently is none, which is the open question below.
Idempotency. The marker lives inside the reply itself
(
<!-- pm-agent:replied-to=comment-9001 -->), so it is one atomic write with the reply andcannot get out of step with it. The failure mode I chose 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. A mention is never silently dropped and a reply is never duplicated. The
alternative (mark first, ask second) is cheaper but lets a crash swallow a mention with
nothing in the thread saying so.
Loop prevention. The self-authored filter is the first check on every comment, and it's
structural — a bot comment can only ever clear work, never create it, and it cannot even
arm continuation, so a reply of ours containing our own handle is inert. The reply also never
writes
@pm-agent. Both are tested.What I verified
PM_SELFTEST=1 pm-dispatch→ exit 0, 87 assertions (55 new), including all 15 newfixture cases: your six required ones plus near-miss handles (
@pm-agent-worker,[email protected]), a marker pasted by a non-bot author not suppressing a mention, aburst of three comments becoming one reply, mentions in a review body, two inline threads
on one file staying separate, per-thread continuation, and a comment made before the
mention not being pulled in.
and issue orders — the assembled payload keeps every fence balanced, carries the
untrusted-input banner, and quotes the hostile text verbatim inside a fence. Nothing
outside a fence in that payload comes from a user.
conversation mention posted into the conversation with the right marker; an inline mention
posted inline on the right file and position; an out-of-vocabulary action was refused with
a marked explanation and no Job; a 403 on
/notificationsfell back to listing open PRs../ci/gate.sh→ exit 3 (UNDETERMINED), nothing failed. Stages that did not run:spec-immutabilityandgitea-settingsonly — the two that cannot run in this container(no sibling spec repo reachable, no
SETTINGS_API_TOKEN).lint,l0,render-golden,l1,decisions-logall green.What I did NOT verify — and need you to measure
I held no Gitea credential, no kubeconfig and no cluster token, so nothing here has run
against the live Gitea and nothing is deployed.
/api/v1/notificationsbehind itsnotificationscope; the pm-agent token is mintedread:user, write:repository, write:issue, read:package. I read that from Gitea'sdocumented scope model, not from the live server. The code handles it: it logs the
refusal, names
read:notificationas the remedy, and falls back to listing open PRs(which only needs
read:repository), so the feature works either way — just not cheaply.If you want the cheap query, re-mint the token with
read:notificationadded andupdate both Secrets. Steps are in
runbooks/pm-agent.md.read-only curl commands to settle them in a minute are in
platform/pm-agent/README.md→ "Mentions on a pull request". Replacing the fixtures with areal capture is the cheapest way to close the gap.
inline review comment, with a usable
subject.url? If not, the cheap query will missinline mentions and only the open-PR fallback finds them.
POST /pulls/{n}/reviewsland in the existing inline thread, or open a secondone at the same line? Gitea groups by (path, position), which is why it should — but I
haven't seen it. There's a fallback for outright refusal, not for mis-placement.
the fixtures into the
verifystage hasn't been run. CI'simagesworkflow exercises it.It ships OFF — the rollout I'd suggest
PM_PR_MENTIONS: "false"in the ConfigMap. Given point 1 above and §2b, shipping it on wouldbe asserting it works. Two ConfigMap edits (each with a
config-generationbump), in order:PM_PLAN: "true"— discovers, assembles and prints the exact payload and the exactthread it would post into, asks no model, posts nothing. Mention the bot on a live PR,
read
kubectl -n pm-agent logs deploy/pm-agent, and you'll see exactly what it would havedone. (
PM_DRY_RUNnow implies this, so your "stop it taking new work" lever silences thispath too — on this path the reply is the action under test.)
PM_PR_MENTIONS: "true"once that looks right.Open questions for you
comment in it triggers a run (bounded to one reply per comment id, one decision per PR per
15 min). I didn't invent a termination rule. Named consequence: another bot commenting on a
PR where the agent was once mentioned would get a reply. There is no ignore-list for
automated authors — adding one is your call, not mine.
PM_PR_DIFF_MAX_CHARSdefaults to 20000, so a mention costs a decision over anoticeably bigger payload than an issue triage. Set it to
0to send only the changed-filelist. No real measurement exists.
agent-owned branch vs. open a fresh PR against
main) is deliberately out of this branchand wants its own issue.
No
R-*requirement is claimed — the brief is right that nothing in the set covers agenttooling, and inventing one would be worse than the gap.
Full detail, including every unverified item:
decisions/2026-11-07-pm-agent-pr-mentions.md.3· did not run:spec-immutability gitea-settingscheck on #117, not this line.
Posted by the PM agent's worker.
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
reviseonfeat/pm-agent-pr-mentions— the newest comment withdraws the continuation requirement, and that is a behaviour change to the branch rather than new work.Taking your 05:32 comment together with your earlier "if comments aren't hierarchical, disregard": the agent will act only on comments that explicitly mention the bot account, uniformly across PR conversation comments and inline review comments. The "armed by the first mention, then every later non-bot comment in the thread" rule comes out; one explicit mention means one reply in the place it was made, and nothing else in the PR is picked up. That also closes the two things the worker flagged as unresolved — there is no longer an open-ended continuation to terminate, and no path by which another automation commenting on a once-mentioned PR draws a reply.
Not being re-done: the in-reply idempotency marker, the self-authored filter, the hostile-payload fencing, the
read:notificationfallback, and shipping off behindPM_PR_MENTIONS/PM_PLAN. The change-authoring half — your 05:16 comment on when to base a new PR on the original PR, onmain, or to file a fresh labelled issue instead — is still deliberately out of this branch and wants its own issue; I'd like you to open it or tell me to.On paths: the worker's correction stands as its own claim, not mine — the dispatcher is under
infra/agent/, notci/**or.gitea/**, so no approval-policy sensitive path is expected and PR #117 should merge on a green gate. The gate'sapproval-policystage is the authority on that, so read its verdict on the PR. I've told the engineer to stop and say so on this issue if the revision pulls in a sensitive path. Nothing here has been exercised against live Gitea or deployed; the five unverified items in the worker's last comment remain unverified.Action:
reviseon branchfeat/pm-agent-pr-mentions— workerpm-agent-task-116-1794030556started.Posted by the PM agent. It opens pull requests and never merges them; every change reaches
mainonly through the merge gate.Pull request #117 is open from
feat/pm-agent-pr-mentionsfor this issue.The worker left no summary of its own; the pull request body and the
decisions/entry on the branch are the record.
3· did not run:spec-immutability gitea-settingscheck on #117, not this line.
Posted by the PM agent's worker.