enhance(#2618): render pending todos as one bounded bullet per todo (#4384)

This commit is contained in:
Tom Boucher
2026-09-06 08:06:39 -04:00
committed by GitHub
parent 66e4034fe4
commit b7406b293f
8 changed files with 424 additions and 10 deletions

View File

@@ -12,6 +12,7 @@ import os from 'node:os';
import { execGit, platformWriteSync, platformReadSync, toNativePath, posixNormalize } from './shell-command-projection.cjs';
import { realClock } from './clock.cjs';
import { escapeRegex } from './pattern.cjs';
import { collectSection } from './markdown-sectionizer.cjs';
// eslint-disable-next-line @typescript-eslint/no-require-imports -- io.cjs is an export= CommonJS module
import io = require('./io.cjs');
// eslint-disable-next-line @typescript-eslint/no-require-imports -- config-loader.cjs is an export= CommonJS module
@@ -2233,15 +2234,118 @@ function cmdInitPhaseOp(cwd: string, phase: string, raw: boolean): void {
output(withProjectRoot(cwd, result), raw);
}
// #2618: bullet-cap and title-floor for renderPendingTodosMarkdown below.
// 240 matches the bound already vetted by maintainer review on the prior
// attempt at this issue (PR #2662) — re-deriving a different number would be
// pure bikeshedding, not a correctness improvement. See
// .gsd/phase/feat-2618-compact-todo-pointers/40-design.md.
const PENDING_TODO_BULLET_MAX_CHARS = 240;
const PENDING_TODO_TITLE_FLOOR = 15;
const PENDING_TODO_AREA_FLOOR = 3;
function sanitizePendingTodoInline(value: string): string {
// Defensive: the regex captures that populate title/area/needs can only
// ever match a single line, so this is belt-and-suspenders against any
// future non-regex-sourced input, not a reachable case today.
return value.replace(/[\r\n]+/g, ' ').trim();
}
function truncatePendingTodoText(value: string, maxLen: number): string {
if (value.length <= maxLen) return value;
if (maxLen <= 1) return value.slice(0, Math.max(0, maxLen));
return `${value.slice(0, maxLen - 1)}…`;
}
/**
* #2618: pure renderer for STATE.md's "### Pending Todos" section BODY (not
* the heading). One bullet per todo, each capped at
* PENDING_TODO_BULLET_MAX_CHARS. `gsd-core/workflows/add-todo.md` and
* `check-todos.md` splice this string in verbatim instead of free-hand
* editing STATE.md — see the design doc for why this is real, unit-tested
* code rather than a prose algorithm (DEFECT.GENERATIVE-FIX: a prose
* algorithm duplicated as a test oracle is exactly the divergence class
* this avoids).
*/
function renderPendingTodosMarkdown(todos: Record<string, unknown>[]): string {
if (!Array.isArray(todos) || todos.length === 0) {
return 'None yet.';
}
return todos.map((todo) => renderPendingTodoBullet(todo)).join('\n');
}
function pendingTodoFieldAsString(value: unknown, fallback: string): string {
return typeof value === 'string' && value.length > 0 ? value : fallback;
}
function renderPendingTodoBullet(todo: Record<string, unknown>): string {
const date = sanitizePendingTodoInline(pendingTodoFieldAsString(todo['created'], 'unknown'));
let area = sanitizePendingTodoInline(pendingTodoFieldAsString(todo['area'], 'general'));
let title = sanitizePendingTodoInline(pendingTodoFieldAsString(todo['title'], 'Untitled'));
// Strip trailing "." so the fixed "Needs ....` template below never
// produces a doubled period when the source text already ended in one.
let needs =
typeof todo['needs'] === 'string'
? sanitizePendingTodoInline(todo['needs']).replace(/\.+$/, '')
: '';
const link = `[todo file](${pendingTodoFieldAsString(todo['path'], '')})`;
const assemble = (): string => {
const needsClause = needs ? ` — Needs ${needs}.` : '';
return `- [${date}] [${area}] ${title} — ${link}${needsClause}`;
};
let line = assemble();
if (line.length <= PENDING_TODO_BULLET_MAX_CHARS) return line;
// 1) Drop the needs clause entirely first — date/area/title/link untouched.
needs = '';
line = assemble();
if (line.length <= PENDING_TODO_BULLET_MAX_CHARS) return line;
// 2) Shorten the title next, down to a floor — date/area/link untouched.
const titleOverage = line.length - PENDING_TODO_BULLET_MAX_CHARS;
const targetTitleLen = Math.max(PENDING_TODO_TITLE_FLOOR, title.length - titleOverage);
if (targetTitleLen < title.length) {
title = truncatePendingTodoText(title, targetTitleLen);
line = assemble();
}
if (line.length <= PENDING_TODO_BULLET_MAX_CHARS) return line;
// 3) Shorten area as a last resort — date and the markdown link are never
// altered (link correctness > strict cap; see design doc "Known limits").
const areaOverage = line.length - PENDING_TODO_BULLET_MAX_CHARS;
const targetAreaLen = Math.max(PENDING_TODO_AREA_FLOOR, area.length - areaOverage);
if (targetAreaLen < area.length) {
area = truncatePendingTodoText(area, targetAreaLen);
line = assemble();
}
return line;
}
function cmdInitTodos(cwd: string, area: string | undefined, raw: boolean): void {
const config = loadConfig(cwd);
const pendingDir = path.join(planningDir(cwd), 'todos', 'pending');
let count = 0;
const todos: Record<string, unknown>[] = [];
// #2618: distinct from "genuinely zero pending todos" — false only when
// readdirSync itself failed for a reason OTHER than the directory simply
// not existing yet (ENOENT), mirroring the ENOENT-vs-other-errno split
// already used above in this file (#3885, ADR-3473 §8.5). Without this,
// a real I/O/permission error on the pending dir would look identical to
// "no pending todos" and could wipe an existing, non-empty Pending Todos
// section in STATE.md on refresh — the fail-safe requirement for #2618.
let pendingReadOk = true;
try {
const files = fs.readdirSync(pendingDir).filter((f) => f.endsWith('.md'));
// #2618: sorted so pending_todos_markdown's bullet order is stable across
// runs — readdirSync's order is filesystem-dependent, not contractually
// stable, and an unstable order would reorder every bullet on an
// unrelated re-render, turning a one-line git diff into a full-section
// rewrite (must-have #3). Filenames are `YYYY-MM-DD-slug.md`, so this
// also yields a sensible chronological order as a side effect.
const files = fs.readdirSync(pendingDir).filter((f) => f.endsWith('.md')).sort();
for (const file of files) {
const content = platformReadSync(path.join(pendingDir, file));
if (content === null) continue;
@@ -2252,6 +2356,21 @@ function cmdInitTodos(cwd: string, area: string | undefined, raw: boolean): void
// #2337: kept in parity with cmdListTodos — surface severity when
// present, omit the key entirely for todos with no severity line.
const severityMatch = content.match(/^severity:\s*(.+)$/m);
// #2618: first non-empty line of the `## Solution` body, used as the
// bullet's "Needs ..." clause. "TBD" (the create_file template's own
// placeholder for an unresolved solution) renders no clause at all
// rather than the useless literal "Needs TBD.".
const solutionSection = collectSection(content, (h) => h.level === 2 && h.text.trim() === 'Solution');
let needs: string | undefined;
if (solutionSection) {
const firstLine = solutionSection.body
.split('\n')
.map((l) => l.trim())
.find((l) => l.length > 0);
if (firstLine && firstLine.toUpperCase() !== 'TBD') {
needs = firstLine;
}
}
const todoArea = areaMatch ? areaMatch[1].trim() : 'general';
if (area && todoArea !== area) continue;
@@ -2265,13 +2384,17 @@ function cmdInitTodos(cwd: string, area: string | undefined, raw: boolean): void
// #2376: absolute — see comment on phase_dir in cmdInitExecutePhase.
path: toPosixPath(path.join(planningDir(cwd), 'todos', 'pending', file)),
...(severityMatch ? { severity: severityMatch[1].trim() } : {}),
...(needs ? { needs } : {}),
});
} catch {
/* intentionally empty */
}
}
} catch {
/* intentionally empty */
} catch (err) {
const code = (err as NodeJS.ErrnoException)?.code;
if (code !== 'ENOENT') {
pendingReadOk = false;
}
}
const result: Record<string, unknown> = {
@@ -2291,6 +2414,13 @@ function cmdInitTodos(cwd: string, area: string | undefined, raw: boolean): void
planning_exists: fs.existsSync(planningDir(cwd)),
todos_dir_exists: fs.existsSync(path.join(planningDir(cwd), 'todos')),
pending_dir_exists: fs.existsSync(path.join(planningDir(cwd), 'todos', 'pending')),
// #2618: see PENDING_TODO_BULLET_MAX_CHARS comment / design doc. Consumed
// by add-todo.md / check-todos.md's update_state step; omitted entirely
// (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) } : {}),
};
output(withProjectRoot(cwd, result), raw);
@@ -4188,4 +4318,5 @@ export = {
cmdAgentSkills,
buildSkillManifest,
cmdSkillManifest,
renderPendingTodosMarkdown,
};