chore(deps): hold sha2 on the 0.10 line - #87
Conversation
Dependabot raised sha2 0.10.9 -> 0.11.0 as #84. It does not build. RustCrypto 0.11 moves digest output from generic-array::GenericArray to hybrid-array::Array, and Array does not implement LowerHex, so every `format!("{:x}", hasher.finalize())` stops compiling: error[E0277]: the trait `LowerHex` is not implemented for `Array<u8, UInt<UInt<UInt<UInt<UInt<...>>>>>>` error: could not compile `pcai_core_lib` (lib) due to 3 previous errors sha2 is a direct dependency with three hex-formatting call sites: pcai_core_lib/src/hash.rs, pcai_core_lib/src/search/duplicates.rs, and pcai_perf_cli/src/main.rs. The bump would also not consolidate anything. cudaforge and openai-harmony still require the 0.10 line, so taking 0.11 directly means compiling two SHA-2 implementations rather than replacing one: cudaforge -> sha2 0.10.9 openai-harmony -> sha2 0.10.9 pcai_core_lib -> sha2 0.11.0 pcai-perf -> sha2 0.11.0 0.10.9 carries no advisory and cargo audit is clean on it, so the cost is three rewritten call sites and a duplicated hash implementation for no gain. Scoped to semver-major only, so minor and patch updates to sha2 keep arriving through the normal cargo-pcai-core group. This is the same reasoning as the candle pin in the /Deploy block: a stack that has to move as one coordinated piece, not one crate at a time. Verified: cargo check -p pcai_core_lib -p pcai-perf --all-targets against the #84 lockfile fails with the three errors above; the dependabot config parses with all 8 update blocks intact. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mu7jB5ysKe4DN5ovjVtX1t
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is a narrowly scoped, syntactically valid Dependabot ignore rule with clear documentation and no functional code impact.
Pull request overview
This PR updates Dependabot configuration to prevent repeated non-actionable major update PRs for sha2 in the Native/pcai_core Rust workspace, while documenting the build-breaking cause and why the upgrade isn’t currently beneficial.
Changes:
- Add a Dependabot ignore rule for
sha2major updates in/Native/pcai_coreonly. - Document (in-file) the compile break caused by
sha20.11’s digest output type change and the rationale for deferring migration until transitive deps align.
File summaries
| File | Description |
|---|---|
| .github/dependabot.yml | Ignores sha2 semver-major updates for /Native/pcai_core and records the justification inline to avoid repeat PR churn. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5d633978c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review on #87 caught that `update-types: ["version-update:semver-major"]` does not match the bump it was written to block. Dependabot classifies an update by which SemVer *component* changed, and 0.10.9 -> 0.11.0 changes the minor component -- the major component stays 0. The consequence is worse than the ignore simply not firing. cargo-pcai-core groups minor and patch updates, so a bump classified as minor is eligible for the group: the next run could fold a known-broken sha2 0.11 into the grouped PR and block an otherwise good batch of updates behind it. `versions: [">=0.11.0"]` cannot be misclassified. It also covers the eventual 1.0 without another edit. Note the empirical picture is not clean either way. In the 2026-09 batch, genuine semver-minor bumps were grouped -- uuid 1.23.1 -> 1.26.0 and rayon 1.11.0 -> 1.12.0 both landed inside #83 -- while sha2 0.10.9 -> 0.11.0 alone was raised as its own PR (#84), which is what a major classification would produce. So Dependabot's Cargo handling may well already treat a 0.x minor as breaking. The range makes that question moot rather than betting on it. Verified: .github/dependabot.yml parses with all 8 update blocks intact and the cargo-pcai-core group unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mu7jB5ysKe4DN5ovjVtX1t
|
Follow-up: the open question in this PR now has an empirical answer. I left it unresolved deliberately — the range works either way — but dependabot ran again an hour after this merged and produced the discriminating evidence, so recording it. Dependabot's Cargo ecosystem classifies a 0.x minor bump as semver-major. The
That matches the So the Nothing to change here. |
* docs(todo): reconcile four items against what actually happened TODO.md still described a state that three merges and a workflow deletion have since changed. Marking resolved items resolved, and correcting one that had become misleading rather than merely stale. Dependabot backlog -- drained to zero, was "6 open PRs, 1 high-severity alert". The alert is fixed and cargo audit on Native/pcai_core is exit 0 (776 dependencies, 0 vulnerabilities, 5 advisory warnings). Grouping landed in #71, so minor+patch now arrive as one PR per ecosystem. #85 took the 17-update grouped bump, #86 the four .NET test-package majors, #87 held sha2 below 0.11. #84 was refused on evidence, not deferred: RustCrypto 0.11 moves digest output to hybrid-array::Array, which lacks LowerHex, and two transitive deps still require the 0.10 line -- so it breaks three call sites and duplicates sha2. #88/#89/#90 were deferred rather than judged, because they target Deploy. Portable CI (Linux) -- moot, the workflow was deleted. The item asked for a measured timeout, which now cannot be measured and should not be: this repo targets Windows by design, and a Linux job running the PowerShell suite was a misconfiguration rather than coverage. Its one piece of real coverage, workspace-wide cargo checks, moved to rust-guidelines.yml on windows-latest. Left as a struck-through entry rather than deleted so the reasoning survives. CargoTools cargo shim -- second instance recorded. The same preflight that breaks `Build.ps1 -Component functiongemma-router-data` also breaks `cargo test --manifest-path <path> -p <crate>`, dying with a rustfmt usage dump that looks like the crate under test but is entirely the shim. `Get-Command cargo` resolves to ~\bin\cargo.ps1, not rustup's. Workaround for scoped builds is rustup's binary directly. Both instances are one bug in the shim's preflight argument construction, which is worth knowing before someone debugs the second as if it were unrelated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mu7jB5ysKe4DN5ovjVtX1t * docs(todo): reconcile the advisory-warning count with the crate list Review caught that "5 advisory warnings (core2, fxhash, number_prefix, paste)" names four crates for five warnings, which is internally inconsistent and exactly the kind of number nobody can later verify. The count is right and the list was incomplete rather than wrong: core2 is reported twice, once unmaintained and once yanked, so four crates produce five warnings. Spelled out, along with why the exit code is still 0 -- cargo audit fails on vulnerabilities, not warnings. Crate: core2 Warning: unmaintained Crate: fxhash Warning: unmaintained Crate: number_prefix Warning: unmaintained Crate: paste Warning: unmaintained Crate: core2 Warning: yanked warning: 5 allowed warnings found Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Mu7jB5ysKe4DN5ovjVtX1t --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Closes out #84, the last of the new dependabot backlog, by recording why it cannot be taken rather than just closing it.
#84 does not build
sha2is a direct dependency here with three hex-formatting call sites —pcai_core_lib/src/hash.rs,pcai_core_lib/src/search/duplicates.rs,pcai_perf_cli/src/main.rs.RustCrypto 0.11 moves digest output from
generic-array::GenericArraytohybrid-array::Array, which does not implementLowerHex. Everyformat!("{:x}", hasher.finalize())stops compiling:And it would duplicate rather than consolidate
Two transitive dependencies still require the 0.10 line, so this is not a replacement — it is an addition. From #84's own lockfile:
cudaforgeopenai-harmonypcai_core_libpcai-perfCost: three rewritten call sites plus two SHA-2 implementations in the binary. Benefit: none — 0.10.9 carries no advisory and
cargo auditis clean on it.Scope of the ignore
Restricted to
version-update:semver-major, so minor and patchsha2updates keep arriving through the normalcargo-pcai-coregroup. Without it, a bare close on #84 would let the next 0.11.x re-raise the same non-starter.This is the same reasoning as the candle pin already in the
/Deployblock: a stack that has to move as one coordinated piece, not one crate at a time. Worth revisiting as a deliberate migration once the 0.11 line reaches the rest of the tree.Verification
cargo check -p pcai_core_lib -p pcai-perf --all-targetsagainst chore(deps): Bump sha2 from 0.10.9 to 0.11.0 in /Native/pcai_core #84's lockfile — fails with the three errors above.github/dependabot.ymlparses, all 8 update blocks intact,cargo-pcai-coregroup unchanged🤖 Generated with Claude Code
https://claude.ai/code/session_01Mu7jB5ysKe4DN5ovjVtX1t
Correction after review (ca81310)
The "Scope of the ignore" section above describes the first attempt,
update-types: ["version-update:semver-major"]. That would not have matched. Dependabot classifies an update by which SemVer component changed, and0.10.9 -> 0.11.0changes the minor component — the major stays0.Now expressed as a version range instead:
The second-order consequence is what makes this worth getting right.
cargo-pcai-coregroups minor and patch, so an update classified as minor is eligible for the group — a known-broken sha2 0.11 could be folded into the grouped PR and block an otherwise good batch behind it, rather than arriving as its own ignorable PR. A range cannot be misclassified, and it covers the eventual 1.0 without another edit.For the record, the empirical picture is not clean either way: in the 2026-09 batch, genuine semver-minors were grouped (
uuid 1.23.1 -> 1.26.0andrayon 1.11.0 -> 1.12.0, both inside #83) whilesha2alone was split into its own PR — which is what a major classification produces. So Dependabot's Cargo handling may already treat a 0.x minor as breaking. The range makes that question moot rather than betting on the answer; the documented reading is the one that fails safe, so that is what the config assumes.