diff --git a/.github/workflows/evals.yml b/.github/workflows/evals.yml index 451f3a150..e7b81e55b 100644 --- a/.github/workflows/evals.yml +++ b/.github/workflows/evals.yml @@ -278,10 +278,14 @@ jobs: # Free Tests red on this head fails the run without paid work (re-run this # workflow after a green Free Tests rerun). Dispatches never wait: they are # explicit (eval:bg, census). Version Gate is not an input, so a stamp push - # waits for its own Free Tests only and always gets its gate. + # waits for its own Free Tests only and always gets its gate. A pr-summary + # run (a repository with a train block) plans no paid slices, so it skips + # the wait: the train gate and the dispatched screen carry the paid work, + # and Free Tests stays the required free check. readiness: runs-on: ubuntu-24.04 - if: github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository + needs: [route] + if: github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository && needs.route.outputs.mode != 'pr-summary' timeout-minutes: 90 permissions: contents: read diff --git a/.github/workflows/free-tests.yml b/.github/workflows/free-tests.yml index 92ec29173..443888a40 100644 --- a/.github/workflows/free-tests.yml +++ b/.github/workflows/free-tests.yml @@ -23,6 +23,22 @@ on: push: branches: [main] workflow_dispatch: + inputs: + free_runner: + description: 'Runner-sizing experiments only: Ubicloud runner for the free-suite shards (ubicloud-standard-2, -4 or -8; empty keeps FREE_SUITE_RUNNER). Never cite such a run as a PR or train Free Tests result.' + type: string + default: '' + free_shards: + description: 'Runner-sizing experiments only: shard count, 1-40 (empty keeps FREE_SUITE_SHARDS)' + type: string + default: '' + +# The one source of the free-suite runner and shard count. pull_request and +# push runs never see workflow_dispatch inputs, so free-plan resolves these +# (or a dispatch override) into outputs that free-suite reads. +env: + FREE_SUITE_RUNNER: ubicloud-standard-2 + FREE_SUITE_SHARDS: '20' # Keyed on the PR number, not head_ref: a bare branch name carries no fork # prefix, so same-name branches from two forks would share one group and a @@ -43,6 +59,7 @@ jobs: timeout-minutes: 5 outputs: matrix: ${{ steps.plan.outputs.matrix }} + runner: ${{ steps.plan.outputs.runner }} steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 with: @@ -56,9 +73,27 @@ jobs: - id: plan name: Inventory and balance the complete free suite # Warns in the job summary when more than 5 files are unseeded. + env: + RUNNER_OVERRIDE: ${{ inputs.free_runner }} + SHARDS_OVERRIDE: ${{ inputs.free_shards }} run: | - matrix=$(bun run scripts/test-free-shards.ts --ci-plan "$RUNNER_TEMP/free-plan.json" --shards 20) + set -euo pipefail + runner="${RUNNER_OVERRIDE:-$FREE_SUITE_RUNNER}" + shards="${SHARDS_OVERRIDE:-$FREE_SUITE_SHARDS}" + case "$runner" in + ubicloud-standard-2|ubicloud-standard-4|ubicloud-standard-8) ;; + *) echo "::error::free_runner '$runner' is not ubicloud-standard-2, -4 or -8"; exit 1 ;; + esac + case "$shards" in + ''|*[!0-9]*) echo "::error::free_shards '$shards' is not a whole number"; exit 1 ;; + esac + if [ "$shards" -lt 1 ] || [ "$shards" -gt 40 ]; then echo "::error::free_shards $shards is outside 1-40"; exit 1; fi + if [ "$runner" != "$FREE_SUITE_RUNNER" ] || [ "$shards" != "$FREE_SUITE_SHARDS" ]; then + echo "- Runner-sizing experiment: $shards shards on $runner (defaults $FREE_SUITE_SHARDS on $FREE_SUITE_RUNNER); not a PR or train Free Tests result" >> "$GITHUB_STEP_SUMMARY" + fi + matrix=$(bun run scripts/test-free-shards.ts --ci-plan "$RUNNER_TEMP/free-plan.json" --shards "$shards") echo "matrix=$matrix" >> "$GITHUB_OUTPUT" + echo "runner=$runner" >> "$GITHUB_OUTPUT" - name: Seed growth ratchet (60 s ceiling, shrink-only allowlist) run: | GSTACK_FREE_SEED_BASE="$(git merge-base HEAD origin/main)" bun test test/free-seed-ratchet.test.ts @@ -329,7 +364,7 @@ jobs: free-suite: needs: free-plan - runs-on: ubicloud-standard-8 + runs-on: ${{ needs.free-plan.outputs.runner }} timeout-minutes: 20 strategy: fail-fast: false @@ -476,6 +511,18 @@ jobs: - name: Build trusted CSO helper used by free contract tests run: bun run build:cso + # Runner sizing: the suite runs one test at a time, so a shard needs RAM + # and disk more than cores. Sample both while it runs and report the peak. + - name: Start the memory and disk sampler + run: | + cat > "$RUNNER_TEMP/sample-resources.sh" <<'EOF' + while :; do + printf '%s %s %s\n' "$(date +%s)" "$(free -k | awk 'NR == 2 { print $3 }')" "$(df -Pk / | awk 'NR == 2 { print $4 }')" + sleep 2 + done + EOF + nohup sh "$RUNNER_TEMP/sample-resources.sh" > "$RUNNER_TEMP/resources.log" 2>&1 & + # GSTACK_EXPECT_BINARIES=1 arms make-pdf/test/e2e/ci-prereqs.test.ts: # if a future edit drops the gate build (or poppler), the lane FAILS # instead of the gates silently self-skipping back to false green. @@ -494,6 +541,12 @@ jobs: GSTACK_FREE_RETRY_FLAKY: "1" GSTACK_FLAKE_LEDGER: ${{ runner.temp }}/flake-ledger.jsonl + - name: Report peak memory and minimum free disk + if: always() + run: | + if [ ! -s "$RUNNER_TEMP/resources.log" ]; then echo "RESOURCES: no samples"; exit 0; fi + awk '{ if ($2 > used) used = $2; if (free == "" || $3 < free) free = $3 } END { printf "RESOURCES: peak_used_mib=%d min_free_disk_mib=%d samples=%d\n", used / 1024, free / 1024, NR }' "$RUNNER_TEMP/resources.log" | tee -a "$GITHUB_STEP_SUMMARY" + - name: Upload strict shard result if: always() uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7 diff --git a/CHANGELOG.md b/CHANGELOG.md index 05b172940..2977c693d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,91 @@ # Changelog +## [1.91.372.0] - 2026-10-31 + +**A test that fails once in CI and passes when run again no longer turns the job red, and every such flake is recorded for the daily flaky-fix pass.** `gstack-flaky run` wraps any test command that writes a JUnit report (Playwright, `bun test`, jest, vitest, pytest). When a few tests fail, it runs the command once more. A test that failed and then passes is a flaky pass, so the job stays green, an Actions `::warning` and a step-summary row name it, and one line goes on a ledger. A rerun counts only when the runner itself reports a full green run, so a real failure, a coverage threshold, a test the first run never reached, a crash or a cancelled job stays red. Before this, each one-off flake meant rerunning the whole CI job by hand (five times in Truther's last evidence window, three of them for one test). + +### Added +- **`bin/gstack-flaky run --junit -- `**: reruns the whole command once when at most `--max-reruns` (3) tests failed and the runner made no retries of its own. `--rerun-scope test` opts in to rerunning only the failed tests (Playwright `--grep`/`--project`, bun `--test-name-pattern`), for suites with no coverage gate and no fail-on-flaky policy; it still falls back to the whole command when it sees an early stop, a coverage table or the runner's own retries. +- **Ledger** at `GSTACK_FLAKE_LEDGER_JUNIT` (default `/flake-ledger-junit.jsonl`), kept apart from the free suite's file-level ledger: one redacted line per flaky pass and per test that failed its rerun, with tree, commit, branch, run URL, attempts, scope and the first lines of the error. +- **`gstack-flaky ids --junit `** prints the test ids a report holds (`:::: title>`). +- Exit codes: 0 pass (flaky passes included), 1 a test failed, 2 usage, 3 no usable report, a crash or a killed runner, each with a `FLAKY_` and a `fix:` line. A missing, stale, empty or malformed report never passes, and an existing `--junit` file that is not a JUnit report is refused rather than deleted. + +### Docs +- `docs/flaky.md`: the GitHub Actions setup, runners, rerun scope and its preconditions, rules, output and exit codes. `docs/TESTING_INTERNALS.md#flake-ledger` points other repositories to it. + +### Tests +- `test/flaky-run.test.ts` with captured Playwright 1.64 and bun 1.4 JUnit reports under `test/fixtures/junit/`: flaky pass, real red, rerun scopes, coverage-threshold and early-stop runs, unreported failures, cancelled runs, report deletion and ledger redaction. + +### Gate integrity +- `test/helpers/free-fixtures-data.ts`: registers `test/fixtures/junit/**` to `test/flaky-run.test.ts`. No assertion, threshold or skip changed. + +## [1.91.371.0] - 2026-10-31 + +**A PR push in a repository with a merge train no longer waits ten minutes for a check that does no work.** Under a train policy every PR push runs E2E Evals in `pr-summary` mode, which plans no paid slices, yet its `readiness` job still waited out the 600-second quiet window and green Free Tests before the run could finish. Now `readiness` waits for `route` and skips in `pr-summary` mode, so E2E Evals on a PR push finishes in about a minute (58 s on #3609) instead of after the 10-minute wait. + +### Changed +- **`.github/workflows/evals.yml`**: `readiness` needs `route` and runs only when the mode is not `pr-summary`. In `pr` mode (a repository without a train) it still gates recovery and planning as before. In `pr-summary` mode a red Free Tests now shows on Free Tests alone, which stays the required free check; the train gate and the dispatched screen still carry the paid work. +- **`bun run test:health`**: the "PR lane push-to-verdict" rule text says `pr-summary` runs skip the wait since GSTA-149, so its p50 drops by about 10 minutes from this change on. +- **docs/TESTING_INTERNALS.md** describes the skip. + +### Tests +- `test/evals-pr-lane-workflow.test.ts` pins `readiness.needs` and the full `if`. `test/evals-train-workflow.test.ts` checks that a `pr-summary` push runs only `build-image`, `route` and `screen-summary`, and that `pr` mode still runs `readiness`. + +## [1.91.370.0] - 2026-10-31 + +**The free suite now runs on `ubicloud-standard-2` instead of `ubicloud-standard-8`, at the same 20 shards, for about a quarter of the runner cost.** Each shard runs its files one at a time, so it waits on disk and processes rather than CPU. In three paired rounds at one commit, the median per-shard time rose by 3 to 8 s, inside the round-to-round spread of −90 to +65 s, while runner-minutes stayed about equal (464 vs 475). Peak memory stayed under 2.5 GiB and free disk above 15 GiB. A 16-shard arm was slower beyond that spread, so the shard count stays at 20. + +### Changed +- **`.github/workflows/free-tests.yml`**: one workflow-level `FREE_SUITE_RUNNER` (`ubicloud-standard-2`) and `FREE_SUITE_SHARDS` (`20`) set the free-suite size for pull_request, push and dispatch runs alike. `free-plan` resolves them into its `runner` output and the plan's shard count, and `free-suite` reads `runs-on` from that output. The duration-seed recorder stays on `ubicloud-standard-8`. +- **Sizing experiments without a workflow edit**: `workflow_dispatch` takes `free_runner` (`ubicloud-standard-2`, `-4` or `-8` only) and `free_shards` (1–40). `free-plan` refuses any other value before planning, and a run that overrides either one says in its job summary that it is a sizing experiment, not a PR or train Free Tests result. +- **Every free-suite shard prints `RESOURCES: peak_used_mib=… min_free_disk_mib=… samples=…`** from a memory and disk sample every 2 s, even when the suite fails. If a shard shows OOM, ENOSPC or an unexplained Chromium launch failure, move `FREE_SUITE_RUNNER` to `ubicloud-standard-4`. + +### Tests +- `test/free-tests-workflow-wiring.test.ts`: runs the real `free-plan` step with a stubbed `bun`. It checks that the constant passes through when no override is given, that a valid override is honored, and that `ubuntu-24.04`, `ubicloud-standard-8 x`, `0`, `41` and `16; true` are refused before planning. It also checks that the sampler starts before the suite and that the report step runs `always()`. +- `test/free-durations-record-workflow.test.ts`: the recorder and `FREE_SUITE_RUNNER` must both be Ubicloud standard runners. They no longer have to be the same size, because the recorder stays on standard-8. + +## [1.91.369.0] - 2026-10-31 + +**`gstack-wtree` works again in a repository that gitignores `.bun/`.** Since GST-421, its `git add -A` carried an exclude pathspec naming `.bun`, and git refuses a pathspec naming an ignored path with exit 1. Every fingerprint call then failed, which broke `gstack-review-log --start`, the evidence checks and /ship in harness-evals (HARN-1, HARN-7, harness-evals#148; GST-449). + +### Fixed +- **The `.bun/` exclude applies only when the root `.bun/` exists and is not ignored.** The check reads the ignore rules alone (`git check-ignore --no-index`), so a file still tracked under an ignored `.bun/` doesn't bring the failure back. An ignored `.bun/` already stays out of `git add -A`, so the fingerprint is the same either way. + +### Tests +- `test/evidence.test.ts`: with `.bun/` or `/.bun` in `.gitignore`, `gstack-wtree` prints a tree and a lane that writes `.bun/` grades FRESH. With a file tracked under an ignored `.bun/`, the tree matches a plain `git add -A`, and editing or deleting that file changes it. The existing unignored `.bun/` cases still pass. + +## [1.91.368.0] - 2026-10-31 + +**`gstack-pregate --remote` no longer dies at 300 s while its gate waits on CI.** CAPY-1 (qm#2837) had a remote gate that waits for a CI run and prints nothing while it waits, so pregate killed it as stalled after 300 s, whatever `--timeout` said, and the heartbeat gave no hint of what the gate was doing (GST-431). + +### Fixed +- **The remote gate is bounded by its timeout only** (`--timeout`, else `timeout_s`, else 600 s; the gate also gets it as `PREGATE_TIMEOUT_S`). The stall timer still applies to the lanes runner and guards. +- **The remote gate's heartbeat names its last output line**, so a gate that prints the CI run it is waiting on shows it every 60 s. + +### Tests +- `test/pregate-remote.test.ts`: a gate quiet past `--stall-timeout` but inside `--timeout` clears its obligation; a gate past `--timeout` stays uncleared with `PREGATE_INCOMPLETE`. +- `test/pregate-supervise.test.ts`: a heartbeat with `lastLine` names the command's last output line. + +## [1.91.367.0] - 2026-10-31 + +**/ship's review collection no longer counts a reviewer that says it could not review as clean.** In the GSTA-196 replay a specialist replied in prose that the diff base did not exist and it was not returning NO FINDINGS. `collect` parsed that reply to zero findings, and `verify` marked the brief collected, so the review would have read clean from a reader that never saw the diff. What reviewers see is unchanged: every brief renders byte for byte as before. + +### Fixed +- **`gstack-review-dispatch collect` needs the clean marker a reader's own brief asks for before it counts a reply with no findings.** That is `NO FINDINGS` alone on a line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check; the `Recommendation:` line for the outside voice; and either one for the native adversarial voice. A reply with no finding and no marker (a refusal, an empty reply, a reader that never got the diff) is recorded with `review: false`, prints `COLLECT_NOT_REVIEWED` and exits 1. Re-sending the brief and collecting a real review voids it, and the outside voice, whose failure never blocks, is collected with `--unavailable ''` and verifies as `UNAVAILABLE`, never as a review. A failure text that still holds findings is refused there and collected normally, so nothing is lost. +- **`gstack-review-dispatch verify` reports that reader's brief `NOT_COLLECTED` and its output `NOT_REVIEWED`, so VERIFY reads dirty.** When the output file is still there and unchanged, verify checks the marker in it again, so a ledger written before this change is judged by the same rule. The GSTA-189 replay's combined ledger still verifies clean, with five `NO FINDINGS` specialists and one outside voice ending in `Recommendation:`. +- **A paragraph finding keeps its detail bullets.** The GSTA-196 native adversarial voice wrote four findings, each with unlabeled bullets and then a `Severity:` line. Every bullet closed the open finding, so each `Severity:` line opened a new one: 4 findings collected as 8. They now collect as 4, each at the stronger of its two labels. Only an unnumbered bullet with no file and line of its own counts as detail; a numbered item is still its own finding. +- **Every other finding collects exactly as before.** Main's parser and this one were run over every recorded reader output: 25 raw outputs from the GSTA-189 and GSTA-196 replays and 2,172 finding records from arms C6, L6, A3668 (both suites), G1 and L. 2,196 of the 2,197 collect the same findings with the same labels. The one difference is the GSTA-196 native sample, where 8 fragments join into 4 findings, each at its strongest fragment's label. + +## [1.91.366.0] - 2026-10-31 + +**A partial `gstack-post pr-body` post keeps the PR's intro.** Since GST-412, any body file with a `## ` section replaced the text above the PR's first heading, so a lane that posted only `## Screenshots` deleted a human intro or a `Review-loop exception:` line under the receipt (GStack Minder: "a partial pr-body post must keep the human intro"). + +### Fixed +- **Only a full post owns the text above the first heading.** A file with a receipt block above its first `## ` section, as /ship composes it, still replaces stale lead text and stale notes under the receipt. Any other file keeps them; its own lead text still replaces the PR's lead text. + +### Tests +- `test/gstack-post.test.ts`: a partial post keeps the intro and the note under the receipt, even with a caption of its own or a receipt further down; the full-post cases now carry a receipt, as /ship's body does. + ## [1.91.365.0] - 2026-10-31 **An unclosed code fence in an outside review no longer hides a later `[P1]` or not-ready verdict (GST-436).** Before, an outside-voice reply that said no issues were found, opened a code fence it never closed, and then listed a `[P1]` finding or ended with `Verdict: not ready` was classified clean, because everything after the fence was treated as quoted code. Now a P0 or P1 severity or a last `Verdict: not ready` after an unclosed fence still blocks. Text inside that fence still can't make a reply clean, so a quoted `Verdict: ready`, `[P3]` or `No issues found.` there is ignored, as before. diff --git a/TODOS.md b/TODOS.md index 778672f8b..89b7d0396 100644 --- a/TODOS.md +++ b/TODOS.md @@ -4,7 +4,6 @@ ### P3: GST-421 and GST-412 follow-ups (filed 2026-10-31, from the PR B /ship review) -- **Partial `gstack-post pr-body` posts drop the intro.** Since GST-412, any body file with a `## ` section replaces the text above the current body's first heading, so a lane that posts only `## Screenshots` deletes a human intro or a `Review-loop exception:` line written there (stderr names it; GitHub's edit history keeps it). Decide whether the preamble should be managed only when the file brings its own lead text or a receipt, or have /ship put the exception line under a named section. **Priority:** P3. - **Check the Approvals table's own cells.** `--check-prose` skips the whole rendered Approvals block, including the Asked, Covers and Still pending cells the lane types. Narrow the skip to the Exact words column. **Priority:** P3. - **The root `.bun/` blind spot.** `gstack-wtree` leaves every untracked file under a root `.bun/` out of the fingerprint, so an edit there never makes evidence STALE. Narrow the exclusion to Bun's cache subfolder if anything starts loading code from it. **Priority:** P3. diff --git a/VERSION b/VERSION index d04265b19..4c1d8c4ff 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.91.365.0 +1.91.372.0 diff --git a/agents-digest/gstack-AGENTS.md b/agents-digest/gstack-AGENTS.md index f731d61ff..fe7b8182c 100644 --- a/agents-digest/gstack-AGENTS.md +++ b/agents-digest/gstack-AGENTS.md @@ -1,4 +1,4 @@ -# gstack digest v1.91.365.0 — regenerate/re-copy after upgrading gstack +# gstack digest v1.91.372.0 — regenerate/re-copy after upgrading gstack Behavioral rules from gstack (https://github.com/garrytan/gstack), compressed for agent hosts without a full skill install. The full skills add workflows, diff --git a/bin/gstack-pregate b/bin/gstack-pregate index d11a0730c..b3f5297eb 100755 --- a/bin/gstack-pregate +++ b/bin/gstack-pregate @@ -75,7 +75,7 @@ const HELP = `gstack-pregate — cheap checks before anything paid or remote (do timeout catches a silent one sooner --stall-timeout kill a test command's process group after this long with no output and record the check incomplete (PREGATE_STALLED); default stall_timeout_s from the base .gstack/pregate.json, - else ${DEFAULT_STALL_TIMEOUT_S}; 0 disables + else ${DEFAULT_STALL_TIMEOUT_S}; 0 disables; the --remote gate is exempt (only --timeout bounds it) --remote run the base branch's remote_gate now instead of local tests (needs --allow-repo-commands; under --policy-from head the gate is still the base copy's); it clears the requires-remote obligation only on its PREGATE_REMOTE: tree= tests= failed=0 line with n > 0 diff --git a/bin/gstack-review-dispatch b/bin/gstack-review-dispatch index 9aaf07aa2..d7dd38f51 100755 --- a/bin/gstack-review-dispatch +++ b/bin/gstack-review-dispatch @@ -20,7 +20,7 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; import { EXIT, exitTableHelp } from '../lib/headless-artifacts'; import { BRIEF_ROLES, BriefError, briefFile, promptHash, renderBrief, sectionTexts, type BriefRole, type Slots } from '../lib/review-brief'; -import { collect, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger'; +import { cleanMarkers, collect, mayBeUnavailable, parseFindings, readJsonl, readLedger, readerKey, recordJudgment, recordSent, appendEvent, verify, type JudgedLine, type MergedLine, type VerifyRow } from '../lib/review-ledger'; import { buildManifest, chunkCount, chunkOf, manifestPath, markReader, readManifest, renderStage, writeManifest, STAGES, type Extra, type Manifest, type PassReader, type ReaderState, type Stage } from '../lib/review-pass'; import type { Dispatch } from '../lib/review-depth'; import { runScriptSync } from '../lib/script-invocation'; @@ -42,7 +42,11 @@ const HELP = `gstack-review-dispatch — verbatim reviewer briefs and loss-free sent --ledger --role [--name ] --sent the exact prompt a dispatch passed: BRIEF_SENT: … match, or BRIEF_MISMATCH: … (exit 1; the pass is not reviewed) collect --ledger --role [--name ] --output --merged - the reader's raw output: hashes it and appends each finding to merged.jsonl with its label as written + the reader's raw output: hashes it and appends each finding to merged.jsonl with its label as written. A reply with + no finding must carry its brief's clean marker (NO FINDINGS; the Recommendation: line for the voices); without one it is + COLLECT_NOT_REVIEWED (exit 1): a refusal, an empty reply or a reader that never saw the diff, and verify reports NOT_COLLECTED + until the brief is re-sent and answered with a review, which voids it. --unavailable '' (the outside voice only; refused when the + output holds findings) records its failure text as missing coverage: COLLECT_UNAVAILABLE, and verify prints UNAVAILABLE and unavailable=N, never a review judge --ledger --merged --judged judged lines {"id","severity","reason"?,"duplicate_of"?}: every merged id kept at its severity or above (a duplicate folds into one at least as severe); added and promoted findings need a reason. JUDGMENT: ok …, or @@ -268,7 +272,16 @@ function main(): number { const output = readFile(outFile, 'reader output'); const merged = flag('--merged'); if (!reader || output === undefined || !merged) { process.stderr.write('gstack-review-dispatch collect: needs --role, --output and --merged \n'); return EXIT.usage; } - const r = collect(ledger, reader, output, merged, path.resolve(outFile!)); + const unavailable = flag('--unavailable'); + if (unavailable !== undefined && !mayBeUnavailable(reader)) { process.stderr.write(`gstack-review-dispatch collect: --unavailable is for the outside voice only; ${reader}'s missing coverage leaves the review incomplete, never clean\n`); return EXIT.usage; } + if (unavailable !== undefined && !unavailable.trim()) { process.stderr.write('gstack-review-dispatch collect: --unavailable needs a reason\n'); return EXIT.usage; } + if (unavailable !== undefined && parseFindings(reader, output).length) { process.stderr.write(`gstack-review-dispatch collect: ${reader}'s output holds findings; collect it without --unavailable so none is lost\n`); return EXIT.usage; } + const r = collect(ledger, reader, output, merged, path.resolve(outFile!), unavailable); + if (unavailable !== undefined) { process.stdout.write(`COLLECT_UNAVAILABLE: reader=${reader} sha256=${r.sha} reason=${JSON.stringify(unavailable)} (missing outside coverage: unavailable, never clean)\n`); return EXIT.ok; } + if (!r.review) { + process.stdout.write(`COLLECT_NOT_REVIEWED: reader=${reader} findings=0 sha256=${r.sha} (no finding and no ${cleanMarkers(reader).map(([m]) => m).join(' or ')} line: a refusal, an empty reply or a reader that never saw the diff is not a review; verify reports this brief NOT_COLLECTED)\n`); + return EXIT.fail; + } process.stdout.write(`COLLECT: reader=${reader} findings=${r.lines.length} sha256=${r.sha} merged=${merged}\n`); return EXIT.ok; } @@ -290,7 +303,7 @@ function main(): number { process.stdout.write(`TARGETED_RECORD: classes=${record.classes.join(',')} status=${record.status} findings=${record.findings} brief=${record.brief_sha.slice(0, 12)} rules=${record.rules}\n`); if (flag('--targeted-out')) fs.writeFileSync(flag('--targeted-out')!, JSON.stringify(record) + '\n'); } - process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length} voided=${v.voided}\n`); + process.stdout.write(`VERIFY: ${v.clean ? 'clean' : 'dirty'} briefs=${v.briefs} outputs=${v.rows.filter(r => r.kind === 'OUTPUT').length} voided=${v.voided}${v.unavailable ? ` unavailable=${v.unavailable}` : ''}\n`); return v.clean ? EXIT.ok : EXIT.fail; } process.stderr.write(`gstack-review-dispatch: unknown command ${cmd}\n${HELP}\n`); diff --git a/bin/gstack-wtree b/bin/gstack-wtree index a843f75d4..c8034e375 100755 --- a/bin/gstack-wtree +++ b/bin/gstack-wtree @@ -7,7 +7,8 @@ # that index. An untracked `.bun/` folder at the repository root (Bun's install # cache, which a test run can create) is left out so a passing run does not # look like a mid-run edit (GST-421); tracked files under it still count, and a -# tree without that folder hashes exactly as a plain `git add -A` would. +# tree without that folder (or with it gitignored) hashes exactly as a plain +# `git add -A` would. # Properties that make this the right staleness fingerprint, vs # `git rev-parse HEAD^{tree}`: # @@ -87,7 +88,14 @@ if [ -n "$REAL_INDEX" ] && [ -f "$REAL_INDEX" ] && cp "$REAL_INDEX" "$TMPIDX" 2> else git -C "$TOP" read-tree HEAD 2>/dev/null || exit 1 fi -if ! ERR=$(git -C "$TOP" add -u 2>&1 >/dev/null) || ! ERR=$(git -C "$TOP" add -A -- . ':(exclude,top).bun' 2>&1 >/dev/null); then +# The exclude is added only when the root .bun/ exists and is not ignored: an +# exclude pathspec naming an ignored path makes `git add -A` exit 1 (GST-449), +# and an ignored .bun/ stays out of a plain `git add -A` anyway. --no-index +# asks the ignore rules alone; without it a tracked file under .bun/ makes git +# call .bun not ignored. +EXCLUDE_BUN="" +if [ -e "$TOP/.bun" ] && ! git -C "$TOP" check-ignore --no-index -q .bun 2>/dev/null; then EXCLUDE_BUN=':(exclude,top).bun'; fi +if ! ERR=$(git -C "$TOP" add -u 2>&1 >/dev/null) || ! ERR=$(git -C "$TOP" add -A -- . ${EXCLUDE_BUN:+"$EXCLUDE_BUN"} 2>&1 >/dev/null); then echo "gstack-wtree: cannot stage the working tree: $(printf '%s' "$ERR" | head -1)" >&2 exit 1 fi diff --git a/changelog.d/gsta158-flaky-run.md b/changelog.d/gsta158-flaky-run.md deleted file mode 100644 index 33d494315..000000000 --- a/changelog.d/gsta158-flaky-run.md +++ /dev/null @@ -1,16 +0,0 @@ -**A test that fails once in CI and passes when run again no longer turns the job red, and every such flake is recorded for the daily flaky-fix pass.** `gstack-flaky run` wraps any test command that writes a JUnit report (Playwright, `bun test`, jest, vitest, pytest). When a few tests fail, it runs the command once more. A test that failed and then passes is a flaky pass, so the job stays green, an Actions `::warning` and a step-summary row name it, and one line goes on a ledger. A rerun counts only when the runner itself reports a full green run, so a real failure, a coverage threshold, a test the first run never reached, a crash or a cancelled job stays red. Before this, each one-off flake meant rerunning the whole CI job by hand (five times in Truther's last evidence window, three of them for one test). - -### Added -- **`bin/gstack-flaky run --junit -- `**: reruns the whole command once when at most `--max-reruns` (3) tests failed and the runner made no retries of its own. `--rerun-scope test` opts in to rerunning only the failed tests (Playwright `--grep`/`--project`, bun `--test-name-pattern`), for suites with no coverage gate and no fail-on-flaky policy; it still falls back to the whole command when it sees an early stop, a coverage table or the runner's own retries. -- **Ledger** at `GSTACK_FLAKE_LEDGER_JUNIT` (default `/flake-ledger-junit.jsonl`), kept apart from the free suite's file-level ledger: one redacted line per flaky pass and per test that failed its rerun, with tree, commit, branch, run URL, attempts, scope and the first lines of the error. -- **`gstack-flaky ids --junit `** prints the test ids a report holds (`:::: title>`). -- Exit codes: 0 pass (flaky passes included), 1 a test failed, 2 usage, 3 no usable report, a crash or a killed runner, each with a `FLAKY_` and a `fix:` line. A missing, stale, empty or malformed report never passes, and an existing `--junit` file that is not a JUnit report is refused rather than deleted. - -### Docs -- `docs/flaky.md`: the GitHub Actions setup, runners, rerun scope and its preconditions, rules, output and exit codes. `docs/TESTING_INTERNALS.md#flake-ledger` points other repositories to it. - -### Tests -- `test/flaky-run.test.ts` with captured Playwright 1.64 and bun 1.4 JUnit reports under `test/fixtures/junit/`: flaky pass, real red, rerun scopes, coverage-threshold and early-stop runs, unreported failures, cancelled runs, report deletion and ledger redaction. - -### Gate integrity -- `test/helpers/free-fixtures-data.ts`: registers `test/fixtures/junit/**` to `test/flaky-run.test.ts`. No assertion, threshold or skip changed. diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index 8950635d8..ecd162ccc 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -197,7 +197,15 @@ shards get duration-aware walls (`max(base, predicted × 3, files × 5s)`). The legacy `--shards N --shard i` path keeps stable hash indices. Required CI uses one duration-packed `--ci-plan`, 20 ordinary `--ci-run` shards plus a separate exclusive-fixture shard, and a `--ci-verify` aggregate. CI shards still run on -independent machines without ordering unrelated jobs. The public +independent machines without ordering unrelated jobs. Two workflow-level +constants in `free-tests.yml` set that size for every event: `FREE_SUITE_RUNNER` +(`ubicloud-standard-2`) and `FREE_SUITE_SHARDS` (`'20'`), which `free-plan` +resolves into the shards' runner and shard count. A `workflow_dispatch` run may +override them with `free_runner` (`ubicloud-standard-2`, `-4` or `-8`) and +`free_shards` (1–40) for runner-sizing experiments only; its job summary says +so, and it is never cited as a PR or train Free Tests result. Each shard prints +`RESOURCES: peak_used_mib=… min_free_disk_mib=… samples=…` from a memory and +disk sample every 2 seconds, even when the suite fails. The public `TREE_MUTATING` map now classifies exclusive host-state fixtures; its sole entry is `test/bootstrap-retention.test.ts`, whose same-UID nondumpable actors affect host-wide procfs permission checks. Locally, this file runs only after @@ -1274,7 +1282,10 @@ re-run keeps the creation time, so it does not wait out the window again). A head that is no longer the PR's head reports "superseded by " and plans nothing. Selection, trials and verdict rules do not change. Version Gate is not an input, so a stamp push waits only for its own Free Tests and always gets a -gate. Dispatches (`eval:bg`, the census) never wait. +gate. Dispatches (`eval:bg`, the census) never wait. Neither does a +`pr-summary` run (a repository whose policy declares a train): it plans no paid +slices, so `readiness` is skipped, the run ends with the screening summary, and +a red Free Tests shows on Free Tests alone. **Verdict reuse (R2a restamp-only, R2 main moved; a gate edit).** A PR head reuses the verdict of the newest executed gate of the same PR instead of @@ -1511,7 +1522,9 @@ refresh to a maintainer. - `free-durations-record.yml` (Mondays 04:00 UTC, before test-health, and on dispatch) records the Linux seed on `ubicloud-standard-8` with `scripts/ubicloud/setup-free-suite.sh` and the Windows seed through - `windows-free-tests.yml`'s `record_durations` path. When either recording adds + `windows-free-tests.yml`'s `record_durations` path. The recorder stays on + `ubicloud-standard-8` while the free-suite shards run on `FREE_SUITE_RUNNER` + (`ubicloud-standard-2`); both are Ubicloud standard runners. When either recording adds or drops a test file, it force-updates the `bot/free-duration-seeds` branch, opens or refreshes one seed PR whose body states the ratchet result, and dispatches Free Tests, Windows Free Tests and Quality gate on it. Those diff --git a/docs/pregate.md b/docs/pregate.md index b3b5ef4ed..b9d7596a3 100644 --- a/docs/pregate.md +++ b/docs/pregate.md @@ -85,7 +85,7 @@ the byte-pinned `/ship` renders in `test/fixtures/golden/`). | `serial_patterns` | test-file globs the `lanes` check runs one process per file, apart from the unit files (default `**/*.serial.test.*`) | | `sealed` | sealed or eval output that must never reach a public destination; read by the pre-push hook and `gstack-pregate sealed` (see [sealed output](#sealed)) | | `price_file` | repo-relative path of the paid gate's call-plan data file (`GSTACK_EVAL_CALLS v1`); `gstack-eval-plan price-gate` prices from it and never executes anything ([docs/eval-calls.md](eval-calls.md)) | -| `stall_timeout_s` | seconds a test command may print nothing before its process group is killed and the check is `incomplete` (`PREGATE_STALLED`); default 300, `0` disables; `--stall-timeout` overrides (see [supervision](#supervision)) | +| `stall_timeout_s` | seconds a test command may print nothing before its process group is killed and the check is `incomplete` (`PREGATE_STALLED`); default 300, `0` disables; `--stall-timeout` overrides; the `--remote` gate is exempt (see [supervision](#supervision)) | | `timeout_s` | seconds each runner process may take (the unit-file group, each serial file, each guard without its own `timeout_s`, and the remote gate's deadline) when `--timeout` is not passed; 1 to 3600, default 600. `gstack-ship-preflight` and the `finish` fallback run pregate without `--timeout`, so this is how a repo with long lanes sets theirs | | `remote_gate` | the repo's remote test gate command; under 8 cores the local test tier becomes a `requires-remote` obligation naming it, and `--remote` runs it (see [remote gate](#remote-gate)) | | `allowlist` | `[{ finding, paths?, pattern?, reason }]`: benign findings the `secrets` check accepts and names as allowlisted, by gstack-redact finding id or non-secret category, path globs and a pattern anchored at the finding; never `env.value` (see [allowlist](#allowlist)) | @@ -411,7 +411,9 @@ test-tier command: - is killed as **stalled** when its log does not grow for the stall timeout (`--stall-timeout `, else `stall_timeout_s`, else 300; `0` disables): the check is `incomplete` with `PREGATE_STALLED` and names the last file - started; the next serial file still runs; + started; the next serial file still runs. The `--remote` gate is exempt: a + gate that waits on CI is quiet by design, so only its timeout bounds it, and + its heartbeats add `; last output: `; - is killed as **timed out** when that runner process reaches its timeout (`--timeout `, else `timeout_s`, else 600; per runner process, so many serial files that each fit still pass): the check is `incomplete`, its detail @@ -452,7 +454,11 @@ the supervision above. Close the obligation one of two ways: `PREGATE_REMOTE: tree= tests= failed= [spent=]`, reporting the tree it actually tested. The obligation clears only when that tree is `PREGATE_TREE`, `tests` > 0, `failed` = 0 and the gate exits 0; anything else - leaves it uncleared and the check `incomplete`. The cap defaults to + leaves it uncleared and the check `incomplete`. The gate runs until it exits + or reaches the timeout (`--timeout`, else `timeout_s`, else 600 s); the stall + timer does not apply, and each 60 s heartbeat names the gate's last output + line, so a gate that prints what it is waiting on (a CI run id and its + status) shows it. The cap defaults to `$GSTACK_SPEND_CAP_USD`; there is no default spend. Spend prints as `spent, reserved, unknown`: a gate that prints no `spent=` is `unknown`, counted at the cap. diff --git a/docs/review-policy.md b/docs/review-policy.md index d86c1b71d..301d187e5 100644 --- a/docs/review-policy.md +++ b/docs/review-policy.md @@ -473,7 +473,14 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: second line for the same field starts another: `Severity` raises its label, `Blocks: yes` makes it CRITICAL and `Blocks: no` never lowers it; an inline `Blocks: yes` is CRITICAL too, and with no finding above it is one. The native - adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167). A phantom finding costs a fold; a lost one costs the gate. + adversarial voice's FIXABLE collects as CRITICAL (normalizer v2, GSTA-167). An unnumbered, unlabeled bullet with no file:line + under a paragraph finding whose Severity or Blocks line has not come yet is that finding's detail, so the field line after it + lands on the finding; a numbered item is its own finding. + A phantom finding costs a fold; a lost one costs the gate. + A reply with no finding counts as a review only with the clean marker its own brief asks for: `NO FINDINGS` alone on a + line for the specialists, the batched specialist, the targeted reader, the Red Team and fix-check, the `Recommendation:` + line for the outside voice, either for the native adversarial voice. Without one (a refusal, an empty reply, a reader + that never saw the diff) collect records `review: false`, prints `COLLECT_NOT_REVIEWED` and exits 1. 4. Lead judgment writes `judged.jsonl`; `judge` refuses a set that drops a merged finding, lowers its severity, folds it into a less severe duplicate, or adds or promotes one without a reason (`JUDGMENT_REJECTED`). A merged file missing any id @@ -481,10 +488,14 @@ ones, and the arm lost blockers. `bin/gstack-review-dispatch` closes both ends: 5. `verify --ledger ` replays all of it from the run's ledger: rendered against dispatched hashes per render (each render is one dispatch of its reader, so a two-pass ledger checks pass-1 sends against pass-1 briefs; a later render prints as - `@2`), a collected output for every sent reader (`NOT_COLLECTED` - otherwise; a reader that failed is collected from its failure text, where any - label word becomes a finding the judge folds, and its coverage is logged - `unavailable` in the review-log row). A mismatched send re-sent + `@2`), a collected output that is a review for every sent reader (`NOT_COLLECTED` + otherwise, and the output row `NOT_REVIEWED`; the marker is checked again in the output file when it is unchanged, so an + older ledger meets the same rule; a non-review the same dispatch later replaced with a review, by re-sending the brief, is `VOIDED` + and counted in `voided=N`). The outside voice may fail without blocking: `collect --unavailable ''` records its failure text + as missing coverage (`COLLECT_UNAVAILABLE`); an empty reason or an output that holds findings is refused (exit 2), so a partial + answer is collected normally and loses nothing. Verify prints its brief and output `UNAVAILABLE` and `unavailable=N` on the + verdict line, and its coverage is logged `unavailable` in the review-log row; any other reader is refused (exit 2), because its + missing coverage leaves the review incomplete. A mismatched send re-sent byte-exact before anything was collected is void and counted (`voided=N` on the verdict line); one that produced an output stays `MISMATCH`. Then output hashes against the merged lines they produced, and the judgment rerun against the diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 2a905e71d..c33b6a56a 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -2522,12 +2522,15 @@ same-titled section (title compared case-insensitively), the ````gstack-ship-receipt```` block is a section of its own so a receipt rewrite replaces only the receipt, sections the file does not rewrite stay in place, and new sections are appended after them. The text above the first -`## ` heading belongs to the file whenever the file has a `## ` section: the -current body's text there (the opening lines, and any lines between the receipt -and the first heading, or the whole body when it has no `## ` heading) is -replaced by the file's own text above its first heading, or dropped when the -file has none. A file with no `## ` section (a receipt-only or text-only post) -leaves that text alone. One stderr line names what was replaced, kept, added +`## ` heading belongs to the file when the file is a full post: a +receipt block above its first `## ` section, as /ship composes it. Then the current body's text there (the opening lines, and +any lines between the receipt and the first heading, or the whole body when it +has no `## ` heading) is replaced by the file's own text above its first +heading, or dropped when the file has none. A partial post (`## ` sections +with no receipt above them, such as a lane posting `## Screenshots`) and a +file with no `## ` section (a receipt-only or text-only post) leave that text alone, so a human +intro or a `Review-loop exception:` line under the receipt survives; only +opening lines the file brings of its own replace the PR's opening lines. One stderr line names what was replaced, kept, added and dropped, a dropped piece with its first line, or with that line withheld when the redaction scan flags it (`gstack-post: pr-body merged by section: replaced What changed; kept ship receipt, Why, Approvals; added @@ -3464,8 +3467,7 @@ the lane log, then rerun or split the selection. A tool missing from PATH is ### `PREGATE_STALLED` -**Meaning.** A test command the lanes check (or a guard, or the `--remote` -gate) launched wrote nothing to its log for the stall timeout (default 300 s, +**Meaning.** A test command the lanes check (or a guard) launched wrote nothing to its log for the stall timeout (default 300 s, `stall_timeout_s` in the base branch's `.gstack/pregate.json` or the user-owned file, or `--stall-timeout`). pregate killed its process group and names the last test file started. The check is incomplete, never pass. @@ -3474,7 +3476,9 @@ file started. The check is incomplete, never pass. child, a server that never exits, a synchronous wait), fix it and rerun `gstack-pregate --stage tests`. A test that is legitimately quiet for longer needs a larger `--stall-timeout`; `0` disables the stall kill and leaves only -`--timeout`. +`--timeout`. The `--remote` gate never stalls: it is bounded by `--timeout` +(else `timeout_s`), so a gate that waits on CI needs a timeout long enough for +the CI run. ### `PREGATE_RECEIPT_INVALID` diff --git a/lib/gstack-post.ts b/lib/gstack-post.ts index dca9b81f5..1860170ac 100644 --- a/lib/gstack-post.ts +++ b/lib/gstack-post.ts @@ -9,8 +9,9 @@ * `pr-body` merges by `## ` section into the PR's current body (read through * the same runner): a section in the file replaces the same-titled one, the * ship-receipt block is a section of its own, sections the file does not - * rewrite are kept, new ones are appended, and a file with a `## ` section - * replaces the text above the first heading; `--replace` overwrites instead. + * rewrite are kept, new ones are appended, and a file with a receipt above + * its first `## ` section replaces the text above the first heading; + * `--replace` overwrites instead. * The scan runs on the merged bytes, which are the bytes sent. * * Exit codes: 0 posted; 1 HIGH finding (or oversize input) refused; @@ -212,25 +213,28 @@ function splitSections(body: string): Section[] { * A file section whose title the current body holds more than once (an old * description pasted under the new one) matches no single section, so the * merge refuses instead of replacing one copy and keeping the stale other. - * A file with at least one `## ` section manages the preamble (GST-412): every + * A full post (a file with a receipt block above its first `## ` section, as + * /ship composes it) manages the preamble (GST-412): every * non-blank text section before the current body's first heading (the leading * text, text after a receipt block) is dropped, the file's own text before its * first heading takes its place (leading text first, text after the file's * receipt right after the receipt), a dropped piece the file brings no text for * is listed in `dropped` with its first line (withheld when the redaction scan * flags it, since the merged bytes no longer hold it), and the receipt keeps the rules - * above. A file without a `## ` section merges as described above. + * above. Any other file (`## ` sections with no receipt above them, or no + * `## ` section) merges as described above, so the current intro is kept. */ export function mergeBodySections(current: string, incoming: string): { body: string; replaced: string[]; kept: string[]; added: string[]; dropped: string[] } { const now = splitSections(current); const next = splitSections(incoming).filter(n => !(n.key.startsWith("text:") && n.text.trim() === "")); const ambiguous = next.filter(n => now.filter(s => s.key === n.key).length > 1).map(n => `"${n.label}"`); if (ambiguous.length) throw new UsageError(`the PR's current body holds ${[...new Set(ambiguous)].join(", ")} more than once, so the file's copy matches no single section; nothing was posted. Rerun with --replace and the whole body`); - const managed = next.some(n => n.key.startsWith("h2:")); const preamble = (list: Section[]) => { const end = list.findIndex(s => s.key.startsWith("h2:")); return list.slice(0, end < 0 ? list.length : end).filter(s => s.key.startsWith("text:") && s.text.trim() !== ""); }; + const firstHeading = next.findIndex(n => n.key.startsWith("h2:")); + const managed = firstHeading > 0 && next.slice(0, firstHeading).some(n => n.key === "receipt"); const stale = managed ? preamble(now) : []; const ours = managed ? preamble(next) : []; const role = (s: Section) => (s.key === "text:lead" ? "lead" : "trailing"); diff --git a/lib/pregate/checks/lanes.ts b/lib/pregate/checks/lanes.ts index 759b3588a..f1206404c 100644 --- a/lib/pregate/checks/lanes.ts +++ b/lib/pregate/checks/lanes.ts @@ -406,7 +406,9 @@ export function remoteReportProblem(report: RemoteReport, tree: string): string * must print `PREGATE_REMOTE: tree= tests= failed= [spent=]`; * the obligation clears only when that tree is the tested tree, tests > 0, * failed = 0 and the gate exited 0. Anything else leaves it uncleared and - * the check incomplete. + * the check incomplete. The gate is bounded by that timeout only: a gate + * that waits on CI is quiet by design, so the stall timer does not apply, + * and its heartbeat names the gate's last output line. */ async function runRemote(ctx: PregateContext, sel: Selection, base: CheckResult): Promise { const lines = base.lines ?? []; @@ -418,7 +420,7 @@ async function runRemote(ctx: PregateContext, sel: Selection, base: CheckResult) const filesFile = path.join(dir, 'files.txt'); fs.writeFileSync(log, ''); fs.writeFileSync(filesFile, sel.localTests.join('\n') + '\n'); - const r = await supervise({ command: obligation.command!, cwd: ctx.repoRoot, env: { ...ctx.env, PREGATE_TREE: ctx.wtree, PREGATE_FILES_FILE: filesFile, PREGATE_SPEND_CAP_USD: String(cap), PREGATE_TIMEOUT_S: String(Math.floor(ctx.timeoutMs / 1000)) }, log, deadline: Date.now() + ctx.timeoutMs, stallMs: ctx.stallTimeoutMs, label: `remote gate ${obligation.lane}`, progress: ctx.progress }); + const r = await supervise({ command: obligation.command!, cwd: ctx.repoRoot, env: { ...ctx.env, PREGATE_TREE: ctx.wtree, PREGATE_FILES_FILE: filesFile, PREGATE_SPEND_CAP_USD: String(cap), PREGATE_TIMEOUT_S: String(Math.floor(ctx.timeoutMs / 1000)) }, log, deadline: Date.now() + ctx.timeoutMs, stallMs: 0, lastLine: true, label: `remote gate ${obligation.lane}`, progress: ctx.progress }); const output = readRange(log, r.start, r.end); const reported = parseRemoteReport(output); const spent = reported?.spent; @@ -426,16 +428,17 @@ async function runRemote(ctx: PregateContext, sel: Selection, base: CheckResult) ctx.progress(`PREGATE_REMOTE_SPEND: ${spend.slice('spend: '.length)}`); const runLines = [...lines, `remote gate: ${obligation.command}`, `log: ${log}`, spend, ...output.trim().split('\n').slice(-15).map(l => ` ${l}`)]; const inputs = { ...base.inputs, command: sha256(obligation.command!) }; - const why = r.outcome !== 'exited' ? `the remote gate did not finish (${r.outcome === 'stalled' ? `no output for ${ctx.stallTimeoutMs / 1000} s` : r.outcome})` + const why = r.outcome === 'timeout' ? `the remote gate did not finish (timed out after ${ctx.timeoutMs / 1000} s, ${ctx.timeoutSource})` + : r.outcome !== 'exited' ? `the remote gate did not finish (${r.outcome})` : !reported ? `the remote gate exited ${r.exit} without a PREGATE_REMOTE: line` : remoteReportProblem(reported, ctx.wtree) ?? (r.exit !== 0 ? `the remote gate exited ${r.exit}` : null); // A gate that printed no report and names a quota, network or missing-tool error in its preamble never ran the tests: the environment, not the diff. // Only the gate's first lines are read (a VM refusal or a network error comes before any test streams); exit 127 counts only when the gate's own binary is off PATH. - const environment = why && !reported && r.outcome !== 'stalled' && r.outcome !== 'timeout' + const environment = why && !reported && r.outcome !== 'timeout' ? r.outcome === 'spawn-failed' ? { ...(classifyEnvironment(r.error ?? '', null) ?? launchCause('missing-binary', r.error ?? 'the remote gate could not be started')), proof: 'launch' as const } : classifyCommandEnvironment(obligation.command!, output.split('\n').slice(0, REMOTE_PREAMBLE_LINES).join('\n'), r.exit, ctx.env) : null; if (environment) return environmentResult({ ...base, inputs, lines: runLines, remote: [...sel.remote, obligation] }, environment, `${why}; requires-remote ${obligation.lane} stays uncleared`); - if (why) return { ...base, inputs, lines: runLines, status: 'incomplete', remote: [...sel.remote, obligation], code: r.outcome === 'stalled' ? 'PREGATE_STALLED' : 'PREGATE_INCOMPLETE', detail: `${why}; requires-remote ${obligation.lane} stays uncleared`, fix: `read ${log}; the obligation clears only on PREGATE_REMOTE: tree=${ctx.wtree.slice(0, 12)}… tests>0 failed=0 with exit 0` }; + if (why) return { ...base, inputs, lines: runLines, status: 'incomplete', remote: [...sel.remote, obligation], code: 'PREGATE_INCOMPLETE', detail: `${why}; requires-remote ${obligation.lane} stays uncleared`, fix: `read ${log}; the obligation clears only on PREGATE_REMOTE: tree=${ctx.wtree.slice(0, 12)}… tests>0 failed=0 with exit 0` }; const cleared = { ...obligation, receipt: `remote-gate tree=${ctx.wtree.slice(0, 12)} tests=${reported!.tests}`, cleared_by: `gstack-pregate --remote ${new Date().toISOString()}` }; return { ...base, inputs, lines: runLines, status: 'requires-remote', remote: [...sel.remote, cleared], detail: `remote gate ${obligation.lane}: ${reported!.tests} tests, 0 failed on tree ${ctx.wtree.slice(0, 12)}; ${spend}` }; } diff --git a/lib/pregate/supervise.ts b/lib/pregate/supervise.ts index 17639f97f..a7ff3ed88 100644 --- a/lib/pregate/supervise.ts +++ b/lib/pregate/supervise.ts @@ -11,7 +11,7 @@ * headers) reports `PREGATE_TEST: / start|pass|fail ` * per file; a run of one file reports it from the launch itself for any * runner. Without either, only heartbeats: a line after each `heartbeatMs` - * with no progress line. + * with no progress line; with `lastLine` it names the last line the command printed. * * Platforms: POSIX signals the negative pid (the group `detached` creates * with setsid). Windows has no process groups here: the kill is @@ -43,6 +43,8 @@ export interface SuperviseOptions { files?: readonly string[]; adapter?: ProgressAdapter | null; counter?: Counter; + /** Heartbeats name the command's last output line (the remote gate, which has no per-file progress). */ + lastLine?: boolean; progress: (line: string) => void; } @@ -183,7 +185,13 @@ export function supervise(o: SuperviseOptions): Promise { if (outcome) return; if (now >= o.deadline) { stop('timeout'); return; } if (o.stallMs > 0 && now - lastGrowth >= o.stallMs) { stop('stalled'); return; } - if (now - lastProgress >= heartbeatMs) emit(`PREGATE_HEARTBEAT: ${o.label} running ${Math.round((now - started) / 1000)}s; last file ${lastFile ?? 'none reported'}; log ${size - start} bytes, last growth ${Math.round((now - lastGrowth) / 1000)}s ago (${o.log})`); + if (now - lastProgress < heartbeatMs) return; + let last: string | undefined; + if (o.lastLine) { + try { last = readRange(o.log, start, size, 4096).replace(/\x1b\[[0-9;?]*[A-Za-z]/g, '').split(/\r\n?|\n/).map(l => l.replace(/[\x00-\x1f\x7f]/g, '').trim()).filter(Boolean).at(-1); } + catch { /* log removed */ } + } + emit(`PREGATE_HEARTBEAT: ${o.label} running ${Math.round((now - started) / 1000)}s; last file ${lastFile ?? 'none reported'}; log ${size - start} bytes, last growth ${Math.round((now - lastGrowth) / 1000)}s ago (${o.log})${last ? `; last output: ${last.slice(0, 160)}` : ''}`); }, pollMs); if (child.pid) ACTIVE.add(child.pid); child.on('error', e => finish(null, null, e.message)); diff --git a/lib/review-ledger.ts b/lib/review-ledger.ts index 0c9a96e9f..0d945d0c4 100644 --- a/lib/review-ledger.ts +++ b/lib/review-ledger.ts @@ -4,7 +4,7 @@ * * rendered a brief gstack-review-dispatch render wrote (role, name, file, sha256) * sent the prompt a dispatch program actually passed, by hash; a mismatch fails the pass - * output a reader's raw output (sha256) and the merged.jsonl ids collect wrote from it + * output a reader's raw output (sha256), the merged.jsonl ids collect wrote from it, and whether it was a review at all * judgment each lead-judgment change: an added or promoted finding, with its reason * judge the verdict on the parent's judged set (ok, or the dropped and downgraded ids) * @@ -72,11 +72,12 @@ export interface MergedLine { id: string; reader: string; label: string; raw: st * of the two labels). A `Severity` or `Blocks` field line continues the open finding, once per field: `Severity` raises its label, * `Blocks: yes` makes it CRITICAL, `Blocks: no` never lowers it; with no finding open it is one, carrying the paragraph's subject. * A list item, a heading, a bold or post-blank paragraph at the margin, or a line naming a file:line after the open finding's fields - * starts the next finding. An inline `Blocks: yes` is CRITICAL too. The native voice's FIXABLE collects as CRITICAL; other labels - * stay as written for lead judgment. When in doubt the parser merges rather than drops: a lost blocker costs the gate. + * starts the next finding, except an unlabeled, unnumbered bullet with no file:line under a paragraph finding whose fields have not + * come yet, which is its detail and joins its text (GSTA-196: 4 findings with detail bullets collected as 8). An inline + * `Blocks: yes` is CRITICAL too. The native voice's FIXABLE collects as CRITICAL; other labels stay as written for lead judgment. When in doubt the parser merges rather than drops: a lost blocker costs the gate. */ export function parseFindings(reader: string, output: string): Array<{ label: string; raw: string }> { - type Finding = { label: string; raw: string; fields: Set }; + type Finding = { label: string; raw: string; fields: Set; list: boolean }; const out: Finding[] = []; let section: string | null = null; let current: Finding | null = null; @@ -85,7 +86,7 @@ export function parseFindings(reader: string, output: string): Array<{ label: st const role = reader.split('/')[0]; const named = (l: string) => (role === 'adversarial-native' && l.toUpperCase() === 'FIXABLE' ? 'CRITICAL' : l); const strongest = (t: string) => [...t.matchAll(ALL_LABELS)].map(m => named(m[1]!)).reduce((a, b) => (a === undefined ? b : stronger(a, b)), undefined); - const push = (label: string, raw: string, fields: string[] = []): Finding => { const f = { label, raw, fields: new Set(fields) }; out.push(f); return f; }; + const push = (label: string, raw: string, fields: string[] = []): Finding => { const f = { label, raw, fields: new Set(fields), list: LIST_ITEM.test(raw) }; out.push(f); return f; }; for (const line of output.replace(/\r\n/g, '\n').split('\n')) { const t = line.trim(); if (!t) { gap = true; continue; } @@ -118,7 +119,9 @@ export function parseFindings(reader: string, output: string): Array<{ label: st if (typeof o.severity === 'string') { current = push(o.severity, t); continue; } } catch { /* prose that starts with a brace */ } } - const fresh = listItem || (!indented && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus); + const detail = listItem && /^[-*+]\s/.test(t) && !locus && !label && !section && !!current && !current.list && !current.fields.size; + if (detail && !blocks) { current!.raw += ` | ${t}`; continue; } + const fresh = (listItem && !detail) || (!indented && !detail && (t.startsWith('**') || afterGap)) || (!!current?.fields.size && locus); if (fresh) { current = null; head = t; } const inherited = section && listItem ? section : undefined; const found = label && inherited ? stronger(inherited, label) : label ?? inherited; @@ -133,15 +136,39 @@ export function parseFindings(reader: string, output: string): Array<{ label: st return out.map(({ label, raw }) => ({ label, raw })); } -/** collect: hash the raw output, append its findings to merged.jsonl and record both. */ -export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string): { sha: string; lines: MergedLine[] } { +/** + * The clean marker each reader's own brief prescribes for a review that found nothing: `NO FINDINGS` alone on a line for the + * specialists, the batched specialist and the targeted reader (ship/sections/review-army.md, the specialist brief; review/specialists/*.md), + * the Red Team (its brief in ship/sections/review-army.md) and fix-check (scripts/resolvers/fix-check.ts); the `Recommendation:` line + * for the outside voice (ship/sections/adversarial.md, the outside prompt); either for the native adversarial voice, whose brief asks + * for both. A reply with no finding and no marker (a refusal, an empty reply, a reader that never saw the diff) is not a review. + */ +const NO_FINDINGS: [string, RegExp] = ['NO FINDINGS', /^[`*]*NO FINDINGS\.?[`*]*$/]; +const RECOMMENDATION: [string, RegExp] = ['Recommendation:', /^[`*]*Recommendation:/i]; +const CLEAN_MARKERS: Record> = { + specialist: [NO_FINDINGS], 'specialist-batched': [NO_FINDINGS], targeted: [NO_FINDINGS], 'red-team': [NO_FINDINGS], 'fix-check': [NO_FINDINGS], + 'adversarial-native': [NO_FINDINGS, RECOMMENDATION], 'outside-voice': [RECOMMENDATION], +}; +/** The markers a reader's zero-finding reply may carry; a role outside the brief roles takes either. */ +export const cleanMarkers = (reader: string) => CLEAN_MARKERS[reader.split('/')[0]!] ?? [NO_FINDINGS, RECOMMENDATION]; +const hasCleanMarker = (reader: string, output: string) => output.replace(/\r\n/g, '\n').split('\n').some(l => cleanMarkers(reader).some(([, re]) => re.test(l.trim()))); + +/** + * collect: hash the raw output, append its findings to merged.jsonl and record both; a reply with no finding and no clean marker records + * review: false. With unavailable (the outside voice only) the output is its failure text, recorded as missing coverage. + */ +export function collect(ledger: string, reader: string, output: string, mergedFile: string, outputFile?: string, unavailable?: string): { sha: string; lines: MergedLine[]; review: boolean } { const sha = promptHash(output); const prior = readLedger(ledger).filter(e => e.event === 'output' && e.reader === reader).length; - const lines = parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha })); + const lines = unavailable ? [] : parseFindings(reader, output).map((f, i) => ({ id: `${reader}#${prior ? `${prior + 1}.` : ''}${i + 1}`, reader, label: f.label, raw: f.raw, output_sha256: sha })); + const review = !unavailable && (lines.length > 0 || hasCleanMarker(reader, output)); fs.appendFileSync(mergedFile, lines.map(l => JSON.stringify(l) + '\n').join(''), { mode: 0o600 }); - appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id) }); - return { sha, lines }; + appendEvent(ledger, { event: 'output', reader, sha256: sha, file: outputFile ? path.resolve(outputFile) : null, merged: path.resolve(mergedFile), ids: lines.map(l => l.id), review, ...(unavailable ? { unavailable } : {}) }); + return { sha, lines, review }; } +/** The outside voice may fail without blocking (ship/sections/adversarial.md): its failure is recorded as unavailable coverage, never as a review. */ +export const mayBeUnavailable = (reader: string) => reader.split('/')[0] === 'outside-voice'; +const unavailableOutput = (o: LedgerEvent) => typeof o.unavailable === 'string' && mayBeUnavailable(String(o.reader)); export function readJsonl(file: string): T[] { return fs.readFileSync(file, 'utf8').split('\n').filter(l => l.trim()).map(l => JSON.parse(l) as T); @@ -190,13 +217,24 @@ export function recordJudgment(ledger: string, merged: MergedLine[], judged: Jud return res; } +/** + * Whether a recorded output was a review: it has findings, or its file (unchanged since collect) carries the reader's clean marker, so + * a ledger written before collect recorded the field is judged by the same rule; with no readable file, collect's own review field. + */ +function reviewed(o: LedgerEvent): boolean { + if (Array.isArray(o.ids) && o.ids.length) return true; + let text: string | undefined; + try { text = typeof o.file === 'string' ? fs.readFileSync(o.file, 'utf8') : undefined; } catch { /* unreadable */ } + return text !== undefined && promptHash(text) === o.sha256 ? hasCleanMarker(String(o.reader), text) : o.review === true; +} + /** Every id collect recorded, in order. */ const collectedIds = (events: LedgerEvent[]) => events.filter(e => e.event === 'output').flatMap(e => (Array.isArray(e.ids) ? (e.ids as string[]) : [])); export interface VerifyRow { kind: 'BRIEF' | 'OUTPUT' | 'JUDGE'; reader: string; status: string; detail: string } /** Replay every check from a finished run's ledger: briefs dispatched as rendered, outputs merged as returned, judgment without loss. */ -export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; briefs: number; voided: number } { +export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; briefs: number; voided: number; unavailable: number } { const events = readLedger(ledger); const rows: VerifyRow[] = []; const short = (s: unknown) => (typeof s === 'string' ? s.slice(0, 12) : '-'); @@ -225,10 +263,12 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri } voided += epVoided; const lastMatch = [...sent].reverse().find(({ e }) => e.sha256 === r.sha256); - const collected = !!lastMatch && inside.some(({ e, i }) => e.event === 'output' && i > lastMatch.i); - const status = !sent.length ? 'NOT_SENT' : mismatched ? 'MISMATCH' : collected ? 'MATCH' : 'NOT_COLLECTED'; + const outputs = lastMatch ? inside.filter(({ e, i }) => e.event === 'output' && i > lastMatch.i) : []; + const collected = outputs.some(({ e }) => reviewed(e)); + const unavailable = !collected && outputs.some(({ e }) => unavailableOutput(e)); + const status = !sent.length ? 'NOT_SENT' : mismatched ? 'MISMATCH' : collected ? 'MATCH' : unavailable ? 'UNAVAILABLE' : 'NOT_COLLECTED'; const tampered = fileNow && fileNow !== r.sha256 ? ' file_changed_since_render' : ''; - rows.push({ kind: 'BRIEF', reader: p.label, status: tampered ? 'MISMATCH' : status, detail: `rendered=${short(r.sha256)} sent=${sent.map(({ e }) => short(e.sha256)).join(',') || '-'}${epVoided ? ` voided=${epVoided}` : ''}${tampered}` }); + rows.push({ kind: 'BRIEF', reader: p.label, status: tampered ? 'MISMATCH' : status, detail: `rendered=${short(r.sha256)} sent=${sent.map(({ e }) => short(e.sha256)).join(',') || '-'}${epVoided ? ` voided=${epVoided}` : ''}${outputs.length && !collected && !unavailable ? ' not_reviewed' : ''}${tampered}` }); } for (const [i, o] of events.entries()) { if (o.event !== 'output') continue; @@ -245,6 +285,15 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri try { lines = typeof o.merged === 'string' ? readJsonl(o.merged).filter(l => ids.includes(l.id)) : []; } catch { notes.push('merged_unreadable'); status = 'MISMATCH'; } if (lines.length !== ids.length) { status = 'MISMATCH'; notes.push(`merged_lines=${lines.length}/${ids.length}`); } if (lines.some(l => l.output_sha256 !== o.sha256)) { status = 'MISMATCH'; notes.push('merged_line_from_other_output'); } + if (status === 'MATCH' && !reviewed(o)) { + // A non-review the same dispatch later replaced with a review (the brief re-sent verbatim, then answered) is void, counted like a voided send. + const later = p ? events.slice(i + 1, p.next) : []; + const review = later.findIndex(e => e.event === 'output' && e.reader === reader && reviewed(e)); + const replaced = review > 0 && later.slice(0, review).some(e => e.event === 'sent' && e.reader === reader && e.sha256 === events[p!.at]!.sha256); + if (unavailableOutput(o)) { status = 'UNAVAILABLE'; notes.push(`reason=${JSON.stringify(o.unavailable)}`); } + else if (replaced) { status = 'VOIDED'; voided++; notes.push('replaced_by_a_review'); } + else { status = 'NOT_REVIEWED'; notes.push('no_finding_no_clean_marker'); } + } rows.push({ kind: 'OUTPUT', reader: p?.label ?? reader, status, detail: `sha=${short(o.sha256)} findings=${ids.length}${notes.length ? ' ' + notes.join(' ') : ''}` }); } const lastJudge = events.map(e => e.event).lastIndexOf('judge'); @@ -266,6 +315,6 @@ export function verify(ledger: string): { rows: VerifyRow[]; clean: boolean; bri const status = !v.ok ? 'REJECTED' : notes.length ? 'MISMATCH' : 'OK'; rows.push({ kind: 'JUDGE', reader: '-', status, detail: `dropped=${JSON.stringify(v.dropped)} downgraded=${JSON.stringify(v.downgraded)} added=${v.added} promoted=${v.promoted} folded=${v.folded}${notes.length ? ' ' + notes.join(' ') : ''}` }); } else if (events.some(e => e.event === 'output')) rows.push({ kind: 'JUDGE', reader: '-', status: 'MISSING', detail: 'outputs were collected but no judged set was checked' }); - const clean = episodes.length > 0 && rows.every(r => r.status === 'MATCH' || r.status === 'OK'); - return { rows, clean, briefs: episodes.length, voided }; + const clean = episodes.length > 0 && rows.every(r => ['MATCH', 'OK', 'VOIDED', 'UNAVAILABLE'].includes(r.status)); + return { rows, clean, briefs: episodes.length, voided, unavailable: rows.filter(r => r.kind === 'BRIEF' && r.status === 'UNAVAILABLE').length }; } diff --git a/package.json b/package.json index 521d596fe..3884072da 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "gstack", - "version": "1.91.365", + "version": "1.91.372", "description": "Garry's Stack — Claude Code skills + fast headless browser. One repo, one install, entire AI engineering workflow.", "license": "MIT", "type": "module", diff --git a/scripts/test-health-report.ts b/scripts/test-health-report.ts index ae09e9614..8d9e55168 100644 --- a/scripts/test-health-report.ts +++ b/scripts/test-health-report.ts @@ -277,7 +277,7 @@ export function collectHealth(inputs: HealthInputs): HealthReport { return { value: `${(total / 60_000 * 7 / sinceDays).toFixed(0)} min/week (${cancelled.length} cancelled run(s))` }; })); - metrics.push(metric('push-to-verdict', 'PR lane push-to-verdict', 'created_at to updated_at of evals.yml pull_request runs that reached success or failure (includes queue and the readiness wait)', () => { + metrics.push(metric('push-to-verdict', 'PR lane push-to-verdict', 'created_at to updated_at of evals.yml pull_request runs that reached success or failure (includes queue and the readiness wait, which pr-summary runs skip since GSTA-149, so their verdicts land about 10 min sooner)', () => { const walls = need(prRuns).filter(run => run.conclusion === 'success' || run.conclusion === 'failure').map(run => span(run.createdAt, run.updatedAt)!); if (walls.length === 0) throw new Error('no PR run reached a verdict in the window'); return { value: `p50 ${minutes(percentile(walls, 0.5))}, p95 ${minutes(percentile(walls, 0.95))} (${walls.length} runs)` }; diff --git a/scripts/ubicloud/pregate-remote.sh b/scripts/ubicloud/pregate-remote.sh index 0759ee9b1..68e5b5692 100755 --- a/scripts/ubicloud/pregate-remote.sh +++ b/scripts/ubicloud/pregate-remote.sh @@ -185,7 +185,7 @@ args=(run -s "$size" --src "$root" --setup "$prelude" [ -z "${GSTACK_FREE_JOBS:-}" ] || args+=(--env "GSTACK_FREE_JOBS=$GSTACK_FREE_JOBS") # The VM run gets its own session (python3, which ubi-runner already needs, for -# setsid on Linux and macOS): pregate's --timeout and stall kill signal this +# setsid on Linux and macOS): pregate's --timeout kill signals this # script's process group and then SIGKILL it, which must never cut ubi-runner's # teardown short. Output goes to files (a dead reader would SIGPIPE ubi-runner), # stamped for the phase timings; this script streams it. A watchdog sends diff --git a/test/evals-pr-lane-workflow.test.ts b/test/evals-pr-lane-workflow.test.ts index 439357bf5..385b40667 100644 --- a/test/evals-pr-lane-workflow.test.ts +++ b/test/evals-pr-lane-workflow.test.ts @@ -83,7 +83,8 @@ describe('evals.yml readiness-gated dispatch (R1, GSTA-48)', () => { test('a tiny PR-only job waits for a quiet PR and green Free Tests before recovery and planning', () => { expect(readiness['runs-on']).toBe('ubuntu-24.04'); - expect(readiness.if).toBe("github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository"); + expect(readiness.if).toBe("github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository && needs.route.outputs.mode != 'pr-summary'"); + expect(readiness.needs).toEqual(['route']); expect(readiness.permissions).toEqual({ contents: 'read', actions: 'read', 'pull-requests': 'read' }); expect(Number(readiness['timeout-minutes'])).toBeGreaterThan(80); const wait = step('readiness', 'Wait for a quiet PR and green Free Tests on this head')!; diff --git a/test/evals-train-workflow.test.ts b/test/evals-train-workflow.test.ts index 891063a13..cf3c8b71e 100644 --- a/test/evals-train-workflow.test.ts +++ b/test/evals-train-workflow.test.ts @@ -193,9 +193,10 @@ describe('without a train block nothing changes (the inert proof)', () => { }); describe('under a train block (the gate edits, each only on its mode)', () => { - test('a PR push starts no paid slice: route, readiness and the free screening summary only', () => { + test('a PR push starts no paid slice and waits for nothing: route, the image and the free screening summary only', () => { const sim = simulate(evals, { github: pr(), inputs: {}, outputs: { route: { ...ROUTE, ...train, mode: 'pr-summary' } } }); - expect(sim.jobs).toEqual(['build-image', 'readiness', 'route', 'screen-summary']); + expect(sim.jobs).toEqual(['build-image', 'route', 'screen-summary']); + expect(simulate(evals, { github: pr(), inputs: {}, outputs: { route: { ...ROUTE, mode: 'pr' } } }).jobs).toContain('readiness'); const summary = evals.jobs['screen-summary']!; expect(summary.permissions).toEqual({ contents: 'read' }); expect(JSON.stringify(summary)).not.toMatch(/secrets\.|restore-deps|bun install/); diff --git a/test/evidence.test.ts b/test/evidence.test.ts index 479095f6a..e40d6769c 100644 --- a/test/evidence.test.ts +++ b/test/evidence.test.ts @@ -304,6 +304,71 @@ describe('gstack-evidence check', () => { expect(chk.status).toBe(0); }); + // Value: protects=a repository that gitignores .bun/ still gets a fingerprint (GST-449): an exclude pathspec naming an ignored path made + // `git add -A` exit 1, so gstack-wtree failed and every review-log start and evidence check lost its fingerprint; fails_when=gstack-wtree + // exits non-zero or the lane records no wtree with .bun/ (or /.bun) in .gitignore; why_new=the GST-421 test covered only an unignored + // .bun/ (harness-evals#148); seam=none; no_claim=Windows git + test('GST-449: with .bun/ gitignored, gstack-wtree still fingerprints and a lane that writes .bun/ grades FRESH', () => { + for (const pattern of ['.bun/', '/.bun']) { + fs.writeFileSync(path.join(repoDir, '.gitignore'), `${pattern}\n`); + git('add .gitignore'); + git(`commit -q -m "ignore ${pattern}"`); + fs.mkdirSync(path.join(repoDir, '.bun'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, '.bun', 'x'), 'cache\n'); + const wt = runBin(path.join(ROOT, 'bin', 'gstack-wtree'), [], { cwd: repoDir }); + expect(wt.stderr).toBe(''); + expect(wt.status).toBe(0); + expect(wt.stdout.trim()).toMatch(/^[0-9a-f]{40}$/); + const r = run(['run', '--label', 'tests', '--', 'echo cache2 > .bun/y && echo green']); + expect(r.status).toBe(0); + expect(records().pop().wtree).toMatch(/^[0-9a-f]{40}$/); + expect(run(['check', '--label', 'tests']).stdout).toContain('EVIDENCE: FRESH label=tests'); + fs.rmSync(path.join(repoDir, '.bun'), { recursive: true, force: true }); + } + }); + + // Value: protects=a repository that ignores .bun/ while still tracking a file under it gets a fingerprint (GST-449 review): the + // index-aware check-ignore called .bun not ignored, so the exclude pathspec came back and `git add -A` exited 1; fails_when=gstack-wtree + // exits non-zero, or its tree differs from a plain `git add -A`, or an edit or delete of the tracked file leaves it unchanged; + // why_new=the pattern loop above starts from an untracked .bun/ only; seam=none; no_claim=Windows git + test('GST-449: an ignored .bun/ that still tracks a file fingerprints like a plain git add -A, and edits to the tracked file count', () => { + fs.mkdirSync(path.join(repoDir, '.bun'), { recursive: true }); + fs.writeFileSync(path.join(repoDir, '.bun', 'tracked'), 'v1\n'); + git('add -f .bun/tracked'); + git('commit -q -m "track .bun/tracked"'); + fs.writeFileSync(path.join(repoDir, '.gitignore'), '.bun/\n'); + git('add .gitignore'); + git('commit -q -m "ignore .bun/"'); + fs.writeFileSync(path.join(repoDir, '.bun', 'cache'), 'cache\n'); + const wtree = () => { + const wt = runBin(path.join(ROOT, 'bin', 'gstack-wtree'), [], { cwd: repoDir }); + expect(wt.stderr).toBe(''); + expect(wt.status).toBe(0); + return wt.stdout.trim(); + }; + const plain = () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'gstack-wtree-idx-')); + try { + const env = { ...process.env, GIT_INDEX_FILE: path.join(dir, 'index') }; + spawnSync('git', ['read-tree', 'HEAD'], { cwd: repoDir, env, timeout: 10_000 }); + spawnSync('git', ['add', '-A'], { cwd: repoDir, env, timeout: 10_000 }); + return spawnSync('git', ['write-tree'], { cwd: repoDir, env, encoding: 'utf-8', timeout: 10_000 }).stdout.trim(); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + }; + const clean = wtree(); + expect(clean).toBe(plain()); + fs.writeFileSync(path.join(repoDir, '.bun', 'tracked'), 'v2\n'); + const edited = wtree(); + expect(edited).not.toBe(clean); + expect(edited).toBe(plain()); + fs.rmSync(path.join(repoDir, '.bun', 'tracked')); + const deleted = wtree(); + expect(deleted).not.toBe(clean); + expect(deleted).toBe(plain()); + }); + test('GST-421: a lane that writes untracked source grades STALE and the reason names the path', () => { const r = run(['run', '--label', 'tests', '--', 'mkdir -p src && echo "export {}" > src/new.ts && echo green']); expect(r.status).toBe(0); diff --git a/test/fixtures/review-ledger/gsta-196-native.txt b/test/fixtures/review-ledger/gsta-196-native.txt new file mode 100644 index 000000000..8c273413b --- /dev/null +++ b/test/fixtures/review-ledger/gsta-196-native.txt @@ -0,0 +1,43 @@ +Adversarial review (native) of `1b68e637c07c..0a8efed`. HEAD is still `0a8efeda8f79`, the working tree is clean and the same 10 files changed, so the findings below are the ones I reported earlier, rechecked against this range. + +**Coverage:** I read the full diffs of the non-fixture source: `.github/workflows/{ci,sealed,train-gate}.yml`, `scripts/gate.ts`, `gate/manifest.json`, `package.json` and `README.md`. **I reviewed the test files in summary mode only**, so their coverage of the review is reduced: +- `tests/ci/gate.test.ts` is new (+299). It covers the train chain walk, `routeDispatch` refusals, `publishable` binding and the gate-surface rule, and `runCases`/junit judging. +- `tests/ci/sealed-workflow.test.ts` (+19/−6) adds the push trigger. +- `tests/ci/workflow-hygiene.test.ts` is new (+85). It covers SHA pins, read-only tokens, no secrets, the checksummed Bun install, the gstack pin and parity between the manifest and ci.yml. + +**1. A PR in the train controls the workflow that judges the train (INVESTIGATE).** `workflow_dispatch` runs the copy of `train-gate.yml` at the dispatched ref, `train/`, and that copy contains every train PR's edits. So a PR that edits `train-gate.yml` changes the `route`, `test` and `publish` jobs themselves. +- It can drop the call to the base's `gate.ts publish`. It can also post the `full_gate_check` check run unconditionally with the publish job's `checks: write`, or give itself more `permissions:` and use repository secrets. +- The header says "a pull request in the train cannot change how the train is judged". In practice only the case list and `gate.ts` come from the base. +- The "a gate surface PR rides alone" rule (`publishable`, `slots > 1`) is enforced by the publish job, which is code the PR can rewrite. So the rule holds only against PRs that don't break it. +- `ci.yml`'s new `workflow_dispatch` has the same property. +- Real mitigations would run the judging workflow from the default branch, for example `repository_dispatch` or a dispatch on `main` that takes the train head as an input, or have gstack verify which workflow file produced the check. Either one changes the protocol with gstack. +- Today an attacker needs a malicious workflow edit to pass review and READY, and no workflow references a secret. + +Severity: High. Blocks: no. + +**2. A case timeout leaks processes and can lose the record (FIXABLE).** `runCases` calls `spawnSync(..., { timeout })` (`scripts/gate.ts:252`), which kills only the `bash` wrapper. Grandchildren such as `bun test` survive with inherited stdio and keep running into later cases. +- A 20-minute case timeout (`CASE_TIMEOUT_MS`) plus the other cases can go past the job's 30-minute `timeout-minutes`. +- GitHub then kills `gate.ts` before it writes `train-record.json`. That run has no record and fails as infrastructure, not as a red verdict. +- Fix: spawn each case detached and kill its whole process group on timeout, and keep the per-case cap well under the job limit (or size the job to cases × cap). + +Severity: Low. Blocks: no. + +**3. Local `run` hides a broken train chain (FIXABLE).** In `gate.ts run` (`:386`), when `trainBase` returns `{ error }`, the code falls back to `merge-base HEAD origin/` without saying so. A malformed train run locally is judged against the wrong base. +- CI always passes `--base`, so this affects only local `bun run gate`. +- Fix: fail on an error result, the way `route` and `publish` already do. + +Severity: Low. Blocks: no. + +**4. No train policy is on the base, so trains cannot run yet (INVESTIGATE).** `.gstack/ship-policy.json` does not exist at HEAD. Until a commit with a `train` block lands on main, `route` refuses every train and bisect dispatch with "needs a train block". The workflow cannot gate a train at merge, and this PR does not add the file. +- Related: `bun.lock` is not in `gate_surface`. A multi-PR train can change resolved dependency versions within `package.json` ranges, for example `@types/bun ^1.4.4`, which changes what `typecheck` and `unit` run against. + +Severity: Low. Blocks: no. + +**Ruled out:** +- `grep -q` after `tee` in the sealed case is not a SIGPIPE risk. `SEALED:` is the last and only line on gstack-pregate's stdout. +- The empty wiki `--base` on dispatch is safe: `resolveBase` falls back to `origin/HEAD`. +- `Bun.Glob("**/tsconfig*.json")` matches `tsconfig.json` at the repository root. +- The bisect artifact name matches gstack's `bisectArtifact`. +- `route` refuses when the bash T1 lookup and `trainBase` disagree. + +Recommendation: Ship as-is and send finding 1 to the train-protocol owner because the strongest issue, a train PR rewriting `train-gate.yml` to post `full_gate_check` itself, needs a malicious workflow edit to pass review, and fixing it means changing the dispatch protocol with gstack, not this diff. \ No newline at end of file diff --git a/test/fixtures/review-ledger/gsta-196-refusal.txt b/test/fixtures/review-ledger/gsta-196-refusal.txt new file mode 100644 index 000000000..c1eb77c28 --- /dev/null +++ b/test/fixtures/review-ledger/gsta-196-refusal.txt @@ -0,0 +1,5 @@ +I couldn't run the review because the diff base doesn't exist. `git diff 1b68e637c07cea38f01c01120e02d184575da82c` fails with "bad object", and fetching that sha from origin is refused with "not our ref", so the commit is neither in this checkout nor on the remote. + +This checkout is on `main` at 047dad7 (v1.91.357.0), tracking origin/main with no uncommitted changes, so there's nothing local to review against either. I'm not returning NO FINDINGS, because that would claim a clean diff I never saw. + +To continue I need one of these: a diff base that exists, or the branch or sha holding the c3-36-p1 change. It may only exist on another machine and was never pushed. I've read all three checklists (testing, security, state-machines) and will apply them as soon as I have a diff. \ No newline at end of file diff --git a/test/free-durations-record-workflow.test.ts b/test/free-durations-record-workflow.test.ts index a86d0fcbc..9bd99d135 100644 --- a/test/free-durations-record-workflow.test.ts +++ b/test/free-durations-record-workflow.test.ts @@ -29,9 +29,10 @@ test('the recorder runs weekly before test-health and on dispatch, read-only exc expect(recorder.jobs.propose.permissions).toEqual({ contents: 'write', 'pull-requests': 'write', actions: 'write' }); }); -test('Linux records on the free-suite runner, turns red on any red file, and uploads only a complete recording', () => { +test('Linux records on the free-suite runner image, turns red on any red file, and uploads only a complete recording', () => { const linux = recorder.jobs.linux; - expect(linux['runs-on']).toBe(freeTests.jobs['free-suite']['runs-on']); + expect(linux['runs-on']).toMatch(/^ubicloud-standard-\d+$/); + expect(freeTests.env.FREE_SUITE_RUNNER).toMatch(/^ubicloud-standard-\d+$/); const runs = linux.steps.map((step: any) => step.run).filter(Boolean).join('\n'); expect(runs).toContain('bash scripts/ubicloud/setup-free-suite.sh'); const record = linux.steps.find((step: any) => step.name === 'Time every free file alone'); diff --git a/test/free-tests-workflow-wiring.test.ts b/test/free-tests-workflow-wiring.test.ts index cd8322971..613936f0d 100644 --- a/test/free-tests-workflow-wiring.test.ts +++ b/test/free-tests-workflow-wiring.test.ts @@ -166,6 +166,66 @@ describe('free-tests workflow wiring', () => { } }); + test.skipIf(!Bun.which('bash'))('free-plan resolves the one runner and shard constant for every event, and validates dispatch overrides', () => { + const workflow = Bun.YAML.parse(source) as any; + const planner = workflow.jobs['free-plan']; + const plan = planner.steps.find((step: any) => step.id === 'plan'); + expect(workflow.env.FREE_SUITE_RUNNER).toMatch(/^ubicloud-standard-(2|4|8)$/); + expect(workflow.env.FREE_SUITE_SHARDS).toMatch(/^[1-9][0-9]?$/); + expect(planner.outputs.runner).toBe('${{ steps.plan.outputs.runner }}'); + expect(workflow.jobs['free-suite']['runs-on']).toBe('${{ needs.free-plan.outputs.runner }}'); + expect(plan.env).toEqual({ RUNNER_OVERRIDE: '${{ inputs.free_runner }}', SHARDS_OVERRIDE: '${{ inputs.free_shards }}' }); + expect(plan.run).not.toContain('${{'); + const directory = fs.mkdtempSync(path.join(os.tmpdir(), 'free plan ')); + const bin = path.join(directory, 'bin'); + fs.mkdirSync(bin); + fs.writeFileSync(path.join(bin, 'bun'), '#!/bin/sh\nprintf "%s\\n" "$*" > "$RUNNER_TEMP/bun-args"\necho \'{"shard":[1]}\'\n', { mode: 0o755 }); + const toShell = (p: string) => p.split(path.sep).join('/'); + const resolve = (overrides: Record) => { + const output = path.join(directory, 'output'); + fs.writeFileSync(output, ''); + const result = spawnSync('bash', ['-c', plan.run], { + env: { ...process.env, ...workflow.env, RUNNER_OVERRIDE: '', SHARDS_OVERRIDE: '', ...overrides, + PATH: `${toShell(bin)}:${process.env.PATH}`, RUNNER_TEMP: toShell(directory), + GITHUB_OUTPUT: toShell(output), GITHUB_STEP_SUMMARY: toShell(path.join(directory, 'summary')) }, + encoding: 'utf8', timeout: 5000, + }); + const outputs = Object.fromEntries(fs.readFileSync(output, 'utf8').trim().split('\n').filter(Boolean).map(line => line.split(/=(.*)/s).slice(0, 2))); + const args = fs.existsSync(path.join(directory, 'bun-args')) ? fs.readFileSync(path.join(directory, 'bun-args'), 'utf8') : ''; + fs.rmSync(path.join(directory, 'bun-args'), { force: true }); + return { status: result.status, outputs, args }; + }; + try { + const unset = resolve({}); + expect(unset.status).toBe(0); + expect(unset.outputs.runner).toBe(workflow.env.FREE_SUITE_RUNNER); + expect(unset.args).toContain(`--shards ${workflow.env.FREE_SUITE_SHARDS}`); + const override = resolve({ RUNNER_OVERRIDE: 'ubicloud-standard-4', SHARDS_OVERRIDE: '16' }); + expect(override.status).toBe(0); + expect(override.outputs.runner).toBe('ubicloud-standard-4'); + expect(override.args).toContain('--shards 16'); + const refusals: Record[] = [{ RUNNER_OVERRIDE: 'ubuntu-24.04' }, { RUNNER_OVERRIDE: 'ubicloud-standard-8 x' }, { SHARDS_OVERRIDE: '0' }, { SHARDS_OVERRIDE: '41' }, { SHARDS_OVERRIDE: '16; true' }]; + for (const bad of refusals) { + const refused = resolve(bad); + expect(refused.status, JSON.stringify(bad)).not.toBe(0); + expect(refused.outputs.runner, JSON.stringify(bad)).toBeUndefined(); + expect(refused.args, JSON.stringify(bad)).toBe(''); + } + } finally { fs.rmSync(directory, { recursive: true, force: true }); } + }); + + test('every shard reports its peak memory and minimum free disk, even when the suite fails', () => { + const steps = (Bun.YAML.parse(source) as any).jobs['free-suite'].steps; + const start = steps.findIndex((step: any) => step.name === 'Start the memory and disk sampler'); + const suite = steps.findIndex((step: any) => step.name === 'Run free suite'); + const report = steps.findIndex((step: any) => step.name === 'Report peak memory and minimum free disk'); + expect(start).toBeGreaterThan(-1); + expect(start).toBeLessThan(suite); + expect(report).toBeGreaterThan(suite); + expect(steps[report].if).toBe('always()'); + expect(steps[report].run).toContain('RESOURCES: peak_used_mib='); + }); + test('least-privilege token: contents read-only, credentials not persisted', () => { // The job executes PR-controlled code (install lifecycle scripts + the // suite itself). A default-grant GITHUB_TOKEN persisted into .git/config diff --git a/test/gstack-post.test.ts b/test/gstack-post.test.ts index 67bd093db..72e6ecbf7 100644 --- a/test/gstack-post.test.ts +++ b/test/gstack-post.test.ts @@ -446,21 +446,21 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(after.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, Why; kept unclosed receipt fence; added none\n"); }); - // Value: protects=a file with sections owns the text above the first heading (GST-412): stale lead text and stale text between the receipt and the + // Value: protects=a full post (a receipt above its first heading, as /ship composes it) owns the text above the first heading (GST-412): stale lead text and stale text between the receipt and the // first heading are dropped and named on stderr with their first line, the file's own preamble takes their place, a receipt-only or text-only file // keeps the older merge, a repost is byte-identical and an attribution callout is never doubled; fails_when=pr-body keeps a stale intro above the // receipt or a stale note under it after /ship reposts the sections; why_new=lead text was kept unless the file brought its own; seam=fake run; // no_claim=real gh - test("a file with sections replaces every text section above the current first heading and names each dropped piece", () => { + test("a full post replaces every text section above the current first heading and names each dropped piece", () => { const stale = withBody("Draft intro from gh pr create, long enough to need truncation in the summary line\nsecond line\n\n" + CURRENT, { "/b": RECEIPT("bbbb") + "\n## What changed\n\n- new\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], stale)).toBe(EXIT.posted); expect(sent(stale).input).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- new\n\n## Why\n\nBecause.\n\n## Approvals\n\n- owner: do it\n"); expect(stale.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, What changed; kept Why, Approvals; added none; dropped leading text (\"Draft intro from gh pr create, long enough to need truncati…\")\n"); - const between = withBody(RECEIPT("aaaa") + "Old note under the receipt\n\n## What changed\n\n- old\n", { "/b": "## What changed\n\n- new\n" }); + const between = withBody(RECEIPT("aaaa") + "Old note under the receipt\n\n## What changed\n\n- old\n", { "/b": RECEIPT("bbbb") + "## What changed\n\n- new\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], between)).toBe(EXIT.posted); - expect(sent(between).input).toBe(RECEIPT("aaaa") + "\n## What changed\n\n- new\n"); - expect(between.stderr()).toBe("gstack-post: pr-body merged by section: replaced What changed; kept ship receipt; added none; dropped trailing text (\"Old note under the receipt\")\n"); + expect(sent(between).input).toBe(RECEIPT("bbbb") + "\n## What changed\n\n- new\n"); + expect(between.stderr()).toBe("gstack-post: pr-body merged by section: replaced ship receipt, What changed; kept none; added none; dropped trailing text (\"Old note under the receipt\")\n"); const own = withBody("Stale intro\n\n" + RECEIPT("aaaa") + "Stale note\n\n## Why\n\nold\n", { "/b": "Fresh intro\n\n" + RECEIPT("bbbb") + "Fresh note\n\n## Why\n\nnew\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], own)).toBe(EXIT.posted); @@ -469,6 +469,23 @@ describe("gstack-post pr-body merges the PR body by section", () => { expect(own.stderr()).toBe("gstack-post: pr-body merged by section: replaced leading text, ship receipt, trailing text, Why; kept none; added none\n"); }); + // Value: protects=a partial post (no receipt above its first `## ` heading, even with a caption of its own or a receipt further down) keeps + // the human intro and the note under the receipt; fails_when=a lane posting only `## Screenshots` + // deletes the intro or a Review-loop exception line; why_new=GST-412 made any file with a section own the preamble (GstackMinder, + // 2026-10-31: "a partial pr-body post must keep the human intro"); seam=fake run; no_claim=real gh + test("a partial post keeps the current intro and the note under the receipt", () => { + const partial = withBody("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n", { "/b": "## Screenshots\n\n![shot](u)\n" }); + expect(run(["pr-body", "5", "--body-file", "/b"], partial)).toBe(EXIT.posted); + expect(sent(partial).input).toBe("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n\n## Screenshots\n\n![shot](u)\n"); + expect(partial.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept leading text, ship receipt, trailing text, What changed; added Screenshots\n"); + + for (const file of ["Screenshots below.\n\n## Screenshots\n\n![shot](u)\n", "## Screenshots\n\n![shot](u)\n\n" + RECEIPT("bbbb")]) { + const merged = mergeBodySections("Human intro\n\n" + RECEIPT("aaaa") + "Review-loop exception: accepted by owner\n\n## What changed\n\n- old\n", file); + expect(merged.body).toContain("Review-loop exception: accepted by owner"); + expect(merged.dropped).toEqual([]); + } + }); + // Value: protects=the dropped-piece excerpt on stderr never repeats a secret the merge removed from the body (the scan only reads the merged bytes); // fails_when=a stale intro holding a credential is echoed to stderr while it leaves the PR; why_new=the excerpt is new in GST-412; seam=fake run; no_claim=real gh test("a dropped piece whose first line the redaction scan flags is named without its text", () => { @@ -504,21 +521,21 @@ describe("gstack-post pr-body merges the PR body by section", () => { }); // Value: protects=--replace keeps the plain overwrite for a caller that composed the whole body, a current body with no H2 headings is kept as the leading - // section by a file without sections and dropped (and named) by a file with sections (GST-412), an empty current body posts the new body byte for byte, + // section by a file without sections and dropped (and named) by a full post (GST-412), an empty current body posts the new body byte for byte, // the scan runs on the merged bytes, and an unreadable current body is exit 3 with the --replace hint rather than an overwrite; fails_when=--replace // merges, headingless text is dropped by a receipt-only post or kept under new sections, the merge changes a lone new body, a secret in a kept section // passes the scan, or a failed read overwrites; why_new=first pr-body merge tests; seam=fake run; no_claim=real gh - test("--replace overwrites; a body without H2 headings is kept by a file without sections and dropped by one with sections; the merged bytes are what is scanned; a failed read refuses", () => { + test("--replace overwrites; a body without H2 headings is kept by a file without a receipt and dropped by a full post; the merged bytes are what is scanned; a failed read refuses", () => { const replaced = withBody(CURRENT, { "/b": "## What changed\n\nonly this\n" }); expect(run(["pr-body", "5", "--replace", "--body-file", "/b"], replaced)).toBe(EXIT.posted); expect(sent(replaced).input).toBe("## What changed\n\nonly this\n"); expect(replaced.stderr()).toBe(""); expect(replaced.calls.some(c => c.cmd === "gh" && c.args[1] === "view")).toBe(false); - const plain = withBody("Just a paragraph.\nAnother line.", { "/b": "## Tests\n\n- one\n" }); + const plain = withBody("Just a paragraph.\nAnother line.", { "/b": RECEIPT("bbbb") + "\n## Tests\n\n- one\n" }); expect(run(["pr-body", "5", "--body-file", "/b"], plain)).toBe(EXIT.posted); - expect(sent(plain).input).toBe("## Tests\n\n- one\n"); - expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept none; added Tests; dropped leading text (\"Just a paragraph.\")\n"); + expect(sent(plain).input).toBe(RECEIPT("bbbb") + "\n## Tests\n\n- one\n"); + expect(plain.stderr()).toBe("gstack-post: pr-body merged by section: replaced none; kept none; added ship receipt, Tests; dropped leading text (\"Just a paragraph.\")\n"); const plainText = withBody("Just a paragraph.\nAnother line.", { "/b": RECEIPT("bbbb") }); expect(run(["pr-body", "5", "--body-file", "/b"], plainText)).toBe(EXIT.posted); expect(sent(plainText).input).toBe(RECEIPT("bbbb") + "\nJust a paragraph.\nAnother line."); diff --git a/test/helpers/free-fixtures-data.ts b/test/helpers/free-fixtures-data.ts index 5fee854af..dfe62b544 100644 --- a/test/helpers/free-fixtures-data.ts +++ b/test/helpers/free-fixtures-data.ts @@ -196,6 +196,7 @@ export const FREE_FIXTURES: Readonly> = { 'test/fixtures/redact-diagram/**': ['test/redact-context-exemptions.test.ts'], 'test/fixtures/remote-identity-vectors.json': ['test/bin-context-windows-slug.test.ts', 'test/gstack-slug-parity.test.ts', 'test/remote-identity-vectors.test.ts'], 'test/fixtures/review-depth/**': ['test/review-dispatch-compare.test.ts'], + 'test/fixtures/review-ledger/**': ['test/review-dispatch.test.ts'], 'test/fixtures/review-design-lite-reports-ci-36633323521.json': ['test/review-finalization-budget.test.ts'], 'test/fixtures/review-gate-integrity-opus-reports.json': ['test/gate-integrity-templates.test.ts'], 'test/fixtures/session-update-heartbeat.sh': ['test/session-update-heartbeat.test.ts'], diff --git a/test/pregate-remote.test.ts b/test/pregate-remote.test.ts index 88308ed8a..ca781b02e 100644 --- a/test/pregate-remote.test.ts +++ b/test/pregate-remote.test.ts @@ -22,6 +22,7 @@ const GATE = 'node scripts/remote-gate.js'; const GATE_SCRIPT = [ "const fs = require('node:fs');", "fs.writeFileSync('../gate-env.json', JSON.stringify({ tree: process.env.PREGATE_TREE, cap: process.env.PREGATE_SPEND_CAP_USD, timeout: process.env.PREGATE_TIMEOUT_S, files: fs.readFileSync(process.env.PREGATE_FILES_FILE, 'utf8') }));", + "if (process.env.GATE_QUIET_MS) Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, Number(process.env.GATE_QUIET_MS));", "if (process.env.GATE_SILENT) process.exit(0);", "const spent = process.env.GATE_SPENT ? ` spent=${process.env.GATE_SPENT}` : '';", "console.log(`PREGATE_REMOTE: tree=${process.env.GATE_TREE || process.env.PREGATE_TREE} tests=${process.env.GATE_TESTS ?? 3} failed=${process.env.GATE_FAILED ?? 0}${spent}`);", @@ -187,6 +188,22 @@ describe('gstack-pregate --remote (C-c)', () => { expect(fs.existsSync(path.join(repo.root, 'gate-env.json'))).toBe(false); }, 60_000); + // Value: protects=a remote gate that waits quietly on CI is bounded by its deadline (--timeout, else timeout_s), not killed by the + // test-output stall timer; a gate past its deadline still stays uncleared (GST-431); fails_when=the stall timer kills a quiet gate + // inside its deadline, or a gate past --timeout clears; why_new=a CI-waiting gate died at 300 s on qm (CAPY-1); seam=fake gate; + // no_claim=the Ubicloud VM + test('a gate that is quiet past the stall timeout but inside its deadline clears; past --timeout it stays uncleared', () => { + const repo = gated(); + const quiet = remote(repo, ['--remote', '--allow-repo-commands', '--spend-cap', '2', '--stall-timeout', '1', '--timeout', '60'], { GATE_QUIET_MS: '2500' }); + expect(quiet.status).toBe(EXIT.ok); + expect(pregateFile(repo).verdict).toBe('requires-remote cleared'); + const late = remote(repo, ['--remote', '--allow-repo-commands', '--spend-cap', '2', '--timeout', '2'], { GATE_QUIET_MS: '8000' }); + expect(late.status).toBe(EXIT.fail); + expect(lanes(repo)).toMatchObject({ status: 'incomplete', code: 'PREGATE_INCOMPLETE' }); + expect(lanes(repo).detail).toContain('the remote gate did not finish (timed out after 2 s, --timeout)'); + expect(pregateFile(repo).requires_remote[0].receipt).toBeNull(); + }, 60_000); + test('a wrong tree, zero tests, a failure, a missing line or a non-zero exit leaves the obligation uncleared and the check incomplete; unreported spend counts at the cap', () => { const repo = gated(); const cases: Array<[Record, string]> = [ diff --git a/test/pregate-supervise.test.ts b/test/pregate-supervise.test.ts index abdf3909b..70921184a 100644 --- a/test/pregate-supervise.test.ts +++ b/test/pregate-supervise.test.ts @@ -91,6 +91,28 @@ describe('supervise: process group, log file, stall and deadline', () => { expect(r.lines.filter(l => l.startsWith('PREGATE_HEARTBEAT: fixture running ')).length).toBeGreaterThanOrEqual(2); expect(r.lines.some(l => l.startsWith('PREGATE_TEST:'))).toBe(false); }); + + // Value: protects=a remote gate's heartbeat says what the gate last printed (GST-431); fails_when=the heartbeat for a gate that + // reports its CI wait shows only "last file none reported"; why_new=a CI-waiting gate looked dead on qm (CAPY-1); seam=none; no_claim=none + test('with lastLine, a heartbeat names the command\'s last output line', async () => { + const dir = scratch(); + const r = await run(dir, 'echo "waiting on run 7 (in_progress)"; sleep 1.2; echo done', { heartbeatMs: 300, lastLine: true }); + expect(r.outcome).toBe('exited'); + expect(r.lines.some(l => l.startsWith('PREGATE_HEARTBEAT: fixture running ') && l.includes('last output: waiting on run 7 (in_progress)'))).toBe(true); + const redraw = await run(dir, "printf 'run 7 queued\\r\\033[2Krun 7 in_progress\\n'; sleep 1.2", { heartbeatMs: 300, lastLine: true }); + expect(redraw.lines.some(l => l.endsWith('last output: run 7 in_progress'))).toBe(true); + expect(redraw.lines.some(l => l.includes('\x1b') || l.includes('\r'))).toBe(false); + }); + + // Value: protects=reading the last line is best effort: a log that disappears mid-run never throws out of the supervisor's timer, which would + // abandon the gate's deadline and cleanup (GST-431 review); fails_when=lastLine sampling throws ENOENT; why_new=lastLine is new; seam=none; no_claim=none + test('with lastLine, a log removed mid-run still ends in a normal outcome', async () => { + const dir = scratch(); + const log = path.join(dir, 'run.log'); + fs.writeFileSync(log, ''); + const r = await supervise({ command: `echo hi; sleep 0.4; rm -f "${log}"; sleep 1.2; echo done`, cwd: dir, env: process.env, log, deadline: Date.now() + 30_000, stallMs: 0, graceMs: 300, pollMs: 50, heartbeatMs: 300, lastLine: true, label: 'fixture', progress: () => {} }); + expect(r.outcome).toBe('exited'); + }); }); describe('supervise: per-file progress adapters', () => { diff --git a/test/review-dispatch.test.ts b/test/review-dispatch.test.ts index 826f502c6..b76de4a38 100644 --- a/test/review-dispatch.test.ts +++ b/test/review-dispatch.test.ts @@ -4,7 +4,7 @@ import * as fs from 'node:fs'; import * as os from 'node:os'; import * as path from 'node:path'; import { BriefError, promptHash, renderBrief, sectionTexts } from '../lib/review-brief'; -import { judge, parseFindings, type MergedLine } from '../lib/review-ledger'; +import { collect, judge, parseFindings, type MergedLine } from '../lib/review-ledger'; const ROOT = path.resolve(import.meta.dir, '..'); const BIN = path.join(ROOT, 'bin', 'gstack-review-dispatch'); @@ -326,6 +326,148 @@ describe('gstack-review-dispatch sent, collect, judge and verify: nothing change expect(parseFindings('outside-voice', '1. Retry loses writes (queue.ts:78). Severity: Medium\n Fix: reject the batch. Blocks: yes')).toEqual([{ label: 'CRITICAL', raw: '1. Retry loses writes (queue.ts:78). Severity: Medium | Fix: reject the batch. Blocks: yes' }]); }); + // Value: protects=a reader reply with no finding and no clean marker (a refusal, an empty reply, a reader that never saw the diff) is + // recorded as a non-review: collect exits 1 and verify reports the brief NOT_COLLECTED, never clean; fails_when=collect hashes the + // GSTA-196 refusal ("I'm not returning NO FINDINGS"), an empty file or an "I could not access the diff" reply as findings=0 and verify + // reads MATCH (replay C3-36: the batched specialist found no diff base and its brief read MATCH); + // why_new=collect recorded whatever parseFindings returned and verify counted any output as collected; seam=collect and verify; + // no_claim=that a reply carrying a marker actually reviewed the diff + test('a reply with no finding and no clean marker is a non-review: collect exits 1 and verify is NOT_COLLECTED, never clean', () => { + const refusal = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/gsta-196-refusal.txt'), 'utf8'); + expect(promptHash(refusal)).toBe('7150c3b9ffe2698f77a988fa422b66d7004e1939d12e81185e8d556a47dff784'); + const b = tmp(); + const batched = field(run(['render', '--role', 'specialist-batched', '--checklists', 'testing,security,state-machines', '--out', b, '--diff-base', SHA]).stdout, 'file'); + const cases = [ + { reader: 'specialist-batched', dir: b, brief: batched, output: refusal, args: ['--role', 'specialist-batched'] }, + ...['', 'I could not access the diff: git diff failed with "bad object", so there is nothing here to review.\n'].map(output => { + const p = pass(); + return { reader: 'specialist/testing', dir: p.out, brief: p.file, output, args: ['--role', 'specialist', '--name', 'testing'] }; + }), + ]; + for (const c of cases) { + const ledger = path.join(c.dir, 'review-ledger.jsonl'); + const merged = path.join(c.dir, 'merged.jsonl'); + const out = path.join(c.dir, 'reply.out'); + fs.writeFileSync(out, c.output); + expect(run(['sent', '--ledger', ledger, ...c.args, '--sent', c.brief]).status).toBe(0); + const col = run(['collect', '--ledger', ledger, ...c.args, '--output', out, '--merged', merged]); + expect([col.status, col.stdout.startsWith(`COLLECT_NOT_REVIEWED: reader=${c.reader} findings=0 sha256=${promptHash(c.output)} `)]).toEqual([1, true]); + const event = JSON.parse(fs.readFileSync(ledger, 'utf8').trim().split('\n').at(-1)!); + expect([event.event, event.ids, event.review]).toEqual(['output', [], false]); + fs.writeFileSync(path.join(c.dir, 'judged.jsonl'), ''); + expect(run(['judge', '--ledger', ledger, '--merged', merged, '--judged', path.join(c.dir, 'judged.jsonl')]).status).toBe(0); + const v = run(['verify', '--ledger', ledger]); + expect([v.status, v.stdout.includes(`BRIEF ${c.reader} NOT_COLLECTED rendered=`), v.stdout.includes(`OUTPUT ${c.reader} NOT_REVIEWED`), v.stdout.includes(' not_reviewed\n'), v.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true, true, true]); + } + }); + + // Value: protects=a clean reply carrying its section's marker and a reply with findings still collect with exit 0 and verify clean under + // the non-review rule; fails_when=the rule makes a real NO FINDINGS specialist or a findings reply dirty or exit 1; + // why_new=the non-review rule above could over-reach; seam=collect and verify; no_claim=other roles (the matrix below covers them) + test('a NO FINDINGS reply and a findings reply still collect with exit 0 and verify clean', () => { + for (const [output, findings] of [['NO FINDINGS\n', 0], ['{"severity":"CRITICAL","path":"a.ts","line":3,"category":"logic","summary":"off by one","specialist":"testing"}\n', 1]] as const) { + const p = pass(); + run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]); + const col = run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', p.w('reply.out', output), '--merged', p.merged]); + expect([col.status, col.stdout.startsWith(`COLLECT: reader=specialist/testing findings=${findings} `)]).toEqual([0, true]); + expect(JSON.parse(fs.readFileSync(p.ledger, 'utf8').trim().split('\n').at(-1)!).review).toBe(true); + const judged = p.w('judged.jsonl', findings ? JSON.stringify({ id: 'specialist/testing#1', severity: 'CRITICAL' }) + '\n' : ''); + expect(run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', judged]).status).toBe(0); + const v = run(['verify', '--ledger', p.ledger]); + expect([v.status, v.stdout.includes(`OUTPUT specialist/testing MATCH sha=${promptHash(output).slice(0, 12)} findings=${findings}`), v.stdout.includes('VERIFY: clean briefs=1 outputs=1')]).toEqual([0, true, true]); + } + }); + + // Value: protects=each role's zero-finding reply counts as a review only with the marker its own section prescribes (NO FINDINGS for + // specialists, the batched specialist, Red Team, fix-check and targeted; the Recommendation: line for the outside voice; either for the + // native adversarial voice); fails_when=a marker named inside a sentence, another role's marker or an empty reply reads as clean, or a + // prescribed marker reads as a non-review; why_new=no marker check existed; seam=collect; no_claim=that the marker's reason is true + test('each role takes only the clean marker its section prescribes', () => { + const d = tmp(); + const outsideClean = 'No actionable findings in the supplied diff. Static review only.\n\nRecommendation: approve because the diff introduces no demonstrated bypass.\n'; + const cases: Array<[string, string, boolean]> = [ + ['specialist/testing', 'NO FINDINGS\n', true], ['specialist-batched', '`NO FINDINGS`\n', true], ['targeted', 'NO FINDINGS\n', true], ['fix-check', 'NO FINDINGS\n', true], + ['red-team', 'Fixtures were reviewed in summary mode only.\n\nNO FINDINGS\n', true], + ['adversarial-native', 'Fixtures were reviewed in summary mode.\n\nRecommendation: Ship as-is because no finding survived tracing\n', true], ['adversarial-native', 'NO FINDINGS\n', true], + ['outside-voice', outsideClean, true], ['outside-voice', 'NO FINDINGS\n', false], + ['specialist/testing', 'Recommendation: ship because it is clean\n', false], ['specialist/testing', "I'm not returning NO FINDINGS, because I never saw the diff.\n", false], + ['red-team', '', false], ['fix-check', 'The fixes look fine to me.\n', false], ['adversarial-native', 'I could not read the diff.\n', false], + ]; + for (const [i, [reader, output, review]] of cases.entries()) { + const ledger = path.join(d, `l${i}.jsonl`); + const r = collect(ledger, reader, output, path.join(d, `m${i}.jsonl`)); + expect([reader, output, r.lines.length, r.review]).toEqual([reader, output, 0, review]); + expect(JSON.parse(fs.readFileSync(ledger, 'utf8').trim()).review).toBe(review); + } + }); + + // Value: protects=a paragraph finding's unlabeled detail bullets and the Severity/Blocks line after them stay one finding (GSTA-196 + // native voice: 4 findings collected as 8, the closing Severity lines opening phantoms judge had to fold); a list item's sibling and + // a labeled bullet still start their own findings; fails_when=the sample collects more or fewer than 4, or a labeled bullet merges + // away; why_new=every margin list item closed the open finding; seam=parseFindings; no_claim=indented detail lists (already attached) + test('unlabeled detail bullets and the Severity line after them stay on the open paragraph finding', () => { + const native = fs.readFileSync(path.join(ROOT, 'test/fixtures/review-ledger/gsta-196-native.txt'), 'utf8'); + expect(parseFindings('adversarial-native', native).map(f => [f.label, f.raw.slice(0, 12), f.raw.endsWith('| Severity: ' + (f.label === 'High' ? 'High' : 'Low') + '. Blocks: no.')])) + .toEqual([['High', '**1. A PR in', true], ['CRITICAL', '**2. A case ', true], ['CRITICAL', '**3. Local `', true], ['Low', '**4. No trai', true]]); + expect(parseFindings('outside-voice', '1. FIXABLE: race in a.ts:3\n- Unlabeled sibling at b.ts:9\nSeverity: Medium').map(f => f.label)).toEqual(['FIXABLE', 'Medium']); + expect(parseFindings('outside-voice', '**Retry storm in r.ts:2 (INVESTIGATE).**\n- detail\n- High: a second defect in s.ts:4\nSeverity: Medium').map(f => f.label)).toEqual(['INVESTIGATE', 'High']); + expect(parseFindings('outside-voice', 'Low: cache TTL mismatch (cache.ts:4)\n\n2. Token never expires (auth.ts:9)\nSeverity: High\nBlocks: yes').map(f => [f.label, f.raw])) + .toEqual([['Low', 'Low: cache TTL mismatch (cache.ts:4)'], ['CRITICAL', '2. Token never expires (auth.ts:9) | Severity: High | Blocks: yes']]); + expect(parseFindings('outside-voice', '**Race in a.ts:3 (High).**\n- the writer holds no lock\nSeverity: Medium').map(f => f.raw)).toEqual(['**Race in a.ts:3 (High).** | - the writer holds no lock | Severity: Medium']); + expect(parseFindings('outside-voice', '**Race in a.ts:3 (Medium).**\n\n- the writer holds no lock\n\nSeverity: Medium. Blocks: yes').map(f => [f.label, f.raw])) + .toEqual([['CRITICAL', '**Race in a.ts:3 (Medium).** | - the writer holds no lock | Severity: Medium. Blocks: yes']]); + }); + + // Value: protects=a non-review the same dispatch later answers with a review is voided (counted, VERIFY clean), and a failed outside + // voice recorded with --unavailable reads UNAVAILABLE (missing coverage, never a review) without blocking, while --unavailable is + // refused for any other reader; fails_when=one outside-voice timeout or one refusal makes VERIFY dirty for good, or --unavailable + // lets a specialist skip coverage; why_new=#3677 Step 11 adversarial voice; seam=collect and verify; no_claim=the review-log row + test('a refusal replaced by a review is voided; an outside voice failure is UNAVAILABLE; --unavailable is the outside voice only', () => { + const p = pass(); + const send = () => run(['sent', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--sent', p.file]); + const take = (f: string) => run(['collect', '--ledger', p.ledger, '--role', 'specialist', '--name', 'testing', '--output', f, '--merged', p.merged]); + send(); + expect(take(p.w('refusal.out', 'I could not access the diff.\n')).status).toBe(1); + expect(run(['verify', '--ledger', p.ledger]).stdout).toContain('VERIFY: dirty'); + send(); + expect(take(p.w('clean.out', 'NO FINDINGS\n')).status).toBe(0); + fs.writeFileSync(path.join(p.out, 'judged.jsonl'), ''); + run(['judge', '--ledger', p.ledger, '--merged', p.merged, '--judged', path.join(p.out, 'judged.jsonl')]); + const v = run(['verify', '--ledger', p.ledger]); + expect([v.status, v.stdout.includes('replaced_by_a_review'), v.stdout.includes('VERIFY: clean briefs=1 outputs=2 voided=1')]).toEqual([0, true, true]); + const head = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: ROOT, encoding: 'utf8', timeout: 10_000 }).stdout.trim(); + const o = tmp(); + const ov = field(run(['render', '--role', 'outside-voice', '--out', o, '--diff-base', head]).stdout, 'file'); + const ledger = path.join(o, 'review-ledger.jsonl'), merged = path.join(o, 'merged.jsonl'); + run(['sent', '--ledger', ledger, '--role', 'outside-voice', '--sent', ov]); + const fail = path.join(o, 'timeout.out'); + fs.writeFileSync(fail, 'Codex timed out after 9 minutes and was terminated; this pass produced NO findings.\n'); + const c = run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', fail, '--merged', merged, '--unavailable', 'timeout']); + expect([c.status, c.stdout.startsWith('COLLECT_UNAVAILABLE: reader=outside-voice ')]).toEqual([0, true]); + fs.writeFileSync(path.join(o, 'judged.jsonl'), ''); + run(['judge', '--ledger', ledger, '--merged', merged, '--judged', path.join(o, 'judged.jsonl')]); + const w = run(['verify', '--ledger', ledger]); + expect([w.status, w.stdout.includes('BRIEF outside-voice UNAVAILABLE'), w.stdout.includes('OUTPUT outside-voice UNAVAILABLE'), w.stdout.includes(' unavailable=1\n')]).toEqual([0, true, true, true]); + const q = pass(); + run(['sent', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--sent', q.file]); + const refused = run(['collect', '--ledger', q.ledger, '--role', 'specialist', '--name', 'testing', '--output', q.w('t.out', 'timed out\n'), '--merged', q.merged, '--unavailable', 'timeout']); + expect([refused.status, refused.stderr.includes('--unavailable is for the outside voice only')]).toEqual([2, true]); + const partial = path.join(o, 'partial.out'); + fs.writeFileSync(partial, '1. **CRITICAL**: token leak in a.ts:3\nCodex then timed out.\n'); + const kept = run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', partial, '--merged', merged, '--unavailable', 'timeout']); + expect([kept.status, kept.stderr.includes('holds findings; collect it without --unavailable')]).toEqual([2, true]); + expect(run(['collect', '--ledger', ledger, '--role', 'outside-voice', '--output', fail, '--merged', merged, '--unavailable', ' ']).status).toBe(2); + const n = pass(); + const takeN = (f: string) => run(['collect', '--ledger', n.ledger, '--role', 'specialist', '--name', 'testing', '--output', f, '--merged', n.merged]); + run(['sent', '--ledger', n.ledger, '--role', 'specialist', '--name', 'testing', '--sent', n.file]); + takeN(n.w('refusal.out', 'I could not access the diff.\n')); + takeN(n.w('clean.out', 'NO FINDINGS\n')); + fs.writeFileSync(path.join(n.out, 'judged.jsonl'), ''); + run(['judge', '--ledger', n.ledger, '--merged', n.merged, '--judged', path.join(n.out, 'judged.jsonl')]); + const unsent = run(['verify', '--ledger', n.ledger]); + expect([unsent.status, unsent.stdout.includes(' NOT_REVIEWED '), unsent.stdout.includes('VERIFY: dirty')]).toEqual([1, true, true]); + }); + test('verify on a ledger that kept no prompts says so and exits 3', () => { const d = tmp(); const ledger = path.join(d, 'ledger.jsonl'); diff --git a/test/review-pass.test.ts b/test/review-pass.test.ts index 624a57452..9bfc99c89 100644 --- a/test/review-pass.test.ts +++ b/test/review-pass.test.ts @@ -128,6 +128,7 @@ function repo(): { dir: string; env: NodeJS.ProcessEnv } { interface Sub { title: string; prompt: string; status: string; polls: number; model?: unknown; summary?: string; late?: boolean } /** A fake Capy host: the shell is real (the repository's bins), subagents are scripted. */ +const cleanReply = (title: string) => (title.endsWith('outside-voice') ? 'Recommendation: ship as-is because no finding survived' : 'NO FINDINGS'); function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) => string; fail?: (title: string) => boolean; reject?: (title: string) => boolean; createThrows?: (title: string) => 'before' | 'after' | undefined; pollsToFinish?: number; lateSummary?: boolean } = {}) { let clock = 0; const subs = new Map(); @@ -135,12 +136,12 @@ function host(dir: string, env: NodeJS.ProcessEnv, o: { output?: (title: string) const starts: number[][] = []; const advance = () => { for (const s of subs.values()) { - if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? 'NO FINDINGS'; continue; } + if (s.late && !s.summary) { s.summary = o.output?.(s.title) ?? cleanReply(s.title); continue; } if (s.status !== 'working') continue; if (++s.polls < (o.pollsToFinish ?? 1)) continue; if (o.fail?.(s.title)) s.status = 'failed'; else if (o.lateSummary && !s.late) { s.status = 'done'; s.late = true; } - else { s.status = 'done'; s.summary = o.output?.(s.title) ?? 'NO FINDINGS'; } + else { s.status = 'done'; s.summary = o.output?.(s.title) ?? cleanReply(s.title); } } }; const tools = {