fix(housekeeping): harden system workflow boundaries

2026-08-29 00:25:47 +02:00
1 parent 3788ecd9f1
commit c325c53774
@@ -50,11 +50,11 @@ Node/pnpm emitted this non-blocking warning during pnpm gates:
 ## Architectural decisions

 - The migration matrix remains the single source of route truth. The System route array is materialized from its exact identifiers and values, and tests assert ordered route/handler equality rather than set-only coverage.
- Query factories accept narrow adapters; production adapters reuse existing ACL, settings, emulator, health, online-user, analytics, log, alert, and maintenance services. This avoided unnecessary edits to `ops-health.ts` and `ops-online-users.ts`.
+- Query factories accept narrow adapters; production adapters reuse existing ACL, settings, emulator, health, online-user, analytics, log, alert, and maintenance services. The fix round added only a strict online-roster helper beside the unchanged tolerant legacy API in `ops-online-users.ts`, so System can report a database outage truthfully.
 - `systemMutationService` is the only public production mutation boundary. It is server-only and repeats capability enforcement even when called by an already-guarded legacy action or an authorized dispatcher. The unguarded production adapter is module-private.
 - Adapter exceptions and unsuccessful RCON sends become typed `DEPENDENCY_UNAVAILABLE` failures. Rank-delete conflict metadata travels in the standard `fieldErrors` shape; the legacy wrapper reconstructs the prior human-readable `ActionError`, keeping the dispatcher result schema strict.
 - Command string schemas use a non-transforming `\S` check to reject whitespace-only values; normalization and trimming remain at the guarded service boundary. This preserves the foundation registry rule that command schemas contain no executable transforms.
- Existing semantic label keys were reused where they matched; missing System labels use stable functional keys without changing i18n catalogs outside the tested scope.
+- System navigation labels use stable `pages.housekeeping.routes.system.*` keys. Complete source strings are present in the tested English and Italian catalogs; repository fallback remains responsible for other locales.
 - Foundation source-boundary changes are a narrow source-to-import allowlist for the new System integration edges. Existing forbidden directions for every other domain remain asserted.

 ## Changed files
@@ -92,3 +92,57 @@ Node/pnpm emitted this non-blocking warning during pnpm gates:
 - `src/features/housekeeping/route-handlers.test.ts`
 - `src/features/housekeeping/route-handlers.ts`
 - `src/lib/admin/acl-management-contract.test.ts`
