Skip to content

fix(ci): unbreak Rust CI, DB Checks and Stress Testing after the dep sweep - #124

Merged
hyperpolymath merged 2 commits into
mainfrom
fix/ci-otel-lock-toolchain
Aug 3, 2026
Merged

hyperpolymath merged 2 commits into
mainfrom
fix/ci-otel-lock-toolchain

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Finishes what #109 set out to do. #109 merged on 29 Jul and correctly fixed the standalone build — but the Dependabot run that landed straight after it (#110–#116) re-broke main in four new ways. Rust CI, DB Checks and Stress Testing are red on main today.

Each fault below is taken from a run log and reproduced locally. None is inferred.

1. opentelemetry_sdk was bumped alone — breaks DB Checks

#110 moved only opentelemetry_sdk 0.27.1 → 0.32.1. Its three siblings stayed on 0.27, and these crates only work version-aligned. src/observability.rs is 0.27-era code and stopped compiling with exactly three errors (run 30792934376):

error site 0.32 change
E0432 unresolved import opentelemetry_sdk::trace::TracerProvider observability.rs:54 renamed SdkTracerProvider
E0624 associated function new is private observability.rs:223 Resource::new → Resource::builder()
E0271 type mismatch on Vec<KeyValue> observability.rs:223 same Resource change

opentelemetry_sdk::runtime::Tokio is gone too.

Reverted to 0.27, realigning manifest ↔ lockfile ↔ code in one line. Moving the family to 0.32 is a genuine piece of work — four bumps plus an observability.rs rewrite — and belongs in its own PR, not in a CI-repair change.

DB Checks builds without --locked, so it resolved 0.32 fresh and died at compile. That is why it failed differently from Rust CI.

2. Cargo.lock no longer satisfied Cargo.toml — breaks Rust CI

Rust CI builds with --locked and died at resolution, before running a single check:

error: cannot update the lock file ... because --locked was passed to prevent this

Reproduced on a pristine origin/main worktree (cargo metadata --locked → exit 101), then repaired by letting cargo resolve.

The lock loses four entries and gains none (533 → 529 packages):

  • opentelemetry 0.32.0, opentelemetry_sdk 0.32.1 — per fault 1
  • is-terminal 0.4.17, portable-atomic 1.14.0 — orphans that criterion 0.8.2 no longer pulls

No crate is upgraded by this PR. Worth stating plainly, since a regenerated lockfile usually implies drift: tower and tower-http are untouched. main's lock already carried both 0.4.13 and 0.5.3 — old majors coexisting legitimately as transitive deps, not a skew. The sole --locked breaker was the orphaned is-terminal entry.

3. SHA-pinned dtolnay/rust-toolchain with no toolchain: — breaks Stress Testing

error: invalid toolchain name ''

The action infers its toolchain from the ref that called it (@stable → "stable"); pinned to a SHA there is no name to infer. #116 moved stress-test.yml onto a revision that no longer tolerates the omission.

The repo carried three different SHAs of this action across five workflows, and only publish.yml passed the input:

workflow pin had toolchain: was
stress-test.yml b3b07ba… (bumped by #116) ✗ failing
release.yml f7ccc83… ✗ (targets: only) latent
cargo-audit.yml 4be9e76… ✗ passing on this SHA
db-checks.yml 4be9e76… ✗ passing on this SHA
publish.yml f7ccc83… ✓ correct

toolchain: stable is now stated at every call site, so behaviour no longer depends on which SHA the pin happens to point at — the next actions bump cannot re-break it. There is no rust-toolchain.toml to fall back on.

4. criterion 0.8.2 deprecated criterion::black_box — also Rust CI

Invisible until now, because resolution failed before clippy ever ran. The shared standards reusable runs cargo clippy --locked --all-targets -- -D warnings, which turns the 14 deprecation warnings in benches/echidnabot_bench.rs into 14 hard errors.

Fixing only faults 1–3 would have moved Rust CI's failure to a later step rather than making it green. The bench now imports std::hint::black_box.

Prevention

.github/dependabot.yml gave the cargo ecosystem no groups: block at all — precisely what let #110 split a must-move-together family. opentelemetry* and tower* are now grouped.

Verification

The same four commands the standards reusable runs, against this branch:

command result
cargo check --locked --all-targets exit 0 — this is the exact command that was failing
cargo fmt --all -- --check exit 0
cargo clippy --locked --all-targets -- -D warnings exit 0
cargo test --locked (5 named suites) exit 0 — 91 passed, 0 failed

Suite breakdown: integration_tests 32, lifecycle 24, property_tests 12, seam_test 15, smoke 8.

The bare library suite is deliberately not run: it is a known hang (>20 min, exit 124), excluded by both rust-ci.yml and publish.yml. All five edited YAML files re-parse.

Local green is not the gate — please confirm Rust CI, DB Checks and Stress Testing go green on this PR before merging. Check with gh run list, not gh pr checks: a parse-rejected workflow produces no check run at all, and gh pr checks reports that as "no checks" rather than a failure.

Not addressed

  • ClusterFuzzLite batch fuzzing is red with could not compile sqlx (lib) under the ASan/FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION flags. Unrelated to all of the above; needs its own diagnosis.
  • OpenTelemetry 0.32 family upgrade — the real version of fault 1, worth doing deliberately.

🤖 Generated with Claude Code

…sweep

Four faults, all introduced by the Dependabot run #110-#116 that landed
after #109 merged. Each is measured from a CI run log and reproduced
locally; none is a guess.

1. opentelemetry_sdk was bumped alone (#110, 0.27.1 -> 0.32.1)

   Its three siblings stayed on 0.27, and these crates only work
   version-aligned. src/observability.rs is 0.27-era code and stopped
   compiling with exactly three errors (run 30792934376): TracerProvider
   is no longer in opentelemetry_sdk::trace (E0432), Resource::new is
   private (E0624), and the Vec<KeyValue> bound no longer holds (E0271).
   opentelemetry_sdk::runtime::Tokio is gone too.

   Reverted to 0.27, which realigns manifest, lockfile and code. Moving
   the whole family to 0.32 is a real piece of work -- four bumps plus an
   observability.rs rewrite -- and is left for its own PR.

   This is what took DB Checks red: it builds without --locked, so it
   resolved 0.32 fresh and died at compile.

2. Cargo.lock no longer satisfied Cargo.toml

   Rust CI builds with --locked and died at resolution before running a
   single check:

     error: cannot update the lock file ... because --locked was passed

   Reproduced on a pristine origin/main worktree (exit 101), then repaired
   by letting cargo resolve. The lock loses four entries and gains none:
   opentelemetry 0.32.0 and opentelemetry_sdk 0.32.1 (per fault 1), plus
   two orphans -- is-terminal 0.4.17 and portable-atomic 1.14.0 -- that
   criterion 0.8.2 no longer pulls. 533 -> 529 packages. No crate is
   upgraded by this commit.

3. dtolnay/rust-toolchain SHA-pinned with no `toolchain:` input

   Stress Testing failed with `error: invalid toolchain name ''`. The
   action infers its toolchain from the ref that called it (@stable ->
   "stable"); pinned to a SHA there is no name to infer. #116 moved
   stress-test.yml onto a revision that no longer tolerates the omission.

   The repo carried three different SHAs of this action across five
   workflows, and only publish.yml passed the input. `toolchain: stable`
   is now stated at every call site, so behaviour no longer depends on
   which SHA the pin happens to point at and the next actions bump cannot
   re-break it. release.yml was armed with the same latent fault --
   `targets:` alone does not satisfy it.

4. criterion 0.8.2 deprecated criterion::black_box (#115)

   Invisible until now, because resolution failed before clippy ran. The
   shared standards reusable runs `cargo clippy --locked --all-targets --
   -D warnings`, which turns the 14 deprecation warnings in
   benches/echidnabot_bench.rs into 14 hard errors. Fixing only faults
   1-3 would have moved Rust CI's failure to a later step, not made it
   green. The bench now imports std::hint::black_box.

Prevention: the cargo ecosystem in dependabot.yml had no `groups:` block
at all, which is precisely what let #110 split a must-move-together
family. opentelemetry* and tower* are now grouped.

Verified locally -- the same four commands the standards reusable runs:

  cargo check --locked --all-targets            exit 0
  cargo fmt --all -- --check                    exit 0
  cargo clippy --locked --all-targets -D warn   exit 0
  cargo test --locked (5 named suites)          exit 0, 91 passed, 0 failed

The bare library suite is deliberately not run: it is a known hang
(>20 min, exit 124) and is excluded by both rust-ci.yml and publish.yml.

Not addressed, needing separate diagnosis: ClusterFuzzLite batch fuzzing
is red with `could not compile sqlx (lib)` under the ASan/fuzzing flags.
Unrelated to any of the above.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@gitar-bot

gitar-bot Bot commented Aug 3, 2026 •

Copy link
Copy Markdown

Note

Automatic reviews are paused because your trial's included automatic processing has been used for this period. Upgrade now, or comment "Gitar review" to run a review anytime.
Learn more

Code Review ✅ Approved

Restores CI pipelines by reverting an incomplete OpenTelemetry upgrade, regenerating the lockfile, fixing rust-toolchain inputs across workflows, and updating benchmark black_box imports. No issues found.

Auto-approved and auto-merge armed: No blocking issues found.
Please see Auto-approve Docs for details on setting custom approval criteria. — merges when pipeline and required approvals pass.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact
gitar display:verbose         

Important

Your trial ends in 7 days — upgrade now to keep code review, CI analysis, auto-apply, custom automations, and more.

Was this helpful? React with 👍 / 👎 | Gitar

@gitar-bot

gitar-bot Bot commented Aug 3, 2026

Copy link
Copy Markdown

⚠️ Gitar auto-approved this PR but could not enable auto-merge: auto-merge is disabled for this repository — enable "Allow auto-merge" in the repository settings.

@gitar-bot gitar-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gitar has auto-approved this PR and enabled auto-merge (configure)

@gitar-bot gitar-bot Bot added the gitar-approved Added by Gitar label Aug 3, 2026
Signed-off-by: Jonathan D.A. Jewell <6759885+hyperpolymath@users.noreply.github.com>
@hyperpolymath
hyperpolymath marked this pull request as ready for review August 3, 2026 13:26
@hyperpolymath
hyperpolymath merged commit 9e2bf1b into main Aug 3, 2026
28 of 32 checks passed
@hyperpolymath
hyperpolymath deleted the fix/ci-otel-lock-toolchain branch August 3, 2026 13:26
hyperpolymath added a commit that referenced this pull request Aug 3, 2026
#124 landed the same criterion/black_box fix and the dtolnay toolchain fix
while this branch was open. Conflict was comment-only in the bench; took #124's
wording.

⚠ Real defect found and fixed while resolving: the cargo ecosystem ended up with
TWO `groups:` keys -- #117 added one, #124 added another. YAML is last-wins and
parses without complaint, so the second block SILENTLY DISCARDED the first:

  ruby -ryaml  ->  cargo: groups=["opentelemetry"]

`tower` was configured and dead. Merged into a single block; now resolves to
["opentelemetry", "tower", "rustcrypto"], with `ignore:` preserved.

A YAML parse check does NOT catch this. Only reading back the resolved keys
does. Added a note in the file so the next person merging a groups change sees
it.
hyperpolymath added a commit that referenced this pull request Aug 3, 2026
The merge auto-resolved Cargo.lock from two sides that disagreed about sha2
(0.10 on main, 0.11 here), producing a lock inconsistent with Cargo.toml.

Caught by the gate, not by inspection:

  error: cannot update the lock file ... because --locked was passed to
         prevent this

This is exactly what `--locked` is for. A merged Cargo.lock is not
automatically a valid one, and nothing else in the toolchain says so.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gitar-approved Added by Gitar

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant