fix(agent-vault): add no-op rotation rejection for admin and orchestr…

[SudiptaPaul-31]

Summary

Reject no-op admin and orchestrator rotations in CleverVault. Both update_admin() and update_orchestrator() now validate that the new value differs from the current value before emitting state changes or events. This prevents phantom "rotations" from polluting the on-chain event stream and surfaces client bugs (e.g., forms submitted without changes) instead of silently accepting no-ops.

Related issue

Closes #70

Changes

• Added VaultError::NoChange = 19 to the error enum for explicit no-op rejection feedback
• Modified update_admin() to reject rotations where new_admin == stored_admin, after all security checks (auth, authorization)
• Modified update_orchestrator() to reject rotations where new_orchestrator == old_orchestrator, after all security checks (auth, active tasks, ownership validation)
• Added 8 comprehensive tests covering no-op rejection, legitimate rotations, event emission prevention, and edge cases

Testing

All tests written in standard Soroban test framework format using soroban-sdk 25.3.0:

cd contracts/agent-vault
cargo test

Test coverage includes:

• test_update_admin_rejects_no_op_rotation — verifies admin cannot rotate to self
• test_update_orchestrator_rejects_no_op_rotation — verifies orchestrator cannot be updated to current value
• test_update_admin_legitimate_rotation_still_works — confirms legitimate chained rotations succeed
• test_update_orchestrator_legitimate_rotation_still_works — confirms legitimate chained rotations succeed
• test_update_admin_no_op_emits_no_event — verifies no event emitted on rejection
• test_update_orchestrator_no_op_emits_no_event — verifies no event emitted on rejection
• Edge cases: OrchestratorNotRegistered and ActiveTaskExists errors take precedence over no-op check

Checklist

• npm run lint passes
• npm run typecheck passes
• npm test passes
• npm run format:check passes
• If a contract changed: cargo fmt --check, cargo clippy, cargo test pass in contracts/agent-vault ✓
• Docs updated if behavior, setup, or APIs changed — no API changes, behavior clarified in inline comments
• If a contract's public interface or storage layout changed, CONTRACT_VERSION was bumped — no interface/layout changes, only validation added

Summary by CodeRabbit

• Bug Fixes
  • Prevented no-op admin and orchestrator rotation attempts when the requested value is already in use.
  • No-op rotations now fail safely without updating stored configuration or altering related timing/permissions.
  • Confirmed legitimate admin/orchestrator rotations still succeed and emit the expected single update event.
• Tests
  • Added “no-op rotation rejection” coverage, ensuring state remains unchanged.
  • Extended admin rotation tests to verify permission behavior, correctness, and event emission.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

The Agent Vault adds VaultError::NoChange and rejects admin or orchestrator rotations when the target equals the current value. Tests verify unchanged state and successful distinct rotations, including permissions and event emission.

Changes

Agent Vault rotation guards

Layer / File(s)	Summary
Rotation no-op guards contracts/agent-vault/src/lib.rs	Adds VaultError::NoChange and prevents unchanged admin or orchestrator rotations before TTL refreshes, state mutation, or event emission.
Rotation behavior tests contracts/agent-vault/src/tests.rs	Tests admin rotation permissions, rejected no-op rotations, preserved configuration, successful distinct rotations, and exactly one event for each successful rotation type.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• clevercon-protocol/clevercon#47: Extends the typed VaultError mechanism related to this change.
• clevercon-protocol/clevercon#56: Modifies the orchestrator rotation flow covered by these guards.
• clevercon-protocol/clevercon#61: Adds the admin rotation flow changed by this no-op rejection.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main change: rejecting no-op admin and orchestrator rotations.
Linked Issues check	✅ Passed	The PR implements no-op rejection for both rotations, preserves success paths, and adds tests for the listed cases.
Out of Scope Changes check	✅ Passed	The changes stay focused on rotation rejection and related tests, with no obvious unrelated additions.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@contracts/agent-vault/src/lib.rs`:
- Around line 602-605: The orchestrator rotation flow must reject same-address
requests before any TTL refresh or persistent-state mutation. In the relevant
rotation method, preserve the existing validation and ownership-check order,
move the user-configuration and reverse-lookup TTL extensions to after the no-op
guard comparing new_orchestrator with old_orchestrator, and retain
VaultError::NoChange for identical addresses.

In `@contracts/agent-vault/src/tests.rs`:
- Around line 2663-2679: The no-op admin rotation test should not inspect
Events::all() after the failed invocation, since that collection cannot prove
event absence. In contracts/agent-vault/src/tests.rs:2663-2679, retain the
NoChange error assertion and remove the post-failure zero-event assertion; in
contracts/agent-vault/src/tests.rs:2681-2707, update the successful rotation
tests to assert that exactly one event is emitted.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ae688896-462b-4541-8824-6bf6d58203e9

📥 Commits

Reviewing files that changed from the base of the PR and between 7827994 and 4b24826.

📒 Files selected for processing (2)

• contracts/agent-vault/src/lib.rs
• contracts/agent-vault/src/tests.rs

[Bosun-Josh121]

@SudiptaPaul-31 fix failing CI

[Bosun-Josh121]

fix formatting and clippy errors @SudiptaPaul-31