v0.1 W2: ClaudeCodeSurface install/uninstall + settings.json merge

[liam-ai-reality]

Summary

• klasp-agents-claude now ships the real ClaudeCodeSurface impl per docs/design.md §3.1, §5, §7: hook-script template, surgical settings.json merge, idempotent install/uninstall.
• klasp install and klasp uninstall are wired through a new SurfaceRegistry in the binary crate.
• 32 new tests (19 unit + 3 fallow-fixture integration + 8 install integration + 2 insta snapshots).

Closes

Closes #2.

What this delivers (issue #2 checklist)

• klasp-agents-claude crate skeleton; depends only on klasp-core (plus serde_json/tempfile/thiserror as documented in design §12)
• klasp-agents-claude/src/hook_template.rs — # klasp:managed marker + KLASP_GATE_SCHEMA=N export + exec klasp gate "$@"
• klasp-agents-claude/src/settings.rs — surgical JSON merge preserving every sibling key, idempotent on exact command match
• klasp-agents-claude/src/surface.rs — ClaudeCodeSurface impl of AgentSurface with MarkerConflict unless --force
• klasp/src/cmd/install.rs + klasp/src/cmd/uninstall.rs wired through SurfaceRegistry
• Settings.json merge unit tests including a real fallow-shaped fixture proving sibling hooks survive byte-for-byte
• insta snapshot of the rendered hook script (v1 + v7 to exercise schema-version interpolation)

Decisions worth flagging for review

1. Hook entry command — ${CLAUDE_PROJECT_DIR}/.claude/hooks/klasp-gate.sh (envar-resolved). Design didn't pin this; documented in klasp-agents-claude/src/surface.rs.
2. SurfaceRegistry location — lives in the klasp binary crate, not in klasp-core, to keep core free of per-agent crate deps. Design §5's SurfaceRegistry::default() example is binary-side.
3. Empty-matcher cleanup on uninstall — when our removal leaves a Bash matcher with an empty hooks array, we drop the matcher entry too (so install→uninstall round-trips a fresh repo to {}). Untouched non-Bash matchers and Bash matchers that started with sibling hooks are preserved.
4. Atomic writes — tempfile::NamedTempFile in the same dir + persist. Avoids torn writes on settings.json (the highest-risk file).
5. Windows path/permission — chmod 0o755 is unix-only; no-op on Windows per docs/design.md §14 3rd bullet (audited at W4).

Smoke test (manual)

$ cd /tmp/smoke && git init -q && mkdir .claude
$ klasp install
claude_code: installed
  wrote /private/tmp/smoke/.claude/hooks/klasp-gate.sh
  wrote /private/tmp/smoke/.claude/settings.json
$ klasp install
claude_code: already installed (no changes)
$ klasp uninstall
claude_code: removed
  /private/tmp/smoke/.claude/hooks/klasp-gate.sh
  /private/tmp/smoke/.claude/settings.json
$ cat .claude/settings.json
{}

Test plan

• cargo build --workspace (verified locally)
• cargo fmt --all -- --check (verified locally)
• cargo clippy --all-targets --workspace -- -D warnings (verified locally)
• cargo test --workspace — 67 passing (was 35 before this PR), 1 ignored (W1's documented git-dash-c case)
• CI green on cargo check + fmt + clippy, build / macos-14, build / ubuntu-latest, build / windows-latest, npm publish --dry-run, pypi build + twine check
• Run /code-review:code-review per the agentic-flow plan — settings.json merge is the highest-risk module of v0.1 (design §5 + §14)

Out of scope

• W3 klasp gate runtime (the hook script execs into klasp gate, which still prints "not yet implemented" — that's the next issue, [v0.1 W3] klasp gate runtime + trigger + Shell CheckSource #3).
• Settings.json key-order roundtrip preservation (design §14 4th bullet) — serde_json::Value normalises key order; the merge tests verify content but not byte-for-byte ordering. File a follow-up if review surfaces real-world drift.
• Windows chmod audit (design §14 3rd bullet, scheduled for W4).

🤖 Generated with Claude Code

[liam-ai-reality]

Code review

Ran /code-review:code-review on commit 217c35f (the W2 implementation, before the simplify pass).

Summary

Implements W2 of klasp v0.1 per docs/design.md §3.1, §5, §7. CLAUDE.md compliance was checked against the parent ~/Projects/CLAUDE.md (no in-repo CLAUDE.md exists). Diff-only and depth-pass bug agents ran in parallel.

Critical issues

#	File	Line	Issue
—	—	—	None

Suggestions

#	File	Issue	Validation
1	klasp-agents-claude/src/surface.rs (atomic_write)	Uses tempfile::NamedTempFile::new_in (mode 0o600) and persist() does a rename — destination inherits 0o600. Hook script is recovered to 0o755 via set_executable, but settings.json has no equivalent: each install/uninstall silently downgrades .claude/settings.json to 0o600.	VALIDATED — high confidence on mechanism, medium on user-visible severity
2	klasp-agents-claude/src/settings.rs (unmerge_hook_entry retain block + doc-comments)	Drops any matcher: "Bash" entry whose hooks: [] is empty after klasp's removal. The inline comment claimed user-owned empty placeholders are preserved, but the code can't distinguish "klasp emptied this" from "user pre-installed an empty placeholder" — it deletes unconditionally. The function doc-comment promising round-trip preservation is also imprecise.	VALIDATED — niche failure (a literal {matcher: "Bash", hooks: []} placeholder is unusual to hand-author), but the comment-vs-code mismatch is real

What looks good

• All new files comfortably under the 500-line workspace rule (largest is settings.rs at 467).
• Settings merge edge cases well covered: empty input, sibling matchers preserved, malformed JSON, shape errors, fallow fixture round-trip, idempotency.
• set_executable correctly gated on #[cfg(unix)] per docs/design.md §14.
• atomic_write uses NamedTempFile::new_in in the destination directory — same-FS rename, no cross-FS-rename failure mode.
• is_none_or MSRV regression caught and replaced with map_or(true, ...) — correct for rust-version = "1.75".

Verdict

Approve with minor notes. Both suggestions are scoped follow-ups, not blockers. Both fixed in 06f448b — see remediation comment below.

🤖 Generated with Claude Code

[liam-ai-reality]

Review remediation + simplify pass

Both review findings fixed in 06f448b, plus a /simplify sweep on the same commit. CI green (6/6 checks).

Review-handoff fixes

Finding	Fix
atomic_write silently downgrades settings.json to 0o600	atomic_write now takes an explicit mode: u32 parameter; install/uninstall capture the file's prior Unix mode via current_mode() and restore it after persist(). Fresh files default to 0o644; the hook script continues to land at 0o755. Three new tests in klasp/tests/install_claude_code.rs pin the mode contract.
unmerge_hook_entry drops user-owned empty Bash matchers; comments imprecise	Tightened the function doc-comment to admit the round-trip caveat (provenance of an empty placeholder is unrecoverable). The corresponding inline comment now points at the doc instead of restating the rule.

/simplify pass (Step 09)

Three review agents (reuse / quality / efficiency) ran in parallel against the diff. Genuine wins applied:

• atomic_write takes mode: u32 instead of Option<u32> — the None branch was dead code.
• get_or_insert_object / get_or_insert_array signatures harmonised to (map, key, path) — eliminates the asymmetric "path derived from key" rule.
• JSON-schema keys (hooks, PreToolUse, matcher, Bash, type, command) are now module-level consts in settings.rs. A typo now becomes a compile error instead of a silent no-op.
• Dropped a handful of comments that restated obvious behaviour (HookState variant docs, narrating-the-change comments in tests, POSIX rationale on a single-line newline assertion, task-reference in the fallow-fixture test header).

Skipped: the TOCTOU note on exists() + read_to_string (efficiency agent's own assessment was "not a real problem at CLI granularity") and the meta-commentary about why the expect_/get_or_insert pairs aren't unifiable (over-explanation of absent abstraction).

Verification

cargo fmt --check + cargo clippy --all-targets --workspace -- -D warnings + cargo test --workspace — all green. 70 passing tests, 1 ignored (W1's documented git -c x=y commit regex case).

🤖 Generated with Claude Code