[mforce]

Before Access's contract is declared, assignment semantics are duplicated across three callers and lifecycle verbs construct the credential/Redis graph. This moves assignment interpretation into IAccessLookup, routes live assignment admission through it, splits the lifecycle graph, and moves the original provisioning outcome instead of copying it.

E1 of the approved E1/E2 split. E2 declares the 28-entry contract, converts the seeders and folds in #1023's two Access ownership claims. Refs #857.

Change map

Before
  middleware / FlockScopeGuard / ConfirmSale
    -> IAccessLookup.ListFlockAssignmentsAsync -> rows -> three local interpretations
  AssignFlock -> IIdentityProvider.GetEffectiveRoleAsync -> shared static read
  suspend / reactivate / rename -> IAccessOperations
    -> bootstrap / recovery / provision / suspension / rename services
    -> Identity -> shared-state registry -> Redis
  provision -> Infrastructure.AccountProvisionOutcome
    -> copied Application.ProvisionedAccountDetails

After
  middleware / FlockScopeGuard / ConfirmSale
    -> IAccessLookup.GetAssignedFlocksAsync -> null or complete Guid set
  AssignFlock -> IAccessLookup.GetEffectiveRoleAsync -> private scoped read
    (proof -> identity.ListUsersAsync -> live role -> flock/duplicate read -> write)
  suspend / reactivate / rename -> IAccessAccountLifecycle
    -> suspension / rename services
  bootstrap / recover / provision -> IAccessOperations
    -> bootstrap / recovery / provision services
  provision -> Application.AccountProvisionOutcome -> direct Result forwarding

The outcome move removes the three stale outcome symbols from Access→Commerce, Access→EggOperations and Access→Farm. The matrix is regenerated. Two tenant-bypass role-query rows are rekeyed to the private instance method with their query token hashes retained. #1037 T1–T4 and R3-1/R3-2, and #1047 F1/F2 land here. The approved #1041 session reshape deferral lands in E2's decision record.

Review in this order

1. IAccessLookup and AccessLookup: null means zero rows or any null-flock row; database and cancellation failures propagate. The read keeps its tenant filter, user predicate, AsNoTracking, full materialization and scoped context.
2. Three consumers and AssignFlockHandler: resolved-actor/role gates, proof-first admission and account→order→live-role→assignments→stock sequencing.
3. IAccessAccountLifecycle, AccessAccountLifecycle, CLI wiring and the original outcome move. The new dependency graph is exercised through the real registrations and commands.
4. Tests and generated ledger/matrix changes.

Properties to try to refute

• A failed assignment read can never grant unrestricted access. A primed lookup observes a later independent commit; confirm uses a fresh read after both locks.
• Zero or mixed farm-wide rows grant the existing account-wide scope; every other assigned Guid survives. A caller never receives another tenant's or user's assignments.
• Missing proof reveals no admission target, whether the requested user exists or not. Disabled and elevated multi-role targets retain refusal.
• ICurrentUser aliases the scoped CurrentUserContext; another scope has a different actor and lookup. Their tenant state remains aligned with the context filter.
• Suspend/reactivate/rename never construct Identity or Redis. Provisioning returns the committed farm/Owner fields and the one working password printed by the real verb.

Mutation proof

All 27 mutants fail an executed named assertion. No compilation, inotify, Docker or other environment error is counted as red. Each mutation is restored before the next run.

