From d54ae5e2a095cbea76e486a507935c17927b66fa Mon Sep 17 00:00:00 2001 From: Mo Date: Mon, 1 Jun 2026 10:30:23 +0200 Subject: [PATCH 1/3] fix(hook,mcp): no-op hook outside zg repos + clear draft pin on tick 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). --- .changeset/hook-noop-when-config-missing.md | 5 ++ .changeset/tick-task-clears-draft-override.md | 5 ++ packages/git-hook/src/install-hook.ts | 6 ++ packages/git-hook/tests/install-hook.test.ts | 29 ++++++++++ packages/mcp-server/src/tools/write.ts | 55 ++++++++++++++++++- .../mcp-server/src/util/write-and-commit.ts | 13 +++++ packages/mcp-server/tests/tools/write.test.ts | 55 +++++++++++++++++++ 7 files changed, 167 insertions(+), 1 deletion(-) create mode 100644 .changeset/hook-noop-when-config-missing.md create mode 100644 .changeset/tick-task-clears-draft-override.md diff --git a/.changeset/hook-noop-when-config-missing.md b/.changeset/hook-noop-when-config-missing.md new file mode 100644 index 0000000..1b7c3a7 --- /dev/null +++ b/.changeset/hook-noop-when-config-missing.md @@ -0,0 +1,5 @@ +--- +'@zettelgeist/cli': patch +--- + +Pre-commit hook now self-disables in repos that aren't zettelgeist repos. Previously, a stale install left over from a removed config (or a partial init) would block every commit with `error: not a zettelgeist repo`. The installed hook block now exits 0 silently when `.zettelgeist.yaml` is missing. Re-run `zettelgeist install-hook` (or `zettelgeist init`) to update existing hooks. diff --git a/.changeset/tick-task-clears-draft-override.md b/.changeset/tick-task-clears-draft-override.md new file mode 100644 index 0000000..3a99fe4 --- /dev/null +++ b/.changeset/tick-task-clears-draft-override.md @@ -0,0 +1,5 @@ +--- +'@zettelgeist/mcp-server': patch +--- + +`tick_task` now clears a stale `status: draft` frontmatter override on `requirements.md` as part of the same commit. This unblocks specs created from the board's "+" column button (which pins `status: draft`) from staying stuck in the draft column after every task is ticked. Other override values (`blocked`, `cancelled`, `planned`, `in-progress`, `in-review`, `done`) are left untouched — they may reflect explicit user intent. `untick_task` does not clear the override, so the draft column reset use-case still works. diff --git a/packages/git-hook/src/install-hook.ts b/packages/git-hook/src/install-hook.ts index 74f9eaf..32791cc 100644 --- a/packages/git-hook/src/install-hook.ts +++ b/packages/git-hook/src/install-hook.ts @@ -8,8 +8,14 @@ export const HOOK_MARKER_END = '# <<< zettelgeist <<<'; // Resolve the zettelgeist binary at hook execution time. Pre-commit hooks // run with the user's login PATH, which won't include ./node_modules/.bin — // so we fall back to the workspace-local binary if PATH lookup misses. +// +// The leading `.zettelgeist.yaml` guard makes the hook self-disabling in +// repos that aren't zettelgeist repos: a stale install left over from a +// removed config, or a partial init, would otherwise block every commit +// with `error: not a zettelgeist repo`. export const HOOK_BLOCK = HOOK_MARKER_BEGIN + '\n' + + '[ -f .zettelgeist.yaml ] || exit 0\n' + 'if command -v zettelgeist >/dev/null 2>&1; then\n' + ' zettelgeist regen --check\n' + 'elif [ -x ./node_modules/.bin/zettelgeist ]; then\n' + diff --git a/packages/git-hook/tests/install-hook.test.ts b/packages/git-hook/tests/install-hook.test.ts index 71d59d4..98beab0 100644 --- a/packages/git-hook/tests/install-hook.test.ts +++ b/packages/git-hook/tests/install-hook.test.ts @@ -50,6 +50,35 @@ describe('mergeHookContent', () => { expect(HOOK_BLOCK).toContain('command -v zettelgeist'); expect(HOOK_BLOCK).toContain('./node_modules/.bin/zettelgeist'); }); + + it('HOOK_BLOCK self-disables when .zettelgeist.yaml is missing', () => { + expect(HOOK_BLOCK).toContain('[ -f .zettelgeist.yaml ] || exit 0'); + }); +}); + +describe('HOOK_BLOCK execution', () => { + let tmp: string; + + beforeEach(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), 'zg-hook-exec-')); + }); + + afterEach(async () => { + await fs.rm(tmp, { recursive: true, force: true }); + }); + + it('exits 0 silently in a directory without .zettelgeist.yaml', async () => { + // We run the block in sh from inside `tmp`, with PATH cleared so the + // command-v fallback would otherwise fail. The pre-flight check must + // short-circuit before any zettelgeist lookup happens. + const { stdout, stderr } = await execFileP( + 'sh', + ['-c', HOOK_BLOCK], + { cwd: tmp, env: { PATH: '/usr/bin:/bin' } }, + ); + expect(stderr).toBe(''); + expect(stdout).toBe(''); + }); }); describe('installPreCommitHook', () => { diff --git a/packages/mcp-server/src/tools/write.ts b/packages/mcp-server/src/tools/write.ts index 9acb653..4a5ac08 100644 --- a/packages/mcp-server/src/tools/write.ts +++ b/packages/mcp-server/src/tools/write.ts @@ -61,6 +61,43 @@ export const writeHandoffTool: ToolDef, { comm const TASK_LINE = /^([\s>]*[-*+]\s+\[)([ xX])(\]\s+.*)$/; +/** + * If `requirements.md` has a `status: draft` frontmatter override, strip + * it and return the new file content + repo-relative path so the caller + * can stage it alongside the tasks.md write. Returns `null` when there + * is nothing to clear (no requirements.md, no frontmatter, status not + * draft, or status is one of the user-intentional values). + * + * Rationale: `draft` is uniquely the "no work yet" state. A `tick_task` + * is unambiguous evidence that work has started, so a pinned `draft` + * override (typically written by the board's "+" button) is provably + * wrong and would otherwise mask all forward progress. Other override + * values (`planned`, `in-progress`, `in-review`, `done`, `blocked`, + * `cancelled`) may reflect explicit user intent — we leave those alone. + */ +async function clearedDraftOverride( + cwd: string, + specDir: string, +): Promise<{ relPath: string; content: string } | null> { + const reqAbs = safeJoin(specDir, 'requirements.md'); + let raw: string; + try { + raw = await fs.readFile(reqAbs, 'utf8'); + } catch { + return null; + } + const parsed = matter(raw, {}); + const data = { ...(parsed.data ?? {}) } as Record; + if (data.status !== 'draft') return null; + delete data.status; + const newFm = Object.keys(data).length > 0 ? `---\n${yaml.dump(data)}---\n` : ''; + const body = parsed.content.startsWith('\n') ? parsed.content.slice(1) : parsed.content; + const newContent = newFm + body; + await fs.writeFile(reqAbs, newContent, 'utf8'); + const relPath = path.relative(cwd, reqAbs).split(path.sep).join('/'); + return { relPath, content: newContent }; +} + async function tickOrUntick(cwd: string, name: string, n: number, checked: boolean): Promise<{ commit: string }> { const reader = makeDiskFsReader(cwd); const cfg = await loadConfig(reader); @@ -83,13 +120,29 @@ async function tickOrUntick(cwd: string, name: string, n: number, checked: boole } } if (!mutated) throw new Error(`no task at index ${n} in ${name}`); + + // Tick (only) clears a stale `status: draft` override on requirements.md + // so the board doesn't keep the card pinned in the draft column after + // the user has started working. Untick intentionally does NOT clear — + // an untick might be undoing an accidental tick on a draft spec, and + // re-pinning to draft would be the right move there. + const extraFiles: string[] = []; + if (checked) { + const cleared = await clearedDraftOverride(cwd, specDir); + if (cleared) extraFiles.push(cleared.relPath); + } + const op = checked ? 'tick' : 'untick'; + const opts: { log: { specName: string; action: string }; extraFiles?: ReadonlyArray } = { + log: { specName: name, action: `${op}_task(${n})` }, + }; + if (extraFiles.length > 0) opts.extraFiles = extraFiles; return writeFileAndCommit( cwd, tasksRel, lines.join('\n'), `[zg] ${op}: ${name}#${n}`, - { log: { specName: name, action: `${op}_task(${n})` } }, + opts, ); } diff --git a/packages/mcp-server/src/util/write-and-commit.ts b/packages/mcp-server/src/util/write-and-commit.ts index 3766ee3..760a37d 100644 --- a/packages/mcp-server/src/util/write-and-commit.ts +++ b/packages/mcp-server/src/util/write-and-commit.ts @@ -28,6 +28,15 @@ export interface WriteAndCommitOptions { action: string; agentId?: string; }; + /** + * Additional file paths (repo-relative, POSIX-style) to include in the + * same commit as the main write. Callers are responsible for having + * already written these to disk; the helper just stages them. Used by + * tools that need to make an atomic multi-file edit (e.g. tick_task + * auto-clearing a stale `status: draft` override in requirements.md + * alongside the tasks.md tick). + */ + extraFiles?: ReadonlyArray; } export async function writeFileAndCommit( @@ -87,6 +96,10 @@ export async function writeFileAndCommit( if (wrote) filesToAdd.push(logRelPath); } + if (options?.extraFiles) { + for (const f of options.extraFiles) filesToAdd.push(f); + } + await execFileP('git', ['add', ...filesToAdd], { cwd }); await execFileP('git', ['commit', '-m', commitMessage], { cwd }); const { stdout } = await execFileP('git', ['rev-parse', 'HEAD'], { cwd }); diff --git a/packages/mcp-server/tests/tools/write.test.ts b/packages/mcp-server/tests/tools/write.test.ts index bcc8753..47c4a8d 100644 --- a/packages/mcp-server/tests/tools/write.test.ts +++ b/packages/mcp-server/tests/tools/write.test.ts @@ -69,6 +69,61 @@ describe('writeTools', () => { expect(after.startsWith('- [ ] one')).toBe(true); }); + it('tick_task clears a stale `status: draft` override on requirements.md', async () => { + // Simulate the board's "+" button: it pins `status: draft` so the + // new card lands in the draft column. Without auto-clear, ticking + // every task would still show the spec as draft because the + // frontmatter override beats the derived status. + const reqPath = path.join(tmp, 'specs', 'foo', 'requirements.md'); + await fs.writeFile(reqPath, '---\nstatus: draft\ndepends_on: []\n---\n# foo\n'); + await execFileP('git', ['add', '.'], { cwd: tmp }); + await execFileP('git', ['commit', '-q', '-m', 'pin draft'], { cwd: tmp }); + + await tickTaskTool.handler({ name: 'foo', n: 1 }, { cwd: tmp }); + + const after = await fs.readFile(reqPath, 'utf8'); + expect(after).not.toContain('status:'); + // Other frontmatter fields survive untouched. + expect(after).toContain('depends_on: []'); + expect(after).toContain('# foo'); + }); + + it('tick_task leaves other status overrides alone', async () => { + // `blocked` is a documented v0.1 frontmatter override. Ticking a + // task on a blocked spec MUST NOT silently un-block it. + const reqPath = path.join(tmp, 'specs', 'foo', 'requirements.md'); + await fs.writeFile(reqPath, '---\nstatus: blocked\nblocked_by: waiting\n---\n# foo\n'); + await execFileP('git', ['add', '.'], { cwd: tmp }); + await execFileP('git', ['commit', '-q', '-m', 'pin blocked'], { cwd: tmp }); + + await tickTaskTool.handler({ name: 'foo', n: 1 }, { cwd: tmp }); + + const after = await fs.readFile(reqPath, 'utf8'); + expect(after).toContain('status: blocked'); + expect(after).toContain('blocked_by: waiting'); + }); + + it('untick_task does NOT clear a draft override', async () => { + // Untick is symmetrical-undo, not a forward-progress signal — leave + // any pin in place so the user can reset a card back to draft. + const reqPath = path.join(tmp, 'specs', 'foo', 'requirements.md'); + await fs.writeFile(reqPath, '---\nstatus: draft\n---\n# foo\n'); + await execFileP('git', ['add', '.'], { cwd: tmp }); + await execFileP('git', ['commit', '-q', '-m', 'pin draft'], { cwd: tmp }); + + // First tick clears the override (per previous test); restore it so + // we're testing untick in isolation. + await tickTaskTool.handler({ name: 'foo', n: 1 }, { cwd: tmp }); + await fs.writeFile(reqPath, '---\nstatus: draft\n---\n# foo\n'); + await execFileP('git', ['add', '.'], { cwd: tmp }); + await execFileP('git', ['commit', '-q', '-m', 'restore pin'], { cwd: tmp }); + + await untickTaskTool.handler({ name: 'foo', n: 1 }, { cwd: tmp }); + + const after = await fs.readFile(reqPath, 'utf8'); + expect(after).toContain('status: draft'); + }); + it('writeSpecFileTool rejects relpath with traversal', async () => { await expect(writeSpecFileTool.handler( { name: 'foo', relpath: '../../evil.txt', content: 'pwn' }, From 6456150fd2e985c4742c2fd2d325957fc4988633 Mon Sep 17 00:00:00 2001 From: Mo Date: Mon, 1 Jun 2026 10:36:52 +0200 Subject: [PATCH 2/3] fix(core): self-heal stale `status: draft` override in deriveStatus MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../derive-status-heals-stale-draft-pin.md | 5 ++ packages/core/src/status.ts | 20 ++++-- packages/core/tests/status.test.ts | 72 +++++++++++++++++++ 3 files changed, 93 insertions(+), 4 deletions(-) create mode 100644 .changeset/derive-status-heals-stale-draft-pin.md diff --git a/.changeset/derive-status-heals-stale-draft-pin.md b/.changeset/derive-status-heals-stale-draft-pin.md new file mode 100644 index 0000000..c4ede40 --- /dev/null +++ b/.changeset/derive-status-heals-stale-draft-pin.md @@ -0,0 +1,5 @@ +--- +'@zettelgeist/core': patch +--- + +`deriveStatus` now ignores a `status: draft` frontmatter override when the spec has at least one counted task and every counted task is checked. The override is provably stale in that case (the board's "+" button writes it on new-card creation and nothing clears it afterwards). The card transparently moves to `in-review` (or `done` if merged) on the next render — no file mutation, no user action. Other override values (`planned`, `in-progress`, `in-review`, `done`, `blocked`, `cancelled`) and `draft` overrides with partial or zero progress are unchanged. diff --git a/packages/core/src/status.ts b/packages/core/src/status.ts index b5c2272..a1bd8a4 100644 --- a/packages/core/src/status.ts +++ b/packages/core/src/status.ts @@ -19,14 +19,26 @@ export function deriveStatus(spec: Spec, repo: RepoState): Status { // blocked/cancelled. Board drag-to-column writes this field; ignoring the // override here would render those drags invisible (the card snaps back). const fm = spec.frontmatter.status; - if (typeof fm === 'string' && VALID_STATUSES.has(fm as Status)) { - return fm as Status; - } - const counted = spec.tasks.filter(isCounted); const claimed = repo.claimedSpecs.has(spec.name); const merged = repo.mergedSpecs.has(spec.name); + if (typeof fm === 'string' && VALID_STATUSES.has(fm as Status)) { + // Self-heal a stale `status: draft` override. The board's "+" button + // writes `status: draft` so a new card lands in the draft column; + // nothing clears it once the user starts (and finishes) the work, so + // before this rule, a spec with every task ticked could remain pinned + // to "draft" indefinitely. We ignore the override only when it is + // provably wrong — `draft` AND at least one counted task exists AND + // every counted task is checked — so partial-progress pins (which + // may reflect deliberate user intent) are still honoured. Other + // override values (planned/in-progress/in-review/done/blocked/ + // cancelled) are never auto-cleared here. + const isStaleDraftPin = + fm === 'draft' && counted.length > 0 && counted.every((t) => t.checked); + if (!isStaleDraftPin) return fm as Status; + } + if (counted.length === 0) { // No counted tasks. A live claim still bumps to in-progress. return claimed ? 'in-progress' : 'draft'; diff --git a/packages/core/tests/status.test.ts b/packages/core/tests/status.test.ts index 7f841bb..bea5096 100644 --- a/packages/core/tests/status.test.ts +++ b/packages/core/tests/status.test.ts @@ -133,6 +133,78 @@ describe('deriveStatus', () => { ).toBe('in-review'); }); + it('self-heals a `status: draft` override when every counted task is checked (unmerged)', () => { + // The board's "+" button pins `status: draft` so new cards appear in + // the draft column. Without a self-heal, the override beats the + // derived status forever — even after the user ticks every task. + expect( + deriveStatus( + spec({ + frontmatter: { status: 'draft' }, + tasks: [ + { index: 1, checked: true, text: 'a', tags: [] }, + { index: 2, checked: true, text: 'b', tags: [] }, + ], + }), + emptyRepoState, + ), + ).toBe('in-review'); + }); + + it('self-healed draft override goes to "done" when also merged', () => { + expect( + deriveStatus( + spec({ + name: 'foo', + frontmatter: { status: 'draft' }, + tasks: [{ index: 1, checked: true, text: 'a', tags: [] }], + }), + { claimedSpecs: new Set(), mergedSpecs: new Set(['foo']) }, + ), + ).toBe('done'); + }); + + it('respects a `status: draft` override when only SOME tasks are checked', () => { + // Partial progress might be deliberate ("I want to rethink this; keep + // it on the draft column"), so we only self-heal at all-done. + expect( + deriveStatus( + spec({ + frontmatter: { status: 'draft' }, + tasks: [ + { index: 1, checked: true, text: 'a', tags: [] }, + { index: 2, checked: false, text: 'b', tags: [] }, + ], + }), + emptyRepoState, + ), + ).toBe('draft'); + }); + + it('respects a `status: draft` override when there are no counted tasks', () => { + // Matches conformance fixture 19-all-statuses/a-draft: status:draft + // override on a spec with no tasks at all stays "draft". + expect( + deriveStatus(spec({ frontmatter: { status: 'draft' } }), emptyRepoState), + ).toBe('draft'); + }); + + it('does NOT self-heal other override values even when all tasks are checked', () => { + // The self-heal is narrowly for `draft` — other overrides may reflect + // intentional user state and must not be silently re-derived. + for (const s of ['planned', 'in-progress', 'in-review', 'done', 'blocked', 'cancelled'] as const) { + expect( + deriveStatus( + spec({ + frontmatter: { status: s }, + tasks: [{ index: 1, checked: true, text: 'a', tags: [] }], + }), + emptyRepoState, + ), + ).toBe(s); + } + }); + it('returns "done" when all non-#skip tasks ticked and merged', () => { expect( deriveStatus( From 4dadf60a3d49c41a540e6c46f40417168acf7b7e Mon Sep 17 00:00:00 2001 From: Mo Date: Mon, 1 Jun 2026 11:04:06 +0200 Subject: [PATCH 3/3] fix(mcp): atomic multi-file write for tick_task draft-clear MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- packages/mcp-server/src/tools/write.ts | 28 +++++++++------- .../mcp-server/src/util/write-and-commit.ts | 33 ++++++++++++++----- packages/mcp-server/tests/tools/write.test.ts | 17 ++++++++++ 3 files changed, 57 insertions(+), 21 deletions(-) diff --git a/packages/mcp-server/src/tools/write.ts b/packages/mcp-server/src/tools/write.ts index 4a5ac08..6260d0b 100644 --- a/packages/mcp-server/src/tools/write.ts +++ b/packages/mcp-server/src/tools/write.ts @@ -62,11 +62,13 @@ export const writeHandoffTool: ToolDef, { comm const TASK_LINE = /^([\s>]*[-*+]\s+\[)([ xX])(\]\s+.*)$/; /** - * If `requirements.md` has a `status: draft` frontmatter override, strip - * it and return the new file content + repo-relative path so the caller - * can stage it alongside the tasks.md write. Returns `null` when there - * is nothing to clear (no requirements.md, no frontmatter, status not - * draft, or status is one of the user-intentional values). + * Compute the new contents of `requirements.md` with a `status: draft` + * frontmatter override stripped, without touching the disk. Returns + * `null` when there is nothing to clear (no requirements.md, no + * frontmatter, status not draft, or status is one of the user- + * intentional values). The caller stages the returned write through + * `writeFileAndCommit`'s `extraWrites` so the actual fs mutation + * happens atomically alongside the tasks.md tick. * * Rationale: `draft` is uniquely the "no work yet" state. A `tick_task` * is unambiguous evidence that work has started, so a pinned `draft` @@ -75,7 +77,7 @@ const TASK_LINE = /^([\s>]*[-*+]\s+\[)([ xX])(\]\s+.*)$/; * values (`planned`, `in-progress`, `in-review`, `done`, `blocked`, * `cancelled`) may reflect explicit user intent — we leave those alone. */ -async function clearedDraftOverride( +async function plannedDraftOverrideClear( cwd: string, specDir: string, ): Promise<{ relPath: string; content: string } | null> { @@ -93,7 +95,6 @@ async function clearedDraftOverride( const newFm = Object.keys(data).length > 0 ? `---\n${yaml.dump(data)}---\n` : ''; const body = parsed.content.startsWith('\n') ? parsed.content.slice(1) : parsed.content; const newContent = newFm + body; - await fs.writeFile(reqAbs, newContent, 'utf8'); const relPath = path.relative(cwd, reqAbs).split(path.sep).join('/'); return { relPath, content: newContent }; } @@ -126,17 +127,20 @@ async function tickOrUntick(cwd: string, name: string, n: number, checked: boole // the user has started working. Untick intentionally does NOT clear — // an untick might be undoing an accidental tick on a draft spec, and // re-pinning to draft would be the right move there. - const extraFiles: string[] = []; + const extraWrites: Array<{ relPath: string; content: string }> = []; if (checked) { - const cleared = await clearedDraftOverride(cwd, specDir); - if (cleared) extraFiles.push(cleared.relPath); + const cleared = await plannedDraftOverrideClear(cwd, specDir); + if (cleared) extraWrites.push(cleared); } const op = checked ? 'tick' : 'untick'; - const opts: { log: { specName: string; action: string }; extraFiles?: ReadonlyArray } = { + const opts: { + log: { specName: string; action: string }; + extraWrites?: ReadonlyArray<{ relPath: string; content: string }>; + } = { log: { specName: name, action: `${op}_task(${n})` }, }; - if (extraFiles.length > 0) opts.extraFiles = extraFiles; + if (extraWrites.length > 0) opts.extraWrites = extraWrites; return writeFileAndCommit( cwd, tasksRel, diff --git a/packages/mcp-server/src/util/write-and-commit.ts b/packages/mcp-server/src/util/write-and-commit.ts index 760a37d..db9061a 100644 --- a/packages/mcp-server/src/util/write-and-commit.ts +++ b/packages/mcp-server/src/util/write-and-commit.ts @@ -29,14 +29,16 @@ export interface WriteAndCommitOptions { agentId?: string; }; /** - * Additional file paths (repo-relative, POSIX-style) to include in the - * same commit as the main write. Callers are responsible for having - * already written these to disk; the helper just stages them. Used by - * tools that need to make an atomic multi-file edit (e.g. tick_task - * auto-clearing a stale `status: draft` override in requirements.md - * alongside the tasks.md tick). + * Additional `{ relPath, content }` pairs written and committed + * atomically with the main file. The helper performs the temp+rename + * write BEFORE running conformance, so all files are on disk by the + * time INDEX.md regenerates — keeping the failure mode symmetric with + * the single-file path (a conformance throw leaves a consistent + * working-tree diff rather than a half-applied state). Used by tools + * that need a multi-file atomic edit, e.g. `tick_task` clearing a + * `status: draft` override in requirements.md alongside the tick. */ - extraFiles?: ReadonlyArray; + extraWrites?: ReadonlyArray<{ relPath: string; content: string }>; } export async function writeFileAndCommit( @@ -52,6 +54,19 @@ export async function writeFileAndCommit( await fs.writeFile(tmp, content, 'utf8'); await fs.rename(tmp, fileAbs); + // Stage any caller-supplied extra writes the same way (temp+rename) + // BEFORE running conformance, so INDEX.md regenerates against the + // post-write state of every file in this transaction. + if (options?.extraWrites) { + for (const w of options.extraWrites) { + const abs = path.join(cwd, w.relPath); + await fs.mkdir(path.dirname(abs), { recursive: true }); + const t = `${abs}.tmp`; + await fs.writeFile(t, w.content, 'utf8'); + await fs.rename(t, abs); + } + } + // Regen — load config first so we can reuse it for the .log.md path too. const reader = makeDiskFsReader(cwd); const cfg = await loadConfig(reader); @@ -96,8 +111,8 @@ export async function writeFileAndCommit( if (wrote) filesToAdd.push(logRelPath); } - if (options?.extraFiles) { - for (const f of options.extraFiles) filesToAdd.push(f); + if (options?.extraWrites) { + for (const w of options.extraWrites) filesToAdd.push(w.relPath); } await execFileP('git', ['add', ...filesToAdd], { cwd }); diff --git a/packages/mcp-server/tests/tools/write.test.ts b/packages/mcp-server/tests/tools/write.test.ts index 47c4a8d..30f6ba6 100644 --- a/packages/mcp-server/tests/tools/write.test.ts +++ b/packages/mcp-server/tests/tools/write.test.ts @@ -88,6 +88,23 @@ describe('writeTools', () => { expect(after).toContain('# foo'); }); + it('tick_task clears a `status: draft`-only frontmatter and drops the fence', async () => { + // When `status: draft` is the only frontmatter key, clearing it + // should produce a file with no frontmatter block at all (the + // dump-empty-then-fence branch in plannedDraftOverrideClear). + const reqPath = path.join(tmp, 'specs', 'foo', 'requirements.md'); + await fs.writeFile(reqPath, '---\nstatus: draft\n---\n# foo\n'); + await execFileP('git', ['add', '.'], { cwd: tmp }); + await execFileP('git', ['commit', '-q', '-m', 'pin draft alone'], { cwd: tmp }); + + await tickTaskTool.handler({ name: 'foo', n: 1 }, { cwd: tmp }); + + const after = await fs.readFile(reqPath, 'utf8'); + expect(after).not.toContain('---'); + expect(after).not.toContain('status'); + expect(after).toContain('# foo'); + }); + it('tick_task leaves other status overrides alone', async () => { // `blocked` is a documented v0.1 frontmatter override. Ticking a // task on a blocked spec MUST NOT silently un-block it.