Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/derive-status-heals-stale-draft-pin.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/hook-noop-when-config-missing.md
Original file line number Diff line number Diff line change
@@ -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.
5 changes: 5 additions & 0 deletions .changeset/tick-task-clears-draft-override.md
Original file line number Diff line number Diff line change
@@ -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.
20 changes: 16 additions & 4 deletions packages/core/src/status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
72 changes: 72 additions & 0 deletions packages/core/tests/status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
6 changes: 6 additions & 0 deletions packages/git-hook/src/install-hook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' +
Expand Down
29 changes: 29 additions & 0 deletions packages/git-hook/tests/install-hook.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
59 changes: 58 additions & 1 deletion packages/mcp-server/src/tools/write.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,44 @@ export const writeHandoffTool: ToolDef<z.infer<typeof writeHandoffInput>, { comm

const TASK_LINE = /^([\s>]*[-*+]\s+\[)([ xX])(\]\s+.*)$/;

/**
* 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`
* 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 plannedDraftOverrideClear(
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<string, unknown>;
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;
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);
Expand All @@ -83,13 +121,32 @@ 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 extraWrites: Array<{ relPath: string; content: string }> = [];
if (checked) {
const cleared = await plannedDraftOverrideClear(cwd, specDir);
if (cleared) extraWrites.push(cleared);
}

const op = checked ? 'tick' : 'untick';
const opts: {
log: { specName: string; action: string };
extraWrites?: ReadonlyArray<{ relPath: string; content: string }>;
} = {
log: { specName: name, action: `${op}_task(${n})` },
};
if (extraWrites.length > 0) opts.extraWrites = extraWrites;
return writeFileAndCommit(
cwd,
tasksRel,
lines.join('\n'),
`[zg] ${op}: ${name}#${n}`,
{ log: { specName: name, action: `${op}_task(${n})` } },
opts,
);
}

Expand Down
28 changes: 28 additions & 0 deletions packages/mcp-server/src/util/write-and-commit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,17 @@ export interface WriteAndCommitOptions {
action: string;
agentId?: string;
};
/**
* 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.
*/
extraWrites?: ReadonlyArray<{ relPath: string; content: string }>;
}

export async function writeFileAndCommit(
Expand All @@ -43,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);
Expand Down Expand Up @@ -87,6 +111,10 @@ export async function writeFileAndCommit(
if (wrote) filesToAdd.push(logRelPath);
}

if (options?.extraWrites) {
for (const w of options.extraWrites) filesToAdd.push(w.relPath);
}

await execFileP('git', ['add', ...filesToAdd], { cwd });
await execFileP('git', ['commit', '-m', commitMessage], { cwd });
const { stdout } = await execFileP('git', ['rev-parse', 'HEAD'], { cwd });
Expand Down
72 changes: 72 additions & 0 deletions packages/mcp-server/tests/tools/write.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,78 @@ 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 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.
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' },
Expand Down
Loading