Fix ci failures - #319
Fix ci failures#319
Conversation
…ml advisories The previously pinned override targets have themselves become vulnerable, turning the CI `audit` job (fails on moderate+) red with 4 HIGH advisories. All four are dev-only transitive dependencies. Bumped overrides: - fast-uri: `>=3.0.0 <=3.1.3` -> `>=3.0.0 <3.1.5`, resolving to ^3.1.5 (GHSA-7p8r-x3mc-p8w7, vulnerable >=3.0.0 <3.1.5; via ajv) - brace-expansion 5.x: `>=5.0.0 <5.0.8` -> `>=4.0.0 <5.0.9`, resolving to ^5.0.9 (GHSA-rgw5-rvv9-x895, vulnerable >=4.0.0 <5.0.9; via glob>minimatch and markdownlint-cli>minimatch) - brace-expansion 2.x: `>=2.0.0 <2.1.3` -> `>=2.0.0 <2.1.4`, resolving to ^2.1.4 (GHSA-rgw5-rvv9-x895, vulnerable >=2.0.0 <2.1.4; via solidity-code-metrics>glob>minimatch) - js-yaml: `>=4.0.0 <4.3.0` -> `>=4.0.0 <4.3.1`, resolving to ^4.3.1 (GHSA-5p4m-2wfm-xmqj, vulnerable >=4.0.0 <4.3.1; via markdownlint-cli) minimumReleaseAgeExclude: - Added `js-yaml@4.3.1`. This exclusion is only needed because the patch is a few hours short of the 7-day (10080 min) window: 4.3.1 was published 2026-07-31T17:39:51Z, making it ~6d19h old at the time of this change. It can be dropped once the release clears the window. - Pruned `brace-expansion@2.1.3`, `brace-expansion@5.0.8` and `fast-uri@3.1.4`, whose pins are replaced here. The new brace-expansion@5.0.9 / @2.1.4 (2026-07-30) and fast-uri@3.1.5 (2026-07-31T09:16Z) all clear the 7-day gate on their own, verified by a clean `pnpm install`. auditConfig: - Removed the `GHSA-mh99-v99m-4gvg` ignore entry. It is obsolete: running `pnpm audit` against the pre-change lockfile with the entry removed still reports the same 4 advisories, so the entry was already suppressing nothing. Its comment also referenced brace-expansion 2.1.3, which this change replaces. `pnpm audit --audit-level moderate` now reports "No known vulnerabilities found". Validation: `pnpm audit --audit-level moderate` (0 vulnerabilities), `pnpm run lint:check` (prettier, forge lint and markdownlint all exit 0). No Solidity sources are affected by this change.
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe pull request adds and documents deprecated bridge tests, excludes them from active test and coverage commands, updates Polygon fork block handling, and refreshes pnpm dependency overrides. ChangesTest maintenance
Dependency override maintenance
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol (1)
62-71: 🩺 Stability & Availability | 🔵 TrivialUse a reproducible Polygon Amoy snapshot for CI.
This change removes the explicit Polygon block and makes
vm.createFork("polygon-amoy")use the latest Amoy state. The fixture also reads live CCIP network contracts during setup. A router or registry change can alter the result between CI runs. Pin a validated finalized Amoy block. If the provider cannot serve the required historical block, document why latest state is required and verify that the fixture does not depend on mutable state. The PR diff confirms that the explicit Polygon block was removed. (github.com)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol` around lines 62 - 71, Pin the Polygon Amoy fork in setUp using a validated finalized block constant, alongside MAINNET_BLOCK, and pass it to vm.createFork("polygon-amoy", ...). Ensure the selected snapshot supports all fixture setup reads and remains reproducible in CI; only retain latest-state behavior if you document and verify that no mutable state is required.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol`:
- Around line 62-71: Pin the Polygon Amoy fork in setUp using a validated
finalized block constant, alongside MAINNET_BLOCK, and pass it to
vm.createFork("polygon-amoy", ...). Ensure the selected snapshot supports all
fixture setup reads and remains reproducible in CI; only retain latest-state
behavior if you document and verify that no mutable state is required.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 48ae9312-b6c8-4d52-a93a-e7fcda7f9acd
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
package.jsonpnpm-workspace.yamlsrc/test/policies/CrossChainBridgeFork.t.solsrc/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol
💤 Files with no reviewable changes (1)
- src/test/policies/CrossChainBridgeFork.t.sol
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/test/deprecated/policies/CrossChainBridgeFork.t.sol (2)
39-71: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winPrefix internal state variables with
_.Rename each internal state variable and update its references.
src/test/deprecated/policies/CrossChainBridgeFork.t.sol#L39-L71: prefix the users, fork IDs, chain constants, helpers, and deployed contract references with_.src/test/deprecated/policies/CrossChainBridge.t.sol#L28-L53: prefix the users, endpoints, chain constants, and deployed contract references with_.As per coding guidelines, internal state variables must use an underscore prefix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/deprecated/policies/CrossChainBridgeFork.t.sol` around lines 39 - 71, Prefix every internal state variable with `_` and update all references consistently. In src/test/deprecated/policies/CrossChainBridgeFork.t.sol lines 39-71, rename the users, fork IDs, chain constants, helper, and deployed contract references; in src/test/deprecated/policies/CrossChainBridge.t.sol lines 28-53, rename the users, endpoints, chain constants, and deployed contract references. Keep behavior unchanged.Source: Coding guidelines
14-33: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse versioned aliases and import only the symbols from
src/Kernel.sol.
src/Kernel.solexposesActionsbeforeKernel, so these named imports cannot import both from the same source. Use the versioned package aliases, sort imports by dependency, and import the needed symbol fromsrc/Kernel.sol.
src/test/deprecated/policies/CrossChainBridgeFork.t.sol#L14-L33: use versionedforge-std/solmatealiases and importKernelby name.
src/test/deprecated/policies/CrossChainBridge.t.sol#L4-L23: use versionedforge-std/solmatealiases and import the neededActionsby name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/deprecated/policies/CrossChainBridgeFork.t.sol` around lines 14 - 33, Update imports in CrossChainBridgeFork.t.sol (14-33) to use the versioned forge-std and solmate aliases, sort them by dependency, and import Kernel explicitly from src/Kernel.sol. Apply the corresponding versioned forge-std/solmate aliases and named Actions import in CrossChainBridge.t.sol (4-23); ensure neither file relies on wildcard Kernel imports.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/deprecated/policies/CrossChainBridge.t.sol`:
- Around line 142-162: The assertions in
src/test/deprecated/policies/CrossChainBridge.t.sol:142-162 require descriptive
messages for every dependency and permission assertEq, including array lengths,
keycodes, and selectors. In
src/test/deprecated/policies/CrossChainBridge.t.sol:175-188 and
src/test/deprecated/policies/CrossChainBridgeFork.t.sol:153-177, add the
requested assertion messages and document the transfer arithmetic: OHM uses 9
decimals, show the calculation steps, and state why the recipient balance is
exactly amount_.
---
Outside diff comments:
In `@src/test/deprecated/policies/CrossChainBridgeFork.t.sol`:
- Around line 39-71: Prefix every internal state variable with `_` and update
all references consistently. In
src/test/deprecated/policies/CrossChainBridgeFork.t.sol lines 39-71, rename the
users, fork IDs, chain constants, helper, and deployed contract references; in
src/test/deprecated/policies/CrossChainBridge.t.sol lines 28-53, rename the
users, endpoints, chain constants, and deployed contract references. Keep
behavior unchanged.
- Around line 14-33: Update imports in CrossChainBridgeFork.t.sol (14-33) to use
the versioned forge-std and solmate aliases, sort them by dependency, and import
Kernel explicitly from src/Kernel.sol. Apply the corresponding versioned
forge-std/solmate aliases and named Actions import in CrossChainBridge.t.sol
(4-23); ensure neither file relies on wildcard Kernel imports.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aa9e614d-5b51-490f-bfbc-eaf4a95f5cd1
📒 Files selected for processing (6)
package.jsonshell/test_coverage.shsrc/test/deprecated/README.mdsrc/test/deprecated/policies/CrossChainBridge.t.solsrc/test/deprecated/policies/CrossChainBridgeFork.t.solsrc/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol
- package.json
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/test/deprecated/policies/CrossChainBridgeFork.t.sol (2)
39-71: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winPrefix internal state variables with
_.Rename each internal state variable and update its references.
src/test/deprecated/policies/CrossChainBridgeFork.t.sol#L39-L71: prefix the users, fork IDs, chain constants, helpers, and deployed contract references with_.src/test/deprecated/policies/CrossChainBridge.t.sol#L28-L53: prefix the users, endpoints, chain constants, and deployed contract references with_.As per coding guidelines, internal state variables must use an underscore prefix.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/deprecated/policies/CrossChainBridgeFork.t.sol` around lines 39 - 71, Prefix every internal state variable with `_` and update all references consistently. In src/test/deprecated/policies/CrossChainBridgeFork.t.sol lines 39-71, rename the users, fork IDs, chain constants, helper, and deployed contract references; in src/test/deprecated/policies/CrossChainBridge.t.sol lines 28-53, rename the users, endpoints, chain constants, and deployed contract references. Keep behavior unchanged.Source: Coding guidelines
14-33: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUse versioned aliases and import only the symbols from
src/Kernel.sol.
src/Kernel.solexposesActionsbeforeKernel, so these named imports cannot import both from the same source. Use the versioned package aliases, sort imports by dependency, and import the needed symbol fromsrc/Kernel.sol.
src/test/deprecated/policies/CrossChainBridgeFork.t.sol#L14-L33: use versionedforge-std/solmatealiases and importKernelby name.
src/test/deprecated/policies/CrossChainBridge.t.sol#L4-L23: use versionedforge-std/solmatealiases and import the neededActionsby name.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/deprecated/policies/CrossChainBridgeFork.t.sol` around lines 14 - 33, Update imports in CrossChainBridgeFork.t.sol (14-33) to use the versioned forge-std and solmate aliases, sort them by dependency, and import Kernel explicitly from src/Kernel.sol. Apply the corresponding versioned forge-std/solmate aliases and named Actions import in CrossChainBridge.t.sol (4-23); ensure neither file relies on wildcard Kernel imports.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/test/deprecated/policies/CrossChainBridge.t.sol`:
- Around line 142-162: The assertions in
src/test/deprecated/policies/CrossChainBridge.t.sol:142-162 require descriptive
messages for every dependency and permission assertEq, including array lengths,
keycodes, and selectors. In
src/test/deprecated/policies/CrossChainBridge.t.sol:175-188 and
src/test/deprecated/policies/CrossChainBridgeFork.t.sol:153-177, add the
requested assertion messages and document the transfer arithmetic: OHM uses 9
decimals, show the calculation steps, and state why the recipient balance is
exactly amount_.
---
Outside diff comments:
In `@src/test/deprecated/policies/CrossChainBridgeFork.t.sol`:
- Around line 39-71: Prefix every internal state variable with `_` and update
all references consistently. In
src/test/deprecated/policies/CrossChainBridgeFork.t.sol lines 39-71, rename the
users, fork IDs, chain constants, helper, and deployed contract references; in
src/test/deprecated/policies/CrossChainBridge.t.sol lines 28-53, rename the
users, endpoints, chain constants, and deployed contract references. Keep
behavior unchanged.
- Around line 14-33: Update imports in CrossChainBridgeFork.t.sol (14-33) to use
the versioned forge-std and solmate aliases, sort them by dependency, and import
Kernel explicitly from src/Kernel.sol. Apply the corresponding versioned
forge-std/solmate aliases and named Actions import in CrossChainBridge.t.sol
(4-23); ensure neither file relies on wildcard Kernel imports.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aa9e614d-5b51-490f-bfbc-eaf4a95f5cd1
📒 Files selected for processing (6)
package.jsonshell/test_coverage.shsrc/test/deprecated/README.mdsrc/test/deprecated/policies/CrossChainBridge.t.solsrc/test/deprecated/policies/CrossChainBridgeFork.t.solsrc/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol
🚧 Files skipped from review as they are similar to previous changes (2)
- src/test/policies/bridge/CCIPBurnMintTokenPoolFork.t.sol
- package.json
🛑 Comments failed to post (1)
src/test/deprecated/policies/CrossChainBridge.t.sol (1)
142-162: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add assertion messages and transfer arithmetic working.
Add a descriptive message to every
assertEq. Before each transfer-balance assertion, document the 9-decimal OHM scale, arithmetic steps, and expected recipient balance.
src/test/deprecated/policies/CrossChainBridge.t.sol#L142-L162: add descriptive messages to each dependency and permission assertion.src/test/deprecated/policies/CrossChainBridge.t.sol#L175-L188: add an assertion message and document why the recipient receives exactlyamount_.src/test/deprecated/policies/CrossChainBridgeFork.t.sol#L153-L177: add an assertion message and document why the recipient receives exactlyamount_.As per coding guidelines, all assertions require descriptive messages, and mathematical assertions require documented working.
📍 Affects 2 files
src/test/deprecated/policies/CrossChainBridge.t.sol#L142-L162(this comment)src/test/deprecated/policies/CrossChainBridge.t.sol#L175-L188src/test/deprecated/policies/CrossChainBridgeFork.t.sol#L153-L177🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/deprecated/policies/CrossChainBridge.t.sol` around lines 142 - 162, The assertions in src/test/deprecated/policies/CrossChainBridge.t.sol:142-162 require descriptive messages for every dependency and permission assertEq, including array lengths, keycodes, and selectors. In src/test/deprecated/policies/CrossChainBridge.t.sol:175-188 and src/test/deprecated/policies/CrossChainBridgeFork.t.sol:153-177, add the requested assertion messages and document the transfer arithmetic: OHM uses 9 decimals, show the calculation steps, and state why the recipient balance is exactly amount_.Source: Coding guidelines
2 participants
Summary by CodeRabbit
Testing
Documentation
Maintenance