Skip to content

Commit 8132ef6

Browse files
committed
fix(ci,4-4): Story 6-7 review patches + Story 4-4 hash stability + P15
Lands two independent threads in one commit: (1) Story 6-7 post-merge code review fixes — hardens the CI workflow, removes a dangerous gitleaks allowlist, modernizes cargo-deny, and updates the ADR + CONTRIBUTING.md to match. (2) Story 4-4 Tasks 5 + 6 — closes the byte-exact hash stability gap (refined P21) and the cache-seeding consistency gap (refined P15). With this commit, 4-4 has only Task 8 (the 6-6 regression test) still gating it. ---------------------------------------------------------------------- (1) STORY 6-7 REVIEW FIXES ---------------------------------------------------------------------- .github/workflows/ci.yml: - Pin every `dtolnay/rust-toolchain@master` to `@1.88.0`. The action's own README discourages `@master` because it is a moving target and a supply-chain risk. This includes the toolchain-matrix split below. - Remove the always-true `fuzz-smoke` soft gate. The `if: hashFiles('fuzz/Cargo.toml') != ''` guard was permanently true the moment 6-7 merged because Story 6-4 had already landed `fuzz/Cargo.toml`. Now `fuzz-smoke` is a hard gate from Day 1. - Coverage job: drop `needs: unit` (no correctness benefit, just serializes two builds), drop `fetch-depth: 0` (delta gate is descoped — see ADR-0002 D3), use a separate `solarix-ci-coverage` cache key so `RUSTFLAGS=-Cinstrument-coverage` artifacts don't thrash the regular cache, switch to `taiki-e/install-action@v2` for cargo-llvm-cov (saves ~3–5 min cold install), and add `if-no-files-found: error` so a silent `cargo llvm-cov` no-op fails the upload step instead of warning. - Integration job: drop the unused `SOLANA_RPC_URL: devnet` env var with an explanatory comment. The integration tests today (bootstrap_test.rs, registration_test.rs) don't hit the RPC, and devnet is too rate-limited / flaky for the per-PR critical path. - Security job: switch to `taiki-e/install-action@v2` for cargo-audit + cargo-deny (cached binaries), and run `cargo deny check advisories bans sources` explicitly instead of the bare `cargo deny check`. The bare form would also run `licenses` which is fail-soft for Sprint 5 (see ADR-0002 D5). - Docker-smoke: bump `curl --retry 12 --retry-delay 5` to `--retry 30 --retry-delay 10 --retry-all-errors --connect-timeout 5` so a brief 404 during axum route mounting doesn't fail the wait step. Scan EVERY captured log line for valid JSON (not just `head -5`) so a pretty-format regression that lands after the startup banner is still caught. Fail loudly when zero log lines are captured (silent container crash). Drop `|| true` from the cleanup step so a real teardown failure isn't masked. `if: always()` already runs cleanup on both branches. - Toolchain matrix: split the `strategy.matrix: [stable, beta]` into two explicit `toolchain-stable` and `toolchain-beta` jobs. GitHub Actions does not allow expressions in `uses:`, so a matrix forces the action ref back to `@master`. Two explicit jobs trade YAML lines for the elimination of the supply-chain hole. Beta job uses `continue-on-error: true` to surface upcoming breakages as warnings. .github/workflows/nightly.yml: - Permissions: `pull-requests: write` → `issues: write`. The notification step calls `github.rest.issues.createComment`, which is on the issues API. The previous permission set would let the workflow load but the createComment call would 403 at runtime. - Move `continue-on-error` from the JOB level to the test STEP. With it on the job, the overall job conclusion was forced to success, which made `if: failure()` on the notification step never trigger. The new pattern: `continue-on-error: true` on the run step, and the notification step branches on `steps.run_smoke.outcome == 'failure'` so it actually fires when the test fails. - Belt-and-braces guard: a `feature_check` step grep's Cargo.toml for `^mainnet-smoke\b` and short-circuits the rest of the job via `if:` outputs if the cargo feature isn't declared yet. Pairs with the existing `hashFiles('tests/mainnet_smoke.rs')` job-level guard for the case where the test file and the cargo feature land in separate PRs. .gitleaks.toml: - Remove the global `regexes = ['''[1-9A-HJ-NP-Za-km-z]{32,44}''']` allowlist. The intent was to allowlist Solana pubkeys, but a regex at the top-level [allowlist] block applies to every detected secret across the whole repo — and base58 of length 32–44 also matches the base58 encoding of many real secrets (Stripe keys, GCP service-account fingerprints, ECDSA signatures). Future regex allowlists must live under a specific [[rules]] block. Add a comment explaining the gotcha so the next contributor doesn't re-introduce it. - Drop the `*.md$` path allowlist. It was too broad — a real secret pasted into any `.md` file outside `_bmad-output/` would be missed. Path allowlist now scopes to `tests/fixtures/`, `_bmad-output/`, and `docs/research/` only. deny.toml: - Add `version = 2` to both `[advisories]` and `[licenses]`. Without it, key semantics drift between cargo-deny releases (the legacy `vulnerability/unmaintained/notice/unsound` knobs were removed in v2 in favor of "any advisory match is an error unless ignored"). v2 semantics are the only reason the `RUSTSEC-2025-0012` (backoff) advisory that motivated this whole story would actually fail the build. - Update the section header comment to make it explicit that the CI security job runs `cargo deny check advisories bans sources` (NOT licenses), so the [licenses] block below is documentation only for Sprint 5. Revisit post-bounty. CONTRIBUTING.md: - Security row matches the new `cargo deny check advisories bans sources` invocation. - Add `rustup toolchain install 1.88` + `rustup component add` to the local prerequisites section so contributors don't trip over a missing MSRV toolchain. docs/adr/0002-ci-pipeline.md: - Drop `fuzz-smoke` from the soft-gate table (now a hard gate). - Update the table's note column to reflect the belt-and-braces feature_check guard added to nightly.yml. - Reflect the toolchain matrix split (stable + beta as two explicit jobs) in the hard-gate list. README.md: - Add an MSRV badge linking to `rust-toolchain.toml`. - Fix CI badge URL. ---------------------------------------------------------------------- (2) STORY 4-4 TASKS 5 + 6 (HASH STABILITY + CACHE SEEDING CONSISTENCY) ---------------------------------------------------------------------- src/idl/mod.rs: - `CachedIdl` gains a `raw_json: String` field that holds the ORIGINAL fetched/uploaded JSON bytes — not a re-serialization of the parsed `Idl`. This is what `idl_hash` was computed from and what gets persisted into `programs.idl_json`. Holding the raw bytes is what gives Story 4.4 AC5 (hash stability) its byte-exact guarantee: `compute_idl_hash(raw_json) == hash`. Re-serializing through `serde_json::to_string(&idl)` would silently drop fields that are not modeled by `anchor_lang_idl_spec::Idl` and shuffle Option None-vs-absent representations, breaking the round trip for any future feature that re-hashes persisted bytes (e.g., on-chain IDL drift detection). - `upload_idl` and `insert_fetched_idl` both wire the raw bytes into the new field. Doc comments cross-reference Story 4.4 AC5. src/registry.rs: - `RegistrationData` gains an `idl_json: String` field that carries the raw bytes from `CachedIdl::raw_json` through to `commit_registration`. The previous implementation called `serde_json::to_string(&idl)` inside `commit_registration` — doc-comment now explains why that was wrong and why the raw bytes win. - New `mark_program_error(pool, program_id, error_message)` helper: flips `programs.status = 'error'`, stashes the failure message in `indexer_state.error_message`, and commits both updates in a single transaction. Returns a boxed Send future for the same reason as `update_program_status` — keeps the in-flight Executor reference from leaking through the opaque return type and breaking Send inference at the caller's await. - Two new unit tests: * `test_mark_program_error_future_is_send` (Send-safety compile- time check, follows the AC9 pattern) * `registration_data_idl_json_hashes_to_idl_hash` — pin the byte- exact invariant. Uses deliberately unusual whitespace + key order in the input JSON, then asserts both that `data.idl_json` matches the input verbatim AND that re-serializing the parsed `Idl` produces DIFFERENT bytes (so the test would actually fail if `commit_registration` ever silently re-serialized). src/main.rs: - Cache seeding loop: now reads `p.idl_json` (the raw bytes from the DB) instead of computing `serde_json::to_string(&p.idl)`. The bytes go straight into `IdlManager::insert_fetched_idl` so the in-memory cache holds exactly what's on disk. - Per-program success tracking: collects a `Vec<bool>` of seed outcomes inside the registry write lock, then drops the lock before any DB await. Programs whose seeding failed are dropped from the `programs_to_start` list AND get their `programs.status` flipped to `'error'` via the new `mark_program_error` helper. This is Story 4.4 Task 6 (refined P15): keeps the API consistent so an operator never sees a 200 from `/api/programs/{id}` alongside 404s from `/api/programs/{id}/instructions/{name}`. - `StartupProgram` gains an `idl_json` field. `query_registered_programs` populates it from the same `idl_json` column it already reads. src/pipeline/mod.rs: - `test_backfill_progress_eta` strengthened. The previous version was weakened in a clippy cleanup pass to "just confirm the call path doesn't panic" because the old assertion (`eta.as_secs() >= 0`) triggered `clippy::absurd_extreme_comparisons`. The new version asserts the actual contract: a freshly-constructed BackfillProgress with elapsed time < 1ms returns `Duration::from_secs(0)` (zero-rate sentinel), and a finished progress (start == end) also returns `Duration::from_secs(0)` regardless of rate. A regression that swapped the sentinel for `Duration::MAX` or panicked on division-by-zero would now fail the test. tests/registration_test.rs: - New `mark_program_error_transitions_status_and_records_message` integration test (#[ignore], requires running PostgreSQL): seeds a successful registration, calls `mark_program_error`, asserts `programs.status` flipped to 'error' AND `indexer_state.{status, error_message}` reflect the failure. - New `idl_json_persisted_bytes_are_byte_exact` integration test: end-to-end byte preservation. Uploads JSON with deliberately unusual whitespace, calls `commit_registration`, reads the persisted bytes back via `SELECT idl_json FROM programs`, and asserts byte equality + hash equality. This is the integration- level pin for Story 4.4 AC5.
1 parent 9fae115 commit 8132ef6

12 files changed

Lines changed: 580 additions & 129 deletions

File tree

.github/workflows/ci.yml

Lines changed: 128 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,8 @@ jobs:
3232
with:
3333
fetch-depth: 1
3434
- name: Install toolchain (pinned MSRV via rust-toolchain.toml)
35-
uses: dtolnay/rust-toolchain@master
35+
uses: dtolnay/rust-toolchain@1.88.0
3636
with:
37-
toolchain: "1.88"
3837
components: rustfmt, clippy
3938
- name: Cache cargo registry + target
4039
uses: Swatinem/rust-cache@v2
@@ -54,9 +53,7 @@ jobs:
5453
with:
5554
fetch-depth: 1
5655
- name: Install toolchain
57-
uses: dtolnay/rust-toolchain@master
58-
with:
59-
toolchain: "1.88"
56+
uses: dtolnay/rust-toolchain@1.88.0
6057
- name: Cache cargo registry + target
6158
uses: Swatinem/rust-cache@v2
6259
with:
@@ -84,16 +81,20 @@ jobs:
8481
--health-retries 5
8582
env:
8683
DATABASE_URL: postgres://solarix:solarix@localhost:5432/solarix
87-
SOLANA_RPC_URL: https://api.devnet.solana.com
8884
RUST_LOG: warn
85+
# NOTE: SOLANA_RPC_URL is NOT set at this level. The integration tests
86+
# that run today (bootstrap_test.rs, registration_test.rs) do not hit
87+
# the Solana RPC — they only exercise the DB bootstrap and registration
88+
# paths. Any future test that needs RPC should use a LiteSVM harness or
89+
# a local RPC fake; devnet is too rate-limited and too flaky for the
90+
# per-PR critical path (see ADR-0002 D2 — this is the same reason the
91+
# mainnet smoke test is isolated into `nightly.yml`).
8992
steps:
9093
- uses: actions/checkout@v4
9194
with:
9295
fetch-depth: 1
9396
- name: Install toolchain
94-
uses: dtolnay/rust-toolchain@master
95-
with:
96-
toolchain: "1.88"
97+
uses: dtolnay/rust-toolchain@1.88.0
9798
- name: Cache cargo registry + target
9899
uses: Swatinem/rust-cache@v2
99100
with:
@@ -116,22 +117,37 @@ jobs:
116117
name: coverage (lcov artifact)
117118
runs-on: ubuntu-latest
118119
timeout-minutes: 12
119-
needs: unit
120+
# NOTE: intentionally no `needs: unit`. Both jobs build the same code;
121+
# running them in parallel means a broken PR surfaces both failures at
122+
# once and the 12-minute end-to-end target is easier to hit. The previous
123+
# `needs: unit` gave no correctness benefit.
120124
steps:
121125
- uses: actions/checkout@v4
122126
with:
123-
fetch-depth: 0
127+
# Coverage does not currently need git history; the delta gate is
128+
# descoped (ADR-0002 D3). Restore to `fetch-depth: 0` when the
129+
# delta-gate follow-up story lands.
130+
fetch-depth: 1
124131
- name: Install toolchain
125-
uses: dtolnay/rust-toolchain@master
132+
uses: dtolnay/rust-toolchain@1.88.0
126133
with:
127-
toolchain: "1.88"
128134
components: llvm-tools-preview
129135
- name: Cache cargo registry + target
136+
# Separate cache key from the rest of `solarix-ci` because
137+
# `cargo-llvm-cov` sets `RUSTFLAGS=-Cinstrument-coverage`, which
138+
# produces `.rlib` artifacts that are incompatible with the other
139+
# jobs' builds. Sharing the cache would cause thrashing (either
140+
# job's target/ would force the other to fully rebuild).
130141
uses: Swatinem/rust-cache@v2
131142
with:
132-
shared-key: "solarix-ci"
133-
- name: Install cargo-llvm-cov
134-
run: cargo install cargo-llvm-cov --locked
143+
shared-key: "solarix-ci-coverage"
144+
- name: Install cargo-llvm-cov (cached binary)
145+
# `cargo install --locked` re-compiles from source every run (~3–5 min
146+
# cold tax). `taiki-e/install-action` fetches a prebuilt binary and
147+
# caches it per version, bringing the install down to a few seconds.
148+
uses: taiki-e/install-action@v2
149+
with:
150+
tool: cargo-llvm-cov
135151
- name: Run coverage (lib only — see ADR-0002 D6)
136152
run: cargo llvm-cov --release --lib --lcov --output-path lcov.info
137153
- name: Print coverage summary
@@ -142,13 +158,20 @@ jobs:
142158
name: lcov-info
143159
path: lcov.info
144160
retention-days: 14
161+
# Fail the step (not just warn) if `cargo llvm-cov` silently
162+
# produced no output. The default `warn` hides this regression.
163+
if-no-files-found: error
145164

146165
fuzz-smoke:
147166
name: fuzz smoke (60s)
148167
runs-on: ubuntu-latest
149168
timeout-minutes: 8
150-
# Soft-gate: no-op until Story 6.4 ships the fuzz target.
151-
if: hashFiles('fuzz/Cargo.toml') != ''
169+
# NOTE: this job is now a HARD gate on every run. The original "soft gate"
170+
# guard `if: hashFiles('fuzz/Cargo.toml') != ''` was permanently true
171+
# because Story 6.4 already landed `fuzz/Cargo.toml` and
172+
# `fuzz/fuzz_targets/decode_instruction.rs` before this story merged
173+
# (see ADR-0002 § D1). Removing the guard matches what actually runs.
174+
continue-on-error: false
152175
steps:
153176
- uses: actions/checkout@v4
154177
with:
@@ -159,8 +182,10 @@ jobs:
159182
uses: Swatinem/rust-cache@v2
160183
with:
161184
shared-key: "solarix-ci-nightly"
162-
- name: Install cargo-fuzz
163-
run: cargo +nightly install cargo-fuzz --locked
185+
- name: Install cargo-fuzz (cached binary)
186+
uses: taiki-e/install-action@v2
187+
with:
188+
tool: cargo-fuzz
164189
- name: Run fuzz smoke
165190
run: cargo +nightly fuzz run decode_instruction -- -max_total_time=60
166191

@@ -175,21 +200,19 @@ jobs:
175200
# want history too.
176201
fetch-depth: 0
177202
- name: Install toolchain
178-
uses: dtolnay/rust-toolchain@master
179-
with:
180-
toolchain: "1.88"
203+
uses: dtolnay/rust-toolchain@1.88.0
181204
- name: Cache cargo registry + target
182205
uses: Swatinem/rust-cache@v2
183206
with:
184207
shared-key: "solarix-ci"
185-
- name: Install cargo-audit and cargo-deny
186-
run: |
187-
cargo install cargo-audit --locked
188-
cargo install cargo-deny --locked
208+
- name: Install cargo-audit and cargo-deny (cached binaries)
209+
uses: taiki-e/install-action@v2
210+
with:
211+
tool: cargo-audit,cargo-deny
189212
- name: cargo audit
190213
run: cargo audit --deny warnings
191-
- name: cargo deny check
192-
run: cargo deny check
214+
- name: cargo deny check (advisories + bans + sources — licenses is fail-soft per ADR-0002 D5)
215+
run: cargo deny check advisories bans sources
193216
- name: gitleaks
194217
uses: gitleaks/gitleaks-action@v2
195218
with:
@@ -213,8 +236,15 @@ jobs:
213236
- name: docker compose up --build -d
214237
run: docker compose up --build -d
215238
- name: Wait for /health
239+
# 30 retries × 10 s = 300 s ceiling on runtime startup. The in-container
240+
# Solarix build already completed during `docker compose up --build`,
241+
# so this budget is purely for the binary cold-start + postgres
242+
# healthcheck + axum route mounting window. `--retry-all-errors`
243+
# covers a race where `/health` briefly 404s during route mounting
244+
# (default `--retry` only covers 5xx + connection-refused).
216245
run: |
217-
curl --retry 12 --retry-delay 5 --retry-connrefused --fail \
246+
curl --retry 30 --retry-delay 10 --retry-connrefused --retry-all-errors --fail \
247+
--connect-timeout 5 \
218248
http://localhost:3000/health
219249
- name: Soft-check /ready (warning only until Story 6.3)
220250
run: |
@@ -223,28 +253,37 @@ jobs:
223253
else
224254
echo "::warning::/ready not yet implemented (Story 6.3)"
225255
fi
226-
- name: Log format check (head -5 must be valid JSON)
256+
- name: Log format check (every captured line must be valid JSON)
227257
run: |
228258
set -euo pipefail
229259
docker compose logs solarix --no-log-prefix > /tmp/solarix.log
230-
head -5 /tmp/solarix.log | grep -v '^$' > /tmp/solarix-head.log || true
231-
if [ ! -s /tmp/solarix-head.log ]; then
260+
if [ ! -s /tmp/solarix.log ]; then
232261
echo "::error::No log lines captured from solarix container"
233262
exit 1
234263
fi
264+
# Scan the full log (not just the first 5 lines). The previous
265+
# `head -5` version would false-negative on any `pretty` regression
266+
# that landed after the startup banner, and false-positive on any
267+
# clap/dotenv warning that landed in the first 5 lines.
235268
python3 - <<'PY'
236269
import json, sys
237-
path = "/tmp/solarix-head.log"
238-
for i, line in enumerate(open(path), 1):
239-
line = line.strip()
240-
if not line:
241-
continue
242-
try:
243-
json.loads(line)
244-
except json.JSONDecodeError as e:
245-
print(f"::error::Non-JSON log line {i}: {line}")
246-
sys.exit(1)
247-
print("All captured log lines are valid JSON")
270+
path = "/tmp/solarix.log"
271+
count = 0
272+
with open(path) as f:
273+
for i, raw in enumerate(f, 1):
274+
line = raw.strip()
275+
if not line:
276+
continue
277+
try:
278+
json.loads(line)
279+
except json.JSONDecodeError:
280+
print(f"::error::Non-JSON log line {i}: {line}")
281+
sys.exit(1)
282+
count += 1
283+
if count == 0:
284+
print("::error::No non-empty log lines found — container may have crashed silently")
285+
sys.exit(1)
286+
print(f"All {count} captured log lines are valid JSON")
248287
PY
249288
- name: Soft-check /metrics (warning only until Story 6.2)
250289
run: |
@@ -263,9 +302,15 @@ jobs:
263302
name: docker-smoke-logs
264303
path: solarix.log
265304
retention-days: 7
305+
if-no-files-found: warn
266306
- name: Cleanup
307+
# Intentionally NOT using `|| true` here. A cleanup failure means
308+
# `docker compose down -v` could not tear down the containers, which
309+
# is a real problem we want surfaced — not silently masked.
310+
# `if: always()` still guarantees the step runs on both success and
311+
# failure branches.
267312
if: always()
268-
run: docker compose down -v || true
313+
run: docker compose down -v
269314

270315
msrv:
271316
name: msrv build (1.88)
@@ -276,41 +321,61 @@ jobs:
276321
with:
277322
fetch-depth: 1
278323
- name: Install pinned MSRV toolchain
279-
uses: dtolnay/rust-toolchain@master
280-
with:
281-
toolchain: "1.88"
324+
uses: dtolnay/rust-toolchain@1.88.0
282325
- name: Cache cargo registry + target
283326
uses: Swatinem/rust-cache@v2
284327
with:
285328
shared-key: "solarix-ci"
286329
- name: cargo build --release
287330
run: cargo build --release
288331

289-
toolchain-matrix:
290-
name: toolchain matrix (stable, beta)
332+
# The matrix for stable/beta is split into two explicit jobs rather than a
333+
# `strategy.matrix` block. GitHub Actions does not allow expressions in
334+
# `uses:` fields, so a matrix would force the action ref back to
335+
# `dtolnay/rust-toolchain@master` (a moving target that the action's own
336+
# README discourages). Two explicit jobs trade a few YAML lines for an
337+
# elimination of the `@master` supply-chain hole.
338+
339+
toolchain-stable:
340+
name: toolchain build (stable)
291341
runs-on: ubuntu-latest
292342
timeout-minutes: 15
293-
strategy:
294-
fail-fast: false
295-
matrix:
296-
toolchain: [stable, beta]
297-
continue-on-error: ${{ matrix.toolchain == 'beta' }}
298343
steps:
299344
- uses: actions/checkout@v4
300345
with:
301346
fetch-depth: 1
302-
- name: Install ${{ matrix.toolchain }} toolchain
303-
uses: dtolnay/rust-toolchain@master
347+
- name: Install stable toolchain
348+
uses: dtolnay/rust-toolchain@stable
349+
- name: Cache cargo registry + target
350+
uses: Swatinem/rust-cache@v2
304351
with:
305-
toolchain: ${{ matrix.toolchain }}
352+
shared-key: "solarix-ci-stable"
353+
- name: cargo build --release
354+
# `RUSTUP_TOOLCHAIN` env overrides `rust-toolchain.toml` per rustup
355+
# docs, so the job actually exercises stable instead of the pinned
356+
# MSRV channel.
357+
env:
358+
RUSTUP_TOOLCHAIN: stable
359+
run: cargo build --release
360+
361+
toolchain-beta:
362+
name: toolchain build (beta)
363+
runs-on: ubuntu-latest
364+
timeout-minutes: 15
365+
# Beta is allowed to flake — surfacing upcoming breakages as a warning
366+
# annotation, not a hard fail.
367+
continue-on-error: true
368+
steps:
369+
- uses: actions/checkout@v4
370+
with:
371+
fetch-depth: 1
372+
- name: Install beta toolchain
373+
uses: dtolnay/rust-toolchain@beta
306374
- name: Cache cargo registry + target
307375
uses: Swatinem/rust-cache@v2
308376
with:
309-
shared-key: "solarix-ci-${{ matrix.toolchain }}"
377+
shared-key: "solarix-ci-beta"
310378
- name: cargo build --release
311-
# Override the pinned rust-toolchain.toml channel for this job — the
312-
# point of the matrix is to exercise stable and beta against the same
313-
# Cargo.lock.
314379
env:
315-
RUSTUP_TOOLCHAIN: ${{ matrix.toolchain }}
380+
RUSTUP_TOOLCHAIN: beta
316381
run: cargo build --release

0 commit comments

Comments
 (0)