Mutation	Failing named assertion	Quoted failure
assignment-before-account-lock	ConfirmSaleAssignmentReadTests.ConfirmSale_ParkedOnTheAccountLock_ReadsAnAssignmentRemovalThatCommittedWhileItWaited	“Assert.Equal() Failure: Values differ; Expected: OK; Actual: UnprocessableEntity”
assignment-before-order-lock	ConfirmSaleOrderBoundaryTests.ConfirmReadsRoleAndAssignmentsAfterTheOrderLock(disableWorker: True)	“Assert.Equal() Failure: Collections differ; Expected: ["account lock", "order lock", "active actor"]; Actual: ["account lock", "assignments", "order lock", "active actor"]”
cache-assignments	AccessAuthorizationContractTests.AssignmentReadObservesIndependentCommitInTheSameScope	“zero committed assignment rows must grant unrestricted scope after an independent deletion”
display-role-instead-of-live-role	AccessAuthorizationContractTests.AssignmentAdmissionRefusesDisabledAndMultipleRoleTargets(disabled: True)	“Assert.Equal() Failure: Values differ; Expected: UnprocessableEntity; Actual: Created”
drop-active-member-predicate	ConfirmSaleOrderBoundaryTests.ConfirmReadsRoleAndAssignmentsAfterTheOrderLock(disableWorker: True)	“Assert.Equal() Failure: Values differ; Expected: Forbidden; Actual: UnprocessableEntity”
drop-assignment-cancellation	AccessLookupTests.GetAssignedFlocks_UsesTheScopedContextsTenantAndForwardsCancellation	“Assert.ThrowsAny() Failure: No exception was thrown”
drop-one-assigned-flock	AccessLookupTests.GetAssignedFlocks_ReturnsNullForMixedFarmWideRowsAndOnlyThatUsersAssignedSet	“Assert.Equal() Failure: HashSets differ”
drop-role-account-predicate	AccessLookupTests.GetEffectiveRole_AnswersOnlyForTheAccountItIsAskedAbout	“Assert.Null() Failure: Value of type 'Nullable' has a value”
drop-tenant-filter	AccessLookupTests.GetAssignedFlocks_UsesTheScopedContextsTenantAndForwardsCancellation	“Assert.Null() Failure: Value is not null”
drop-unresolved-tenant-refusal	AccessLookupTests.GetAssignedFlocks_RefusesAnUnresolvedTenant	“Assert.Throws() Failure: No exception was thrown”
drop-user-predicate	AccessLookupTests.GetAssignedFlocks_ReturnsNullForMixedFarmWideRowsAndOnlyThatUsersAssignedSet	“Assert.Equal() Failure: Collections differ”
effective-role-before-proof	AccessAuthorizationContractTests.MissingAssignmentProofReadsNoTargetsForKnownOrUnknownUsers	“Assert.Empty() Failure: Collection was not empty”
identity-in-lifecycle	AccessLifecycleDependencyTests.RealLifecycleVerbNeverConstructsIdentityOrRedis(verb: "rename-account", poisonRedis: True)	“Assert.Equal() Failure: Values differ; Expected: 0; Actual: 1”
mixed-farm-wide-is-restricted	AccessLookupTests.AWorkerWithAFlockRowAndAFarmWideRow_IsUnrestricted	“Assert.Equal() Failure: HashSets differ”
printed-provision-password-corrupt	ProvisionAccountCommandTests.ProvisionAccount_UnderDmlOnlyRole_PrintsOneWorkingPasswordAfterCommit	“Assert.Equal() Failure: Values differ; Expected: OK; Actual: Unauthorized”
provision-account-id-corrupt	AccessLifecycleDependencyTests.ProvisioningOutcomeNamesTheCommittedFarmAndOwnerAndCarriesTheirWorkingPassword	“Assert.Equal() Failure: Values differ”
provision-owner-email-corrupt	AccessLifecycleDependencyTests.ProvisioningOutcomeNamesTheCommittedFarmAndOwnerAndCarriesTheirWorkingPassword	“Assert.Equal() Failure: Strings differ”
provision-slug-corrupt	AccessLifecycleDependencyTests.ProvisioningOutcomeNamesTheCommittedFarmAndOwnerAndCarriesTheirWorkingPassword	“Assert.Equal() Failure: Strings differ”
redis-in-lifecycle	AccessLifecycleDependencyTests.RealLifecycleVerbNeverConstructsIdentityOrRedis(verb: "suspend-account", poisonRedis: True)	“Assert.Equal() Failure: Values differ; Expected: 0; Actual: 1”
role-before-account-lock	SaleAllocationPolicyTests.ConfirmSale_ParkedOnTheAccountLock_IsForbidden_WhenTheActorIsDisabledWhileItWaits	“Assert.Equal() Failure: Values differ; Expected: Forbidden; Actual: OK”
role-before-order-lock	ConfirmSaleOrderBoundaryTests.ConfirmReadsRoleAndAssignmentsAfterTheOrderLock(disableWorker: False)	“Assert.Equal() Failure: Collections differ; Expected: ["account lock", "order lock", "active actor", "effective role", "assignments", ···]; Actual: ["account lock", "active actor", "effective role", "order lock", "assignments", ···]”
second-actor-instance	AccessAuthorizationContractTests.ScopedActorAndLookupShareStateAndRemainIndependent	“Assert.Same() Failure: Values are not the same instance”
singleton-actor	AccessAuthorizationContractTests.ScopedActorAndLookupShareStateAndRemainIndependent	“Assert.NotSame() Failure: Values are the same instance”
singleton-lookup	AccessAuthorizationContractTests.ScopedActorAndLookupShareStateAndRemainIndependent	“Assert.NotSame() Failure: Values are the same instance”
swallow-assignment-failure	AccessAuthorizationContractTests.AssignmentReadFailurePropagatesInsteadOfGrantingUnrestrictedScope	“Assert.Throws() Failure: No exception was thrown”
target-before-proof	AccessAuthorizationContractTests.MissingAssignmentProofReadsNoTargetsForKnownOrUnknownUsers	“Assert.Empty() Failure: Collection was not empty”
zero-is-restricted	AccessLookupTests.GetAssignedFlocks_ReturnsNullForMixedFarmWideRowsAndOnlyThatUsersAssignedSet	“Assert.Null() Failure: Value is not null”

