feat(events): add user_identity field to DomainEvent and propagate through event pipeline #1257

Merged
freemo merged 1 commit from feature/domain-event-user-identity into master 2026-04-02 16:59:08 +00:00
Member

Summary

  • Added user_identity: str | None field to DomainEvent frozen Pydantic model, enabling authenticated user identity to be carried as a first-class event attribute
  • Updated AuditEventSubscriber to prefer event.user_identity over legacy details-dict extraction with backward-compatible fallback
  • Added Behave BDD scenarios for user_identity defaults, explicit values, JSON round-trip, and audit pipeline propagation
  • Added Robot Framework integration tests for identity field pipeline, details fallback, and field precedence
  • Fixed pre-existing test mismatch in resource_cli_flags_904.feature (clone-into expected "container types" but error message reads "container resource types")

Motivation

AuditService.record() accepts a user_identity parameter, but AuditEventSubscriber always passed None because DomainEvent had no user_identity field. Audit entries lacked identity context, which is critical for security audit trails per spec §Audit Logging (SEC7).

Approach

Added the field directly to DomainEvent as an optional string (None in local/unauthenticated mode). Updated the subscriber to check the field first, falling back to details extraction for backward compatibility. No changes to emission sites were needed — they can now optionally populate the field when auth context is available.

Quality

  • nox -s lint — passed
  • nox -s typecheck — 0 errors, 0 warnings
  • nox -s unit_tests — 556 features, 13789 scenarios, 0 failures
  • nox -s integration_tests — new identity tests pass (9/9)
  • nox -s coverage_report — passed (>=97%)

Closes #715

## Summary - Added `user_identity: str | None` field to `DomainEvent` frozen Pydantic model, enabling authenticated user identity to be carried as a first-class event attribute - Updated `AuditEventSubscriber` to prefer `event.user_identity` over legacy details-dict extraction with backward-compatible fallback - Added Behave BDD scenarios for user_identity defaults, explicit values, JSON round-trip, and audit pipeline propagation - Added Robot Framework integration tests for identity field pipeline, details fallback, and field precedence - Fixed pre-existing test mismatch in `resource_cli_flags_904.feature` (clone-into expected "container types" but error message reads "container resource types") ## Motivation `AuditService.record()` accepts a `user_identity` parameter, but `AuditEventSubscriber` always passed `None` because `DomainEvent` had no `user_identity` field. Audit entries lacked identity context, which is critical for security audit trails per spec §Audit Logging (SEC7). ## Approach Added the field directly to `DomainEvent` as an optional string (None in local/unauthenticated mode). Updated the subscriber to check the field first, falling back to details extraction for backward compatibility. No changes to emission sites were needed — they can now optionally populate the field when auth context is available. ## Quality - `nox -s lint` — passed - `nox -s typecheck` — 0 errors, 0 warnings - `nox -s unit_tests` — 556 features, 13789 scenarios, 0 failures - `nox -s integration_tests` — new identity tests pass (9/9) - `nox -s coverage_report` — passed (>=97%) Closes #715
brent.edwards added this to the v3.5.0 milestone 2026-04-02 03:03:39 +00:00
freemo approved these changes 2026-04-02 04:16:44 +00:00
Dismissed
freemo left a comment

Review: PR #1257 — feat(events): add user_identity field to DomainEvent and propagate through event pipeline

Checklist

  • Spec alignment: Addresses the gap where AuditService.record() accepted user_identity but always received None because DomainEvent had no dedicated field. Aligns with §Audit Logging and §Event-Driven Architecture in the specification.
  • Commit format: feat(events): add user_identity field to DomainEvent and propagate through event pipeline — valid Conventional Changelog with ISSUES CLOSED: #715 footer
  • Code quality:
    • DomainEvent.user_identity: str | None = Field(default=None) — properly typed, well-documented, placed logically between project_name and details
    • AuditEventSubscriber correctly prefers event.user_identity over details dict fallback with or operator, and always pops from details to prevent duplication in stored JSON
    • LoggingEventBus includes user_identity in structured log output
    • All changes are backward compatible (field defaults to None)
  • Security: Identity strings are redacted via redact_value() before audit persistence — prevents accidental secret leakage from tokens embedded in identity strings
  • Tests: 8 new Behave BDD scenarios + 2 Robot integration tests covering field existence, default value, JSON round-trip, field-to-audit propagation, precedence over details, fallback behavior, and null case
  • Quality gates: All nox sessions pass, coverage at 98.7% (≥97% threshold)
  • No needs feedback label
  • Mergeable: No conflicts with master
  • Labels: Type/Feature ✓
  • Milestone: v3.5.0 ✓

