From e5f93033ebadbc81b8326515a9beb20aaad98c06 Mon Sep 17 00:00:00 2001 From: Remylus Losius Date: Mon, 21 Sep 2026 13:55:27 -0400 Subject: [PATCH 1/2] ci(go): run internal/server in its own test invocation, alone internal/server is the database-heavy suite. On the hosted runner it took 794 s on main a5a05056 and timed out at the 900 s per-package ceiling on two PRs with no failing test: the same tests, running 27% to 48% slower at the median, while sharing the PostgreSQL service with three sibling packages under -p 4. A rerun of the same head then passed in 314 s. The cause of the variance is not confirmed; the margin is (CP bugs/OW-066). The test step is now two invocations inside one step, every package exactly once: phase 1 runs every package except internal/server under the shared 900 s ceiling, so a hang anywhere else still fails at 15 minutes; phase 2 runs internal/server alone. This commit keeps phase 2 at 900 s on purpose, to measure isolation on its own before any budget change. Both exit statuses are preserved (a green phase 2 never masks a red phase 1; verified for all four combinations with a stub go), both JSON streams are written and uploaded on every outcome, specter ingests both, and a package-count guard fails the step if the split ever stops closing. The job declares timeout-minutes 60 instead of relying on the 360-minute platform default: two ceilings plus the measured 5 to 8 minutes of build, vet, lint, vuln, specter, vitest and upload. release-ci-gates 1.18.1 restates AC-09 for the two-invocation shape and its test pins the exclusion, the separate invocation with its own -timeout, both ingest paths, the second upload path and the explicit job budget; removing the second ingest path turns it red. The workflows README describes the same. The detect-secrets baseline is the hook's own line-number refresh. CP: bugs/OW-066 --- .github/workflows/README.md | 15 +++++--- .github/workflows/go-ci.yml | 59 ++++++++++++++++++++++++-------- .secrets.baseline | 6 ++-- packaging/tests/ci_gates_test.go | 23 +++++++++++-- specs/release/ci-gates.spec.yaml | 19 ++++++++-- 5 files changed, 95 insertions(+), 27 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 72476b6d..89498800 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -40,11 +40,16 @@ container and executes: - `make vet` - `make lint` (golangci-lint, built from source to match the runner toolchain) - `make vuln` (govulncheck) -- a single `go test -race -json -timeout 600s -p 4 ./...` run: the race detector - plus the full integration suite against PostgreSQL, emitting the JSON that - `specter` ingests. This is one pass, not two. It replaced the former separate - `make test-race` + non-race `go test -json` runs (which walked the DB-bound - suite twice) +- the `go test -race -json` run: the race detector plus the full integration + suite against PostgreSQL, emitting the JSON that `specter` ingests. Every + package runs exactly once, in two invocations: everything except + `internal/server` under the shared 900 s per-package budget, then + `internal/server` alone under its own budget, so the database-heavy suite + does not compete with sibling packages for the PostgreSQL service and a + hang elsewhere still fails at the shared budget. Both exit statuses and + both JSON streams are kept. It replaced the former separate `make + test-race` + non-race `go test -json` runs (which walked the DB-bound suite + twice) - frontend `vitest` (JUnit), also ingested by `specter` for spec AC coverage - `specter sync` to enforce coverage thresholds diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index 8c79796b..519615c0 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -1,9 +1,10 @@ # Pre-merge gates for OpenWatch (repo root). # # Runs: make vet, make lint (staticcheck + gosec + ...), make vuln -# (govulncheck), a single `go test -race -json` run (data races + full +# (govulncheck), the `go test -race -json` run (data races + full # integration suite against a Postgres service container, AND the JSON -# specter ingests — one pass, not two), and `specter sync` (spec +# specter ingests; every package once, in two invocations so that +# internal/server runs alone, CP bugs/OW-066), and `specter sync` (spec # validation + strict AC coverage). # # Spec: specs/release/ci-gates.spec.yaml. @@ -29,6 +30,13 @@ jobs: gates: name: Quality + security gates runs-on: ubuntu-latest + # Explicit budget (CP bugs/OW-066), not the 360-minute platform default: + # the shared test phase can run to its 900 s per-package ceiling, the + # internal/server phase to its own ceiling, and the rest of the job + # (build, vet, lint, vuln, specter, vitest, upload) has measured at 5 to + # 8 minutes. 60 minutes covers both ceilings plus that with margin and + # still fails a job that has wedged. + timeout-minutes: 60 services: postgres: @@ -288,21 +296,41 @@ jobs: # ships inside the kensa module, so api-system-scan-config/AC-08 runs # here instead of skipping (100% outcome coverage requires it). export OPENWATCH_KENSA_RULES_DIR="$(go list -m -f '{{.Dir}}' github.com/Hanalyx/kensa)/rules" - # -timeout is per package. internal/server is the heavy suite (~470s - # without -race) and rides near the old 600s limit under -race + -p 4 - # DB contention, so it intermittently times out (e.g. main run #976). - # 900s gives headroom without masking a genuine hang. - go test -race -json -timeout 900s -p 4 ./... > /tmp/go-test.json - EXIT=$? - if [ "$EXIT" -ne 0 ]; then - echo "::group::Failed tests" - jq -r 'select(.Action=="fail" and .Test != null) | "FAIL \(.Package) \(.Test)"' /tmp/go-test.json | sort -u + # -timeout is per package. Two invocations, every package exactly + # once (CP bugs/OW-066). internal/server is the database-heavy suite: + # on the hosted runner it ran 794 s on main and timed out at 900 s on + # two PRs with no failing test, the same tests simply running 27% to + # 48% slower while sharing the PostgreSQL service with three sibling + # packages under -p 4. Phase 1 runs everything else under the shared + # 900 s ceiling, so a hang anywhere else still fails at 15 minutes. + # Phase 2 runs internal/server alone. Both exit statuses are kept: + # a green phase 2 never masks a red phase 1. Both JSON streams are + # kept for specter ingest and the artifact upload. + report() { + echo "::group::Failed tests ($1)" + jq -r 'select(.Action=="fail" and .Test != null) | "FAIL \(.Package) \(.Test)"' "$2" | sort -u echo "::endgroup::" - echo "::group::Failure output (last 80 lines)" - jq -r 'select(.Action=="output" and (.Output | ascii_downcase | test("--- fail|panic:|fatal error|error:"))) | "\(.Package): \(.Output)"' /tmp/go-test.json | tail -80 + echo "::group::Failure output ($1, last 80 lines)" + jq -r 'select(.Action=="output" and (.Output | ascii_downcase | test("--- fail|panic:|fatal error|error:"))) | "\(.Package): \(.Output)"' "$2" | tail -80 echo "::endgroup::" + } + ALL=$(go list ./...) + OTHERS=$(printf '%s\n' "$ALL" | grep -v '^github.com/Hanalyx/openwatch/internal/server$') + # The arithmetic has to close, or a package could be tested twice + # or not at all without anyone noticing. + if [ "$(printf '%s\n' "$ALL" | wc -l)" -ne $(( $(printf '%s\n' "$OTHERS" | wc -l) + 1 )) ]; then + echo "::error::package split does not close: internal/server must be exactly one package of $(printf '%s\n' "$ALL" | wc -l)" + exit 1 fi - exit $EXIT + # shellcheck disable=SC2086 + go test -race -json -timeout 900s -p 4 $OTHERS > /tmp/go-test.json + EXIT1=$? + [ "$EXIT1" -ne 0 ] && report "phase 1: every package except internal/server" /tmp/go-test.json + go test -race -json -timeout 900s ./internal/server/ > /tmp/go-test-server.json + EXIT2=$? + [ "$EXIT2" -ne 0 ] && report "phase 2: internal/server" /tmp/go-test-server.json + echo "phase 1 exit $EXIT1, phase 2 exit $EXIT2" + [ "$EXIT1" -eq 0 ] && [ "$EXIT2" -eq 0 ] # Frontend Vitest run for specter ingest. The specs/frontend/ specs # reference tests in frontend/tests/pages/*. Without these JUnit @@ -342,7 +370,7 @@ jobs: - name: specter ingest if: steps.paths.outputs.go == 'true' - run: specter ingest --go-test /tmp/go-test.json --junit /tmp/vitest-junit.xml + run: specter ingest --go-test /tmp/go-test.json --go-test /tmp/go-test-server.json --junit /tmp/vitest-junit.xml - name: specter sync (AC coverage thresholds) if: steps.paths.outputs.go == 'true' @@ -378,6 +406,7 @@ jobs: path: | .specter-results.json /tmp/go-test.json + /tmp/go-test-server.json /tmp/vitest-junit.xml retention-days: 7 diff --git a/.secrets.baseline b/.secrets.baseline index ebd9373f..7e759472 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -133,14 +133,14 @@ "filename": ".github/workflows/go-ci.yml", "hashed_secret": "0e4f7f719374de1c33da2c0e5c0db5bcdc2e15d4", "is_verified": false, - "line_number": 38 + "line_number": 46 }, { "type": "Basic Auth Credentials", "filename": ".github/workflows/go-ci.yml", "hashed_secret": "0e4f7f719374de1c33da2c0e5c0db5bcdc2e15d4", "is_verified": false, - "line_number": 53 + "line_number": 61 } ], "api/openapi.yaml": [ @@ -818,5 +818,5 @@ } ] }, - "generated_at": "2026-09-19T22:42:43Z" + "generated_at": "2026-09-21T17:55:15Z" } diff --git a/packaging/tests/ci_gates_test.go b/packaging/tests/ci_gates_test.go index ea7f4e25..6db3ff53 100644 --- a/packaging/tests/ci_gates_test.go +++ b/packaging/tests/ci_gates_test.go @@ -206,11 +206,30 @@ func TestCIGates_WorkflowRunsAllGates(t *testing.T) { t.Errorf("workflow missing step that runs %q", g) } } - // The single race+coverage run must still detect data races AND - // emit JSON for specter ingest. + // The race+coverage run must still detect data races AND emit JSON + // for specter ingest. if !strings.Contains(wf, "go test -race") || !strings.Contains(wf, "-json") { t.Error("workflow missing the race+JSON test run (`go test -race ... -json`) — race detection must still gate") } + // v1.18.1: two invocations, every package once. The first excludes + // internal/server, the second runs it alone with its own budget, + // both streams are ingested and uploaded, and the job budget is + // explicit rather than the platform default. + if !strings.Contains(wf, "grep -v '^github.com/Hanalyx/openwatch/internal/server$'") { + t.Error("first go test invocation must exclude internal/server so the package runs once, in its own phase") + } + if !regexp.MustCompile(`go test -race -json -timeout \d+s ./internal/server/`).MatchString(wf) { + t.Error("internal/server must run in its own go test invocation with its own -timeout") + } + for _, want := range []string{ + "--go-test /tmp/go-test.json --go-test /tmp/go-test-server.json", + "/tmp/go-test-server.json\n", + "timeout-minutes:", + } { + if !strings.Contains(wf, want) { + t.Errorf("workflow missing %q: both JSON streams must be ingested and uploaded, and the job budget must be explicit", want) + } + } }) } diff --git a/specs/release/ci-gates.spec.yaml b/specs/release/ci-gates.spec.yaml index a29a10a3..dea36952 100644 --- a/specs/release/ci-gates.spec.yaml +++ b/specs/release/ci-gates.spec.yaml @@ -1,7 +1,7 @@ spec: id: release-ci-gates title: CI quality + security gates - version: "1.18.0" + version: "1.18.1" status: approved tier: 1 @@ -388,7 +388,22 @@ spec: priority: critical references_constraints: [C-06] - id: AC-09 - description: The workflow executes (in order) make vet, make lint, make vuln, a single race-detector + JSON test run (`go test -race -json`, which both detects data races and feeds specter ingest — replacing the former separate `make test-race` + non-race json passes), and specter sync — each as its own step so a failure pinpoints which gate broke. + description: > + The workflow executes (in order) make vet, make lint, make vuln, the + race-detector + JSON test run, and specter sync — each as its own step + so a failure pinpoints which gate broke. The test run is `go test -race + -json` (it both detects data races and feeds specter ingest, replacing + the former separate `make test-race` + non-race json passes). v1.18.1 + (CP bugs/OW-066) — it is two invocations inside one step, not one, so + that every package is still tested exactly once under -race: the first + covers every package except internal/server under the shared per-package + budget; the second runs internal/server alone under its own, so the + database-heavy suite no longer competes with sibling packages for the + PostgreSQL service and a hang elsewhere still fails at the shared + budget. Both exit statuses are preserved (a later success never masks an + earlier failure), both JSON streams are kept and uploaded on every + outcome, specter ingests both, and the job declares an explicit timeout- + minutes that covers both phases plus the artifact upload. priority: critical references_constraints: [C-06] - id: AC-10 From a1841d49fffa6ee1a87be7cb8e7495427e8d81dd Mon Sep 17 00:00:00 2001 From: Remylus Losius Date: Mon, 21 Sep 2026 14:25:39 -0400 Subject: [PATCH 2/2] ci(go): give the isolated internal/server test run an 1800 s budget The preliminary run (35635263290, revision e5f93033) isolated internal/server at the old 900 s ceiling and passed at 885 s: 442 tests, median per-test ratio 1.00 against the contended base run on main, so isolation alone does not restore the margin. Across five hosted runs of the same inventory the package has ranged from 314 s to more than 900 s. The budget is the operative change. The internal/server invocation now runs under 1800 s: base 794 s times the worst observed slowdown 1.63 is 1294 s, with room for one more such excursion. Every other package keeps 900 s, so a hang elsewhere still fails at 15 minutes; a hang in internal/server fails at 30, accepted by the founder as the cost. The job budget of 60 minutes already covers both ceilings. Nothing else changes; this is the revision the three validation runs measure. CP: bugs/OW-066 --- .github/workflows/README.md | 2 +- .github/workflows/go-ci.yml | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 89498800..7762f6aa 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -44,7 +44,7 @@ container and executes: suite against PostgreSQL, emitting the JSON that `specter` ingests. Every package runs exactly once, in two invocations: everything except `internal/server` under the shared 900 s per-package budget, then - `internal/server` alone under its own budget, so the database-heavy suite + `internal/server` alone under its own 1800 s budget, so the database-heavy suite does not compete with sibling packages for the PostgreSQL service and a hang elsewhere still fails at the shared budget. Both exit statuses and both JSON streams are kept. It replaced the former separate `make diff --git a/.github/workflows/go-ci.yml b/.github/workflows/go-ci.yml index 519615c0..5c3b7d58 100644 --- a/.github/workflows/go-ci.yml +++ b/.github/workflows/go-ci.yml @@ -303,7 +303,13 @@ jobs: # 48% slower while sharing the PostgreSQL service with three sibling # packages under -p 4. Phase 1 runs everything else under the shared # 900 s ceiling, so a hang anywhere else still fails at 15 minutes. - # Phase 2 runs internal/server alone. Both exit statuses are kept: + # Phase 2 runs internal/server alone under 1800 s. Isolation on its + # own measured 885 s (run 35635263290): the same tests at the same + # per-test speed as the contended base, so the budget is what closes + # the margin. 1800 s is sized from evidence, base 794 s times the + # worst observed slowdown 1.63 is 1294 s, and doubles the time to + # detect a hang in this one package, which the founder accepted. + # Both exit statuses are kept: # a green phase 2 never masks a red phase 1. Both JSON streams are # kept for specter ingest and the artifact upload. report() { @@ -326,7 +332,7 @@ jobs: go test -race -json -timeout 900s -p 4 $OTHERS > /tmp/go-test.json EXIT1=$? [ "$EXIT1" -ne 0 ] && report "phase 1: every package except internal/server" /tmp/go-test.json - go test -race -json -timeout 900s ./internal/server/ > /tmp/go-test-server.json + go test -race -json -timeout 1800s ./internal/server/ > /tmp/go-test-server.json EXIT2=$? [ "$EXIT2" -ne 0 ] && report "phase 2: internal/server" /tmp/go-test-server.json echo "phase 1 exit $EXIT1, phase 2 exit $EXIT2"