The display-role mutation explicitly maps the list's Worker label to the Worker enum. Feeding that label into Roles.ResolveEffective would create a different mutation, because actual Workers have zero role rows. The corrected mutation admits the disabled target and fails the status assertion. The order-lock mutations fail the exact transaction statement sequence, in addition to the parked-request behavior.

Security-suite edits approved by the owner

The four mechanical edits are: delete the obsolete decorator forwarder in MustChangePasswordGateTests and StepUpAuthTests; add the Users import in AccountProvisioningTests and TwoFarmIsolationMatrixTests. Their assertions remain intact. Separate tidy-ups fix the shared tenant fixture and two stale comments. New tests cover the additional security observations.

Deslop and quality record

Ran deslop, pstack:thermo-nuclear-code-quality-review and ponytail-review myself. Deleted duplicate assignment interpretation, the outcome copy/mapping and the dead role forwarding surface. Reused TestHarness.SeedSalesOrderAsync in the new lock-boundary fixture, deleting unrelated HTTP setup. No file crosses 1000 lines; the two existing giant files shrink. No remaining structural or over-engineering finding. Independent Astra and Opus security review remains the hold point for this PR.

Validation

• Solution build: zero warnings/errors.
• Full Application suite: 704 passed, zero failed/skipped; includes ledger, matrix, peer, compatibility and tenant-bypass guards.
• 353 targeted integration cases across 41 walked classes, one class at a time, zero failed/skipped. Includes cookie/refresh sibling classes, step-up, credential epoch, suspension, retries, one-shot verbs, provisioning, purge and both farms.
• 27 mutants red, with restoration between cases and final clean runs.
• Base/head B trace: all 42 SQL/HTTP files identical. D trace: both Production verb/snapshot and purge files identical, after normalization of @t and quoted/numeric elapsed only.
• Six real lifecycle DI cases: 6 red on main, 6 green after the split, for all three verbs with both throwing factories.
• The four approved test-file edits were compared to main after removing only their mechanical change; assertion bodies are unchanged.

