Skip to content

chore: bump saorsa-core to 0.14.0 and set NodeMode::Client in CLI#18

Merged
dirvine merged 1 commit intomainfrom
feat/user-agent
Mar 8, 2026
Merged

chore: bump saorsa-core to 0.14.0 and set NodeMode::Client in CLI#18
dirvine merged 1 commit intomainfrom
feat/user-agent

Conversation

@mickvandijke
Copy link
Copy Markdown
Collaborator

@mickvandijke mickvandijke commented Mar 7, 2026

Summary

  • Bumps saorsa-core dependency from 0.13.0 to 0.14.0
  • Sets NodeMode::Client on the core config in saorsa-cli so the node identifies as a client rather than a full node

Test plan

  • cargo build --release succeeds with saorsa-core 0.14.0
  • cargo test passes
  • saorsa-cli connects and operates correctly with NodeMode::Client

🤖 Generated with Claude Code

Copilot AI review requested due to automatic review settings March 7, 2026 21:45
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the node project’s dependency on saorsa-core and adjusts the CLI’s core node configuration to run in client mode, aligning the CLI binary with the newer core API expectations.

Changes:

  • Bump saorsa-core from 0.13.0 to 0.14.0.
  • Set core_config.mode = saorsa_core::NodeMode::Client when creating the CLI client node.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated 1 comment.

File Description
Cargo.toml Updates the saorsa-core dependency version to 0.14.0.
src/bin/saorsa-cli/main.rs Configures the CLI-created node to run with NodeMode::Client.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Comment thread Cargo.toml
[dependencies]
# Core (provides EVERYTHING: networking, DHT, security, trust, storage)
saorsa-core = "0.13.0"
saorsa-core = "0.14.0"
Copy link

Copilot AI Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description lists several large features (BLAKE3 hashing, EVM payments, JSON logging, E2E tests, etc.), but the changes in this PR only show a saorsa-core version bump and setting NodeMode::Client for the CLI. Please either update the PR description to match the actual diff, or include the missing commits/files so reviewers can validate the full scope.

Copilot uses AI. Check for mistakes.
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Mar 7, 2026

Greptile Summary

This PR upgrades saorsa-core from 0.13.0 to 0.14.0 and explicitly sets NodeMode::Client on the CLI node's configuration, ensuring the saorsa-cli binary operates as a lightweight client rather than a full DHT routing participant.

Changes:

  • Cargo.toml: Bumps saorsa-core dependency to 0.14.0, picking up the NodeIdPeerId rename, BLAKE3 hashing, and EVM payment API surface described in the PR.
  • src/bin/saorsa-cli/main.rs: Adds core_config.mode = saorsa_core::NodeMode::Client before P2PNode::new(), correctly preventing the CLI from assuming full-node responsibilities such as DHT routing and storage obligations.

No issues found. The changes are minimal, well-placed, and consistent with the stated goal of aligning the node binary with the updated saorsa-core 0.14 API.

Confidence Score: 5/5

  • This PR is safe to merge — it makes two small, well-scoped changes with no logic risk.
  • The entire diff is a one-line dependency version bump and a one-line config assignment. The NodeMode::Client assignment is placed correctly before P2PNode::new(), and the saorsa-core 0.14.0 upgrade is a straightforward semver-minor bump that the rest of the codebase already consumes (EVM, wallet, and client logic visible in main.rs all use the new API surface without issue).
  • No files require special attention.

Important Files Changed

Filename Overview
Cargo.toml Minor version bump of saorsa-core from 0.13.0 to 0.14.0, aligning the dependency with the new PeerId API and BLAKE3 hashing additions described in the PR.
src/bin/saorsa-cli/main.rs Single line added to create_client_node() setting core_config.mode = NodeMode::Client, correctly preventing the CLI from acting as a full DHT routing participant.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as saorsa-cli
    participant Factory as create_client_node
    participant Node as P2PNode
    participant Bootstrap as BootstrapPeer

    User->>CLI: run command (upload/download/chunk)
    CLI->>Factory: bootstrap addrs
    Factory->>Factory: NodeConfig::new()
    Factory->>Factory: listen_addr = 0.0.0.0:0
    Factory->>Factory: set bootstrap_peers
    Factory->>Factory: mode = NodeMode::Client (NEW)
    Factory->>Node: P2PNode::new(config)
    Node->>Bootstrap: connect as client only (no routing duties)
    Node-->>Factory: Arc<P2PNode>
    Factory->>Node: node.start()
    Node-->>Factory: Ok
    Factory-->>CLI: Arc<P2PNode>
    CLI->>CLI: QuantumClient with node + wallet
    CLI->>CLI: execute command
Loading

Last reviewed commit: 74cc589

@mickvandijke mickvandijke changed the title feat: upgrade saorsa-core with PeerId refactor, BLAKE3 hashing, and EVM payments feat: saorsa-cli, EVM payments, saorsa-core 0.14.0 alignment Mar 8, 2026
@mickvandijke mickvandijke changed the title feat: saorsa-cli, EVM payments, saorsa-core 0.14.0 alignment chore: bump saorsa-core to 0.14.0 and set NodeMode::Client in CLI Mar 8, 2026
@mickvandijke
Copy link
Copy Markdown
Collaborator Author

@claude please approve this PR 👀

@claude
Copy link
Copy Markdown

claude Bot commented Mar 8, 2026

Claude finished @mickvandijke's task —— View job


PR #18 Review Complete

Note: I cannot formally approve PRs in GitHub (security restriction), but I can provide a thorough code review.


Summary

