From f5deaaee98864f1d32c1060ae14966a4f5975872 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 11 Jul 2026 03:37:27 +0800 Subject: [PATCH 1/2] fix: close architecture review guard gaps --- docs/branch-review-ledger.md | 1 + src/lib/local-server-utils.mjs | 19 +++++++------ tests/architecture-boundaries.test.ts | 39 ++++++++++++++++++--------- tests/local-server-utils.test.ts | 26 ++++++++++++++++++ 4 files changed, 65 insertions(+), 20 deletions(-) create mode 100644 tests/local-server-utils.test.ts diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 80b660b16..237b8ec5f 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -29,3 +29,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-10 | codex/architecture-review-fixes | 648abfa3f7c91395b5eeca543f70e0b6ea59e9e0 | architecture-review | Seven findings fixed in the working tree: three runtime cycles, unbounded owner caches, a client/server env boundary breach, reversed runtime-to-scripts ownership, and architecture-doc drift. | `npm run test -- tests/architecture-boundaries.test.ts tests/bounded-ttl-cache.test.ts tests/rag-score.test.ts tests/rag-cache-utils.test.ts tests/rag-cache-invalidation.test.ts tests/evidence-panels.test.ts tests/clinical-dashboard-merge-artifacts.test.ts`; `npm run verify:cheap`; `npm run check:production-readiness:ci` | | 2026-07-10 | codex/architecture-review-fixes | 648abfa3f7c91395b5eeca543f70e0b6ea59e9e0 | frontend-architecture-review | Shared cycle/env findings confirmed and fixed; three additional findings fixed for defeated lazy boundaries, duplicate shell/dashboard subscriptions, and unstable search-context values. | `npm run test -- tests/architecture-boundaries.test.ts tests/evidence-panels.test.ts tests/clinical-dashboard-merge-artifacts.test.ts`; `npm run verify:cheap`; UI gate deferred pending explicit local-API approval | | 2026-07-11 | codex/architecture-review-integration | b45df727b29aad8ba4ec5d4e96d1f0599d7dad8a | branch-integration-review | Replayed the reviewed architecture fixes onto current `origin/main`; preserved current CI/autofix history and found no new high-confidence defect in the integrated diff. | `npm run check:runtime`; `npm run check:github-actions`; `npm run sitemap:check`; `npm run lint`; `npm run typecheck`; focused Vitest (24 passed); full Vitest with `--testTimeout=30000` (1,433 passed, 1 skipped); `git diff --check` | +| 2026-07-11 | codex/architecture-review-integration | 665103250ccc33b5870862b8d8467607a1ae5d23 | coderabbit-followup | Fixed POSIX project-root identity collisions and closed dynamic-import and self-cycle gaps in the architecture regression guard. | Local-server Vitest passed; architecture-boundaries Vitest passed (6 tests); `npm run typecheck`; focused Prettier; `git diff --check` | diff --git a/src/lib/local-server-utils.mjs b/src/lib/local-server-utils.mjs index efe9a6f81..2133081a8 100644 --- a/src/lib/local-server-utils.mjs +++ b/src/lib/local-server-utils.mjs @@ -1,22 +1,25 @@ import crypto from "node:crypto"; +import path from "node:path"; export const appName = "Clinical KB"; export const projectPortStart = 3100; export const projectPortEnd = 4599; -export function normalizeProjectRoot(projectRoot) { - return projectRoot.replaceAll("\\", "/").toLowerCase(); +export function normalizeProjectRoot(projectRoot, platform = process.platform) { + const pathApi = platform === "win32" ? path.win32 : path.posix; + const resolvedRoot = pathApi.resolve(projectRoot); + return platform === "win32" ? resolvedRoot.replaceAll("\\", "/").toLowerCase() : resolvedRoot; } -export function projectHash(projectRoot) { - return crypto.createHash("sha256").update(normalizeProjectRoot(projectRoot)).digest(); +export function projectHash(projectRoot, platform = process.platform) { + return crypto.createHash("sha256").update(normalizeProjectRoot(projectRoot, platform)).digest(); } -export function stableProjectPort(projectRoot) { - const offset = projectHash(projectRoot).readUInt32BE(0) % (projectPortEnd - projectPortStart + 1); +export function stableProjectPort(projectRoot, platform = process.platform) { + const offset = projectHash(projectRoot, platform).readUInt32BE(0) % (projectPortEnd - projectPortStart + 1); return projectPortStart + offset; } -export function localProjectId(projectRoot) { - return `clinical-kb:${projectHash(projectRoot).toString("hex").slice(0, 12)}`; +export function localProjectId(projectRoot, platform = process.platform) { + return `clinical-kb:${projectHash(projectRoot, platform).toString("hex").slice(0, 12)}`; } diff --git a/tests/architecture-boundaries.test.ts b/tests/architecture-boundaries.test.ts index 6b19685c5..5bb00fec5 100644 --- a/tests/architecture-boundaries.test.ts +++ b/tests/architecture-boundaries.test.ts @@ -27,14 +27,8 @@ function scriptKind(filePath: string) { return ts.ScriptKind.TS; } -function moduleSpecifiers(filePath: string) { - const source = ts.createSourceFile( - filePath, - fs.readFileSync(filePath, "utf8"), - ts.ScriptTarget.Latest, - true, - scriptKind(filePath), - ); +function moduleSpecifiersFromSource(filePath: string, sourceText: string) { + const source = ts.createSourceFile(filePath, sourceText, ts.ScriptTarget.Latest, true, scriptKind(filePath)); const staticImports = new Set(); const dynamicImports = new Set(); @@ -71,13 +65,15 @@ function moduleSpecifiers(filePath: string) { } const visit = (node: ts.Node) => { + const moduleSpecifier = ts.isCallExpression(node) ? node.arguments[0] : undefined; if ( ts.isCallExpression(node) && node.expression.kind === ts.SyntaxKind.ImportKeyword && - node.arguments.length === 1 && - ts.isStringLiteral(node.arguments[0]) + (node.arguments.length === 1 || node.arguments.length === 2) && + moduleSpecifier && + ts.isStringLiteralLike(moduleSpecifier) ) { - dynamicImports.add(node.arguments[0].text); + dynamicImports.add(moduleSpecifier.text); } ts.forEachChild(node, visit); }; @@ -86,6 +82,10 @@ function moduleSpecifiers(filePath: string) { return { source, staticImports, dynamicImports }; } +function moduleSpecifiers(filePath: string) { + return moduleSpecifiersFromSource(filePath, fs.readFileSync(filePath, "utf8")); +} + function resolveModule(fromFile: string, specifier: string, fileSet: Set) { let base: string; if (specifier.startsWith("@/")) base = path.join(projectRoot, "src", specifier.slice(2)); @@ -150,7 +150,8 @@ function runtimeCycles(graph: Map) { onStack.delete(current); component.push(current); } while (current !== file); - if (component.length > 1) cycles.push(component); + const hasSelfCycle = (graph.get(file) ?? []).includes(file); + if (component.length > 1 || hasSelfCycle) cycles.push(component); }; for (const file of graph.keys()) { @@ -164,6 +165,20 @@ function relative(filePath: string) { } describe("architecture boundaries", () => { + it("tracks statically resolvable dynamic imports", () => { + const parsed = moduleSpecifiersFromSource( + "fixture.ts", + 'import(`./template`); import("./with-options", { with: { type: "json" } }); import(`./${name}`);', + ); + + expect([...parsed.dynamicImports]).toEqual(["./template", "./with-options"]); + }); + + it("counts a runtime self-import as a cycle", () => { + const file = path.join(projectRoot, "src", "self.ts"); + expect(runtimeCycles(new Map([[file, [file]]]))).toEqual([[file]]); + }); + it("has no runtime import cycles", () => { const { graph } = runtimeGraph(); const cycles = runtimeCycles(graph).map((cycle) => cycle.map(relative).sort()); diff --git a/tests/local-server-utils.test.ts b/tests/local-server-utils.test.ts new file mode 100644 index 000000000..1506e72e2 --- /dev/null +++ b/tests/local-server-utils.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from "vitest"; + +import { localProjectId, normalizeProjectRoot, stableProjectPort } from "../src/lib/local-server-utils.mjs"; + +describe("local server project identity", () => { + it("normalizes Windows roots case-insensitively", () => { + const first = "C:\\Work\\Clinical-KB"; + const second = "c:/work/clinical-kb"; + + expect(normalizeProjectRoot(first, "win32")).toBe(normalizeProjectRoot(second, "win32")); + expect(localProjectId(first, "win32")).toBe(localProjectId(second, "win32")); + expect(stableProjectPort(first, "win32")).toBe(stableProjectPort(second, "win32")); + }); + + it("preserves POSIX case and backslash semantics", () => { + const upperCaseRoot = "/work/Clinical-KB"; + const lowerCaseRoot = "/work/clinical-kb"; + const backslashRoot = "/work/Clinical\\KB"; + const slashRoot = "/work/Clinical/KB"; + + expect(normalizeProjectRoot(backslashRoot, "linux")).toContain("\\"); + expect(localProjectId(upperCaseRoot, "linux")).not.toBe(localProjectId(lowerCaseRoot, "linux")); + expect(stableProjectPort(upperCaseRoot, "linux")).not.toBe(stableProjectPort(lowerCaseRoot, "linux")); + expect(localProjectId(backslashRoot, "linux")).not.toBe(localProjectId(slashRoot, "linux")); + }); +}); From f08b7f055a08946f9e712c38ed77ad1bf09041e7 Mon Sep 17 00:00:00 2001 From: BigSimmo <87357024+BigSimmo@users.noreply.github.com> Date: Sat, 11 Jul 2026 04:28:15 +0800 Subject: [PATCH 2/2] test: avoid probabilistic project identity assertions --- docs/branch-review-ledger.md | 1 + tests/local-server-utils.test.ts | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/docs/branch-review-ledger.md b/docs/branch-review-ledger.md index 237b8ec5f..50b0489a8 100644 --- a/docs/branch-review-ledger.md +++ b/docs/branch-review-ledger.md @@ -30,3 +30,4 @@ Use this ledger to prevent repeated branch and PR reviews when the reviewed HEAD | 2026-07-10 | codex/architecture-review-fixes | 648abfa3f7c91395b5eeca543f70e0b6ea59e9e0 | frontend-architecture-review | Shared cycle/env findings confirmed and fixed; three additional findings fixed for defeated lazy boundaries, duplicate shell/dashboard subscriptions, and unstable search-context values. | `npm run test -- tests/architecture-boundaries.test.ts tests/evidence-panels.test.ts tests/clinical-dashboard-merge-artifacts.test.ts`; `npm run verify:cheap`; UI gate deferred pending explicit local-API approval | | 2026-07-11 | codex/architecture-review-integration | b45df727b29aad8ba4ec5d4e96d1f0599d7dad8a | branch-integration-review | Replayed the reviewed architecture fixes onto current `origin/main`; preserved current CI/autofix history and found no new high-confidence defect in the integrated diff. | `npm run check:runtime`; `npm run check:github-actions`; `npm run sitemap:check`; `npm run lint`; `npm run typecheck`; focused Vitest (24 passed); full Vitest with `--testTimeout=30000` (1,433 passed, 1 skipped); `git diff --check` | | 2026-07-11 | codex/architecture-review-integration | 665103250ccc33b5870862b8d8467607a1ae5d23 | coderabbit-followup | Fixed POSIX project-root identity collisions and closed dynamic-import and self-cycle gaps in the architecture regression guard. | Local-server Vitest passed; architecture-boundaries Vitest passed (6 tests); `npm run typecheck`; focused Prettier; `git diff --check` | +| 2026-07-11 | codex/architecture-review-followup | f5deaaee98864f1d32c1060ae14966a4f5975872 | coderabbit-test-followup | Removed probabilistic no-collision assertions from the local identity test and replaced them with deterministic normalization, repeatability, ID-shape, and port-range checks. | Local-server Vitest (2 passed); focused Prettier; `git diff --check`; hosted CI/SAST/Secret Scan passed on the reviewed head | diff --git a/tests/local-server-utils.test.ts b/tests/local-server-utils.test.ts index 1506e72e2..efca5b614 100644 --- a/tests/local-server-utils.test.ts +++ b/tests/local-server-utils.test.ts @@ -18,9 +18,18 @@ describe("local server project identity", () => { const backslashRoot = "/work/Clinical\\KB"; const slashRoot = "/work/Clinical/KB"; + expect(normalizeProjectRoot(upperCaseRoot, "linux")).not.toBe(normalizeProjectRoot(lowerCaseRoot, "linux")); expect(normalizeProjectRoot(backslashRoot, "linux")).toContain("\\"); - expect(localProjectId(upperCaseRoot, "linux")).not.toBe(localProjectId(lowerCaseRoot, "linux")); - expect(stableProjectPort(upperCaseRoot, "linux")).not.toBe(stableProjectPort(lowerCaseRoot, "linux")); - expect(localProjectId(backslashRoot, "linux")).not.toBe(localProjectId(slashRoot, "linux")); + expect(normalizeProjectRoot(backslashRoot, "linux")).not.toBe(normalizeProjectRoot(slashRoot, "linux")); + + for (const root of [upperCaseRoot, lowerCaseRoot, backslashRoot, slashRoot]) { + const projectId = localProjectId(root, "linux"); + const port = stableProjectPort(root, "linux"); + expect(projectId).toBe(localProjectId(root, "linux")); + expect(projectId).toMatch(/^clinical-kb:[0-9a-f]{12}$/); + expect(port).toBe(stableProjectPort(root, "linux")); + expect(port).toBeGreaterThanOrEqual(3100); + expect(port).toBeLessThanOrEqual(4599); + } }); });