From 6a1cd1419b99092fe5b67185d87a5029447c7668 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sun, 16 Aug 2026 19:59:47 +0800 Subject: [PATCH] feat(ledger): add durable ULID + Crockford display IDs to outstanding-issues tooling --- .claude/hooks/issues-surface.sh | 8 +- docs/scripts-index.md | 2 +- scripts/check-outstanding-issues.mjs | 173 ++++++++++++------------ scripts/issue-id.mjs | 119 ++++++++++++++++ scripts/issues-report.mjs | 6 +- scripts/ledger-inbox.mjs | 42 ++++-- scripts/outstanding-issues.mjs | 58 ++++---- tests/issues-report.test.ts | 7 +- tests/outstanding-issues-writer.test.ts | 41 ++++-- tests/repo-hygiene.test.ts | 12 +- 10 files changed, 327 insertions(+), 141 deletions(-) create mode 100644 scripts/issue-id.mjs diff --git a/.claude/hooks/issues-surface.sh b/.claude/hooks/issues-surface.sh index 933b801ce6..5206c40ec9 100755 --- a/.claude/hooks/issues-surface.sh +++ b/.claude/hooks/issues-surface.sh @@ -32,10 +32,11 @@ source_val="$(printf '%s' "$payload" \ rows="$(awk ' /^## Open items/ { inopen=1; next } /^## / { if (inopen) inopen=0 } - inopen && /^\| #[0-9]/ { + inopen && /^\| #[0-9A-HJKMNP-TV-Z]/ { n=split($0, c, "|") id=c[2]; pri=c[3]; typ=c[4]; sum=c[5] gsub(/^[ \t]+|[ \t]+$/, "", id) + sub(/[ \t]+[ \t]*$/, "", id) gsub(/^[ \t]+|[ \t]+$/, "", pri) gsub(/^[ \t]+|[ \t]+$/, "", typ) gsub(/^[ \t]+|[ \t]+$/, "", sum) @@ -63,10 +64,11 @@ queue_rows="$(awk ' NR==FNR { if ($0 ~ /^## Open items/) { inopen=1; next } if ($0 ~ /^## /) { inopen=0 } - if (inopen && $0 ~ /^\| #[0-9]/) { + if (inopen && $0 ~ /^\| #[0-9A-HJKMNP-TV-Z]/) { split($0, oc, "|") oid=oc[2]; odetail=oc[6] gsub(/^[ \t]+|[ \t]+$/, "", oid) + sub(/[ \t]+[ \t]*$/, "", oid) gsub(/^[ \t]+|[ \t]+$/, "", odetail) detail[oid]=odetail } @@ -114,7 +116,7 @@ fi # Keep the priority summary complementary to the queue instead of repeating # the same recommended IDs in both sections. -queued_ids=" $(printf '%s\n' "$queue_rows" | grep -oE '#[0-9]+' | tr '\n' ' ' || true)" +queued_ids=" $(printf '%s\n' "$queue_rows" | grep -oE '#([0-9A-HJKMNP-TV-Z]{6,16}|[0-9]{3,})' | tr '\n' ' ' || true)" unqueued_rows="$(printf '%s\n' "$rows" | awk -F'\t' -v queued="$queued_ids" ' index(queued, " " $2 " ") == 0 ' || true)" diff --git a/docs/scripts-index.md b/docs/scripts-index.md index 390cb1bab3..810ef988f8 100644 --- a/docs/scripts-index.md +++ b/docs/scripts-index.md @@ -1,6 +1,6 @@ # Scripts index -Curated map of `scripts/` (238 files) and the `package.json` script surface (246 entries), +Curated map of `scripts/` (239 files) and the `package.json` script surface (246 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. diff --git a/scripts/check-outstanding-issues.mjs b/scripts/check-outstanding-issues.mjs index ac586f1b58..2b6e9bc232 100644 --- a/scripts/check-outstanding-issues.mjs +++ b/scripts/check-outstanding-issues.mjs @@ -1,8 +1,10 @@ #!/usr/bin/env node // Structural gate for docs/outstanding-issues.md. // -// Ledger #112. The `issues:next-id` marker is a plain HTML comment that every -// editor read-modify-writes with no lock. A `merge=union` driver was tried (PR +// Ledger #112/#168. The former `issues:next-id` allocator was a plain HTML +// comment that every editor read-modify-wrote with no lock. New rows now carry +// durable ULIDs and collision-extended display locators; a transition marker, +// if still present, is deliberately ignored. A `merge=union` driver was tried (PR // #1416) and removed: unlike docs/branch-review-ledger.md this file allocates // IDs by read-modify-write, so union could not allocate unique IDs either, and // it silently concatenated conflicting hunks — two marker bumps became two @@ -15,7 +17,7 @@ // // This makes each of those failures loud: // - an id used twice is a merge that kept both sides' rows under one number -// - an id above the marker is a merge that kept a row and lost the bump +// - a durable ULID or permanent display locator used twice is an identity collision // - an id in both tables is an archive move that copied instead of moving // - an id absent from both tables relative to the base is a row deletion // - a malformed row is usually a hand-edit that broke the column count @@ -30,6 +32,8 @@ import { execFileSync } from "node:child_process"; import { readFileSync } from "node:fs"; +import { canonicalLegacyIssueId, issueIdCitations, parseIssueIdCell } from "./issue-id.mjs"; + export const ISSUES_PATH = "docs/outstanding-issues.md"; const OPEN_HEADING = "## Open items"; @@ -37,21 +41,6 @@ const ARCHIVE_HEADING = "## Resolved / archive"; const QUEUE_HEADING = "## Recommended execution queue"; const MARKER = //; const PRETTIER_IGNORE = ""; -/** Match `#NNN` citations inside a queue ID(s) cell (backticks/commas allowed). */ -const QUEUE_ID_CITATION = /#(\d+)/g; -/** - * An id cell's shape, e.g. `#042`. Used to READ the number, never to decide - * whether a line is a row. - * - * That distinction is the whole correctness argument. Matching only well-formed - * ids and skipping the rest would make this gate claim more than it does: a - * hand edit turning `#001` into `001` or `#OO1` would drop that row from EVERY - * check below — duplicate detection, the marker comparison, the width check — - * and the file would pass while carrying exactly the malformed row the gate - * advertises. Rows are found positionally instead (see `tableBodies`), and the - * id shape is validated rather than assumed. - */ -const ID_CELL = /^#\d+$/; /** * A table's separator row, e.g. `| ---- | --- |`, which declares its width. * The inner pipes must be in the class: without them this only ever matched a @@ -158,14 +147,14 @@ function orphanRuns(lines, headingIndex, limit, bodies) { } /** - * The canonical rendering of an id number: zero-padded to at least three + * The canonical rendering of a legacy id number: zero-padded to at least three * digits. `#1` and `#001` are the SAME allocation, so accepting both lets a * conflict keep two rows for one number while a string-keyed uniqueness check * calls them distinct. Comparing against this form rejects `#1`, `#0001` and * `#00042` while still allowing the scheme to grow past `#999`. */ export function canonicalId(number) { - return `#${String(number).padStart(3, "0")}`; + return canonicalLegacyIssueId(number); } export function parseIssues(markdown) { @@ -202,6 +191,7 @@ export function parseIssues(markdown) { ...record, id: "", number: null, + ulid: null, valid: false, cellCount: null, expectedCells: body.width, @@ -210,13 +200,13 @@ export function parseIssues(markdown) { continue; } const parsed = cells(line); - const id = parsed[0] ?? ""; - const number = ID_CELL.test(id) ? Number(id.slice(1)) : null; + const identity = parseIssueIdCell(parsed[0] ?? ""); rows.push({ ...record, - id, - number, - valid: number !== null && id === canonicalId(number), + id: identity.id, + number: identity.number, + ulid: identity.ulid, + valid: identity.valid, cellCount: parsed.length, // Each block declares its own width, so a row is checked against the // table it is actually in rather than a section-wide assumption. @@ -236,11 +226,10 @@ export function parseIssues(markdown) { const parsed = cells(line); // Column 0 is Order; column 1 is ID(s). Ignore other cells (evidence prose). const idCell = parsed[1] ?? ""; - for (const match of idCell.matchAll(QUEUE_ID_CITATION)) { + for (const id of issueIdCitations(idCell)) { queueCitations.push({ line: index + 1, - number: Number(match[1]), - raw: match[0], + id, }); } } @@ -261,8 +250,7 @@ export function parseIssues(markdown) { export function checkIssues(markdown, { prettierIgnored = false } = {}) { const problems = []; - const { openStart, archiveStart, nextId, markerCount, rows, orphans, bodyCount, queueCitations } = - parseIssues(markdown); + const { openStart, archiveStart, markerCount, rows, orphans, bodyCount, queueCitations } = parseIssues(markdown); const lines = markdown.split("\n"); // Prettier pads every Markdown table cell to the widest value in its column. @@ -293,68 +281,63 @@ export function checkIssues(markdown, { prettierIgnored = false } = {}) { if (openStart >= 0 && archiveStart >= 0 && archiveStart < openStart) { problems.push(`"${ARCHIVE_HEADING}" appears before "${OPEN_HEADING}"`); } - if (nextId === null) problems.push("missing the marker"); if (markerCount > 1) { - // Only the first is ever read, so a conflict that kept both leaves a stale - // value that a later editor can follow straight into a reused id. problems.push( - `${markerCount} markers — exactly one is allowed; ` + - "a second is a conflict resolution that kept both sides", + `${markerCount} deprecated markers — at most one transition marker is allowed`, ); } - if (rows.length === 0) problems.push("no `| #NNN |` rows found — the parser or the file shape has drifted"); + if (rows.length === 0) problems.push("no outstanding-issue rows found — the parser or the file shape has drifted"); - // The failure this gate exists for: a lost-row merge that left two rows - // sharing one number, so one item's evidence is silently attributed to - // another and the next allocation collides again. + // Invalid spellings stay visible to every downstream collision check. for (const row of rows.filter((entry) => !entry.valid)) { problems.push( row.shape === "not-a-table-row" ? `line ${row.line} (${row.table} table body) is not a table row: ${JSON.stringify(row.raw.slice(0, 60))} — ` + "a row that lost its leading or trailing pipe has left the table while still sitting in it" : `line ${row.line} (${row.table} table) has a non-canonical id ${JSON.stringify(row.id)} — ` + - `ids are zero-padded to three digits (${row.number === null ? "#NNN" : canonicalId(row.number)}), ` + - "so two spellings of one number cannot both exist", + "use a zero-padded legacy #NNN id, or a stored collision-free display id with its derived issue-ulid comment", ); } - // Keyed by NUMBER, not by the raw string: `#1` and `#001` are one allocation, - // and a string key would call them distinct and pass. + // Display locators are permanent citations, so they must remain unique even + // though modern rows also carry a durable ULID. const byId = new Map(); - for (const row of rows.filter((entry) => entry.number !== null)) { - if (!byId.has(row.number)) byId.set(row.number, []); - byId.get(row.number).push(row); + for (const row of rows.filter((entry) => entry.valid || entry.number !== null)) { + const key = row.valid ? row.id : canonicalId(row.number); + if (!byId.has(key)) byId.set(key, []); + byId.get(key).push(row); } - for (const [number, entries] of byId) { + for (const [id, entries] of byId) { if (entries.length > 1) { problems.push( - `${canonicalId(number)} appears ${entries.length} times (lines ${entries.map((entry) => entry.line).join(", ")}) — ` + + `${id} appears ${entries.length} times (lines ${entries.map((entry) => entry.line).join(", ")}) — ` + "ids are never reused; a collision usually means a merge kept both sides under one number", ); } } - // An item cannot be open and resolved at once. This catches an archive move - // that copied the row instead of moving it — the shape a reader trusts least, - // because the two copies then disagree about whether the work is done. - for (const [number, entries] of byId) { - const tables = new Set(entries.map((entry) => entry.table)); - if (tables.size > 1) problems.push(`${canonicalId(number)} is in BOTH the open and archive tables`); + const byUlid = new Map(); + for (const row of rows.filter((entry) => entry.ulid !== null)) { + if (!byUlid.has(row.ulid)) byUlid.set(row.ulid, []); + byUlid.get(row.ulid).push(row); } - - // The marker must lead the whole file, not just the open table: ids are never - // reused, so an archived row still burns its number. - const numbered = rows.filter((row) => row.number !== null); - if (nextId !== null && numbered.length > 0) { - const highest = Math.max(...numbered.map((row) => row.number)); - if (nextId <= highest) { + for (const [ulid, entries] of byUlid) { + if (entries.length > 1) { problems.push( - `issues:next-id=${nextId} is not above the highest id #${String(highest).padStart(3, "0")} — ` + - "the next allocation would reuse a number that is already taken", + `issue ULID ${ulid} appears ${entries.length} times (lines ${entries.map((entry) => entry.line).join(", ")}) — ` + + "durable identities are never reused", ); } } + // An item cannot be open and resolved at once. This catches an archive move + // that copied the row instead of moving it — the shape a reader trusts least, + // because the two copies then disagree about whether the work is done. + for (const [id, entries] of byId) { + const tables = new Set(entries.map((entry) => entry.table)); + if (tables.size > 1) problems.push(`${id} is in BOTH the open and archive tables`); + } + // Rows stranded outside every table. A blank line mid-table is the usual // cause and the least visible one: the rows keep their pipes, so a diff looks // ordinary while GFM stops rendering them as a table at that point. @@ -397,13 +380,11 @@ export function checkIssues(markdown, { prettierIgnored = false } = {}) { // Recommended execution queue may only cite currently open IDs (#201). Parse // the ID(s) column alone so archive mentions in evidence prose do not fail. - const openNumbers = new Set( - rows.filter((row) => row.table === "open" && row.number !== null).map((row) => row.number), - ); + const openIds = new Set(rows.filter((row) => row.table === "open" && row.valid).map((row) => row.id)); for (const citation of queueCitations ?? []) { - if (!openNumbers.has(citation.number)) { + if (!openIds.has(citation.id)) { problems.push( - `recommended queue line ${citation.line} cites ${canonicalId(citation.number)} which is not in Open items — ` + + `recommended queue line ${citation.line} cites ${citation.id} which is not in Open items — ` + "prune the queue row or restore the open item; do not treat evidence prose as queue membership", ); } @@ -422,15 +403,15 @@ export function prettierIgnoreCoversIssues(prettierIgnore) { * therefore passes; deleting it from both tables does not. */ export function missingIssueIds(baseMarkdown, currentMarkdown) { - const numbers = (markdown) => + const ids = (markdown) => new Set( parseIssues(markdown) - .rows.filter((row) => row.number !== null) - .map((row) => row.number), + .rows.filter((row) => row.valid) + .map((row) => row.id), ); - const baseIds = numbers(baseMarkdown); - const currentIds = numbers(currentMarkdown); - return [...baseIds].filter((number) => !currentIds.has(number)).sort((left, right) => left - right); + const baseIds = ids(baseMarkdown); + const currentIds = ids(currentMarkdown); + return [...baseIds].filter((id) => !currentIds.has(id)).sort(); } function argumentValue(name) { @@ -499,9 +480,9 @@ function selfTest() { ], ["a padded table row", good.replace("| #001 | P2 | a |", "| #001 | P2 | a |"), 1], ["a duplicated id", good.replace("| #002 | b |", "| #001 | b |"), 2], // duplicate + both-tables - ["an id at the marker", good.replace("next-id=3", "next-id=2"), 1], + ["a stale transition marker", good.replace("next-id=3", "next-id=2"), 0], ["a row with a stray pipe", good.replace("| #001 | P2 | a |", "| #001 | P2 | a | b |"), 1], - ["a missing marker", good.replace("", ""), 1], + ["a removed transition marker", good.replace("", ""), 0], // A literal pipe inside a cell is escaped, not a column boundary. The real // file has rows like this and an earlier draft of the checker failed them. ["an escaped pipe inside a cell", good.replace("| #001 | P2 | a |", "| #001 | P2 | a \\| b |"), 0], @@ -524,7 +505,7 @@ function selfTest() { // non-canonical + duplicate #001 + in-both-tables: the collision a // string-keyed uniqueness check would have called two distinct ids. ["a short id that collides with a padded one", good.replace("| #002 | b |", "| #1 | b |"), 3], - ["an over-padded id", good.replace("| #001 | P2 | a |", "| #0001 | P2 | a |"), 1], + ["an over-padded id", good.replace("| #001 | P2 | a |", "| #0001 | P2 | a |"), 2], // Deleting a separator used to disable the width check for its whole table // silently — the check had nothing to compare against and skipped. ["a deleted separator row", good.replace("| --- | --- | --- |\n", ""), 3], // no table + orphan pipes + queue cites missing open @@ -542,6 +523,26 @@ function selfTest() { ["a blank line stranding the rows below it", good.replace("| #002 | b |", "| #002 | b |\n\n| #003 | c |"), 1], // #201: queue ID(s) column must cite open items only. ["a queue row citing an archived id", good.replace("| 1 | `#001` |", "| 1 | `#002` |"), 1], + [ + "a modern durable id", + good + .replace("`#001`", "`#ABCDEF`") + .replace("| #001 | P2 | a |", "| #ABCDEF | P2 | a |"), + 0, + ], + [ + "a modern display id without its durable identity", + good.replace("`#001`", "`#ABCDEF`").replace("| #001 | P2 | a |", "| #ABCDEF | P2 | a |"), + 2, + ], + [ + "a reused modern durable identity", + good + .replace("`#001`", "`#ABCDEF`") + .replace("| #001 | P2 | a |", "| #ABCDEF | P2 | a |") + .replace("| #002 | b |", "| #ABCDEF0 | b |"), + 1, + ], ]; let failures = 0; for (const [name, markdown, expected] of cases) { @@ -569,7 +570,7 @@ function selfTest() { const deletionCases = [ ["an unchanged id set", good, []], - ["an open row deleted from both tables", good.replace("| #001 | P2 | a |\n", ""), [1]], + ["an open row deleted from both tables", good.replace("| #001 | P2 | a |\n", ""), ["#001"]], [ "an open row moved to the archive", good.replace("| #001 | P2 | a |\n", "").replace("| #002 | b |", "| #002 | b |\n| #001 | a |"), @@ -645,9 +646,9 @@ function main() { try { const missing = missingIssueIds(readIssuesAtRevision(base.ref), markdown); checkedBase = base.ref; - for (const number of missing) { + for (const id of missing) { problems.push( - `${canonicalId(number)} existed at base ${base.ref.slice(0, 12)} but is absent from both open and archive tables — ` + + `${id} existed at base ${base.ref.slice(0, 12)} but is absent from both open and archive tables — ` + "move resolved or superseded rows to the archive; never delete their allocation", ); } @@ -663,23 +664,25 @@ function main() { // A merge driver on this file is a regression, not an improvement: union // concatenated conflicting hunks and duplicated the whole table rather than // failing (#133, and four times on PR #1430). Honest conflicts are the - // contract; ids still need manual renumbering either way. + // contract. Immutable inbox requests plus serialized reconciliation remain + // the only supported way to mutate the canonical ledger. const mergeProblem = mergeAttributeProblem(effectiveMergeAttribute()); if (mergeProblem) problems.push(mergeProblem); if (problems.length > 0) { console.error(`${ISSUES_PATH} check FAILED:`); for (const problem of problems) console.error(` - ${problem}`); console.error( - "\nIds are never reused. If a merge collided, renumber the incoming rows above the marker " + - "and bump it — do not resolve by taking one side wholesale, which drops the other's rows.", + "\nIds are never reused. Do not hand-edit or renumber rows after a conflict: preserve immutable " + + "inbox requests and run one serialized reconciliation from a fresh origin/main base.", ); process.exit(1); } - const { rows, nextId } = parseIssues(markdown); + const { rows, markerCount } = parseIssues(markdown); const open = rows.filter((row) => row.table === "open").length; console.log( `Outstanding-issues guard passed: ${rows.length} rows (${open} open, ${rows.length - open} archived), ` + - `unique ids, next-id=${nextId} above the highest, no merge driver` + + `unique display and durable ids, collision-free allocation enabled` + + `${markerCount ? ", deprecated next-id marker ignored" : ""}, no merge driver` + `${checkedBase ? `, no ids deleted from base ${checkedBase.slice(0, 12)}` : ", deletion baseline unavailable"}.`, ); } diff --git a/scripts/issue-id.mjs b/scripts/issue-id.mjs new file mode 100644 index 0000000000..8b6c4bc353 --- /dev/null +++ b/scripts/issue-id.mjs @@ -0,0 +1,119 @@ +import { createHash, randomBytes } from "node:crypto"; + +const CROCKFORD = "0123456789ABCDEFGHJKMNPQRSTVWXYZ"; +const CROCKFORD_CHARS = "[0-9A-HJKMNP-TV-Z]"; + +export const ISSUE_ULID_PATTERN = new RegExp(`^[0-7]${CROCKFORD_CHARS}{25}$`); +export const LEGACY_ISSUE_ID_PATTERN = /^#\d{3,}$/; +export const DISPLAY_ISSUE_ID_PATTERN = new RegExp(`^#${CROCKFORD_CHARS}{6,16}$`); +const ISSUE_ID_CELL_PATTERN = new RegExp( + `^(#(?:\\d{3,}|${CROCKFORD_CHARS}{6,16}))(?:\\s+)?$`, +); +const ISSUE_CITATION_PATTERN = new RegExp(`#(?:${CROCKFORD_CHARS}{6,16}|\\d{3,})`, "g"); + +function encodeBase32(value, length) { + let remaining = BigInt(value); + const output = Array.from({ length }, () => "0"); + for (let index = length - 1; index >= 0; index -= 1) { + output[index] = CROCKFORD[Number(remaining & 31n)]; + remaining >>= 5n; + } + if (remaining !== 0n) throw new Error(`value does not fit in ${length} Crockford base32 characters`); + return output.join(""); +} + +function entropyValue(bytes) { + if (!(bytes instanceof Uint8Array) || bytes.length !== 10) { + throw new Error("ULID entropy must be exactly 10 bytes"); + } + let value = 0n; + for (const byte of bytes) value = (value << 8n) | BigInt(byte); + return value; +} + +export function issueUlid(timestamp = Date.now(), entropy = randomBytes(10)) { + if (!Number.isSafeInteger(timestamp) || timestamp < 0 || timestamp > 0xffffffffffff) { + throw new Error("ULID timestamp must be a non-negative 48-bit integer"); + } + return `${encodeBase32(BigInt(timestamp), 10)}${encodeBase32(entropyValue(entropy), 16)}`; +} + +/** Stable identity for legacy version-1 add requests during the schema transition. */ +export function issueUlidFromRequest(createdOn, requestId) { + const timestamp = Date.parse(`${createdOn}T00:00:00.000Z`); + if (!Number.isSafeInteger(timestamp)) throw new Error(`cannot derive an issue ULID from createdOn=${createdOn}`); + const entropy = createHash("sha256").update(String(requestId)).digest().subarray(0, 10); + return issueUlid(timestamp, entropy); +} + +export function isIssueUlid(value) { + return ISSUE_ULID_PATTERN.test(String(value ?? "")); +} + +export function canonicalLegacyIssueId(number) { + return `#${String(number).padStart(3, "0")}`; +} + +export function isIssueDisplayId(value) { + const id = String(value ?? ""); + if (DISPLAY_ISSUE_ID_PATTERN.test(id)) return true; + if (!LEGACY_ISSUE_ID_PATTERN.test(id)) return false; + const number = Number(id.slice(1)); + return Number.isSafeInteger(number) && id === canonicalLegacyIssueId(number); +} + +export function displayIdForUlid(ulid, length = 6) { + if (!isIssueUlid(ulid)) throw new Error(`invalid issue ULID: ${ulid}`); + if (!Number.isInteger(length) || length < 6 || length > 16) { + throw new Error("issue display-id length must be between 6 and 16"); + } + return `#${ulid.slice(10, 10 + length)}`; +} + +export function allocateDisplayId(ulid, usedIds) { + const allocated = usedIds instanceof Set ? usedIds : new Set(usedIds); + for (let length = 6; length <= 16; length += 1) { + const candidate = displayIdForUlid(ulid, length); + if (!allocated.has(candidate)) return candidate; + } + throw new Error(`issue ULID ${ulid} duplicates an existing durable identity`); +} + +export function issueIdCell(displayId, ulid) { + if (!DISPLAY_ISSUE_ID_PATTERN.test(String(displayId ?? ""))) { + throw new Error(`invalid collision-free issue display id: ${displayId}`); + } + if (!isIssueUlid(ulid)) throw new Error(`invalid issue ULID: ${ulid}`); + const suffix = ulid.slice(10); + if (!suffix.startsWith(displayId.slice(1))) { + throw new Error(`issue display id ${displayId} is not derived from ULID ${ulid}`); + } + return `${displayId} `; +} + +export function parseIssueIdCell(cell) { + const raw = String(cell ?? "").trim(); + const match = raw.match(ISSUE_ID_CELL_PATTERN); + if (!match) { + const numeric = raw.match(/^#(\d+)$/); + const number = numeric ? Number(numeric[1]) : null; + return { id: raw, number: Number.isSafeInteger(number) ? number : null, ulid: null, valid: false }; + } + const [, id, ulid = null] = match; + if (ulid) { + const valid = DISPLAY_ISSUE_ID_PATTERN.test(id) && ulid.slice(10).startsWith(id.slice(1)); + return { id, number: null, ulid, valid }; + } + if (!LEGACY_ISSUE_ID_PATTERN.test(id)) return { id, number: null, ulid: null, valid: false }; + const number = Number(id.slice(1)); + return { + id, + number, + ulid: null, + valid: Number.isSafeInteger(number) && id === canonicalLegacyIssueId(number), + }; +} + +export function issueIdCitations(value) { + return [...String(value ?? "").matchAll(ISSUE_CITATION_PATTERN)].map((match) => match[0]); +} diff --git a/scripts/issues-report.mjs b/scripts/issues-report.mjs index f85f01964b..42447a3a7d 100644 --- a/scripts/issues-report.mjs +++ b/scripts/issues-report.mjs @@ -4,6 +4,7 @@ import { readFileSync } from "node:fs"; import { fileURLToPath } from "node:url"; import { parseIssues } from "./check-outstanding-issues.mjs"; +import { issueIdCitations } from "./issue-id.mjs"; import { splitCells } from "./outstanding-issues.mjs"; const LEDGER_PATH = "docs/outstanding-issues.md"; @@ -31,7 +32,7 @@ function queueRows(markdown) { if (cells.length !== 7) continue; rows.push({ order: Number(cells[0]), - ids: [...cells[1].matchAll(/#\d+/g)].map((match) => match[0]), + ids: issueIdCitations(cells[1]), acuity: cells[2], capability: cells[3], when: cells[4], @@ -75,7 +76,8 @@ export function buildIssuesReport(markdown, source) { .map((row) => { const cells = splitCells(row.raw); return { - id: cells[0], + // parseIssues strips the durable ULID comment from the human-facing id. + id: row.id, priority: cells[1], type: cells[2], summary: cells[3], diff --git a/scripts/ledger-inbox.mjs b/scripts/ledger-inbox.mjs index 05c74844e2..952f718bd3 100644 --- a/scripts/ledger-inbox.mjs +++ b/scripts/ledger-inbox.mjs @@ -3,8 +3,8 @@ * Conflict-free intake for the outstanding-issues ledger. * * Feature branches write one immutable request file; only a deliberately serialized - * `reconcile` operation edits docs/outstanding-issues.md and allocates numeric IDs. - * This keeps a busy PR queue from contending on the next-id marker or one table row. + * `reconcile` operation edits docs/outstanding-issues.md. Add requests carry a + * durable ULID, so independent branches no longer contend on a numeric marker. */ import { randomUUID } from "node:crypto"; import { execFileSync } from "node:child_process"; @@ -14,6 +14,7 @@ import { fileURLToPath } from "node:url"; import { addIssue, resolveIssue, updateIssue } from "./outstanding-issues.mjs"; import { ISSUES_PATH, checkIssues } from "./check-outstanding-issues.mjs"; +import { isIssueDisplayId, isIssueUlid, issueUlid, issueUlidFromRequest } from "./issue-id.mjs"; const ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), ".."); const INBOX_DIR = "docs/outstanding-issues-inbox"; @@ -41,7 +42,7 @@ function requestPath(id) { export function validateRequest(request) { const problems = []; if (!request || typeof request !== "object") return ["request must be an object"]; - if (request.version !== 1) problems.push("version must be 1"); + if (![1, 2].includes(request.version)) problems.push("version must be 1 or 2"); if (!REQUEST_ID.test(request.id ?? "")) problems.push("id must be a UUID"); if (!/^\d{4}-\d{2}-\d{2}$/.test(request.createdOn ?? "")) problems.push("createdOn must be YYYY-MM-DD"); if (!ACTIONS.has(request.action)) problems.push("action must be add, done, update, or cancel"); @@ -49,13 +50,19 @@ export function validateRequest(request) { if (request.action === "add") { for (const field of ["pri", "type", "summary"]) if (!request.payload?.[field]) problems.push(`add requires ${field}`); + if (request.payload?.issueUlid !== undefined && !isIssueUlid(request.payload.issueUlid)) { + problems.push("add issueUlid must be a valid ULID"); + } + if (request.version === 2 && request.payload?.issueUlid === undefined) { + problems.push("version 2 add requires a valid issueUlid"); + } } if (request.action === "done") { - if (!/^#\d{3,}$/.test(request.payload?.id ?? "")) problems.push("done requires a canonical #NNN id"); + if (!isIssueDisplayId(request.payload?.id)) problems.push("done requires a canonical issue display id"); if (!request.payload?.outcome) problems.push("done requires outcome"); } if (request.action === "update") { - if (!/^#\d{3,}$/.test(request.payload?.id ?? "")) problems.push("update requires a canonical #NNN id"); + if (!isIssueDisplayId(request.payload?.id)) problems.push("update requires a canonical issue display id"); // `pri` counts as a mutation on its own: a re-prioritisation with no prose // change is a legitimate and common triage edit, and leaving it out here // made `--pri` unusable alone even once the CLI could emit it (ledger #313). @@ -82,7 +89,10 @@ export function applyRequest(markdown, request) { throw new Error("cancel requests must be applied through batch reconciliation"); } const options = { date: request.createdOn }; - if (request.action === "add") return addIssue(markdown, request.payload, options); + if (request.action === "add") { + const durableId = request.payload.issueUlid ?? issueUlidFromRequest(request.createdOn, request.id); + return addIssue(markdown, request.payload, { ...options, issueUlid: durableId }); + } if (request.action === "done") return resolveIssue(markdown, request.payload.id, request.payload.outcome, options); return updateIssue(markdown, request.payload.id, request.payload); } @@ -374,6 +384,7 @@ function createRequest(action, argv) { summary: argValue(argv, "summary"), detail: argValue(argv, "detail"), source: argValue(argv, "source"), + issueUlid: issueUlid(), } : action === "done" ? { id: argv[1], outcome: argValue(argv, "outcome") } @@ -392,7 +403,7 @@ function createRequest(action, argv) { detail: argValue(argv, "detail"), source: argValue(argv, "source"), }; - const request = { version: 1, id: randomUUID(), createdOn: date(), action, payload }; + const request = { version: 2, id: randomUUID(), createdOn: date(), action, payload }; const problems = validateRequest(request); if (problems.length > 0) throw new Error(problems.join("; ")); const relative = requestPath(request.id); @@ -584,7 +595,22 @@ function selfTest() { } const added = applyRequest(base, add); - if (!added.includes("#002")) throw new Error("self-test failed: queued add did not preserve ledger invariants"); + const replayed = applyRequest(base, add); + const legacyUlid = issueUlidFromRequest(add.createdOn, add.id); + if (!added.includes(`issue-ulid:${legacyUlid}`) || added !== replayed) { + throw new Error("self-test failed: legacy queued add did not derive a stable durable id"); + } + const v2Add = { + ...add, + version: 2, + payload: { ...add.payload, issueUlid: issueUlid(1, new Uint8Array(10)) }, + }; + if (validateRequest(v2Add).length > 0 || !applyRequest(base, v2Add).includes(v2Add.payload.issueUlid)) { + throw new Error("self-test failed: version 2 add did not preserve its durable id"); + } + if (validateRequest({ ...v2Add, payload: { ...v2Add.payload, issueUlid: "invalid" } }).length === 0) { + throw new Error("self-test failed: invalid version 2 durable id accepted"); + } const resolved = applyRequest(base, done); if (resolved.includes("`#001`")) throw new Error("self-test failed: queued resolve did not preserve ledger invariants"); diff --git a/scripts/outstanding-issues.mjs b/scripts/outstanding-issues.mjs index 0ae1d6b593..2bbe836b75 100644 --- a/scripts/outstanding-issues.mjs +++ b/scripts/outstanding-issues.mjs @@ -12,18 +12,17 @@ // 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 +// - ids formerly allocated by reading a numeric 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 #156 / #168 territory and needs a -// different id scheme, not a better writer. +// New rows use a request-owned durable ULID and a permanent Crockford display +// locator. The locator starts at six characters and extends only on collision; +// existing sequential ids remain valid and are never rewritten. // // Usage: // node scripts/outstanding-issues.mjs add --pri P2 --type issue \ @@ -43,7 +42,8 @@ import { readFileSync, writeFileSync } from "node:fs"; import { pathToFileURL } from "node:url"; -import { ISSUES_PATH, canonicalId, checkIssues, parseIssues } from "./check-outstanding-issues.mjs"; +import { ISSUES_PATH, checkIssues, parseIssues } from "./check-outstanding-issues.mjs"; +import { allocateDisplayId, displayIdForUlid, issueIdCell, issueIdCitations, issueUlid } from "./issue-id.mjs"; const OPEN_CELLS = 7; // ID | Pri | Type | Summary | Detail / next action | Source | Added const ARCHIVE_CELLS = 5; // ID | Type | Summary | Outcome | Resolved @@ -124,8 +124,7 @@ function isQueueSeparatorRow(cells) { * - Renumbers Order 1..N to close gaps (skill contract). */ export function pruneResolvedIdFromQueue(markdown, id) { - const target = Number(String(id).replace(/^#/, "")); - if (!Number.isFinite(target)) return markdown; + const target = String(id); const lines = markdown.split("\n"); const queueStart = lines.findIndex((line) => line.startsWith("## Recommended execution queue")); @@ -142,15 +141,15 @@ export function pruneResolvedIdFromQueue(markdown, id) { if (cells.length < 2 || isQueueHeaderRow(cells) || isQueueSeparatorRow(cells)) continue; const idCell = cells[1] ?? ""; - const cited = [...idCell.matchAll(/#(\d+)/g)].map((match) => Number(match[1])); + const cited = issueIdCitations(idCell); if (!cited.includes(target)) continue; - const remaining = cited.filter((number) => number !== target); + const remaining = cited.filter((candidate) => candidate !== target); if (remaining.length === 0) { lines.splice(index, 1); continue; } - cells[1] = remaining.map((number) => `\`${canonicalId(number)}\``).join(", "); + cells[1] = remaining.map((candidate) => `\`${candidate}\``).join(", "); lines[index] = buildRow(cells); } @@ -196,12 +195,12 @@ export function addIssue(markdown, fields, options = {}) { 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 ulid = String(fields.issueUlid ?? options.issueUlid ?? issueUlid()); + const id = allocateDisplayId(ulid, new Set(parsed.rows.filter((entry) => entry.valid).map((entry) => entry.id))); const row = buildRow([ - id, + issueIdCell(id, ulid), pri, type, escapeCell(fields.summary), @@ -218,9 +217,7 @@ export function addIssue(markdown, fields, options = {}) { const lines = current.split("\n"); lines.splice(anchor + 1, 0, row); - let next = lines.join("\n"); - next = next.replace(//, ``); - return next; + return lines.join("\n"); }); } @@ -323,32 +320,35 @@ function selfTest() { if (!condition) failures.push(label); }; - // add: lands in the OPEN table, takes the marker's id, bumps it. + const testUlid = issueUlid(Date.UTC(2026, 1, 2), new Uint8Array([1, 2, 3, 4, 5, 6, 7, 8, 9, 10])); + const testId = displayIdForUlid(testUlid); + + // add: lands in the OPEN table with a durable, collision-free identity. const added = addIssue( fixture, { pri: "P1", type: "rec", summary: "third", detail: "d", source: "s" }, - { date: "2026-02-02" }, + { date: "2026-02-02", issueUlid: testUlid }, ); const addedParsed = parseIssues(added); check( - "add uses the marker id", - addedParsed.rows.some((r) => r.id === "#017" && r.table === "open"), + "add stores the derived display id", + addedParsed.rows.some((r) => r.id === testId && r.ulid === testUlid && r.table === "open"), ); - check("add bumps the marker", addedParsed.nextId === 18); - check("add appends after the last open row", added.indexOf("#017") > added.indexOf("#016")); - check("add stays out of the archive", !addedParsed.rows.some((r) => r.id === "#017" && r.table === "archive")); + check("add leaves the deprecated marker untouched", addedParsed.nextId === 17); + check("add appends after the last open row", added.indexOf(testId) > added.indexOf("#016")); + check("add stays out of the archive", !addedParsed.rows.some((r) => r.id === testId && 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("| #017 ") < added.indexOf("## Resolved / archive")); + check("add lands before the archive heading", added.indexOf(`| ${testId} `) < 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" }, + { date: "2026-02-02", issueUlid: testUlid }, ); - const pipedRow = parseIssues(piped).rows.find((r) => r.id === "#017"); + const pipedRow = parseIssues(piped).rows.find((r) => r.id === testId); 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")); @@ -528,7 +528,7 @@ function main() { 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}.`); + console.log(`${ISSUES_PATH} updated: ${open} open, ${archived} archived, collision-free id allocation enabled.`); } const invokedDirectly = process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href; diff --git a/tests/issues-report.test.ts b/tests/issues-report.test.ts index 0484b5ebca..f73b24b41b 100644 --- a/tests/issues-report.test.ts +++ b/tests/issues-report.test.ts @@ -120,12 +120,12 @@ describe("issues report", () => { "| Order | ID(s) | Acuity | Capability | When | Estimate | Outcome, gate, verification, and stopping condition |", "| ----: | ---- | ---- | ---- | ---- | ---- | ---- |", "| 1 | `#001` | A1 | Operator | Now | 1 hour | Live action |", - "| 2 | `#002` | A2 | Standard | Next | 30 min | Offline guard |", + "| 2 | `#ABCDEF` | A2 | Standard | Next | 30 min | Offline guard |", "## Open items", "| ID | Pri | Type | Summary | Detail / next action | Source | Added |", "| ---- | --- | ---- | ---- | ---- | ---- | ---- |", "| #001 | P1 | task | urgent | detail | src | 2026-01-01 |", - "| #002 | P2 | task | safe | detail | src | 2026-01-01 |", + "| #ABCDEF | P2 | task | safe | detail | src | 2026-01-01 |", "## Resolved / archive", "| ID | Type | Summary | Outcome | Resolved |", "| ---- | ---- | ---- | ---- | ---- |", @@ -134,7 +134,8 @@ describe("issues report", () => { const report = buildIssuesReport(markdown, { ref: "origin/main", revalidated: true }); expect(report.counts).toEqual({ open: 2, recommended: 2 }); expect(report.priorityBlockers[0].ids).toEqual(["#001"]); - expect(report.agentSafeWins.map((row: { ids: string[] }) => row.ids[0])).toEqual(["#002"]); + expect(report.agentSafeWins.map((row: { ids: string[] }) => row.ids[0])).toEqual(["#ABCDEF"]); + expect(report.open[1].id).toBe("#ABCDEF"); expect(report.open[0].added).toBe("2026-01-01"); }); diff --git a/tests/outstanding-issues-writer.test.ts b/tests/outstanding-issues-writer.test.ts index 6cdd1b3dd8..c1f5b7bd49 100644 --- a/tests/outstanding-issues-writer.test.ts +++ b/tests/outstanding-issues-writer.test.ts @@ -1,10 +1,13 @@ import { describe, expect, it } from "vitest"; import { parseIssues } from "../scripts/check-outstanding-issues.mjs"; +import { displayIdForUlid, issueUlid } from "../scripts/issue-id.mjs"; import { addIssue, escapeCell, resolveIssue, splitCells, updateIssue } from "../scripts/outstanding-issues.mjs"; const OPEN_CELLS = 7; const ARCHIVE_CELLS = 5; +const TEST_ULID = issueUlid(Date.UTC(2026, 1, 2), Buffer.from([1, 2, 3, 4, 5, 6, 7, 8, 9, 10])); +const TEST_ID = displayIdForUlid(TEST_ULID); const ledger = [ "# Outstanding", @@ -32,21 +35,26 @@ 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"); + const next = addIssue( + ledger, + { pri: "P1", type: "rec", summary: "third" }, + { date: "2026-02-02", issueUlid: TEST_ULID }, + ); + const row = rowFor(next, TEST_ID); expect(row?.table).toBe("open"); - expect(next.indexOf("| #007 ")).toBeLessThan(next.indexOf("## Resolved / archive")); + expect(row?.ulid).toBe(TEST_ULID); + expect(next.indexOf(`| ${TEST_ID} `)).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("derives a permanent display id and leaves the deprecated marker untouched", () => { + const next = addIssue(ledger, { summary: "third" }, { date: "2026-02-02", issueUlid: TEST_ULID }); + expect(rowFor(next, TEST_ID)?.ulid).toBe(TEST_ULID); + expect(parseIssues(next).nextId).toBe(7); }); 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"); + const next = addIssue(ledger, { summary: "a | b", detail: "c | d" }, { date: "2026-02-02", issueUlid: TEST_ULID }); + const row = rowFor(next, TEST_ID); expect(splitCells(row!.raw)).toHaveLength(OPEN_CELLS); expect(row!.raw).toContain("a \\| b"); }); @@ -68,6 +76,21 @@ describe("outstanding-issues writer", () => { expect(row!.raw).toContain("replaced \\| detail"); }); + it("extends a colliding display id and preserves it through update and archive", () => { + const firstUlid = "0000000000ABCDEF0000000000"; + const secondUlid = "0000000000ABCDEF1000000000"; + const first = addIssue(ledger, { summary: "first modern" }, { date: "2026-02-02", issueUlid: firstUlid }); + const second = addIssue(first, { summary: "second modern" }, { date: "2026-02-03", issueUlid: secondUlid }); + expect(rowFor(second, "#ABCDEF")?.ulid).toBe(firstUlid); + expect(rowFor(second, "#ABCDEF1")?.ulid).toBe(secondUlid); + + const updated = updateIssue(second, "#ABCDEF1", { detail: "retained identity" }); + const archived = resolveIssue(updated, "#ABCDEF1", "done", { date: "2026-03-03" }); + const row = rowFor(archived, "#ABCDEF1"); + expect(row?.table).toBe("archive"); + expect(row?.ulid).toBe(secondUlid); + }); + 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/); diff --git a/tests/repo-hygiene.test.ts b/tests/repo-hygiene.test.ts index 8d443d34ae..4caf4cf47a 100644 --- a/tests/repo-hygiene.test.ts +++ b/tests/repo-hygiene.test.ts @@ -641,7 +641,15 @@ describe("outstanding-issues inbox", () => { payload: { pri: "P2", type: "issue", summary: "queued" }, }; expect(validateRequest(add)).toEqual([]); + const currentAdd = { + ...add, + version: 2, + payload: { ...add.payload, issueUlid: "0000000000ABCDEF0000000000" }, + }; + expect(validateRequest(currentAdd)).toEqual([]); + expect(validateRequest({ ...currentAdd, payload: { ...currentAdd.payload, issueUlid: "bad" } })).not.toEqual([]); expect(validateRequest({ ...add, action: "done", payload: { outcome: "no id" } })).not.toEqual([]); + expect(validateRequest({ ...add, action: "done", payload: { id: "#ABCDEF", outcome: "done" } })).toEqual([]); const ledger = [ "", @@ -671,6 +679,8 @@ describe("outstanding-issues inbox", () => { "| #000 | issue | old | done | 2026-01-01 |", "", ].join("\n"); - expect(applyRequest(ledger, add)).toContain("| #002 | P2 | issue | queued |"); + const applied = applyRequest(ledger, currentAdd); + expect(applied).toContain("| #ABCDEF | P2 | issue | queued |"); + expect(applied).toContain(""); }); });