diff --git a/.changeset/wise-foxes-march.md b/.changeset/wise-foxes-march.md new file mode 100644 index 000000000..75b924512 --- /dev/null +++ b/.changeset/wise-foxes-march.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 4416 +--- +**macOS todo rendering no longer drops the Needs clause under long temp paths** — the 240-char bound is now deterministic w.r.t. base-path length. (#4384 regression) diff --git a/docs/COMMANDS.md b/docs/COMMANDS.md index d45e1346e..a2c2f33ca 100644 --- a/docs/COMMANDS.md +++ b/docs/COMMANDS.md @@ -1938,7 +1938,7 @@ Capture ideas, tasks, notes, and seeds to their appropriate destination. Default **Produces:** `.planning/todos/` (default), note files (--note), ROADMAP.md backlog section (--backlog), `.planning/seeds/SEED-NNN-slug.md` (--seed) -**STATE.md rendering:** each capture (or `--list` action that changes the pending count) refreshes STATE.md's "### Pending Todos" section to one bullet per pending todo, each capped at 240 characters — `- [date] [area] title — [todo file](path) — Needs ...`. A todo with no clear next step omits the "Needs ..." clause rather than the bullet. Refresh is fail-safe: a failed or malformed lookup leaves the existing section untouched rather than clearing it. +**STATE.md rendering:** each capture (or `--list` action that changes the pending count) refreshes STATE.md's "### Pending Todos" section to one bullet per pending todo, each capped at 240 characters — `- [date] [area] title — [todo file](path) — Needs ...`. The todo-file link is repo-relative (`.planning/todos/pending/...`), so the cap is independent of where the repo is checked out — a long absolute path never consumes the budget or drops the "Needs ..." clause. A todo with no clear next step omits the "Needs ..." clause rather than the bullet. Refresh is fail-safe: a failed or malformed lookup leaves the existing section untouched rather than clearing it. ```bash /gsd-capture "Consider adding dark mode support" # Add todo diff --git a/docs/reference/state-md.md b/docs/reference/state-md.md index 21a8c4e1a..1c2d96b18 100644 --- a/docs/reference/state-md.md +++ b/docs/reference/state-md.md @@ -224,7 +224,7 @@ Updated after each plan completion. **Decisions** — a summary of recent decisions affecting current work (full log lives in `PROJECT.md`). Added via `gsd-tools state add-decision`. -**Pending Todos** — count and reference to `.planning/todos/pending/`. Captured via `/gsd-capture`. +**Pending Todos** — one bullet per pending todo (`- [date] [area] title — [todo file](repo-relative path) — Needs ...`, capped at 240 characters; repo-relative link keeps the cap independent of checkout path length). Captured via `/gsd-capture`. **Blockers/Concerns** — issues affecting future work, prefixed with the originating phase. Added via `gsd-tools state add-blocker`; resolved via `gsd-tools state resolve-blocker`. diff --git a/gsd-core/templates/state.md b/gsd-core/templates/state.md index 32afd9a90..a3deba6d8 100644 --- a/gsd-core/templates/state.md +++ b/gsd-core/templates/state.md @@ -173,7 +173,8 @@ Updated after each plan completion. **Pending Todos:** Ideas captured via /gsd-add-todo - One bullet per pending todo, rendered by `init.todos`'s `pending_todos_markdown` - (each bullet capped at 240 characters: `- [date] [area] title — [todo file](path) — Needs ...`) + (each bullet capped at 240 characters: `- [date] [area] title — [todo file](path) — Needs ...`; + the todo-file link is repo-relative, so the cap does not depend on checkout path length) - `None yet.` when there are no pending todos - No collapse-by-count fallback — every pending todo gets its own line, always (see #2618 design doc for why a "count if many" fallback was rejected) diff --git a/src/init.cts b/src/init.cts index 0192d76ce..b11de2485 100644 --- a/src/init.cts +++ b/src/init.cts @@ -2265,19 +2265,49 @@ function truncatePendingTodoText(value: string, maxLen: number): string { * code rather than a prose algorithm (DEFECT.GENERATIVE-FIX: a prose * algorithm duplicated as a test oracle is exactly the divergence class * this avoids). + * + * #4384 regression fix: the optional `projectRoot` makes the bullet's + * `[todo file](…)` link repo-relative (see pendingTodoLinkTarget) so the cap + * is deterministic w.r.t. where the repo is checked out. Omitting it keeps + * the legacy absolute-link behavior for existing direct callers. */ -function renderPendingTodosMarkdown(todos: Record[]): string { +function renderPendingTodosMarkdown(todos: Record[], projectRoot?: string): string { if (!Array.isArray(todos) || todos.length === 0) { return 'None yet.'; } - return todos.map((todo) => renderPendingTodoBullet(todo)).join('\n'); + return todos.map((todo) => renderPendingTodoBullet(todo, projectRoot)).join('\n'); } function pendingTodoFieldAsString(value: unknown, fallback: string): string { return typeof value === 'string' && value.length > 0 ? value : fallback; } -function renderPendingTodoBullet(todo: Record): string { +/** + * #4384 regression fix: the bullet's markdown link target, rendered + * repo-relative when `projectRoot` is given and the todo's `path` is + * absolute. The JSON `todos[].path` field stays absolute (#2376 contract); + * only the rendered display link changes — embedding the machine-variable + * absolute base let macOS's /private/var/folders/… temp paths consume the + * 240-char budget and drop the "Needs " clause on long-path + * machines only (next's own macos CI shard went red on exactly this, run + * 34038716700). Repo-relative links also resolve correctly from STATE.md at + * the repo root and survive repo moves. + */ +function pendingTodoLinkTarget(todo: Record, projectRoot: string | undefined): string { + const raw = pendingTodoFieldAsString(todo['path'], ''); + if (typeof projectRoot !== 'string' || projectRoot.length === 0 || !path.isAbsolute(raw)) { + return raw; + } + const rel = toPosixPath(path.relative(projectRoot, raw)); + if (rel.length === 0 || path.isAbsolute(rel)) { + // Degenerate (path === projectRoot) or Windows cross-drive fallback: + // keep the raw target rather than emitting an empty or incorrect link. + return raw; + } + return rel; +} + +function renderPendingTodoBullet(todo: Record, projectRoot?: string): string { const date = sanitizePendingTodoInline(pendingTodoFieldAsString(todo['created'], 'unknown')); let area = sanitizePendingTodoInline(pendingTodoFieldAsString(todo['area'], 'general')); let title = sanitizePendingTodoInline(pendingTodoFieldAsString(todo['title'], 'Untitled')); @@ -2287,7 +2317,7 @@ function renderPendingTodoBullet(todo: Record): string { typeof todo['needs'] === 'string' ? sanitizePendingTodoInline(todo['needs']).replace(/\.+$/, '') : ''; - const link = `[todo file](${pendingTodoFieldAsString(todo['path'], '')})`; + const link = `[todo file](${pendingTodoLinkTarget(todo, projectRoot)})`; const assemble = (): string => { const needsClause = needs ? ` — Needs ${needs}.` : ''; @@ -2420,7 +2450,9 @@ function cmdInitTodos(cwd: string, area: string | undefined, raw: boolean): void // (rather than emitted with possibly-wrong data) when pendingReadOk is // false, so the workflow's fail-safe check can key off field presence. pending_read_ok: pendingReadOk, - ...(pendingReadOk ? { pending_todos_markdown: renderPendingTodosMarkdown(todos) } : {}), + // #4384 fix: pass cwd as projectRoot so the bullet link renders + // repo-relative — see pendingTodoLinkTarget. + ...(pendingReadOk ? { pending_todos_markdown: renderPendingTodosMarkdown(todos, cwd) } : {}), }; output(withProjectRoot(cwd, result), raw); diff --git a/tests/state-todos-render.test.cjs b/tests/state-todos-render.test.cjs index 5574f0e39..d47b5db80 100644 --- a/tests/state-todos-render.test.cjs +++ b/tests/state-todos-render.test.cjs @@ -167,6 +167,102 @@ test('property: rendered body always has one line per todo, each line <= 240 cha ); }); +// ─── #4384 regression: the 240-char cap must be deterministic w.r.t. where the +// repo is checked out (macOS /private/var/folders/… bases blew the budget and +// dropped the "Needs" clause; Linux /tmp passed — next's own macos shard 3/3 +// went red on exactly this, CI run 34038716700) ────────────────────────────── + +// Long enough that the pre-fix absolute-link bullet exceeds 240 chars on EVERY +// OS (Linux /tmp included): base > ~90 chars is over the threshold since the +// fixed skeleton + relative tail + needs clause land near 150. +const LONG_BASE_SEGMENT = 'd'.repeat(110); +const TODO_RELATIVE_TAIL = path + .join('.planning', 'todos', 'pending', '2026-09-01-fix-retry-logic.md') + .split(path.sep) + .join('/'); + +test('renderPendingTodosMarkdown: cap is deterministic w.r.t. base-path length (needs clause survives long absolute bases)', () => { + const shortBase = path.resolve('/', 'gsd-2618-short-base'); + const longBase = path.join(shortBase, LONG_BASE_SEGMENT); + const todoAt = (base) => + makeTodo({ path: path.join(base, TODO_RELATIVE_TAIL), needs: 'Add a max-attempts cap.' }); + + const fromShort = renderPendingTodosMarkdown([todoAt(shortBase)], shortBase); + const fromLong = renderPendingTodosMarkdown([todoAt(longBase)], longBase); + + assert.equal(fromLong, fromShort, 'bullet must not depend on where the repo is checked out'); + assert.match(fromShort, /Needs Add a max-attempts cap\.$/); + assert.match(fromLong, /Needs Add a max-attempts cap\.$/); + assert.ok( + fromLong.includes(`[todo file](${TODO_RELATIVE_TAIL})`), + 'link must be the repo-relative tail, not the absolute path', + ); +}); + +test('renderPendingTodosMarkdown: already-relative path is byte-stable with and without projectRoot', () => { + const todo = makeTodo({ needs: 'define retry behavior' }); + const withRoot = renderPendingTodosMarkdown([todo], path.resolve('/', 'gsd-2618-rel-root')); + assert.equal(withRoot, renderPendingTodosMarkdown([todo])); +}); + +test('renderPendingTodosMarkdown: absolute path without projectRoot keeps the legacy absolute link and drop order', () => { + const base = path.join(path.resolve('/', 'gsd-2618-legacy-base'), LONG_BASE_SEGMENT); + const todo = makeTodo({ path: path.join(base, TODO_RELATIVE_TAIL), needs: 'a real needs clause' }); + const line = renderPendingTodosMarkdown([todo]); + // Opt-out callers keep today's behavior: over-cap drops needs first, link + // verbatim (raw separators — the legacy renderer never posix-normalizes). + assert.doesNotMatch(line, /Needs/); + assert.ok(line.includes(`[todo file](${todo.path})`)); +}); + +test('renderPendingTodosMarkdown: long base + long title still bounds the bullet and keeps the drop order', () => { + const base = path.join(path.resolve('/', 'gsd-2618-order-base'), LONG_BASE_SEGMENT); + const todo = makeTodo({ + path: path.join(base, TODO_RELATIVE_TAIL), + title: 'A'.repeat(300), + needs: 'something', + }); + const line = renderPendingTodosMarkdown([todo], base); + assert.ok(line.length <= MAX, `expected <= ${MAX}, got ${line.length}`); + assert.doesNotMatch(line, /Needs/, 'needs clause must be dropped before title is touched'); + assert.match(line, /…/); + assert.ok(line.includes(`[todo file](${TODO_RELATIVE_TAIL})`), 'relative link verbatim'); +}); + +test('renderPendingTodosMarkdown: absolute path outside projectRoot renders a ../ relative link', () => { + const root = path.resolve('/', 'gsd-2618-outside-root'); + const outsideTodo = path.join( + path.resolve('/', 'gsd-2618-outside-sibling'), + TODO_RELATIVE_TAIL, + ); + const line = renderPendingTodosMarkdown([makeTodo({ path: outsideTodo })], root); + // The ../-form legitimately CONTAINS the absolute path as a substring, so a + // plain includes() negative is a false positive — assert the property + // itself: the link target is relative, never the absolute path. + const linkMatch = line.match(/\[todo file\]\(([^)]*)\)/); + assert.ok(linkMatch, 'bullet must contain a todo link'); + assert.ok(!path.isAbsolute(linkMatch[1]), 'link target must be relative, not absolute'); + assert.notEqual(linkMatch[1], outsideTodo, 'link target must not be the machine-variable absolute path'); + assert.ok( + line.includes('[todo file](../'), + 'link must be relative to the project root, not absolute', + ); +}); + +test('renderPendingTodosMarkdown: path equal to projectRoot falls back to the raw link target', () => { + const root = path.resolve('/', 'gsd-2618-eq-root'); + const line = renderPendingTodosMarkdown([makeTodo({ path: root })], root); + assert.ok(line.includes(`[todo file](${root})`)); +}); + +test('renderPendingTodosMarkdown: non-string path keeps the empty-link fallback', () => { + const line = renderPendingTodosMarkdown( + [makeTodo({ path: 42 })], + path.resolve('/', 'gsd-2618-num-root'), + ); + assert.match(line, /\[todo file\]\(\)$/); +}); + // ─── pending_read_ok / pending_todos_markdown via the real CLI surface ───── const { spawnSync } = require('node:child_process'); @@ -228,6 +324,46 @@ test('cmdInitTodos: real todo file produces a rendered bullet via the CLI', (t) assert.match(json.pending_todos_markdown, /Needs Add a max-attempts cap\.$/m); }); +test('cmdInitTodos: needs clause survives a deterministically long base path (#4384 macOS shape)', (t) => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2618-long-')); + t.after(() => cleanup(root)); + // One long path segment: deterministic on every OS (no real macOS dependency). + // Pre-fix, the absolute-link bullet exceeds 240 chars even on Linux /tmp, + // reproducing next's macos shard failure (CI run 34038716700). + const dir = path.join(root, LONG_BASE_SEGMENT); + const pendingDir = path.join(dir, '.planning', 'todos', 'pending'); + fs.mkdirSync(pendingDir, { recursive: true }); + fs.writeFileSync( + path.join(pendingDir, '2026-09-01-fix-retry-logic.md'), + [ + '---', + 'created: 2026-09-01T00:00:00.000Z', + 'title: Fix retry logic', + 'area: api', + '---', + '', + '## Solution', + '', + 'Add a max-attempts cap.', + '', + ].join('\n'), + ); + + const json = runQueryInitTodos(dir); + assert.equal(json.pending_read_ok, true); + assert.equal(json.todo_count, 1); + assert.match(json.pending_todos_markdown, /Fix retry logic/); + assert.match(json.pending_todos_markdown, /Needs Add a max-attempts cap\.$/m); + assert.ok( + json.pending_todos_markdown.includes(`[todo file](${TODO_RELATIVE_TAIL})`), + 'bullet link must be repo-relative', + ); + assert.ok( + !json.pending_todos_markdown.includes(dir.split(path.sep).join('/')), + 'absolute cwd must not leak into the rendered bullet', + ); +}); + test('cmdInitTodos: bullet order is filename-sorted regardless of write/insertion order', (t) => { const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gsd-2618-')); t.after(() => cleanup(dir));