mirror of
https://github.com/mvschwarz/openrig.git
synced 2026-11-07 05:56:21 -10:00
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.
This commit is contained in:
1 parent
8baec13548
commit
9b3f91ecae
6 files changed
+126
-2
No files matched your search
@@ -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)")
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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") });
|
||||
});
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
@@ -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 = [
|
||||
|
||||
@@ -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 });
|
||||
|
||||
Reference in new issue
Block a user