fix: let the Product Owner fire Stage 2, so its decisions carry its own face - #645
Conversation
…wn face `backlog`'s Autonomous spend rule authorises the PO agent to fire Stage 2 on an item entering Analysis that is labelled `bug`, opened by an external reporter, carries a debug log, and has no prior analyze comment. `issue-analyze.yml` gated on `comment.user.login == github.repository_owner` and nothing else. So the rule and the gate disagreed, and the only way to satisfy both was to post the trigger as the maintainer. That put the maintainers name on comments they never wrote -- #624, #627 and #643 all show `@claude-bot analyze` from `johanzander` -- and erased the distinction between what the maintainer decided and what the agent decided. Every other PO write (labels, board fields, reporter replies) already goes out as `bess-product-owner`; this was the one hole, and it was the one that mattered most, because it is the write that SPENDS. The gate now accepts `bess-product-owner` alongside the owner, and the skill tells the PO to use `gh-agent.sh --as po` rather than plain `gh`. STAGES 1, 3 AND 5 STAY OWNER-ONLY and the comment says they must not copy this. Their spend ($1-4 and $2-10) is authorised by no autonomous rule, so nothing would be enforcing a bar on the far side of the gate. The exception here is safe precisely because the rule that bounds it already exists. Naming an account in a gate is only safe when it is REGISTERED. `bess-product-owner` exists today and is a collaborator. This is the same reason CLAUDE.md refuses to pre-authorise `bess-developer` before the rename: on a public repo an unregistered username can be claimed by anyone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012LExo6fcbup75vtc9NfoAR
|
@claude-bot review |
There was a problem hiding this comment.
Summary
Diff verified with git diff main...<PR head> (merge-base three-dot diff, matching gh pr diff): exactly the 3 files the PR body claims — .github/workflows/issue-analyze.yml, CLAUDE.md, .claude/skills/backlog/SKILL.md. No scope creep.
Root cause match: Confirmed. issue-analyze.yml's actor gate (github.event.comment.user.login == github.repository_owner) only ever accepted the maintainer, while .claude/skills/backlog/SKILL.md's pre-existing "Autonomous spend" section (already on main before this PR, lines ~492–508) already authorized the PO agent to fire Stage 2 under specific bounds. That rule/gate mismatch is real and is exactly what this PR closes, by adding github.event.comment.user.login == 'bess-product-owner' as an OR clause (.github/workflows/issue-analyze.yml).
Test coverage: No new tests, and the PR is explicit about why ("Outcome-level coverage: None — this is a workflow actor gate... First PO-fired analyze after merge is the proof"). That's an honest, correct call — a GitHub Actions if: actor gate can't be exercised by the local test suite, and the PR doesn't pretend otherwise. ./scripts/quality-check.sh and YAML validity are the only checks applicable here; I independently re-validated the PR's issue-analyze.yml with yaml.safe_load — parses cleanly, and the multi-line if: expression follows the same style as the pre-existing 3-line version and the already-merged bess-agent exception in pr-review.yml.
Minimal / matches precedent: Yes. The new exception is structurally identical to the existing Stage 4 (bess-agent/bess-developer) exception in pr-review.yml, and the PR explicitly calls out why Stages 1/3/5 must NOT copy this (no autonomous-spend rule authorizes their cost). It also correctly reasons that only a registered collaborator account (bess-product-owner, already referenced elsewhere in the unmodified parts of CLAUDE.md) belongs in an actor gate — consistent with the existing rule about not pre-authorizing bess-developer before the rename.
Scope-assessment statement (checklist item 5): The PR body doesn't explicitly label this "local fix" vs "structural" per rules.md Debugging Protocol step 9, but the change itself is unambiguous — it's a single-clause addition to an existing conditional, mirroring an already-merged pattern 1:1. Not asking for a restatement here.
Nits (non-blocking):
.github/workflows/issue-analyze.yml:24-25(new comment block):which put the maintainers name on comments they never wrote / and hid which decisions were the agents.— missing apostrophes, should readmaintainer's nameandthe agent's. Comment-only, no functional effect.
No rule violations found (no Optional[x], no hasattr/getattr fallbacks, no new classes, no exception string-matching, no hardcoded entity IDs — none of that is even in scope for this diff). CI is green (gh pr checks 645).
APPROVE.
Problem
Every automated comment the Product Owner makes goes out as
bess-product-owner— labels, board fields, reporter replies — except the one that spends money.backlog's Autonomous spend rule authorises the PO agent to fire Stage 2, bounded: labelledbug, opened by an external reporter, debug log attached, no prior analyze comment. Butissue-analyze.ymlgated on:github.event.comment.user.login == github.repository_ownerand nothing else. The rule and the gate disagreed, and the only way to satisfy both was to post the trigger as the maintainer.
Live consequence — three comments on the tracker right now say
@claude-bot analyzefromjohanzander, on #624, #627 and #643. He wrote none of them. Worse than cosmetic: it erases the line between what the maintainer decided and what the agent decided, on the single write that costs money.Fix
The gate accepts
bess-product-owneralongside the owner, and the skill tells the PO to usegh-agent.sh --as poinstead of plaingh.Stages 1, 3 and 5 stay owner-only
Stated in the workflow comment so it is not copied by pattern-matching. Their spend (~$1–4 and ~$2–10) is authorised by no autonomous rule, so nothing would be enforcing a bar on the far side of the gate.
The exception here is safe precisely because the bounding rule already exists — the gate is being brought into line with a constraint that was already written down, not loosened.
Only registered accounts belong in a gate
bess-product-ownerexists today and is a collaborator on this repo. This is the same reason CLAUDE.md refuses to pre-authorisebess-developerbefore the account rename: on a public repo, an unregistered username can be claimed by anyone, so naming one in an actor gate is exploitable.Test plan
./scripts/quality-check.shgreenissue-analyze.ymlparses as valid YAML after the editissue_commentevent. First PO-fired analyze after merge is the proof. If it is skipped, the gate is wrong and that is the finding — the skill now says to report it rather than fall back to plaingh.Outcome-level coverage
None — this is a workflow actor gate. No optimizer path, no test harness reaches it. Stating that rather than implying the green checkbox above covers it.
Scope
.github/workflows/issue-analyze.yml(the gate),CLAUDE.md(the bot rules),.claude/skills/backlog/SKILL.md(the spend instruction). No CHANGELOG entry: agent tooling, no user-visible effect.🤖 Generated with Claude Code