fix(tooling): review follow-ups from #146/#148/#162

docs/effect-conventions.md

@@ -124,9 +124,12 @@ end-to-end.
 
 Helped:
 
-- `Semaphore.make(1)` + `withPermit` is a drop-in for the hand-rolled serial
-  queues (FIFO waiters), including the process-wide per-project lease mutex
-  map in the epoch store.
+- `Semaphore.make(1)` + `withPermit` replaces hand-rolled mutual exclusion
+  where admission order is not observable, including the process-wide
+  per-project lease mutex map in the epoch store. It bounds access and
+  releases the permit when the guarded effect exits; the public API does not
+  guarantee FIFO waiter admission. Keep an explicit queue and ordering test
+  wherever submission order is part of the contract.
 - `Deferred` gives coalesced rebuild waiters one shared completion;
   `Effect.onExit` sits exactly where a `.finally` drain hook sat.
 - `Effect.forEach(..., { concurrency: 'unbounded' })` with per-element

package.json

@@ -43,7 +43,7 @@
     "example:audiobook": "pnpm build && pnpm --filter @agent-bundle-example/audiobook-curator dev",
     "example:mcp-app": "pnpm build && pnpm --filter @agent-bundle-example/mcp-app dev",
     "example:skills": "pnpm build && pnpm --filter @agent-bundle-example/skills-starter dev",
