Review: fix: make bd setup claude project-local by default and auto-setup in bd init

Verdict: Correct behavioral change with clean polarity inversion. The settings.json vs settings.local.json distinction aligns with Claude Code's team-vs-personal semantics. Legacy migration is safe. One notable test coverage gap for the migration paths.

4 files, +126 −32. 1 commit. Author: maphew (co-authored with Amp).

Core change: Default scope inversion ✅

Old: bd setup claude → global (~/.claude/settings.json), --project → project (settings.local.json)
New: bd setup claude → project (.claude/settings.json), --global → global (~/.claude/settings.json)

The boolean parameter flips from project bool to global bool throughout the call chain. Every call site correctly updated:

Settings file semantics: Exactly right ✅

The move from settings.local.json → settings.json for the project default is semantically correct per Claude Code's design:

• .claude/settings.json = shared/team (committed to repo, all collaborators get the hooks)
• .claude/settings.local.json = personal (git-ignored, per-developer overrides)

bd prime hooks should be shared with the team. Writing to the committed file is the correct default.

bd init auto-setup ✅

Placement is right: after AGENTS.md generation, before git auto-stage. Guards are correct:

• !stealth — stealth mode wants invisible setup
• !skipAgents — if agents are skipped, Claude hooks are irrelevant
• !isBareGitRepo() — bare repos have no working tree for .claude/
• Error is non-fatal (warning only) — init continues even if Claude setup fails

Git staging of .claude/settings.json and CLAUDE.md in the auto-commit block is correct.

InstallClaudeProject export ✅

Clean thin wrapper: resolves claudeEnv, calls installClaude(env, false, stealth), returns error instead of calling os.Exit. This is the right pattern for use from bd init where failures should be non-fatal.

Legacy migration: Safe and thorough ✅

Install path: After writing to settings.json, if !global, checks settings.local.json for beads hooks → removes them → writes back → prints migration message. Preserves non-beads hooks in the legacy file. Does not delete the file.

Remove path: After cleaning settings.json, if !global, also cleans settings.local.json. Correct: bd setup claude --remove cleans both locations.

Check path: Priority order is settings.json → global → legacy, with migration hint for legacy-only installs. Correct.

Edge case: both files have hooks → install writes to new, removes from legacy. After migration, hooks only in settings.json. Clean.

Edge case: legacy file has non-beads hooks → only bd prime variants removed, other hooks preserved. Safe.

Edge case: legacy file is malformed JSON → json.Unmarshal fails silently, migration skipped. Correct — don't break install because a legacy file is corrupted.

🟡 --project flag becomes a silent no-op for Claude

The --project flag still exists (used by gemini/mux) but the Claude recipe now reads setupGlobal instead of setupProject. Running bd setup claude --project does the same thing as bd setup claude — project-local install, since that's the default. No error, no warning. The flag help text correctly removes claude from --project's description.

This is technically correct (you get what you asked for — a project install) but could surprise users who relied on --project semantically. Acceptable given --project was previously the non-default path; now it is the default.

🟡 No dedicated tests for legacy migration

The new migration code (install, remove, and check paths) has no dedicated test coverage:

• No test for install with existing settings.local.json hooks → verifying migration message and hook removal from legacy
• No test for remove with existing settings.local.json hooks → verifying both files cleaned
• No test for check with only legacy hooks → verifying migration hint output

The migration logic is straightforward (reuses existing removeHookCommand + hasBeadsHooks), so the risk is low, but given this is the primary upgrade path for existing users, a TestLegacyMigration table-driven test would add confidence.

🟢 Minor: hasBeadsHooks uses os.ReadFile directly

Pre-existing pattern, not introduced by this PR, but worth noting: hasBeadsHooks uses os.ReadFile while the migration block uses env.readFile. Works in tests because they use real temp directories, but creates a subtle inconsistency in the testability model.

Summary

Ship it. Migration test coverage can come in a follow-up.