Home-Network/home-network-k3s-impl
2
0 Fork 0
Code Issues 2 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

feat: PM-Agent also need to look at PR comments #117

Merged
jakman85 merged 2 commits from feat/pm-agent-pr-mentions into main 2026-11-07 12:07:34 +00:00
Conversation 0 Commits 2 Files Changed 9
+2337 -32
pm-agent commented 2026-11-07 05:20:43 +00:00
Member
Copy link
Copy source

Closes #116.

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-agent on a pull request (in
the 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's
infra/agent/pm-dispatch.py, the program in the pm-agent Deployment. Those two directories
are 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.json by hand; the gate's approval-policy stage
is the authority, so read its verdict on the PR rather than my word. Changed paths are
infra/agent/, platform/pm-agent/, runbooks/ and decisions/.

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 and
cannot 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 new
    fixture 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, a
    burst 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.
  • A hostile payload test: a PR body and a diff that each try to close the code fence
    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.
  • The plumbing, against a stub Gitea: plan mode printed and posted nothing; a
    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 /notifications fell back to listing open PRs.
  • ./ci/gate.sh → exit 3 (UNDETERMINED), nothing failed. Stages that did not run:
    spec-immutability and gitea-settings only — the two that cannot run in this container
    (no sibling spec repo reachable, no SETTINGS_API_TOKEN). lint, l0, render-golden,
    l1, decisions-log all 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.

  1. The cheap mention query needs a token scope your bot doesn't have. Gitea puts
    /api/v1/notifications behind its notification scope; the pm-agent token is minted
    read:user, write:repository, write:issue, read:package. I read that from Gitea's
    documented scope model, not from the live server. The code handles it: it logs the
    refusal, names read:notification as 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:notification added and
    update both Secrets. Steps are in runbooks/pm-agent.md.
  2. All five new API calls are unexercised, written to Gitea's published 1.24 swagger. The
    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 a
    real capture is the cheapest way to close the gap.
  3. The likeliest actual gap: does Gitea raise a notification for a mention inside an
    inline review comment, with a usable subject.url?     If not, the cheap query will miss
    inline mentions and only the open-PR fallback finds them.
  4. Does POST /pulls/{n}/reviews land in the existing inline thread, or open a second
    one 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.
  5. No image was built (no container engine here), so the Containerfile change that copies
    the fixtures into the verify stage hasn't been run. CI's images workflow 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 would
be asserting it works. Two ConfigMap edits (each with a config-generation bump), in order:

  1. PM_PLAN: "true" — discovers, assembles and prints the exact payload and the exact
    thread 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 have
    done. (PM_DRY_RUN now implies this, so your "stop it taking new work" lever silences this
    path too — on this path the reply is the action under test.)
  2. PM_PR_MENTIONS: "true" once that looks right.

Open questions for you

  • No end condition on continuation. Once a thread holds a mention, every later non-bot
    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.
  • Spend. PM_PR_DIFF_MAX_CHARS defaults to 20000, so a mention costs a decision over a
    noticeably bigger payload than an issue triage. Set it to 0 to send only the changed-file
    list. No real measurement exists.
  • The second issue. As the PM said on the thread, the change-authoring half (revise an
    agent-owned branch vs. open a fresh PR against main) is deliberately out of this branch
    and wants its own issue.

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

Full detail, including every unverified item: decisions/2026-11-07-pm-agent-pr-mentions.md.

Gate run inside the worker (the pinned toolbox, on this exact commit, before the
branch was pushed): exit 3, stages that did not run: spec-immutability gitea-settings.
That is the worker's own run, not this pull request's CI run — read the check on this
pull request for that. A green tick is not proof anything ran; read what executed.

Opened by the PM agent from issue #116. It does not merge its own pull requests.

🤖 Generated with Claude Code

Closes #116. ## 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-agent` on a pull request (in the 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's `infra/agent/pm-dispatch.py`, the program in the pm-agent Deployment. Those two directories are 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.json` by hand; the gate's `approval-policy` stage is the authority, so read its verdict on the PR rather than my word. Changed paths are `infra/agent/`, `platform/pm-agent/`, `runbooks/` and `decisions/`. 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 and cannot 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 new fixture 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, a burst 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. - **A hostile payload test**: a PR body *and* a diff that each try to close the code fence 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. - **The plumbing, against a stub Gitea**: plan mode printed and posted nothing; a 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 `/notifications` fell back to listing open PRs. - **`./ci/gate.sh` → exit 3 (UNDETERMINED)**, nothing failed. Stages that did not run: `spec-immutability` and `gitea-settings` only — the two that cannot run in this container (no sibling spec repo reachable, no `SETTINGS_API_TOKEN`). `lint`, `l0`, `render-golden`, `l1`, `decisions-log` all 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.** 1. **The cheap mention query needs a token scope your bot doesn't have.** Gitea puts `/api/v1/notifications` behind its `notification` scope; the pm-agent token is minted `read:user, write:repository, write:issue, read:package`. I read that from Gitea's documented scope model, **not** from the live server. The code handles it: it logs the refusal, names `read:notification` as 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:notification` added** and update both Secrets. Steps are in `runbooks/pm-agent.md`. 2. **All five new API calls are unexercised**, written to Gitea's published 1.24 swagger. The 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 a real capture is the cheapest way to close the gap. 3. **The likeliest actual gap: does Gitea raise a notification for a mention inside an *inline review* comment, with a usable `subject.url`?** If not, the cheap query will miss inline mentions and only the open-PR fallback finds them. 4. **Does `POST /pulls/{n}/reviews` land in the *existing* inline thread**, or open a second one 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. 5. **No image was built** (no container engine here), so the Containerfile change that copies the fixtures into the `verify` stage hasn't been run. CI's `images` workflow 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 would be asserting it works. Two ConfigMap edits (each with a `config-generation` bump), in order: 1. `PM_PLAN: "true"` — discovers, assembles and **prints** the exact payload and the exact thread 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 have done. (`PM_DRY_RUN` now implies this, so your "stop it taking new work" lever silences this path too — on this path the reply *is* the action under test.) 2. `PM_PR_MENTIONS: "true"` once that looks right. ### Open questions for you - **No end condition on continuation.** Once a thread holds a mention, every later non-bot 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. - **Spend.** `PM_PR_DIFF_MAX_CHARS` defaults to 20000, so a mention costs a decision over a noticeably bigger payload than an issue triage. Set it to `0` to send only the changed-file list. No real measurement exists. - **The second issue.** As the PM said on the thread, the change-authoring half (revise an agent-owned branch vs. open a fresh PR against `main`) is deliberately out of this branch and wants its own issue. No `R-*` requirement is claimed — the brief is right that nothing in the set covers agent tooling, and inventing one would be worse than the gap. Full detail, including every unverified item: `decisions/2026-11-07-pm-agent-pr-mentions.md`. --- **Gate run inside the worker** (the pinned toolbox, on this exact commit, before the branch was pushed): exit `3`, stages that did not run: `spec-immutability gitea-settings`. That is the worker's own run, not this pull request's CI run — read the check on this pull request for that. A green tick is not proof anything ran; read what executed. Opened by the PM agent from issue #116. It does not merge its own pull requests. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
pm-agent added 1 commit 2026-11-07 05:20:43 +00:00
feat: PM-Agent also need to look at PR comments
gate / gate (pull_request) Successful in 2m55s
Details
images / images (pull_request) Successful in 1m10s
Details
gate / conformance (pull_request) Has been skipped
Details
2fdb95393c 
From Gitea issue #116.

Authored by the PM agent's worker; the merge gate is the approver. Refs #116.

Agent: pm-agent (issue #116, mode implement)
pm-agent added 1 commit 2026-11-07 06:05:20 +00:00
feat: PM-Agent also need to look at PR comments
gate / gate (pull_request) Successful in 2m58s
Details
images / images (pull_request) Successful in 1m6s
Details
gate / conformance (pull_request) Has been skipped
Details
e8e9de8611 
From Gitea issue #116.

Authored by the PM agent's worker; the merge gate is the approver. Refs #116.

Agent: pm-agent (issue #116, mode revise)
jakman85 merged commit d7317606e7 into main 2026-11-07 12:07:34 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
agent
automated-update
blast-radius
breaking
ci-tooling
dependencies
needs-review
own-image
range-widen
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
gitea-admin jakman85 (Keith Jackson) pm-agent renovate-bot
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Home-Network/home-network-k3s-impl#117