Fix #646: make Stage 2 publish by command, and fail the job when it doesn't by johanzander · Pull Request #649 · johanzander/bess-manager

johanzander · 2026-08-18T20:32:10Z

Summary

Stage 2's prompt now publishes by running gh issue comment --body-file instead of describing a comment it never posts.
A post-agent step fails the job when no comment was posted or neither analyzed nor needs-human-review was applied — the issue's acceptance criterion.
PROCESS renumbered (it had two steps numbered 3).
quality-check.sh gains a static gate so neither half can be removed silently.

Root cause

The prompt never told the agent to run anything to publish. Step 4 read:

4. Post ONE comment on the issue with this structure: — followed by a markdown template.

That is a description of a document, not an instruction to execute a command. These workflows run in agent mode (use_sticky_comment: false, track_progress: false, confirmed in the run log), so the action posts nothing on the agent's behalf and creates no tracking comment. An agent that renders the template as its final assistant message has, by its own lights, posted the comment: the run exits success and nothing reaches GitHub. Having "finished" at step 4 it never reaches step 5, which is why the label was untouched too.

The same-version contrast is what identifies it rather than merely fitting it:

Workflow	How its prompt says to publish	Result on action `459ad358` / CLI 2.1.234
Stage 1 triage	`gh issue comment <n> -b "…"` — literal	posts
Stage 4 PR review	`gh pr review` — literal, named 3×	`APPROVED` landed 2026-08-18 06:38
Stage 2 analyze	prose template, no command anywhere	silent 3/3 the same day

