From 7cbf78dab91b407cfab0bfc6771b4674574498d8 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Sun, 4 Oct 2026 22:40:19 -0500 Subject: [PATCH] docs(review): row 36 S1 round 1, changes (filbert) Co-Authored-By: Claude Opus 5.5 --- agents/filbert/work/slice1-s1-review/hook.mjs | 11 ++ .../work/slice1-s1-review/mut-summary.txt | 20 ++ .../filbert/work/slice1-s1-review/mutants.sh | 41 +++++ .../filbert/work/slice1-s1-review/node24.txt | 15 ++ .../filbert/work/slice1-s1-review/probe.mjs | 160 ++++++++++++++++ .../filbert/work/slice1-s1-review/probe.txt | 40 ++++ .../work/slice1-s1-review/review-r1.md | 174 ++++++++++++++++++ .../filbert/work/slice1-s1-review/summary.txt | 5 + .../work/slice1-s1-review/v1-compat.txt | 17 ++ docs/SESSIONS.md | 1 + 10 files changed, 484 insertions(+) create mode 100644 agents/filbert/work/slice1-s1-review/hook.mjs create mode 100644 agents/filbert/work/slice1-s1-review/mut-summary.txt create mode 100755 agents/filbert/work/slice1-s1-review/mutants.sh create mode 100644 agents/filbert/work/slice1-s1-review/node24.txt create mode 100644 agents/filbert/work/slice1-s1-review/probe.mjs create mode 100644 agents/filbert/work/slice1-s1-review/probe.txt create mode 100644 agents/filbert/work/slice1-s1-review/review-r1.md create mode 100644 agents/filbert/work/slice1-s1-review/summary.txt create mode 100644 agents/filbert/work/slice1-s1-review/v1-compat.txt diff --git a/agents/filbert/work/slice1-s1-review/hook.mjs b/agents/filbert/work/slice1-s1-review/hook.mjs new file mode 100644 index 00000000..0a9dea00 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/hook.mjs @@ -0,0 +1,11 @@ +import fs from "node:fs"; +import { syncBuiltinESMExports } from "node:module"; +globalThis.__opened = []; +for (const name of ["openSync", "readFileSync", "open", "readFile", "createReadStream"]) { + const orig = fs[name]; + fs[name] = function (p, ...rest) { globalThis.__opened.push(typeof p === "number" ? `fd:${p}` : String(p)); return orig.call(this, p, ...rest); }; +} +const po = fs.promises.open, pr = fs.promises.readFile; +fs.promises.open = (p, ...r) => { globalThis.__opened.push(String(p)); return po(p, ...r); }; +fs.promises.readFile = (p, ...r) => { globalThis.__opened.push(String(p)); return pr(p, ...r); }; +syncBuiltinESMExports(); diff --git a/agents/filbert/work/slice1-s1-review/mut-summary.txt b/agents/filbert/work/slice1-s1-review/mut-summary.txt new file mode 100644 index 00000000..94840af0 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/mut-summary.txt @@ -0,0 +1,20 @@ +M1-vars-narrower-wider business_fail=2 +M2-resolve-network-wider business_fail=2 +M3-cross-not-narrowed business_fail=0 task:selftest: 98 passed, 0 failed +M4-layer-check-off business_fail=5 +M5-no-realpath-file business_fail=1 +M6-gated-only-allowed business_fail=2 +M7-launcher-in-instances business_fail=1 +M8-sync-bot-shared business_fail=1 +M9-within-cross-overlap business_fail=1 +M10-project-id-mismatch business_fail=1 +M11-token-empty-ok business_fail=1 +M12-token-uid-off business_fail=1 +M13-contract-symlink-ok business_fail=0 task:selftest: 98 passed, 0 failed +M14-vikunja-expired-ok business_fail=2 +M15-credmap-not-exact business_fail=2 +M16-tracker-without-vikunja business_fail=1 +M17-max-ceiling-off business_fail=1 +M18-project-undeclared-instance business_fail=2 +M19-unknown-var-ok business_fail=2 +M20-launch-gate-cross-only business_fail=1 diff --git a/agents/filbert/work/slice1-s1-review/mutants.sh b/agents/filbert/work/slice1-s1-review/mutants.sh new file mode 100755 index 00000000..d4afa37b --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/mutants.sh @@ -0,0 +1,41 @@ +#!/bin/bash +# name|file|perl substitution +cd ~/filbert-scratch/r36/repo +S=packages/business/src +L=~/filbert-scratch/r36/logs +: > $L/mut-summary.txt +while IFS='|' read -r name file expr; do + [ -z "$name" ] && continue + rm -rf $S && cp -a ~/filbert-scratch/r36/src-orig $S + perl -0pi -e "$expr" $S/$file + if cmp -s $S/$file ~/filbert-scratch/r36/src-orig/$file; then echo "$name NOAPPLY" >> $L/mut-summary.txt; continue; fi + node --test packages/business/tests/ > $L/mut-$name.txt 2>&1 + f=$(grep -E '^ℹ fail' $L/mut-$name.txt | awk '{print $3}') + extra="" + if [ "$f" = 0 ]; then bash scripts/test-task.sh > $L/mut-$name-task.txt 2>&1; extra=" task:$(grep -E 'passed' $L/mut-$name-task.txt | tail -1)"; fi + echo "$name business_fail=$f$extra" >> $L/mut-summary.txt +done <<'LIST' +M1-vars-narrower-wider|vars.mjs|s/NETWORKS\.indexOf\(a\) <= NETWORKS\.indexOf\(b\)/NETWORKS.indexOf(a) >= NETWORKS.indexOf(b)/ +M2-resolve-network-wider|resolve.mjs|s/NETWORKS\.indexOf\(a\) <= NETWORKS\.indexOf\(b\)/NETWORKS.indexOf(a) >= NETWORKS.indexOf(b)/ +M3-cross-not-narrowed|resolve.mjs|s/crossRole = crossRole\.filter\(\(a\) => vars\["limits\.authority"\]\.includes\(a\)\);// +M4-layer-check-off|vars.mjs|s/if \(!entry\.layers\.includes\(layer\)\)/if (false)/ +M5-no-realpath-file|credentials.mjs|s/real = realpathSync\(ref\.file\);/real = ref.file;/ +M6-gated-only-allowed|role.mjs|s/if \(GATED_ONLY\.includes\(action\)\) refuse/if (false) refuse/ +M7-launcher-in-instances|business.mjs|s/if \(name === launch\.by\) refuse/if (false) refuse/ +M8-sync-bot-shared|business.mjs|s/role\.tracker && \(role\.tracker\.bot === tracker\.sync\.bot/false && (role.tracker.bot === tracker.sync.bot/ +M9-within-cross-overlap|role.mjs|s/if \(both\.length > 0\) refuse/if (false) refuse/ +M10-project-id-mismatch|resolve.mjs|s/\|\| declared\[0\] !== project\.id// +M11-token-empty-ok|credentials.mjs|s/if \(stat\.size === 0\) problems/if (false) problems/ +M12-token-uid-off|credentials.mjs|s/if \(stat\.uid !== uid\) problems/if (false) problems/ +M13-contract-symlink-ok|role.mjs|s/!stat\.isFile\(\) \|\| stat\.isSymbolicLink\(\) \|\| stat\.size === 0/!stat.isFile() || stat.size === 0/ +M14-vikunja-expired-ok|credentials.mjs|s/problems\.push\(`\$\{label\}: expired/warnings.push(`\${label}: expired/ +M15-credmap-not-exact|business.mjs|s/refuse\(`\$\{where\} must reference exactly/if (false) refuse(`\${where} must reference exactly/ +M16-tracker-without-vikunja|business.mjs|s/refuse\(`\$\{at\} has "tracker" but/if (false) refuse(`\${at} has "tracker" but/ +M17-max-ceiling-off|business.mjs|s/ \|\| count > ceiling// +M18-project-undeclared-instance|resolve.mjs|s/if \(!Object\.hasOwn\(business\.roles, name\)\) refuse\(`project/if (false) refuse(`project/ +M19-unknown-var-ok|vars.mjs|s/if \(!entry\) refuse\(`\$\{where\}: unknown variable/if (!entry) continue; if (false) refuse(`\${where}: unknown variable/ +M20-launch-gate-cross-only|resolve.mjs|s/withinRole = withinRole\.filter\(\(a\) => a !== "role\.launch"\);// +LIST +rm -rf $S && cp -a ~/filbert-scratch/r36/src-orig $S +diff -r $S ~/filbert-scratch/r36/src-orig && echo RESTORED +cat $L/mut-summary.txt diff --git a/agents/filbert/work/slice1-s1-review/node24.txt b/agents/filbert/work/slice1-s1-review/node24.txt new file mode 100644 index 00000000..75b58bae --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/node24.txt @@ -0,0 +1,15 @@ +v24.21.0 +✔ merge doesn't change its inputs (0.172055ms) +ℹ tests 57 +ℹ suites 0 +ℹ pass 57 +ℹ fail 0 +ℹ cancelled 0 +ℹ skipped 0 +ℹ todo 0 +ℹ duration_ms 2014.384314 +---dirform + +test at packages/business/tests:1:1 +✖ packages/business/tests (28.511224ms) + 'test failed' diff --git a/agents/filbert/work/slice1-s1-review/probe.mjs b/agents/filbert/work/slice1-s1-review/probe.mjs new file mode 100644 index 00000000..c0a56826 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/probe.mjs @@ -0,0 +1,160 @@ +// Row 36 review probes (Filbert). Imports the candidate from the scratch clone. +import { mkdirSync, mkdtempSync, writeFileSync, readFileSync, chmodSync, symlinkSync } from "node:fs"; +import { join } from "node:path"; +const R = `${process.env.HOME}/filbert-scratch/r36/repo`; +const B = await import(`${R}/packages/business/src/index.mjs`); +const H = await import(`${R}/packages/business/tests/helpers.mjs`); +const base = `${process.env.HOME}/filbert-scratch/r36/probe/work`; +mkdirSync(base, { recursive: true }); +let n = 0; +const out = (name, ok, detail = "") => console.log(`${ok ? "PASS" : "FAIL"} ${name}${detail ? ` :: ${detail}` : ""}`); +const tryRun = (f) => { try { return { ok: true, v: f() }; } catch (e) { return { ok: false, e: e.message, code: e.exitCode }; } }; + +function setup(mut = (d) => d, project = null) { + const root = mkdtempSync(join(base, "b-")); + H.systemConfig(root); + const env = { MOSAIC_CONFIG: join(root, "config", "config.json") }; + const doc = mut(H.businessDoc(root)); + const dir = join(root, "config"); + H.writeJson(join(dir, "businesses", "acme.json"), doc); + if (project) H.writeJson(join(root, "project", ".mosaic", "project.json"), project); + const rolesDir = join(R, "roles"); + return { root, env, dir, rolesDir }; +} +function load(s) { return B.loadBusiness("acme", { rolesDir: s.rolesDir, dir: s.dir, env: s.env }); } + +// A. example refuses as shipped, and each placeholder refuses alone. +{ + const ex = JSON.parse(readFileSync(`${R}/packages/business/examples/mosaic-stack.example.json`, "utf8")); + const r = tryRun(() => B.validateBusinessDocument(ex, "mosaic-stack.json", { rolesDir: `${R}/roles` })); + out("A1 example refuses as shipped", !r.ok, r.e); + const fixBots = structuredClone(ex); let id = 10; + fixBots.tracker.sync.botId = id++; for (const v of Object.values(fixBots.roles)) v.tracker.botId = id++; + const r2 = tryRun(() => B.validateBusinessDocument(fixBots, "mosaic-stack.json", { rolesDir: `${R}/roles` })); + out("A2 bot ids fixed, dates still placeholders: refuses", !r2.ok, r2.e); + const fixDates = JSON.parse(JSON.stringify(ex).replaceAll("YYYY-MM-DD", "2099-01-01")); + const r3 = tryRun(() => B.validateBusinessDocument(fixDates, "mosaic-stack.json", { rolesDir: `${R}/roles` })); + out("A3 dates fixed, bot ids 0: refuses", !r3.ok, r3.e); + const both = JSON.parse(JSON.stringify(fixBots).replaceAll("YYYY-MM-DD", "2099-01-01")); + const r4 = tryRun(() => B.validateBusinessDocument(both, "mosaic-stack.json", { rolesDir: `${R}/roles` })); + out("A4 both fixed: document validates (shape only; token files are a CLI check)", r4.ok, r4.e); +} + +// B. resolver precedence, intersect, refusals. +const sys = (root) => H.systemFor(root); +{ + const s = setup((d) => { d.vars["limits.tools"] = ["read", "grep", "bash", "write"]; d.vars["limits.network"] = "open"; + d.roles.coder.vars = { harness: "pi", "limits.tools": ["read", "bash", "edit", "write"], "limits.network": "open" }; return d; }, + { projectVersion: 1, id: "stack", vars: { "tracker.pollSeconds": 45, "limits.tools": ["read", "bash", "grep", "write"] }, + roles: { coder: { vars: { "tracker.pollSeconds": 20 } } } }); + const b = load(s); + const p = B.loadProject(join(s.root, "project")); + const r = B.resolveInstance({ system: sys(s.root), business: b, project: p, instance: "coder" }); + out("B1 replace: project roles. beats project beats business", r.vars["tracker.pollSeconds"] === 20, `${r.vars["tracker.pollSeconds"]} from ${r.provenance["tracker.pollSeconds"]}`); + out("B2 limits.tools intersects business ∩ project ∩ agent ∩ ceiling", JSON.stringify(r.limits.tools) === JSON.stringify(["read", "write", "bash"]), JSON.stringify(r.limits.tools)); + out("B3 limits.network open can't widen role api-only", r.limits.network === "api-only", r.limits.network); + const r0 = B.resolveInstance({ system: sys(s.root), business: b, instance: "coder" }); + out("B4 no project: business value", r0.vars["tracker.pollSeconds"] === 60, String(r0.vars["tracker.pollSeconds"])); + out("B5 default kept when unset", r0.vars["tracker.reconcileMinutes"] === 60 && r0.provenance["tracker.reconcileMinutes"] === "default"); +} +for (const [name, mut] of [ + ["B6 unknown key at business refuses", (d) => { d.vars["tracker.pollseconds"] = 30; return d; }], + ["B7 agent-only key (harness) at business refuses", (d) => { d.vars.harness = "pi"; return d; }], + ["B8 business-only key at agent refuses", (d) => { d.roles.coder.vars = { "tracker.baseUrl": "http://x" }; return d; }], + ["B9 system key at business refuses", (d) => { d.vars.dataRoot = "/x"; return d; }], + ["B10 __proto__ key refuses", (d) => JSON.parse(JSON.stringify(d).replace('"vars":{', '"vars":{"__proto__":{"a":1},')) ], + ["B11 limits.network outside vocabulary refuses", (d) => { d.vars["limits.network"] = "lan"; return d; }], + ["B12 limits.authority unknown action refuses", (d) => { d.vars["limits.authority"] = ["task.delete"]; return d; }], +]) { + const s = setup(mut); + const r = tryRun(() => load(s)); + out(name, !r.ok && r.code === 2, r.e); +} +for (const [name, project] of [ + ["B13 project file: agent key (model) at project vars refuses", { projectVersion: 1, id: "stack", vars: { model: "x" }, roles: {} }], + ["B14 project roles.: agent key refuses (project layer)", { projectVersion: 1, id: "stack", vars: {}, roles: { coder: { vars: { harness: "pi" } } } }], + ["B15 project roles names undeclared instance refuses", { projectVersion: 1, id: "stack", vars: {}, roles: { ghost: { vars: {} } } }], + ["B16 project roles.constructor refuses", { projectVersion: 1, id: "stack", vars: {}, roles: { constructor: { vars: {} } } }], +]) { + const s = setup((d) => d, project); + const b = load(s); + const r = tryRun(() => B.resolveInstance({ system: sys(s.root), business: b, project: B.loadProject(join(s.root, "project")), instance: "coder" })); + out(name, !r.ok, r.e); +} +{ + const s = setup(); + const b = load(s); + for (const inst of ["constructor", "toString", "__proto__"]) { + const r = tryRun(() => B.resolveInstance({ system: sys(s.root), business: b, instance: inst })); + out(`B17 instance ${inst} refuses`, !r.ok, r.e); + } +} + +// C. authority: limits.authority allowlist, role.launch. +{ + const s = setup((d) => { d.vars["limits.authority"] = ["task.create", "task.assign", "role.launch", "message.send", "task.scope.change", "review.verdict"]; + d.roles.pm.vars = { ...d.roles.pm.vars, "limits.authority": ["task.create", "role.launch", "task.scope.change", "task.close"] }; return d; }); + const b = load(s); + const pm = B.resolveInstance({ system: sys(s.root), business: b, instance: "pm" }); + out("C1 pm within = definition ∩ business ∩ agent", JSON.stringify(pm.limits.authority.withinRole) === JSON.stringify(["task.create", "role.launch"]), JSON.stringify(pm.limits.authority)); + out("C2 pm cross keeps scope.change, drops priority.change", JSON.stringify(pm.limits.authority.crossRole) === JSON.stringify(["task.scope.change"])); + out("C3 classify: dropped action is gated", B.classify(pm, "task.assign") === "gated" && B.classify(pm, "task.priority.change") === "gated"); + out("C4 classify: gated-only action is gated", B.classify(pm, "deploy") === "gated"); + out("C5 limits can't grant an action outside the definition (review.verdict for pm)", B.classify(pm, "review.verdict") === "gated"); + const rv = B.resolveInstance({ system: sys(s.root), business: b, instance: "reviewer" }); + out("C6 reviewer keeps only listed actions", JSON.stringify(rv.limits.authority.withinRole) === JSON.stringify(["review.verdict", "message.send"]), JSON.stringify(rv.limits.authority.withinRole)); + const e = tryRun(() => B.classify(pm, "task.delete")); + out("C7 classify unknown action refuses", !e.ok, e.e); +} +{ + const s = setup(); + const b = load(s); + const pm = B.resolveInstance({ system: sys(s.root), business: b, instance: "pm" }); + out("C8 pm with launch.by=pm: role.launch within", B.classify(pm, "role.launch") === "within" && pm.launch !== null); + const s2 = setup((d) => { delete d.launch; return d; }); + const r2 = tryRun(() => load(s2)); + if (r2.ok) { const pm2 = B.resolveInstance({ system: sys(s2.root), business: r2.v, instance: "pm" }); + out("C9 no launch block: pm role.launch gated", B.classify(pm2, "role.launch") === "gated" && pm2.launch === null); } + else out("C9 no launch block: business refuses (launch required)", true, r2.e); + const s3 = setup((d) => { d.launch.by = "cto"; d.launch.instances = ["coder", "reviewer"]; return d; }); + const r3 = tryRun(() => load(s3)); + if (r3.ok) { + const cto = B.resolveInstance({ system: sys(s3.root), business: r3.v, instance: "cto" }); + const pm3 = B.resolveInstance({ system: sys(s3.root), business: r3.v, instance: "pm" }); + out("C10 launch.by=cto (no role.launch in cto definition) accepted", true, `cto.launch=${cto.launch ? "set" : "null"} classify(cto, role.launch)=${B.classify(cto, "role.launch")} classify(pm, role.launch)=${B.classify(pm3, "role.launch")}`); + } else out("C10 launch.by=cto refuses", true, r3.e); + const s4 = setup((d) => { d.roles.pm.vars = { ...d.roles.pm.vars, "limits.authority": ["task.create"] }; return d; }); + const b4 = load(s4); + const pm4 = B.resolveInstance({ system: sys(s4.root), business: b4, instance: "pm" }); + out("C11 limits.authority drops role.launch for launch.by", true, `launch=${pm4.launch ? "set" : "null"} classify=${B.classify(pm4, "role.launch")}`); +} + +// D. credentials: never opened (fs hook counts opens of token paths). +{ + const s = setup(); + const b = load(s); + const refs = [...Object.values(b.tracker.sync.credentials), ...Object.values(b.roles).flatMap((r) => Object.values(r.credentials))]; + const opened = globalThis.__opened; + opened.length = 0; + const problems = refs.flatMap((ref) => B.checkCredentialRef(ref, { forbiddenRoots: [R] }).problems); + const tokenOpens = opened.filter((p) => refs.some((ref) => ref.file && String(p).startsWith(ref.file))); + out("D1 checkCredentialRef on 9 token files: 0 opens", tokenOpens.length === 0 && problems.length === 0, `refs=${refs.length} opens=${JSON.stringify(tokenOpens)} problems=${problems.length}`); + // symlinked parent pointing into a forbidden root + const inRepo = join(s.root, "fakerepo"); mkdirSync(join(inRepo, "sec"), { recursive: true }); + const tok = join(inRepo, "sec", "t.token"); writeFileSync(tok, "x"); chmodSync(tok, 0o600); + const link = join(s.root, "outside"); symlinkSync(join(inRepo, "sec"), link); + const ref = B.parseCredentialRef({ file: join(link, "t.token"), rotateBy: "2099-01-01" }, "gitea", "p"); + const r = B.checkCredentialRef(ref, { forbiddenRoots: [inRepo] }); + out("D2 token reached through a symlinked parent inside a forbidden root refuses", r.problems.some((p) => p.includes("inside")), JSON.stringify(r.problems)); + chmodSync(tok, 0o640); + const r2 = B.checkCredentialRef(B.parseCredentialRef({ file: tok, expires: "2099-01-01" }, "vikunja", "p"), {}); + out("D3 mode 640 refuses", r2.problems.some((p) => p.includes("mode")), JSON.stringify(r2.problems)); + const r3 = B.checkCredentialRef(B.parseCredentialRef({ file: tok, expires: "2000-01-01" }, "vikunja", "p"), {}); + out("D4 expired vikunja refuses", r3.problems.some((p) => p.includes("expired"))); + const r4 = B.checkCredentialRef(B.parseCredentialRef({ env: "GITEA_TOKEN", rotateBy: "2000-01-01" }, "gitea", "p"), { env: {} }); + out("D5 gitea rotateBy past only warns; env unset only warns", r4.problems.length === 0 && r4.warnings.length === 2, JSON.stringify(r4.warnings)); +} +{ // hook sanity: the business file read is seen + const s = setup(); globalThis.__opened.length = 0; load(s); + out("D0 hook sees the business file read", globalThis.__opened.some((p) => p.endsWith("acme.json")), String(globalThis.__opened.length)); +} diff --git a/agents/filbert/work/slice1-s1-review/probe.txt b/agents/filbert/work/slice1-s1-review/probe.txt new file mode 100644 index 00000000..84880f09 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/probe.txt @@ -0,0 +1,40 @@ +PASS A1 example refuses as shipped :: mosaic-stack.json roles.pm.tracker.botId must be a positive integer (got 0) +PASS A2 bot ids fixed, dates still placeholders: refuses :: mosaic-stack.json roles.pm.credentials.gitea.rotateBy must be a date YYYY-MM-DD (got "YYYY-MM-DD") +PASS A3 dates fixed, bot ids 0: refuses :: mosaic-stack.json roles.pm.tracker.botId must be a positive integer (got 0) +PASS A4 both fixed: document validates (shape only; token files are a CLI check) +PASS B1 replace: project roles. beats project beats business :: 20 from project:stack:roles.coder +PASS B2 limits.tools intersects business ∩ project ∩ agent ∩ ceiling :: ["read","write","bash"] +PASS B3 limits.network open can't widen role api-only :: api-only +PASS B4 no project: business value :: 60 +PASS B5 default kept when unset +PASS B6 unknown key at business refuses :: ~/filbert-scratch/r36/probe/work/b-G17WAF/config/businesses/acme.json vars: unknown variable "tracker.pollseconds" +PASS B7 agent-only key (harness) at business refuses :: ~/filbert-scratch/r36/probe/work/b-UMkx0k/config/businesses/acme.json vars: variable harness can't be set at the business layer (allowed: agent) +PASS B8 business-only key at agent refuses :: ~/filbert-scratch/r36/probe/work/b-wJNvm9/config/businesses/acme.json roles.coder.vars: variable tracker.baseUrl can't be set at the agent layer (allowed: business) +PASS B9 system key at business refuses :: ~/filbert-scratch/r36/probe/work/b-mx86C2/config/businesses/acme.json vars: variable dataRoot can't be set at the business layer (allowed: system) +PASS B10 __proto__ key refuses :: ~/filbert-scratch/r36/probe/work/b-Jp95ie/config/businesses/acme.json vars: unknown variable "__proto__" +PASS B11 limits.network outside vocabulary refuses :: variable limits.network must be one of: none, api-only, open (got "lan") +PASS B12 limits.authority unknown action refuses :: variable limits.authority entry names an unknown action: "task.delete" +PASS B13 project file: agent key (model) at project vars refuses :: ~/filbert-scratch/r36/probe/work/b-DucpG1/project/.mosaic/project.json vars: variable model can't be set at the project layer (allowed: agent) +PASS B14 project roles.: agent key refuses (project layer) :: ~/filbert-scratch/r36/probe/work/b-xuQ991/project/.mosaic/project.json roles.coder.vars: variable harness can't be set at the project layer (allowed: agent) +PASS B15 project roles names undeclared instance refuses :: project stack sets vars for role instance ghost, which business acme doesn't declare +PASS B16 project roles.constructor refuses :: project stack sets vars for role instance constructor, which business acme doesn't declare +PASS B17 instance constructor refuses :: business acme declares no role instance "constructor" +PASS B17 instance toString refuses :: business acme declares no role instance "toString" +PASS B17 instance __proto__ refuses :: business acme declares no role instance "__proto__" +PASS C1 pm within = definition ∩ business ∩ agent :: {"withinRole":["task.create","role.launch"],"crossRole":["task.scope.change"]} +PASS C2 pm cross keeps scope.change, drops priority.change +PASS C3 classify: dropped action is gated +PASS C4 classify: gated-only action is gated +PASS C5 limits can't grant an action outside the definition (review.verdict for pm) +PASS C6 reviewer keeps only listed actions :: ["review.verdict","message.send"] +PASS C7 classify unknown action refuses :: unknown action: "task.delete" +PASS C8 pm with launch.by=pm: role.launch within +PASS C9 no launch block: pm role.launch gated +PASS C10 launch.by=cto (no role.launch in cto definition) accepted :: cto.launch=set classify(cto, role.launch)=gated classify(pm, role.launch)=gated +PASS C11 limits.authority drops role.launch for launch.by :: launch=set classify=gated +PASS D1 checkCredentialRef on 9 token files: 0 opens :: refs=9 opens=[] problems=0 +PASS D2 token reached through a symlinked parent inside a forbidden root refuses :: ["gitea file ~/filbert-scratch/r36/probe/work/b-hFyvw1/outside/t.token: inside ~/filbert-scratch/r36/probe/work/b-hFyvw1/fakerepo; token files live outside the repository and dataRoot"] +PASS D3 mode 640 refuses :: ["vikunja file ~/filbert-scratch/r36/probe/work/b-hFyvw1/fakerepo/sec/t.token: mode 640 gives group or other access; use 600"] +PASS D4 expired vikunja refuses +PASS D5 gitea rotateBy past only warns; env unset only warns :: ["gitea env GITEA_TOKEN: not set in this environment; the launcher must provide it","gitea env GITEA_TOKEN: rotation was due on 2000-01-01"] +PASS D0 hook sees the business file read :: 5 diff --git a/agents/filbert/work/slice1-s1-review/review-r1.md b/agents/filbert/work/slice1-s1-review/review-r1.md new file mode 100644 index 00000000..db323830 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/review-r1.md @@ -0,0 +1,174 @@ +# Slice 1 S1, row 36, round 1 review (Filbert) + +Issue #1518, request comment 26720. Candidate: manifest +`agents/darkwing/work/slice1-s1/build-manifest.sha256`, sha256 +`d934e5af30cd646194bbf1d9bffc2b4d51486a4bb4c097ec43376352830b0195`, 34 +files. Packet: `agents/darkwing/work/slice1-s1/build.md` and +`build.patch` (sha256 `8067882052abc6e75684b318f738ef990f53c7a35913a8313a341282d259bd94`), +base `fef4b362`. + +Verdict: **changes.** The code does what the brief asks, and every point +Sage and Darkwing asked me to check holds. Two small blockers remain: the +`launch` block and `role.launch` can disagree in the resolved record, and +the cross-role half of the `limits.authority` allowlist has no test. + +## Method + +- Scratch clone `~/filbert-scratch/r36/repo` at `fef4b362`, push URL + `DISABLED`. `git apply build.patch`, then `sha256sum -c` on the + manifest: 34 OK. `node_modules` is a symlink to the canonical + checkout's. +- Probes: `probe.mjs` (40 checks, run under `hook.mjs`, which records + every `open`, `readFile` and `createReadStream` path) and a version 1 + comparison of `resolve-role` between the base and the candidate. +- Mutants: `mutants.sh`, 20 mutants, each against the business tests, and + against `test-task.sh` when the business tests didn't catch it. +- The scripts and teed logs are in this directory. + +## Suites + +| Suite | Result | +|---|---| +| `node --test packages/business/tests/` (Node 26.8.1) | 57 pass, 0 fail | +| `node --test "packages/business/tests/*.test.mjs"` (Node 24.21.0, `node:24`, no network, clone read-only) | 57 pass, 0 fail | +| The directory form on Node 24 | fails (`test at packages/business/tests:1:1`), as Darkwing reported | +| test-config, test-task | 24 and 98 passed, 0 failed | +| test-conductor, test-queue | 17 and 27 passed, 0 failed | + +## Sage's points + +1. **Vocabulary placement (not under `contracts/`).** I agree. The + `Containerfile` copies only `contracts`, `src` and `adapters` into the + image. None of those three trees mention roles, authority or the + vocabulary. The consumers are `scripts/mosaic-task.mjs` + (`resolve-role`), `scripts/agent.sh` (through `resolve-role`) and the + new CLI, and all of them run on the host. Workers get + `--no-context-files` and the generated prompt. If S5's broker ever + runs inside the image, revisit this. +2. **Resolver.** It works as the brief says (probes B1 to B17): + - `roles.` in the project file beats project `vars`, and + those beat business `vars`. Provenance names the winning source. + - `limits.tools` is the intersection of the business, project and + agent layers and the role ceiling. + - A `limits.network` of `open` doesn't widen a role at `api-only`, and + a value outside the vocabulary refuses. + - These refuse with exit 2: an unknown key, `__proto__`, an agent key + at the business or project layer, a business key at the agent layer + and a system key at the business layer. + - Instances named `constructor`, `toString` or `__proto__` refuse. +3. **Version 1 roles.** I ran 17 role files through `resolve-role` with + the base `mosaic-task.mjs` and the candidate. They gave the same + stdout and exit code in every case: `researcher.json`, the default + network, an explicit network, and 13 refusal cases, including empty + tools, a duplicate, an unknown tool, a name mismatch, a v2 key on v1, + network `lan` or `null`, a version 3 file and a missing file + (`v1-compat.txt`). `agent.sh` still reads only `MOSAIC_ROLE_TOOLS`, + and the `roleseat` cases in test-task pass. +4. **The `business` branch in `scripts/mosaic`.** Keep it. The brief asks + for TOOLS.md entries "for any new command", so it expects commands. + The branch copies the `queue` branch, and the documented surface is + `scripts/mosaic business`. The first argument used to fall through + to the seat CLI's usage error, so no existing call changes. + test-queue is green after the change. +5. **The example refuses as shipped.** Copied under its own id, it + refuses at `roles.pm.tracker.botId` (0). With the bot ids fixed, it + refuses at the first `"YYYY-MM-DD"`. With the dates fixed and the ids + left at 0, it refuses at the bot id. With both fixed, the document + validates. So each placeholder refuses by itself (A1 to A4). The + `/home/you` token paths would then fail the CLI's stat check. + +## Darkwing's points + +- **Choice 2 (pm authority).** I agree with all three additions. + Addendum A section 2 gives `task.update.assigned` to all four roles and + reassign to pm. Its field table changes title and description after + assignment only through `task.scope.change`. pm needs that verb to + split and rewrite tasks, and making it cross-role sends technical scope + to the CTO as arbiter. +- **Choice 5 (`role.launch`).** The verb is removed unless + `launch.by === instance` (C8, C9; mutant M20 is caught). See B1 for + the gap. +- **Choice 6 (`limits.authority` as an allowlist).** The code narrows + both lists, and a limit can't add an action the definition lacks (C1 + to C6). See B2 for the test gap. +- **Credentials are never opened.** `credentials.mjs` imports only + `lstatSync` and `realpathSync`. Under `hook.mjs`, checking the nine + token files of a full business recorded no open of any token path (D1). + The hook did see the business file read (D0), so it was active. Two + more checks: + - A token reached through a symlinked parent directory that sits + inside a forbidden root refuses (D2). + - Mode 640 refuses, a past Vikunja `expires` refuses, and a past Gitea + `rotateBy` or an unset `env` only warns (D3 to D5). + +## Blockers + +- **B1. `launch` and `role.launch` can disagree.** `checkLaunch` accepts a + `launch.by` whose role definition doesn't hold `role.launch`. With + `launch.by: "cto"`, the business validates and `resolveInstance(cto)` + returns `launch` set while `classify(cto, "role.launch")` is `gated` + (C10). The same thing happens when `limits.authority` leaves out + `role.launch` for the `by` instance (C11). + - The README says "Only that instance keeps `role.launch` as + within-role". It also lists `launch` in the frozen record that S2 + consumes, and S6 will consume it too. + - Nothing launches today, so the effect is fail-closed. But the record + carries two answers to "may this instance launch", and a consumer + that checks `launch !== null` gets the wrong one. + - Fix: + - Refuse a `launch.by` whose definition's `withinRole` lacks + `role.launch`. + - In `resolveInstance`, set `launch` to null when `role.launch` isn't + within-role after narrowing. + - Add a test for each. +- **B2. The cross-role half of the allowlist is untested.** Mutant M3 + deletes `crossRole = crossRole.filter(...)` in `resolve.mjs`. It + survives the 57 business tests and the 98 test-task checks. The one + authority test uses reviewer, whose `crossRole` is empty. Without the + filter, a business that leaves `task.reassign` out of coder's + allowlist still gets `cross` (arbitrated) instead of `gated`. That + widens the limit, and choice 6 exists to prevent it. Fix: add a test + where a cross-role action is left out of `limits.authority` and + `classify` returns `gated`. + +## Notes (not blocking) + +- **n1.** `loadBusiness` reads the file, then runs `lstat` again to check + the owner and mode. Another writer could swap the file between the two + calls. The window is small and the file is in the user's own config + directory. Checking before the read, or using `fstat` on the opened + descriptor, would close it. +- **n2.** Mutant M13 removes the `isSymbolicLink()` check on the contract, + and it survives. It is equivalent: `lstat` of a link is never + `isFile()`. Leave it; it states the intent. +- **n3.** I agree with both follow-ups Darkwing names: the directory form + of `node --test` on Node 24, and the duplicate `SUPPORTED_TOOLS` and + `TOOLS` lists. +- **n4.** I own S6. Whatever B1's fix is, the launcher will call + `classify(record, "role.launch")` and won't read `launch` alone. + +## Mutants + +| Mutant | Target | Result | +|---|---|---| +| M1, M2 network narrowing picks the wider | `vars.mjs`, `resolve.mjs` | killed (2, 2) | +| M3 crossRole not narrowed by `limits.authority` | `resolve.mjs` | **survived** (B2) | +| M4 layer check off | `vars.mjs` | killed (5) | +| M5 no realpath of the token file | `credentials.mjs` | killed (1) | +| M6 gated-only action allowed in a role | `role.mjs` | killed (2) | +| M7 launcher may list itself | `business.mjs` | killed (1) | +| M8 sync bot shared with a role | `business.mjs` | killed (1) | +| M9 action in both lists | `role.mjs` | killed (1) | +| M10 project id mismatch accepted | `resolve.mjs` | killed (1) | +| M11 empty token accepted | `credentials.mjs` | killed (1) | +| M12 token owner unchecked | `credentials.mjs` | killed (1) | +| M13 contract symlink check off | `role.mjs` | survived, equivalent (n2) | +| M14 expired Vikunja only warns | `credentials.mjs` | killed (2) | +| M15 credential map need not be exact | `business.mjs` | killed (2) | +| M16 tracker without a Vikunja credential | `business.mjs` | killed (1) | +| M17 launch max ceiling off | `business.mjs` | killed (1) | +| M18 project sets vars for an undeclared instance | `resolve.mjs` | killed (2) | +| M19 unknown variable skipped | `vars.mjs` | killed (2) | +| M20 `role.launch` kept in withinRole | `resolve.mjs` | killed (1) | + +Darkwing's six mutants are a separate set; I didn't rerun them. diff --git a/agents/filbert/work/slice1-s1-review/summary.txt b/agents/filbert/work/slice1-s1-review/summary.txt new file mode 100644 index 00000000..6f39c191 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/summary.txt @@ -0,0 +1,5 @@ +business rc=0 +test-config rc=0 selftest: 24 passed, 0 failed +test-task rc=0 selftest: 98 passed, 0 failed +test-conductor rc=0 selftest: 17 passed, 0 failed +test-queue rc=0 queue suite: 27 passed, 0 failed diff --git a/agents/filbert/work/slice1-s1-review/v1-compat.txt b/agents/filbert/work/slice1-s1-review/v1-compat.txt new file mode 100644 index 00000000..35fa94c5 --- /dev/null +++ b/agents/filbert/work/slice1-s1-review/v1-compat.txt @@ -0,0 +1,17 @@ +SAME a.json base=0 new=0 MOSAIC_ROLE_TOOLS=read,bash MOSAIC_ROLE_NETWORK=none +SAME b.json base=0 new=0 MOSAIC_ROLE_TOOLS=read MOSAIC_ROLE_NETWORK=open +SAME c.json base=2 new=2 +SAME d.json base=2 new=2 +SAME e.json base=2 new=2 +SAME f.json base=2 new=2 +SAME g.json base=2 new=2 +SAME h.json base=2 new=2 +SAME i.json base=2 new=2 +SAME j.json base=2 new=2 +SAME k.json base=2 new=2 +SAME l.json base=2 new=2 +SAME M.json base=2 new=2 +SAME n.json base=2 new=2 +SAME o.json base=2 new=2 +SAME researcher.json base=0 new=0 MOSAIC_ROLE_TOOLS=read,grep,find,ls,bash MOSAIC_ROLE_NETWORK=none +SAME missing.json base=4 new=4 diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 0f4fae0b..ddcd25db 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -496,3 +496,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-05T03:31:20Z | Sage (T3 Claude Code, thread 1ef1e4f8) | S1 packet, Q1 shapes | pushed Darkwing's S1 packet (b851f83c, revs 99-100); Filbert asked to review row 36; Rocko's Q1 shapes sent to Dewey; DEFERRED: test-task.sh unguarded live call, Node 24 dir test form 2026-10-05T03:34:37Z | Sage (T3 Claude Code, thread 1ef1e4f8) | schema v3b | reran proto-v3b on Node 24 and 26 (byte match); lead decision 60 accepts v3b, the S3 poller compares against task_current, writers use one UTC stamp form; row 37 note; pushed Darkwing's 48d76de7 2026-10-05T03:37:57Z | Sage (T3 Claude Code, thread 1ef1e4f8) | questioning Jason, round 1 | Jason approved PRD 0.4 as 1.0, accepted the same-user boundary and service-token-only REQ-CRED, chose Path B; lead decision 61; PRD status 1.0; row 35 note +2026-10-05T03:40:19Z | Filbert (T3 Claude Code, thread 9cb9731e) | row 36 S1 review, round 1 | changes on #1518 (comment 26724, queue rev 103, 779b01bd): B1 launch block and role.launch can disagree, B2 crossRole allowlist narrowing untested (mutant M3 survives); suites green incl. Node 24 glob; record agents/filbert/work/slice1-s1-review/