Skip to content

fix: doctor false-positive on managed-block hooks, setup output polish#110

Merged
liam-ai-reality merged 2 commits into
mainfrom
fix/polish-102-105-106
May 10, 2026
Merged

fix: doctor false-positive on managed-block hooks, setup output polish#110
liam-ai-reality merged 2 commits into
mainfrom
fix/polish-102-105-106

Conversation

@liam-ai-reality
Copy link
Copy Markdown
Contributor

Summary

  • doctor false-positive 'schema drift' on codex hook with managed-block append #102 — doctor false-positive on Codex hook with user content above block: check_hook previously compared the full on-disk hook file byte-for-byte against render_hook_script, which always returns a fresh-install layout (shebang + blank + managed block). When a user already had content in .git/hooks/pre-commit, install correctly appended the managed block after that content — but doctor then flagged schema drift because the full files don't match. Fix: extract only the managed block portion from both actual and expected (by scanning for MANAGED_START/MANAGED_END markers) and compare those. Byte-equality is still tried first for surfaces without managed blocks (Claude Code).
  • klasp setup --dry-run banner appears on line 2 instead of line 1 #106 — dry-run banner appeared after the gate-count line: moved --dry-run mode println before the klasp setup — detected N gate(s) line so users immediately know no I/O will occur.
  • klasp setup prints stale "Next:" instructions for steps it already ran #105 — stale "Next: klasp init" footer in setup output: render_plan was called in setup, which appends a "Next:" footer suggesting klasp init — but setup already ran init. Switched to render_plan_no_next (added in this workspace) for the setup code path.

Test plan

  • cargo test --package klasp -- doctor_codex_hook_with_user_content_above_block_exits_0 — previously failed, now passes
  • cargo test --package klasp — 501 passed, 0 failed
  • cargo test --package klasp --test setup — 12 passed
  • cargo test --package klasp --test doctor — all doctor tests pass

🤖 Generated with Claude Code

#102, #105, #106)

- doctor(#102): compare only the managed block portion of on-disk hooks
  instead of the full file; user content above/below the klasp block no
  longer triggers a spurious schema-drift FAIL for Codex installs
- setup(#106): emit --dry-run banner before gate-count line so it's clear
  no I/O will happen before any output is printed
- setup(#105): call render_plan_no_next in setup so the "Next: klasp init"
  footer doesn't appear after setup already ran init

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@liam-ai-reality
Copy link
Copy Markdown
Contributor Author

Code review

No issues found. Checked for bugs and CLAUDE.md compliance.

@liam-ai-reality
Copy link
Copy Markdown
Contributor Author

Code review

Summary: Three targeted bug fixes with regression tests. No new abstractions beyond what the fixes require.

Area Finding
doctor.rs check_hook (#102) Managed-block extraction logic is correct — handles out-of-order markers, missing trailing newline, and Claude Code surfaces (no markers → byte-equality only).
render.rs render_plan_no_next (#105) Bool-flag refactor is idiomatic and preserves all existing callers unchanged.
setup.rs output order (#106) Swap is correct.
Tests Both regression tests are well-scoped and exercise the exact failure shapes the bugs produced.

Critical: none

Suggestions: none

Verdict: 🟢 all-green

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@liam-ai-reality
Copy link
Copy Markdown
Contributor Author

Review remediation

Step 10 (triage-followups) skipped — code-review returned 🟢 all-green with zero validated findings. No deferred issues. No fixes required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant