Conversation
| @@ -0,0 +1,167 @@ | |||
| --- | |||
There was a problem hiding this comment.
Wonder if we migrated this from a commands dir -> skills dir if claude and then copilot would just be able to use them https://docs.github.com/en/copilot/how-tos/copilot-cli/customize-copilot/create-skills
There was a problem hiding this comment.
I don't know---follow-up PR or that after we start playing with it? At the moment I've been just doing /create-issues then cycling through /red /green /refactor and it's been working nicely. I haven't tried playing with letting it run completely freely. I wanted to get some mileage on just using claude in this more supervised mode first
| | Backend/API | `routes/`, `api/`, `controllers/`, `services/`, `*.controller.*`, `*.service.*`, `*.resolver.*` | | ||
| | Backend/Data | `migrations/`, `models/`, `prisma/`, `schema.*`, `*.model.*`, `*.entity.*` | |
There was a problem hiding this comment.
Wonder if we will want to add some *.py things in here to direct it better 🤷
|
|
||
| - The test must fail for the RIGHT reason (not syntax/import errors) | ||
| - Only one test at a time - this is critical for TDD discipline | ||
| - Exception: For browser-level tests or expensive setup (e.g., Storybook `*.stories.tsx`), group multiple assertions within a single test block to avoid redundant setup - but only when adding assertions to an existing interaction flow. If new user interactions are required, still create a new test. Split files by category if they exceed ~1000 lines. |
There was a problem hiding this comment.
Wonder if we should tweak this to call out all E2E type tests?
my only opinion right now is "punt as much as possible to follow-up, let's get something minimal working" 😅 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.devcontainer/docker-compose.yml (1)
2-3:⚠️ Potential issue | 🟠 MajorAdd readiness gating for
beads-doltto prevent startup failures.
devcontainerdepends onbeads-doltthrough environment variables (BEADS_DOLT_SERVER_HOST,BEADS_DOLT_HOST), but no healthcheck or dependency gate exists. This can cause intermittent failures whendevcontainerstarts beforebeads-doltis ready to accept connections.Suggested readiness fix
services: devcontainer: + depends_on: + beads-dolt: + condition: service_healthy @@ beads-dolt: @@ environment: - DOLT_ROOT_HOST=% - DOLT_DATABASE=beads_work + healthcheck: + test: ["CMD", "dolt", "sql", "-q", "SELECT 1;"] + interval: 5s + timeout: 3s + retries: 20🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/docker-compose.yml around lines 2 - 3, The devcontainer service currently can start before beads-dolt is ready causing intermittent failures; add a healthcheck to the beads-dolt service (command that verifies Dolt is accepting connections and sensible interval/retries) and update the devcontainer service to include depends_on: beads-dolt with condition: service_healthy (or the Compose v3+ equivalent wait-for-health approach) so devcontainer waits until beads-dolt is healthy before starting; reference the service names "beads-dolt" and "devcontainer" and ensure the healthcheck uses a deterministic probe (e.g., curl/tcp check or dolt status command) and appropriate timeout/retries.
♻️ Duplicate comments (4)
.claude/settings/permissions/bash.jsonc (2)
60-62: 🧹 Nitpick | 🔵 TrivialConsider adding
pnpm installanduv installto the ask list.A prior review raised concern about prompt injection leading to unauthorized dependency installation. Moving these to
askwould require confirmation before installing packages, providing an additional safeguard.🛡️ Proposed change
"ask": [ "Bash(gh *)", "Bash(aws *)", "Bash(curl *)", "Bash(ln *)", "WebFetch", + "Bash(pnpm install *)", + "Bash(pnpm add *)", + "Bash(uv add *)", + "Bash(uv pip install *)", ],Note:
uv syncanduv runare likely safe to keep in allow since they work with locked dependencies.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings/permissions/bash.jsonc around lines 60 - 62, Update the permissions so package installation commands require explicit confirmation: change the rule "Bash(pnpm *)" to restrict or move install-related patterns (e.g., "pnpm install", "pnpm i") into the ask list and similarly move "uv install" patterns from "Bash(uv *)" into ask; keep safe locked-dependency commands like "uv sync" and "uv run" in allow. Locate the entries "Bash(pnpm *)" and "Bash(uv *)" and adjust their patterns to only allow non-install safe commands while adding explicit ask rules for install actions.
105-105:⚠️ Potential issue | 🟡 MinorAdd
pnpm publishanduv publishto the deny list as previously requested.A prior review suggested adding these alongside
npm publishto prevent accidental or malicious package publishing. While local credentials may not typically exist, this provides defense-in-depth against lateral movement scenarios.🔧 Proposed fix
// Node.js "Bash(npm publish *)", + "Bash(pnpm publish *)", + // Python + "Bash(uv publish *)", // System Administration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings/permissions/bash.jsonc at line 105, The deny list currently contains "Bash(npm publish *)" but is missing the equivalent entries for pnpm and uv; add two new deny entries "Bash(pnpm publish *)" and "Bash(uv publish *)" alongside the existing "Bash(npm publish *)" entry in the permissions array so the policy blocks pnpm and uv publish commands as well.template/.devcontainer/docker-compose.yml.jinja-base (1)
28-34:⚠️ Potential issue | 🟠 Major
beads-doltstill lacks health/readiness gating in template output.This remains race-prone for first-start initialization paths.
#!/bin/bash # Verify readiness controls are absent in template compose rg -n 'depends_on|healthcheck|beads-dolt|BEADS_DOLT' template/.devcontainer/docker-compose.yml.jinja-base🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 28 - 34, The beads-dolt service in the compose template is missing a health/readiness probe which makes startup race-prone; add a healthcheck stanza to the beads-dolt service (e.g., using a simple dolt SQL ping or TCP check against the server port) and then update any dependent services to wait for beads-dolt to be healthy (use depends_on with service_healthy where supported or document adding a wait-for/entrypoint check). Target the beads-dolt service block (look for `beads-dolt`, `DOLT_ROOT_HOST`, `DOLT_DATABASE`) and insert a healthcheck and appropriate depends_on gating so other services only start after beads-dolt reports healthy..devcontainer/docker-compose.yml (1)
27-28:⚠️ Potential issue | 🟠 MajorRemote root exposure is still open (
DOLT_ROOT_HOST=%) without password.Line 27 keeps root reachable from any host pattern, and there is still no
DOLT_ROOT_PASSWORDin the service env. Please lock this down before merge.#!/bin/bash # Verify wildcard root host + missing password in compose files rg -n 'DOLT_ROOT_HOST|DOLT_ROOT_PASSWORD|beads-dolt' .devcontainer/docker-compose.yml template/.devcontainer/docker-compose.yml.jinja-baseSuggested hardening
beads-dolt: image: dolthub/dolt-sql-server:1.83.0 # no explicit reason for this version, just pinning for best practice @@ environment: - - DOLT_ROOT_HOST=% + - DOLT_ROOT_HOST=devcontainer + - DOLT_ROOT_PASSWORD=${DOLT_ROOT_PASSWORD} - DOLT_DATABASE=beads_work🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.devcontainer/docker-compose.yml around lines 27 - 28, The compose file currently exposes the Dolt root user to all hosts via the DOLT_ROOT_HOST=% env and has no DOLT_ROOT_PASSWORD set; change DOLT_ROOT_HOST from "%" to a restrictive host pattern (e.g., "127.0.0.1" or the internal service name) and add a DOLT_ROOT_PASSWORD environment variable (or reference a secret/compose .env variable) for the beads-dolt service so the root account is not accessible without a password; update the service's env block where DOLT_ROOT_HOST and DOLT_DATABASE are defined to use the new host value and add DOLT_ROOT_PASSWORD (or a secret reference) and ensure any template file (template/.devcontainer/docker-compose.yml.jinja-base) is updated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 1-8: The MD linter reports heading spacing and top-level heading
issues in AGENTS.md: convert the first heading to a top-level title (e.g.,
replace or add "# AGENTS") to satisfy MD041 and ensure there is a blank line
before and after all section headings (including "## Code Style" and the
headings at the other flagged locations) to satisfy MD022; update the file so
each heading is surrounded by a blank line and the document has one proper
top-level heading.
In `@template/.devcontainer/docker-compose.yml.jinja-base`:
- Around line 24-25: The template uses BEADS_DOLT_SERVER while concrete compose
uses BEADS_DOLT_HOST; pick one canonical env var and update the template entry
and any code consuming it to match. Replace BEADS_DOLT_SERVER with
BEADS_DOLT_HOST (or vice versa consistently) in the template lines that set the
env (the two lines containing BEADS_DOLT_SERVER and BEADS_DOLT_PORT) and search
for usages of BEADS_DOLT_SERVER/BEADS_DOLT_HOST in startup scripts,
docker-compose files, and code that reads env vars to ensure all references
(e.g., any docker env parsing or connection code) use the chosen key
consistently.
- Around line 33-34: Replace the open Dolt root configuration by adding an
explicit DOLT_ROOT_PASSWORD and tightening DOLT_ROOT_HOST: set DOLT_ROOT_HOST to
a restricted address or service name (e.g., 127.0.0.1 or the compose service
hostname) instead of '%' and add a new DOLT_ROOT_PASSWORD environment variable
with a generated strong password; update the template lines around the existing
DOLT_ROOT_HOST and DOLT_DATABASE entries (referencing the DOLT_ROOT_HOST and
DOLT_DATABASE symbols) to include DOLT_ROOT_PASSWORD and ensure any jinja
conditionals still render correctly.
---
Outside diff comments:
In @.devcontainer/docker-compose.yml:
- Around line 2-3: The devcontainer service currently can start before
beads-dolt is ready causing intermittent failures; add a healthcheck to the
beads-dolt service (command that verifies Dolt is accepting connections and
sensible interval/retries) and update the devcontainer service to include
depends_on: beads-dolt with condition: service_healthy (or the Compose v3+
equivalent wait-for-health approach) so devcontainer waits until beads-dolt is
healthy before starting; reference the service names "beads-dolt" and
"devcontainer" and ensure the healthcheck uses a deterministic probe (e.g.,
curl/tcp check or dolt status command) and appropriate timeout/retries.
---
Duplicate comments:
In @.claude/settings/permissions/bash.jsonc:
- Around line 60-62: Update the permissions so package installation commands
require explicit confirmation: change the rule "Bash(pnpm *)" to restrict or
move install-related patterns (e.g., "pnpm install", "pnpm i") into the ask list
and similarly move "uv install" patterns from "Bash(uv *)" into ask; keep safe
locked-dependency commands like "uv sync" and "uv run" in allow. Locate the
entries "Bash(pnpm *)" and "Bash(uv *)" and adjust their patterns to only allow
non-install safe commands while adding explicit ask rules for install actions.
- Line 105: The deny list currently contains "Bash(npm publish *)" but is
missing the equivalent entries for pnpm and uv; add two new deny entries
"Bash(pnpm publish *)" and "Bash(uv publish *)" alongside the existing "Bash(npm
publish *)" entry in the permissions array so the policy blocks pnpm and uv
publish commands as well.
In @.devcontainer/docker-compose.yml:
- Around line 27-28: The compose file currently exposes the Dolt root user to
all hosts via the DOLT_ROOT_HOST=% env and has no DOLT_ROOT_PASSWORD set; change
DOLT_ROOT_HOST from "%" to a restrictive host pattern (e.g., "127.0.0.1" or the
internal service name) and add a DOLT_ROOT_PASSWORD environment variable (or
reference a secret/compose .env variable) for the beads-dolt service so the root
account is not accessible without a password; update the service's env block
where DOLT_ROOT_HOST and DOLT_DATABASE are defined to use the new host value and
add DOLT_ROOT_PASSWORD (or a secret reference) and ensure any template file
(template/.devcontainer/docker-compose.yml.jinja-base) is updated consistently.
In `@template/.devcontainer/docker-compose.yml.jinja-base`:
- Around line 28-34: The beads-dolt service in the compose template is missing a
health/readiness probe which makes startup race-prone; add a healthcheck stanza
to the beads-dolt service (e.g., using a simple dolt SQL ping or TCP check
against the server port) and then update any dependent services to wait for
beads-dolt to be healthy (use depends_on with service_healthy where supported or
document adding a wait-for/entrypoint check). Target the beads-dolt service
block (look for `beads-dolt`, `DOLT_ROOT_HOST`, `DOLT_DATABASE`) and insert a
healthcheck and appropriate depends_on gating so other services only start after
beads-dolt reports healthy.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
.claude/settings/permissions/bash.jsonc.claude/settings/permissions/read.jsonc.devcontainer/devcontainer.json.devcontainer/docker-compose.yml.devcontainer/post-start-command.sh.pre-commit-config.yamlAGENTS.mdtemplate/.devcontainer/devcontainer.json.jinja-basetemplate/.devcontainer/docker-compose.yml.jinja-basetemplate/.devcontainer/post-start-command.sh.jinja-base
| ## Code Style | ||
| - Comments should be used very rarely. Code should generally express its intent. | ||
| - Never write a one-line docstring — either the name is sufficient or the behavior warrants a full explanation. | ||
| - Don't sort or remove imports manually — pre-commit handles it. | ||
| - Always include type hints for pyright in Python | ||
| - Respect the pyright rule reportUnusedCallResult; assign unneeded return values to `_` | ||
| - Prefer keyword-only parameters: use `*` in Python signatures and destructured options objects in TypeScript. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor markdown formatting issues flagged by linter.
Static analysis indicates headings at lines 1, 9, and 15 should be surrounded by blank lines (MD022), and line 1 should be a top-level heading (MD041). Consider adding a blank line before ## Code Style or converting it to # AGENTS as the document title.
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 1 - 8, The MD linter reports heading spacing and
top-level heading issues in AGENTS.md: convert the first heading to a top-level
title (e.g., replace or add "# AGENTS") to satisfy MD041 and ensure there is a
blank line before and after all section headings (including "## Code Style" and
the headings at the other flagged locations) to satisfy MD022; update the file
so each heading is surrounded by a blank line and the document has one proper
top-level heading.
| - BEADS_DOLT_SERVER=beads-dolt # for some weird reason, unless we specify both of these envvars it doesn't seem to reliably work | ||
| - BEADS_DOLT_PORT=3306 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Compare BEADS_DOLT key names across template and concrete compose files
rg -n 'BEADS_DOLT_(SERVER|HOST|SERVER_HOST|PORT)' .devcontainer/docker-compose.yml template/.devcontainer/docker-compose.yml.jinja-baseRepository: LabAutomationAndScreening/copier-base-template
Length of output: 1301
Align BEADS Dolt env key naming between template and concrete compose files.
Template uses BEADS_DOLT_SERVER (line 24), while concrete compose uses BEADS_DOLT_HOST (line 19). Pick one canonical key to prevent config drift and hard-to-debug startup differences.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 24 - 25,
The template uses BEADS_DOLT_SERVER while concrete compose uses BEADS_DOLT_HOST;
pick one canonical env var and update the template entry and any code consuming
it to match. Replace BEADS_DOLT_SERVER with BEADS_DOLT_HOST (or vice versa
consistently) in the template lines that set the env (the two lines containing
BEADS_DOLT_SERVER and BEADS_DOLT_PORT) and search for usages of
BEADS_DOLT_SERVER/BEADS_DOLT_HOST in startup scripts, docker-compose files, and
code that reads env vars to ensure all references (e.g., any docker env parsing
or connection code) use the chosen key consistently.
| - DOLT_ROOT_HOST=% | ||
| - DOLT_DATABASE=beads{% endraw %}{% endif %}{% raw %} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify wildcard host and missing password in template compose
rg -n 'DOLT_ROOT_HOST|DOLT_ROOT_PASSWORD' template/.devcontainer/docker-compose.yml.jinja-baseRepository: LabAutomationAndScreening/copier-base-template
Length of output: 117
🏁 Script executed:
#!/bin/bash
# Get the full context of the docker-compose template
cat -n template/.devcontainer/docker-compose.yml.jinja-base | head -60Repository: LabAutomationAndScreening/copier-base-template
Length of output: 2310
Add explicit root password and restrict Dolt host scope.
Line 33 configures DOLT_ROOT_HOST=% without a DOLT_ROOT_PASSWORD, allowing unauthenticated connections from any source. Even in a dev container, restrict the host to specific containers (e.g., 127.0.0.1) and add a strong root password via environment variable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 33 - 34,
Replace the open Dolt root configuration by adding an explicit
DOLT_ROOT_PASSWORD and tightening DOLT_ROOT_HOST: set DOLT_ROOT_HOST to a
restricted address or service name (e.g., 127.0.0.1 or the compose service
hostname) instead of '%' and add a new DOLT_ROOT_PASSWORD environment variable
with a generated strong password; update the template lines around the existing
DOLT_ROOT_HOST and DOLT_DATABASE entries (referencing the DOLT_ROOT_HOST and
DOLT_DATABASE symbols) to include DOLT_ROOT_PASSWORD and ensure any jinja
conditionals still render correctly.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings/permissions/bash.jsonc:
- Line 106: The deny list currently includes the entry "Bash(npm publish *)" but
is missing equivalents for pnpm and uv; add two additional entries "Bash(pnpm
publish *)" and "Bash(uv publish *)" to the same array alongside "Bash(npm
publish *)" so pnpm and uv publish commands are also denied as a
defense-in-depth measure.
- Around line 91-92: The deny-list entries "Bash(rm -rf / *)" and "Bash(rm -rf ~
*)" are too narrow; add explicit deny patterns to cover common dangerous
variants such as "Bash(rm -rf /)", "Bash(rm -rf /*)", "Bash(rm -rf ~)", and
"Bash(rm -rf ~/*)" so the permissions file blocks both forms with and without a
trailing path segment; update the array that contains "Bash(rm -rf / *)" and
"Bash(rm -rf ~ *)" to include these additional pattern strings.
- Around line 108-110: Update the deprecated colon-suffix pattern usage to the
consistent space-based pattern: replace the entries "Bash(doas:*)",
"Bash(passwd:*)", and "Bash(su:*)" with "Bash(doas *)", "Bash(passwd *)", and
"Bash(su *)" respectively so they match the current pattern syntax used
elsewhere in the rules; ensure you update the exact string literals in the
permissions array where those symbols appear.
| "Bash(rm -rf / *)", | ||
| "Bash(rm -rf ~ *)", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .claude/settings/permissions/bash.jsonc | sed -n '85,100p'Repository: LabAutomationAndScreening/copier-base-template
Length of output: 717
🌐 Web query:
Claude Bash permission pattern matching syntax glob
💡 Result:
Claude/Claude Code Bash permission rules are not “glob” patterns. They use prefix matching, with only one wildcard form: :* at the end of the rule to mean “anything after this prefix”. Patterns like * / ** in the Bash command body aren’t supported as globbing for permissions. [1], [2]
Bash permission syntax (prefix match)
- Exact command:
Bash(npm run build)[1] - Prefix allow (only at end):
Bash(npm run test:*)matches commands starting withnpm run test[1] - Example shown in settings:
Bash(git diff:*)[2] - Claude is aware of shell operators (e.g.,
&&), so a prefix rule likeBash(safe-cmd:*)doesn’t automatically allowsafe-cmd && other-cmd. [1]
Where “glob” patterns apply (not Bash)
Other tool permission rules (notably file/path tools like Read, Write, etc.) can use glob-style path patterns such as **/… in the tool argument, e.g. Read(./secrets/**) or Write(**/test.ts). [2], [3]
Sources: Anthropic Claude Code IAM (tool-specific Bash rules/limitations) [1]; Claude Code settings (permissions examples + note about Bash prefix matching) [2]; claude-code permission system summary (glob examples for non-Bash tools) [3].
Expand deny patterns to cover more dangerous rm -rf command forms.
The patterns "Bash(rm -rf / *)" and "Bash(rm -rf ~ *)" only match those specific command strings. They miss dangerous variants like rm -rf / or rm -rf /* that could cause data loss. Add explicit patterns for each variant:
Suggested broader coverage
// Destructive File Operations
"Bash(chmod -R *)",
"Bash(chown -R *)",
+ "Bash(rm -rf /)",
+ "Bash(rm -rf /*)",
+ "Bash(rm -rf / *)",
+ "Bash(rm -rf ~)",
+ "Bash(rm -rf ~/*)",
"Bash(rm -rf ~ *)",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Bash(rm -rf / *)", | |
| "Bash(rm -rf ~ *)", | |
| "Bash(rm -rf /)", | |
| "Bash(rm -rf /*)", | |
| "Bash(rm -rf / *)", | |
| "Bash(rm -rf ~)", | |
| "Bash(rm -rf ~/*)", | |
| "Bash(rm -rf ~ *)", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings/permissions/bash.jsonc around lines 91 - 92, The deny-list
entries "Bash(rm -rf / *)" and "Bash(rm -rf ~ *)" are too narrow; add explicit
deny patterns to cover common dangerous variants such as "Bash(rm -rf /)",
"Bash(rm -rf /*)", "Bash(rm -rf ~)", and "Bash(rm -rf ~/*)" so the permissions
file blocks both forms with and without a trailing path segment; update the
array that contains "Bash(rm -rf / *)" and "Bash(rm -rf ~ *)" to include these
additional pattern strings.
There was a problem hiding this comment.
@ejfine we should probably address this one and then we can 
afterwards.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
so what do you want, rm -rf*? does the r technically have to come before the f? I mean also, this is all in a devcontainer...so the "worst case scenario" of an rm-rf has an upper bound...we just rebuild the devcontainer and we've lost any changes since the last git commit, right?
There was a problem hiding this comment.
Yah 100% can be different orders and have other flags that get out and around the issue today 😬. I mean sure it "should" be in a devcontainer. I think we will atm have to just live with it.
| "Bash(git push -f *)", | ||
| "Bash(git push --force*)", | ||
| // Node.js | ||
| "Bash(npm publish *)", |
There was a problem hiding this comment.
Add pnpm publish and uv publish to the deny list.
Per the earlier discussion, pnpm publish and uv publish should be denied alongside npm publish as a defense-in-depth measure against potential credential exposure in future configurations.
Proposed fix
// Node.js
"Bash(npm publish *)",
+ "Bash(pnpm publish *)",
+ // Python
+ "Bash(uv publish *)",
// System Administration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings/permissions/bash.jsonc at line 106, The deny list currently
includes the entry "Bash(npm publish *)" but is missing equivalents for pnpm and
uv; add two additional entries "Bash(pnpm publish *)" and "Bash(uv publish *)"
to the same array alongside "Bash(npm publish *)" so pnpm and uv publish
commands are also denied as a defense-in-depth measure.
There was a problem hiding this comment.
♻️ Duplicate comments (2)
template/.devcontainer/docker-compose.yml.jinja-base (2)
2-4:⚠️ Potential issue | 🟠 MajorAdd readiness gating between
devcontainerandbeads-dolt.Line 28 introduces
beads-dolt, but there is no healthcheck and nodepends_onhealth gate fordevcontainer(Line 2), which can cause intermittent startup failures.Proposed fix
services: devcontainer: +{% if install_claude_cli %} + depends_on: + beads-dolt: + condition: service_healthy +{% endif %} @@ beads-dolt: image: dolthub/dolt-sql-server:1.83.0 # no explicit reason for this version, just pinning for best practice @@ environment: - DOLT_ROOT_HOST=% - DOLT_DATABASE=beads_work + healthcheck: + test: ["CMD-SHELL", "dolt sql -q 'SELECT 1' >/dev/null 2>&1"] + interval: 5s + timeout: 3s + retries: 20#!/bin/bash # Verify readiness-gating primitives exist in the template. rg -n 'beads-dolt|depends_on|service_healthy|healthcheck' template/.devcontainer/docker-compose.yml.jinja-baseAlso applies to: 28-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 2 - 4, The devcontainer service lacks a readiness gate for beads-dolt; add a healthcheck block to the beads-dolt service (define test, interval, timeout, retries) and update the devcontainer service to include depends_on with beads-dolt using condition: service_healthy so devcontainer waits for beads-dolt to be healthy before starting; look for the service names "devcontainer" and "beads-dolt" and the fields "platform", "healthcheck", and "depends_on" to insert these changes.
33-34:⚠️ Potential issue | 🟠 MajorHarden Dolt root access (missing authentication).
Line 33 allows broad root host access and there is no root password configured. Even for devcontainers, unauthenticated root DB access is a risky default.
Proposed hardening
environment: - - DOLT_ROOT_HOST=% + - DOLT_ROOT_HOST=% + - DOLT_ROOT_PASSWORD=${BEADS_DOLT_ROOT_PASSWORD:?BEADS_DOLT_ROOT_PASSWORD is required} - DOLT_DATABASE=beads_workWhat is the recommended secure docker-compose configuration for dolthub/dolt-sql-server:1.83.0 regarding root authentication and host scoping (DOLT_ROOT_HOST / DOLT_ROOT_PASSWORD)?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 33 - 34, The docker-compose template currently sets DOLT_ROOT_HOST=% (wildcard) and no root password, which allows unauthenticated root DB access; change the DOLT_ROOT_HOST env to a scoped host (e.g., 127.0.0.1 or a specific container/network alias) instead of '%' and add a DOLT_ROOT_PASSWORD environment variable (populated from a secret or .env, not hard-coded) so the dolthub/dolt-sql-server container requires authentication; update the template entries that reference DOLT_ROOT_HOST and add DOLT_ROOT_PASSWORD (and document using devcontainer secrets or an env file) and ensure any orchestration that connects to the DB uses the new password.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@template/.devcontainer/docker-compose.yml.jinja-base`:
- Around line 2-4: The devcontainer service lacks a readiness gate for
beads-dolt; add a healthcheck block to the beads-dolt service (define test,
interval, timeout, retries) and update the devcontainer service to include
depends_on with beads-dolt using condition: service_healthy so devcontainer
waits for beads-dolt to be healthy before starting; look for the service names
"devcontainer" and "beads-dolt" and the fields "platform", "healthcheck", and
"depends_on" to insert these changes.
- Around line 33-34: The docker-compose template currently sets DOLT_ROOT_HOST=%
(wildcard) and no root password, which allows unauthenticated root DB access;
change the DOLT_ROOT_HOST env to a scoped host (e.g., 127.0.0.1 or a specific
container/network alias) instead of '%' and add a DOLT_ROOT_PASSWORD environment
variable (populated from a secret or .env, not hard-coded) so the
dolthub/dolt-sql-server container requires authentication; update the template
entries that reference DOLT_ROOT_HOST and add DOLT_ROOT_PASSWORD (and document
using devcontainer secrets or an env file) and ensure any orchestration that
connects to the DB uses the new password.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
.claude/.beads/config.yaml (1)
11-14:⚠️ Potential issue | 🟡 MinorRemove or deprecate the JSONL/no-db guidance to prevent invalid setup paths.
Line 11-Line 14 describe JSONL-only source-of-truth mode, but this PR’s objective explicitly says beads no longer supports JSONL-only persistence. Keeping this block risks misconfiguration and confusion.
Proposed doc-only fix
-# Use no-db mode: load from JSONL, write back after each command -# When true, bd will use .beads/issues.jsonl as the source of truth -# instead of the Dolt database -# no-db: false +# NOTE: JSONL-only persistence is no longer supported in current beads versions. +# This repository uses Dolt-backed persistence.In the current beads config format (including v0.57+), is `no-db` / JSONL-only persistence still supported? If not, what exact replacement guidance should be documented in config.yaml comments?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/.beads/config.yaml around lines 11 - 14, Remove the obsolete JSONL/no-db guidance block that references the no-db key and .beads/issues.jsonl (the commented lines around "no-db: false"), and replace it with a short deprecation note stating that JSONL-only persistence is no longer supported in beads v0.57+ and users must use Dolt-backed persistence; include a pointer to the correct setup docs or migration steps for converting existing .beads/issues.jsonl data into the Dolt database..claude/settings/permissions/bash.jsonc (2)
106-106:⚠️ Potential issue | 🟠 MajorAdd deny parity for
pnpm publishanduv publish.Only
npm publishis denied right now; publishing through other package managers is still open.Suggested patch
// Node.js "Bash(npm publish *)", + "Bash(pnpm publish *)", + // Python + "Bash(uv publish *)", // System Administration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings/permissions/bash.jsonc at line 106, The permission list currently only denies "Bash(npm publish *)" which leaves other package managers open; add equivalent deny entries for pnpm and uv by inserting new permission strings matching the existing pattern such as "Bash(pnpm publish *)" and "Bash(uv publish *)" alongside "Bash(npm publish *)" so the rules in the permissions array block (the entry with "Bash(npm publish *)") explicitly block those publish commands as well.
91-92:⚠️ Potential issue | 🟠 MajorExpand destructive
rm -rfdenies to cover no-operand variants.These deny rules still miss common dangerous forms like
rm -rf /andrm -rf ~, which are separate command strings from the current entries.Suggested patch
"Bash(chmod -R *)", "Bash(chown -R *)", + "Bash(rm -rf /)", + "Bash(rm -rf /*)", "Bash(rm -rf / *)", + "Bash(rm -rf ~)", + "Bash(rm -rf ~/*)", "Bash(rm -rf ~ *)",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/settings/permissions/bash.jsonc around lines 91 - 92, Add explicit deny entries for the no-operand variants of the destructive commands so the rules catch bare targets; specifically add "Bash(rm -rf /)" and "Bash(rm -rf ~)" alongside the existing "Bash(rm -rf / *)" and "Bash(rm -rf ~ *)" entries so both forms (with and without extra args) are denied.template/.devcontainer/docker-compose.yml.jinja-base (2)
25-26: 🛠️ Refactor suggestion | 🟠 MajorUnify BEADS host env naming across template and concrete compose files.
Line 25 uses
BEADS_DOLT_SERVER, while.devcontainer/docker-compose.ymlusesBEADS_DOLT_HOST. Keep one canonical key to prevent template/runtime drift.🔧 Proposed fix
- - BEADS_DOLT_SERVER=beads-dolt # for some weird reason, unless we specify both of these envvars it doesn't seem to reliably work + - BEADS_DOLT_HOST=beads-dolt # keep key consistent with .devcontainer/docker-compose.yml - BEADS_DOLT_PORT=3306🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 25 - 26, The template uses BEADS_DOLT_SERVER while concrete compose files expect BEADS_DOLT_HOST; unify them by replacing BEADS_DOLT_SERVER with BEADS_DOLT_HOST in the template (.devcontainer/docker-compose.yml.jinja-base) and ensure all references (env keys, docker-compose templates, and any lookup code that reads BEADS_DOLT_*) use the canonical BEADS_DOLT_HOST name so runtime and template keys match.
34-35:⚠️ Potential issue | 🟠 MajorRestrict Dolt root access and require a password.
Line 34 sets
DOLT_ROOT_HOST=%and Line 35 has noDOLT_ROOT_PASSWORD, which leaves root broadly reachable on the compose network.🔧 Proposed fix
environment: - - DOLT_ROOT_HOST=% + - DOLT_ROOT_HOST=devcontainer + - DOLT_ROOT_PASSWORD=${DOLT_ROOT_PASSWORD} - DOLT_DATABASE=beads_workFor the `dolthub/dolt-sql-server` Docker image, what are the security implications of setting `DOLT_ROOT_HOST=%` without `DOLT_ROOT_PASSWORD`?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/docker-compose.yml.jinja-base` around lines 34 - 35, Current compose template sets DOLT_ROOT_HOST=% and omits DOLT_ROOT_PASSWORD, leaving root accessible across the network; update the environment block to restrict host and require a password by replacing DOLT_ROOT_HOST=% with a more restrictive host (e.g., 127.0.0.1 or remove the variable) and add a DOLT_ROOT_PASSWORD env var (use a generated secret or require injection via .env/Jinja variable) so the template always supplies DOLT_ROOT_PASSWORD when enabling Dolt; ensure the changes reference the DOLT_ROOT_HOST and DOLT_ROOT_PASSWORD environment variables in the same template/Jinja block that defines DOLT_DATABASE.template/.devcontainer/post-start-command.sh.jinja-base (1)
8-13:⚠️ Potential issue | 🟠 Major
bd readyis not a safe guard for destructive re-initialization.Line 8 treats any
bd readyfailure as “needs init”, but Line 13 then deletes.claude/.beads. This can wipe state on transient CLI/server errors.🔧 Proposed fix
-if ! bd ready; then +if ! bd doctor >/dev/null 2>&1; then echo "It's likely the Dolt server has not yet been initialized to support beads, running that now" - rm -rf .claude/.beads && bd init --server-host="$BEADS_DOLT_SERVER_HOST" --database="$BEADS_DOLT_SERVER_DATABASE" --skip-hooks --stealth --prefix=work </dev/null && git -c core.hooksPath=/dev/null restore --source=HEAD --staged --worktree .claude/.beads + rm -rf .claude/.beads + if ! bd init --server-host="$BEADS_DOLT_SERVER_HOST" --database="$BEADS_DOLT_SERVER_DATABASE" --skip-hooks --stealth --prefix=work </dev/null; then + echo "bd init failed for host '$BEADS_DOLT_SERVER_HOST' (check Dolt server reachability)." + exit 1 + fi + git -c core.hooksPath=/dev/null restore --source=HEAD --staged --worktree .claude/.beads fiIn the `@beads/bd` CLI documentation, is `bd ready` intended to indicate initialization/readiness state, or should `bd doctor` (or directory existence) be used for that check?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@template/.devcontainer/post-start-command.sh.jinja-base` around lines 8 - 13, The script currently treats any failure of bd ready as a signal to delete .claude/.beads and re-init (rm -rf .claude/.beads, bd init), which is unsafe; change the check to a non-destructive validation (e.g., use bd doctor or test for the existence/expected contents of .claude/.beads) and only perform rm -rf/.reinitialize when that validation explicitly indicates the beads directory is corrupt or missing; keep bd ready/bd init calls but gate the destructive rm -rf .claude/.beads behind a concrete check (bd doctor exit code or a directory/file-existence check) or an explicit FORCE_INIT flag so transient bd ready errors won’t wipe state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/.beads/config.yaml:
- Line 44: Move the long inline rationale out of the key/value line so the
setting stays tidy: keep the line "backup.enabled: false" by itself and add one
or more short comment lines immediately above it containing the version-specific
note (e.g., "# v0.57: backup auto-stages files into git; disabling for now" and
any other operational notes). Update the comment style to be brief and precede
the key so the config parser sees only the simple "backup.enabled: false" line
while the explanatory text remains in preceding comment lines.
In @.claude/settings/permissions/bash.jsonc:
- Around line 99-100: The permissions list allows Bash(kill *) while denying
only Bash(kill -9 *), which leaves equivalent SIGKILL forms bypassable; update
the deny rules to cover all SIGKILL syntaxes by adding deny entries for
Bash(kill -s KILL *), Bash(kill -SIGKILL *), Bash(kill --signal KILL *), and a
broader pattern like Bash(kill * -s KILL *) or use a single deny pattern that
matches any kill invocation with signal name or numeric 9 (e.g. a
wildcard/regex-based deny for "Bash(kill * ( -9 | -s KILL | -SIGKILL | --signal
KILL )*)") so that functions/entries referencing Bash(kill *), Bash(kill -9 *),
and Bash(killall *) cannot bypass SIGKILL denial via alternate flag forms.
---
Duplicate comments:
In @.claude/.beads/config.yaml:
- Around line 11-14: Remove the obsolete JSONL/no-db guidance block that
references the no-db key and .beads/issues.jsonl (the commented lines around
"no-db: false"), and replace it with a short deprecation note stating that
JSONL-only persistence is no longer supported in beads v0.57+ and users must use
Dolt-backed persistence; include a pointer to the correct setup docs or
migration steps for converting existing .beads/issues.jsonl data into the Dolt
database.
In @.claude/settings/permissions/bash.jsonc:
- Line 106: The permission list currently only denies "Bash(npm publish *)"
which leaves other package managers open; add equivalent deny entries for pnpm
and uv by inserting new permission strings matching the existing pattern such as
"Bash(pnpm publish *)" and "Bash(uv publish *)" alongside "Bash(npm publish *)"
so the rules in the permissions array block (the entry with "Bash(npm publish
*)") explicitly block those publish commands as well.
- Around line 91-92: Add explicit deny entries for the no-operand variants of
the destructive commands so the rules catch bare targets; specifically add
"Bash(rm -rf /)" and "Bash(rm -rf ~)" alongside the existing "Bash(rm -rf / *)"
and "Bash(rm -rf ~ *)" entries so both forms (with and without extra args) are
denied.
In `@template/.devcontainer/docker-compose.yml.jinja-base`:
- Around line 25-26: The template uses BEADS_DOLT_SERVER while concrete compose
files expect BEADS_DOLT_HOST; unify them by replacing BEADS_DOLT_SERVER with
BEADS_DOLT_HOST in the template (.devcontainer/docker-compose.yml.jinja-base)
and ensure all references (env keys, docker-compose templates, and any lookup
code that reads BEADS_DOLT_*) use the canonical BEADS_DOLT_HOST name so runtime
and template keys match.
- Around line 34-35: Current compose template sets DOLT_ROOT_HOST=% and omits
DOLT_ROOT_PASSWORD, leaving root accessible across the network; update the
environment block to restrict host and require a password by replacing
DOLT_ROOT_HOST=% with a more restrictive host (e.g., 127.0.0.1 or remove the
variable) and add a DOLT_ROOT_PASSWORD env var (use a generated secret or
require injection via .env/Jinja variable) so the template always supplies
DOLT_ROOT_PASSWORD when enabling Dolt; ensure the changes reference the
DOLT_ROOT_HOST and DOLT_ROOT_PASSWORD environment variables in the same
template/Jinja block that defines DOLT_DATABASE.
In `@template/.devcontainer/post-start-command.sh.jinja-base`:
- Around line 8-13: The script currently treats any failure of bd ready as a
signal to delete .claude/.beads and re-init (rm -rf .claude/.beads, bd init),
which is unsafe; change the check to a non-destructive validation (e.g., use bd
doctor or test for the existence/expected contents of .claude/.beads) and only
perform rm -rf/.reinitialize when that validation explicitly indicates the beads
directory is corrupt or missing; keep bd ready/bd init calls but gate the
destructive rm -rf .claude/.beads behind a concrete check (bd doctor exit code
or a directory/file-existence check) or an explicit FORCE_INIT flag so transient
bd ready errors won’t wipe state.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
.claude/.beads/config.yaml.claude/settings/permissions/bash.jsonc.devcontainer/devcontainer.json.devcontainer/docker-compose.yml.devcontainer/post-start-command.shtemplate/.devcontainer/docker-compose.yml.jinja-basetemplate/.devcontainer/post-start-command.sh.jinja-base
| # - github.org | ||
| # - github.repo | ||
| no-git-ops: true | ||
| backup.enabled: false # in v0.57 the backup seems to automatically add stage the files into git, bypassing the .gitignore I tried to set up, so disabling for now (also we don't really need backup anyway if we're not pushing into git, but this would be nicer long term than having to run the export command manually all the time) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Move the version-specific rationale to preceding comment lines for readability.
Line 44 packs a long operational note inline with the value; keeping backup.enabled: false on its own line with short preceding comments is easier to maintain.
Suggested cleanup
-backup.enabled: false # in v0.57 the backup seems to automatically add stage the files into git, bypassing the .gitignore I tried to set up, so disabling for now (also we don't really need backup anyway if we're not pushing into git, but this would be nicer long term than having to run the export command manually all the time)
+# Temporary workaround for beads v0.57 behavior that stages backup artifacts in git.
+# Revisit after upgrading beads and confirming backup/export behavior.
+backup.enabled: false📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| backup.enabled: false # in v0.57 the backup seems to automatically add stage the files into git, bypassing the .gitignore I tried to set up, so disabling for now (also we don't really need backup anyway if we're not pushing into git, but this would be nicer long term than having to run the export command manually all the time) | |
| # Temporary workaround for beads v0.57 behavior that stages backup artifacts in git. | |
| # Revisit after upgrading beads and confirming backup/export behavior. | |
| backup.enabled: false |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/.beads/config.yaml at line 44, Move the long inline rationale out of
the key/value line so the setting stays tidy: keep the line "backup.enabled:
false" by itself and add one or more short comment lines immediately above it
containing the version-specific note (e.g., "# v0.57: backup auto-stages files
into git; disabling for now" and any other operational notes). Update the
comment style to be brief and precede the key so the config parser sees only the
simple "backup.enabled: false" line while the explanatory text remains in
preceding comment lines.
| "Bash(kill -9 *)", | ||
| "Bash(killall *)", |
There was a problem hiding this comment.
kill protections are bypassable via non--9 SIGKILL forms.
allow permits Bash(kill *), but deny blocks only kill -9 *. Equivalent forms like kill -s KILL/kill -SIGKILL are currently not denied.
Suggested patch
// Process Management
"Bash(kill -9 *)",
+ "Bash(kill -s KILL *)",
+ "Bash(kill -SIGKILL *)",
"Bash(killall *)",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "Bash(kill -9 *)", | |
| "Bash(killall *)", | |
| "Bash(kill -9 *)", | |
| "Bash(kill -s KILL *)", | |
| "Bash(kill -SIGKILL *)", | |
| "Bash(killall *)", |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.claude/settings/permissions/bash.jsonc around lines 99 - 100, The
permissions list allows Bash(kill *) while denying only Bash(kill -9 *), which
leaves equivalent SIGKILL forms bypassable; update the deny rules to cover all
SIGKILL syntaxes by adding deny entries for Bash(kill -s KILL *), Bash(kill
-SIGKILL *), Bash(kill --signal KILL *), and a broader pattern like Bash(kill *
-s KILL *) or use a single deny pattern that matches any kill invocation with
signal name or numeric 9 (e.g. a wildcard/regex-based deny for "Bash(kill * ( -9
| -s KILL | -SIGKILL | --signal KILL )*)") so that functions/entries referencing
Bash(kill *), Bash(kill -9 *), and Bash(killall *) cannot bypass SIGKILL denial
via alternate flag forms.
2 participants
Why is this change necessary?
Make better use of Claude Code
How does this change address the issue?
Makes use of the beads library of AI agent issue tracking. just locally for now for someone to use while working on a Jira/github issue. not using beads as a distributed source of truth. This necessitated setting up a dolt server as a sidecar container since beads no longer supports using solely JSONL files as the persistence mechanism.
Brings in a subset of the commands from the claude-instructions library, focusing on TDD
Since Claude does not support comments in its settings.json file, using an existing repo's approach to creating settings in separate JSONC files, and then running a pre-commit script to merge them together
Adds github copilot instructions (symlinked to AGENTS.md)
What side effects does this change have?
N/A
How is this change tested?
building devcontainers and playing around with the claude commands
Other
Bumped some versions.
Added tests for some of the copier hook scripts
At the moment, the grandchild repos will always include the
.claudefolder. I have some ideas about handling that so theinstall_cli_toolingjinja arg will exclude that folder if false, but wanted to get this rolling firstSummary by CodeRabbit
New Features
Chores
Documentation
Tests