Skip to content

fix(server): drop the unwraps in apply_tactic; mark the test fixture module - #490

Merged
hyperpolymath merged 2 commits into
mainfrom
claude/hypatia-baseline-findings
Nov 10, 2026
Merged

hyperpolymath merged 2 commits into
mainfrom
claude/hypatia-baseline-findings

Conversation

@hyperpolymath

@hyperpolymath hyperpolymath commented Nov 10, 2026 •

Copy link
Copy Markdown
Owner

Summary

The Hypatia baseline gate fails with 3 unfiltered findings. This PR removes them at the source and adds no baseline entries.

Changes

  • src/rust/server/session.rs (code_safety/unwrap_without_check, high, a real finding): apply_tactic checked state.is_none() and then called .unwrap() 4 times. It now binds the state with let ... else, takes the goal counts from the values it already holds, and clears the goals with if let on QED. The behaviour does not change.
  • src/rust/provers/test_fixture.rs (code_safety/expect_in_hot_path and code_safety/panic_macro, false positives): the module compiles only under #[cfg(all(test, unix))], declared in provers/mod.rs, and a failed fixture setup must fail the test. Hypatia removes only a trailing #[cfg(test)] block in the same file, so it cannot see that this whole module is test-only. I added 2 file-level hypatia: allow directives with reasons. Hypatia's guidance prefers these to baseline entries.

RSR Quality Checklist

Required

  • Tests pass: cargo test --lib server (2 passed)
  • Code is formatted: cargo fmt --check on both files
  • Linter is clean: cargo clippy --lib -D warnings
  • No banned functions added
  • SPDX license headers present (unchanged)

Testing

I ran Hypatia main's code_safety collector on echidna:

  • Without this change: the 3 findings from the failing run (unwrap_without_check in session.rs; expect_in_hot_path and panic_macro in test_fixture.rs).
  • With this change: none in either file.

🤖 Generated with Claude Code

https://claude.ai/code/session_0136P7ZnmBahbhfkFEP6ark4

…module

The Hypatia baseline gate fails with three unfiltered findings:

* code_safety/unwrap_without_check in src/rust/server/session.rs. The
  handler checked state.is_none() and then unwrapped four times. Bind
  the state with let-else instead, take the goal counts from the values
  in hand, and clear goals through if-let on QED. Behaviour is the same.
* code_safety/expect_in_hot_path and code_safety/panic_macro in
  src/rust/provers/test_fixture.rs. The module is compiled only under
  #[cfg(all(test, unix))] (provers/mod.rs); a failed fixture setup must
  fail the test. Hypatia strips only a trailing #[cfg(test)] block in
  the same file, so it cannot see this. Record that with file-level
  hypatia: allow directives rather than baseline entries.

Checked with Hypatia main's code_safety scan: the three findings are
present without this change and absent with it. cargo fmt --check and
cargo clippy --lib -D warnings pass; cargo test --lib server passes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0136P7ZnmBahbhfkFEP6ark4
@coderabbitai

coderabbitai Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 24 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 3c62e53b-22c4-40d8-904d-7734afe2ad63
Commits

Reviewing files that changed from the base of the PR and between d98b0e2 and b82bdb2.

Files selected for processing: 2

  • src/rust/provers/test_fixture.rs
  • src/rust/server/session.rs
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

Copy link
Copy Markdown
Owner Author

Containerfile reproducibility (same image ID twice) fails here, but the failure does not come from this PR:

  • The same job also fails on main: the last 3 Container Build Verification push runs on main failed, including a261ac6.
  • This PR changes only src/rust/server/session.rs and src/rust/provers/test_fixture.rs. It touches no Containerfile and no build input of the image.
  • In this run, the 2 builds match on 15 of 16 layers. Only layer 12 differs (9ab59d19… against c1c28b58…). That is a non-determinism in the image build.

No fix exists yet, and it is outside this PR's scope.


Generated by Claude Code

@hyperpolymath
hyperpolymath marked this pull request as ready for review November 10, 2026 17:55
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ❌ Failed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T17:57:29.054039Z 29c2cd8 Draft marked ready
🔒 Security Review ❌ Failed 2026-11-10T17:55:21.066180Z
We couldn’t complete this review. Comment @codex security review to try again.
29c2cd8 Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

This security review couldn’t start because the paying account has exhausted its Codex allowance and has insufficient available credits. The payer can check their balance and add credits in the Codex usage dashboard, then try again. If you do not manage the paying account, contact this repository's admins.

@hyperpolymath
hyperpolymath enabled auto-merge (squash) November 10, 2026 18:09
@sonarqubecloud

Copy link
Copy Markdown

@hyperpolymath
hyperpolymath merged commit 7cb26bf into main Nov 10, 2026
74 of 75 checks passed
@hyperpolymath
hyperpolymath deleted the claude/hypatia-baseline-findings branch November 10, 2026 18:16
hyperpolymath added a commit that referenced this pull request Nov 10, 2026
…0.2.0 (#495)

## Summary

`governance / Actions lockfile verify` fails on main (d98b0e2). Every
open PR fails it too, including #490 and #492.

The governance gate installs `gh-actions-lock` v0.2.0. That version
reports unused lockfile entries as errors (upstream
github/gh-actions-lock#159). echidna's lockfile has 8 such entries:

- job-level reusable-workflow calls in `governance`, `hypatia-scan`,
`mirror`, `scorecard`, `secret-scanner`, `security-scan`,
`spark-theatre-gate` and the SLSA publish workflow (standards,
proven-cicd, panic-attack, slsa-github-generator);
- the deleted `boj-build.yml`.

The lockfile does not track reusable workflows. Each one stays
SHA-pinned inline in its workflow file, which `workflow-linter` checks.

## Changes

- `.github/workflows/actions.lock` only. I regenerated it with
`gh-actions-lock` v0.2.0, built from `github/gh-actions-lock` at tag
`v0.2.0`, with `--no-onboard --no-narrow`.
- No workflow file and no step pin changes. A plain `gh actions-lock`
run would also have rewritten 2 bare-SHA pins to refs, one of them to
`@main`. I did not apply those rewrites.

## RSR Quality Checklist

### Required

- [x] No secrets, credentials, or `.env` files included

## Testing

- Before the change: `gh-actions-lock --verify-local` gives "8 of 34
workflows failed: 8 stale", the same entries as the CI failure.
- After the change: "All 34 workflows have complete lockfile coverage."
- The run printed "could not verify" warnings for some pins, because the
tool's GraphQL calls are blocked in this session. Those pins are
unchanged, and the runner re-checks them at job start.

---
_Generated by [Claude
Code](https://claude.ai/code/session_0136P7ZnmBahbhfkFEP6ark4)_

Co-authored-by: Claude <noreply@anthropic.com>
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.

3 participants