From 92be04d6d065ccc3933bbeaba52be3cd4cb380a2 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Fri, 31 Jul 2026 14:32:44 +0800 Subject: [PATCH 1/4] feat(issues): give docs/outstanding-issues.md a writer, not just a gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/branch-review-ledger.md has had a writer since it was introduced and hand-authoring a row there is forbidden. This file had a gate but no writer, so every mutation was hand-authored — and the 2026-07-30/31 session produced exactly the failures that predicts, none of them judgement calls: rows appended into the archive table because the author anchored on an id that had since been archived, an unescaped pipe splitting a row into extra cells, and ids read off the marker by eye. scripts/outstanding-issues.mjs adds add/done/update. It imports the gate's parser rather than re-deriving table bounds or widths, so the rules live in one place, and it re-runs the gate against its own output before writing — a refusal here is the same refusal CI would give, minus the round trip. Wired as issues:add / issues:done / issues:update, with the writer's self-test chained into check:outstanding-issues so CI exercises it, mirroring check:branch-review-ledger. The issues skill now points at the CLI, since a tool nobody is told to use changes nothing. It deliberately does NOT fix id collisions between concurrent branches: allocation is still read-modify-write against the marker. #159, added with the writer itself, records that fix — collision-free ids, after which a union merge driver becomes safe to reinstate. Co-Authored-By: Claude Opus 5 --- .claude/skills/issues/SKILL.md | 16 ++ docs/outstanding-issues.md | 3 +- docs/scripts-index.md | 37 +-- package.json | 5 +- scripts/outstanding-issues.mjs | 348 ++++++++++++++++++++++++ tests/outstanding-issues-writer.test.ts | 87 ++++++ 6 files changed, 476 insertions(+), 20 deletions(-) create mode 100644 scripts/outstanding-issues.mjs create mode 100644 tests/outstanding-issues-writer.test.ts diff --git a/.claude/skills/issues/SKILL.md b/.claude/skills/issues/SKILL.md index cbd39b8a5b..70a4ff6a03 100644 --- a/.claude/skills/issues/SKILL.md +++ b/.claude/skills/issues/SKILL.md @@ -56,6 +56,22 @@ paragraph; put the smallest next action in **Detail / next action**. ## Writing rules +**Use the writer, not an editor.** Row mechanics are handled by +`scripts/outstanding-issues.mjs`, the counterpart to the gate: + +```bash +npm run issues:add -- --pri P2 --type issue --summary "…" --detail "…" --source "…" +npm run issues:done -- '#151' --outcome "Resolved 2026-07-31 by PR #1494. …" +npm run issues:update -- '#151' --detail "…" +``` + +It allocates the id from the marker and bumps it, appends into the **open** table (never the +archive), moves rather than copies on `done`, reshapes to each table's width, escapes `|`, and +re-runs the gate against its own output — refusing to write anything CI would reject. Hand-editing +is what produced the wrong-table inserts, unescaped pipes and broken cell counts this writer exists +to prevent; treat it like `ledger:append` for the review ledger. It does **not** solve id collisions +between concurrent branches (see `#154`) — that needs a different id scheme, not a better writer. + - Keep the table format and column order exactly as in `docs/outstanding-issues.md`. One row per item. - Add a retained task to the recommended queue with order, acuity, capability, timing, estimate, gate, success criteria, verification, and stop rule. Reorder rather than duplicate related work. diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 70deeaec5a..5ca248cf91 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -79,7 +79,7 @@ removed after current-main verification; it is not missing recommended work. | 31 | `#099` | A3 | Specialist — answer path | After `#098` | Half a day per sub-item | Remaining fixed per-request round trips: the 8 `setCachedSearch` deferrals (abort semantics + mutation window), the anonymous subject+global limiter pair (needs a new atomic RPC first), and proxy→route identity duplication. Stop before hand-authoring locking SQL. | | 32 | `#103` | A3 | Operator — Supabase schema | Same window as `#102` | 30–60 minutes | Confirm whether the wide `document_table_facts` trigram index from `20260714190000` exists live, then either mirror it into `schema.sql` (retained) or drop it via a forward migration (redundant). **Not the allowlist** — it suppresses live-vs-`schema.sql` findings only and cannot make the migration chain and the mirror agree. Stop: do not drop it without live scan evidence. | - + ## Open items @@ -141,6 +141,7 @@ removed after current-main verification; it is not missing recommended work. | #157 | P3 | issue | `--med-accent-soft` is dead plumbing | **Outcome:** the medication accent trio has no unused member, or its presence is deliberate and recorded. **Detail:** `medicationAccentStyle()` sets `--med-accent-soft`, but no repository consumer reads it; the other two runtime-set medication accent properties are consumed. It was retained because the nearby comment documents the trio as a contract. **Next:** either consume it for the intended section wash or remove the declaration after confirming that contract. **Stop:** do not delete it as a drive-by change. | PR #1451 design-sync token triage | 2026-07-30 | | #156 | P3 | issue | Outstanding-issues ids are still allocated read-modify-write, and Update-branch corrupts the merge | **Outcome:** two branches cannot silently claim the same outstanding-issues id, and no merge path can commit a file where they have. **Detail:** `#133` fixed the two causes of *conflict frequency* — `#1444` removed `merge=union` and `#1479` excluded the ledger from Prettier so a maximum-width row stops re-padding the whole table. Neither touches **id allocation**, which is still read-modify-write against the `issues:next-id` marker, so two branches open at the same time still pick the same number. Measured on PR #1451 (2026-07-30): one P3 row was renumbered `#135` -> `#141` -> `#145` -> `#147` -> `#149` across four sync cycles, because `main` had taken each id in turn — every renumber was manual. The sharper finding is the resolution path: the GitHub **Update branch** button pushed a sync to that PR head (`df3f3aeed`) whose auto-merge produced **two rows numbered `#141` and two `next-id` markers**, leaving the marker at `142` — below `main`'s highest id, so the next allocation would have reused a live number. `git merge` reported success; only `npm run check:outstanding-issues` caught it. That guard runs in `verify:cheap` and `static-pr`, so such a head cannot merge — but the corruption is produced by a one-click path that runs no guard, and the cost lands on whoever notices. A second session on the same branch later dropped an entire appended evidence block while resolving this file, which the guard cannot detect at all: it validates ids and structure, not whether a merge kept both sides' prose. **Next:** cheapest first — document that Update branch must not be used on PRs touching this file (prefer `npm run sync:pr-branches:apply`, which the repo already prefers for other reasons), then consider allocating ids from a source that cannot collide (per-row files, or a date-plus-slug id) so concurrent branches never contend. **Stop:** do not reintroduce a merge driver here — `#133` settled that; this is about allocation and about merges that silently drop rows, not about the driver. | PR #1451 sync cycles; `df3f3aeed`; session 2026-07-30 | 2026-07-31 | | #155 | P2 | rec | Several agent sessions edit the same branch and ledger concurrently | **Outcome:** concurrent sessions stop silently undoing each other on shared `claude/*` branches and on this file. **Observed across one task on 2026-07-30/31:** (a) PR #1490 was **closed unmerged by another actor while auto-merge was armed**, and because arming had been treated as "done", the only record of four preservation snapshots went with it and had to be reconstructed as #152. (b) Three branches (`claude/organize-local-worktree-d22bc3`, `claude/root-dir-coverage-gate-v2`, `claude/capture-session-followups`) received pushes from a Cursor Agent and a Codex session mid-task, producing repeated non-fast-forward rejections; one rejection was masked because the push was piped to `tail`, so the reported exit code was `tail`'s and the push looked successful. (c) `scripts/guard-push.mjs` correctly refused a push with `Pushing now races the squash-merge and can orphan this commit`, requiring disarm to push then re-arm. (d) Ledger ids were renumbered underneath in-flight work (#135 to #141 to #144), which is the mechanism behind #154. **Next:** for a green ledger-only PR prefer merging it immediately over arming auto-merge; confirm a push landed with `git ls-remote` rather than the command's exit code; expect ids and row wording to move between reading and writing. **Stop:** do not treat auto-merge as completion, and do not assume a branch you pushed an hour ago still has your commit at its tip. | session 2026-07-30/31; PRs #1490, #1508, #1511 | 2026-07-31 | +| #159 | P2 | rec | Sequential issue ids force every concurrent append to conflict | **Outcome:** two sessions can append to this ledger at the same time without conflicting. **Detail:** ids are allocated read-modify-write against the `issues:next-id` marker inside the file being edited, so two branches both read N and both write N. Because duplicate ids are unacceptable, a union merge driver is unsafe — .gitattributes says so explicitly — which is why this file deliberately has no driver and every overlapping append conflicts by hand. Manual resolution is where rows get dropped: PR #1490 was closed during one and took the only record of four snapshots with it (#152), and ids were renumbered under in-flight work three times in one session (#154, #155). The new writer (scripts/outstanding-issues.mjs) removes the mechanical errors but explicitly not this one. **Next:** replace the counter with a collision-free id (ULID, timestamp+suffix, or a content hash), keeping a short display number derived at render time if #151 reads better than 01JQ…; then a union driver becomes safe to reinstate and concurrent appends stop conflicting at all. A larger variant is one row per file under docs/issues/ with the table generated, which the repo already does for site-map.md. **Stop:** do not reinstate merge=union while ids are sequential — that combination was tried in PR #1416 and removed for duplicating rows and the marker. | session 2026-07-31; .gitattributes; #154/#155 | 2026-07-31 | ## Resolved / archive diff --git a/docs/scripts-index.md b/docs/scripts-index.md index f4453f0eac..d59a88c4dd 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -1,6 +1,6 @@ # Scripts index -Curated map of `scripts/` (195 files) and the `package.json` script surface (209 entries), +Curated map of `scripts/` (196 files) and the `package.json` script surface (212 entries), grouped by purpose. This is orientation, not an exhaustive per-file listing — the authoritative command list is `package.json`, and `npm run docs:check-scripts` verifies every `npm run ` referenced in docs resolves to a real script. `npm run docs:update` refreshes the exact counts above. @@ -16,23 +16,24 @@ migration has shipped (see `docs/maturity-backlog-workorders.md` L1). ## Runner & guard infrastructure [infra] -| Script | Role | -| ------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------- | -| `run-heavy.mjs` | Acquires shared/exclusive cross-worktree leases (`test-run-lock.mjs`) so focused checks can overlap safely | -| `run-tsx.mjs`, `run-vitest.mjs`, `run-playwright.mjs`, `run-eval-safe.mjs` | Typed/test/e2e/eval entrypoint wrappers | -| `dev-free-port.mjs`, `ensure-local-server.mjs` | Project-stable localhost port selection + background server ensure | -| `check-node-engine.cjs`, `install-git-hooks.mjs`, `guard-push.mjs`, `guard-next-build.mjs` | Install/preflight guards | -| `setup-codex-cloud.sh`, `maintain-codex-cloud.sh`, `check-codex-cloud-setup.mjs`, `ensure-codex-cloud-git-remote.mjs` | Reproducible Codex Cloud toolchain/profile setup, sanitized acceptance, and safe credential-free `origin` repair | -| `ci-change-scope.mjs`, `ci-triage.mjs`, `pr-policy.mjs`, `pr-mergeability.mjs` | CI change classification + PR policy + conflict signal (self-tested via `check:ci-scope`/`check:ci-triage`/`check:pr-policy`/`check:pr-mergeability`) | -| `check-outstanding-issues.mjs`, `check-pr-mergeability-workflow.mjs` | Outstanding-issues ID/marker/no-driver guard + PR mergeability workflow contract | -| `check-installed-lock-parity.mjs`, `phone-chrome-plan.mjs`, `verify-phone-chrome.mjs`, `playwright-browser-preflight.mjs` | Lock-trust preflight, change-scoped phone contracts, and Playwright browser-binary preflight before build | -| `final-merge-audit.mjs` | Fail-closed local merge-tree audit; explicit provider mode adds PR/check/thread/tree/deployment proof | -| `child-process-result.mjs`, `cli-utils.ts`, `productivity-core.mjs` | Shared helpers | -| `test-focused.mjs`, `test-run-selection.mjs`, `test-cache-path.mjs`, `test-environment.mjs` | Backs `npm run test:focused` — change-scoped selection, cache pathing, env setup; fails closed for deleted files and test infrastructure | -| `primary-checkout-lease.mjs`, `test-run-lock.mjs`, `clean-worktree.mjs` | Cross-worktree lease arbitration for the primary checkout, plus worktree cleanup | -| `resolve-tsx-cli.mjs`, `register-server-only.mjs`, `enable-server-only-stub.mjs` | tsx CLI resolution and `server-only` import shims | -| `check-format-changed.mjs`, `check-base-freshness.mjs`, `check-local-presence.mjs` | Push-time helpers behind `guard-push.mjs`: changed-file formatting, stale-base and local-presence checks | -| `yaml-contract.mjs`, `sensitive-text.mjs`, `design-system-contract-utils.mjs` | Shared parsing/redaction/contract helpers used by the gates | +| Script | Role | +| ------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `run-heavy.mjs` | Acquires shared/exclusive cross-worktree leases (`test-run-lock.mjs`) so focused checks can overlap safely | +| `run-tsx.mjs`, `run-vitest.mjs`, `run-playwright.mjs`, `run-eval-safe.mjs` | Typed/test/e2e/eval entrypoint wrappers | +| `dev-free-port.mjs`, `ensure-local-server.mjs` | Project-stable localhost port selection + background server ensure | +| `check-node-engine.cjs`, `install-git-hooks.mjs`, `guard-push.mjs`, `guard-next-build.mjs` | Install/preflight guards | +| `setup-codex-cloud.sh`, `maintain-codex-cloud.sh`, `check-codex-cloud-setup.mjs`, `ensure-codex-cloud-git-remote.mjs` | Reproducible Codex Cloud toolchain/profile setup, sanitized acceptance, and safe credential-free `origin` repair | +| `ci-change-scope.mjs`, `ci-triage.mjs`, `pr-policy.mjs`, `pr-mergeability.mjs` | CI change classification + PR policy + conflict signal (self-tested via `check:ci-scope`/`check:ci-triage`/`check:pr-policy`/`check:pr-mergeability`) | +| `check-outstanding-issues.mjs`, `check-pr-mergeability-workflow.mjs` | Outstanding-issues ID/marker/no-driver guard + PR mergeability workflow contract | +| `outstanding-issues.mjs` | Writer for `docs/outstanding-issues.md` (`issues:add` / `issues:done` / `issues:update`) — allocates the id, picks the right table, escapes `\|`, and re-runs the guard on its own output. Never hand-edit that file, as with `ledger:append` | +| `check-installed-lock-parity.mjs`, `phone-chrome-plan.mjs`, `verify-phone-chrome.mjs`, `playwright-browser-preflight.mjs` | Lock-trust preflight, change-scoped phone contracts, and Playwright browser-binary preflight before build | +| `final-merge-audit.mjs` | Fail-closed local merge-tree audit; explicit provider mode adds PR/check/thread/tree/deployment proof | +| `child-process-result.mjs`, `cli-utils.ts`, `productivity-core.mjs` | Shared helpers | +| `test-focused.mjs`, `test-run-selection.mjs`, `test-cache-path.mjs`, `test-environment.mjs` | Backs `npm run test:focused` — change-scoped selection, cache pathing, env setup; fails closed for deleted files and test infrastructure | +| `primary-checkout-lease.mjs`, `test-run-lock.mjs`, `clean-worktree.mjs` | Cross-worktree lease arbitration for the primary checkout, plus worktree cleanup | +| `resolve-tsx-cli.mjs`, `register-server-only.mjs`, `enable-server-only-stub.mjs` | tsx CLI resolution and `server-only` import shims | +| `check-format-changed.mjs`, `check-base-freshness.mjs`, `check-local-presence.mjs` | Push-time helpers behind `guard-push.mjs`: changed-file formatting, stale-base and local-presence checks | +| `yaml-contract.mjs`, `sensitive-text.mjs`, `design-system-contract-utils.mjs` | Shared parsing/redaction/contract helpers used by the gates | ## Verification gates [live] diff --git a/package.json b/package.json index fdfa6b08a1..eaa4666ca0 100644 --- a/package.json +++ b/package.json @@ -70,7 +70,10 @@ "check:ci-triage": "node scripts/ci-triage.mjs --self-test", "check:gate-manifest": "node scripts/check-gate-manifest.mjs", "check:branch-review-ledger": "node scripts/check-branch-review-ledger.mjs --self-test && node scripts/branch-review-ledger.mjs --self-test && node scripts/merge-branch-review-ledger.mjs --self-test && node scripts/check-branch-review-ledger.mjs", - "check:outstanding-issues": "node scripts/check-outstanding-issues.mjs --self-test && node scripts/check-outstanding-issues.mjs", + "check:outstanding-issues": "node scripts/check-outstanding-issues.mjs --self-test && node scripts/outstanding-issues.mjs --self-test && node scripts/check-outstanding-issues.mjs", + "issues:add": "node scripts/outstanding-issues.mjs add", + "issues:done": "node scripts/outstanding-issues.mjs done", + "issues:update": "node scripts/outstanding-issues.mjs update", "ledger:lookup": "node scripts/branch-review-ledger.mjs lookup", "ledger:append": "node scripts/branch-review-ledger.mjs append", "ledger:dedupe": "node scripts/branch-review-ledger.mjs dedupe", diff --git a/scripts/outstanding-issues.mjs b/scripts/outstanding-issues.mjs new file mode 100644 index 0000000000..328c038b06 --- /dev/null +++ b/scripts/outstanding-issues.mjs @@ -0,0 +1,348 @@ +#!/usr/bin/env node +// Writer for docs/outstanding-issues.md — the counterpart to +// check-outstanding-issues.mjs, which only ever validated. +// +// Why this exists. docs/branch-review-ledger.md has had a writer +// (branch-review-ledger.mjs) since it was introduced, and hand-authoring a row +// there is forbidden by AGENTS.md. This file had a gate but no writer, so every +// mutation was hand-authored — and the 2026-07-30/31 session produced exactly +// the failures that predicts, none of which were judgement calls: +// +// - rows appended into the archive table because the author anchored on an id +// that had since been archived (the gate caught it as a cell-count error, +// which is a confusing way to be told "wrong table") +// - an unescaped `|` inside prose splitting one row into extra cells +// - ids allocated by reading the marker by eye and colliding with a +// concurrent branch +// +// So the rules live in ONE place: this writer imports the gate's parser rather +// than re-deriving where the tables are or how wide they are, and it re-runs the +// gate against its own output before writing. A refusal here is the same +// refusal CI would give, minus the round trip. +// +// It deliberately does NOT solve id collisions between concurrent branches: +// allocation is still read-modify-write against the marker, so two branches can +// still pick the same number. That is ledger #154's territory and needs a +// different id scheme, not a better writer. +// +// Usage: +// node scripts/outstanding-issues.mjs add --pri P2 --type issue \ +// --summary "..." --detail "..." --source "..." +// node scripts/outstanding-issues.mjs done '#151' --outcome "Resolved ..." +// node scripts/outstanding-issues.mjs update '#151' --detail "..." +// node scripts/outstanding-issues.mjs --self-test + +import { readFileSync, writeFileSync } from "node:fs"; +import { pathToFileURL } from "node:url"; + +import { ISSUES_PATH, canonicalId, checkIssues, parseIssues } from "./check-outstanding-issues.mjs"; + +const OPEN_CELLS = 7; // ID | Pri | Type | Summary | Detail / next action | Source | Added +const ARCHIVE_CELLS = 5; // ID | Type | Summary | Outcome | Resolved +const PRIORITIES = new Set(["P1", "P2", "P3"]); +const TYPES = new Set(["task", "issue", "rec"]); + +/** + * Make one cell safe to place in a markdown table. + * + * The pipe escape is the whole point: prose in these rows routinely contains + * `a | b`, and an unescaped pipe silently becomes a column boundary, which the + * gate then reports as a width error somewhere else on the line. Newlines + * collapse for the same reason — a row is one line, by construction. + */ +export function escapeCell(value) { + return String(value ?? "") + .replace(/\r?\n+/g, " ") + .replace(/\|/g, "\\|") + .replace(/\s+/g, " ") + .trim(); +} + +/** Split a row into cells, honouring `\|` escapes so prose pipes stay put. */ +export function splitCells(line) { + const inner = line.trim().replace(/^\|/, "").replace(/\|$/, ""); + const out = []; + let cell = ""; + for (let i = 0; i < inner.length; i += 1) { + if (inner[i] === "\\" && inner[i + 1] === "|") { + cell += "\\|"; + i += 1; + continue; + } + if (inner[i] === "|") { + out.push(cell.trim()); + cell = ""; + continue; + } + cell += inner[i]; + } + out.push(cell.trim()); + return out; +} + +export function buildRow(cells) { + return `| ${cells.join(" | ")} |`; +} + +function today(options = {}) { + if (options.date) return options.date; + return new Date().toISOString().slice(0, 10); +} + +/** Last line index of the last body block of a table, or null when absent. */ +function lastRowIndex(parsed, table) { + const rows = parsed.rows.filter((row) => row.table === table); + if (rows.length === 0) return null; + return rows[rows.length - 1].line - 1; +} + +function findRow(parsed, id) { + return parsed.rows.find((row) => row.id === id) ?? null; +} + +/** + * Apply an edit, then re-run the gate on the result. Returning the markdown + * only when it passes is what makes a wrong-table insert impossible rather than + * merely detectable later. + */ +function guarded(markdown, mutate) { + const next = mutate(markdown); + const problems = checkIssues(next, { prettierIgnored: true }); + if (problems.length > 0) { + throw new Error(`refusing to write, the result would fail the gate:\n - ${problems.join("\n - ")}`); + } + return next; +} + +export function addIssue(markdown, fields, options = {}) { + const pri = String(fields.pri ?? "P2"); + const type = String(fields.type ?? "task"); + if (!PRIORITIES.has(pri)) throw new Error(`--pri must be one of ${[...PRIORITIES].join(", ")}, got ${pri}`); + if (!TYPES.has(type)) throw new Error(`--type must be one of ${[...TYPES].join(", ")}, got ${type}`); + if (!fields.summary) throw new Error("--summary is required"); + + return guarded(markdown, (current) => { + const parsed = parseIssues(current); + if (parsed.nextId === null) throw new Error("no issues:next-id marker found"); + if (parsed.openStart < 0) throw new Error("no '## Open items' heading found"); + + const id = canonicalId(parsed.nextId); + const row = buildRow([ + id, + pri, + type, + escapeCell(fields.summary), + escapeCell(fields.detail ?? ""), + escapeCell(fields.source ?? `session ${today(options)}`), + today(options), + ]); + if (splitCells(row).length !== OPEN_CELLS) { + throw new Error(`built an open row with ${splitCells(row).length} cells, expected ${OPEN_CELLS}`); + } + + const anchor = lastRowIndex(parsed, "open"); + if (anchor === null) throw new Error("the open-items table has no rows to append after"); + + const lines = current.split("\n"); + lines.splice(anchor + 1, 0, row); + let next = lines.join("\n"); + next = next.replace(//, ``); + return next; + }); +} + +export function resolveIssue(markdown, id, outcome, options = {}) { + if (!outcome) throw new Error("--outcome is required"); + return guarded(markdown, (current) => { + const parsed = parseIssues(current); + const row = findRow(parsed, id); + if (!row) throw new Error(`${id} is not in ${ISSUES_PATH}`); + if (row.table === "archive") throw new Error(`${id} is already archived`); + + const cells = splitCells(row.raw); + // Open is ID|Pri|Type|Summary|Detail|Source|Added; archive drops Pri, + // Detail and Source and gains Outcome + Resolved. + const archived = buildRow([cells[0], cells[2], cells[3], escapeCell(outcome), today(options)]); + if (splitCells(archived).length !== ARCHIVE_CELLS) { + throw new Error(`built an archive row with ${splitCells(archived).length} cells, expected ${ARCHIVE_CELLS}`); + } + + const lines = current.split("\n"); + lines.splice(row.line - 1, 1); + const afterRemoval = parseIssues(lines.join("\n")); + const anchor = lastRowIndex(afterRemoval, "archive"); + if (anchor === null) throw new Error("the archive table has no rows to append after"); + lines.splice(anchor + 1, 0, archived); + return lines.join("\n"); + }); +} + +export function updateIssue(markdown, id, fields) { + const editable = { summary: 3, detail: 4, source: 5 }; + const requested = Object.keys(editable).filter((key) => fields[key] !== undefined); + if (requested.length === 0) throw new Error("pass at least one of --summary, --detail, --source"); + + return guarded(markdown, (current) => { + const parsed = parseIssues(current); + const row = findRow(parsed, id); + if (!row) throw new Error(`${id} is not in ${ISSUES_PATH}`); + if (row.table !== "open") throw new Error(`${id} is archived; archived rows are history and are not edited`); + + const cells = splitCells(row.raw); + for (const key of requested) cells[editable[key]] = escapeCell(fields[key]); + const lines = current.split("\n"); + lines[row.line - 1] = buildRow(cells); + return lines.join("\n"); + }); +} + +function argValue(argv, name) { + const index = argv.indexOf(`--${name}`); + return index >= 0 ? argv[index + 1] : undefined; +} + +function selfTest() { + const fixture = [ + "# Outstanding", + "", + "", + "", + "## Open items", + "", + "| ID | Pri | Type | Summary | Detail / next action | Source | Added |", + "| ---- | --- | ---- | ---- | ---- | ---- | ---- |", + "| #005 | P2 | issue | first | detail one | src | 2026-01-01 |", + "| #006 | P3 | task | second | detail two | src | 2026-01-02 |", + "", + "## Resolved / archive", + "", + "| ID | Type | Summary | Outcome | Resolved |", + "| ---- | ---- | ---- | ---- | ---- |", + "| #001 | task | old | done long ago | 2025-12-01 |", + "", + ].join("\n"); + + const failures = []; + const check = (label, condition) => { + if (!condition) failures.push(label); + }; + + // add: lands in the OPEN table, takes the marker's id, bumps it. + const added = addIssue( + fixture, + { pri: "P1", type: "rec", summary: "third", detail: "d", source: "s" }, + { date: "2026-02-02" }, + ); + const addedParsed = parseIssues(added); + check( + "add uses the marker id", + addedParsed.rows.some((r) => r.id === "#007" && r.table === "open"), + ); + check("add bumps the marker", addedParsed.nextId === 8); + check("add appends after the last open row", added.indexOf("#007") > added.indexOf("#006")); + check("add stays out of the archive", !addedParsed.rows.some((r) => r.id === "#007" && r.table === "archive")); + + // The wrong-table failure that motivated this writer: appending must not land + // in the archive even though an archived row sits later in the file. + check("add lands before the archive heading", added.indexOf("| #007 ") < added.indexOf("## Resolved / archive")); + + // escaping: a pipe in prose must not become a column. + const piped = addIssue( + fixture, + { pri: "P2", type: "task", summary: "a | b", detail: "c | d" }, + { date: "2026-02-02" }, + ); + const pipedRow = parseIssues(piped).rows.find((r) => r.id === "#007"); + check("pipes are escaped, not new cells", splitCells(pipedRow.raw).length === OPEN_CELLS); + check("escaped pipe survives in the text", pipedRow.raw.includes("a \\| b")); + + // done: moves rather than copies, and reshapes to the archive width. + const resolved = resolveIssue(fixture, "#005", "Resolved by PR #1", { date: "2026-03-03" }); + const resolvedParsed = parseIssues(resolved); + const moved = resolvedParsed.rows.filter((r) => r.id === "#005"); + check("done leaves exactly one #005", moved.length === 1); + check("done puts it in the archive", moved[0]?.table === "archive"); + check("done reshapes to archive width", splitCells(moved[0].raw).length === ARCHIVE_CELLS); + check("done keeps the summary", moved[0].raw.includes("first")); + check("done records the outcome", moved[0].raw.includes("Resolved by PR #1")); + + // update: edits in place, same table, same width. + const updated = updateIssue(fixture, "#006", { detail: "replaced | detail" }); + const updatedRow = parseIssues(updated).rows.find((r) => r.id === "#006"); + check("update stays in the open table", updatedRow.table === "open"); + check("update keeps the width", splitCells(updatedRow.raw).length === OPEN_CELLS); + check("update escapes the new text", updatedRow.raw.includes("replaced \\| detail")); + + // refusals + const rejects = (label, run) => { + try { + run(); + failures.push(`${label} should have thrown`); + } catch { + /* expected */ + } + }; + rejects("unknown id", () => resolveIssue(fixture, "#999", "x")); + rejects("double archive", () => resolveIssue(resolved, "#005", "again")); + rejects("bad priority", () => addIssue(fixture, { pri: "P9", summary: "x" })); + rejects("bad type", () => addIssue(fixture, { type: "nope", summary: "x" })); + rejects("missing summary", () => addIssue(fixture, {})); + rejects("empty update", () => updateIssue(fixture, "#006", {})); + + if (failures.length > 0) { + console.error("outstanding-issues writer self-test FAILED:"); + for (const failure of failures) console.error(` - ${failure}`); + process.exitCode = 1; + return; + } + console.log("outstanding-issues writer self-test passed."); +} + +function main() { + const argv = process.argv.slice(2); + if (argv.includes("--self-test")) { + selfTest(); + return; + } + + const [command, positional] = argv; + const markdown = readFileSync(ISSUES_PATH, "utf8"); + let next; + + try { + if (command === "add") { + next = addIssue(markdown, { + pri: argValue(argv, "pri"), + type: argValue(argv, "type"), + summary: argValue(argv, "summary"), + detail: argValue(argv, "detail"), + source: argValue(argv, "source"), + }); + } else if (command === "done") { + next = resolveIssue(markdown, positional, argValue(argv, "outcome")); + } else if (command === "update") { + next = updateIssue(markdown, positional, { + summary: argValue(argv, "summary"), + detail: argValue(argv, "detail"), + source: argValue(argv, "source"), + }); + } else { + console.error("usage: outstanding-issues.mjs [id] [--flags] (see file header)"); + process.exitCode = 1; + return; + } + } catch (error) { + console.error(`outstanding-issues: ${error.message}`); + process.exitCode = 1; + return; + } + + writeFileSync(ISSUES_PATH, next, "utf8"); + const parsed = parseIssues(next); + const open = parsed.rows.filter((r) => r.table === "open").length; + const archived = parsed.rows.filter((r) => r.table === "archive").length; + console.log(`${ISSUES_PATH} updated: ${open} open, ${archived} archived, next-id=${parsed.nextId}.`); +} + +const invokedDirectly = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href; +if (invokedDirectly) main(); diff --git a/tests/outstanding-issues-writer.test.ts b/tests/outstanding-issues-writer.test.ts new file mode 100644 index 0000000000..6cdd1b3dd8 --- /dev/null +++ b/tests/outstanding-issues-writer.test.ts @@ -0,0 +1,87 @@ +import { describe, expect, it } from "vitest"; + +import { parseIssues } from "../scripts/check-outstanding-issues.mjs"; +import { addIssue, escapeCell, resolveIssue, splitCells, updateIssue } from "../scripts/outstanding-issues.mjs"; + +const OPEN_CELLS = 7; +const ARCHIVE_CELLS = 5; + +const ledger = [ + "# Outstanding", + "", + "", + "", + "## Open items", + "", + "| ID | Pri | Type | Summary | Detail / next action | Source | Added |", + "| ---- | --- | ---- | ---- | ---- | ---- | ---- |", + "| #005 | P2 | issue | first | detail one | src | 2026-01-01 |", + "| #006 | P3 | task | second | detail two | src | 2026-01-02 |", + "", + "## Resolved / archive", + "", + "| ID | Type | Summary | Outcome | Resolved |", + "| ---- | ---- | ---- | ---- | ---- |", + "| #001 | task | old | done long ago | 2025-12-01 |", + "", +].join("\n"); + +const rowFor = (markdown: string, id: string) => parseIssues(markdown).rows.find((row) => row.id === id); + +describe("outstanding-issues writer", () => { + it("appends into the open table, never the archive", () => { + // The defect this writer exists for: hand edits anchored on an id that had + // been archived, so the new row landed in the archive table. + const next = addIssue(ledger, { pri: "P1", type: "rec", summary: "third" }, { date: "2026-02-02" }); + const row = rowFor(next, "#007"); + expect(row?.table).toBe("open"); + expect(next.indexOf("| #007 ")).toBeLessThan(next.indexOf("## Resolved / archive")); + }); + + it("allocates the marker's id and bumps it", () => { + const next = addIssue(ledger, { summary: "third" }, { date: "2026-02-02" }); + expect(rowFor(next, "#007")).toBeDefined(); + expect(parseIssues(next).nextId).toBe(8); + }); + + it("escapes pipes in prose instead of creating columns", () => { + const next = addIssue(ledger, { summary: "a | b", detail: "c | d" }, { date: "2026-02-02" }); + const row = rowFor(next, "#007"); + expect(splitCells(row!.raw)).toHaveLength(OPEN_CELLS); + expect(row!.raw).toContain("a \\| b"); + }); + + it("moves a row to the archive rather than copying it, reshaping its width", () => { + const next = resolveIssue(ledger, "#005", "Resolved by PR #1", { date: "2026-03-03" }); + const rows = parseIssues(next).rows.filter((row) => row.id === "#005"); + expect(rows).toHaveLength(1); + expect(rows[0].table).toBe("archive"); + expect(splitCells(rows[0].raw)).toHaveLength(ARCHIVE_CELLS); + expect(rows[0].raw).toContain("Resolved by PR #1"); + }); + + it("edits an open row in place without changing its width", () => { + const next = updateIssue(ledger, "#006", { detail: "replaced | detail" }); + const row = rowFor(next, "#006"); + expect(row?.table).toBe("open"); + expect(splitCells(row!.raw)).toHaveLength(OPEN_CELLS); + expect(row!.raw).toContain("replaced \\| detail"); + }); + + it("refuses edits that would not survive the gate", () => { + expect(() => resolveIssue(ledger, "#999", "x")).toThrow(/not in/); + expect(() => addIssue(ledger, { pri: "P9", summary: "x" })).toThrow(/--pri/); + expect(() => addIssue(ledger, { type: "nope", summary: "x" })).toThrow(/--type/); + expect(() => addIssue(ledger, {})).toThrow(/--summary/); + expect(() => updateIssue(ledger, "#006", {})).toThrow(/at least one/); + }); + + it("refuses to archive a row twice", () => { + const once = resolveIssue(ledger, "#005", "first", { date: "2026-03-03" }); + expect(() => resolveIssue(once, "#005", "again")).toThrow(/already archived/); + }); + + it("collapses newlines so a row stays one line", () => { + expect(escapeCell("a\nb\r\nc")).toBe("a b c"); + }); +}); From c7212ec5b1d9f596d1cc5d5f8673197d6d395a14 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 31 Jul 2026 08:19:03 +0000 Subject: [PATCH 2/4] docs(ledger): record PR #1524 review+bugbot+fix at 83dec1f5 Co-authored-by: BigSimmo --- docs/branch-review-ledger.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 592ad8b5f8..d2b048a41a 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -538,3 +538,4 @@ Records before 2026-07-28 were written by hand and had drifted: 146 lines carrie | 2026-07-31 | codex/complete-and-merge-p2-tasks-to-main | 1761a100464992486fc7979a448c7a9773bd3e55 | PR #1471 review+bugbot+fix+heavy | PASS: synced origin/main (behind-but-clean); deep review+Bugbot no P0-P2; 0 threads; therapy index contracts OK; verify:cheap + verify:pr-local green; PR body RAG impact accurate | merge-tree clean; 0 behind; therapy --check 205; vitest therapy 14 passed; verify:cheap 445 files/4661 tests; verify:pr-local runtime+lint+typecheck+test+build+rag fixtures; prior tip ea92b37 CI pr-required green | | 2026-07-31 | claude/warning-consolidation-mockups-09jyj7 | 7b41fcf581085872da76270b109e2795c6940677 | PR #1437 warning consolidation mockups reopen prep | ready-closed: main merged clean; follow-ups renumbered #155-#157; bugbot P2s fixed; origin insteadOf false-positive fixed; verify:pr-local green (444/4646) | verify:pr-local;check:outstanding-issues;merge-tree:clean;pr-bugbot;diff-review | | 2026-07-31 | claude/warning-consolidation-mockups-09jyj7 | b02cfc9258446f6f46bb6acfadd4e978950865c2 | PR #1437 warning consolidation mockups reopen prep | ready-closed at tip (ledger row + prior fixes); PR remains CLOSED; body update attempted | verify:pr-local@7b41fcf5;merge-tree:clean | +| 2026-07-31 | claude/issues-writer-cli (PR #1524) | 83dec1f5a36d577c87ee9a5382ef8431d197d93d | PR #1524 review+bugbot+fix | before: dirty/CONFLICTING vs main (outstanding-issues.md + scripts-index.md), missing pull_request CI, 0 review threads, NOT REVIEWED. after: merged origin/main (prefer main queues; renumbered this PR collision-free-ids note #159→#168, next-id=169); fixed wrong skill/writer cite #154→#156/#168; no other P0–P2 writer defects; 0 threads. Residual: concurrent id RMW (#156/#168) still open. | check:outstanding-issues pass (166 rows, next-id=169); outstanding-issues.mjs --self-test pass; vitest tests/outstanding-issues-writer.test.ts 8/8; merge-tree clean vs origin/main; format clean; no provider-backed checks | From 16050576489dff9ea9ae7e6d4f1cc12443533013 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 31 Jul 2026 08:42:00 +0000 Subject: [PATCH 3/4] fix(docs): refresh scripts-index count after outstanding-issues writer Static PR docs:check-inventory expected 197 script files (this PR adds outstanding-issues.mjs); the merge resolution left the stale 196 count. Co-authored-by: BigSimmo --- docs/scripts-index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/scripts-index.md b/docs/scripts-index.md index a255db7ae6..bf88e3f60b 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -1,6 +1,6 @@ # Scripts index -Curated map of `scripts/` (196 files) and the `package.json` script surface (212 entries), +Curated map of `scripts/` (197 files) and the `package.json` script surface (212 entries), grouped by purpose. This is orientation, not an exhaustive per-file listing — the authoritative command list is `package.json`, and `npm run docs:check-scripts` verifies every `npm run ` referenced in docs resolves to a real script. `npm run docs:update` refreshes the exact counts above. From fb807b11311d8b42198d3bf0ccb87e379e65ba91 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Fri, 31 Jul 2026 08:46:37 +0000 Subject: [PATCH 4/4] fix(docs): stop docs:check-links treating docs/issues/ as a real path #168 proposed a future per-row directory as `docs/issues/`; the trailing slash made check-docs-links require that path to exist. Drop the slash so it is treated as an extensionless directory mention, not a missing file. Co-authored-by: BigSimmo --- docs/outstanding-issues.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/outstanding-issues.md b/docs/outstanding-issues.md index 8b972b79d2..812c49c34f 100644 --- a/docs/outstanding-issues.md +++ b/docs/outstanding-issues.md @@ -149,7 +149,7 @@ removed after current-main verification; it is not missing recommended work. | #165 | P2 | task | Adopt a consolidated answer-home notice block — the studies exist, nothing adopts them | **Outcome:** the answer hero states its safety obligation, its scope, and its verification requirement as one block in one voice. **Detail:** `/mockups/warning-consolidation` (PR #1437) diagnoses today's three stacked notices — the APP-5 privacy warning at 11px muted, a bare `/privacy` link, and an accent-blue `ShieldCheck` capability claim at 14px semibold — and shows the hierarchy is inverted: the least important line is the loudest, and two shields with opposite meanings sit ~40px apart. Three consolidations are drawn at 1440px and 390px. Recommended: **02 Safety card** on the hero (obligation on a warning-tinted top row, everything descriptive in one grey voice below) and **01 Assurance bar** on the docked composer — the same content model at two densities, so one component with a `density` prop covers both. **This is a governance change, not just a design one:** `PrivacyInputNotice` is the single site-wide APP-5 line and renders on the answer, documents and calculators composers, so all three move together; `tests/privacy-ui.test.ts`, `tests/ui-accessibility.spec.ts` and the phone-chrome reserve coverage all assert against the current markup and must change in the same commit; and the PR will need a full `## Clinical Governance Preflight` (the mockup PR correctly did not). **Third study (before/after):** `/mockups/answer-home-proposal` draws the concrete D-direction proposal as a full hero before/after rather than an isolated notice. **Second study (words only):** `/mockups/warning-line` answers a narrower brief — no icon, border, tint or background, one line where width allows. Six variants A-F; line counts measured from the rendered DOM, not asserted. Only B (middot clauses), D (obligation + verify) and F (compressed obligation) hold one line at desktop width, and **none fit one line on a 390px phone while the pinned APP-5 sentence stays verbatim** — 46 characters of obligation plus the 27-character link exceeds the ~60 available at 11px. Recommended there: **D**, the only compliant variant that is both one line and keeps weight-only hierarchy, reached by dropping the scope claim (a capability statement already visible on the answer itself). F fits best but rewrites the pinned obligation to \|No patient-identifiable information.\| and so needs the same privacy sign-off as `#166` plus a matching `tests/privacy-ui.test.ts` update. **Status:** PR #1437 was closed unmerged on 2026-07-30 as a deliberate pause during an owner-authorized ordered merge sweep, to be reopened at its queued place; branch `claude/warning-consolidation-mockups-09jyj7` is preserved and merged onto current `main`; these follow-up rows have been renumbered on each sync because `main` kept claiming the next ids while the PR was paused; the superseded numbers are deliberately not listed, since they now belong to unrelated rows. **Next:** decide block (02 + 01) versus line (D) direction, get wording sign-off for `#166`, then implement behind one component and run `verify:phone-chrome` before `verify:ui`. | session 2026-07-30; PR #1437; `/mockups/warning-consolidation`; `/mockups/warning-line` | 2026-07-30 | | #166 | P2 | issue | Answer mode ships no verify-before-use caveat; every other clinical mode does | **Outcome:** the surface that actually generates prose from retrieved sources says so, and says it must be checked. **Detail:** differentials carry "Clinical decision support only. Review before use.", prescribing carries "Confirm against source", specifiers carry a confirm-the-manual line, and calculators carry "Scores support clinical judgement — they never replace a full assessment." The answer hero carries neither an equivalent nor anything about generation: only the APP-5 privacy line and "Searches indexed clinical sources", which reads as assurance rather than caution. `CLAUDE.md` calls this repo a clinical reference prototype and explicitly **not** validated clinical decision support, so the one mode that synthesises text is the one most needing the caveat. Proposed wording, matching the registers above rather than opening a new one: "Answers are AI-generated — verify against the cited source before clinical use." **Independent of `#165`:** even keeping today's three-notice layout, the missing sentence is the gap. **Next:** clinical-governance sign-off on the exact wording, then add it to the answer hero (bundled with `#165` if that lands first). | session 2026-07-30; PR #1437; `src/components/clinical-dashboard/answer-status.tsx` | 2026-07-30 | | #167 | P2 | issue | `verify:pr-local` exits 0 when its own build step refuses to run | **Outcome:** the PR-local gate cannot report success for a step that never executed. **Detail:** on 2026-07-30 `npm run verify:pr-local` selected the conditional production build for a UI diff; `scripts/guard-next-build.mjs` printed `Refusing to run next build while Clinical KB dev server is running. Stop the dev server first, or set ALLOW_BUILD_WITH_DEV_SERVER=1` — and the aggregate still exited **0**, so the run reported green with the build never run. Caught only by reading the tail of the log; `npm run build` was then re-run separately with the server stopped and passed. Same family as `#120` (`verify:phone-chrome` exits 0 while reporting failed browser tests) and exactly the trap `AGENTS.md` names — "exit code 0 alone is not proof". The guard itself is correct and protects the dev cache; what is wrong is the aggregate treating a refusal as a pass. **Next:** make the refusal exit non-zero, or have `verify:pr-local` list skipped-but-selected steps in its closing summary so a green exit cannot be misread as a build. | session 2026-07-30; PR #1437; `scripts/guard-next-build.mjs` | 2026-07-30 | -| #168 | P2 | rec | Sequential issue ids force every concurrent append to conflict | **Outcome:** two sessions can append to this ledger at the same time without conflicting. **Detail:** ids are allocated read-modify-write against the `issues:next-id` marker inside the file being edited, so two branches both read N and both write N. Because duplicate ids are unacceptable, a union merge driver is unsafe — .gitattributes says so explicitly — which is why this file deliberately has no driver and every overlapping append conflicts by hand. Manual resolution is where rows get dropped: PR #1490 was closed during one and took the only record of four snapshots with it (#152), and ids were renumbered under in-flight work three times in one session (#154, #155). The new writer (`scripts/outstanding-issues.mjs`) removes the mechanical errors but explicitly not this one. **Next:** replace the counter with a collision-free id (ULID, timestamp+suffix, or a content hash), keeping a short display number derived at render time if `#151` reads better than 01JQ…; then a union driver becomes safe to reinstate and concurrent appends stop conflicting at all. A larger variant is one row per file under `docs/issues/` with the table generated, which the repo already does for `site-map.md`. **Stop:** do not reinstate `merge=union` while ids are sequential — that combination was tried in PR #1416 and removed for duplicating rows and the marker. Renumbered from this PR's original `#159` because `main` already used `#159` for the duplicated test-file-list finding. | session 2026-07-31; .gitattributes; #154/#155; PR #1524 sync | 2026-07-31 | +| #168 | P2 | rec | Sequential issue ids force every concurrent append to conflict | **Outcome:** two sessions can append to this ledger at the same time without conflicting. **Detail:** ids are allocated read-modify-write against the `issues:next-id` marker inside the file being edited, so two branches both read N and both write N. Because duplicate ids are unacceptable, a union merge driver is unsafe — .gitattributes says so explicitly — which is why this file deliberately has no driver and every overlapping append conflicts by hand. Manual resolution is where rows get dropped: PR #1490 was closed during one and took the only record of four snapshots with it (#152), and ids were renumbered under in-flight work three times in one session (#154, #155). The new writer (`scripts/outstanding-issues.mjs`) removes the mechanical errors but explicitly not this one. **Next:** replace the counter with a collision-free id (ULID, timestamp+suffix, or a content hash), keeping a short display number derived at render time if `#151` reads better than 01JQ…; then a union driver becomes safe to reinstate and concurrent appends stop conflicting at all. A larger variant is one row per file under `docs/issues` with the table generated, which the repo already does for `site-map.md`. **Stop:** do not reinstate `merge=union` while ids are sequential — that combination was tried in PR #1416 and removed for duplicating rows and the marker. Renumbered from this PR's original `#159` because `main` already used `#159` for the duplicated test-file-list finding. | session 2026-07-31; .gitattributes; #154/#155; PR #1524 sync | 2026-07-31 | ## Resolved / archive