PR Workflow

Pre-push gate

The local pre-push gate (scripts/pre-push-gate.sh, run automatically by the pre-push git hook) is a deterministic, fast, entirely BLOCKING set of checks. Two rules define what belongs in it (#2983). First, no advisory step: every check in the default path either blocks the push or is not in that path at all, so the closing "All hard checks passed" means every check that ran, passed. Second, a check is only eligible to leave the gate if a REQUIRED CI check covers it -- a job that is not in the Protect main ruleset can go red and the PR merges anyway (#2503), so "CI does something similar" never justifies dropping a local check. scripts/check-gate-invariant.sh enforces the first rule mechanically, from inside the gate and from CI's Gate Invariant job. The pre-push hook skips the gate when a receipt proves the same gate already passed for exactly what is being pushed (#3436), so a push that went through the orchestrate gate runner does not pay for the gate twice. The skip prints one line naming the pushed commit, its tree, the receipt's commit, and the receipt's age; when a receipt exists but is not honored, one line says why and the gate runs as usual. All of these must hold: <git dir>/prep-pr-receipt.json parses, has schema gate-receipt/v1, producer gate-runner and result pass; its tree_sha equals the tree of every commit being pushed (read from the hook's stdin, never assumed to be HEAD; a branch delete, a push whose source is a tag or a non-commit object, or no refs runs the gate, and the destination ref matters only for the object-id form; the full object id that safe-push pushes as the local ref counts as a branch push only when it equals the line's sha and the destination is refs/heads/*); the working tree is clean (untracked files count as dirty); and the stamp the gate writes on a passing run (<git dir>/pre-push-gate-stamp.json) records the same tree, the merge-base with main of each pushed commit (a rebase moves it), the same tip of local main (any move of main re-runs the gate, because one gate step diffs against it), the same patch-coverage base (patch coverage is always measured against the origin/main-first base, which the gate passes to the helper explicitly, so an inherited BASE cannot change it; a fetch plus rebase that leaves the tree and local main alone still re-runs the gate), and RUN_* modes at least as strict as the push's own (a gate run with RUN_RACE=0 never satisfies a default push). The gate writes the stamp only when it can show nothing was skipped for a reason the stamp does not record. Skip arms in the gate and its child helpers print through gate_skip (scripts/lib/gate-stamp.sh; the few exceptions are listed in the test) and are classified deterministic (the same tree, the same base, and the same modes always skip it: no Go changes, a file absent from the tree, a RUN_* mode) or blocking (it depends on the host: SKIP_OPENAPI_BREAKING, a missing python3, PyYAML or tomllib, a missing tailwindcss in scripts/check-generated.sh, an unreadable file in scripts/prefs-coverage.py). A blocking skip appends a line to a blocker file (GATE_STAMP_BLOCK_FILE), which works from child processes and subshells, and any line there means no stamp; one line names what blocked it. The stamp is withheld when any of a known list of verdict-changing environment variables is set at gate start, or in the environment of the push that would skip the gate (BASE, any SKIP_*, GOFLAGS, GOTOOLCHAIN, GOEXPERIMENT, GOOS, GOARCH, CGO_ENABLED, GOWORK, NODE_OPTIONS, PATCH_COVERAGE_ALLOW_DIRTY, SW_TEST_URL, SW_BINARY, SW_BASE, SW_PORT; the list is in scripts/lib/gate-stamp.sh and is a known list, not a proof). It is also withheld when the tree was dirty at the start or the end of the run, when HEAD's tree changed, or when local main or the patch-coverage base moved during it. scripts/test-gate-receipt-valid.sh fails when a skip-style echo, printf or print line (one mentioning skip or warn) appears in the gate, in the hook, or in a helper it runs without going through gate_skip or being listed there with a reason. That is a text heuristic: a skip that prints no such word, one built from a variable, or one inside a helper outside the repository is not caught and needs review. A stamp older than 4 hours (its created epoch is recorded at write; a missing, non-numeric or future-dated value is refused too) is not honored, which bounds what no list can cover: tool versions, the out-of-repository patch-coverage helper, the vulnerability database. Any missing, malformed or mismatched file, or a missing jq, runs the gate, as does a validator that crashes or exits 0 without printing its skip line. PRE_PUSH_FORCE_GATE=1 git push forces the full gate; an unrecognized value is refused, like the RUN_* flags, and blocks the push. Only that refusal blocks; a crash of the decision script runs the gate, never skips it. The decision lives in scripts/gate-receipt-valid.sh, tested by scripts/test-gate-receipt-valid.sh.

The local test step is a fast, changed-packages-only, non-race run (no -race, no -coverpkg=./...): a quick "did I obviously break a test" signal that also produces the coverage profile the patch-coverage step consumes. It runs the EXACT packages whose files changed, not their /... subtrees -- an edit under internal/provider/ no longer drags in all twelve adapter subpackages, which are what CI's sharded Test job is for. Every failure mode of this step blocks, including an ordinary failing assertion: the reason to keep a test run local at all is that a break caught here costs a re-run, the same break caught in CI costs a red check plus a re-push, and caught by a reviewer costs an entire review round -- an advisory local failure pays the full cost of the run and then discards its only output. A failure that prevents the changed packages from compiling gets a different message, but blocks identically -- both cases exit 1. The message is a best-effort HINT only: it is chosen from whether a coverage profile was produced, and that signal is not reliable (on Go 1.26 a deliberate syntax error produced [build failed] and a non-empty profile). Read the test output for the real cause; the gate never routes on that signal. Because the default profile lacks -coverpkg=./..., a package covered mainly by other packages' integration tests can read lower locally than CI on the patch-coverage check -- a false-positive block, never a false-negative. Set RUN_RACE=1 git push (the pre-push hook inherits the variable from the push) for the full, CI-equivalent -race -coverpkg=./... run, or RUN_RACE=0 to skip the test run and patch-coverage check together (the deliberate escape hatch when the changed package's own suite is expensive -- internal/api alone is ~160s locally). The opt-in/opt-out accepts any of 1/true/yes/on or 0/false/no/off (case-insensitive). Anything else -- a typo such as RUN_RACE=truee -- is refused outright: the gate exits 2 naming the variable and the value, rather than treating an unrecognized value as "unset" and silently skipping a tier you asked to run.

The accessibility (axe-core) tier, the provider-failure smoke test (scripts/smoke-provider-failure.sh), and govulncheck are skipped by default and available as blocking opt-ins: RUN_A11Y=1, RUN_PROVIDER_SMOKE=1, RUN_VULN=1 (same three-state 1/true/yes/on / 0/false/no/off convention as RUN_RACE). All three are integration-shaped -- they boot a server, drive a browser, or download the vulnerability database -- and each duplicates a REQUIRED CI check: A11y Smoke Tests (Playwright + axe-core) (ci.yml), Provider Failure Smoke (gate.yml), and Go Vulnerability Check (security.yml), all three configured as required status checks in the Protect main branch-protection ruleset (that requirement lives in the ruleset, not in-repo, so it can differ on a fork). Until #2983 each auto-ran on a Go-relevant diff and then treated a failure as advisory, which is the pattern the gate no longer contains: the a11y tier in particular cost ~2.4 minutes to produce a verdict the gate itself declared non-authoritative. Bruno route parity left the local gate entirely for the same reason, and is now owned by the required Bruno Route Parity job (gate.yml), which runs the same scripts/check-bruno-parity.sh.

When a change warrants the local run, take it: run RUN_A11Y=1 on a round that touches templates, CSS, or tests/a11y/, and RUN_PROVIDER_SMOKE=1 on one that touches provider failure handling. Skipped-by-default is a statement about the DEFAULT diff, not a claim that CI catching it is free -- CI catching it costs a red check and a re-push.

The UI-preference coverage step (scripts/prefs-coverage.py, backed by .prefs.toml) is Layer 1 only, and it is REGRESSION-ONLY: for a changed surface file that matches a tracked preference's surface glob, it flags a failure only when the file referenced the preference's driving token/class at the base revision and no longer does at HEAD. It does not require every file matching a (possibly broad) surface glob to carry every preference's token -- a file that never carried it, including a brand-new file, is never flagged. It does not catch a CSS cascade-override (a more specific rule beating a preference-driven variable) -- confirming that requires rendered evidence (a computed-style assertion against the live page), which is out of scope for this static, diff-scoped check.

Then run /pr-review-toolkit:review-pr for code review. Fix all critical/important findings before pushing.

Parallel pushes from sibling worktrees

Multiple agents pushing concurrently from sibling worktrees (../stillwater-<slug>/) is supported. The gate path is fully per-worktree isolated:

• $SW_RUN_DIR (which holds cover.out and openapi-base.yaml) keys off the worktree's basename plus a 12-char sha256 of its absolute path -- see scripts/lib/run-paths.sh. Two worktrees that share a basename across separate parent directories still get disjoint dirs.
• The pre-push gate takes an atomic mkdir-based lock at $SW_RUN_DIR/.gate-lock to block same-worktree concurrent runs (PR #1481). Sibling worktrees take separate locks.
• golangci-lint's shared cache is safe under allow-parallel-runners: true in .golangci.yml.
• Go's build cache ($GOCACHE) is documented concurrent-safe.

What is not robust is the common invocation pattern that hides push failures:

git push -u origin "$branch" 2>&1 | tail -30   # AVOID

Without set -o pipefail, the pipeline returns tail's exit code (always 0), masking a real git push failure -- transient SSH blip, remote ref rejection, hook abort, network drop. The visible output then looks identical to a quiet success, and the next step opens a PR against a branch that never reached the remote.

Use scripts/safe-push.sh instead:

bash scripts/safe-push.sh                  # push current branch -u origin
bash scripts/safe-push.sh "$branch"        # push named branch
bash scripts/safe-push.sh "$branch" --force-with-lease   # extra flags forwarded

It writes the full push transcript to <git-dir>/safe-push.log (.git/safe-push.log for the main worktree, or the worktree-specific .git/worktrees/<name>/safe-push.log for linked worktrees), with mode 0600 so the transcript is private to the current user. The wrapper then queries git ls-remote origin <branch> after the push returns and exits non-zero with a clear message if the remote SHA does not match the local HEAD. Catches silent failures that cmd | tail would have swallowed.

Squash before first push

Squash all development/fixup commits into clean, logical commits before the first push. Copilot's initial review covers the full diff present when the PR is first opened; incremental commits added after opening are not automatically re-reviewed (see Copilot review policy below).

git rebase -i main
# mark first: "pick", rest: "squash" or "fixup"

For larger PRs, two or three coherent commits is fine. Do not squash after opening the PR -- it resets Copilot's diff window.

Creating issues

When creating a GitHub issue with gh issue create:

1. Pick the right template from .github/ISSUE_TEMPLATE/: feature.md, bug.md, or task.md.
2. Read the template and fill in every section, including the [mode:], [model:], and [effort:] hints.
3. Write the populated body to a temp file, then:

   gh issue create --title "<title>" --body-file <path> --label <label>
   

4. Delete the temp file after creation.

PR templates

Two PR templates are available:

• Default (.github/pull_request_template.md) -- for feature, bug, and user-visible change PRs. Applied automatically when opening a PR via gh pr create with no --body/--body-file flag, or via the GitHub compare URL without a ?template= parameter.
• Chore (.github/PULL_REQUEST_TEMPLATE/chore.md) -- for chore, CI, refactor, and dependency PRs. Omits screenshot, UAT, OpenAPI, and templ generate rows. Select it with:

# gh pr create: pass the template file as the body
gh pr create --body-file .github/PULL_REQUEST_TEMPLATE/chore.md --label chore

In a browser, append ?template=chore.md&expand=1 to the compare URL:

https://github.com/sydlexius/stillwater/compare/main...<branch>?expand=1&template=chore.md

Reading PR comments (gh API)

The ! character triggers bash history expansion inside double quotes. Never use != in --jq expressions. Use select(.field == "value" | not) instead:

# List all PR review comments:
gh api "repos/{owner}/{repo}/pulls/{number}/comments" --paginate \
  --jq '[.[] | select(.body | length > 0) | {id, user: .user.login, path, line, body}]'

# Filter out a specific user:
gh api "repos/{owner}/{repo}/pulls/{number}/comments" --paginate \
  --jq '[.[] | select(.user.login == "some-bot" | not) | {id, user: .user.login, body}]'

# Reply to a review comment:
gh api "repos/{owner}/{repo}/pulls/{number}/comments/{comment_id}/replies" \
  -f body='Fixed in <commit>.'

Copilot review policy

Copilot reviews a PR once, automatically, when it is opened. That single round is all the automation there is, by design.

The "Copilot auto-review" ruleset sets review_on_push: false, so pushing a fix round does not trigger another review. To get one, re-request Copilot by hand from the PR page's Reviewers panel.

A copilot-re-review.yml workflow used to defeat this: it fired on every synchronize event and re-requested a review whenever Copilot had already left one, so a PR taking three fix rounds collected three extra reviews nobody asked for. It was removed. Note that the API call it made does work -- re-requesting copilot-pull-request-reviewer[bot] via POST repos/{owner}/{repo}/pulls/{number}/requested_reviewers succeeds, contrary to an earlier note here claiming a 422. Being possible is not the same as being wanted: one automatic round is the policy, and further rounds are a deliberate manual choice.

Dependabot templ bumps regenerate themselves

Every committed *_templ.go file records the templ version that generated it, and the Generated Files and Lint Check jobs fail when regenerating changes anything. Dependabot updates go.mod and go.sum but cannot run generators, so a github.com/a-h/templ bump would otherwise stay red until someone regenerated by hand (#3503).

dependabot-regenerate.yml closes that gap. On a Dependabot PR that changes the templ version, one job with a read-only token runs go tool templ generate and uploads the changed files. A second job, which runs no repository code, accepts only web/**/*_templ.go files that already exist on the branch and commits them to the PR through the GitHub API. GitHub signs that commit, so Signed Commits passes, and the commit carries a user identity, so the checks on it start without manual approval. Ordinary Go bumps, and any head commit Dependabot did not author, are no-ops. Once the regenerated commit lands, Dependabot stops rebasing that PR on its own, so a later merge conflict needs a human to comment @dependabot recreate; the workflow then runs again on the recreated PR.

One-time setup: the workflow reads DEPENDABOT_UPDATE_TOKEN from the Dependabot secret store, which is separate from the Actions store that dependabot-merge.yml reads. Store the same fine-grained token in both: gh secret set DEPENDABOT_UPDATE_TOKEN --app dependabot --repo sydlexius/stillwater. Without it, the commit job fails with that command in its error.

If the regenerated output also changes rendered HTML (templ release notes sometimes change whitespace handling), the golden-file tests in web/templates fail on the PR. Regenerate them with the update flag named in each test file's header (for example -update or -update-platform-state-golden) and push that as a separate commit.

Stacked PRs run full CI (#3002)

Every workflow with a pull_request trigger runs with no branches: filter, so a PR whose base is another feature branch (a stacked PR) runs the same checks as a PR to main. push triggers stay restricted to main. Exempt, and still restricted to main: dependabot-auto-approve.yml (an approval path) and pages.yml (docs-site build, not a required check); the pull_request_target workflows (privileged token) are out of scope. scripts/check-pr-trigger-scope.sh scans every pull_request workflow and fails the gate, and the Gate Invariant CI job, if one restricts its base branch without being on its EXEMPT list; it also fails if a required-check workflow loses its pull_request trigger. After a stacked PR's base merges and GitHub retargets it to main, re-run CI (push, or re-run the workflows) before merging: the retarget alone fires no run, so the green on the PR was earned against the old base.

Required-check x paths-filter invariant (#2199, #2200)

.github/workflows/ci.yml gates most work on the changes job's dorny/paths-filter output (code, js, a11y) so docs-only PRs skip the expensive Go/Node jobs. The live "Protect main" branch-protection ruleset requires eighteen status contexts (verified live via gh api repos/{owner}/{repo}/rulesets/13340463, #2445; re-verified for #3002): Build, Test, Lint, Bruno API Tests, Coverage Floor, A11y Smoke Tests (Playwright + axe-core), Go Vulnerability Check, Provider Failure Smoke, Analyze Go, Analyze Actions, Generated Files, Tool Version Drift, Doc Facts Drift, Bruno Route Parity, Shellcheck, Signed Commits, zizmor Suppression Scope, and Gate Invariant. The five ci.yml contexts gated on the changes job are Build, Test, Lint, Coverage Floor, and A11y Smoke Tests (Playwright + axe-core); the other thirteen come from bruno-ci.yml, security.yml, codeql.yml, gate.yml, and signed-commits.yml, whose jobs carry no job-level if, so they always run and report a real result (any path filtering is step-level; see below). Every one of the eighteen must still report success (not skipped) on every PR, including docs-only ones, and must fail closed if its changes (or equivalent) detector job itself errors. Two defects motivated the current shape (issue #2199):

• D1 (skip instead of pass): a required context produced by a job gated if: needs.changes.outputs.code == 'true' skips entirely on a non-code PR. GitHub never resolves a skipped required check to success, so the PR becomes permanently unmergeable.
• D2 (trusting outputs without checking result): needs.<job>.outputs is empty both when the job's condition was false AND when the job crashed/was canceled. A wrapper that reads outputs.code != 'true' as "safe to pass" also passes when the changes job itself failed, silently satisfying the gate without lint/test/build/coverage-floor ever running.

The fix: every required context in ci.yml is owned by an always-running (if: always()) aggregator/wrapper job that (1) asserts needs.changes.result == 'success' before trusting needs.changes.outputs.* (fails closed on a crashed detector), (2) exits 0 if code != 'true' (docs-only PR: pass, not skip), and (3) otherwise mirrors the underlying worker job's result (and, for build/coverage-floor-summary, also fails closed if the worker was left skipped by an upstream dependency failure on an actual code change rather than genuinely running). See lint/lint-summary, test/test-summary, coverage-floor/coverage-floor-summary, and build-matrix/build in ci.yml for the pattern.

Bruno API Tests (.github/workflows/bruno-ci.yml) is already compliant: its bruno job carries no job-level if at all (only individual steps are conditioned on dorny/paths-filter), so the job always runs and always reports a real result -- no wrapper needed. gate.yml's required Pre-Push Gate jobs are the same shape (job never gated, only inner steps skipped) and also need no change.

JS Unit Tests and A11y Smoke Tests were originally gated the same D1-prone way as the five above. A11y Smoke Tests now has the same always-running wrapper treatment (a11y-test/a11y-summary, mirroring lint/lint-summary; #2223) so it reports success rather than skipped on non-a11y PRs; it is one of the eighteen contexts the live ruleset requires (re-verified live, #3002). JS Unit Tests has no in-repo evidence (no .github/settings.yml or ruleset-as-code) that it is currently required; if it is promoted, it needs the same wrapper treatment.

Signed Commits (.github/workflows/signed-commits.yml, #2625) is D1-compliant by construction: its verify job carries no job-level if and no dorny/paths-filter, so it always runs and always reports a real result on every PR. Every commit is checked, not only changed files, so there is nothing to filter on. It fails closed -- an API error or a zero-commit response is treated as a failure, never as "nothing to check". It is a required context in the Protect main ruleset, so an unsigned commit blocks the merge -- the #2624 failure this check exists to prevent.

A11y Smoke Tests runs the full Playwright/axe-core suite in CI when a11y-relevant files changed and fails hard on any real violation -- CI is the strict, authoritative a11y gate (sharded per browser, #3442; the allowlist staleness check in tests/a11y/global-teardown.js runs only in unsharded local runs, so KNOWN_VIOLATIONS is held empty by a unit test until CI can fan it in); the always-running a11y-summary wrapper (#2223) exists only so the check name always reports a result, not to change when the real suite executes. The local pre-push gate does not run the tier by default (#2983); RUN_A11Y=1 git push runs it locally and BLOCKS on failure. The advisory local behavior that sat between those two states was introduced because a local-only harness flake -- a CPU-starved theme-toggle timeout in tests/a11y/contrast.spec.js, not a real contrast violation -- hard-blocked pushes unrelated to the flaking page (#2223). Skipping by default addresses that without the cost of a run whose verdict is then discarded; the flake does not occur in CI, which is not competing for a developer machine's CPU.

Harness-touching trim (#2453, supersedes the #2131 workflow-only trim)

A diff that does not touch the Go build/test harness no longer pays for the Go legs. The application code is byte-identical to main on such a diff, so the build matrix, the race suite, the Go cache primer, and CodeQL's Go analysis add zero marginal signal.

The original #2131 trim keyed on workflow_only / workflow_touched: any file under .github/workflows/** forced a real (if trimmed) Go leg. That was too coarse. Measured on #2490 -- a two-file diff touching only codeql.yml and dependabot.yml -- it ran a 144s Go Cache Primer and a 248s test shard, about 95% of the run's wall clock, to prove nothing about a change containing no Go.

ci.yml's changes job now computes a single harness_touched output. It is derived, not an enumerated allowlist of "safe" workflow files: a hand-maintained list asserts something nothing enforces, and when it goes stale it fails toward skipping the suite -- silent under-testing, which #2199 and #2445 both establish as the dangerous direction.

The derivation follows from what the Go legs are actually built out of:

Changed path	Harness?	Why
.github/workflows/ci.yml	yes	defines the Go legs
.github/actions/**	yes	composite actions those legs use
Makefile	yes	invoked by them
scripts/lib/**	yes	shared infrastructure the CI scripts source
scripts/<x>	only if ci.yml references it	grepped, not listed, so the set cannot go stale
any other workflow file	no	see below
docs, OpenAPI, JS, Python, non-CI shell	no	unchanged from before

A workflow other than ci.yml is non-harness by construction -- including one added tomorrow. This is deliberately not "any workflow that mentions the Go toolchain": codeql.yml runs setup-go and go build for its own analysis, as do nightly/fuzz/mutation. None of them feed ci.yml's Go legs, so none can invalidate ci.yml's Go results. There is no list to update and no way to forget.

An unresolvable base SHA yields harness_touched=true -- fail closed, run the suite.

When harness_touched is true, build_matrix and test_matrix collapse to a single REAL leg/shard (linux/amd64 build, the rest test shard) rather than the full fan-out -- the representative path that exercises the changed harness for real. Otherwise they take the existing skip: true no-op branch, so the required Build / Test contexts still report success while Go compute drops to zero.

Coverage Floor stays gated on code == 'true' only -- a single shard's profile covers a fraction of internal/**, so running the per-package ratchet against it would spuriously fail every package the rest shard misses. Docker Build is likewise code-only.

Lint hosts the action-pin drift guard (#2492) as a step, because it is a required, always-running check -- see below.

Bruno API Tests needed no change: its per-step dorny/paths-filter already excludes ci.yml from its relevant filter.

CodeQL gate pointers (#2491)

CodeQL's two analyses follow the same "always REPORT, only WORK when relevant" split as Lint and Coverage Floor:

• Analyze Go Check / Analyze Actions Check are the real workers. Each is gated on a purpose-built dorny/paths-filter output in codeql.yml (go and actions respectively) and skips when its language did not change. Analyze Actions previously had no gating at all and ran on every PR, including docs-only ones.
• Analyze Go / Analyze Actions are if: always() aggregators that own those literal check names. They fail closed when the detector reports failure or cancelled, pass when the language legitimately did not change, and otherwise require the worker to have succeeded.

The actions filter covers .github/workflows/**, .github/actions/** and **/action.{yml,yaml} only. Standalone shell scripts are deliberately excluded: CodeQL's actions language analyses workflow and composite-action files, and Shellcheck already owns scripts/**.

This structure exists so the two names can be added to the branch ruleset's required status checks. Until a maintainer does that, a failing CodeQL still blocks nothing -- which is how #2484 and #2486 broke CodeQL on every PR and merged anyway. The aggregators make that promotion safe: without them, a required check that legitimately skips would block every non-Go PR forever.

Copilot instruction files

Global instructions: .github/copilot-instructions.md (must stay under 4,000 characters). Domain-specific guidance in .github/instructions/:

• go-api.instructions.md -- OpenAPI semantic review, error paths, concurrency
• go-tests.instructions.md -- data races, multipart errors, assertion quality
• ci-actions.instructions.md -- version pinning, smoke test alignment

Pre-push checklist (categories Copilot consistently flags)

OpenAPI spec: - Every new/changed response field has a matching entry in internal/api/openapi.yaml - Descriptions accurately describe the invariant - $ref schemas match their actual shape

Error path completeness: - Functions emitting user-visible warnings do so on ALL error paths - No raw error.Error() output in client-visible warning strings - Full errors logged server-side; sanitized message sent to client

Generated files: - If any .templ changed, templ generate was run and *_templ.go committed - If any HTTP status code changed, scripts/smoke.sh and integration test assertions updated

SQL correctness: - ORDER BY on enum-like string columns uses CASE expression, not lexicographic sort - Dynamic SQL builders use whitelisted column maps, not user input

Accessibility: - Interactive elements have aria-label, aria-expanded, or aria-controls as appropriate

Frontend fetch calls: - All fetch() calls check resp.ok before parsing the response body

Concurrency: - Background goroutines use context.WithoutCancel(reqCtx), not context.Background() (gosec G118)

Test code: - No unprotected shared variables written in test handler goroutines and read in the test goroutine - multipart.Writer method errors checked in test helpers - io.ReadAll(r.Body) errors checked before using the result - Engine/rule tests assert relative properties, not exact counts

PR closing keywords: - PR body includes Closes #N for every issue the branch addresses