fix: wire four dropped injection seams (F1–F4 from the latent-bug sweep)

.erpaval/solutions/test-failures/run-biome-standalone-mise-lint-precheck-sandbox-fails.md

@@ -0,0 +1,53 @@
+# Run Biome standalone — the `mise run lint` precheck SIGTERMs in-sandbox
+
+**Category:** test-failures · **Track:** bug
+**Discovered:** session-3b8ca0 (CI `lint` failed on a PR that passed local typecheck+test)
+
+## What happened
+
+Local validation ran `tsc --noEmit` + the test suites directly and called the
+PR green. CI's `lint` job failed in 22s. Cause: every `mise run <task>` depends
+on an `install` task that runs `pnpm install`, which gets **SIGTERM'd under the
+sandbox** — so `mise run lint` (and `check`, which includes lint) never
+actually executes Biome locally. typecheck and test were run via direct `pnpm`
+invocations, but Biome was simply never run.
+
+Two classes of failure shipped to CI:
+- `lint/suspicious/noConsole`: `console.log` is **off** in
+  `packages/cli/src/commands/**` (biome.json override) but **warns** in
+  `packages/cli/src/index.ts` (allows only `warn`/`error`). A `console.log`
+  added to `index.ts` fails lint; the fix is to emit stdout from a
+  command-module helper, not the CLI entrypoint.
+- formatter line-wrapping nits Biome would rewrap.
+
+## The rule
+
+Before pushing, run Biome **standalone**, never via `mise`:
+
+```bash
+pnpm exec biome check .            # repo-wide, mirrors the CI lint job
+pnpm exec biome check --write <files>   # auto-fix format + safe lint
+```
+
+The mise tasks (`lint`, `check`, `check:full`) all gate on the `install`
+precheck that dies in-sandbox, so they are NOT a reliable local signal — invoke
+the underlying tool directly (same lesson as running `tsc`/`node --test` via
+`pnpm` instead of `mise run`). Generalizes: any mise task with
+`depends = ["install"]` is unrunnable in-sandbox; reach for the underlying CLI.
+
+## stdout/console map (cli)
+
+`console.log` → only in `packages/cli/src/commands/**`. In `index.ts` and
+elsewhere, only `console.warn` / `console.error` are allowed. Machine `--json`
+output therefore belongs in a command module helper.
+
+## Update (session-3b8ca0, F1-F4 PR): CI runs `biome ci`, not `biome check`
+
+The CI `lint` job is `pnpm exec biome ci .` (see .github/workflows/ci.yml). `biome ci`
+is STRICTER than `biome check`: it enforces the organize-imports/exports assist as an
+ERROR (barrel exports must be alphabetically sorted), which `biome check` only auto-fixes
+silently. A locally-clean `biome check .` still failed CI lint on export ordering.
+
+RULE: before push, run `pnpm exec biome ci .` (the exact CI command), not just
+`biome check .`. To fix ordering: `pnpm exec biome check --write --unsafe .` reorders
+exports (verify the diff is export-only — it is safe here, just reordering).

packages/analysis/src/index.ts

@@ -96,7 +96,7 @@ export type {
 } from "./license-classify.js";
 export { classifyDependencies } from "./license-classify.js";
 export type { OwnerRow } from "./owners.js";
-export { listOwners } from "./owners.js";
+export { collectOwnersByPath, listOwners } from "./owners.js";
 export type { Adjacency, EdgeLike } from "./page-rank.js";
 export { buildAdjacency, pageRank } from "./page-rank.js";
 export type { OrphanGrade } from "./risk.js";
@@ -127,6 +127,7 @@ export {
 } from "./risk-snapshot.js";
 export type { RouteMapFilter, RouteMapRow } from "./route-map.js";
 export { listRouteMap } from "./route-map.js";
+export { buildScanEnrichment } from "./scan-enrich.js";
 export type { ShapeStatus } from "./shape.js";
 export { classifyShape } from "./shape.js";
 export { computeStaleness } from "./staleness.js";

packages/analysis/src/owners.test.ts

@@ -0,0 +1,103 @@
+/**
+ * Tests for `collectOwnersByPath` — the per-path owner map the verdict CLI
+ * feeds the policy engine's `ownership_required` rule.
+ *
+ * Regression context: the CLI used to hardcode `ownersByPath: new Map()`, so a
+ * rule with an empty `require_approval_from` (relying on graph owners) could
+ * never be satisfied — it always hit the "no owners" branch. These tests pin
+ * that the map is actually built from OWNED_BY edges.
+ */
+
+import { strict as assert } from "node:assert";
+import { test } from "node:test";
+import { collectOwnersByPath } from "./owners.js";
+import { FakeStore } from "./test-utils.js";
+
+function contributorNode(id: string, name: string, emailPlain?: string, emailHash?: string) {
+  return {
+    id,
+    kind: "Contributor",
+    name,
+    filePath: "<contributors>",
+    ...(emailPlain !== undefined ? { emailPlain } : {}),
+    ...(emailHash !== undefined ? { emailHash } : {}),
+  };
+}
+
+test("collectOwnersByPath maps each path to its OWNED_BY owner emails", async () => {
+  const store = new FakeStore();
+  store.addNode(contributorNode("Contributor:alice", "Alice", "alice@example.com"));
+  store.addNode(contributorNode("Contributor:bob", "Bob", "bob@example.com"));
+  store.addEdge({
+    fromId: "File:src/a.ts:src/a.ts",
+    toId: "Contributor:alice",
+    type: "OWNED_BY",
+    confidence: 0.9,
+  });
+  store.addEdge({
+    fromId: "File:src/a.ts:src/a.ts",
+    toId: "Contributor:bob",
+    type: "OWNED_BY",
+    confidence: 0.4,
+  });
+
+  const map = await collectOwnersByPath(store, ["src/a.ts"]);
+  assert.deepEqual(map.get("src/a.ts"), ["alice@example.com", "bob@example.com"]);
+});
+
+test("collectOwnersByPath omits paths with no owner edges", async () => {
+  const store = new FakeStore();
+  store.addNode(contributorNode("Contributor:alice", "Alice", "alice@example.com"));
+  store.addEdge({
+    fromId: "File:src/a.ts:src/a.ts",
+    toId: "Contributor:alice",
+    type: "OWNED_BY",
+    confidence: 1,
+  });
+
+  const map = await collectOwnersByPath(store, ["src/a.ts", "src/unowned.ts"]);
+  assert.deepEqual(map.get("src/a.ts"), ["alice@example.com"]);
+  assert.equal(map.has("src/unowned.ts"), false);
+});
+
+test("collectOwnersByPath falls back to emailHash when plain email is absent", async () => {
+  const store = new FakeStore();
+  store.addNode(contributorNode("Contributor:priv", "Private", undefined, "deadbeefhash"));
+  store.addEdge({
+    fromId: "File:src/p.ts:src/p.ts",
+    toId: "Contributor:priv",
+    type: "OWNED_BY",
+    confidence: 1,
+  });
+
+  const map = await collectOwnersByPath(store, ["src/p.ts"]);
+  assert.deepEqual(map.get("src/p.ts"), ["deadbeefhash"]);
+});
+
+test("collectOwnersByPath returns an empty map for no files", async () => {
+  const store = new FakeStore();
+  const map = await collectOwnersByPath(store, []);
+  assert.equal(map.size, 0);
+});
+
+test("collectOwnersByPath keeps per-path owners distinct (no cross-file bleed)", async () => {
+  const store = new FakeStore();
+  store.addNode(contributorNode("Contributor:alice", "Alice", "alice@example.com"));
+  store.addNode(contributorNode("Contributor:bob", "Bob", "bob@example.com"));
+  store.addEdge({
+    fromId: "File:src/a.ts:src/a.ts",
+    toId: "Contributor:alice",
+    type: "OWNED_BY",
+    confidence: 1,
+  });
+  store.addEdge({
+    fromId: "File:src/b.ts:src/b.ts",
+    toId: "Contributor:bob",
+    type: "OWNED_BY",
+    confidence: 1,
+  });
+
+  const map = await collectOwnersByPath(store, ["src/a.ts", "src/b.ts"]);
+  assert.deepEqual(map.get("src/a.ts"), ["alice@example.com"]);
+  assert.deepEqual(map.get("src/b.ts"), ["bob@example.com"]);
+});

packages/analysis/src/owners.ts

@@ -51,3 +51,65 @@ export async function listOwners(
   }
   return owners;
 }
+
+/**
+ * Map each repo-relative path to its `OWNED_BY` contributor identifiers, for
+ * the policy engine's `ownership_required` rule. One batched edge query over
+ * every File node, grouped by source file — so a verdict over N changed files
+ * is a single graph round-trip, not N calls to {@link listOwners}.
+ *
+ * Owner identity is the contributor's plain email when present (what an
+ * operator writes in `require_approval_from`), falling back to the email hash.
+ * Identifiers per path are deduped and sorted for deterministic output. Paths
+ * with no owner edges are omitted (the policy engine treats a missing entry as
+ * "no graph owners"). An empty `files` list returns an empty map.
+ */
+export async function collectOwnersByPath(
+  graph: IGraphStore,
+  files: readonly string[],
+): Promise<ReadonlyMap<string, readonly string[]>> {
+  const out = new Map<string, readonly string[]>();
+  if (files.length === 0) return out;
+  // Defensive: legacy / minimal test fakes implement only part of IGraphStore.
+  // A store without the edge/node readers yields an empty map rather than
+  // throwing — the policy engine then treats every path as "no graph owners",
+  // exactly as before this wiring existed.
+  if (typeof graph.listEdgesByType !== "function" || typeof graph.listNodesByKind !== "function") {
+    return out;
+  }
+
+  const fileNodeIds = files.map((f) => `File:${f}:${f}`);
+  const edges = await graph.listEdgesByType("OWNED_BY", { fromIds: fileNodeIds });
+  if (edges.length === 0) return out;
+
+  const contributors = await graph.listNodesByKind("Contributor");
+  const contribById = new Map<string, (typeof contributors)[number]>();
+  for (const c of contributors) contribById.set(c.id, c);
+
+  // File node id back to its repo-relative path (id form is `File:<path>:<path>`).
+  const pathByNodeId = new Map<string, string>();
+  for (let i = 0; i < files.length; i += 1) {
+    const path = files[i];
+    const id = fileNodeIds[i];
+    if (path !== undefined && id !== undefined) pathByNodeId.set(id, path);
+  }
+
+  const idsByPath = new Map<string, Set<string>>();
+  for (const edge of edges) {
+    const path = pathByNodeId.get(edge.from);
+    if (path === undefined) continue;
+    const c = contribById.get(edge.to);
+    if (c === undefined) continue;
+    const plain = typeof c.emailPlain === "string" ? c.emailPlain : "";
+    const identifier = plain.length > 0 ? plain : c.emailHash;
+    if (identifier.length === 0) continue;
+    const set = idsByPath.get(path) ?? new Set<string>();
+    set.add(identifier);
+    idsByPath.set(path, set);
+  }
+
+  for (const [path, set] of idsByPath) {
+    out.set(path, [...set].sort());
+  }
+  return out;
+}

packages/analysis/src/scan-enrich.test.ts