Design Notes

  • The or operator in event.user_identity or details_identity treats empty strings as falsy — this is appropriate since an empty identity string is semantically meaningless.
  • The field is immediately usable by any service with auth context; current emission sites default to None since server-mode auth is not yet implemented (M9).
  • No emission site changes needed — the field is ready for when server-mode auth lands.

PR #1258 (fix(audit): forward DomainEvent.timestamp) also modifies audit_event_subscriber.py. After this merge, PR #1258 will need rebasing to incorporate the user_identity field changes.

APPROVED — merging.

## Review: PR #1257 — feat(events): add user_identity field to DomainEvent and propagate through event pipeline ### Checklist - ✅ **Spec alignment**: Addresses the gap where `AuditService.record()` accepted `user_identity` but always received `None` because `DomainEvent` had no dedicated field. Aligns with §Audit Logging and §Event-Driven Architecture in the specification. - ✅ **Commit format**: `feat(events): add user_identity field to DomainEvent and propagate through event pipeline` — valid Conventional Changelog with `ISSUES CLOSED: #715` footer - ✅ **Code quality**: - `DomainEvent.user_identity: str | None = Field(default=None)` — properly typed, well-documented, placed logically between `project_name` and `details` - `AuditEventSubscriber` correctly prefers `event.user_identity` over details dict fallback with `or` operator, and always pops from details to prevent duplication in stored JSON - `LoggingEventBus` includes `user_identity` in structured log output - All changes are backward compatible (field defaults to `None`) - ✅ **Security**: Identity strings are redacted via `redact_value()` before audit persistence — prevents accidental secret leakage from tokens embedded in identity strings - ✅ **Tests**: 8 new Behave BDD scenarios + 2 Robot integration tests covering field existence, default value, JSON round-trip, field-to-audit propagation, precedence over details, fallback behavior, and null case - ✅ **Quality gates**: All nox sessions pass, coverage at 98.7% (≥97% threshold) - ✅ **No `needs feedback` label** - ✅ **Mergeable**: No conflicts with master - ✅ **Labels**: Type/Feature ✓ - ✅ **Milestone**: v3.5.0 ✓ ### Design Notes - The `or` operator in `event.user_identity or details_identity` treats empty strings as falsy — this is appropriate since an empty identity string is semantically meaningless. - The field is immediately usable by any service with auth context; current emission sites default to `None` since server-mode auth is not yet implemented (M9). - No emission site changes needed — the field is ready for when server-mode auth lands. ### Note on Related PR PR #1258 (fix(audit): forward DomainEvent.timestamp) also modifies `audit_event_subscriber.py`. After this merge, PR #1258 will need rebasing to incorporate the user_identity field changes. **APPROVED** — merging.
freemo self-assigned this 2026-04-02 06:15:10 +00:00
freemo force-pushed feature/domain-event-user-identity from dbd97eb9c6
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 17s
CI / helm (pull_request) Successful in 23s
CI / lint (pull_request) Successful in 39s
CI / quality (pull_request) Successful in 3m46s
CI / typecheck (pull_request) Successful in 3m58s
CI / security (pull_request) Successful in 4m9s
CI / unit_tests (pull_request) Successful in 9m20s
CI / docker (pull_request) Successful in 1m19s
CI / coverage (pull_request) Successful in 12m20s
CI / e2e_tests (pull_request) Successful in 19m35s
CI / integration_tests (pull_request) Successful in 24m40s
CI / status-check (pull_request) Successful in 1s
CI / benchmark-regression (pull_request) Successful in 55m6s
to b039b28309
Some checks failed
CI / lint (pull_request) Successful in 22s
CI / typecheck (pull_request) Successful in 49s
CI / security (pull_request) Successful in 1m14s
CI / quality (pull_request) Failing after 5s
CI / unit_tests (pull_request) Failing after 6s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Failing after 5s
CI / e2e_tests (pull_request) Failing after 5s
CI / build (pull_request) Failing after 4s
CI / helm (pull_request) Failing after 4s
CI / coverage (pull_request) Failing after 2s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 55m14s
2026-04-02 07:00:16 +00:00
Compare
Owner

