feat(events): add user_identity field to DomainEvent and propagate through event pipeline #1257
No reviewers
Labels
No labels
auto/needs-reevaluation
controller-managed
auto/blocked-by-deps
auto/ci-timeout
auto/claimed-implementer
auto/claimed-merge
auto/claimed-reviewer
auto/driver-down
auto/invariant-violation
auto/last-attempt-tier-0
auto/last-attempt-tier-1
auto/last-attempt-tier-2
auto/last-attempt-tier-min
Automation Tracking
auto/needs-conflict-resolution
auto/needs-implementer
auto/postmortem
auto/ready-to-merge
auto/restart-throttled
auto/revert
auto/sentinel
auto/stale-inactivity
auto/unstable
Blocked
Bounty
$100
Bounty
$1000
Bounty
$10000
Bounty
$20
Bounty
$2000
Bounty
$250
Bounty
$50
Bounty
$500
Bounty
$5000
Bounty
$750
MoSCoW
Could have
MoSCoW
Must have
MoSCoW
Should have
Needs Feedback
Points
1
Points
13
Points
2
Points
21
Points
3
Points
34
Points
5
Points
55
Points
8
Points
88
Priority
Backlog
Priority
CI Blocker
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Signed-off: Owner
Signed-off: Scrum Master
Signed-off: Tech Lead
Spike
State
Completed
State
Duplicate
State
In Progress
State
In Review
State
Paused
State
Unverified
State
Verified
State
Wont Do
Type
Automation
Type
Bug
Type
Discussion
Type
Documentation
Type
Epic
Type
Feature
Type
Legendary
Type
Refactor
Type
Support
Type
Task
Type
Testing
No project
No assignees
3 participants
Notifications
Due date
No due date set.
Blocks
#715 feat(events): add user_identity field to DomainEvent and propagate through event pipeline
cleveragents/cleveragents-core
Reference
cleveragents/cleveragents-core!1257
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "feature/domain-event-user-identity"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
user_identity: str | Nonefield toDomainEventfrozen Pydantic model, enabling authenticated user identity to be carried as a first-class event attributeAuditEventSubscriberto preferevent.user_identityover legacy details-dict extraction with backward-compatible fallbackresource_cli_flags_904.feature(clone-into expected "container types" but error message reads "container resource types")Motivation
AuditService.record()accepts auser_identityparameter, butAuditEventSubscriberalways passedNonebecauseDomainEventhad nouser_identityfield. Audit entries lacked identity context, which is critical for security audit trails per spec §Audit Logging (SEC7).Approach
Added the field directly to
DomainEventas 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— passednox -s typecheck— 0 errors, 0 warningsnox -s unit_tests— 556 features, 13789 scenarios, 0 failuresnox -s integration_tests— new identity tests pass (9/9)nox -s coverage_report— passed (>=97%)Closes #715
Review: PR #1257 — feat(events): add user_identity field to DomainEvent and propagate through event pipeline
Checklist
AuditService.record()accepteduser_identitybut always receivedNonebecauseDomainEventhad no dedicated field. Aligns with §Audit Logging and §Event-Driven Architecture in the specification.feat(events): add user_identity field to DomainEvent and propagate through event pipeline— valid Conventional Changelog withISSUES CLOSED: #715footerDomainEvent.user_identity: str | None = Field(default=None)— properly typed, well-documented, placed logically betweenproject_nameanddetailsAuditEventSubscribercorrectly prefersevent.user_identityover details dict fallback withoroperator, and always pops from details to prevent duplication in stored JSONLoggingEventBusincludesuser_identityin structured log outputNone)redact_value()before audit persistence — prevents accidental secret leakage from tokens embedded in identity stringsneeds feedbacklabelDesign Notes
oroperator inevent.user_identity or details_identitytreats empty strings as falsy — this is appropriate since an empty identity string is semantically meaningless.Nonesince server-mode auth is not yet implemented (M9).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.
dbd97eb9c6b039b28309🔒 Claimed by pr-reviewer-5. Starting independent code review.
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 betweenproject_nameanddetails, fully documented in class docstring with spec reference (§Audit Logging). Backward compatible.AuditEventSubscriber(audit_event_subscriber.py):user_identityfrom details dict to prevent duplication, then prefersevent.user_identityover the popped value viaoroperator. Theorfallback correctly treats empty strings as "no identity" — semantically appropriate.redact_value()applied to identity before persistence — good security practice.LoggingEventBus(logging_bus.py):user_identity=event.user_identityto 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
Compliance
ISSUES CLOSED: #715footer# type: ignoresuppressionsneeds feedbacklabelAPPROVED — no issues found.
b039b283096a31f376acNew commits pushed, approval review dismissed automatically according to repository settings
Review claimed by reviewer pool instance reviewer-pool-1. Dispatching independent code review.
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 betweenproject_nameanddetails, fully documented in class docstring with spec reference. Backward compatible (defaults toNone). Frozen model ensures immutability.AuditEventSubscriber(audit_event_subscriber.py):if/elsebranching:event.user_identity is not None: uses field value, pops from details to prevent duplicationevent.user_identity is None: falls back toraw_details.pop("user_identity", None)for backward compatoroperator approach (from earlier revisions) because it correctly handles the case whereevent.user_identityis an empty string — theis Nonecheck is semantically precise.redact_value()applied to identity before persistence — good security practice.LoggingEventBus(logging_bus.py):emit()method does NOT includeuser_identityin its structured log output. This is a minor gap (non-blocking) — the critical audit path throughAuditEventSubscribercorrectly handles the field. A follow-up could adduser_identity=event.user_identityto the_logger.info()call for completeness.ReactiveEventBus(reactive.py): Correctly NOT modified — it doesn't do per-field structured logging.Test Review
user_identityis stripped from details after field wins — thorough.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
feat(events): ...withISSUES CLOSED: #715footer# type: ignoresuppressions in new codeCloses #715in PR bodyneeds feedbacklabelMinor Non-Blocking Observation
LoggingEventBus.emit()does not includeuser_identityin its structured log output. Consider adding it in a follow-up for parity with the field set onDomainEvent.APPROVED — merging via squash.