From c325c53774470ac988a471f3aef61f081c5cf392 Mon Sep 17 00:00:00 2001 From: simoleo89 Date: Sat, 29 Aug 2026 00:25:47 +0200 Subject: [PATCH] fix(housekeeping): harden system workflow boundaries --- .../task-10-report.md | 58 ++++- src/actions/permissions.test.ts | 122 ++++++++++ src/actions/permissions.ts | 2 +- .../domains/system/pages/access.tsx | 2 +- .../domains/system/pages/configuration.tsx | 2 +- .../domains/system/pages/observability.tsx | 5 +- .../system/pages/system-pages.test.tsx | 49 ++++ .../queries/operations-production.test.ts | 64 +++++ .../domains/system/queries/operations.ts | 4 +- .../domains/system/routes.test.ts | 34 +-- .../housekeeping/domains/system/routes.ts | 36 +-- .../domains/system/services/mutations.ts | 39 ++- .../rank-mutations-production.test.ts | 225 ++++++++++++++++++ .../foundation/localization-contract.test.ts | 27 +++ .../foundation/preview-route-contract.test.ts | 74 ++++++ src/lib/admin/ops-online-users.ts | 87 ++++--- src/messages/en.json | 29 +++ src/messages/it.json | 29 +++ 18 files changed, 809 insertions(+), 79 deletions(-) create mode 100644 src/actions/permissions.test.ts create mode 100644 src/features/housekeeping/domains/system/queries/operations-production.test.ts create mode 100644 src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts diff --git a/.superpowers/sdd/2026-08-26-housekeeping-completion/task-10-report.md b/.superpowers/sdd/2026-08-26-housekeeping-completion/task-10-report.md index d92f7d52..a0105134 100644 --- a/.superpowers/sdd/2026-08-26-housekeeping-completion/task-10-report.md +++ b/.superpowers/sdd/2026-08-26-housekeeping-completion/task-10-report.md @@ -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` diff --git a/src/actions/permissions.test.ts b/src/actions/permissions.test.ts new file mode 100644 index 00000000..e29976de --- /dev/null +++ b/src/actions/permissions.test.ts @@ -0,0 +1,122 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const doubles = vi.hoisted(() => ({ + canAccess: vi.fn(), + getApiAdminContext: vi.fn(), + mutationExecute: vi.fn(), + reportError: vi.fn(), + revalidateTag: vi.fn(), +})); + +vi.mock("@/features/housekeeping/domains/system/services/mutations", () => ({ + systemMutationService: { execute: doubles.mutationExecute }, +})); + +vi.mock("@/lib/permissions", () => ({ + canAccess: doubles.canAccess, + getApiAdminContext: doubles.getApiAdminContext, +})); + +vi.mock("@/lib/admin/authorization-events", () => ({ + logAuthorizationEvent: vi.fn(), +})); + +vi.mock("@/lib/auth", () => ({ auth: vi.fn() })); +vi.mock("@/lib/rate-limit", () => ({ rateLimit: vi.fn() })); +vi.mock("@/lib/report-error", () => ({ reportError: doubles.reportError })); +vi.mock("@/lib/foundation/security", () => ({ + extractClientIpAsync: vi.fn(async () => "198.51.100.8"), +})); +vi.mock("@/lib/foundation/request-context", () => ({ + createStore: vi.fn(() => ({})), + getRequestId: vi.fn(() => "legacy-permissions-request"), + runWithStore: vi.fn((_store: unknown, callback: () => Promise) => + callback(), + ), + setContextUserId: vi.fn(), +})); +vi.mock("next/cache", () => ({ revalidateTag: doubles.revalidateTag })); + +import { deleteRank, setCmsPermissions } from "./permissions"; + +const permissions = { + has: () => true, + hasAny: () => true, + hasAll: () => true, + isSuperAdmin: false, +}; + +beforeEach(() => { + vi.clearAllMocks(); + doubles.canAccess.mockReturnValue(true); + doubles.getApiAdminContext.mockResolvedValue({ + session: { + expires: "2099-01-01T00:00:00.000Z", + user: { + id: 42, + name: "operator", + username: "operator", + rank: 7, + look: "hd-180-1", + mail: "operator@example.test", + }, + }, + permissions, + }); +}); + +describe("legacy permission action error parity", () => { + it("sanitizes typed infrastructure failure through the real adminAction boundary", async () => { + doubles.mutationExecute.mockResolvedValue({ + ok: false, + error: { + code: "DEPENDENCY_UNAVAILABLE", + messageKey: "errors.housekeeping.dependencyUnavailable", + }, + correlationId: "dependency-correlation", + }); + + await expect(deleteRank({ id: 7 })).resolves.toEqual({ + ok: false, + error: "Internal server error", + fieldErrors: undefined, + }); + }); + + it("preserves the established rank-in-use ActionError text", async () => { + doubles.mutationExecute.mockResolvedValue({ + ok: false, + error: { + code: "CONFLICT", + messageKey: "errors.housekeeping.system.rankInUse", + fieldErrors: { rank: ["3"] }, + }, + correlationId: "rank-in-use-correlation", + }); + + await expect(deleteRank({ id: 7 })).resolves.toEqual({ + ok: false, + error: "Cannot delete: 3 users have this rank", + fieldErrors: undefined, + }); + }); + + it("preserves the established role-not-found ActionError text", async () => { + doubles.mutationExecute.mockResolvedValue({ + ok: false, + error: { + code: "NOT_FOUND", + messageKey: "errors.housekeeping.system.roleNotFound", + }, + correlationId: "role-not-found-correlation", + }); + + await expect( + setCmsPermissions({ roleId: 7, permissionSlugs: [] }), + ).resolves.toEqual({ + ok: false, + error: "Role not found", + fieldErrors: undefined, + }); + }); +}); diff --git a/src/actions/permissions.ts b/src/actions/permissions.ts index f1ad61df..4768f5b5 100644 --- a/src/actions/permissions.ts +++ b/src/actions/permissions.ts @@ -49,7 +49,7 @@ async function runAccessMutation( if (result.error.messageKey === "errors.housekeeping.system.roleNotFound") { throw new ActionError("Role not found"); } - throw new ActionError(result.error.messageKey); + throw new Error(result.error.messageKey); } const createRankSchema = z.object({ diff --git a/src/features/housekeeping/domains/system/pages/access.tsx b/src/features/housekeeping/domains/system/pages/access.tsx index 627e93c4..1f8d4d8a 100644 --- a/src/features/housekeeping/domains/system/pages/access.tsx +++ b/src/features/housekeeping/domains/system/pages/access.tsx @@ -138,7 +138,7 @@ export function SystemAccessPage({ result }: SystemAccessPageProps) { data.ranks.length === 0 && data.acl.roles === 0 && data.acl.permissions === 0; - if (empty) { + if (empty && data.partialDependencies.length === 0) { body = state( "empty", "No access data", diff --git a/src/features/housekeeping/domains/system/pages/configuration.tsx b/src/features/housekeeping/domains/system/pages/configuration.tsx index 1b7ac371..fc68d6a1 100644 --- a/src/features/housekeeping/domains/system/pages/configuration.tsx +++ b/src/features/housekeeping/domains/system/pages/configuration.tsx @@ -111,7 +111,7 @@ export function SystemConfigurationPage({ data.kind === "settings" ? data.settings.length === 0 : data.settings.length === 0 && data.texts.length === 0; - if (empty) { + if (empty && data.partialDependencies.length === 0) { body = pageState( "empty", "No configuration entries", diff --git a/src/features/housekeeping/domains/system/pages/observability.tsx b/src/features/housekeeping/domains/system/pages/observability.tsx index 0fb1a568..798bca18 100644 --- a/src/features/housekeeping/domains/system/pages/observability.tsx +++ b/src/features/housekeeping/domains/system/pages/observability.tsx @@ -162,7 +162,10 @@ export function SystemObservabilityPage({ "Observability data unavailable", `Request ${result.correlationId} could not be completed.`, ); - } else if (isEmpty(result.data)) { + } else if ( + isEmpty(result.data) && + result.data.partialDependencies.length === 0 + ) { body = pageState( "empty", "No observations", diff --git a/src/features/housekeeping/domains/system/pages/system-pages.test.tsx b/src/features/housekeeping/domains/system/pages/system-pages.test.tsx index bfcd0efe..eebe8ef3 100644 --- a/src/features/housekeeping/domains/system/pages/system-pages.test.tsx +++ b/src/features/housekeeping/domains/system/pages/system-pages.test.tsx @@ -155,3 +155,52 @@ describe.each(pageCases)( }); }, ); + +it.each([ + [ + "access", + (result: HousekeepingResult) => + renderToStaticMarkup(), + { + kind: "permissions", + ranks: [], + acl: { roles: 0, permissions: 0 }, + partialDependencies: ["acl"], + }, + ], + [ + "configuration", + (result: HousekeepingResult) => + renderToStaticMarkup( + , + ), + { + kind: "emulator", + settings: [], + texts: [], + textsTotal: 0, + partialDependencies: ["emulator-texts"], + }, + ], + [ + "observability", + (result: HousekeepingResult) => + renderToStaticMarkup( + , + ), + { + kind: "devops", + health: null, + errors: [], + partialDependencies: ["health"], + }, + ], +] as const)( + "renders %s as partial when a dependency failed and surviving data is empty", + (_name, render, data) => { + const html = render(ok(data, correlationId)); + + expect(html).toContain('data-housekeeping-state="partial"'); + expect(html).not.toContain('data-housekeeping-state="empty"'); + }, +); diff --git a/src/features/housekeeping/domains/system/queries/operations-production.test.ts b/src/features/housekeeping/domains/system/queries/operations-production.test.ts new file mode 100644 index 00000000..1d426624 --- /dev/null +++ b/src/features/housekeeping/domains/system/queries/operations-production.test.ts @@ -0,0 +1,64 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const { select } = vi.hoisted(() => ({ select: vi.fn() })); + +vi.mock("drizzle-orm", () => ({ + count: vi.fn(() => "count"), + desc: vi.fn(() => "descending"), + eq: vi.fn(() => "equals"), +})); + +vi.mock("@/lib/db", () => ({ + db: { select }, + User: { + id: "id", + username: "username", + look: "look", + lastOnline: "lastOnline", + online: "online", + }, +})); + +import { fetchOpsOnlineUsers } from "@/lib/admin/ops-online-users"; +import { systemOperationsAdapters } from "./operations"; + +function rejectOnlineUserQueries() { + select + .mockImplementationOnce(() => ({ + from: () => ({ + where: () => Promise.reject(new Error("online count unavailable")), + }), + })) + .mockImplementationOnce(() => ({ + from: () => ({ + where: () => ({ + orderBy: () => ({ + limit: () => Promise.reject(new Error("online roster unavailable")), + }), + }), + }), + })); +} + +describe("System online-users production adapter", () => { + beforeEach(() => { + select.mockReset(); + }); + + it("preserves the tolerant empty fallback for legacy callers", async () => { + rejectOnlineUserQueries(); + + await expect(fetchOpsOnlineUsers(40)).resolves.toEqual({ + count: 0, + users: [], + }); + }); + + it("surfaces database failure to the System query boundary", async () => { + rejectOnlineUserQueries(); + + await expect(systemOperationsAdapters.loadOnlineUsers()).rejects.toThrow( + "online count unavailable", + ); + }); +}); diff --git a/src/features/housekeeping/domains/system/queries/operations.ts b/src/features/housekeeping/domains/system/queries/operations.ts index 46ba68c7..e54123b5 100644 --- a/src/features/housekeeping/domains/system/queries/operations.ts +++ b/src/features/housekeeping/domains/system/queries/operations.ts @@ -207,10 +207,10 @@ export const systemOperationsAdapters: SystemOperationsAdapters = { return fetchOpsHealth(); }, async loadOnlineUsers() { - const { fetchOpsOnlineUsers } = await import( + const { fetchOpsOnlineUsersStrict } = await import( "@/lib/admin/ops-online-users" ); - return fetchOpsOnlineUsers(40); + return fetchOpsOnlineUsersStrict(40); }, async loadMaintenance() { const [{ inArray }, { db, WebsiteSetting }] = await Promise.all([ diff --git a/src/features/housekeeping/domains/system/routes.test.ts b/src/features/housekeeping/domains/system/routes.test.ts index 2adea78d..abecdffc 100644 --- a/src/features/housekeeping/domains/system/routes.test.ts +++ b/src/features/housekeeping/domains/system/routes.test.ts @@ -7,103 +7,103 @@ const expectedRoutes = [ [ "system.access.permissions", "/ase/system/access/permissions", - "pages.admin.hubs.tabs.permissions", + "pages.housekeeping.routes.system.access.permissions", PERMS.PERMISSIONS_MANAGE, ], [ "system.access.permission-detail", "/ase/system/access/permissions/:id", - "pages.admin.hubs.tabs.permissions", + "pages.housekeeping.routes.system.access.permission-detail", PERMS.PERMISSIONS_MANAGE, ], [ "system.configuration.settings", "/ase/system/configuration/settings", - "pages.admin.hubs.tabs.cms", + "pages.housekeeping.routes.system.configuration.settings", PERMS.SETTINGS_VIEW, ], [ "system.configuration.emulator", "/ase/system/configuration/emulator", - "pages.admin.hubs.tabs.emulator", + "pages.housekeeping.routes.system.configuration.emulator", PERMS.SETTINGS_VIEW, ], [ "system.observability.analytics", "/ase/system/observability/analytics", - "pages.admin.hubs.tabs.analytics", + "pages.housekeeping.routes.system.observability.analytics", PERMS.ANALYTICS_VIEW, ], [ "system.observability.analytics-activity", "/ase/system/observability/analytics/activity", - "pages.admin.hubs.tabs.activity", + "pages.housekeeping.routes.system.observability.analytics-activity", PERMS.ANALYTICS_VIEW, ], [ "system.observability.analytics-economy", "/ase/system/observability/analytics/economy", - "pages.admin.hubs.tabs.economy", + "pages.housekeeping.routes.system.observability.analytics-economy", PERMS.ANALYTICS_VIEW, ], [ "system.observability.devops", "/ase/system/observability/devops", - "pages.admin.hubs.tabs.devops", + "pages.housekeeping.routes.system.observability.devops", PERMS.DEVOPS_VIEW, ], [ "system.observability.devops-errors", "/ase/system/observability/devops/errors", - "pages.admin.hubs.tabs.errors", + "pages.housekeeping.routes.system.observability.devops-errors", PERMS.DEVOPS_VIEW, ], [ "system.observability.logs-staff", "/ase/system/observability/logs/staff", - "pages.admin.hubs.tabs.logs", + "pages.housekeeping.routes.system.observability.logs-staff", PERMS.LOGS_VIEW, ], [ "system.observability.logs-audit", "/ase/system/observability/logs/audit", - "pages.admin.hubs.tabs.audit", + "pages.housekeeping.routes.system.observability.logs-audit", PERMS.LOGS_VIEW, ], [ "system.observability.logs-chat", "/ase/system/observability/logs/chat", - "pages.admin.hubs.tabs.chat", + "pages.housekeeping.routes.system.observability.logs-chat", PERMS.LOGS_VIEW, ], [ "system.observability.logs-commands", "/ase/system/observability/logs/commands", - "pages.admin.hubs.tabs.commands", + "pages.housekeeping.routes.system.observability.logs-commands", PERMS.LOGS_VIEW, ], [ "system.observability.logs-trades", "/ase/system/observability/logs/trades", - "pages.admin.hubs.tabs.trades", + "pages.housekeeping.routes.system.observability.logs-trades", PERMS.LOGS_VIEW, ], [ "system.operations.alerts", "/ase/system/operations/alerts", - "pages.admin.hubs.tabs.alerts", + "pages.housekeeping.routes.system.operations.alerts", PERMS.NOTIFICATIONS_VIEW, ], [ "system.operations.command-center", "/ase/system/operations/command-center", - "pages.admin.hubs.tabs.commando", + "pages.housekeeping.routes.system.operations.command-center", PERMS.RCON_EXECUTE, ], [ "system.operations.maintenance", "/ase/system/operations/maintenance", - "pages.admin.hubs.tabs.maintenance", + "pages.housekeeping.routes.system.operations.maintenance", PERMS.SETTINGS_VIEW, ], ] as const; diff --git a/src/features/housekeeping/domains/system/routes.ts b/src/features/housekeeping/domains/system/routes.ts index 62f5dd81..b5a4499c 100644 --- a/src/features/housekeeping/domains/system/routes.ts +++ b/src/features/housekeeping/domains/system/routes.ts @@ -29,103 +29,105 @@ export type SystemRouteId = (typeof SYSTEM_ROUTE_IDS)[number]; export const SYSTEM_ROUTES = [ { id: "system.access.permissions", - labelKey: "pages.admin.hubs.tabs.permissions", + labelKey: "pages.housekeeping.routes.system.access.permissions", href: "/ase/system/access/permissions", capability: anyCapability(PERMS.PERMISSIONS_MANAGE), }, { id: "system.access.permission-detail", - labelKey: "pages.admin.hubs.tabs.permissions", + labelKey: "pages.housekeeping.routes.system.access.permission-detail", href: "/ase/system/access/permissions/:id", capability: anyCapability(PERMS.PERMISSIONS_MANAGE), }, { id: "system.configuration.settings", - labelKey: "pages.admin.hubs.tabs.cms", + labelKey: "pages.housekeeping.routes.system.configuration.settings", href: "/ase/system/configuration/settings", capability: anyCapability(PERMS.SETTINGS_VIEW), }, { id: "system.configuration.emulator", - labelKey: "pages.admin.hubs.tabs.emulator", + labelKey: "pages.housekeeping.routes.system.configuration.emulator", href: "/ase/system/configuration/emulator", capability: anyCapability(PERMS.SETTINGS_VIEW), }, { id: "system.observability.analytics", - labelKey: "pages.admin.hubs.tabs.analytics", + labelKey: "pages.housekeeping.routes.system.observability.analytics", href: "/ase/system/observability/analytics", capability: anyCapability(PERMS.ANALYTICS_VIEW), }, { id: "system.observability.analytics-activity", - labelKey: "pages.admin.hubs.tabs.activity", + labelKey: + "pages.housekeeping.routes.system.observability.analytics-activity", href: "/ase/system/observability/analytics/activity", capability: anyCapability(PERMS.ANALYTICS_VIEW), }, { id: "system.observability.analytics-economy", - labelKey: "pages.admin.hubs.tabs.economy", + labelKey: + "pages.housekeeping.routes.system.observability.analytics-economy", href: "/ase/system/observability/analytics/economy", capability: anyCapability(PERMS.ANALYTICS_VIEW), }, { id: "system.observability.devops", - labelKey: "pages.admin.hubs.tabs.devops", + labelKey: "pages.housekeeping.routes.system.observability.devops", href: "/ase/system/observability/devops", capability: anyCapability(PERMS.DEVOPS_VIEW), }, { id: "system.observability.devops-errors", - labelKey: "pages.admin.hubs.tabs.errors", + labelKey: "pages.housekeeping.routes.system.observability.devops-errors", href: "/ase/system/observability/devops/errors", capability: anyCapability(PERMS.DEVOPS_VIEW), }, { id: "system.observability.logs-staff", - labelKey: "pages.admin.hubs.tabs.logs", + labelKey: "pages.housekeeping.routes.system.observability.logs-staff", href: "/ase/system/observability/logs/staff", capability: anyCapability(PERMS.LOGS_VIEW), }, { id: "system.observability.logs-audit", - labelKey: "pages.admin.hubs.tabs.audit", + labelKey: "pages.housekeeping.routes.system.observability.logs-audit", href: "/ase/system/observability/logs/audit", capability: anyCapability(PERMS.LOGS_VIEW), }, { id: "system.observability.logs-chat", - labelKey: "pages.admin.hubs.tabs.chat", + labelKey: "pages.housekeeping.routes.system.observability.logs-chat", href: "/ase/system/observability/logs/chat", capability: anyCapability(PERMS.LOGS_VIEW), }, { id: "system.observability.logs-commands", - labelKey: "pages.admin.hubs.tabs.commands", + labelKey: "pages.housekeeping.routes.system.observability.logs-commands", href: "/ase/system/observability/logs/commands", capability: anyCapability(PERMS.LOGS_VIEW), }, { id: "system.observability.logs-trades", - labelKey: "pages.admin.hubs.tabs.trades", + labelKey: "pages.housekeeping.routes.system.observability.logs-trades", href: "/ase/system/observability/logs/trades", capability: anyCapability(PERMS.LOGS_VIEW), }, { id: "system.operations.alerts", - labelKey: "pages.admin.hubs.tabs.alerts", + labelKey: "pages.housekeeping.routes.system.operations.alerts", href: "/ase/system/operations/alerts", capability: anyCapability(PERMS.NOTIFICATIONS_VIEW), }, { id: "system.operations.command-center", - labelKey: "pages.admin.hubs.tabs.commando", + labelKey: "pages.housekeeping.routes.system.operations.command-center", href: "/ase/system/operations/command-center", capability: anyCapability(PERMS.RCON_EXECUTE), }, { id: "system.operations.maintenance", - labelKey: "pages.admin.hubs.tabs.maintenance", + labelKey: "pages.housekeeping.routes.system.operations.maintenance", href: "/ase/system/operations/maintenance", capability: anyCapability(PERMS.SETTINGS_VIEW), }, diff --git a/src/features/housekeeping/domains/system/services/mutations.ts b/src/features/housekeeping/domains/system/services/mutations.ts index cd4df23c..efb75fe1 100644 --- a/src/features/housekeeping/domains/system/services/mutations.ts +++ b/src/features/housekeeping/domains/system/services/mutations.ts @@ -208,6 +208,25 @@ async function requireRcon(result: boolean): Promise { } } +async function requireRankPermissionSynchronization( + operation: "access.rank.create" | "access.rank.delete" | "access.rank.update", +): Promise { + try { + if (await rcon.send("updatepermissions")) return; + } catch { + // Report the already committed local changes through the typed envelope. + } + throw new SystemMutationFailure( + "DEPENDENCY_UNAVAILABLE", + "errors.housekeeping.system.rankSynchronizationIncomplete", + { + operation: [operation], + completed: ["database", "audit"], + pending: ["emulator-permission-cache"], + }, + ); +} + async function upsertWebsiteSetting( key: string, value: string, @@ -244,7 +263,7 @@ async function executeAccessMutation( targetType: "rank", targetId: id, }); - await rcon.send("updatepermissions"); + await requireRankPermissionSynchronization("access.rank.create"); return { id }; } @@ -289,7 +308,7 @@ async function executeAccessMutation( targetType: "rank", targetId: id, }); - await rcon.send("updatepermissions"); + await requireRankPermissionSynchronization("access.rank.delete"); return null; } @@ -316,7 +335,7 @@ async function executeAccessMutation( targetType: "rank", targetId: id, }); - await rcon.send("updatepermissions"); + await requireRankPermissionSynchronization("access.rank.update"); return null; } @@ -658,7 +677,19 @@ async function executeRconMutation( } } await requireRcon(await rcon.setRank(userId, rank)); - await db.update(User).set({ rank }).where(eq(User.id, userId)); + try { + await db.update(User).set({ rank }).where(eq(User.id, userId)); + } catch { + throw new SystemMutationFailure( + "DEPENDENCY_UNAVAILABLE", + "errors.housekeeping.system.rankPersistenceIncomplete", + { + operation: ["rcon.set-rank"], + completed: ["emulator"], + pending: ["database"], + }, + ); + } break; } case "rcon.execute-command": diff --git a/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts b/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts new file mode 100644 index 00000000..6a05d03a --- /dev/null +++ b/src/features/housekeeping/domains/system/services/rank-mutations-production.test.ts @@ -0,0 +1,225 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { PERMS } from "@/lib/permission-slugs"; +import { dispatchHousekeepingCommand } from "../../../foundation/commands/dispatcher"; +import { + type HousekeepingCommand, + registerHousekeepingCommand, +} from "../../../foundation/commands/registry"; +import type { HousekeepingCapabilityContext } from "../../../foundation/contracts"; + +const doubles = vi.hoisted(() => ({ + createEmulatorRank: vi.fn(), + dbExecute: vi.fn(), + dbInsert: vi.fn(), + dbSelect: vi.fn(), + dbTransaction: vi.fn(), + dbUpdate: vi.fn(), + deleteEmulatorRank: vi.fn(), + logStaffActivity: vi.fn(), + rconSend: vi.fn(), + rconSetRank: vi.fn(), + updateEmulatorRank: vi.fn(), +})); + +vi.mock("@/lib/db", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + db: { + ...actual.db, + execute: doubles.dbExecute, + insert: doubles.dbInsert, + select: doubles.dbSelect, + transaction: doubles.dbTransaction, + update: doubles.dbUpdate, + }, + }; +}); + +vi.mock("@/lib/services/permission-ranks", () => ({ + createEmulatorRank: doubles.createEmulatorRank, + deleteEmulatorRank: doubles.deleteEmulatorRank, + updateEmulatorRank: doubles.updateEmulatorRank, +})); + +vi.mock("@/lib/services/rcon", () => ({ + rcon: { send: doubles.rconSend, setRank: doubles.rconSetRank }, +})); + +vi.mock("@/lib/services/staff-activity", () => ({ + logStaffActivity: doubles.logStaffActivity, +})); + +import { createSystemCommands } from "../commands/system-commands"; +import { + type SystemMutationOperation, + systemMutationService, +} from "./mutations"; + +function capabilityContext( + granted: readonly string[], +): HousekeepingCapabilityContext { + const permissions = new Set(granted); + return { + actor: { id: 42, username: "operator", rank: 500 }, + isSuperAdmin: false, + has: (slug) => permissions.has(slug), + hasAny: (...slugs) => slugs.some((slug) => permissions.has(slug)), + hasAll: (...slugs) => slugs.every((slug) => permissions.has(slug)), + }; +} + +function serviceContext(permission: string) { + return { + capability: capabilityContext([permission]), + correlationId: "rank-mutation-correlation", + }; +} + +function insertChain() { + return { + values: () => ({ + onDuplicateKeyUpdate: vi.fn().mockResolvedValue(undefined), + }), + }; +} + +function limitedSelection(rows: readonly unknown[]) { + return { + from: () => ({ + where: () => ({ limit: () => Promise.resolve(rows) }), + }), + }; +} + +function plainSelection(rows: readonly unknown[]) { + return { + from: () => ({ where: () => Promise.resolve(rows) }), + }; +} + +beforeEach(() => { + for (const mock of Object.values(doubles)) mock.mockReset(); + doubles.createEmulatorRank.mockResolvedValue(7); + doubles.deleteEmulatorRank.mockResolvedValue(undefined); + doubles.updateEmulatorRank.mockResolvedValue(undefined); + doubles.logStaffActivity.mockResolvedValue(undefined); + doubles.dbInsert.mockImplementation(insertChain); +}); + +describe("rank synchronization failures", () => { + it("returns failure and audits failure when create committed but updatepermissions returned false", async () => { + doubles.rconSend.mockResolvedValue(false); + const command = createSystemCommands(systemMutationService).find( + (entry) => entry.id === "system.access.rank.create", + ) as + | HousekeepingCommand<{ name: string; level: number }, unknown> + | undefined; + if (!command) throw new Error("rank create command missing"); + registerHousekeepingCommand(command); + const outcomes: string[] = []; + + const result = await dispatchHousekeepingCommand( + { + commandId: command.id, + input: { name: "Administrator", level: 7 }, + reason: "Create the administrator rank", + }, + { + context: capabilityContext([PERMS.PERMISSIONS_MANAGE]), + ipAddress: "198.51.100.8", + audit: { + write: async (entry) => { + if (entry.outcome) outcomes.push(entry.outcome); + }, + }, + rateLimit: async () => true, + }, + ); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "DEPENDENCY_UNAVAILABLE", + messageKey: "errors.housekeeping.system.rankSynchronizationIncomplete", + fieldErrors: { + operation: ["access.rank.create"], + completed: ["database", "audit"], + pending: ["emulator-permission-cache"], + }, + }, + }); + expect(outcomes).toEqual(["intent", "failure"]); + }); + + it.each([ + [ + "access.rank.delete", + { id: 7 }, + () => { + doubles.dbSelect + .mockImplementationOnce(() => plainSelection([{ total: 0 }])) + .mockImplementationOnce(() => limitedSelection([])); + }, + ], + ["access.rank.update", { id: 7, fields: { badge: "ADM" } }, () => {}], + ] as const)( + "returns failure when %s committed but updatepermissions returned false", + async (operation, input, prepare) => { + prepare(); + doubles.rconSend.mockResolvedValue(false); + + const result = await systemMutationService.execute( + serviceContext(PERMS.PERMISSIONS_MANAGE), + operation as SystemMutationOperation, + input, + ); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "DEPENDENCY_UNAVAILABLE", + messageKey: + "errors.housekeeping.system.rankSynchronizationIncomplete", + fieldErrors: { + operation: [operation], + completed: ["database", "audit"], + pending: ["emulator-permission-cache"], + }, + }, + }); + }, + ); + + it("reports external completion when set-rank RCON succeeded but DB persistence failed", async () => { + doubles.dbSelect.mockImplementationOnce(() => + limitedSelection([{ rank: 3 }]), + ); + doubles.dbExecute.mockResolvedValue([[{ id: 4 }]]); + doubles.rconSetRank.mockResolvedValue(true); + doubles.dbUpdate.mockReturnValue({ + set: () => ({ + where: () => Promise.reject(new Error("users update unavailable")), + }), + }); + + const result = await systemMutationService.execute( + serviceContext(PERMS.RCON_EXECUTE), + "rcon.set-rank", + { userId: 8, rank: 4 }, + ); + + expect(result).toMatchObject({ + ok: false, + error: { + code: "DEPENDENCY_UNAVAILABLE", + messageKey: "errors.housekeeping.system.rankPersistenceIncomplete", + fieldErrors: { + operation: ["rcon.set-rank"], + completed: ["emulator"], + pending: ["database"], + }, + }, + }); + }); +}); diff --git a/src/features/housekeeping/foundation/localization-contract.test.ts b/src/features/housekeeping/foundation/localization-contract.test.ts index b86768fa..12c5eb7c 100644 --- a/src/features/housekeeping/foundation/localization-contract.test.ts +++ b/src/features/housekeeping/foundation/localization-contract.test.ts @@ -21,6 +21,26 @@ const requiredKeys = [ "pages.housekeeping.states.error.description", ]; +const systemRouteMessageKeys = [ + "pages.housekeeping.routes.system.access.permissions", + "pages.housekeeping.routes.system.access.permission-detail", + "pages.housekeeping.routes.system.configuration.settings", + "pages.housekeeping.routes.system.configuration.emulator", + "pages.housekeeping.routes.system.observability.analytics", + "pages.housekeeping.routes.system.observability.analytics-activity", + "pages.housekeeping.routes.system.observability.analytics-economy", + "pages.housekeeping.routes.system.observability.devops", + "pages.housekeeping.routes.system.observability.devops-errors", + "pages.housekeeping.routes.system.observability.logs-staff", + "pages.housekeeping.routes.system.observability.logs-audit", + "pages.housekeeping.routes.system.observability.logs-chat", + "pages.housekeeping.routes.system.observability.logs-commands", + "pages.housekeeping.routes.system.observability.logs-trades", + "pages.housekeeping.routes.system.operations.alerts", + "pages.housekeeping.routes.system.operations.command-center", + "pages.housekeeping.routes.system.operations.maintenance", +] as const; + const expectedDomainMessages = [ { id: "operations", @@ -74,6 +94,7 @@ describe("housekeeping localization contract", () => { "preview", "navigation", "domains", + "routes", "states", ]); @@ -81,6 +102,12 @@ describe("housekeeping localization contract", () => { expect(resolveMessage(messages, key), key).toEqual(expect.any(String)); } + for (const key of systemRouteMessageKeys) { + const message = resolveMessage(messages, key); + expect(message, key).toEqual(expect.any(String)); + expect((message as string).trim(), key).not.toBe(""); + } + for (const [index, expected] of expectedDomainMessages.entries()) { const manifest = HOUSEKEEPING_MANIFESTS[index]; diff --git a/src/features/housekeeping/foundation/preview-route-contract.test.ts b/src/features/housekeeping/foundation/preview-route-contract.test.ts index 5470bbdd..68c97827 100644 --- a/src/features/housekeeping/foundation/preview-route-contract.test.ts +++ b/src/features/housekeeping/foundation/preview-route-contract.test.ts @@ -19,6 +19,42 @@ const routeMocks = vi.hoisted(() => { "domains.people.description": "Localized People description", "domains.economy.title": "Localized Economy", "domains.economy.description": "Localized Economy description", + "domains.operations.title": "Localized Operations", + "domains.operations.description": "Localized Operations description", + "domains.content.title": "Localized Content", + "domains.content.description": "Localized Content description", + "domains.hotel.title": "Localized Hotel", + "domains.hotel.description": "Localized Hotel description", + "domains.system.title": "HK::system-title", + "domains.system.description": "Localized System description", + "routes.system.access.permissions": "HK::system-access-permissions", + "routes.system.access.permission-detail": + "HK::system-access-permission-detail", + "routes.system.configuration.settings": "HK::system-configuration-settings", + "routes.system.configuration.emulator": "HK::system-configuration-emulator", + "routes.system.observability.analytics": + "HK::system-observability-analytics", + "routes.system.observability.analytics-activity": + "HK::system-observability-analytics-activity", + "routes.system.observability.analytics-economy": + "HK::system-observability-analytics-economy", + "routes.system.observability.devops": "HK::system-observability-devops", + "routes.system.observability.devops-errors": + "HK::system-observability-devops-errors", + "routes.system.observability.logs-staff": + "HK::system-observability-logs-staff", + "routes.system.observability.logs-audit": + "HK::system-observability-logs-audit", + "routes.system.observability.logs-chat": + "HK::system-observability-logs-chat", + "routes.system.observability.logs-commands": + "HK::system-observability-logs-commands", + "routes.system.observability.logs-trades": + "HK::system-observability-logs-trades", + "routes.system.operations.alerts": "HK::system-operations-alerts", + "routes.system.operations.command-center": + "HK::system-operations-command-center", + "routes.system.operations.maintenance": "HK::system-operations-maintenance", "states.empty.title": "Localized empty title", "states.empty.description": "Localized empty description", }; @@ -599,6 +635,44 @@ describe("/ase-next/[domain] layout", () => { ); expect(routeMocks.getTranslations).toHaveBeenCalledTimes(1); }); + + it("builds and translates the complete real System navigation", async () => { + routeMocks.getHousekeepingCapabilityContext.mockResolvedValue({ + ...capabilityContext([]), + isSuperAdmin: true, + }); + + const html = await renderRoute( + AdminNextDomainLayout({ + children: createElement("p", null, "System body"), + params: Promise.resolve({ domain: "system" }), + }), + ); + + for (const label of [ + "HK::system-access-permissions", + "HK::system-access-permission-detail", + "HK::system-configuration-settings", + "HK::system-configuration-emulator", + "HK::system-observability-analytics", + "HK::system-observability-analytics-activity", + "HK::system-observability-analytics-economy", + "HK::system-observability-devops", + "HK::system-observability-devops-errors", + "HK::system-observability-logs-staff", + "HK::system-observability-logs-audit", + "HK::system-observability-logs-chat", + "HK::system-observability-logs-commands", + "HK::system-observability-logs-trades", + "HK::system-operations-alerts", + "HK::system-operations-command-center", + "HK::system-operations-maintenance", + ]) { + expect(html).toContain(label); + } + expect(html).toContain("HK::system-title"); + expect(html).toContain("System body"); + }); }); describe("/ase-next/[domain]/[[...segments]] page", () => { diff --git a/src/lib/admin/ops-online-users.ts b/src/lib/admin/ops-online-users.ts index 3b60597d..94b1b0ef 100644 --- a/src/lib/admin/ops-online-users.ts +++ b/src/lib/admin/ops-online-users.ts @@ -1,41 +1,40 @@ +import "server-only"; + import { count, desc, eq } from "drizzle-orm"; import type { OnlineUser } from "@/components/admin/dashboard"; import { db, User } from "@/lib/db"; -/** Shared online roster used by CommandoCentrum (and linked from other ops hubs). */ -export async function fetchOpsOnlineUsers( - limit = 40, -): Promise<{ count: number; users: OnlineUser[] }> { - const [countRow, onlineUsersRaw] = await Promise.all([ - db - .select({ total: count() }) - .from(User) - .where(eq(User.online, "1")) - .then((rows) => rows[0]?.total ?? 0) - .catch(() => 0), - db - .select({ - id: User.id, - username: User.username, - look: User.look, - lastOnline: User.lastOnline, - }) - .from(User) - .where(eq(User.online, "1")) - .orderBy(desc(User.lastOnline)) - .limit(limit) - .then((rows) => rows) - .catch( - () => - [] as Array<{ - id: number; - username: string; - look: string; - lastOnline: number; - }>, - ), - ]); +interface OpsOnlineUserRow { + readonly id: number; + readonly username: string; + readonly look: string; + readonly lastOnline: number; +} +function loadOnlineCount(): Promise { + return db + .select({ total: count() }) + .from(User) + .where(eq(User.online, "1")) + .then((rows) => rows[0]?.total ?? 0); +} + +function loadOnlineRows(limit: number): Promise { + return db + .select({ + id: User.id, + username: User.username, + look: User.look, + lastOnline: User.lastOnline, + }) + .from(User) + .where(eq(User.online, "1")) + .orderBy(desc(User.lastOnline)) + .limit(limit) + .then((rows) => rows); +} + +function onlineRoster(countRow: number, onlineUsersRaw: OpsOnlineUserRow[]) { const users: OnlineUser[] = onlineUsersRaw.map((u) => ({ id: u.id, username: u.username, @@ -47,3 +46,25 @@ export async function fetchOpsOnlineUsers( return { count: countRow, users }; } + +/** Fail-aware roster for guarded System workflows. */ +export async function fetchOpsOnlineUsersStrict( + limit = 40, +): Promise<{ count: number; users: OnlineUser[] }> { + const [countRow, onlineUsersRaw] = await Promise.all([ + loadOnlineCount(), + loadOnlineRows(limit), + ]); + return onlineRoster(countRow, onlineUsersRaw); +} + +/** Shared tolerant roster used by the legacy CommandoCentrum. */ +export async function fetchOpsOnlineUsers( + limit = 40, +): Promise<{ count: number; users: OnlineUser[] }> { + const [countRow, onlineUsersRaw] = await Promise.all([ + loadOnlineCount().catch(() => 0), + loadOnlineRows(limit).catch(() => []), + ]); + return onlineRoster(countRow, onlineUsersRaw); +} diff --git a/src/messages/en.json b/src/messages/en.json index 01bd05db..3866eeef 100644 --- a/src/messages/en.json +++ b/src/messages/en.json @@ -3294,6 +3294,35 @@ "description": "Configuration, observability, and access" } }, + "routes": { + "system": { + "access": { + "permissions": "Permissions", + "permission-detail": "Permission details" + }, + "configuration": { + "settings": "CMS settings", + "emulator": "Emulator settings" + }, + "observability": { + "analytics": "Analytics", + "analytics-activity": "Activity analytics", + "analytics-economy": "Economy analytics", + "devops": "DevOps", + "devops-errors": "Emulator errors", + "logs-staff": "Staff logs", + "logs-audit": "Audit logs", + "logs-chat": "Chat logs", + "logs-commands": "Command logs", + "logs-trades": "Trade logs" + }, + "operations": { + "alerts": "Alerts", + "command-center": "Command center", + "maintenance": "Maintenance" + } + } + }, "states": { "loading": { "title": "Loading housekeeping", diff --git a/src/messages/it.json b/src/messages/it.json index 4b02a6d7..301d1b8d 100644 --- a/src/messages/it.json +++ b/src/messages/it.json @@ -3299,6 +3299,35 @@ "description": "Configurazione, osservabilità e accesso" } }, + "routes": { + "system": { + "access": { + "permissions": "Permessi", + "permission-detail": "Dettagli permesso" + }, + "configuration": { + "settings": "Impostazioni CMS", + "emulator": "Impostazioni emulatore" + }, + "observability": { + "analytics": "Analisi", + "analytics-activity": "Analisi attività", + "analytics-economy": "Analisi economia", + "devops": "DevOps", + "devops-errors": "Errori emulatore", + "logs-staff": "Log staff", + "logs-audit": "Log di audit", + "logs-chat": "Log chat", + "logs-commands": "Log comandi", + "logs-trades": "Log scambi" + }, + "operations": { + "alerts": "Avvisi", + "command-center": "Centro comandi", + "maintenance": "Manutenzione" + } + } + }, "states": { "loading": { "title": "Caricamento in corso",