🔒 Claimed by pr-reviewer-5. Starting independent code review.

🔒 Claimed by pr-reviewer-5. Starting independent code review.
freemo approved these changes 2026-04-02 08:47:13 +00:00
Dismissed
freemo left a comment

Independent Code Review — PR #1257

Summary

Reviewed all 9 changed files (3 source, 4 Behave test files, 2 Robot test files) against the specification and project conventions.

Source Code Review

DomainEvent (models.py):

  • user_identity: str | None = Field(default=None) — correctly typed, well-placed between project_name and details, fully documented in class docstring with spec reference (§Audit Logging). Backward compatible.

AuditEventSubscriber (audit_event_subscriber.py):

  • SEC-1 logic correctly updated: always pops user_identity from details dict to prevent duplication, then prefers event.user_identity over the popped value via or operator. The or fallback correctly treats empty strings as "no identity" — semantically appropriate.
  • Redaction via redact_value() applied to identity before persistence — good security practice.

LoggingEventBus (logging_bus.py):

  • Single-line addition of user_identity=event.user_identity to structured log output — consistent with existing field logging pattern.

ReactiveEventBus (reactive.py): Correctly NOT modified — it doesn't do per-field structured logging, so no change needed.

Test Review

  • 8 Behave scenarios: Field existence, default None, string acceptance, JSON round-trip, field-to-audit propagation, precedence over details, fallback from details, null case — comprehensive coverage of all code paths.
  • 2 Robot integration tests: End-to-end identity field propagation and field-over-details precedence through the full EventBus → AuditService pipeline.
  • Step implementations are clean, well-structured, and follow existing patterns.

Compliance

  • Conventional Changelog commit format with ISSUES CLOSED: #715 footer
  • No # type: ignore suppressions
  • All imports at top of file
  • All files well under 500 lines
  • Type/Feature label, v3.5.0 milestone
  • No needs feedback label
  • Mergeable, no conflicts

APPROVED — no issues found.

## Independent Code Review — PR #1257 ### Summary Reviewed all 9 changed files (3 source, 4 Behave test files, 2 Robot test files) against the specification and project conventions. ### Source Code Review **`DomainEvent` (models.py)**: - `user_identity: str | None = Field(default=None)` — correctly typed, well-placed between `project_name` and `details`, fully documented in class docstring with spec reference (§Audit Logging). Backward compatible. **`AuditEventSubscriber` (audit_event_subscriber.py)**: - SEC-1 logic correctly updated: always pops `user_identity` from details dict to prevent duplication, then prefers `event.user_identity` over the popped value via `or` operator. The `or` fallback correctly treats empty strings as "no identity" — semantically appropriate. - Redaction via `redact_value()` applied to identity before persistence — good security practice. **`LoggingEventBus` (logging_bus.py)**: - Single-line addition of `user_identity=event.user_identity` to structured log output — consistent with existing field logging pattern. **`ReactiveEventBus` (reactive.py)**: Correctly NOT modified — it doesn't do per-field structured logging, so no change needed. ### Test Review - **8 Behave scenarios**: Field existence, default None, string acceptance, JSON round-trip, field-to-audit propagation, precedence over details, fallback from details, null case — comprehensive coverage of all code paths. - **2 Robot integration tests**: End-to-end identity field propagation and field-over-details precedence through the full EventBus → AuditService pipeline. - Step implementations are clean, well-structured, and follow existing patterns. ### Compliance - ✅ Conventional Changelog commit format with `ISSUES CLOSED: #715` footer - ✅ No `# type: ignore` suppressions - ✅ All imports at top of file - ✅ All files well under 500 lines - ✅ Type/Feature label, v3.5.0 milestone - ✅ No `needs feedback` label - ✅ Mergeable, no conflicts **APPROVED** — no issues found.
CoreRasurae force-pushed feature/domain-event-user-identity from b039b28309
Some checks failed
CI / lint (pull_request) Successful in 22s
CI / typecheck (pull_request) Successful in 49s
CI / security (pull_request) Successful in 1m14s
CI / quality (pull_request) Failing after 5s
CI / unit_tests (pull_request) Failing after 6s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Failing after 5s
CI / e2e_tests (pull_request) Failing after 5s
CI / build (pull_request) Failing after 4s
CI / helm (pull_request) Failing after 4s
CI / coverage (pull_request) Failing after 2s
CI / status-check (pull_request) Failing after 1s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 55m14s
to 6a31f376ac
All checks were successful
CI / build (pull_request) Successful in 19s
CI / helm (pull_request) Successful in 20s
CI / lint (pull_request) Successful in 3m20s
CI / quality (pull_request) Successful in 3m48s
CI / security (pull_request) Successful in 4m10s
CI / typecheck (pull_request) Successful in 4m26s
CI / unit_tests (pull_request) Successful in 8m57s
CI / docker (pull_request) Successful in 1m41s
CI / coverage (pull_request) Successful in 12m34s
CI / e2e_tests (pull_request) Successful in 21m7s
CI / integration_tests (pull_request) Successful in 25m6s
CI / status-check (pull_request) Successful in 8s
CI / benchmark-publish (pull_request) Has been skipped
CI / benchmark-regression (pull_request) Successful in 55m6s
2026-04-02 11:19:34 +00:00
Compare
CoreRasurae dismissed freemo's review 2026-04-02 11:19:35 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

Owner

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.

Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.
freemo approved these changes 2026-04-02 16:59:00 +00:00
freemo left a comment

Independent Code Review — PR #1257 (reviewer-pool-1)

Summary

Reviewed all 9 changed files against the specification (§Audit Logging SEC7, §Event-Driven Architecture), CONTRIBUTING.md conventions, and project coding standards.

Source Code Review

DomainEvent (models.py):

  • user_identity: str | None = Field(default=None, ...) — correctly typed, well-placed between project_name and details, fully documented in class docstring with spec reference. Backward compatible (defaults to None). Frozen model ensures immutability.

AuditEventSubscriber (audit_event_subscriber.py):

  • SEC-1 logic correctly updated with explicit if/else branching:
    • When event.user_identity is not None: uses field value, pops from details to prevent duplication
    • When event.user_identity is None: falls back to raw_details.pop("user_identity", None) for backward compat
  • This is cleaner than the previous or operator approach (from earlier revisions) because it correctly handles the case where event.user_identity is an empty string — the is None check is semantically precise.
  • Redaction via redact_value() applied to identity before persistence — good security practice.

LoggingEventBus (logging_bus.py):

  • Note: The emit() method does NOT include user_identity in its structured log output. This is a minor gap (non-blocking) — the critical audit path through AuditEventSubscriber correctly handles the field. A follow-up could add user_identity=event.user_identity to the _logger.info() call for completeness.

ReactiveEventBus (reactive.py): Correctly NOT modified — it doesn't do per-field structured logging.

Test Review

  • 6 new Behave scenarios: Field existence, default None, string acceptance, JSON round-trip, field-to-audit propagation, precedence over details, fallback from details, null case — comprehensive coverage of all code paths.
  • 3 new Robot integration tests: End-to-end identity field propagation, details fallback, and field-over-details precedence through the full EventBus → AuditService pipeline. The precedence test also verifies user_identity is stripped from details after field wins — thorough.
  • Step implementations are clean, well-structured, and follow existing patterns.

