refactor(access): put the operator verbs behind IAccessOperations and the purge behind IRefreshTokenPurge

[mforce]

Based on main. D needs no code from B or C. The main it is based on already contains B (#1037). Its overlap with #1041 (C) is in generated data only: both PRs change the Platform → Access count in the coupling matrix. Whichever merges second regenerates it.

Nothing changes for operators. The six account verbs, bootstrap-admin, recover-admin, provision-account, suspend-account, reactivate-account and rename-account, keep every argument, message, exit code, audit row and generated password. The refresh-token purge deletes the same rows in the same batches. The trace below shows both.

For maintainers, the verbs now reach Access through one surface, IAccessOperations, instead of five Identity services. The purge's DELETE moves behind IRefreshTokenPurge, so the Platform job no longer queries an Access table directly.

This is PR D of the Access contract (#857, design v2). It moves the break-glass and provisioning verbs, so it needs a security review.

Refs #857

What changed

1. Commit 1, da5672aa: tests first, green on main. Three things the verbs and the purge do that no suite pinned exactly:
   • provision-account's success line was checked with Contains(email), which a result that swapped the farm code and the owner email still satisfies. OperatorVerbContractTests requires the exact line, with the account id read back from the database.
   • No CLI test passed recover-admin --account. The new test gives one email an Owner in two farms, runs --account <farm B> --reason, and requires that only farm B's user is reset and audited with that reason, and that farm A's password still works.
   • RefreshTokenPurgeSweepTests checks that every aged row goes, which an unbounded DELETE also satisfies. RefreshTokenPurgeBatchTests counts the rows each purge DELETE removes and requires every statement to stay within the batch size. The host's own job worker also sweeps on its poll, so the test seeds and runs again, up to three times, until one of its statements sees a full batch.
2. Commit 2, 810fb67d: the move.
   • IAccessOperations (Application.Features.Users): BootstrapAdminAsync, RecoverAdminAsync, ProvisionAccountAsync, SuspendAccountAsync, ReactivateAccountAsync and RenameAccountAsync. AccessOperations (Infrastructure.Identity) forwards each to the service the verb called before. Every argument goes through unchanged, and CancellationToken.None is still the token.
   • Records. FirstRunAdminOutcome, AdminRecoveryResult, AccountLifecycleOutcome and AccountRenameOutcome move unchanged, comments included, into Application.Features.Users, so the contract can name them. No test names them. AccountProvisionOutcome stays, because existing suites name it. The verb receives ProvisionedAccountDetails, which AccessOperations copies field by field.
   • The six verbs resolve IAccessOperations. Option parsing, slug normalisation, AccountSlugLookup, bootstrap's MigrateAsync, every message and every exit code stay in the verbs, unchanged. AccountSlugLookup keeps its signature, behaviour and place for [C] #514 slice 16: Platform composition, jobs, CLI and seeder conversion #858.
   • IRefreshTokenPurge.DeleteExpiredBatchAsync(cutoff, batchSize, ct) holds the DELETE, moved verbatim with its comment. RefreshTokenPurgeSweep keeps PurgeGrace, BatchSize, MaxBatchesPerRun, the loop, the logging and the error containment.
   • Registrations. Both ports are scoped, beside the services they forward to, and registered for both process roles like those services.
   • Registries. The ledger drops the four records from Access → Farm and adds the sweep's adapter row reaching Access. The tenant-bypass row follows the query to RefreshTokenPurge.DeleteExpiredBatchAsync. Its token hash changes, c09304b6 to 7d79d25a, because Take(BatchSize) became Take(batchSize), and its reason is unchanged after review. The matrix is regenerated.

Parity trace

A scratch test (not committed) runs every verb as a real subprocess, in Production with only a connection string, against a fresh database. It runs 28 steps in one scripted sequence:

• migrate;
• bootstrap-admin without an email, provisioning, and the idempotent re-run;
• provision-account succeeding with locale, currency and zone, plus a taken code, an unknown option, an invalid code and an unknown zone;
• recover-admin succeeding, plus an unknown email, no email and a malformed --account;
• a second farm whose Owner shares an email, recover-admin refusing it as ambiguous, then succeeding with --account;
• suspend-account without a code, with an unknown code, succeeding with an upper-case code, and the no-op re-run;
• reactivate-account succeeding, and its no-op re-run;
• rename-account succeeding, the no-op, a taken code, an invalid code and an unknown farm;
• list-accounts.

After each step it records the exit code, normalised stdout and stderr, and a database snapshot:

• accounts: code, name, active, Version, zone, locale and currency;
• users: MustChangePassword, CredentialEpoch, disabled, lockout, StepUpLogoutEpoch, and whether the password hash or security stamp changed;
• role grants;
• each farm's default grades and conversions;
• every audit row in Sequence order: action, entity, reason, actor, and its details keys;
• the refresh-token count.

A second fact records the purge sweep's DELETE: its SQL text, its parameters and the rows it removed.

Production logs are compact JSON, so two values vary from run to run: each line's @t timestamp and EF's elapsed milliseconds. With those two normalised, two runs on main are identical, and the head is identical to main across all 28 steps and the purge statement. The purge SQL is the same text on both sides, DELETE … WHERE r."Id" IN (SELECT … WHERE r0."ExpiresAt" < @cutoff ORDER BY r0."ExpiresAt" LIMIT @p), with @p = 500.

Mutation table

Each mutant was applied by an exact single-match replacement, built, and run against the classes that own its path, then reverted. Every run set DOTNET_USE_POLLING_FILE_WATCHER=1. My first mutation run did not. Other agents' test runs on this host exhausted the inotify limit (128), and some CLI subprocesses died with IOException or exit 134 instead of reaching their assertions. I discarded that run and reran everything. Every red below is an assertion failure, and the run logged no environment error.

#	Mutant	Red	Failing assertion, quoted
D1	SuspendAccountAsync calls ReactivateAsync	AccountLifecycleCommandTests ×6	SuspendVerb_RunTwice…: Expected: 1 Actual: 0
D2	ReactivateAccountAsync calls SuspendAsync	AccountLifecycleCommandTests ×3	ReactivateVerb_OnAnAlreadyActiveFarm_ChangesNothing…: Expected: 1 Actual: 0
D3	suspend drops the reason	AccountLifecycleCommandTests ×2	Expected: "non-payment drill" Actual: null
D4	reactivate drops the reason	AccountLifecycleCommandTests.ReactivateVerb_BringsTheFarmBack…	Expected: "paid up" Actual: null
D5	rename drops the reason	AccountLifecycleCommandTests.RenameVerb_ChangesTheCode…	Expected: "rebrand drill" Actual: null
D6	rename swaps the current and new codes	AccountLifecycleCommandTests ×5	expected exit 0, got 1. stderr=rename-account failed: Accounts.NotFound
D7	recover drops --account	OperatorVerbContractTests.RecoverAdmin_WithAccount… (new; survived without it)	expected exit 0, got 1 (the ambiguous-email refusal)
D8	recover drops the reason	the same test (new; survived without it)	Expected: "two-farm drill" Actual: null
D9	ProvisionedAccountDetails swaps slug and owner email	OperatorVerbContractTests.ProvisionAccount_PrintsTheCommittedFarmCodeAccountAndOwner (new; survived without it)	Assert.Contains() Failure: Sub-string not found "Farm provisioned: line-… (account …"
D10	ProvisionedAccountDetails drops the account id	the same test (new; survived without it)	Assert.Contains() Failure: Sub-string not found
D11	provision drops the zone	ProvisionAccountCommandTests ×4	Expected: "Asia/Manila" Actual: "UTC"
D12	provision drops the locale	ProvisionAccountCommandTests.ProvisionAccount_UnderDmlOnlyRole…	Expected: "es-MX" Actual: "en-US"
D13	provision drops the currency	the same test	Expected: "MXN" Actual: "USD"
D14	bootstrap drops the email	BootstrapAdminCommandTests ×5	expected exit 0, got 1. stderr=Bootstrap failed: Bootstrap.EmailRequired
D15	the purge loses its Take(batchSize)	RefreshTokenPurgeBatchTests (new; survived without it)	Item: 525 Error: one purge DELETE removed 525 rows; the batch bound is 500
D16	the purge reports 0 rows deleted	RefreshTokenPurgeSweepTests.Sweep_DrainsABacklogLargerThanOneBatch	every aged row should drain, not just the first batch
D17	the purge ignores the cutoff (a year late)	RefreshTokenPurgeSweepTests ×4	Sweep_LeavesActiveTokensUntouched…: Assert.True() Failure
D18	the suspend-account verb calls ReactivateAccountAsync	AccountLifecycleCommandTests ×6	Expected: 1 Actual: 0
—	all reverted	55 of 55 across the seven classes

Not run, with reasons:

• Ordering. Dropping the purge's OrderBy changes which aged rows go first only when a run hits its 20-batch cap. Every candidate is already past its own ExpiresAt plus the grace, so no order is unsafe. The parity trace pins the ORDER BY text instead.
• Lifetime. A Singleton IAccessOperations behaves the same in a one-shot process, which resolves it once. A Singleton IRefreshTokenPurge would hold one AppDbContext across the leader's sequential sweeps, and its untracked ExecuteDelete makes that unobservable. Both stay Scoped, beside the services they forward to.

Test counts

Measured on this machine. Every integration run has #1039's query-shape check on.

Suite	main 75e2b1be	head 810fb67d
Cluckwork.Domain.Tests	—	495
Cluckwork.Application.Tests (full, with every ledger, adapter-reach, tenant-bypass, seam and coupling-matrix guard)	—	702
Commit 1's new tests	3 of 3	3 of 3
Integration: every class declared in a test file that names a verb, a service, the sweep, the dispatcher, ProcessRoles, AccountSlugLookup or the new ports (58 classes, found by walking the test files)	—	320 of 321 in one run, no query-shape warning; the one failure is below

The one failure in that run was commit 1's batch test, run from a build made before its table-name fix. With the fix, commit 1's tests pass 3 of 3 on main and on the head, in a separate run.

The full integration suite was not run locally; CI runs it.

Test edits

None. No existing test file changed. git diff --numstat origin/main...HEAD -- tests/ lists the two new test files and the three registry data files: the ledger, the coupling matrix and the tenant-bypass sites.

Deslop record

/deslop and a by-hand /no-comments pass (subagents are off for this slice):

• The src diff moves and forwards code only. It adds no logic, no caching and no try/catch. The one non-forwarding line is the ProvisionedAccountDetails copy, which the new exact-line test pins.
• The moved comments (the four records, the purge's strictly-older note) move verbatim.
• The new comments give reasons the code cannot show: the operator surface's authority on IAccessOperations, the sweep/port split on IRefreshTokenPurge, and why AccountProvisionOutcome stays.
• One fix before the first push: commit 1's batch test first matched "RefreshTokens" in the command text. The table is refresh_tokens, so the test failed on main with an empty capture. It now matches the real table name and passes on main.

Change map

BEFORE
 bootstrap-admin    --> FirstRunAdminService.ProvisionAsync
 recover-admin      --> AdminRecoveryService.RecoverAsync
 provision-account  --> AccountProvisioner.ProvisionAsync            (AccountProvisionOutcome)
 suspend-account    --> AccountSuspensionService.SuspendAsync
 reactivate-account --> AccountSuspensionService.ReactivateAsync
 rename-account     --> AccountRenameService.RenameAsync
 RefreshTokenPurgeSweep [Platform] --> db.RefreshTokens ... ExecuteDeleteAsync   (Access table, read and deleted directly)
 FirstRunAdminOutcome, AdminRecoveryResult, AccountLifecycleOutcome, AccountRenameOutcome  [Infrastructure.Identity]

AFTER
 the six verbs --> IAccessOperations [Application.Features.Users] --> AccessOperations [Infrastructure.Identity]
                                                                       --> the same five services, unchanged
 provision-account receives ProvisionedAccountDetails, copied field for field from AccountProvisionOutcome
 RefreshTokenPurgeSweep --> IRefreshTokenPurge.DeleteExpiredBatchAsync(cutoff, batchSize) --> RefreshTokenPurge (same query)
   the sweep keeps PurgeGrace, BatchSize, MaxBatchesPerRun, the loop and its error containment
 the four outcome records --> Application.Features.Users, unchanged
 ledger: Access -> Farm W 22 -> 18 (the four records leave the edge); Platform/adapters -> Access A 45 -> 46 (+RefreshTokenPurgeSweep.RunAsync)
 tenant-bypass: the purge's filter-free row follows the query to RefreshTokenPurge.DeleteExpiredBatchAsync
 unchanged: AccountSlugLookup (signature, behaviour and place; #858 P3 moves it), bootstrap's MigrateAsync,
            list-accounts, every service, AccountProvisionOutcome (named by existing suites)
 not here: the seeders and the contract declaration (PR E)

Review notes: properties to try to refute

• Forwarding. Each IAccessOperations member calls the same service with the same arguments, in the same order, as the verb did. Account, reason, slug, locale, currency and zone are never dropped or swapped.
• Provisioning output. ProvisionedAccountDetails copies AccountId, Slug, OwnerEmail and TemporaryPassword exactly. The printed password is still the one that logs in, printed once, never through the logger.
• Break-glass. recover-admin resolves the same target, refuses the same ambiguous and disabled cases, and writes the same User.BreakGlassReset row with the operator's reason. It still never migrates, so it still runs under the DML-only role.
• Process role (refactor(api): make process role explicit for boot guards #347). Resolving IAccessOperations builds all five services, so suspend-account now also constructs IdentityProvider's dependency closure. OneShotVerbMinimalConfigTests runs bootstrap-admin, provision-account and recover-admin with only a connection string, and the broken-section cases run recover-admin. Each of those reaches the service call, so the whole closure is constructed under minimal and broken configuration. The suspend, reactivate and rename cases still exit at the slug lookup before construction. Try to find a configuration under which a lifecycle verb now fails where it did not.
• Purge. The cutoff, PurgeGrace, BatchSize, MaxBatchesPerRun, the ordering and the strictly-older predicate are unchanged. Every statement removes at most one batch, and the sweep still contains its own failures. The port has no tenant scope, as before; refresh tokens carry no query filter, and the reason is on the tenant-bypass row.
• Single-leader (Background worker has no single-runner guarantee — double-runs if scaled >1 instance #271). The sweep still runs only under the worker's leader gate. Nothing else calls the port.

[mforce]

Independent security + code-quality review (Claude Opus 5.5, high: security-review, thermo-nuclear, ponytail) at head 810fb67d. Posted by the coordinator.

PR #1047 (#857 PR D): independent security review, Opus

Head 810fb67d61c2de3e96fddbc9aeac6a2c522f2c87, base 75e2b1be. Detached checkout /home/mforce/.cluckwork-slices/review-1047-opus; the diff was confirmed non-empty (22 files, +369/-76).
Scratch logs and probes: /home/mforce/.cluckwork-slices/reviews/1047-opus-work/.

Bottom line

No product defect and no security defect. Every verb forwards to the same service with the same arguments, the same token and the same output. The purge runs the same SQL under the same bound. One operational change is not stated in the PR's "Nothing changes for operators": suspend-account, reactivate-account and rename-account now construct the Identity/Redis dependency closure (F1). The timing confirms it. It changes latency, not correctness or security.

Findings

F1 — P3 — src/Cluckwork.Infrastructure/Identity/AccessOperations.cs:5-11 — lifecycle verbs now open a Redis connection they never use — CONFIRMED

• Defect. AccessOperations takes all five services through its constructor. Resolving IAccessOperations for suspend-account, reactivate-account or rename-account therefore constructs FirstRunAdminService and AdminRecoveryService. That builds the chain IIdentityProvider → IStepUpGrantRegistry (PersistentStepUpGrantRegistry) → IClaimOnceStore → singleton IConnectionMultiplexer, and the multiplexer calls ConnectionMultiplexer.Connect(parsed) (SharedStateRegistration.cs:50) synchronously. With AbortOnConnectFail=false the connect does not throw; it blocks for the connect timeout. Before this PR these three verbs resolved only AccountSuspensionService or AccountRenameService, whose closures contain no Redis.

• Scenario. An operator suspends a farm during an incident while Redis is unreachable or slow; the reference deploy/docker-compose.yml:60 sets SharedState__Redis__ConnectionString. The verb still succeeds with the same exit code and output, but it waits on Redis first.

• Evidence. A scratch probe ran suspend-account --slug <farm> --reason probe as a Production subprocess against the Testcontainers database, with SharedState__Redis__ConnectionString=10.255.255.1:6379,connectTimeout=5000. The probe file was not committed and was deleted afterwards; a copy is at 1047-opus-work/ZzScratchRedisProbeTests.cs.

  tree	Redis blank	Redis black-holed
  base 75e2b1be src	2,530 ms, exit 0	2,491 ms, exit 0
  head 810fb67d src	2,532 ms, exit 0	13,082 ms, exit 0

  Output is in redis-probe-base.txt and redis-probe-head.txt. Stdout and stderr match in kind, and stderr is empty in all four runs.

• Not a security defect. Nothing fails open. The suspension, revocation and audit run unchanged, and the extra closure is only constructed, never invoked. bootstrap-admin and recover-admin already carried this closure before the PR, so refactor(api): make process role explicit for boot guards #347's minimal-config posture holds; OneShotVerbMinimalConfigTests constructs it under minimal configuration.

• Fix, optional. Split the port by dependency closure: one port for suspend, reactivate and rename, one for bootstrap, recover and provision (thermo T1). Alternatively, record the cost in the PR body and stop claiming "nothing changes for operators".

F2 — P3 — src/Cluckwork.Application/Features/Users/AccessOperationOutcomes.cs:192-196 — ProvisionedAccountDetails is a duplicate, exact today — CONFIRMED (exact copy)

• Defect. None today. The record matches AccountProvisionOutcome (AccountProvisioner.cs:204-208) field for field: Guid AccountId, string Slug, string OwnerEmail, string TemporaryPassword, with the same order, names and nullability. The copy at AccessOperations.cs:277 maps by name.
• Risk. It is the only hand-written mapping in the slice, and it carries the generated password. A future field added to one record and not the other drifts silently.
• Evidence. Mutant D9 (swap slug and email) and my O1 (TemporaryPassword := OwnerEmail) both go red; see below.
• Fix, optional. Move AccountProvisionOutcome to Application like the other four records. That needs one using line each in AccountProvisioningTests.cs and TwoFarmIsolationMatrixTests.cs, and lets the method forward in one line.

No other finding. Every question in the brief is answered below.

Adversarial checklist

Question	Answer	Basis
A generated password leaks beyond stdout?	No. The password still reaches only Console.Out in the three verbs. AccessOperations does not log, and the result is never passed to a logger. The record's synthesized ToString includes the password, but so did the original's, and nothing formats either one.	Read all six verbs and AccessOperations
Recover, provision or rename in the wrong farm?	No. accountId, slug, currentSlug and newSlug are forwarded positionally, with the same parameter order as the service signatures. Recovery with --account resets only farm B; mutant D7 is red.	Code, D7, the new two-farm test
Suspend without same-transaction revocation and epoch bump?	No. SuspendAccountAsync → suspension.SuspendAsync, and the service is untouched apart from deleting the record. The verb passes the same accountId and reason.	Diff of AccountSuspensionService.cs is record deletion and one using
Audit row with the wrong actor or reason?	No. Actor resolution lives in the services, which are unchanged. The reason is forwarded for all four verbs that take one. The new test pins two-farm drill on farm B only.	Code; PR mutants D3–D5 and D8
Exit codes or output changed?	No. Every message, branch and return in the verbs is untouched. The provision-account exact-line test passes at the head.	Diff and baseline run
Rename's slug + Version comparison broken?	No. AccountRenameService keeps its body; only the record moved.	Diff
Process-role scoping escaped?	No. Both ports register in AddCluckworkIdentity beside the five services, which run for both roles, and no new guard or config read was added. F1 is the only closure change, and that closure was already proven buildable under minimal configuration.	CluckworkIdentityServiceCollectionExtensions.cs:131-143
Purge SQL, bound, grace, errors, single leader unchanged?	Yes, unchanged. It is the same LINQ chain with Take(batchSize) taking the sweep's constant 500. PurgeGrace, MaxBatchesPerRun, the loop and the try/catch stay in the sweep. The scope and port are resolved where AppDbContext was resolved before, also outside the try, so failure behaviour is unchanged. The only caller is still DurableJobWorker under the leader gate.	Code; D15 and O2
Captive dependency?	No. AccessOperations and RefreshTokenPurge are scoped over scoped dependencies. The singleton sweep resolves the port from its own CreateScope(). The verbs resolve from a created scope; bootstrap-admin and recover-admin use scope.ServiceProvider. The only singleton in the new closure is IConnectionMultiplexer/IClaimOnceStore, consumed by scoped services, which is legal.	Code
ProvisionedAccountDetails an exact copy?	Yes. See F2.	Code
Tenant-bypass row	The hash change c09304b6→7d79d25a is explained by BatchSize→batchSize; the reason is unchanged and accurate. Application tests are green in CI.	CI Tests (application) pass

Mutation reproduction

Each mutant was a single-match replacement in a script (1047-opus-work/run.sh), followed by a test run of the owning classes, then git checkout to revert. Every run used DOTNET_USE_POLLING_FILE_WATCHER=1. Baseline at the head: OperatorVerbContractTests, RefreshTokenPurgeBatchTests and ProvisionAccountCommandTests, 14/14 pass.

#	Mutant	Result	Failing assertion
D9 (PR)	ProvisionedAccountDetails swaps slug and owner email	red	ProvisionAccount_PrintsTheCommittedFarmCodeAccountAndOwner: Assert.Contains() Failure: Sub-string not found
D7 (PR)	recover forwards null for accountId	red	RecoverAdmin_WithAccount_…: expected exit 0, got 1
D15 (PR)	purge loses .Take(batchSize)	red	one purge DELETE removed 525 rows; the batch bound is 500
O1 (mine)	the details copy puts OwnerEmail into TemporaryPassword	red	ProvisionAccountCommandTests.ProvisionAccount_UnderDmlOnlyRole_PrintsOneWorkingPasswordAfterCommit: Expected: 1 Actual: 2
O2 (mine)	off-by-one bound, .Take(batchSize + 1)	red	one purge DELETE removed 501 rows; the batch bound is 500

The PR's table matches what I reproduced. The batch test fails closed if EF's SQL text ever stops matching DELETE FROM refresh_tokens: the capture is empty, and attempt < 3 goes red on the third attempt.

security-review skill output

I invoked the skill. Its git preamble ran in the coordinator's worktree, not the review checkout, so its embedded diff was empty. I ran its methodology myself, without subagents, as the brief requires, over /home/mforce/.cluckwork-slices/reviews/1047-810fb67d.diff. It checks input validation, authN/authZ, crypto and secrets, injection, and data exposure, and applies its false-positive filter.

• No vulnerability reached confidence ≥ 8. The diff adds no HTTP surface. CLI arguments are trusted per the skill's precedent 3 and pass through unchanged to the services that validate them. The new SQL is the same parameterized EF ExecuteDelete. No secret reaches a logger. No authorization path changes.
• Candidates considered and dropped:
  • The password in a synthesized record ToString: pre-existing and never formatted.
  • The tenant-free purge: pre-existing, with a documented bypass row.
  • The Redis closure (F1): no security impact, and an availability concern sits under the skill's DoS exclusion.

thermo-nuclear-code-quality-review (P3, slice-scoped)

Full text: 1047-opus-work/thermo/00_summary.md. No structural regression. No file crosses 1k lines, and no new branch enters an existing flow.

• T1 (P3). IAccessOperations groups operations by audience, not by dependency closure. One five-constructor facade makes every verb build the union of five closures, which is the root of F1. The fix is to split it into a lifecycle port (suspend, reactivate, rename) and a credential port (bootstrap, recover, provision). Both stay thin forwarders.
• T2 (P3). ProvisionedAccountDetails plus its mapping exists only to avoid two test using lines. Moving AccountProvisionOutcome like the other four records deletes a type, a mapping and a class of swap bug.
• T3 (P3). The moved record comments now sit in the Application contract but describe Infrastructure internals: lockedAccount, and "not a mirror of AccountProvisionOutcome". Trim them to the contract-level fact on the next touch.
• Nit. using …Features.Users; sorts before …Features.Accounts in four services. Nothing enforces the order.

ponytail-review (P3, slice-scoped)

• AccessOperationOutcomes.cs:L190-196 and AccessOperations.cs:L269-278: reuse: ProvisionedAccountDetails duplicates AccountProvisioner.cs:204 AccountProvisionOutcome. Move the original to Application.Features.Users and forward in one line, as the other five members do.
• IAccessOperations and IRefreshTokenPurge each have one implementation but are not flagged as yagni. They are the module-boundary ports the Modular-monolith architecture design + implementation plan (from PR #423) #514 ledger and adapter-reach guard exist for, and the abstraction is the deliverable.

net: -12 lines possible.

Verdict

Merge. There is no product or security defect. F1 and F2 are optional P3 follow-ups; at minimum, the PR body should drop "nothing changes for operators" or note F1.

[mforce]

Review of record (Codex Astra, xhigh) at head 810fb67d. Posted by the coordinator. The later head 9296283b only merges main.

PR #1047 security review, #857 PR D. Reviewed 810fb67d61c2de3e96fddbc9aeac6a2c522f2c87 against 75e2b1be on 2026-10-03. The supplied 22-file diff matches those revisions byte-for-byte. I performed the review myself, without delegation.

No CONFIRMED or PLAUSIBLE P0–P3 findings. I found no product or security defect in this change.

Operator parity

Each operation reaches its original service with the original arguments. The CLI retains parsing, normalization, output and exit-code handling. Every cancellation token remains CancellationToken.None; provision's previous omitted default was equivalent.

Verb and head location	Preserved behavior
bootstrap-admin, src/Cluckwork.Api/Cli/BootstrapAdminCliCommand.cs:42	Calls FirstRunAdminService.ProvisionAsync(email, ct). Migration still precedes provisioning. The whole advisory-lock/check/create region remains single-attempt, with the second lock-ownership proof inside the creation transaction. First-run creation sets MustChangePassword, audits User.Create as (bootstrap-admin), and returns the generated password to the unchanged stdout sink. An existing Owner retains the same no-op message, no new credential and exit 0.
recover-admin, src/Cluckwork.Api/Cli/RecoverAdminCliCommand.cs:50	Calls AdminRecoveryService.RecoverAsync(email, accountId, reason, ct). The optional account selector, ambiguous-email refusal, disabled-user checks and locked recheck remain. Reset, stamp rotation, epoch increment, refresh revocation and User.BreakGlassReset share the original transaction, attributed to (break-glass) with the supplied reason and host/OS-user details. It still does not migrate.
provision-account, src/Cluckwork.Api/Cli/ProvisionAccountCliCommand.cs:51	Calls AccountProvisioner.ProvisionAsync(name, slug, ownerEmail, locale, currencyCode, timeZoneId, ct). Account, defaults, Owner and audit rows retain their single transaction and (provision-account) actor. The DML-only path and validation are unchanged.
suspend-account, src/Cluckwork.Api/Cli/SuspendAccountCliCommand.cs:48	Calls AccountSuspensionService.SuspendAsync(accountId, reason, ct). Account suspension, every user's epoch/stamp changes, refresh revocation and the transition audit remain in one transaction. A repeated suspension still revokes again without another transition audit or account-Version bump. Actor remains (suspend-account).
reactivate-account, src/Cluckwork.Api/Cli/ReactivateAccountCliCommand.cs:38	Calls the same service's ReactivateAsync(accountId, reason, ct). A real transition revokes the credentials minted during the accepted #579 window. An already-active farm retains its sessions and Version. Actor remains (reactivate-account).
rename-account, src/Cluckwork.Api/Cli/RenameAccountCliCommand.cs:102	Calls AccountRenameService.RenameAsync(currentSlug, newSlug, reason, ct). The lookup snapshot, tenant-keyed lock, both slug and Version comparisons, domain mutation, unique-index exception mapping and same-transaction audit remain. Actor remains (rename-account).

src/Cluckwork.Infrastructure/Identity/AccessOperations.cs:28 copies all four fields exactly: AccountId, Slug, OwnerEmail, TemporaryPassword. Its failure path preserves the same Error. No password is routed through a logger. The three original password-producing CLI sinks still use Console.Out; stdout collectors remain the existing documented operational caveat.

The four moved result declarations and their comments are identical. The four affected service bodies are identical after removing their new import and relocated records. AccountProvisioner, IdentityProvider, AmbientTransaction, SingleAttemptExecution, Program, process-role classification and the job worker are byte-identical. Static comparison.

I independently ran the supplied 28-step process recorder on the head and tests-first commit da5672aa, whose production source is identical to the supplied base. Children ran in Production with an explicitly cleared environment, database settings and polling enabled. Every normalized stdout/stderr line, exit code and recorded database snapshot matches. The recorder compares account/user state, credential changes, role grants, reference data and audit rows. Comparison, base trace, head trace.

Purge and lifetimes

src/Cluckwork.Infrastructure/Identity/RefreshTokenPurge.cs:13 preserves the strictly-older expiry predicate, expiry ordering and Take(batchSize). The independently recorded SQL is byte-identical, including ORDER BY, LIMIT @p and @p = 500; both statements removed two eligible rows. Base SQL, head SQL.

RefreshTokenPurgeSweep.cs:47 retains the one-day grace, batch size 500, cap of 20 batches, termination condition, logging, cancellation propagation and error containment. At line 64 it creates the scope inside each invocation and resolves IRefreshTokenPurge there. The singleton sweep captures neither that port nor its context. The worker's unchanged leader gate still controls scheduled execution; the sweep is the port's only production caller. The filter-free query remains deliberately global, with its classification moved to the new member.

CluckworkIdentityServiceCollectionExtensions.cs:142 registers IAccessOperations Scoped, and line 143 registers the purge port Scoped. Their dependencies retain the same scoped lifetimes. No captive dependency, new outer transaction or retry was introduced.

The operations constructor does construct all five services, broadening the lifecycle verbs' dependency graph. I traced its configured Redis connection path rather than assuming construction performs no I/O. A scratch subprocess probe supplied an unavailable Sentinel endpoint, 127.0.0.1:1,serviceName=review-primary,connectTimeout=100,connectRetry=0, while targeting existing farms. Suspend, reactivate and rename all exited 0 and made the expected changes on both revisions. The suspected availability failure did not reproduce. The ordinary trace separately verifies missing serving configuration. Probe source, head results, base results. This is not an exhaustive configuration or startup-latency guarantee.

Mutation evidence

Each mutant was applied alone, rebuilt, run against one targeted class and restored. These are deliberately broken review copies, not findings against the PR.

Mutant	Independent result
D1: suspend forwards to reactivate	Six of 24 AccountLifecycleCommandTests fail. The farm remains active, and the expected suspension audit is absent. Log, patch.
D7: recover drops the account selector	The new two-farm recovery test fails with exit 1 and Recovery.Ambiguous. This exercises one of the four requested commit-1-only forwarding guards. Log, patch.
D15: purge loses Take(batchSize)	The new batch test fails because one DELETE removes 525 rows against its 500-row bound. Log, patch.
Additional: suspend skips its revocation block	Two of 16 AccountSuspensionTests fail: the epoch remains 1 instead of 2, and the expected refresh revocation timestamp is absent. Log, patch.

The restored head passed 90/90 targeted integration tests across 12 classes and 12/12 Application architecture checks across five classes. The new committed tests also pass 3/3 before the move. Counts. Initial Application invocations returned 0 with empty logs before that project's restore; I discarded those results and reran with restore and explicit discovered-test counts. Verification notes.

I did not reproduce the ordering or lifetime mutants. Ordering is independently checked by SQL comparison; scoped registrations and resolution were inspected directly. All Docker consumers used sg docker, all test processes used polling, and classes ran sequentially. I did not run the full integration suite or repeat the worker's trial merge with newer main.

Ledger and code quality

The ledger counts are exact: Access-to-Farm falls from 22 to 18 symbols when the four result records move, and Platform-to-Access grows from 45 to 46 adapters through the purge sweep. The remaining Access-to-Farm writes are unchanged. The regenerated matrix and tenant-bypass classification pass their real-tree checks.

The thermo-nuclear review found no structural regression, added policy branch or unjustified file growth. The requested module boundary explains the direct forwarding layer; the small provision DTO copy preserves the agreed compatibility scope. Quality summary, detail.

Ponytail-review: Lean already. Ship.

Nits

None.

All reviewer mutations and scratch tests were removed. The checkout was clean at the reviewed head before removal. No commit, push or GitHub comment was made, and no other checkout was edited. Cleanup record.

Verdict: APPROVE PR #1047 at 810fb67d; no product or security defect found.