-    "examples:check": "pnpm build && AGENT_BUNDLE_TEST_TIME_SCALE=${AGENT_BUNDLE_TEST_TIME_SCALE:-2} pnpm --filter './examples/*' --workspace-concurrency=3 check"
+    "examples:check": "pnpm build && node scripts/run-examples-check.mjs"
   },
   "devDependencies": {
     "@arethetypeswrong/cli": "0.18.5",

packages/agent-bundle/tests/examples-check-script.test.ts

@@ -0,0 +1,60 @@
+import { execFile as executeFile } from 'node:child_process';
+import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { promisify } from 'node:util';
+
+import { expect, it } from '@rstest/core';
+
+const execFile = promisify(executeFile);
+const workspaceRoot = process.cwd();
+const examplesCheckScript = join(workspaceRoot, 'scripts', 'run-examples-check.mjs');
+
+it('runs pnpm without shell syntax and floors the example time scale at two', async () => {
+  const fixtureRoot = await mkdtemp(join(tmpdir(), 'agent-bundle-examples-check-'));
+  const fakePnpm = join(fixtureRoot, 'fake-pnpm.mjs');
+  const capturePath = join(fixtureRoot, 'capture.json');
+  await writeFile(fakePnpm, `
+import { writeFile } from 'node:fs/promises';
+
+await writeFile(process.env.FAKE_PNPM_CAPTURE, JSON.stringify({
+  args: process.argv.slice(2),
+  timeScale: process.env.AGENT_BUNDLE_TEST_TIME_SCALE,
+}));
+`, 'utf8');
+
+  try {
+    for (const [requestedScale, expectedScale] of [
+      [undefined, '2'],
+      ['1', '2'],
+      ['invalid', '2'],
+      ['4', '4'],
+    ] as const) {
+      const environment: NodeJS.ProcessEnv = {
+        ...process.env,
+        FAKE_PNPM_CAPTURE: capturePath,
+        npm_execpath: fakePnpm,
+      };
+      if (requestedScale === undefined) {
+        delete environment.AGENT_BUNDLE_TEST_TIME_SCALE;
+      } else {
+        environment.AGENT_BUNDLE_TEST_TIME_SCALE = requestedScale;
+      }
+
+      await execFile(process.execPath, [examplesCheckScript], {
+        cwd: workspaceRoot,
+        env: environment,
+      });
+      const capture = JSON.parse(await readFile(capturePath, 'utf8')) as {
+        readonly args: readonly string[];
+        readonly timeScale: string;
+      };
+      expect(capture).toEqual({
+        args: ['--filter', './examples/*', '--workspace-concurrency=3', 'check'],
+        timeScale: expectedScale,
+      });
+    }
+  } finally {
+    await rm(fixtureRoot, { force: true, recursive: true });
+  }
+});

packages/workbench/tests/runtime-playground-capture-cleanup.test.ts

@@ -0,0 +1,75 @@
+import { mkdtemp, rm, stat, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+
+import { expect, test } from '@rstest/core';
+
+// @ts-expect-error The executable capture script is intentionally imported as the cleanup-boundary test seam.
+import { atomically, cleanupCaptureResources, captureFailureAfterCleanup, formatCaptureFailure } from '../scripts/capture-runtime-playground.mjs';
+
+test('settles every capture cleanup action without masking the primary failure', async () => {
+  const outputRoot = await mkdtemp(join(tmpdir(), 'agent-bundle-runtime-capture-cleanup-'));
+  const temporary = join(outputRoot, '.desktop.png.temporary');
+  const primary = new Error('primary capture failed');
+  const order: string[] = [];
+  let fixtureCloseCount = 0;
+  try {
+    await expect(atomically(temporary, async (path: string) => {
+      await writeFile(path, 'partial output', 'utf8');
+      throw primary;
+    })).rejects.toBe(primary);
+
+    const cleanup = await cleanupCaptureResources({
+      browser: {
+        close: async () => {
+          order.push('browser.close');
+          throw new Error('browser close rejected');
+        },
+      },
+      fixture: {
+        close: async () => {
+          fixtureCloseCount += 1;
+          order.push('fixture.close');
+          throw new Error('fixture close rejected with fixture-secret');
+        },
+      },
+      restores: [
+        async () => {
+          order.push('restore-one');
+          throw new Error('restore one rejected');
+        },
+        async () => {
+          order.push('restore-two');
+        },
+      ],
+    });
+
+    expect(order).toEqual(['restore-one', 'restore-two', 'browser.close', 'fixture.close']);
+    expect(fixtureCloseCount).toBe(1);
+    expect(cleanup).toEqual({ attemptedRestores: 2, failedSteps: ['restore-1', 'browser.close', 'fixture.close'] });
+    await expect(stat(temporary)).rejects.toMatchObject({ code: 'ENOENT' });
+
+    const failure = captureFailureAfterCleanup(primary, cleanup);
+    expect(failure).toBeInstanceOf(AggregateError);
+    expect(failure).toMatchObject({ message: 'primary capture failed' });
+    expect((failure as AggregateError).errors[0]).toBe(primary);
+    expect((failure as AggregateError).errors[1]).toMatchObject({ message: 'Capture cleanup failed: restore-1, browser.close, fixture.close.' });
+    const formatted = formatCaptureFailure(failure);
+    expect(formatted).toBe('primary capture failed\nCapture cleanup failed: restore-1, browser.close, fixture.close.');
+    expect(formatted).not.toContain('restore one rejected');
+    expect(formatted).not.toContain('browser close rejected');
+    expect(formatted).not.toContain('fixture-secret');
+  } finally {
+    await rm(outputRoot, { force: true, recursive: true });
+  }
+});
+
+test('bounds a wedged cleanup step instead of holding the capture process open', async () => {
+  const cleanup = await cleanupCaptureResources({
+    browser: { close: async () => new Promise(() => {}) },
+    fixture: { close: async () => {} },
+    restores: [async () => new Promise(() => {})],
+    stepTimeout: 50,
+  });
+  expect(cleanup).toEqual({ attemptedRestores: 1, failedSteps: ['restore-1', 'browser.close'] });
+});

packages/workbench/tests/runtime-playground-capture.test.ts

@@ -1,14 +1,11 @@
 import { execFile as executeFile } from 'node:child_process';
-import { mkdtemp, readFile, readdir, rm, stat, writeFile } from 'node:fs/promises';
+import { mkdtemp, readFile, readdir, rm, stat } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { promisify } from 'node:util';
 
 import { expect, test } from '@rstest/core';
 
-// @ts-expect-error The executable capture script is intentionally imported as the cleanup-boundary test seam.
-import { atomically, cleanupCaptureResources, captureFailureAfterCleanup, formatCaptureFailure } from '../scripts/capture-runtime-playground.mjs';
-
 const execFile = promisify(executeFile);
 const workspaceRoot = process.cwd();
 const captureScript = join(workspaceRoot, 'packages', 'workbench', 'scripts', 'capture-runtime-playground.mjs');
@@ -64,73 +61,6 @@ const expectPng = async (path: string, width: number, height: number): Promise<v
   expect(contents.readUInt32BE(20)).toBe(height);
 };
 
-test('settles every capture cleanup action without masking the primary failure', async () => {
-  const outputRoot = await mkdtemp(join(tmpdir(), 'agent-bundle-runtime-capture-cleanup-'));
-  const temporary = join(outputRoot, '.desktop.png.temporary');
-  const primary = new Error('primary capture failed');
-  const order: string[] = [];
-  let fixtureCloseCount = 0;
-  try {
-    await expect(atomically(temporary, async (path: string) => {
-      await writeFile(path, 'partial output', 'utf8');
-      throw primary;
-    })).rejects.toBe(primary);
-
-    const cleanup = await cleanupCaptureResources({
-      browser: {
-        close: async () => {
-          order.push('browser.close');
-          throw new Error('browser close rejected');
-        },
-      },
-      fixture: {
-        close: async () => {
-          fixtureCloseCount += 1;
-          order.push('fixture.close');
-          throw new Error('fixture close rejected with fixture-secret');
-        },
-      },
-      restores: [
-        async () => {
-          order.push('restore-one');
-          throw new Error('restore one rejected');
-        },
-        async () => {
-          order.push('restore-two');
-        },
-      ],
-    });
-
-    expect(order).toEqual(['restore-one', 'restore-two', 'browser.close', 'fixture.close']);
-    expect(fixtureCloseCount).toBe(1);
-    expect(cleanup).toEqual({ attemptedRestores: 2, failedSteps: ['restore-1', 'browser.close', 'fixture.close'] });
-    await expect(stat(temporary)).rejects.toMatchObject({ code: 'ENOENT' });
-
-    const failure = captureFailureAfterCleanup(primary, cleanup);
-    expect(failure).toBeInstanceOf(AggregateError);
-    expect(failure).toMatchObject({ message: 'primary capture failed' });
-    expect((failure as AggregateError).errors[0]).toBe(primary);
-    expect((failure as AggregateError).errors[1]).toMatchObject({ message: 'Capture cleanup failed: restore-1, browser.close, fixture.close.' });
-    const formatted = formatCaptureFailure(failure);
-    expect(formatted).toBe('primary capture failed\nCapture cleanup failed: restore-1, browser.close, fixture.close.');
-    expect(formatted).not.toContain('restore one rejected');
-    expect(formatted).not.toContain('browser close rejected');
-    expect(formatted).not.toContain('fixture-secret');
-  } finally {
-    await rm(outputRoot, { force: true, recursive: true });
-  }
-});
-
-test('bounds a wedged cleanup step instead of holding the capture process open', async () => {
-  const cleanup = await cleanupCaptureResources({
-    browser: { close: async () => new Promise(() => {}) },
-    fixture: { close: async () => {} },
-    restores: [async () => new Promise(() => {})],
-    stepTimeout: 50,
-  });
-  expect(cleanup).toEqual({ attemptedRestores: 1, failedSteps: ['restore-1', 'browser.close'] });
-});
-
 test('captures identity-backed HMR, last-good, recovery, and desktop browser evidence', { timeout: 600_000 }, async () => {
   const outputRoot = await mkdtemp(join(tmpdir(), 'agent-bundle-runtime-capture-'));
   const outputs = Object.freeze({

rstest.integration-tests.ts

@@ -27,6 +27,7 @@ export const integrationTestFiles: readonly string[] = [
   'packages/agent-bundle/tests/eval-harness.test.ts',
   'packages/agent-bundle/tests/eval-service.test.ts',
   'packages/agent-bundle/tests/eval-workbench.test.ts',
+  'packages/agent-bundle/tests/examples-check-script.test.ts',
   'packages/agent-bundle/tests/examples-contract.test.ts',
   'packages/agent-bundle/tests/generated-route-server.test.ts',
   'packages/agent-bundle/tests/hook-playground-service.test.ts',
@@ -62,6 +63,7 @@ export const integrationTestFiles: readonly string[] = [
   'packages/workbench/tests/rsbuild-workbench.test.ts',
   'packages/workbench/tests/runtime-inspector.test.ts',
   'packages/workbench/tests/runtime-consent-dialog.test.ts',
+  'packages/workbench/tests/runtime-playground-capture-cleanup.test.ts',
   'packages/workbench/tests/runtime-playground.e2e.test.ts',
   'packages/workbench/tests/runtime-playground-hmr.e2e.test.ts',
   'packages/workbench/tests/workbench-dev-command.test.ts',
@@ -73,9 +75,10 @@ export const integrationTestFiles: readonly string[] = [
  * behavioral proof. The behavioral contracts they exercise (HMR activation,
  * last-good retention, recovery) are already covered per PR by
  * runtime-playground.e2e.test.ts and runtime-playground-hmr.e2e.test.ts in
- * the integration pool, so these run through the root `test:evidence`
- * script in CI's nightly schedule instead — evidence regenerates when the
- * flow changes, not on every PR (#128).
+ * the integration pool. Fast capture cleanup contracts stay per PR in
+ * runtime-playground-capture-cleanup.test.ts. The evidence journey runs
+ * through the root `test:evidence` script in CI's nightly schedule instead —
+ * evidence regenerates when the flow changes, not on every PR (#128).
  */
 export const nightlyEvidenceTestFiles: readonly string[] = [
   'packages/workbench/tests/runtime-playground-capture.test.ts',

scripts/run-examples-check.mjs

@@ -0,0 +1,30 @@
+import { spawn } from 'node:child_process';
+
+const requestedTimeScale = Number(process.env.AGENT_BUNDLE_TEST_TIME_SCALE ?? '');
+const timeScale = Number.isSafeInteger(requestedTimeScale) && requestedTimeScale >= 1
+  ? Math.max(requestedTimeScale, 2)
+  : 2;
+const pnpmEntrypoint = process.env.npm_execpath;
+
+if (pnpmEntrypoint === undefined || pnpmEntrypoint.length === 0) {
+  throw new Error('run-examples-check.mjs must be launched through a pnpm package script.');
+}
+
+const child = spawn(process.execPath, [
+  pnpmEntrypoint,
+  '--filter',
+  './examples/*',
+  '--workspace-concurrency=3',
+  'check',
+], {
+  env: {
+    ...process.env,
+    AGENT_BUNDLE_TEST_TIME_SCALE: String(timeScale),
+  },
+  stdio: 'inherit',
+});
+
+process.exitCode = await new Promise((resolve, reject) => {
+  child.once('error', reject);
+  child.once('close', (code) => resolve(code ?? 1));
+});