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
21 changes: 21 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,15 @@ jobs:
if: needs.changes.outputs.static_heavy_changed == 'true'
run: npm run check:medication-lexicon-report

# The interaction index is generated from the medication snapshot plus the
# lexicon, and the UI reads it to decide whether a drug can be shown as
# clear. Same #333 shape as the lexicon report above: local-only until audit
# M30, so a snapshot-only merge through the bare-PR route shipped a stale
# index green. Same static-heavy condition verify-pr-local.mjs uses.
- name: Medication interaction index drift
if: needs.changes.outputs.static_heavy_changed == 'true'
run: npm run check:medication-interactions

- name: GitHub Actions pin check
if: needs.changes.outputs.workflow_changed == 'true'
run: npm run check:github-actions
Expand Down Expand Up @@ -436,6 +445,18 @@ jobs:
if: needs.changes.outputs.static_heavy_changed == 'true'
run: npm run check:owner-scope

# The clinician-facing hazard register (docs/clinical-hazard-controls.json)
# names control symbols, control paths and proving tests per hazard. Until
# audit M33 the validator ran only inside the provider-backed
# governance:release chain, so a control rename, a gutted proof test or the
# register's own reviewExpiresAt lapsing produced no PR signal. Needs the
# fetch-depth 0 checkout above for the reviewedCommit ancestry checks; the
# script says so and skips only those on a shallow clone. Note: this step
# goes red on reviewExpiresAt until the register is re-reviewed — by design.
- name: Clinical hazard-controls register
if: needs.changes.outputs.docs_changed == 'true' || needs.changes.outputs.static_heavy_changed == 'true'
run: npm run check:clinical-hazard-controls

- name: Lint
if: needs.changes.outputs.static_heavy_changed == 'true'
run: npm run lint
Expand Down
10 changes: 10 additions & 0 deletions docs/agents/test-deletion-guard.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,16 @@ still measured as the whole drop. Two rules:
This is the actual signature, and unlike the first rule it also covers the same tool failure
landing in `src/`.

A second known limitation: the gate measures the diff a **pull request** presents, never a
merge commit's own resolution. On 2026-08-17 merge commit `ab7c5134f` (PR #2025) resolved a
conflict in `tests/ui-tools.spec.ts` by dropping the 320 px low-confidence `AccessibleTable`
journey that PR #2006 had added the day before; no non-merge commit ever removed it, so no PR
diff ever showed the loss, and it went unnoticed until the 2026-09-02 audit (M31). A journey
that must survive is therefore pinned **by name** in a unit test as well — see
`tests/playwright-pr-shards.test.ts` for that spec — because a spec no matcher names is a
spec that can vanish without anything failing. Prefer a dedicated spec file over a case
appended to a large shared spec when a conflict there is likely.

