test(governance-store): cover governance-op rejection paths

[chefsale]

Add negative-path unit tests for three per-op apply handlers whose
authorization gates were previously untested:

• transfer_ownership: reject a non-owner signer, a successor who is a
  plain Member (not Admin), and a successor who is not a group member.
• context_capability_granted / context_capability_revoked: reject a
  signer lacking MANAGE_MEMBERS (a plain member), asserting the store
  is left untouched (no grant written / existing grant preserved).

Each test drives the full signed-op path (apply_local_signed_group_op)
and asserts both rejection and the intended rejection reason, since
authorization lives entirely in the per-op handlers (no top-level gate).

Co-Authored-By: Claude Opus 4.8 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01RQnhNsCjGBeh5Hh45gbeHZ

[meroreviewer]

🤖 MeroReviewer

Reviewed by 1 agents | Quality score: 28% | Review time: 37.3s

🟡 2 warnings. See inline comments.

---

_{🤖 Generated by MeroReviewer | Review ID: review-59568935}

[meroreviewer]

🟡 Error message string-match is fragile and couples tests to implementation wording

The three TransferOwnership rejection tests assert on substrings of the error message (e.g. err.to_string().contains("can transfer ownership"), "must be an Admin", "not a member"). If the error message in the handler is ever reworded, these tests silently become dead assertions — they will compile and pass even though the rejection reason changed. The existing ReadOnlyTeeViaAttestationOnly tests in the same file use downcast_ref::<MembershipError>() which is strongly typed and immune to rewording.

Suggested fix:

Downcast to the concrete error type (e.g. `err.downcast_ref::<TransferOwnershipError>()` or whatever the handler returns) and match on the enum variant, mirroring the pattern used in `reject_read_only_tee_via_member_added` at line ~990.

[chefsale]

Done in 62a603d. Switched all three TransferOwnership tests to err.downcast_ref::<MembershipError>() + variant match (OnlyOwnerCanTransfer, TransferTargetNotAdmin { role, .. }, TransferTargetNotMember). The not-admin / not-member arms of the handler previously bailed with bare strings, so I added the two typed MembershipError variants and the handler now bails with those (messages unchanged).

---

Generated by Claude Code

[meroreviewer]

🟡 Error message string-match is fragile for capability-gate rejection tests

The context_capability_granted_rejects_unauthorized_signer and context_capability_revoked_rejects_unauthorized_signer tests assert err.to_string().contains("grant context capability") / "revoke context capability". Same fragility as the TransferOwnership tests: a handler refactor that changes the error message text would silently break the intent of these tests without a compile error.

Suggested fix:

Use `err.downcast_ref::<ConcreteErrorType>()` and match on the specific error variant, consistent with the typed-downcast pattern used elsewhere in this test file.

[chefsale]

Done in 62a603d. Both capability tests now match CapabilitiesError::Unauthorized { operation, .. }, guarded on the stable operation identifier ("grant context capability" / "revoke context capability") — which the enum doc explicitly sanctions matching on for routing.

---

Generated by Claude Code

[meroreviewer]

Documentation Review

The following documentation may need updates based on the changes in this PR:

• 🟡 architecture/auto-follow.html: Files matching crates/governance-store/** were changed but architecture/auto-follow.html was not updated (per source_to_docs_mapping).
• 🟡 architecture/local-governance.html: Files matching crates/governance-store/** were changed but architecture/local-governance.html was not updated (per source_to_docs_mapping).

[meroreviewer]

🤖 AI Code Reviewer

Reviewed by 2 agents | Quality score: 55% | Review time: 381.8s

🟡 1 warnings. See inline comments.

---

_{🤖 Generated by AI Code Reviewer | Review ID: review-25e43fa9}

[meroreviewer]

🟡 Error message substring match is fragile and couples tests to implementation strings

The test asserts err.to_string().contains("can transfer ownership"). If the error message in the handler is ever rephrased (e.g. "only the owner can transfer"), this test silently passes even though the rejection reason changed. The same pattern is repeated for "must be an Admin" (line ~1155), "not a member" (line ~1195), "grant context capability" (line ~1237), and "revoke context capability" (line ~1280). A stronger assertion would downcast to the concrete error type (as done in reject_read_only_tee_via_member_added which uses err.downcast_ref::<MembershipError>()). If the error types are not exported, at minimum document that the substring is load-bearing and must be kept in sync with the handler.

Suggested fix:

Use `err.downcast_ref::<OwnershipError>()` (or whatever the concrete error type is) and match on the variant, mirroring the pattern already used in `reject_read_only_tee_via_member_added`. If no typed error exists, add one so the test can be precise.

[chefsale]

Done in 62a603d. All five assertions now downcast to the concrete error type (MembershipError / CapabilitiesError) and match the variant instead of substring-matching the message. Where no typed error existed (the new-owner-not-admin / not-member arms), I added MembershipError::TransferTargetNotAdmin / TransferTargetNotMember and the handler bails with those.

---

Generated by Claude Code

[meroreviewer]

🤖 AI Code Reviewer

Reviewed by 2 agents | Quality score: 59% | Review time: 139.6s

---

📝 Nitpick (1)

1. Nit: import placement breaks StdExternalCrate ordering convention

File: crates/governance-store/src/errors.rs (line 160-175) | Consensus: 1/2 agents

The new use calimero_primitives::context::GroupMemberRole; import at line 54 is placed before use thiserror::Error;. Per the project's StdExternalCrate convention, external crates (thiserror) should come before local crates (calimero_primitives). The current ordering puts a local crate import ahead of an external one.

Suggested fix:

Reorder to: `use thiserror::Error;` first, then `use calimero_primitives::context::GroupMemberRole;`.

Found by: patterns-reviewer

---

_{🤖 Generated by AI Code Reviewer | Review ID: review-1af87e88}