refactor(ownership): collapse OwnerId+Identity into UserId with role variants

[ilblackdragon]

Summary

PR 1 of 4 type-safety refactors. Enforces the .claude/rules/types.md rule that identifiers must be newtypes with validation at construction and no silent From<&str>/From<String> conversions.

What changed

• Collapsed OwnerId + Identity into a single UserId carrying both the id and its role. Scope constructors (TenantScope::with_identity, AdminScope::new, TenantCtx::new) now take one argument where they previously took two entangled ones.
• Expanded UserRole from {Admin, Member} to {Owner, Admin, Regular}. Owner is a super-admin tier (satisfies is_admin() too). Legacy "member" DB rows keep deserializing transparently via UserRole::from_db_role which maps unknowns to Regular.
• Removed From<String> and From<&str> on UserId — the whole point of the rule. Callers now pick explicitly between:
  • UserId::new(id, role)? — validated (rejects empty / whitespace-only)
  • UserId::from_trusted(id, role) — opt-out for DB/config-sourced values (the choice itself is the audit trail)
  • UserId::try_from((String, UserRole)) — validating tuple form
• Removed Deref<Target = str>. The boundary is visible at every call site: .as_str() / .role() / .is_admin().
• Three role predicates on UserId: is_owner(), is_admin() (covers Owner + Admin), is_regular(). These delegate to the matching UserRole methods so the rule stays centralized.
• Updated AdminScope::new to gate on identity.is_admin() rather than == UserRole::Admin, so Owner users automatically clear admin gates.
• Added UserIdentity::is_admin() on the web-auth struct (also routes through UserRole::from_db_role) instead of comparing role == "admin" raw.

Why now

Fixes the bug pattern behind #2561, #2620, #2349, #2126 — each of those shipped because a stringly-typed owner_id round-tripped through more than one layer with no compile-time discipline. UserId with explicit construction makes each of those a type error at the next regression.

Scope note

This PR migrates src/ownership/, src/agent/, src/channels/ (except engine-v2 DB row structs), src/bridge/, src/cli/pairing.rs, src/db/, src/extensions/, src/tenant.rs, src/pairing/, and the tests/ownership_integration.rs + tests/pairing_integration.rs tiers. Raw owner_id: String fields on DB row structs and wire DTOs are left as-is per types.md boundary rules. The engine-crate OwnerId<'a> enum (a shared/user sum type inside crates/ironclaw_engine/src/types/mod.rs) is a distinct type and out of scope.

Test plan

• cargo check --all-features — clean
• cargo clippy --all --benches --tests --examples --all-features — zero warnings
• cargo test -p ironclaw_common — 23 passed
• cargo test --lib ownership::tests — 15 passed (new validation/role tests included)
• cargo test --lib tenant::tests — 12 passed (includes owner-gets-admin-scope regression test)
• cargo test --features integration — rerun with PostgreSQL available in CI

[Copilot]

Pull request overview

Refactors the ownership/identity model by collapsing OwnerId + Identity into a single validated UserId that carries both the user id string and an expanded UserRole (Owner, Admin, Regular). This propagates through tenant scopes, pairing flows, and channel integrations to reduce stringly-typed ID handling.

Changes:

• Introduces UserId (validated constructor + explicit from_trusted) and expands UserRole with centralized role parsing (from_db_role) and predicates.
• Migrates tenant/admin scope constructors and role gating to use UserId + is_admin() (Owner or Admin).
• Updates pairing resolution/caching + multiple integration tests and channel entrypoints to pass/return UserId.

