From 950cff358ac1f49b02c0392cf8e6199539ffed25 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:22:56 +0000 Subject: [PATCH 01/10] I1: run Codex paid slices on a host-run job where bubblewrap can start Docker's default seccomp/AppArmor profiles and the Ubuntu 24.04 host's kernel.apparmor_restrict_unprivileged_userns=1 keep Codex's bubblewrap sandbox from starting in a job container, so every Codex command failed. The planner now packs Codex shards into their own slices (plan.codexSlices, validated against the entries); evals-periodic runs those slices on a new host-run job that lifts the host restriction and runs the same CI image with seccomp and AppArmor unconfined. Artifact names match eval-slices, so the report reconciles one manifest. Gate plans have no Codex slices. --- .github/workflows/evals-periodic.yml | 147 +++++++++++++++++++++++++-- scripts/lib/paid-plan.ts | 34 +++++-- test/codex-ci-access.test.ts | 50 ++++++++- test/evals-workflow-wiring.test.ts | 73 ++++++++++++- 4 files changed, 285 insertions(+), 19 deletions(-) diff --git a/.github/workflows/evals-periodic.yml b/.github/workflows/evals-periodic.yml index 2cfcfdb2e..2bb7af627 100644 --- a/.github/workflows/evals-periodic.yml +++ b/.github/workflows/evals-periodic.yml @@ -98,6 +98,7 @@ jobs: contents: read outputs: periodic_slices: ${{ steps.periodic-matrix.outputs.slices }} + periodic_codex_slices: ${{ steps.periodic-matrix.outputs.codex_slices }} periodic_timeout_minutes: ${{ steps.periodic-matrix.outputs.timeout_minutes }} gate_slices: ${{ steps.gate-matrix.outputs.slices }} gate_timeout_minutes: ${{ steps.gate-matrix.outputs.timeout_minutes }} @@ -115,11 +116,16 @@ jobs: EVALS_ALL: "1" run: EVALS_TIER=periodic bun --no-install run scripts/test-paid-shards.ts --tier periodic --emit-plan /tmp/paid-plan/manifest.json --slice-budget 540 --jobs 2 --max-parallel 25 + # Codex-only slices run on the host-run eval-codex-slices job; the + # container executor runs every other slice. - name: Derive the periodic executor matrix from the plan id: periodic-matrix run: | - echo "slices=$(jq -c '[range(1; .sliceCount + 1)]' /tmp/paid-plan/manifest.json)" >> "$GITHUB_OUTPUT" - echo "timeout_minutes=$(jq -e '.plan.ciTimeoutMinutes' /tmp/paid-plan/manifest.json)" >> "$GITHUB_OUTPUT" + { + echo "slices=$(jq -c '[range(1; .sliceCount + 1)] - .plan.codexSlices' /tmp/paid-plan/manifest.json)" + echo "codex_slices=$(jq -c '.plan.codexSlices' /tmp/paid-plan/manifest.json)" + echo "timeout_minutes=$(jq -e '.plan.ciTimeoutMinutes' /tmp/paid-plan/manifest.json)" + } >> "$GITHUB_OUTPUT" - uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 with: @@ -280,6 +286,133 @@ jobs: if-no-files-found: ignore retention-days: 30 + # Codex shards on the host. Codex runs every shell command inside a + # bubblewrap sandbox, which needs an unprivileged user namespace. A job + # container cannot get one: Docker's default seccomp and AppArmor profiles + # block it, and Ubuntu 24.04 hosts set + # kernel.apparmor_restrict_unprivileged_userns=1, which no container option + # can change. This job runs on the runner VM itself (ephemeral, one job), + # lifts that host restriction, and runs the same CI image with both + # profiles unconfined. Only the planner's Codex slices run here + # (scripts/lib/paid-plan.ts codexSlicesOf); slice artifacts use the same + # names as eval-slices, so the report reconciles one manifest. + eval-codex-slices: + runs-on: ubicloud-standard-8 + needs: [build-image, plan-slices] + if: ${{ needs.plan-slices.outputs.periodic_codex_slices != '[]' }} + env: + EVALS_RUN_ID: ci-${{ github.run_id }}-${{ github.run_attempt }}-eval-slices-${{ matrix.slice }} + timeout-minutes: ${{ fromJSON(needs.plan-slices.outputs.periodic_timeout_minutes) }} + permissions: + contents: read + packages: read + strategy: + fail-fast: false + max-parallel: 25 + matrix: + slice: ${{ fromJSON(needs.plan-slices.outputs.periodic_codex_slices) }} + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + fetch-depth: 0 + persist-credentials: false + + - uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 + with: + name: paid-plan + path: ${{ runner.temp }}/paid-plan + + - name: Let Codex's bubblewrap sandbox create user namespaces + run: | + if [ -e /proc/sys/kernel/apparmor_restrict_unprivileged_userns ]; then + sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0 + fi + + - uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + # The same steps as eval-slices (bun temp, deps, build, Codex login, + # slice run) inside the CI image. The container's home is a fresh + # runner-temp directory, so logs and captures upload from there. + - name: Run periodic Codex slice ${{ matrix.slice }} + shell: bash + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} + GEMINI_API_KEY: ${{ secrets.GEMINI_API_KEY }} + EVAL_IMAGE: ${{ needs.build-image.outputs.image-tag }} + SLICE: ${{ matrix.slice }} + EVALS_JOBS: "2" + run: | + mkdir -p "$RUNNER_TEMP/eval-home" "$RUNNER_TEMP/paid-slice-results" + docker run --rm --user "$(id -u):$(id -g)" \ + --security-opt seccomp=unconfined --security-opt apparmor=unconfined \ + -e HOME=/home/runner -e ANTHROPIC_API_KEY -e OPENAI_API_KEY -e GEMINI_API_KEY -e SLICE -e EVALS_JOBS \ + -e EVALS_RUN_ID -e EVALS_PROFILE -e EVALS_FRESH -e EVALS_CACHE_PURPOSE -e CI -e GITHUB_ACTIONS \ + -e GITHUB_RUN_ID -e GITHUB_RUN_ATTEMPT -e GITHUB_SHA -e GITHUB_REF -e GITHUB_REF_NAME -e GITHUB_REPOSITORY \ + -e GITHUB_EVENT_NAME -e GITHUB_BASE_REF -e GITHUB_HEAD_REF \ + -e PLAYWRIGHT_BROWSERS_PATH=/opt/playwright-browsers -e EVALS_CONCURRENCY=2 -e GSTACK_EVAL_DIR=/tmp/paid-slice-results \ + -v "$GITHUB_WORKSPACE:/workspace/gstack" -w /workspace/gstack \ + -v "$RUNNER_TEMP/eval-home:/home/runner" \ + -v "$RUNNER_TEMP/paid-plan:/tmp/paid-plan:ro" \ + -v "$RUNNER_TEMP/paid-slice-results:/tmp/paid-slice-results" \ + "$EVAL_IMAGE" bash -c ' + set -euo pipefail + mkdir -p "$HOME/.cache/bun" + export BUN_INSTALL_CACHE_DIR="$HOME/.cache/bun" BUN_TMPDIR="$HOME/.cache/bun" TMPDIR="$HOME/.cache" + if diff -q /opt/node_modules_cache/.bun.lock bun.lock >/dev/null 2>&1; then + rm -rf node_modules && cp -r /opt/node_modules_cache node_modules + else + bun install --frozen-lockfile + fi + bun run build + codex="${GSTACK_CI_CODEX_BIN_DIR:?}/codex" + export CODEX_HOME="$HOME/.gstack-ci-codex" + mkdir -p "$CODEX_HOME" + if [ -n "${OPENAI_API_KEY:-}" ]; then + printenv OPENAI_API_KEY | "$codex" login --with-api-key + "$codex" login status >/dev/null 2>&1 + else + echo "::warning::OPENAI_API_KEY is unavailable; Codex eval cases cannot authenticate and will fail." + fi + export GSTACK_CI_CODEX_HOME="$CODEX_HOME" + unset CODEX_HOME + EVALS_TIER=periodic bun run scripts/test-paid-shards.ts --tier periodic --plan /tmp/paid-plan/manifest.json --slice "$SLICE" + ' + + - name: Upload slice results + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: paid-slice-${{ matrix.slice }}-a${{ github.run_attempt }} + path: ${{ runner.temp }}/paid-slice-results + retention-days: 90 + + - name: Upload native capture evidence + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: native-captures-${{ env.EVALS_RUN_ID }} + include-hidden-files: true + path: | + ${{ runner.temp }}/eval-home/.gstack/projects/*/e2e-runs + ${{ runner.temp }}/eval-home/.gstack-dev/e2e-runs + if-no-files-found: ignore + retention-days: 90 + + - name: Upload shard logs + if: always() + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7 + with: + name: paid-logs-slice-${{ matrix.slice }}-a${{ github.run_attempt }} + include-hidden-files: true + path: ${{ runner.temp }}/eval-home/.cache/gstack-paid-shard-*.log + if-no-files-found: ignore + retention-days: 30 + # Weekly EVALS_ALL gate-tier census: PR lanes are diff-billed, so without # this the full gate census might never execute anywhere and the selector's # blind spots rot invisibly. Census health, not selector correctness — @@ -389,7 +522,7 @@ jobs: report: runs-on: ubicloud-standard-2 - needs: [plan-slices, eval-slices, gate-census] + needs: [plan-slices, eval-slices, eval-codex-slices, gate-census] # !cancelled(): the report must run (and FAIL) when an executor died — a # missing slice artifact reading as green is the class this lane kills — # but a cancelled run stops here. @@ -521,7 +654,7 @@ jobs: # issue (never a new issue per week) with the headline and failure block # of both lanes, and close it on the next green run. - name: Upsert tracking issue on failure - if: always() && (steps.reconcile.outputs.exit != '0' || steps.gate-reconcile.outputs.exit != '0' || steps.pass-rates.outputs.exit != '0' || needs.eval-slices.result != 'success' || needs.gate-census.result != 'success') + if: always() && (steps.reconcile.outputs.exit != '0' || steps.gate-reconcile.outputs.exit != '0' || steps.pass-rates.outputs.exit != '0' || needs.eval-slices.result != 'success' || !contains(fromJSON('["success","skipped"]'), needs.eval-codex-slices.result) || needs.gate-census.result != 'success') env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REDISPATCH: ${{ steps.verdict.outputs.redispatch }} @@ -536,7 +669,7 @@ jobs: if [ -n "$REDISPATCH_OF" ]; then echo; echo "This run is the one INFRA/INCOMPLETE re-dispatch of run ${GITHUB_SERVER_URL}/${GITHUB_REPOSITORY}/actions/runs/${REDISPATCH_OF}; both runs are reported."; fi if [ "$REDISPATCH" = "true" ]; then echo; echo "Every red verdict is machine-classified INFRA/INCOMPLETE: re-dispatching once as a new run (EVAL_POLICY.infraRedispatch). This run stays red and reported."; fi echo - echo "- periodic reconciliation exit: ${{ steps.reconcile.outputs.exit }} (slices job: ${{ needs.eval-slices.result }})" + echo "- periodic reconciliation exit: ${{ steps.reconcile.outputs.exit }} (slices job: ${{ needs.eval-slices.result }}, Codex slices job: ${{ needs.eval-codex-slices.result }})" echo "- gate census reconciliation exit: ${{ steps.gate-reconcile.outputs.exit }} (census job: ${{ needs.gate-census.result }})" echo "- pass-rate history gate exit: ${{ steps.pass-rates.outputs.exit }}" echo @@ -573,7 +706,7 @@ jobs: fi - name: Close the tracking issue on a green run - if: always() && steps.reconcile.outputs.exit == '0' && steps.gate-reconcile.outputs.exit == '0' && steps.pass-rates.outputs.exit == '0' && needs.eval-slices.result == 'success' && needs.gate-census.result == 'success' + if: always() && steps.reconcile.outputs.exit == '0' && steps.gate-reconcile.outputs.exit == '0' && steps.pass-rates.outputs.exit == '0' && needs.eval-slices.result == 'success' && contains(fromJSON('["success","skipped"]'), needs.eval-codex-slices.result) && needs.gate-census.result == 'success' env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} REDISPATCH_OF: ${{ inputs.redispatch_of }} @@ -588,7 +721,7 @@ jobs: fi - name: Fail the workflow when reconciliation failed - if: always() && (steps.reconcile.outputs.exit != '0' || steps.gate-reconcile.outputs.exit != '0' || steps.pass-rates.outputs.exit != '0' || needs.eval-slices.result != 'success' || needs.gate-census.result != 'success') + if: always() && (steps.reconcile.outputs.exit != '0' || steps.gate-reconcile.outputs.exit != '0' || steps.pass-rates.outputs.exit != '0' || needs.eval-slices.result != 'success' || !contains(fromJSON('["success","skipped"]'), needs.eval-codex-slices.result) || needs.gate-census.result != 'success') run: exit 1 # The one INFRA/INCOMPLETE re-dispatch (UC-E1). Its own job so the report diff --git a/scripts/lib/paid-plan.ts b/scripts/lib/paid-plan.ts index b46a9449d..7a60c53b2 100644 --- a/scripts/lib/paid-plan.ts +++ b/scripts/lib/paid-plan.ts @@ -55,7 +55,7 @@ export { PAID_TEST_GLOBS, isPaidTestFile }; export { PERIODIC_CI_EXCLUDE }; type E2EShardReuse = NonNullable>; -import { CASE_KEY_SEPARATOR, CASE_SHARDED_FILES, type CaseTrialPlan, caseTrialPlan, expandCaseShards, expandTrialShards, isIsolatedCase, partitionCaseExclusions, sameTrialPlan, shardCaseId, shardFile, shardTrial } from './paid-cases'; +import { CASE_KEY_SEPARATOR, CASE_SHARDED_FILES, type CaseTrialPlan, caseTrialPlan, codexShardAccess, expandCaseShards, expandTrialShards, isIsolatedCase, partitionCaseExclusions, sameTrialPlan, shardCaseId, shardFile, shardTrial } from './paid-cases'; import { DEFAULT_JOBS, OVERLAY_MAX_ACTIVE_SHARDS, PAID_TIERS, type PaidCaseSelection, type PaidProfile, type PaidShardBudget, type PaidTier, ROOT, type ShardOutcome, type ShardStatus, type ShardTrialRecord, collectPaidTestFiles, computePaidCaseSelection, expectedPrCaseCount, isAllSkippedPass, isOverlayTestFile, paidShardWallUpperBoundMs, partitionShardsByDiffSelection, planPaidShards, prProfileFileSelected, resolvePaidShardBudget, resolvePaidShardTimeoutMs, sameBudget, selectPaidTestFiles, shardSlug, validatedProfile } from '../test-paid-shards'; // ─── Planner / executor / report (the CI re-platform surface) ────────────── @@ -89,6 +89,8 @@ export interface PaidSlicePlan { jobs: number; estimatedSliceMs: number[]; ciTimeoutMinutes: number; + /** Slices that run only Codex shards; CI executes them on the host-run Codex job, where bubblewrap can start. */ + codexSlices?: number[]; } export interface PaidRunManifest { @@ -187,6 +189,16 @@ export function mergePaidTestDurations(seed: Record, results: Sl return Object.fromEntries(Object.entries(merged).sort(([a], [b]) => (a < b ? -1 : 1))); } +/** True when a shard key's file sees the CI Codex install in `tier` (scripts/lib/paid-cases.ts codexShardAccess). */ +export function isCodexShard(key: string, tier: PaidTier): boolean { + return codexShardAccess([shardFile(key)], tier) !== 'none'; +} + +/** Sorted slices holding at least one planned Codex shard. */ +export function codexSlicesOf(tier: PaidTier, entries: readonly ManifestEntry[]): number[] { + return [...new Set(entries.filter(entry => entry.status === 'planned' && isCodexShard(entry.file, tier)).map(entry => entry.slice))].sort((a, b) => a - b); +} + /** Panel identity of a trial shard key (`#`), else null. */ export function trialPanelKey(key: string): string | null { return shardTrial(key) === null ? null : durationKey(key); @@ -228,11 +240,12 @@ export function sliceSupervisedWallMs(files: string[], jobs: number, overrideMs? * within the budget (best fit), else into a new slice. A file with no recorded * wall weighs the whole budget, so unknown cost gets a runner of its own. A * file longer than the budget runs alone. Overlay wrappers keep one shared - * final slice (one wrapper at a time). The CI timeout covers every slice's - * supervised worst case plus the setup allowance. + * final slice (one wrapper at a time). Codex shards never share a slice with + * other shards. The CI timeout covers every slice's supervised worst case plus + * the setup allowance. */ export function packBySliceBudget(files: string[], budgetMs: number, jobs: number, - recorded: Record, timeoutMs?: number): { + recorded: Record, timeoutMs?: number, codexShard: (file: string) => boolean = () => false): { slices: string[][]; estimates: Record; estimatedSliceMs: number[]; ciTimeoutMinutes: number; } { const estimates = Object.fromEntries(files.map(file => [file, recordedShardMs(recorded, file) ?? budgetMs])); @@ -244,6 +257,8 @@ export function packBySliceBudget(files: string[], budgetMs: number, jobs: numbe // Trials of one case never share a runner: independent machines, and // the panel's wall stays one trial long. if (sharesPanel(planned, file)) return; + // Codex shards run on the host-run Codex job; other shards keep the default container sandbox. + if (codexShard(planned[0]!) !== codexShard(file)) return; const ms = estimatedSliceMs([...planned, file], weight, jobs); if (ms <= budgetMs && ms > bestMs) { best = index; bestMs = ms; } }); @@ -339,7 +354,7 @@ export function buildRunManifest(opts: { const entries: ManifestEntry[] = []; if (budgetMode) { const plan = packBySliceBudget(runnable.map(files => files[0]!), opts.sliceBudgetMs!, opts.jobs!, - opts.durations ?? loadPaidTestDurations(rootDir, opts.tier), opts.timeoutMs); + opts.durations ?? loadPaidTestDurations(rootDir, opts.tier), opts.timeoutMs, file => isCodexShard(file, opts.tier)); plan.slices.forEach((files, index) => files.forEach(file => entries.push({ file, slice: index + 1, status: 'planned', estimatedMs: plan.estimates[file]!, ...extras(file), ...(FILE_RETRY_BUDGETS.some(budget => budget.file === shardFile(file)) ? { budget: resolvePaidShardBudget([file], opts.timeoutMs) } : {}) }))); @@ -350,7 +365,8 @@ export function buildRunManifest(opts: { version: 1, tier: opts.tier, evalsAll: opts.evalsAll, sliceCount: plan.slices.length, selectionReason: cases.reason, profile, selection: cases.selection, ...(cases.coverage ? { prCoverage: cases.coverage } : {}), - plan: { sliceBudgetMs: opts.sliceBudgetMs!, jobs: opts.jobs!, estimatedSliceMs: plan.estimatedSliceMs, ciTimeoutMinutes: plan.ciTimeoutMinutes }, + plan: { sliceBudgetMs: opts.sliceBudgetMs!, jobs: opts.jobs!, estimatedSliceMs: plan.estimatedSliceMs, ciTimeoutMinutes: plan.ciTimeoutMinutes, + codexSlices: codexSlicesOf(opts.tier, entries) }, entries, } satisfies PaidRunManifest)); } @@ -616,6 +632,12 @@ export function parseRunManifest(raw: string): PaidRunManifest { || parsed.entries.some(entry => entry.status === 'planned' && entry.estimatedMs === undefined)) { throw new Error('manifest slice plan malformed'); } + const codexSlices = codexSlicesOf(parsed.tier, parsed.entries); + if (JSON.stringify(plan.codexSlices ?? []) !== JSON.stringify(codexSlices)) { + throw new Error(`manifest Codex slices must be exactly the slices holding Codex shards: ${JSON.stringify(codexSlices)}`); + } + const mixed = parsed.entries.find(entry => entry.status === 'planned' && codexSlices.includes(entry.slice) && !isCodexShard(entry.file, parsed.tier)); + if (mixed) throw new Error(`Codex slice ${mixed.slice} also holds ${mixed.file}; only Codex shards run on the host-run Codex job`); } const keys = parsed.entries.map(entry => normalizeRelativePath(entry.file)); if (new Set(keys).size !== keys.length) throw new Error('Duplicate manifest entry'); diff --git a/test/codex-ci-access.test.ts b/test/codex-ci-access.test.ts index 837e3938e..a311fb90c 100644 --- a/test/codex-ci-access.test.ts +++ b/test/codex-ci-access.test.ts @@ -12,7 +12,7 @@ import { 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 { CODEX_CI_CASES, CODEX_CI_ENV, CODEX_CI_FILES, buildRunManifest, codexShardAccess, runPaidShards, scopeCodexAccess, shardFile } from '../scripts/test-paid-shards'; +import { CODEX_CI_CASES, CODEX_CI_ENV, CODEX_CI_FILES, buildRunManifest, codexShardAccess, isCodexShard, packBySliceBudget, parseRunManifest, runPaidShards, scopeCodexAccess, shardFile } from '../scripts/test-paid-shards'; import { buildHermeticEnv } from './helpers/hermetic-env'; import { E2E_TIERS, E2E_TOUCHFILES } from './helpers/touchfiles'; @@ -135,6 +135,42 @@ describe('codex CI access scope', () => { }, 60_000); }); +describe('Codex slices for the host-run job', () => { + const budgetPlan = () => buildRunManifest({ tier: 'periodic', profile: 'full', sliceBudgetMs: 540_000, jobs: 2, evalsAll: true, env: { EVALS_ALL: '1' } }); + + test('packing never puts a Codex shard on a slice with other shards', () => { + const files = ['test/codex-e2e.test.ts', 'test/skill-e2e-outside-voice.test.ts', 'test/skill-e2e-plan.test.ts', 'test/skill-e2e-review.test.ts']; + const recorded = Object.fromEntries(files.map(file => [file, 60_000])); + const plan = packBySliceBudget(files, 540_000, 2, recorded, undefined, file => isCodexShard(file, 'periodic')); + expect(plan.slices).toEqual([['test/codex-e2e.test.ts', 'test/skill-e2e-outside-voice.test.ts'], ['test/skill-e2e-plan.test.ts', 'test/skill-e2e-review.test.ts']]); + expect(packBySliceBudget(files, 540_000, 2, recorded).slices).toHaveLength(1); + }); + + test('the live periodic plan names exactly the slices holding Codex shards, and they hold nothing else', () => { + const manifest = budgetPlan(); + const planned = manifest.entries.filter(entry => entry.status === 'planned'); + const codexSlices = manifest.plan!.codexSlices!; + expect(codexSlices.length).toBeGreaterThan(0); + expect(new Set(planned.filter(entry => isCodexShard(entry.file, 'periodic')).map(entry => shardFile(entry.file)))) + .toEqual(new Set([...CODEX_CI_FILES, ...Object.keys(CODEX_CI_CASES)])); + for (const entry of planned) expect(codexSlices.includes(entry.slice), entry.file).toBe(isCodexShard(entry.file, 'periodic')); + }); + + test('a manifest that hides a Codex slice or mixes it with another shard fails to parse', () => { + const manifest = budgetPlan(); + const codexSlice = manifest.plan!.codexSlices![0]!; + const hidden = structuredClone(manifest); + hidden.plan!.codexSlices = []; + expect(() => parseRunManifest(JSON.stringify(hidden))).toThrow(/Codex slices must be exactly/); + const omitted = structuredClone(manifest); + delete omitted.plan!.codexSlices; + expect(() => parseRunManifest(JSON.stringify(omitted))).toThrow(/Codex slices must be exactly/); + const mixed = structuredClone(manifest); + mixed.entries.find(entry => entry.status === 'planned' && !isCodexShard(entry.file, 'periodic'))!.slice = codexSlice; + expect(() => parseRunManifest(JSON.stringify(mixed))).toThrow(new RegExp(`Codex slice ${codexSlice} also holds`)); + }); +}); + describe('codex CI access in the image and workflows', () => { test('the image installs Codex off PATH at the directory it advertises', () => { const dockerfile = fs.readFileSync(path.join(ROOT, '.github/docker/Dockerfile.ci'), 'utf8'); @@ -158,9 +194,17 @@ describe('codex CI access in the image and workflows', () => { } } } - expect(steps.length).toBeGreaterThanOrEqual(4); + expect(steps.length).toBeGreaterThanOrEqual(5); for (const run of steps) { - expect(run).toContain('echo "GSTACK_CI_CODEX_HOME=$CODEX_HOME" >> "$GITHUB_ENV"'); + // The host-run Codex job logs in inside its one `docker run` script, so the home stays a shell export there. + if (run.includes('docker run ')) { + const exported = run.indexOf('export GSTACK_CI_CODEX_HOME="$CODEX_HOME"'); + expect(exported).toBeGreaterThan(run.indexOf('login --with-api-key')); + expect(run.indexOf('unset CODEX_HOME')).toBeGreaterThan(exported); + expect(run.indexOf('unset CODEX_HOME')).toBeLessThan(run.indexOf('scripts/test-paid-shards.ts')); + } else { + expect(run).toContain('echo "GSTACK_CI_CODEX_HOME=$CODEX_HOME" >> "$GITHUB_ENV"'); + } expect(run).not.toMatch(/echo "CODEX_HOME=/); expect(run).not.toContain('$HOME/.codex"'); expect(run).toContain('"${GSTACK_CI_CODEX_BIN_DIR:?}/codex"'); diff --git a/test/evals-workflow-wiring.test.ts b/test/evals-workflow-wiring.test.ts index 485f10080..4268c160e 100644 --- a/test/evals-workflow-wiring.test.ts +++ b/test/evals-workflow-wiring.test.ts @@ -22,7 +22,7 @@ import { describe, test, expect } from 'bun:test'; import * as fs from 'fs'; import * as path from 'path'; -import { buildRunManifest, parseCliOptions, sliceExecutionOrder, sliceSupervisedWallMs, CI_SETUP_ALLOWANCE_MINUTES } from '../scripts/test-paid-shards'; +import { buildRunManifest, isCodexShard, parseCliOptions, sliceExecutionOrder, sliceSupervisedWallMs, CI_SETUP_ALLOWANCE_MINUTES } from '../scripts/test-paid-shards'; const ROOT = path.join(import.meta.dir, '..'); const read = (rel: string) => fs.readFileSync(path.join(ROOT, rel), 'utf-8'); @@ -38,7 +38,7 @@ function plannerSites(source: string): Array<{ manifest: string; budgetSeconds: .map((m) => ({ manifest: m[1]!, budgetSeconds: Number(m[2]), jobs: Number(m[3]) })); } -type Step = { id?: string; name?: string; run?: string; env?: Record; with?: Record }; +type Step = { id?: string; name?: string; run?: string; uses?: string; env?: Record; with?: Record }; type Job = { needs?: string[]; env?: Record; outputs?: Record; 'timeout-minutes': string | number; strategy?: { 'max-parallel': number; matrix: { slice: string } }; steps: Step[] }; @@ -57,7 +57,7 @@ function expectPlannedExecutor(source: string, executorName: string, prefix: str const [stepId] = /^\$\{\{ steps\.([\w-]+)\.outputs\.slices \}\}$/.exec(planner.outputs![`${prefix}slices`]!)!.slice(1); expect(planner.outputs![`${prefix}timeout_minutes`]).toBe(`\${{ steps.${stepId}.outputs.timeout_minutes }}`); const matrixStep = planner.steps.find(step => step.id === stepId)!; - const manifest = /jq -c '\[range\(1; \.sliceCount \+ 1\)\]' (\S+)\)/.exec(matrixStep.run!)![1]!; + const manifest = /jq -c '\[range\(1; \.sliceCount \+ 1\)\](?: - \.plan\.codexSlices)?' (\S+)\)/.exec(matrixStep.run!)![1]!; expect(matrixStep.run).toContain(`jq -e '.plan.ciTimeoutMinutes' ${manifest})`); const emit = planner.steps.filter(step => step.run?.includes(`--emit-plan ${manifest} `)); expect(emit).toHaveLength(1); @@ -201,6 +201,73 @@ describe('evals-periodic.yml sliced-lane wiring', () => { }); }); +describe('evals-periodic.yml host-run Codex slices', () => { + type HostJob = Job & { if?: string; container?: unknown; 'runs-on': string; permissions?: Record; + steps: Array }; + const jobs = (Bun.YAML.parse(periodicYml) as { jobs: Record }).jobs; + const codex = jobs['eval-codex-slices']!; + const container = jobs['eval-slices']!; + const planner = jobs['plan-slices']!; + const uploadNames = (job: HostJob) => job.steps.filter(step => step.uses?.startsWith('actions/upload-artifact@')).map(step => step.with!.name); + + test('the planner gives Codex slices to the host job and every other slice to the container job', () => { + const matrixStep = planner.steps.find(step => step.id === 'periodic-matrix')!; + expect(matrixStep.run).toContain(`echo "slices=$(jq -c '[range(1; .sliceCount + 1)] - .plan.codexSlices' /tmp/paid-plan/manifest.json)"`); + expect(matrixStep.run).toContain(`echo "codex_slices=$(jq -c '.plan.codexSlices' /tmp/paid-plan/manifest.json)"`); + expect(matrixStep.run).toContain('} >> "$GITHUB_OUTPUT"'); + expect(planner.outputs!.periodic_codex_slices).toBe('${{ steps.periodic-matrix.outputs.codex_slices }}'); + expect(codex.needs).toEqual(['build-image', 'plan-slices']); + expect(codex.if).toBe("${{ needs.plan-slices.outputs.periodic_codex_slices != '[]' }}"); + expect(codex.strategy!.matrix.slice).toBe('${{ fromJSON(needs.plan-slices.outputs.periodic_codex_slices) }}'); + expect(codex['timeout-minutes']).toBe('${{ fromJSON(needs.plan-slices.outputs.periodic_timeout_minutes) }}'); + const manifest = buildRunManifest({ tier: 'periodic', profile: 'full', sliceBudgetMs: 540_000, jobs: 2, evalsAll: true, env: { EVALS_ALL: '1' }, rootDir: ROOT }); + const codexSlices = manifest.plan!.codexSlices!; + expect(codexSlices.length).toBeGreaterThan(0); + expect(codex.strategy!['max-parallel']).toBeGreaterThanOrEqual(codexSlices.length); + for (const entry of manifest.entries.filter(entry => entry.status === 'planned')) { + expect(codexSlices.includes(entry.slice), entry.file).toBe(isCodexShard(entry.file, 'periodic')); + } + // Gate plans (evals.yml and the weekly gate census) never give a shard Codex, so they have no Codex slices. + expect(buildRunManifest({ tier: 'gate', profile: 'full', sliceBudgetMs: 540_000, jobs: 2, evalsAll: true, env: { EVALS_ALL: '1' }, rootDir: ROOT }) + .plan!.codexSlices).toEqual([]); + }); + + test('runs on the runner VM, lifts the user-namespace restriction, then runs the CI image with seccomp and AppArmor unconfined', () => { + expect(codex.container).toBeUndefined(); + expect(codex['runs-on']).toBe(container['runs-on']); + expect(codex.permissions).toEqual({ contents: 'read', packages: 'read' }); + const sysctl = codex.steps.findIndex(step => step.run?.includes('sudo sysctl -w kernel.apparmor_restrict_unprivileged_userns=0')); + const execute = codex.steps.findIndex(step => step.run?.includes('docker run ')); + expect(sysctl).toBeGreaterThan(-1); + expect(execute).toBeGreaterThan(sysctl); + const step = codex.steps[execute]!; + expect(step.run).toContain('--security-opt seccomp=unconfined --security-opt apparmor=unconfined'); + expect(step.run).toContain('--user "$(id -u):$(id -g)"'); + expect(step.run).not.toMatch(/--privileged|--cap-add|--pid[ =]host|--network[ =]host|docker\.sock/); + expect(step.env!.EVAL_IMAGE).toBe('${{ needs.build-image.outputs.image-tag }}'); + expect(step.run).toContain('"$EVAL_IMAGE" bash -c'); + expect(step.env!.SLICE).toBe('${{ matrix.slice }}'); + expect(step.run).toContain('EVALS_TIER=periodic bun run scripts/test-paid-shards.ts --tier periodic --plan /tmp/paid-plan/manifest.json --slice "$SLICE"'); + expect(step.run).toContain('-v "$RUNNER_TEMP/paid-plan:/tmp/paid-plan:ro"'); + expect(codex.steps.some(other => other.uses?.startsWith('actions/download-artifact@') && other.with?.name === 'paid-plan' + && other.with?.path === '${{ runner.temp }}/paid-plan')).toBe(true); + const containerRun = container.steps.find(other => other.run?.includes('--plan '))!; + expect(step.env!.EVALS_JOBS).toBe(containerRun.env!.EVALS_JOBS); + for (const name of ['EVALS_CONCURRENCY', 'GSTACK_EVAL_DIR', 'PLAYWRIGHT_BROWSERS_PATH']) { + expect(step.run, name).toContain(`-e ${name}=${containerRun.env![name]}`); + } + }); + + test('uploads under the container executor\'s artifact names, and the report waits for it and fails on it', () => { + expect(uploadNames(codex)).toEqual(uploadNames(container)); + const report = jobs.report!; + expect(report.needs).toContain('eval-codex-slices'); + const guards = (report.steps as Array).filter(step => step.if?.includes('needs.eval-slices.result')); + expect(guards.length).toBeGreaterThanOrEqual(3); + for (const guard of guards) expect(guard.if, guard.name).toContain('needs.eval-codex-slices.result'); + }); +}); + describe('evals-marathon.yml non-blocking lane', () => { const workflow = Bun.YAML.parse(marathonYml) as { on: Record; env: Record; jobs: Record }; From 34854c211bd4485c9f11f44cbe3dfd44b466af4b Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:22:56 +0000 Subject: [PATCH 02/10] I1: the /codex review eval passes only when Codex executed Run 37158847998 recorded codex-review as passed while the sandbox preflight said unavailable and no review ran: the case only checked the session exit and an optional output file. It now requires the shared validator's executed verdict (findings, clean or unverified) in the codex command's own output and a written, non-fail-closed GATE. --- ...eview-sandbox-unavailable-37158847998.json | 244 ++++++++++++++++++ test/helpers/outside-voice-evidence.ts | 11 + test/outside-voice-evidence.test.ts | 24 +- test/skill-e2e-workflow.test.ts | 21 +- 4 files changed, 289 insertions(+), 11 deletions(-) create mode 100644 test/fixtures/codex-review-sandbox-unavailable-37158847998.json diff --git a/test/fixtures/codex-review-sandbox-unavailable-37158847998.json b/test/fixtures/codex-review-sandbox-unavailable-37158847998.json new file mode 100644 index 000000000..0223e886a --- /dev/null +++ b/test/fixtures/codex-review-sandbox-unavailable-37158847998.json @@ -0,0 +1,244 @@ +{ + "source": "https://github.com/garrytan/gstack/actions/runs/37158847998 (paid-slice-9, /codex review, recorded passed)", + "transcript": [ + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_01Mb4hJBn3WjkgygN9VgC9dK", + "name": "Bash", + "input": { + "command": "cat /home/runner/.cache/gstack-paid-shard-yQbZbo/tmp/skill-e2e-codex-lYUSE0/codex-SKILL.md", + "description": "Read the codex skill instructions" + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_01Mb4hJBn3WjkgygN9VgC9dK", + "type": "tool_result", + "content": "[trimmed: skill text]", + "is_error": false + } + ] + } + }, + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_013KDMX84cHDfJGraY3MSeMy", + "name": "Bash", + "input": { + "command": "CODEX_BIN=$(command -v codex || echo \"\"); [ -z \"$CODEX_BIN\" ] && echo \"NOT_FOUND\" || echo \"FOUND: $CODEX_BIN\"; codex --version 2>&1 | head -1", + "description": "Check for codex binary and version" + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_013KDMX84cHDfJGraY3MSeMy", + "type": "tool_result", + "content": "FOUND: /opt/codex/bin/codex\ncodex-cli 0.160.0", + "is_error": false + } + ] + } + }, + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_019TMYmxDs6QmmSrWV77BdkB", + "name": "Bash", + "input": { + "command": "ls -la ~/.claude/skills/gstack/bin/gstack-codex-probe ~/.claude/skills/gstack/bin/gstack-paths ~/.claude/skills/gstack/bin/gstack-review-log ~/.claude/skills/gstack/lib/outside-review-result.ts 2>&1; command -v bun; echo \"CODEX_THREAD_ID=${CODEX_THREAD_ID:-} CODEX_SANDBOX=${CODEX_SANDBOX:-} GSTACK_ACTIVE_HOST=${GSTACK_ACTIVE_HOST:-} CLAUDECODE=${CLAUDECODE:-}\"; echo \"CODEX_API_KEY set: $([ -n \"${CODEX_API_KEY:-}\" ] && echo yes || echo no); OPENAI_API_KEY set: $([ -n \"${OPENAI_API_KEY:-}\" ] && echo yes || echo no)\"; ls \"${CODEX_HOME:-$HOME/.codex}/auth.json\" 2>&1", + "description": "Check gstack helper scripts, harness markers, and codex auth signals" + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_019TMYmxDs6QmmSrWV77BdkB", + "type": "tool_result", + "content": "-rwxr-xr-x 1 runner runner 24955 Oct 3 22:35 /github/home/.claude/skills/gstack/bin/gstack-codex-probe\n-rwxr-xr-x 1 runner runner 7380 Oct 3 22:35 /github/home/.claude/skills/gstack/bin/gstack-paths\n-rwxr-xr-x 1 runner runner 4008 Oct 3 22:35 /github/home/.claude/skills/gstack/bin/gstack-review-log\n-rw-r--r-- 1 runner runner 10979 Oct 3 22:35 /github/home/.claude/skills/gstack/lib/outside-review-result.ts\n/usr/local/bin/bun\nCODEX_THREAD_ID= CODEX_SANDBOX= GSTACK_ACTIVE_HOST= CLAUDECODE=1\nCODEX_API_KEY set: no; OPENAI_API_KEY set: no\n/github/home/.gstack-ci-codex/auth.json", + "is_error": false + } + ] + } + }, + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_01Qi418cvKMX8CqZsBNP86To", + "name": "Bash", + "input": { + "command": "git diff origin/main --stat 2>/dev/null | tail -1 || true; echo \"---local main---\"; git diff main...HEAD --stat; echo \"---diff---\"; git diff main...HEAD", + "description": "Show the branch diff against main" + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_01Qi418cvKMX8CqZsBNP86To", + "type": "tool_result", + "content": "---local main---\n user_controller.rb | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n---diff---\ndiff --git a/user_controller.rb b/user_controller.rb\nnew file mode 100644\nindex 0000000..6344e0f\n--- /dev/null\n+++ b/user_controller.rb\n@@ -0,0 +1,14 @@\n+class UserController < ApplicationController\n+ def show\n+ # SQL injection — interpolating user input directly into query\n+ @user = User.where(\"id = #{params[:id]}\").first\n+ render json: @user\n+ end\n+\n+ def promote\n+ # Bypasses ActiveRecord validations — update_column skips callbacks + validation\n+ @user = User.find(params[:id])\n+ @user.update_column(:role, 'admin')\n+ head :ok\n+ end\n+end", + "is_error": false + } + ] + } + }, + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_01HfjNsSdDDWLaY72ZPrAhYa", + "name": "Bash", + "input": { + "command": "cd /home/runner/.cache/gstack-paid-shard-yQbZbo/tmp/skill-e2e-codex-lYUSE0\n_TEL=$(~/.claude/skills/gstack/bin/gstack-config get telemetry 2>/dev/null || echo off)\nsource ~/.claude/skills/gstack/bin/gstack-codex-probe\nif { [ -n \"${CODEX_THREAD_ID:-}\" ] || [ -n \"${CODEX_SANDBOX:-}\" ] || [ \"${GSTACK_ACTIVE_HOST:-}\" = codex ]; }; then\n echo 'Codex outside review unavailable: harness mismatch; no outside process started. Missing coverage.' >&2\n exit 78\nfi\nif ! _gstack_codex_auth_probe >/dev/null; then\n _gstack_codex_log_event \"codex_auth_failed\"\n echo \"AUTH_FAILED\"\nelif _gstack_codex_sandbox_preflight; then\n _gstack_codex_model_probe\nfi\n_gstack_codex_version_check\necho \"PROBE_EXIT=$?\"", + "description": "Run codex auth, sandbox, model and version probes", + "timeout": 120000 + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_01HfjNsSdDDWLaY72ZPrAhYa", + "type": "tool_result", + "content": "CODEX_SANDBOX: unavailable\nCodex outside review unavailable: Codex's sandbox could not start here (bwrap: Failed to make / slave: Permission denied). No review ran; this is missing coverage, not a pass. Fix: enable unprivileged user namespaces for this container, or set GSTACK_CODEX_NO_SANDBOX=1.\nPROBE_EXIT=0", + "is_error": false + } + ] + } + }, + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_016szorKD1XPWENVS2ZmNQrs", + "name": "Bash", + "input": { + "command": "GSTACK_STATE_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get GSTACK_STATE_ROOT); echo \"GSTACK_STATE_ROOT=$GSTACK_STATE_ROOT\"\necho \"PLAN_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get PLAN_ROOT)\"\necho \"TMP_ROOT=$(~/.claude/skills/gstack/bin/gstack-paths --get TMP_ROOT)\"", + "description": "Resolve gstack plan and temp roots" + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_016szorKD1XPWENVS2ZmNQrs", + "type": "tool_result", + "content": "GSTACK_STATE_ROOT=/home/runner/.cache/gstack-paid-shard-yQbZbo/tmp/gstack-hermetic-4342-Fa8WON/gstack-home\nPLAN_ROOT=/github/home/.claude/plans\nTMP_ROOT=/home/runner/.cache/gstack-paid-shard-yQbZbo/tmp", + "is_error": false + } + ] + } + }, + { + "type": "assistant", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "type": "tool_use", + "id": "toolu_01X2HQFyV9BstdEQBat9XWzV", + "name": "Bash", + "input": { + "command": "cd /home/runner/.cache/gstack-paid-shard-yQbZbo/tmp/skill-e2e-codex-lYUSE0\ncat > codex-output.md <<'EOF'\n# /codex review — feature/add-vuln vs main\n\n## Preflight\n\n- Step 0.4 — codex binary: `FOUND: /opt/codex/bin/codex` (codex-cli 0.160.0)\n- Step 0.5 — harness guard: clean (no `CODEX_THREAD_ID` / `CODEX_SANDBOX` / `GSTACK_ACTIVE_HOST=codex`)\n- Step 0.5 — auth probe: passed (`${CODEX_HOME}/auth.json` present)\n- Step 0.5 — sandbox preflight: **FAILED**\n\nProbe output (verbatim):\n\n```\nCODEX_SANDBOX: unavailable\nCodex outside review unavailable: Codex's sandbox could not start here (bwrap: Failed to make / slave: Permission denied). No review ran; this is missing coverage, not a pass. Fix: enable unprivileged user namespaces for this container, or set GSTACK_CODEX_NO_SANDBOX=1.\n```\n\nModel probe and version check did not run because the sandbox preflight short-circuited them.\nPer the skill (\"If the output contains `CODEX_SANDBOX: unavailable`, stop\"), no `codex review`\ncommand was issued. No paid call was made. Tokens: 0 | Est. cost: $0.00\n\n## Scope that would have been reviewed\n\n`git diff main...HEAD --stat`:\n\n```\n user_controller.rb | 14 ++++++++++++++\n 1 file changed, 14 insertions(+)\n```\n\n## CODEX SAYS (code review):\n════════════════════════════════════════════════════════════\n(no output — Codex did not run; sandbox unavailable)\n════════════════════════════════════════════════════════════\nGATE: FAIL (fail-closed: Codex sandbox unavailable, no review ran — needs human attention)\n\nThe review did not complete. This is a fail-closed verdict, not a judgment on the code:\n`VERDICT: unavailable` maps to GATE: FAIL under gate check 2 (nothing was reviewed).\n\nRecommendation: Fix the Codex sandbox (enable unprivileged user namespaces, or set GSTACK_CODEX_NO_SANDBOX=1 and accept the full-access warning) and re-run `/codex review` before merging, because the diff adds a controller with string-interpolated SQL in `User.where(\"id = #{params[:id]}\")` and an unauthenticated `update_column(:role, 'admin')` path, both of which an outside review would almost certainly flag as P1, and shipping on a fail-closed gate would mean merging that with zero outside coverage.\nEOF\necho written; wc -l codex-output.md", + "description": "Write the fail-closed codex review result to the output file" + }, + "caller": { + "type": "direct" + } + } + ] + } + }, + { + "type": "user", + "session_id": "7836fde2-a2fc-4453-b81a-02f58922990c", + "message": { + "content": [ + { + "tool_use_id": "toolu_01X2HQFyV9BstdEQBat9XWzV", + "type": "tool_result", + "content": "written\n39 codex-output.md", + "is_error": false + } + ] + } + } + ] +} diff --git a/test/helpers/outside-voice-evidence.ts b/test/helpers/outside-voice-evidence.ts index bf440f6d4..7c4cec7ed 100644 --- a/test/helpers/outside-voice-evidence.ts +++ b/test/helpers/outside-voice-evidence.ts @@ -197,6 +197,17 @@ export function outsideExecutionTranscript(executions: OutsideExecution[], provi })); } +/** + * Verdicts the shared validator (lib/outside-review-result.ts) printed for the + * /codex review command: the Bash execution that ran `codex review|exec` and + * piped it through the validator. A review that never ran prints none, or + * `unavailable`. + */ +export function codexReviewVerdicts(executions: OutsideExecution[]): string[] { + return executions.filter(({ command }) => outsideInvocation('codex').test(command) && command.includes('outside-review-result.ts')) + .flatMap(({ output }) => [...output.matchAll(/^VERDICT: ([a-z]+)\s*$/gm)].map(match => match[1]!)); +} + export function foundInvoiceAuthorizationDefect(executions: OutsideExecution[], provider: 'codex' | 'claude-code'): boolean { return executions.some(({ command, output, succeeded }) => succeeded && outsideInvocation(provider).test(command) diff --git a/test/outside-voice-evidence.test.ts b/test/outside-voice-evidence.test.ts index 2d0f6249c..e22e6ae95 100644 --- a/test/outside-voice-evidence.test.ts +++ b/test/outside-voice-evidence.test.ts @@ -3,7 +3,8 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { EvalCollector } from './helpers/eval-store'; -import { claudeOutsideExecutions, codexOutsideExecutions, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence'; +import { claudeOutsideExecutions, codexOutsideExecutions, codexReviewVerdicts, foundInvoiceAuthorizationDefect, outsideExecutionTranscript } from './helpers/outside-voice-evidence'; +import captured_codex_review_unavailable from './fixtures/codex-review-sandbox-unavailable-37158847998.json'; import captured_outside_background_ai from './fixtures/outside-background-ai.json'; import { parseNDJSON } from './helpers/session-runner'; import captured_outside_voice_async from './fixtures/outside-async-task-m-events.json'; @@ -36,6 +37,27 @@ describe('cross-harness live eval evidence', () => { }); }); +describe('/codex review execution evidence', () => { + const run = (command: string, output: string, isError = false) => [ + { type: 'assistant', message: { content: [{ type: 'tool_use', id: 'review', name: 'Bash', input: { command } }] } }, + { type: 'user', message: { content: [{ type: 'tool_result', tool_use_id: 'review', content: output, is_error: isError }] } }, + ]; + const command = '_gstack_codex_timeout_wrapper 330 codex review --base main >"$TMPOUT" 2>"$TMPERR"\n' + + 'bun ~/.claude/skills/gstack/lib/outside-review-result.ts --label \'Codex review\' --exit "$_CODEX_EXIT" --stderr "$TMPERR" structured "$TMPOUT"'; + + test('the captured sandbox-unavailable review (recorded passed in CI) has no executed verdict', () => { + expect(codexReviewVerdicts(claudeOutsideExecutions(captured_codex_review_unavailable.transcript))).toEqual([]); + }); + + test('only the validator verdict printed by the codex command counts', () => { + expect(codexReviewVerdicts(claudeOutsideExecutions(run(command, `[P1] SQL injection\nVERDICT: findings\nFINDINGS: P1`, true)))).toEqual(['findings']); + expect(codexReviewVerdicts(claudeOutsideExecutions(run(command, 'Codex outside review unavailable: sandbox\nVERDICT: unavailable', true)))).toEqual(['unavailable']); + expect(codexReviewVerdicts(claudeOutsideExecutions(run('cat > codex-output.md < { for (const provider of ['codex', 'claude-code'] as const) { test(`${provider} persists full successful and failed results before cleanup`, () => { diff --git a/test/skill-e2e-workflow.test.ts b/test/skill-e2e-workflow.test.ts index 6cd9672bc..8d566469d 100644 --- a/test/skill-e2e-workflow.test.ts +++ b/test/skill-e2e-workflow.test.ts @@ -15,6 +15,7 @@ import * as path from 'path'; import * as os from 'os'; import { extractSkillBody } from './helpers/skill-fixture'; import { createCoverageAuditFixture } from './fixtures/coverage-audit-fixture'; +import { claudeOutsideExecutions, codexReviewVerdicts } from './helpers/outside-voice-evidence'; import { validateCoverageAudit, type CoverageFile } from './helpers/coverage-audit'; import { runRecordedOfficeHoursAttempt, OFFICE_HOURS_BUN_GRACE_MS } from './helpers/office-hours-attempt'; @@ -472,17 +473,17 @@ Write the full output (including the GATE verdict) to ${codexDir}/codex-output.m }); logCost('/codex review', result); - recordE2E(evalCollector, '/codex review', 'Codex skill E2E', result); - expect(result.exitReason).toBe('success'); - - // Check that output file was created with review content + // A review counts only when Codex ran: the shared validator printed an executed + // verdict for the codex command, and the written gate is not fail-closed. + const verdicts = codexReviewVerdicts(claudeOutsideExecutions(result.transcript)); const outputPath = path.join(codexDir, 'codex-output.md'); - if (fs.existsSync(outputPath)) { - const output = fs.readFileSync(outputPath, 'utf-8'); - // Should contain the CODEX SAYS header or GATE verdict - const hasCodexOutput = output.includes('CODEX') || output.includes('GATE') || output.includes('codex'); - expect(hasCodexOutput).toBe(true); - } + const output = fs.existsSync(outputPath) ? fs.readFileSync(outputPath, 'utf-8') : ''; + const executed = verdicts.some(verdict => ['findings', 'clean', 'unverified'].includes(verdict)); + const gated = /GATE: (?:PASS|FAIL \(\d+ critical|UNVERIFIED)/.test(output) && !/fail-closed/i.test(output); + recordE2E(evalCollector, '/codex review', 'Codex skill E2E', result, { passed: result.exitReason === 'success' && executed && gated }); + expect(result.exitReason).toBe('success'); + expect(executed, `no executed Codex review verdict in the codex command output (verdicts: ${verdicts.join(', ') || 'none'})`).toBe(true); + expect(gated, `codex-output.md has no executed GATE verdict:\n${output.slice(0, 1000)}`).toBe(true); }, CAPTURE_LONG_MS); }); From ba50f756f80846ba31ac6f50a652c8b2ed09e54e Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:29:45 +0000 Subject: [PATCH 03/10] I1: QA evidence names the probes to rerun after an input change, before the one-shot materialize ship-exploratory-late-input returned inconclusive in gate census 37158847998: the fixture locale changed after captures 001 and 002, the agent reran only input 9, and materialize (which publishes once) recorded capture 001 as open. Its next line then said to rerun what is open, which the published verdict no longer allowed. Every capture now lists the commands that observed an older input snapshot and have no rerun on current inputs (revalidate, plus the next line), and materialize's next line says the verdict is final. --- lib/qa-evidence.ts | 28 +++++++++++++++++++++++++--- test/qa-evidence.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/lib/qa-evidence.ts b/lib/qa-evidence.ts index 2f84c8e0f..efabc5c83 100644 --- a/lib/qa-evidence.ts +++ b/lib/qa-evidence.ts @@ -147,6 +147,26 @@ function requiredRemaining(root: string): { requiredRemaining?: string[] } { return { requiredRemaining: required.filter(command => !run.has(command)) }; } +const snapshotOf = (observed: unknown) => object(observed) && typeof observed.snapshot === 'string' ? observed.snapshot : undefined; + +/** + * Commands whose complete captures declared an older input snapshot than the + * latest capture and were not rerun on the current snapshot. Materialize keeps + * each one open (the verdict cannot pass), and it publishes only once, so + * every capture names them while they can still be rerun. + */ +function staleCommands(root: string): string[][] { + const observed = completeReceipts(root).flatMap(receipt => { + try { return [{ argv: receipt.argv as string[], snapshot: snapshotOf(readQaCapture(root, receipt.id).observed) }]; } catch { return []; } + }); + const current = observed.at(-1)?.snapshot; + if (current === undefined) return []; + const rerun = new Set(observed.filter(row => row.snapshot === current).map(row => JSON.stringify(row.argv))); + const stale = new Map(observed.filter(row => row.snapshot !== undefined && !rerun.has(JSON.stringify(row.argv))) + .map(row => [JSON.stringify(row.argv), row.argv])); + return [...stale.values()]; +} + async function capture(root: string, captureId: string, publicOutput: boolean, option: string, budget: string, command: string, args: string[], after?: { capture: string; hypothesis: string }) { id(captureId); if (!command || !['--deadline', '--timeout-ms'].includes(option)) throw new QaEvidenceError('Capture requires a deadline or finite command timeout'); @@ -235,10 +255,13 @@ async function capture(root: string, captureId: string, publicOutput: boolean, o completedAt, exitCode, signal: result.signal, status, observation, publicOutput, ...streams }; const sha256 = publish(root, `.qa-evidence/${captureId}/receipt.json`, receipt); + const revalidate = status === 'complete' ? staleCommands(root) : []; return { action: 'capture', id: captureId, status, sha256, exitCode, signal: result.signal, publicOutput, startedAt, completedAt, durationMs: Date.parse(completedAt) - Date.parse(startedAt), ...(remainingMs === undefined ? {} : { remainingMs }), ...(checkpointSha256 ? { checkpoint: captureId, checkpointSha256, link: `[checkpoint ${captureId}](exploration-${captureId}.json)` } : {}), - ...(status === 'complete' ? { next: `Another probe requires a checkpoint anchored on capture ${captureId}: add --after ${captureId} --hypothesis 'TEXT' before --. To stop exploring, run none.` } : {}), + ...(status === 'complete' ? { next: `Another probe requires a checkpoint anchored on capture ${captureId}: add --after ${captureId} --hypothesis 'TEXT' before --. To stop exploring, run none.${revalidate.length + ? ` Inputs changed since these commands ran; materialize runs once and keeps each one open (the verdict cannot pass) until it is rerun on current inputs: ${revalidate.map(argv => argv.join(' ')).join('; ')}.` : ''}` } : {}), + ...(revalidate.length ? { revalidate: revalidate.map(argv => argv.join(' ')) } : {}), ...requiredRemaining(root) }; } @@ -298,7 +321,6 @@ function materialize(root: string, source: string) { argv.push(JSON.stringify(captured.receipt.argv)); return { command: row.command, contract: row.contract, expected: row.expected, classification: row.classification, observed: captured.observed }; }); - const snapshotOf = (observed: unknown) => object(observed) && typeof observed.snapshot === 'string' ? observed.snapshot : undefined; const latestCapture = latestCompleteCapture(root); const currentSnapshot = latestCapture ? snapshotOf(readQaCapture(root, latestCapture).observed) : undefined; const superseded = currentSnapshot === undefined ? [] : annotations.evidence.filter((row: any, index: number) => { @@ -338,7 +360,7 @@ function materialize(root: string, source: string) { const sha256 = publish(root, 'evidence.json', { ...annotations, evidence, learning, verdict }); return { action: 'materialize', status: 'complete', sha256, annotationsSha256: hash(bytes), exitCode: 0, verdict, reportLinks: notes.map(note => `[checkpoint ${note.name.slice(12, 15)}](${note.name})`), - next: `Include every reportLinks entry in the Markdown report, and report the overall status as ${verdict.status}${verdict.open.length ? ` (open: ${verdict.open.join('; ')})` : ''}; rerun what is open first if a pass is required.` }; + next: `Include every reportLinks entry in the Markdown report, and report the overall status as ${verdict.status}${verdict.open.length ? ` (open: ${verdict.open.join('; ')})` : ''}; this verdict is final for this report root.` }; } const QA_EVIDENCE_USAGE = 'capture ROOT ID [--public] --deadline FILE|--timeout-ms MS [--after PREVIOUS_CAPTURE --hypothesis TEXT] -- COMMAND ARGS (--after publishes checkpoint ID linking PREVIOUS_CAPTURE to this probe; required after the first complete capture unless a checkpoint was published) | checkpoint ROOT ID CAPTURE OBSERVATION_COMMAND HYPOTHESIS NEXT_COMMAND | checkpoint ROOT ID INTENT_FILE | materialize ROOT ANNOTATIONS (annotations: {evidence: [{capture, command, contract, expected, classification: pass|superseded|product-defect|fail|setup-blocked|blocked|inconclusive}], limits: [..]}; revision, runtime, cwd and learning are filled in)'; diff --git a/test/qa-evidence.test.ts b/test/qa-evidence.test.ts index 11cb8204b..77299ad42 100644 --- a/test/qa-evidence.test.ts +++ b/test/qa-evidence.test.ts @@ -436,6 +436,34 @@ test('a superseded row closes only when the same native probe was rerun on the c expect(materialize(row('001', 'superseded'), row('002', 'pass'), row('003', 'pass'))).toEqual({ status: 'pass', open: [] }); }); +test('every capture after an input change names the commands still to rerun, until each has run on current inputs', () => { + // gate-census-6 of run 37158847998: the fixture locale changed after captures 001 and 002; the agent reran + // only input 9, materialized once with capture 001 still open, and the published verdict stayed inconclusive. + const f = fixture(); + const hypothesis = 'The input snapshot changed, so the earlier probes must be rerun on current inputs next.'; + const program = (input: string) => `console.log(JSON.stringify({ snapshot: require('node:fs').readFileSync('snap', 'utf8'), input: '${input}' }))`; + const probe = (id: string, after: string, input: string) => receipt(f.run('capture', f.root, id, '--timeout-ms', '4000', '--after', after, '--hypothesis', hypothesis, '--', process.execPath, '-e', program(input)).stdout); + const command = (input: string) => [process.execPath, '-e', program(input)].join(' '); + fs.writeFileSync(path.join(f.root, 'snap'), 'C'); + expect(receipt(f.capture('001', program('3')).stdout).revalidate).toBeUndefined(); + expect(probe('002', '001', '9').revalidate).toBeUndefined(); + fs.writeFileSync(path.join(f.root, 'snap'), 'POSIX'); + const changed = probe('003', '002', '10'); + expect(changed.revalidate).toEqual([command('3'), command('9')]); + expect(changed.next).toContain('materialize runs once and keeps each one open (the verdict cannot pass) until it is rerun on current inputs'); + expect(probe('004', '003', '9').revalidate).toEqual([command('3')]); + const current = probe('005', '004', '3'); + expect(current.revalidate).toBeUndefined(); + expect(current.next).not.toContain('Inputs changed'); + const row = (capture: string, classification: string) => ({ capture, command: `capture ${capture}`, contract: 'README.md', expected: 'declared', classification }); + f.json('annotations.json', { revision: 'fixture-revision', limits: ['Locale changed after capture 002.'], + evidence: [row('001', 'superseded'), row('002', 'superseded'), row('003', 'pass'), row('004', 'pass'), row('005', 'pass')] }); + const materialized = f.run('materialize', f.root, 'annotations.json'); + expect(materialized.status, materialized.stderr).toBe(0); + expect(receipt(materialized.stdout).verdict).toEqual({ status: 'pass', open: [] }); + expect(receipt(materialized.stdout).next).toContain('this verdict is final for this report root'); +}); + test('a descriptive classification is rejected before publication, so the corrected label can still materialize', () => { // gate-census-6 of run 36920606897: "pass (current snapshot)" left the one-shot verdict inconclusive. const f = fixture(); From 4e7447e691ddb42c01223d398e96310389ef916a Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:37:05 +0000 Subject: [PATCH 04/10] =?UTF-8?q?I1:=20the=20eng=20batching=20counter=20re?= =?UTF-8?q?ads=20records=20under=20"Section=20N=20=E2=80=94=20=20r?= =?UTF-8?q?eview"=20headings?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit plan-eng-multi-finding-batching failed in 6 of 12 runs with reviewCount 0-1 (or a timeout with every ask counted as Step 0). In run 37158847998 the agent asked 11 separate questions, each with a saved ledger record, but filed records R2-R10 under '## Section 1 — Architecture review'. The counter strips 'Section 1' but not the dash separator, so it rejected every record outside the Decision ledger and the floor never filled. Quoted, historical and unrelated headings still do not count. --- test/eng-batching-saved-ledger.test.ts | 11 +++ ...-batching-section-heading-37158847998.json | 70 +++++++++++++++++++ test/helpers/eng-seeded-coverage.ts | 2 +- 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 test/fixtures/eng-batching-section-heading-37158847998.json diff --git a/test/eng-batching-saved-ledger.test.ts b/test/eng-batching-saved-ledger.test.ts index e7f3d2210..34a62d90f 100644 --- a/test/eng-batching-saved-ledger.test.ts +++ b/test/eng-batching-saved-ledger.test.ts @@ -484,3 +484,14 @@ await import(${JSON.stringify(path.join(ROOT, 'test/skill-e2e-plan-eng-multi-fin fs.rmSync(temp, { recursive: true, force: true }); } }); + +test('a record under a "Section N — review" heading counts (run 37158847998 read 1 of 11 separate asks)', () => { + const run = JSON.parse(fs.readFileSync(path.join(import.meta.dir, 'fixtures/eng-batching-section-heading-37158847998.json'), 'utf8')); + const [d1, d2] = run.calls as NativePlanQuestionCall[]; + const counted = (plan: string) => factory(() => plan).isReviewAUQ(nativePlanCallFingerprint(structuredClone(d2!), 0, true), [d1!]); + expect(run.planAtD2).toContain('## Section 1 — Architecture review'); + expect(counted(run.planAtD2)).toBe(true); + for (const heading of ['## Section 1 — Appendix', '## Quoted example — Architecture review', '## Section 1 — Architecture review (historical)']) { + expect(counted(run.planAtD2.replace('## Section 1 — Architecture review', heading)), heading).toBe(false); + } +}); diff --git a/test/fixtures/eng-batching-section-heading-37158847998.json b/test/fixtures/eng-batching-section-heading-37158847998.json new file mode 100644 index 000000000..0477c5a63 --- /dev/null +++ b/test/fixtures/eng-batching-section-heading-37158847998.json @@ -0,0 +1,70 @@ +{ + "source": "https://github.com/garrytan/gstack/actions/runs/37158847998 (paid-slice-1, plan-eng-multi-finding-batching, reviewCount=1). The native D1/D2 calls are the retained observation.json calls; the saved plan is reconstructed from the PTY render of its Write/Edit diffs (R1 and R2 records verbatim; R1 tail and section prose shortened) because the plan file itself was not retained.", + "calls": [ + { + "sessionId": "4ed970bb-01eb-489a-a6c1-698160fe2666", + "toolUseId": "toolu_01WmzT44t4AJMDLMchxzCFs2", + "questions": [ + { + "question": "D1 — Use the job library's retry hook or build a custom inline scheduler?\nProject/branch/task: main branch, plan \"Add background job retry framework\" (PLAN.md), adding retries to 5 workers.\nELI10: When a job fails, something has to remember how many times it failed, wait the right amount of time, and run it again, even if the worker process crashes in between. The job library you already use does all of that and lets you plug in your own formula for the wait time. The plan proposes rebuilding that machinery by hand inside each of the 5 workers. The stakes: hand-rolled retry state is where lost jobs and double-runs come from after a deploy or crash.\nStakes if we pick wrong: a worker restart mid-backoff silently drops or double-runs jobs, and five copies of the bug have to be found and fixed separately.\nRecommendation: A because the plan's own words (\"same shape as the library version\") say the curve is the only thing you want to own, and the hook gives you exactly that with none of the persistence risk.\nCompleteness: A=9/10, B=5/10, C=investigate only, no score\nPros / cons:\nA) Library hook + one custom backoff function (recommended)\n ✅ Attempt count, delay scheduling and terminal failure state stay crash-safe in the library's store, not in worker memory\n ✅ Full control of the curve: one project-owned function returns the delay per attempt (human: ~1 day / CC: ~20 min)\n ❌ If this library's hook turns out not to expose attempt number or a custom function, you fall back to B or C after discovering it\nB) Custom inline scheduler in each worker, as planned\n ✅ Zero dependence on library hook semantics; the team reads every line of the retry path\n ✅ Can diverge per worker later without touching shared config\n ❌ Re-implements attempt persistence and rescheduling five times (human: ~1 week / CC: ~1 hour) and owns every crash and double-run edge case forever\nC) Bounded spike first: verify the hook, then decide\n ✅ Removes the one unknown (hook capability for this specific library) before committing\n ✅ Costs under an hour and leaves every other decision untouched\n ❌ Delays the decision; R1 stays pending and no retry code should start until it closes\nNet: You are trading one hour of verification risk (A) against a week of hand-rolled durability code and five copies of its bugs (B).", + "header": "Retry mechanism", + "multiSelect": false, + "options": [ + { + "label": "A) Library hook + custom backoff fn (recommended)", + "description": "Register one project-owned backoff function (and one attempt-log listener) with the existing job library's retry hook; each of the 5 workers opts in via config. Attempt state, scheduling and terminal failure live in the library. Full control of the curve. Completeness 9/10. human: ~1 day / CC: ~20 min. Risk: hook capability unverified for this library." + }, + { + "label": "B) Custom inline scheduler ×5", + "description": "Keep the plan as written: each worker computes delay, logs the attempt and re-dispatches itself. Owns attempt persistence and crash safety in project code, five times. Completeness 5/10. human: ~1 week / CC: ~1 hour plus ongoing maintenance." + }, + { + "label": "C) Spike the hook first", + "description": "Bounded investigation (≤1 hour human / ≤10 min CC): confirm the library's retry hook accepts a custom delay function and exposes the attempt number. R1 stays pending; no retry implementation starts. All other decisions unchanged." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D1 — Use the job library's retry hook or build a custom inline scheduler?\nProject/branch/task: main branch, plan \"Add background job retry framework\" (PLAN.md), adding retries to 5 workers.\nELI10: When a job fails, something has to remember how many times it failed, wait the right amount of time, and run it again, even if the worker process crashes in between. The job library you already use does all of that and lets you plug in your own formula for the wait time. The plan proposes rebuilding that machinery by hand inside each of the 5 workers. The stakes: hand-rolled retry state is where lost jobs and double-runs come from after a deploy or crash.\nStakes if we pick wrong: a worker restart mid-backoff silently drops or double-runs jobs, and five copies of the bug have to be found and fixed separately.\nRecommendation: A because the plan's own words (\"same shape as the library version\") say the curve is the only thing you want to own, and the hook gives you exactly that with none of the persistence risk.\nCompleteness: A=9/10, B=5/10, C=investigate only, no score\nPros / cons:\nA) Library hook + one custom backoff function (recommended)\n ✅ Attempt count, delay scheduling and terminal failure state stay crash-safe in the library's store, not in worker memory\n ✅ Full control of the curve: one project-owned function returns the delay per attempt (human: ~1 day / CC: ~20 min)\n ❌ If this library's hook turns out not to expose attempt number or a custom function, you fall back to B or C after discovering it\nB) Custom inline scheduler in each worker, as planned\n ✅ Zero dependence on library hook semantics; the team reads every line of the retry path\n ✅ Can diverge per worker later without touching shared config\n ❌ Re-implements attempt persistence and rescheduling five times (human: ~1 week / CC: ~1 hour) and owns every crash and double-run edge case forever\nC) Bounded spike first: verify the hook, then decide\n ✅ Removes the one unknown (hook capability for this specific library) before committing\n ✅ Costs under an hour and leaves every other decision untouched\n ❌ Delays the decision; R1 stays pending and no retry code should start until it closes\nNet: You are trading one hour of verification risk (A) against a week of hand-rolled durability code and five copies of its bugs (B).": "A) Library hook + custom backoff fn (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-10-03T22:38:43.760Z" + }, + { + "sessionId": "4ed970bb-01eb-489a-a6c1-698160fe2666", + "toolUseId": "toolu_01AwVJeKpu15hVeZXdJpZyrE", + "questions": [ + { + "question": "D2 — Webhook retries: send a stable idempotency key on every attempt?\nProject/branch/task: main branch, plan \"Add background job retry framework\" (PLAN.md), `processWebhookJob()` now retries under the library hook (D1).\nELI10: Today a webhook is sent once; if it fails, it is gone. With retries, a webhook that actually arrived but whose \"200 OK\" got lost on the way back will be sent again, and the customer gets it twice. The standard fix is to stamp every attempt with the same unique id so the receiver can recognize the repeat and ignore it. Without that stamp, duplicate orders, double emails and double charges on the customer side become your support tickets.\nStakes if we pick wrong: customers process the same event twice with no way to tell it was a repeat, and you cannot fix it after the fact because the attempts carry nothing in common.\nRecommendation: A because it keeps the retry benefit and gives every receiver a dedupe handle for roughly ten minutes of CC work.\nCompleteness: A=9/10, B=4/10, C=6/10\nPros / cons:\nA) Stable idempotency key per job on every attempt (recommended)\n ✅ Receivers can dedupe deterministically; the key is the job id (or existing event id) so no new storage is needed\n ✅ Documented at-least-once contract that matches what Stripe, GitHub and every major webhook sender ship (human: ~half day / CC: ~10 min)\n ❌ Receivers that ignore the header still see duplicates; the contract shifts from \"never twice\" to \"detectably twice\"\nB) Retry without a key, accept duplicates\n ✅ Smallest code change; nothing new on the wire\n ✅ Fine if every receiver is already idempotent by payload content\n ❌ Repeat deliveries are indistinguishable from new events; duplicate side effects at customers are permanent\nC) Exclude the webhook job from retries\n ✅ Preserves the exact current at-most-once behavior; the regression test stays trivial\n ✅ No wire or documentation change for customers\n ❌ Webhook failures stay unrecovered, which is the problem the plan set out to fix\nNet: A trades a header and a line of docs for a dedupe guarantee; B keeps the duplicates; C keeps the drops.", + "header": "Webhook idempotency", + "multiSelect": false, + "options": [ + { + "label": "A) Stable idempotency key per attempt (recommended)", + "description": "Every attempt of a webhook job sends the same per-job key (job id or existing event id) as a header and body field; delivery contract documented as at-least-once with dedupe key. Completeness 9/10. human: ~half day / CC: ~10 min. Other decisions unchanged." + }, + { + "label": "B) Retry without a key", + "description": "Webhook job retries under the library hook with no idempotency key; at-least-once with undetectable duplicates, documented. Completeness 4/10. human: ~1 hour / CC: ~2 min." + }, + { + "label": "C) Exclude webhook job from retries", + "description": "`processWebhookJob()` keeps at-most-once; the other 4 workers retry under D1. Webhook failures stay unrecovered. Completeness 6/10. R10 becomes moot for the webhook job only." + } + ] + } + ], + "answered": true, + "failed": false, + "answers": { + "D2 — Webhook retries: send a stable idempotency key on every attempt?\nProject/branch/task: main branch, plan \"Add background job retry framework\" (PLAN.md), `processWebhookJob()` now retries under the library hook (D1).\nELI10: Today a webhook is sent once; if it fails, it is gone. With retries, a webhook that actually arrived but whose \"200 OK\" got lost on the way back will be sent again, and the customer gets it twice. The standard fix is to stamp every attempt with the same unique id so the receiver can recognize the repeat and ignore it. Without that stamp, duplicate orders, double emails and double charges on the customer side become your support tickets.\nStakes if we pick wrong: customers process the same event twice with no way to tell it was a repeat, and you cannot fix it after the fact because the attempts carry nothing in common.\nRecommendation: A because it keeps the retry benefit and gives every receiver a dedupe handle for roughly ten minutes of CC work.\nCompleteness: A=9/10, B=4/10, C=6/10\nPros / cons:\nA) Stable idempotency key per job on every attempt (recommended)\n ✅ Receivers can dedupe deterministically; the key is the job id (or existing event id) so no new storage is needed\n ✅ Documented at-least-once contract that matches what Stripe, GitHub and every major webhook sender ship (human: ~half day / CC: ~10 min)\n ❌ Receivers that ignore the header still see duplicates; the contract shifts from \"never twice\" to \"detectably twice\"\nB) Retry without a key, accept duplicates\n ✅ Smallest code change; nothing new on the wire\n ✅ Fine if every receiver is already idempotent by payload content\n ❌ Repeat deliveries are indistinguishable from new events; duplicate side effects at customers are permanent\nC) Exclude the webhook job from retries\n ✅ Preserves the exact current at-most-once behavior; the regression test stays trivial\n ✅ No wire or documentation change for customers\n ❌ Webhook failures stay unrecovered, which is the problem the plan set out to fix\nNet: A trades a header and a line of docs for a dedupe guarantee; B keeps the duplicates; C keeps the drops.": "A) Stable idempotency key per attempt (recommended)" + }, + "unansweredQuestionIndices": [], + "answeredAt": "2026-10-03T22:40:17.387Z" + } + ], + "planAtD2": "# Plan\n\n## Decision ledger\n\n### R1: Retry mechanism — library hook or custom inline scheduler\nFinding: #1, P1, confidence 8/10, PLAN.md:4-6, plan-eng-review (native)\nPlan baseline: custom exponential-backoff scheduler inline in each of the 5 workers (original proposal, PLAN.md:4-6); no prior approval.\nRuntime evidence: unknown. The job library is named but not identified in the repo; hook capability is documented for BullMQ/Sidekiq/Celery/Oban (WebSearch, 2026-10-03), unverified for this project's library.\nComparison grid:\n\n| Choice | Current | A | B | C |\n|---|---|---|---|---|\n| R1 retry mechanism | custom inline scheduler ×5 (PLAN.md:4-6), pending | library retry hook + one project-owned backoff function | custom inline scheduler ×5 as planned | bounded spike (≤1 hour human / ≤10 min CC): confirm the library hook accepts a custom delay function and exposes attempt number; R1 stays pending |\n| Backoff curve shape | exponential, parameters unspecified (PLAN.md:4) | exponential, parameters pending (R3-R5) | exponential, parameters pending (R3-R5) | unchanged, pending |\n| R2 webhook delivery semantics | pending | pending | pending | pending |\n| R3 jitter / R4 delay cap / R5 exhaustion | pending | pending | pending | pending |\n| R6 shared envelope extraction | pending | pending (mostly moot under A) | pending | pending |\n| R7 regression contract / R8 graph cache | pending | pending | pending | pending |\n\nQuestion D1:\nD1 — Use the job library's retry hook or build a custom inline scheduler?\nProject/branch/task: main branch, plan \"Add background job retry framework\" (PLAN.md), adding retries to 5 workers.\nELI10: When a job fails, something has to remember how many times it failed, wait the right amount of time, and run it again, even if the worker process crashes in between. The job library you already use does all of that and lets you plug in your own formula for the wait time. The plan proposes rebuilding that machinery by hand inside each of the 5 workers. The stakes: hand-rolled retry state is where lost jobs and double-runs come from after a deploy or crash.\nStakes if we pick wrong: a worker restart mid-backoff silently drops or double-runs jobs, and five copies of the bug have to be found and fixed separately.\nRecommendation: A because the plan's own words (\"same shape as the library version\") say the curve is the only thing you want to own, and the hook gives you exactly that with none of the persistence risk.\nCompleteness: A=9/10, B=5/10, C=investigate only, no score\nPros / cons:\nA) Library hook + one custom backoff function (recommended)\n ✅ Attempt count, delay scheduling and terminal failure state stay crash-safe in the library's store, not in worker memory\n ✅ Full control of the curve: one project-owned function returns the delay per attempt (human: ~1 day / CC: ~20 min)\n ❌ If this library's hook turns out not to expose attempt number or a custom function, you fall back to B or C after discovering it\nB) Custom inline scheduler in each worker, as planned\n ✅ Zero dependence on library hook semantics; the team reads every line of the retry path\n ✅ Can diverge per worker later without touching shared config\n ❌ Re-implements attempt persistence and rescheduling five times (human: ~1 week / CC: ~1 hour) and owns every crash and double-run edge case forever\nC) Bounded spike first: verify the hook, then decide\n ✅ Removes the one unknown (hook capability for this specific library) before committing\n ✅ Costs under an hour and leaves every other decision untouched\n ❌ Delays the decision; R1 stays pending and no retry code should start until it closes\nNet: You are trading one hour of verification risk (A) against a week of hand-rolled durability code and five copies of its bugs (B).\nHeader: Retry mechanism\nOptions:\nA) Library hook + custom backoff fn (recommended)\nRegister one project-owned backoff function (and one attempt-log listener) with the existing job library's retry hook; each of the 5 workers opts in via config. Attempt state, scheduling and terminal failure live in the library. Full control of the curve. Completeness 9/10. human: ~1 day / CC: ~20 min. Risk: hook capability unverified for this library.\nB) Custom inline scheduler ×5\nKeep the plan as written: each worker computes delay, logs the attempt and re-dispatches itself. Owns attempt persistence and crash safety in project code, five times. Completeness 5/10. human: ~1 week / CC: ~1 hour plus ongoing maintenance.\nC) Spike the hook first\nState: approved\nActual answer: A) Library hook + custom backoff fn (recommended) (D1, user answer)\nAccepted scope: library hook\nHistory: none\n\n## Section 1 — Architecture review\n\nFindings (grounded in PLAN.md).\n\n### R2: Idempotency key on retried webhook deliveries\nFinding: #2, P1, confidence 8/10, PLAN.md:16-18, plan-eng-review (native)\nPlan baseline: original proposal: rewrite `processWebhookJob()` with retries and no stated delivery guarantee (PLAN.md:16-18). No prior approval.\nRuntime evidence: unknown. Whether outbound webhooks already carry an event/delivery id header is not visible in this repo.\nComparison grid:\n\n| Choice | Current | A | B | C |\n|---|---|---|---|---|\n| R1 retry mechanism | approved: library hook + custom backoff fn (D1) | unchanged (D1) | unchanged (D1) | unchanged (D1) |\n| R2 delivery guarantee for webhook retries | at-most-once today (PLAN.md:17); after plan: at-least-once with no dedupe handle, pending | at-least-once with a stable per-job idempotency key sent on every attempt (header + body field), documented for receivers | at-least-once, no key; duplicates accepted and documented | `processWebhookJob()` excluded from retries; at-most-once preserved, failures not recovered |\n| R10 retryable vs permanent classification | pending | pending | pending | moot for webhook, pending for other 4 workers |\n| R3 jitter / R4 delay cap / R5 exhaustion | pending | pending | pending | pending |\n| R7 regression contract / R8 graph cache / R9 sequencing | pending | pending | pending | pending |\n\nQuestion D2:\nD2 — Webhook retries: send a stable idempotency key on every attempt?\nProject/branch/task: main branch, plan \"Add background job retry framework\" (PLAN.md), `processWebhookJob()` now retries under the library hook (D1).\nELI10: Today a webhook is sent once; if it fails, it is gone. With retries, a webhook that actually arrived but whose \"200 OK\" got lost on the way back will be sent again, and the customer gets it twice. The standard fix is to stamp every attempt with the same unique id so the receiver can recognize the repeat and ignore it. Without that stamp, duplicate orders, double emails and double charges on the customer side become your support tickets.\nStakes if we pick wrong: customers process the same event twice with no way to tell it was a repeat, and you cannot fix it after the fact because the attempts carry nothing in common.\nRecommendation: A because it keeps the retry benefit and gives every receiver a dedupe handle for roughly ten minutes of CC work.\nCompleteness: A=9/10, B=4/10, C=6/10\nPros / cons:\nA) Stable idempotency key per job on every attempt (recommended)\n ✅ Receivers can dedupe deterministically; the key is the job id (or existing event id) so no new storage is needed\n ✅ Documented at-least-once contract that matches what Stripe, GitHub and every major webhook sender ship (human: ~half day / CC: ~10 min)\n ❌ Receivers that ignore the header still see duplicates; the contract shifts from \"never twice\" to \"detectably twice\"\nB) Retry without a key, accept duplicates\n ✅ Smallest code change; nothing new on the wire\n ✅ Fine if every receiver is already idempotent by payload content\n ❌ Repeat deliveries are indistinguishable from new events; duplicate side effects at customers are permanent\nC) Exclude the webhook job from retries\n ✅ Preserves the exact current at-most-once behavior; the regression test stays trivial\n ✅ No wire or documentation change for customers\n ❌ Webhook failures stay unrecovered, which is the problem the plan set out to fix\nNet: A trades a header and a line of docs for a dedupe guarantee; B keeps the duplicates; C keeps the drops.\nHeader: Webhook idempotency\nOptions:\nA) Stable idempotency key per attempt (recommended)\nEvery attempt of a webhook job sends the same per-job key (job id or existing event id) as a header and body field; delivery contract documented as at-least-once with dedupe key. Completeness 9/10. human: ~half day / CC: ~10 min. Other decisions unchanged.\nB) Retry without a key\nWebhook job retries under the library hook with no idempotency key; at-least-once with undetectable duplicates, documented. Completeness 4/10. human: ~1 hour / CC: ~2 min.\nC) Exclude webhook job from retries\n`processWebhookJob()` keeps at-most-once; the other 4 workers retry under D1. Webhook failures stay unrecovered. Completeness 6/10. R10 becomes moot for the webhook job only.\n\nState: pending\nActual answer: unanswered\nAccepted scope: none\nHistory: none\n" +} \ No newline at end of file diff --git a/test/helpers/eng-seeded-coverage.ts b/test/helpers/eng-seeded-coverage.ts index 12924bf7e..528e5138f 100644 --- a/test/helpers/eng-seeded-coverage.ts +++ b/test/helpers/eng-seeded-coverage.ts @@ -202,7 +202,7 @@ function recordedBatchingIssue(call: NativePlanQuestionCall, savedPlan: string): const recordSection = (at: number, depth: number) => { const owner = tokens.slice(0, at).filter(token => token.type === 'heading' && token.depth < depth).at(-1); if (!owner || owner.type !== 'heading') return false; - const name = clean(owner.text).replace(/^(?:Section\s+)?[1-9]\d*[.:]?\s*/i, ''); + const name = clean(owner.text).replace(/^(?:Section\s+)?[1-9]\d*(?:[.:]|\s+[—–-])?\s*/i, ''); return /^Decision ledger$/i.test(name) || /^(?:Architecture|Code quality|Tests?|Testing|Performance) review(?:\s*[—–:-]\s+[A-Za-z0-9][A-Za-z0-9 ,/()&-]*)?$/i.test(name); }; const words = (s: string) => (clean(s).toLowerCase().replace(/\(recommended\)/g, '').match(/[a-z][a-z0-9_]*/g) ?? []) From 814fc1548190c8b23adb73956859ea976496b436 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:37:05 +0000 Subject: [PATCH 05/10] I1: plan-eng-review-artifact names its report file without making it the only output Runs 37151477069 and 37158847998 wrote the whole review, including the GSTACK REVIEW REPORT, into review-output.md and never ran the Test Plan Artifact block. #3017 made the artifact a hard assertion and dropped the prompt's explicit artifact instruction, leaving 'Write your review to review-output.md' as the one named destination. The prompt now names that file as the selected report file and says every other output keeps the destination the skill gives it, without naming the artifact. --- test/skill-e2e-plan.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/skill-e2e-plan.test.ts b/test/skill-e2e-plan.test.ts index 8ea1d215d..03caa9893 100644 --- a/test/skill-e2e-plan.test.ts +++ b/test/skill-e2e-plan.test.ts @@ -482,7 +482,7 @@ Proceed directly to the full review. Skip any AskUserQuestion calls — this is In this fixture the gstack state root is GSTACK_HOME=${gstackHome} and the project slug is ${slug}. -Write your review to ${planDir}/review-output.md`, +Use ${planDir}/review-output.md as the selected report file. Every other output keeps the destination the skill gives it.`, workingDirectory: planDir, env: { GSTACK_HOME: gstackHome }, maxTurns: 25, From a818a991fb63699b6af142d5022d781fb418b2ef Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:39:08 +0000 Subject: [PATCH 06/10] I1: office-hours-brain-writeback reads the long skill with Read and skips the preamble 4 of 18 trials ended error_max_turns at the unchanged 12-turn budget with a gbrain search but no gbrain put. The failing trial in 37151477069 read the 1260-line rendered SKILL.md with cat, got truncated output, spent four more turns re-reading it with sed, then ran the preamble's skill-start block. The passing trials skipped the preamble. The prompt now says to read the skill with the Read tool and skip the preamble, onboarding, telemetry and contributor sections, as the plan-eng fixtures already do. Budget unchanged. --- test/skill-e2e-office-hours-brain-writeback.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/skill-e2e-office-hours-brain-writeback.test.ts b/test/skill-e2e-office-hours-brain-writeback.test.ts index 6b073aae8..f77b34f33 100644 --- a/test/skill-e2e-office-hours-brain-writeback.test.ts +++ b/test/skill-e2e-office-hours-brain-writeback.test.ts @@ -235,7 +235,7 @@ exit 0 model: resolveEvalModel('capture'), run: (signal) => runSkillTest({ signal, - prompt: `Read office-hours/SKILL.md for the workflow. + prompt: `Read office-hours/SKILL.md for the workflow with the Read tool (it is long; Bash output truncates it). Skip its preamble bash block, onboarding, telemetry and contributor sections — go straight to the workflow. Read pitch.md — that's a founder pitch coming to office hours. Select Startup Mode. Skip any AskUserQuestion — this is non-interactive; auto-decide the recommended option for any question. From dee7ba9dc1b64ef1e63c1069a1ae3af5eb79754c Mon Sep 17 00:00:00 2001 From: garrytan Date: Sat, 3 Oct 2026 23:59:23 +0000 Subject: [PATCH 07/10] I1: a QA helper call bun could not load is checked at the installed path, not failed outright review-exploratory-small-cli failed gate census 37162480720 with 'command outside declared caller observation interface': the actor dropped one character from the shard directory in a gstack-qa-evidence capture path, bun refused to load it ('Module not found', exit 1), and the next call ran the same capture correctly. A failed call whose only output is bun's refusal for a gstack-qa-evidence/deadline path is now held to the interface as the same command at the installed helper path, so a compound or undeclared command still fails. --- test/helpers/qa-callers-fixture.ts | 16 +++++++++++++++- test/qa-exploratory-callers.test.ts | 20 +++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/test/helpers/qa-callers-fixture.ts b/test/helpers/qa-callers-fixture.ts index eb4828ccc..c8391e519 100644 --- a/test/helpers/qa-callers-fixture.ts +++ b/test/helpers/qa-callers-fixture.ts @@ -249,6 +249,18 @@ export function qaCallerCommandAllowed(command: string, workflowCommands: string || /^\/?[\w./-]+\/bin\/gstack-(?:review-read|specialist-stats)$/.test(text); } +/** + * A QA helper call whose script path bun could not load (a mistyped path) + * executed nothing. Returns the same command at the installed helper path, so + * the caller can hold it to the declared interface; anything else is undefined. + */ +export function unloadedHelperCommand(tool: Pick, runtime: string): string | undefined { + const command = String(tool.input.command).trim(); + const script = /^bun (\/[\w./-]+\/bin\/(gstack-qa-(?:evidence|deadline))) /.exec(command); + if (!tool.failed || !script || tool.output.trim() !== `Exit code 1\nerror: Module not found "${script[1]}"`) return; + return `bun ${path.join(runtime, 'bin', script[2]!)}${command.slice(script[0].length - 1)}`; +} + const REVIEW_RECORD_STATUSES: Record = { review: ['clean', 'issues_found'], ship: ['clean', 'issues_found', 'unavailable'] }; export function validateCallerEvidence(input: { @@ -300,7 +312,9 @@ export function validateCallerEvidence(input: { if (input.requireCapturedEvidence && /^reports\/exploration-\d{3}\.json$/.test(relative)) errors.push('actor transcribed or overwrote helper-owned checkpoint'); } catch { errors.push('write outside the declared report/fixture interface'); } } - if (tool.name === 'Bash' && !qaCallerCommandAllowed(String(tool.input.command), input.workflowCommands, deadline)) { + const corrected = tool.name === 'Bash' && deadline ? unloadedHelperCommand(tool, deadline.runtime) : undefined; + if (tool.name === 'Bash' && !qaCallerCommandAllowed(String(tool.input.command), input.workflowCommands, deadline) + && !(corrected && qaCallerCommandAllowed(corrected, input.workflowCommands, deadline))) { errors.push('command outside declared caller observation interface'); } const record = tool.name === 'Bash' ? String(tool.input.command).match(/gstack-review-log '(.*)'(?: --finish \S+)?$/s)?.[1] : undefined; diff --git a/test/qa-exploratory-callers.test.ts b/test/qa-exploratory-callers.test.ts index b1ed75eb3..7471b23e8 100644 --- a/test/qa-exploratory-callers.test.ts +++ b/test/qa-exploratory-callers.test.ts @@ -6,7 +6,7 @@ import { spawnSync } from 'node:child_process'; import { callerExcerpt, callerReviewRecordTemplate, callerSnapshot, callerTools, createQaCallerFixture, qaCallerInstructions, QA_CALLER_CASES, QA_CALLER_TEST_MS, - qaCallerSessionOptions, qaCallerCommandAllowed, readCallerReceipt, retainQaCallerEvidence, runQaCaller, validateCallerEvidence, + qaCallerSessionOptions, qaCallerCommandAllowed, readCallerReceipt, retainQaCallerEvidence, runQaCaller, unloadedHelperCommand, validateCallerEvidence, type CallerProbe, type CallerReceipt, type QaCallerFixture, } from './helpers/qa-callers-fixture'; import type { runSkillTest, SkillTestResult } from './helpers/session-runner'; @@ -146,6 +146,24 @@ describe('caller native-event observer controls', () => { expect(prompt('ship-exploratory-small-cli')).toContain('otherwise issues_found (unavailable for missing dispatched reviewer output)'); }); + test('captured gate-census-5 typo: a helper path bun could not load is held to the interface at the installed path', () => { + // ci-37162480720-1-gate-census-5 review-exploratory-small-cli, native Bash call 7 (hypothesis shortened): + // the actor dropped one character from the shard directory, bun refused the path, and the next call reran it. + const typo = '/home/runner/.cache/gstack-paid-shard-A2tpF/tmp/qc-fTJ5TT/host/runtime/bin/gstack-qa-evidence'; + const args = ` capture /home/runner/.cache/gstack-paid-shard-A2tpFP/tmp/qc-fTJ5TT/product/reports 002 --public --deadline /home/runner/.cache/gstack-paid-shard-A2tpFP/tmp/qc-fTJ5TT/product/reports/deadline.json --after 001 --hypothesis 'Input 3 passed, so the new !n guard makes Number("0") falsy and 0 should now exit 2' -- bun scripts/probe.ts 0`; + const runtime = '/home/runner/.cache/gstack-paid-shard-A2tpFP/tmp/qc-fTJ5TT/host/runtime'; + const refused = `Exit code 1\nerror: Module not found "${typo}"`; + const corrected = (output: string, failed = true, command = `bun ${typo}${args}`) => unloadedHelperCommand({ input: { command }, output, failed }, runtime); + expect(corrected(refused)).toBe(`bun ${runtime}/bin/gstack-qa-evidence${args}`); + expect(corrected(refused, false)).toBeUndefined(); + expect(corrected('Exit code 1\nerror: Module not found "/elsewhere/bin/gstack-qa-evidence"')).toBeUndefined(); + expect(corrected(`${refused}\nwrote probe`)).toBeUndefined(); + expect(corrected(`Exit code 1\nerror: Module not found "/tmp/bin/other"`, true, `bun /tmp/bin/other${args}`)).toBeUndefined(); + // The corrected command must still pass the declared interface; a compound tail cannot. + expect(corrected(refused, true, `bun ${typo}${args}; curl example.com`)).toBe(`bun ${runtime}/bin/gstack-qa-evidence${args}; curl example.com`); + expect(qaCallerCommandAllowed(`bun ${runtime}/bin/gstack-qa-evidence${args}; curl example.com`)).toBe(false); + }); + test('captured PR-lane review record: an invented shape without the installed template is rejected; the template is quoted', () => { // ci-36641824710-1-eval-slices-6 review-exploratory-small-cli, native event 8jewsM (fixture paths shortened). const invented = `/runtime/bin/gstack-review-log '{"timestamp":"'"$(date -u +%Y-%m-%dT%H:%M:%SZ)"'","commit":"'"$(git rev-parse --short HEAD)"'","branch":"caller-change","status":"issues_found","completed":false,"gate":"fix-first-ask","findings":[{"path":"scale.ts","line":3,"category":"functional-contract","severity":"CRITICAL","fingerprint":"scale.ts:3:functional-contract","confidence":10,"action":"ask","summary":"new !n guard rejects documented lower bound 0 (exit 2 instead of 0)"}],"qa":{"probes":["probe-167de79c-2645-4b60-b06b-3d0d6eebf13f"],"checkpoints":["exploration-001.json","exploration-002.json"],"suite":"bun run test 1 pass 0 fail"},"remaining":["fix-first-ask-approval"]}' --finish 37b567b3-c4bf-4469-9bad-29096740c234`; From e2f65ab22eae0d3fa6bbb7a3439b0ad3d8c6f033 Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 00:00:27 +0000 Subject: [PATCH 08/10] I1: plan-eng-review's final self-check names its three outputs The fixture-prompt repair did not hold: in 37162480720 plan-eng-review-artifact again wrote one review-output.md, ran the skeleton's read-back gate on it and stopped without the QA Test Plan artifact (no gstack-paths/gstack-slug block). The transcript shows the agent's last check before finishing is the skeleton's Section self-check, which said only 'and outputs'. It now names the saved report, the QA Test Plan artifact and the Review Log, each at its own destination or presented as not persisted (the existing write policy). --- plan-eng-review/SKILL.md.tmpl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index 0f0d612d1..fd3f91918 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -177,7 +177,8 @@ follow **Blocked outcome** first; then resume here. ## Section self-check (before you finish) Confirm you read the section and completed Scope Challenge, Sections 1–4, -Outside Voice and outputs. If evidence is missing, Read `~/.claude/skills/gstack/plan-eng-review/sections/review-sections.md` +Outside Voice and outputs: the saved report, the QA Test Plan artifact and the +Review Log, each at its own destination (or presented as not persisted). If evidence is missing, Read `~/.claude/skills/gstack/plan-eng-review/sections/review-sections.md` and use Recovery routing above. Preserve verified work. {{EXIT_PLAN_MODE_GATE}} From 4de4432c853f2d1178f5f1e39b89e251b3da410c Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 00:03:36 +0000 Subject: [PATCH 09/10] I1: revert the plan-eng-review self-check wording; it did not change the artifact outcome A local run of plan-eng-review-artifact in the CI image (Claude Code 2.1.284) at e2f65ab22 still wrote one review-output.md in 8 turns and ran no Bash, so the QA Test Plan artifact block never ran. Keep the skill text unchanged; the case is deferred with its evidence. --- plan-eng-review/SKILL.md.tmpl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plan-eng-review/SKILL.md.tmpl b/plan-eng-review/SKILL.md.tmpl index fd3f91918..0f0d612d1 100644 --- a/plan-eng-review/SKILL.md.tmpl +++ b/plan-eng-review/SKILL.md.tmpl @@ -177,8 +177,7 @@ follow **Blocked outcome** first; then resume here. ## Section self-check (before you finish) Confirm you read the section and completed Scope Challenge, Sections 1–4, -Outside Voice and outputs: the saved report, the QA Test Plan artifact and the -Review Log, each at its own destination (or presented as not persisted). If evidence is missing, Read `~/.claude/skills/gstack/plan-eng-review/sections/review-sections.md` +Outside Voice and outputs. If evidence is missing, Read `~/.claude/skills/gstack/plan-eng-review/sections/review-sections.md` and use Recovery routing above. Preserve verified work. {{EXIT_PLAN_MODE_GATE}} From 9244437ab2df97164d54565f763c3eea3d2f0f0f Mon Sep 17 00:00:00 2001 From: garrytan Date: Sun, 4 Oct 2026 00:03:54 +0000 Subject: [PATCH 10/10] I1: seed the Codex files' periodic durations from their first executing run Every Codex file recorded ~1 s because its commands could not start in the container (codex-e2e*, outside-voice, workflow). Run 37162480720 executed them on the host-run job; its slice 24 walls replace the placeholders so the planner packs the Codex slice by real work (7 files, ~4.9 min estimated). --- scripts/paid-test-durations.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/scripts/paid-test-durations.json b/scripts/paid-test-durations.json index abeacf666..63ffa1375 100644 --- a/scripts/paid-test-durations.json +++ b/scripts/paid-test-durations.json @@ -116,10 +116,10 @@ "test/carve-section-loading-review.test.ts": 247902, "test/carve-section-loading-setup-gbrain.test.ts": 80994, "test/carve-section-loading-spec.test.ts": 176639, - "test/codex-e2e-recommendation-substance.test.ts": 1000, - "test/codex-e2e-shared-libs.test.ts": 1000, - "test/codex-e2e-sol-scope.test.ts": 1000, - "test/codex-e2e.test.ts": 1000, + "test/codex-e2e-recommendation-substance.test.ts": 17984, + "test/codex-e2e-shared-libs.test.ts": 53442, + "test/codex-e2e-sol-scope.test.ts": 37063, + "test/codex-e2e.test.ts": 120390, "test/llm-judge-recommendation.test.ts": 15334, "test/skill-e2e-arm-benchmark.test.ts": 75588, "test/skill-e2e-aside.test.ts": 1000, @@ -152,8 +152,8 @@ "test/skill-e2e-office-hours-phase4.test.ts": 41421, "test/skill-e2e-office-hours-section-loading.test.ts": 1200000, "test/skill-e2e-office-hours.test.ts": 136970, - "test/skill-e2e-outside-plan-disabled.test.ts": 53292, - "test/skill-e2e-outside-voice.test.ts": 1000, + "test/skill-e2e-outside-plan-disabled.test.ts": 54255, + "test/skill-e2e-outside-voice.test.ts": 186881, "test/skill-e2e-overlay-harness-claude-dedicated-tools-vs-bash-sonnet.test.ts": 85282, "test/skill-e2e-overlay-harness-claude-dedicated-tools-vs-bash.test.ts": 80465, "test/skill-e2e-overlay-harness-opus-4-7-effort-match-trivial.test.ts": 49282, @@ -198,7 +198,7 @@ "test/skill-e2e-test-value.test.ts": 332000, "test/skill-e2e-third-party-actions.test.ts": 1000, "test/skill-e2e-triage.test.ts": 1000, - "test/skill-e2e-workflow.test.ts": 1000, + "test/skill-e2e-workflow.test.ts": 110357, "test/skill-llm-eval.test.ts": 383454, "test/skill-routing-e2e.test.ts": 68843 }