Skip to content

fix(hook,mcp): no-op hook outside zg repos + clear draft pin on tick - #15

Merged
morrieinmaas merged 3 commits into
mainfrom
fix/hook-noop-and-clear-draft-on-tick
Jun 1, 2026
Merged

fix(hook,mcp): no-op hook outside zg repos + clear draft pin on tick#15
morrieinmaas merged 3 commits into
mainfrom
fix/hook-noop-and-clear-draft-on-tick

Conversation

@morrieinmaas

@morrieinmaas morrieinmaas commented Jun 1, 2026

Copy link
Copy Markdown
Owner

Summary

Three real-use bugs surfaced by a user on sensor-sender:

  1. Pre-commit hook blocks every commit in non-zg repos. A stale install (or a partial init) left a hook that ran zettelgeist regen --check, which hard-errors with not a zettelgeist repo whenever .zettelgeist.yaml is missing. Fix: the installed hook block now exits 0 silently when the config file is missing. Existing broken installs heal on the next zettelgeist install-hook / zettelgeist init run.

  2. Spec stuck in draft after all tasks ticked (write-side prevention). The board's "+" button pins status: draft in frontmatter so the new card lands in the draft column. Nothing ever cleared the pin, so the frontmatter override beat the derived status forever. Fix: tick_task strips a status: draft override on requirements.md as part of the same commit (other override values left alone; untick_task does not clear, so reset-to-draft still works).

  3. Existing stuck specs self-heal on next render (read-side rescue). The write-side fix only helps if the user ticks ANOTHER task — useless for the spec already in the bug state (every task ticked, override pinned). Fix: deriveStatus now ignores a status: draft override when at least one counted task exists and every counted task is checked. No file mutation, no migration, no user action — stuck specs unstick on next viewer/extension load.

Changes

  • packages/git-hook/src/install-hook.ts — add [ -f .zettelgeist.yaml ] || exit 0 to HOOK_BLOCK.
  • packages/mcp-server/src/tools/write.tstickOrUntick strips a status: draft override on tick via a multi-file atomic write.
  • packages/mcp-server/src/util/write-and-commit.ts — new extraWrites?: ReadonlyArray<{ relPath, content }> option. Helper writes all files via temp+rename before running conformance, so a conformance throw leaves a consistent working-tree diff rather than a torn state (failure mode symmetric with the existing single-file case).
  • packages/core/src/status.tsderiveStatus ignores status: draft override when all counted tasks are checked.
  • Tests added across all three layers (incl. negative: other overrides untouched, untick does not clear, partial-progress draft pin honored, conformance fixture 19-all-statuses/a-draft unaffected because counted.length === 0).
  • Three changesets (core patch, cli patch, mcp-server patch). git-hook is in ignore so its change ships via the cli's install-hook command.

Code review

Independent code-reviewer agent flagged one real issue: the original write-side patch wrote requirements.md to disk before writeFileAndCommit ran conformance — an asymmetric write pattern with a half-applied failure mode. Addressed in commit 4dadf60 (atomic multi-file write via extraWrites). Also added a test for the status-only-frontmatter edge case that the reviewer noted was uncovered.

Reviewer-noted concerns acknowledged but not changed:

  • Read-side self-heal could surprise a user who pinned draft on a single-task spec deliberately — narrow enough we accept it; the alternative is users with stuck cards.
  • Hook block test brittleness on PATH — accepted as-is.

Test plan

  • pnpm -r typecheck — clean
  • pnpm -r test — all 9 packages, all green (190 core / 45 mcp / 117 cli / +others)
  • pnpm conformance — 46 fixtures pass
  • After merge: confirm Changesets bot opens "Version Packages" PR with core + cli + mcp-server patch bumps
  • After version PR merges: rebuild VSCode extension with new git-hook block + new core and ship the vsix

Two surgical fixes for issues hit during real use:

- packages/git-hook: pre-commit hook now exits 0 silently when
  .zettelgeist.yaml is missing, instead of failing every commit with
  "error: not a zettelgeist repo". This unblocks repos where a stale
  install (or a partial init) left the hook behind. Users with broken
  hooks today must re-run `zettelgeist install-hook` to pick this up.

- packages/mcp-server: tick_task now strips a `status: draft`
  frontmatter override on requirements.md as part of the same commit.
  The board's "+" button pins `status: draft` so new cards land in the
  draft column; without this, the override beat the derived status and
  cards stayed stuck on "draft" even after every task was ticked.
  Other override values are intentionally left alone; untick_task does
  not clear (lets users reset cards back to draft).
Read-side counterpart to the tick-side fix in this PR. Without this,
specs already stuck (every task ticked, frontmatter still pins draft)
required a manual frontmatter edit to unstick — the tick-clear path
only fires on a fresh tick, which never happens once everything's done.

deriveStatus now ignores `status: draft` when at least one counted
task exists and every counted task is checked. Render-time fix —
no file mutation, no migration. Other override values and partial-
progress draft pins are untouched.

Conformance fixture 19-all-statuses/a-draft is unaffected: its spec
has zero counted tasks, so the override branch still wins.
Code review caught that the previous shape wrote requirements.md
directly (no temp+rename, before conformance ran), so a conformance
throw between the two writes would leave a half-applied state.

Refactor: replace `extraFiles` (caller pre-writes) with `extraWrites`
({ relPath, content }; helper writes via temp+rename inside its own
flow, before regen). Now both files reach disk via the same atomic
pattern as the primary write, and the failure mode is symmetric with
the existing single-file case — a conformance throw leaves a
consistent working-tree diff rather than a torn state.

Add a test for the status-only-frontmatter case (no other keys) so
the dump-empty-then-drop-fence branch is covered.
@morrieinmaas
morrieinmaas merged commit 8853133 into main Jun 1, 2026
2 checks passed
@morrieinmaas
morrieinmaas deleted the fix/hook-noop-and-clear-draft-on-tick branch June 1, 2026 09:06
morrieinmaas added a commit that referenced this pull request Jun 1, 2026
- Version bump for the marketplace publish carrying the hook + tick +
  deriveStatus fixes from #15 (via @zettelgeist/core and
  @zettelgeist/git-hook workspace deps). Already published to Open VSX
  as morrieinmaas.zettelgeist@0.4.2; Microsoft Marketplace gets the
  vsix uploaded manually.
- .env to .gitignore so local publish tokens (npm, ovsx) can't be
  committed accidentally.
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