💚 Fixing CI error report #212
Conversation
WalkthroughThis pull request updates GitHub Actions workflows to use checkout v5, enhances Bolt CLI test error handling with result propagation and context wrapping, adds system resource limits to the test workflow, enables previously-skipped tests in C# and TypeScript, removes unused test bindings, and renames the workspace member "component-small" to "small" across all examples. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant BoltTest as bolt-cli test
participant AnchorCLI as anchor_cli::entry
participant Validator as EphemeralValidator
participant Output as Output
Caller->>BoltTest: invoke test command
BoltTest->>AnchorCLI: spawn task (anchor_cli::entry)
BoltTest->>Validator: start with await
Validator-->>Validator: initialize
AnchorCLI-->>BoltTest: returns Result<T>
Validator-->>BoltTest: returns Result (await)
alt Validator Success & AnchorCLI Success
BoltTest->>Output: "Bolt test succeeded"
BoltTest-->>Caller: Ok(())
else Validator Failure
BoltTest->>Output: "Bolt test failed: {error}"
BoltTest-->>Caller: Err(Context)
else AnchorCLI Failure
BoltTest->>Output: "Bolt test failed: {error}"
BoltTest-->>Caller: Err(Context)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Rationale: The diff spans multiple distinct change categories—workflow upgrades (homogeneous, low-effort), error propagation logic (moderate complexity), test activation/control flow (moderate complexity), and a large but highly repetitive workspace rename pattern (9 example files, 18 instances of the same mechanical transformation). While the renaming appears voluminous, the repetition reduces cognitive load. The logic changes in Bolt CLI require careful attention to error handling semantics. Mixed complexity across categories warrants moderate review effort. Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/bolt-cli/src/commands/test.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/bolt-cli/src/commands/test.rs (2)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)crates/bolt-cli/src/ephemeral_validator.rs (1)
start(31-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
🔇 Additional comments (2)
crates/bolt-cli/src/commands/test.rs (2)
17-19: LGTM! Correct fix for CI failure detection.Propagating the test result ensures CI workflows can detect failures via exit codes. This addresses the PR objective of fixing silent failures.
21-27: Approve with connection timing check. Validator lifecycle and error handling look correct; verify that the anchor task’s early spawn won’t attempt connections before the validator is ready.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/bolt-cli/src/commands/test.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/bolt-cli/src/commands/test.rs (2)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)crates/bolt-cli/src/ephemeral_validator.rs (1)
start(31-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: setup
🔇 Additional comments (1)
crates/bolt-cli/src/commands/test.rs (1)
12-20: Properly propagates anchor test results.The spawned task now returns the
Result<()>fromanchor_cli::entry, ensuring test failures are no longer silently ignored. This is the key fix for the CI issue described in the PR.
notdanilo
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/framework.ts(0 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)
💤 Files with no reviewable changes (1)
- clients/typescript/test/framework.ts
🧰 Additional context used
🧬 Code graph analysis (2)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: setup
🔇 Additional comments (2)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-26: LGTM! Test reactivation looks correct.The uncommented test block properly runs the accelerator test 10 times through the Profiler.Run wrapper, which is consistent with the PR objective of fixing CI test execution.
clients/typescript/test/intermediate-level/acceleration.ts (1)
67-97: Verify transaction submission parameters for accelerator testing.
skipPreflight: true is used consistently across tests; the initial accelerator call uses commitment "confirmed" (line 58) but the 10× loop drops to "processed" (line 91). Confirm that this lower confirmation level is acceptable for test reliability and CI.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/framework.ts(0 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)clients/typescript/test/intermediate-level/ecs.ts(1 hunks)
💤 Files with no reviewable changes (1)
- clients/typescript/test/framework.ts
🧰 Additional context used
🧬 Code graph analysis (3)
clients/typescript/test/intermediate-level/ecs.ts (2)
clients/typescript/src/world/transactions.ts (3)
AddEntity(300-335)InitializeComponent(398-432)ApplySystem(545-576)clients/typescript/test/framework.ts (1)
report(99-124)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: lint (clippy)
- GitHub Check: lint (yarn)
🔇 Additional comments (2)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-26: LGTM! Test activation aligns with CI improvements.The previously commented-out profiler block is now active, which will help surface acceleration-related issues in CI. The test structure is correct and mirrors the TypeScript equivalent.
Note: The past review comment about a TODO on line 24 appears outdated, as no TODO is present in the current code.
clients/typescript/test/intermediate-level/acceleration.ts (1)
67-97: Verify skipPreflight usage and consider adding assertions.The test activation looks good and mirrors the C# equivalent. However, please verify:
skipPreflight: true - This bypasses transaction simulation, which can hide errors. Is this intentional for acceleration testing, or should preflight checks be enabled?
No assertions - The test runs 10 iterations but doesn't verify that the system applications succeeded or that the position component was updated. Consider adding a final assertion to confirm the expected state.
Example assertion to add after the loop:
const position = await framework.exampleComponentPosition.account.position.fetch( framework.acceleratedComponentPositionPda, ); expect(position.y.toNumber()).to.equal(10); // Should be 10 after 10 "Up" movements
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
crates/bolt-cli/src/commands/test.rs(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/bolt-cli/src/commands/test.rs (2)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)crates/bolt-cli/src/ephemeral_validator.rs (1)
start(31-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: setup
🔇 Additional comments (2)
crates/bolt-cli/src/commands/test.rs (2)
2-2: LGTM!The
Contexttrait import is correctly added to support the.context()usage on line 34.
12-20: LGTM!The spawned task now correctly propagates the result from
anchor_cli::entry(opts), which aligns with the PR objective to surface test failures in CI workflows.
| // Keep the validator alive while tests run by retaining the handle | ||
| // in scope, and return the actual test result. | ||
| return anchor | ||
| .await | ||
| .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e))); |
There was a problem hiding this comment.
Inconsistent error handling and status reporting between code paths.
The ephemeral validator path is missing:
- The
.context("Bolt test failed")that is applied on line 34 in the non-ephemeral path - The status printing (lines 35-39) that appears in the non-ephemeral path
This creates divergent behavior where errors from the ephemeral validator path have less context and no status output, making debugging harder.
To ensure consistent behavior, apply this diff:
- // Keep the validator alive while tests run by retaining the handle
- // in scope, and return the actual test result.
- return anchor
- .await
- .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e)));
+ // Keep the validator alive while tests run by retaining the handle
+ // in scope, and return the actual test result.
+ let result = anchor
+ .await
+ .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e)))
+ .context("Bolt test failed");
+ if result.is_err() {
+ println!("Something went wrong");
+ } else {
+ println!("Bolt test completed successfully");
+ }
+ return result;Alternatively, consider refactoring both paths to use a shared helper function that applies context and prints status, addressing both this issue and the duplication flagged in past reviews.
🤖 Prompt for AI Agents
In crates/bolt-cli/src/commands/test.rs around lines 23 to 27, the ephemeral
validator return path unwraps the anchor result directly and therefore lacks the
.context("Bolt test failed") error context and the status printing performed in
the non-ephemeral path (lines ~34–39); update this branch so that after awaiting
anchor you apply the same .context("Bolt test failed") to the Result and invoke
the same status-printing logic (or call the shared helper) before returning, or
refactor both branches to a common helper that wraps the anchor result with
context and prints status to ensure consistent error context and output.
6e0085c to
d42d9b0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/script.sh(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: setup
notdanilo
force-pushed
the
ci/fixing-error-report
branch
2 times, most recently
from
6675aca to
455062d
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/run-tests.yml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)tests/script.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
🔇 Additional comments (3)
.github/workflows/run-tests.yml (2)
71-78: Good addition of strace for debugging, but clarify ephemeral-validator status.Installing
straceand collecting logs will help diagnose startup failures, aligning well with the PR objective to surface errors. However, commenting out the ephemeral-validator installation (lines 76–78) creates ambiguity, especially given the comment on line 106 that references "being able to run ephemeral-validator test process."Verify whether:
- Ephemeral-validator is truly no longer needed, or if the installation step should remain active
- The comment on line 106 should be updated or removed if ephemeral-validator is not installed
If the validator is disabled intentionally, update or remove the comment on line 106 to avoid confusion.
113-118: LGTM—artifact upload aids troubleshooting.Uploading the strace log as a build artifact (even on failure with
if: always()) is a solid debugging improvement that directly supports the PR objective of surfacing CI errors.clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-29: The review comment mischaracterizes the PR objective.The review claims the PR goal is to "surface and report errors," but commit c62617f—the actual PR (#169)—is explicitly titled "Disabling acceleration test." The runtime guard is not contradicting the PR's intent; it is the PR's intent.
The recent commit 455062d ("Debugging os error 2") shows later investigation into the root cause (with strace), but that's a separate effort from PR #169's goal of disabling the test in CI due to ephemeral-validator infrastructure issues.
Likely an incorrect or invalid review comment.
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
455062d to
b406ff7
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
clients/typescript/test/intermediate-level/acceleration.ts (1)
67-99: This issue was already flagged in a previous review.The runtime guard that skips this test in CI contradicts the PR's stated objective of surfacing errors. Please see the previous review comment on these lines for full details.
.github/workflows/run-tests.yml (1)
106-109: This issue was already flagged in a previous review.The comment references ephemeral-validator but it's not installed. Please see the previous review comment on these lines for suggested resolutions.
tests/script.sh (1)
12-12: This change was already approved in a previous review.The subshell consolidation is clean and maintains proper error handling. No concerns.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/run-tests.yml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)tests/script.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: setup
🔇 Additional comments (1)
.github/workflows/run-tests.yml (1)
113-118: Verify that strace captures meaningful diagnostic information.The strace collection and upload is a good step toward debugging, but ensure it will capture enough information to diagnose the ephemeral-validator startup failures once the
|| trueis removed and actual failures are surfaced.Consider verifying that the strace flags
-e trace=execve,%filewill capture the relevant failure modes. You may want to review the uploaded strace logs from the referenced failing run to confirm this approach will surface the root cause.
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
b406ff7 to
0346f1c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
tests/script.sh (1)
8-14: Critical: Tests remain completely disabled, contradicting PR objective.This is still the same critical issue raised in previous reviews. The PR states the goal is to "surface and report the error so failures are visible," yet all test execution lines remain commented out (8-9, 11-12). The script now only sleeps for 5 seconds and prints a message—no tests run, no errors can be surfaced.
This completely contradicts the stated PR objective.
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-29: Critical: Runtime guard still hides test failures in CI.This is the same critical issue raised in previous reviews. The runtime environment check causes the test to be skipped entirely when
GITHUB_ACTIONS="true", which means any failures in this test are hidden rather than surfaced in CI.This directly contradicts the PR's stated objective to "surface and report the error so failures are visible."
clients/typescript/test/intermediate-level/acceleration.ts (1)
67-99: Critical: Runtime guard hides test failures in CI.This is the same pattern flagged in previous reviews. The runtime environment check skips this test entirely when
process.env.GITHUB_ACTIONS === "true", which means any failures are hidden rather than surfaced.This directly contradicts the PR's stated objective to "surface and report the error so failures are visible."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/run-tests.yml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)tests/script.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
🪛 YAMLlint (1.37.1)
.github/workflows/run-tests.yml
[error] 120-120: too many blank lines (1 > 0)
(empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: setup
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
0346f1c to
fa75736
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (4)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-29: Critical: This runtime guard still contradicts the PR objective by hiding test failures in CI.The PR explicitly states the goal is to "surface and report the error so failures are visible," but this guard skips the test entirely when running in GitHub Actions CI. This hides failures rather than surfacing them.
If the ephemeral-validator prevents this test from running in CI, the proper approach is either:
- Use the test framework's skip/ignore attribute (e.g.,
[Ignore("Ephemeral-validator failing in CI - tracked in issue #XXX")]) so skips are visible in test reports, or- Fix the underlying ephemeral-validator startup issue so the test can actually run and surface failures
Runtime environment checks make test skips invisible in CI reports and harder to discover.
clients/typescript/test/intermediate-level/acceleration.ts (1)
67-99: Runtime guard still contradicts the PR objective.This CI guard skips the test in GitHub Actions, which hides failures rather than surfacing them—directly contradicting the PR's stated goal to "surface and report the error so failures are visible."
The previous review comment on these lines comprehensively identified the same concerns:
- Root cause (ephemeral-validator startup) remains unaddressed
- Runtime environment checks are less discoverable than test framework mechanisms
- No inline explanation for the CI bypass
The suggestion to use Mocha's
.skip()with an explanatory comment remains valid.crates/bolt-cli/src/commands/test.rs (1)
35-36: Use stderr for error output and improve message specificity.Two issues with the error reporting:
- The message "Something went wrong" is too generic to aid debugging
- Error output should go to stderr, not stdout
Apply this diff:
if result.is_err() { - println!("Something went wrong"); + eprintln!("Bolt test failed"); } else {Note: The detailed error will already be displayed by the caller when the
Resultis unwrapped, so a brief stderr message indicating failure is sufficient. If you need more detail here, useeprintln!("Bolt test failed: {:?}", result.as_ref().unwrap_err());Based on learnings
.github/workflows/run-tests.yml (1)
120-120: Remove extra blank line.Static analysis flagged too many blank lines here. Remove the redundant empty line to satisfy YAML linting.
path: logs/bolt-test.strace.log -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/run-tests.yml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)crates/bolt-cli/src/commands/test.rs(2 hunks)tests/script.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
crates/bolt-cli/src/commands/test.rs (1)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)
🪛 YAMLlint (1.37.1)
.github/workflows/run-tests.yml
[error] 120-120: too many blank lines (1 > 0)
(empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: setup
🔇 Additional comments (1)
crates/bolt-cli/src/commands/test.rs (1)
2-2: Error propagation fix addresses the PR objective.The addition of
Contextand the change from.ok()to.context("Failed to run anchor")ensures that test failures are properly surfaced instead of being silently swallowed, directly addressing the CI silent failure issue described in the PR.Also applies to: 17-19
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
fa75736 to
b31d831
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (5)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-29: Critical: Test skip contradicts PR objective to surface errors.This conditional skip hides test failures in CI rather than surfacing them, directly contradicting the PR's stated goal to "surface and report the error so failures are visible."
Either fix the underlying ephemeral-validator startup issue so the test can run, or use the test framework's skip/ignore mechanism (e.g.,
[Ignore("reason")]) to make the skip explicit and discoverable.clients/typescript/test/intermediate-level/acceleration.ts (1)
67-99: Environment guard still contradicts PR objective.This conditional skip mirrors the concerns raised in the previous review. While re-enabling this test is a step forward, the runtime guard hides CI failures rather than surfacing them—directly contradicting the PR's stated goal: "Update CI to surface and report the error so failures are visible."
The fundamental issues remain:
- Root cause (ephemeral-validator startup) is unaddressed
- Runtime checks are less discoverable than Mocha's
.skip()with an explanatory comment- No inline documentation explaining the CI bypass
Consider one of these alternatives:
- Use
it.skip("Apply Simple Movement...", ...)with a comment referencing a tracking issue- Fix the underlying ephemeral-validator initialization instead of bypassing the test
- At minimum, add an inline comment explaining why CI must skip this test
tests/script.sh (1)
8-12: ** Tests remain commented out—contradicts PR objective to surface errors.**This issue was previously flagged as critical in prior reviews: commenting out test execution (lines 8–9 and 11–12) directly contradicts the PR objective to "surface and report the error so failures are visible." Disabling tests prevents infrastructure startup failures from being detected and reported.
To complete the fix as identified in prior feedback:
- Uncomment the test invocations (lines 8–9 and 11–12)
- Add explicit validation of any service startup (e.g., ephemeral-validator or other infrastructure) before running tests, with a timeout and clear error messages on failure
- Ensure test failures propagate with non-zero exit codes so CI workflows can detect and report them
The current state silently hides the very failures this PR aims to surface.
.github/workflows/run-tests.yml (1)
120-120: Remove extra blank line to satisfy static analysis.YAMLlint flags too many blank lines at line 120.
Apply this diff:
path: logs/bolt-test.strace.log -crates/bolt-cli/src/commands/test.rs (1)
35-39: Improve error message specificity and use stderr for errors.The generic message
"Something went wrong"on line 36 provides no useful debugging information. Additionally, error messages should be written to stderr, not stdout.Apply this diff to improve the error reporting:
if result.is_err() { - println!("Something went wrong"); + eprintln!("Bolt test failed: {:?}", result.as_ref().unwrap_err()); } else { println!("Bolt test completed successfully"); }Alternatively, since the error is being returned and will be displayed by the caller, consider removing the error print entirely to avoid duplicate error output.
Based on past review comments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/run-tests.yml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)crates/bolt-cli/src/commands/test.rs(2 hunks)tests/script.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
crates/bolt-cli/src/commands/test.rs (1)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)
🪛 YAMLlint (1.37.1)
.github/workflows/run-tests.yml
[error] 120-120: too many blank lines (1 > 0)
(empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test
- GitHub Check: setup
crates/bolt-cli/src/commands/test.rs
Outdated
| // if !skip_local_validator { | ||
| // if let Ok(_ephemeral_validator) = EphemeralValidator::start().await { | ||
| // // Keep the validator alive while tests run by retaining the handle | ||
| // // in scope, and return the actual test result. | ||
| // return anchor | ||
| // .await | ||
| // .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e))); | ||
| // } | ||
| // } |
There was a problem hiding this comment.
🧩 Analysis chain
Commented-out code should be removed or tracked with a TODO.
The ephemeral validator block is entirely commented out, which:
- Disables the local validator functionality for tests
- Leaves the
skip_local_validatorparameter unused (line 10) - Creates technical debt
If this is temporary while debugging the validator startup issue mentioned in the PR description, add a TODO comment with a tracking issue number and consider using #[allow(unused_variables)] or prefixing the parameter with _skip_local_validator. If the validator functionality is being permanently removed, delete this commented block and remove the parameter from the function signature and all call sites.
Would you like me to open an issue to track this technical debt?
Based on past review comments.
I'll verify the claims in the review comment by inspecting the actual codebase.
---
🏁 Script executed:
```shell
#!/bin/bash
# Check the exact content of the file around lines 21-29
echo "=== Lines 21-29 of test.rs ==="
sed -n '21,29p' crates/bolt-cli/src/commands/test.rs
echo -e "\n=== Full context around lines 10-35 ==="
sed -n '1,35p' crates/bolt-cli/src/commands/test.rs
Length of output: 1887
🏁 Script executed:
#!/bin/bash
# Search for usage of skip_local_validator in the test.rs file
echo "=== Searching for skip_local_validator usage ==="
rg -n 'skip_local_validator' crates/bolt-cli/src/commands/test.rs -A2 -B2Length of output: 510
🏁 Script executed:
#!/bin/bash
# Check if there are any other files referencing skip_local_validator
echo "=== Checking for skip_local_validator in all files ==="
rg -n 'skip_local_validator' --type rustLength of output: 389
---
🏁 Script executed:
```shell
#!/bin/bash
# Check context in lib.rs where skip_local_validator is being passed
echo "=== Context around lib.rs lines 140-150 ==="
sed -n '140,150p' crates/bolt-cli/src/lib.rs
echo -e "\n=== Full context of the commands::test call ==="
sed -n '135,155p' crates/bolt-cli/src/lib.rs
Length of output: 1101
🏁 Script executed:
#!/bin/bash
# Check where skip_local_validator originates from in lib.rs
echo "=== Search for skip_local_validator definition/origin in lib.rs ==="
rg -n 'skip_local_validator' crates/bolt-cli/src/lib.rs -B5 -A5Length of output: 538
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for TODO/FIXME comments related to validator or ephemeral
echo "=== Searching for TODO/FIXME about validator/ephemeral ==="
rg -n '(TODO|FIXME|XXX).*?(validator|ephemeral)' --type rust -i
echo -e "\n=== Checking test.rs for any TODO comments ==="
cat crates/bolt-cli/src/commands/test.rs | grep -n 'TODO\|FIXME\|XXX\|HACK'
Length of output: 106
🏁 Script executed:
#!/bin/bash
# Check if there are comments explaining the commented-out code
echo "=== Full content of test.rs to see all comments ==="
cat crates/bolt-cli/src/commands/test.rsLength of output: 1564
Add a TODO comment or suppress the unused parameter to track this refactoring.
The review findings are verified and accurate: the ephemeral validator block is commented out (lines 21-29), and skip_local_validator is unused in the active code path. However, there are no TODO or FIXME comments documenting this temporary state, leaving the technical debt untracked.
To address this:
- Add a TODO comment explaining the validator refactoring and when it should be revisited, or
- Use
#[allow(unused_variables)]on the parameter with an explanatory comment, or - If this refactoring is permanent, remove the parameter from the function signature and update the corresponding destructuring in lib.rs (line 144-146)
The parameter cannot be simply ignored since it's part of the anchor_cli::Command::Test enum structure passed from lib.rs.
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
b31d831 to
196a6d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (6)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-29: Critical: Runtime guard contradicts PR objective and creates inconsistent test behavior.The past review comment correctly identified that this guard contradicts the PR's stated goal of surfacing CI failures. Additionally, selectively gating only this test while leaving the three tests above (lines 15-23) ungated creates inconsistency. If the ephemeral-validator startup issue affects this test, it should affect
AddAccelerationEntity,InitializePositionComponentOnAccelerationEntity, andDelegateComponentas well, since they all interact with the sameframeworkand validator infrastructure.Either:
- All four tests should be gated (if all depend on the problematic validator), or
- None should be gated (and the underlying validator issue should be fixed per PR objectives), or
- Use the test framework's skip/ignore attribute with a tracking issue link
The selective gating suggests these tests may have different failure modes that need investigation.
clients/typescript/test/intermediate-level/acceleration.ts (1)
67-99: Runtime guard contradicts the PR objective by hiding failures in CI.The conditional check
process.env.GITHUB_ACTIONS !== "true"skips this test entirely in CI, which directly contradicts the PR's stated objective to "surface and report the error so failures are visible." Instead of fixing the underlying issue (ephemeral-validator startup), this change simply hides the failing test.This approach has several problems:
- Masks failures: Tests that fail due to the validator issue are silently skipped in CI, providing false confidence
- Low discoverability: Runtime checks are less visible than test framework mechanisms (e.g., Mocha's
.skip()with a comment explaining why)- Root cause unresolved: The ephemeral-validator startup issue remains unfixed
- Coverage gap: CI no longer validates this functionality
Based on learnings.
Consider these alternatives:
Option A (Recommended): Fix the ephemeral-validator issue so this test can run in CI
- Address the root cause in the workflow (see comments on
.github/workflows/run-tests.yml)- Remove the runtime guard once the validator works
Option B: Make the skip explicit and tracked
- if (process.env.GITHUB_ACTIONS !== "true") { - it("Apply Simple Movement System (Up) on Entity 1 on Accelerator 10 times", async () => { + it.skip("Apply Simple Movement System (Up) on Entity 1 on Accelerator 10 times", async () => { + // TODO: Re-enable once ephemeral-validator startup is fixed (issue #XXX)This makes the skip visible in test output and provides context.
Option C: Open a tracking issue
If the validator fix will take time, create an issue to track re-enabling these tests and reference it in a comment.crates/bolt-cli/src/commands/test.rs (2)
21-29: Commented-out ephemeral validator code leaves technical debt and an unused parameter.The entire ephemeral validator block is commented out, which:
- Disables local validator functionality for tests
- Makes the
skip_local_validatorparameter unused (line 10)- Creates technical debt without tracking
While this aligns with the PR's approach of disabling problematic components, it doesn't help "surface and report" errors as stated in the PR objective.
Based on learnings.
If this is temporary while the validator issue is being fixed, add a TODO comment:
+ // TODO: Re-enable ephemeral validator once startup issue is resolved + // Tracked in issue #XXX // if !skip_local_validator {If the validator functionality is being permanently removed:
- Delete this commented block
- Remove the
skip_local_validatorparameter from this function signature (line 10)- Update the call site in
crates/bolt-cli/src/lib.rs(around line 147) to not pass this parameter- Update
anchor_cli::Command::Testdestructuring if neededWould you like me to generate a script to verify all references to
skip_local_validatoracross the codebase?
35-36: Generic error message provides no diagnostic value.Line 36 prints
"Something went wrong", which is too vague to be useful for debugging. The actual error is already being returned (line 40), so this print statement adds little value.Choose one approach:
Option A (Recommended): Remove the print statement
- if result.is_err() { - println!("Something went wrong"); - } else { + if result.is_ok() { println!("Bolt test completed successfully"); }Let the caller handle displaying the error (which they will when the Result is returned).
Option B: Print the actual error details
if result.is_err() { - println!("Something went wrong"); + eprintln!("Bolt test failed: {:?}", result.as_ref().unwrap_err()); } else {This provides diagnostic value and uses
eprintln!for error output..github/workflows/run-tests.yml (2)
120-120: Remove extra blank line.Static analysis detected an extra blank line here.
Apply this diff:
path: logs/bolt-test.strace.log -
114-120: Artifact upload will fail silently because strace logs are never created.The upload step expects
logs/bolt-test.strace.log, but this file will never exist because:
- The
mkdir -p logscommand is commented out (line 110)- The strace invocation is commented out (line 111)
The default behavior of
upload-artifactwhen no files are found is to output a warning (not fail), so this step will silently succeed without uploading anything—the opposite of surfacing errors.Apply one of these fixes:
Fix 1: Enable strace tracing so logs are actually created
- Uncomment lines 106-111
- Ensure this step runs even on test failure by keeping
if: always()Fix 2: Make the upload fail when logs are missing
- name: Upload strace log if: always() uses: actions/upload-artifact@v4 with: name: bolt-test-strace-log path: logs/bolt-test.strace.log + if-no-files-found: errorFix 3: Remove this step entirely if strace diagnostic logging is not needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/run-tests.yml(2 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)crates/bolt-cli/src/commands/test.rs(2 hunks)tests/script.sh(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
crates/bolt-cli/src/commands/test.rs (1)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(545-576)
🪛 YAMLlint (1.37.1)
.github/workflows/run-tests.yml
[error] 120-120: too many blank lines (1 > 0)
(empty-lines)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: setup
- GitHub Check: setup
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
196a6d8 to
36003aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/publish-bolt-crates.yml(1 hunks).github/workflows/publish-bolt-sdk.yml(1 hunks).github/workflows/run-tests.yml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: test
🔇 Additional comments (1)
.github/workflows/run-tests.yml (1)
101-113: Clarify the semantics and intent of exit code 2 handling.The code treats bolt test exit code 2 as non-fatal, logs a message, and continues the workflow. However, the meaning of exit code 2 is not documented, making it unclear whether this is the correct behavior or if it should instead fail the job.
The PR objective states the goal is to "surface and report the error so failures are visible." The current approach logs the error (visible in CI logs) but the step still succeeds (status is green). This is an improvement over silent failure, but verify that:
- Exit code 2 actually represents the expected ephemeral-validator issue (or skipped tests, etc.)
- Non-failing but logged behavior aligns with the PR intent—i.e., is surfacing to logs sufficient, or should CI fail when exit code 2 occurs?
Consider adding a more descriptive comment explaining why exit code 2 is treated specially.
notdanilo
force-pushed
the
ci/fixing-error-report
branch
from
36003aa to
b8b6e63
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
clients/typescript/test/intermediate-level/ecs.ts (1)
273-342: The previous review's suggestion to use Mocha's skip feature remains unaddressed.This implementation uses a manual environment variable check to conditionally skip the test, which was already flagged in the previous review. The suggested approach using Mocha's
it.skipmechanism provides clearer test reporting and consistent test counts between local and CI environments.crates/bolt-cli/src/commands/test.rs (1)
36-36: Generic error message provides limited debugging value.The message "Something went wrong" is too generic to be useful. Since the error is already being returned and will be displayed by the caller, consider either:
- Removing this print statement entirely, or
- Printing the actual error:
eprintln!("Bolt test failed: {:?}", result.as_ref().unwrap_err())Apply this diff to improve the error message:
if result.is_err() { - println!("Something went wrong"); + eprintln!("Bolt test failed: {:?}", result.as_ref().unwrap_err()); } else {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/publish-bolt-crates.yml(3 hunks).github/workflows/publish-bolt-sdk.yml(3 hunks).github/workflows/publish-packages.yml(3 hunks).github/workflows/run-lint.yml(2 hunks).github/workflows/run-tests.yml(3 hunks)clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs(1 hunks)clients/typescript/test/framework.ts(0 hunks)clients/typescript/test/intermediate-level/acceleration.ts(1 hunks)clients/typescript/test/intermediate-level/ecs.ts(1 hunks)crates/bolt-cli/src/commands/test.rs(2 hunks)
💤 Files with no reviewable changes (1)
- clients/typescript/test/framework.ts
🧰 Additional context used
🧬 Code graph analysis (4)
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
clients/csharp/Solana.Unity.Bolt.Test/Profiler.cs (1)
Profiler(3-12)
clients/typescript/test/intermediate-level/acceleration.ts (1)
clients/typescript/src/world/transactions.ts (1)
ApplySystem(539-570)
crates/bolt-cli/src/commands/test.rs (2)
crates/bolt-cli/src/lib.rs (1)
entry(119-210)crates/bolt-cli/src/ephemeral_validator.rs (1)
start(31-50)
clients/typescript/test/intermediate-level/ecs.ts (2)
clients/typescript/src/world/transactions.ts (3)
AddEntity(296-331)InitializeComponent(393-428)ApplySystem(539-570)clients/typescript/test/framework.ts (1)
report(99-124)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
🔇 Additional comments (10)
.github/workflows/publish-bolt-crates.yml (1)
17-17: LGTM! Consistent checkout action upgrade across the workflow.The upgrade from
actions/checkout@v4tov5is applied consistently across all three jobs (install, clippy-lint, test-and-publish). This aligns with the repository-wide upgrade pattern across other workflows in this PR.Also applies to: 74-74, 95-95
.github/workflows/run-tests.yml (3)
17-17: LGTM! Checkout action upgrade aligns with repository-wide pattern.The upgrade to
actions/checkout@v5is applied consistently in both the setup and test jobs, matching the upgrade pattern across all workflows in this PR.Also applies to: 54-54
71-73: Good addition! Ephemeral validator installation enables proper test execution.Installing the ephemeral validator directly addresses the PR objective to surface errors rather than hide them. This is a significant improvement over previous approaches that skipped or disabled validator-dependent tests.
101-105: LGTM! Resource limits support stable test execution.The
prlimitandsysctladjustments increase file descriptor and inotify limits, which are necessary for running the ephemeral validator and bolt tests reliably in CI. The comment clearly explains the purpose..github/workflows/publish-packages.yml (1)
67-67: LGTM! Consistent checkout action upgrade.The upgrade to
actions/checkout@v5is applied uniformly across all three jobs, maintaining consistency with the repository-wide upgrade pattern.Also applies to: 190-190, 217-217
.github/workflows/run-lint.yml (1)
17-17: LGTM! Checkout action upgrade matches repository-wide pattern.The upgrade to
actions/checkout@v5is consistently applied in both the setup and lint jobs.Also applies to: 57-57
clients/csharp/Solana.Unity.Bolt.Test/AccelerationTest.cs (1)
24-26: LGTM! Test activation aligns with PR objective.Uncommenting and wrapping the accelerator test in
Profiler.Runenables the test to execute in CI, which directly supports the PR goal of surfacing errors instead of hiding them. The test will now run its 10 iterations and report any failures.clients/typescript/test/intermediate-level/acceleration.ts (1)
67-97: LGTM! Test activation supports error visibility in CI.Uncommenting this test enables it to run in CI, which directly supports the PR's objective to surface errors. The test properly exercises the accelerator functionality with appropriate transaction options (
skipPreflight: true,commitment: "processed"), and the 50ms delay between iterations prevents overwhelming the system..github/workflows/publish-bolt-sdk.yml (1)
17-17: LGTM! Consistent checkout action upgrade.The upgrade to
actions/checkout@v5is uniformly applied across install, yarn-lint, and test jobs, maintaining consistency with the repository-wide upgrade.Also applies to: 68-68, 100-100
crates/bolt-cli/src/commands/test.rs (1)
2-2: LGTM! Proper error propagation enables CI to detect test failures.Adding
Contextto imports and propagating the result ofanchor_cli::entry(opts)(instead of discarding it) is critical for the PR's objective. This ensures that test failures produce non-zero exit codes, making failures visible in CI workflows.Also applies to: 17-19
| return anchor | ||
| .await | ||
| .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e))); |
There was a problem hiding this comment.
Inconsistent error handling and status reporting between code paths.
The ephemeral validator path (lines 25-27) returns early without applying the error context or status printing that the non-ephemeral path uses (lines 31-40). This creates divergent behavior:
- Missing error context: The ephemeral path doesn't apply
.context("Bolt test failed")like line 34 - No status output: The ephemeral path skips the success/failure messages (lines 35-39)
This makes debugging harder when tests fail with the ephemeral validator.
Apply this diff to ensure consistent behavior:
if let Ok(_ephemeral_validator) = EphemeralValidator::start().await {
// Keep the validator alive while tests run by retaining the handle
// in scope, and return the actual test result.
- return anchor
+ let result = anchor
.await
- .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e)));
+ .unwrap_or_else(|e| Err(anyhow::anyhow!("Failed to run anchor: {}", e)))
+ .context("Bolt test failed");
+ if result.is_err() {
+ println!("Something went wrong");
+ } else {
+ println!("Bolt test completed successfully");
+ }
+ return result;
}
}Also applies to: 31-40
🤖 Prompt for AI Agents
In crates/bolt-cli/src/commands/test.rs around lines 25-27 (and likewise ensure
lines 31-40 follow the same pattern), replace the early-return anchor.await path
with logic that captures the Result into a variable, apply .context("Bolt test
failed") to any Err, and then call the same status-printing code used by the
non-ephemeral branch (print success message on Ok and print/fail on Err) before
propagating the error; in short, stop returning immediately from the ephemeral
validator branch, wrap errors with .context("Bolt test failed"), run the
existing success/failure status output, and then return the (possibly
contextualized) Result so both code paths behave identically.
2f697d3 to
819a5aa
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/run-tests.yml(3 hunks).gitmodules(1 hunks)3rd/anchor(1 hunks)Cargo.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
- GitHub Check: test
🔇 Additional comments (6)
.github/workflows/run-tests.yml (4)
54-56: Enable recursive submodules in checkout (approved).Adding
submodules: recursiveensures the3rd/anchorsubmodule (added in.gitmodules) is cloned during CI. This aligns with the Cargo.toml dependency changes sourcing anchor-cli from a local path.Please verify that the
3rd/anchorsubmodule is correctly configured and that local path dependencies in Cargo.toml (3rd/anchor/cli) will resolve properly in the CI environment.
73-75: Ephemeral validator installation now active (good).The validator is no longer commented out, which means CI will now attempt to install and (implicitly) start it via
bolt test. This change enables error surfacing when startup fails, aligning with the PR objective. However, consider adding explicit diagnostics if startup is slow or fails intermittently.Verify that
bolt test(which runs later on line 107) properly awaits ephemeral-validator readiness and propagates startup errors to the workflow. The AI summary mentions "Bolt CLI test command now awaits and propagates the anchor task result"—confirm that this applies to ephemeral-validator startup as well.
103-106: Resource limits applied for ephemeral-validator (review recommended).Lines 104–106 set resource limits before running
bolt test:
prlimit --nofile=1048576(open file descriptors)sysctl fs.inotify.max_user_instances=1280sysctl fs.inotify.max_user_watches=655360These tunings likely prevent resource exhaustion during ephemeral-validator or test operations. However:
- Verify sufficiency: Are these values known to support ephemeral-validator startup in CI? If startup still fails with these limits, increase or investigate the bottleneck.
- No visibility into failures: If these limits are insufficient, the workflow will fail later (during
bolt test) without clear indication that it's a resource limit issue. Consider adding explicit resource checks or debug output.- Idempotence concern: The
sysctlcommands modify host-level settings. Confirm this doesn't cause issues in parallel workflows or affect other jobs.Please confirm:
- Are these resource limit values known to be sufficient for ephemeral-validator in this Ubuntu environment?
- Are there any documented constraints or known-good settings from the ephemeral-validator project?
17-17: Upgrade to actions/checkout@v5 (approved).Bumping checkout from v4 to v5 in both setup and test jobs is a standard maintenance update. Ensure that any breaking changes between v4 and v5 (if any) have been tested locally or are documented in the repository's changelog.
Confirm that actions/checkout@v5 does not introduce breaking changes for this repository's workflows. If there are known issues or required migration steps, document them in a comment or update records.
Also applies to: 54-54
.gitmodules (1)
1-3: Submodule added, but uses personal fork of Anchor (verify intent).The
.gitmodulesentry points tohttps://github.com/notdanilo/anchorrather than the upstream Anchor repository. While this may be intentional (e.g., custom patches for Bolt), verify:
- Fork intent: Is this fork intentionally diverged from upstream AnchorLang/anchor, or should it mirror upstream?
- Long-term viability: Is this fork actively maintained? Consider documenting why the fork exists and how it will be kept in sync with upstream if needed.
- Commit pinning: Ensure the submodule commit (mentioned in AI summary as
eeb353d053c22a7f3b03f91c1412eee08e5feb20) is stable and documented. Verify the submodule is correctly pinned after this PR merges.
- Confirm the fork URL is correct and intentional.
- Verify the submodule commit is known to work with the current Bolt codebase.
- Document (in comments or README) why a fork is necessary and how it will be maintained.
Alternatively, if this should use the upstream Anchor repository, update the URL accordingly.
3rd/anchor (1)
1-1: Clarify why a custom fork is required instead of the official Anchor repository.The commit
eeb353d053c22a7f3b03f91c1412eee08e5feb20exists in the submodule fork athttps://github.com/notdanilo/anchoron the branchmore-context. However:
- The submodule points to an individual's fork rather than the official Anchor repository at solana-foundation/anchor
- The commit is on a feature branch (
more-context), not a stable release branch- The PR lacks documentation explaining why this fork is necessary
Confirm:
- Whether this fork contains fixes unavailable in the official Anchor repository
- Whether
more-contextis stable or if a release tag should be pinned instead- Add a comment to
.gitmodulesor PR description documenting the fork choice for future maintainers
Cargo.toml
Outdated
| session-keys = { version = "^2", features = ["no-entrypoint"] } | ||
| anchor-lang = { version = "^0", features = ["init-if-needed"] } | ||
| anchor-cli = { version = "^0" } | ||
| anchor-cli = { path = "3rd/anchor/cli" } |
There was a problem hiding this comment.
🧩 Analysis chain
Verify submodule path and CI workflow alignment for local dependency.
The change from a versioned remote dependency to a local path dependency aligns with the PR objective to surface CI errors. However, this introduces a build prerequisite: the 3rd/anchor/cli submodule must be properly initialized and available.
Run the following to verify the submodule configuration and path validity:
Additionally, confirm that:
- CI workflows are updated to fetch submodules (actions/checkout@v5 with
submodules: trueor equivalent) - Local development isn't broken; developers should either have submodule guidance in docs or the build should fail gracefully with clear instructions
- Dependency compatibility is maintained between the local anchor-cli and the rest of the workspace (this is typically ensured by version alignment in the submodule commit)
🏁 Script executed:
#!/bin/bash
# Verify that 3rd/anchor is registered as a submodule and contains anchor-cli
git config --file .gitmodules --get-regexp "path.*3rd/anchor"
git config --file .gitmodules --get-regexp "url.*3rd/anchor"
# Check if the path contains a Cargo.toml for anchor-cli
if [ -f "3rd/anchor/cli/Cargo.toml" ]; then
echo "✓ Cargo.toml found at 3rd/anchor/cli/"
# Verify it defines the anchor-cli crate
grep -A2 "\[package\]" "3rd/anchor/cli/Cargo.toml" | grep "name.*anchor-cli"
else
echo "✗ Cargo.toml not found at 3rd/anchor/cli/"
fiLength of output: 41
🏁 Script executed:
#!/bin/bash
# Check if 3rd/anchor/cli directory exists and list its contents
echo "=== Checking directory structure ==="
if [ -d "3rd/anchor/cli" ]; then
echo "✓ 3rd/anchor/cli/ directory exists"
ls -la "3rd/anchor/cli/" | head -20
else
echo "✗ 3rd/anchor/cli/ directory does not exist"
fi
echo -e "\n=== Checking 3rd/ directory ==="
if [ -d "3rd" ]; then
ls -la "3rd/"
else
echo "✗ 3rd/ directory does not exist"
fi
echo -e "\n=== Checking .gitmodules ==="
if [ -f ".gitmodules" ]; then
cat .gitmodules
else
echo "✗ .gitmodules not found"
fi
echo -e "\n=== Searching for Cargo.toml files with anchor-cli ==="
find . -name "Cargo.toml" -type f 2>/dev/null | head -20 | xargs -I {} sh -c 'grep -l "anchor-cli" "{}" 2>/dev/null && echo "Found in: {}"'Length of output: 520
🏁 Script executed:
#!/bin/bash
# Check if 3rd/anchor is a submodule and inspect its structure
echo "=== Checking 3rd/anchor as submodule ==="
git config --file .gitmodules --get-all submodule.3rd/anchor.path
echo -e "\n=== Contents of 3rd/anchor (if initialized) ==="
if [ -d "3rd/anchor" ]; then
find 3rd/anchor -maxdepth 3 -type f -name "*.toml" 2>/dev/null | head -20
echo -e "\nDirectory listing:"
find 3rd/anchor -maxdepth 2 -type d 2>/dev/null
else
echo "3rd/anchor not accessible"
fi
echo -e "\n=== Checking Cargo.toml for exact anchor-cli reference ==="
grep -n "anchor-cli" Cargo.toml | head -10Length of output: 308
🏁 Script executed:
#!/bin/bash
# Inspect the submodule registration and structure
echo "=== Submodule registration details ==="
git config --file .gitmodules --get submodule.3rd/anchor.path
git config --file .gitmodules --get submodule.3rd/anchor.url
echo -e "\n=== Contents of 3rd/anchor directory ==="
ls -la 3rd/anchor/
find 3rd/anchor -maxdepth 3 2>/dev/null | head -20
echo -e "\n=== Anchor-cli references in Cargo.toml ==="
grep -n "anchor-cli" Cargo.tomlLength of output: 407
🌐 Web query:
notdanilo/anchor repository structure cli subdirectory
💡 Result:
Do you mean you want the file/tree listing of the repo's cli/ folder, or an explanation of what the cli subdirectory contains and its purpose? Which do you want?
🌐 Web query:
github notdanilo anchor cli cargo package
💡 Result:
Do you mean the Anchor CLI from the GitHub user "notdanilo"? I couldn't find a GitHub repo or published crate named "notdanilo/anchor-cli". The official Anchor CLI sources and install methods are:
- Anchor docs (install via AVM or from GitHub). [1]
- Official Anchor GitHub (solana-foundation/anchor) and releases (use avm / anchor-cli). [2][3]
- anchor-cli crate/docs on docs.rs. [4]
If you want, tell me exactly what you found for "notdanilo" (link or error) and I’ll look it up.
Sources:
[1] https://www.anchor-lang.com/docs/installation
[2] https://github.com/solana-foundation/anchor
[3] https://www.anchor-lang.com/docs/updates/release-notes/0-30-0
[4] https://docs.rs/crate/anchor-cli
Path mismatch between dependency specification and submodule registration will cause build failure.
Critical issues found:
-
Path mismatch: Cargo.toml line 43 specifies
{ path = "3rd/anchor/cli" }but .gitmodules registers the submodule atpath = 3rd/anchor(without/cli). Cargo will fail to locate the package. -
Unverified submodule URL: The submodule URL
https://github.com/notdanilo/anchordoes not match the official Anchor repository (solana-foundation/anchor). Verify this is the intended dependency source. -
Empty submodule: The 3rd/anchor directory is empty and uninitialized. Even if the submodule initializes, the path mismatch will prevent the build from succeeding.
Required fix: Align the path in Cargo.toml with the .gitmodules registration. Either change Cargo.toml to { path = "3rd/anchor" } or update .gitmodules to register at path = 3rd/anchor/cli (if that subdirectory exists in the repository).
🤖 Prompt for AI Agents
In Cargo.toml around line 43, the dependency path is set to { path =
"3rd/anchor/cli" } but .gitmodules registers the submodule at path = 3rd/anchor
and the 3rd/anchor directory is currently empty/uninitialized; to fix, align the
paths by either updating Cargo.toml to use { path = "3rd/anchor" } or updating
.gitmodules to register the submodule at path = 3rd/anchor/cli (and ensure the
submodule URL is the intended repository and the submodule is initialized and
populated), then reinitialize/update the submodule (git submodule sync && git
submodule update --init --recursive) so Cargo can find the package.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Cargo.toml (1)
43-43: Duplicate critical issue: Path mismatch with .gitmodules submodule registration.As noted in the .gitmodules review, this path dependency (
"3rd/anchor/cli") does not align with the submodule registration path (3rd/anchorin .gitmodules). The build will fail until paths are aligned.Verify the structure of https://github.com/notdanilo/anchor and align both files accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.github/workflows/run-tests.yml(3 hunks).gitmodules(1 hunks)3rd/anchor(1 hunks)Cargo.toml(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Run Bolt Lint
3rd/anchor
[error] 1-1: cargo metadata failed: failed to load manifest for workspace member referenced by workspace. Caused by: failed to read '/home/runner/work/bolt/bolt/3rd/anchor/cli/Cargo.toml'. No such file or directory (os error 2). Command: cargo metadata
Cargo.toml
[error] 1-1: cargo metadata failed: failed to load manifest for workspace member referenced by workspace. Caused by: failed to read '/home/runner/work/bolt/bolt/3rd/anchor/cli/Cargo.toml'. No such file or directory (os error 2). Command: cargo metadata
🔇 Additional comments (4)
.github/workflows/run-tests.yml (3)
54-56: Submodule configuration is correct, but submodule path registration may mismatch Cargo.toml dependency.The checkout action is correctly configured with
submodules: recursive, which should initialize nested submodules. However, this depends on the submodule being properly registered in .gitmodules and the path matching the dependency path in Cargo.toml.The notdanilo user has repositories available, but verify that the submodule URL and path structure align with what Cargo.toml expects (see details in Cargo.toml review below).
73-75: Good: Ephemeral-validator installation is uncommented and will be invoked.The PR objective to "surface and report the error" is now achievable because ephemeral-validator will actually be installed, allowing its startup failures to be visible in CI logs. This is a significant improvement over previous attempts to hide the problem.
103-107: System tuning commands are properly placed before bolt test execution.The prlimit and sysctl commands will apply resource limits before running tests, which supports the ephemeral-validator test process (as referenced in the comment on line 103). The comment remains accurate and the resource tuning is now uncommented and active.
Line 107 correctly runs
bolt testwithout suppressing failures, allowing test failures to propagate to the workflow status..gitmodules (1)
1-3: The original review comment is incorrect. The .gitmodules and Cargo.toml paths are properly aligned.The notdanilo/anchor repository contains a
cli/subdirectory at its root. When the submodule is registered atpath = 3rd/anchor, it clones the full repository, including thecli/subdirectory. The Cargo.toml dependency path"3rd/anchor/cli"correctly references the CLI crate within that structure. The configuration is valid.Any pipeline failures should be investigated separately, as they are not caused by a path mismatch.
Likely an incorrect or invalid review comment.
3rd/anchor
Outdated
| @@ -0,0 +1 @@ | |||
| Subproject commit 8c1b1a759b4cc4a789ec7773834f5dda0cc34468 | |||
There was a problem hiding this comment.
🧩 Analysis chain
Critical: CI workflow must initialize submodules.
The pipeline is failing because the Anchor submodule contents are not being checked out in the CI environment. When using Git submodules, the CI workflow must be configured to initialize and update them during checkout.
Ensure your GitHub Actions workflows use actions/checkout@v5 (or later) with submodules enabled:
- uses: actions/checkout@v5
with:
submodules: recursiveOr if you only need to fetch this specific submodule without nested submodules:
- uses: actions/checkout@v5
with:
submodules: trueRun the following script to verify all CI workflow files are properly configured to checkout submodules:
🏁 Script executed:
#!/bin/bash
# Description: Check if GitHub Actions workflows are configured to checkout submodules
echo "=== Checking workflow files for submodule configuration ==="
fd -e yml -e yaml . .github/workflows/ --exec bash -c '
echo "File: {}"
if grep -q "actions/checkout" "{}"; then
echo " Uses actions/checkout: YES"
if grep -A 5 "actions/checkout" "{}" | grep -q "submodules:"; then
echo " Submodules configured: YES"
grep -A 5 "actions/checkout" "{}" | grep "submodules:"
else
echo " Submodules configured: NO ❌"
fi
else
echo " Uses actions/checkout: NO"
fi
echo ""
'Length of output: 603
🏁 Script executed:
#!/bin/bash
# Description: Identify which workflows run cargo commands and need submodule configuration
echo "=== Checking which workflows use cargo commands ==="
fd -e yml -e yaml . .github/workflows/ --exec bash -c '
echo "File: {}"
if grep -qE "cargo|Cargo\.toml" "{}"; then
echo " Uses cargo: YES"
grep -n -E "cargo|Cargo\.toml" "{}" | head -5
else
echo " Uses cargo: NO"
fi
echo ""
'
echo ""
echo "=== Viewing run-tests.yml checkout configuration ==="
grep -A 10 "actions/checkout" .github/workflows/run-tests.yml
echo ""
echo "=== Viewing publish-bolt-crates.yml checkout configuration ==="
grep -A 10 "actions/checkout" .github/workflows/publish-bolt-crates.ymlLength of output: 3066
Critical: Most CI workflows lack submodule configuration for Anchor submodule access.
Verification confirms the issue: all 5 workflows use cargo commands, but 4 workflows completely lack submodule configuration. The pipeline is failing because cargo cannot find the Anchor submodule contents.
Workflows requiring fixes:
.github/workflows/run-lint.yml– missing submodules config.github/workflows/publish-bolt-crates.yml– missing submodules config.github/workflows/publish-packages.yml– missing submodules config.github/workflows/publish-bolt-sdk.yml– missing submodules config
Additionally, .github/workflows/run-tests.yml has an inconsistent configuration with two separate actions/checkout steps (first without submodules, second with submodules: recursive). Consolidate to a single checkout step with submodules enabled.
Update all workflow files to include:
- uses: actions/checkout@v5
with:
submodules: recursive🧰 Tools
🪛 GitHub Actions: Run Bolt Lint
[error] 1-1: cargo metadata failed: failed to load manifest for workspace member referenced by workspace. Caused by: failed to read '/home/runner/work/bolt/bolt/3rd/anchor/cli/Cargo.toml'. No such file or directory (os error 2). Command: cargo metadata
🤖 Prompt for AI Agents
In 3rd/anchor (comment covers workflows listed), CI workflows are missing
submodule checkout config causing cargo to fail; update the four workflows
.github/workflows/run-lint.yml, publish-bolt-crates.yml, publish-packages.yml,
and publish-bolt-sdk.yml to use a single actions/checkout@v5 step with with:
submodules: recursive, and in .github/workflows/run-tests.yml remove the
duplicate checkout and consolidate into one actions/checkout@v5 step that
includes with: submodules: recursive so the Anchor submodule is available to
cargo.
819a5aa to
0496bf2
Compare
There was a problem hiding this comment.
Actionable comments posted: 15
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
Cargo.toml(2 hunks)crates/anchor-cli/Cargo.toml(1 hunks)crates/anchor-cli/npm-package/anchor.js(1 hunks)crates/anchor-cli/npm-package/package.json(1 hunks)crates/anchor-cli/src/bin/main.rs(1 hunks)crates/anchor-cli/src/checks.rs(1 hunks)crates/anchor-cli/src/config.rs(1 hunks)crates/anchor-cli/src/rust_template.rs(1 hunks)crates/anchor-cli/src/solidity_template.rs(1 hunks)crates/bolt-cli/Cargo.toml(1 hunks)crates/bolt-cli/src/instructions.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
crates/anchor-cli/src/bin/main.rs (1)
crates/anchor-cli/src/lib.rs (1)
entry(540-546)
crates/anchor-cli/npm-package/anchor.js (1)
crates/anchor-cli/src/config.rs (1)
version(94-99)
crates/anchor-cli/src/checks.rs (2)
crates/anchor-cli/src/config.rs (8)
fs(557-559)path(55-57)from_path(74-79)from_path(556-560)from_path(873-908)from_path(943-945)from_str(687-704)new(51-53)crates/anchor-cli/src/rust_template.rs (2)
cargo_toml(177-219)package_json(377-416)
crates/anchor-cli/src/solidity_template.rs (2)
crates/anchor-cli/src/lib.rs (3)
create_files(1200-1221)std(679-682)new(1135-1188)crates/anchor-cli/src/rust_template.rs (4)
create_program(32-49)mocha(335-354)jest(356-375)ts_mocha(468-493)
crates/anchor-cli/src/config.rs (1)
crates/anchor-cli/src/lib.rs (8)
get_keypair(535-538)is_hidden(4773-4779)keys_sync(4616-4684)new(1135-1188)entry(540-546)programs(4310-4320)programs(4326-4338)value(3545-3556)
crates/anchor-cli/src/rust_template.rs (2)
crates/anchor-cli/src/lib.rs (8)
create_files(1200-1221)override_or_create_files(1230-1247)std(679-682)new(1135-1188)programs(4310-4320)programs(4326-4338)idl(2252-2340)test(3218-3348)crates/anchor-cli/src/solidity_template.rs (4)
create_program(7-13)mocha(43-88)ts_mocha(111-161)solidity(15-41)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: lint (clippy)
- GitHub Check: lint (yarn)
🔇 Additional comments (6)
crates/bolt-cli/Cargo.toml (1)
33-34: LGTM! Dependency additions align with workspace configuration.The addition of
solana-clientas a direct dependency is consistent with the import path changes incrates/bolt-cli/src/instructions.rs(line 9), whereRpcSendTransactionConfigis now imported directly fromsolana_client::rpc_config.crates/bolt-cli/src/instructions.rs (1)
9-9: LGTM! Import refactored to use direct dependency.The import path change from
anchor_client::solana_clienttosolana_clientaligns with the addition ofsolana-clientas a workspace dependency. This reduces dependency indirection without any behavioral changes.crates/anchor-cli/src/bin/main.rs (1)
5-7: LGTM! Clean entry point with proper error propagation.The main function correctly returns
Result<()>, which ensures that errors fromanchor_cli::entryare properly propagated and converted to non-zero exit codes by Rust's runtime. This aligns with the PR objective to surface CI errors.crates/anchor-cli/Cargo.toml (2)
21-21: Verify that the pinned anyhow version is intentional.The manifest pins
anyhow = "1.0.32", which is from 2020. While this may be intentional for compatibility, it's worth verifying whether using a more recent version (e.g.,"1.0"to allow patch updates) would be appropriate, especially for new code.Based on learnings, anyhow 1.0.32 is quite old. Please verify if this specific version is required for compatibility or if it should be updated to allow newer patch versions.
24-24: Verify the cargo_toml version requirement.The manifest specifies
cargo_toml = "0.19.2". Consider whether this should allow patch updates (e.g.,"0.19") or if this specific version is required for API stability.Based on learnings, cargo_toml 0.19.2 may have newer patch releases. Please verify if this specific version pin is intentional.
Cargo.toml (1)
10-10: LGTM! Workspace configuration correctly integrates anchor-cli.The workspace changes properly integrate the new
anchor-clicrate:
- Line 10 adds
crates/anchor-clias a workspace member- Line 44 configures it as a path dependency within the workspace
- Line 49 adds
solana-clientas a workspace dependencyThis resolves the path mismatch issue mentioned in the previous review comment, which referenced a problematic
3rd/anchor/clisubmodule path. The current implementation uses a local workspace member atcrates/anchor-cli, which is the correct approach.Also applies to: 44-44, 49-49
| function trySystemAnchor() { | ||
| console.error("Trying globally installed anchor."); | ||
|
|
||
| const absolutePath = process.env.PATH.split(":") | ||
| .filter((dir) => dir !== path.dirname(process.argv[1])) | ||
| .find((dir) => { | ||
| try { | ||
| fs.accessSync(`${dir}/anchor`, fs.constants.X_OK); | ||
| } catch { | ||
| return false; | ||
| } | ||
| return true; | ||
| }); | ||
|
|
||
| if (!absolutePath) { | ||
| console.error( | ||
| `Could not find globally installed anchor, install with cargo.`, | ||
| ); | ||
| process.exit(); | ||
| } | ||
|
|
||
| const absoluteBinaryPath = `${absolutePath}/anchor`; | ||
|
|
||
| const [error, binaryVersion] = getBinaryVersion(absoluteBinaryPath); | ||
| if (error !== null) { | ||
| console.error(`Failed to get version of global binary: ${error}`); | ||
| return; | ||
| } | ||
| if (binaryVersion !== PACKAGE_VERSION) { | ||
| console.error( | ||
| `Globally installed anchor version is not correct. Expected "${PACKAGE_VERSION}", found "${binaryVersion}".`, | ||
| ); | ||
| return; | ||
| } | ||
|
|
||
| runAnchor(absoluteBinaryPath); | ||
| } |
There was a problem hiding this comment.
Fix inconsistent error handling in trySystemAnchor.
The trySystemAnchor function has inconsistent error handling:
- Lines 87 and 93 return early without calling
process.exit(), leaving the script running - Line 79 calls
process.exit()but without an exit code (see separate comment) - Line 96 calls
runAnchorwhich properly handles exit codes
This inconsistency means that when version mismatches occur (lines 89-93), the process continues running instead of exiting with an error code, which will cause CI to report success despite the failure.
Apply this diff to ensure all error paths exit properly:
if (error !== null) {
console.error(`Failed to get version of global binary: ${error}`);
- return;
+ process.exit(1);
}
if (binaryVersion !== PACKAGE_VERSION) {
console.error(
`Globally installed anchor version is not correct. Expected "${PACKAGE_VERSION}", found "${binaryVersion}".`,
);
- return;
+ process.exit(1);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function trySystemAnchor() { | |
| console.error("Trying globally installed anchor."); | |
| const absolutePath = process.env.PATH.split(":") | |
| .filter((dir) => dir !== path.dirname(process.argv[1])) | |
| .find((dir) => { | |
| try { | |
| fs.accessSync(`${dir}/anchor`, fs.constants.X_OK); | |
| } catch { | |
| return false; | |
| } | |
| return true; | |
| }); | |
| if (!absolutePath) { | |
| console.error( | |
| `Could not find globally installed anchor, install with cargo.`, | |
| ); | |
| process.exit(); | |
| } | |
| const absoluteBinaryPath = `${absolutePath}/anchor`; | |
| const [error, binaryVersion] = getBinaryVersion(absoluteBinaryPath); | |
| if (error !== null) { | |
| console.error(`Failed to get version of global binary: ${error}`); | |
| return; | |
| } | |
| if (binaryVersion !== PACKAGE_VERSION) { | |
| console.error( | |
| `Globally installed anchor version is not correct. Expected "${PACKAGE_VERSION}", found "${binaryVersion}".`, | |
| ); | |
| return; | |
| } | |
| runAnchor(absoluteBinaryPath); | |
| } | |
| function trySystemAnchor() { | |
| console.error("Trying globally installed anchor."); | |
| const absolutePath = process.env.PATH.split(":") | |
| .filter((dir) => dir !== path.dirname(process.argv[1])) | |
| .find((dir) => { | |
| try { | |
| fs.accessSync(`${dir}/anchor`, fs.constants.X_OK); | |
| } catch { | |
| return false; | |
| } | |
| return true; | |
| }); | |
| if (!absolutePath) { | |
| console.error( | |
| `Could not find globally installed anchor, install with cargo.`, | |
| ); | |
| process.exit(); | |
| } | |
| const absoluteBinaryPath = `${absolutePath}/anchor`; | |
| const [error, binaryVersion] = getBinaryVersion(absoluteBinaryPath); | |
| if (error !== null) { | |
| console.error(`Failed to get version of global binary: ${error}`); | |
| process.exit(1); | |
| } | |
| if (binaryVersion !== PACKAGE_VERSION) { | |
| console.error( | |
| `Globally installed anchor version is not correct. Expected "${PACKAGE_VERSION}", found "${binaryVersion}".`, | |
| ); | |
| process.exit(1); | |
| } | |
| runAnchor(absoluteBinaryPath); | |
| } |
🤖 Prompt for AI Agents
In crates/anchor-cli/npm-package/anchor.js around lines 61 to 97, the function
trySystemAnchor has inconsistent error handling: replace the bare process.exit()
on line ~79 with process.exit(1), and change the two early returns after
getBinaryVersion error and version mismatch (around lines ~87 and ~93) to
instead call process.exit(1) so every failure path exits with a non-zero code;
leave runAnchor(absoluteBinaryPath) as the successful path that manages its own
exit handling.
| console.error( | ||
| `Could not find globally installed anchor, install with cargo.`, | ||
| ); | ||
| process.exit(); |
There was a problem hiding this comment.
Fix missing exit code - critical for CI error visibility.
The process.exit() call on line 79 does not specify an exit code, which defaults to 0 (success). This means CI will treat the failure to find a globally installed anchor as a success, directly undermining the PR objective to surface CI errors.
Apply this diff:
- process.exit();
+ process.exit(1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| process.exit(); | |
| process.exit(1); |
🤖 Prompt for AI Agents
In crates/anchor-cli/npm-package/anchor.js around line 79, the current
process.exit() call omits an exit code (defaults to 0), so change it to exit
with a non-zero code to signal failure to CI — replace the call to
process.exit() with a call that passes a failure code (e.g., process.exit(1)) so
the script returns a non-zero status when the global anchor binary is not found.
| "scripts": { | ||
| "prepack": "[ \"$(uname -op)\" != \"x86_64 GNU/Linux\" ] && (echo Can be packed only on x86_64 GNU/Linux && exit 1) || ([ \"$(./anchor --version)\" != \"anchor-cli $(jq -r .version package.json)\" ] && (echo Check anchor binary version && exit 2) || exit 0)" | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider refactoring the prepack script for maintainability.
The prepack script packs multiple validations into a single complex command line, which impacts readability and debuggability. Consider breaking it into a separate shell script or using a Node.js script for better error handling and cross-platform compatibility.
Additionally, the script relies on jq without checking if it's installed, which could lead to cryptic errors.
Example approach using a separate validation script:
"scripts": {
"prepack": "node scripts/validate-package.js"
}Then create scripts/validate-package.js:
const { execSync } = require('child_process');
const { version } = require('../package.json');
const os = require('os');
// Platform check
if (os.arch() !== 'x64' || os.platform() !== 'linux') {
console.error('Error: Package can only be built on x86_64 GNU/Linux');
process.exit(1);
}
// Version check
try {
const binaryVersion = execSync('./anchor --version', { encoding: 'utf8' }).trim();
const expectedVersion = `anchor-cli ${version}`;
if (binaryVersion !== expectedVersion) {
console.error(`Error: Binary version mismatch. Expected "${expectedVersion}", found "${binaryVersion}"`);
process.exit(2);
}
} catch (error) {
console.error(`Error: Failed to verify anchor binary: ${error.message}`);
process.exit(3);
}🤖 Prompt for AI Agents
In crates/anchor-cli/npm-package/package.json around lines 17-19, replace the
long one-liner prepack with a call to a separate validation script; add
scripts/validate-package.js which (1) uses Node's os module to assert x86_64
Linux (exit 1 on failure), (2) reads ../package.json.version, (3) runs ./anchor
--version via child_process.execSync, compares to `anchor-cli ${version}` and
exits 2 on mismatch, and (4) catches and logs execution errors (exit 3) so
missing jq or binary errors are reported clearly; update package.json "prepack"
to "node scripts/validate-package.js".
crates/anchor-cli/src/checks.rs
Outdated
| pub fn check_overflow(cargo_toml_path: impl AsRef<Path>) -> Result<bool> { | ||
| Manifest::from_path(cargo_toml_path)? | ||
| .profile | ||
| .release | ||
| .as_ref() | ||
| .and_then(|profile| profile.overflow_checks) | ||
| .ok_or(anyhow!( | ||
| "`overflow-checks` is not enabled. To enable, add:\n\n\ | ||
| [profile.release]\n\ | ||
| overflow-checks = true\n\n\ | ||
| in workspace root Cargo.toml", | ||
| )) | ||
| } |
There was a problem hiding this comment.
Overflow-checks: treat disabled as an error (not only “missing”).
Currently returns Ok(false) when explicitly set to false, which likely masks the intended failure. Make the check fail unless the flag is true.
Apply:
pub fn check_overflow(cargo_toml_path: impl AsRef<Path>) -> Result<bool> {
- Manifest::from_path(cargo_toml_path)?
- .profile
- .release
- .as_ref()
- .and_then(|profile| profile.overflow_checks)
- .ok_or(anyhow!(
- "`overflow-checks` is not enabled. To enable, add:\n\n\
- [profile.release]\n\
- overflow-checks = true\n\n\
- in workspace root Cargo.toml",
- ))
+ let enabled = Manifest::from_path(cargo_toml_path)?
+ .profile
+ .release
+ .as_ref()
+ .and_then(|profile| profile.overflow_checks)
+ .unwrap_or(false);
+ if !enabled {
+ return Err(anyhow!(
+ "`overflow-checks` is not enabled. To enable, add:\n\n\
+ [profile.release]\n\
+ overflow-checks = true\n\n\
+ in workspace root Cargo.toml"
+ ));
+ }
+ Ok(true)
}🤖 Prompt for AI Agents
In crates/anchor-cli/src/checks.rs around lines 14 to 26, the current
implementation returns Ok(false) when overflow-checks is explicitly set to
false; change the logic so the check only succeeds when overflow-checks is
present and true. Replace the chained and_then(...).ok_or(...) flow with a
conditional that returns Ok(true) when profile.overflow_checks is Some(true) and
returns the same anyhow! error when it's None or Some(false), ensuring an
explicit false causes an error rather than Ok(false).
crates/anchor-cli/src/checks.rs
Outdated
| let mismatched_lang_version = cfg | ||
| .get_rust_program_list()? | ||
| .into_iter() | ||
| .map(|path| path.join("Cargo.toml")) | ||
| .map(cargo_toml::Manifest::from_path) | ||
| .filter_map(|man| man.ok()) | ||
| .filter_map(|man| man.dependencies.get("anchor-lang").map(|d| d.to_owned())) | ||
| .filter_map(|dep| Version::parse(dep.req()).ok()) | ||
| .find(|ver| ver != &cli_version); // Only log the warning once | ||
|
|
There was a problem hiding this comment.
Parse anchor-lang dependency as VersionReq, not Version.
Cargo specs are ranges; parsing with Version will drop non-exact specs and miss mismatches.
Apply:
- .filter_map(|dep| Version::parse(dep.req()).ok())
- .find(|ver| ver != &cli_version); // Only log the warning once
+ .filter_map(|dep| VersionReq::parse(dep.req()).ok())
+ .find(|req| !req.matches(&cli_version)); // Only log the warning onceBased on learnings.
🤖 Prompt for AI Agents
In crates/anchor-cli/src/checks.rs around lines 38 to 47, the code currently
parses anchor-lang dependency strings as a concrete Version which drops range
specs; instead parse dependency requirements as a semver::VersionReq and check
whether that requirement matches the cli_version. Replace the Version::parse
step with VersionReq::parse(dep.req()) and change the find predicate to look for
a requirement that does not match the cli_version (e.g., |req|
!req.matches(&cli_version)) so range specs are handled correctly and mismatches
are detected.
| /// Read the program keypair file or create a new one if it doesn't exist. | ||
| pub fn get_or_create_program_id(name: &str) -> Pubkey { | ||
| let keypair_path = Path::new("target") | ||
| .join("deploy") | ||
| .join(format!("{}-keypair.json", name.to_snake_case())); | ||
|
|
||
| read_keypair_file(&keypair_path) | ||
| .unwrap_or_else(|_| { | ||
| let keypair = Keypair::new(); | ||
| write_keypair_file(&keypair, keypair_path).expect("Unable to create program keypair"); | ||
| keypair | ||
| }) | ||
| .pubkey() | ||
| } |
There was a problem hiding this comment.
Ensure target/deploy exists before writing keypair.
Without creating the directory, write_keypair_file can fail on fresh workspaces.
Apply:
pub fn get_or_create_program_id(name: &str) -> Pubkey {
let keypair_path = Path::new("target")
.join("deploy")
.join(format!("{}-keypair.json", name.to_snake_case()));
- read_keypair_file(&keypair_path)
+ if let Some(dir) = keypair_path.parent() {
+ let _ = fs::create_dir_all(dir);
+ }
+ read_keypair_file(&keypair_path)
.unwrap_or_else(|_| {
let keypair = Keypair::new();
write_keypair_file(&keypair, keypair_path).expect("Unable to create program keypair");
keypair
})
.pubkey()
}🤖 Prompt for AI Agents
In crates/anchor-cli/src/rust_template.rs around lines 227 to 240, the code may
attempt to write the new keypair to target/deploy without ensuring that
directory exists, causing write_keypair_file to fail on fresh workspaces; update
the unwrap_or_else closure to create the parent directory (e.g., use
std::fs::create_dir_all(keypair_path.parent().unwrap()) or equivalent) before
calling write_keypair_file, then proceed to write the keypair and return its
pubkey.
| pub fn node_shell( | ||
| cluster_url: &str, | ||
| wallet_path: &str, | ||
| programs: Vec<ProgramWorkspace>, | ||
| ) -> Result<String> { | ||
| let mut eval_string = format!( | ||
| r#" | ||
| const anchor = require('@coral-xyz/anchor'); | ||
| const web3 = anchor.web3; | ||
| const PublicKey = anchor.web3.PublicKey; | ||
| const Keypair = anchor.web3.Keypair; | ||
| const __wallet = new anchor.Wallet( | ||
| Keypair.fromSecretKey( | ||
| Buffer.from( | ||
| JSON.parse( | ||
| require('fs').readFileSync( | ||
| "{wallet_path}", | ||
| {{ | ||
| encoding: "utf-8", | ||
| }}, | ||
| ), | ||
| ), | ||
| ), | ||
| ), | ||
| ); | ||
| const __connection = new web3.Connection("{cluster_url}", "processed"); | ||
| const provider = new anchor.AnchorProvider(__connection, __wallet, {{ | ||
| commitment: "processed", | ||
| preflightcommitment: "processed", | ||
| }}); | ||
| anchor.setProvider(provider); | ||
| "#, | ||
| ); | ||
|
|
There was a problem hiding this comment.
Fix Program constructor and provider options in node_shell; pass programId and correct preflightCommitment.
- new anchor.Program(idl, programId, provider) is required; current code passes provider as 2nd arg.
- Property should be preflightCommitment (camelCase).
- Also actually use program_id.
Apply:
-const provider = new anchor.AnchorProvider(__connection, __wallet, {
- commitment: "processed",
- preflightcommitment: "processed",
-});
+const provider = new anchor.AnchorProvider(__connection, __wallet, {
+ commitment: "processed",
+ preflightCommitment: "processed",
+});
anchor.setProvider(provider);And:
-anchor.workspace.{} = new anchor.Program({}, provider);
+anchor.workspace.{} = new anchor.Program({}, new PublicKey("{}"), provider);Adjust the write! call accordingly:
- r#"
-anchor.workspace.{} = new anchor.Program({}, provider);
-"#,
- program.name.to_lower_camel_case(),
- serde_json::to_string(&program.idl)?,
+ r#"
+anchor.workspace.{} = new anchor.Program({}, new PublicKey("{}"), provider);
+"#,
+ program.name.to_lower_camel_case(),
+ serde_json::to_string(&program.idl)?,
+ program.program_id,Also applies to: 611-615
🤖 Prompt for AI Agents
In crates/anchor-cli/src/rust_template.rs around lines 572 to 606 (and similarly
at 611-615), the generated JS uses the wrong anchor.Program constructor and an
incorrect provider option name: change instantiation to new anchor.Program(idl,
programId, provider) (pass the program's public key as programId, not the
provider as the second arg), ensure the provider options use preflightCommitment
(camelCase) instead of preflightcommitment, and reference the actual program_id
value when emitting the program variable; also update the surrounding write!
call(s) that render this snippet to supply the program_id/programId variable and
the corrected provider option name.
| Self::Jest => { | ||
| // Build the test suite. | ||
| fs::create_dir_all("tests")?; | ||
|
|
||
| let mut test = File::create(format!("tests/{}.test.js", &project_name))?; | ||
| if solidity { | ||
| test.write_all(solidity_template::jest(project_name).as_bytes())?; | ||
| } else { | ||
| test.write_all(jest(project_name).as_bytes())?; | ||
| } | ||
| } |
There was a problem hiding this comment.
Generate .test.ts for TS + Jest (current code always writes .js).
TS projects expect .test.ts and TS content.
Apply:
- Self::Jest => {
+ Self::Jest => {
// Build the test suite.
fs::create_dir_all("tests")?;
- let mut test = File::create(format!("tests/{}.test.js", &project_name))?;
- if solidity {
- test.write_all(solidity_template::jest(project_name).as_bytes())?;
- } else {
- test.write_all(jest(project_name).as_bytes())?;
- }
+ if js {
+ let mut test = File::create(format!("tests/{}.test.js", &project_name))?;
+ if solidity {
+ test.write_all(solidity_template::jest(project_name).as_bytes())?;
+ } else {
+ test.write_all(jest(project_name).as_bytes())?;
+ }
+ } else {
+ let mut test = File::create(format!("tests/{}.test.ts", &project_name))?;
+ if solidity {
+ test.write_all(solidity_template::ts_jest(project_name).as_bytes())?;
+ } else {
+ test.write_all(ts_jest(project_name).as_bytes())?;
+ }
+ }
}Committable suggestion skipped: line range outside the PR's diff.
| fn solidity(name: &str) -> String { | ||
| format!( | ||
| r#" | ||
| contract {} {{ | ||
| bool private value = true; | ||
| @payer(payer) | ||
| constructor() {{ | ||
| print("Hello, World!"); | ||
| }} | ||
| /// A message that can be called on instantiated contracts. | ||
| /// This one flips the value of the stored `bool` from `true` | ||
| /// to `false` and vice versa. | ||
| function flip() public {{ | ||
| value = !value; | ||
| }} | ||
| /// Simply returns the current value of our `bool`. | ||
| function get() public view returns (bool) {{ | ||
| return value; | ||
| }} | ||
| }} | ||
| "#, | ||
| name.to_snake_case(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Verify Solidity contract naming convention.
Line 39 converts the contract name to snake_case, but Solidity convention typically uses PascalCase for contract names (e.g., MyContract rather than my_contract). This inconsistency with standard Solidity style could confuse users.
Compare with the Rust template in rust_template.rs which uses to_pascal_case() for TypeScript type names (line 467), suggesting PascalCase is the project's preference for type-like entities.
Consider changing line 39 to use PascalCase:
- name.to_snake_case(),
+ name.to_upper_camel_case(),📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| fn solidity(name: &str) -> String { | |
| format!( | |
| r#" | |
| contract {} {{ | |
| bool private value = true; | |
| @payer(payer) | |
| constructor() {{ | |
| print("Hello, World!"); | |
| }} | |
| /// A message that can be called on instantiated contracts. | |
| /// This one flips the value of the stored `bool` from `true` | |
| /// to `false` and vice versa. | |
| function flip() public {{ | |
| value = !value; | |
| }} | |
| /// Simply returns the current value of our `bool`. | |
| function get() public view returns (bool) {{ | |
| return value; | |
| }} | |
| }} | |
| "#, | |
| name.to_snake_case(), | |
| ) | |
| } | |
| fn solidity(name: &str) -> String { | |
| format!( | |
| r#" | |
| contract {} {{ | |
| bool private value = true; | |
| @payer(payer) | |
| constructor() {{ | |
| print("Hello, World!"); | |
| }} | |
| /// A message that can be called on instantiated contracts. | |
| /// This one flips the value of the stored `bool` from `true` | |
| /// to `false` and vice versa. | |
| function flip() public {{ | |
| value = !value; | |
| }} | |
| /// Simply returns the current value of our `bool`. | |
| function get() public view returns (bool) {{ | |
| return value; | |
| }} | |
| }} | |
| "#, | |
| name.to_upper_camel_case(), | |
| ) | |
| } |
🤖 Prompt for AI Agents
In crates/anchor-cli/src/solidity_template.rs around lines 15 to 41, the
contract name is formatted with name.to_snake_case(), producing snake_case, but
Solidity contract names should follow PascalCase; change the formatted name to
use name.to_pascal_case() instead (i.e., replace the to_snake_case() call with
to_pascal_case()) so generated contracts follow standard Solidity naming
conventions.
| pub fn ts_mocha(name: &str) -> String { | ||
| format!( | ||
| r#"import * as anchor from "@coral-xyz/anchor"; | ||
| import {{ Program }} from "@coral-xyz/anchor"; | ||
| import {{ {} }} from "../target/types/{}"; | ||
| describe("{}", () => {{ | ||
| // Configure the client to use the local cluster. | ||
| const provider = anchor.AnchorProvider.env(); | ||
| anchor.setProvider(provider); | ||
| const dataAccount = anchor.web3.Keypair.generate(); | ||
| const program = anchor.workspace.{} as Program<{}>; | ||
| it("Is initialized!", async () => {{ | ||
| // Add your test here. | ||
| const tx = await program.methods | ||
| .new() | ||
| .accounts({{ dataAccount: dataAccount.publicKey }}) | ||
| .signers([dataAccount]) | ||
| .rpc(); | ||
| console.log("Your transaction signature", tx); | ||
| const val1 = await program.methods | ||
| .get() | ||
| .accounts({{ dataAccount: dataAccount.publicKey }}) | ||
| .view(); | ||
| console.log("state", val1); | ||
| await program.methods | ||
| .flip() | ||
| .accounts({{ dataAccount: dataAccount.publicKey }}) | ||
| .rpc(); | ||
| const val2 = await program.methods | ||
| .get() | ||
| .accounts({{ dataAccount: dataAccount.publicKey }}) | ||
| .view(); | ||
| console.log("state", val2); }}); | ||
| }}); | ||
| "#, | ||
| name.to_upper_camel_case(), | ||
| name.to_snake_case(), | ||
| name, | ||
| name.to_upper_camel_case(), | ||
| name.to_upper_camel_case(), | ||
| ) | ||
| } |
There was a problem hiding this comment.
Verify template formatting at line 152.
Line 152 has console.log("state", val2); }}); with two spaces before the closing braces, which appears inconsistent with the rest of the template formatting. This might be unintentional.
Consider fixing the spacing:
- console.log("state", val2); }});
+ console.log("state", val2);
+ }});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn ts_mocha(name: &str) -> String { | |
| format!( | |
| r#"import * as anchor from "@coral-xyz/anchor"; | |
| import {{ Program }} from "@coral-xyz/anchor"; | |
| import {{ {} }} from "../target/types/{}"; | |
| describe("{}", () => {{ | |
| // Configure the client to use the local cluster. | |
| const provider = anchor.AnchorProvider.env(); | |
| anchor.setProvider(provider); | |
| const dataAccount = anchor.web3.Keypair.generate(); | |
| const program = anchor.workspace.{} as Program<{}>; | |
| it("Is initialized!", async () => {{ | |
| // Add your test here. | |
| const tx = await program.methods | |
| .new() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .signers([dataAccount]) | |
| .rpc(); | |
| console.log("Your transaction signature", tx); | |
| const val1 = await program.methods | |
| .get() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .view(); | |
| console.log("state", val1); | |
| await program.methods | |
| .flip() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .rpc(); | |
| const val2 = await program.methods | |
| .get() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .view(); | |
| console.log("state", val2); }}); | |
| }}); | |
| "#, | |
| name.to_upper_camel_case(), | |
| name.to_snake_case(), | |
| name, | |
| name.to_upper_camel_case(), | |
| name.to_upper_camel_case(), | |
| ) | |
| } | |
| pub fn ts_mocha(name: &str) -> String { | |
| format!( | |
| r#"import * as anchor from "@coral-xyz/anchor"; | |
| import {{ Program }} from "@coral-xyz/anchor"; | |
| import {{ {} }} from "../target/types/{}"; | |
| describe("{}", () => {{ | |
| // Configure the client to use the local cluster. | |
| const provider = anchor.AnchorProvider.env(); | |
| anchor.setProvider(provider); | |
| const dataAccount = anchor.web3.Keypair.generate(); | |
| const program = anchor.workspace.{} as Program<{}>; | |
| it("Is initialized!", async () => {{ | |
| // Add your test here. | |
| const tx = await program.methods | |
| .new() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .signers([dataAccount]) | |
| .rpc(); | |
| console.log("Your transaction signature", tx); | |
| const val1 = await program.methods | |
| .get() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .view(); | |
| console.log("state", val1); | |
| await program.methods | |
| .flip() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .rpc(); | |
| const val2 = await program.methods | |
| .get() | |
| .accounts({{ dataAccount: dataAccount.publicKey }}) | |
| .view(); | |
| console.log("state", val2); | |
| }}); | |
| }}); | |
| "#, | |
| name.to_upper_camel_case(), | |
| name.to_snake_case(), | |
| name, | |
| name.to_upper_camel_case(), | |
| name.to_upper_camel_case(), | |
| ) | |
| } |
🤖 Prompt for AI Agents
In crates/anchor-cli/src/solidity_template.rs around lines 111 to 161, the
template line at ~152 contains an extra two spaces before the closing braces
(`console.log("state", val2); }}});`) which breaks consistency; remove the
extra spaces so the line ends with a single space (or none) before the closing
braces to match the surrounding formatting and preserve the template's
indentation/newline style.
0496bf2 to
3cb402f
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (24)
Cargo.toml(1 hunks)examples/component-small/Cargo.toml(1 hunks)examples/escrow-funding/Cargo.toml(1 hunks)examples/escrow-funding/src/lib.rs(1 hunks)examples/system-with-1-component/Cargo.toml(1 hunks)examples/system-with-1-component/src/lib.rs(1 hunks)examples/system-with-10-components/Cargo.toml(1 hunks)examples/system-with-10-components/src/lib.rs(1 hunks)examples/system-with-2-components/Cargo.toml(1 hunks)examples/system-with-2-components/src/lib.rs(1 hunks)examples/system-with-3-components/Cargo.toml(1 hunks)examples/system-with-3-components/src/lib.rs(1 hunks)examples/system-with-4-components/Cargo.toml(1 hunks)examples/system-with-4-components/src/lib.rs(1 hunks)examples/system-with-5-components/Cargo.toml(1 hunks)examples/system-with-5-components/src/lib.rs(1 hunks)examples/system-with-6-components/Cargo.toml(1 hunks)examples/system-with-6-components/src/lib.rs(1 hunks)examples/system-with-7-components/Cargo.toml(1 hunks)examples/system-with-7-components/src/lib.rs(1 hunks)examples/system-with-8-components/Cargo.toml(1 hunks)examples/system-with-8-components/src/lib.rs(1 hunks)examples/system-with-9-components/Cargo.toml(1 hunks)examples/system-with-9-components/src/lib.rs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: test
- GitHub Check: lint (yarn)
- GitHub Check: lint (clippy)
🔇 Additional comments (21)
examples/system-with-8-components/src/lib.rs (1)
2-2: Import rename verified and correct.The change from
component_small::Smalltosmall::Smallis complete and properly configured:
- Workspace member in Cargo.toml correctly aliases
smalltoexamples/component-small- No stale
component_smallreferences remain in the codebase- The target module exports the
Smallstruct as expected- Import statement will compile and resolve correctly
examples/system-with-4-components/src/lib.rs (1)
2-2: LGTM! Import path updated correctly.The import path change from
component_small::Smalltosmall::Smallaligns with the workspace member rename and is applied consistently throughout the file.examples/system-with-10-components/src/lib.rs (1)
2-2: LGTM! Import path updated correctly.The import path change from
component_small::Smalltosmall::Smallaligns with the workspace member rename and is applied consistently throughout the file.examples/escrow-funding/src/lib.rs (1)
3-3: LGTM! Import path updated correctly.The import path change from
component_small::Smalltosmall::Smallaligns with the workspace member rename and is applied consistently throughout the file.examples/escrow-funding/Cargo.toml (1)
26-26: LGTM! Dependency reference updated correctly.The dependency reference change from
component-small.workspacetosmall.workspacealigns with the workspace member rename.examples/system-with-6-components/src/lib.rs (1)
2-2: LGTM! Import path updated correctly.The import path change from
component_small::Smalltosmall::Smallaligns with the workspace member rename and is applied consistently throughout the file.examples/system-with-7-components/src/lib.rs (1)
2-2: LGTM! Import path updated correctly.The import path change from
component_small::Smalltosmall::Smallaligns with the workspace member rename and is applied consistently throughout the file.Cargo.toml (1)
37-37: The directory path in Cargo.toml is correct and requires no changes.The verification confirms that
examples/component-small/exists and Cargo.toml correctly references it. The workspace dependency key was renamed fromcomponent-smalltosmall(the alias), but the actual directory path and structure remain unchanged and properly aligned. No mismatch exists.Likely an incorrect or invalid review comment.
examples/system-with-3-components/Cargo.toml (1)
25-25: LGTM! Dependency rename is syntactically correct.The workspace dependency has been correctly updated from
component-smalltosmall, aligning with the repository-wide crate rename.examples/system-with-5-components/src/lib.rs (1)
2-2: LGTM! Import path correctly updated.The import path has been properly updated from
component_small::Smalltosmall::Small, consistent with the workspace crate rename.examples/system-with-3-components/src/lib.rs (1)
2-2: LGTM! Import path correctly updated.The import path has been properly updated from
component_small::Smalltosmall::Small, matching the crate rename.examples/system-with-6-components/Cargo.toml (1)
25-25: LGTM! Dependency rename is syntactically correct.The workspace dependency has been correctly updated from
component-smalltosmall.examples/system-with-4-components/Cargo.toml (1)
25-25: LGTM! Dependency rename is syntactically correct.The workspace dependency has been correctly updated from
component-smalltosmall.examples/system-with-1-component/src/lib.rs (1)
2-2: LGTM! Import path correctly updated.The import path has been properly updated from
component_small::Smalltosmall::Small.examples/system-with-9-components/src/lib.rs (1)
2-2: LGTM! Import path correctly updated.The import path has been properly updated from
component_small::Smalltosmall::Small.examples/system-with-9-components/Cargo.toml (1)
25-25: LGTM! Workspace dependency rename applied correctly.The dependency declaration has been correctly updated from
component-smalltosmallto align with the workspace-wide rename.examples/system-with-8-components/Cargo.toml (1)
25-25: LGTM! Consistent with workspace rename.examples/system-with-5-components/Cargo.toml (1)
25-25: LGTM! Workspace dependency updated correctly.examples/system-with-7-components/Cargo.toml (1)
25-25: LGTM! Dependency alias updated correctly.examples/system-with-1-component/Cargo.toml (1)
25-25: LGTM! Workspace alias updated correctly.examples/system-with-10-components/Cargo.toml (1)
25-25: LGTM! Dependency updated consistently.
| @@ -1,12 +1,12 @@ | |||
| [package] | |||
| name = "component-small" | |||
| name = "small" | |||
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Package and library names updated correctly.
The package name change from "component-small" to "small" and library name change from "component_small" to "small" are correctly applied. The naming convention is appropriate (kebab-case for package, snake_case for library).
Verify that the directory name has also been updated to match the new package name:
Also applies to: 9-9
🏁 Script executed:
#!/bin/bash
# Verify the directory structure matches the renamed package
echo "=== Checking for old directory name ==="
fd -t d "component-small" examples/
echo -e "\n=== Checking for new directory name ==="
fd -t d "^small$" examples/
echo -e "\n=== Verifying path in workspace dependency matches actual directory ==="
if [ -d "examples/small" ]; then
echo "✓ examples/small/ directory exists"
elif [ -d "examples/component-small" ]; then
echo "✗ examples/component-small/ still exists (should be renamed to examples/small/)"
else
echo "⚠ Neither directory found"
fiLength of output: 259
Directory rename incomplete — examples/component-small/ must be renamed to examples/small/
The Cargo.toml package name was updated correctly, but the directory containing it was not renamed. The old directory examples/component-small/ still exists; it must be renamed to examples/small/ to match the new package name in Cargo.toml.
🤖 Prompt for AI Agents
In examples/component-small/Cargo.toml around line 2, the package name was
changed to "small" but the containing directory was not renamed; rename the
directory examples/component-small/ to examples/small/ to match Cargo.toml, and
update any workspace/Cargo references or paths (if present in workspace members,
CI scripts, README examples, or import paths) to use examples/small/.
| [dependencies] | ||
| bolt-lang.workspace = true | ||
| component-small.workspace = true | ||
| small.workspace = true |
There was a problem hiding this comment.
🧩 Analysis chain
Verify the workspace rename is complete across the repository.
The workspace dependency has been correctly updated from component-small to small. However, for a rename operation like this, it's critical to ensure all references have been consistently updated throughout the codebase.
Run the following script to verify that no stale references remain:
🏁 Script executed:
#!/bin/bash
# Description: Verify all references to component-small/component_small have been updated
echo "=== Checking for stale references to 'component-small' in Cargo.toml files ==="
rg -n --type=toml 'component-small' -g '!**/target/**'
echo ""
echo "=== Checking for stale references to 'component_small' in Rust source files ==="
rg -n --type=rust 'component_small' -g '!**/target/**'
echo ""
echo "=== Verifying the 'small' crate definition exists ==="
fd -t f '^Cargo\.toml$' examples/ -x grep -l 'name = "small"' {} \;Length of output: 1124
Complete the workspace rename—stale references remain in root Cargo.toml and Anchor.toml.
The verification confirms the workspace rename from component-small to small is incomplete:
- Root
Cargo.toml:37: Path still referencesexamples/component-small Anchor.toml:13: Program ID still declared ascomponent-smallAnchor.toml:39: Members list still containsexamples/component-small- Directory:
examples/component-small/has not been renamed toexamples/small
These stale references must be updated to complete the rename operation.
🤖 Prompt for AI Agents
In examples/system-with-2-components/Cargo.toml around line 25 and related
project files, the workspace rename from `component-small` to `small` is
incomplete: update the root Cargo.toml (around line 37) to reference
`examples/small` instead of `examples/component-small`; update Anchor.toml (line
~13) to change the program ID from `component-small` to `small` and update the
members list (around line ~39) to reference `examples/small`; rename the
directory `examples/component-small/` to `examples/small/` and verify any path
usages/imports/tests are adjusted accordingly so no stale references remain.
| @@ -1,5 +1,5 @@ | |||
| use bolt_lang::*; | |||
| use component_small::Small; | |||
| use small::Small; | |||
There was a problem hiding this comment.
🧩 Analysis chain
LGTM! Import path updated to match workspace rename.
The import path has been correctly updated from component_small::Small to small::Small, aligning with the workspace dependency rename. The public API remains unchanged.
To ensure completeness of the workspace-wide rename, please run the following verification script:
Expected results:
- No references to
component-smallin Cargo.toml files - No
use component_smallimports in Rust source files - Root Cargo.toml should show
smallas workspace member and dependency
🏁 Script executed:
#!/bin/bash
# Description: Verify that all references to the old component-small name have been updated.
echo "=== Checking for remaining 'component-small' references in Cargo.toml files ==="
rg -n --type toml 'component-small' -g '!**/Cargo.lock'
echo -e "\n=== Checking for remaining 'component_small' imports in Rust source files ==="
rg -n --type rust 'use\s+component_small' -g '!**/target/**'
echo -e "\n=== Checking workspace root Cargo.toml for member and dependency updates ==="
fd -t f '^Cargo\.toml$' --max-depth 1 --exec cat {} | rg -n -A 2 '\[workspace\]|\[workspace\.dependencies\]' || echo "No workspace configuration found at root"
echo -e "\n=== Checking if 'small' workspace member exists ==="
fd -t f '^Cargo\.toml$' --max-depth 1 --exec rg -n 'members.*small|"small"' {}Length of output: 1366
I need to gather more information about the directory structure and the full context of the Cargo.toml changes to properly assess the state of the workspace rename.
🏁 Script executed:
#!/bin/bash
# Check the actual directory structure
echo "=== Checking if examples/component-small exists ==="
test -d examples/component-small && echo "EXISTS: examples/component-small" || echo "NOT FOUND: examples/component-small"
echo -e "\n=== Checking if examples/small exists ==="
test -d examples/small && echo "EXISTS: examples/small" || echo "NOT FOUND: examples/small"
echo -e "\n=== Listing all examples directories ==="
fd -t d '^(component-|small|component_small)' examples/ --max-depth 1
echo -e "\n=== Full context of root Cargo.toml around line 37 ==="
sed -n '30,50p' Cargo.toml
echo -e "\n=== Checking what Cargo.toml the import path points to ==="
test -f "examples/component-small/Cargo.toml" && echo "Found: examples/component-small/Cargo.toml" || echo "Not found: examples/component-small/Cargo.toml"
test -f "examples/small/Cargo.toml" && echo "Found: examples/small/Cargo.toml" || echo "Not found: examples/small/Cargo.toml"Length of output: 1723
🏁 Script executed:
#!/bin/bash
# Check the package name in examples/component-small/Cargo.toml
echo "=== Package name in examples/component-small/Cargo.toml ==="
rg -n '^\[package\]|^name\s*=' examples/component-small/Cargo.toml -A 1
echo -e "\n=== Full Anchor.toml context for cluster config ==="
cat Anchor.toml | head -50Length of output: 2577
Incomplete workspace rename—Anchor.toml still references old component-small name.
The import has been correctly updated to use the workspace alias, but the rename is incomplete. The package itself in examples/component-small/Cargo.toml was renamed to small, and the root Cargo.toml correctly aliases it. However, Anchor.toml still references the old name:
- Line 13:
component-smallshould besmallin the[programs.localnet]section - Line 39:
examples/component-smallin the members list should be updated for consistency
Update Anchor.toml to reflect the renamed package.
🤖 Prompt for AI Agents
In Anchor.toml around lines 13 and 39, the workspace rename is incomplete:
replace the program name reference `component-small` with `small` in the
[programs.localnet] section (line 13), and update the members list entry
`examples/component-small` to the new path/name `examples/small` (line 39) so
the Anchor config matches the renamed package and workspace aliases; save and
run a quick `anchor build` to verify no further references remain.
Problem
CI is silently failing. ephemeral-validator isn't properly starting up
Solution
Successfully reporting error @ https://github.com/magicblock-labs/bolt/actions/runs/18571425558/job/52946245448?pr=212
Summary by CodeRabbit
New Features
Bug Fixes
Chores