feat: add the board bootstrap script, with the Priority tiers the board actually has - #656
Conversation
Split out of #614, whose review's primary blocker was that this board work was undescribed and orthogonal to that PR's mypy/review-bot fixes. Most of the original bundle is now redundant: main already carries the 'Ready for Dev'/'In Progress' column names, the awaiting-before-analyzed precedence fix, and the 'there is no P0' correction in the backlog skill. What was left unlanded is the bootstrap script and the two PO memory files. backlog-board-init.sh created the Priority field with P0,P1,P2 — a tier the live board does not have and no consumer reads. It now creates P1-P4, which is what the board carries, what the skill documents and what the digest ranks on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012vHvj9qtD1jDYVnm3LjTnt
|
@claude-bot review |
There was a problem hiding this comment.
Summary
This is a clean split-out of #614's board-only work, and its narrative checks out: I diffed the "already landed" table in the PR description against current origin/main and confirmed each item really is already there (the Ready for Dev/In Progress naming, the awaiting-before-analyzed precedence fix, and the "no P0" note in SKILL.md). The P0→P1-P4 fix in scripts/backlog-board-init.sh is correct: I grepped the whole tree and SKILL.md:58 is the only surviving P0 reference anywhere, and it correctly says there is no P0. shellcheck scripts/backlog-board-init.sh is clean, and CI is green.
No linked issue (this is a split, not a fix), so there's no root-cause diagnosis to match against — the PR body's own rationale (mirrors #614's CHANGES_REQUESTED follow-up comment) is the closest thing, and it's consistent.
Blocking: new memory file mirrors board/issue state, which SKILL.md explicitly forbids
.claude/skills/backlog/SKILL.md:74-78 (already on main, unchanged by this PR) states:
State lives on GitHub, nowhere else
Never write a local file that mirrors board or issue state. Priority is a board field, rationale is an issue comment, dedupe is close-as-duplicate, blocked-by is a
Blocked by #Nline in the issue body.
This PR adds .claude/agent-memory/product-owner/project_backlog_board_state_2026_08_16.md, whose lines 22-30 do exactly that:
Several issues surfaced during this pass that are candidates for maintainer
attention but were out of scope (board-writes-only run):
- #520/#393/#571 form a cluster (LOAD_SUPPORT / discharge-gate shadow-price
correctness) — likely worth a combined roadmap push rather than fixing
piecewise.
- #118 (Solax-Growatt MIN-inverter correction) has 29 comments — the busiest
thread in the backlog — and is still unlabeled and un-triaged.
- #96 is the only `analyzed`/Ready for Dev item; everything else needing
Stage 2 analysis is still sitting in Analysis.
This is a point-in-time snapshot of per-issue triage/board state (open item counts, which issues are unlabeled, which is the only Ready-for-Dev item) written 2026-08-16 — already 2 days stale as of this review. It's precisely the failure mode the rule exists to prevent: a future PO session reading this memory could treat "#96 is the only analyzed item" as still true instead of re-querying GitHub, the same class of staleness bug this very PR's memory file (project_backlog_board_state_2026_08_16.md:8-12) diagnoses as the root cause of the board being out of sync in the first place ("the digest's presence doesn't imply board presence... made this easy to miss without diffing against gh project item-list directly").
By contrast, lines 8-20 of the same file (the "why the board drifted" narrative and the "diff item-list against issue-list before trusting sync" lesson) and the sibling project_board_field_ids.md (GraphQL schema/field IDs — structural, not board contents) are legitimate, generalizable "project" memories and don't have this problem.
Neither this PR's description nor SKILL.md reconciles the conflict — SKILL.md has no exception carved out for the agent-memory system, and the PR's "Documentation check" section addresses bess-knowledge.md/SOFTWARE_DESIGN.md/SKILL.md's board-schema content but doesn't address this specific rule.
Suggested fix: trim project_backlog_board_state_2026_08_16.md to the durable lesson (lines 8-20) and drop the per-issue snapshot (lines 22-30), or explicitly amend SKILL.md's "never mirror" rule to carve out an exception for agent memory (distinct from files consumed programmatically by scripts) and say why that's safe despite the staleness risk the memory system itself warns about.
Test coverage / minimality
- No tests added, and the PR explicitly states why (a one-shot, idempotent-by-lookup bootstrap script whose every effect is a live
gh projectGraphQL call — no local execution model). Givenscripts/backlog-board-init.shonly touches infra outside this repo's test harness, that's a reasonable call, not a shortcut. - Diff is minimal and scoped to exactly the two items the PR claims (
backlog-board-init.sh+ the two memory files) — verified nothing else in the original #614 bundle snuck back in.
Verdict
Requesting changes only for the memory-file/SKILL.md conflict above — everything else (the P0 fix, the split rationale, script correctness, CI) checks out cleanly.
The Stage 4 review found this violates the backlog skill's own rule ('State
lives on GitHub, nowhere else' — never write a local file that mirrors board
or issue state). The per-issue triage list was exactly that, and was already
stale two days after being written.
What stays is the generalizable lesson: the digest derives a column for an
issue whether or not a card exists, so board presence has to be checked
against gh project item-list directly. The name and index line now say that
rather than framing the file as a snapshot.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012vHvj9qtD1jDYVnm3LjTnt
|
@claude-bot review |
There was a problem hiding this comment.
Review
Linked issue: none — this is an explicit split-out of #614's board changes, not an issue fix (Closes correctly omitted). Cross-checked the PR body's claim table against origin/main: the "already landed" items really are landed, and the two items claimed as carried over here (scripts/backlog-board-init.sh, the two PO memory files) really are the only diff (git diff dd47d59..pr-656 --stat — 4 files, 146 insertions, matches gh pr diff exactly).
Scope: minimal and matches the description — no drive-by changes.
The P0 fix: scripts/backlog-board-init.sh creates Priority with P1,P2,P3,P4 (line 63: --single-select-options "P1,P2,P3,P4"), consistent with project_board_field_ids.md ("Priority... options are P1, P2, P3, P4. There is no P0 option") and with .claude/skills/backlog/SKILL.md:58 ("Priority | P1, P2, P3, P4 — there is no P0"). All three sources agree; this isn't a case I can prove wrong.
Shellcheck: clean on the extracted script.
Two findings for the author to weigh
-
file: scripts/backlog-board-init.sh:35-36— the idempotency check swallows failures:existing=$(gh project list --owner "$owner" --format json \ --jq ".projects[] | select(.title == \"$title\") | .number" || true)
If
gh project listfails for any reason other than missing scope (rate limit, transient network error, GraphQL hiccup) — theprojectscope check above only confirms the token has the scope, not that this specific call succeeds —|| trueswallows it,$existingcomes back empty, and the script falls through togh project create, producing a duplicate "BESS Manager Backlog" project. That contradicts the script's own header comment: "Idempotent: if a project with this title already exists, print its number and change nothing." Practical severity is low since a human runs this by hand and would notice a second project, but the failure mode is real and the header's guarantee doesn't hold for it. -
Testability claim vs. repo precedent. The PR body says (Evidence section): "it cannot be exercised without creating a real board... idempotent-by-lookup rather than by state this repo holds." But this repo already has an established pattern for exactly this class of script —
backend/tests/test_backlog_digest.pyandtest_backlog_rhythm.pyshimgh/gitonPATHwith canned JSON to test one-shotgh-wrapping bash scripts without a live board, and the immediately-prior PR #614 used the same technique forquality-check.sh's mypy gate ("same approach astest_backlog_digest.py," per #614's own description). The two things most worth a regression test here are exactly the two things this PR is about — theP1,P2,P3,P4option string (a plain string literal that could silently drift back toP0,P1,P2with no test to catch it) and the idempotent-exit branch (no field-create calls when a project already exists) — and both are checkable purely from the arguments passed to a shimmedgh, no live board required. Not a blocker given the script is a human-run, one-shot bootstrap rather than something exercised on every run like the digest, but the specific claim that it can't be tested doesn't hold up against this repo's own precedent, including the sibling PR this was split from.
Checklist
- Architecture compliance: N/A (no Python/HA sensor code touched)
- Error handling rules: N/A (no exception handling in diff)
- Code quality: shellcheck clean; memory files well-formed (frontmatter,
[[links]]present and resolve) - Tests: none added; see finding 2 above
- Security: no secrets — GraphQL node/field/option IDs in the memory files are not credentials
- Fitness of approach / workaround check: no routing-around-a-problem pattern here; this is new capability, not a patch over an ordering/timing issue
- Scope statement (rules.md Debugging Protocol step 9): N/A — not a bug fix, and the PR body's own "what was left to move" table serves the equivalent purpose for a split-PR
Verdict: COMMENT. No rule violation blocks this — the P0 fix is correct and cross-verified, the diff is minimal and matches its description, and the duplicate-project risk is low-severity given a human runs this script interactively. The two findings above are worth addressing (or explicitly deciding against) before merge, but neither is a correctness bug in what ships today.
The review was right that 'this cannot be tested' does not hold against this repo's own precedent — test_backlog_digest.py shims gh on PATH for exactly this shape of script, and #614 used the same technique. Three things are worth pinning and all are visible from the arguments the script passes: - Priority is created as P1,P2,P3,P4. It is a bare string literal that already drifted once (P0,P1,P2), and nothing else would catch it. - An existing board creates nothing at all. - A failed lookup does not become a second board. That last one is also a fix, not just a test. The lookup ended in '|| true', which makes 'no such board' and 'the lookup failed' indistinguishable — a rate limit or transient GraphQL error would have created a duplicate 'BESS Manager Backlog', contradicting the idempotence the header promises. It now stops instead. Each test verified to redden under the matching mutation: restoring the P0 tiers fails the first, restoring '|| true' fails the third. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012vHvj9qtD1jDYVnm3LjTnt
|
@claude-bot review |
There was a problem hiding this comment.
Summary
This is a clean, minimal revision that fully resolves both prior review rounds' findings. No linked issue (explicit split-out of #614, Closes correctly omitted) — verified the "already landed on main" table against origin/main directly for the 3 outstanding items and confirmed via the checks below.
Round 1 (CHANGES_REQUESTED) — resolved
The blocking finding was that project_backlog_board_state_2026_08_16.md mirrored per-issue board state (the #520/#393/#571, #118, #96 snapshot), which SKILL.md's "State lives on GitHub, nowhere else" rule forbids. That block is gone — the file is now 22 lines (wc -l confirmed), containing only the durable "diff item-list against issue-list before trusting sync" lesson round 1 explicitly called legitimate. Fixed.
Round 2 (COMMENT) — both findings resolved
-
|| trueswallowing a failed lookup (previouslyscripts/backlog-board-init.sh:35-36): grepped the current script, no|| trueremains. The lookup is nowif ! existing=$(gh project list ...); then ... exit 1; fi, which stops the run instead of reading a failed call as "no board exists." I reproduced this manually with aghshim returninglist_rc=1: the script correctly exits 1 withcould not list projects for 'johanzander'on stderr and issues noproject createcall. -
Testability claim (previously said untestable):
backend/tests/test_backlog_board_init.pynow shimsghon PATH exactly per thetest_backlog_digest.pyprecedent round 2 pointed to, covering the two things worth pinning — theP1,P2,P3,P4tier string and the idempotent/failed-lookup branches.
Independent verification
I didn't just read the tests — I reproduced all three scenarios by hand with an equivalent gh shim, without pytest (no venv available in this environment):
- No existing board → creates project,
field-createcalled with--single-select-options P1,P2,P3,P4, noP0anywhere. - Existing board (
list_output="3") → printsPROJECT_NUMBER 3, zeroproject create/field-createcalls. - Failed lookup (
list_rc=1) → exits 1, printscould not list projects, zeroproject createcalls.
All three match the tests' assertions and the PR description's RED/GREEN mutation-testing claims exactly. shellcheck scripts/backlog-board-init.sh is clean (verified locally, exit 0). python3 -m py_compile and bash -n both pass on the changed files. CI is fully green (Fast tests, Code quality, E2E, Docker build, Frontend checks, Merge gate).
Checklist
- Root-cause match: N/A — no linked issue, this is a scope split; the PR's own "what was left to move" table stands in for it and I verified it against
origin/main. - Would the test catch a regression: yes — verified by hand-reverting both fixes' logic via the shim and confirming the corresponding assertion fails, matching the PR's stated RED/GREEN evidence.
- Minimal / no scope creep: yes — 5 files, matches the description exactly (script + 3 memory files + 1 test file), nothing else from the original #614 bundle snuck back in.
- Rule violations: none found. No
Optional[x]/hasattr/sensor-access/API-camelCase rules apply (no Python app code touched). No new classes. Comments in the script/tests are WHY-only (rationale for|| trueavoidance, forP1-P4vsP0), consistent with the repo's existing test-file docstring style (test_backlog_digest.py). - Fitness of approach: new capability (bootstrap script), not a workaround/patch — the workaround-check and separation-of-concerns questions in the checklist don't apply here.
No blockers. Approving.
Summary
scripts/backlog-board-init.shand the two Product Owner board-memory files.P0Priority tier the review flagged.What was left to move, and what was not
The original bundle was mostly overtaken by main while #614 sat. Checked against current
origin/main:Ready for Dev/In Progresscolumn names inbacklog-digest.sh.claude/skills/backlog/SKILL.mdtest_backlog_digest.pycolumn updatesawaitingchecked beforeanalyzed(the #96 dead-end)SKILL.mddocumenting that noP0existsscripts/backlog-board-init.sh.claude/agent-memory/product-owner/(2 memories + index)So this PR carries only the two unlanded pieces. Nothing from #614's board half is dropped: its full state is preserved at
eb4d00e9.The
P0fixbacklog-board-init.shcreated the Priority field with optionsP0,P1,P2. The live board hasP1–P4and noP0— confirmed by this PR's own memory file (the GraphQL option ids), and bySKILL.mdon main, which states "there is noP0". A board bootstrapped by the old script would have produced a tier no consumer reads, and would have been missingP3/P4, which the digest ranks on. NowP1,P2,P3,P4, with the reason recorded in the script so it cannot drift back.The memory file's note that the skill text was "stale" is also corrected — that was true when it was written, and is not any more.
Test plan
./scripts/quality-check.shpasses locally (0 errors, 0 warnings)Evidence the test discriminates
backend/tests/test_backlog_board_init.pyshimsghon PATH and asserts on the arguments the script passes — the patterntest_backlog_digest.pyalready uses for this shape of script. Review round 1 correctly pointed out that this PR's original "cannot be tested without a live board" claim did not survive contact with the repo's own precedent, so that claim is withdrawn and replaced by the tests.Each test reddens under the mutation it guards:
--single-select-options "P1,P2,P3,P4"back to"P0,P1,P2"test_priority_field_is_created_with_the_tiers_the_board_hasFAILED, 1 failed / 2 passed... || truetest_a_failed_lookup_does_not_create_a_second_boardFAILED, 1 failed / 2 passedAlso from review round 1
The duplicate-board risk is fixed, not merely noted.
gh project listended in|| true, which makes "no such board" and "the lookup failed" indistinguishable — a rate limit or transient GraphQL error would have produced a second "BESS Manager Backlog", contradicting the idempotence the script's own header promises. It now stops with a message, and the third test pins that.The PO board-state memory was trimmed to its durable lesson: the per-issue triage snapshot it carried was mirrored board state, which
.claude/skills/backlog/SKILL.md("State lives on GitHub, nowhere else") forbids, and it was already stale two days on.Outcome-level coverage
The assertions are on what the script does — which
ghcalls it makes, with which arguments — not on its log text. For a bootstrap whose entire effect is those calls, that is the outcome. No optimizer fixture or golden applies; this diff touches no DP, intent or control path.Documentation check
docs/agents/bess-knowledge.mdanddocs/SOFTWARE_DESIGN.mddescribe optimizer behaviour and system design; this PR touches neither — no mechanism, formula or threshold either file documents is affected..claude/skills/backlog/SKILL.mdis the doc for this area and already describes the board schema correctly on main, including the absentP0.No
Closes— this is a split, not an issue fix.