Local evidence: /home/mforce/.cluckwork-slices/reviews/857-E1-evidence/; quality reports: /home/mforce/.cluckwork-slices/reviews/857-E1-quality/. Scratch harnesses and mutation tools stay outside the repository.

[mforce]

Opus security review: #1055 at 0a84a2eb (#857 E1)

Verdict: no P0, P1 or P2 findings. Security can approve. Two P3 notes for E2 and three P3 quality nits follow. I wrote no product code and pushed nothing.

Method: I read the saved diff and the head source. I traced auth, credential epoch, suspension, tenant isolation, cross-farm reads, flock-scope semantics and the lifecycle verbs' dependency closure. I ran targeted test classes one at a time and four independent mutants, each restored before the next run.

Verified properties

• The three callers keep the same assignment semantics. Old middleware, FlockScopeGuard and ConfirmSaleHandler each treated zero rows or any FlockId == null row as unrestricted, and otherwise allowed only the assigned set. AccessLookup.GetAssignedFlocksAsync (AccessLookup.cs:17-34) now returns null for exactly that case and the complete set otherwise. FlockScope.Resolve copies the set defensively, so no shared reference leaks.
• The unresolved-tenant refusal still runs before any connection. The lookup still filters by tenant and by user id. In production, TenantContext is scoped and AppDbContext is registered with AddDbContext, not a pool. The lookup and the context's query filter therefore read one instance. The FlockScopeGuardTests change (R3-1) fixes a test that passed two different TenantContexts.
• AssignFlock admission keeps its order: proof, then ListUsersAsync membership, then the live role. The role read is the same active-member, account-predicated query on the same scoped context, now private to AccessLookup. IdentityProvider has lost the member, so the two copies cannot drift apart.
• ConfirmSale is unchanged except for the assignment call. The new ConfirmSaleOrderBoundaryTests pins this order: account lock, order lock, live role, assignments, stock lock.
• Credential epoch and suspension: CredentialEpochMiddleware/Verifier, AccountSuspensionService and AccountRenameService are untouched. The verbs call the same services, so revocation in the same transaction and the audit rows are unchanged.
• The lifecycle closure does not reach Identity, the credential graph or Redis. AccessAccountLifecycle depends only on AccountSuspensionService and AccountRenameService. Their constructors take AppDbContext, TenantContext, IAccountRepository, TimeProvider, IAuditWriter and CurrentUserContext.
• Cross-farm reads: the diff adds no IgnoreQueryFilters and no filter-free set. The two TSV rows are re-keyed with unchanged token hashes (4adcdb84).
• Provisioning outcome: it keeps the same four fields. The temporary password still reaches only stdout (ProvisionAccountCliCommand.cs:71). Like the record it replaces, the record's ToString would include the password. No logger is passed the outcome, so this is not new exposure.

Findings

