[Workflow] Reduce redundant validation and review-cycle latency - #74
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6470f6403
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ok\ *|ok$'\t'*) | ||
| case "${line}" in | ||
| *'[no tests to run]'*) ;; | ||
| *) matched_test=1 ;; |
There was a problem hiding this comment.
When GOFLAGS=-json is configured, go test succeeds and emits JSON records, but every output line begins with {, so this parser never sets matched_test and make fast incorrectly exits 2. The Makefile deliberately exports GOFLAGS, and go help testflag documents that -json logs test results in machine-readable JSON; normalize/reject output-changing flags or parse a stable format so valid focused checks work with this supported Go setting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced on e6470f6: GOFLAGS=-json made a successful selected test return false no-match exit 2. Fixed in 543b147 using explicit go test -json=false while retaining other GOFLAGS. TestFastCheckHandlesJSONGOFLAGS exercises the Make entry with GOFLAGS=-json -race; implementer reports focused TestFastCheck suite and JSON/Make-entry probes PASS. Also removed inherited FAST selector contamination in tooling fixtures. Final head a7d86b2 is awaiting independent delta review plus exact-head CI/Codex; no full-suite local reruns or stale-as-resolved shortcut.
| for part in "${module_parts[@]}" "${package_parts[@]}"; do | ||
| [[ "${part}" != ".." ]] || fail 'selectors must not contain a .. path segment' |
There was a problem hiding this comment.
When FAST_PACKAGE names an in-repository symlink whose target is outside the selected module or repository, this lexical .. check accepts it and go test follows the symlink, allowing make fast to execute an external package despite the documented fail-closed boundary. go help packages defines a leading ./ selector as a filesystem directory; resolve or list the selected package directories and reject any whose real path falls outside module_root.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced on e6470f6: an external package symlink incorrectly returned exit 0. Fixed in 543b147 by physically resolving the package/pattern prefix and rejecting paths outside the selected module. TestFastCheckResolvesPackageSymlinks covers external rejection and a legitimate in-module symlink wildcard. Implementer reports GOTOOLCHAIN=go1.26.8 go test ./scripts -run '^TestFastCheck' -count=1 PASS and shellcheck/diff checks PASS. Final head a7d86b2 differs only by an evidence SHA typo correction. Independent delta review and final-head CI/Codex remain pending; staleness is not treated as resolution.
|
Exact e6470f6 CI is green (run 34424463906). Independent review and Codex findings are consolidated into one correction batch: package symlink boundary, GOFLAGS JSON output, hermetic FAST selector test environment, unsupported actor-side run-count arithmetic, and completion-vs-metadata timing wording. No local full public matrix was duplicated. Existing CI and make check remain unchanged; corrected candidate will receive focused delta review plus its own exact-head Codex/CI. Current head is not approved for merge. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7d86b291c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ( | ||
| cd "${module_root}" | ||
| set +e | ||
| test_output="$("${go_cmd}" test -json=false -count=1 -run "${fast_test}" "${fast_package}" 2>&1)" |
There was a problem hiding this comment.
Fresh evidence after the JSON-specific fix: when a developer has GOFLAGS=-list=., this command lists matching names and emits a successful ok line, so the parser sets matched_test=1 and make fast reports success even though no test executed. go help testflag explicitly states for -list that “No tests, benchmarks, fuzz tests, or examples will be run”; override or reject this mode before treating package success as test evidence.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Corrected in 2ba63c4 by passing explicit -list= to go test. Added TestFastCheckExecutesSelectedTestWithListGOFLAGS with a deliberately failing selected fixture and a no-match control under GOFLAGS=-list=.; implementer reports pre-fix regression red. Independent delta review is verifying actual execution with the correct focused test selector; not treating a too-narrow reported aggregate regex as proof. Fresh exact-head Codex review requested; no merge pending all required gates.
| case "${package_path}" in | ||
| ...) package_prefix='.' ;; | ||
| */...) package_prefix="${package_path%/...}" ;; | ||
| esac |
There was a problem hiding this comment.
/...
When FAST_PACKAGE uses a valid Go pattern such as ./.../capacity, this case does not extract a real directory prefix, so line 73 tries to cd into a literal ... directory and rejects the selector even though Go expands it to ./internal/scheduler/capacity. Verified against go help packages, which says each ... wildcard can match any string, including strings containing slashes; either support general wildcard positions when resolving package directories or explicitly reject and document the narrower syntax.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Corrected in 2ba63c4: go list resolves matched directories for general Go package patterns before physical module-boundary validation. Added ./.../capacity regression and retained in-module symlink coverage. Implementer reports pre-fix regression failure and successful real FAST_MODULE=. FAST_PACKAGE=./.../capacity FAST_TEST='^TestFixedTarget$' make fast probe. The reported aggregate green regex was too narrow to prove TestFastCheck coverage; independent delta review explicitly runs the correct focused selector. Exact-head CI/Codex and independent confirmation remain pending.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2ba63c4635
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ( | ||
| cd "${module_root}" | ||
| set +e | ||
| test_output="$("${go_cmd}" test -json=false -list= -count=1 -run "${fast_test}" "${fast_package}" 2>&1)" |
There was a problem hiding this comment.
When GOFLAGS contains -bench=. or -fuzz=..., this command preserves those execution-expanding flags, so make fast can run work outside FAST_TEST; worse, a benchmark's successful ok line makes a nonexistent FAST_TEST appear matched. Reproducing with GOFLAGS='-bench=. -benchtime=1x' and FAST_TEST='^NoSuchTest$' executed an unrelated benchmark and exited 0. go help testflag confirms that -bench=. runs all benchmarks and that -fuzz starts fuzzing after tests, so override or reject these flags just as this invocation already does for -list.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced on 2ba63c4 before implementation: bounded benchmark/fuzz fixtures unexpectedly ran and could make missing ^NoSuchTest$ exit 0. Fixed source 647c53c explicitly clears bench/fuzz/list/skip/build-only c, retains run/count and useful race/mod flags. Implementer reports exact-source env GOTOOLCHAIN=go1.26.8 GOWORK=off GOFLAGS= go test ./scripts -race -run '^TestFastCheck' -count=1 PASS 13.715s; shellcheck, bash -n, gofmt and diff checks PASS. A reporting shell-interpolation mistake created local revert 9741ce8; normal revert-of-revert restored final head e7c8def. Coordinator verified git diff --exit-code 647c53cec9d9114e8b789587538792a396dbb34d e7c8defd383fbebe44cbcf37d690c2418188f4a3 is empty. No reset/force or live changes. Final independent delta review and exact-head Codex/CI pending; no clean claim from staleness.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7c8defd38
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ( | ||
| cd "${module_root}" | ||
| set +e | ||
| test_output="$("${go_cmd}" test -json=false -list= -bench= -fuzz= -skip= -c=false -count=1 -run "${fast_test}" "${fast_package}" 2>&1)" |
There was a problem hiding this comment.
Fresh evidence on the final commit: with GOFLAGS=-exec=true, a deliberately failing selected test is never invoked, but go test prints ok and this script reports success; with GOFLAGS=-cpu=1,2, the selected test runs twice despite the documented one-run bound. go help test documents -exec as running the test binary through the supplied program, and go help testflag states that -cpu executes tests for every listed GOMAXPROCS value. Override or reject these inherited execution controls before claiming the selected test ran once.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7751693: explicit -exec= removes inherited bypass wrappers and -cpu=1 prevents repeated CPU-list execution. New exec/CPU fixtures failed before the fix; implementer reports GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 -run '^TestFastCheck' ./scripts PASS 13.907s afterward, including selected-failure and single-run controls. Independent exact-source delta review plus final-head Codex/CI pending; no full root/G01/G02 reruns.
| ok\ *|ok$'\t'*) | ||
| case "${line}" in | ||
| *'[no tests to run]'*) ;; | ||
| *) matched_test=1 ;; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier JSON-output finding: when GOFLAGS=-v is inherited and a package's TestMain prints a line beginning with ok before calling m.Run, a nonexistent FAST_TEST produces that package-controlled line plus the real [no tests to run] summary, and this loop sets matched_test=1 and exits successfully. go help testflag documents -v as enabling verbose output; do not interpret arbitrary verbose test-process output as a Go package summary—use structured test run/pass events or otherwise authenticate the summary before claiming a compiled test matched.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7751693: forced go test JSON output and per-test Action=run/Test evidence replace plain ok-line recognition. Verbose TestMain/no-match fixture reproduced red before correction; focused '^TestFastCheck' suite reported PASS 13.907s, including dry-run fail-closed checks. Independent reviewer is examining escaped arbitrary output and actual event recognition, not accepting package summaries as proof. Exact-head CI/Codex still required.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7751693e4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| *'"Action":"run"'*) | ||
| case "${line}" in | ||
| *'"Test":"'*) matched_test=1 ;; |
There was a problem hiding this comment.
When FAST_TEST targets a nonexistent subtest, such as ^TestParent$/^NoSuch$, Go still emits an Action:"run" event for TestParent while it searches for the child, so this check sets matched_test=1 and reports success even though no complete test name matched. go help testflag explicitly notes that “possible parents of matches are run too”; require evidence that the full selector matched rather than accepting any ancestor run event.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced before implementation on 7751693: TestFastCheckRequiresCompleteSubtestSelector failed because parent-only events made ^TestFastCheckParent$/^MissingChild$ succeed. Fixed in 91cfbee with a stdlib JSON event helper that distinguishes complete selector matches from partial ancestors using pinned Go selector splitting/matching semantics. Implementer reports focused TestFastCheck PASS24.519s, helper package tests PASS0.439s, shellcheck/bash syntax/diff checks PASS; matrix includes real/missing children, bracket/group slashes and alternation, root/nested modules and flag/boundary controls. Independent exact-source review and final Codex/CI pending; no complete claim based only on parent execution.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91cfbeed0a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| line, err := reader.ReadString('\n') | ||
| if len(strings.TrimSpace(line)) != 0 { | ||
| var event testEvent | ||
| if json.Unmarshal([]byte(line), &event) == nil && event.Action == "run" && event.Test != "" && selector.matches(event.Test) { |
There was a problem hiding this comment.
Fresh evidence beyond the prior TestMain ok case: if TestMain prints === RUN NoSuchTest before calling m.Run, go test -json -run '^NoSuchTest$' converts that package-controlled line into an Action:"run", Test:"NoSuchTest" event, then exits successfully with [no tests to run]; this condition therefore makes make fast claim success although no compiled test matched. go help testflag defines -run in terms of matching actual test/example/fuzz identifiers, so a synthesized output event is insufficient evidence of execution.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-fix synthetic TestMain RUN regression failed. Changed evidence tracking to require a complete matched run followed by terminal pass, keyed by package/test; a synthetic run without a corresponding terminal pass cannot qualify. Candidate a78792d; focused verification selected 19 top-level tests plus 32 subtests using env GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 ./scripts -run '^(TestFastCheck|TestToolingGOFLAGSUsesGoEnvironment|TestToolingPinsNewerSystemGo)' (PASS, about31s), helper tests PASS. Independent delta review and exact-head CI/Codex remain pending.
| fail 'FAST_MODULE must resolve inside the current repository' | ||
| fi | ||
|
|
||
| if ! package_dirs="$(cd "${module_root}" && "${go_cmd}" list -json=false -f '{{.Dir}}' "${fast_package}")"; then |
There was a problem hiding this comment.
When a developer has GOFLAGS=-deps, this go list emits every dependency directory as well as the selected package, so the following module-boundary loop encounters standard-library directories outside module_root and rejects an otherwise valid FAST_PACKAGE. go help list explicitly says -deps makes list iterate over all dependencies; override it with -deps=false for this internal resolution query so inherited list formatting does not break make fast.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-fix inherited -deps package-resolution regression failed. Internal go list explicitly disables dependency expansion so stdlib directories do not masquerade as selected packages. Candidate a78792d; focused verification selected 19 top-level tests plus 32 subtests using env GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 ./scripts -run '^(TestFastCheck|TestToolingGOFLAGSUsesGoEnvironment|TestToolingPinsNewerSystemGo)' (PASS, about31s), helper tests PASS. Independent delta review and exact-head CI/Codex remain pending.
| override GOTOOLCHAIN := go1.26.8 | ||
| export GOTOOLCHAIN | ||
| export GOFLAGS | ||
| export FAST_MODULE FAST_PACKAGE FAST_TEST |
There was a problem hiding this comment.
When FAST_TEST is supplied using the documented environment-prefix form and contains a nonterminal $, GNU make imports it as a recursively expanded make variable and this export expands the $ reference before the script receives it. For example, FAST_TEST='^TestA$.*' make fast passes ^TestA*, which can run and accept TestB even though the requested selector only matched TestA; GNU make's variable-reference rules treat $x as a one-character variable reference. Preserve the raw environment value or require $$ escaping for environment assignments too.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Preserve raw FAST selector values with immediate Make assignments using $(value ...). The regression was already green on this host GNU Make3.81; no pre-fix red is claimed for this finding. Independent review will verify preservation and scope rather than infer resolution from staleness. Candidate a78792d; focused verification selected 19 top-level tests plus 32 subtests using env GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 ./scripts -run '^(TestFastCheck|TestToolingGOFLAGSUsesGoEnvironment|TestToolingPinsNewerSystemGo)' (PASS, about31s), helper tests PASS. Independent delta review and exact-head CI/Codex remain pending.
| if json.Unmarshal([]byte(line), &event) == nil && event.Action == "run" && event.Test != "" && selector.matches(event.Test) { | ||
| found = true |
There was a problem hiding this comment.
When the only test matching FAST_TEST calls t.Skip, Go emits an Action:"run" event followed by Action:"skip" and exits zero, so this predicate records a match and the script prints fast check passed. Track the terminal event for matched tests and reject a selector for which no matched test actually passes; otherwise a platform- or environment-gated skip is reported as passing evidence.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-fix selected-skip regression failed. Matched tests now require terminal pass; skip/fail terminal outcomes are not passing evidence. Ancestor-only matches still do not satisfy full selectors. Candidate a78792d; focused verification selected 19 top-level tests plus 32 subtests using env GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 ./scripts -run '^(TestFastCheck|TestToolingGOFLAGSUsesGoEnvironment|TestToolingPinsNewerSystemGo)' (PASS, about31s), helper tests PASS. Independent delta review and exact-head CI/Codex remain pending.
|
|
||
| set -euo pipefail | ||
|
|
||
| go_cmd="${GO:-go}" |
There was a problem hiding this comment.
When make fast is invoked with a repository-relative Go wrapper such as GO=./tools/go, this value is retained verbatim while the script changes into module_root and later scripts/fast-check-events. The root-module test command may run, but the event-checker invocation then looks for ./tools/go beneath the checker directory and fails; a nested FAST_MODULE fails as early as go list. Resolve slash-containing relative GO values against the original working directory before any cd, matching the override behavior exposed by the Make target.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pre-fix nested-module relative GO wrapper regression failed. Slash-containing relative GO overrides are anchored to original repository cwd before any module/helper directory change. Candidate a78792d; focused verification selected 19 top-level tests plus 32 subtests using env GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 ./scripts -run '^(TestFastCheck|TestToolingGOFLAGSUsesGoEnvironment|TestToolingPinsNewerSystemGo)' (PASS, about31s), helper tests PASS. Independent delta review and exact-head CI/Codex remain pending.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67aafbbb56
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| local value="$2" | ||
|
|
||
| [[ -n "${value}" ]] || fail "${name} is required; specify FAST_MODULE, FAST_PACKAGE, and FAST_TEST" | ||
| [[ "${value}" != *[[:space:]]* ]] || fail "${name} must not contain whitespace" |
There was a problem hiding this comment.
When FAST_TEST targets a subtest whose name contains whitespace, such as ^TestParent$/^hello world$, this rejects the selector even though go test -run accepts it and runs the sanitized TestParent/hello_world name. Checked go help testflag: -run accepts a regexp split only at unbracketed /, and Go's matcher rewrites whitespace to _—behavior this commit already mirrors in rewrite. Since the value is already passed as a quoted argument, exempt FAST_TEST from this path-oriented whitespace restriction so valid focused subtest checks work.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 7a560ef: whitespace rejection now applies only to FAST_MODULE/FAST_PACKAGE; FAST_TEST remains one quoted argument and follows Go subtest name rewriting. Independent immutable Luna/max probe FAST_TEST='^TestProbe$/^hello world$' exercised a real parallel hello_world subtest and exited 0; targeted tooling race tests passed 7.018s. Complete-selector no-match controls remain covered. Exact-head CI and Codex clean confirmed, but merge remains held for an unrelated independent synthetic terminal-sequencing correction.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a78792ddbe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "pass", "skip", "fail": | ||
| if result.ran && !result.terminal { | ||
| result.terminal = true | ||
| result.passed = event.Action == "pass" |
There was a problem hiding this comment.
Fresh evidence beyond the earlier synthetic-run finding: when TestMain prints both === RUN NoSuchTest and --- PASS: NoSuchTest (0.00s) before calling m.Run, go test -json -run '^NoSuchTest$' converts those package-controlled lines into matching run and pass events and exits zero, so this assignment makes make fast report success although no compiled test matched. go help testflag defines -run as running actual tests, examples, and fuzz tests matching the expression; require evidence that cannot be synthesized from test-process output before claiming the selected test passed.
AGENTS.md reference: AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Reproduced the ordinary unframed TestMain RUN/PASS false-green on a78792d. Correction 7a560ef forces pinned Go test2json framing and rejects ordinary unframed output. Independent Luna/max immutable delta review ran the targeted tooling race tests TestFastCheckRequiresCompleteSubtestSelector and TestFastCheckRejectsSpoofedTestMainEvents (PASS, 7.018s), plus framed parser/selector race tests (PASS, 1.460s). Exact-head CI all four passed and Codex clean at 2026-09-10T04:50:05Z. Deliberate control-byte spoofing remains explicitly outside trusted-helper authentication scope. A separate independent synthetic first-terminal regression is being corrected; no merge yet.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Checkpoint for 7a560ef: exact-head Codex clean verdict posted 2026-09-10T04:50:05Z; hosted run 34438095024 passed all four checks (Root/tooling 12m10s, offline 8m8s, vulnerability 27s, aggregate 3s). Independent Luna/max delta review passed framed-boundary/selector race tests 1.460s and two scoped tooling race tests 7.018s, including ordinary TestMain spoof and whitespace. It found a synthetic protocol-sequence robustness regression: framed RUN -> FAIL/SKIP -> PASS is accepted after first-terminal tracking was removed. No truthful pinned-Go false-green was demonstrated for that sequence; deliberate frame spoof remains outside trusted-helper authentication scope. Merge remains held while a minimal TDD correction restores per-run terminal tracking and preserves a fresh same-name RUN in another package. No full local suite repetition or live operation. New candidate will require fresh exact-head review/CI. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d18a1155c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| result := results[name] | ||
| switch action { | ||
| case "run": | ||
| result = testResult{ran: true} |
There was a problem hiding this comment.
When FAST_PACKAGE expands to multiple packages defining the same selected test name, an earlier package may pass and a later package may skip; this reset overwrites the recorded pass, so hasCompleteMatch returns false even though a selected compiled test passed and go test succeeded. A framed run/pass/run/skip sequence reproduces the false no-match result; retain completed successful lifecycles separately or key results by package/lifecycle.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed independently on immutable d18a115 with real pinned Go 1.26.8 output: a passing package followed by a same-name skipped test causes make fast to exit 2, while the reverse order passes. The per-RUN reset discards earlier successful evidence. A bounded TDD correction is assigned to retain aggregate success across lifecycles while preserving first-terminal handling, reader errors, and outer go-test failure propagation. Both package orders and pass-then-fail will be covered together. All current CI checks passed, but this open finding blocks merge; no resolution is claimed until the corrected head is independently reviewed and Codex-reviewed.
There was a problem hiding this comment.
Corrected in 2e1a555 with sticky passedAny aggregation independent of per-RUN terminal resets. Meaningful direct and real Go pass-then-skip regressions failed before implementation. Pinned Go1.26.8/GOWORK=off helper race tests passed 1.292s; focused tooling race tests passed 13.484s, including both package orders, pass-then-fail outer status, no-match, skipped tests and ordinary TestMain output. Reader error after successful event remains an error; no early pass return. Independent delta review and fresh exact-head Codex/CI remain pending, so this is fix evidence, not merge clearance.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2e1a555016
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
Final merge audit for exact head 2e1a555: independent Luna/max delta PASS plus independent framing-scope adjudication (reproduced limitation, evidence-based non-goal) complete. Fresh Codex clean verdict at 2026-09-10T06:00:19Z names this exact unique SHA prefix: #74 (comment) . All four CI checks passed on run34441028690 (root/tooling12m7s, offline6m56s, vulnerability31s, aggregate2s). Wrapper all/detail audit includes all17 historical/current findings across inline and issue-comment sources; 16 have reproduced-fix/compatibility evidence in their individual replies, and r3975882705 has independent reproduced-scope rebuttal r3975964922 followed by fresh exact-head clean verdict. The wrapper still counts the historical same-head finding; that count is not being mistaken for a missing review or an unresolved actionable defect. No staleness or untimestamped reaction is used as proof. No new source edits, full local suite repetition, live operations or authentication promise. Original G01 issue1/goal remains active; this PR completes only authorized issue73. Merge will be SHA-constrained. |
|
Post-merge integration verification: Public CI run 34443563704 completed successfully on exact merged main commit ae61375; all four jobs passed. This is separate from the pre-merge exact-candidate CI already recorded. Issue73/Project remain complete; G01 issue1 and PR72 work continue without live-operation authorization changes. |
1 participant
Merged outcome
Merged 2026-09-10T06:03:26Z as
ae6137530eb77455f42804d610f2a646a40c4e30, constrained to reviewed head2e1a555016dfeb081dcddce3bb88aea1acfd9f74. Issue #73 is closed and Project Done. Independent delta review and framing adjudication completed; fresh exact-head Codex clean verdict followed the documented evidence-based rebuttal. All four PR CI checks passed. Final finding audit. Main merged-tree CI is a separate integration check, not claimed complete here. G01 issue #1 and its active goal remain open.The following candidate and historical sections preserve pre-merge evidence; their pending-status statements are superseded by this outcome.
Current candidate status
Current head:
2e1a555016dfeb081dcddce3bb88aea1acfd9f74. Older sections below are historical source-specific evidence, not final-head attestations.env GOTOOLCHAIN=go1.26.8 GOWORK=off GOFLAGS='' go test -race -count=1 ./scripts/fast-check-eventsPASS 1.292s.env GOTOOLCHAIN=go1.26.8 GOWORK=off GOFLAGS='' go test -race -count=1 ./scripts -run '^(TestFastCheckAcceptsPassingSameNameAcrossPackages|TestFastCheckRequiresCompleteSubtestSelector|TestFastCheckRejectsSpoofedTestMainEvents|TestFastCheckRejectsSkippedSelectedTest|TestFastCheckIgnoresTestMainOutputForMatchEvidence)$'PASS 13.484s. Changed-file format/diff checks pass.env GOTOOLCHAIN=go1.26.8 GOWORK=off GOFLAGS='' go test -race -count=1 -run '^TestHasCompleteMatchPropagatesReaderError$' .from the immutable helper directory PASS 1.359s. No duplicated full root/G01/G02 suite.Historical implementation and review evidence
Outcome
Closes #73
Adds an opt-in, fail-closed
make fastselector and documents incremental validation/review tiers. The completemake checkcontract and hosted Public CI matrix remain unchanged.Evidence
GOTOOLCHAIN=go1.26.8 go test ./scripts -run '^TestFastCheckRequiresExplicitSelectors$' -count=1failed because the newscripts/fast-check.shentry point was absent; this was the expected missing behavior, not a fabricated product result.GOTOOLCHAIN=go1.26.8 go test ./scripts -run '^TestFastCheck' -count=1passed, including missing selectors, invalid paths/regexp, no-match selectors, root/nested modules and package-pattern selection.shellcheck scripts/fast-check.sh;make fmt-check;git diff --check;GOTOOLCHAIN=go1.26.8 go test ./scripts -run '^(TestFastCheck|TestPublicWorkflowCapacityContract|TestToolingCheckRequiresExecutableLink)$' -count=1all passed.docs/CI.md: mainorigin/mainatcf67d4a, PR feat(g01): observe bounded idle drain ordering #72's bounded review-history snapshot (review records do not establish actor-side full-suite counts), hosted run 34419651240's 11m12s start-to-aggregator completion (11m13s is only the metadata-update interval), and focused direct-vs-make fastresults (0.32s vs 0.34s; no speedup claim).make checkwas not run locally per issue instructions; hosted CI is the stable candidate full gate.Operational effect
No production runtime, dependency, CI workflow, cache policy, runner, credential, live GitHub, or release/live operation changes. Rollback is a normal revert of this PR; removing the
fasttarget/script and associated docs/tests restores the prior workflow whilemake checkremains the complete gate.Incremental review ledger
7751693e4b3bcd2f004946a6638c893c5cd79074; implementation corrections:543b147dab9985e51066ca23d68633f72937b880; initial reviewed baseline:e6470f64035cb33da605b489e439371d9f2df468.7751693e4b3bcd2f004946a6638c893c5cd79074; exact-head CI/Codex completion remains pending.Latest correction verification
2ba63c4635b6909c0200086bd3f03cac5943c756resolves general Go package patterns using physically bounded matched directories and overrides list-only GOFLAGS during test execution.env GOTOOLCHAIN=go1.26.8 GOWORK=off go test -v ./scripts -count=1 -run '^TestFastCheck'PASS, 8.041s; all seven focused tests ran. List-only selected-failure/no-match regression PASS, 1.202s. Combined JSON/list/race no-match probe correctly exits 2. Real wildcard-selected test succeeds under all five tested GOFLAGS combinations. Shellcheck and diff checks pass; no full root/G01/G02/make check rerun.^(TestFast|TestToolingGOFLAGSUsesGoEnvironment)$selected only the tooling-environment test, not TestFastCheck tests. It is not counted as focused coverage; the independent run above supplies that evidence.Execution-flag correction and recovery
647c53cec9d9114e8b789587538792a396dbb34dadds deterministic benchmark/fuzz regressions and clears execution-expanding/suppressing flags while preserving useful build/race flags. Pre-fix focused regression failed as expected on2ba63c4; implementer reports exact-source focused race tests PASS 13.715s usinggo test ./scripts -race -run '^TestFastCheck' -count=1with pinned toolchain, workspace off and outer GOFLAGS empty. Shellcheck, gofmt, bash syntax and diff checks pass.9741ce8) after pushing the fix. A normal revert-of-revert produced finale7c8defd383fbebe44cbcf37d690c2418188f4a3. Coordinator verified the complete tree is byte-identical to tested647c53c; no history rewrite or live changes occurred. Tests were not duplicated for identical restored source. Final independent delta review and exact-head CI/Codex remain pending.Current candidate verification (7751693)
GOTOOLCHAIN=go1.26.8 GOWORK=off go test -count=1 -run '^TestFastCheck' ./scripts.7751693e4b3bcd2f004946a6638c893c5cd79074finds no actionable defect: exec/CPU/TestMain/race-preservation tests PASS 3.367s; root/nested invocation PASS 1.970s; JSON/list compatibility PASS 3.240s. Separate JSON-looking TestMain output with missing selector correctly exits 2; deliberately failing selected test runs exactly once with inherited count=7,cpu=1,2,exec=true. Shellcheck/diff checks pass. No full root/G01/G02/make check repeat.