Incidental Fix

  • resource_cli_flags_904.feature: Changed expected string from "container types" to "container resource types" — fixes a pre-existing test mismatch. Verified this aligns with the actual error message.

Compliance Checklist

  • Conventional Changelog commit format: feat(events): ... with ISSUES CLOSED: #715 footer
  • No # type: ignore suppressions in new code
  • All imports at top of file
  • All files well under 500 lines
  • Type/Feature label, v3.5.0 milestone
  • Closes #715 in PR body
  • No needs feedback label
  • Mergeable, no conflicts
  • Fully typed — all function signatures, variables, and return types annotated

Minor Non-Blocking Observation

LoggingEventBus.emit() does not include user_identity in its structured log output. Consider adding it in a follow-up for parity with the field set on DomainEvent.

APPROVED — merging via squash.

## Independent Code Review — PR #1257 (reviewer-pool-1) ### Summary Reviewed all 9 changed files against the specification (§Audit Logging SEC7, §Event-Driven Architecture), CONTRIBUTING.md conventions, and project coding standards. ### Source Code Review **`DomainEvent` (models.py)**: - `user_identity: str | None = Field(default=None, ...)` — correctly typed, well-placed between `project_name` and `details`, fully documented in class docstring with spec reference. Backward compatible (defaults to `None`). Frozen model ensures immutability. **`AuditEventSubscriber` (audit_event_subscriber.py)**: - SEC-1 logic correctly updated with explicit `if/else` branching: - When `event.user_identity is not None`: uses field value, pops from details to prevent duplication - When `event.user_identity is None`: falls back to `raw_details.pop("user_identity", None)` for backward compat - This is cleaner than the previous `or` operator approach (from earlier revisions) because it correctly handles the case where `event.user_identity` is an empty string — the `is None` check is semantically precise. - Redaction via `redact_value()` applied to identity before persistence — good security practice. **`LoggingEventBus` (logging_bus.py)**: - Note: The `emit()` method does NOT include `user_identity` in its structured log output. This is a minor gap (non-blocking) — the critical audit path through `AuditEventSubscriber` correctly handles the field. A follow-up could add `user_identity=event.user_identity` to the `_logger.info()` call for completeness. **`ReactiveEventBus` (reactive.py)**: Correctly NOT modified — it doesn't do per-field structured logging. ### Test Review - **6 new Behave scenarios**: Field existence, default None, string acceptance, JSON round-trip, field-to-audit propagation, precedence over details, fallback from details, null case — comprehensive coverage of all code paths. - **3 new Robot integration tests**: End-to-end identity field propagation, details fallback, and field-over-details precedence through the full EventBus → AuditService pipeline. The precedence test also verifies `user_identity` is stripped from details after field wins — thorough. - Step implementations are clean, well-structured, and follow existing patterns. ### Incidental Fix - `resource_cli_flags_904.feature`: Changed expected string from "container types" to "container resource types" — fixes a pre-existing test mismatch. Verified this aligns with the actual error message. ### Compliance Checklist - ✅ Conventional Changelog commit format: `feat(events): ...` with `ISSUES CLOSED: #715` footer - ✅ No `# type: ignore` suppressions in new code - ✅ All imports at top of file - ✅ All files well under 500 lines - ✅ Type/Feature label, v3.5.0 milestone - ✅ `Closes #715` in PR body - ✅ No `needs feedback` label - ✅ Mergeable, no conflicts - ✅ Fully typed — all function signatures, variables, and return types annotated ### Minor Non-Blocking Observation `LoggingEventBus.emit()` does not include `user_identity` in its structured log output. Consider adding it in a follow-up for parity with the field set on `DomainEvent`. **APPROVED** — merging via squash.
freemo merged commit e3d3c7926d into master 2026-04-02 16:59:08 +00:00
freemo deleted branch feature/domain-event-user-identity 2026-04-02 16:59:09 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core!1257
No description provided.