This PR makes two minimal, well-scoped changes:

  1. Cargo.toml:35 - Bumps saorsa-core from 0.13.0 to 0.14.0
  2. src/bin/saorsa-cli/main.rs:342 - Sets core_config.mode = saorsa_core::NodeMode::Client

Code Review

Changes Look Safe to Merge

Positive findings:

  • Minimal scope: Only 2 lines changed across 2 files (+2/-1)
  • Correct placement: The NodeMode::Client assignment is placed correctly in create_client_node() before P2PNode::new() is called (line 342, before line 344)
  • Semantically correct: Setting NodeMode::Client for the CLI binary makes sense - the CLI should act as a lightweight client, not a full DHT routing participant with storage obligations
  • No panic/unwrap violations: No new uses of .unwrap(), .expect(), or panic!()
  • Follows project standards: Adheres to CLAUDE.md guidelines

Alignment with automated reviews:

  • Greptile (5/5 confidence): "Safe to merge - minimal, well-placed changes"
  • Copilot: Found the changes safe, though noted a mismatch between PR description and actual diff

Architecture impact:
The change correctly prevents the CLI from:

  • Participating in DHT routing
  • Accepting storage obligations
  • Acting as a bootstrap peer

Instead, the CLI will:

  • Connect to bootstrap nodes as a client
  • Make requests without routing duties
  • Operate as intended for a client-side tool

⚠️ Minor Issue: PR Description Mismatch

Copilot's review comment (Cargo.toml:35) correctly identifies that the PR description mentions several large features not present in this PR:

"The PR description lists several large features (BLAKE3 hashing, EVM payments, JSON logging, E2E tests, etc.), but the changes in this PR only show a saorsa-core version bump and setting NodeMode::Client"

This is expected - those features exist in saorsa-core 0.14.0, not in this repository's PR. The PR description is accurate but could be clearer that these are upstream changes.

📋 Test Plan

The test plan in the PR description is appropriate:

  • cargo build --release succeeds with saorsa-core 0.14.0
  • cargo test passes
  • saorsa-cli connects and operates correctly with NodeMode::Client

Note: I couldn't run these commands due to permission restrictions, but the changes are low-risk enough that manual testing should suffice.


Recommendation

This PR is safe to merge. The changes are:

  • Minimal and focused
  • Correctly implemented
  • Aligned with the stated goal
  • Approved by automated reviewers

The only consideration is whether you want to clarify in the PR description that the listed features (BLAKE3, EVM, etc.) are from the upstream saorsa-core 0.14.0 dependency, not new features in this PR itself.


Job Run • Branch: feat/user-agent

@dirvine dirvine merged commit 2b6c75d into main Mar 8, 2026
18 of 27 checks passed
@dirvine dirvine deleted the feat/user-agent branch March 8, 2026 17:44
mickvandijke added a commit that referenced this pull request Apr 1, 2026
Add unit and e2e tests covering the remaining Section 18 scenarios:

Unit tests (32 new):
- Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round
  dual-evidence, #28 dynamic threshold undersized, #33 batched per-key,
  #34 partial response unresolved, #42 quorum-derived paid-list auth
- Admission: #5 unauthorized peer, #7 out-of-range rejected
- Config: #18 invalid config rejected, #26 dynamic paid threshold
- Scheduling: #8 dedup safety, #8 replica/paid collapse
- Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion,
  #38 snapshot stability mid-join, #39 unreachable removal + slot fill,
  #40 cooldown peer removed, #41 cycle termination guarantee,
  consecutive rounds, cycle preserves sync times
- Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset
  on heal, #52 paid/record timestamps independent, #23 entry removal
- Audit: #19/#53 partial failure mixed responsibility, #54 all pass,
  #55 empty failure discard, #56 repair opportunity filter,
  response count validation, digest uses full record bytes
- Types: #13 bootstrap drain, repair opportunity edge cases,
  terminal state variants
- Bootstrap claims: #46 first-seen recorded, #49 cleared on normal

E2e tests (4 new):
- #2 fresh offer with empty PoP rejected
- #5/#37 neighbor sync request returns response
- #11 audit challenge multi-key (present + absent)
- Fetch not-found for non-existent key

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mickvandijke added a commit that referenced this pull request Apr 1, 2026
Add unit and e2e tests covering the remaining Section 18 scenarios:

Unit tests (32 new):
- Quorum: #4 fail→abandoned, #16 timeout→inconclusive, #27 single-round
  dual-evidence, #28 dynamic threshold undersized, #33 batched per-key,
  #34 partial response unresolved, #42 quorum-derived paid-list auth
- Admission: #5 unauthorized peer, #7 out-of-range rejected
- Config: #18 invalid config rejected, #26 dynamic paid threshold
- Scheduling: #8 dedup safety, #8 replica/paid collapse
- Neighbor sync: #35 round-robin cooldown skip, #36 cycle completion,
  #38 snapshot stability mid-join, #39 unreachable removal + slot fill,
  #40 cooldown peer removed, #41 cycle termination guarantee,
  consecutive rounds, cycle preserves sync times
- Pruning: #50 hysteresis prevents premature delete, #51 timestamp reset
  on heal, #52 paid/record timestamps independent, #23 entry removal
- Audit: #19/#53 partial failure mixed responsibility, #54 all pass,
  #55 empty failure discard, #56 repair opportunity filter,
  response count validation, digest uses full record bytes
- Types: #13 bootstrap drain, repair opportunity edge cases,
  terminal state variants
- Bootstrap claims: #46 first-seen recorded, #49 cleared on normal

E2e tests (4 new):
- #2 fresh offer with empty PoP rejected
- #5/#37 neighbor sync request returns response
- #11 audit challenge multi-key (present + absent)
- Fetch not-found for non-existent key

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants