Store: reserve identity audit dimension (AuditEvent.actor + bootstrap/mismatch kinds)

[George-RD]

Parent

#208 (Spec: deepened Store interface)

Blocked by

#215 (with_audited_effect), #201 (Retype ApprovalRecord.approved_by and add AuditEvent.actor — typed owner identity spec #197)

Deliverables

Fill the reserved identity audit dimension: move owner facts out of reason strings into a typed dimension, without a resolution write path and without persisting raw identifiers.

• Carry the actor via AuditEvent.actor: Option<PrincipalId> (delivered by Retype ApprovalRecord.approved_by and add AuditEvent.actor #201) instead of the store/identity.rs:245 identity.bound reason string owner={} identity={}.
• Add audit kinds for bootstrap binding and owner-config-mismatch rejection (beyond the single identity.bound kind), emitted through with_audited_effect.
• Resolution paths (get_identity, principal_exists) stay read-only — no write path added.
• Identifiers stay hash-only (identity_identifiers.value_hash); no raw identifier is ever persisted.

Rationale in spec #208 (decision D-007). Aligns to typed-owner-identity spec #197 D-003 (audit actor dimension). Independent of the T6 conn flip.

Verification

./scripts/check.sh green. Test: an identity.bound (and the new kinds) event carries actor = Some(principal_id); resolution remains read-only; no raw identifier appears in any persisted row.

🤖 Generated from spec #208 (wayfinder #189, map #182).

[coderabbitai]

Coding Plan

Summary

• Introduce PrincipalId as a minimal transparent newtype over Ulid. Add AuditEvent.actor: Option<PrincipalId> with a serde default, following the established payload_json legacy-compatibility pattern.
• Thread actor through append_audit_conn_with_options, the meta_json hash pre-image, and the event_bus.rs cross-validation list. Keep the field JSON-only with no new SQL column.
• Add a with_audited_effect helper modeled on mutate_briefcase_and_audit. Use it to emit identity.bound, the new identity.bootstrapped, and the new identity.owner_config_mismatch kinds with a typed actor.
• Keep resolution paths read-only and all identifiers hash-only. Verify with targeted tests plus ./scripts/check.sh.

Design Choices

Design Choice 1: Handle missing prerequisite tickets

Options Considered:

1. Assume the blocking tickets land first and plan only the identity-store wiring.
2. Include the prerequisite work (PrincipalId, AuditEvent.actor, with_audited_effect) as foundation phases in this plan.

Chosen Option: 2

Rationale: The ticket lists #201 (which adds PrincipalId and AuditEvent.actor) and a with_audited_effect ticket as blockers. Neither PrincipalId, AuditEvent.actor, nor with_audited_effect exists in the current codebase snapshot. Verification depends on actor being present and populated, so the plan must account for these prerequisites. The prerequisites are absent from the codebase, so this plan introduces them. If the blocking tickets merge first, Phases 1 and 2 become no-ops and only Phase 3 remains.

Design Choice 2: Define the PrincipalId type shape

Options Considered:

1. Introduce PrincipalId as a transparent newtype over Ulid used only for the actor dimension, converting from Ulid at audit call sites.
2. Replace every raw Ulid principal id across the codebase with PrincipalId.

Chosen Option: 1

Rationale: The codebase documents runtime ids as bare ulid::Ulid and has no Ulid-wrapping newtype. The only *Id newtypes wrap validated String slugs. This ticket only needs PrincipalId for AuditEvent.actor. A wholesale retype has a large blast radius and belongs to #201. So PrincipalId is introduced minimally, and other code keeps raw Ulid and converts when it supplies an actor.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Introduce the typed actor dimension in the schema

This phase adds the PrincipalId type and the AuditEvent.actor field with backward-compatible serde defaults, so legacy audit rows still deserialize.

Task 1: Add the PrincipalId newtype

Introduce a typed principal identifier for the audit actor dimension.

• Add a PrincipalId newtype over ulid::Ulid in crates/openspine-schemas/src/, next to the principal model (principal.rs) or the id conventions (ids.rs).
• Use #[serde(transparent)] so the wire form stays a plain ULID string.
• Provide From<Ulid> / accessor conversions so callers holding a raw Ulid can build a PrincipalId without wide refactors.
• Extend the ids.rs module doc to record this as the first Ulid-backed nominal id, per the existing convention notes.

Task 2: Add AuditEvent.actor

Reserve and populate the typed actor field on the audit event.

• Add pub actor: Option<PrincipalId> to AuditEvent in crates/openspine-schemas/src/audit.rs.
• Mark it #[serde(default)], following the existing payload_json pattern, so legacy event_json blobs that omit actor deserialize to None.
• Keep the struct #[serde(deny_unknown_fields)].

Task 3: Cover the new field with serde tests

Prove the field round-trips and defaults safely.

• Extend round_trips_through_serde in audit.rs to include a populated actor.
• Extend target_and_payload_refs_default_to_empty_when_omitted (or add a sibling test) to omit actor from the JSON and assert it deserializes to None.

🤖 Prompt for AI agents

Implement the typed actor dimension for the audit schema. Goal: add
`PrincipalId` and `AuditEvent.actor` with backward-compatible serde defaults so
legacy audit rows still deserialize.

- In `crates/openspine-schemas/src/` (next to `principal.rs` or `ids.rs`), add a
`PrincipalId` newtype over `ulid::Ulid`.
  - Mark it `#[serde(transparent)]` so the wire form is a plain ULID string.
- Provide `From<Ulid>` / accessor conversions so callers holding a raw `Ulid`
can build a `PrincipalId` without wide refactors.
- Update the `ids.rs` module doc to note this is the first `Ulid`-backed nominal
id, following the existing convention notes.

- In `crates/openspine-schemas/src/audit.rs`, add `pub actor:
Option<PrincipalId>` to `AuditEvent`.
- Mark it `#[serde(default)]`, following the existing `payload_json` pattern, so
legacy `event_json` blobs without `actor` deserialize to `None`.
  - Keep `#[serde(deny_unknown_fields)]` on the struct.

- Add test coverage in `audit.rs`:
  - Extend `round_trips_through_serde` to include a populated `actor`.
- Extend `target_and_payload_refs_default_to_empty_when_omitted` (or add a
sibling test) to omit `actor` from the JSON and assert it deserializes to
`None`.

Phase 2: Thread the actor through the audit-write path and add the audited-effect helper

This phase makes the write path carry actor, keeps the meta_json/event_json consistency guarantee intact, and introduces the with_audited_effect combinator that identity code will use.

Task 1: Carry actor through the audit append primitive

Persist actor in both JSON blobs without adding a SQL column.

• Add an actor: Option<PrincipalId> parameter to append_audit_conn_with_options in crates/openspine-kernel/src/store/audit_append.rs.
• Add "actor" to the meta serde_json::json!({ ... }) pre-image so it is part of the hashed bytes.
• Set actor on the constructed AuditEvent literal.
• Keep actor JSON-only (no ALTER TABLE, no new column), mirroring payload_json.
• Keep the plain append_audit_conn wrapper unchanged by passing None for actor, so its ~64 callers stay untouched; update the ~15 direct callers of append_audit_conn_with_options to pass None.

Task 2: Update audit cross-validation

Keep the meta_json and event_json agreement check complete.

• Add "actor" to the explicit field-agreement list in replay_audit_conn in crates/openspine-kernel/src/store/event_bus.rs.
• This is required because actor now appears in both blobs; omitting it would leave the new field outside the consistency guarantee.

Task 3: Add the with_audited_effect helper

Provide a general atomic "run effect, then append its audit row" combinator that accepts an actor.

• Create a new file under crates/openspine-kernel/src/store/ (for example audit_effect.rs) to respect the 500-line file cap, and wire it in with mod/#[path].
• Model the helper on mutate_briefcase_and_audit in briefcase_support.rs: lock the connection, BEGIN IMMEDIATE, run the caller closure, append the audit row on the same connection, then COMMIT on Ok and ROLLBACK on Err.
• Accept kind, reason, actor: Option<PrincipalId>, and an effect closure that performs the mutation.
• Route the audit write through append_audit_conn_with_options so the actor argument is threaded.

🤖 Prompt for AI agents

Thread the `actor` field through the audit-write path and add an audited-effect
helper. Goal: keep the `meta_json`/`event_json` consistency guarantee intact and
provide the atomic combinator that identity code will use in Phase 3.

- In `crates/openspine-kernel/src/store/audit_append.rs`:
- Add an `actor: Option<PrincipalId>` parameter to
`append_audit_conn_with_options`.
- Add `"actor"` to the `meta` `serde_json::json!({ ... })` pre-image so it is
part of the hashed bytes.
  - Set `actor` on the constructed `AuditEvent` literal.
- Keep `actor` JSON-only: do not add an `ALTER TABLE` or a new column, mirroring
the existing `payload_json` handling.
- Keep the plain `append_audit_conn` wrapper unchanged by passing `None` for
`actor`, so its approximately 64 callers stay untouched.
- Update the approximately 15 direct callers of `append_audit_conn_with_options`
to pass `None` for `actor`.

- In `crates/openspine-kernel/src/store/event_bus.rs`:
- Add `"actor"` to the explicit field-agreement list in `replay_audit_conn`,
because `actor` now appears in both `meta_json` and `event_json` and must be
covered by the consistency check.

- Add a `with_audited_effect` helper:
- Create a new file under `crates/openspine-kernel/src/store/` (for example
`audit_effect.rs`) to respect the 500-line file cap, and wire it into the module
tree with `mod`/`#[path]`.
- Model it on `mutate_briefcase_and_audit` in `briefcase_support.rs`: lock the
connection, `BEGIN IMMEDIATE`, run the caller closure, append the audit row on
the same connection, then `COMMIT` on `Ok` and `ROLLBACK` on `Err`.
- Accept `kind`, `reason`, `actor: Option<PrincipalId>`, and an effect closure
that performs the mutation.
- Route the audit write through `append_audit_conn_with_options` so the `actor`
argument is threaded through.

Phase 3: Wire identity binding, bootstrap, and mismatch to the typed actor and new kinds

This phase moves owner facts out of reason strings, adds the two new audit kinds, and keeps resolution read-only and identifiers hash-only.

Task 1: Rewire identity.bound to carry actor

Replace the embedded owner fact with the typed actor dimension.

• Update owner_assert_identity_binding in crates/openspine-kernel/src/store/identity.rs to emit identity.bound through with_audited_effect.
• Set actor = Some(PrincipalId::from(owner_principal_id)).
• Remove the owner={} identity={} owner fact from the reason string; keep only the bound-identity reference in reason (or move it to target_refs) if a target reference is still wanted.
• Preserve the existing atomicity guarantee (binding insert and audit row commit together).

Task 2: Add and emit the bootstrap audit kind

Record successful owner bootstrap binding as its own kind.

• Add a new audit kind string literal, identity.bootstrapped, emitted from the successful owner-creation path (try_insert_owner_principal / bootstrap_owner_principal) in store/identity.rs.
• Emit it through with_audited_effect so the owner principal insert and its audit row commit atomically.
• Set actor = Some(PrincipalId::from(new_owner_principal_id)).

Task 3: Add and emit the owner-config-mismatch rejection kind

Record a rejected owner-config match as a durable audit row.

• Add a new audit kind string literal, identity.owner_config_mismatch, emitted from ensure_owner_matches_config in store/identity.rs when the configured Telegram id does not match the stored owner identifier hash.
• Structure the effect so the rejection audit commits successfully, then return StoreError::NotOwner(...) to the caller; do not roll the audit back with the rejection.
• Set actor = Some(PrincipalId::from(stored_owner_principal_id)).
• A verified store with an intact audit_log is available at the main.rs bootstrap call site, so the audit row can commit before startup aborts.

Task 4: Keep resolution paths read-only

Confirm no write path is added to lookup functions.

• Leave get_identity and principal_exists in store/identity.rs as read-only SELECT queries.
• Do not add audit emission or writes to these functions.

Task 5: Add and extend tests

Prove the actor dimension, new kinds, read-only resolution, and hash-only storage.

• In crates/openspine-kernel/src/store/identity_tests.rs, assert that the identity.bound row carries actor = Some(<owner principal id>), using all_audit_event_jsons plus a serde_json::Value lookup by kind.
• Add tests that the bootstrap path emits identity.bootstrapped with a populated actor, and that the mismatch path emits identity.owner_config_mismatch with a populated actor while still returning StoreError::NotOwner.
• Assert that get_identity and principal_exists do not change any audit-row count.
• Assert that no raw identifier value appears in any persisted event_json, meta_json, reason, or identity_identifiers row (only hashes and ULIDs).

🤖 Prompt for AI agents

Wire identity binding, bootstrap, and mismatch handling to the typed actor and
the new audit kinds. Goal: move owner facts out of `reason` strings, add two new
audit kinds, and keep resolution read-only with hash-only identifiers.

- In `crates/openspine-kernel/src/store/identity.rs`, update
`owner_assert_identity_binding` to emit `identity.bound` through
`with_audited_effect`.
  - Set `actor = Some(PrincipalId::from(owner_principal_id))`.
- Remove the `owner={} identity={}` owner fact from the `reason` string. Keep
only the bound-identity reference in `reason` (or move it to `target_refs`) if a
target reference is still wanted.
- Preserve the existing atomicity guarantee so the binding insert and the audit
row commit together.

- Add a new audit kind string literal `identity.bootstrapped`.
- Emit it from the successful owner-creation path (`try_insert_owner_principal`
/ `bootstrap_owner_principal`) in `store/identity.rs`, through
`with_audited_effect`, so the owner principal insert and its audit row commit
atomically.
  - Set `actor = Some(PrincipalId::from(new_owner_principal_id))`.

- Add a new audit kind string literal `identity.owner_config_mismatch`.
- Emit it from `ensure_owner_matches_config` in `store/identity.rs` when the
configured Telegram id does not match the stored owner identifier hash.
- Structure the effect so the rejection audit commits successfully, then return
`StoreError::NotOwner(...)` to the caller. Do not roll back the audit together
with the rejection.
  - Set `actor = Some(PrincipalId::from(stored_owner_principal_id))`.
- Note that a verified store with an intact `audit_log` is available at the
`main.rs` bootstrap call site, so the audit row can commit before startup
aborts.

- Keep resolution read-only: leave `get_identity` and `principal_exists` in
`store/identity.rs` as read-only `SELECT` queries. Do not add audit emission or
writes to these functions.

- Add and extend tests in `crates/openspine-kernel/src/store/identity_tests.rs`:
- Assert that the `identity.bound` row carries `actor = Some(<owner principal
id>)`, using `all_audit_event_jsons` plus a `serde_json::Value` lookup by
`kind`.
- Add a test that the bootstrap path emits `identity.bootstrapped` with a
populated `actor`.
- Add a test that the mismatch path emits `identity.owner_config_mismatch` with
a populated `actor`, while still returning `StoreError::NotOwner`.
- Assert that `get_identity` and `principal_exists` do not change any audit-row
count.
- Assert that no raw identifier value appears in any persisted `event_json`,
`meta_json`, `reason`, or `identity_identifiers` row (only hashes and ULIDs are
present).

Research

The audit trail lives in crates/openspine-schemas/src/audit.rs (the AuditEvent struct and the AuditKind open-vocabulary newtype). It is written by crates/openspine-kernel/src/store/audit_append.rs through append_audit_conn and append_audit_conn_with_options.

The audit_log table stores each event as both a canonical meta_json hash pre-image and a full event_json blob. crates/openspine-kernel/src/store/event_bus.rs cross-validates the two on read.

Identity binding lives in crates/openspine-kernel/src/store/identity.rs. owner_assert_identity_binding emits the only identity.bound audit today (with owner facts embedded in a reason string). bootstrap_owner_principal and ensure_owner_matches_config emit no audit at all.

crates/openspine-kernel/src/store/briefcase_support.rs::mutate_briefcase_and_audit is the closest existing "audited effect" combinator.

Identifiers are stored hash-only in identity_identifiers.value_hash.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Implement the typed actor dimension for the audit schema. Goal: add
`PrincipalId` and `AuditEvent.actor` with backward-compatible serde defaults so
legacy audit rows still deserialize.

- In `crates/openspine-schemas/src/` (next to `principal.rs` or `ids.rs`), add a
`PrincipalId` newtype over `ulid::Ulid`.
  - Mark it `#[serde(transparent)]` so the wire form is a plain ULID string.
- Provide `From<Ulid>` / accessor conversions so callers holding a raw `Ulid`
can build a `PrincipalId` without wide refactors.
- Update the `ids.rs` module doc to note this is the first `Ulid`-backed nominal
id, following the existing convention notes.

- In `crates/openspine-schemas/src/audit.rs`, add `pub actor:
Option<PrincipalId>` to `AuditEvent`.
- Mark it `#[serde(default)]`, following the existing `payload_json` pattern, so
legacy `event_json` blobs without `actor` deserialize to `None`.
  - Keep `#[serde(deny_unknown_fields)]` on the struct.

- Add test coverage in `audit.rs`:
  - Extend `round_trips_through_serde` to include a populated `actor`.
- Extend `target_and_payload_refs_default_to_empty_when_omitted` (or add a
sibling test) to omit `actor` from the JSON and assert it deserializes to
`None`.
===============================================================================

Task: 2

Thread the `actor` field through the audit-write path and add an audited-effect
helper. Goal: keep the `meta_json`/`event_json` consistency guarantee intact and
provide the atomic combinator that identity code will use in Phase 3.

- In `crates/openspine-kernel/src/store/audit_append.rs`:
- Add an `actor: Option<PrincipalId>` parameter to
`append_audit_conn_with_options`.
- Add `"actor"` to the `meta` `serde_json::json!({ ... })` pre-image so it is
part of the hashed bytes.
  - Set `actor` on the constructed `AuditEvent` literal.
- Keep `actor` JSON-only: do not add an `ALTER TABLE` or a new column, mirroring
the existing `payload_json` handling.
- Keep the plain `append_audit_conn` wrapper unchanged by passing `None` for
`actor`, so its approximately 64 callers stay untouched.
- Update the approximately 15 direct callers of `append_audit_conn_with_options`
to pass `None` for `actor`.

- In `crates/openspine-kernel/src/store/event_bus.rs`:
- Add `"actor"` to the explicit field-agreement list in `replay_audit_conn`,
because `actor` now appears in both `meta_json` and `event_json` and must be
covered by the consistency check.

- Add a `with_audited_effect` helper:
- Create a new file under `crates/openspine-kernel/src/store/` (for example
`audit_effect.rs`) to respect the 500-line file cap, and wire it into the module
tree with `mod`/`#[path]`.
- Model it on `mutate_briefcase_and_audit` in `briefcase_support.rs`: lock the
connection, `BEGIN IMMEDIATE`, run the caller closure, append the audit row on
the same connection, then `COMMIT` on `Ok` and `ROLLBACK` on `Err`.
- Accept `kind`, `reason`, `actor: Option<PrincipalId>`, and an effect closure
that performs the mutation.
- Route the audit write through `append_audit_conn_with_options` so the `actor`
argument is threaded through.
===============================================================================

Task: 3

Wire identity binding, bootstrap, and mismatch handling to the typed actor and
the new audit kinds. Goal: move owner facts out of `reason` strings, add two new
audit kinds, and keep resolution read-only with hash-only identifiers.

- In `crates/openspine-kernel/src/store/identity.rs`, update
`owner_assert_identity_binding` to emit `identity.bound` through
`with_audited_effect`.
  - Set `actor = Some(PrincipalId::from(owner_principal_id))`.
- Remove the `owner={} identity={}` owner fact from the `reason` string. Keep
only the bound-identity reference in `reason` (or move it to `target_refs`) if a
target reference is still wanted.
- Preserve the existing atomicity guarantee so the binding insert and the audit
row commit together.

- Add a new audit kind string literal `identity.bootstrapped`.
- Emit it from the successful owner-creation path (`try_insert_owner_principal`
/ `bootstrap_owner_principal`) in `store/identity.rs`, through
`with_audited_effect`, so the owner principal insert and its audit row commit
atomically.
  - Set `actor = Some(PrincipalId::from(new_owner_principal_id))`.

- Add a new audit kind string literal `identity.owner_config_mismatch`.
- Emit it from `ensure_owner_matches_config` in `store/identity.rs` when the
configured Telegram id does not match the stored owner identifier hash.
- Structure the effect so the rejection audit commits successfully, then return
`StoreError::NotOwner(...)` to the caller. Do not roll back the audit together
with the rejection.
  - Set `actor = Some(PrincipalId::from(stored_owner_principal_id))`.
- Note that a verified store with an intact `audit_log` is available at the
`main.rs` bootstrap call site, so the audit row can commit before startup
aborts.

- Keep resolution read-only: leave `get_identity` and `principal_exists` in
`store/identity.rs` as read-only `SELECT` queries. Do not add audit emission or
writes to these functions.

- Add and extend tests in `crates/openspine-kernel/src/store/identity_tests.rs`:
- Assert that the `identity.bound` row carries `actor = Some(<owner principal
id>)`, using `all_audit_event_jsons` plus a `serde_json::Value` lookup by
`kind`.
- Add a test that the bootstrap path emits `identity.bootstrapped` with a
populated `actor`.
- Add a test that the mismatch path emits `identity.owner_config_mismatch` with
a populated `actor`, while still returning `StoreError::NotOwner`.
- Assert that `get_identity` and `principal_exists` do not change any audit-row
count.
- Assert that no raw identifier value appears in any persisted `event_json`,
`meta_json`, `reason`, or `identity_identifiers` row (only hashes and ULIDs are
present).

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!