Repository navigation
feat(bitwarden-beads-tools): add a plugin of tools that complement beads - #284
SaintPatrck wants to merge 7 commits into
Conversation
The plugin passes --brief, which only bd 1.3 and later accept, so an older bd already read as no beads project and the bare-array fallback could never run. Headless runs have no one to see the pane, so they skip it and the 30-second poll, and the status line now reports a bd list it could not parse instead of reading it as none open.
![aikido-pr-checks[bot]](https://avatars.githubusercontent.com/in/898896?s=60&v=4){kind=small}
Claude Code validationResult: Pass Re-reviewed the Not covered: Findings
Dropped after documentation check:
Dropped under the re-run rule: one candidate — that the 30-second poll is never disarmed when the session leaves the beads project ( Previous findings, re-checked against the current diff:
|
SaintPatrck
changed the title
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| void $.ui.open({ id: PANE, title: "Beads" }); | ||
| } | ||
| }); | ||
| $.clock.every(POLL_MS, () => void refresh($)); |
There was a problem hiding this comment.
bd every 30s in directories that are not beads projects — contradicting the README's "stays out of the way".
Details and fix
$.clock.every(POLL_MS, …) runs for the whole life of every interactive session, and it is never gated on whether the first refresh found anything. refresh → load always calls $.process.run(["bd", "list", …], { cwd, timeoutMs: 10_000 }), so when bd is missing, older than 1.3, or the directory has no beads database, every user who installs this plugin still pays a process spawn every 30 seconds — up to a 10-second hang each time if bd stalls — for the rest of the session.
plugins/bitwarden-beads-tools/README.md:29 promises the opposite: "In a directory that is not a beads project, bd list fails and the tool stays out of the way: no pane opens and the status line is left empty." The pane and status line do stay empty, but the subprocess does not stop.
Arm the interval only once a refresh has succeeded, and let classic.CwdChanged arm it when the session moves into a beads project:
if (e.isInteractive) {
void refresh($).then((found) => {
if (found !== null) {
void $.ui.open({ id: PANE, title: "Beads" });
startPolling($);
}
});
}where startPolling registers $.clock.every(POLL_MS, …) at most once per session.
Reference: Mods reference — $.clock
There was a problem hiding this comment.
Fixed: the poll now starts on the first refresh that finds a beads project, so a session outside one never runs bd on a timer. We arm it from refresh rather than only session.start and CwdChanged, which also covers a bd init mid-session.
…pane current Outside a beads project the 30-second poll spawned bd for the whole session, a slow bd list could overwrite a newer result, and /clear, /resume, or /branch left the pane claiming there was no beads project until the next tick.
| const isQuiet = | ||
| next === null || (next.error === undefined && next.beads.length === 0); |
There was a problem hiding this comment.
The isQuiet condition combines || and && on one line, requiring readers to track multiple logical sequences.
Show fix
| const isQuiet = | |
| next === null || (next.error === undefined && next.beads.length === 0); | |
| const hasNoError = next !== null && next.error === undefined; | |
| const hasNoBeads = next !== null && next.beads.length === 0; | |
| const isQuiet = next === null || (hasNoError && hasNoBeads); |
Details
✨ AI Reasoning
The isQuiet expression combines an OR with a grouped AND on the same line. Readers must track how those conditions interact to determine whether the snapshot is quiet, making this more cognitively demanding than a single logical sequence.
Reply on this comment to give feedback or ignore the issue.
More info
…tools # Conflicts: # .claude-plugin/marketplace.json # README.md
🤖 Bitwarden Claude Code ReviewOverall Assessment: APPROVE Since the last review on 2026-10-23, the branch has gained only two merges from |
…tools # Conflicts: # .claude-plugin/marketplace.json # README.md
| .run(argv, { cwd, timeoutMs: 10_000 }) | ||
| .catch(() => null); | ||
|
|
||
| if (ran === null || ran.exitCode !== 0) { |
There was a problem hiding this comment.
🎨 SUGGESTED: A missing or broken bd is indistinguishable from "not a beads project" — both go silent, while a JSON parse failure does report an error.
Details and fix
Every failure mode collapses into the same null snapshot: bd absent from PATH (which rejects, so .catch(() => null) gives ran === null), a bd older than 1.3 rejecting --brief, a corrupt database, and a genuine non-project directory. All four suppress the pane and leave the status line empty, so a user whose bd is broken gets no signal at all. The asymmetry is visible in the code itself — the parse-failure path at line 88 does surface "bd list returned output that is not JSON".
This is partly deliberate: README.md:15 documents that "an older bd reads as a directory with no beads project," and silence outside a project is the stated design. The gap is narrower than the whole branch — a spawn failure is not the same as a clean non-zero exit, and only the former means the CLI is missing.
Separate the spawn failure from the non-zero exit, keeping null for the documented cases:
if (ran === null) {
return { beads: [], error: "bd not found on PATH" };
}
if (ran.exitCode !== 0) {
return null;
}validate-ai
| } | ||
|
|
||
| try { | ||
| const parsed: unknown = JSON.parse(ran.stdout || "{}"); |
There was a problem hiding this comment.
🎨 SUGGESTED: Truncated bd stdout is reported to the user as "not JSON" rather than as truncation.
Details and fix
$.process.run caps stdout at 4 MiB and sets isStdoutTruncated when it cuts the output, but that field is never read here — the tests mock it on every process.run stub, so the shape is already known to the module. With -n 0 asking bd list for every row, a large enough beads project overflows the cap; the partial JSON then fails JSON.parse and the user sees beads: error reading bd list, which points at bd rather than at output size.
Low likelihood — --brief rows are small, so this needs on the order of tens of thousands of beads — but the failure is silent about its real cause when it happens.
Check for the cut before parsing:
if (ran.isStdoutTruncated) {
return { beads: [], error: "bd list output was too large to read" };
}Reference: Mods reference — Limits — "$.process.run stdout and stderr | 4 MiB each. isStdoutTruncated or isStderrTruncated is true when one was cut."
validate-ai
| const rows = (parsed as { issues?: unknown } | null)?.issues; | ||
|
|
||
| return { | ||
| beads: Array.isArray(rows) ? rows.map(toBead).filter(isBead) : [], |
There was a problem hiding this comment.
🎨 SUGGESTED: A payload without an issues array renders as "No open beads." rather than as an unreadable response.
Details and fix
Array.isArray(rows) ? rows.map(toBead).filter(isBead) : [] collapses "bd returned a shape we do not understand" into the same state as "this project has no open work". If a future bd renames the field or wraps the list, the pane and the status line confidently report an empty backlog while the project is full of open beads — a wrong answer is worse here than a visible error, and the module already has an error path for exactly this class of problem one line below.
Route the unexpected shape to that existing error path:
if (!Array.isArray(rows)) {
return { beads: [], error: "bd list returned an unexpected shape" };
}
return { beads: rows.map(toBead).filter(isBead) };validate-ai
|
|
||
| const ran = await $.command.run(RUN); | ||
| expect(JSON.stringify(ran)).toContain("No beads project here"); | ||
| }); |
There was a problem hiding this comment.
🎨 SUGGESTED: Two of the four documented refresh triggers — the Bash tool.call hook and classic.CwdChanged — have no test.
Details and fix
The suite covers session start, /beads-pane, the poll, the overtake guard, and /clear, but nothing exercises the tool.call handler (register.tsx:185-193) or classic.CwdChanged (register.tsx:195-200). Both are listed in README.md:30 and CHANGELOG.md:16 as ways the pane stays current.
The Bash path turns on the regex /(^|[\s;&|(])bd\s/, which is the kind of expression that regresses silently — it already misses sh -c "bd close x", since the preceding " is not in the character class. A test would pin the behavior that is intended.
Add a case that fires a compound Bash call and asserts the refresh ran:
test("a bd command in a Bash call refreshes the beads", async ($, on) => {
let runs = 0;
on("session.cwd", () => ({ value: "/work/demo" }));
on("process.run", () => {
runs += 1;
return {
value: {
exitCode: 0,
stdout: JSON.stringify({ issues: BEADS }),
stderr: "",
isStdoutTruncated: false,
isStderrTruncated: false,
},
};
});
on("ui.status", () => ({ value: undefined }));
await $.tool.call({ tool: "Bash", command: "git commit -m x && bd close demo-1" });
expect(runs).toBe(1);
});validate-ai
🎟️ Tracking
Self-identified improvement; no ticket.
📔 Objective
This adds bitwarden-beads-tools, starting with a beads pane that keeps the project's open, in-progress, and blocked beads beside the chat and in the status line, refreshing as
bdcommands run. Engineers tracking work in beads otherwise have no view of it inside Claude Code short of runningbdby hand. It requires bd 1.3 or later, the first release withbd list --brief. The whole tool is one mods module, sohooks/register.tsxis the place to start reading.📸 Screenshots
🤖 AI-assisted review
Reviewed locally with
code-review-localandvalidate-ai-local; the one IMPORTANT finding (bd list --briefneeds bd 1.3) is addressed on this branch.