Reviewed changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/pairing_integration.rs	Updates pairing integration tests to use UserId/UserRole.
tests/ownership_integration.rs	Updates ownership integration tests; adds new UserId/role behavior assertions.
src/tenant.rs	Replaces stored identity type with UserId; updates admin gating to use is_admin().
src/pairing/store.rs	Pairing store APIs now return/accept UserId and cache it.
src/ownership/mod.rs	Introduces UserId/UserRole (expanded role tiers, validation, predicates); removes old types.
src/ownership/cache.rs	Cache now stores UserId and evicts using UserId::as_str().
src/extensions/manager.rs	Updates pairing lookups to pass UserId.
src/db/postgres.rs	Returns UserId from pairing identity resolution; role parsed via from_db_role.
src/db/mod.rs	Updates ChannelPairingStore trait signature to return UserId.
src/db/libsql/pairing.rs	Returns UserId from pairing identity resolution; role parsed via from_db_role.
src/cli/pairing.rs	CLI pairing approval now passes UserId (Owner role) to store.
src/channels/web/server.rs	Web pairing approval handler now binds to authenticated user via UserId.
src/channels/web/auth.rs	Adds UserIdentity::is_admin() and uses it for admin route gating.
src/channels/wasm/wrapper.rs	WASM host pairing callback now uses UserId and extracts id via as_str().
src/channels/wasm/setup.rs	WASM channel setup uses UserId (Owner) when resolving owner actor id.
src/channels/signal.rs	Signal pairing resolution now uses UserId and extracts id via as_str().
src/agent/dispatcher.rs	Switches admin policy check to UserId::is_admin().
src/agent/agent_loop.rs	Tenant context creation now uses UserId bridge construction.

---

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

[Copilot]

This code treats the CLI identity as UserRole::Owner, but the earlier get_or_create_user upsert still writes role: "admin". That means any later DB-sourced role checks will see Admin rather than Owner. Consider persisting role: "owner" here (or using UserRole::Owner.as_db_role()) for consistency.

[ilblackdragon]

Fixed in 7b01366. Added UserRole::as_db_role() and the CLI now upserts role:"owner" to match the in-memory identity.

[Copilot]

In this block, AdminScope is now gated by identity.is_admin() (Admin or Owner), but the nearby section header / doc comment still says it “requires UserRole::Admin” / “Constructable only with UserRole::Admin”. Please update those comments to avoid misleading future readers.

[ilblackdragon]

Fixed in 7b01366. AdminScope doc now reflects that Owner also passes is_admin().

[Copilot]

The rejection message still says "Admin role required", but is_admin() now also permits owner. Consider updating the message to something like “Admin privileges required” (admin or owner) to match the new authorization rule.

Suggested change

	return Err((StatusCode::FORBIDDEN, "Admin role required"));
	return Err((
	StatusCode::FORBIDDEN,
	"Admin privileges required (admin or owner)",
	));

[ilblackdragon]

Fixed in 7b01366. Applied the suggested wording.

[gemini-code-assist]

Code Review

This pull request refactors the ownership model by consolidating the previous OwnerId and Identity types into a single UserId struct that encapsulates both the user identifier and their UserRole. The role system is expanded to include Owner, Admin, and Regular variants, with updated logic to treat owners as administrators. These changes are propagated across the agent loop, database backends, pairing store, and tenant-scoped execution contexts. Feedback suggests that UserId should implement PartialEq and Hash based solely on the id string to prevent identity conflicts when roles vary, and that the role field should be required during deserialization to maintain data integrity.

[gemini-code-assist]

Including the 'role' in the derived 'PartialEq' and 'Hash' implementations for 'UserId' is a potential footgun. It means that two 'UserId' instances representing the same user (same 'id' string) will be considered unequal if their roles differ. This can lead to subtle bugs if 'UserId' is used as a key in a 'HashMap' or for identity comparisons. Consider implementing 'PartialEq' and 'Hash' manually to only compare the 'id' field. Additionally, avoid using '#[serde(default)]' for the 'role' field; as a critical field, it is safer for deserialization to fail if the data is missing rather than defaulting to a potentially incorrect role.

#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)]
pub struct UserId {
    id: String,
    role: UserRole,
}

impl PartialEq for UserId {
    fn eq(&self, other: &Self) -> bool {
        self.id == other.id
    }
}

impl Eq for UserId {}

impl std::hash::Hash for UserId {
    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
        self.id.hash(state);
    }
}

References

1. Avoid using #[serde(default)] for critical fields in data structures deserialized from external sources to prevent silent failures and ensure data integrity.

[ilblackdragon]

Good catch — fixed in 7b01366. UserId now manually implements PartialEq/Eq/Hash on id only; role is metadata. Added a regression test that builds a HashSet keyed on UserId and confirms cross-role membership.