Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
| 2026-08-17 | 2018 | a4338471f29c12c4f98b5abf4910aaf6f461d992 | merge-conflict resolution + review fixes for PR #2009 (docs/filter-contract.md, scripts/check-outstanding-issues.mjs, scripts/ledger-inbox.mjs, src/components/clinical-dashboard/account-setup-dialog.tsx, tests/ui-smoke.spec.ts, tests/ui-tools.spec.ts) | reviewed and fixed: resolved 6-file merge conflict against main, fixed 2 CodeRabbit findings (fingerprint case-sensitivity, URL regex boundary), left 2 findings unaddressed (fingerprint-mandatory migration risk, design-token nitpick) | check-outstanding-issues self-test, ledger-inbox self-test, check:outstanding-issues, check-ledger-write-discipline, focused vitest (outstanding-issues-writer, repo-hygiene, ledger-inbox-cancellation, favourites-auth-gate), prettier --check, typecheck, eslint |
22 changes: 22 additions & 0 deletions scripts/check-outstanding-issues.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -31,6 +31,7 @@

import { execFileSync } from "node:child_process";
import { readFileSync } from "node:fs";
import { createHash } from "node:crypto";

import { canonicalLegacyIssueId, issueIdCitations, parseIssueIdCell } from "./issue-id.mjs";

Expand All@@ -41,6 +42,7 @@ const ARCHIVE_HEADING = "## Resolved / archive";
const QUEUE_HEADING = "## Recommended execution queue";
const MARKER = /<!--\s*issues:next-id=(\d+)\s*-->/;
const PRETTIER_IGNORE = "<!-- prettier-ignore -->";
const ISSUE_ROW_FINGERPRINT = /^[0-9a-f]{64}$/i;
/**
* 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
Expand DownExpand Up@@ -248,6 +250,26 @@ export function parseIssues(markdown) {
};
}

export function issueRowFingerprint(markdown, issueId) {
const match = String(issueId)
.trim()
.match(/^#(\d+)$/);
if (!match) return null;
const number = Number(match[1]);
if (!Number.isFinite(number)) return null;

const row = parseIssues(markdown).rows.find(
(entry) => entry.number === number && entry.table === "open" && entry.valid && entry.raw,
);
if (!row) return null;
const normalized = `| ${cells(row.raw).join(" | ")} |`;
return createHash("sha256").update(normalized).digest("hex");
}

export function isValidIssueRowFingerprint(value) {
return ISSUE_ROW_FINGERPRINT.test(String(value ?? ""));
}

export function checkIssues(markdown, { prettierIgnored = false } = {}) {
const problems = [];
const { openStart, archiveStart, markerCount, rows, orphans, bodyCount, queueCitations } = parseIssues(markdown);
Expand Down
62 changes: 58 additions & 4 deletions scripts/ledger-inbox.mjs
Original file line numberDiff line numberDiff line change
Expand Up@@ -13,7 +13,12 @@ import path from "node:path";
import { fileURLToPath } from "node:url";

import { addIssue, resolveIssue, updateIssue } from "./outstanding-issues.mjs";
import { ISSUES_PATH, checkIssues } from "./check-outstanding-issues.mjs";
import {
ISSUES_PATH,
checkIssues,
issueRowFingerprint,
isValidIssueRowFingerprint,
} from "./check-outstanding-issues.mjs";
import { isIssueDisplayId, isIssueUlid, issueUlid, issueUlidFromRequest } from "./issue-id.mjs";

const ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), "..");
Expand All@@ -39,6 +44,10 @@ function requestPath(id) {
return path.posix.join(INBOX_DIR, `${id}.json`);
}

function readOutstandingIssues() {
return readFileSync(path.join(ROOT, ISSUES_PATH), "utf8");
}

export function validateRequest(request) {
const problems = [];
if (!request || typeof request !== "object") return ["request must be an object"];
Expand All@@ -60,6 +69,11 @@ export function validateRequest(request) {
if (request.action === "done") {
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.payload?.baseRowFingerprint !== undefined &&
!isValidIssueRowFingerprint(request.payload.baseRowFingerprint)
)
problems.push("done requires a valid baseRowFingerprint");
}
if (request.action === "update") {
if (!isIssueDisplayId(request.payload?.id)) problems.push("update requires a canonical issue display id");
Expand All@@ -69,6 +83,11 @@ export function validateRequest(request) {
if (!["pri", "summary", "detail", "source"].some((field) => request.payload?.[field] !== undefined)) {
problems.push("update requires pri, summary, detail, or source");
}
if (
request.payload?.baseRowFingerprint !== undefined &&
!isValidIssueRowFingerprint(request.payload.baseRowFingerprint)
)
problems.push("update requires a valid baseRowFingerprint");
if (request.payload?.pri !== undefined && !["P1", "P2", "P3"].includes(String(request.payload.pri))) {
problems.push("update pri must be P1, P2, or P3");
}
Expand All@@ -89,6 +108,18 @@ export function applyRequest(markdown, request) {
throw new Error("cancel requests must be applied through batch reconciliation");
}
const options = { date: request.createdOn };
if ((request.action === "done" || request.action === "update") && request.payload?.baseRowFingerprint) {
const id = request.payload.id;
const fingerprint = issueRowFingerprint(markdown, id);
if (!fingerprint) {
throw new Error(`${id} is no longer open; reread and reissue this request from the latest ledger`);
}
if (fingerprint !== String(request.payload.baseRowFingerprint).toLowerCase()) {
throw new Error(
`${id} is stale: the ledger row changed after this request was queued; reread and reissue from the latest ledger`,
);
}
}
if (request.action === "add") {
const durableId = request.payload.issueUlid ?? issueUlidFromRequest(request.createdOn, request.id);
return addIssue(markdown, request.payload, { ...options, issueUlid: durableId });
Expand DownExpand Up@@ -403,6 +434,13 @@ function createRequest(action, argv) {
detail: argValue(argv, "detail"),
source: argValue(argv, "source"),
};
if (["done", "update"].includes(action) && typeof payload.id === "string") {
const currentFingerprint = issueRowFingerprint(readOutstandingIssues(), payload.id);
if (currentFingerprint === null) {
throw new Error(`ledger request rejected: ${payload.id} is not in Open items`);
}
payload.baseRowFingerprint = currentFingerprint;
}
const request = { version: 2, id: randomUUID(), createdOn: date(), action, payload };
const problems = validateRequest(request);
if (problems.length > 0) throw new Error(problems.join("; "));
Expand DownExpand Up@@ -557,14 +595,14 @@ function selfTest() {
id: "22222222-2222-4222-8222-222222222222",
createdOn: "2026-08-13",
action: "done",
payload: { id: "#001", outcome: "done" },
payload: { id: "#001", outcome: "done", baseRowFingerprint: issueRowFingerprint(base, "#001") },
};
const update = {
version: 1,
id: "33333333-3333-4333-8333-333333333333",
createdOn: "2026-08-13",
action: "update",
payload: { id: "#001", summary: "updated" },
payload: { id: "#001", summary: "updated", baseRowFingerprint: issueRowFingerprint(base, "#001") },
};
const cancel = {
version: 1,
Expand All@@ -581,7 +619,7 @@ function selfTest() {
id: "55555555-5555-4555-8555-555555555555",
createdOn: "2026-08-13",
action: "update",
payload: { id: "#001", pri: "P3" },
payload: { id: "#001", pri: "P3", baseRowFingerprint: issueRowFingerprint(base, "#001") },
};
if (validateRequest(reprioritise).length > 0) {
throw new Error("self-test failed: a pri-only update request must validate");
Expand DownExpand Up@@ -617,6 +655,22 @@ function selfTest() {
if (validateRequest({ ...add, payload: {} }).length === 0)
throw new Error("self-test failed: invalid request accepted");

const staleBase = base.replace("one", "stale");
let staleRejected = false;
try {
applyRequest(staleBase, done);
} catch (error) {
staleRejected = /stale/.test(String(error));
}
if (!staleRejected) throw new Error("self-test failed: stale done request was not rejected");
let staleUpdateRejected = false;
try {
applyRequest(staleBase, update);
} catch (error) {
staleUpdateRejected = /stale/.test(String(error));
}
if (!staleUpdateRejected) throw new Error("self-test failed: stale update request was not rejected");

let conflictRejected = false;
try {
applyRequestBatch(base, [done, update]);
Expand Down
55 changes: 54 additions & 1 deletion tests/repo-hygiene.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -36,7 +36,7 @@ import {
sanitizeCell,
} from "../scripts/branch-review-ledger.mjs";
import { validateLedger } from "../scripts/check-branch-review-ledger.mjs";
import { mergeAttributeProblem } from "../scripts/check-outstanding-issues.mjs";
import { issueRowFingerprint, mergeAttributeProblem } from "../scripts/check-outstanding-issues.mjs";
import { applyRequest, validateRequest } from "../scripts/ledger-inbox.mjs";

describe("check-env-parity name parsing", () => {
Expand DownExpand Up@@ -683,4 +683,57 @@ describe("outstanding-issues inbox", () => {
expect(applied).toContain("| #ABCDEF <!-- issue-ulid:0000000000ABCDEF0000000000 --> | P2 | issue | queued |");
expect(applied).toContain("<!-- issues:next-id=2 -->");
});

it("rejects stale done/update requests when the row hash changed after queueing", () => {
const ledger = [
"<!-- issues:next-id=2 -->",
"",
"## Recommended execution queue",
"",
"<!-- prettier-ignore -->",
"",
"| Order | ID(s) |",
"| --- | --- |",
"| 1 | `#001` |",
"",
"## Open items",
"",
"<!-- prettier-ignore -->",
"",
"| ID | Pri | Type | Summary | Detail / next action | Source | Added |",
"| --- | --- | --- | --- | --- | --- | --- |",
"| #001 | P2 | issue | original summary | original detail | source | 2026-01-01 |",
"",
"## Resolved / archive",
"",
"<!-- prettier-ignore -->",
"",
"| ID | Type | Summary | Outcome | Resolved |",
"| ---- | ---- | ---- | ---- | ---- |",
"| #000 | issue | old | done | 2026-01-01 |",
"",
].join("\n");
const baseFingerprint = issueRowFingerprint(ledger, "#001");
expect(baseFingerprint).not.toBeNull();

const done = {
version: 1,
id: "77777777-7777-4777-8777-777777777777",
createdOn: "2026-08-13",
action: "done",
payload: { id: "#001", outcome: "done", baseRowFingerprint: baseFingerprint },
};
const update = {
version: 1,
id: "88888888-8888-4888-8888-888888888888",
createdOn: "2026-08-13",
action: "update",
payload: { id: "#001", summary: "new summary", baseRowFingerprint: baseFingerprint },
};

const stale = ledger.replace("original summary", "mutated summary");
expect(() => applyRequest(ledger, done)).not.toThrow();
expect(() => applyRequest(stale, done)).toThrow(/stale|no longer open/);
expect(() => applyRequest(stale, update)).toThrow(/stale|no longer open/);
});
});
2 changes: 1 addition & 1 deletion tests/ui-tools.spec.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -1454,7 +1454,7 @@ test.describe("Clinical KB tools directory and legacy launcher", () => {
await expect(page).toHaveURL(/group=urgent/);
await expect(page.getByRole("button", { name: "Remove Crisis & urgent filter" })).toBeVisible();
await filterPanel.getByTestId("service-filter-panel-clear").click();
await expect(page).toHaveURL(/q=13YARN/);
await expect(page).toHaveURL(/[?&]q=13YARN(?:&|#|$)/);
await expect(page).not.toHaveURL(/group=/);
await expect(page.getByTestId("service-search-result-13yarn")).toBeVisible();
await filterPanel.getByRole("button", { name: "Close", exact: true }).click();
Expand Down
Loading