The five internal/app partitions ran end to end inside one job, so the app
shard's wall clock was the sum of all five: 780s in CI, of which the schema
partition owned 357s. Each partition is now its own matrix shard, so they run
concurrently and the shard's wall clock is set by its slowest partition rather
than by their total. Every partition shard still selects the same single
internal/app package, so the impacted-package query maps the shard name back to
app and the partition only chooses which tests run.
The helper gains a partition argument and a list-partitions mode. APP_PARTITIONS
is the single source of truth for the set, and the discovery pass still runs in
every job, so each one independently verifies that the partition patterns cover
every top-level test exactly once before running the one it was asked for.
Two fail-closed checks guard the split, because the helper's own coverage check
can no longer prove the whole package ran once the partitions are separate jobs:
- The helper cross-checks APP_PARTITIONS against the coverage counters in both
directions, so a counted partition that nothing dispatches and a dispatchable
partition with no counter both fail instead of silently skipping tests.
- TestCIAppRacePartitionMatrixMatchesHelper pins the workflow's app-<partition>
shards to list-partitions output in both directions, so a partition cannot
lose its job while every job stays green.
The discovery loop variable is renamed from partition to spec: it would
otherwise shadow the partition requested on the command line, which run mode
reads after the discovery pass completes.
The schema partition's 52 tests assert structural Schema-to-Cobra contracts over
a single goroutine: none of them call t.Parallel or start a goroutine, so the
race detector has no concurrent access to observe there. The process-global lazy
metadata that does need race coverage (schema_source_root's atomic.Value, the
parameter-binding lazy loaders) is exercised by internal/cli's concurrent tests,
which stay instrumented.
The instrumentation was not free here. The partition shares a single sync.Once
Catalog build whose work is allocation-heavy, and -race made it roughly 11x
slower: 26s -> 291s locally, and 357s of the app shard's 780s in CI. Within that
partition TestFinalSchemaToolsHaveExecutableBaseCommands alone accounted for
262s, not because the test is expensive but because it is the first caller to pay
for the shared snapshot; its 1121 subtests together measure 0.00s.
run_partition now takes the instrumentation mode explicitly and fails closed on
an unrecognized value, so a typo cannot silently drop -race from a partition that
is supposed to carry it.
The focused path tested every impacted package in a single job with a
plain `go test -race`, so internal/app ran inside one long-lived process
alongside all of its reverse dependencies. That is exactly the shape
scripts/ci/run-app-race-tests.sh exists to avoid: a single app test
process retains every constructed command tree in framework registries,
so the run grows to 900s and the job stays alive long enough to be
reclaimed by the runner. Recent focused runs failed with SIGTERM after
9-10 minutes without a single test failure, and one earlier run failed
at `internal/app 902.651s`, 2.65s past the package timeout.
Fan the same package plan across the shard matrix test-race already
uses, and run each shard the way test-race runs it: internal/app through
the process-isolating helper, cli/smoke with their wider package budget,
release-scripts without race and with archive tooling.
changed-test-packages.sh gains `list-shard`, which intersects the
impacted set with scripts/ci/test-packages.sh shard membership so shard
definitions stay single-sourced — and so an unknown shard name aborts
there rather than reporting an empty selection, which would let a
mistyped shard skip every test while reporting success.
release-scripts is in the matrix on purpose: its dedicated job only runs
at full-suite or release-sensitive scope, so omitting it here would stop
testing test/scripts changes altogether. A test pins that the shard
selections partition the impacted set exactly, so shard-plan drift
cannot silently shrink focused coverage.
The Coverage context was the PR critical path (~17 min end to end):
coverage-current re-ran the whole suite serially (-p 1, ~13 min) and
coverage-baseline re-ran it again at the merge-base (~13 min) although
that profile is a pure function of the base commit.
- coverage-current now owns only the scoped (standard-tier) profile;
full-suite candidate profiles come from a 5-way shard matrix
(app/cli/generators/helpers/remaining) that keeps -p 1 inside each
shard on isolated runners. scripts/ci/test-packages.sh list-coverage
defines the shards and verify proves the union equals the previous
single-run package set exactly once.
- the aggregate Coverage job reassembles the disjoint shard profiles
into coverage.txt before make coverage-gate, failing closed when a
shard file is missing, so gate semantics (100% changed-code +
scope-matched overall non-regression) are byte-compatible.
- coverage-baseline restores the merge-base full-suite profile from an
exact-key cache (merge-base SHA + resolved Go version) written by the
last green main push; any miss falls back to recomputing in the
merge-base worktree. Exact key only - no prefix fallback, a near-miss
profile would compare the candidate against the wrong commit.
- new contract tests pin the shard matrix, the assembly step, the
exact-key cache pair, and the absence of restore-keys; the package
plan test also covers the coverage shard partition.
Bind each accepted marker to the exact workflow run attempt, immutable artifact, source comment, and current PR head so a historical successful run cannot authorize a different payload.
Address P1 finding: extract_payload now strictly requires the parsed JSON
to be a dict, and validates each field's type and format:
- pr_number: string of digits
- pr_head_sha: 40-char lowercase hex string
- products: alphanumeric with commas/dots/hyphens/underscores only
- run_id: string of digits
- cases_ref: string (may be empty)
validate_run_id also guards against non-string input.
Added tests for: integer/array/string/null JSON, numeric field types,
invalid SHA format, injection in products, missing required fields.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address P1 lint finding: structured eval-dispatch comments could be
forged by unauthorized users. Add three-layer consumer-side validation:
1. comment.user.login == 'github-actions[bot]' (platform-enforced identity)
2. comment.performed_via_github_app.slug == 'github-actions' (App signature)
3. payload.run_id verified against actual successful workflow run via API
Also adds:
- eval_poll_validate.py: consumer validation module (in-repo, auditable)
- test_eval_poll_validate.py: unit tests proving forged comments are rejected
- Go security contract test updated to assert run_id and validate reference
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- /eval on one's own PR may omit sha=: the guard auto-pins the
dispatch-time head (commenter == PR author leaves no third-party
swap window); dispatching another author's PR still requires the
explicit reviewed SHA (keeps the P1-2 TOCTOU remedy where the
threat lives)
- cases= is now validated structurally per git check-ref-format
semantics (leading/trailing//double slashes, '..', dot-leading
components, .lock suffixes) and rejects '-'-leading values to
prevent git fetch option injection (review P2)
Users listed in .github/eval-allowlist.txt (default branch, PR-reviewed)
may dispatch /eval for their own PRs only; write/maintain/admin retain
dispatch for any PR. Fail-closed on permission API 404/network errors.
race:remaining was SIGTERM'd (exit 143) mid test/smoke after mock_mcp with
no FAIL/DATA RACE; NewRootCommand public-tree smoke under -race is too heavy
to share that shard. Mirror the cli split and give smoke a 15m budget.
Co-authored-by: Cursor <cursoragent@cursor.com>
克谨