From 35dd014c580037d1c009776d8b0584587c1bd42c Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Mon, 21 Sep 2026 12:27:25 -0400 Subject: [PATCH] v1.87.5.0 perf: remove idle waits from tests and CI planning (#2897) * v1.87.5.0 perf: remove idle waits from tests and CI planning * fix: settle split PTY redraws before routing input * docs: record final burst-safe test benchmarks * fix: keep cold-setup snapshot metadata dependency-free * fix: avoid early-reader pipe races in artifact URL parsing * fix: preserve safety matches for multiline command payloads * fix: recognize concurrent CSO publication removal * test: preload the UI design-review target before invocation * docs: record validation blocker fixes * fix: bind plan observer rejection to the invoked command * fix: count only native design decisions in the UI gate * docs: clarify UI-positive eval evidence requirements * test: recognize native UI decisions without weakening finding counts * test: decouple native UI evidence from question punctuation * test: recognize concrete native UI decisions independently of prose format * fix: retain failed eval logs under the hidden CI cache * test: await telemetry completion instead of racing disk writes --- .github/workflows/evals-periodic.yml | 20 +- .github/workflows/evals.yml | 36 +- CHANGELOG.md | 38 ++ VERSION | 2 +- agents-digest/gstack-AGENTS.md | 2 +- bin/gstack-artifacts-url | 4 +- browse/src/snapshot-flags.ts | 30 ++ browse/src/snapshot.ts | 41 +- browse/src/telemetry.ts | 6 +- browse/test/bun-polyfill.test.ts | 15 +- browse/test/cookie-picker-routes.test.ts | 12 +- browse/test/telemetry-optout.test.ts | 10 +- browse/test/telemetry.test.ts | 10 +- browse/test/watchdog.test.ts | 14 +- careful/bin/check-careful.sh | 36 +- design/test/daemon-discovery.test.ts | 18 +- design/test/daemon-tests-fixtures.ts | 13 +- docs/TESTING_INTERNALS.md | 25 + lib/cso/state.ts | 4 +- package.json | 2 +- scripts/resolvers/browse.ts | 2 +- test/ci-paid-coordination.test.ts | 181 +++++++ test/conductor-prose-observation-ao.test.ts | 1 + test/cso-git-hardening.test.ts | 2 +- test/cso-snapshot-state.test.ts | 19 +- test/design-count-outside.test.ts | 31 ++ test/design-ui-scope.test.ts | 124 +++++ test/eng-seeded-completion-ai.test.ts | 1 + test/evals-workflow-wiring.test.ts | 8 +- test/fixtures/plan-design-ui-scope.json | 540 ++++++++++++++++++++ test/gstack-artifacts-url.test.ts | 10 + test/helpers/claude-pty-runner.ts | 83 ++- test/helpers/design-count-outside.ts | 6 +- test/helpers/design-ui-scope.ts | 27 + test/helpers/touchfiles-data.ts | 1 + test/hook-scripts.test.ts | 27 + test/plan-count-checkbox.test.ts | 3 +- test/plan-count-completion.test.ts | 6 +- test/plan-count-file-permission.test.ts | 3 +- test/plan-count-fixture.test.ts | 55 +- test/pty-output-wake.test.ts | 254 +++++++++ test/skill-e2e-plan-design-with-ui.test.ts | 145 ++---- 42 files changed, 1583 insertions(+), 284 deletions(-) create mode 100644 browse/src/snapshot-flags.ts create mode 100644 test/ci-paid-coordination.test.ts create mode 100644 test/design-ui-scope.test.ts create mode 100644 test/fixtures/plan-design-ui-scope.json create mode 100644 test/helpers/design-ui-scope.ts create mode 100644 test/pty-output-wake.test.ts diff --git a/.github/workflows/evals-periodic.yml b/.github/workflows/evals-periodic.yml index 7c2abe496..bfb51561e 100644 --- a/.github/workflows/evals-periodic.yml +++ b/.github/workflows/evals-periodic.yml @@ -78,29 +78,22 @@ jobs: plan-slices: runs-on: ubicloud-standard-8 - needs: build-image timeout-minutes: 10 permissions: contents: read - packages: read - container: - image: ${{ needs.build-image.outputs.image-tag }} - credentials: - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - options: --user runner steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: persist-credentials: false - - name: Restore deps - uses: ./.github/actions/restore-deps + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 + with: + bun-version: 1.4.0 - name: Emit run manifest (ALL periodic tests minus reasoned excludes) env: EVALS_ALL: "1" - run: EVALS_TIER=periodic bun run scripts/test-paid-shards.ts --tier periodic --emit-plan /tmp/paid-plan/manifest.json --slices 7 --autoplan-slice + run: EVALS_TIER=periodic bun --no-install run scripts/test-paid-shards.ts --tier periodic --emit-plan /tmp/paid-plan/manifest.json --slices 7 --autoplan-slice - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: @@ -189,6 +182,7 @@ jobs: uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: paid-slice-${{ matrix.slice }}-logs + include-hidden-files: true # The Fix-bun-temp step points TMPDIR at /home/runner/.cache, so the # runner's spool lands THERE, not /tmp — the original /tmp glob # uploaded nothing and a red slice's diagnostics were unreachable. @@ -275,8 +269,6 @@ jobs: with: bun-version: 1.4.0 - - run: bun install --frozen-lockfile - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: name: paid-plan @@ -292,7 +284,7 @@ jobs: id: reconcile run: | set +e - EVALS_TIER=periodic bun run scripts/test-paid-shards.ts --tier periodic --report /tmp/paid-report | tee /tmp/report.txt + EVALS_TIER=periodic bun --no-install run scripts/test-paid-shards.ts --tier periodic --report /tmp/paid-report | tee /tmp/report.txt # PIPESTATUS[0], NOT $?: GitHub's default run-step shell is # `bash -e {0}` with NO pipefail, so $? after the pipe is tee's # exit (always 0) — the fail-closed gate was silently fail-open diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 4194ac092..9b8a57231 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -22,7 +22,7 @@ jobs: # Dependabot-triggered pull_request runs get a read-only GITHUB_TOKEN, so # a lockfile bump = new hash = failed ghcr push = permanently red check # (EV6, fork port wave 2). Skip the build for dependabot; the evals job's - # needs-chain tolerates it because no eval test selects on a lockfile-only + # explicit actor guard mirrors it because no eval test selects on a lockfile-only # diff — a maintainer's next push rebuilds the image with real perms. if: github.actor != 'dependabot[bot]' runs-on: ubicloud-standard-8 @@ -103,35 +103,26 @@ jobs: # branch (see CLAUDE.md's garrytan-agents workflow). plan-slices: runs-on: ubicloud-standard-8 - needs: build-image - if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository + if: github.actor != 'dependabot[bot]' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository) timeout-minutes: 10 permissions: contents: read - packages: read - container: - image: ${{ needs.build-image.outputs.image-tag }} - credentials: - username: ${{ github.actor }} - password: ${{ secrets.GITHUB_TOKEN }} - options: --user runner steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: - # The planner is the ONE place that needs history: diff selection - # resolves a merge-base. Executors run from the manifest and stay - # shallow. Selection fails OPEN (run-all) if resolution fails — the - # documented posture; a planner bug can only run extra work. + # Preserve full history for merge-base diff selection. Moving the + # planner off the eval image must not change its selection inputs. fetch-depth: 0 persist-credentials: false - - name: Restore deps - uses: ./.github/actions/restore-deps + - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2 + with: + bun-version: 1.4.0 - name: Emit run manifest env: EVALS_ALL: ${{ (github.event_name == 'workflow_dispatch' && inputs.evals_all) && '1' || '' }} - run: EVALS_TIER=gate bun run scripts/test-paid-shards.ts --tier gate --emit-plan /tmp/paid-plan/manifest.json --slices 6 + run: EVALS_TIER=gate bun --no-install run scripts/test-paid-shards.ts --tier gate --emit-plan /tmp/paid-plan/manifest.json --slices 6 - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: @@ -142,7 +133,7 @@ jobs: eval-slices: runs-on: ubicloud-standard-8 needs: [build-image, plan-slices] - if: always() && needs.plan-slices.result == 'success' + if: always() && needs.build-image.result == 'success' && needs.plan-slices.result == 'success' # Aggregate spawn-concurrency budget: 6 slices x EVALS_JOBS=2 x # EVALS_CONCURRENCY=2 = 24 concurrent tests lane-wide (the old matrix's # 40-way per row queued claude session STARTUP behind 39 siblings and ate @@ -226,6 +217,7 @@ jobs: uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: name: paid-slice-${{ matrix.slice }}-logs + include-hidden-files: true # The Fix-bun-temp step points TMPDIR at /home/runner/.cache, so the # runner's spool lands THERE, not /tmp — the original /tmp glob # uploaded nothing and a red slice's diagnostics were unreachable. @@ -242,8 +234,8 @@ jobs: # missing slice artifact reading as green is the class this lane kills. if: always() && needs.plan-slices.result == 'success' timeout-minutes: 5 - # contents:read ONLY — this job executes PR-authored code (bun install - # lifecycle scripts + the reconcile runner from the PR checkout), so it + # contents:read ONLY — this job executes the PR-authored reconcile + # runner from the PR checkout, so it # must never hold a write-scoped token. The PR comment lives in the # separate slices-comment job below, which runs NO repo code: a # $GITHUB_ENV/BASH_ENV persistence trick is job-scoped, so the split is @@ -263,8 +255,6 @@ jobs: with: bun-version: 1.4.0 - - run: bun install --frozen-lockfile - - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: name: paid-plan @@ -280,7 +270,7 @@ jobs: id: reconcile run: | set +e - EVALS_TIER=gate bun run scripts/test-paid-shards.ts --tier gate --report /tmp/paid-report | tee /tmp/report.txt + EVALS_TIER=gate bun --no-install run scripts/test-paid-shards.ts --tier gate --report /tmp/paid-report | tee /tmp/report.txt # PIPESTATUS[0], NOT $?: GitHub's default run-step shell is # `bash -e {0}` with NO pipefail, so $? after the pipe is tee's # exit (always 0) — the fail-closed gate was silently fail-open diff --git a/CHANGELOG.md b/CHANGELOG.md index ffafc1473..41057d8d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,43 @@ # Changelog +## [1.87.5.0] - 2026-09-17 + +**Tests finish sooner without dropping checks.** +**CI stops waiting on unused tooling.** + +This release speeds up gstack's development feedback loop. Terminal-driven checks react to new output instead of waiting through a fixed polling interval. Synthetic CLIs can announce when their input handlers are ready, while real CLI sessions retain their startup grace and input debounces. Completed processes no longer stay alive just to finish unused timeout timers. + +### The three numbers that matter + +Measured on the same Linux machine with Bun 1.4.0. Run each named file with `bun test ` on the previous release and this version. The polyfill and daemon numbers are three-run wall-time medians on both sides; the permission baseline is one observed run, and its after value is a three-run median. These are individual test-file measurements, not whole-suite or production latency claims. + +| Test file | Before | After | Δ | +|---|---:|---:|---:| +| `browse/test/bun-polyfill.test.ts` | 17.30s | 1.29s | −93% | +| `design/test/daemon-discovery.test.ts` | 17.78s | 11.99s | −33% | +| `test/plan-count-file-permission.test.ts` | 97.05s | 30.92s | −68% | + +The permission suite still makes all 115 assertions, including its deliberate stale-prompt delay. Terminal observations are capped at four per second so animated output cannot turn faster responses into a busy loop. + +### What this means for contributors + +Local test runs spend less time waiting after work is already complete. CI planning no longer pulls the execution image or waits for image lookup before producing its manifest; executors still require both prerequisites, and missing or failed results still fail reconciliation. Run `bun run test` for the complete free suite. + +### Itemized changes + +#### For contributors + +- Wake plan-count checks on output and exit, retain a silent metadata fallback, and settle output bursts before reading split redraws. Three synthetic CLI suites use explicit startup readiness without changing real-CLI startup behavior. +- Cancel unused PTY and Node deadlines, stop already-exited daemon fixtures immediately, clear cookie-picker fixture sessions between suites, and await watchdog shutdown with a bounded completion signal. +- Await telemetry append completion in consent tests instead of assuming disk writes finish within 30ms. Consent checks and error swallowing are unchanged. +- Run gate and periodic CI planners directly on pinned Bun without dependency installation. Reports also skip unused installs; fork restrictions, executor images, and failure checks remain intact. +- Retain failed eval shard logs from the hidden CI cache directory without uploading unrelated cache files. +- Inject the CSO Git-pointer race at its first bounded read, preserving the original rejection assertion. +- Preload the UI-positive design-review eval in an isolated plan fixture and require an answered native design question, rather than accepting scope or outside-review menus. Bind command-rejection checks to the invoked skill, not a child tool's diagnostics. +- Generate skill documentation in a fresh checkout without importing browser runtime dependencies. Keep snapshot flag metadata and public exports unchanged. +- Eliminate early-reader pipe races in artifact URL normalization and safety-hook matching. Multiline commands retain their warnings even with large trailing content. +- Treat a publication removed by another CSO recovery helper during candidate enumeration as a bounded retry; replacement inodes still fail closed. + ## [1.87.4.0] - 2026-09-16 **Failed checks stay failed.** diff --git a/VERSION b/VERSION index bf58942b8..133a1bb9d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.87.4.0 +1.87.5.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index aacc09e01..79fdcac47 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.87.4.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.87.5.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-artifacts-url b/bin/gstack-artifacts-url index baa0af2f2..e2a12349d 100755 --- a/bin/gstack-artifacts-url +++ b/bin/gstack-artifacts-url @@ -103,8 +103,8 @@ parse_url() { } parsed=$(parse_url "$url") -host=$(echo "$parsed" | head -1) -owner_repo=$(echo "$parsed" | tail -1) +host="${parsed%%$'\n'*}" +owner_repo="${parsed##*$'\n'}" case "$mode" in to) diff --git a/browse/src/snapshot-flags.ts b/browse/src/snapshot-flags.ts new file mode 100644 index 000000000..68c3b7a02 --- /dev/null +++ b/browse/src/snapshot-flags.ts @@ -0,0 +1,30 @@ +export interface SnapshotOptions { + interactive?: boolean; + compact?: boolean; + depth?: number; + selector?: string; + diff?: boolean; + annotate?: boolean; + outputPath?: string; + cursorInteractive?: boolean; + heatmap?: string; +} + +export const SNAPSHOT_FLAGS: Array<{ + short: string; + long: string; + description: string; + takesValue?: boolean; + valueHint?: string; + optionKey: keyof SnapshotOptions; +}> = [ + { short: '-i', long: '--interactive', description: 'Interactive elements only (buttons, links, inputs) with @e refs. Also auto-enables cursor-interactive scan (-C) to capture dropdowns and popovers.', optionKey: 'interactive' }, + { short: '-c', long: '--compact', description: 'Compact (no empty structural nodes)', optionKey: 'compact' }, + { short: '-d', long: '--depth', description: 'Limit tree depth (0 = root only, default: unlimited)', takesValue: true, valueHint: '', optionKey: 'depth' }, + { short: '-s', long: '--selector', description: 'Scope to CSS selector', takesValue: true, valueHint: '', optionKey: 'selector' }, + { short: '-D', long: '--diff', description: 'Unified diff against previous snapshot (first call stores baseline)', optionKey: 'diff' }, + { short: '-a', long: '--annotate', description: 'Annotated screenshot with red overlay boxes and ref labels', optionKey: 'annotate' }, + { short: '-o', long: '--output', description: 'Output path for annotated screenshot (default: /browse-annotated.png)', takesValue: true, valueHint: '', optionKey: 'outputPath' }, + { short: '-C', long: '--cursor-interactive', description: 'Cursor-interactive elements (@c refs — divs with pointer, onclick). Auto-enabled when -i is used.', optionKey: 'cursorInteractive' }, + { short: '-H', long: '--heatmap', description: 'Color-coded overlay screenshot from JSON map: \'{"@e1":"green","@e3":"red"}\'. Valid colors: green, yellow, red, blue, orange, gray.', takesValue: true, valueHint: '', optionKey: 'heatmap' }, +]; diff --git a/browse/src/snapshot.ts b/browse/src/snapshot.ts index 22ea345f9..cb5312800 100644 --- a/browse/src/snapshot.ts +++ b/browse/src/snapshot.ts @@ -24,6 +24,9 @@ import { TEMP_DIR, isPathWithin } from './platform'; import { escapeEnvelopeSentinels } from './content-security'; import { stripLoneSurrogates } from './sanitize'; import { guardScreenshotPath } from './screenshot-size-guard'; +import { SNAPSHOT_FLAGS, type SnapshotOptions } from './snapshot-flags'; + +export { SNAPSHOT_FLAGS } from './snapshot-flags'; // Roles considered "interactive" for the -i flag const INTERACTIVE_ROLES = new Set([ @@ -33,44 +36,6 @@ const INTERACTIVE_ROLES = new Set([ 'treeitem', ]); -interface SnapshotOptions { - interactive?: boolean; // -i: only interactive elements - compact?: boolean; // -c: remove empty structural elements - depth?: number; // -d N: limit tree depth - selector?: string; // -s SEL: scope to CSS selector - diff?: boolean; // -D / --diff: diff against last snapshot - annotate?: boolean; // -a / --annotate: annotated screenshot - outputPath?: string; // -o / --output: path for annotated screenshot - cursorInteractive?: boolean; // -C / --cursor-interactive: scan cursor:pointer etc. - heatmap?: string; // -H / --heatmap: JSON color map for ref overlays -} - -/** - * Snapshot flag metadata — single source of truth for CLI parsing and doc generation. - * - * Imported by: - * - gen-skill-docs.ts (generates {{SNAPSHOT_FLAGS}} tables) - * - skill-parser.ts (validates flags in SKILL.md examples) - */ -export const SNAPSHOT_FLAGS: Array<{ - short: string; - long: string; - description: string; - takesValue?: boolean; - valueHint?: string; - optionKey: keyof SnapshotOptions; -}> = [ - { short: '-i', long: '--interactive', description: 'Interactive elements only (buttons, links, inputs) with @e refs. Also auto-enables cursor-interactive scan (-C) to capture dropdowns and popovers.', optionKey: 'interactive' }, - { short: '-c', long: '--compact', description: 'Compact (no empty structural nodes)', optionKey: 'compact' }, - { short: '-d', long: '--depth', description: 'Limit tree depth (0 = root only, default: unlimited)', takesValue: true, valueHint: '', optionKey: 'depth' }, - { short: '-s', long: '--selector', description: 'Scope to CSS selector', takesValue: true, valueHint: '', optionKey: 'selector' }, - { short: '-D', long: '--diff', description: 'Unified diff against previous snapshot (first call stores baseline)', optionKey: 'diff' }, - { short: '-a', long: '--annotate', description: 'Annotated screenshot with red overlay boxes and ref labels', optionKey: 'annotate' }, - { short: '-o', long: '--output', description: 'Output path for annotated screenshot (default: /browse-annotated.png)', takesValue: true, valueHint: '', optionKey: 'outputPath' }, - { short: '-C', long: '--cursor-interactive', description: 'Cursor-interactive elements (@c refs — divs with pointer, onclick). Auto-enabled when -i is used.', optionKey: 'cursorInteractive' }, - { short: '-H', long: '--heatmap', description: 'Color-coded overlay screenshot from JSON map: \'{"@e1":"green","@e3":"red"}\'. Valid colors: green, yellow, red, blue, orange, gray.', takesValue: true, valueHint: '', optionKey: 'heatmap' }, -]; - interface ParsedNode { indent: number; role: string; diff --git a/browse/src/telemetry.ts b/browse/src/telemetry.ts index 094f6a2f1..c71ca8ae2 100644 --- a/browse/src/telemetry.ts +++ b/browse/src/telemetry.ts @@ -97,10 +97,10 @@ export interface TelemetryEvent { } /** Fire-and-forget log. Never throws. */ -export function logTelemetry(payload: TelemetryEvent): void { - if (isTelemetryDisabled()) return; +export function logTelemetry(payload: TelemetryEvent): Promise { + if (isTelemetryDisabled()) return Promise.resolve(); const enriched = { ...payload, ts: new Date().toISOString() }; - ensureDir() + return ensureDir() .then(() => fs.appendFile(telemetryFile(), JSON.stringify(enriched) + '\n', 'utf8')) .catch(() => { // Telemetry must never crash the caller. If the disk is full or perms diff --git a/browse/test/bun-polyfill.test.ts b/browse/test/bun-polyfill.test.ts index 7df28b0d4..4028630e1 100644 --- a/browse/test/bun-polyfill.test.ts +++ b/browse/test/bun-polyfill.test.ts @@ -114,10 +114,11 @@ describe('bun-polyfill', () => { const p = Bun.spawn(['this-binary-does-not-exist-zzz-' + Date.now()], { stdio: ['ignore', 'pipe', 'pipe'] }); + let deadline; const code = await Promise.race([ p.exited, - new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000)) - ]).catch(() => 'TIMEOUT'); + new Promise((_, r) => { deadline = setTimeout(() => r(new Error('timeout')), 3000); }) + ]).catch(() => 'TIMEOUT').finally(() => clearTimeout(deadline)); console.log('exit:' + code); })(); `], { stdout: 'pipe', stderr: 'pipe', timeout: 30_000 }); @@ -159,10 +160,11 @@ describe('bun-polyfill', () => { ['node', '-e', 'process.stdout.write("y".repeat(10 * 1024)); process.exit(0)'], { stdio: ['ignore', 'pipe', 'ignore'] } ); + let deadline; const code = await Promise.race([ p.exited, - new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 3000)) - ]).catch(() => 'TIMEOUT'); + new Promise((_, r) => { deadline = setTimeout(() => r(new Error('timeout')), 3000); }) + ]).catch(() => 'TIMEOUT').finally(() => clearTimeout(deadline)); const out = await new Response(p.stdout).text(); console.log(out.length + ':' + code); })(); @@ -190,10 +192,11 @@ describe('bun-polyfill', () => { ['node', '-e', 'process.stdout.write("x".repeat(' + ONE_MB + '), () => process.exit(0))'], { stdio: ['ignore', 'pipe', 'ignore'] } ); + let deadline; const code = await Promise.race([ p.exited, - new Promise((_, r) => setTimeout(() => r(new Error('timeout')), 10000)) - ]).catch(e => 'TIMEOUT'); + new Promise((_, r) => { deadline = setTimeout(() => r(new Error('timeout')), 10000); }) + ]).catch(e => 'TIMEOUT').finally(() => clearTimeout(deadline)); const out = await new Response(p.stdout).text(); console.log(out.length + ':' + code); })().catch((e) => { console.log('THREW:' + e.message); }); diff --git a/browse/test/cookie-picker-routes.test.ts b/browse/test/cookie-picker-routes.test.ts index c1934cd86..b7119b894 100644 --- a/browse/test/cookie-picker-routes.test.ts +++ b/browse/test/cookie-picker-routes.test.ts @@ -6,9 +6,19 @@ * CORS headers, and JSON response formats. */ -import { describe, test, expect } from 'bun:test'; +import { afterAll, describe, test, expect } from 'bun:test'; import { handleCookiePickerRoute, generatePickerCode, hasActivePicker } from '../src/cookie-picker-routes'; +afterAll(() => { + const realNow = Date.now; + Date.now = () => realNow() + 3_700_000; + try { + expect(hasActivePicker()).toBe(false); + } finally { + Date.now = realNow; + } +}); + // ─── Mock BrowserManager ────────────────────────────────────── function mockBrowserManager() { diff --git a/browse/test/telemetry-optout.test.ts b/browse/test/telemetry-optout.test.ts index 0c4626c16..1f0185fd5 100644 --- a/browse/test/telemetry-optout.test.ts +++ b/browse/test/telemetry-optout.test.ts @@ -151,23 +151,19 @@ describe('telemetry env tier + cache semantics', () => { describe('enforcement: logTelemetry writes only with granted consent', () => { test('config-tier opt-out suppresses the JSONL append', async () => { const dir = tmpHomeWith('telemetry: off\n'); - logTelemetry({ event: 'domain_skill_fired', host: 'example.com' }); - // Fire-and-forget path: give any (incorrect) async append time to land. - await new Promise((r) => setTimeout(r, 30)); + await logTelemetry({ event: 'domain_skill_fired', host: 'example.com' }); expect(fs.existsSync(path.join(dir, 'analytics', 'browse-telemetry.jsonl'))).toBe(false); }); test('no consent ever recorded (absent key) suppresses the JSONL append', async () => { const dir = tmpHomeWith('pair_agent: on\n'); - logTelemetry({ event: 'domain_skill_fired', host: 'example.com' }); - await new Promise((r) => setTimeout(r, 30)); + await logTelemetry({ event: 'domain_skill_fired', host: 'example.com' }); expect(fs.existsSync(path.join(dir, 'analytics', 'browse-telemetry.jsonl'))).toBe(false); }); test('granted `community` tier appends the event', async () => { const dir = tmpHomeWith('telemetry: community\n'); - logTelemetry({ event: 'domain_skill_fired', host: 'example.com' }); - await new Promise((r) => setTimeout(r, 30)); + await logTelemetry({ event: 'domain_skill_fired', host: 'example.com' }); const file = path.join(dir, 'analytics', 'browse-telemetry.jsonl'); expect(fs.existsSync(file)).toBe(true); expect(fs.readFileSync(file, 'utf-8')).toContain('domain_skill_fired'); diff --git a/browse/test/telemetry.test.ts b/browse/test/telemetry.test.ts index 71a182eee..0e0b600e2 100644 --- a/browse/test/telemetry.test.ts +++ b/browse/test/telemetry.test.ts @@ -33,8 +33,6 @@ afterAll(async () => { }); async function readEvents(): Promise { - // Wait briefly for fire-and-forget appends to flush. - await new Promise((r) => setTimeout(r, 30)); try { const raw = await fs.readFile(TELEMETRY_FILE, 'utf8'); return raw.trim().split('\n').filter(Boolean).map((l) => JSON.parse(l)); @@ -47,7 +45,7 @@ describe('telemetry: signals fire to ~/.gstack/analytics/browse-telemetry.jsonl' it('logTelemetry writes a JSONL line with ts injected', async () => { const { logTelemetry, _resetTelemetryCache } = await import('../src/telemetry'); _resetTelemetryCache(); - logTelemetry({ event: 'domain_skill_saved', host: 'test.com', scope: 'project', state: 'quarantined', bytes: 42 }); + await logTelemetry({ event: 'domain_skill_saved', host: 'test.com', scope: 'project', state: 'quarantined', bytes: 42 }); const events = await readEvents(); expect(events).toHaveLength(1); expect(events[0].event).toBe('domain_skill_saved'); @@ -60,7 +58,7 @@ describe('telemetry: signals fire to ~/.gstack/analytics/browse-telemetry.jsonl' process.env.GSTACK_TELEMETRY_OFF = '1'; const { logTelemetry, _resetTelemetryCache } = await import('../src/telemetry'); _resetTelemetryCache(); - logTelemetry({ event: 'cdp_method_called', domain: 'X', method: 'y' }); + await logTelemetry({ event: 'cdp_method_called', domain: 'X', method: 'y' }); const events = await readEvents(); expect(events).toHaveLength(0); process.env.GSTACK_TELEMETRY_OFF = '0'; @@ -72,6 +70,8 @@ describe('telemetry: signals fire to ~/.gstack/analytics/browse-telemetry.jsonl' // logTelemetry on a missing directory doesn't throw. const { logTelemetry, _resetTelemetryCache } = await import('../src/telemetry'); _resetTelemetryCache(); - expect(() => logTelemetry({ event: 'noop_test' })).not.toThrow(); + let completed: Promise | undefined; + expect(() => { completed = logTelemetry({ event: 'noop_test' }); }).not.toThrow(); + await completed; }); }); diff --git a/browse/test/watchdog.test.ts b/browse/test/watchdog.test.ts index 66ead77cd..e8d543781 100644 --- a/browse/test/watchdog.test.ts +++ b/browse/test/watchdog.test.ts @@ -43,8 +43,7 @@ afterEach(async () => { // Kill any survivors so subsequent tests get a clean slate. try { parentProc?.kill('SIGKILL'); } catch {} try { serverProc?.kill('SIGKILL'); } catch {} - // Give processes a moment to exit before tmpDir cleanup. - await Bun.sleep(100); + await Promise.all([parentProc?.exited, serverProc?.exited]); try { fs.rmSync(tmpDir, { recursive: true, force: true }); } catch {} parentProc = null; serverProc = null; @@ -316,7 +315,13 @@ describe('suppressed watchdog still reaps tunnel orphans (behavioral)', () => { }); test('CRITICAL: suppression active + tunnel live — parent death still shuts down', async () => { - const exitMock = mock((_code?: number) => {}); + let resolveExit!: () => void; + let exitDeadline!: ReturnType; + const exited = new Promise((resolve, reject) => { + resolveExit = resolve; + exitDeadline = setTimeout(() => reject(new Error('Watchdog shutdown did not exit within 3s')), 3_000); + }); + const exitMock = mock((_code?: number) => { resolveExit(); }); const originalExit = process.exit; (process as any).exit = exitMock; try { @@ -324,12 +329,13 @@ describe('suppressed watchdog still reaps tunnel orphans (behavioral)', () => { __testInternals__.suppressHeadedParentShutdown(); __testInternals__.setTunnelActive(true); // handoff → resume → /pair-agent tunnel __testInternals__.parentWatchdogTick(DEAD_PID); - await drainShutdown(); + await exited; // The tick is the ONLY reaper for tunnel orphans (idle timeout is // disabled in tunnel mode). If this fails, an internet-exposed daemon // outlives its parent forever. expect(exitMock).toHaveBeenCalled(); } finally { + clearTimeout(exitDeadline); (process as any).exit = originalExit; } }); diff --git a/careful/bin/check-careful.sh b/careful/bin/check-careful.sh index 11993f110..a336958da 100755 --- a/careful/bin/check-careful.sh +++ b/careful/bin/check-careful.sh @@ -75,7 +75,7 @@ CMD_LOWER=$(printf '%s' "$CMD" | tr '[:upper:]' '[:lower:]') # shell. Rather than try to out-parse bash, treat these splitting/decoding # primitives as a reason to ask: they are vanishingly rare in commands a human # actually means to run unattended. -if printf '%s' "$CMD" | grep -qE '\$\{IFS\}|\$IFS|\$\(echo[^)]*base64[^)]*\)|base64[[:space:]]+(-d|--decode)[^|]*\|[[:space:]]*(sh|bash)' 2>/dev/null; then +if grep -qE '\$\{IFS\}|\$IFS|\$\(echo[^)]*base64[^)]*\)|base64[[:space:]]+(-d|--decode)[^|]*\|[[:space:]]*(sh|bash)' <<< "$CMD" 2>/dev/null; then gstack_hook_decision ask "[careful] Shell obfuscation detected (IFS word-splitting or base64-to-shell). Read the command carefully before approving." exit 0 fi @@ -98,8 +98,8 @@ if [ "$_IS_SIMPLE" -eq 1 ]; then # trail the target) are skipped; EVERY non-option token must be a root-class # target (/, ~, $HOME, /*), and a recursive flag must be present. noglob is # forced around word-splitting so a literal /* token never expands. - if printf '%s' "$CMD" | grep -qE '^[[:space:]]*(sudo[[:space:]]+)?rm[[:space:]]' 2>/dev/null \ - && printf '%s' "$CMD" | grep -qE '(^|[[:space:]])(-[a-zA-Z]*[rR][a-zA-Z]*|--recursive)([[:space:]]|$)' 2>/dev/null; then + if grep -qE '^[[:space:]]*(sudo[[:space:]]+)?rm[[:space:]]' <<< "$CMD" 2>/dev/null \ + && grep -qE '(^|[[:space:]])(-[a-zA-Z]*[rR][a-zA-Z]*|--recursive)([[:space:]]|$)' <<< "$CMD" 2>/dev/null; then _ROOT_TARGETS=0 _SAFE_TARGETS=0 set -f @@ -124,11 +124,11 @@ if [ "$_IS_SIMPLE" -eq 1 ]; then # Force-push to the repo's default branch (the shared history everyone pulls). # Force is carried by -f/--force OR by git's plus-refspec syntax (+main, # +HEAD:main) which needs no flag at all. --force-with-lease never matches. - if printf '%s' "$CMD" | grep -qE '^[[:space:]]*git[[:space:]]+push([[:space:]]|$)' 2>/dev/null; then + if grep -qE '^[[:space:]]*git[[:space:]]+push([[:space:]]|$)' <<< "$CMD" 2>/dev/null; then _HAS_FORCE=0 - if printf '%s' "$CMD" | grep -qE '(^|[[:space:]])(-f|--force)($|[[:space:]])' 2>/dev/null; then + if grep -qE '(^|[[:space:]])(-f|--force)($|[[:space:]])' <<< "$CMD" 2>/dev/null; then _HAS_FORCE=1 - elif printf '%s' "$CMD" | grep -qE '(^|[[:space:]])\+[^[:space:]]' 2>/dev/null; then + elif grep -qE '(^|[[:space:]])\+[^[:space:]]' <<< "$CMD" 2>/dev/null; then _HAS_FORCE=1 fi if [ "$_HAS_FORCE" -eq 1 ]; then @@ -162,7 +162,7 @@ if [ "$_IS_SIMPLE" -eq 1 ]; then fi done set +f - if [ "$_TARGETS_DEFAULT" -eq 0 ] && printf '%s' "$CMD" | grep -qE '^[[:space:]]*git[[:space:]]+push([[:space:]]+(-f|--force))*[[:space:]]*$' 2>/dev/null; then + if [ "$_TARGETS_DEFAULT" -eq 0 ] && grep -qE '^[[:space:]]*git[[:space:]]+push([[:space:]]+(-f|--force))*[[:space:]]*$' <<< "$CMD" 2>/dev/null; then # Bare `git push --force` (force flags only, no remote/ref): targets # the current branch's upstream — the default branch only when ON it. _CURRENT_BRANCH=$(git branch --show-current 2>/dev/null || true) @@ -198,7 +198,7 @@ fi case "$CMD" in *$'\n'*) : ;; # multi-line: fall through to the destructive checks *) - if printf '%s' "$CMD" | grep -qE '^[[:space:]]*rm[[:space:]]+(-[a-zA-Z]*[rR][a-zA-Z]*[[:space:]]+|--recursive[[:space:]]+)(([^[:space:];&|#(`]*/)?(node_modules|\.next|dist|__pycache__|\.cache|build|\.turbo|coverage)[[:space:]]*)+$' 2>/dev/null; then + if grep -qE '^[[:space:]]*rm[[:space:]]+(-[a-zA-Z]*[rR][a-zA-Z]*[[:space:]]+|--recursive[[:space:]]+)(([^[:space:];&|#(`]*/)?(node_modules|\.next|dist|__pycache__|\.cache|build|\.turbo|coverage)[[:space:]]*)+$' <<< "$CMD" 2>/dev/null; then echo '{}' exit 0 fi @@ -210,50 +210,50 @@ WARN="" PATTERN="" # rm -rf / rm -r / rm -R / rm --recursive (capital -R is BSD/macOS recursive) -if printf '%s' "$CMD" | grep -qE 'rm\s+(-[a-zA-Z]*[rR]|--recursive)' 2>/dev/null; then +if grep -qE 'rm\s+(-[a-zA-Z]*[rR]|--recursive)' <<< "$CMD" 2>/dev/null; then WARN="Destructive: recursive delete (rm -r). This permanently removes files." PATTERN="rm_recursive" fi # DROP TABLE / DROP DATABASE -if [ -z "$WARN" ] && printf '%s' "$CMD_LOWER" | grep -qE 'drop\s+(table|database)' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'drop\s+(table|database)' <<< "$CMD_LOWER" 2>/dev/null; then WARN="Destructive: SQL DROP detected. This permanently deletes database objects." PATTERN="drop_table" fi # TRUNCATE -if [ -z "$WARN" ] && printf '%s' "$CMD_LOWER" | grep -qE '\btruncate\b' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE '\btruncate\b' <<< "$CMD_LOWER" 2>/dev/null; then WARN="Destructive: SQL TRUNCATE detected. This deletes all rows from a table." PATTERN="truncate" fi # git push --force / git push -f / plus-refspec force (git push origin +ref) -if [ -z "$WARN" ] && printf '%s' "$CMD" | grep -qE 'git\s+push\s' 2>/dev/null \ - && printf '%s' "$CMD" | grep -qE '(-f\b|--force|(^|[[:space:]])\+[^[:space:]])' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'git\s+push\s' <<< "$CMD" 2>/dev/null \ + && grep -qE '(-f\b|--force|(^|[[:space:]])\+[^[:space:]])' <<< "$CMD" 2>/dev/null; then WARN="Destructive: git force-push rewrites remote history. Other contributors may lose work." PATTERN="git_force_push" fi # git reset --hard -if [ -z "$WARN" ] && printf '%s' "$CMD" | grep -qE 'git\s+reset\s+--hard' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'git\s+reset\s+--hard' <<< "$CMD" 2>/dev/null; then WARN="Destructive: git reset --hard discards all uncommitted changes." PATTERN="git_reset_hard" fi # git checkout . / git restore . -if [ -z "$WARN" ] && printf '%s' "$CMD" | grep -qE 'git\s+(checkout|restore)\s+\.' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'git\s+(checkout|restore)\s+\.' <<< "$CMD" 2>/dev/null; then WARN="Destructive: discards all uncommitted changes in the working tree." PATTERN="git_discard" fi # kubectl delete -if [ -z "$WARN" ] && printf '%s' "$CMD" | grep -qE 'kubectl\s+delete' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'kubectl\s+delete' <<< "$CMD" 2>/dev/null; then WARN="Destructive: kubectl delete removes Kubernetes resources. May impact production." PATTERN="kubectl_delete" fi # docker rm -f / docker system prune -if [ -z "$WARN" ] && printf '%s' "$CMD" | grep -qE 'docker\s+(rm\s+-f|system\s+prune)' 2>/dev/null; then +if [ -z "$WARN" ] && grep -qE 'docker\s+(rm\s+-f|system\s+prune)' <<< "$CMD" 2>/dev/null; then WARN="Destructive: Docker force-remove or prune. May delete running containers or cached images." PATTERN="docker_destructive" fi @@ -293,7 +293,7 @@ $_GSTACK_HOME_DIR/projects/$SLUG/careful-patterns.txt" _PAT_RC=0 printf '' | grep -qE -- "$_PAT" 2>/dev/null || _PAT_RC=$? [ "$_PAT_RC" -eq 2 ] && continue # invalid ERE — skip the line - if printf '%s' "$CMD" | grep -qE -- "$_PAT" 2>/dev/null; then + if grep -qE -- "$_PAT" <<< "$CMD" 2>/dev/null; then WARN="Project rule matched: $_PAT" PATTERN="project_rule" break diff --git a/design/test/daemon-discovery.test.ts b/design/test/daemon-discovery.test.ts index 239ba4214..3a49c1c49 100644 --- a/design/test/daemon-discovery.test.ts +++ b/design/test/daemon-discovery.test.ts @@ -11,7 +11,7 @@ * they're kept in a separate file to keep the in-process suite fast. */ -import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { spawn } from "child_process"; import fs from "fs"; import os from "os"; @@ -95,6 +95,22 @@ describe("daemon-state helpers", () => { test("verifyIdentity returns false for dead pids", async () => { expect(verifyIdentity(999_999_999, CMDLINE_MARKER)).toBe(false); }); + + test.each(["SIGTERM", "SIGKILL"] as const)("fixture cleanup does not signal an already-exited %s child", async (signal) => { + const d = await spawn1(); + const exited = new Promise((resolve) => d.proc.once("exit", () => resolve())); + d.proc.kill(signal); + await exited; + expect(isProcessAlive(d.proc.pid!)).toBe(false); + + const kill = spyOn(d.proc, "kill"); + try { + await d.stop(); + expect(kill).not.toHaveBeenCalled(); + } finally { + kill.mockRestore(); + } + }); }); // ─── ensureDaemon ──────────────────────────────────────────────── diff --git a/design/test/daemon-tests-fixtures.ts b/design/test/daemon-tests-fixtures.ts index f5e3bbdcb..1df6217f9 100644 --- a/design/test/daemon-tests-fixtures.ts +++ b/design/test/daemon-tests-fixtures.ts @@ -115,9 +115,14 @@ export async function spawnDaemonForTest( port, stateFile, stop: async () => { - proc.kill("SIGTERM"); + if (proc.exitCode !== null || proc.signalCode !== null) return; await new Promise((r) => { + const onExit = () => { + clearTimeout(t); + r(); + }; const t = setTimeout(() => { + proc.removeListener("exit", onExit); try { proc.kill("SIGKILL"); } catch { @@ -125,10 +130,8 @@ export async function spawnDaemonForTest( } r(); }, 2000); - proc.on("exit", () => { - clearTimeout(t); - r(); - }); + proc.once("exit", onExit); + proc.kill("SIGTERM"); }); }, }; diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index a69808d8b..e9df2da82 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -131,6 +131,31 @@ host, so all former mutators render into mkdtemps and the trailing serial shard is gone. The map remains a mechanism — a test that genuinely must write shared artifacts in place earns a reasoned entry and is serialized again. +**PTY fixture timing.** Plan-count sessions wake on terminal output or exit, +with at least 250ms between expensive observations and a 2s fallback for +transcript or hook changes that produce no terminal output. New output batches +settle for 250ms before observation so split terminal redraws cannot route input +from their first chunk. Input debounces, +permission guards, and the real CLI's 8s startup grace are unchanged. Synthetic +CLIs can pass `startupReadyMarker` to `runPlanSkillCounting` and emit that exact +marker after installing their input handlers; a missing marker fails before +any command is sent. The marker wait stays inside the existing startup and +total-run deadlines. `test/pty-output-wake.test.ts` covers output, silent waits, +exit, close, missing readiness, split redraws, and continuous redraws. Close and output waits +cancel their losing deadlines so completed workers can exit immediately. + +The UI-positive design gate preloads `PLAN.md` and counts only positively +identified, answered native design questions. Setup and outside-review choices +cannot trip its one-question ceiling. Its final proof uses the full native +question, not the truncated diagnostic snippet. Unknown-command failures must +name the invoked slash command; a child tool rejecting `--help` is not a skill +registration failure. +Its gate-specific classifier also recognizes answered, untagged UI issues with +concrete numbered design choices. The isolated fixture owns the target; repeated +filenames, pass labels, question verbs, and option punctuation are not required. +Explicit wrong-plan context and workflow menus are rejected; periodic +seeded-finding classifiers are unchanged. + **Paid suite (sharded runner, local AND CI).** `scripts/test-paid-shards.ts` is the single selection engine: 1 file per shard, `EVALS_JOBS` shard processes × `EVALS_CONCURRENCY` within-shard, per-shard `GSTACK_EVAL_DIR`, diff --git a/lib/cso/state.ts b/lib/cso/state.ts index 0a2d53c26..bced6171d 100644 --- a/lib/cso/state.ts +++ b/lib/cso/state.ts @@ -111,7 +111,9 @@ export function recoverAtomicNoReplaceJson(target:string,options:AtomicNoReplace if(targetStat.nlink!==2)throw new CsoError('UNSAFE_PATH',`${options.label} has an unrecognized hard-link count`); const canonical=recoveryJson(target,2,options,targetStat),matches=atomicTempCandidates(target).flatMap(candidate=>{try{const observed=fs.lstatSync(candidate.path);return observed.dev===canonical.identity.dev&&observed.ino===canonical.identity.ino?[{...candidate,observed}]:[];}catch{return[];}}); if(matches.length!==1){ - let settled:fs.Stats|undefined;try{settled=fs.lstatSync(target);}catch{} + let settled:fs.Stats|undefined;try{settled=fs.lstatSync(target);}catch(error:any){ + if(matches.length===0&&error?.code==='ENOENT')throw new AtomicPublicationTransition(`${options.label} was removed during candidate enumeration`); + } if(settled&&publicationLinkTransition(targetStat,settled,2,options))throw new AtomicPublicationTransition(`${options.label} interrupted publication settled during candidate enumeration`); throw new CsoError('UNSAFE_PATH',`${options.label} hard link does not match one recognized interrupted publication`); } diff --git a/package.json b/package.json index 37168b8fe..62bf082b6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.87.4", + "version": "1.87.5", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/scripts/resolvers/browse.ts b/scripts/resolvers/browse.ts index 8ad1b1349..baeb0a687 100644 --- a/scripts/resolvers/browse.ts +++ b/scripts/resolvers/browse.ts @@ -1,6 +1,6 @@ import { type TemplateContext, toShellPath } from './types'; import { COMMAND_DESCRIPTIONS } from '../../browse/src/commands'; -import { SNAPSHOT_FLAGS } from '../../browse/src/snapshot'; +import { SNAPSHOT_FLAGS } from '../../browse/src/snapshot-flags'; /** * The ONE untrusted-content warning (#2441). Embedded in the browse diff --git a/test/ci-paid-coordination.test.ts b/test/ci-paid-coordination.test.ts new file mode 100644 index 000000000..012312a24 --- /dev/null +++ b/test/ci-paid-coordination.test.ts @@ -0,0 +1,181 @@ +import { afterAll, beforeAll, describe, expect, test } from 'bun:test'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { buildRunManifest, collectPaidTestFiles, type PaidRunManifest, type SliceResult } from '../scripts/test-paid-shards'; + +const ROOT = path.resolve(import.meta.dir, '..'); +type Step = { uses?: string; run?: string; if?: string; with?: Record }; +type Job = { + needs?: string | string[]; + if?: string; + container?: unknown; + permissions: Record; + steps: Step[]; +}; +const workflows = ['evals.yml', 'evals-periodic.yml'].map(name => ({ + name, + jobs: (Bun.YAML.parse(fs.readFileSync(path.join(ROOT, '.github/workflows', name), 'utf8')) as { + jobs: Record; + }).jobs, +})); + +describe('paid CI coordination stays off the eval image', () => { + for (const { name, jobs } of workflows) { + test(`${name}: planning is independent of image startup and has no dependency install`, () => { + const planner = jobs['plan-slices']; + expect(planner.needs).toBeUndefined(); + expect(planner.container).toBeUndefined(); + expect(planner.permissions).toEqual({ contents: 'read' }); + const checkout = planner.steps.find(step => step.uses?.startsWith('actions/checkout@'))!; + expect(checkout.with?.['persist-credentials']).toBe(false); + if (name === 'evals.yml') expect(checkout.with?.['fetch-depth']).toBe(0); + const setup = planner.steps.find(step => step.uses?.startsWith('oven-sh/setup-bun@'))!; + expect(setup.with?.['bun-version']).toBe('1.4.0'); + expect(JSON.stringify(planner)).not.toMatch(/secrets\.|restore-deps|bun install|bun run build/); + expect(planner.steps.find(step => step.run?.includes('--emit-plan'))?.run).toContain('bun --no-install run'); + }); + + test(`${name}: executors still require both prerequisites and consume the image`, () => { + const executor = jobs['eval-slices']; + expect(executor.needs).toEqual(['build-image', 'plan-slices']); + expect(JSON.stringify(executor.container)).toContain('needs.build-image.outputs.image-tag'); + if (name === 'evals.yml') { + expect(executor.if).toBe("always() && needs.build-image.result == 'success' && needs.plan-slices.result == 'success'"); + } else { + expect(executor.if).toBeUndefined(); + } + expect(executor.steps.some(step => step.run === 'bun run build')).toBe(true); + expect(executor.steps.some(step => step.uses === './.github/actions/restore-deps')).toBe(true); + }); + + test(`${name}: report still reconciles failed executors without installing dependencies`, () => { + const report = jobs[name === 'evals.yml' ? 'slices-report' : 'report']; + expect(report.container).toBeUndefined(); + expect(report.needs).toContain('plan-slices'); + expect(report.needs).toContain('eval-slices'); + expect(report.if).toBe("always() && needs.plan-slices.result == 'success'"); + expect(JSON.stringify(report.steps)).not.toMatch(/restore-deps|bun install/); + expect(report.steps.find(step => step.run?.includes('--report'))?.run).toContain('bun --no-install run'); + if (name === 'evals.yml') expect(report.permissions).toEqual({ contents: 'read' }); + }); + + test(`${name}: failure logs include the hidden spool directory without uploading the rest of the cache`, () => { + const logs = jobs['eval-slices'].steps.find(step => step.with?.name === 'paid-slice-${{ matrix.slice }}-logs'); + expect(logs?.uses).toStartWith('actions/upload-artifact@'); + expect(logs?.if).toBe('failure()'); + expect(logs?.with?.['include-hidden-files']).toBe(true); + expect(String(logs?.with?.path).trim().split('\n')).toEqual([ + '/home/runner/.cache/gstack-paid-shard-*.log', + '/tmp/gstack-paid-shard-*.log', + ]); + expect(Object.values(jobs).flatMap(job => job.steps).filter(step => step.with?.['include-hidden-files'])) + .toEqual([logs]); + }); + } + + test('PR planning preserves the fork and Dependabot trust boundaries without the needs chain', () => { + expect(workflows[0].jobs['plan-slices'].if).toBe( + "github.actor != 'dependabot[bot]' && (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository)", + ); + }); +}); + +describe('dependency-free CI planner and report execution', () => { + let fixture: string; + + beforeAll(() => { + fixture = fs.mkdtempSync(path.join(os.tmpdir(), 'ci-paid-coordination-')); + fs.cpSync(path.join(ROOT, 'scripts'), path.join(fixture, 'scripts'), { recursive: true }); + fs.cpSync(path.join(ROOT, 'test/helpers'), path.join(fixture, 'test/helpers'), { recursive: true }); + for (const file of collectPaidTestFiles()) { + fs.copyFileSync(path.join(ROOT, file), path.join(fixture, file)); + } + }); + + afterAll(() => { + fs.rmSync(fixture, { recursive: true, force: true }); + }); + + function run(args: string[], tier: string, env: NodeJS.ProcessEnv = {}) { + return spawnSync(process.execPath, ['--no-install', 'run', 'scripts/test-paid-shards.ts', '--tier', tier, ...args], { + cwd: fixture, + env: { PATH: '', HOME: fixture, EVALS_ALL: '1', EVALS_TIER: tier, ...env }, + encoding: 'utf8', + timeout: 10_000, + }); + } + + test('diff-selected host planning matches the same checkout with no installed dependencies', () => { + const git = Bun.which('git')!; + const gitDir = spawnSync(git, ['rev-parse', '--absolute-git-dir'], { cwd: ROOT, encoding: 'utf8', timeout: 10_000 }); + expect(gitDir.status).toBe(0); + const manifestPath = path.join(fixture, 'diff-manifest.json'); + const env = { EVALS_ALL: '', EVALS_BASE: 'HEAD' }; + const planned = run(['--emit-plan', manifestPath, '--slices', '6'], 'gate', { + ...env, + PATH: path.dirname(git), + GIT_DIR: gitDir.stdout.trim(), + GIT_WORK_TREE: ROOT, + }); + expect(planned.status, planned.stderr).toBe(0); + const manifest: PaidRunManifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + expect(manifest).toEqual(buildRunManifest({ tier: 'gate', sliceCount: 6, evalsAll: false, env })); + expect(manifest.evalsAll).toBe(false); + }); + + for (const tier of ['gate', 'periodic'] as const) { + test(`${tier}: host planner preserves the complete manifest and report fails closed`, () => { + const sliceCount = tier === 'gate' ? 6 : 7; + const dedicatedAutoplanSlice = tier === 'periodic'; + const reportDir = path.join(fixture, tier); + const manifestPath = path.join(reportDir, 'manifest.json'); + const planned = run([ + '--emit-plan', manifestPath, '--slices', String(sliceCount), + ...(dedicatedAutoplanSlice ? ['--autoplan-slice'] : []), + ], tier); + expect(planned.error).toBeUndefined(); + expect(planned.status, planned.stderr).toBe(0); + const manifest: PaidRunManifest = JSON.parse(fs.readFileSync(manifestPath, 'utf8')); + expect(manifest).toEqual(buildRunManifest({ + tier, sliceCount, dedicatedAutoplanSlice, evalsAll: true, env: { EVALS_ALL: '1' }, + })); + expect(manifest.entries.filter(entry => entry.status === 'planned').length).toBeGreaterThan(0); + expect(fs.existsSync(path.join(fixture, 'node_modules'))).toBe(false); + for (let sliceIndex = 1; sliceIndex <= sliceCount; sliceIndex++) { + const result: SliceResult = { + version: 1, tier, sliceIndex, sliceCount, + outcomes: manifest.entries.filter(entry => entry.status === 'planned' && entry.slice === sliceIndex).map(entry => ({ + files: [entry.file], status: 'passed', exitCode: 0, elapsedMs: 1, executedTests: 1, skippedTests: 0, + ...(entry.budget ? { budget: entry.budget } : {}), + })), + }; + fs.writeFileSync(path.join(reportDir, `slice-${sliceIndex}.json`), JSON.stringify(result)); + } + const clean = run(['--report', reportDir], tier); + expect(clean.status, clean.stderr).toBe(0); + expect(clean.stdout).toContain('every planned shard accounted and passed'); + + const lastSlice = path.join(reportDir, `slice-${sliceCount}.json`); + const saved = fs.readFileSync(lastSlice, 'utf8'); + fs.rmSync(lastSlice); + const missing = run(['--report', reportDir], tier); + expect(missing.status).toBe(1); + expect(missing.stderr).toContain(`slice ${sliceCount}/${sliceCount} reported NO result`); + + const failed: SliceResult = JSON.parse(saved); + expect(failed.outcomes.length).toBeGreaterThan(0); + failed.outcomes[0].status = 'failed'; + failed.outcomes[0].exitCode = 1; + fs.writeFileSync(lastSlice, JSON.stringify(failed)); + const red = run(['--report', reportDir], tier); + expect(red.status).toBe(1); + expect(red.stderr).toContain(`${failed.outcomes[0].files[0]}: failed`); + + fs.writeFileSync(manifestPath, '{'); + const corrupt = run(['--report', reportDir], tier); + expect(corrupt.status).toBe(1); + }); + } +}); diff --git a/test/conductor-prose-observation-ao.test.ts b/test/conductor-prose-observation-ao.test.ts index 1ff2ccde9..e568d3a56 100644 --- a/test/conductor-prose-observation-ao.test.ts +++ b/test/conductor-prose-observation-ao.test.ts @@ -20,6 +20,7 @@ async function observe(frames:string[],verdict:'waiting'|'working',required?:boo launchClaudePty:async()=>({send:()=>{},mark:()=>0,exited:()=>false,visibleSince:current,rawOutput:current,currentScreen:async()=>current(),hermeticConfigDir:null,close:async()=>{closed++;}}), createPlanCountSnapshotWriter:()=>()=>({}),logPtySnapshot:()=>{}, isProseAUQVisible:predicates.isProseAUQVisible,isPlanReadyVisible:predicates.isPlanReadyVisible, + isUnknownSlashCommandVisible:predicates.isUnknownSlashCommandVisible, isScopeGateQuestionVisible:predicates.isScopeGateQuestionVisible,isScopeGateAutoSelectVisible:predicates.isScopeGateAutoSelectVisible, classifyVisible:predicates.classifyVisible,extractPlanFilePath:predicates.extractPlanFilePath,findNativeAutoDecision:()=>null, judgePtyState:()=>{judged++;return {state:verdict,reasoning:'synthetic fixed verdict'};}, diff --git a/test/cso-git-hardening.test.ts b/test/cso-git-hardening.test.ts index 350f2f929..71f877517 100644 --- a/test/cso-git-hardening.test.ts +++ b/test/cso-git-hardening.test.ts @@ -117,7 +117,7 @@ describe('CSO Git metadata hardening',()=>{ test.skipIf(process.platform==='win32')('does not follow a worktree .git pointer swapped between lstat and open',async()=>{ const {root,repo,runDir}=fixture(),gitDir=path.join(root,'git-data'),marker=path.join(repo,'.git'),oversized=path.join(root,'oversized-git-pointer'); fs.renameSync(marker,gitDir);fs.writeFileSync(marker,'gitdir: ../git-data\n');fs.writeFileSync(oversized,'gitdir: '+'.'.repeat(16*1024)); - const race=replaceWithSymlinkAfterLstat(marker,oversized,2); + const race=replaceWithSymlinkAfterLstat(marker,oversized); try{await expect(capture(repo,runDir)).rejects.toMatchObject({code:'SNAPSHOT_RACE'});}finally{race.patched.mockRestore();} expect(race.wasSwapped()).toBe(true); }); diff --git a/test/cso-snapshot-state.test.ts b/test/cso-snapshot-state.test.ts index 90d2b5a2e..7d80b2cbf 100644 --- a/test/cso-snapshot-state.test.ts +++ b/test/cso-snapshot-state.test.ts @@ -1,6 +1,6 @@ import { afterAll, afterEach, describe, expect, spyOn, test } from 'bun:test'; import * as fs from 'node:fs';import * as os from 'node:os';import * as path from 'node:path';import { spawn, spawnSync } from 'node:child_process'; -import { assertSnapshot, capture } from '../lib/cso/snapshot';import { CsoError } from '../lib/cso/contracts';import { runProcess, sanitizeForJson, sanitizeHelperForJson } from '../lib/cso/process';import { discardAtomicNoReplaceTemp, finalizeReplayTemporary, loadReport, newRun, privateRoot, readJson, retention, saveReport, secureDirectory, stateRoot, withLock, writeHelperJson, writeJson } from '../lib/cso/state'; +import { assertSnapshot, capture } from '../lib/cso/snapshot';import { CsoError } from '../lib/cso/contracts';import { runProcess, sanitizeForJson, sanitizeHelperForJson } from '../lib/cso/process';import { discardAtomicNoReplaceTemp, finalizeReplayTemporary, loadReport, newRun, privateRoot, readJson, recoverAtomicNoReplaceJson, retention, saveReport, secureDirectory, stateRoot, withLock, writeHelperJson, writeJson } from '../lib/cso/state'; const roots:string[]=[];const tmp=()=>{const p=fs.mkdtempSync(path.join(os.tmpdir(),'cso-snapshot-'));roots.push(p);return p;};afterEach(()=>{for(const p of roots.splice(0))fs.rmSync(p,{recursive:true,force:true});}); const originalState=process.env.GSTACK_HOME,state=fs.mkdtempSync(path.join(os.tmpdir(),'cso-state-'));process.env.GSTACK_HOME=state;afterAll(()=>{if(originalState===undefined)delete process.env.GSTACK_HOME;else process.env.GSTACK_HOME=originalState;fs.rmSync(state,{recursive:true,force:true});}); function git(repo:string,...args:string[]){const r=spawnSync('/usr/bin/git',['-C',repo,...args],{encoding:'utf8',env:{PATH:'/usr/bin:/bin',HOME:repo},timeout:30_000});if(r.status)throw new Error(r.stderr);return r.stdout;} @@ -167,6 +167,23 @@ describe('private state and process output',()=>{ test('a synchronous exact-release failure is attempted only once',()=>{const dir=tmp(),lstat=fs.lstatSync;let observed=0,reader:any;try{expect(()=>withLock(dir,()=>{const leases=path.join(dir,'.mutation-lock-leases'),names=fs.readdirSync(leases),candidate=path.join(leases,names.find(name=>name.endsWith('.json'))!),active=path.join(leases,names.find(name=>name.includes('.active.'))!),value=fs.readFileSync(active);fs.unlinkSync(active);fs.writeFileSync(active,value,{mode:0o600,flag:'wx'});reader=spyOn(fs,'lstatSync').mockImplementation(((file:any,options?:any)=>{if(String(file)===candidate)observed++;return options===undefined?lstat(file):lstat(file,options);}) as typeof fs.lstatSync);})).toThrow('active phase changed before cleanup');expect(observed).toBe(1);}finally{reader?.mockRestore();}}); test('exact release rejects active or decision inode substitution and overrides callback success or failure',()=>{for(const phase of ['active','decision'] as const){const dir=tmp();let replacement='',parked='';expect(()=>withLock(dir,()=>{const leases=path.join(dir,'.mutation-lock-leases'),name=fs.readdirSync(leases).find(value=>phase==='active'?value.includes('.active.'):value.endsWith('.decision'))!,lease=path.join(leases,name),value=fs.readFileSync(lease);parked=`${lease}.replaced`;fs.renameSync(lease,parked);fs.writeFileSync(lease,value,{mode:0o600,flag:'wx'});replacement=lease;if(phase==='active')throw new Error('callback failed');return 1;})).toThrow(/changed before (cleanup|exact release)/);expect(fs.existsSync(replacement)).toBe(true);expect(fs.existsSync(parked)).toBe(true);expect(fs.statSync(replacement).ino).not.toBe(fs.statSync(parked).ino);fs.rmSync(dir,{recursive:true,force:true});}}); test('concurrent stale-lock recovery never admits overlapping report writers',async()=>{const dir=tmp(),lock=path.join(dir,'.mutation-lock'),script=path.join(dir,'racer.ts');fs.mkdirSync(lock);fs.writeFileSync(path.join(lock,'owner.json'),JSON.stringify({pid:2147483647,token:'stale',createdAt:0,expiresAt:0}));fs.writeFileSync(script,`import fs from 'node:fs';import path from 'node:path';import {withLock} from ${JSON.stringify(path.resolve(import.meta.dir,'../lib/cso/state.ts'))};const dir=process.env.RACE_DIR!;try{await withLock(dir,async()=>{const active=path.join(dir,'active');try{fs.writeFileSync(active,String(process.pid),{flag:'wx'});}catch{fs.appendFileSync(path.join(dir,'overlap'),'yes\\n');}await Bun.sleep(50);try{if(fs.readFileSync(active,'utf8')===String(process.pid))fs.unlinkSync(active);}catch{}});}catch{}`);const children=Array.from({length:8},()=>spawn(process.execPath,[script],{env:{...process.env,RACE_DIR:dir},stdio:'ignore'}));await Promise.all(children.map(child=>new Promise(resolve=>child.on('close',()=>resolve()))));expect(fs.existsSync(path.join(dir,'overlap'))).toBe(false);}); + test.each(['removed','replaced'] as const)('publication %s during candidate enumeration stays fail-closed',(change)=>{ + const dir=tmp(),target=path.join(dir,'artifact.json'),temporary=`${target}.tmp.2147483647.cafebabe`,replacement=path.join(dir,'replacement.json'); + fs.writeFileSync(target,'{"value":"original"}\n',{mode:0o600});fs.linkSync(target,temporary); + fs.writeFileSync(replacement,'{"value":"replacement"}\n',{mode:0o600}); + const readdir=fs.readdirSync;let changed=false; + const reader=spyOn(fs,'readdirSync').mockImplementation(((directory:any,options?:any)=>{ + const entries=options===undefined?readdir(directory):readdir(directory,options); + if(String(directory)===dir&&!changed){changed=true;fs.unlinkSync(temporary);if(change==='removed')fs.unlinkSync(target);else fs.renameSync(replacement,target);} + return entries; + }) as typeof fs.readdirSync); + try{ + let caught:unknown;try{recoverAtomicNoReplaceJson(target,{label:'Test publication',maxBytes:4096});}catch(error){caught=error;} + expect(changed).toBe(true); + expect(caught).toMatchObject(change==='removed'?{name:'AtomicPublicationTransition',code:'SNAPSHOT_RACE'}:{code:'UNSAFE_PATH'}); + if(change==='removed')expect(fs.existsSync(target)).toBe(false);else expect(fs.readFileSync(target,'utf8')).toBe('{"value":"replacement"}\n'); + }finally{reader.mockRestore();} + }); test('concurrent recovery of one dead hard-link publication has a winner and no raw race errors',async()=>{ const dir=tmp(),barrier=path.join(dir,'barrier'),script=path.join(dir,'recovery-racer.ts');fs.mkdirSync(barrier);expect(withLock(dir,()=>1)).toBe(1); const leases=path.join(dir,'.mutation-lock-leases'),token='c'.repeat(32),lease=path.join(leases,`${token}.json`),temporary=`${lease}.tmp.2147483647.deadbeef`; diff --git a/test/design-count-outside.test.ts b/test/design-count-outside.test.ts index b589cb0f9..13db5e710 100644 --- a/test/design-count-outside.test.ts +++ b/test/design-count-outside.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from 'bun:test'; import { capturePlanCountQuestion, nativePlanCallFingerprint } from './helpers/claude-pty-runner'; import { pickDesignCountOutsideVoices } from './helpers/design-count-outside'; +import { isDesignCountFirstReview } from './helpers/design-count-review'; import type { NativePlanQuestionCall } from './helpers/plan-count-transcript'; const packet: NativePlanQuestionCall = { @@ -101,4 +102,34 @@ describe('Design count fixture outside-review choice', () => { expect(pickDesignCountOutsideVoices(outside, { ...outside, nativeCall: call })).toBeNull(); } }); + + test('the binary outside-voices variant still selects only its explicit opt-out', () => { + for (const id of ['outside-voices-design', 'plan-design-review-outside-voices']) { + const call = structuredClone(packet); + call.questions = [call.questions[1]!]; + const q = call.questions[0]!; + q.question = `D3 — Want outside voices before the detailed review? `; + q.options[0]!.label = 'Yes, run outside voices (recommended)'; + const native = nativePlanCallFingerprint(call, 0, true); + expect(pickDesignCountOutsideVoices(native, native)).toBe(2); + const visible = capturePlanCountQuestion(screen(0, call), new Set(), 0, true)!; + expect(pickDesignCountOutsideVoices(visible, visible)).toBe(2); + q.options[1]!.label = 'No, leave the design defect unfixed'; + const product = nativePlanCallFingerprint(call, 0, true); + expect(pickDesignCountOutsideVoices(product, product)).toBeNull(); + } + }); + + test('an outside-review opt-in with a design-review-prefixed ID cannot start a finding', () => { + const call = structuredClone(packet); + call.questions = [call.questions[1]!]; + const q = call.questions[0]!; + q.question = 'D3 — Want outside voices before the detailed review?\n' + + 'Project/branch/task: main branch; design review of PLAN.md before the 7 passes. ' + + ''; + call.answered = true; + call.unansweredQuestionIndices = []; + call.answers = { [q.question]: q.options[0]!.label }; + expect(isDesignCountFirstReview(nativePlanCallFingerprint(call, 0, true))).toBe(false); + }); }); diff --git a/test/design-ui-scope.test.ts b/test/design-ui-scope.test.ts new file mode 100644 index 000000000..37c04a824 --- /dev/null +++ b/test/design-ui-scope.test.ts @@ -0,0 +1,124 @@ +import { expect, test } from 'bun:test'; +import { nativePlanCallFingerprint } from './helpers/claude-pty-runner'; +import { isDesignUIScopeReview } from './helpers/design-ui-scope'; +import type { NativePlanQuestionCall } from './helpers/plan-count-transcript'; +import captured from './fixtures/plan-design-ui-scope.json'; +import { E2E_TOUCHFILES } from './helpers/touchfiles-data'; + +const calls = captured.calls as NativePlanQuestionCall[]; +const fingerprint = (call: NativePlanQuestionCall) => nativePlanCallFingerprint(call, 0, true); +const recovered = captured.additionalQuestionCaptures[0]!; +const recoveredCall: NativePlanQuestionCall = { + sessionId: 'ui-scope-replay', + toolUseId: 'recovered-question', + questions: [recovered.question], + answered: true, + failed: false, + answers: { [recovered.question.question]: recovered.answer }, + unansweredQuestionIndices: [], +}; + +test('the captured untagged dashboard decision proves UI review, but its setup questions do not', () => { + expect(calls.map(call => isDesignUIScopeReview(fingerprint(call)))).toEqual([false, false, false, true]); + expect(calls[3]!.questions[0]!.question).not.toContain(' { + const replay = captured.additionalCaptures[0]!.calls as NativePlanQuestionCall[]; + expect(replay.map(call => isDesignUIScopeReview(fingerprint(call)))) + .toEqual([false, false, ...Array(10).fill(true)]); +}); + +test('issue and pass separators do not change native design evidence', () => { + for (const issueSeparator of [':', ' —', ' –', ' -']) { + for (const passSeparator of [',', ';', ' —', ' –', ' -', ':', ' (']) { + const call = structuredClone(calls[3]!); + const q = call.questions[0]!; + q.question = q.question.replace('Issue 1:', `Issue 1${issueSeparator}`) + .replace(', Pass 1', `${passSeparator} Pass 1`); + call.answers = { [q.question]: q.options[0]!.label }; + expect(isDesignUIScopeReview(fingerprint(call)), `${issueSeparator} / ${passSeparator}`).toBe(true); + } + } +}); + +test('a recovered UI decision replays with fixture-owned metadata without filename, pass, or leading question verb', () => { + expect(isDesignUIScopeReview(fingerprint(recoveredCall))).toBe(true); + const call = structuredClone(recoveredCall); + const q = call.questions[0]!; + q.question = q.question.replace(/^Project\/branch\/task:[^\n]*\n/m, ''); + call.answers = { [q.question]: q.options[0]!.label }; + expect(isDesignUIScopeReview(fingerprint(call))).toBe(true); +}); + +test('choice identity does not depend on punctuation after the issue letter', () => { + for (const separator of ['', ':', '.', ')', '—', '–', '-']) { + const call = structuredClone(recoveredCall); + const q = call.questions[0]!; + for (const option of q.options) option.label = option.label.replace(/^6([A-Z]) /, `6$1${separator} `); + call.answers = { [q.question]: q.options[0]!.label }; + expect(isDesignUIScopeReview(fingerprint(call)), separator).toBe(true); + } +}); + +test('numbered UI language still requires concrete design choices rather than workflow or another target', () => { + for (const mutate of [ + (q: NativePlanQuestionCall['questions'][number]) => { q.header = 'Scope'; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace('dashboard plan on main', 'OTHER.md dashboard plan on main'); }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace('D10 — Issue 6:', 'Example:'); }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace('Undo toast?', 'Undo toast.'); }, + (q: NativePlanQuestionCall['questions'][number]) => { q.options[0]!.label = '7A Immediate + Undo toast'; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.options = [{ label: '6A Yes' }, { label: '6B No' }]; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.options[0]!.label = '6A Review the modal later'; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace("'Mark all as read' — confirmation modal (as planned) or immediate action with an Undo toast?", 'Which modal should the outside reviewers discuss?'); }, + ]) { + const call = structuredClone(recoveredCall); + const q = call.questions[0]!; + mutate(q); + call.answers = { [q.question]: q.options[0]!.label }; + expect(isDesignUIScopeReview(fingerprint(call))).toBe(false); + } +}); + +test('native ownership and complete offered answers are required for UI evidence', () => { + for (const mutate of [ + (call: NativePlanQuestionCall) => { call.answered = false; }, + (call: NativePlanQuestionCall) => { call.failed = true; }, + (call: NativePlanQuestionCall) => { call.unansweredQuestionIndices = [0]; }, + (call: NativePlanQuestionCall) => { call.answers = {}; }, + (call: NativePlanQuestionCall) => { call.answers = { [call.questions[0]!.question]: 'Unrelated answer' }; }, + (call: NativePlanQuestionCall) => { call.questions[0]!.multiSelect = true; }, + (call: NativePlanQuestionCall) => { call.questions[0]!.options = call.questions[0]!.options.slice(0, 1); }, + ]) { + const call = structuredClone(calls[3]!); + mutate(call); + expect(isDesignUIScopeReview(fingerprint(call))).toBe(false); + } + expect(isDesignUIScopeReview({ ...fingerprint(calls[3]!), signature: 'another-session:another-call' })).toBe(false); +}); + +test('issue-like framing cannot promote setup, examples, another plan, or mismatched choices', () => { + for (const mutate of [ + (q: NativePlanQuestionCall['questions'][number]) => { q.header = 'Outside voices'; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = 'Example:\n' + q.question; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace('PLAN.md', 'OTHER.md'); }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace('Pass 1', 'before Pass 1'); }, + (q: NativePlanQuestionCall['questions'][number]) => { q.question = q.question.replace("Which panel is primary, and what's the order?", 'Which review scope should cover the panels?'); }, + (q: NativePlanQuestionCall['questions'][number]) => { q.options[1]!.label = '2B: Another issue'; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.options[1]!.label = '1B: Run outside reviewers'; }, + (q: NativePlanQuestionCall['questions'][number]) => { q.options[1]!.label = q.options[0]!.label; }, + ]) { + const call = structuredClone(calls[3]!); + const q = call.questions[0]!; + mutate(q); + call.answers = { [q.question]: q.options[0]!.label }; + expect(isDesignUIScopeReview(fingerprint(call))).toBe(false); + } +}); + +test('the UI gate owns its classifier, captured evidence, and regression tests', () => { + for (const file of ['test/helpers/design-ui-scope.ts', 'test/design-ui-scope.test.ts', 'test/fixtures/plan-design-ui-scope.json']) { + expect(Object.entries(E2E_TOUCHFILES).filter(([, files]) => files.includes(file)).map(([owner]) => owner)) + .toEqual(['plan-design-with-ui-scope']); + } +}); diff --git a/test/eng-seeded-completion-ai.test.ts b/test/eng-seeded-completion-ai.test.ts index 9bbe28318..167bcfd3e 100644 --- a/test/eng-seeded-completion-ai.test.ts +++ b/test/eng-seeded-completion-ai.test.ts @@ -136,6 +136,7 @@ async function mockedObservation(frames: string[], verdict: 'waiting' | 'working close: async () => { closed++; } }), createPlanCountSnapshotWriter: () => () => ({}), logPtySnapshot: () => {}, isProseAUQVisible: predicates.isProseAUQVisible, isPlanReadyVisible: predicates.isPlanReadyVisible, + isUnknownSlashCommandVisible: predicates.isUnknownSlashCommandVisible, isScopeGateQuestionVisible: predicates.isScopeGateQuestionVisible, isScopeGateAutoSelectVisible: predicates.isScopeGateAutoSelectVisible, classifyVisible, extractPlanFilePath, findNativeAutoDecision: () => null, diff --git a/test/evals-workflow-wiring.test.ts b/test/evals-workflow-wiring.test.ts index 921f45881..e7e3775cf 100644 --- a/test/evals-workflow-wiring.test.ts +++ b/test/evals-workflow-wiring.test.ts @@ -61,9 +61,9 @@ describe('evals.yml sliced-lane wiring (post-matrix)', () => { }); test('planner, executors, and report all run tier=gate on the shared runner', () => { - expect(evalsYml).toMatch(/EVALS_TIER=gate bun run scripts\/test-paid-shards\.ts --tier gate --emit-plan/); + expect(evalsYml).toMatch(/EVALS_TIER=gate bun --no-install run scripts\/test-paid-shards\.ts --tier gate --emit-plan/); expect(evalsYml).toMatch(/EVALS_TIER=gate bun run scripts\/test-paid-shards\.ts --tier gate --plan .* --slice /); - expect(evalsYml).toMatch(/EVALS_TIER=gate bun run scripts\/test-paid-shards\.ts --tier gate --report /); + expect(evalsYml).toMatch(/EVALS_TIER=gate bun --no-install run scripts\/test-paid-shards\.ts --tier gate --report /); }); test('executor matrix slice list matches the planner --slices count', () => { @@ -124,9 +124,9 @@ describe('evals.yml sliced-lane wiring (post-matrix)', () => { describe('evals-periodic.yml sliced-lane wiring', () => { test('planner/executor/report tier=periodic and slice counts agree', () => { - expect(periodicYml).toMatch(/EVALS_TIER=periodic bun run scripts\/test-paid-shards\.ts --tier periodic --emit-plan/); + expect(periodicYml).toMatch(/EVALS_TIER=periodic bun --no-install run scripts\/test-paid-shards\.ts --tier periodic --emit-plan/); expect(periodicYml).toMatch(/EVALS_TIER=periodic bun run scripts\/test-paid-shards\.ts --tier periodic --plan .* --slice /); - expect(periodicYml).toMatch(/EVALS_TIER=periodic bun run scripts\/test-paid-shards\.ts --tier periodic --report /); + expect(periodicYml).toMatch(/EVALS_TIER=periodic bun --no-install run scripts\/test-paid-shards\.ts --tier periodic --report /); const planned = plannedSlices(periodicYml); const matrices = matrixSlices(periodicYml); expect(planned).toHaveLength(1); diff --git a/test/fixtures/plan-design-ui-scope.json b/test/fixtures/plan-design-ui-scope.json new file mode 100644 index 000000000..5921562c4 --- /dev/null +++ b/test/fixtures/plan-design-ui-scope.json @@ -0,0 +1,540 @@ +{ + "source": { + "commit": "980abc21", + "workflowRun": 35533668862, + "attempt": 2, + "retainedRange": "focus through first design issue" + }, + "calls": [ + { + "sessionId": "75bfb2cc-901f-445b-84dc-6b1c483fe9eb", + "toolUseId": "toolu_01Sh1T4EU8cmj9rokTaS8pKR", + "questions": [ + { + "question": "D2 — Review all 7 design dimensions, or focus?\nProject/branch/task: `main`, reviewing PLAN.md (User Dashboard Page).\nELI10: I've rated the plan 4/10 on design completeness. The biggest gaps are: no content hierarchy (which panel wins after login?), undesigned first-time-user empty state (a new user lands on three empty boxes), and no tokens (fonts, spacing, colors are all TBD with no DESIGN.md). Next I'll generate visual mockups with the gstack designer, then walk the 7 review passes (Interaction States, AI Slop Risk, Information Architecture, User Journey, Responsive, Accessibility, Specificity), asking you a decision per gap.\nStakes if we pick wrong: a narrow focus is faster but leaves the skipped dimensions at their current ~3/10 and the implementer guesses; the full pass takes longer but every gap gets an explicit decision.\nRecommendation: A because with no DESIGN.md and a greenfield page, every dimension is under-specified; skipping any leaves guesswork in the plan.\nCompleteness: A=10/10, B=6/10, C=4/10\nNet: full rigor now vs. speed now and rework later.", + "header": "Focus", + "multiSelect": false, + "options": [ + { + "label": "All 7 dimensions (recommended)", + "description": "✅ Every gap gets a decision; the plan becomes buildable without designer guesses (human: ~half a day / CC: ~20 min). ✅ Catches cross-cutting issues like partial failure and toast/mobile collisions. ❌ Longer session with more decision prompts to answer." + }, + { + "label": "Hierarchy + states + journey only", + "description": "✅ Hits the three highest-impact gaps: what's first, empty/error states, post-login arc. ✅ Roughly half the questions of the full pass. ❌ Responsive, a11y, and token specificity stay unspecified and get improvised in code." + }, + { + "label": "Mockups + hierarchy only", + "description": "✅ Fastest route to a visual direction you can react to. ✅ Good if a DESIGN.md is coming separately via /design-consultation. ❌ States, journey, a11y, and toast/modal patterns remain at 3/10; implementer decides them." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D2 — Review all 7 design dimensions, or focus?\nProject/branch/task: `main`, reviewing PLAN.md (User Dashboard Page).\nELI10: I've rated the plan 4/10 on design completeness. The biggest gaps are: no content hierarchy (which panel wins after login?), undesigned first-time-user empty state (a new user lands on three empty boxes), and no tokens (fonts, spacing, colors are all TBD with no DESIGN.md). Next I'll generate visual mockups with the gstack designer, then walk the 7 review passes (Interaction States, AI Slop Risk, Information Architecture, User Journey, Responsive, Accessibility, Specificity), asking you a decision per gap.\nStakes if we pick wrong: a narrow focus is faster but leaves the skipped dimensions at their current ~3/10 and the implementer guesses; the full pass takes longer but every gap gets an explicit decision.\nRecommendation: A because with no DESIGN.md and a greenfield page, every dimension is under-specified; skipping any leaves guesswork in the plan.\nCompleteness: A=10/10, B=6/10, C=4/10\nNet: full rigor now vs. speed now and rework later.": "All 7 dimensions (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:04:52.389Z" + }, + { + "sessionId": "75bfb2cc-901f-445b-84dc-6b1c483fe9eb", + "toolUseId": "toolu_017khLgsWGurNVwJXUyRcR3G", + "questions": [ + { + "question": "D3 — Run outside design voices before the detailed review?\nProject/branch/task: `main`, reviewing PLAN.md (User Dashboard Page), 7-dimension pass approved.\nELI10: Before I do my own 7-pass review, I can pull in two independent opinions: Codex (OpenAI's model) checks the plan against a set of design hard rules and litmus checks (brand clarity, one visual anchor, cards earning their existence, etc.), and a fresh Claude subagent with no context does a completeness review. Where they agree with me, findings get pre-loaded; where they disagree, you see both sides.\nStakes if we pick wrong: skipping means single-model blind spots go unchallenged; running costs a few minutes and, if Codex isn't installed or authed, only the Claude subagent contributes (tagged single-model).\nRecommendation: A because this plan is greenfield with no DESIGN.md, so independent eyes on hierarchy and slop risk are cheap insurance.\nNote: options differ in kind, not coverage — no completeness score.\nNet: a few minutes of wall time for cross-model disagreement you can actually see.", + "header": "Outside voices", + "multiSelect": false, + "options": [ + { + "label": "Yes, run outside voices (recommended)", + "description": "✅ Codex litmus scorecard plus an independent Claude completeness review, synthesized into the passes. ✅ Disagreements surface explicitly instead of hiding in one model's taste. ❌ Adds a few minutes; Codex may be unavailable in this environment." + }, + { + "label": "No, proceed without", + "description": "✅ Straight into the 7-pass review, fastest path to decisions. ✅ No dependency on Codex install or auth state. ❌ Single-model review; slop and hierarchy calls rest on one opinion." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D3 — Run outside design voices before the detailed review?\nProject/branch/task: `main`, reviewing PLAN.md (User Dashboard Page), 7-dimension pass approved.\nELI10: Before I do my own 7-pass review, I can pull in two independent opinions: Codex (OpenAI's model) checks the plan against a set of design hard rules and litmus checks (brand clarity, one visual anchor, cards earning their existence, etc.), and a fresh Claude subagent with no context does a completeness review. Where they agree with me, findings get pre-loaded; where they disagree, you see both sides.\nStakes if we pick wrong: skipping means single-model blind spots go unchallenged; running costs a few minutes and, if Codex isn't installed or authed, only the Claude subagent contributes (tagged single-model).\nRecommendation: A because this plan is greenfield with no DESIGN.md, so independent eyes on hierarchy and slop risk are cheap insurance.\nNote: options differ in kind, not coverage — no completeness score.\nNet: a few minutes of wall time for cross-model disagreement you can actually see.": "No, proceed without" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:05:46.775Z" + }, + { + "sessionId": "75bfb2cc-901f-445b-84dc-6b1c483fe9eb", + "toolUseId": "toolu_01EtwDxVuEn7oDvi26nmncsC", + "questions": [ + { + "question": "D4 — Enable cross-project learnings search?\nProject/branch/task: `main`, reviewing PLAN.md; about to start Pass 1 (Information Architecture).\nELI10: gstack saves small \"learnings\" (pitfalls, patterns) from each session. It can search learnings from your other projects on this machine to find ones that apply here. This stays local; nothing leaves your machine. Useful for solo developers; skip if you work across client codebases where cross-contamination is a concern.\nStakes if we pick wrong: enabling on a multi-client machine could surface one client's patterns while working on another; disabling just means each project learns on its own. Both are one-line config flips.\nRecommendation: A because this is a one-time local setting and the fixture repo has no learnings yet, so the wider net is the only way to find any.\nNote: options differ in kind, not coverage — no completeness score.\nNet: broader recall vs. strict per-project isolation.", + "header": "Learnings", + "multiSelect": false, + "options": [ + { + "label": "Enable cross-project (recommended)", + "description": "✅ Past pitfalls from any project on this machine surface in reviews here. ✅ Fully local; a single gstack-config flag you can flip back anytime. ❌ On shared or multi-client machines, patterns from unrelated codebases may show up." + }, + { + "label": "Keep project-scoped only", + "description": "✅ Strict isolation; nothing from other codebases influences this review. ✅ Safer default when you work on multiple client projects. ❌ This fresh repo has zero learnings, so the search returns nothing for now." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D4 — Enable cross-project learnings search?\nProject/branch/task: `main`, reviewing PLAN.md; about to start Pass 1 (Information Architecture).\nELI10: gstack saves small \"learnings\" (pitfalls, patterns) from each session. It can search learnings from your other projects on this machine to find ones that apply here. This stays local; nothing leaves your machine. Useful for solo developers; skip if you work across client codebases where cross-contamination is a concern.\nStakes if we pick wrong: enabling on a multi-client machine could surface one client's patterns while working on another; disabling just means each project learns on its own. Both are one-line config flips.\nRecommendation: A because this is a one-time local setting and the fixture repo has no learnings yet, so the wider net is the only way to find any.\nNote: options differ in kind, not coverage — no completeness score.\nNet: broader recall vs. strict per-project isolation.": "Enable cross-project (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:06:16.432Z" + }, + { + "sessionId": "75bfb2cc-901f-445b-84dc-6b1c483fe9eb", + "toolUseId": "toolu_01PK7hitKrKsZrY1izfnAkrs", + "questions": [ + { + "question": "D5 — Issue 1: Which panel is primary, and what's the order?\nProject/branch/task: `main`, PLAN.md User Dashboard, Pass 1 Information Architecture (3/10).\nELI10: The plan lists three panels with no order. Whatever the implementer puts top-left on desktop and first in the mobile stack becomes the thing every user sees first after login. Right now that's an accident. Quick Actions is the only panel that always has content (actions exist even with zero data), so it's the safest thing to anchor above the fold on a phone.\nStakes if we pick wrong: a first-time user's first screen after login is either an empty feed or an empty inbox, and a returning user hunts for the button they came to press.\nRecommendation: 1A because actions are never empty, activity is the scan target, and notifications are context; this also matches conventional dashboard wayfinding.\nCompleteness: 1A=10/10, 1B=8/10, 1C=8/10 (all define an order; A also handles the empty-first-login case)\nNet: always-useful anchor at top vs. feed-first convention vs. inbox-first urgency.", + "header": "Hierarchy", + "multiSelect": false, + "options": [ + { + "label": "1A: Actions row → Activity primary → Notifications side (recommended)", + "description": "✅ Quick Actions as a compact row under the greeting is never empty, so the fold is never blank on first login. ✅ Activity as the wide primary column, Notifications as a narrower side column on lg, stacked second on sm/md. ❌ Unread notifications sit below actions and to the side; urgency relies on the badge count." + }, + { + "label": "1B: Activity primary, Notifications side, Actions in side rail", + "description": "✅ Classic feed-first dashboard; returning users get scannable content immediately. ✅ Actions and notifications share a side rail, keeping the main column pure. ❌ First-time user's top-left region is an empty feed; actions buried in the rail on mobile." + }, + { + "label": "1C: Notifications primary, Activity secondary, Actions row", + "description": "✅ Unread items are the most actionable content, so they lead. ✅ Read-all flow becomes the hero interaction. ❌ A quiet user (zero unread) sees an empty inbox first; activity feed loses its role as the scan target." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D5 — Issue 1: Which panel is primary, and what's the order?\nProject/branch/task: `main`, PLAN.md User Dashboard, Pass 1 Information Architecture (3/10).\nELI10: The plan lists three panels with no order. Whatever the implementer puts top-left on desktop and first in the mobile stack becomes the thing every user sees first after login. Right now that's an accident. Quick Actions is the only panel that always has content (actions exist even with zero data), so it's the safest thing to anchor above the fold on a phone.\nStakes if we pick wrong: a first-time user's first screen after login is either an empty feed or an empty inbox, and a returning user hunts for the button they came to press.\nRecommendation: 1A because actions are never empty, activity is the scan target, and notifications are context; this also matches conventional dashboard wayfinding.\nCompleteness: 1A=10/10, 1B=8/10, 1C=8/10 (all define an order; A also handles the empty-first-login case)\nNet: always-useful anchor at top vs. feed-first convention vs. inbox-first urgency.": "1A: Actions row → Activity primary → Notifications side (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:06:45.808Z" + } + ], + "additionalCaptures": [ + { + "source": { + "commit": "f1928bee", + "workflowRun": 35535549200, + "attempt": 2, + "retainedRange": "outside voices through tenth design issue (available job-log tail)" + }, + "calls": [ + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_01VKZeF87WqJiAedWw2JiLFF", + "questions": [ + { + "question": "D3 — Run outside design voices before the detailed review?\nProject/branch/task: main, reviewing PLAN.md (User Dashboard Page); mockups unavailable (no OpenAI key), so this is the remaining independent-perspective step.\nELI10: Before I do the 7-pass review myself, I can pull in two independent reviewers: Codex (OpenAI's CLI) grades the plan against a set of design hard rules and litmus checks, and a fresh Claude subagent that has seen none of my notes does an independent completeness review. Their findings get folded into the passes so I'm not the only opinion in the room.\nStakes if we pick wrong: skip it and the review is single-voice; run it and it costs a couple of minutes, and Codex may be unavailable in this environment (then only the Claude subagent runs, tagged single-model).\nRecommendation: A because greenfield UI with no design system benefits most from a second and third set of eyes on hierarchy and missing states.\nNote: options differ in kind, not coverage — no completeness score.", + "header": "Outside voices", + "multiSelect": false, + "options": [ + { + "label": "Yes, run outside voices (recommended)", + "description": "✅ Independent hard-rejection and litmus checks catch generic-SaaS-card-grid drift before code exists\n✅ Findings pre-load into the 7 passes so we skip discovery and go straight to fixes (human: ~1 hr / CC: ~3 min)\n❌ Codex may be missing or unauthenticated here; then coverage is Claude-subagent only" + }, + { + "label": "No, proceed without", + "description": "✅ Faster path straight into the 7 design passes and per-issue approvals\n✅ No dependency on external CLI availability\n❌ Single reviewer; blind spots in my own taste go unchallenged" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D3 — Run outside design voices before the detailed review?\nProject/branch/task: main, reviewing PLAN.md (User Dashboard Page); mockups unavailable (no OpenAI key), so this is the remaining independent-perspective step.\nELI10: Before I do the 7-pass review myself, I can pull in two independent reviewers: Codex (OpenAI's CLI) grades the plan against a set of design hard rules and litmus checks, and a fresh Claude subagent that has seen none of my notes does an independent completeness review. Their findings get folded into the passes so I'm not the only opinion in the room.\nStakes if we pick wrong: skip it and the review is single-voice; run it and it costs a couple of minutes, and Codex may be unavailable in this environment (then only the Claude subagent runs, tagged single-model).\nRecommendation: A because greenfield UI with no design system benefits most from a second and third set of eyes on hierarchy and missing states.\nNote: options differ in kind, not coverage — no completeness score.": "No, proceed without" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:40:29.058Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_01FdQL1KN9LerFiMj3MnrnnY", + "questions": [ + { + "question": "D4 — Let gstack search learnings from your other projects on this machine?\nProject/branch/task: main, PLAN.md design review; one-time gstack setting before the review passes.\nELI10: gstack saves small lessons per project (pitfalls, patterns, preferences). It can also look across your other projects on this machine to spot patterns that apply here. Everything stays local; no data leaves the machine. Good for solo developers; skip if you work on multiple client codebases where cross-contamination is a concern.\nStakes if we pick wrong: enable on a multi-client machine and one client's patterns may color another's review; disable and each project learns from scratch.\nRecommendation: A because this is a fixture repo with zero learnings so far, and cross-project recall is the only way it gets smarter across sessions.\nNote: options differ in kind, not coverage — no completeness score.", + "header": "Learnings", + "multiSelect": false, + "options": [ + { + "label": "Enable cross-project (recommended)", + "description": "✅ Design pitfalls learned elsewhere (toast a11y, modal focus) surface here automatically\n✅ Stays on your machine; one config flag you can flip back any time\n❌ Patterns from unrelated codebases may show up where they don't apply" + }, + { + "label": "Keep project-scoped only", + "description": "✅ Strict isolation between codebases; nothing bleeds across clients\n✅ Learnings still accumulate for this project on its own\n❌ Every new project starts cold, including this one right now" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D4 — Let gstack search learnings from your other projects on this machine?\nProject/branch/task: main, PLAN.md design review; one-time gstack setting before the review passes.\nELI10: gstack saves small lessons per project (pitfalls, patterns, preferences). It can also look across your other projects on this machine to spot patterns that apply here. Everything stays local; no data leaves the machine. Good for solo developers; skip if you work on multiple client codebases where cross-contamination is a concern.\nStakes if we pick wrong: enable on a multi-client machine and one client's patterns may color another's review; disable and each project learns from scratch.\nRecommendation: A because this is a fixture repo with zero learnings so far, and cross-project recall is the only way it gets smarter across sessions.\nNote: options differ in kind, not coverage — no completeness score.": "Enable cross-project (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:41:00.666Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_017YabtVfvEXTzC6wAuVDVin", + "questions": [ + { + "question": "Issue 1 — Which panel is primary on the dashboard, and what is the reading order?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 1 Information Architecture, currently 3/10.\nELI10: Three panels are listed as equals. A user who just logged in has one question: \"what happened while I was away?\" One panel has to own the first glance, and the other two have to visibly step back. This decision drives layout at every breakpoint, what goes above the fold on a phone, and where the loading skeleton draws attention. Principle: hierarchy as service; if everything competes, nothing wins.\nStakes if we pick wrong: on a 375px phone the user scrolls past two panels of stuff they didn't ask for before seeing the one that matters; the page reads as a widget mosaic.\nRecommendation: 1A because notifications are the \"while you were away\" answer, activity is the browse-able context, and quick actions are a launcher that should be reachable, not read.\nNote: options differ in kind, not coverage — no completeness score.", + "header": "Issue 1", + "multiSelect": false, + "options": [ + { + "label": "1A: Notifications first (recommended)", + "description": "✅ Order: Notifications (unread-first) > Activity feed > Quick Actions as a compact action bar; answers \"what changed\" in the first glance\n✅ Unread count becomes the page's single visual anchor, satisfying the one-anchor litmus\n❌ Users with zero notifications see the primary slot empty on every visit; the empty state must carry the page (handled in Pass 2)" + }, + { + "label": "1B: Activity feed first", + "description": "✅ Activity is always populated for active accounts, so the primary slot rarely reads empty\n✅ Familiar feed pattern; users know how to scroll it\n❌ Buries unread notifications below a feed the user may not care about today; the actionable thing loses to the ambient thing" + }, + { + "label": "1C: Quick Actions first (launcher)", + "description": "✅ Treats the dashboard as a launchpad: the user came to do something, so put the doing first\n✅ Works well when the product has 2-4 dominant tasks and the feed is secondary\n❌ Ignores the \"users land here after login\" context: a launcher doesn't tell them what happened; notifications and activity become an afterthought" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Issue 1 — Which panel is primary on the dashboard, and what is the reading order?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 1 Information Architecture, currently 3/10.\nELI10: Three panels are listed as equals. A user who just logged in has one question: \"what happened while I was away?\" One panel has to own the first glance, and the other two have to visibly step back. This decision drives layout at every breakpoint, what goes above the fold on a phone, and where the loading skeleton draws attention. Principle: hierarchy as service; if everything competes, nothing wins.\nStakes if we pick wrong: on a 375px phone the user scrolls past two panels of stuff they didn't ask for before seeing the one that matters; the page reads as a widget mosaic.\nRecommendation: 1A because notifications are the \"while you were away\" answer, activity is the browse-able context, and quick actions are a launcher that should be reachable, not read.\nNote: options differ in kind, not coverage — no completeness score.": "1A: Notifications first (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:41:45.355Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_01JNoKum3CkUH1TidSdGtexa", + "questions": [ + { + "question": "Issue 2 — Add a screen-structure diagram (page frame + panel placement) to the plan?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 1 Information Architecture, now 6/10 after Issue 1.\nELI10: The plan says nothing about the page frame: is there a top nav, a page title, where the three panels sit on a wide screen versus a phone. Without this, the implementer picks a layout on the fly and the trunk test fails (cover everything but the nav: can you tell what site and page you're on?). I'd add an ASCII structure diagram for desktop (lg) and mobile (sm) that fixes panel placement per the approved order. Principle: users scan, they don't read; clearly defined areas are how they scan.\nStakes if we pick wrong: three equal-width columns or a stacked card mosaic, the hard-rejection pattern for app UI.\nRecommendation: 2A because the two-zone layout gives Notifications a real anchor position and keeps Quick Actions out of the reading flow.\nCompleteness: 2A=10/10, 2B=7/10, 2C=3/10", + "header": "Issue 2", + "multiSelect": false, + "options": [ + { + "label": "2A: Two-zone layout, diagram at lg + sm (recommended)", + "description": "✅ lg: sticky top nav; page header row with title + Quick Actions bar right-aligned; body is 2 columns, Notifications left (7/12) and Activity right (5/12)\n✅ sm: single column, Quick Actions as a horizontal scroll strip under the header, then Notifications, then Activity; diagram fixes both\n❌ Notifications-left at 7/12 gives the feed less room; long activity rows will truncate harder on lg" + }, + { + "label": "2B: Three-column body, diagram at lg only", + "description": "✅ Each panel gets a column; simple grid, simple to build (human: ~1h / CC: ~5min)\n✅ Quick Actions visible as a full column on desktop\n❌ Three equal peers contradicts the approved Notifications-first hierarchy; mobile behavior stays unspecified" + }, + { + "label": "2C: Skip the diagram, prose only", + "description": "✅ Zero extra planning time; implementer decides layout in code\n✅ Leaves room for the implementer's judgment\n❌ Layout by omission; the first review of a real screen becomes the layout discussion we could have had now" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Issue 2 — Add a screen-structure diagram (page frame + panel placement) to the plan?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 1 Information Architecture, now 6/10 after Issue 1.\nELI10: The plan says nothing about the page frame: is there a top nav, a page title, where the three panels sit on a wide screen versus a phone. Without this, the implementer picks a layout on the fly and the trunk test fails (cover everything but the nav: can you tell what site and page you're on?). I'd add an ASCII structure diagram for desktop (lg) and mobile (sm) that fixes panel placement per the approved order. Principle: users scan, they don't read; clearly defined areas are how they scan.\nStakes if we pick wrong: three equal-width columns or a stacked card mosaic, the hard-rejection pattern for app UI.\nRecommendation: 2A because the two-zone layout gives Notifications a real anchor position and keeps Quick Actions out of the reading flow.\nCompleteness: 2A=10/10, 2B=7/10, 2C=3/10": "2A: Two-zone layout, diagram at lg + sm (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:42:15.329Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_01D6hkqEL4BpbLKWUECMsBYE", + "questions": [ + { + "question": "Issue 3 — Specify what the user sees in every state of every panel (state table with copy and actions)?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, currently 3/10.\nELI10: The plan lists \"empty, loading, error\" per panel but never says what any of them look like or say. Left alone, an engineer ships \"No notifications.\" in gray text, and that's the first thing a brand-new user sees in the page's primary slot. I'd add a table covering loading / empty / error / success / partial for Notifications, Activity, and Quick Actions, with the exact copy, the primary action, and skeleton shape. Principle: empty states are features; every one needs warmth, a primary action, and context.\nStakes if we pick wrong: first-login dashboard reads as broken or abandoned; error states offer no recovery path.\nRecommendation: 3A because the first-run and zero-data cases are the ones most users hit first, and writing the copy now costs minutes.\nCompleteness: 3A=10/10, 3B=6/10, 3C=3/10", + "header": "Issue 3", + "multiSelect": false, + "options": [ + { + "label": "3A: Full state table, copy + actions + skeletons (recommended)", + "description": "✅ Every cell written: e.g. Notifications empty = \"You're all caught up\" + last-checked time; Activity empty on first login = \"Your activity will show up here\" + the top quick action as CTA\n✅ Error cells get a retry button scoped to that panel plus a plain-language reason; skeletons match row anatomy so layout doesn't jump (human: ~2h / CC: ~10min)\n❌ Copy is a proposal; product voice may want to rewrite it later" + }, + { + "label": "3B: Table with placeholders, copy written during implementation", + "description": "✅ Locks the structure (which states exist per panel) without committing to words now\n✅ Faster to approve; leaves copy to whoever owns voice\n❌ Placeholders become the copy; \"No items found\" ships because nobody circles back" + }, + { + "label": "3C: Keep the one-line checklist as is", + "description": "✅ No planning time spent\n✅ Implementer has freedom\n❌ Guaranteed \"No notifications.\" in the primary slot; error states with no retry" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Issue 3 — Specify what the user sees in every state of every panel (state table with copy and actions)?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, currently 3/10.\nELI10: The plan lists \"empty, loading, error\" per panel but never says what any of them look like or say. Left alone, an engineer ships \"No notifications.\" in gray text, and that's the first thing a brand-new user sees in the page's primary slot. I'd add a table covering loading / empty / error / success / partial for Notifications, Activity, and Quick Actions, with the exact copy, the primary action, and skeleton shape. Principle: empty states are features; every one needs warmth, a primary action, and context.\nStakes if we pick wrong: first-login dashboard reads as broken or abandoned; error states offer no recovery path.\nRecommendation: 3A because the first-run and zero-data cases are the ones most users hit first, and writing the copy now costs minutes.\nCompleteness: 3A=10/10, 3B=6/10, 3C=3/10": "3A: Full state table, copy + actions + skeletons (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:43:02.148Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_015dYhiwCXPuYiXYwwthqfoB", + "questions": [ + { + "question": "Issue 4 — Shape the API response so each panel can fail independently?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, now 6/10.\nELI10: The plan wants per-panel error states but fetches everything in one GET /api/dashboard call. Those two goals conflict unless the response itself can say \"activity failed, notifications are fine.\" Today's shape ({ activity, notifications, quickActions }) can't express that, so any single slow or broken query takes down all three panels. This is a design decision because it determines whether the user ever sees a panel-level error or only whole-page failure. Principle: seeing the system, not the screen.\nStakes if we pick wrong: one slow activity query blanks the notifications the user came for; or three separate requests triple the latency on a cold phone connection.\nRecommendation: 4A because it keeps one round-trip (fast first paint on mobile) while letting each panel degrade on its own.\nCompleteness: 4A=10/10, 4B=8/10, 4C=4/10", + "header": "Issue 4", + "multiSelect": false, + "options": [ + { + "label": "4A: One call, per-key result envelopes (recommended)", + "description": "✅ Response becomes { notifications: {ok, data|error}, activity: {ok, data|error}, quickActions: {...} }; server runs the three queries in parallel with per-query timeouts and never fails the whole response for one key\n✅ One round trip preserves fast first paint on slow mobile; per-panel Retry hits the same endpoint with ?only=activity (human: ~3h / CC: ~15min)\n❌ Slightly more server code and a non-trivial response contract to document" + }, + { + "label": "4B: Three endpoints, one per panel", + "description": "✅ Simplest failure isolation; each panel owns its fetch, retry, and cache\n✅ Panels can stream in independently as each resolves\n❌ Three round trips on login; on a bad connection the primary panel is no faster than before and the page flickers in three stages" + }, + { + "label": "4C: Keep single flat response, whole-page error only", + "description": "✅ Zero contract change; simplest to build\n✅ One error state to design\n❌ Contradicts the plan's own per-panel error requirement; one slow query blanks everything the user logged in to see" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Issue 4 — Shape the API response so each panel can fail independently?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, now 6/10.\nELI10: The plan wants per-panel error states but fetches everything in one GET /api/dashboard call. Those two goals conflict unless the response itself can say \"activity failed, notifications are fine.\" Today's shape ({ activity, notifications, quickActions }) can't express that, so any single slow or broken query takes down all three panels. This is a design decision because it determines whether the user ever sees a panel-level error or only whole-page failure. Principle: seeing the system, not the screen.\nStakes if we pick wrong: one slow activity query blanks the notifications the user came for; or three separate requests triple the latency on a cold phone connection.\nRecommendation: 4A because it keeps one round-trip (fast first paint on mobile) while letting each panel degrade on its own.\nCompleteness: 4A=10/10, 4B=8/10, 4C=4/10": "4A: One call, per-key result envelopes (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:43:55.568Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_01LnFZiSt49gf7W5mJjVdG7R", + "questions": [ + { + "question": "Issue 5 — Replace the \"Mark all as read\" confirmation modal with instant action + undo toast?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, now 7/10.\nELI10: The plan puts a confirmation modal in front of \"Mark all as read.\" Modals are for one-way doors (delete, pay, send). Marking read is low-stakes and reversible, so the convention (Gmail, GitHub, Slack) is: do it immediately, show a toast with Undo for a few seconds. A modal here makes the user answer a question they didn't ask, every time. Principle: the goodwill reservoir; punishing users with an extra step for a safe action depletes it.\nStakes if we pick wrong: keep the modal and the most-used action on the primary panel gains a click and a read; drop undo and a mis-tap wipes the unread list with no recovery.\nRecommendation: 5A because it removes a step from the page's most frequent action while keeping recovery.\nNote: options differ in kind, not coverage — no completeness score.", + "header": "Issue 5", + "multiSelect": false, + "options": [ + { + "label": "5A: Instant + Undo toast, drop the modal (recommended)", + "description": "✅ One click: unread dots clear optimistically, badge goes to 0, toast \"Marked 12 as read. [Undo]\" for 6s; Undo restores client state and calls the server\n✅ Removes the Modal component from this plan entirely (one less primitive to build and make accessible)\n❌ The 6-second undo window needs a live-region announcement and pause-on-hover, which the toast spec must cover (Issue 6)" + }, + { + "label": "5B: Keep modal, but only when unread > N", + "description": "✅ Guards the rare large-clear case (say > 50 unread) where an accident costs more\n✅ Small clears stay one-click\n❌ Two behaviors for one button confuses users (\"why did it ask this time?\"); modal still has to be built and made accessible" + }, + { + "label": "5C: Keep the confirmation modal as planned", + "description": "✅ Zero chance of accidental mass-mark; explicit intent\n✅ Modal primitive may be needed elsewhere later anyway\n❌ Adds friction to the primary panel's main action; mark-read is recoverable, so the modal solves a problem that doesn't exist" + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "Issue 5 — Replace the \"Mark all as read\" confirmation modal with instant action + undo toast?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, now 7/10.\nELI10: The plan puts a confirmation modal in front of \"Mark all as read.\" Modals are for one-way doors (delete, pay, send). Marking read is low-stakes and reversible, so the convention (Gmail, GitHub, Slack) is: do it immediately, show a toast with Undo for a few seconds. A modal here makes the user answer a question they didn't ask, every time. Principle: the goodwill reservoir; punishing users with an extra step for a safe action depletes it.\nStakes if we pick wrong: keep the modal and the most-used action on the primary panel gains a click and a read; drop undo and a mis-tap wipes the unread list with no recovery.\nRecommendation: 5A because it removes a step from the page's most frequent action while keeping recovery.\nNote: options differ in kind, not coverage — no completeness score.": "5A: Instant + Undo toast, drop the modal (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-09-20T20:44:29.271Z" + }, + { + "sessionId": "03736bff-d38b-4f3e-a849-84f17064a75b", + "toolUseId": "toolu_01An1yzjWxro5Sx3VBjP4fHJ", + "questions": [ + { + "question": "Issue 6 — Specify the toast system (position, timing, stacking, dismiss, screen reader behavior)?\nProject/branch/task: main, PLAN.md User Dashboard; Pass 2 Interaction States, now 8/10.\nELI10: \"Toast notification system for action feedback\" is a component name, not a spec. And after Issue 5 the toast carries Undo, so its timing and accessibility now decide whether a user can recover from a mis-click. I'd pin down: where it appears, how long it stays, what happens with several at once, how to dismiss, and how screen readers hear it (a live region, so the Undo offer is announced and reachable by keyboard). Principle: accessibility is not optional; specify it in the plan or it won't exist.\nStakes if we pick wrong: a screen-reader user never hears \"Undo\"; toasts stack over the Quick Actions bar on mobile; a 3-second toast makes Undo a race.\nRecommendation: 6A because the toast is now the recovery mechanism for the primary panel's main action.\nCompleteness: 6A=10/10, 6B=6/10", + "header": "Issue 6", + "multiSelect": false, + "options": [ + { + "label": "6A: Full toast spec (recommended)", + "description": "✅ Bottom-center on sm (above safe-area, never over the action strip), bottom-right on md+; 6s default, 10s when it carries an action, pause on hover/focus; max 3 stacked, oldest drops\n✅ role=status live region for info, role=alert for errors; action button is a real