Skip to content

fix: unbreak main — rand 0.10 API move and napi runtime/derive skew - #149

Merged
stephane-segning merged 1 commit into
mainfrom
fix/rand-010-rngext
Sep 18, 2026
Merged

stephane-segning merged 1 commit into
mainfrom
fix/rand-010-rngext

Conversation

@stephane-segning

Copy link
Copy Markdown
Contributor

Summary

main has been red since 2026-09-13 — every push failing Code Linting and SAST, Tests, Code Checks, Lint, Build CLI (Linux) and Build Docker image. Two independent breakages, neither previously root-caused. Both are fixed here; cargo check --workspace --all-targets and cargo test --workspace are clean afterwards.

Intent

Unblocks #147 (org-wide SAST/lint/Trivy adoption), which cannot be judged on its own merits while main fails the same jobs for unrelated reasons. Found while fixing the Trivy severity-gate findings in #148.

Scope

  • crates/vym-fyi-model/src/services/slug.rs — one import line.
  • Cargo.lock — napi 3.8.6 → 3.12.2, napi-sys 3.2.1 → 3.3.1, ctor consolidation.

No manifest version range changed. Nothing else touched.

1. rand 0.10 moved Rng::fill onto RngExt

The workspace pins rand = "0.10", so the breaking major arrived through the caret range. rand::rng().fill(buf.as_mut_slice()) no longer resolves:

error[E0599]: no method named `fill` found for struct `ThreadRng`
help: trait `RngExt` which provides `fill` is implemented but not in scope

use rand::Rng; → use rand::RngExt;. Rng was imported for fill alone in that file, so this is a replacement, not an addition.

2. napi and napi-derive drifted apart under one caret range

Both pinned "3" in the workspace manifest. The lockfile held napi 3.8.6 against napi-derive 3.6.5 (napi-derive-backend 6.1.3). The derive macro expands to calls the older runtime crate does not expose:

error[E0425]: cannot find function `from_raw_optional_field` in module `napi::bindgen_prelude`
error[E0425]: cannot find function `get_named_property_raw` in module `napi::bindgen_prelude`
error[E0433]: cannot find `NativeBorrowBarrier` in `bindgen_prelude`
error[E0433]: cannot find `NativeBorrowScope` in `bindgen_prelude`

19 errors, all originating inside the #[napi] attribute macro rather than at any source line a reader would suspect — which is why this was hard to see. cargo update -p napi resolves it.

This supersedes dependabot #141, which proposes the same napi bump without the rand half; either alone leaves main red.

Verification

Run locally against a clean clone of main (914411f):

  • Before: cargo check --workspace --all-targets fails — vym-fyi-model on E0599, then vym-fyi-node on 19 E0425/E0433.
  • After the rand fix alone: vym-fyi-model compiles; vym-fyi-node still fails with the same 19 errors — confirming the two breakages are genuinely independent and neither fix alone is sufficient.
  • After both: cargo check --workspace --all-targets → Finished dev profile.
  • cargo test --workspace → 8 test binaries, 8 passed, 0 failed (5 + 1 + 2 across the three crates that have tests).

Screenshots/Evidence

Pasted above: the real compiler output for each failure mode, and the resolved versions (Updating napi v3.8.6 -> v3.12.2, Updating napi-sys v3.2.1 -> v3.3.1).

Risk Assessment

Low, with one thing a reviewer should confirm rather than take on trust.

  • The rand change is a trait-import swap. RngExt::fill fills a byte slice with random bytes exactly as Rng::fill did; generate_slug's two existing tests pass unchanged. No behavioural change to slug generation.
  • The napi bump is a minor version within the pinned "3" range and is lockfile-only — cargo update would have picked it up on any unrelated dependency refresh. The native addon is the part worth a second look: cargo check and cargo test prove it compiles and links, but neither loads the built .node from Node.js. CI's Build CLI (Linux) and Build Docker image jobs are the real confirmation, and they run on this PR.
  • main is currently red, so CI on this branch should be read as "does it go green", not "does it stay green".

AI Usage Declaration

Claude Opus 5 root-caused both failures and wrote the fix. Every claim in this description was verified by running the command and reading its output, not inferred: the before/after compile states, the intermediate state proving the two breakages are independent, and the test results. Human accountability:

  • The diagnosis was reproduced from a clean clone, not taken from a summary.
  • Each fix was verified to be necessary on its own (neither alone makes the workspace compile).
  • The full test suite was run, not just the affected crate.
  • A human has confirmed the native addon loads correctly from Node.js at runtime — deferred to CI's own build jobs.

Reviewer Focus

  1. Whether napi should be pinned more tightly than "3" alongside napi-derive. This class of skew recurs by construction while both sit on the same caret range with no lockstep constraint, and the failure it produces points at the macro rather than at the mismatch.
  2. Whether dependabot chore(deps): bump napi from 3.12.2 to 3.12.6 #141 should now be closed as superseded, or rebased.
  3. The Build CLI (Linux) / Build Docker image results on this PR — those are what actually exercise the native addon.

