From 9b3f91ecaedb7bbf086a6138608e2d60c3ffa9df Mon Sep 17 00:00:00 2001 From: Mike Schwarz <171890339+mvschwarz@users.noreply.github.com> Date: Wed, 4 Nov 2026 11:01:45 -0800 Subject: [PATCH] fix: scope audit names a slice another mission also uses (#2492) When two missions each have a slice folder of the same name, the slice list (GET /api/slices) keeps only one of them, and the routes that find a slice by name serve that one, so nothing told a person the other slice existed. rig scope audit, and the daemon's audit route behind the dashboard, now add a low-severity slice_name_shared finding to each such slice, naming the other missions and suggesting a rename. It is advice: it doesn't change the audit's result or exit code, which slice the list keeps, or any route. Limits: the check looks for a same-named folder under each other mission's slices directory, which is narrower than every case the indexer can drop; renaming a slice folder unbinds rows that name the old slice; addressing a slice by mission is a separate, larger change. Reported in #2487 by @ziadeh. --- packages/cli/src/commands/scope.ts | 30 +++++++++++++ packages/cli/src/lib/scope/scope-audit.ts | 5 ++- .../scope-audit-slice-name-shared.test.ts | 43 +++++++++++++++++++ .../daemon/src/domain/scope/scope-audit.ts | 5 ++- packages/daemon/src/routes/scope-audit.ts | 30 +++++++++++++ .../daemon/test/scope-audit-routes.test.ts | 15 +++++++ 6 files changed, 126 insertions(+), 2 deletions(-) create mode 100644 packages/cli/test/scope-audit-slice-name-shared.test.ts diff --git a/packages/cli/src/commands/scope.ts b/packages/cli/src/commands/scope.ts index 6818d5c19..044af0d8f 100644 --- a/packages/cli/src/commands/scope.ts +++ b/packages/cli/src/commands/scope.ts @@ -1236,6 +1236,8 @@ function buildAuditCommand(): Command { for (const sr of sliceResults) { const shadow = shadowedNodeFileFinding(path.join(slicesDir, sr.name), "slice"); if (shadow) sr.result.findings.push(shadow); + const shared = sharedSliceNameFinding(missionsRoot, missionName, sr.name); + if (shared) sr.result.findings.push(shared); } const allFindings = [ @@ -2156,6 +2158,34 @@ function shadowedNodeFileFinding(dir: string, level: "mission" | "slice"): { }; } +/** + * Advisory-only: another mission has a slice folder of the same name. The slice list and the routes that + * find a slice by name keep one slice per name, so the others are hidden there with no sign they exist. + * Twin of the daemon's finding of the same name. + */ +function sharedSliceNameFinding(missionsRoot: string, missionName: string, sliceName: string): { + kind: "slice_name_shared"; severity: "low"; path: string; message: string; remediation: string; +} | null { + let others: string[]; + try { + others = fs.readdirSync(missionsRoot, { withFileTypes: true }) + .filter((e) => e.isDirectory() && !e.name.startsWith(".") && e.name !== missionName + && fs.statSync(path.join(missionsRoot, e.name, "slices", sliceName), { throwIfNoEntry: false })?.isDirectory() === true) + .map((e) => e.name) + .sort(); + } catch { + return null; + } + if (others.length === 0) return null; + return { + kind: "slice_name_shared", + severity: "low", + path: path.join(missionsRoot, missionName, "slices", sliceName), + message: `Mission ${others.join(", ")} also ${others.length === 1 ? "has" : "have"} a slice named "${sliceName}". The slice list (GET /api/slices) and the routes that find a slice by name keep one slice per name, so all but one of these slices are hidden there.`, + remediation: "Rename one of the slice folders so each slice name is unique across missions. Advisory only — nothing is blocked.", + }; +} + export function scopeCommand(): Command { const cmd = new Command("scope") .description("Scope tree primitive: missions, slices, sub-slices (per conventions/scope-and-versioning)") diff --git a/packages/cli/src/lib/scope/scope-audit.ts b/packages/cli/src/lib/scope/scope-audit.ts index 5dce216a6..bfd8e3257 100644 --- a/packages/cli/src/lib/scope/scope-audit.ts +++ b/packages/cli/src/lib/scope/scope-audit.ts @@ -31,7 +31,10 @@ export type FindingKind = // SPEC.md compatibility — a node carrying BOTH authored files. Advisory by // construction (low severity never flips the exit code): SPEC.md wins, and the // shadowed README.md is a state to notice, never a failure to gate on. - | "shadowed_node_file"; + | "shadowed_node_file" + // Another mission has a slice folder of the same name, which the slice list and the name-only routes + // hide. Advisory (low severity): renaming is the person's call. + | "slice_name_shared"; export interface AuditFinding { kind: FindingKind; diff --git a/packages/cli/test/scope-audit-slice-name-shared.test.ts b/packages/cli/test/scope-audit-slice-name-shared.test.ts new file mode 100644 index 000000000..8f144ebd4 --- /dev/null +++ b/packages/cli/test/scope-audit-slice-name-shared.test.ts @@ -0,0 +1,43 @@ +// The CLI twin of the daemon's slice_name_shared advisory: two missions with a slice folder of the same +// name leave one of them out of the slice list and the routes that find a slice by name. Advisory only. +import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { Command } from "commander"; +import { scopeCommand } from "../src/commands/scope.js"; + +let workspace: string; +let missionsRoot: string; +beforeEach(() => { + workspace = fs.mkdtempSync(path.join(os.tmpdir(), "cli-slice-name-audit-")); + missionsRoot = path.join(workspace, "missions"); + for (const mission of ["alpha", "beta"]) { + fs.mkdirSync(path.join(missionsRoot, mission, "slices", "01-login"), { recursive: true }); + fs.writeFileSync(path.join(missionsRoot, mission, "SPEC.md"), "# mission\n"); + fs.writeFileSync(path.join(missionsRoot, mission, "slices", "01-login", "SPEC.md"), "# login\n"); + } +}); +afterEach(() => fs.rmSync(workspace, { recursive: true, force: true })); + +async function audit(mission: string): Promise<{ slices: Array<{ name: string; findings: Array<{ kind: string; severity: string; message: string }> }> }> { + const buf: string[] = []; + const origWrite = process.stdout.write.bind(process.stdout); + process.stdout.write = ((c: unknown) => { buf.push(String(c)); return true; }) as typeof process.stdout.write; + const program = new Command(); + program.addCommand(scopeCommand()); + program.exitOverride(); + try { + await program.parseAsync(["node", "rig", "scope", "audit", "--mission", mission, "--json", "--workspace", workspace]); + } finally { + process.stdout.write = origWrite; + } + return JSON.parse(buf.join("")); +} + +describe("rig scope audit — a slice name another mission also uses", () => { + it("names the other mission, as advice that blocks nothing", async () => { + const shared = (await audit("alpha")).slices.find((s) => s.name === "01-login")!.findings.find((f) => f.kind === "slice_name_shared"); + expect(shared).toMatchObject({ severity: "low", message: expect.stringContaining("beta") }); + }); +}); diff --git a/packages/daemon/src/domain/scope/scope-audit.ts b/packages/daemon/src/domain/scope/scope-audit.ts index 5dce216a6..bfd8e3257 100644 --- a/packages/daemon/src/domain/scope/scope-audit.ts +++ b/packages/daemon/src/domain/scope/scope-audit.ts @@ -31,7 +31,10 @@ export type FindingKind = // SPEC.md compatibility — a node carrying BOTH authored files. Advisory by // construction (low severity never flips the exit code): SPEC.md wins, and the // shadowed README.md is a state to notice, never a failure to gate on. - | "shadowed_node_file"; + | "shadowed_node_file" + // Another mission has a slice folder of the same name, which the slice list and the name-only routes + // hide. Advisory (low severity): renaming is the person's call. + | "slice_name_shared"; export interface AuditFinding { kind: FindingKind; diff --git a/packages/daemon/src/routes/scope-audit.ts b/packages/daemon/src/routes/scope-audit.ts index cd73c4cac..a9b77fda1 100644 --- a/packages/daemon/src/routes/scope-audit.ts +++ b/packages/daemon/src/routes/scope-audit.ts @@ -128,6 +128,34 @@ function shadowedNodeFileFinding(dir: string, level: "mission" | "slice"): Audit }; } +/** + * Advisory-only: another mission has a slice folder of the same name. The slice list and the routes that + * find a slice by name keep one slice per name, so the others are hidden there with no sign they exist. + * Twin of the CLI's finding of the same name. + */ +function sharedSliceNameFinding(missionsRoot: string, missionName: string, sliceName: string): { + kind: "slice_name_shared"; severity: "low"; path: string; message: string; remediation: string; +} | null { + let others: string[]; + try { + others = fs.readdirSync(missionsRoot, { withFileTypes: true }) + .filter((e) => e.isDirectory() && !e.name.startsWith(".") && e.name !== missionName + && fs.statSync(path.join(missionsRoot, e.name, "slices", sliceName), { throwIfNoEntry: false })?.isDirectory() === true) + .map((e) => e.name) + .sort(); + } catch { + return null; + } + if (others.length === 0) return null; + return { + kind: "slice_name_shared", + severity: "low", + path: path.join(missionsRoot, missionName, "slices", sliceName), + message: `Mission ${others.join(", ")} also ${others.length === 1 ? "has" : "have"} a slice named "${sliceName}". The slice list (GET /api/slices) and the routes that find a slice by name keep one slice per name, so all but one of these slices are hidden there.`, + remediation: "Rename one of the slice folders so each slice name is unique across missions. Advisory only — nothing is blocked.", + }; +} + export function scopeAuditRoutes(): Hono { const app = new Hono(); @@ -298,6 +326,8 @@ export function scopeAuditRoutes(): Hono { for (const sr of sliceResults) { const shadow = shadowedNodeFileFinding(path.join(slicesDir, sr.name), "slice"); if (shadow) sr.result.findings.push(shadow); + const shared = sharedSliceNameFinding(missionsRoot, missionName, sr.name); + if (shared) sr.result.findings.push(shared); } const allFindings = [ diff --git a/packages/daemon/test/scope-audit-routes.test.ts b/packages/daemon/test/scope-audit-routes.test.ts index 6673d8cf4..5309204f6 100644 --- a/packages/daemon/test/scope-audit-routes.test.ts +++ b/packages/daemon/test/scope-audit-routes.test.ts @@ -233,6 +233,21 @@ describe("GET /api/scope/audit", () => { expect(sl!.severity).toBe("low"); }); + it("says when another mission has a slice of the same name, which the slice list would hide", async () => { + for (const mission of ["alpha", "beta"]) { + const sliceDir = path.join(missionsRoot, mission, "slices", "01-login"); + fs.mkdirSync(sliceDir, { recursive: true }); + fs.writeFileSync(path.join(missionsRoot, mission, "SPEC.md"), "# mission\n", "utf8"); + fs.writeFileSync(path.join(sliceDir, "SPEC.md"), "# login\n", "utf8"); + } + fs.mkdirSync(path.join(missionsRoot, "alpha", "slices", "02-only-here"), { recursive: true }); + const res = await app.request("/api/scope/audit?mission=alpha"); + const body = await res.json() as { ok: boolean; slices: Array<{ name: string; findings: Array<{ kind: string; severity: string; message: string }> }> }; + const shared = body.slices.find((s) => s.name === "01-login")!.findings.find((f) => f.kind === "slice_name_shared"); + expect(shared).toMatchObject({ severity: "low", message: expect.stringContaining("beta") }); + expect(body.slices.find((s) => s.name === "02-only-here")!.findings.some((f) => f.kind === "slice_name_shared")).toBe(false); + }); + it("says nothing about a node that has only ONE authored file", async () => { const missionDir = path.join(missionsRoot, "single-mission"); fs.mkdirSync(missionDir, { recursive: true });