Known limitation, deliberately accepted: a `test()` inside a `for (const viewport of …)` loop
counts once, not once per iteration, so shrinking that loop's array loses real cases without
moving the count. Any static count has this hole; the aggregate and the truncation rule are
Expand Down
1 change: 1 addition & 0 deletions docs/design-system/adoption-manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@
"tests/accessible-table.dom.test.tsx",
"tests/design-sync-visual-exports.test.ts",
"tests/design-system-adoption.test.ts",
"tests/playwright-pr-shards.test.ts",
"tests/ward-output.test.ts"
],
"baseline": {
Expand Down
4 changes: 2 additions & 2 deletions playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ const chromiumExecutablePath = process.env.PLAYWRIGHT_CHROMIUM_EXECUTABLE_PATH;
const productionSpecPattern =
/.*(?:answer-progress-ui-smoke|dsm-ui-smoke|ui-(smoke|stress|accessibility|caring-contacts-workspace|clinical-ask|dictionary|document-canvas|tools|tools-show-all|overlap|universal-search|specifiers|sources|formulation(?:-result-cards)?|forms-section-nav|chrome-scroll|therapy-nav-scroll|therapy-pathways|mode-nav-density|phone-motion|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|token-layer-resolution|visual-artifacts|hydration))\.spec\.ts/;
const mockupSpecPattern =
/.*ui-(answer-chat-perfected-mockup|care-plan-mockup|caring-contact-mockup|document-image-status-mockup|document-top-navigation-mockup|sidebar-live-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-search-mode-mockup|tools-task-directory|ward-management|ward-coordinator|ward-roles|ward-discharges|ward-morning|ward-referrals|ward-forced-colors)\.spec\.ts/;
/.*ui-(accessible-table-mockup|answer-chat-perfected-mockup|care-plan-mockup|caring-contact-mockup|document-image-status-mockup|document-top-navigation-mockup|sidebar-live-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-search-mode-mockup|tools-task-directory|ward-management|ward-coordinator|ward-roles|ward-discharges|ward-morning|ward-referrals|ward-forced-colors)\.spec\.ts/;
const mockupTag = /@mockup/;

// The production specs that need a POPULATED Caring Contacts store, and therefore run against
Expand All @@ -51,7 +51,7 @@ const seededBaseURL = process.env.PLAYWRIGHT_SEEDED_BASE_URL;
export default defineConfig({
testDir: "./tests",
testMatch:
/.*(?:answer-progress-ui-smoke|dsm-ui-smoke|ui-(smoke|stress|accessibility|answer-chat-perfected-mockup|care-plan-mockup|caring-contact-mockup|caring-contacts-activation|caring-contacts-populated|caring-contacts-workspace|clinical-ask|dictionary|document-canvas|document-image-status-mockup|document-top-navigation-mockup|sidebar-live-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-show-all|tools-search-mode-mockup|tools-task-directory|ward-(?:management|coordinator|roles|discharges|morning|referrals|forced-colors)|overlap|universal-search|specifiers|sources|formulation(?:-result-cards)?|forms-section-nav|chrome-scroll|therapy-nav-scroll|therapy-pathways|mode-nav-density|phone-motion|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|token-layer-resolution|visual-artifacts|hydration))\.spec\.ts/,
/.*(?:answer-progress-ui-smoke|dsm-ui-smoke|ui-(smoke|stress|accessibility|accessible-table-mockup|answer-chat-perfected-mockup|care-plan-mockup|caring-contact-mockup|caring-contacts-activation|caring-contacts-populated|caring-contacts-workspace|clinical-ask|dictionary|document-canvas|document-image-status-mockup|document-top-navigation-mockup|sidebar-live-mockup|therapy-navigation-mockup|tools|tools-collapse|tools-show-all|tools-search-mode-mockup|tools-task-directory|ward-(?:management|coordinator|roles|discharges|morning|referrals|forced-colors)|overlap|universal-search|specifiers|sources|formulation(?:-result-cards)?|forms-section-nav|chrome-scroll|therapy-nav-scroll|therapy-pathways|mode-nav-density|phone-motion|phone-scroll(?:-[a-z0-9-]+)?|pwa|route-coverage|style-contract|token-layer-resolution|visual-artifacts|hydration))\.spec\.ts/,
timeout: 60_000,
retries: 0,
// Fail the run if a stray `test.only` is committed: otherwise it silently
Expand Down
67 changes: 65 additions & 2 deletions scripts/check-clinical-hazard-controls.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,44 @@ function gitCheck(args) {
}
}

/**
* A word-bounded match for a control symbol inside source text. The escape class is the
* repository's standard one (see scripts/pr-policy.mjs); the earlier `[...[\\]\\]` form
* parsed as a class followed by a literal `\]`, so no metacharacter was ever escaped and a
* dotted symbol matched as a wildcard (audit L22).
*/
function symbolPattern(symbol) {
return new RegExp(`\\b${String(symbol).replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\b`);
}

/**
* Whether a listed test actually exercises the control it is cited for: it names a
* control symbol, or imports a control path module (alias `@/lib/x`, relative
* `../src/lib/x`, or the bare repository path). Existence alone proved nothing (M33).
*/
function testReferencesControl(testSource, hazard) {
if ((hazard.controlSymbols ?? []).some((symbol) => symbolPattern(symbol).test(testSource))) return true;
return (hazard.controlPaths ?? []).some((controlPath) => {
const modulePath = String(controlPath).replace(/\.(?:ts|tsx|mjs|js)$/, "");
const withoutSrc = modulePath.replace(/^src\//, "");
return [`@/${withoutSrc}`, `/src/${withoutSrc}`, modulePath].some((specifier) => testSource.includes(specifier));
});
}

function isShallowClone() {
try {
return (
execFileSync("git", ["rev-parse", "--is-shallow-repository"], {
cwd: root,
encoding: "utf8",
stdio: ["ignore", "pipe", "ignore"],
}).trim() === "true"
);
} catch {
return false;
}
}

const commitTreeCache = new Map();

function pathExistsAtCommit(commit, file) {
Expand Down Expand Up @@ -174,10 +212,23 @@ export function validateClinicalHazardControls(
.map((path) => readFileSync(path.absolute, "utf8"))
.join("\n");
for (const symbol of hazard.controlSymbols ?? []) {
if (!new RegExp(`\\b${String(symbol).replace(/[.*+?^${}()|[\\]\\]/g, "\\$&")}\\b`).test(controlSource)) {
if (!symbolPattern(symbol).test(controlSource)) {
errors.push(`${label}: control symbol ${symbol} not found in controlPaths`);
}
}
// A test that exists but never touches the control is not proof of it.
const listedTests = (hazard.tests ?? []).map(repositoryPath).filter((path) => path && existsSync(path.absolute));
if (
["controlled", "partial"].includes(hazard.state) &&
listedTests.length > 0 &&
!listedTests.some((path) => testReferencesControl(readFileSync(path.absolute, "utf8"), hazard))
) {
errors.push(
`${label}: no listed test references a control symbol or imports a control path (${listedTests
.map((path) => path.file)
.join(", ")})`,
);
}
}
}
for (const id of requiredHazards) if (!ids.has(id)) errors.push(`missing required hazard ${id}`);
Expand Down Expand Up @@ -238,7 +289,19 @@ export function validateClinicalHazardControls(

function main() {
const manifest = JSON.parse(readFileSync(manifestPath, "utf8"));
const errors = validateClinicalHazardControls(manifest);
// The reviewedCommit ancestry checks need history. On a depth-one clone they
// would report every reviewed commit as missing — a checkout artefact, not a
// register defect — so say exactly what was skipped instead of failing on it.
// CI's static-pr job checks out with fetch-depth 0, where the checks do run.
const shallow = isShallowClone();
if (shallow) {
console.warn(
"CLINICAL_HAZARD_CONTROLS_SHALLOW_CLONE: this is a shallow git clone, so the reviewedCommit " +
"existence/ancestry checks were skipped. Run on a full-history checkout (git fetch --unshallow) " +
"to prove them; every file, symbol, test-reference and date check below still ran.",
);
}
const errors = validateClinicalHazardControls(manifest, { checkGit: !shallow });
if (errors.length) {
console.error("CLINICAL_HAZARD_CONTROLS_FAIL");
for (const error of errors) console.error(`- ${error}`);
Expand Down
68 changes: 54 additions & 14 deletions scripts/check-owner-scope-api.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -48,20 +48,30 @@ import { fileURLToPath } from "node:url";
import { emptyTierNames, scanRpcDispatch, scanTenancy } from "./lib/tenancy-scan.mjs";

// Recognised owner-scoping constructs. If any appears in the enclosing handler of an
// owner-scoped `.from(...)`, that query is considered scoped. `owner_id` (as a substring)
// covers `.eq("owner_id"...)`, `.is("owner_id"...)`, `.or("owner_id.eq...")`, insert/update
// `owner_id:` payloads, and `documents.owner_id` inner-join predicates. The named helpers
// cover the cases where scoping is delegated to a shared primitive.
// owner-scoped `.from(...)`, that query is considered scoped. These are FILTER shapes,
// not the bare `owner_id` substring the guard accepted until audit L47: a projection such
// as `.select("id,owner_id,title")`, a comment, or an unrelated object key all contain the
// substring and none of them scopes anything. Recognised:
// - a filter call naming the column: `.eq("owner_id"`, `.is("owner_id"`, `.in("owner_id"`,
// `.neq/.not/.match/.filter("owner_id"`, and the `documents.owner_id` inner-join form
// - PostgREST filter strings: `owner_id.eq.`, `owner_id.is.`, `owner_id.in.`
// - an `owner_id:` insert/update payload key (a write is scoped by what it writes)
// - a `documents!inner(owner_id)` join projection, which only makes sense with the
// `documents.owner_id` predicate that follows it
// The named helpers cover the cases where scoping is delegated to a shared primitive.
const SCOPE_TOKENS = [
"owner_id",
"withOwnerReadScope",
"requireOwnerScope",
"retrievalOwnerFilter",
"requireOwnedDocument",
"loadOwnedDocument",
"ownedDocumentId",
"assertGlobalSearchAllowed",
"resolveSearchScope",
/\.(?:eq|neq|is|in|not|match|filter)\(\s*["'`](?:documents\.)?owner_id["'`]/,
/\bowner_id\.(?:eq|is|in|neq)\./,
/\bowner_id\s*:/,
/documents!inner\(owner_id\)/,
/\bwithOwnerReadScope\b/,
/\brequireOwnerScope\b/,
/\bretrievalOwnerFilter\b/,
/\brequireOwnedDocument\b/,
/\bloadOwnedDocument\b/,
/\bownedDocumentId\b/,
/\bassertGlobalSearchAllowed\b/,
/\bresolveSearchScope\b/,
];

// Intentional exceptions: a handler that queries an owner-scoped table where ownership
Expand Down Expand Up @@ -201,7 +211,7 @@ export function analyzeFile(file, text, ownerTables) {
// ownership is enforced by the handler(s) that call them within the same file (e.g. a
// `selectLabels` helper reached only after `requireOwnedDocument`).
const scopeText = segment && segment.isHandler ? segment.text : text;
const scoped = SCOPE_TOKENS.some((tok) => scopeText.includes(tok));
const scoped = SCOPE_TOKENS.some((tok) => tok.test(scopeText));
if (!scoped) violations.push({ file, table, line: lineNo });
}
return violations;
Expand Down Expand Up @@ -292,6 +302,36 @@ function runSelfTest() {
"nested helper / sibling handler must not mask an unscoped query",
);

// The substring shape the guard used to accept (audit L47): a projection that merely
// NAMES owner_id is not an owner filter, and neither is a comment. Both must be flagged.
const selectListOnly = `export async function GET(request) {
// owner_id is returned to the client for display
const { data } = await supabase.from("documents").select("id,owner_id,title");
return NextResponse.json({ data });
}`;
expect(
analyzeFile("fixture-select-list.ts", selectListOnly, ownerTables).length === 1,
"select-list-only handler should be flagged",
);

// Every filter construct the codebase uses to scope a query must still pass.
const filterShapes = [
`.is("owner_id", null)`,
`.in("owner_id", [user.id])`,
`.or("owner_id.eq." + user.id)`,
`.eq("documents.owner_id", user.id)`,
`.select("id, documents!inner(owner_id)").eq("documents.owner_id", user.id)`,
`.insert({ owner_id: user.id, title })`,
`.eq('owner_id', user.id)`,
];
for (const shape of filterShapes) {
const scopedShape = `export async function POST(request) {
const { data } = await supabase.from("documents")${shape};
return NextResponse.json({ data });
}`;
expect(analyzeFile("fixture-shape.ts", scopedShape, ownerTables).length === 0, `${shape} handler should pass`);
}

// A non-owner-scoped table is not PHASE 1's concern — it has no owner_id column to filter
// on. This is blind spot A, and it is closed by phase 2's derived-tier inventory, not by
// widening this regex sweep. Do not "fix" this assertion; changing it would only make
Expand Down
4 changes: 3 additions & 1 deletion scripts/playwright-pr-shards.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ export const prUiSpecProfiles = Object.freeze([
// `tools-show-all` while this matcher still said only `tools` — so the spec was a production
// journey that belonged to no shard. Zero timing follows the convention above: keep it on the
// lightest measured shard until hosted timing is available, and replace this at the next
// refresh. A spec wired into no gate is a spec that never runs.
// refresh. A spec wired into no gate is a spec that never runs — the spec (PR #2008) was
// collected by NO project from 2026-08-16 until audit M32, and the two-way parity test in
// tests/playwright-pr-shards.test.ts now fails on that shape.
{ file: "tests/ui-tools-show-all.spec.ts", shard: 1, fullSeconds: 0, criticalSeconds: 0 },
// Critical-only acceptance coverage; the required critical job owns its runtime.
{ file: "tests/ui-clinical-ask.spec.ts", shard: 1, fullSeconds: 1, criticalSeconds: 1 },
Expand Down
Loading
Loading