S1 (P3, for E2's contract): null means account-wide, and the lookup also returns it for unknown or other-farm user ids.

• Where: IAccessLookup.cs:13-16, AccessLookup.cs:31-33. The tests pin the behaviour at AccessLookupTests.cs:74 (random id → null) and :126 (farm B's worker read under farm A → null).
• Today it is safe. All three callers pass the resolved actor's own id under the same tenant.
• Failure scenario: E2 declares this port as an Access contract. A future caller asks "may user X see flock Y" with a target id. A stale, mistyped or foreign id then gets the most permissive answer instead of a refusal.
• Remedy (E2): state in the contract comment that userId must be the resolved actor or a verified member of the tenant. Or return a named result, such as Unrestricted | Restricted(set), so the open answer cannot be a bare null. This is also the main quality point under Q1.

S2 (P3, test coverage): the lifecycle closure test does not cover UserManager.

• Where: AccessLifecycleDependencyTests.cs:67,76 poison only IConnectionMultiplexer and IIdentityProvider. The design's F1 also names UserManager.
• Evidence: mutant 4 added a UserManager<ApplicationUser> dependency to AccessAccountLifecycle, and the test stayed green (7/7).
• Impact: none at E1, because UserManager does not reach Redis or the credential graph.
• Remedy: if E2 relies on "UserManager never enters the lifecycle closure", poison it too, or narrow the claim to Identity and Redis.

Mutants I ran (independent of the PR's 27)

Mutant	Result
M1: zero rows → restricted (drop Count == 0)	Red. AccessLookupTests failed 2 and AccessAuthorizationContractTests.AssignmentReadObservesIndependentCommitInTheSameScope failed
M2: middleware always Resolve(true, [])	FlockScopeMiddlewareTests stayed green (13/13). FlockScopeTests went red with 6 failures, including ScopedWorker_FlocksList_SeesOnlyAssignedFlock and ScopedWorker_UnassignedFlockDetail_Returns404
M3: UserManager + ICredentialEpochVerifier + IStepUpGrantService in the lifecycle	Red. 3 failures, all on the poisonRedis: True rows
M4: UserManager alone in the lifecycle	Green (S2)

For M2, FlockScopeTests is the class that guards the changed middleware line, not FlockScopeMiddlewareTests. It is listed in the PR's affected-classes.txt.

Head runs (each class alone; integration under sg docker)

AccessLookupTests 6/6, AccessAuthorizationContractTests 6/6, AccessLifecycleDependencyTests 7/7, ConfirmSaleOrderBoundaryTests 2/2, FlockScopeMiddlewareTests 13/13, ConfirmSaleAssignmentReadTests 1/1. The Application guards passed 14/14: FlockScopeGuardTests, ModuleLedgerRealTreeTests, CouplingMatrixRealTreeTests, TenantBypassRealTreeTests, AdapterReachRealTreeTests, SeamSurfaceRealAssemblyTests and PeerContractRealTreeTests. Logs are in /home/mforce/.cluckwork-slices/reviews/1055-opus-evidence/.

Code quality (P3, slice-scoped)

Thermo-nuclear review. The slice deletes more than it adds, and the main restructuring is already done: three local interpretations collapse into one answer owned by the lookup. No file crosses 1k lines (IdentityProvider.cs shrinks), and no new conditionals are added to shared paths.

• Q1: the IReadOnlySet<Guid>? boundary uses null for the most permissive state. The middleware then has to check it twice: scope.Resolve(assigned is null, assigned is null ? [] : assigned) at FlockScopeResolutionMiddleware.cs:58. A named result (S1) would remove both checks.
• Q2 (accepted): AccessAccountLifecycle only forwards to two services. It is accepted as the Application port that E2 declares, and it mirrors AccessOperations.

Ponytail review.

• Q3, MustChangePasswordGateTests.cs:L276 and StepUpAuthTests.cs:L712: delete: blank line left before the closing brace after the forwarders were removed.
• Q3, AccountProvisioner.cs:L2: shrink: using Cluckwork.Application.Features.Users; sits above ...Features.Accounts. Sort it.

Net: -2 lines possible. The rest is lean.

[mforce]

PR #1055 independent Astra review

Approve E1 at 0a84a2eb9ebd9f1c5580b9f543577496c6d1b4fa. No P0, P1 or P2 product findings. One optional P3 simplification from the two requested quality passes. Reviewed against 7014566c, the E1 design and the earlier Astra design check, without delegation or product edits.

Finding

P3, optional, E1 only: remove the now single-caller effective-role helper. src/Cluckwork.Infrastructure/Identity/AccessLookup.cs:13 forwards to the private implementation at line 43. Removing IdentityProvider's forwarder leaves no other caller. Put the async body directly on GetEffectiveRoleAsync, retain its predicates and cancellation, and rekey the two TSV classifications without changing query tokens. This removes one navigation step and four lines. There is no runtime failure at this head; the concern is unnecessary indirection during future authorization changes. The approved design explicitly requested a private helper, so this is a nonblocking refinement. Source.

Both pstack:thermo-nuclear-code-quality-review and ponytail-review produced this same P3 finding. The lifecycle facade earns its delegations by isolating the dependency graph. The production diff removes 41 net lines; no file crosses 1,000 lines, and both existing files above that threshold shrink. No broader session or IdentityProvider rewrite is requested. Ponytail net: 4 lines removable.

Product checks

• Tenant and assignment semantics hold. AccessLookup.cs:23 refuses unresolved tenants before querying; lines 28-33 retain the account filter, user predicate, untracked read and cancellation. Real DI supplies the same scoped tenant to the lookup and context. Zero rows or any null flock returns null; otherwise every assigned Guid survives. Database failures propagate. The middleware, write guard and confirm preserve their actor/role gates.
• Lock and admission order hold. ConfirmSaleHandler.cs:113 still locks account, then order, reads active/effective role, reads Worker assignments, then locks stock. Both parked-lock tests pass; the new test also checks statement order and final quantities. AssignFlockHandler.cs:27 validates proof before target and live-role reads. Missing proof reads no targets; disabled and elevated multi-role targets remain refused.
• Lifecycle dependencies and provisioning hold. AccessAccountLifecycle.cs:7 takes only suspension/rename services. All six real-command cases pass with throwing Identity or Redis factories. The moved AccountProvisionOutcome at AccessOperationOutcomes.cs:39 retains all four non-null fields, and AccessOperations forwards the original Result. The real provisioning command prints one working password after commit; lifecycle command behavior and outputs pass their existing tests.
• Ledger and query checks hold. The three stale outcome symbols are removed without widening edge permissions. Matrix, adapter, peer, compatibility, seam and contract guards pass. The two effective-role TSV entries retain hash 4adcdb84. Query-shape tests and the selected integration queries pass with test: fail the integration suite on EF Core query-shape warnings #1039 enabled. No query-policy opt-out was added.

Independent verification

172 passed, 0 failed, 0 skipped: 155 integration cases across 14 classes and 17 Application cases across 9 classes. Every invocation selected one class. Integration ran through sg docker -c; every run used DOTNET_USE_POLLING_FILE_WATCHER=1. No full integration suite was run.

Coverage	Classes	Passed
Lookup, live authorization and lifecycle dependencies	AccessLookupTests, AccessAuthorizationContractTests, AccessLifecycleDependencyTests	19
Account/order locks and flock scope	ConfirmSaleAssignmentReadTests, ConfirmSaleOrderBoundaryTests, FlockScopeMiddlewareTests, SaleAllocationPolicyTests	29
Real lifecycle and provisioning commands/outcomes	AccountLifecycleCommandTests, AccountProvisioningTests, ProvisionAccountCommandTests	53
Protected authorization and tenant suites	StepUpAuthTests, MustChangePasswordGateTests, TwoFarmIsolationMatrixTests	51
#1039	QueryShapeWarningTests	3
Application guards	FlockScopeGuardTests, TenantBypassRealTreeTests, ModuleLedgerRealTreeTests, CouplingMatrixRealTreeTests, AdapterReachRealTreeTests, PeerContractRealTreeTests, CompatibilityExceptionRealTreeTests, SeamSurfaceRealAssemblyTests, ModuleContractRealAssemblyTests	17

I also inspected the author's evidence. All 27 mutant records contain executed assertion failures. All 42 stored SQL/HTTP files match base/head exactly. The purge trace matches exactly; the verb trace matches after normalizing only @t and quoted/numeric elapsed. These are checks of supplied artifacts, not independently rerun mutations or recaptured traces.

E2's contract declaration, seeder conversions and deferred Common identity-port ownership claims remain outside this approval. Source unchanged; no commit or push.

Local report: /home/mforce/.cluckwork-slices/reviews/1055-0a84a2eb-astra.md. Fresh logs/TRX, the test runner, detailed product and quality reports, size measurements and artifact audits: /home/mforce/.cluckwork-slices/reviews/1055-0a84a2eb-astra-evidence/.