From 00187b4fe8ec182e54422902c07fd769d44827e8 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 18:52:07 +0100 Subject: [PATCH 1/8] test: reproduce archive purge readiness flake Co-authored-by: bobbit-ai --- tests2/core/archive-purge-async.test.ts | 54 ++++++++++++++++++++++++- 1 file changed, 53 insertions(+), 1 deletion(-) diff --git a/tests2/core/archive-purge-async.test.ts b/tests2/core/archive-purge-async.test.ts index 0232f0d4f..4e6296f1c 100644 --- a/tests2/core/archive-purge-async.test.ts +++ b/tests2/core/archive-purge-async.test.ts @@ -1,7 +1,10 @@ import assert from "node:assert/strict"; import { afterEach, describe, it, vi } from "vitest"; import { BACKGROUND_IO_CONCURRENCY } from "../../src/server/agent/bounded-async-work.ts"; -import { SessionManager } from "../../src/server/agent/session-manager.ts"; +import { + SessionManager, + type SessionPreviewPurgeOperation, +} from "../../src/server/agent/session-manager.ts"; import { createManualClock, type ManualClock } from "../harness/clock.ts"; const DAY_MS = 24 * 60 * 60 * 1_000; @@ -46,6 +49,7 @@ function makeManager(options: { clock?: ManualClock; archiveStat?: (filePath: string) => Promise<{ size: number }>; purgeAsync?: (id: string) => Promise; + previewPurgeOperation?: SessionPreviewPurgeOperation; }): { manager: SessionManager; records: Map; clock: ManualClock } { const clock = options.clock ?? createManualClock(20 * DAY_MS); const records = new Map(options.records.map(record => [record.id, record])); @@ -77,6 +81,7 @@ function makeManager(options: { clock, projectContextManager: projectContextManager as any, archiveStat: options.archiveStat, + previewPurgeOperation: options.previewPurgeOperation, }); const internal = manager as any; if (internal._statusHeartbeatTimer) { @@ -174,6 +179,53 @@ describe("asynchronous archive purge lifecycle", () => { assert.equal(purgeCalls, 1, "no stale timer callback may start cleanup after stop"); }); + it("does not assume purge readiness within a fixed number of event-loop turns", async () => { + const now = 20 * DAY_MS; + const previewCleanupStarted = deferred(); + const allowPreviewCleanup = deferred(); + const purgeStarted = deferred(); + let purgeStartedFlag = false; + const { manager, clock } = makeManager({ + records: [archivedSession("archive-delayed-readiness", now)], + clock: createManualClock(now), + previewPurgeOperation: async (_sessionId, operation) => { + previewCleanupStarted.resolve(); + await allowPreviewCleanup.promise; + return operation(); + }, + purgeAsync: async () => { + purgeStartedFlag = true; + purgeStarted.resolve(); + }, + }); + managers.push(manager); + manager.startPurgeSchedule(); + + clock.advance(DAY_MS); + await previewCleanupStarted.promise; + const releaseAfterFinitePoll = (async () => { + for (let turn = 0; turn < 1_001; turn++) { + await new Promise(resolve => setImmediate(resolve)); + } + allowPreviewCleanup.resolve(); + })(); + + try { + await waitFor( + () => purgeStartedFlag, + "archive purge readiness after controlled 1001-turn cleanup", + ); + } finally { + // Never strand the production stop barrier when the readiness assertion + // fails: release the test-owned hold, prove the purge eventually starts, + // and join it before Vitest enters afterEach. + allowPreviewCleanup.resolve(); + await releaseAfterFinitePoll; + await purgeStarted.promise; + await manager.stopPurgeSchedule(); + } + }); + it("awaits termination listeners and isolates a rejected purge listener", async () => { const now = 20 * DAY_MS; const listenerRelease = deferred(); From e53e1fdef480474b30d80bb66a62a30a05036072 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 18:53:08 +0100 Subject: [PATCH 2/8] test: reproduce preview enumeration mismatch Co-authored-by: bobbit-ai --- tests2/integration/search-preview-api.test.ts | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/tests2/integration/search-preview-api.test.ts b/tests2/integration/search-preview-api.test.ts index e4b92352f..86aab4a44 100644 --- a/tests2/integration/search-preview-api.test.ts +++ b/tests2/integration/search-preview-api.test.ts @@ -117,9 +117,8 @@ test.describe("Search/preview/archive API migrations", () => { expect(mountResponse.status).toBe(200); const mounted = await mountResponse.json() as { artifactId: string; contentHash: string }; - // Add a second byte-identical valid candidate. The production contract is - // filesystem enumeration order, so derive the expected exact winner from - // the same raw enumeration rather than assuming lexical ordering. + // Add a second byte-identical valid candidate, then deliberately make the + // streamed lookup order disagree with this separate bulk-readdir oracle. const cloneId = `clone_${randomUUID().replace(/-/g, "").slice(0, 12)}`; const cloneDir = previewArtifacts.artifactDir(sessionId, cloneId); await fs.promises.cp(previewArtifacts.artifactDir(sessionId, mounted.artifactId), cloneDir, { recursive: true }); @@ -127,18 +126,31 @@ test.describe("Search/preview/archive API migrations", () => { const cloneMetadata = JSON.parse(await fs.promises.readFile(cloneMetadataPath, "utf-8")); cloneMetadata.artifactId = cloneId; await fs.promises.writeFile(cloneMetadataPath, JSON.stringify(cloneMetadata, null, 2), "utf-8"); - const candidateOrder = (await fs.promises.readdir(previewArtifacts.artifactSessionDir(sessionId), { withFileTypes: true })) - .filter(entry => entry.isDirectory() && (entry.name === mounted.artifactId || entry.name === cloneId)) - .map(entry => entry.name); + const artifactSessionDir = previewArtifacts.artifactSessionDir(sessionId); + const candidateEntries = (await fs.promises.readdir(artifactSessionDir, { withFileTypes: true })) + .filter(entry => entry.isDirectory() && (entry.name === mounted.artifactId || entry.name === cloneId)); + const candidateOrder = candidateEntries.map(entry => entry.name); expect(candidateOrder).toHaveLength(2); + const streamedCandidateEntries = [...candidateEntries].reverse(); + expect(streamedCandidateEntries[0]?.name).not.toBe(candidateOrder[0]); const baseFs = previewMount.createPreviewAsyncFs(fs); const hashStarted = deferred(); let held = false; previewArtifacts.setPreviewArtifactFsForTesting({ ...baseFs, + opendir: async (filePath: fs.PathLike) => { + if (path.resolve(String(filePath)) !== path.resolve(artifactSessionDir)) { + return baseFs.opendir(filePath); + } + let index = 0; + return { + read: async () => streamedCandidateEntries[index++] ?? null, + close: async () => {}, + } as fs.Dir; + }, open: async (filePath: fs.PathLike, flags: "r") => { - if (!held && under(previewArtifacts.artifactSessionDir(sessionId), filePath)) { + if (!held && under(artifactSessionDir, filePath)) { held = true; hashStarted.resolve(); await releaseHash.promise; @@ -171,7 +183,10 @@ test.describe("Search/preview/archive API migrations", () => { expect(scanResponse.status).toBe(200); const snapshot = await scanResponse.json() as { artifactId?: string; contentHash?: string }; expect(snapshot.contentHash).toBe(mounted.contentHash); - expect(snapshot.artifactId).toBe(candidateOrder[0]); + expect( + snapshot.artifactId, + "deterministic invalid oracle: bulk readdir first candidate must win over reversed opendir stream", + ).toBe(candidateOrder[0]); const mutationResponse = await mutationPromise; expect(mutationResponse.status).toBe(200); const mutated = await mutationResponse.json() as { contentHash?: string }; From 02ae4a6baa000377417a12aa7a0c570df42859f2 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:03:01 +0100 Subject: [PATCH 3/8] test: assert streamed preview artifact order Co-authored-by: bobbit-ai --- tests2/core/preview-artifacts.test.ts | 67 +++++++++++++------ tests2/integration/search-preview-api.test.ts | 22 +++--- 2 files changed, 62 insertions(+), 27 deletions(-) diff --git a/tests2/core/preview-artifacts.test.ts b/tests2/core/preview-artifacts.test.ts index 5b867ec46..c840dfde1 100644 --- a/tests2/core/preview-artifacts.test.ts +++ b/tests2/core/preview-artifacts.test.ts @@ -266,6 +266,17 @@ function writeCandidate( } } +function directoryEntriesInOrder(directory: string, orderedNames: readonly string[]): fs.Dirent[] { + const entries = memoryFs.readdirSync(directory, { withFileTypes: true }); + const entriesByName = new Map(entries.map(entry => [entry.name, entry])); + return orderedNames.map((name) => { + const entry = entriesByName.get(name); + assert.ok(entry, `missing controlled directory entry ${name}`); + assert.equal(entry.isDirectory(), true, `controlled entry is not a directory: ${name}`); + return entry; + }); +} + describe("preview artifacts", () => { it("stores exact bytes, sorted POSIX files, and stable nested binary hashes", async () => { await resetSession(); @@ -321,13 +332,29 @@ describe("preview artifacts", () => { writeCandidate("ffffff", mounted, { metadata: candidateRecord("ffffff", mounted) }); writeCandidate("gggggg", mounted, { metadata: candidateRecord("gggggg", mounted) }); - const exactOrder = memoryFs.readdirSync(path.join(artifactRoot, SID), { withFileTypes: true }) - .filter(ent => ent.isDirectory() && (ent.name === "ffffff" || ent.name === "gggggg")) - .map(ent => ent.name); - const found = await findPreviewArtifactByHash(SID, mounted.contentHash); - assert.equal(found?.artifactId, exactOrder[0]); - const reused = await persistPreviewArtifact(SID, mounted); - assert.equal(reused.artifactId, exactOrder[0]); + const sessionDir = path.resolve(path.join(artifactRoot, SID)); + const streamedIds = ["aaaaaa", "bbbbbb", "cccccc", "dddddd", "eeeeee", "ffffff", "gggggg"]; + const streamedEntries = directoryEntriesInOrder(sessionDir, streamedIds); + const orderedFs: PreviewAsyncFs = { + ...baseAsyncFs, + opendir: async filePath => { + if (path.resolve(String(filePath)) !== sessionDir) return baseAsyncFs.opendir(filePath); + let index = 0; + return { + read: async () => streamedEntries[index++] ?? null, + close: async () => {}, + } as fs.Dir; + }, + }; + setPreviewArtifactFsForTesting(orderedFs); + try { + const found = await findPreviewArtifactByHash(SID, mounted.contentHash); + assert.equal(found?.artifactId, "ffffff", "lookup must skip invalid streamed candidates in order"); + const reused = await persistPreviewArtifact(SID, mounted); + assert.equal(reused.artifactId, "ffffff", "catalog reuse must retain the streamed candidate order"); + } finally { + setPreviewArtifactFsForTesting(memoryFs); + } }); it("batches metadata reads with bounded read-ahead while preserving first-valid order and failure isolation", async () => { @@ -342,12 +369,10 @@ describe("preview artifacts", () => { metadata: candidateRecord(artifactId, mounted, { contentHash: "f".repeat(64) }), }); } - const candidateSet = new Set(candidateIds); - const orderedIds = memoryFs.readdirSync(path.join(artifactRoot, SID), { withFileTypes: true }) - .filter(ent => candidateSet.has(ent.name)) - .map(ent => ent.name); - assert.equal(orderedIds.length, candidateIds.length); - const firstBatch = orderedIds.slice(0, RECOVERY_IO_CONCURRENCY); + const streamedIds = [...candidateIds].reverse(); + assert.deepEqual(streamedIds, [...candidateIds].sort().reverse(), "controlled stream must be reverse lexical"); + const streamedEntries = directoryEntriesInOrder(path.join(artifactRoot, SID), streamedIds); + const firstBatch = streamedIds.slice(0, RECOVERY_IO_CONCURRENCY); const metadataPath = (artifactId: string): string => path.join(artifactDir(SID, artifactId), "artifact.json"); // Every failure shape is isolated inside the first batch. The two exact @@ -362,7 +387,7 @@ describe("preview artifacts", () => { memoryFs.writeFileSync(metadataPath(firstBatch[4]!), JSON.stringify(candidateRecord(firstBatch[4]!, mounted))); memoryFs.writeFileSync(metadataPath(firstBatch[5]!), JSON.stringify(candidateRecord(firstBatch[5]!, mounted))); - const gates = new Map(orderedIds.map(artifactId => [artifactId, deferred()])); + const gates = new Map(streamedIds.map(artifactId => [artifactId, deferred()])); const metadataStarted: string[] = []; const metadataFinished: string[] = []; let activeMetadataReads = 0; @@ -372,14 +397,14 @@ describe("preview artifacts", () => { const heldFs: PreviewAsyncFs = { ...baseAsyncFs, opendir: async filePath => { - const directory = await baseAsyncFs.opendir(filePath); - if (path.resolve(String(filePath)) !== sessionDir) return directory; + if (path.resolve(String(filePath)) !== sessionDir) return baseAsyncFs.opendir(filePath); + let index = 0; return { read: async () => { sessionDirectoryReads++; - return directory.read(); + return streamedEntries[index++] ?? null; }, - close: () => directory.close(), + close: async () => {}, } as fs.Dir; }, lstat: async filePath => { @@ -429,7 +454,11 @@ describe("preview artifacts", () => { gates.get(firstBatch[0]!)!.resolve(); const found = await scan; - assert.equal(found?.artifactId, firstBatch[4]); + assert.equal( + found?.artifactId, + firstBatch[4], + "the earlier valid opendir index must win when later metadata finishes first", + ); assert.equal(metadataStarted.length, RECOVERY_IO_CONCURRENCY, "exact validation must not read the next batch"); assert.equal(sessionDirectoryReads, RECOVERY_IO_CONCURRENCY); assert.ok(maximumMetadataReads <= RECOVERY_IO_CONCURRENCY); diff --git a/tests2/integration/search-preview-api.test.ts b/tests2/integration/search-preview-api.test.ts index 86aab4a44..e5cc49bb6 100644 --- a/tests2/integration/search-preview-api.test.ts +++ b/tests2/integration/search-preview-api.test.ts @@ -117,8 +117,8 @@ test.describe("Search/preview/archive API migrations", () => { expect(mountResponse.status).toBe(200); const mounted = await mountResponse.json() as { artifactId: string; contentHash: string }; - // Add a second byte-identical valid candidate, then deliberately make the - // streamed lookup order disagree with this separate bulk-readdir oracle. + // Add a second byte-identical valid candidate. The injected root stream is + // explicitly reverse lexical so a hidden sort cannot satisfy this contract. const cloneId = `clone_${randomUUID().replace(/-/g, "").slice(0, 12)}`; const cloneDir = previewArtifacts.artifactDir(sessionId, cloneId); await fs.promises.cp(previewArtifacts.artifactDir(sessionId, mounted.artifactId), cloneDir, { recursive: true }); @@ -129,10 +129,16 @@ test.describe("Search/preview/archive API migrations", () => { const artifactSessionDir = previewArtifacts.artifactSessionDir(sessionId); const candidateEntries = (await fs.promises.readdir(artifactSessionDir, { withFileTypes: true })) .filter(entry => entry.isDirectory() && (entry.name === mounted.artifactId || entry.name === cloneId)); - const candidateOrder = candidateEntries.map(entry => entry.name); - expect(candidateOrder).toHaveLength(2); - const streamedCandidateEntries = [...candidateEntries].reverse(); - expect(streamedCandidateEntries[0]?.name).not.toBe(candidateOrder[0]); + expect(candidateEntries).toHaveLength(2); + const candidateEntriesById = new Map(candidateEntries.map(entry => [entry.name, entry])); + const lexicalCandidateIds = [mounted.artifactId, cloneId].sort(); + const streamedCandidateIds = [...lexicalCandidateIds].reverse(); + const streamedCandidateEntries = streamedCandidateIds.map((artifactId) => { + const entry = candidateEntriesById.get(artifactId); + expect(entry, `missing real candidate directory ${artifactId}`).toBeDefined(); + return entry!; + }); + expect(streamedCandidateIds[0]).not.toBe(lexicalCandidateIds[0]); const baseFs = previewMount.createPreviewAsyncFs(fs); const hashStarted = deferred(); @@ -185,8 +191,8 @@ test.describe("Search/preview/archive API migrations", () => { expect(snapshot.contentHash).toBe(mounted.contentHash); expect( snapshot.artifactId, - "deterministic invalid oracle: bulk readdir first candidate must win over reversed opendir stream", - ).toBe(candidateOrder[0]); + "artifact reuse must choose the first valid candidate from the controlled opendir stream", + ).toBe(streamedCandidateIds[0]); const mutationResponse = await mutationPromise; expect(mutationResponse.status).toBe(200); const mutated = await mutationResponse.json() as { contentHash?: string }; From bee07f73e0fc24682afa57959540806c4b6c56bb Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:07:18 +0100 Subject: [PATCH 4/8] Stabilize archive purge test lifecycle Co-authored-by: bobbit-ai --- tests2/core/archive-purge-async.test.ts | 139 ++++++++++++++---------- 1 file changed, 82 insertions(+), 57 deletions(-) diff --git a/tests2/core/archive-purge-async.test.ts b/tests2/core/archive-purge-async.test.ts index 4e6296f1c..4d08423b0 100644 --- a/tests2/core/archive-purge-async.test.ts +++ b/tests2/core/archive-purge-async.test.ts @@ -22,12 +22,12 @@ function deferred(): Deferred { return { promise, resolve, reject }; } -async function waitFor(predicate: () => boolean, label: string): Promise { - for (let attempt = 0; attempt < 1_000; attempt++) { - if (predicate()) return; - await new Promise(resolve => setImmediate(resolve)); - } - throw new Error(`timed out waiting for ${label}`); +const heldDeferredReleases: Array<() => void> = []; + +function heldDeferred(): Deferred { + const hold = deferred(); + heldDeferredReleases.push(() => hold.resolve()); + return hold; } function archivedSession(id: string, now: number, agentSessionFile?: string): any { @@ -94,8 +94,12 @@ function makeManager(options: { const managers: SessionManager[] = []; afterEach(async () => { - vi.restoreAllMocks(); - await Promise.all(managers.splice(0).map(manager => manager.stopPurgeSchedule())); + for (const release of heldDeferredReleases.splice(0)) release(); + try { + await Promise.all(managers.splice(0).map(manager => manager.stopPurgeSchedule())); + } finally { + vi.restoreAllMocks(); + } }); describe("asynchronous archive purge lifecycle", () => { @@ -104,7 +108,8 @@ describe("asynchronous archive purge lifecycle", () => { const count = BACKGROUND_IO_CONCURRENCY * 2 + 1; const records = Array.from({ length: count }, (_, index) => archivedSession(`archive-${index}`, now, `/transcripts/${index}.jsonl`)); - const release = deferred(); + const release = heldDeferred(); + const workersStarted = deferred(); let calls = 0; let active = 0; let maxActive = 0; @@ -115,26 +120,35 @@ describe("asynchronous archive purge lifecycle", () => { calls++; active++; maxActive = Math.max(maxActive, active); - await release.promise; - active--; - return { size: Number(/(\d+)\.jsonl$/.exec(filePath)?.[1] ?? 0) + 1 }; + if (calls === BACKGROUND_IO_CONCURRENCY) workersStarted.resolve(); + try { + await release.promise; + return { size: Number(/(\d+)\.jsonl$/.exec(filePath)?.[1] ?? 0) + 1 }; + } finally { + active--; + } }, }); managers.push(manager); let settled = false; + let stats: Awaited> | undefined; const statsPromise = manager.getExpiredArchiveStats().then(value => { settled = true; return value; }); - let unrelatedWorkRan = false; - setImmediate(() => { unrelatedWorkRan = true; }); - await waitFor(() => calls === BACKGROUND_IO_CONCURRENCY && unrelatedWorkRan, "bounded archive stat workers"); - - assert.equal(settled, false); - assert.equal(active, BACKGROUND_IO_CONCURRENCY); - assert.equal(maxActive, BACKGROUND_IO_CONCURRENCY); - assert.equal(calls, BACKGROUND_IO_CONCURRENCY, "no work above the shared ceiling starts while every worker is held"); + const unrelatedWork = deferred(); + setImmediate(() => unrelatedWork.resolve()); + try { + await Promise.all([workersStarted.promise, unrelatedWork.promise]); + assert.equal(settled, false); + assert.equal(active, BACKGROUND_IO_CONCURRENCY); + assert.equal(maxActive, BACKGROUND_IO_CONCURRENCY); + assert.equal(calls, BACKGROUND_IO_CONCURRENCY, "no work above the shared ceiling starts while every worker is held"); - release.resolve(); - const stats = await statsPromise; + release.resolve(); + stats = await statsPromise; + } finally { + release.resolve(); + await statsPromise.catch(() => undefined); + } assert.deepEqual(stats, { count, totalSizeBytes: count * (count + 1) / 2, @@ -145,44 +159,52 @@ describe("asynchronous archive purge lifecycle", () => { it("coalesces scheduled purge ticks and stopPurgeSchedule joins the active run", async () => { const now = 20 * DAY_MS; - const releasePurge = deferred(); + const releasePurge = heldDeferred(); + const purgeStarted = deferred(); let purgeCalls = 0; const { manager, clock } = makeManager({ records: [archivedSession("archive-held", now)], clock: createManualClock(now), purgeAsync: async () => { purgeCalls++; + purgeStarted.resolve(); await releasePurge.promise; }, }); managers.push(manager); manager.startPurgeSchedule(); - clock.advance(DAY_MS); - await waitFor(() => purgeCalls === 1, "first scheduled purge"); - clock.advance(DAY_MS); - await new Promise(resolve => setImmediate(resolve)); - assert.equal(purgeCalls, 1, "a second timer tick must join, not overlap, the active purge"); + let stop: Promise | undefined; + try { + clock.advance(DAY_MS); + await purgeStarted.promise; + assert.equal(purgeCalls, 1); - let stopSettled = false; - const stop = manager.stopPurgeSchedule().then(() => { stopSettled = true; }); - let unrelatedWorkRan = false; - setImmediate(() => { unrelatedWorkRan = true; }); - await waitFor(() => unrelatedWorkRan, "event-loop work during purge stop barrier"); - assert.equal(stopSettled, false, "stop must await the in-flight purge"); - assert.equal(clock.pending(), 0, "stop must cancel the future interval before joining"); + clock.advance(DAY_MS); + await new Promise(resolve => setImmediate(resolve)); + assert.equal(purgeCalls, 1, "a second timer tick must join, not overlap, the active purge"); + + let stopSettled = false; + stop = manager.stopPurgeSchedule().then(() => { stopSettled = true; }); + const unrelatedWork = deferred(); + setImmediate(() => unrelatedWork.resolve()); + await unrelatedWork.promise; + assert.equal(stopSettled, false, "stop must await the in-flight purge"); + assert.equal(clock.pending(), 0, "stop must cancel the future interval before joining"); + } finally { + releasePurge.resolve(); + await (stop ?? manager.stopPurgeSchedule()); + } - releasePurge.resolve(); - await stop; clock.advance(2 * DAY_MS); await new Promise(resolve => setImmediate(resolve)); assert.equal(purgeCalls, 1, "no stale timer callback may start cleanup after stop"); }); - it("does not assume purge readiness within a fixed number of event-loop turns", async () => { + it("waits for explicit purge readiness after more than 1,000 event-loop turns", async () => { const now = 20 * DAY_MS; const previewCleanupStarted = deferred(); - const allowPreviewCleanup = deferred(); + const allowPreviewCleanup = heldDeferred(); const purgeStarted = deferred(); let purgeStartedFlag = false; const { manager, clock } = makeManager({ @@ -203,7 +225,7 @@ describe("asynchronous archive purge lifecycle", () => { clock.advance(DAY_MS); await previewCleanupStarted.promise; - const releaseAfterFinitePoll = (async () => { + const releaseAfterControlledDelay = (async () => { for (let turn = 0; turn < 1_001; turn++) { await new Promise(resolve => setImmediate(resolve)); } @@ -211,30 +233,26 @@ describe("asynchronous archive purge lifecycle", () => { })(); try { - await waitFor( - () => purgeStartedFlag, - "archive purge readiness after controlled 1001-turn cleanup", - ); + await purgeStarted.promise; + assert.equal(purgeStartedFlag, true); } finally { - // Never strand the production stop barrier when the readiness assertion - // fails: release the test-owned hold, prove the purge eventually starts, - // and join it before Vitest enters afterEach. allowPreviewCleanup.resolve(); - await releaseAfterFinitePoll; - await purgeStarted.promise; + await releaseAfterControlledDelay; await manager.stopPurgeSchedule(); } }); it("awaits termination listeners and isolates a rejected purge listener", async () => { const now = 20 * DAY_MS; - const listenerRelease = deferred(); + const listenerRelease = heldDeferred(); + const listenerStarted = deferred(); const order: string[] = []; const { manager } = makeManager({ records: [archivedSession("archive-listener", now)] }); managers.push(manager); manager.addTerminationListener(async (_id, info) => { assert.equal(info.reason, "purged"); order.push("first-start"); + listenerStarted.resolve(); await listenerRelease.promise; order.push("first-end"); }); @@ -245,15 +263,22 @@ describe("asynchronous archive purge lifecycle", () => { const errors = vi.spyOn(console, "error").mockImplementation(() => undefined); let settled = false; + let purgeResult: boolean | undefined; const purge = manager.purgeArchivedSession("archive-listener").then(value => { settled = true; return value; }); - await waitFor(() => order.includes("first-start"), "first purge listener"); - let unrelatedWorkRan = false; - setImmediate(() => { unrelatedWorkRan = true; }); - await waitFor(() => unrelatedWorkRan, "event-loop work during listener barrier"); - assert.equal(settled, false, "purge completion must await its async listener contract"); + try { + await listenerStarted.promise; + const unrelatedWork = deferred(); + setImmediate(() => unrelatedWork.resolve()); + await unrelatedWork.promise; + assert.equal(settled, false, "purge completion must await its async listener contract"); - listenerRelease.resolve(); - assert.equal(await purge, true); + listenerRelease.resolve(); + purgeResult = await purge; + } finally { + listenerRelease.resolve(); + await purge.catch(() => undefined); + } + assert.equal(purgeResult, true); assert.deepEqual(order, ["first-start", "first-end", "second"]); assert.ok(errors.mock.calls.some(args => String(args[0]).includes("purge listener failed"))); }); From 5105df47c1f90fcb13e81bcddddce1292e21adcb Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 19:52:35 +0100 Subject: [PATCH 5/8] test: remove registry-dependent packed audit check Co-authored-by: bobbit-ai --- tests/e2e/pi-packed-consumer.spec.ts | 68 ---------------------------- 1 file changed, 68 deletions(-) diff --git a/tests/e2e/pi-packed-consumer.spec.ts b/tests/e2e/pi-packed-consumer.spec.ts index adbacdb7a..92da7eda6 100644 --- a/tests/e2e/pi-packed-consumer.spec.ts +++ b/tests/e2e/pi-packed-consumer.spec.ts @@ -20,9 +20,6 @@ const PI_PACKAGES = [ ] as const; const INSPECTED_PACKAGES = [...PI_PACKAGES, "brace-expansion", "protobufjs"]; const COMPATIBILITY_BASELINE = "0.81.1"; -const KNOWN_PROTOBUF_ADVISORY = "GHSA-j3f2-48v5-ccww"; -const KNOWN_VULNERABLE_PROTOBUF_PATH = - "node_modules/@earendil-works/pi-coding-agent/node_modules/protobufjs"; interface JsonRecord { [key: string]: unknown; @@ -39,7 +36,6 @@ interface PackedConsumerReport { selectedPiVersion?: string; pack?: unknown; tree?: unknown; - audit?: unknown; binaries?: unknown; } @@ -123,64 +119,6 @@ function expectSuccess(result: PiPackedConsumerCommandResult): void { ).toBe(0); } -function expectExactVulnerabilityCounts(audit: JsonRecord, moderate: number): void { - const metadata = asRecord(audit.metadata, "npm audit metadata"); - const counts = asRecord(metadata.vulnerabilities, "npm audit vulnerability counts"); - expect(counts).toMatchObject({ - info: 0, - low: 0, - moderate, - high: 0, - critical: 0, - total: moderate, - }); -} - -function assertKnown0811Audit(auditResult: PiPackedConsumerCommandResult, auditJson: unknown): void { - expect( - auditResult.code, - `npm audit must exit 1 for the exact known ${COMPATIBILITY_BASELINE} moderate`, - ).toBe(1); - const audit = asRecord(auditJson, "npm audit result"); - expect(audit.auditReportVersion).toBe(2); - expectExactVulnerabilityCounts(audit, 1); - - const vulnerabilities = asRecord(audit.vulnerabilities, "npm audit vulnerabilities"); - expect(Object.keys(vulnerabilities)).toEqual(["protobufjs"]); - const protobuf = asRecord(vulnerabilities.protobufjs, "protobufjs vulnerability"); - expect(protobuf).toMatchObject({ - name: "protobufjs", - severity: "moderate", - isDirect: false, - effects: [], - nodes: [KNOWN_VULNERABLE_PROTOBUF_PATH], - }); - - expect(Array.isArray(protobuf.via), "protobufjs advisory list must be an array").toBe(true); - const advisoryIds = (protobuf.via as unknown[]).map((entry, index) => { - const advisory = asRecord(entry, `protobufjs advisory ${index}`); - expect(advisory).toMatchObject({ - name: "protobufjs", - dependency: "protobufjs", - severity: "moderate", - }); - expect(typeof advisory.url).toBe("string"); - return (advisory.url as string).split("/").at(-1); - }); - expect(advisoryIds).toEqual([KNOWN_PROTOBUF_ADVISORY]); -} - -function assertCleanLaterPatchAudit(auditResult: PiPackedConsumerCommandResult, auditJson: unknown): void { - expect( - auditResult.code, - `npm audit must exit 0 after Pi advances beyond ${COMPATIBILITY_BASELINE}`, - ).toBe(0); - const audit = asRecord(auditJson, "npm audit result"); - expect(audit.auditReportVersion).toBe(2); - expectExactVulnerabilityCounts(audit, 0); - expect(asRecord(audit.vulnerabilities, "npm audit vulnerabilities")).toEqual({}); -} - async function attachReport(testInfo: TestInfo, report: PackedConsumerReport): Promise { await testInfo.attach("pi-packed-consumer-report.json", { body: Buffer.from(`${JSON.stringify(report, null, 2)}\n`), @@ -312,12 +250,6 @@ test.describe("published Bobbit package dependency security", () => { ).toBe(true); } - const auditResult = await runNpm(["audit", "--omit=dev", "--json"], consumerDir, 3 * 60_000, consumerEnv); - const auditJson = parseJson(auditResult.stdout, "npm audit"); - report.audit = { exitCode: auditResult.code, result: auditJson }; - if (isKnown0811) assertKnown0811Audit(auditResult, auditJson); - else assertCleanLaterPatchAudit(auditResult, auditJson); - const binariesModulePath = join(consumerDir, "node_modules", "bobbit", "dist", "server", "binaries.js"); const binaries = await import(pathToFileURL(binariesModulePath).href) as { expectedBinaryPackage(): string | null; From 62966719f7d86d3ec0b236521d8c7711547ba255 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 20:33:19 +0100 Subject: [PATCH 6/8] Add packed consumer release audit Co-authored-by: bobbit-ai --- .claude/skills/release/SKILL.md | 12 +- package.json | 1 + scripts/release-packed-consumer-audit.mjs | 396 ++++++++++++++++++ .../release-skill-preflight-order.test.ts | 108 ++++- 4 files changed, 510 insertions(+), 7 deletions(-) create mode 100644 scripts/release-packed-consumer-audit.mjs diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index cdc0730ee..7e41c4edb 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -97,8 +97,9 @@ Run, in this order, and **stop on any failure**: ```bash npm ci # clean install, lockfile authoritative -npm audit --omit=dev # zero high/critical in runtime deps +npm audit --omit=dev # zero vulnerabilities in root runtime deps npm run build # full build; emits declarations used by test type-checks +npm run audit:packed-consumer # zero vulnerabilities in a fresh tarball consumer npm run check # type-check server + web + tests against fresh dist npm run test:unit # fast unit suite npm run test:browser # Playwright browser journeys @@ -106,12 +107,13 @@ npm run test:e2e # API + worktree/Docker/MCP/restart E2E ``` Rules: -- **`npm audit` must show 0 vulnerabilities** (any severity, runtime deps). If it doesn't, stop and report what's flagged — do not release with known vulns. If a finding is genuinely a false positive (e.g. dev-only path), have the user explicitly acknowledge before continuing. +- **Both audits must show 0 vulnerabilities** at every severity. The packed-consumer command installs the just-built tarball under normal npm settings because a clean root audit cannot see dependency-owned shrinkwrap findings. Any finding blocks publish; there are no release exceptions. +- Registry advisory availability is deliberately release-only, not part of normal unit, browser, or E2E gates. If the advisory service or clean consumer install is unavailable, stop the release rather than skipping the packed-consumer audit. - Don't skip browser or E2E tests "because they're slow" — releases are the one place flakes bite users. -- Build must precede `check`: `tsconfig.tests2.json` follows intentional imports of emitted `dist/server/*.js` declarations, so a clean checkout cannot type-check the test graph before the build. +- Build must precede both `audit:packed-consumer` and `check`: the audit packs built output, while `tsconfig.tests2.json` follows intentional imports of emitted `dist/server/*.js` declarations. - If any test fails, the failure is the bug. Fix it or abort the release; do not retry hoping it's flaky. -Long-running steps (`build`, `test:browser`, `test:e2e`) should use `bash_bg` so output stays inspectable. +Long-running steps (`build`, `audit:packed-consumer`, `test:browser`, `test:e2e`) should use `bash_bg` so output stays inspectable. ## 3. Decide whether to bump the binary sub-packages @@ -320,7 +322,7 @@ Report to the user: - npm package URL (`https://www.npmjs.com/package/bobbit/v/`) - Whether provenance was attached - Whether binaries were republished, and which versions -- Any audit findings that were explicitly accepted +- Root and packed-consumer audit results (both must be clean) ## Rules / best practices diff --git a/package.json b/package.json index 2f0eeaf1c..2cdcd8eac 100644 --- a/package.json +++ b/package.json @@ -21,6 +21,7 @@ "build:ui": "vite build", "build:packs": "node scripts/build-market-packs.mjs", "build": "npm run build:packs && npm run build:server && npm run build:ui", + "audit:packed-consumer": "node scripts/release-packed-consumer-audit.mjs", "dev": "concurrently -n gw,ui -c blue,green \"node dist/server/cli.js --cwd . --no-ui\" \"vite\"", "dev:harness": "node src/server/harness-deps.ts && npm run build:server && concurrently -n harness,ui -c yellow,green \"node dist/server/harness.js -- --cwd . --no-ui\" \"vite\"", "dev:nord": "node src/server/harness-deps.ts && npm run build:server && node scripts/dev-nord.mjs", diff --git a/scripts/release-packed-consumer-audit.mjs b/scripts/release-packed-consumer-audit.mjs new file mode 100644 index 000000000..ab8883b8c --- /dev/null +++ b/scripts/release-packed-consumer-audit.mjs @@ -0,0 +1,396 @@ +#!/usr/bin/env node + +import { spawn, spawnSync } from "node:child_process"; +import { existsSync } from "node:fs"; +import { mkdir, mkdtemp, rm, stat, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { dirname, isAbsolute, join, relative, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); +const AUDIT_SEVERITIES = ["info", "low", "moderate", "high", "critical"]; +const MAX_OUTPUT_BYTES = 32 * 1024 * 1024; + +function isRecord(value) { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +function excerpt(value, limit = 8_000) { + const text = String(value ?? "").trim(); + if (text.length <= limit) return text; + return `[...${text.length - limit} characters omitted...]\n${text.slice(-limit)}`; +} + +function displayCommand(command, args) { + return [command, ...args] + .map(value => /\s/.test(value) ? JSON.stringify(value) : value) + .join(" "); +} + +function terminateProcessTree(child) { + if (!child.pid) return; + if (process.platform === "win32") { + spawnSync("taskkill", ["/pid", String(child.pid), "/T", "/F"], { + stdio: "ignore", + windowsHide: true, + }); + return; + } + try { + process.kill(-child.pid, "SIGTERM"); + } catch { + child.kill("SIGTERM"); + } +} + +function runCommand(command, args, { cwd, env = process.env, timeoutMs }) { + const rendered = displayCommand(command, args); + return new Promise((resolvePromise, reject) => { + const child = spawn(command, args, { + cwd, + env, + windowsHide: true, + detached: process.platform !== "win32", + stdio: ["ignore", "pipe", "pipe"], + }); + const stdout = []; + const stderr = []; + let outputBytes = 0; + let terminalError; + let settled = false; + + const failAndTerminate = message => { + if (terminalError) return; + terminalError = new Error(message); + terminateProcessTree(child); + }; + const collect = (target, chunk) => { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + outputBytes += buffer.byteLength; + if (outputBytes > MAX_OUTPUT_BYTES) { + failAndTerminate(`${rendered} exceeded the ${MAX_OUTPUT_BYTES}-byte output limit`); + return; + } + target.push(buffer); + }; + + child.stdout?.on("data", chunk => collect(stdout, chunk)); + child.stderr?.on("data", chunk => collect(stderr, chunk)); + + const timer = setTimeout(() => { + failAndTerminate(`${rendered} timed out after ${timeoutMs}ms`); + }, timeoutMs); + + child.once("error", error => { + if (settled) return; + settled = true; + clearTimeout(timer); + reject(new Error(`Failed to spawn ${rendered}: ${error.message}`, { cause: error })); + }); + child.once("close", (code, signal) => { + if (settled) return; + settled = true; + clearTimeout(timer); + const stdoutText = Buffer.concat(stdout).toString("utf8"); + const stderrText = Buffer.concat(stderr).toString("utf8"); + if (terminalError) { + reject(new Error( + `${terminalError.message}\nstdout:\n${excerpt(stdoutText)}\nstderr:\n${excerpt(stderrText)}`, + { cause: terminalError }, + )); + return; + } + if (signal || code === null) { + reject(new Error(`${rendered} terminated without an exit code (signal: ${signal ?? "unknown"})`)); + return; + } + resolvePromise({ code, stdout: stdoutText, stderr: stderrText, rendered }); + }); + }); +} + +function npmInvocation(args) { + const npmExecPath = process.env.npm_execpath; + if (npmExecPath && existsSync(npmExecPath)) { + return { command: process.execPath, args: [npmExecPath, ...args] }; + } + if (process.platform === "win32") { + const npmCli = join(dirname(process.execPath), "node_modules", "npm", "bin", "npm-cli.js"); + if (existsSync(npmCli)) return { command: process.execPath, args: [npmCli, ...args] }; + throw new Error("Cannot locate npm without a shell. Run this audit through `npm run audit:packed-consumer`."); + } + return { command: "npm", args }; +} + +function runNpm(args, options) { + const invocation = npmInvocation(args); + return runCommand(invocation.command, invocation.args, options); +} + +function normalConsumerNpmEnv(cwd) { + const env = { ...process.env }; + const projectScopedKeys = new Set([ + "npm_config_local_prefix", + "npm_config_package_lock", + "npm_config_shrinkwrap", + "npm_config_workspace", + "npm_config_workspaces", + "npm_config_include_workspace_root", + "npm_config_ignore_scripts", + "npm_config_omit", + "npm_config_include", + "npm_config_optional", + "npm_config_audit_level", + "npm_config_dry_run", + ]); + for (const key of Object.keys(env)) { + const lower = key.toLowerCase(); + if (projectScopedKeys.has(lower) || lower.startsWith("npm_package_") || lower.startsWith("npm_lifecycle_")) { + delete env[key]; + } + } + delete env.INIT_CWD; + delete env.init_cwd; + // Dependency lifecycle scripts should see the clean consumer as their initiator. + env.INIT_CWD = cwd; + return env; +} + +function requireSuccess(label, result) { + if (result.code === 0) return; + throw new Error( + `${label} failed with exit code ${result.code}: ${result.rendered}` + + `\nstdout:\n${excerpt(result.stdout)}\nstderr:\n${excerpt(result.stderr)}`, + ); +} + +export function parseAuditJson(stdout) { + if (typeof stdout !== "string" || stdout.trim() === "") { + throw new Error("npm audit emitted no JSON on stdout"); + } + let parsed; + try { + parsed = JSON.parse(stdout); + } catch (error) { + throw new Error(`npm audit emitted malformed JSON: ${error.message}\nstdout:\n${excerpt(stdout)}`, { cause: error }); + } + if (!isRecord(parsed)) throw new Error("npm audit JSON root must be an object"); + return parsed; +} + +function printable(value, fallback = "unknown") { + if (typeof value === "string" && value.trim()) return value.trim(); + if (typeof value === "number" || typeof value === "boolean") return String(value); + return fallback; +} + +export function formatAuditFindings(report) { + const lines = []; + if (report.error !== undefined) { + if (isRecord(report.error)) { + lines.push(`npm audit error: ${printable(report.error.summary, printable(report.error.code))}`); + if (typeof report.error.detail === "string" && report.error.detail.trim()) { + lines.push(` ${report.error.detail.trim()}`); + } + } else { + lines.push(`npm audit returned a malformed error: ${JSON.stringify(report.error)}`); + } + } + if (!isRecord(report.vulnerabilities)) return lines; + + for (const [packageName, rawFinding] of Object.entries(report.vulnerabilities)) { + if (!isRecord(rawFinding)) { + lines.push(`- ${packageName}: malformed vulnerability entry ${JSON.stringify(rawFinding)}`); + continue; + } + const nodes = Array.isArray(rawFinding.nodes) + ? rawFinding.nodes.map(node => String(node)).join(", ") + : "unknown path"; + lines.push( + `- ${packageName}: severity=${printable(rawFinding.severity)}, range=${printable(rawFinding.range)}, paths=${nodes}`, + ); + if (!Array.isArray(rawFinding.via)) continue; + for (const rawAdvisory of rawFinding.via) { + if (typeof rawAdvisory === "string") { + lines.push(` - via dependency ${rawAdvisory}`); + continue; + } + if (!isRecord(rawAdvisory)) { + lines.push(` - malformed advisory ${JSON.stringify(rawAdvisory)}`); + continue; + } + const advisoryId = printable(rawAdvisory.source, printable(rawAdvisory.name, "unidentified advisory")); + const title = printable(rawAdvisory.title, "untitled advisory"); + const url = printable(rawAdvisory.url, "no advisory URL"); + lines.push( + ` - ${advisoryId}: ${title}; severity=${printable(rawAdvisory.severity)}, range=${printable(rawAdvisory.range)}; ${url}`, + ); + } + } + return lines; +} + +export function evaluatePackedConsumerAudit(report, exitCode) { + const diagnostics = []; + const counts = {}; + if (!Number.isInteger(report.auditReportVersion) || report.auditReportVersion < 1) { + diagnostics.push(`npm audit reported an invalid schema version: ${JSON.stringify(report.auditReportVersion)}`); + } + const metadata = isRecord(report.metadata) ? report.metadata : undefined; + const vulnerabilityCounts = metadata && isRecord(metadata.vulnerabilities) + ? metadata.vulnerabilities + : undefined; + + if (!vulnerabilityCounts) { + diagnostics.push("npm audit JSON is missing metadata.vulnerabilities"); + } else { + for (const severity of AUDIT_SEVERITIES) { + const count = vulnerabilityCounts[severity]; + counts[severity] = count; + if (!Number.isInteger(count) || count < 0) { + diagnostics.push(`npm audit reported an invalid ${severity} vulnerability count: ${JSON.stringify(count)}`); + } else if (count !== 0) { + diagnostics.push(`npm audit reported ${count} ${severity} vulnerabilit${count === 1 ? "y" : "ies"}`); + } + } + const total = vulnerabilityCounts.total; + counts.total = total; + if (!Number.isInteger(total) || total < 0) { + diagnostics.push(`npm audit reported an invalid total vulnerability count: ${JSON.stringify(total)}`); + } else if (total !== 0) { + diagnostics.push(`npm audit reported ${total} total vulnerabilities`); + } + } + + const vulnerabilityEntries = isRecord(report.vulnerabilities) + ? Object.keys(report.vulnerabilities) + : undefined; + if (!vulnerabilityEntries) { + diagnostics.push("npm audit JSON is missing the vulnerabilities object"); + } + if ((vulnerabilityEntries && vulnerabilityEntries.length > 0) || report.error !== undefined) { + diagnostics.push(...formatAuditFindings(report)); + } + if (!Number.isInteger(exitCode) || exitCode !== 0) { + diagnostics.push(`npm audit exited with code ${JSON.stringify(exitCode)}; an audit-clean consumer must exit 0`); + } + + return { clean: diagnostics.length === 0, counts, diagnostics }; +} + +function parsePackedTarball(stdout, packDir) { + let parsed; + try { + parsed = JSON.parse(stdout); + } catch (error) { + throw new Error(`npm pack emitted malformed JSON: ${error.message}\nstdout:\n${excerpt(stdout)}`, { cause: error }); + } + if (!Array.isArray(parsed) || parsed.length !== 1 || !isRecord(parsed[0]) || typeof parsed[0].filename !== "string") { + throw new Error(`npm pack must report one tarball with a filename; received:\n${excerpt(stdout)}`); + } + const tarballPath = resolve(packDir, parsed[0].filename); + const fromPackDir = relative(packDir, tarballPath); + if (fromPackDir === "" || fromPackDir.startsWith("..") || isAbsolute(fromPackDir)) { + throw new Error(`npm pack reported a filename outside its destination: ${JSON.stringify(parsed[0].filename)}`); + } + return tarballPath; +} + +async function performPackedConsumerAudit(tempRoot) { + const packDir = join(tempRoot, "pack"); + const consumerDir = join(tempRoot, "consumer"); + await mkdir(packDir, { recursive: true }); + await mkdir(consumerDir, { recursive: true }); + await writeFile(join(consumerDir, "package.json"), `${JSON.stringify({ + name: "bobbit-release-audit-consumer", + version: "1.0.0", + private: true, + }, null, 2)}\n`); + + console.log("[audit:packed-consumer] Packing the built Bobbit package..."); + const packed = await runNpm(["pack", "--json", "--pack-destination", packDir], { + cwd: REPO_ROOT, + timeoutMs: 3 * 60_000, + }); + requireSuccess("npm pack", packed); + const tarballPath = parsePackedTarball(packed.stdout, packDir); + await stat(tarballPath); + + const consumerEnv = normalConsumerNpmEnv(consumerDir); + const lockConfig = await runNpm(["config", "get", "package-lock"], { + cwd: consumerDir, + env: consumerEnv, + timeoutMs: 30_000, + }); + requireSuccess("npm config get package-lock", lockConfig); + if (lockConfig.stdout.trim() !== "true") { + throw new Error( + "The clean consumer is not using npm's normal package-lock=true setting. " + + `Received ${JSON.stringify(lockConfig.stdout.trim())}; check user npm configuration.`, + ); + } + + console.log("[audit:packed-consumer] Installing the tarball into a clean private consumer..."); + const installed = await runNpm(["install", tarballPath], { + cwd: consumerDir, + env: consumerEnv, + timeoutMs: 10 * 60_000, + }); + requireSuccess("clean consumer install", installed); + await stat(join(consumerDir, "package-lock.json")); + + console.log("[audit:packed-consumer] Querying the registry advisory service..."); + const audited = await runNpm(["audit", "--omit=dev", "--json"], { + cwd: consumerDir, + env: consumerEnv, + timeoutMs: 3 * 60_000, + }); + // npm intentionally exits nonzero for vulnerability findings. Always parse + // stdout first so those exits retain package, path, and advisory evidence. + const report = parseAuditJson(audited.stdout); + const evaluation = evaluatePackedConsumerAudit(report, audited.code); + if (!evaluation.clean) { + const stderr = audited.stderr.trim() ? `\nnpm audit stderr:\n${excerpt(audited.stderr)}` : ""; + throw new Error( + "Packed consumer audit is not clean; publishing is blocked.\n" + + evaluation.diagnostics.join("\n") + + stderr, + ); + } + console.log("[audit:packed-consumer] PASS: the freshly installed consumer reports zero runtime vulnerabilities."); +} + +export async function runPackedConsumerAudit() { + const tempRoot = await mkdtemp(join(tmpdir(), "bobbit-release-packed-audit-")); + let operationError; + try { + await performPackedConsumerAudit(tempRoot); + } catch (error) { + operationError = error; + } + + let cleanupError; + try { + await rm(tempRoot, { recursive: true, force: true, maxRetries: 5, retryDelay: 200 }); + } catch (error) { + cleanupError = error; + } + if (operationError && cleanupError) { + throw new AggregateError( + [operationError, cleanupError], + `Packed consumer audit failed: ${String(operationError)}\nTemporary cleanup also failed for ${tempRoot}: ${String(cleanupError)}`, + ); + } + if (operationError) throw operationError; + if (cleanupError) { + throw new Error(`Packed consumer audit passed, but temporary cleanup failed for ${tempRoot}`, { cause: cleanupError }); + } +} + +const invokedPath = process.argv[1] ? resolve(process.argv[1]) : ""; +if (invokedPath === fileURLToPath(import.meta.url)) { + runPackedConsumerAudit().catch(error => { + console.error(`[audit:packed-consumer] FAIL: ${error instanceof Error ? error.stack ?? error.message : String(error)}`); + process.exitCode = 1; + }); +} diff --git a/tests2/core/release-skill-preflight-order.test.ts b/tests2/core/release-skill-preflight-order.test.ts index 60c325270..533879940 100644 --- a/tests2/core/release-skill-preflight-order.test.ts +++ b/tests2/core/release-skill-preflight-order.test.ts @@ -2,10 +2,26 @@ import assert from "node:assert/strict"; import { readFileSync } from "node:fs"; import { resolve } from "node:path"; import { describe, it } from "vitest"; +import { + evaluatePackedConsumerAudit, + parseAuditJson, +} from "../../scripts/release-packed-consumer-audit.mjs"; const skill = readFileSync(resolve(process.cwd(), ".claude/skills/release/SKILL.md"), "utf8"); +const packageJson = JSON.parse(readFileSync(resolve(process.cwd(), "package.json"), "utf8")) as { + scripts?: Record; +}; const preflight = skill.match(/## 2\. Pre-flight quality gates[\s\S]*?```bash\n([\s\S]*?)\n```/)?.[1]; +const zeroCounts = { + info: 0, + low: 0, + moderate: 0, + high: 0, + critical: 0, + total: 0, +}; + function position(command: string): number { assert.ok(preflight, "release skill must contain a fenced pre-flight command block"); const index = preflight.indexOf(command); @@ -14,11 +30,99 @@ function position(command: string): number { } describe("release skill pre-flight order", () => { - it("builds declarations before type-checking the test graph", () => { + it("audits the built tarball consumer before type-checking and tests", () => { + assert.equal( + packageJson.scripts?.["audit:packed-consumer"], + "node scripts/release-packed-consumer-audit.mjs", + ); assert.ok(position("npm ci") < position("npm audit --omit=dev")); assert.ok(position("npm audit --omit=dev") < position("npm run build")); - assert.ok(position("npm run build") < position("npm run check")); + assert.ok(position("npm run build") < position("npm run audit:packed-consumer")); + assert.ok(position("npm run audit:packed-consumer") < position("npm run check")); assert.ok(position("npm run check") < position("npm run test:unit")); assert.ok(position("npm run test:unit") < position("npm run test:e2e")); }); + + it("keeps mutable advisory availability release-only and blocks every finding", () => { + assert.match(skill, /Registry advisory availability is deliberately release-only/); + assert.match(skill, /Any finding blocks publish; there are no release exceptions/); + }); +}); + +describe("packed-consumer audit decision", () => { + it("accepts only a zero exit with explicit zero counts at every severity", () => { + const report = parseAuditJson(JSON.stringify({ + auditReportVersion: 2, + vulnerabilities: {}, + metadata: { vulnerabilities: zeroCounts }, + })); + + assert.deepEqual(evaluatePackedConsumerAudit(report, 0), { + clean: true, + counts: zeroCounts, + diagnostics: [], + }); + }); + + it("retains actionable package, path, and advisory details from a vulnerability exit", () => { + const report = parseAuditJson(JSON.stringify({ + auditReportVersion: 2, + vulnerabilities: { + protobufjs: { + name: "protobufjs", + severity: "moderate", + range: "<=7.6.4", + nodes: ["node_modules/@earendil-works/pi-coding-agent/node_modules/protobufjs"], + via: [{ + source: 1109682, + title: "Prototype pollution in protobufjs", + severity: "moderate", + range: "<=7.6.4", + url: "https://github.com/advisories/GHSA-j3f2-48v5-ccww", + }], + }, + }, + metadata: { + vulnerabilities: { ...zeroCounts, moderate: 1, total: 1 }, + }, + })); + + const evaluation = evaluatePackedConsumerAudit(report, 1); + assert.equal(evaluation.clean, false); + const diagnostics = evaluation.diagnostics.join("\n"); + assert.match(diagnostics, /1 moderate vulnerability/); + assert.match(diagnostics, /protobufjs/); + assert.match(diagnostics, /pi-coding-agent/); + assert.match(diagnostics, /GHSA-j3f2-48v5-ccww/); + assert.match(diagnostics, /exited with code 1/); + }); + + it("fails closed on malformed counts, inconsistent entries, and nonzero clean exits", () => { + const missingSeverity = evaluatePackedConsumerAudit({ + vulnerabilities: {}, + metadata: { vulnerabilities: { ...zeroCounts, critical: undefined } }, + }, 0); + assert.equal(missingSeverity.clean, false); + assert.match(missingSeverity.diagnostics.join("\n"), /invalid critical vulnerability count/); + + const hiddenFinding = evaluatePackedConsumerAudit({ + vulnerabilities: { unexpected: { severity: "low", via: [], nodes: [] } }, + metadata: { vulnerabilities: zeroCounts }, + }, 0); + assert.equal(hiddenFinding.clean, false); + assert.match(hiddenFinding.diagnostics.join("\n"), /unexpected: severity=low/); + + const failedCleanAudit = evaluatePackedConsumerAudit({ + vulnerabilities: {}, + metadata: { vulnerabilities: zeroCounts }, + }, 2); + assert.equal(failedCleanAudit.clean, false); + assert.match(failedCleanAudit.diagnostics.join("\n"), /exited with code 2/); + }); + + it("rejects absent and malformed npm audit JSON", () => { + assert.throws(() => parseAuditJson(""), /emitted no JSON/); + assert.throws(() => parseAuditJson("npm error"), /malformed JSON/); + assert.throws(() => parseAuditJson("[]"), /root must be an object/); + }); }); From c1689f741050bb02d182b8180f65512c500e8958 Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:18:49 +0100 Subject: [PATCH 7/8] Harden release consumer audit isolation Co-authored-by: bobbit-ai --- scripts/release-packed-consumer-audit.mjs | 161 ++++++++++++---- .../release-skill-preflight-order.test.ts | 179 +++++++++++++++++- 2 files changed, 300 insertions(+), 40 deletions(-) diff --git a/scripts/release-packed-consumer-audit.mjs b/scripts/release-packed-consumer-audit.mjs index ab8883b8c..9a8799935 100644 --- a/scripts/release-packed-consumer-audit.mjs +++ b/scripts/release-packed-consumer-audit.mjs @@ -10,6 +10,35 @@ import { fileURLToPath } from "node:url"; const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), ".."); const AUDIT_SEVERITIES = ["info", "low", "moderate", "high", "critical"]; const MAX_OUTPUT_BYTES = 32 * 1024 * 1024; +const PUBLIC_NPM_REGISTRY = "https://registry.npmjs.org/"; +const CHILD_ENV_PASSTHROUGH = [ + // Executable lookup and Windows process startup essentials. + "PATH", + "SystemRoot", + "WINDIR", + "COMSPEC", + "PATHEXT", + // Locale-only process settings. + "LANG", + "LC_ALL", + "LC_CTYPE", + "TZ", + // Public-network routing and trust settings. Auth and registry settings are + // deliberately not inherited; the public registry is pinned below. + "HTTP_PROXY", + "HTTPS_PROXY", + "NO_PROXY", + "ALL_PROXY", + "NODE_EXTRA_CA_CERTS", + "SSL_CERT_FILE", + "SSL_CERT_DIR", + "NPM_CONFIG_PROXY", + "NPM_CONFIG_HTTPS_PROXY", + "NPM_CONFIG_NOPROXY", + "NPM_CONFIG_STRICT_SSL", + "NPM_CONFIG_CAFILE", + "NPM_CONFIG_CA", +]; function isRecord(value) { return value !== null && typeof value === "object" && !Array.isArray(value); @@ -43,7 +72,10 @@ function terminateProcessTree(child) { } } -function runCommand(command, args, { cwd, env = process.env, timeoutMs }) { +function runCommand(command, args, { cwd, env, timeoutMs }) { + if (!env || typeof env !== "object") { + throw new Error(`Refusing to spawn ${command} without an explicit restricted environment`); + } const rendered = displayCommand(command, args); return new Promise((resolvePromise, reject) => { const child = spawn(command, args, { @@ -127,35 +159,85 @@ function runNpm(args, options) { return runCommand(invocation.command, invocation.args, options); } -function normalConsumerNpmEnv(cwd) { - const env = { ...process.env }; - const projectScopedKeys = new Set([ - "npm_config_local_prefix", - "npm_config_package_lock", - "npm_config_shrinkwrap", - "npm_config_workspace", - "npm_config_workspaces", - "npm_config_include_workspace_root", - "npm_config_ignore_scripts", - "npm_config_omit", - "npm_config_include", - "npm_config_optional", - "npm_config_audit_level", - "npm_config_dry_run", - ]); - for (const key of Object.keys(env)) { - const lower = key.toLowerCase(); - if (projectScopedKeys.has(lower) || lower.startsWith("npm_package_") || lower.startsWith("npm_lifecycle_")) { - delete env[key]; - } +function inheritedEnvValue(sourceEnv, expectedKey) { + const expected = expectedKey.toLowerCase(); + for (const [key, value] of Object.entries(sourceEnv)) { + if (key.toLowerCase() === expected && typeof value === "string" && value !== "") return value; } - delete env.INIT_CWD; - delete env.init_cwd; - // Dependency lifecycle scripts should see the clean consumer as their initiator. - env.INIT_CWD = cwd; + return undefined; +} + +export function buildRestrictedNpmEnv(sourceEnv, paths) { + const env = {}; + for (const key of CHILD_ENV_PASSTHROUGH) { + const value = inheritedEnvValue(sourceEnv, key); + if (value !== undefined) env[key] = value; + } + + Object.assign(env, { + HOME: paths.homeDir, + USERPROFILE: paths.homeDir, + APPDATA: paths.appDataDir, + LOCALAPPDATA: paths.localAppDataDir, + XDG_CONFIG_HOME: paths.xdgConfigDir, + XDG_CACHE_HOME: paths.cacheDir, + TMPDIR: paths.tempDir, + TMP: paths.tempDir, + TEMP: paths.tempDir, + npm_config_userconfig: paths.userConfigPath, + npm_config_globalconfig: paths.globalConfigPath, + npm_config_cache: paths.cacheDir, + npm_config_registry: PUBLIC_NPM_REGISTRY, + npm_config_ignore_scripts: "true", + npm_config_update_notifier: "false", + npm_config_fund: "false", + }); return env; } +async function prepareRestrictedNpmEnv(tempRoot) { + const homeDir = join(tempRoot, "home"); + const cacheDir = join(tempRoot, "cache"); + const tempDir = join(tempRoot, "tmp"); + const configDir = join(tempRoot, "config"); + const appDataDir = join(homeDir, "AppData", "Roaming"); + const localAppDataDir = join(homeDir, "AppData", "Local"); + const xdgConfigDir = join(homeDir, ".config"); + const userConfigPath = join(configDir, "user.npmrc"); + const globalConfigPath = join(configDir, "global.npmrc"); + await Promise.all([ + mkdir(homeDir, { recursive: true }), + mkdir(cacheDir, { recursive: true }), + mkdir(tempDir, { recursive: true }), + mkdir(configDir, { recursive: true }), + mkdir(appDataDir, { recursive: true }), + mkdir(localAppDataDir, { recursive: true }), + mkdir(xdgConfigDir, { recursive: true }), + ]); + await Promise.all([ + writeFile(userConfigPath, "\n", { mode: 0o600 }), + writeFile(globalConfigPath, "\n", { mode: 0o600 }), + ]); + return buildRestrictedNpmEnv(process.env, { + homeDir, + cacheDir, + tempDir, + appDataDir, + localAppDataDir, + xdgConfigDir, + userConfigPath, + globalConfigPath, + }); +} + +export function packedConsumerPackArgs(packDir) { + return ["pack", "--ignore-scripts", "--json", "--pack-destination", packDir, REPO_ROOT]; +} + +export function packedConsumerInstallArgs(tarballPath) { + return ["install", "--ignore-scripts", tarballPath]; +} + function requireSuccess(label, result) { if (result.code === 0) return; throw new Error( @@ -296,7 +378,7 @@ function parsePackedTarball(stdout, packDir) { return tarballPath; } -async function performPackedConsumerAudit(tempRoot) { +async function performPackedConsumerAudit(tempRoot, npmRunner) { const packDir = join(tempRoot, "pack"); const consumerDir = join(tempRoot, "consumer"); await mkdir(packDir, { recursive: true }); @@ -306,20 +388,23 @@ async function performPackedConsumerAudit(tempRoot) { version: "1.0.0", private: true, }, null, 2)}\n`); + const restrictedNpmEnv = await prepareRestrictedNpmEnv(tempRoot); console.log("[audit:packed-consumer] Packing the built Bobbit package..."); - const packed = await runNpm(["pack", "--json", "--pack-destination", packDir], { - cwd: REPO_ROOT, + const packed = await npmRunner(packedConsumerPackArgs(packDir), { + // Invoking from the empty temporary root prevents repository .npmrc + // settings from becoming project configuration for this npm process. + cwd: tempRoot, + env: restrictedNpmEnv, timeoutMs: 3 * 60_000, }); requireSuccess("npm pack", packed); const tarballPath = parsePackedTarball(packed.stdout, packDir); await stat(tarballPath); - const consumerEnv = normalConsumerNpmEnv(consumerDir); - const lockConfig = await runNpm(["config", "get", "package-lock"], { + const lockConfig = await npmRunner(["config", "get", "package-lock"], { cwd: consumerDir, - env: consumerEnv, + env: restrictedNpmEnv, timeoutMs: 30_000, }); requireSuccess("npm config get package-lock", lockConfig); @@ -331,18 +416,18 @@ async function performPackedConsumerAudit(tempRoot) { } console.log("[audit:packed-consumer] Installing the tarball into a clean private consumer..."); - const installed = await runNpm(["install", tarballPath], { + const installed = await npmRunner(packedConsumerInstallArgs(tarballPath), { cwd: consumerDir, - env: consumerEnv, + env: restrictedNpmEnv, timeoutMs: 10 * 60_000, }); requireSuccess("clean consumer install", installed); await stat(join(consumerDir, "package-lock.json")); console.log("[audit:packed-consumer] Querying the registry advisory service..."); - const audited = await runNpm(["audit", "--omit=dev", "--json"], { + const audited = await npmRunner(["audit", "--omit=dev", "--json"], { cwd: consumerDir, - env: consumerEnv, + env: restrictedNpmEnv, timeoutMs: 3 * 60_000, }); // npm intentionally exits nonzero for vulnerability findings. Always parse @@ -360,11 +445,11 @@ async function performPackedConsumerAudit(tempRoot) { console.log("[audit:packed-consumer] PASS: the freshly installed consumer reports zero runtime vulnerabilities."); } -export async function runPackedConsumerAudit() { +export async function runPackedConsumerAudit({ npmRunner = runNpm } = {}) { const tempRoot = await mkdtemp(join(tmpdir(), "bobbit-release-packed-audit-")); let operationError; try { - await performPackedConsumerAudit(tempRoot); + await performPackedConsumerAudit(tempRoot, npmRunner); } catch (error) { operationError = error; } diff --git a/tests2/core/release-skill-preflight-order.test.ts b/tests2/core/release-skill-preflight-order.test.ts index 533879940..1d80923a8 100644 --- a/tests2/core/release-skill-preflight-order.test.ts +++ b/tests2/core/release-skill-preflight-order.test.ts @@ -1,10 +1,14 @@ import assert from "node:assert/strict"; -import { readFileSync } from "node:fs"; -import { resolve } from "node:path"; +import { readFileSync, writeFileSync } from "node:fs"; +import { join, resolve } from "node:path"; import { describe, it } from "vitest"; import { + buildRestrictedNpmEnv, evaluatePackedConsumerAudit, + packedConsumerInstallArgs, + packedConsumerPackArgs, parseAuditJson, + runPackedConsumerAudit, } from "../../scripts/release-packed-consumer-audit.mjs"; const skill = readFileSync(resolve(process.cwd(), ".claude/skills/release/SKILL.md"), "utf8"); @@ -49,6 +53,177 @@ describe("release skill pre-flight order", () => { }); }); +describe("packed-consumer audit subprocess isolation", () => { + it("disables lifecycle scripts for both pack and consumer installation", () => { + const packArgs = packedConsumerPackArgs("isolated-pack-dir"); + assert.equal(packArgs[0], "pack"); + assert.equal(packArgs.filter((arg: string) => arg === "--ignore-scripts").length, 1); + + assert.deepEqual(packedConsumerInstallArgs("bobbit.tgz"), [ + "install", + "--ignore-scripts", + "bobbit.tgz", + ]); + }); + + it("wires the restricted environment into every npm child without contacting the registry", async () => { + const secretEnv = { + NPM_TOKEN: "publish-secret", + NODE_AUTH_TOKEN: "node-auth-secret", + _auth: "legacy-auth-secret", + _authToken: "legacy-token-secret", + npm_config__auth: "config-auth-secret", + npm_config__authToken: "config-token-secret", + "npm_config_//registry.npmjs.org/:_authToken": "scoped-token-secret", + npm_config_userconfig: resolve("credentials/npmrc"), + npm_config_globalconfig: resolve("credentials/global-npmrc"), + npm_config_registry: "https://private-registry.example.invalid/", + npm_config_always_auth: "true", + npm_config_otp: "123456", + GITHUB_TOKEN: "github-secret", + }; + const previousEnv = new Map(Object.keys(secretEnv).map(key => [key, process.env[key]])); + const calls: Array<{ + args: string[]; + cwd: string; + env: Record; + }> = []; + const npmRunner = async ( + args: string[], + options: { cwd: string; env: Record }, + ) => { + assert.equal(readFileSync(options.env.npm_config_userconfig, "utf8"), "\n"); + assert.equal(readFileSync(options.env.npm_config_globalconfig, "utf8"), "\n"); + calls.push({ args, cwd: options.cwd, env: { ...options.env } }); + const result = { code: 0, stdout: "", stderr: "", rendered: `npm ${args.join(" ")}` }; + if (args[0] === "pack") { + const packDir = args[args.indexOf("--pack-destination") + 1]; + writeFileSync(join(packDir, "bobbit-test.tgz"), "fake tarball"); + result.stdout = JSON.stringify([{ filename: "bobbit-test.tgz" }]); + } else if (args[0] === "config") { + result.stdout = "true\n"; + } else if (args[0] === "install") { + writeFileSync(join(options.cwd, "package-lock.json"), "{}\n"); + } else if (args[0] === "audit") { + result.stdout = JSON.stringify({ + auditReportVersion: 2, + vulnerabilities: {}, + metadata: { vulnerabilities: zeroCounts }, + }); + } else { + assert.fail(`unexpected npm command: ${args[0]}`); + } + return result; + }; + + try { + Object.assign(process.env, secretEnv); + await runPackedConsumerAudit({ npmRunner }); + + assert.deepEqual(calls.map(call => call.args[0]), ["pack", "config", "install", "audit"]); + assert.notEqual(calls[0].cwd, resolve(process.cwd()), "pack must not inherit repository project config"); + for (const call of calls) { + const childKeys = new Set(Object.keys(call.env).map(key => key.toLowerCase())); + for (const forbiddenKey of Object.keys(secretEnv).map(key => key.toLowerCase())) { + if (["npm_config_userconfig", "npm_config_globalconfig", "npm_config_registry"].includes(forbiddenKey)) { + continue; + } + assert.equal(childKeys.has(forbiddenKey), false, `${forbiddenKey} reached ${call.args[0]}`); + } + assert.equal(call.env.npm_config_registry, "https://registry.npmjs.org/"); + assert.equal(call.env.npm_config_ignore_scripts, "true"); + assert.notEqual(call.env.npm_config_userconfig, secretEnv.npm_config_userconfig); + assert.notEqual(call.env.npm_config_globalconfig, secretEnv.npm_config_globalconfig); + assert.match(call.env.npm_config_userconfig, /user\.npmrc$/); + assert.match(call.env.npm_config_globalconfig, /global\.npmrc$/); + assert.ok(call.env.npm_config_cache); + assert.ok(call.env.HOME); + assert.equal(call.env.USERPROFILE, call.env.HOME); + } + assert.ok(calls[0].args.includes("--ignore-scripts")); + assert.ok(calls[2].args.includes("--ignore-scripts")); + } finally { + for (const [key, value] of previousEnv) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } + }); + + it("passes only process essentials and public network settings without inherited credentials", () => { + const paths = { + homeDir: resolve("isolated/home"), + cacheDir: resolve("isolated/cache"), + tempDir: resolve("isolated/tmp"), + appDataDir: resolve("isolated/home/AppData/Roaming"), + localAppDataDir: resolve("isolated/home/AppData/Local"), + xdgConfigDir: resolve("isolated/home/.config"), + userConfigPath: resolve("isolated/config/user.npmrc"), + globalConfigPath: resolve("isolated/config/global.npmrc"), + }; + const inherited = { + Path: "safe-bin-path", + SystemRoot: "safe-system-root", + HTTPS_PROXY: "https://proxy.example.invalid", + NODE_EXTRA_CA_CERTS: resolve("public-network-ca.pem"), + NPM_TOKEN: "publish-secret", + NODE_AUTH_TOKEN: "node-auth-secret", + _auth: "legacy-auth-secret", + _authToken: "legacy-token-secret", + npm_config__auth: "config-auth-secret", + npm_config__authToken: "config-token-secret", + "npm_config_//registry.npmjs.org/:_authToken": "scoped-token-secret", + npm_config_always_auth: "true", + npm_config_otp: "123456", + npm_config_userconfig: resolve("credentials/npmrc"), + npm_config_globalconfig: resolve("credentials/global-npmrc"), + npm_config_registry: "https://private-registry.example.invalid/", + GITHUB_TOKEN: "github-secret", + GH_TOKEN: "gh-secret", + AWS_SECRET_ACCESS_KEY: "aws-secret", + NODE_OPTIONS: "--require=credential-stealer.cjs", + INIT_CWD: resolve("credential-bearing-release-worktree"), + npm_lifecycle_event: "publish", + UNRELATED_SETTING: "not-an-essential", + }; + + const childEnv = buildRestrictedNpmEnv(inherited, paths); + assert.equal(childEnv.PATH, inherited.Path); + assert.equal(childEnv.SystemRoot, inherited.SystemRoot); + assert.equal(childEnv.HTTPS_PROXY, inherited.HTTPS_PROXY); + assert.equal(childEnv.NODE_EXTRA_CA_CERTS, inherited.NODE_EXTRA_CA_CERTS); + assert.equal(childEnv.HOME, paths.homeDir); + assert.equal(childEnv.USERPROFILE, paths.homeDir); + assert.equal(childEnv.npm_config_userconfig, paths.userConfigPath); + assert.equal(childEnv.npm_config_globalconfig, paths.globalConfigPath); + assert.equal(childEnv.npm_config_cache, paths.cacheDir); + assert.equal(childEnv.npm_config_registry, "https://registry.npmjs.org/"); + assert.equal(childEnv.npm_config_ignore_scripts, "true"); + + const childKeys = new Set(Object.keys(childEnv).map(key => key.toLowerCase())); + for (const forbiddenKey of [ + "npm_token", + "node_auth_token", + "_auth", + "_authtoken", + "npm_config__auth", + "npm_config__authtoken", + "npm_config_//registry.npmjs.org/:_authtoken", + "npm_config_always_auth", + "npm_config_otp", + "github_token", + "gh_token", + "aws_secret_access_key", + "node_options", + "init_cwd", + "npm_lifecycle_event", + "unrelated_setting", + ]) { + assert.equal(childKeys.has(forbiddenKey), false, `${forbiddenKey} must not reach npm children`); + } + }); +}); + describe("packed-consumer audit decision", () => { it("accepts only a zero exit with explicit zero counts at every severity", () => { const report = parseAuditJson(JSON.stringify({ From fd87eb77a3579b9fe3f2d62a88c8e6c0b6105dae Mon Sep 17 00:00:00 2001 From: "Josh S." <8768403+SuuBro@users.noreply.github.com> Date: Sun, 26 Jul 2026 21:50:16 +0100 Subject: [PATCH 8/8] Document CI stabilization contracts Co-authored-by: bobbit-ai --- ...async-background-cleanup-issue-analysis.md | 14 +++++----- docs/design/async-background-cleanup.md | 8 +++--- docs/design/pi-0.81-upgrade.md | 26 +++++++++++++------ docs/pi-runtime-compatibility.md | 24 ++++++++++------- docs/releasing.md | 22 +++++++++++++--- 5 files changed, 62 insertions(+), 32 deletions(-) diff --git a/docs/design/async-background-cleanup-issue-analysis.md b/docs/design/async-background-cleanup-issue-analysis.md index 42e1dc98d..5b4c6fdf3 100644 --- a/docs/design/async-background-cleanup-issue-analysis.md +++ b/docs/design/async-background-cleanup-issue-analysis.md @@ -79,7 +79,7 @@ POST /api/preview/mount -> JSON response ``` -`persistPreviewArtifact` is synchronous today. The route does not broadcast or return success when artifact persistence fails, although the live mount has already been written. Reuse is per session and returns the first valid matching candidate in filesystem enumeration order. +`persistPreviewArtifact` is synchronous today. The route does not broadcast or return success when artifact persistence fails, although the live mount has already been written. Reuse is per session and returns the first valid matching candidate in the order produced by that lookup's own candidate enumeration. #### Restore @@ -152,7 +152,7 @@ Direct immutable artifact serving in `src/server/preview/content-route.ts::handl | `restorePreviewArtifact` | sync copy/list/hash/stat/rename/remove | Stage and validate before wipe; backup only when live mount existed; rollback on swap failure | | `removeArtifacts` | `rmSync({ recursive: true })` | Invalid session ID is a no-op; missing tree is success; idempotent | | `sweepOrphanArtifacts` | `existsSync`, `readdirSync`, per-directory `rmSync` | Keep valid known session IDs case-insensitively; delete every other directory, including non-session directory names; ignore non-directories; sorted `removed`/`kept` results; per-item failure isolation | -| `findPreviewArtifactByHash` | `existsSync`, `readdirSync`, then sync read/list/hash per candidate | Preserve `readdir` candidate order and first valid reuse; corrupt/mismatched candidates are skipped, not deleted | +| `findPreviewArtifactByHash` | `existsSync`, `readdirSync`, then sync read/list/hash per candidate | Preserve the sequence returned to that lookup and first valid reuse; never infer order from a separate enumeration; corrupt/mismatched candidates are skipped, not deleted | | `validateArtifactMount` | sync stat/list/hash | Entry present, exact file-set equality, then content hash equality | | `copyDirectory`, `writeJsonAtomic`, `hashDirectory`, `listFiles`, `safeStat`, `wipeContents`, `moveContents` | all filesystem work is synchronous; recursive listing can overflow/defer for a deep/wide tree | Preserve sorted POSIX relative paths and fallback copy-on-rename-failure behavior | | `src/server/preview/mount.ts::mountDir` as called above | injected `mountFs.mkdirSync` | Do not create a mount merely to test whether it existed during restore | @@ -179,7 +179,7 @@ Directories and non-files are not hashed. Directory-read failure currently contr 5. Serialize preview mutation/persist/restore by session ID. Synchronous code currently gives these operations an implicit critical section; converting them independently would allow a second mount write to change the live tree while the first request is hashing or copying it. The lock must cover mount mutation through artifact persistence and cover the whole restore swap/rollback. 6. GET mount snapshot takes the same session lock so entry, mtime, content hash, and artifact ID describe one tree version. 7. For SSE, wait for the session lock, subscribe while holding it, take/write the bootstrap snapshot, then release. Events that occurred before the lock are represented by the snapshot; mutations cannot broadcast a newer event until bootstrap is written. This preserves bootstrap-before-live ordering without a missed-event window. -8. Candidate validation in `findPreviewArtifactByHash` remains sequential in the `readdir` result order. Internal traversal/hash work is bounded, but candidates must not race for “first valid match.” +8. Candidate validation in `findPreviewArtifactByHash` remains sequential in the order emitted by that lookup's own enumeration. After migration, this is the `Dirent` sequence from its `opendir()`/`Dir.read()` stream; no equivalence is promised with a separate `readdir()` call, another stream, or lexical sorting. Concurrent metadata work retains stream indexes, but exact validation must not race for “first valid match.” 9. `sweepOrphanArtifacts` may delete independent session directories through the bounded mapper. Collect outcomes in enumeration order and sort both public arrays at the end exactly as today. 10. Bounded removal must propagate non-`ENOENT` errors to the owning purge listener. `force`/missing remains idempotent. This makes the existing listener error log meaningful instead of hiding every removal failure inside `removeArtifacts`. @@ -474,7 +474,7 @@ Missing acceptance coverage: - wide/deep artifact stores and traversal ceiling; - event-loop progress while lookup/hash/copy/delete is deferred; -- several candidates in a controlled enumeration order; +- several candidates in a controlled lookup stream order; - invalid session/artifact metadata, wrong hash, extra/missing files and missing entry as independent skip cases; - exact first valid reuse selection when multiple candidates match; - stable hash parity for nested/binary/empty files and sorted record file names; @@ -553,7 +553,7 @@ Do not scan all of `server.ts::handleApiRoute` as one root, which would pull unr ### 9.2 Deterministic unit tests -Use deferred promise filesystem/command-runner fakes. Start an operation, queue an unrelated microtask/`setImmediate`, assert it runs while I/O is held, then release the I/O and assert the result. Do not use wall-clock performance thresholds. +Use deferred promise filesystem/command-runner fakes. Start an operation, queue an unrelated microtask/`setImmediate`, assert it runs while I/O is held, then release the I/O and assert the result. Do not use wall-clock performance thresholds. Signal readiness explicitly from the operation boundary being observed; a finite number of event-loop turns is not proof that prerequisite asynchronous work has reached that boundary. Register every test-owned hold for failure-safe cleanup, release it before awaiting stop/listener barriers, and also release it in the test's `finally` path so an earlier assertion cannot strand teardown. Add focused tests for: @@ -572,7 +572,7 @@ Add an in-process integration test with injectable/deferred preview I/O: 2. start a preview bootstrap/reuse scan and hold a deep metadata/hash read; 3. concurrently issue `GET /api/health` and `POST /api/sessions`; 4. assert both requests complete before releasing the artifact traversal; -5. release and assert exact artifact selection. +5. release and assert selection of the first valid artifact from that controlled stream. Repeat the ordering assertion for purge-triggered mount/artifact deletion: hold deletion, prove health/session creation complete, then release and assert the purge response waits for cleanup. Use existing browser preview journeys to verify POST, SSE, historical artifact switching, reload restoration, and cleanup still work; add a browser-v2 journey only if those existing journeys cannot exercise the new awaited route seams. @@ -596,7 +596,7 @@ No excluded finding is declared safe; it is simply not part of this focused firs Implementation and review should explicitly confirm: - preview hashes and sorted file lists are byte-for-byte stable; -- first valid artifact candidate selection is unchanged; +- first valid artifact candidate selection follows the lookup's own streamed enumeration order; - corrupt artifacts are skipped, never reused or auto-deleted by lookup; - restore validation precedes mutation and rollback remains available; - all cleanup APIs remain idempotent and response shapes/counts remain unchanged; diff --git a/docs/design/async-background-cleanup.md b/docs/design/async-background-cleanup.md index 0e8d950bc..b7aa5cf64 100644 --- a/docs/design/async-background-cleanup.md +++ b/docs/design/async-background-cleanup.md @@ -51,7 +51,7 @@ Missing entries are handled at the operation that owns them. `ENOENT` remains su | One-root deletion | `removeTree` is an iterative streaming lane over an identity-detached root, not a recursive promise tree. | | Multi-root deletion | `removePreviewTrees` and `sweepOrphanArtifacts` run no more than the shared number of independent root lanes. Preview backup/quarantine cleanup also uses a single global ownership lane. | -Metadata reads may settle concurrently within one candidate batch, but results are processed in filesystem enumeration order. Exact mount validation is sequential and the next batch is not prefetched during validation. This preserves the first valid matching candidate even when metadata reads complete out of order. +Here, filesystem enumeration order means the `Dirent` sequence emitted by this lookup's own `opendir()`/`Dir.read()` stream. A separate `readdir()` call—or even a separate stream—is not an ordering oracle: no cross-call, cross-API, or lexical equivalence is promised. Metadata reads may settle concurrently within one candidate batch, but the bounded mapper retains each stream index. Exact mount validation then consumes those indexed results sequentially, without prefetching the next batch, so reuse remains the first valid matching candidate from the lookup's own stream. A candidate is reusable only when: @@ -199,7 +199,7 @@ A missing worktree short-circuits the transcript stat; missing paths and I/O err Asynchrony does not change public selection or response semantics: - bounded maps retain input result order; inventories apply their existing final sort; -- artifact metadata may finish out of order, but reuse remains first-valid filesystem order; +- artifact metadata may finish out of order, but reuse remains first-valid order from that lookup's own `Dir.read()` stream; - hashes and metadata file lists remain stable and sorted; - orphan cleanup returns an exact count and deterministic bounded samples; - mutation prune and archive stats retain exact count rules; @@ -219,10 +219,10 @@ Behavioral coverage is split by invariant: - Preview unit suites cover wide/deep traversal, ordered batched candidates, corruption and missing entries, exact hash/file-list parity, catalog bounds and generation races, descriptor/root/parent identity substitution, transactional install/rollback, quarantine restoration, cleanup idempotency, and per-item failure isolation. - Worktree suites cover deferred event-loop progress, shared ceilings, pure pool construction and initialization/stop/drain barriers, boot sweep-before-pool ordering, deterministic inventory results, live ownership arriving during a scan, archived-branch and shared/container/pool protections, and targeted symlink-safe deletion. - Plan-mutation, archive-purge, and orphan suites use deferred fake I/O to pin event-loop progress, bounded read-ahead, goal serialization, atomic decisions, timer single-flight, stale callback fencing, awaited stop/shutdown, retention boundaries, listener ownership, scanner sampling, and every orphan-preservation decision. -- `tests2/integration/search-preview-api.test.ts` holds artifact validation and purge deletion while `GET /api/health` and `POST /api/sessions` complete, then proves the preview operation itself still waits and selects the exact artifact. It also pins SSE bootstrap-before-live ordering. +- `tests2/integration/search-preview-api.test.ts` holds artifact validation and purge deletion while `GET /api/health` and `POST /api/sessions` complete, then proves the preview operation itself still waits and selects the first valid artifact from a controlled `Dir.read()` stream. It also pins SSE bootstrap-before-live ordering. - `tests2/integration/preview-purge-listener-error.test.ts` proves the awaited listener owns an artifact deletion failure without making the gateway unhealthy. Existing preview route and browser journeys preserve POST, restore, reload, and historical artifact behavior. -The tests use deferred ordering and observed concurrency, not machine-speed thresholds. +The tests use deferred ordering and observed concurrency, not machine-speed thresholds. Readiness comes from an explicit signal at the operation boundary under test; a finite number of event-loop turns cannot prove that earlier asynchronous filesystem work has reached that boundary under contention. Test-owned deferreds that can hold a stop or listener barrier are released in failure-safe cleanup before awaiting the barrier. Otherwise an earlier assertion failure can skip the normal release and make correct production teardown wait indefinitely for the test's hold. ## Deferred synchronous-I/O areas diff --git a/docs/design/pi-0.81-upgrade.md b/docs/design/pi-0.81-upgrade.md index ce5abcf9c..4218ee4cc 100644 --- a/docs/design/pi-0.81-upgrade.md +++ b/docs/design/pi-0.81-upgrade.md @@ -1,10 +1,12 @@ # Pi 0.81 upgrade design +**Status:** Implemented historical design. The dependency selection records the `0.81.1` upgrade; the verification sections below reflect the current policy that live advisory checks are release-only. + ## Decision Pin `@earendil-works/pi-agent-core`, `@earendil-works/pi-ai`, and `@earendil-works/pi-coding-agent` exactly to `0.81.1`, the latest common release on 2026-07-21. It restores the pre-0.81 `streamFn` fallback via coding-agent's `setDefaultStreamFn(streamSimple)`. If a compatible patch appears first, advance all three pins together and repeat every check; mixed versions are invalid. -Only coding-agent publishes a shrinkwrap: it fixes `brace-expansion` at `5.0.7` but pins vulnerable `protobufjs@7.6.4`. Clean consumers ignore Bobbit's override and report sole moderate `GHSA-j3f2-48v5-ccww`. Prefer a Pi patch with `protobufjs@7.6.5+`; until then `0.81.1` fixes the high advisory but is not release-audit-clean. +Only coding-agent publishes a shrinkwrap: it fixes `brace-expansion` at `5.0.7` but pins `protobufjs@7.6.4`, below the required `7.6.5` floor. At upgrade selection time the registry reported the moderate `GHSA-j3f2-48v5-ccww` for that edge. Prefer a Pi patch with `protobufjs@7.6.5+`; until then `0.81.1` fixes the targeted high advisory but is not release-audit-clean or release-eligible. ## Implementation @@ -44,7 +46,7 @@ Extend `tests2/core/oauth-external-callbacks.test.ts`, `tests2/core/pi-rpc-agent Add `tests2/core/pi-tool-lifecycle-contract.test.ts`: emit tool execution start/update/end plus extension tool call/result payloads and assert ordering, policy/steer boundaries, partial-result forwarding, final output/error normalization, and omission/presence of optional `usage` and `addedToolNames`. Include compile-time type imports for the newly exported lifecycle types without adopting new runtime behavior. -Add `tests2/core/pi-published-shrinkwrap-security.test.ts`: a fixture with a secure root override and vulnerable dependency-owned pin must fail. Explicitly register both `tests2/core/pi-tool-lifecycle-contract.test.ts` and `tests2/core/pi-published-shrinkwrap-security.test.ts` in `tests2/tests-map.json` under the core unit suite. Add/register `tests/e2e/pi-packed-consumer.spec.ts`: pack Bobbit, install into an empty temp project, and apply the version-conditional compatibility assertions below. This detects root-audit false negatives. +Add `tests2/core/pi-published-shrinkwrap-security.test.ts`: a local fixture with a secure root override and below-floor dependency-owned pin must fail. Explicitly register both `tests2/core/pi-tool-lifecycle-contract.test.ts` and `tests2/core/pi-published-shrinkwrap-security.test.ts` in `tests2/tests-map.json` under the core unit suite. Add/register `tests/e2e/pi-packed-consumer.spec.ts`: pack Bobbit, install it into an empty temporary project, and apply the version-conditional graph assertions below. These deterministic checks expose the root-only false negative without querying mutable registry advisory data. ### Browser E2E journey @@ -62,21 +64,29 @@ npm ls @earendil-works/pi-agent-core @earendil-works/pi-ai @earendil-works/pi-co It passes only when npm exits zero, the three Pi roots are the same selected version, every resolved `brace-expansion` is `5.0.7+`, and the `0.81.1` tree contains exactly the known coding-agent-nested `protobufjs@7.6.4` edge (or every `protobufjs` is `7.6.5+` for a later selected patch), with no invalid, missing, stale, or extraneous Pi edge. -Retain packed-consumer output from: +Retain deterministic packed-consumer output from: ```text npm pack --json npm install npm ls @earendil-works/pi-agent-core @earendil-works/pi-ai @earendil-works/pi-coding-agent brace-expansion protobufjs --all -npm audit --omit=dev --json ``` -The packed-consumer audit helper must capture stdout and parse its JSON even when `npm audit` exits nonzero. For `0.81.1`, it accepts a nonzero exit only when the parsed result has the exact expected single-moderate-advisory shape: the consumer has aligned Pi versions, every `brace-expansion` is `5.0.7+`, the sole vulnerable dependency is the coding-agent-nested `protobufjs@7.6.4`, the only advisory is `GHSA-j3f2-48v5-ccww`, and the counts are exactly one moderate with zero low, high, or critical vulnerabilities. It must fail infrastructure/command-spawn errors, missing or malformed stdout, JSON parse errors, and any divergent exit status, advisory, count, package, version, or dependency path. For a later selected patch, it instead requires a zero exit, every `protobufjs@7.6.5+`, and a zero-vulnerability audit. +The normal E2E verifies consumer lock creation, coding-agent shrinkwrap presence, a valid installed graph, aligned Pi versions, `brace-expansion@5.0.7+`, and the version-conditional protobuf version/path floor. Import `node_modules/bobbit/dist/server/binaries.js`; require bundled `getFdResolution()`/`getRgResolution()` on supported platforms and execute both with `--version`. These deterministic checks remain in normal CI, but unit, browser, and E2E suites do not run or assert `npm audit` because registry advisory output can change independently of the source revision. + +The current release-only policy adds this required preflight after the build: + +```bash +npm run build +npm run audit:packed-consumer +``` + +The command packs Bobbit, installs it into a clean consumer, and runs `npm audit --omit=dev --json`. It isolates npm behind fresh home/config/cache/temp directories, an explicit public registry, no inherited auth or registry credentials, and disabled lifecycle scripts. Publication is blocked unless the audit exits zero and every severity and total count is zero; an advisory-service or install failure is also blocking. A root audit remains required but cannot replace this consumer check because it does not install dependency-owned shrinkwraps. -Import `node_modules/bobbit/dist/server/binaries.js`; require bundled `getFdResolution()`/`getRgResolution()` on supported platforms and execute both with `--version`. Run focused tests, `npm run build`, `npm run check`, `npm run test:unit`, `npm run test:browser`, and `npm run test:e2e`; the known `0.81.1` moderate is an asserted compatibility outcome, not a failing E2E. +Run focused tests, `npm run check`, `npm run test:unit`, `npm run test:browser`, and `npm run test:e2e` as normal. ## Pass/fail -Compatibility requires aligned pins/lock, all commands green, preserved OAuth/tool lifecycle/retry/compaction behavior, stable history UI, narrow browser imports, working extensions, `brace-expansion@5.0.7+` in both trees, the version-conditional protobuf/audit assertions above, and binary smoke success. Any extra advisory, stale/mixed Pi edge, missing shrinkwrap, lock mutation under restored `.npmrc`, or behavior regression fails. +Compatibility requires aligned pins/lock, all commands green, preserved OAuth/tool lifecycle/retry/compaction behavior, stable history UI, narrow browser imports, working extensions, `brace-expansion@5.0.7+` in both trees, the version-conditional protobuf graph assertions above, and binary smoke success. A stale or mixed Pi edge, missing shrinkwrap, lock mutation under restored `.npmrc`, below-floor unexpected path, or behavior regression fails. -Release eligibility is a separate blocking condition, not the `0.81.1` compatibility E2E verdict: Pi must publish a compatible common patch whose coding-agent shrinkwrap resolves every `protobufjs@7.6.5+`, and a fresh packed-consumer audit must report zero vulnerabilities. Therefore `0.81.1` may complete implementation and all required test gates with exactly its known moderate, but Bobbit must not declare the next release audit-clean or release-eligible until that condition passes. +Release eligibility is a separate blocking condition, not the normal E2E verdict: Pi must publish a compatible common patch whose coding-agent shrinkwrap resolves every `protobufjs@7.6.5+`, and the required release-only packed-consumer audit must report zero vulnerabilities. Therefore `0.81.1` may complete implementation and normal test gates, but Bobbit must not declare it audit-clean or release-eligible. diff --git a/docs/pi-runtime-compatibility.md b/docs/pi-runtime-compatibility.md index 98ec8cbc4..2d9685c99 100644 --- a/docs/pi-runtime-compatibility.md +++ b/docs/pi-runtime-compatibility.md @@ -12,16 +12,16 @@ Keep all three packages pinned exactly to the same Pi patch. A mixed Pi line can ## Compatibility and release eligibility -Pi `0.81.1` is the compatibility baseline selected on 2026-07-21. It removes the high-severity `brace-expansion` finding from Pi's published dependency tree, but **the next Bobbit release is not audit-clean or release-eligible**. +Pi `0.81.1` is the compatibility baseline selected on 2026-07-21. It removes the targeted high-severity `brace-expansion` edge from Pi's published dependency tree, but **the next Bobbit release is not audit-clean or release-eligible**. -`@earendil-works/pi-coding-agent@0.81.1` publishes its own `npm-shrinkwrap.json`. That shrinkwrap pins `protobufjs@7.6.4`, which is affected by the moderate advisory [`GHSA-j3f2-48v5-ccww`](https://github.com/advisories/GHSA-j3f2-48v5-ccww). A Bobbit root override can make the development checkout resolve `protobufjs@7.6.5`, but npm ignores that override when Bobbit is installed as a dependency and honors coding-agent's published shrinkwrap. The root checkout can consequently report zero audit findings while the packed consumer remains vulnerable; root `npm audit` output is not consumer evidence. +`@earendil-works/pi-coding-agent@0.81.1` publishes its own `npm-shrinkwrap.json`. That shrinkwrap pins `protobufjs@7.6.4`, below the required `7.6.5` floor. At selection time the registry associated that edge with [`GHSA-j3f2-48v5-ccww`](https://github.com/advisories/GHSA-j3f2-48v5-ccww). A Bobbit root override can make the development checkout resolve `protobufjs@7.6.5`, but npm ignores a dependency's override and honors coding-agent's shrinkwrap when Bobbit is installed as a package. Root `npm audit` output is therefore not evidence about the installed consumer graph. Compatibility and release eligibility are separate decisions: -- **Compatibility may pass for `0.81.1`** when all behavior gates pass and the packed consumer has exactly the one known moderate advisory, with no low, high, critical, or additional moderate findings. -- **Release eligibility remains blocked** until a compatible common Pi patch publishes coding-agent with every `protobufjs` edge at `7.6.5` or newer, all three Pi pins advance together, and a fresh packed-consumer audit reports zero vulnerabilities. +- **Compatibility may pass for `0.81.1`** when behavior gates and deterministic packed-consumer checks pass. Those checks cover graph validity, consumer lock creation and dependency-owned shrinkwrap presence, coordinated Pi versions, known version/path floors, and bundled binaries. +- **Release eligibility remains blocked** until a compatible common Pi patch publishes coding-agent with every `protobufjs` edge at `7.6.5` or newer, all three Pi pins advance together, and the required release-only packed-consumer audit reports zero vulnerabilities. -Do not describe `0.81.1` as audit-clean. The packed-consumer E2E treats the known moderate as an asserted compatibility outcome, not as a security exception for release. +Normal unit, browser, and E2E suites deliberately do not query or assert the mutable registry advisory feed. Advisory data can change independently of the code under test, so using it as a normal gate makes compatibility results nondeterministic. Live consumer advisory enforcement instead runs only during release preflight through `npm run audit:packed-consumer`; every severity must be zero, with no exceptions. Do not describe `0.81.1` as audit-clean or release-eligible. ### Verified dependency outcomes @@ -33,15 +33,15 @@ The controlled development-checkout regeneration produced this result: - `npm ls` exits successfully with no invalid, missing, stale, or extraneous Pi edge; and - a plain `npm install` with the repository `.npmrc` restored leaves `package-lock.json` unchanged. -A clean project installing the packed Bobbit tarball under normal consumer npm settings produced this result: +A clean project installing the packed Bobbit tarball under normal consumer npm settings deterministically verifies that: - all three Pi packages remain aligned at `0.81.1`; +- the consumer creates and owns its `package-lock.json`, while coding-agent retains its published shrinkwrap; - every packed-consumer `brace-expansion` occurrence is `5.0.7` or newer; -- exactly one `protobufjs@7.6.4` occurrence exists under coding-agent's published shrinkwrap; -- `npm audit --omit=dev --json` exits nonzero with exactly one moderate finding, `GHSA-j3f2-48v5-ccww`, and zero low, high, or critical findings; and +- exactly one `protobufjs@7.6.4` occurrence exists under coding-agent's published shrinkwrap; and - Bobbit's bundled `fd` and `rg` resolve from the installed package and execute `--version` on supported platforms. -The `brace-expansion@5.0.7+` floor fixes the targeted high advisory. It does not resolve the separate protobuf release blocker. +These package-graph facts remain stable test inputs; registry advisory output does not. The `brace-expansion@5.0.7+` floor addresses the targeted high advisory, but the deterministic protobuf floor still blocks release eligibility for `0.81.1`. ### Lockfile invariant @@ -315,6 +315,10 @@ Run `npm run test:manual` when credentials and Docker are available. Also retain npm ls @earendil-works/pi-agent-core @earendil-works/pi-ai @earendil-works/pi-coding-agent brace-expansion protobufjs --all ``` -For release evaluation, the packed-consumer path must build and pack Bobbit, install the tarball into an empty project under normal npm settings, inspect the same dependency tree, parse `npm audit --omit=dev --json` even when it exits nonzero, and smoke the installed `fd`/`rg` binaries. `tests2/core/pi-published-shrinkwrap-security.test.ts` separately pins why a clean root audit cannot replace that consumer test. +`tests/e2e/pi-packed-consumer.spec.ts` builds and packs Bobbit, installs the tarball into an empty project under normal consumer lock settings, inspects the dependency graph and shrinkwrap-owned paths, and smokes the installed `fd`/`rg` binaries. It intentionally does not call `npm audit`; normal unit, browser, and E2E gates must remain independent of mutable registry advisory output. + +For release evaluation, first build Bobbit, then run `npm run audit:packed-consumer`. The command packs the built package and installs it into a clean temporary consumer, then runs `npm audit --omit=dev --json` against the public registry. Its child npm processes use fresh home/config/cache/temp directories, do not inherit registry credentials or auth tokens, and disable lifecycle scripts. Publication is blocked unless the audit exits successfully and every severity and total count is zero; an unavailable advisory service also blocks rather than bypasses the check. See [Releasing Bobbit](releasing.md#required-packed-consumer-audit). + +`tests2/core/pi-published-shrinkwrap-security.test.ts` uses local fixtures to pin why a clean root audit cannot replace consumer evidence without querying the registry. Historical upgrade note: [Pi 0.77 / Claude Opus 4.8 compatibility](pi-0.77-opus-4.8.md) records the Opus-specific model, thinking-level, spawn, and sandbox auth contracts from that earlier Pi line. diff --git a/docs/releasing.md b/docs/releasing.md index fb2886592..1f2ad0abc 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -1,8 +1,24 @@ # Releasing Bobbit -This doc covers the parts of a Bobbit release that are easy to get wrong. -For now it focuses on the **bundled fd/rg binaries** — the rest of the -release flow (changelog, version bump, `npm publish`) is conventional. +This doc covers the Bobbit release checks that cannot be inferred from a normal development checkout: the installed consumer's security report and the bundled `fd`/`rg` binaries. + +## Required packed-consumer audit + +A root audit and a packed-consumer audit are both required before publication: + +```bash +npm audit --omit=dev +npm run build +npm run audit:packed-consumer +``` + +The build must precede `audit:packed-consumer` because the command packs the built Bobbit package. It then installs that tarball into a clean private consumer with normal lockfile ownership and runs `npm audit --omit=dev --json`. Publication is blocked unless npm exits successfully and the report contains zero `info`, `low`, `moderate`, `high`, `critical`, and total vulnerabilities. An install, advisory-service, malformed-report, or cleanup failure is also blocking; do not skip or waive the preflight. + +The command runs npm with fresh home, config, cache, and temporary directories; empty user/global npm configuration; and the public npm registry. It does not inherit auth tokens or custom registry credentials, and pack/install lifecycle scripts are disabled. This isolates release evidence from developer credentials and prevents a package lifecycle hook from executing during the security check. + +Normal unit, browser, and E2E suites intentionally do not query or assert registry advisory output because that feed can change without a source change. The packed-consumer E2E remains deterministic: it verifies consumer lock creation, dependency-owned shrinkwrap presence, installed graph validity, coordinated Pi versions, known dependency version/path floors, and bundled binary resolution/smoke behavior. Live advisory enforcement belongs only to this required release preflight. + +A clean root audit is still useful, but it is not consumer evidence. npm may honor a dependency's published `npm-shrinkwrap.json` only after Bobbit is installed as a package, so the consumer can resolve a different tree from the repository checkout. See [Pi runtime compatibility](pi-runtime-compatibility.md#compatibility-and-release-eligibility) for the current dependency constraints. ## Bundled fd/rg binaries