From ace73f1e030a335faa101414ac2ab658569802b0 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Tue, 14 Jul 2026 19:45:05 -0500 Subject: [PATCH] fix(fleet): close persona resolver authority edges Co-Authored-By: Claude Opus 4.8 --- .../758-fcm-m1-002-shared-role-resolution.md | 20 +++-- .../src/commands/fleet-personas.spec.ts | 82 ++++++++++++++++++- .../mosaic/src/commands/fleet-personas.ts | 57 ++++++++----- 3 files changed, 132 insertions(+), 27 deletions(-) diff --git a/docs/scratchpads/758-fcm-m1-002-shared-role-resolution.md b/docs/scratchpads/758-fcm-m1-002-shared-role-resolution.md index d5d92d3..5c6943a 100644 --- a/docs/scratchpads/758-fcm-m1-002-shared-role-resolution.md +++ b/docs/scratchpads/758-fcm-m1-002-shared-role-resolution.md @@ -96,12 +96,22 @@ DB migration, typecheck, lint, Prettier, and `git diff --check` also passed. Cov - marker-defined identity and domain metadata are revalidated on the second read; - `LIBRARY.md` rows and incidental later markers cannot define, shadow, or advertise personas. -Final independent finding-specific rereview: **APPROVE**. The reviewer verified exact absolute and -relative dangling-ancestor traversal, cached-missing revalidation, second-read marker identity, and -async/sync/list/status behavior. The source remained at remote head `1c41adad…` during remediation; -a fresh pushed head still requires terminal-green CI and durable exact-head reviewer-of-record before -merge. +Exact-head pipeline `1819` passed for rebased head `4d990eee…`, but the independent reviewer-of-record +returned **REQUEST CHANGES** after reproducing three additional edge failures: a canonical filename could +inherit protected authority despite a conflicting explicit first marker, cached `scanned` absence could +miss an override created before baseline fallback, and inherited plain-object names such as `constructor` +could corrupt alias/authority lookup. Merge remained held. +Each failure was reproduced red-first in the persona suite (4 failing assertions), then remediated without +expanding card scope. Explicit first markers now own identity and filename fallback applies only to +markerless contracts; every second read rejects a newly introduced conflicting marker regardless of +cached classification; cached async resolution re-scans the override layer immediately before every +baseline fallback; alias and authority registries require own-property matches. Current uncommitted +evidence is persona **52/52**, focused affected suites **6 files / 143 tests**, and full Mosaic package +**50 files / 738 tests**, plus typecheck, lint, Prettier, and `git diff --check`. Independent +finding-specific rereview **APPROVED** the complete uncommitted three-file remediation after direct +adversarial reproduction of all three findings and the follow-up markerless TOCTOU. All post-commit +exact-head gates remain required. ## Risks and boundaries diff --git a/packages/mosaic/src/commands/fleet-personas.spec.ts b/packages/mosaic/src/commands/fleet-personas.spec.ts index 6fbed91..2b3eadc 100644 --- a/packages/mosaic/src/commands/fleet-personas.spec.ts +++ b/packages/mosaic/src/commands/fleet-personas.spec.ts @@ -71,7 +71,7 @@ describe('FCM class canonicalization and authority', () => { }); }); - it.each(['worker', 'analyst', 'canary', 'tess', 'ultron'])( + it.each(['worker', 'analyst', 'canary', 'tess', 'ultron', 'constructor'])( 'does not infer an alias for %s', (requested: string) => { expect(canonicalizeRoleClass(requested)).toEqual({ @@ -81,6 +81,28 @@ describe('FCM class canonicalization and authority', () => { }, ); + it('resolves inherited-property class names without granting protected authority', async () => { + await mkdir(overrideDir, { recursive: true }); + await writeFile( + join(overrideDir, 'constructor.md'), + overridePersona('constructor', 'custom'), + 'utf8', + ); + + const resolved = await resolvePersona('constructor', { rolesDir, overrideDir }); + expect(resolved).toMatchObject({ + requestedClass: 'constructor', + canonicalClass: 'constructor', + klass: 'constructor', + layer: 'override', + }); + expect(authorityForCanonicalClass('constructor')).toMatchObject({ + mayMerge: false, + mayIssueValidationCertificate: false, + mayOrchestrate: false, + }); + }); + it('canonicalizes before override lookup and lets the canonical override win', async () => { await mkdir(overrideDir, { recursive: true }); await writeFile( @@ -258,6 +280,23 @@ describe('resolvePersona — override wins', () => { ).toBe(false); }); + it('rejects a canonical filename whose explicit marker names another class', async () => { + await mkdir(overrideDir, { recursive: true }); + await writeFile(join(overrideDir, 'merge-gate.md'), overridePersona('mascot', 'fun'), 'utf8'); + await writeFile( + join(rolesDir, 'merge-gate.md'), + baselinePersona('merge-gate', 'governance'), + 'utf8', + ); + + expect(await resolvePersona('merge-gate', { rolesDir, overrideDir })).toBeNull(); + expect(resolvePersonaSync('merge-gate', { rolesDir, overrideDir })).toBeNull(); + expect((await listPersonaClasses({ rolesDir, overrideDir })).has('merge-gate')).toBe(false); + expect( + (await personaStatus({ rolesDir, overrideDir })).some(({ klass }) => klass === 'merge-gate'), + ).toBe(false); + }); + it('fails closed if a marker-defined override becomes unreadable after extraction', async () => { await mkdir(overrideDir, { recursive: true }); const overrideFile = join(overrideDir, 'engineering.md'); @@ -294,6 +333,26 @@ describe('resolvePersona — override wins', () => { expect(status.get('mascot')?.status).toBe('custom'); }); + it('fails closed if a markerless cached override gains a conflicting marker', async () => { + await mkdir(overrideDir, { recursive: true }); + const overrideFile = join(overrideDir, 'merge-gate.md'); + await writeFile(overrideFile, '# markerless merge gate\n', 'utf8'); + await writeFile( + join(rolesDir, 'merge-gate.md'), + baselinePersona('merge-gate', 'governance'), + 'utf8', + ); + const [base, over] = await Promise.all([ + extractClassesFromDir(rolesDir), + extractClassesFromDir(overrideDir), + ]); + await writeFile(overrideFile, overridePersona('mascot', 'fun'), 'utf8'); + + expect( + await resolvePersonaFrom('merge-gate', { rolesDir, overrideDir, base, over }), + ).toBeNull(); + }); + it('fails closed asynchronously when the override directory cannot be scanned', async () => { await writeFile(overrideDir, 'not a directory', 'utf8'); expect(await resolvePersona('code', { rolesDir, overrideDir })).toBeNull(); @@ -364,6 +423,27 @@ describe('resolvePersona — override wins', () => { ).toBeNull(); }); + it('revalidates a cached scanned override before baseline fallback', async () => { + await mkdir(overrideDir, { recursive: true }); + const [base, over] = await Promise.all([ + extractClassesFromDir(rolesDir), + extractClassesFromDir(overrideDir), + ]); + expect(over.scanState).toBe('scanned'); + expect(over.classes.size).toBe(0); + + await writeFile(join(overrideDir, 'engineering.md'), overridePersona('code', 'engineering')); + + const resolved = await resolvePersonaFrom('code', { + rolesDir, + overrideDir, + base, + over, + }); + expect(resolved?.layer).toBe('override'); + expect(resolved?.file).toBe(join(overrideDir, 'engineering.md')); + }); + it('falls back to baseline asynchronously and synchronously when override directory is missing', async () => { const asyncResolution = await resolvePersona('code', { rolesDir, overrideDir }); const syncResolution = resolvePersonaSync('code', { rolesDir, overrideDir }); diff --git a/packages/mosaic/src/commands/fleet-personas.ts b/packages/mosaic/src/commands/fleet-personas.ts index 2e83e5f..c3fe3fe 100644 --- a/packages/mosaic/src/commands/fleet-personas.ts +++ b/packages/mosaic/src/commands/fleet-personas.ts @@ -71,7 +71,9 @@ export interface CanonicalRoleClass { /** Canonicalize only the three explicitly approved compatibility aliases. */ export function canonicalizeRoleClass(requestedClass: string): CanonicalRoleClass { const requested = requestedClass.trim(); - const canonical = ROLE_CLASS_ALIASES[requested as keyof typeof ROLE_CLASS_ALIASES] ?? requested; + const canonical = Object.hasOwn(ROLE_CLASS_ALIASES, requested) + ? ROLE_CLASS_ALIASES[requested as keyof typeof ROLE_CLASS_ALIASES] + : requested; return Object.freeze({ requestedClass: requested, canonicalClass: canonical }); } @@ -121,7 +123,9 @@ export const ROLE_AUTHORITY_BY_CANONICAL_CLASS: Readonly { - try { - await readdir(dir); - return false; - } catch (error) { - return (await classifyScanFailure(dir, error)) === 'missing'; - } -} - function isMissingPathError(error: unknown): boolean { return ( typeof error === 'object' && @@ -314,10 +309,9 @@ function accumulateEntry(acc: DirClasses, dir: string, entry: string, text: stri const { classes, byClass } = acc; if (entry === 'LIBRARY.md') return; - // A successfully read role file is addressable by filename, including - // marker-less compatibility contracts such as planner.md. + // An explicit first marker owns identity. Filename identity is a fallback only + // for markerless compatibility contracts such as planner.md. const stem = basename(entry, '.md'); - classes.add(stem); const domainMatch = DOMAIN_MARKER.exec(text); const domain = domainMatch?.[1]; const firstMarker = text.matchAll(CLASS_MARKER).next().value as RegExpExecArray | undefined; @@ -330,8 +324,9 @@ function accumulateEntry(acc: DirClasses, dir: string, entry: string, text: stri markerDefined: true, ...(domain ? { domain } : {}), }); - } else if (!byClass.has(stem)) { - byClass.set(stem, { klass: stem, file: join(dir, entry) }); + } else { + classes.add(stem); + if (!byClass.has(stem)) byClass.set(stem, { klass: stem, file: join(dir, entry) }); } } @@ -386,6 +381,10 @@ async function readPersonaFromLayer( const byName = join(dir, `${canonicalClass}.md`); try { const content = await readFile(byName, 'utf8'); + const currentMarker = content.matchAll(CLASS_MARKER).next().value as + | RegExpExecArray + | undefined; + if (currentMarker && currentMarker[1] !== canonicalClass) return null; const dm = DOMAIN_MARKER.exec(content); return { requestedClass, @@ -405,7 +404,7 @@ async function readPersonaFromLayer( const currentMarker = content.matchAll(CLASS_MARKER).next().value as | RegExpExecArray | undefined; - if (pf.markerDefined && currentMarker?.[1] !== canonicalClass) return null; + if (currentMarker && currentMarker[1] !== canonicalClass) return null; const dm = DOMAIN_MARKER.exec(content); return { requestedClass, @@ -442,6 +441,10 @@ function readPersonaFromLayerSync( const byName = join(dir, `${canonicalClass}.md`); try { const content = readFileSync(byName, 'utf8'); + const currentMarker = content.matchAll(CLASS_MARKER).next().value as + | RegExpExecArray + | undefined; + if (currentMarker && currentMarker[1] !== canonicalClass) return null; const dm = DOMAIN_MARKER.exec(content); return { requestedClass, @@ -461,7 +464,7 @@ function readPersonaFromLayerSync( const currentMarker = content.matchAll(CLASS_MARKER).next().value as | RegExpExecArray | undefined; - if (pf.markerDefined && currentMarker?.[1] !== canonicalClass) return null; + if (currentMarker && currentMarker[1] !== canonicalClass) return null; const dm = DOMAIN_MARKER.exec(content); return { requestedClass, @@ -512,10 +515,22 @@ export async function resolvePersonaFrom( const override = await readPersonaFromLayer(requested, klass, overrideDir, over, 'override'); if (override) return override; - // An explicit canonical override that cannot be resolved/read must shadow the - // baseline fail-closed; only an absent override may fall back. + // An observed explicit override that cannot resolve/read shadows the baseline. if (overrideShadowsBaseline(over, klass)) return null; - if (over.scanState === 'missing' && !(await missingScanStillValid(overrideDir))) return null; + + // Cached scans are only snapshots. Re-scan immediately before baseline fallback + // so a newly created canonical or marker-defined override cannot be skipped. + const currentOver = await extractClassesFromDir(overrideDir); + const currentOverride = await readPersonaFromLayer( + requested, + klass, + overrideDir, + currentOver, + 'override', + ); + if (currentOverride) return currentOverride; + if (overrideShadowsBaseline(currentOver, klass)) return null; + if (currentOver.scanState === 'error') return null; return readPersonaFromLayer(requested, klass, rolesDir, base, 'baseline'); }