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/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/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/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 } 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/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/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 }; 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/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) ?? []) 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/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/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/qa-evidence.test.ts b/test/qa-evidence.test.ts index 2b3960156..49a7faa99 100644 --- a/test/qa-evidence.test.ts +++ b/test/qa-evidence.test.ts @@ -469,6 +469,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(); 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`; 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. 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, 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); });