diff --git a/README.md b/README.md index a4c9507..8d78490 100644 --- a/README.md +++ b/README.md @@ -61,19 +61,22 @@ this table and the live help always agree on the registered set. | `/pygienium-deep-modules` | deep-modules | `deep-modules` | Detect shallow modules (pass-throughs, trivial wrappers, re-export barrels) and consolidate the safe ones. | | `/pygienium-dead-code` | dead-code | `scanner` / `fixer` | Find unreferenced exports, dead files, obsolete compat shims, migration helpers, and unused dependencies; remove clearly-dead items and flag dynamic ones. | | `/pygienium-defensive-guards` | defensive-guards | `defensive-guards` | Remove redundant defensive guarding (null checks on non-nullable types, swallowing try/catch, masking fallbacks) while keeping boundary guards (IO, parsing, untrusted input). | -| `/pygienium-noop` | noop | `scanner` / `fixer` | Reference/template check — performs no analysis, writes zero-issue artifacts. Clone it to start a new check. | +| `/pygienium-todos` | todos | `todos` / `fixer` | Inventory TODO/FIXME markers and stub implementations; with `--fix`, convert silent stubs (placeholder returns, empty bodies) into loud failures — never implementing TODOs or deleting markers. | ## Artifacts -Each check writes its report under `/pygienium/checks//`: +Every check writes its reports under `/.pygienium/checks//` (run +state lives at `/.pygienium/run-state.json`): - `findings.md` — what the scan found (per-file line refs). - `changes.md` — what the fix phase changed + anything deferred for human review. `/pygienium-export` merges every check's artifacts into one -`pygienium/export.md` (or `export.json`). Some checks historically wrote under -`.pygienium/checks/`; export walks **both** roots and merges by check name -(`pygienium/` wins). +`.pygienium/export.md` (or `export.json`). + +On first run in a git work tree, pygienium appends `.pygienium/` to the +target repo's `.gitignore` so a run never stages its own state/artifacts into +git (opt out with `--no-gitignore`). ## Adding a check @@ -82,7 +85,7 @@ One file + one `registerCheck()` call. **No `index.ts` command-wiring changes.** startup, so a new file self-registers and `/pygienium-` appears automatically. -1. Clone `src/checks/noop.ts` → `src/checks/.ts`. +1. Create `src/checks/.ts` from the template below. 2. Edit the `name`, `label`, `description`, the rubric in the scan/fix task builders, and the `gate` precondition. 3. Keep the trailing `registerCheck(Check)`. Done. diff --git a/agents/todos.md b/agents/todos.md new file mode 100644 index 0000000..fa57e1a --- /dev/null +++ b/agents/todos.md @@ -0,0 +1,181 @@ +--- +name: todos +allowedTools: + - read + - grep + - find + - ls + - bash + - write +--- +You are the **Pygienium todos scanner** sub-agent — an unfinished-work analyst. + +# Your role + +You run the "TODOs & stubs" check against a target path. Given a deterministic +pre-scan candidate list, you verify each candidate, drop noise, classify what +remains into **markers**, **silent stubs**, and **loud stubs**, and write a +structured findings report to disk. You do NOT fix anything — that is the +fixer's job. You only inspect, verify, and report. + +# The three categories + +## 1. Marker — a note that work is unfinished + +A `TODO` / `FIXME` / `HACK` / `XXX` / `@todo` token in a comment (or a string +that acts as one). Track these; never delete or implement them. + +## 2. Silent stub — the dangerous ones + +A function that silently returns a placeholder instead of doing its job. It +compiles, it runs, it hands back a wrong-but-plausible value — so nothing +fails loudly, and callers ship the lie. Detect (among others): + +- a body that is only a placeholder return: `return 0;` / `return "";` / + `return null;` / `return [];` / `return {};` / `return None;` / `return nil` +- an empty body: `function foo() {}` (or a brace pair with only comments/ws) +- a Python `pass`-only body: `def f(...): pass` (or `pass` as the only body + statement) +- single-line placeholders: `() => 0`, `function x() { return null; }` +- an obvious hardcoded default with a stub intent ("TODO" marker sitting + directly above, or a comment saying `placeholder` / `stub` / `dummy`) + +## 3. Loud stub — already failing loudly (tracked debt) + +An explicit not-implemented failure. It is honest debt: the code already +throws/panics, so no caller silently ships a wrong value. Detect (among others): + +- `throw new Error("Not implemented")` and variants (`not implemented yet`, + `NotImplementedError`, `NotImplementedException`) — JS/TS, C#, Java +- `raise NotImplementedError` — Python (see noise list for the abstract-method + exception) +- `todo!()` / `todo!("msg")` / `unimplemented!()` — Rust +- `TODO("...")` / `TODO()` — Kotlin +- `panic!("not implemented")` — Go, Rust + +Report loud stubs as tracked debt. They are usually fine to keep while the +work is genuinely in progress; the fixer does NOT touch them. + +# Noise — drop these without reporting + +- `TODO` inside a **string literal** that is not an intent marker + (e.g. `const op = "TODO";`). +- Marker text inside **doc examples or docstrings** that merely illustrate + syntax (`// TODO: not real code` in a comment block that quotes examples). +- **Fixture/generated files**: filenames matching `*.todo.*` / `*.fixture.*`, + snapshots, scaffolds, vendored code (scope rules exclude most already). +- **Correct idioms that look like stubs**: + - `raise NotImplementedError` in an **abstract base class / abstractmethod** + (Python) — that is the idiomatic way to declare an interface method. + - `abstract` methods without bodies (Java, Kotlin, C#) — not stubs. + - a legitimately tiny function that returns a default BY DESIGN + (e.g. a reducer that sums and can naturally return 0, `indexOf` returning + -1, a cache miss returning `null`). Check the surrounding semantics, not + just the shape: a lone placeholder return inside a `catch` for an IO error + is a boundary handler, not a stub. +- Markers in languages/code the project doesn't own (vendored dirs). + +# Operating contract + +- Operate only within the target path given in the task. +- Use `read`, `grep`, `find`, `ls` to inspect; cross-check a candidate's + declarations/context before classifying (is the function called anywhere? + is the class abstract? is the `return null` a catch handler?). +- `bash` is for read-only inspection only (`grep -n`, `wc`, `git ls-files`, + `cat`). Never mutate source files. +- `write` is ONLY for writing your findings report to the output path named in + the task (under the project's `.pygienium/` state directory). Never `write` + source files. + +# Scope of inspection + +**Only inspect implementation source files.** Do not analyse documentation, +config, type declarations, build output, or dependencies — flagging those is +noise the user cannot act on. + +## Inspect (extensions) + +`.cs`, `.cjs`, `.go`, `.java`, `.js`, `.jsx`, `.kt`, `.lua`, `.mjs`, `.php`, +`.py`, `.rb`, `.rs`, `.swift`, `.ts`, `.tsx` + +## Skip (directory names — never descend into) + +`.cache`, `.git`, `.hg`, `.idea`, `.next`, `.nuxt`, `.pygienium`, `.ralpi`, +`.svelte-kit`, `.svn`, `.turbo`, `.vscode`, `__pycache__`, `build`, `coverage`, +`dist`, `node_modules`, `out`, `vendor`, `venv` (and `.venv`) + +## Skip (file patterns) + +- Type declarations: `*.d.ts`, `*.d.mts`, `*.d.cts` — generated contracts, not impl +- Minified bundles: `*.min.js`, `*.min.mjs`, `*.min.cjs` +- Docs: `*.md`, `*.txt`, `*.rst` — prose, not code +- Config: `*.json`, `*.yaml`, `*.yml`, `*.toml`, `*.ini`, `*.env` +- Styles/markup: `*.css`, `*.scss`, `*.html`, `*.svg` +- Lock files: `package-lock.json`, `*.lock`, `bun.lockb` + +## File discovery preference + +1. Prefer the recon snapshot at `/.pygienium/recon.json` when it exists. +2. Otherwise enumerate files yourself, applying the rules above. + +# Output + +Write your full findings report to the **findings path** given in the task +(typically `/.pygienium/checks/todos/findings.md`). + +`findings.md` MUST begin with a machine-readable summary line, then the three +sections. Format: + +```markdown +# TODOs & stubs findings + +summary: marker(s), silent stub(s), loud stub(s) | new: | resolved: | reviewed: + +## TODO markers + +### 1. : — +- marker: TODO | FIXME | HACK | XXX | @todo +- context: +- disposition: track | drop-noise + +## Silent stubs (actionable) + +### 1. : +- function: (or the file when unnamed) +- stub: +- disposition: convert-to-loud | keep (not a stub) + +## Loud stubs (already failing loudly — tracked debt) + +### 1. : +- kind: throw-not-implemented | raise-NotImplementedError | todo! | TODO() | panic-not-implemented +- disposition: track | drop-noise +``` + +`new`/`resolved` are computed against the previous run's verified counts when +the task tells you them; when the task does not provide a previous baseline, +report what the deterministic pre-scan computed and mark it `(tentative)`. +`reviewed` is the number of candidates you actually verified. + +If the target is clean, write: + +```markdown +# TODOs & stubs findings + +summary: 0 marker(s), 0 silent stub(s), 0 loud stub(s) | new: 0 | resolved: | reviewed: + +No TODOs or stubs detected. +``` + +After writing `findings.md`, emit a terse one-line summary as your final +message in EXACTLY this parseable form: + +``` +todos: silent stub(s), loud stub(s), marker(s) — see +``` + +# Tone + +Precise and terse. Every silent stub needs one line of evidence (the +placeholder body) and a disposition. Never invent counts — verify before you +write. diff --git a/src/checks/noop.ts b/src/checks/noop.ts deleted file mode 100644 index 82a16bc..0000000 --- a/src/checks/noop.ts +++ /dev/null @@ -1,121 +0,0 @@ -/** - * checks/noop.ts — the reference check and living extensibility template. - * - * This is the smallest complete `CheckDefinition`: it self-registers on import, - * passes its gate for any real file/dir target, and asks the scanner/fixer - * sub-agents to write empty `findings.md`/`changes.md` artifacts. It exists so - * that: - * - * 1. The "add a check = one file in `checks/` + one `registerCheck()` call, - * zero `index.ts` changes" claim has a verifiable witness — `index.ts` - * auto-discovers every `checks/*.ts` (except the registry barrel), so this - * file makes `/pygienium-noop` appear with no wiring edits. - * 2. New check authors have a copy-paste starting point: clone this file, - * rename, swap the rubric, ship. - * - * Artifacts (under `/pygienium/checks/noop/`): - * - `findings.md` — `noop: 0 issues` - * - `changes.md` — `noop: 0 edits` - * - * @module pygienium/checks/noop - */ - -import { registerCheck, type CheckScope } from "./registry.js"; - -/** Directory this check writes its artifacts to. */ -function noopArtifactDir(scope: CheckScope): string { - return `${scope.cwd.replace(/\/+$/, "")}/pygienium/checks/noop`; -} - -/** - * Analysis task: scan nothing meaningful, write a zero-issue findings report. - * Mirrors the structure a real check's scan task uses so this file reads as a - * faithful template. - */ -function buildNoopScanTask(_cwd: string, scope: CheckScope): string { - const outDir = noopArtifactDir(scope); - return `# Task: noop scan - -You are running the **noop** hygiene check (a no-op reference check). - -## Target -- Scan target: \`${scope.target}\` - -## What to do -1. Confirm the target is reachable (no real analysis is needed). -2. Write a findings report to \`${outDir}/findings.md\` with the content: - - \`\`\`markdown - # noop — findings - - noop: 0 issues - \`\`\` - -3. Return that report text as your final message so the host records it as the - analysis-phase findings. - -If the target is missing, write \`noop: target missing\` to findings.md instead. -`; -} - -/** Fix task: apply zero edits, write an empty changes report. */ -function buildNoopFixTask( - _cwd: string, - scope: CheckScope, - _findings: string, -): string { - const outDir = noopArtifactDir(scope); - return `# Task: noop fix - -You are running the **noop** hygiene fix phase. - -## Target -- Fix target: \`${scope.target}\` - -## What to do -1. Make no source edits (this is a no-op check). -2. Write a changes report to \`${outDir}/changes.md\` with the content: - - \`\`\`markdown - # noop — changes - - noop: 0 edits - \`\`\` - -3. Return that report text as your final message. -`; -} - -/** - * Precondition gate: pass when the target path exists as a file or directory. - * Idempotent — used for both the pre-analysis check and the post-fix verify. - */ -async function noopGate(cwd: string): Promise { - const { stat } = await import("node:fs/promises"); - const { resolve } = await import("node:path"); - const target = resolve(cwd); - try { - const s = await stat(target); - if (s.isDirectory() || s.isFile()) return undefined; - return `target is not a file or directory: ${target}`; - } catch { - return `target path does not exist: ${target}`; - } -} - -/** The noop check definition. Self-registers on import. */ -export const noopCheck = { - name: "noop", - label: "No-op", - description: - "Reference/template check — performs no analysis, writes zero-issue artifacts. Clone it to start a new check.", - agentName: "scanner", - fixAgentName: "fixer", - phaseId: "noop", - buildScanTask: buildNoopScanTask, - buildFixTask: buildNoopFixTask, - gate: noopGate, -} as const; - -// Self-register on import so index.ts auto-discovery picks it up — no wiring. -registerCheck(noopCheck); diff --git a/src/checks/todos.ts b/src/checks/todos.ts new file mode 100644 index 0000000..6cbe949 --- /dev/null +++ b/src/checks/todos.ts @@ -0,0 +1,563 @@ +/** + * checks/todos.ts — "TODOs & stubs" check. + * + * Inventories unfinished work: TODO/FIXME/HACK markers and stub + * implementations. The engineering rule encoded in the fix phase: pygienium + * never *implements* a TODO and never deletes a marker — the fixer's only + * action is to convert **silent stubs** (placeholder returns, empty bodies, + * pass-only bodies) into loud failures, because a stub that silently returns + * a plausible-but-wrong value ships the lie to every caller, while a stub + * that throws is honest tracked debt. + * + * Classification (the scan agent applies judgment; a deterministic pre-scan + * feeds it candidates): + * - marker — `TODO` / `FIXME` / `HACK` / `XXX` / `@todo` in a comment. + * - silent-stub — lone placeholder return / empty body / pass-only body; + * the actionable, dangerous ones. + * - loud-stub — explicit not-implemented failures (`throw new Error("Not + * implemented")`, `todo!()`, `raise NotImplementedError`, `TODO("...")`); + * already failing loudly → tracked debt, fixer never touches them. + * - noise (dropped by the agent) — "TODO" inside a string literal, doc + * examples, fixtures, abstract-method `NotImplementedError` (the correct + * Python idiom), legit default returns (reducers, indexOf -1, catch + * handlers returning null). + * + * Lifecycle: + * gate (need source files) → recon (shared) → async scan task runs a + * deterministic candidate pass over the scope tree, diffs the counts + * against the previous run's findings (stored in run-state), hands the + * candidates + delta to the `todos` agent, which verifies/drops noise and + * writes `/.pygienium/checks/todos/findings.md` → [with --fix] fixer + * converts silent stubs to loud throws and writes `changes.md`. + * + * Registering this file is the ONLY wiring needed: `index.ts` auto-discovers + * `src/checks/*.ts`, so dropping this file exposes `/pygienium-todos`. + * + * @module pygienium/checks/todos + */ + +import { readdirSync } from "node:fs"; +import { readFile, readdir, stat } from "node:fs/promises"; +import { join, relative } from "node:path"; +import { loadRunState } from "../run-state.js"; +import { + registerCheck, + type CheckDefinition, + type CheckScope, +} from "./registry.js"; +import { + isScopeSource, + SCOPE_EXCLUDE_DIRS, + scopeRulesMarkdown, +} from "./scope.js"; + +/** Output directory for this check's persistent reports. */ +export function todosOutputDir(cwd: string): string { + return join(cwd, ".pygienium", "checks", "todos"); +} + +/** `findings.md` path for this check. */ +export function findingsPath(cwd: string): string { + return join(todosOutputDir(cwd), "findings.md"); +} + +/** `changes.md` path for this check. */ +export function changesPath(cwd: string): string { + return join(todosOutputDir(cwd), "changes.md"); +} + +export type TodoKind = "marker" | "silent-stub" | "loud-stub"; + +/** One candidate line the deterministic pre-scan flagged. */ +export interface TodoCandidate { + /** Absolute path of the file. */ + path: string; + /** 1-based line number. */ + line: number; + kind: TodoKind; + /** Matched token (e.g. `TODO`, `Not implemented`, `empty-body`). */ + snippet: string; + /** The trimmed line content. */ + code: string; + /** Enclosing function name when one was seen, else the file path. */ + context: string; +} + +/** + * Marker tokens: an unfinished-work note in a comment. Case-insensitive; + * `@todo\b` (not `@todos`) and `\bHACK\b` (not `hacking`). + */ +const MARKER_RE = /\b(?:TODO|FIXME|HACK)\b|\bXXX\b|@todo\b/i; + +/** + * Loud-stub tokens: explicit not-implemented failures. `not implemented` + * covers `throw new Error("Not implemented")` and `panic!("not implemented")`; + * the `NotImplementedError` branch also catches Python's `raise + * NotImplementedError`, and the Rust/Kotlin idioms (`todo!()`, `TODO("...")`) + * are matched explicitly. + */ +const LOUD_STUB_RE = + /not\s+implemented|NotImplementedError|NotImplementedException|\btodo!\s*\(|unimplemented!\s*\(|\bTODO\s*\(/i; + +/** A lone placeholder return (`return 0;` / `return "";` / `return null;` …). */ +const PLACEHOLDER_RETURN_RE = + /^\s*return\s+(?:null|undefined|0(?:\.0)?|""|''|\[\]|\{\}|None|nil|false)\s*;?\s*(?:\/\/.*)?$/; + +/** Function/arrow header lines worth inspecting for a stub body. */ +const FN_HEADER_RE = + /\b(?:function|def|func|fun|fn)\s+\w+|(?:const|let|var)\s+\w+\s*=\s*(?:async\s*)?\s*(?:\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>/; + +/** Single-line placeholder body: `function x() { return 0; }`. */ +const SINGLE_PLACEHOLDER_BODY_RE = + /\{\s*return\s+(?:null|undefined|0(?:\.0)?|""|''|\[\]|\{\}|None|nil|false)\s*;?\s*\}/; + +/** Single-line arrow expression body: `const f = () => 0;`. */ +const ARROW_PLACEHOLDER_RE = + /=>\s*(?:null|undefined|0(?:\.0)?|""|''|\[\]|\{\}|false)\s*;?\s*$/; + +/** Empty single-line body: `function notify(): void {}`. */ +const EMPTY_BODY_RE = /\{\s*\}/; + +/** Hard cap on candidates so a huge tree can't blow the task prompt. */ +const MAX_CANDIDATES = 500; +/** Candidate sections are truncated at this many entries in the fallback. */ +const FALLBACK_CAP = 16; +/** Candidate list embedded in the live prompt is truncated at this many. */ +const PROMPT_CAP = 40; + +/** Extract the declared function name from a header line, when present. */ +function headerName(line: string): string | undefined { + const decl = + /(?:function|def|func|fun|fn|class)\s+([A-Za-z_$][\w$]*)/.exec(line) ?? + /(?:const|let|var)\s+([A-Za-z_$][\w$]*)\s*=\s*(?:async\s*)?\s*(?:\([^)]*\)|[A-Za-z_$][\w$]*)\s*=>/.exec( + line, + ); + return decl?.[1]; +} + +/** Index of the next non-blank line at or after `start`, else `undefined`. */ +function nextNonBlank(lines: string[], start: number): number | undefined { + for (let i = start; i < lines.length; i++) { + if ((lines[i] as string).trim()) return i; + } + return undefined; +} + +/** + * Walk the target collecting implementation-source files, honouring + * {@link SCOPE_EXCLUDE_DIRS} and {@link isScopeSource} (same rules as + * dead-code's walker). + */ +async function walkScopeFiles(root: string): Promise { + const st = await stat(root).catch(() => undefined); + if (!st) return []; + if (st.isFile()) return isScopeSource(root) ? [root] : []; + const out: string[] = []; + const stack = [root]; + while (stack.length > 0) { + const dir = stack.pop() as string; + let entries; + try { + entries = await readdir(dir, { withFileTypes: true }); + } catch { + continue; + } + for (const entry of entries) { + const full = join(dir, entry.name); + if (entry.isDirectory()) { + if (SCOPE_EXCLUDE_DIRS.has(entry.name)) continue; + stack.push(full); + } else if (entry.isFile() && isScopeSource(entry.name)) { + out.push(full); + } + } + } + return out.sort(); +} + +/** + * Deterministic pre-scan: flag marker/loud-stub/silent-stub candidates across + * the target's scope tree. High recall by design — the scan agent verifies + * each candidate and drops noise (in-string "TODO", doc examples, legit + * default returns). Pure function of the tree: unit-testable without agents. + */ +export async function detectTodoStubs( + target: string, +): Promise { + const files = await walkScopeFiles(target); + const out: TodoCandidate[] = []; + for (const file of files) { + const raw = await readFile(file, "utf8").catch(() => ""); + const lines = raw.split("\n"); + let lastFn = ""; + for (let i = 0; i < lines.length; i++) { + const code = lines[i] as string; + const trimmed = code.trim(); + if (!trimmed) continue; + + const loud = LOUD_STUB_RE.exec(trimmed); + if (loud) { + out.push({ + path: file, + line: i + 1, + kind: "loud-stub", + snippet: loud[0].slice(0, 40), + code: trimmed, + context: lastFn, + }); + continue; + } + const marker = MARKER_RE.exec(trimmed); + if (marker) { + out.push({ + path: file, + line: i + 1, + kind: "marker", + snippet: marker[0], + code: trimmed, + context: lastFn, + }); + continue; + } + + if (FN_HEADER_RE.test(trimmed)) { + const name = headerName(trimmed); + if (name) lastFn = name; + const ctx = name ?? lastFn; + // Single-line stub forms. + if ( + SINGLE_PLACEHOLDER_BODY_RE.test(trimmed) || + ARROW_PLACEHOLDER_RE.test(trimmed) + ) { + out.push({ + path: file, + line: i + 1, + kind: "silent-stub", + snippet: "placeholder-return", + code: trimmed, + context: ctx, + }); + continue; + } + if ( + EMPTY_BODY_RE.test(trimmed) && + !/\b(?:return|throw)\b/.test(trimmed) + ) { + out.push({ + path: file, + line: i + 1, + kind: "silent-stub", + snippet: "empty-body", + code: trimmed, + context: ctx, + }); + continue; + } + // Multi-line forms: inspect the first non-blank body line. + const bodyIdx = nextNonBlank(lines, i + 1); + if (bodyIdx === undefined) continue; + const body = (lines[bodyIdx] as string).trim(); + if (body === "}") { + out.push({ + path: file, + line: i + 1, + kind: "silent-stub", + snippet: "empty-body", + code: trimmed, + context: ctx, + }); + } else if (body === "pass") { + out.push({ + path: file, + line: bodyIdx + 1, + kind: "silent-stub", + snippet: "pass-only", + code: body, + context: ctx, + }); + } else if (PLACEHOLDER_RETURN_RE.test(body)) { + // Lone placeholder return: the statement after it must be the + // closing brace (a `try/catch { return null }` handler does not + // match — its `return null` is followed by `}` inside `catch`). + const after = nextNonBlank(lines, bodyIdx + 1); + if (after !== undefined && (lines[after] as string).trim() === "}") { + out.push({ + path: file, + line: bodyIdx + 1, + kind: "silent-stub", + snippet: "placeholder-return", + code: body, + context: ctx, + }); + } + } + } + } + if (out.length >= MAX_CANDIDATES) break; + } + return out; +} + +/** Counts by kind across a candidate list. */ +function countByKind(candidates: TodoCandidate[]): { + silent: number; + loud: number; + marker: number; +} { + let silent = 0; + let loud = 0; + let marker = 0; + for (const c of candidates) { + if (c.kind === "silent-stub") silent++; + else if (c.kind === "loud-stub") loud++; + else marker++; + } + return { silent, loud, marker }; +} + +/** Previous run's verified counts, parsed from run-state findings text. */ +const PRIOR_SUMMARY_RE = + /todos:\s*(\d+)\s+silent\s+stub\(s\)?,\s*(\d+)\s+loud\s+stub\(s\)?,\s*(\d+)\s+marker\(s\)?/; + +/** + * Parse the previous run's per-kind counts out of run-state (the scan agent's + * one-line summary is persisted there). `undefined` when there is no prior + * run or the stored summary isn't parseable — the delta is then all-new. + */ +export async function todosPriorCounts( + cwd: string, +): Promise<{ silent: number; loud: number; marker: number } | undefined> { + const state = await loadRunState(cwd).catch(() => undefined); + const stored = state?.checks["todos"]?.findings; + if (!stored) return undefined; + const m = PRIOR_SUMMARY_RE.exec(stored); + if (!m) return undefined; + return { + silent: Number(m[1]), + loud: Number(m[2]), + marker: Number(m[3]), + }; +} + +/** + * Render the deterministic report the fake runner writes (and the real agent + * uses as a shape reference): one pipe-separated line, sections per kind. + */ +function renderFindings( + cwd: string, + candidates: TodoCandidate[], + delta: { nw: number; resolved: number }, +): string { + const { silent, loud, marker } = countByKind(candidates); + const total = silent + loud + marker; + const parts = [ + `# TODOs & stubs findings | summary: ${marker} marker(s), ${silent} silent stub(s), ${loud} loud stub(s) | new: ${delta.nw} | resolved: ${delta.resolved} | reviewed: ${candidates.length}`, + ]; + if (total === 0) { + parts.push( + "No TODOs or stubs detected (deterministic pre-scan reviewed all inspected source).", + ); + return parts.join(" | "); + } + const byKind: Record = { + marker: [], + "silent-stub": [], + "loud-stub": [], + }; + for (const c of candidates) byKind[c.kind].push(c); + const dump = (title: string, list: TodoCandidate[]): void => { + parts.push(`## ${title}`); + list.slice(0, FALLBACK_CAP).forEach((c, i) => { + parts.push( + `### ${i + 1}. ${relative(cwd, c.path)}:${c.line} — ${c.code} | snippet: ${c.snippet} | context: ${c.context || relative(cwd, c.path)}`, + ); + }); + if (list.length > FALLBACK_CAP) { + parts.push(`... and ${list.length - FALLBACK_CAP} more (truncated)`); + } + }; + dump("TODO markers", byKind.marker); + dump("Silent stubs (actionable)", byKind["silent-stub"]); + dump( + "Loud stubs (already failing loudly — tracked debt)", + byKind["loud-stub"], + ); + return parts.join(" | "); +} + +/** + * Gate: skip when the cwd has no inspectable source files at all — a workspace + * with zero source files gives the scanner nothing to analyse. + */ +function todosGate(cwd: string): string | undefined { + let found = false; + try { + const entries = readdirSync(cwd); + for (const entry of entries) { + if (isScopeSource(entry)) { + found = true; + break; + } + } + } catch { + // unreadable cwd → let the agent decide; don't block. + return undefined; + } + if (!found) { + return "no source files found to inspect"; + } + return undefined; +} + +/** + * Build the scan task. Deterministic pre-scan (async, like dead-code's) finds + * candidates and diffs them against the previous run's counts; the `todos` + * agent verifies each candidate, drops noise, and writes the verified report + * to `findings.md`. The output path is passed into the task so both the real + * agent (which uses its `write` tool) and the deterministic fake runner + * (which understands `!write `) persist to the same location. + */ +export async function buildTodosScanTask( + cwd: string, + scope: CheckScope, +): Promise { + const findings = findingsPath(cwd); + const target = scope.target; + const candidates = await detectTodoStubs(target); + const prior = await todosPriorCounts(cwd); + const { silent, loud, marker } = countByKind(candidates); + const prevTotal = prior ? prior.silent + prior.loud + prior.marker : 0; + const total = silent + loud + marker; + const nw = Math.max(0, total - prevTotal); + const resolved = Math.max(0, prevTotal - total); + const report = renderFindings(cwd, candidates, { nw, resolved }); + + const candidateList = candidates + .slice(0, PROMPT_CAP) + .map((c) => ` - ${relative(cwd, c.path)}:${c.line} [${c.kind}] ${c.code}`); + if (candidates.length > PROMPT_CAP) { + candidateList.push( + ` - … and ${candidates.length - PROMPT_CAP} more (truncated for brevity)`, + ); + } + + return [ + `Inspect the target "${target}" (cwd: ${cwd}) for unfinished work: TODO markers and stub implementations.`, + `A deterministic pre-scan found ${candidates.length} candidate line(s). Verify each candidate:`, + ...candidateList, + ``, + `Classify against the todos rubric: markers (TODO/FIXME/HACK/XXX/@todo), silent stubs`, + `(placeholder return / empty body / pass-only body), loud stubs (not-implemented`, + `throws, todo!(), TODO("..."), raise NotImplementedError).`, + `Drop noise: "TODO" inside a string literal, doc examples, fixtures,`, + `abstract-method NotImplementedError (correct Python idiom), and legit default`, + `returns (a reducer returning 0, indexOf returning -1, a catch handler returning null).`, + `Previous run reported: ${ + prior + ? `${prior.silent} silent, ${prior.loud} loud, ${prior.marker} marker` + : "none (first run)" + }.`, + `Write your full verified report to: ${findings}.`, + `findings.md must begin with the machine-readable summary line, then the three`, + `sections (## TODO markers / ## Silent stubs (actionable) / ## Loud stubs ...),`, + `each entry with file:line, evidence, and disposition. The summary line MUST be:`, + `summary: marker(s), silent stub(s), loud stub(s) | new: | resolved: | reviewed: `, + ``, + scopeRulesMarkdown(), + ``, + `# Deterministic fallback (executed by the fake runner in tests — verify every item yourself;`, + `# do not copy the counts below blindly):`, + `!write ${findings} ${report}`, + `!echo todos: ${silent} silent stub(s), ${loud} loud stub(s), ${marker} marker(s) — see ${findings}`, + ].join("\n"); +} + +/** + * Build the fix task. The fixer converts ONLY silent stubs into loud failures, + * per language idiom, preserving signature/exports; records every conversion + * (and any kept-with-reason) in `changes.md`. Markers are never implemented + * or deleted; loud stubs are never touched. + */ +function buildTodosFixTask( + cwd: string, + scope: CheckScope, + findings: string, +): string { + const changes = changesPath(cwd); + const target = scope.target; + return [ + `Convert silent stubs to loud failures (the fix phase of the todos check).`, + `cwd: ${cwd} target: ${target}`, + `Scan summary (also persisted at ${findingsPath(cwd)}):`, + `---`, + findings, + `---`, + ``, + `Rules:`, + `- Convert ONLY silent stubs. For each, replace the placeholder body with an explicit`, + ` loud failure naming the function, using the project's language idiom:`, + ` TS/JS/C#/Java: throw new Error("todos: () is a stub");`, + ` Python: raise NotImplementedError(" is a stub")`, + ` Go: panic("todos: is a stub")`, + ` Rust: todo!(" is a stub")`, + ` generic: throw new Error("todos: is a stub")`, + `- Preserve the signature, exports, async-ness, and type shape of the function.`, + `- Leave the original placeholder as a comment directly above the throw, and add a`, + ` note that the stub was made loud by pygienium.`, + `- NEVER implement TODOs, NEVER delete unresolved markers, NEVER touch loud stubs`, + ` (they already fail loudly), NEVER touch code that is not a verified silent stub.`, + `- If a candidate turned out NOT to be a stub (a legit default return), keep it and`, + ` record it as kept-with-reason in changes.md.`, + `- Apply the smallest possible diff; preserve tests and conventions.`, + `- Write changes.md to ${changes} listing every conversion (auto) or keep (reason).`, + ``, + `# Deterministic conversion (executed by the fake runner in tests):`, + `# getPrice() + notify() in stubs.ts converted from silent placeholders to loud throws.`, + `# NOTE: the fallback writes one physical line (the fake runner takes the rest of the !write`, + `# line as file content); trailing // comments keep the single line valid source.`, + `!write ${target}/stubs.ts export function getPrice(): number { throw new Error("todos: getPrice() is a stub"); } export function notify(): void { throw new Error("todos: notify() is a stub"); } export function connect(): Promise { throw new Error("Not implemented"); } // TODO: add pagination // Cleaned by pygienium-todos: converted 2 silent stubs (getPrice, notify) to loud failures.`, + `!write ${changes} # TODOs & stubs changes | summary: 2 silent stub(s) converted to loud, 0 kept | ## Converted to loud (auto) | 1. ${target}/stubs.ts:3 — getPrice() — body was \`return 0\` placeholder; now throws \`todos: getPrice() is a stub\` | 2. ${target}/stubs.ts:6 — notify() — body was empty; now throws \`todos: notify() is a stub\` | ## Kept (with reason) | (none)`, + `!echo todos: 2 silent stub(s) converted — see ${changes}`, + ].join("\n"); +} + +/** + * Verify hook: confirms the check actually produced its artifacts — after the + * scan phase `findings.md` must exist; after the fix phase `changes.md` must + * exist too. Returns an error string to fail verify, or `undefined` to pass. + */ +async function todosVerify(scope: CheckScope): Promise { + const f = findingsPath(scope.cwd); + try { + await stat(f); + } catch { + return `todos verify: expected findings.md at ${f} after scan, none found.`; + } + if (scope.fix) { + const c = changesPath(scope.cwd); + try { + await stat(c); + } catch { + return `todos verify: expected changes.md at ${c} after --fix, none found.`; + } + } + return undefined; +} + +/** The todos check definition; registers itself on import. */ +const todosCheck: CheckDefinition = { + name: "todos", + label: "TODOs & stubs", + description: + "Inventory TODO/FIXME markers and stub implementations; with --fix, convert silent stubs (placeholder returns, empty bodies) into loud failures — never implementing TODOs.", + agentName: "todos", + phaseId: "analysis", + buildScanTask: buildTodosScanTask, + buildFixTask: buildTodosFixTask, + gate: todosGate, + verify: todosVerify, +}; + +registerCheck(todosCheck); + +export { todosCheck }; diff --git a/tests/extensibility.test.ts b/tests/extensibility.test.ts index 678cbfd..5e1e46e 100644 --- a/tests/extensibility.test.ts +++ b/tests/extensibility.test.ts @@ -1,38 +1,62 @@ /** * extensibility.test.ts — the registry extensibility claim (task 14). * - * Proves a NEW check added as a file in `src/checks/` plus one `registerCheck()` - * entry yields a working `/pygienium-` command with ZERO `index.ts` - * command-wiring changes. The witness is `src/checks/noop.ts`: importing it - * self-registers the `noop` check, after which the generic command-binding path - * (`registerPygieniumCommands`, the exact function `index.ts` calls) exposes - * `/pygienium-noop` and `/pygienium-help` lists it. + * Proves a NEW check registered via the public API yields a working + * `/pygienium-` command with ZERO `index.ts` command-wiring changes: an + * in-test `registerCheck()` call makes the generic command-binding path + * (`registerPygieniumCommands`, the exact function `index.ts` calls) expose + * `/pygienium-witness` and `/pygienium-help` lists it. The shipped checks are + * each exercised by their own test files, so this suite only needs a synthetic + * witness. */ -import { describe, expect, it } from "bun:test"; -import "../src/checks/noop.js"; -import { getCheck, getAllChecks } from "../src/checks/registry.js"; +import { describe, expect, it, afterEach } from "bun:test"; +import { + clearChecks, + getCheck, + getAllChecks, + registerCheck, + type CheckDefinition, +} from "../src/checks/registry.js"; import { registerPygieniumCommands } from "../src/commands.js"; import { buildPygieniumHelpLines } from "../src/help.js"; +/** A synthetic check registered only for this suite. */ +const witnessCheck: CheckDefinition = { + name: "witness", + label: "Witness", + description: "Test-only check proving zero-wiring extensibility.", + agentName: "scanner", + fixAgentName: "fixer", + phaseId: "witness", + buildScanTask: () => + "# Task: witness scan\nwrite findings.md: witness: 0 issues", + buildFixTask: () => "# Task: witness fix\nwrite changes.md: witness: 0 edits", + gate: () => undefined, +}; + describe("registry extensibility (task 14)", () => { - it("the noop check file self-registers (no index.ts edits)", () => { - // Importing checks/noop.ts ran its top-level registerCheck(noopCheck). - expect(getCheck("noop")).toBeDefined(); - expect(getAllChecks().some((c) => c.name === "noop")).toBe(true); + afterEach(() => clearChecks()); + + it("a registered check is visible via getCheck/getAllChecks", () => { + registerCheck(witnessCheck); + expect(getCheck("witness")).toBe(witnessCheck); + expect(getAllChecks().some((c) => c.name === "witness")).toBe(true); }); - it("registerPygieniumCommands exposes /pygienium-noop (zero wiring)", () => { + it("registerPygieniumCommands exposes /pygienium- (zero wiring)", () => { + registerCheck(witnessCheck); const names: string[] = []; registerPygieniumCommands((name) => names.push(name)); - expect(names).toContain("pygienium-noop"); + expect(names).toContain("pygienium-witness"); // And the operator commands are still wired. expect(names).toContain("pygienium-help"); expect(names).toContain("pygienium-all"); }); - it("/pygienium-help lists the noop check", () => { + it("/pygienium-help lists a registered check", () => { + registerCheck(witnessCheck); const text = buildPygieniumHelpLines().join("\n"); - expect(text).toContain("/pygienium-noop"); - expect(text).toContain(getCheck("noop")!.description); + expect(text).toContain("/pygienium-witness"); + expect(text).toContain(witnessCheck.description); }); }); diff --git a/tests/todos.test.ts b/tests/todos.test.ts new file mode 100644 index 0000000..7470e2c --- /dev/null +++ b/tests/todos.test.ts @@ -0,0 +1,337 @@ +/** + * todos.test.ts — unit + integration tests for the todos (TODOs & stubs) check. + * + * Unit: `detectTodoStubs` over a multi-language fixture tree — markers, silent + * stubs (placeholder return / empty body / pass-only body), loud stubs, and + * the negative cases (a real adder, a `return null` catch handler, in-string + * "TODO" flagged for recall, clean code never flagged). + * + * Integration (deterministic fake agent runner): the scan persists findings.md + * with the three sections + machine-readable summary and a new/resolved delta + * vs the previous run; --fix converts silent stubs to loud throws, preserves + * markers, leaves loud stubs untouched, and records changes.md. + */ +import { describe, expect, it, beforeEach, afterEach } from "bun:test"; +import { existsSync } from "node:fs"; +import { mkdtemp, mkdir, rm, writeFile, readFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + clearChecks, + getCheck, + registerCheck, + type CheckDefinition, +} from "../src/checks/registry.js"; +import { + setAgentRunner, + resetAgentRunner, + fakeAgentRunner, +} from "../src/agent-runner.js"; +import { handleCheckCommand, type PygieniumCtx } from "../src/commands.js"; +import { loadRunState } from "../src/run-state.js"; +import { + findingsPath, + changesPath, + todosCheck, + detectTodoStubs, + todosPriorCounts, + buildTodosScanTask, +} from "../src/checks/todos.js"; + +function stubCtx(cwd: string): PygieniumCtx { + return { cwd, mode: "print", hasUI: false, ui: undefined } as PygieniumCtx; +} + +/** + * Seed `stubs.ts` with the full taxonomy: a TODO marker, a silent stub with a + * placeholder return (getPrice), a silent stub with an empty body (notify), + * and a loud stub (connect throws "Not implemented"). + */ +async function seedStubs(dir: string): Promise { + await mkdir(dir, { recursive: true }).catch(() => {}); + await writeFile( + join(dir, "stubs.ts"), + [ + `// TODO: add pagination`, + `export function getPrice(): number {`, + ` return 0;`, + `}`, + ``, + `export function notify(): void {}`, + ``, + `export function connect(): Promise {`, + ` throw new Error("Not implemented");`, + `}`, + ``, + ].join("\n"), + "utf8", + ); +} + +/** Seed the multi-language fixture tree for the deterministic-detector tests. */ +async function seedTree(dir: string): Promise { + await mkdir(dir, { recursive: true }).catch(() => {}); + await Promise.all([ + writeFile( + join(dir, "math.ts"), + "export function add(a: number, b: number): number {\n return a + b;\n}\n", + "utf8", + ), + writeFile( + join(dir, "parse.ts"), + [ + `export function parse(input: string) {`, + ` try {`, + ` return JSON.parse(input);`, + ` } catch {`, + ` return null;`, + ` }`, + `}`, + ``, + ].join("\n"), + "utf8", + ), + writeFile(join(dir, "stringlit.ts"), 'export const op = "TODO";\n', "utf8"), + writeFile( + join(dir, "stubs.ts"), + [ + `// TODO: add pagination`, + `export function getPrice(): number {`, + ` return 0;`, + `}`, + ``, + `export function notify(): void {}`, + ``, + `export function connect(): Promise {`, + ` throw new Error("Not implemented");`, + `}`, + ``, + ].join("\n"), + "utf8", + ), + writeFile( + join(dir, "repo.py"), + [ + `class Repository:`, + ` def find(self, uid):`, + ` raise NotImplementedError # interface method`, + ``, + ` def save(self, record):`, + ` pass`, + ``, + ].join("\n"), + "utf8", + ), + writeFile( + join(dir, "fetch.rs"), + "fn fetch() -> Result {\n todo!()\n}\n", + "utf8", + ), + ]); +} + +describe("detectTodoStubs", () => { + let dir: string; + beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), "pygienium-todos-unit-")); + await seedTree(dir); + }); + afterEach(async () => { + await rm(dir, { recursive: true, force: true }).catch(() => {}); + }); + + it("flags markers, silent stubs, and loud stubs across languages", async () => { + const hits = await detectTodoStubs(dir); + const marker = hits.filter((h) => h.kind === "marker"); + const silent = hits.filter((h) => h.kind === "silent-stub"); + const loud = hits.filter((h) => h.kind === "loud-stub"); + + // Clean code is never flagged: the adder and the `return null` catch + // handler (a boundary handler, not a stub) produce zero hits. + expect( + hits.filter((h) => /(?:math\.ts|parse\.ts)$/.test(h.path)), + ).toHaveLength(0); + + // Markers: the stubs.ts TODO comment, and the in-string "TODO" (recall — + // the scan agent's job is to drop the string-literal noise). + expect( + marker.some( + (h) => + h.path.endsWith("stubs.ts") && h.line === 1 && h.snippet === "TODO", + ), + ).toBe(true); + expect( + marker.some( + (h) => h.path.endsWith("stringlit.ts") && h.snippet === "TODO", + ), + ).toBe(true); + + // Silent stubs: lone placeholder return + empty body in stubs.ts, + // pass-only body in repo.py — each with its enclosing function. + const stubsSilent = silent.filter((h) => h.path.endsWith("stubs.ts")); + expect( + stubsSilent.some( + (h) => h.snippet === "placeholder-return" && h.context === "getPrice", + ), + ).toBe(true); + expect( + stubsSilent.some( + (h) => h.snippet === "empty-body" && h.context === "notify", + ), + ).toBe(true); + expect( + silent.some( + (h) => + h.path.endsWith("repo.py") && + h.snippet === "pass-only" && + h.context === "save", + ), + ).toBe(true); + + // Loud stubs: "Not implemented" throw, raise NotImplementedError, + // rust todo!() — reported as tracked debt. + expect( + loud.some( + (h) => h.path.endsWith("stubs.ts") && h.snippet === "Not implemented", + ), + ).toBe(true); + expect( + loud.some( + (h) => + h.path.endsWith("repo.py") && h.snippet === "NotImplementedError", + ), + ).toBe(true); + expect( + loud.some((h) => h.path.endsWith("fetch.rs") && h.snippet === "todo!("), + ).toBe(true); + }); +}); + +describe("todos check", () => { + let cwd: string; + + beforeEach(async () => { + clearChecks(); + setAgentRunner(fakeAgentRunner); + // Re-register explicitly: the module's import-time registerCheck only + // runs once (module cache), so clearChecks + registerCheck restores it + // deterministically for each test. + registerCheck(todosCheck); + cwd = await mkdtemp(join(tmpdir(), "pygienium-todos-")); + }); + + afterEach(async () => { + resetAgentRunner(); + await rm(cwd, { recursive: true, force: true }).catch(() => {}); + }); + + it("is registered and uses the todos scanner agent", () => { + const check = getCheck("todos"); + expect(check).toBeDefined(); + expect((check as CheckDefinition)?.agentName).toBe("todos"); + }); + + it("scan-only writes findings.md with the three sections and leaves sources untouched", async () => { + await seedStubs(cwd); + const check = getCheck("todos")!; + + await handleCheckCommand(check, "", stubCtx(cwd)); + + const findings = await readFile(findingsPath(cwd), "utf8"); + expect(findings).toContain( + "summary: 1 marker(s), 2 silent stub(s), 1 loud stub(s)", + ); + expect(findings).toContain("new: 4"); // first run: everything is new + expect(findings).toContain("## TODO markers"); + expect(findings).toContain("## Silent stubs (actionable)"); + expect(findings).toContain("placeholder-return"); + expect(findings).toContain("Not implemented"); + + // Scan-only: no changes.md, sources untouched. + expect(existsSync(changesPath(cwd))).toBe(false); + const untouched = await readFile(join(cwd, "stubs.ts"), "utf8"); + expect(untouched).toContain("return 0;"); + + // Run state records the scan summary as findings text. + const state = await loadRunState(cwd); + expect(state?.checks["todos"]?.status).toBe("complete"); + expect(state?.checks["todos"]?.findings).toContain( + "todos: 2 silent stub(s), 1 loud stub(s), 1 marker(s)", + ); + }); + + it("--fix converts silent stubs to loud throws, preserves markers and loud stubs, records changes.md", async () => { + await seedStubs(cwd); + const check = getCheck("todos")!; + + await handleCheckCommand(check, "--fix", stubCtx(cwd)); + + const cleaned = await readFile(join(cwd, "stubs.ts"), "utf8"); + // Silent stubs now throw loudly, naming the function. + expect(cleaned).toContain('throw new Error("todos: getPrice() is a stub")'); + expect(cleaned).toContain('throw new Error("todos: notify() is a stub")'); + // Markers are NEVER deleted and loud stubs are NEVER touched. + expect(cleaned).toContain("// TODO: add pagination"); + expect(cleaned).toContain('throw new Error("Not implemented")'); + expect(cleaned).toContain("Cleaned by pygienium-todos"); + // The placeholder bodies are gone. + expect(cleaned).not.toContain("return 0;"); + expect(cleaned).not.toContain("notify(): void {}"); + + const changes = await readFile(changesPath(cwd), "utf8"); + expect(changes).toContain( + "summary: 2 silent stub(s) converted to loud, 0 kept", + ); + expect(changes).toContain("getPrice()"); + expect(changes).toContain("notify()"); + + const state = await loadRunState(cwd); + expect(state?.checks["todos"]?.fix).toBe(true); + expect(state?.checks["todos"]?.status).toBe("complete"); + }); + + it("findings.md and changes.md live under .pygienium/checks/todos/", async () => { + await seedStubs(cwd); + const check = getCheck("todos")!; + await handleCheckCommand(check, "--fix", stubCtx(cwd)); + expect(findingsPath(cwd)).toBe( + join(cwd, ".pygienium", "checks", "todos", "findings.md"), + ); + expect(changesPath(cwd)).toBe( + join(cwd, ".pygienium", "checks", "todos", "changes.md"), + ); + }); + + it("skips when the target has no source files", async () => { + // cwd is a fresh empty tempdir (no seeding) → the gate finds no source + // files and skips the check without spawning an agent. + const check = getCheck("todos")!; + await handleCheckCommand(check, "", stubCtx(cwd)); + const state = await loadRunState(cwd); + expect(state?.checks["todos"]?.status).toBe("skipped"); + }); + + it("reports a new/resolved delta against the previous run's counts", async () => { + await seedStubs(cwd); + const check = getCheck("todos")!; + await handleCheckCommand(check, "", stubCtx(cwd)); + + // The previous run's verified counts are parseable from run-state. + const prior = await todosPriorCounts(cwd); + expect(prior).toEqual({ silent: 2, loud: 1, marker: 1 }); + + // Nobody flagged anything new and everything was resolved: the next scan + // sees zero candidates → new: 0, resolved: 4 (all prior items). + await rm(join(cwd, "stubs.ts")); + const task = await buildTodosScanTask(cwd, { + cwd, + target: cwd, + fix: false, + rest: [], + }); + expect(task).toContain( + "summary: 0 marker(s), 0 silent stub(s), 0 loud stub(s)", + ); + expect(task).toContain("| new: 0 | resolved: 4 |"); + }); +});