Stage 2 was the only bot workflow whose publish step was prose, and the only one that failed to publish. The action is not broken — Stage 4 wrote to GitHub fine on the identical version hours earlier. Every step the silent runs did perform is one carrying a literal command (gh issue view ran; the analysis itself got done — #542 burned 17 turns and $0.72 of real investigation before publishing nothing).

Limits of this diagnosis, stated plainly: the action sets show_full_output: false, so every turn is hidden and only the result JSON survives. I could not observe the agent emitting the template as prose and stopping — this is inference from what the runs did and did not do, not from a transcript. That is precisely why the fix does not rest on the prompt change alone.

Fix

1. Publishing is a command. Both the conclusive and the inconclusive path now write the body to a file and run gh issue comment --body-file (a file, not -b, because the body carries backticks, quotes and newlines that do not survive shell quoting). The prompt says explicitly that writing the diagnosis as a reply does not publish it.

2. The workflow verifies it afterwards. A step keyed if: always() re-reads the issue and fails the job if no comment appeared since the agent started, or if neither label was applied. A timestamp step before the agent lets it distinguish a comment the agent posted from the @claude-bot analyze trigger comment that is always already present.

The guard asserts and deliberately does not repair. Posting the comment on the agent's behalf would be a second publishing path whose only job is to route around the first one failing — it would mask the regression instead of surfacing it (docs/agents/rules.md, Debugging Protocol step 8). A loud red run is the product.

Worth being precise about what this buys, since the issue is framed around cost: it does not prevent the spend. A silent run still burns its $0.33–1.58 — the guard makes that visible instead of green. The saving is the second cost, the one on #593 where the maintainer wrote the diagnosis by hand three days later.

Test plan

./scripts/quality-check.sh passes (Errors: 0, Warnings: 0), re-run after merging origin/main
.venv/bin/pytest -m slow — 554 passed, 8 skipped, 423s
YAML parses; job has 5 steps in the right order (checkout, app-token, Mark analysis start, claude-code-action, Verify the analysis reached the issue)
The guard executed for real against the live GitHub API, all three branches, using the exact script body from the workflow:
- silent run simulated (ISSUE=646 SINCE=2030-…) → posted: 0, error annotation, exit 1
- healthy run (ISSUE=646 SINCE=2020-…) → posted: 1, labels bug,analyzed, exit 0
- ISSUE=643, one of the real issues whose Stage 2 run went silent → comments present but labels still bug,ready-for-analysis → label branch fires independently, exit 1
Not verified, and cannot be from here: that the prompt change actually stops the silence. GitHub Actions cannot be run locally, so nothing on my machine tests it. The honest test is a live re-fire of @claude-bot analyze on Runtime error: Check grid charge state #643 or Growatt VPP order of register values #593 after this merges — both reproduce on demand, ~$0.50. If the prompt change is wrong, the guard turns that run red instead of green, so we find out immediately rather than in another three days.

Evidence the test discriminates

Reverted: .github/workflows/issue-analyze.yml to origin/main (git checkout origin/main -- …), leaving the gate in place
Result: the gate failed on both halves independently —
- ❌ issue-analyze.yml never names 'gh issue comment'. Its publish step is prose…
- ❌ issue-analyze.yml has no post-agent verification step…
- Errors: 1
Restored: git checkout HEAD -- …; gate green again, git status clean

The two assertions were also checked as predicates before the fix — names_command=False, has_always=False, has_readback=False — and after: all three True.

Near-miss check on the label matcher: a label named not-analyzed does not satisfy the analyzed requirement (the comma-wrapping is what makes that hold).

Outcome-level coverage

quality-check.sh → "bot workflow publish contract": pins that both Stage 1 and Stage 2 name their publish command, and that Stage 2 carries an if: always() step re-reading --json comments / --json labels. Checked by shape rather than step name, which renames freely.
The behavioural outcome — a silent run goes red — is covered by the live three-branch execution above rather than by a unit test, because the outcome only exists against the GitHub API. There is no execution model for a workflow run in this repo to assert against.

Scope assessment

Workaround check: the diff adds no parameter, flag, default-fallback, second construction site, extra trigger or branch whose job is to route around an ordering/timing/dependency problem. The guard is an assertion that fails the job — not a retry, and explicitly not a step that publishes on the agent's behalf.

Category: local. Both changes stay inside issue-analyze.yml's existing contract (its prompt text, its job steps), plus one assertion in a file that already does exactly this kind of assertion. The other three bot workflows were checked by grep and already name their command, so none of them needed touching.

Notes for the reviewer

No CHANGELOG.md entry, deliberately. This is agent-infrastructure with zero user-visible effect — no BESS Manager user is affected — and the curated changelog omits internal changes of that kind. Say the word and I'll add one.

Documentation check: docs/agents/bess-knowledge.md and docs/SOFTWARE_DESIGN.md describe no mechanism this diff touches (SOFTWARE_DESIGN.md's "Stage 2" hits are sensor-discovery stages, unrelated). docs/agents/workflow.md is updated — its pipeline diagram now records that Stage 2 fails the job when it publishes nothing, so a red Stage 2 run is interpretable.

Flagged, not fixed: all five workflows track the floating anthropics/claude-code-action@v1 tag, so the code running in CI can change with no commit in this repo. It moved across this regression's boundary (9d7150bc/2.1.233 → 459ad358/2.1.234).

To be clear about what that is and is not: it is not this bug's cause — Stage 4 published fine on 459ad358 hours after the switch, which is what rules it out. It is a reason future upstream changes arrive unannounced. Pinning each uses: to a full SHA would address that class, at the cost of a standing maintenance commitment (a pinned action needs deliberate bumping, so it wants a Dependabot rule alongside). Five files plus a policy is a separate call, not something to fold into a fix PR.

Closes #646

Stage 2 (issue-analyze.yml) exited `success` while posting nothing on five of nine non-skipped runs over three days, billing $0.33-1.58 each. Every signal said the run was healthy: green job, is_error false, permission_denials_count 0. The `ready-for-analysis` label staying put is indistinguishable from "nobody has run analyze yet", so the backlog pass could not see it either. On #593 the maintainer eventually wrote the diagnosis by hand -- the stage was paid for and the work done twice. The prompt never told the agent to RUN anything to publish. Step 4 read "Post ONE comment on the issue with this structure:" followed by a markdown template -- a description of a document, not an instruction to execute a command. These workflows run in agent mode (use_sticky_comment false, track_progress false), so the action posts nothing on the agent's behalf and creates no tracking comment. An agent that renders the template as its final assistant message has, by its own lights, finished; it then never reaches the labelling step, which is why the label was untouched too. The contrast is what identifies it. On the same action version (459ad358 / CLI 2.1.234), Stage 4 -- whose prompt names `gh pr review` explicitly -- landed an APPROVED verdict, while Stage 2 went silent three times within three seconds. Stage 2 was the only bot workflow whose publish step was prose, and the only one that failed to publish. So publishing is now a literal `gh issue comment --body-file` command on both the conclusive and inconclusive paths, and PROCESS is renumbered (it had two steps numbered 3, so "am I done with step 3" was satisfiable by the wrong one). That fix is a behavioural argument about what a model will infer, so it is not trusted on its own: a post-agent step re-reads the issue and fails the job when no comment was posted or neither label applied. It asserts and deliberately does not repair -- posting on the agent's behalf would be a second publishing path routing around the first one failing, which masks the regression instead of surfacing it. quality-check.sh gains a matching static gate so neither half can be removed silently. Closes #646

…silent-analyze

bess-agent · 2026-08-18T20:32:59Z

@claude-bot review

bess-manager-claude-bot

Review: PR #649 (fixes #646)

Root-cause match

Confirmed against origin/main's prior .github/workflows/issue-analyze.yml: step 4 ("Post ONE comment ... with this structure:") was indeed a markdown template with no shell command, while the label step directly below it (gh issue edit ...) was literal. That asymmetry — the only bot workflow whose publish step was prose — matches the issue's evidence table (Stage 1/Stage 4 both name a literal gh publish command; Stage 2 didn't). The fix makes both the conclusive and inconclusive paths run gh issue comment --body-file /tmp/analysis.md explicitly, and the PROCESS renumbering (the old file really did have two steps labeled 3, at .github/workflows/issue-analyze.yml:71 and :112 pre-fix) is real, not invented.

Does the added check catch a regression?

Two independent layers, both verified:

Static (scripts/quality-check.sh): confirmed bash -n and python3 compile() clean on the new heredoc block; confirmed issue-triage.yml really does contain gh issue comment (checked against origin/main), so the PUBLISHERS dict's claim about the other workflow isn't speculative.
Runtime (.github/workflows/issue-analyze.yml, new "Verify the analysis reached the issue" step, if: always()): the SINCE/createdAt comparison uses matching ISO-8601 format from date -u +%Y-%m-%dT%H:%M:%SZ and GitHub's createdAt, so the lexical > comparison is sound. gh issue comment --body-file is confirmed as a real, valid gh flag (-F, --body-file). The label check's comma-wrapping (case ",$labels," in *,analyzed,*...) correctly rejects a false-positive substring match like not-analyzed — verified by hand-tracing the pattern.

This converts the exact failure mode in #646 (green run, is_error: false, nothing posted) into a hard job failure — that's the issue's literal acceptance criterion, and the PR's own live three-branch test against the real GitHub API (silent-run simulation, healthy run, and a replay against the actual silent #643) is solid evidence for the runtime half. The PR is honest that it cannot prove the prompt-wording change itself stops the silence (GitHub Actions can't run locally) — that's correctly called out as unverified rather than glossed over.

One minor looseness, not a blocker: quality-check.sh's guard checks that if: always() and --json comments/--json labels exist anywhere in issue-analyze.yml, not that they belong to the same step. Currently accurate for this file, and the tradeoff (shape-based over step-name matching, to survive renames) is stated explicitly in the script's own comment, so this reads as a deliberate, documented choice rather than an oversight.

Scope

Touches exactly .github/workflows/issue-analyze.yml, docs/agents/workflow.md, scripts/quality-check.sh — all directly load-bearing for the fix. No unrelated changes. The PR explicitly does not add an auto-repair/auto-post-on-behalf-of-agent path, which is correctly identified as what the workaround-check in docs/agents/rules.md (Debugging Protocol step 8) would flag — the guard asserts and fails loudly instead of masking the regression.

Scope-assessment statement

Present and correct: PR body states "Category: local" with an explicit workaround-check pass, matching my read that this change stays entirely within issue-analyze.yml's existing job/prompt contract.

Verdict

No rule violations found (no Python touched, so most of the architecture checklist is N/A). No scope creep. Documentation (docs/agents/workflow.md) is updated consistently with the behavior change. Approving.

…stage loads on every turn (#651) * docs: move the local-environment section out of CLAUDE.md (#650) Every bot stage loads CLAUDE.md before it does anything, on the main agent and again on its sub-agent, and re-sends it on every turn. It was 45,488 B, of which 33,743 B (74%) was one section describing this machine: the macOS sandbox knobs, podman, the permission deny/ask/allow rules, worktree layout, the Playwright install hang. None of it can apply in CI. All five stages run on a fresh ubuntu-latest checkout under `--permission-mode bypassPermissions`, so there is no prompt to avoid, no sandbox, no podman VM, no worktrees and no .venv symlinks. The `git stash` prohibition's own reasoning is "~20 worktrees share one refs/stash"; a runner has one checkout and no other agents. No workflow references the section. Measured on Stage 2, whose floor before it reads the issue, the bundle or any source was 92,438 B: CLAUDE.md 45,488 + bess-analyst.md 25,681 + rules.md 10,548 + the inline prompt 7,098 + architecture.md 3,623. The relocated section alone was 36% of that, twice over, on every turn. So it moves to docs/agents/local-agent-environment.md and is reached through the Agent Documentation Index, which is the mechanism CLAUDE.md already uses for rules.md, architecture.md, patterns.md and the rest. Nothing is deleted or condensed -- the content is hard-won, much of it marked (measured), and it is still required reading for autonomous local work. CLAUDE.md goes 45,488 B -> 11,926 B. One wording change was forced by the move: the text said "CLAUDE.md itself teaches `git -C .claude/worktrees/<name>`", which stops being true once the text is the teacher. It now says "this document itself". quality-check.sh gains a byte-budget gate so the section cannot creep back inline, written RED first (45,488 B against a 16,000 B cap) and green after at 11,926 B. Its failure message points at the relocate-and-link mechanism rather than at raising the cap, because raising the cap is how this grew. Five inbound references repointed, two of which only surface on a failure path and would have sent the reader to a file that no longer had the answer: worktree-setup.sh's blocked-cache diagnostic and verify-sandbox.sh's two assertion messages, plus sweep-prs/SKILL.md and skill-architecture.md. No CHANGELOG entry: zero user-visible effect, agent-facing documentation only, matching #649 which changed the same pipeline and added none. Closes #650 * docs: repoint the fifth Worktree Conventions reference (#650) The Stage 4 review found one more dangling pointer the sweep missed: release/SKILL.md:29 still cited "the Worktree Conventions in CLAUDE.md", a section that no longer exists there. The sweep missed it because the exclusion pattern was wrong, not because the search was too narrow. It filtered `\.venv` to skip the vendored tree, but grep -v applies to the whole line, and this line's TEXT contains `.venv` ("If this branch/worktree has no `.venv` yet"), so a real hit was dropped silently. Same shape as the earlier `head -20` truncation: a filter that reads as coverage while removing results. Re-swept with path exclusions anchored to path separators. Four references to "Worktree Conventions" remain and all four resolve; every other file citing CLAUDE.md points at a section still in it (Verification Before Action, Cost Discipline, Release Workflow, General bot rules, the index).

…655) Stage 2's main agent listed `.claude/agents/bess-analyst.md` (25,681 B) as REQUIRED READING item 3. That file IS the bess-analyst sub-agent's system prompt -- frontmatter `name: bess-analyst`, `tools: Read, Grep, Glob, Bash, WebFetch` -- so the main agent was loading a second copy of another agent's instructions into its own context, on every turn. Nothing consumed it. The six PROCESS steps are: get issue context, identify the current problem, delegate, verify the cited file:line, publish, label. No step applies a "domain expertise checklist". The one distinctive task, step 4, needs the cited code -- not a triage procedure the main agent never runs. The content was in fact loaded three times. The section that could plausibly serve as a judging standard, "Separate Evidence from Claims", is already restated almost verbatim in the sub-agent task the same prompt passes: hypothesis-not-diagnosis, triage-the-bundle-first, verify-the-code-path- matches-this-user's-setup, intentional-design-is-not-a-bug. Sub-agent system prompt, main-agent file read, and inline paraphrase. All four landed Stage 2 analyses (#118, #252, #624, #627) use the workflow's four headings, never bess-analyst.md's seven-item Output Format or its six-item type-B shape -- the main agent follows the prompt, not the file. So item 3 is removed, with an explicit note saying why, and step 4 gains the check that was missing: a report fails verification if it claims a CODE bug with no file:line, if a cited location does not say what is claimed, or if it blends P-optimality / forecast error / control noise into one verdict when actual behavior diverged from plan. Dead weight becomes an instruction that fires. Three things the first draft got wrong, found in review: - "cites no file:line" alone would have rejected CORRECT diagnoses. The same prompt tells the analyst that unavailable sensors, wrong inverter type or an HA integration mismatch are likely the real cause, and those cite no code. Now qualified to code-bug claims only. - The inconclusive path never removed `ready-for-analysis`, and backlog-digest.sh checks that label BEFORE `needs-human-review`, so the backlog pass reports the issue as un-analysed and re-dispatches analyze -- re-billing the stage. Widening that path without fixing it would have made a cost bug worse. It now clears the label. - The new criterion used vocabulary defined only in the file this diff stops the agent reading. P-optimality had a gloss; P≠R never expanded P or R and control noise had no definition. All three are now glossed inline. quality-check.sh gains a context contract gate, written RED first: no workflow may list a `.claude/agents/*.md` file as a numbered required read, and Stage 2 must still delegate. Both assertions were mutation-tested -- removing the delegation reddens it, and reformatting `subagent_type` with backticks or quotes does not, which the first bare-substring version got wrong in both directions. Saving is up to 25,681 B off the main agent per turn; the prompt itself grew 7,098 -> 7,821 B, so net is roughly -24.9 KB. "Up to", because whether a run obeyed the read instruction is not observable -- #646 showed it need not. No CHANGELOG entry: agent-facing, zero user-visible effect, as #649 and #651. Closes #654

johanzander added 2 commits August 18, 2026 22:20

Merge remote-tracking branch 'origin/main' into fix/issue-646-stage2-…

b890c1d

…silent-analyze

bess-manager-claude-bot Bot approved these changes Aug 18, 2026

View reviewed changes

johanzander marked this pull request as ready for review August 18, 2026 20:36

johanzander merged commit a3e6553 into main Aug 18, 2026
8 checks passed

johanzander deleted the fix/issue-646-stage2-silent-analyze branch August 18, 2026 20:52

johanzander mentioned this pull request Aug 18, 2026

Move the local-environment section out of CLAUDE.md, which every bot stage loads on every turn #651

Merged

5 tasks

johanzander mentioned this pull request Aug 18, 2026

Stage 2's main agent reads the sub-agent's own system prompt, and nothing uses it #654

Closed

Navigation Menu

Fix #646: make Stage 2 publish by command, and fail the job when it doesn't - #649

Fix #646: make Stage 2 publish by command, and fail the job when it doesn't#649
johanzander merged 2 commits into
mainfrom
fix/issue-646-stage2-silent-analyze

johanzander commented Aug 18, 2026 •

edited

Loading

Uh oh!

bess-agent commented Aug 18, 2026

Uh oh!

bess-manager-claude-bot Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

johanzander commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

Fix

Test plan

Evidence the test discriminates

Outcome-level coverage

Scope assessment

Notes for the reviewer

Uh oh!

bess-agent commented Aug 18, 2026

Uh oh!

bess-manager-claude-bot Bot left a comment

Choose a reason for hiding this comment

Review: PR #649 (fixes #646)

Root-cause match

Does the added check catch a regression?

Scope

Scope-assessment statement

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

johanzander commented Aug 18, 2026 •

edited

Loading