@@ -0,0 +1,110 @@
+/**
+ * Tests for `buildScanEnrichment` — the per-result graph-signal map the scan
+ * pipeline feeds `enrichWithProperties`.
+ *
+ * Regression context: `enrichWithProperties` had zero production callers, so
+ * `scan.sarif` shipped with no `opencodehub.*` graph signals. These tests pin
+ * that the builder maps a result to its File node's signals, keyed by the
+ * `primaryLocationLineHash` fingerprint (run-structure-independent).
+ */
+
+import { strict as assert } from "node:assert";
+import { test } from "node:test";
+import type { SarifLog } from "@opencodehub/sarif";
+import { buildScanEnrichment } from "./scan-enrich.js";
+import { FakeStore } from "./test-utils.js";
+
+/** Minimal SARIF result with a primary-location uri + fingerprint. */
+function result(uri: string, fingerprint: string) {
+  return {
+    ruleId: "demo-rule",
+    level: "warning",
+    message: { text: "x" },
+    locations: [{ physicalLocation: { artifactLocation: { uri } } }],
+    partialFingerprints: { primaryLocationLineHash: fingerprint },
+  };
+}
+
+function logWith(results: ReadonlyArray<ReturnType<typeof result>>): SarifLog {
+  return {
+    version: "2.1.0",
+    runs: [{ tool: { driver: { name: "demo", rules: [] } }, results }],
+  } as unknown as SarifLog;
+}
+
+test("buildScanEnrichment maps a result to its File node signals by fingerprint", async () => {
+  const store = new FakeStore();
+  store.addNode({
+    id: "File:src/a.ts:src/a.ts",
+    kind: "File",
+    name: "a.ts",
+    filePath: "src/a.ts",
+    busFactor: 2,
+    fixFollowFeatDensity: 0.5,
+  });
+  const log = logWith([result("src/a.ts", "fp-a")]);
+
+  const enrichment = await buildScanEnrichment(store, log, "/repo");
+  const byFp = enrichment.byResultFingerprint;
+  assert.ok(byFp !== undefined, "byResultFingerprint must be present");
+  assert.deepEqual(byFp?.get("fp-a"), { busFactor: 2, temporalFixDensity: 0.5 });
+  // Run-level stamp is deterministic (no clock / run id).
+  assert.deepEqual(enrichment.run, { enrichmentVersion: "1", sources: ["graph"] });
+});
+
+test("buildScanEnrichment normalizes an absolute result uri to the repo-relative node id", async () => {
+  // Scanners (e.g. semgrep) emit absolute uris; the File node id is keyed by
+  // the repo-relative path, so the builder must strip the repoPath prefix or
+  // it would never match — the bug this normalization fixes.
+  const store = new FakeStore();
+  store.addNode({
+    id: "File:src/a.ts:src/a.ts",
+    kind: "File",
+    name: "a.ts",
+    filePath: "src/a.ts",
+    busFactor: 4,
+  });
+  const log = logWith([result("/repo/src/a.ts", "fp-abs")]);
+
+  const enrichment = await buildScanEnrichment(store, log, "/repo");
+  assert.deepEqual(enrichment.byResultFingerprint?.get("fp-abs"), { busFactor: 4 });
+});
+
+test("buildScanEnrichment omits results whose file has no materialized signals", async () => {
+  const store = new FakeStore();
+  store.addNode({
+    id: "File:src/bare.ts:src/bare.ts",
+    kind: "File",
+    name: "bare.ts",
+    filePath: "src/bare.ts",
+  });
+  const log = logWith([result("src/bare.ts", "fp-bare")]);
+
+  const enrichment = await buildScanEnrichment(store, log, "/repo");
+  // No signals → no per-result map, but the run-level stamp still returns.
+  assert.equal(enrichment.byResultFingerprint, undefined);
+  assert.deepEqual(enrichment.run, { enrichmentVersion: "1", sources: ["graph"] });
+});
+
+test("buildScanEnrichment is byte-stable across two runs (no clock/run id)", async () => {
+  const store = new FakeStore();
+  store.addNode({
+    id: "File:src/a.ts:src/a.ts",
+    kind: "File",
+    name: "a.ts",
+    filePath: "src/a.ts",
+    busFactor: 3,
+  });
+  const log = logWith([result("src/a.ts", "fp-a")]);
+
+  const a = await buildScanEnrichment(store, log, "/repo");
+  const b = await buildScanEnrichment(store, log, "/repo");
+  assert.deepEqual(a, b);
+});
+
+test("buildScanEnrichment returns only the run stamp for an empty log", async () => {
+  const store = new FakeStore();
+  const enrichment = await buildScanEnrichment(store, logWith([]), "/repo");
+  assert.equal(enrichment.byResultFingerprint, undefined);
+  assert.deepEqual(enrichment.run, { enrichmentVersion: "1", sources: ["graph"] });
+});