+
+## Official review fix round 1
+
+The official review was addressed on exact base `3788ecd9f1a4e32e68abac5ee2dae3418cdebfb2`. The three parked Minor findings were deliberately left unchanged.
+
+### Findings resolved
+
+1. **Housekeeping label namespace:** all 17 System routes used legacy `pages.admin.*` keys, which the Task 9 preview layout correctly rejected. Routes now use 17 stable `pages.housekeeping.routes.system.*` keys, EN/IT provide non-empty source strings, and a preview-contract test builds and translates the real System navigation without broadening layout validation.
+2. **Truthful partial/outage states:** access, configuration, and observability previously rendered empty before considering failed dependencies. Partial now takes precedence whenever any dependency failed. System online-user queries use a strict helper that exposes database failure; the existing tolerant `fetchOpsOnlineUsers` API and legacy behavior remain intact.
+3. **Rank synchronization failures:** create, delete, and update no longer report success when `updatepermissions` returns false, and set-rank no longer reports success when RCON committed but database persistence failed. Both paths return the existing strict `DEPENDENCY_UNAVAILABLE` envelope with stable message keys and explicit `fieldErrors` describing `operation`, `completed`, and `pending` effects. Dispatcher audit records `intent` then `failure`, never `success`.
+4. **Legacy permission error parity:** known rank-in-use and role-not-found conflicts retain their established `ActionError` text. Unknown infrastructure failures now cross the real `adminAction` boundary as ordinary errors and are sanitized to `Internal server error`; internal Housekeeping message keys are not exposed to the legacy UI.
+
+### Fix-round TDD evidence
+
+| Cycle | Exact command | RED | GREEN |
+| --- | --- | --- | --- |
+| Labels and runtime navigation | `pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/system/routes.test.ts src/features/housekeeping/foundation/localization-contract.test.ts src/features/housekeeping/foundation/preview-route-contract.test.ts` | 3 files failed; 4 tests failed and 66 passed. The route labels mismatched, EN/IT lacked the routes subtree, and preview layout rejected `pages.admin.hubs.tabs.permissions`. | 3 files, 70 tests passed. |
+| Partial precedence and online-user outage | `pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/system/pages/system-pages.test.tsx src/features/housekeeping/domains/system/queries/operations-production.test.ts` | 2 files failed; 4 tests failed and 9 passed. Three pages rendered empty, and the production adapter resolved a false ready zero-user state on database failure. | 2 files, 13 tests passed. |
+| Rank synchronization | `pnpm exec vitest run --coverage.enabled=false src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts` | 1 file failed; 4 tests failed. Create/delete/update returned success after failed permission synchronization, and set-rank lacked explicit partial-completion metadata. | 1 file, 4 tests passed. The first post-production run had 3 passed and 1 test-only audit expectation failure; aligning it with the established `intent` then `failure` envelope produced the final GREEN without a further production change. |
+| Legacy permission parity | `pnpm exec vitest run --coverage.enabled=false src/actions/permissions.test.ts` | 1 file failed; 1 test failed and 2 passed. The generic infrastructure case leaked `errors.housekeeping.dependencyUnavailable`; both known business conflicts already retained their prior text. | 1 file, 3 tests passed. |
+
+The combined focused rerun passed 7 files and 90 tests. The first fix-round `pnpm typecheck` found two test-only narrowing errors in the new rank test; after the minimal annotations, its focused test remained green and `tsc --noEmit` passed.
+
+### Fix-round verification
+
+- Full affected Task 10 System, action, audit, authorization, bootstrap, localization, and preview suite: 22 files, 264 tests passed.
+- Legacy operations, staff smoke, and authorization suite: 3 files, 97 tests passed.
+- `pnpm test:housekeeping`: 45 files, 395 tests passed.
+- `pnpm typecheck`: passed (`tsc --noEmit`).
+- Exact changed-file `pnpm exec biome check --formatter-enabled=false ...`: checked 17 code, test, and locale files; no fixes applied after the one mechanical import-order correction.
+- UTF-8 source verification confirmed the Italian `Analisi attività` label contains U+00E0, followed by a 3-file/70-test route-localization-preview GREEN rerun.
+- `git diff --check` and the pre-stage `git diff --cached --check`: exit 0; only expected Git autocrlf warnings.
+- Independent read-only re-review: 0 Critical, 0 Important, 0 new Minor; all four official Important findings resolved, all three parked Minors unchanged, ready-to-merge verdict.
+
+### Fix-round changed files
+
+- `.superpowers/sdd/2026-08-26-housekeeping-completion/task-10-report.md`
+- `src/actions/permissions.test.ts`
+- `src/actions/permissions.ts`
+- `src/features/housekeeping/domains/system/pages/access.tsx`
+- `src/features/housekeeping/domains/system/pages/configuration.tsx`
+- `src/features/housekeeping/domains/system/pages/observability.tsx`
+- `src/features/housekeeping/domains/system/pages/system-pages.test.tsx`
+- `src/features/housekeeping/domains/system/queries/operations-production.test.ts`
+- `src/features/housekeeping/domains/system/queries/operations.ts`
+- `src/features/housekeeping/domains/system/routes.test.ts`
+- `src/features/housekeeping/domains/system/routes.ts`
+- `src/features/housekeeping/domains/system/services/mutations.ts`
+- `src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts`
+- `src/features/housekeeping/foundation/localization-contract.test.ts`
+- `src/features/housekeeping/foundation/preview-route-contract.test.ts`
+- `src/lib/admin/ops-online-users.ts`
+- `src/messages/en.json`
+- `src/messages/it.json`