🤖 Generated with Claude Code

`main` has failed `Code Linting and SAST`, `Tests`, `Code Checks`, `Lint`,
`Build CLI (Linux)` and `Build Docker image` on every push since 2026-09-13.
Two independent breakages, neither previously root-caused:

1. `rand` 0.10 moved `Rng::fill` onto a new `RngExt` trait, so
   `crates/vym-fyi-model/src/services/slug.rs`'s `rand::rng().fill(..)` no
   longer resolves. The workspace pins `rand = "0.10"`, so the caret range
   picked the breaking major up silently. One import line.

2. `napi` and `napi-derive` are both pinned `"3"`, and drifted apart under
   that range: the lockfile held `napi` 3.8.6 against `napi-derive` 3.6.5
   (backed by `napi-derive-backend` 6.1.3). The derive macro expands to
   calls the older runtime crate does not expose —
   `napi::bindgen_prelude::{from_raw_optional_field, from_raw_required_field,
   get_named_property_raw, set_named_property_raw}`, `NativeBorrowBarrier`,
   `NativeBorrowScope` — producing 19 E0425/E0433 errors that all originate
   inside the `#[napi]` attribute macro rather than in any source line a
   reader would suspect. `cargo update -p napi` resolves 3.8.6 -> 3.12.2.

Lockfile-only for the second; no manifest range changed for either.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@stephane-segning
stephane-segning merged commit 3a8a671 into main Sep 18, 2026
10 of 16 checks passed
@stephane-segning
stephane-segning deleted the fix/rand-010-rngext branch September 18, 2026 13:18
stephane-segning added a commit that referenced this pull request Sep 18, 2026
…152)

`Docker build for Server`/`Docker build for Redirect` (build.yml) were
failing on main before #149, and #148's run showed the same
`rand::RngExt` compile error #149 fixed. #149's own build.yml run
(triggered on push, run 35349420384) proves that compile fix is
complete and sufficient on its own: both matrix legs now genuinely
finish `cargo build --profile prod` for both amd64 and arm64
(`Finished 'prod' profile [optimized] target(s)`), and the image is
built and exported successfully.

But that same run then fails at push, with a different, unrelated
error:

    ERROR: failed to push ghcr.io/vymalo/fyi-redirect:latest: denied:
    permission_denied: The requested installation does not exist.

`IMAGE_NAME: vymalo/fyi` (env, this file) is stale: this repository
used to live at `vymalo/fyi` and was renamed/transferred to
`vaam-apps/fyi` (confirmed: `gh api repos/vymalo/fyi` resolves to
`vaam-apps/fyi` via GitHub's own rename redirect). The `GITHUB_TOKEN`
this workflow authenticates to `ghcr.io` with is scoped to this
repository's *actual current* owner (`vaam-apps`) -- it has no
"installation" letting it push to a package namespace under a
different, no-longer-current owner (`vymalo`), regardless of the
rename redirect still resolving the repo itself.

Fixed by making `IMAGE_NAME` track the real repository dynamically
(`${{ github.repository }}`) instead of a literal that can go stale on
the next rename -- the same fix shape already used elsewhere for this
exact class of problem (see the sibling `vsms` repo's own release
pipeline, which already publishes to `ghcr.io/${{
github.repository_owner }}/...` for the identical reason after going
through two of its own org renames).

Not touched, deliberately out of scope for "the two Docker build
jobs": `charts/vym-fyi-server-{crud,redirect}/values.yaml` (`image.
repository: ghcr.io/vymalo/fyi-{crud,redirect}`, the Helm chart's
default pull target -- equally stale, but a helm-publish.yml/chart
concern, not build.yml) and `charts/.../Chart.yaml` `home`/`sources`
URLs, `mkdocs.yml`'s site description, `docs/index.md`/`docs/arc42.md`
prose, `README.md`'s illustrative image tag, and `.github/FUNDING.yml`
(also all reference the old `vymalo` name, cosmetic/documentation or a
separate chart-publish concern rather than something breaking this
task's own two CI jobs). Flagged for the maintainer as a follow-up
rather than swept in the same PR.

Verification:
- `python3 -c "import yaml; yaml.safe_load(...)"`: valid YAML
- `actionlint .github/workflows/build.yml`: exit 0, no findings
- `zizmor .github/workflows/build.yml`: "No findings to report", exit 0
- Pushing this branch (build.yml triggers on `push` to any branch,
  paths including `.github/workflows/build.yml` itself) is the actual
  end-to-end verification -- see the PR for the resulting run's
  conclusion.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@vaam-apps vaam-apps Bot mentioned this pull request Sep 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant