From fdb1b6b34448593626db64ec664e473a432e39d1 Mon Sep 17 00:00:00 2001 From: Todd Hebebrand Date: Tue, 8 Sep 2026 04:23:01 -0600 Subject: [PATCH 1/4] feat(ai-operator): task admission route POST /ai/operator/tasks (W08 of #5205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The only door that creates an Operator task. `ai_agents:write` + the MFA step-up (same pair `POST /ai/agents/:id/runs` carries), explicit org from the body checked against the caller's access, device resolved and site-gated with the same non-enumerating 404 the W07 read routes use, readiness recomputed on launch (422 with an actionable code), a 100-per-org pending cap (429), and 202 with the task id. Idempotency is a DATABASE constraint, not a route check. A duplicate POST can dispatch a second service restart to a customer machine, and a route-level read-then-insert loses the race between two concurrent clicks. So `ai_operator_tasks` gains a nullable `client_idempotency_key` and a PARTIAL unique index on `(org_id, client_idempotency_key)`, and admission inserts with `ON CONFLICT ... WHERE ... DO NOTHING` — `DO NOTHING` rather than catching 23505, because a unique violation aborts the transaction the read-back would have to run in. A conflict then reads the winner, scoped by org AND key, and answers 202 with that id: a replay is a success, not a conflict. Threading the key required an additive change to W06's `admitServiceRecoveryTask` (an optional input field and a `replayed` flag on its result). That crosses the wave boundary deliberately: every alternative that left the file untouched put the reservation and the task insert in different transactions, and the coordinator can pick a task up before the route binds anything. Design was put to an advisor quorum (this session + codex `xhigh`, read-only); both reached the same answer independently. Also: `features.aiOperatorTasks` on `GET /config` (AND of both flags, both default off per decision D2) plus a default-CLOSED `useAiOperatorTasksGate()` web hook, so the UI action is absent — not merely disabled — when the feature is off or `/config` is unreachable. Contract registration: the new column is classified in `CORE_TENANT_EXPORT_POLICY` (a new COLUMN on an org-cascade table breaks that contract, which is the point). No RLS change — shape 1, policies are column-agnostic. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YMbWgjQdqXzi98eJ5P3Uqp --- ...00-ai-operator-task-client-idempotency.sql | 30 ++ .../aiOperatorAdmission.integration.test.ts | 460 ++++++++++++++++++ apps/api/src/db/schema/aiOperatorTasks.ts | 19 + apps/api/src/routes/aiOperatorTasks.test.ts | 283 ++++++++++- apps/api/src/routes/aiOperatorTasks.ts | 207 +++++++- apps/api/src/routes/config.test.ts | 33 +- apps/api/src/routes/config.ts | 6 +- .../src/services/aiOperator/taskService.ts | 73 ++- .../services/tenantExportPolicyRegistry.ts | 2 +- .../auth/PartnerRegisterPage.test.tsx | 2 +- apps/web/src/stores/featuresStore.test.ts | 10 +- apps/web/src/stores/featuresStore.ts | 23 +- packages/shared/src/validators/aiOperator.ts | 59 +++ 13 files changed, 1184 insertions(+), 23 deletions(-) create mode 100644 apps/api/migrations/2026-10-14-100500-ai-operator-task-client-idempotency.sql create mode 100644 apps/api/src/__tests__/integration/aiOperatorAdmission.integration.test.ts diff --git a/apps/api/migrations/2026-10-14-100500-ai-operator-task-client-idempotency.sql b/apps/api/migrations/2026-10-14-100500-ai-operator-task-client-idempotency.sql new file mode 100644 index 0000000000..ac9a29d2c8 --- /dev/null +++ b/apps/api/migrations/2026-10-14-100500-ai-operator-task-client-idempotency.sql @@ -0,0 +1,30 @@ +-- AI Operator: admission idempotency key (#5205 W08, #5246), spec §12. +-- +-- `POST /api/v1/ai/operator/tasks` carries a client idempotency key. A +-- duplicate POST must return the SAME task id and must never admit a second +-- task, because each task can dispatch a real service-restart command to a +-- customer machine — a second admission is a duplicate remediation effect. +-- +-- The guarantee is a PARTIAL UNIQUE INDEX rather than a route-level +-- read-then-insert: two concurrent clicks (or a client retry racing its own +-- first request) both pass a SELECT and both insert. Only the database can +-- serialize them. +-- +-- Partial on `client_idempotency_key IS NOT NULL` so every pre-existing row +-- and every internally-admitted task (the coordinator's successors, recovery +-- scans) keeps a NULL and never collides. Scoped by `org_id` so one tenant can +-- neither probe nor squat another tenant's keys. +-- +-- No RLS change: `ai_operator_tasks` is tenancy shape 1 (direct NOT NULL +-- `org_id`) and its policies are column-agnostic. The new column IS registered +-- in CORE_TENANT_EXPORT_POLICY in the same PR — adding a column to an +-- org-cascade table breaks that contract test, which is exactly the point. +-- +-- No DML in this file, so no `breeze.scope` elevation is required. + +ALTER TABLE ai_operator_tasks + ADD COLUMN IF NOT EXISTS client_idempotency_key text; + +CREATE UNIQUE INDEX IF NOT EXISTS ai_operator_tasks_client_idempotency_uq + ON ai_operator_tasks (org_id, client_idempotency_key) + WHERE client_idempotency_key IS NOT NULL; diff --git a/apps/api/src/__tests__/integration/aiOperatorAdmission.integration.test.ts b/apps/api/src/__tests__/integration/aiOperatorAdmission.integration.test.ts new file mode 100644 index 0000000000..96efdb9312 --- /dev/null +++ b/apps/api/src/__tests__/integration/aiOperatorAdmission.integration.test.ts @@ -0,0 +1,460 @@ +import './setup'; + +import { randomUUID } from 'node:crypto'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { Hono } from 'hono'; +import { and, eq, sql } from 'drizzle-orm'; +import { db, withDbAccessContext, withSystemDbAccessContext, type DbAccessContext } from '../../db'; +import { aiAgents, aiOperatorTasks, devices, organizationUsers } from '../../db/schema'; +import { aiOperatorTasksRoutes } from '../../routes/aiOperatorTasks'; +import { clearPermissionCache } from '../../services/permissions'; +import { createOrganization, createSite, setupTestEnvironment, type TestEnvironment } from './db-utils'; +import { createAccessToken } from '../../services/jwt'; +import { getTestDb } from './setup'; + +/** + * Real-Postgres proof for W08 (#5205, #5246): `POST /ai/operator/tasks`. + * + * Four of these contracts CANNOT be evaluated against a mocked Drizzle client, + * which is the whole reason this file exists alongside the route unit suite: + * + * 1. **Idempotency is a database constraint, not a route check.** The unit + * suite can only assert that the route forwards `clientIdempotencyKey`; it + * cannot prove that a second admission is actually refused. That proof + * needs the partial unique index + * `ai_operator_tasks_client_idempotency_uq` and a real `ON CONFLICT ... + * WHERE ... DO NOTHING`. If the conflict target fails to match the PARTIAL + * index, Postgres raises 42P10 — a failure mode no mock has. + * 2. **Cross-org and site-restricted targets** are enforced by RLS on + * `ai_operator_tasks`/`devices` as the `breeze_app` role, with the route's + * predicates as defence in depth. Only a real role can show both hold. + * 3. **The pending cap** counts non-terminal states with a real `NOT IN` + * against real rows. + * 4. **The admitted row is durable without Redis.** Redis is never touched + * here; the assertion is that the committed row carries + * `state = 'queued'` and `next_wake_at <= now()`, which is what makes the + * coordinator's `queued_past_wake` scan pick it up. That is why a 202 is + * truthful (spec §12) even with Redis down. + */ +const runDb = it.runIf(!!process.env.DATABASE_URL); + +const SYSTEM_CTX: DbAccessContext = { + scope: 'system', + orgId: null, + accessibleOrgIds: null, + accessiblePartnerIds: null, +}; + +function buildApp(): Hono { + const app = new Hono(); + app.route('/api/v1/ai/operator', aiOperatorTasksRoutes); + return app; +} + +/** + * `setupTestEnvironment` mints tokens with `mfa: false`, and this route is MFA + * step-up gated (spec §5.1). Re-minting the SAME principal with `mfa: true` is + * what a real step-up produces; the "refuses without the MFA step-up" case + * below uses `env.token` unchanged as the control, so the gate is proved in + * both directions rather than assumed away. + */ +async function mfaToken(env: TestEnvironment): Promise { + return createAccessToken({ + sub: env.user.id, + email: env.user.email, + roleId: env.role.id, + orgId: env.organization.id, + partnerId: env.partner.id, + scope: 'organization', + mfa: true, + aep: 1, + mep: 1, + sid: randomUUID(), + }); +} + +async function post(env: TestEnvironment, payload: unknown, token?: string) { + const response = await buildApp().request('/api/v1/ai/operator/tasks', { + method: 'POST', + headers: { + Authorization: `Bearer ${token ?? await mfaToken(env)}`, + 'Content-Type': 'application/json', + }, + body: JSON.stringify(payload), + }); + const text = await response.text(); + let body: unknown = null; + try { body = JSON.parse(text); } catch { body = text; } + return { status: response.status, body: body as Record }; +} + +async function insertAgent(orgId: string | null, createdBy: string): Promise { + const [agent] = await withDbAccessContext(SYSTEM_CTX, () => + db.insert(aiAgents).values({ + orgId, partnerId: null, kind: 'triage', name: 'Operator', enabled: true, createdBy, + }).returning(), + ); + return agent!.id; +} + +async function insertDevice(orgId: string, siteId: string): Promise { + // Devices are inserted with the ADMIN connection: seeding a device is not + // the thing under test, and the request path below is the one that must run + // as breeze_app. + const adminDb = getTestDb() as unknown as typeof db; + const unique = randomUUID().slice(0, 8); + const [device] = await adminDb.insert(devices).values({ + orgId, + siteId, + agentId: `operator-admit-agent-${unique}`, + hostname: `operator-admit-host-${unique}`, + osType: 'windows', + osVersion: '10', + architecture: 'x86_64', + agentVersion: '0.0.0-test', + status: 'online', + }).returning(); + return (device as { id: string }).id; +} + +async function restrictUserToSites(env: TestEnvironment, siteIds: string[]) { + await withSystemDbAccessContext(async () => { + await db.update(organizationUsers).set({ siteIds }) + .where(and( + eq(organizationUsers.userId, env.user.id), + eq(organizationUsers.orgId, env.organization.id), + )); + }); + await clearPermissionCache(env.user.id); +} + +async function readTasks(orgId: string) { + return withDbAccessContext(SYSTEM_CTX, () => + db.select().from(aiOperatorTasks).where(eq(aiOperatorTasks.orgId, orgId))); +} + +interface Fixture { + env: TestEnvironment; + deviceId: string; + siteId: string; + agentId: string; +} + +async function seed(): Promise { + const env = await setupTestEnvironment({ scope: 'organization' }); + // ONE agent per org: `ai_agents_org_kind_uq` is unique on (org_id, kind), so + // the fixture's agent is reused everywhere rather than re-inserted. + const agentId = await insertAgent(env.organization.id, env.user.id); + const siteId = env.site.id; + const deviceId = await insertDevice(env.organization.id, siteId); + return { env, deviceId, siteId, agentId }; +} + +function bodyFor(f: Fixture, overrides: Record = {}) { + return { + mode: 'live', + recipeKey: 'service_recovery', + recipeVersion: 1, + orgId: f.env.organization.id, + deviceId: f.deviceId, + inputs: { serviceName: 'spooler' }, + clientIdempotencyKey: `delegate-${randomUUID()}`, + ...overrides, + }; +} + +describe('POST /ai/operator/tasks — admission against real Postgres (W08, #5246)', () => { + beforeEach(() => { + process.env.AI_OPERATOR_TASKS_ENABLED = 'true'; + process.env.AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED = 'true'; + }); + + afterEach(() => { + delete process.env.AI_OPERATOR_TASKS_ENABLED; + delete process.env.AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED; + }); + + runDb('commits a queued, immediately-wakeable task row and answers 202', async () => { + const f = await seed(); + const res = await post(f.env, bodyFor(f)); + + expect(res.status).toBe(202); + const taskId = res.body.taskId as string; + expect(taskId).toBeTruthy(); + + const rows = await readTasks(f.env.organization.id); + expect(rows).toHaveLength(1); + const task = rows[0]!; + expect(task.id).toBe(taskId); + expect(task.state).toBe('queued'); + expect(task.deviceId).toBe(f.deviceId); + expect(task.workflowKey).toBe('service_recovery'); + expect(task.mode).toBe('live'); + expect(task.requesterUserId).toBe(f.env.user.id); + // Durable WITHOUT Redis: the coordinator's `queued_past_wake` scan selects + // exactly this shape, so the 202 is already truthful at commit. + expect(task.nextWakeAt).not.toBeNull(); + expect(task.nextWakeAt!.getTime()).toBeLessThanOrEqual(Date.now() + 1_000); + // Admission freezes a bound; `evaluateTaskClaimPredicate` fails CLOSED on a + // null deadline, so a task admitted without one could never dispatch. + expect(task.deadlineAt).not.toBeNull(); + }); + + runDb('a replayed idempotency key returns the SAME task id and admits no second task', async () => { + const f = await seed(); + const body = bodyFor(f); + + const first = await post(f.env, body); + expect(first.status).toBe(202); + expect(first.body.replayed).toBe(false); + + const second = await post(f.env, body); + expect(second.status).toBe(202); + expect(second.body.taskId).toBe(first.body.taskId); + expect(second.body.replayed).toBe(true); + + // The contract that matters is not the status code — it is that no second + // task exists to dispatch a second restart. + expect(await readTasks(f.env.organization.id)).toHaveLength(1); + }); + + runDb('two CONCURRENT posts with one key still produce exactly one task', async () => { + const f = await seed(); + const body = bodyFor(f); + + // The race a route-level SELECT-then-INSERT loses: both requests read "no + // existing task", both insert. Only the partial unique index serializes + // them, and only a real database can demonstrate that it does. + const [a, b] = await Promise.all([post(f.env, body), post(f.env, body)]); + + expect([a.status, b.status]).toEqual([202, 202]); + expect(a.body.taskId).toBe(b.body.taskId); + expect(await readTasks(f.env.organization.id)).toHaveLength(1); + // Exactly one of the two was the real admission. + expect([a.body.replayed, b.body.replayed].filter((r) => r === false)).toHaveLength(1); + }); + + runDb('the same idempotency key in a DIFFERENT org admits its own task', async () => { + const f = await seed(); + const other = await setupTestEnvironment({ scope: 'organization' }); + await insertAgent(other.organization.id, other.user.id); + const otherDevice = await insertDevice(other.organization.id, other.site.id); + + const key = `delegate-${randomUUID()}`; + const first = await post(f.env, bodyFor(f, { clientIdempotencyKey: key })); + const second = await post(other, { + mode: 'live', recipeKey: 'service_recovery', recipeVersion: 1, + orgId: other.organization.id, deviceId: otherDevice, + inputs: { serviceName: 'spooler' }, clientIdempotencyKey: key, + }); + + expect(first.status).toBe(202); + expect(second.status).toBe(202); + // Uniqueness is scoped by org, so one tenant can neither collide with nor + // probe another tenant's keys. + expect(second.body.taskId).not.toBe(first.body.taskId); + }); + + runDb('refuses without the MFA step-up, and admits nothing', async () => { + const f = await seed(); + // `env.token` is the same principal WITHOUT `mfa: true` — the only + // difference from every passing case above. + const res = await post(f.env, bodyFor(f), f.env.token); + + expect(res.status).toBe(403); + expect(res.body.code).toBe('MFA_REQUIRED'); + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + }); + + runDb('refuses a caller holding only ai_agents:read', async () => { + const readOnly = await setupTestEnvironment({ + scope: 'organization', + rolePermissions: [{ resource: 'ai_agents', action: 'read' }], + }); + const agentId = await insertAgent(readOnly.organization.id, readOnly.user.id); + const deviceId = await insertDevice(readOnly.organization.id, readOnly.site.id); + + const res = await post(readOnly, { + mode: 'live', recipeKey: 'service_recovery', recipeVersion: 1, + orgId: readOnly.organization.id, deviceId, + inputs: { serviceName: 'spooler' }, clientIdempotencyKey: `delegate-${randomUUID()}`, + }); + + expect(res.status).toBe(403); + expect(await readTasks(readOnly.organization.id)).toHaveLength(0); + expect(agentId).toBeTruthy(); + }); + + runDb('a device in another org 404s and admits nothing', async () => { + const f = await seed(); + const otherOrg = await createOrganization({ partnerId: f.env.partner.id }); + const otherSite = await createSite({ orgId: otherOrg.id }); + const foreignDevice = await insertDevice(otherOrg.id, otherSite.id); + + const res = await post(f.env, bodyFor(f, { deviceId: foreignDevice })); + + expect(res.status).toBe(404); + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + expect(await readTasks(otherOrg.id)).toHaveLength(0); + }); + + runDb('a device outside the caller\'s allowed sites 404s and admits nothing', async () => { + const f = await seed(); + const otherSite = await createSite({ orgId: f.env.organization.id }); + const offSiteDevice = await insertDevice(f.env.organization.id, otherSite.id); + await restrictUserToSites(f.env, [f.siteId]); + + const res = await post(f.env, bodyFor(f, { deviceId: offSiteDevice })); + + // Non-enumerating: the same 404 a device that does not exist would give. + expect(res.status).toBe(404); + expect(res.body.error).toBe('Device not found'); + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + }); + + runDb('a site-restricted caller may still delegate against a device inside their sites', async () => { + const f = await seed(); + await restrictUserToSites(f.env, [f.siteId]); + // Positive control: without this, the previous test would pass just as + // well if site restriction rejected EVERYTHING. + const res = await post(f.env, bodyFor(f)); + expect(res.status).toBe(202); + }); + + runDb('422s with an actionable reason when the task flag is off', async () => { + const f = await seed(); + process.env.AI_OPERATOR_TASKS_ENABLED = 'false'; + + const res = await post(f.env, bodyFor(f)); + + expect(res.status).toBe(422); + expect(res.body.code).toBe('OPERATOR_TASKS_DISABLED'); + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + }); + + runDb('422s when the recipe flag is off', async () => { + const f = await seed(); + process.env.AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED = 'false'; + + const res = await post(f.env, bodyFor(f)); + + expect(res.status).toBe(422); + expect(res.body.code).toBe('OPERATOR_RECIPE_DISABLED'); + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + }); + + runDb('429s when the org is already at the pending-task cap', async () => { + const f = await seed(); + const agentId = f.agentId; + + // 100 non-terminal tasks, inserted directly — going through the route 100 + // times would prove the same thing far more slowly. + await withDbAccessContext(SYSTEM_CTX, () => + db.insert(aiOperatorTasks).values(Array.from({ length: 100 }, () => ({ + orgId: f.env.organization.id, + agentId, + agentKind: 'triage', + agentName: 'Operator', + workflowKey: 'service_recovery', + workflowVersion: 1, + originKind: 'manual' as const, + requesterUserId: f.env.user.id, + objective: 'filler', + deviceId: f.deviceId, + state: 'waiting' as const, + deadlineAt: new Date(Date.now() + 3_600_000), + })))); + + const res = await post(f.env, bodyFor(f)); + expect(res.status).toBe(429); + expect(res.body.code).toBe('OPERATOR_PENDING_CAP_REACHED'); + expect(await readTasks(f.env.organization.id)).toHaveLength(100); + }); + + runDb('terminal tasks do not count against the pending cap', async () => { + const f = await seed(); + const agentId = f.agentId; + + await withDbAccessContext(SYSTEM_CTX, () => + db.insert(aiOperatorTasks).values(Array.from({ length: 100 }, () => ({ + orgId: f.env.organization.id, + agentId, + agentKind: 'triage', + agentName: 'Operator', + workflowKey: 'service_recovery', + workflowVersion: 1, + originKind: 'manual' as const, + requesterUserId: f.env.user.id, + objective: 'filler', + deviceId: f.deviceId, + state: 'completed' as const, + deadlineAt: new Date(Date.now() + 3_600_000), + })))); + + // Discriminating control for the test above: same 100 rows, terminal + // state, and admission must go through. + const res = await post(f.env, bodyFor(f)); + expect(res.status).toBe(202); + }); + + runDb('rejects a body carrying a forged `task` field and admits nothing', async () => { + const f = await seed(); + const res = await post(f.env, bodyFor(f, { + task: { taskId: randomUUID(), taskStepKey: 'execute', operationKey: 'forged', attemptOrdinal: 0 }, + })); + + expect(res.status).toBe(400); + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + }); + + runDb('rejects a body carrying a forged policy snapshot or approval result', async () => { + const f = await seed(); + for (const forged of [{ policySnapshot: { effective: { mode: 'act' } } }, { approval: { decided: 'approved' } }]) { + const res = await post(f.env, bodyFor(f, forged)); + expect(res.status).toBe(400); + } + expect(await readTasks(f.env.organization.id)).toHaveLength(0); + }); + + runDb('the partial unique index really is partial — internal admissions with a null key never collide', async () => { + const f = await seed(); + const agentId = f.agentId; + + // Two rows with NULL keys must both insert. If the index were declared + // without its WHERE clause, the second would raise 23505 here — which is + // exactly the regression this asserts against. + await withDbAccessContext(SYSTEM_CTX, () => + db.insert(aiOperatorTasks).values([0, 1].map(() => ({ + orgId: f.env.organization.id, + agentId, + agentKind: 'triage', + agentName: 'Operator', + workflowKey: 'service_recovery', + workflowVersion: 1, + originKind: 'manual' as const, + requesterUserId: f.env.user.id, + objective: 'internal admission with no client key', + deviceId: f.deviceId, + state: 'queued' as const, + deadlineAt: new Date(Date.now() + 3_600_000), + })))); + + expect(await readTasks(f.env.organization.id)).toHaveLength(2); + }); + + runDb('the idempotency index exists in the shape the migration declares', async () => { + // Guards the migration itself: a route that relies on ON CONFLICT matching + // a partial index breaks with 42P10, not a wrong answer, if the index + // predicate drifts. + const rows = await withDbAccessContext(SYSTEM_CTX, () => db.execute(sql` + SELECT indexdef FROM pg_indexes + WHERE tablename = 'ai_operator_tasks' + AND indexname = 'ai_operator_tasks_client_idempotency_uq' + `)); + const def = String((rows as unknown as Array<{ indexdef: string }>)[0]?.indexdef ?? ''); + expect(def).toContain('UNIQUE'); + expect(def).toContain('org_id'); + expect(def).toContain('client_idempotency_key'); + expect(def).toContain('WHERE (client_idempotency_key IS NOT NULL)'); + }); +}); diff --git a/apps/api/src/db/schema/aiOperatorTasks.ts b/apps/api/src/db/schema/aiOperatorTasks.ts index 24a646fab6..206afd4831 100644 --- a/apps/api/src/db/schema/aiOperatorTasks.ts +++ b/apps/api/src/db/schema/aiOperatorTasks.ts @@ -196,6 +196,18 @@ export const aiOperatorTasks = pgTable( accountingRootTaskId: uuid('accounting_root_task_id'), successorOfTaskId: uuid('successor_of_task_id'), + // W08 (#5246): the client-supplied admission idempotency key from + // `POST /ai/operator/tasks` (spec §12 — "client idempotency key"). + // Nullable because every pre-W08 row and every internally-admitted task + // has none; uniqueness is a PARTIAL unique index on + // `(org_id, client_idempotency_key) WHERE client_idempotency_key IS NOT + // NULL`, so nulls never collide. This column is what makes a duplicate + // POST return the SAME task instead of dispatching a second restart to a + // customer machine — the guarantee has to be a database constraint, + // because a read-then-insert in the route loses the race between two + // concurrent clicks. + clientIdempotencyKey: text('client_idempotency_key'), + createdAt: timestamp('created_at', { withTimezone: true }).defaultNow().notNull(), updatedAt: timestamp('updated_at', { withTimezone: true }).defaultNow().notNull(), }, @@ -241,6 +253,13 @@ export const aiOperatorTasks = pgTable( queuedWakeIdx: index('ai_operator_tasks_queued_wake_idx') .on(table.nextWakeAt) .where(sql`state = 'queued'`), + + // W08 (#5246): admission idempotency. Partial so the column stays + // nullable for internal admissions; scoped by `org_id` so one tenant + // cannot probe or squat another tenant's keys. + clientIdempotencyUq: uniqueIndex('ai_operator_tasks_client_idempotency_uq') + .on(table.orgId, table.clientIdempotencyKey) + .where(sql`client_idempotency_key IS NOT NULL`), }), ); diff --git a/apps/api/src/routes/aiOperatorTasks.test.ts b/apps/api/src/routes/aiOperatorTasks.test.ts index ecf3b5f7db..69063cd615 100644 --- a/apps/api/src/routes/aiOperatorTasks.test.ts +++ b/apps/api/src/routes/aiOperatorTasks.test.ts @@ -4,10 +4,17 @@ import { PgDialect } from 'drizzle-orm/pg-core'; import type { SQL } from 'drizzle-orm'; import { AI_OPERATOR_TASK_LEAK_TRIPWIRE_KEYS } from '@breeze/shared'; -const { selectMock, hasPermMock, authOkMock } = vi.hoisted(() => ({ +const { + selectMock, hasPermMock, authOkMock, mfaOkMock, + tasksEnabledMock, recipeEnabledMock, admitMock, +} = vi.hoisted(() => ({ selectMock: vi.fn(), hasPermMock: vi.fn<(resource: string, action: string) => boolean>(() => true), authOkMock: vi.fn(() => true), + mfaOkMock: vi.fn(() => true), + tasksEnabledMock: vi.fn(() => true), + recipeEnabledMock: vi.fn(() => true), + admitMock: vi.fn(), })); vi.mock('../middleware/auth', async (importOriginal) => { @@ -22,10 +29,27 @@ vi.mock('../middleware/auth', async (importOriginal) => { c: { json: (body: unknown, status: number) => Response }, next: () => Promise, ) => (hasPermMock(resource, action) ? next() : c.json({ error: 'Permission denied' }, 403)), + // W08: the POST admission route is MFA step-up gated, same as + // `POST /ai/agents/:id/runs`. Mocked with the real middleware's own + // failure shape so a test can prove the gate is wired, not just present. + requireMfa: () => async ( + c: { json: (body: unknown, status: number) => Response }, + next: () => Promise, + ) => (mfaOkMock() ? next() : c.json({ error: 'MFA required', code: 'MFA_REQUIRED' }, 403)), buildOrgAccessClosures: actual.buildOrgAccessClosures, }; }); +vi.mock('../config/env', async (importOriginal) => ({ + ...(await importOriginal>()), + aiOperatorTasksEnabled: () => tasksEnabledMock(), + aiOperatorServiceRecoveryEnabled: () => recipeEnabledMock(), +})); + +vi.mock('../services/aiOperator/taskService', () => ({ + admitServiceRecoveryTask: (input: unknown) => admitMock(input), +})); + vi.mock('../db', () => ({ db: { select: selectMock }, })); @@ -305,3 +329,260 @@ describe('GET /ai/operator/tasks/:id (detail)', () => { expect(sqlText(capturedPredicate)).toContain('site_id'); }); }); + +/** + * W08 (#5246) — `POST /ai/operator/tasks`, the only route that creates a task. + * + * Every case below is a REFUSAL contract except the 202s, because the whole + * value of this route is what it declines to admit: each accepted task can + * dispatch a real service restart to a customer machine. + */ +describe('POST /ai/operator/tasks (W08 admission)', () => { + const SITE_ID = '99999999-9999-4999-8999-999999999999'; + const ADMITTED_TASK_ID = '77777777-7777-4777-8777-777777777777'; + + function body(overrides: Record = {}) { + return { + mode: 'live', + recipeKey: 'service_recovery', + recipeVersion: 1, + orgId: ORG_ID, + deviceId: DEVICE_ID, + inputs: { serviceName: 'spooler' }, + clientIdempotencyKey: 'delegate-abcdef0123456789', + ...overrides, + }; + } + + function post(payload: unknown, authOverrides: Record = {}) { + return buildApp(authOverrides).request('/ai/operator/tasks', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(payload), + }); + } + + const deviceRow = { id: DEVICE_ID, orgId: ORG_ID, siteId: SITE_ID, hostname: 'WS-01' }; + + /** device row, then agent row, then the pending-cap count. */ + function happyPathSelects(pendingCount = 0) { + selectMock.mockReturnValueOnce(selectChain([deviceRow])); + selectMock.mockReturnValueOnce(selectChain([{ id: AGENT_ID }])); + selectMock.mockReturnValueOnce(selectChain([{ count: pendingCount }])); + } + + beforeEach(() => { + selectMock.mockClear(); + admitMock.mockClear(); + mfaOkMock.mockReturnValue(true); + hasPermMock.mockReturnValue(true); + tasksEnabledMock.mockReturnValue(true); + recipeEnabledMock.mockReturnValue(true); + admitMock.mockResolvedValue({ ok: true, taskId: ADMITTED_TASK_ID, replayed: false }); + }); + + it('admits a valid request with 202 and the new task id', async () => { + happyPathSelects(); + const res = await post(body()); + expect(res.status).toBe(202); + expect(await res.json()).toEqual({ taskId: ADMITTED_TASK_ID, replayed: false }); + }); + + it('passes the client idempotency key and the resolved agent through to admission', async () => { + happyPathSelects(); + await post(body()); + expect(admitMock).toHaveBeenCalledWith(expect.objectContaining({ + orgId: ORG_ID, + agentId: AGENT_ID, + clientIdempotencyKey: 'delegate-abcdef0123456789', + requesterUserId: USER_ID, + recipeInput: expect.objectContaining({ deviceId: DEVICE_ID, serviceName: 'spooler' }), + })); + }); + + it('returns the SAME task id with 202 on an idempotent replay', async () => { + happyPathSelects(); + admitMock.mockResolvedValue({ ok: true, taskId: ADMITTED_TASK_ID, replayed: true }); + const res = await post(body()); + expect(res.status).toBe(202); + // Identical id to the first admission: a replay is indistinguishable to + // the client, which is what stops a double-click becoming two restarts. + expect(await res.json()).toEqual({ taskId: ADMITTED_TASK_ID, replayed: true }); + }); + + it('records origin `alert` and threads the alert id into the verification criterion', async () => { + happyPathSelects(); + const alertId = '88888888-8888-4888-8888-888888888888'; + await post(body({ sourceKind: 'alert', sourceId: alertId })); + expect(admitMock).toHaveBeenCalledWith(expect.objectContaining({ + originKind: 'alert', + recipeInput: expect.objectContaining({ triggeringAlertId: alertId }), + })); + }); + + it('records origin `manual` and a null alert id for a device-page delegate', async () => { + happyPathSelects(); + await post(body({ sourceKind: 'device', sourceId: DEVICE_ID })); + expect(admitMock).toHaveBeenCalledWith(expect.objectContaining({ + originKind: 'manual', + recipeInput: expect.objectContaining({ triggeringAlertId: null }), + })); + }); + + // ---- Spec §12: "Requests cannot supply a principal, effective policy, + // approval result, or trusted continuation token." ---- + + it.each(['task', 'policySnapshot', 'approval', 'principal', 'agentId'])( + 'rejects a body carrying a forged `%s` field before any admission', + async (field) => { + const res = await post(body({ [field]: { forged: true } })); + expect(res.status).toBe(400); + expect(admitMock).not.toHaveBeenCalled(); + }, + ); + + it('rejects a trial mode through the live admission route', async () => { + const res = await post(body({ mode: 'trial' })); + expect(res.status).toBe(400); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('rejects a request with no idempotency key', async () => { + const payload = body() as Record; + delete payload.clientIdempotencyKey; + const res = await post(payload); + expect(res.status).toBe(400); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('rejects a source id with no source kind', async () => { + const res = await post(body({ sourceId: DEVICE_ID })); + expect(res.status).toBe(400); + }); + + // ---- RBAC / MFA (spec §5.1) ---- + + it('requires ai_agents:write, not merely ai_agents:read', async () => { + hasPermMock.mockImplementation((_r, action) => action !== 'write'); + const res = await post(body()); + expect(res.status).toBe(403); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('requires the MFA step-up', async () => { + mfaOkMock.mockReturnValue(false); + const res = await post(body()); + expect(res.status).toBe(403); + expect(await res.json()).toMatchObject({ code: 'MFA_REQUIRED' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + // ---- Target authorization: non-enumerating 404 (spec §12) ---- + + it('404s without touching the database when the org is outside the caller access', async () => { + const res = await post(body({ orgId: OTHER_ORG_ID }), { canAccessOrg: () => false }); + expect(res.status).toBe(404); + expect(selectMock).not.toHaveBeenCalled(); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('404s when the device does not resolve inside the named org', async () => { + selectMock.mockReturnValueOnce(selectChain([])); + const res = await post(body()); + expect(res.status).toBe(404); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('404s (not 403) for a device outside a site-restricted caller sites', async () => { + selectMock.mockReturnValueOnce(selectChain([ + { ...deviceRow, siteId: 'aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa' }, + ])); + const res = await post(body(), { + allowedSiteIds: [SITE_ID], + canAccessSite: (id: string | null) => id === SITE_ID, + }); + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ error: 'Device not found' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + // ---- Readiness, recomputed on launch (spec §12) ---- + + it('422s with an actionable reason when the task infrastructure flag is off', async () => { + tasksEnabledMock.mockReturnValue(false); + selectMock.mockReturnValueOnce(selectChain([deviceRow])); + const res = await post(body()); + expect(res.status).toBe(422); + expect(await res.json()).toMatchObject({ code: 'OPERATOR_TASKS_DISABLED' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('422s when the service-recovery recipe flag is off', async () => { + recipeEnabledMock.mockReturnValue(false); + selectMock.mockReturnValueOnce(selectChain([deviceRow])); + const res = await post(body()); + expect(res.status).toBe(422); + expect(await res.json()).toMatchObject({ code: 'OPERATOR_RECIPE_DISABLED' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('422s rather than silently upgrading a stale reviewed recipe version', async () => { + selectMock.mockReturnValueOnce(selectChain([deviceRow])); + const res = await post(body({ recipeVersion: 99 })); + expect(res.status).toBe(422); + expect(await res.json()).toMatchObject({ code: 'OPERATOR_RECIPE_VERSION_MISMATCH' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('422s when the org has no enabled agent to run the task', async () => { + selectMock.mockReturnValueOnce(selectChain([deviceRow])); + selectMock.mockReturnValueOnce(selectChain([])); + const res = await post(body()); + expect(res.status).toBe(422); + expect(await res.json()).toMatchObject({ code: 'OPERATOR_NO_AGENT' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('surfaces an admission refusal as 422 with its reason', async () => { + happyPathSelects(); + admitMock.mockResolvedValue({ ok: false, refusal: 'invalid_input', detail: 'serviceName is required' }); + const res = await post(body()); + expect(res.status).toBe(422); + expect(await res.json()).toMatchObject({ error: 'serviceName is required', code: 'INVALID_INPUT' }); + }); + + it('keeps an admission-time device refusal non-enumerating (404, not 422)', async () => { + happyPathSelects(); + admitMock.mockResolvedValue({ ok: false, refusal: 'device_not_in_org', detail: 'device x is not in org y' }); + const res = await post(body()); + expect(res.status).toBe(404); + expect(await res.json()).toEqual({ error: 'Device not found' }); + }); + + // ---- Capacity (spec §7.2: pending cap 100 per org) ---- + + it('429s when the org is already at the pending-task cap', async () => { + happyPathSelects(100); + const res = await post(body()); + expect(res.status).toBe(429); + expect(await res.json()).toMatchObject({ code: 'OPERATOR_PENDING_CAP_REACHED' }); + expect(admitMock).not.toHaveBeenCalled(); + }); + + it('admits at one below the cap', async () => { + happyPathSelects(99); + const res = await post(body()); + expect(res.status).toBe(202); + }); + + it('counts the cap over non-terminal states for the ONE named org', async () => { + let capturedPredicate: unknown; + selectMock.mockReturnValueOnce(selectChain([deviceRow])); + selectMock.mockReturnValueOnce(selectChain([{ id: AGENT_ID }])); + selectMock.mockReturnValueOnce(selectChain([{ count: 0 }], (p) => { capturedPredicate = p; })); + await post(body()); + const text = sqlText(capturedPredicate).toLowerCase(); + expect(text).toContain('org_id'); + expect(text).toContain('not in'); + }); +}); diff --git a/apps/api/src/routes/aiOperatorTasks.ts b/apps/api/src/routes/aiOperatorTasks.ts index 304374e12f..32d0cb3708 100644 --- a/apps/api/src/routes/aiOperatorTasks.ts +++ b/apps/api/src/routes/aiOperatorTasks.ts @@ -1,8 +1,12 @@ /** - * Wave W07 of #5205 (P3-1e, read side) — `GET /ai/operator/tasks` (org-scoped - * keyset list) and `GET /ai/operator/tasks/:id` (detail). Read-only: no - * POST/PUT/PATCH/DELETE here — admission, answers, pause/resume/cancel, and - * the delegate action are later waves (W06/W08). + * The Operator task HTTP surface for #5205. + * + * W07 (P3-1e) shipped the read side — `GET /ai/operator/tasks` (org-scoped + * keyset list) and `GET /ai/operator/tasks/:id` (detail). + * + * W08 (P3-1f, #5246) adds the ONE write: `POST /ai/operator/tasks`, the only + * door through which a human creates a task. Answers, pause/resume/cancel and + * retry are still later waves (P3-5). * * Mounted at `/api/v1/ai/operator` — a separate route module from the * already-large `aiAgentsRoutes`, per spec §12 ("Add routes under @@ -33,15 +37,23 @@ import { Hono, type Context } from 'hono'; import { z } from 'zod'; import { and, desc, eq, inArray, isNull, or, sql, type SQL } from 'drizzle-orm'; import { + createOperatorTaskSchema, operatorTaskListQuerySchema, type AiOperatorTaskDto, type AiOperatorTaskListItemDto, } from '@breeze/shared'; import { zValidator } from '../lib/validation'; import { db } from '../db'; -import { aiAgentRuns, aiOperatorOperations, aiOperatorTasks, devices } from '../db/schema'; -import { authMiddleware, requirePermission, requireScope } from '../middleware/auth'; +import { aiAgents, aiAgentRuns, aiOperatorOperations, aiOperatorTasks, devices } from '../db/schema'; +import { authMiddleware, requireMfa, requirePermission, requireScope } from '../middleware/auth'; import { PERMISSIONS } from '../services/permissions'; +import { aiOperatorServiceRecoveryEnabled, aiOperatorTasksEnabled } from '../config/env'; +import { admitServiceRecoveryTask } from '../services/aiOperator/taskService'; +import { + SERVICE_RECOVERY_WORKFLOW_KEY, + SERVICE_RECOVERY_WORKFLOW_VERSION, +} from '../services/aiOperator/recipes/serviceRecovery'; +import { TERMINAL_TASK_STATES } from '../services/aiOperator/taskTransitions'; import { mapOperatorTask, mapOperatorTaskListItem, @@ -62,6 +74,10 @@ aiOperatorTasksRoutes.use('*', authMiddleware); // Same capability as task inspection everywhere else in the AI surface (spec // §5.1) — no new permission minted for a read-only view. const requireAiRead = requirePermission(PERMISSIONS.AI_AGENTS_READ.resource, PERMISSIONS.AI_AGENTS_READ.action); +// W08: launching a task is a WRITE — spec §5.1, "ai_agents:write plus MFA for +// interactive launch". Same pair `POST /ai/agents/:id/runs` carries, because +// delegating a task is at least as consequential as triggering a single run. +const requireAiWrite = requirePermission(PERMISSIONS.AI_AGENTS_WRITE.resource, PERMISSIONS.AI_AGENTS_WRITE.action); const scopes = requireScope('organization', 'partner', 'system'); const UUID = z.string().guid(); @@ -129,6 +145,185 @@ function siteVisibilityCondition(allowedSiteIds: string[] | undefined): SQL | un ); } +/** + * Spec §7.2: "Proposed pending cap is 100 per org; admission returns a visible + * capacity result when full." Pending = every non-terminal state, because a + * `paused` or `waiting` task still holds a deadline, a device target and a + * reconciler obligation — it is exactly the resource the cap bounds. A + * terminal task holds none of those. + */ +const OPERATOR_PENDING_TASK_CAP_PER_ORG = 100; + +/** + * `POST /api/v1/ai/operator/tasks` — the only door that creates a task + * (#5205 W08, #5246; spec §12's `POST /tasks` row). + * + * ORDER OF CHECKS IS THE CONTRACT, not a style choice: + * + * 1. `.strict()` body (400). Spec §12: "Requests cannot supply a principal, + * effective policy, approval result, or trusted continuation token." A + * body carrying `task`, `policySnapshot` or `approval` is rejected here, + * before anything reads it. + * 2. Org access (non-enumerating 404). The body names an explicit `orgId`; + * a caller without access to it learns nothing about whether it exists. + * 3. Device resolution + site access (non-enumerating 404) — the SAME + * posture the W07 read routes take, so delegating cannot be used to probe + * for devices a read cannot see. + * 4. Readiness recompute (422). Spec §12: "Recompute readiness on launch... + * A cached catalog or draft is never authority." Flags and recipe version + * are re-read here even though `admitServiceRecoveryTask` checks the flags + * again — the second check is the one that actually gates the insert; this + * one exists to produce an ACTIONABLE reason instead of a bare refusal. + * 5. Pending cap (429). + * 6. Admission, which is idempotent on `clientIdempotencyKey`. + * + * Returns 202 (not 201): admission commits a `queued` task with + * `next_wake_at = now`, and the coordinator's `queued_past_wake` scan — not + * Redis — is what picks it up, so the acceptance is durable the instant the + * row commits even with Redis down. That is spec §12's "once task + outbox + * commit, 202 is truthful", satisfied by a stronger mechanism than an outbox + * row: W06 deliberately writes NO outbox row at admission, because a queued + * task has no authoritative source row to re-derive a wake from. + */ +aiOperatorTasksRoutes.post( + '/tasks', + scopes, + requireAiWrite, + requireMfa(), + zValidator('json', createOperatorTaskSchema), + async (c) => { + const auth = c.get('auth'); + const body = c.req.valid('json'); + + // (2) Explicit org from the body. `canAccessOrg` is the same closure the + // org-scoping predicate is built from, so this cannot drift from what a + // read would allow. 404 rather than 403: confirming the org exists is + // itself a cross-tenant disclosure. + if (!auth.canAccessOrg(body.orgId)) { + return c.json({ error: 'Device not found' }, 404); + } + + // (3) Resolve the target under the caller's own visibility. Note this + // read runs in the REQUEST's db context (RLS-scoped), unlike admission, + // which runs as system — so the device must be visible to the caller + // before any system-scoped work happens on their behalf. + const [device] = await db + .select({ id: devices.id, orgId: devices.orgId, siteId: devices.siteId, hostname: devices.hostname }) + .from(devices) + .where(and(eq(devices.id, body.deviceId), eq(devices.orgId, body.orgId))) + .limit(1); + // A site-restricted caller may not delegate against a device outside their + // sites. Same non-enumerating 404 as the W07 detail route. + if (!device || (auth.canAccessSite && !auth.canAccessSite(device.siteId))) { + return c.json({ error: 'Device not found' }, 404); + } + + // (4) Readiness, recomputed now. + if (!aiOperatorTasksEnabled()) { + return c.json({ + error: 'AI Operator tasks are not enabled for this deployment', + code: 'OPERATOR_TASKS_DISABLED', + }, 422); + } + if (!aiOperatorServiceRecoveryEnabled()) { + return c.json({ + error: 'The service recovery workflow is not enabled for this deployment', + code: 'OPERATOR_RECIPE_DISABLED', + }, 422); + } + if (body.recipeVersion !== SERVICE_RECOVERY_WORKFLOW_VERSION) { + return c.json({ + error: `Workflow ${SERVICE_RECOVERY_WORKFLOW_KEY} is at version ${SERVICE_RECOVERY_WORKFLOW_VERSION}; ` + + `this request reviewed version ${body.recipeVersion}. Reload and review the current workflow.`, + code: 'OPERATOR_RECIPE_VERSION_MISMATCH', + }, 422); + } + + // The SERVER picks the agent (spec §5.1 — the request cannot name a + // principal). Enabled agents visible to this org, org-owned preferred over + // partner-wide, then oldest first so the choice is deterministic and a + // replay resolves the same way. + const [agent] = await db + .select({ id: aiAgents.id }) + .from(aiAgents) + .where( + and( + eq(aiAgents.enabled, true), + or(eq(aiAgents.orgId, body.orgId), isNull(aiAgents.orgId)), + ), + ) + .orderBy(sql`${aiAgents.orgId} IS NULL`, aiAgents.createdAt, aiAgents.id) + .limit(1); + if (!agent) { + return c.json({ + error: 'No enabled AI agent is available for this organization. ' + + 'Enable an agent in Settings → AI Agents before delegating a task.', + code: 'OPERATOR_NO_AGENT', + }, 422); + } + + // (5) Capacity. Counted over non-terminal states for this ONE org (not the + // caller's whole accessible set) — the cap is a per-tenant resource bound, + // so a partner-scope caller must not be able to exhaust one org's quota + // faster because they can see many. + const [pending] = await db + .select({ count: sql`count(*)::int` }) + .from(aiOperatorTasks) + .where( + and( + eq(aiOperatorTasks.orgId, body.orgId), + sql`${aiOperatorTasks.state} NOT IN ${TERMINAL_TASK_STATES}`, + ), + ); + if ((pending?.count ?? 0) >= OPERATOR_PENDING_TASK_CAP_PER_ORG) { + return c.json({ + error: `This organization already has ${OPERATOR_PENDING_TASK_CAP_PER_ORG} unfinished Operator tasks. ` + + 'Wait for tasks to finish, or stop ones that are no longer needed, before delegating more.', + code: 'OPERATOR_PENDING_CAP_REACHED', + }, 429); + } + + // (6) Admit. The requester's authorized ceiling is what was just checked + // above (org + site + write permission + MFA); `requesterUserId` records + // WHOSE ceiling it is, which is what spec §5.1's "loss of that access + // pauses delegated execution" is later evaluated against. + const result = await admitServiceRecoveryTask({ + orgId: body.orgId, + agentId: agent.id, + objective: `Restore the ${body.inputs.serviceName} service on ${device.hostname ?? body.deviceId}`, + // Provenance, not authority (spec §5.1: origins are explicit and never + // silently converted). A delegate from alert detail is origin 'alert'; + // everything else through this route is a manual delegation. + originKind: body.sourceKind === 'alert' ? 'alert' : 'manual', + requesterUserId: auth.user?.id ?? null, + recipeInput: { + deviceId: body.deviceId, + serviceName: body.inputs.serviceName, + // The alert is the recurrence signal the verification criterion needs + // (W06: with no alertId the best achievable outcome is + // `investigation_complete`, never `verified_resolved`). + triggeringAlertId: body.sourceKind === 'alert' ? body.sourceId ?? null : null, + }, + clientIdempotencyKey: body.clientIdempotencyKey, + }); + + if (!result.ok) { + // Every refusal here is a readiness/scope problem, not a client format + // error: 422 per spec §12 ("422 for unsupported workflow/criteria/ + // setup"), except the two target refusals, which stay non-enumerating. + if (result.refusal === 'device_not_in_org') { + return c.json({ error: 'Device not found' }, 404); + } + return c.json({ error: result.detail, code: result.refusal.toUpperCase() }, 422); + } + + // A replay is a success, not a conflict: the caller asked for a task with + // this key and there is one. Same 202 and the SAME id — the client cannot + // tell the two apart, which is the whole point of idempotency. + return c.json({ taskId: result.taskId, replayed: result.replayed }, 202); + }, +); + /** * Org-wide keyset-paginated task list — every task the caller's accessible * orgs admitted, newest-created first. Optional `deviceId`/`state` filters diff --git a/apps/api/src/routes/config.test.ts b/apps/api/src/routes/config.test.ts index 1f50fa7029..1593a980af 100644 --- a/apps/api/src/routes/config.test.ts +++ b/apps/api/src/routes/config.test.ts @@ -75,14 +75,43 @@ describe('GET /config', () => { it('returns both flags false when BREEZE_BILLING_URL unset', async () => { const { status, body } = await request(); expect(status).toBe(200); - expect(body.features).toEqual({ billing: false, support: false }); + expect(body.features).toEqual({ billing: false, support: false, aiOperatorTasks: false }); }); it('returns both flags true when BREEZE_BILLING_URL is set', async () => { process.env.BREEZE_BILLING_URL = 'http://localhost:4000'; const { status, body } = await request(); expect(status).toBe(200); - expect(body.features).toEqual({ billing: true, support: true }); + expect(body.features).toEqual({ billing: true, support: true, aiOperatorTasks: false }); + }); + + it('features.aiOperatorTasks is false when neither AI Operator env var is set', async () => { + const { body } = await request(); + expect(body.features.aiOperatorTasks).toBe(false); + }); + + it('features.aiOperatorTasks is false when only AI_OPERATOR_TASKS_ENABLED is set', async () => { + vi.stubEnv('AI_OPERATOR_TASKS_ENABLED', 'true'); + vi.stubEnv('AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED', 'false'); + const { body } = await request(); + expect(body.features.aiOperatorTasks).toBe(false); + vi.unstubAllEnvs(); + }); + + it('features.aiOperatorTasks is false when only AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED is set', async () => { + vi.stubEnv('AI_OPERATOR_TASKS_ENABLED', 'false'); + vi.stubEnv('AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED', 'true'); + const { body } = await request(); + expect(body.features.aiOperatorTasks).toBe(false); + vi.unstubAllEnvs(); + }); + + it('features.aiOperatorTasks is true when both AI Operator env vars are set', async () => { + vi.stubEnv('AI_OPERATOR_TASKS_ENABLED', 'true'); + vi.stubEnv('AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED', 'true'); + const { body } = await request(); + expect(body.features.aiOperatorTasks).toBe(true); + vi.unstubAllEnvs(); }); it('registration.enabled defaults to false when ENABLE_REGISTRATION unset', async () => { diff --git a/apps/api/src/routes/config.ts b/apps/api/src/routes/config.ts index 5cc48caf58..4fcb2c9763 100644 --- a/apps/api/src/routes/config.ts +++ b/apps/api/src/routes/config.ts @@ -1,7 +1,7 @@ import { Hono } from 'hono'; import { zValidator } from '../lib/validation'; import { z } from 'zod'; -import { cfAccessTrustEnabled } from '../config/env'; +import { aiOperatorServiceRecoveryEnabled, aiOperatorTasksEnabled, cfAccessTrustEnabled } from '../config/env'; import { envFlag } from '../utils/envFlag'; import { isS3Configured } from '../services/s3Storage'; import { authMiddleware, requireScope, type AuthContext } from '../middleware/auth'; @@ -22,6 +22,10 @@ configRoutes.get('/', (c) => { features: { billing: hasExternalServices, support: hasExternalServices, + // W08 of #5205 (#5246) — gates the "Delegate to Operator" action; AND of + // `AI_OPERATOR_TASKS_ENABLED` and `AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED`, + // both default off (decision D2: internal/test orgs only). + aiOperatorTasks: aiOperatorTasksEnabled() && aiOperatorServiceRecoveryEnabled(), }, cfAccessLogin: { enabled: cfAccessTrustEnabled(), diff --git a/apps/api/src/services/aiOperator/taskService.ts b/apps/api/src/services/aiOperator/taskService.ts index e6274f923f..c6b610324f 100644 --- a/apps/api/src/services/aiOperator/taskService.ts +++ b/apps/api/src/services/aiOperator/taskService.ts @@ -27,7 +27,7 @@ */ import { randomUUID } from 'node:crypto'; -import { and, eq } from 'drizzle-orm'; +import { and, eq, sql } from 'drizzle-orm'; import { db, runOutsideDbContext, withSystemDbAccessContext } from '../../db'; import { aiOperatorTasks } from '../../db/schema/aiOperatorTasks'; import { aiAgents } from '../../db/schema/aiAgents'; @@ -56,7 +56,13 @@ export type AdmitTaskRefusal = | 'invalid_input'; export type AdmitTaskResult = - | { ok: true; taskId: string } + /** + * `replayed` is true when `clientIdempotencyKey` matched a task this org had + * already admitted, so NOTHING was created by this call (W08, #5246). The + * caller must still answer 202 with this id — a replay is a success, not a + * conflict — but it must not treat the call as having produced new work. + */ + | { ok: true; taskId: string; replayed: boolean } | { ok: false; refusal: AdmitTaskRefusal; detail: string }; export interface AdmitServiceRecoveryTaskInput { @@ -69,6 +75,17 @@ export interface AdmitServiceRecoveryTaskInput { /** Override for tests; defaults to the recipe's own bound. */ deadlineMs?: number; now?: Date; + /** + * Client-supplied admission idempotency key (spec §12), W08 (#5246). + * + * Unique per org via the PARTIAL unique index + * `ai_operator_tasks_client_idempotency_uq`. Re-admitting with a key this + * org has already used returns that task's id and inserts nothing, which is + * what stops a double-clicked "Delegate to Operator" from dispatching two + * service restarts to the same machine. Null (the default) means "no + * idempotency" and is what every internal admission passes. + */ + clientIdempotencyKey?: string | null; } /** @@ -159,7 +176,11 @@ export async function admitServiceRecoveryTask( const taskId = randomUUID(); const deadlineMs = input.deadlineMs ?? SERVICE_RECOVERY_BOUNDS.deadlineMs; - await db.insert(aiOperatorTasks).values({ + const clientIdempotencyKey = input.clientIdempotencyKey ?? null; + + const inserted = await db + .insert(aiOperatorTasks) + .values({ id: taskId, orgId: input.orgId, agentId: agent.id, @@ -191,9 +212,51 @@ export async function admitServiceRecoveryTask( // The root of its own accounting tree (spec §6.1: "root has no root // pointer"), left null rather than self-referencing. accountingRootTaskId: null, - }); + clientIdempotencyKey, + }) + // W08 (#5246). `DO NOTHING` rather than catching 23505: a unique + // violation ABORTS the surrounding transaction, so the read-back + // needed to answer with the winner's id could not run in it. Letting + // Postgres swallow the conflict keeps the transaction alive and makes + // the replay read a plain follow-up statement. The conflict target + // must repeat the index's WHERE clause, or Postgres cannot match the + // PARTIAL index and raises 42P10 instead of deduplicating. + .onConflictDoNothing({ + target: [aiOperatorTasks.orgId, aiOperatorTasks.clientIdempotencyKey], + where: sql`client_idempotency_key IS NOT NULL`, + }) + .returning({ id: aiOperatorTasks.id }); + + if (inserted.length > 0) { + return { ok: true as const, taskId, replayed: false }; + } + + // Nothing inserted => the partial unique index rejected it, which can + // only happen when this org already holds a task under this key. Read + // the winner. Scoped by BOTH org and key: the index is org-scoped, and + // a key-only lookup would hand one tenant another tenant's task id. + const [existing] = await db + .select({ id: aiOperatorTasks.id }) + .from(aiOperatorTasks) + .where( + and( + eq(aiOperatorTasks.orgId, input.orgId), + eq(aiOperatorTasks.clientIdempotencyKey, clientIdempotencyKey as string), + ), + ) + .limit(1); + + if (!existing) { + // Structurally unreachable: DO NOTHING with a non-null key fires only + // on that index. Refusing beats inventing a task id. + return { + ok: false as const, + refusal: 'invalid_input' as const, + detail: 'admission conflicted but no existing task was found for the idempotency key', + }; + } - return { ok: true as const, taskId }; + return { ok: true as const, taskId: existing.id, replayed: true }; })); } diff --git a/apps/api/src/services/tenantExportPolicyRegistry.ts b/apps/api/src/services/tenantExportPolicyRegistry.ts index 0589cbc2bb..3396eec711 100644 --- a/apps/api/src/services/tenantExportPolicyRegistry.ts +++ b/apps/api/src/services/tenantExportPolicyRegistry.ts @@ -89,7 +89,7 @@ export const CORE_TENANT_EXPORT_POLICY: TenantExportPolicyRegistry = { "ai_operator_task_outbox": tablePolicy("org_id", {"included":["id","org_id","task_id","source_kind","source_id","transition_seq","due_at","published_at","attempts","created_at"],"reviewedIncluded":[],"excludedSensitive":[],"excludedOpen":[]}), // lease_owner is a coordinator instance label, not credential material, but // it does not trip SUSPICIOUS_NAME_PARTS either — plain `included`. - "ai_operator_tasks": tablePolicy("org_id", {"included":["id","org_id","agent_id","agent_kind","agent_name","workflow_key","workflow_version","mode","origin_kind","requester_user_id","objective","device_id","target_label","target_detached_at","target_detached_reason","state","phase","wait_reason","wait_dependency_kind","wait_dependency_id","revision","lease_epoch","lease_owner","lease_expires_at","attempt_ordinal","current_step_key","deadline_at","next_wake_at","outcome","outcome_detail","handoff_summary","accounting_root_task_id","successor_of_task_id","created_at","updated_at"],"reviewedIncluded":[],"excludedSensitive":[],"excludedOpen":["checkpoint"]}), + "ai_operator_tasks": tablePolicy("org_id", {"included":["id","org_id","agent_id","agent_kind","agent_name","workflow_key","workflow_version","mode","origin_kind","requester_user_id","objective","device_id","target_label","target_detached_at","target_detached_reason","state","phase","wait_reason","wait_dependency_kind","wait_dependency_id","revision","lease_epoch","lease_owner","lease_expires_at","attempt_ordinal","current_step_key","deadline_at","next_wake_at","outcome","outcome_detail","handoff_summary","accounting_root_task_id","successor_of_task_id","client_idempotency_key","created_at","updated_at"],"reviewedIncluded":[],"excludedSensitive":[],"excludedOpen":["checkpoint"]}), "ai_screenshots": tablePolicy("org_id", {"included":["id","device_id","org_id","session_id","storage_key","width","height","size_bytes","captured_by","reason","expires_at","created_at"],"reviewedIncluded":[],"excludedSensitive":[],"excludedOpen":[]}), "ai_sessions": tablePolicy("org_id", {"included":["id","org_id","user_id","device_id","status","type","title","model","system_prompt","billing_source","catalog_entry_id","catalog_revision_id","total_cost_cents","turn_count","max_turns","sdk_session_id","last_activity_at","created_at","updated_at","flagged_at","flagged_by","flag_reason","delegant_m365_connection_id","client_user_id","workbook_name","agent_id"],"reviewedIncluded":["total_input_tokens","total_output_tokens"],"excludedSensitive":[],"excludedOpen":["context_snapshot"]}), "ai_unattended_exposure": tablePolicy("org_id", {"included":["id","org_id","partner_id","agent_id","run_id","device_id","intent_id","source","reserved_at"],"reviewedIncluded":[],"excludedSensitive":[],"excludedOpen":[]}), diff --git a/apps/web/src/components/auth/PartnerRegisterPage.test.tsx b/apps/web/src/components/auth/PartnerRegisterPage.test.tsx index a32a301185..4026063207 100644 --- a/apps/web/src/components/auth/PartnerRegisterPage.test.tsx +++ b/apps/web/src/components/auth/PartnerRegisterPage.test.tsx @@ -31,7 +31,7 @@ const mockNavigateTo = vi.mocked(navigateTo); // "loaded + enabled" so the form renders; the disabled path has its own test. function setRegistration(enabled: boolean, loaded = true) { useFeaturesStore.setState({ - features: { billing: false, support: false }, + features: { billing: false, support: false, aiOperatorTasks: false }, cfAccessLogin: { enabled: false }, registration: { enabled }, loaded, diff --git a/apps/web/src/stores/featuresStore.test.ts b/apps/web/src/stores/featuresStore.test.ts index f47d37ae04..fdbd5cc3de 100644 --- a/apps/web/src/stores/featuresStore.test.ts +++ b/apps/web/src/stores/featuresStore.test.ts @@ -16,7 +16,7 @@ describe('featuresStore', () => { beforeEach(() => { vi.clearAllMocks(); useFeaturesStore.setState({ - features: { billing: false, support: false }, + features: { billing: false, support: false, aiOperatorTasks: false }, cfAccessLogin: { enabled: false }, registration: { enabled: false }, softwarePackages: { uploadsEnabled: true }, @@ -52,7 +52,7 @@ describe('featuresStore', () => { res({ features: { billing: true, support: true } }) ); await useFeaturesStore.getState().load(); - expect(useFeaturesStore.getState().features).toEqual({ billing: true, support: true }); + expect(useFeaturesStore.getState().features).toEqual({ billing: true, support: true, aiOperatorTasks: false }); expect(useFeaturesStore.getState().loaded).toBe(true); }); @@ -72,7 +72,7 @@ describe('featuresStore', () => { const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); fetchMock.mockRejectedValueOnce(new Error('network')); await useFeaturesStore.getState().load(); - expect(useFeaturesStore.getState().features).toEqual({ billing: false, support: false }); + expect(useFeaturesStore.getState().features).toEqual({ billing: false, support: false, aiOperatorTasks: false }); expect(useFeaturesStore.getState().loaded).toBe(true); expect(errSpy).toHaveBeenCalled(); errSpy.mockRestore(); @@ -82,7 +82,7 @@ describe('featuresStore', () => { const errSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); fetchMock.mockResolvedValueOnce(res({}, false, 500)); await useFeaturesStore.getState().load(); - expect(useFeaturesStore.getState().features).toEqual({ billing: false, support: false }); + expect(useFeaturesStore.getState().features).toEqual({ billing: false, support: false, aiOperatorTasks: false }); expect(useFeaturesStore.getState().loaded).toBe(true); expect(errSpy).toHaveBeenCalled(); errSpy.mockRestore(); @@ -98,6 +98,6 @@ describe('featuresStore', () => { it('coerces missing fields to false', async () => { fetchMock.mockResolvedValueOnce(res({})); await useFeaturesStore.getState().load(); - expect(useFeaturesStore.getState().features).toEqual({ billing: false, support: false }); + expect(useFeaturesStore.getState().features).toEqual({ billing: false, support: false, aiOperatorTasks: false }); }); }); diff --git a/apps/web/src/stores/featuresStore.ts b/apps/web/src/stores/featuresStore.ts index 0a62b49952..ba6fb79155 100644 --- a/apps/web/src/stores/featuresStore.ts +++ b/apps/web/src/stores/featuresStore.ts @@ -5,6 +5,7 @@ import { fetchWithAuth } from './auth'; export interface Features { billing: boolean; support: boolean; + aiOperatorTasks: boolean; } export interface CfAccessLoginConfig { @@ -28,7 +29,12 @@ interface FeaturesState { load: () => Promise; } -const DEFAULT_FEATURES: Features = { billing: false, support: false }; +// aiOperatorTasks default CLOSED: this gates a write action that starts +// autonomous remediation on a customer machine, and the underlying server +// flags are off by default (decision D2). An unreachable or older /config +// (missing the field) must hide the "Delegate to Operator" button, never +// show it. +const DEFAULT_FEATURES: Features = { billing: false, support: false, aiOperatorTasks: false }; const DEFAULT_CF_ACCESS: CfAccessLoginConfig = { enabled: false }; // Default closed: until /config confirms registration is open we hide the // registration UI rather than flash a link that may be disabled (#1308). @@ -63,6 +69,7 @@ export const useFeaturesStore = create()((set, get) => ({ features: { billing: !!data.features?.billing, support: !!data.features?.support, + aiOperatorTasks: !!data.features?.aiOperatorTasks, }, cfAccessLogin: { enabled: !!data.cfAccessLogin?.enabled, @@ -101,6 +108,20 @@ export function useRegistrationGate(): { enabled: boolean; loaded: boolean } { return { enabled, loaded }; } +// useAiOperatorTasksGate ensures the runtime /config is loaded and reports +// whether the "Delegate to Operator" action should be shown. `loaded` lets +// callers distinguish "not yet known" from "known disabled" so they can avoid +// flashing the button before the answer arrives (W08 of #5205, #5246). +export function useAiOperatorTasksGate(): { enabled: boolean; loaded: boolean } { + const enabled = useFeaturesStore((s) => s.features.aiOperatorTasks); + const loaded = useFeaturesStore((s) => s.loaded); + const load = useFeaturesStore((s) => s.load); + useEffect(() => { + void load(); + }, [load]); + return { enabled, loaded }; +} + // Whether software package file uploads are possible (S3 storage configured on // the server). Defaults open until /config says otherwise — see // DEFAULT_SOFTWARE_PACKAGES above. Pass `active: false` to defer the /config diff --git a/packages/shared/src/validators/aiOperator.ts b/packages/shared/src/validators/aiOperator.ts index cbc015c43b..0e5ef1f00a 100644 --- a/packages/shared/src/validators/aiOperator.ts +++ b/packages/shared/src/validators/aiOperator.ts @@ -204,3 +204,62 @@ export const taskCheckpointSchema = z.object({ fixWatchId: z.string().uuid().nullable().default(null), }).strict(); export type TaskCheckpoint = z.infer; + +// ---- W08 (#5246): admission request ---- + +/** + * The record a task may cite as the reason it was delegated (spec §12, + * "optional source record"). Provenance only — it never widens authority and + * never selects a target: the target is `deviceId`, which the server + * re-resolves and re-authorizes on its own. + */ +export const OPERATOR_TASK_SOURCE_KINDS = ['alert', 'device'] as const; +export type OperatorTaskSourceKind = (typeof OPERATOR_TASK_SOURCE_KINDS)[number]; + +/** + * Body validator for `POST /api/v1/ai/operator/tasks` (#5205 W08, spec §12). + * + * `.strict()` is load-bearing, not tidiness. Spec §12: "Requests cannot supply + * a principal, effective policy, approval result, or trusted continuation + * token." Every one of those would arrive as an extra property — `task`, + * `policySnapshot`, `approval`, `principal`, `agentId` — and `.strict()` is + * what turns each into a 400 instead of a silently ignored field that a later + * refactor might start honouring. There is deliberately no `agentId` here + * either: §5.1 says the SERVER resolves the agent, so letting a client name + * one would be a client-chosen principal by another door. + * + * `mode` is a literal `'live'`; trials are P3-5, and admitting one through + * this route would produce a live task labelled as a trial. + */ +export const createOperatorTaskSchema = z.object({ + mode: z.literal('live'), + recipeKey: z.literal('service_recovery'), + /** + * The recipe version the CLIENT reviewed. Checked against the server's + * released version and refused (422) on mismatch rather than silently + * upgraded — spec §5.1: the operator approves a specific reviewed workflow, + * and a cached catalog is never authority. + */ + recipeVersion: z.number().int().min(1), + orgId: z.string().guid(), + deviceId: z.string().guid(), + inputs: z.object({ + serviceName: z.string().min(1).max(255), + }).strict(), + sourceKind: z.enum(OPERATOR_TASK_SOURCE_KINDS).optional(), + sourceId: z.string().guid().optional(), + /** + * Required, not optional. A caller with no key gets no idempotency, and the + * effect this admits (a service restart on a customer machine) is the kind + * you cannot take back — so admission is refused without one rather than + * defaulting to at-least-once. + */ + clientIdempotencyKey: z.string().min(8).max(200), +}).strict() + // A source id without its kind (or the reverse) is a half-formed citation; + // accepting it would record provenance that cannot be resolved. + .refine((v) => (v.sourceKind === undefined) === (v.sourceId === undefined), { + message: 'sourceKind and sourceId must be provided together', + path: ['sourceId'], + }); +export type CreateOperatorTaskInput = z.infer; From b0e07a2662d4a33d4ff7c9f835d03141e11a0e91 Mon Sep 17 00:00:00 2001 From: Todd Hebebrand Date: Tue, 8 Sep 2026 04:28:59 -0600 Subject: [PATCH 2/4] feat(web): Delegate to Operator action on the device and alert pages (W08 of #5205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DelegateToOperatorButton` — hidden entirely (not disabled) unless `features.aiOperatorTasks` is on, because the gate defaults CLOSED: an unreachable or older `/config` must never surface an action that starts autonomous remediation on a customer machine. The client idempotency key is minted ONCE per opened dialog and held in a ref, so a double-click or a retry after a network blip reuses it and the server returns the same task. It is re-minted only when the dialog is re-opened, which is the one case where the operator really is asking for a second task. Alert detail passes the alert's OWN orgId (newly declared on the web type; the API always returned it), never the globally selected org — spec §5.1, "changing global organization context while drafting cannot retarget the task" — and cites the alert as the source so the verification criterion has a recurrence signal. Without one, W06's criterion can only reach `investigation_complete`, never `verified_resolved`. `extractServiceNameFromAlert` prefills the service field from the alert's prose, since no structured service name exists on an alert row. It returns null rather than guessing, and the field stays required and editable. The device-page button has no `online` gate: a task is durable work with its own deadline, not an immediate command. Compose: both operator flags are mapped explicitly in `x-api-env`. Compose interpolates only what that block names, so a value in `.env` alone never reaches the container — without these the feature could not be switched on in any deployment and `/config` would report it disabled with no error anywhere. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YMbWgjQdqXzi98eJ5P3Uqp --- .../DelegateToOperatorButton.test.tsx | 169 +++++++++++++++ .../aiOperator/DelegateToOperatorButton.tsx | 197 ++++++++++++++++++ .../aiOperator/alertServiceName.test.ts | 47 +++++ .../components/aiOperator/alertServiceName.ts | 50 +++++ .../src/components/alerts/AlertDetailPage.tsx | 24 +++ .../src/components/devices/DeviceActions.tsx | 14 ++ apps/web/src/locales/de-DE/aiOperator.json | 15 ++ apps/web/src/locales/en/aiOperator.json | 15 ++ apps/web/src/locales/es-419/aiOperator.json | 15 ++ apps/web/src/locales/fr-CA/aiOperator.json | 15 ++ apps/web/src/locales/fr-FR/aiOperator.json | 15 ++ apps/web/src/locales/it-IT/aiOperator.json | 15 ++ apps/web/src/locales/pt-BR/aiOperator.json | 15 ++ apps/web/src/locales/tr-TR/aiOperator.json | 15 ++ docker-compose.yml | 7 + 15 files changed, 628 insertions(+) create mode 100644 apps/web/src/components/aiOperator/DelegateToOperatorButton.test.tsx create mode 100644 apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx create mode 100644 apps/web/src/components/aiOperator/alertServiceName.test.ts create mode 100644 apps/web/src/components/aiOperator/alertServiceName.ts diff --git a/apps/web/src/components/aiOperator/DelegateToOperatorButton.test.tsx b/apps/web/src/components/aiOperator/DelegateToOperatorButton.test.tsx new file mode 100644 index 0000000000..7517e03aba --- /dev/null +++ b/apps/web/src/components/aiOperator/DelegateToOperatorButton.test.tsx @@ -0,0 +1,169 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import '../../lib/i18n'; + +const fetchWithAuth = vi.fn(); +const showToast = vi.fn(); +const navigateTo = vi.fn(); + +vi.mock('../../stores/auth', () => ({ + fetchWithAuth: (...args: unknown[]) => fetchWithAuth(...args), +})); +vi.mock('../shared/Toast', async (importOriginal) => { + const actual = await importOriginal>(); + return { ...actual, showToast: (...args: unknown[]) => showToast(...args) }; +}); +vi.mock('@/lib/navigation', () => ({ + navigateTo: (...args: unknown[]) => navigateTo(...args), +})); + +let gateEnabled = true; +vi.mock('../../stores/featuresStore', () => ({ + useAiOperatorTasksGate: () => ({ enabled: gateEnabled, loaded: true }), +})); + +import { DelegateToOperatorButton } from './DelegateToOperatorButton'; + +const okJson = (payload: unknown): Response => + ({ ok: true, status: 200, json: vi.fn().mockResolvedValue(payload) }) as unknown as Response; + +const errJson = (status: number, payload: unknown): Response => + ({ ok: false, status, json: vi.fn().mockResolvedValue(payload) }) as unknown as Response; + +const BASE_PROPS = { + orgId: 'org-1', + deviceId: 'device-1', + deviceLabel: 'WKS-01', + orgLabel: 'Acme Corp', + source: { kind: 'alert' as const, id: 'alert-1' }, +}; + +beforeEach(() => { + gateEnabled = true; + fetchWithAuth.mockReset(); + showToast.mockReset(); + navigateTo.mockReset(); +}); + +describe('DelegateToOperatorButton', () => { + it('renders nothing when the gate is disabled', () => { + gateEnabled = false; + render(); + expect(screen.queryByTestId('delegate-to-operator')).toBeNull(); + }); + + it('renders the button when the gate is enabled', () => { + render(); + expect(screen.getByTestId('delegate-to-operator')).toBeInTheDocument(); + }); + + it('prefills the service input from defaultServiceName', async () => { + render(); + await userEvent.click(screen.getByTestId('delegate-to-operator')); + + const input = await screen.findByTestId('delegate-to-operator-service'); + expect(input).toHaveValue('spooler'); + }); + + it('disables confirm and shows an error when the service name is cleared', async () => { + render(); + await userEvent.click(screen.getByTestId('delegate-to-operator')); + + const input = await screen.findByTestId('delegate-to-operator-service'); + + // No error before any interaction. + expect(screen.queryByTestId('delegate-to-operator-error')).toBeNull(); + + await userEvent.clear(input); + await userEvent.tab(); // blur the empty field + + expect(screen.getByTestId('delegate-to-operator-error')).toBeInTheDocument(); + expect(screen.getByTestId('delegate-to-operator-confirm')).toHaveAttribute('aria-disabled', 'true'); + }); + + it('happy path: submits and matches the expected body', async () => { + fetchWithAuth.mockResolvedValue(okJson({ taskId: 'task-99' })); + + render(); + await userEvent.click(screen.getByTestId('delegate-to-operator')); + + const input = await screen.findByTestId('delegate-to-operator-service'); + await userEvent.type(input, 'spooler'); + await userEvent.click(screen.getByTestId('delegate-to-operator-confirm')); + + await waitFor(() => expect(fetchWithAuth).toHaveBeenCalledTimes(1)); + const [url, init] = fetchWithAuth.mock.calls[0]; + expect(url).toBe('/ai/operator/tasks'); + expect(init.method).toBe('POST'); + + const body = JSON.parse(init.body as string); + expect(body).toMatchObject({ + mode: 'live', + recipeKey: 'service_recovery', + recipeVersion: 1, + orgId: 'org-1', + deviceId: 'device-1', + inputs: { serviceName: 'spooler' }, + sourceKind: 'alert', + sourceId: 'alert-1', + }); + expect(typeof body.clientIdempotencyKey).toBe('string'); + expect(body.clientIdempotencyKey.length).toBeGreaterThan(0); + }); + + it('keeps the idempotency key stable across two confirms of the same opened dialog', async () => { + fetchWithAuth + .mockResolvedValueOnce(errJson(500, { error: 'boom' })) + .mockResolvedValueOnce(okJson({ taskId: 'task-1' })); + + render(); + await userEvent.click(screen.getByTestId('delegate-to-operator')); + + const input = await screen.findByTestId('delegate-to-operator-service'); + await userEvent.type(input, 'spooler'); + + // First confirm fails. + await userEvent.click(screen.getByTestId('delegate-to-operator-confirm')); + await waitFor(() => expect(fetchWithAuth).toHaveBeenCalledTimes(1)); + await waitFor(() => expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ type: 'error' }), + )); + + // Second confirm on the SAME still-open dialog. + await userEvent.click(screen.getByTestId('delegate-to-operator-confirm')); + await waitFor(() => expect(fetchWithAuth).toHaveBeenCalledTimes(2)); + + const firstBody = JSON.parse(fetchWithAuth.mock.calls[0][1].body as string); + const secondBody = JSON.parse(fetchWithAuth.mock.calls[1][1].body as string); + expect(secondBody.clientIdempotencyKey).toBe(firstBody.clientIdempotencyKey); + }); + + it('failure path shows an error toast', async () => { + fetchWithAuth.mockResolvedValue(errJson(500, { error: 'boom' })); + + render(); + await userEvent.click(screen.getByTestId('delegate-to-operator')); + + const input = await screen.findByTestId('delegate-to-operator-service'); + await userEvent.type(input, 'spooler'); + await userEvent.click(screen.getByTestId('delegate-to-operator-confirm')); + + await waitFor(() => expect(showToast).toHaveBeenCalledWith( + expect.objectContaining({ type: 'error' }), + )); + }); + + it('navigates to the new task on success', async () => { + fetchWithAuth.mockResolvedValue(okJson({ taskId: 'task-42' })); + + render(); + await userEvent.click(screen.getByTestId('delegate-to-operator')); + + const input = await screen.findByTestId('delegate-to-operator-service'); + await userEvent.type(input, 'spooler'); + await userEvent.click(screen.getByTestId('delegate-to-operator-confirm')); + + await waitFor(() => expect(navigateTo).toHaveBeenCalledWith('/operator/tasks/task-42')); + }); +}); diff --git a/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx b/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx new file mode 100644 index 0000000000..3105f7dff4 --- /dev/null +++ b/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx @@ -0,0 +1,197 @@ +/** + * "Delegate to Operator" action (W08 of #5205, #5246). Renders nothing unless + * the `aiOperatorTasks` runtime flag is on (`useAiOperatorTasksGate`) — this + * gates a write action that starts autonomous remediation on a customer + * machine, and the gate defaults closed (see featuresStore.ts). + * + * Opens a ConfirmDialog collecting a required service name, then POSTs a + * fixed `service_recovery` recipe task and navigates to the new task's + * detail page. The recipe is not selectable in this slice. + */ +import { useCallback, useEffect, useRef, useState } from 'react'; +import { useTranslation } from 'react-i18next'; +import { Bot } from 'lucide-react'; +import '@/lib/i18n'; +import { fetchWithAuth } from '../../stores/auth'; +import { navigateTo } from '@/lib/navigation'; +import { useAiOperatorTasksGate } from '../../stores/featuresStore'; +import { ConfirmDialog } from '../shared/ConfirmDialog'; +import { showToast } from '../shared/Toast'; +import { ActionError, runAction } from '@/lib/runAction'; + +const SERVICE_NAME_MAX_LENGTH = 255; + +export interface DelegateToOperatorButtonProps { + orgId: string; + deviceId: string; + /** Hostname / display name, shown read-only in the confirm dialog. */ + deviceLabel: string; + /** Org name, shown read-only when provided. */ + orgLabel?: string; + source: { kind: 'alert'; id: string } | { kind: 'device'; id: string }; + /** Prefill for the service name field (e.g. from the triggering alert). */ + defaultServiceName?: string; + className?: string; +} + +export function DelegateToOperatorButton({ + orgId, + deviceId, + deviceLabel, + orgLabel, + source, + defaultServiceName, + className, +}: DelegateToOperatorButtonProps) { + const { t } = useTranslation('aiOperator'); + const { enabled, loaded } = useAiOperatorTasksGate(); + + const [open, setOpen] = useState(false); + const [serviceName, setServiceName] = useState(defaultServiceName ?? ''); + const [touched, setTouched] = useState(false); + const [submitting, setSubmitting] = useState(false); + + // Minted once per OPENED dialog and kept stable across retries of that same + // dialog, so a double-click on Confirm — or a retry after a network blip — + // reuses the same key and the server can dedupe it into a single task + // instead of creating two. Only re-minted when the dialog is (re-)opened. + const idempotencyKeyRef = useRef(null); + + useEffect(() => { + if (open) { + idempotencyKeyRef.current = `delegate-${crypto.randomUUID()}`; + setServiceName(defaultServiceName ?? ''); + setTouched(false); + } + // defaultServiceName intentionally excluded: only re-read when the dialog + // opens, not on every prop change while it's open. + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [open]); + + if (!loaded || !enabled) return null; + + const trimmedServiceName = serviceName.trim(); + const showError = touched && trimmedServiceName.length === 0; + + const handleOpen = useCallback(() => setOpen(true), []); + + const handleClose = useCallback(() => { + if (submitting) return; + setOpen(false); + }, [submitting]); + + const handleConfirm = useCallback(() => { + if (trimmedServiceName.length === 0) { + setTouched(true); + return; + } + const clientIdempotencyKey = idempotencyKeyRef.current; + if (!clientIdempotencyKey) return; + + setSubmitting(true); + void (async () => { + try { + const result = await runAction<{ taskId: string }>({ + request: () => fetchWithAuth('/ai/operator/tasks', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + mode: 'live', + recipeKey: 'service_recovery', + recipeVersion: 1, + orgId, + deviceId, + inputs: { serviceName: trimmedServiceName }, + sourceKind: source.kind, + sourceId: source.id, + clientIdempotencyKey, + }), + }), + errorFallback: t('delegate.failed'), + successMessage: t('delegate.queued'), + }); + setOpen(false); + await navigateTo(`/operator/tasks/${result.taskId}`); + } catch (err) { + if (err instanceof ActionError && err.status === 401) return; // auth redirect handles it + if (!(err instanceof ActionError)) { + showToast({ type: 'error', message: t('delegate.failed') }); + } + // non-401 ActionError was already toasted by runAction — don't double-toast + } finally { + setSubmitting(false); + } + })(); + }, [trimmedServiceName, orgId, deviceId, source, t]); + + return ( + <> + + + +
+
+ {orgLabel !== undefined && ( +
+
{t('delegate.orgLabel')}
+
{orgLabel}
+
+ )} +
+
{t('delegate.deviceLabel')}
+
{deviceLabel}
+
+
+ +
+ + setServiceName(e.target.value)} + onBlur={() => setTouched(true)} + className="h-10 w-full rounded-md border bg-background px-3 text-sm focus:outline-hidden focus:ring-2 focus:ring-ring" + placeholder={t('delegate.serviceNamePlaceholder')} + /> + {showError && ( +

+ {t('delegate.serviceNameError')} +

+ )} +
+ +

+ {t('delegate.workflowLabel')}: {t('delegate.workflowName')} +

+
+ + + ); +} + +export default DelegateToOperatorButton; diff --git a/apps/web/src/components/aiOperator/alertServiceName.test.ts b/apps/web/src/components/aiOperator/alertServiceName.test.ts new file mode 100644 index 0000000000..66df3b0f79 --- /dev/null +++ b/apps/web/src/components/aiOperator/alertServiceName.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; +import { extractServiceNameFromAlert } from './alertServiceName'; + +describe('extractServiceNameFromAlert', () => { + it('reads the exact prose the service alert condition writes', () => { + // Verbatim from `alertConditions/handlers/service.ts`. + expect(extractServiceNameFromAlert({ + message: 'Service spooler stopped (3 consecutive failures, threshold: 3)', + })).toBe('spooler'); + }); + + it('reads the "is running" phrasing too', () => { + expect(extractServiceNameFromAlert({ message: 'Service W32Time is running' })).toBe('W32Time'); + }); + + it('falls back to the title when the message has no match', () => { + expect(extractServiceNameFromAlert({ + title: 'Service MSSQLSERVER stopped', + message: 'A monitored condition changed state.', + })).toBe('MSSQLSERVER'); + }); + + it('prefers a structured context value over parsed prose', () => { + expect(extractServiceNameFromAlert({ + message: 'Service spooler stopped', + context: { serviceName: 'Spooler' }, + })).toBe('Spooler'); + }); + + it('returns null rather than guessing when nothing matches', () => { + // The dialog then asks the operator, which is the only honest outcome — + // a wrong prefill would be a different operation under a different digest. + expect(extractServiceNameFromAlert({ message: 'CPU above 90% for 10 minutes' })).toBeNull(); + expect(extractServiceNameFromAlert({})).toBeNull(); + expect(extractServiceNameFromAlert({ message: null, title: null })).toBeNull(); + }); + + it('never returns a filler word as a service name', () => { + expect(extractServiceNameFromAlert({ message: 'Service is not reachable' })).toBeNull(); + }); + + it('bounds the name to the column width', () => { + const long = 'a'.repeat(400); + const got = extractServiceNameFromAlert({ context: { serviceName: long } }); + expect(got).toHaveLength(255); + }); +}); diff --git a/apps/web/src/components/aiOperator/alertServiceName.ts b/apps/web/src/components/aiOperator/alertServiceName.ts new file mode 100644 index 0000000000..106934471b --- /dev/null +++ b/apps/web/src/components/aiOperator/alertServiceName.ts @@ -0,0 +1,50 @@ +/** + * Best-effort service-name prefill for the "Delegate to Operator" dialog + * (#5205 W08, #5246). + * + * There is NO structured service name on an alert. The service condition + * handler (`apps/api/src/services/alertConditions/handlers/service.ts`) writes + * the name only into prose: + * + * "Service spooler stopped (3 consecutive failures, threshold: 3)" + * "Service spooler is running" + * + * so the only thing available is a text match. That is exactly why this + * returns `null` rather than a guess it cannot justify, and why the dialog's + * service field is a REQUIRED, editable input rather than a hidden value: a + * prefill is a convenience, and the operator confirms the actual name before + * anything is admitted. The server re-validates it either way — `serviceName` + * is part of the argument digest, so a wrong one is a different operation and + * a different approval (spec §7.1), never a silently widened one. + */ +const SERVICE_PATTERNS: readonly RegExp[] = [ + // "Service spooler stopped", "Service 'W32Time' is running" + /\bservice\s+["'`]?([A-Za-z0-9._$-]{1,255})["'`]?\s+(?:stopped|is\s|has\s|not\s|failed)/i, + // Trailing form: "... stopped: spooler" + /\bservice\s*:\s*["'`]?([A-Za-z0-9._$-]{1,255})["'`]?/i, +]; + +/** Words that are never a service name, only the sentence around one. */ +const NON_NAMES = new Set(['is', 'was', 'has', 'not', 'the', 'a', 'an']); + +export function extractServiceNameFromAlert(alert: { + title?: string | null; + message?: string | null; + context?: Record | null; +}): string | null { + // A structured value, if a future rule ever supplies one, always wins over + // parsing prose. + const fromContext = alert.context?.serviceName; + if (typeof fromContext === 'string' && fromContext.trim()) { + return fromContext.trim().slice(0, 255); + } + + for (const text of [alert.message, alert.title]) { + if (!text) continue; + for (const pattern of SERVICE_PATTERNS) { + const name = pattern.exec(text)?.[1]?.trim(); + if (name && !NON_NAMES.has(name.toLowerCase())) return name.slice(0, 255); + } + } + return null; +} diff --git a/apps/web/src/components/alerts/AlertDetailPage.tsx b/apps/web/src/components/alerts/AlertDetailPage.tsx index 0ca871b64f..aabbb84b4e 100644 --- a/apps/web/src/components/alerts/AlertDetailPage.tsx +++ b/apps/web/src/components/alerts/AlertDetailPage.tsx @@ -19,6 +19,8 @@ import CreateTicketFromAlertDialog from './CreateTicketFromAlertDialog'; import { useOrgStore } from '@/stores/orgStore'; import type { TicketStatus, TicketPriority } from '../tickets/ticketConfig'; import RemediationSuggestionsPanel from '../remediation/RemediationSuggestionsPanel'; +import { DelegateToOperatorButton } from '../aiOperator/DelegateToOperatorButton'; +import { extractServiceNameFromAlert } from '../aiOperator/alertServiceName'; import { formatAnomalyConfidence, formatAnomalyType, @@ -37,6 +39,14 @@ type Alert = { status: AlertStatus; deviceId: string; deviceName: string; + /** + * The alert's OWN org. W08 (#5246): the delegate action targets this org, + * never the globally selected one — spec §5.1, "changing global + * organization context while drafting cannot retarget the task". The API + * has always returned it (the detail route spreads the whole alert row); + * it simply was not declared here before. + */ + orgId: string; ruleId?: string; ruleName?: string; triggeredAt: string; @@ -347,6 +357,20 @@ export default function AlertDetailPage({ alertId }: AlertDetailPageProps) { {t('alertDetailPage.acknowledge')} )} + {/* W08 of #5205 (#5246). Hidden entirely unless the AI Operator + flags are on. Targets the ALERT's org and device, and cites + the alert as its source so the verification criterion gets a + recurrence signal (without one, the best achievable outcome is + `investigation_complete`, never `verified_resolved`). */} + {(alert.status === 'active' || alert.status === 'acknowledged') && ( + + )} {(alert.status === 'active' || alert.status === 'acknowledged') && ( + {/* W08 of #5205 (#5246). Renders nothing unless the AI Operator flags + are on, so this row is unchanged for every deployment that has not + enabled the feature (decision D2: internal/test orgs only). No + `online` gate: a task is durable work with its own deadline, not an + immediate command — the coordinator waits for the device rather + than the technician having to. */} + Date: Tue, 8 Sep 2026 05:03:21 -0600 Subject: [PATCH 3/4] test(ai-operator): Playwright approve-after-browser-close flow (W08 of #5205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acceptance scenario 3 / spec §7.1 "authority across time": a technician delegates a service-recovery task from a device page, closes the browser, and the approval the operator is waiting on is still decidable from a different browser session later. What the spec really drives: the Delegate to Operator button and dialog, the POST /ai/operator/tasks admission (202 + the server's own task id), the navigation to /operator/tasks/ and that page's render, an idempotent replay of the same client key resolving to the same task with no second row, the destruction of the creating browser context, a second independent login, and a real WebAuthn approve ceremony clicked in the approvals inbox. What is seeded, and why: the pending intent/approval the task waits on (seed-operator-task-approval.sql). Reaching it for real needs a live LLM run plus a coordinator tick; the e2e stack has neither. That transition is already proven against real Postgres by aiOperatorServiceRecoveryE2E.integration.test.ts. The task itself is NEVER seeded — it must come from the real button. What the spec deliberately cannot reach: completed + verified_resolved. That needs a connected agent to execute the restart and an independent device read to verify it, and the e2e stack runs no agent (tests/script-cancel.spec.ts records the same limit). Faking the device would prove nothing about W08. Also: - e2e-tests/webauthn.ts — virtual-authenticator + approver-device enrolment helpers, including credential export/import so a key enrolled in one browser context can sign in the next. Registration is grant-gated since #2707, so it mints a registerGrantId first; without one the options route 403s. - README: two stack prerequisites these WebAuthn specs have (PUBLIC_APP_URL must match the browser origin or the RP ID check fails, and the enrolment ceremony rotates the refresh token and burns the session it runs in). - .env.example: document the two AI Operator flags, both default off (D2). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YGPxXGTWpJKUNbEdR3TZEL --- .env.example | 9 + e2e-tests/README.md | 18 ++ e2e-tests/pages/OperatorTaskPage.ts | 129 ++++++++ e2e-tests/seed-operator-agent.sql | 31 ++ e2e-tests/seed-operator-task-approval.sql | 146 +++++++++ ...erator-approve-after-browser-close.spec.ts | 283 ++++++++++++++++++ e2e-tests/webauthn.ts | 166 ++++++++++ 7 files changed, 782 insertions(+) create mode 100644 e2e-tests/pages/OperatorTaskPage.ts create mode 100644 e2e-tests/seed-operator-agent.sql create mode 100644 e2e-tests/seed-operator-task-approval.sql create mode 100644 e2e-tests/tests/ai-operator-approve-after-browser-close.spec.ts create mode 100644 e2e-tests/webauthn.ts diff --git a/.env.example b/.env.example index d1515d2fee..2935bd967d 100644 --- a/.env.example +++ b/.env.example @@ -1043,6 +1043,15 @@ MANAGED_SOFTWARE_POLICY_MODE=compat # unattended until a later release even when true. # BREEZE_AI_AGENTS_ENABLED=false +# AI Operator task delegation (#5205). Both default false — decision D2 keeps +# the feature to internal and test orgs until it graduates. TASKS_ENABLED gates +# the admission route POST /ai/operator/tasks and the "Delegate to Operator" +# action; the RECIPE flag additionally gates the one recipe that exists +# (service recovery). With either off, admission answers 422 with an +# actionable reason and the web action does not render. +# AI_OPERATOR_TASKS_ENABLED=false +# AI_OPERATOR_RECIPE_SERVICE_RECOVERY_ENABLED=false + # Wave 5 Part B (#3827) sub-flag of BREEZE_AI_AGENTS_ENABLED above. Gates # attemptPolicyDecision: an agent-originated, supervised-scope action-intent # whose operation is in the operator's per-agent actAssets.supervisedActionKeys diff --git a/e2e-tests/README.md b/e2e-tests/README.md index 167b887467..e7a6b7de1d 100644 --- a/e2e-tests/README.md +++ b/e2e-tests/README.md @@ -155,6 +155,24 @@ dashboard. Pick one before running the suite: - set `MFA_FORCE_FOR_PARTNER_ADMIN=false` in the stack's `.env` (the documented relief valve; it suppresses only the role-force component) and restart `api`. +### WebAuthn specs need `PUBLIC_APP_URL` to match the browser origin + +`intent-self-approve.spec.ts` and `ai-operator-approve-after-browser-close.spec.ts` +run real WebAuthn ceremonies against Chrome's virtual authenticator. The server +derives its Relying Party ID from `PUBLIC_APP_URL`, and the browser refuses any +ceremony whose RP ID is not a suffix of the page's own origin +(`SecurityError: The relying party ID is not a registrable domain suffix ...`). +A stack whose `.env` still carries the production `PUBLIC_APP_URL` will fail +these two specs and only these two. Point it at the stack's own base URL (e.g. +`PUBLIC_APP_URL=http://localhost:` from `.breeze-stack.json`) and +restart `api`. + +Note also that the register-and-refresh ceremony ROTATES the refresh token, +which revokes the JTI the page's session store is holding — the browser session +that performs it is unusable afterwards ("Your session expired"). Enrol the key +in a session that has no in-app work left, and carry it to the next context with +`exportCredentials` / `importCredentials` (`e2e-tests/webauthn.ts`). + ## Troubleshooting ### `globalSetup` fails on docker exec diff --git a/e2e-tests/pages/OperatorTaskPage.ts b/e2e-tests/pages/OperatorTaskPage.ts new file mode 100644 index 0000000000..198e35d10e --- /dev/null +++ b/e2e-tests/pages/OperatorTaskPage.ts @@ -0,0 +1,129 @@ +import type { Page, Response } from '@playwright/test'; +import { BasePage } from './BasePage'; +import { waitForAppReady, waitForHydration } from './hydration'; + +/** + * Page Objects for the AI Operator delegate flow (#5205 W08, #5246). + * + * Two surfaces, kept in one file because the spec drives them as one flow: + * the "Delegate to Operator" action that lives on the device detail page, and + * the task detail page it navigates to. + */ + +/** The "Delegate to Operator" action, wherever it is rendered. */ +export class DelegateToOperatorAction extends BasePage { + button = () => this.page.getByTestId('delegate-to-operator'); + serviceInput = () => this.page.getByTestId('delegate-to-operator-service'); + confirmButton = () => this.page.getByTestId('delegate-to-operator-confirm'); + validationError = () => this.page.getByTestId('delegate-to-operator-error'); + + /** Opens the device detail page and waits for the action to be clickable. */ + async gotoDevice(deviceId: string) { + await this.page.goto(`/devices/${deviceId}`); + await this.button().waitFor({ timeout: 30_000 }); + await waitForHydration(this.page, 'delegate-to-operator'); + } + + async openDialog() { + await this.button().click(); + await this.serviceInput().waitFor(); + } + + /** + * Fills the service name and confirms, returning the admission response. + * + * The task id is read from the `POST /ai/operator/tasks` response rather + * than scraped from the URL: the URL is the thing under test (the action + * must navigate to the task the SERVER created), so reading the id from it + * would make the navigation assertion circular. + */ + async confirm(serviceName: string): Promise<{ status: number; taskId: string; request: DelegateRequest }> { + await this.serviceInput().fill(serviceName); + + let captured: DelegateRequest | null = null; + const onRequest = (req: { url(): string; method(): string; headers(): Record; postData(): string | null }) => { + if (req.method() === 'POST' && req.url().includes('/ai/operator/tasks')) { + captured = { + url: req.url(), + headers: req.headers(), + body: req.postData() ?? '', + }; + } + }; + this.page.on('request', onRequest); + + let response: Response; + try { + [response] = await Promise.all([ + this.page.waitForResponse( + (r) => r.url().includes('/ai/operator/tasks') && r.request().method() === 'POST', + { timeout: 30_000 }, + ), + this.confirmButton().click(), + ]); + } finally { + this.page.off('request', onRequest); + } + + const body = (await response.json()) as { taskId?: string }; + if (!captured) throw new Error('never observed the delegate POST request'); + if (!body.taskId) throw new Error(`admission returned no taskId: ${JSON.stringify(body)}`); + return { status: response.status(), taskId: body.taskId, request: captured }; + } +} + +export type DelegateRequest = { + url: string; + headers: Record; + body: string; +}; + +/** The AI Operator task detail page at /operator/tasks/. */ +export class OperatorTaskPage extends BasePage { + state = () => this.page.getByTestId('operator-task-state'); + nextAction = () => this.page.getByTestId('operator-task-next-action'); + target = () => this.page.getByTestId('operator-task-target'); + operationsList = () => this.page.getByTestId('operator-task-operations-list'); + operationsEmpty = () => this.page.getByTestId('operator-task-operations-empty'); + error = () => this.page.getByTestId('operator-task-error'); + + async goto(taskId: string) { + await this.page.goto(`/operator/tasks/${taskId}`); + await waitForAppReady(this.page, 'operator-task-state'); + } + + /** Waits for the detail page to finish loading after an in-app navigation. */ + async waitForLoaded() { + await waitForAppReady(this.page, 'operator-task-state'); + } +} + +/** The approvals inbox at /approvals. */ +export class ApprovalsInboxPage extends BasePage { + url = '/approvals'; + + inbox = () => this.page.getByTestId('approvals-inbox'); + row = (approvalId: string) => this.page.getByTestId(`approval-row-${approvalId}`); + approveButton = (approvalId: string) => this.page.getByTestId(`approval-approve-${approvalId}`); + + async goto() { + await this.page.goto(this.url); + await waitForAppReady(this.page, 'approvals-inbox'); + } + + async approve(approvalId: string): Promise { + await this.row(approvalId).waitFor({ timeout: 30_000 }); + const [response] = await Promise.all([ + this.page.waitForResponse( + (r) => r.url().includes(`/approvals/${approvalId}/approve`) && r.request().method() === 'POST', + { timeout: 30_000 }, + ), + this.approveButton(approvalId).click(), + ]); + return response.status(); + } +} + +export function delegateAction(page: Page): DelegateToOperatorAction { + return new DelegateToOperatorAction(page); +} diff --git a/e2e-tests/seed-operator-agent.sql b/e2e-tests/seed-operator-agent.sql new file mode 100644 index 0000000000..e7a79eba83 --- /dev/null +++ b/e2e-tests/seed-operator-agent.sql @@ -0,0 +1,31 @@ +-- Seeds the ONE enabled AI agent that `POST /ai/operator/tasks` resolves for +-- the dev org (#5205 W08, #5246). +-- +-- The route refuses with 422 OPERATOR_NO_AGENT when an org has no enabled +-- agent, and `pnpm db:seed` creates none — so without this the Playwright +-- delegate flow cannot reach admission at all. Seeding the AGENT (not the +-- task) is deliberate: everything downstream of the button must be produced +-- by the real route, or the test proves nothing about it. +-- +-- Idempotent: `ai_agents_org_kind_uq` is a PARTIAL unique index on (org_id, +-- kind) WHERE disabled_at IS NULL, so the conflict target must repeat that +-- predicate or Postgres raises 42P10. A rerun +-- updates the existing row instead of failing. + +SELECT set_config('breeze.scope', 'system', true); + +INSERT INTO ai_agents (org_id, partner_id, kind, name, enabled, mode, tool_allowlist, created_by) +SELECT + o.id, + NULL, + 'triage', + 'Operator', + true, + 'shadow', + '["manage_services", "list_services"]'::jsonb, + u.id +FROM organizations o +CROSS JOIN LATERAL (SELECT id FROM users ORDER BY created_at LIMIT 1) u +ON CONFLICT (org_id, kind) WHERE disabled_at IS NULL DO UPDATE SET enabled = true; + +SELECT 'OPERATOR_AGENT_ID=' || id FROM ai_agents WHERE kind = 'triage' LIMIT 1; diff --git a/e2e-tests/seed-operator-task-approval.sql b/e2e-tests/seed-operator-task-approval.sql new file mode 100644 index 0000000000..840c93edb4 --- /dev/null +++ b/e2e-tests/seed-operator-task-approval.sql @@ -0,0 +1,146 @@ +-- E2E fixture: the pending Tier-3 approval an AI Operator service-recovery +-- task waits on, attached to a task that the BROWSER really created +-- (#5205 W08, #5246). +-- +-- Why this is seeded rather than driven: reaching this state for real needs a +-- live LLM (the investigate run must propose the restart and call +-- createActionIntent) and a BullMQ coordinator tick. The e2e stack has +-- neither — `tests/script-cancel.spec.ts` records the same constraint for +-- device commands: "There is no fake/live agent in the e2e stack". The +-- coordinator's own transition into this state is proven against real +-- Postgres by aiOperatorServiceRecoveryE2E.integration.test.ts; what this +-- fixture exists for is the half nothing else covers — that the approval a +-- delegated task is waiting on is still decidable by a human in a BROWSER +-- SESSION THAT DID NOT CREATE IT, after the creating session is gone +-- (spec §7.1, "authority across time"; acceptance scenario 3). +-- +-- The task itself is NOT seeded. It must already exist, created through the +-- real admission route by the real button, and is passed in as :task_id. +-- +-- Mirrors advanceInvestigate's `awaiting_approval` branch +-- (services/aiOperator/taskCoordinator.ts): the task yields to a wait keyed on +-- the intent, with the step advanced to `execute`. +-- +-- Idempotent-ish: reruns append a new intent/approval pair for the task, which +-- is fine — each run passes a freshly delegated task id. +-- +-- Emits APPROVAL_ID / INTENT_ID on stdout for the spec to parse. + +\set ON_ERROR_STOP on + +SELECT set_config('breeze.scope', 'system', true); + +-- psql does NOT substitute :variables inside a dollar-quoted body (its lexer +-- treats $$...$$ as a quoted literal), so the task id is handed to the block +-- through a GUC instead of interpolated into it. +SELECT set_config('e2e.task_id', :'task_id', false); + +DO $$ +DECLARE + v_task_id uuid := current_setting('e2e.task_id')::uuid; + v_org_id uuid; + v_partner uuid; + v_user_id uuid; + v_service text; + v_device uuid; + v_intent uuid; + v_approval uuid; + v_digest char(64) := repeat('b', 64); +BEGIN + SELECT t.org_id, t.requester_user_id, t.device_id + INTO v_org_id, v_user_id, v_device + FROM ai_operator_tasks t + WHERE t.id = v_task_id; + + IF v_org_id IS NULL THEN + RAISE EXCEPTION 'seed: ai_operator_tasks row % not found — did the delegate action really create it?', v_task_id; + END IF; + + SELECT partner_id INTO v_partner FROM organizations WHERE id = v_org_id; + + -- Drop what earlier runs of this spec left behind. Two identical pending + -- approvals in one org CLUSTER into a single grouped card in the inbox, + -- which renders `approval-group-approve-` instead of the per-row + -- `approval-approve-` the spec clicks — so a rerun would fail on a + -- selector that is missing for a reason unrelated to what is under test. + DELETE FROM approval_requests ar + USING action_intents ai + WHERE ar.intent_id = ai.id + AND ai.idempotency_key LIKE 'e2e-operator-task-%'; + DELETE FROM action_intents WHERE idempotency_key LIKE 'e2e-operator-task-%'; + + -- The service name the browser typed, recovered from the task objective so + -- the seeded approval describes the SAME action the task was admitted for + -- rather than an unrelated one. + SELECT COALESCE(NULLIF(t.checkpoint #>> '{recipeInput,serviceName}', ''), 'Spooler') + INTO v_service + FROM ai_operator_tasks t WHERE t.id = v_task_id; + + INSERT INTO action_intents ( + org_id, partner_id, requested_by_user_id, source, requesting_client_label, + action_name, arguments, argument_digest, target_summary, impact_summary, + reason, risk_tier, idempotency_key, correlation_id, status, expires_at, + approval_scope, task_id, task_step_key, operation_key + ) VALUES ( + v_org_id, v_partner, v_user_id, 'chat', 'Breeze AI Operator', + 'manage_services', + jsonb_build_object('action', 'restart', 'deviceId', v_device, 'serviceName', v_service), + v_digest, + 'Restart ' || v_service, + 'Restarts a service on the target device', + 'E2E: AI Operator service recovery', + 3, + 'e2e-operator-task-' || gen_random_uuid()::text, + gen_random_uuid(), + 'pending_approval', + now() + interval '30 minutes', + -- Sole-operator shape: the row is fanned out to the requester, and + -- isIntentRowLiveAuthorized's supervised branch authorizes exactly them. + -- The column defaults to 'four_eyes', under which the requester is not an + -- authorized decider and the row never reaches their inbox at all. + 'supervised', + v_task_id, + -- action_intents_task_link_chk: task_id, task_step_key and operation_key + -- are all-or-nothing. + 'execute', + 'restart_service' + ) RETURNING id INTO v_intent; + + INSERT INTO approval_requests ( + user_id, requesting_client_label, action_label, action_tool_name, + action_arguments, risk_tier, risk_summary, status, expires_at, + intent_id, bound_argument_digest, is_recursive + ) VALUES ( + v_user_id, 'Breeze AI Operator', + 'Restart ' || v_service, 'manage_services', + jsonb_build_object('action', 'restart', 'deviceId', v_device, 'serviceName', v_service), + 'high', 'Restarts a service on the target device', 'pending', + now() + interval '30 minutes', + v_intent, v_digest, false + ) RETURNING id INTO v_approval; + + -- Park the task exactly where the coordinator parks it while an approval is + -- outstanding, so the task detail page under test shows the real waiting + -- shape and not a queued one. + UPDATE ai_operator_tasks + SET state = 'waiting', + phase = 'execute', + current_step_key = 'execute', + wait_reason = 'approval', + wait_dependency_kind = 'intent', + wait_dependency_id = v_intent, + next_wake_at = now() + interval '1 hour' + WHERE id = v_task_id; + + RAISE NOTICE 'APPROVAL_ID=%', v_approval; + RAISE NOTICE 'INTENT_ID=%', v_intent; +END $$; + +-- RAISE NOTICE goes to stderr, which execFileSync callers don't capture. +-- Re-emit on stdout. +SELECT 'APPROVAL_ID=' || ar.id || ' INTENT_ID=' || ai.id + FROM approval_requests ar + JOIN action_intents ai ON ai.id = ar.intent_id + WHERE ai.task_id = :'task_id' + ORDER BY ar.created_at DESC + LIMIT 1; diff --git a/e2e-tests/tests/ai-operator-approve-after-browser-close.spec.ts b/e2e-tests/tests/ai-operator-approve-after-browser-close.spec.ts new file mode 100644 index 0000000000..32712d7003 --- /dev/null +++ b/e2e-tests/tests/ai-operator-approve-after-browser-close.spec.ts @@ -0,0 +1,283 @@ +import { test, expect } from '../fixtures'; +import { clearRefreshState } from '../test-helpers'; +import { + ApprovalsInboxPage, + DelegateToOperatorAction, + OperatorTaskPage, +} from '../pages/OperatorTaskPage'; +import { + addVirtualAuthenticator, + exportCredentials, + importCredentials, + registerApproverDevice, + removeVirtualAuthenticator, +} from '../webauthn'; +import type { BrowserContext, Page } from '@playwright/test'; +import { execFileSync } from 'node:child_process'; +import { existsSync, readFileSync } from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); + +/** + * AI Operator — delegate here, approve there (#5205 W08, #5246). + * + * Acceptance scenario 3, and the property spec §7.1 calls "authority across + * time": a technician delegates a service-recovery task from a device page, + * CLOSES THE BROWSER, and the work the operator is waiting on is still there + * to be approved from a different browser session later. Nothing in the unit + * or integration suites can show that — they have no browser and no session + * boundary to cross. + * + * WHAT IS REAL HERE + * - the delegate button, its dialog, and the `POST /ai/operator/tasks` + * admission it fires (202 + the server's own task id), + * - the navigation to /operator/tasks/ and that page's render, + * - client-idempotency: the same key replayed answers the same task id and + * leaves exactly one row, + * - the destruction of the creating browser context, + * - a second, independent login, and a real WebAuthn approve ceremony driven + * by clicking Approve in the approvals inbox. + * + * WHAT IS SEEDED, AND WHY + * - the pending intent/approval the task waits on + * (`seed-operator-task-approval.sql`). Producing it for real needs a live + * LLM run and a coordinator tick; the e2e stack has neither. Its own + * transition is covered against real Postgres by + * `aiOperatorServiceRecoveryE2E.integration.test.ts`. + * + * WHAT THIS SPEC CANNOT REACH, DELIBERATELY + * - `completed` + `verified_resolved`. That needs a connected agent to + * execute the restart and an independent device read to verify it, and the + * e2e stack runs no agent — `tests/script-cancel.spec.ts` records the same + * limit ("the command this creates is never delivered"). Asserting it here + * would mean faking the device, which would prove nothing about W08. The + * execute→verify→document tail is covered by the W06 integration suite. + */ + +function stackDescriptor(): { pgContainer?: string; redisContainer?: string } | null { + const p = process.env.E2E_STACK_FILE ?? path.resolve(__dirname, '../..', '.breeze-stack.json'); + return existsSync(p) ? (JSON.parse(readFileSync(p, 'utf8')) as { pgContainer?: string; redisContainer?: string }) : null; +} + +/** + * Clears the per-email login rate limiter. + * + * globalSetup clears it once, for its own single login. This spec logs in + * TWICE more (that is the point — two independent sessions), and on a rerun + * those add up until the limiter answers 429 and the login form simply never + * navigates. Clearing before each login keeps the spec rerunnable. + */ +function clearLoginRateLimit(): void { + const container = process.env.E2E_REDIS_CONTAINER ?? stackDescriptor()?.redisContainer ?? 'breeze-redis'; + const args = ['exec', container, 'redis-cli']; + if (process.env.REDIS_PASSWORD) args.push('-a', process.env.REDIS_PASSWORD, '--no-auth-warning'); + args.push('EVAL', "local k=redis.call('KEYS','login:*'); for _,v in ipairs(k) do redis.call('DEL',v) end; return #k", '0'); + try { + execFileSync('docker', args, { stdio: 'ignore' }); + } catch { + // Non-fatal: the login below surfaces a clearer failure if it really is limited. + } +} + +function pgContainer(): string { + return process.env.E2E_PG_CONTAINER ?? stackDescriptor()?.pgContainer ?? 'breeze-postgres'; +} + +function psql(sql: string): string { + return execFileSync( + 'docker', + ['exec', '-i', pgContainer(), 'psql', '-U', 'breeze', '-d', 'breeze', '-t', '-A', '-v', 'ON_ERROR_STOP=1', '-c', sql], + { encoding: 'utf8' }, + ).trim(); +} + +function psqlFile(file: string, vars: Record = {}): string { + const args = ['exec', '-i', pgContainer(), 'psql', '-U', 'breeze', '-d', 'breeze', '-t', '-A', '-v', 'ON_ERROR_STOP=1']; + for (const [k, v] of Object.entries(vars)) args.push('-v', `${k}=${v}`); + args.push('-f', '-'); + return execFileSync('docker', args, { + encoding: 'utf8', + input: readFileSync(path.resolve(__dirname, '..', file), 'utf8'), + }); +} + +/** Seeds the one enabled AI agent the admission route resolves for the org. */ +function seedOperatorAgent(): void { + psqlFile('seed-operator-agent.sql'); +} + +/** Seeds the pending approval the delegated task waits on; returns its id. */ +function seedTaskApproval(taskId: string): string { + const out = psqlFile('seed-operator-task-approval.sql', { task_id: taskId }); + const id = /APPROVAL_ID=([0-9a-f-]{36})/.exec(out)?.[1]; + if (!id) throw new Error(`seed did not report an APPROVAL_ID:\n${out}`); + return id; +} + +async function login(page: Page): Promise { + clearLoginRateLimit(); + // Mirrors global-setup.ts exactly: the /login route (not /auth) is the one + // that lands on `/` for the seeded admin, and it is the login path the whole + // suite already relies on. + await page.goto('/login'); + await page.getByTestId('login-email-input').waitFor({ timeout: 30_000 }); + // A fill that lands before the Astro island hydrates is silently dropped + // (see pages/hydration.ts), so wait for React to attach to the form first. + await page.waitForFunction(() => { + const form = document.querySelector('form'); + return !!form && Object.keys(form).some((k) => k.startsWith('__reactFiber$')); + }, undefined, { timeout: 30_000 }); + await page.getByTestId('login-email-input').fill(process.env.E2E_ADMIN_EMAIL!); + await page.getByTestId('login-password-input').fill(process.env.E2E_ADMIN_PASSWORD!); + await page.getByTestId('login-submit').click(); + await page.waitForURL('/', { timeout: 30_000 }); +} + +const DEVICE_ID = process.env.E2E_MACOS_DEVICE_ID ?? process.env.E2E_WINDOWS_DEVICE_ID; +const SERVICE_NAME = 'e2e-operator-svc'; + +test.describe.configure({ mode: 'serial' }); +test.beforeEach(clearRefreshState); + +test.describe('AI Operator: delegate, close the browser, approve from a new session', () => { + test.skip(!DEVICE_ID, 'needs E2E_MACOS_DEVICE_ID (or E2E_WINDOWS_DEVICE_ID) for a seeded device'); + + test('a delegated task outlives its creating browser session and is approvable from another', async ({ browser }) => { + test.setTimeout(240_000); + + seedOperatorAgent(); + + // ---- Session A: delegate from the device page. + const ctxA: BrowserContext = await browser.newContext(); + const pageA = await ctxA.newPage(); + let taskId: string; + let approverCredentials: unknown[] = []; + try { + await login(pageA); + + const delegate = new DelegateToOperatorAction(pageA); + await delegate.gotoDevice(DEVICE_ID!); + await delegate.openDialog(); + + const admitted = await delegate.confirm(SERVICE_NAME); + expect(admitted.status, 'admission must answer 202 Accepted').toBe(202); + taskId = admitted.taskId; + + // The action must land on the task the SERVER created, not one the + // client invented. + await pageA.waitForURL(new RegExp(`/operator/tasks/${taskId}`), { timeout: 30_000 }); + const detailA = new OperatorTaskPage(pageA); + await detailA.waitForLoaded(); + await expect(detailA.state()).toBeVisible(); + await expect(detailA.target()).toBeVisible(); + // Nothing has run yet: admission creates a task, never an operation. + await expect(detailA.operationsEmpty()).toBeVisible(); + + // The row is real, is this org's, and carries the client key the button + // minted — that key is the whole basis of the replay guarantee below. + const [rowCount, state, hasKey, requesterMatches] = psql( + `SELECT count(*)::text, max(t.state), (max(t.client_idempotency_key) IS NOT NULL)::text, + (max(u.email) = '${process.env.E2E_ADMIN_EMAIL}')::text + FROM ai_operator_tasks t + LEFT JOIN users u ON u.id = t.requester_user_id + WHERE t.id = '${taskId}'`, + ).split('|'); + expect(rowCount, 'exactly one task row').toBe('1'); + expect(state, 'a freshly admitted task is queued').toBe('queued'); + expect(hasKey, 'the client idempotency key must be persisted').toBe('true'); + expect(requesterMatches, 'the task must be attributed to the delegating user').toBe('true'); + + // ---- Idempotent replay: the identical request (same client key) must + // answer the same task id and must NOT create a second task. Replayed + // through the context's request client with the captured headers so it + // is byte-for-byte the request the button sent. + const replay = await ctxA.request.post(admitted.request.url, { + headers: { + 'content-type': 'application/json', + authorization: admitted.request.headers['authorization'] ?? '', + }, + data: admitted.request.body, + }); + expect(replay.status(), 'a replayed admission is still accepted').toBe(202); + expect((await replay.json()).taskId, 'a replay must resolve to the SAME task').toBe(taskId); + + const key = psql(`SELECT client_idempotency_key FROM ai_operator_tasks WHERE id = '${taskId}'`); + expect( + psql(`SELECT count(*)::text FROM ai_operator_tasks WHERE client_idempotency_key = '${key}'`), + 'the replay must not have created a second task', + ).toBe('1'); + + // Register the approver device HERE, at the very end of session A, not + // in session B: the ceremony mints its own access token through + // /auth/refresh, and that rotation revokes the JTI the page's session + // store is holding — the session it runs in is unusable afterwards + // ("Your session expired"). Session A has no in-app work left; session B + // must have a clean session to click Approve with. The user's registered + // key persists in the DB across both, which is the real-world shape + // anyway (you enrol a key once, then approve from wherever). + const authenticatorA = await addVirtualAuthenticator(pageA); + const registered = await registerApproverDevice( + pageA, + process.env.E2E_ADMIN_PASSWORD!, + `E2E Operator Approver ${Date.now()}`, + ); + expect(registered, `approver-device registration failed: ${JSON.stringify(registered)}`).toMatchObject({ ok: true }); + approverCredentials = await exportCredentials(authenticatorA); + expect(approverCredentials.length, 'the enrolled key must be exportable to the next session').toBeGreaterThan(0); + await removeVirtualAuthenticator(authenticatorA); + } finally { + // The point of the scenario: the session that created the work is GONE — + // context, cookies, in-memory access token and all. + await ctxA.close(); + } + + // ---- The operator reaches the approval gate while nobody is watching. + const approvalId = seedTaskApproval(taskId!); + expect(psql(`SELECT status FROM approval_requests WHERE id = '${approvalId}'`)).toBe('pending'); + + // ---- Session B: a brand-new browser session approves it. + const ctxB: BrowserContext = await browser.newContext(); + const pageB = await ctxB.newPage(); + try { + // Installed before any navigator.credentials call: the inbox always + // requires a WebAuthn proof to approve (lib/intentApprovals.ts). The + // credential it signs with was enrolled in session A — the virtual + // authenticator is per-context, but so is a real laptop's, and CDP's + // resident key is what makes the same key usable here. + const authenticator = await addVirtualAuthenticator(pageB); + await importCredentials(authenticator, approverCredentials); + await login(pageB); + + const inbox = new ApprovalsInboxPage(pageB); + await inbox.goto(); + expect(await inbox.approve(approvalId), 'the approve ceremony must be accepted').toBe(200); + + // The HTTP 200 only says "not rejected". What matters is that the + // decision released the intent the TASK is waiting on. + await expect + .poll( + () => + psql( + `SELECT ar.status || '|' || ai.status || '|' || (ai.task_id = '${taskId}')::text + FROM approval_requests ar + JOIN action_intents ai ON ai.id = ar.intent_id + WHERE ar.id = '${approvalId}'`, + ), + { timeout: 30_000, message: 'approval and its task-linked intent must both settle approved' }, + ) + .toBe('approved|approved|true'); + + // And the task itself is still addressable from this new session. + const detailB = new OperatorTaskPage(pageB); + await detailB.goto(taskId!); + await expect(detailB.state()).toBeVisible(); + await expect(detailB.error()).toHaveCount(0); + + await removeVirtualAuthenticator(authenticator); + } finally { + await ctxB.close(); + } + }); +}); diff --git a/e2e-tests/webauthn.ts b/e2e-tests/webauthn.ts new file mode 100644 index 0000000000..2e4e14a417 --- /dev/null +++ b/e2e-tests/webauthn.ts @@ -0,0 +1,166 @@ +import type { CDPSession, Page } from '@playwright/test'; + +/** + * Chrome virtual-authenticator helpers for specs that must clear the L3 + * approval gate in a real browser. + * + * The approvals inbox NEVER submits an approve without a WebAuthn assertion + * (`apps/web/src/lib/intentApprovals.ts` — "we do NOT submit without a proof: + * the self-approve gate requires L3"), so any spec that clicks Approve in the + * UI needs a registered approver device and an authenticator that can sign for + * it. Extracted from `tests/intent-self-approve.spec.ts`, which proved the + * recipe end to end and remains the spec that tests the gate ITSELF; callers + * here just need the ceremony to succeed so they can test something else. + */ + +export type VirtualAuthenticator = { cdp: CDPSession; authenticatorId: string }; + +/** Must be installed BEFORE the page makes any navigator.credentials call. */ +export async function addVirtualAuthenticator(page: Page): Promise { + const cdp = await page.context().newCDPSession(page); + await cdp.send('WebAuthn.enable'); + const { authenticatorId } = await cdp.send('WebAuthn.addVirtualAuthenticator', { + options: { + protocol: 'ctap2', + ctap2Version: 'ctap2_1', + transport: 'internal', // platform authenticator (Touch ID / Hello) + hasResidentKey: true, + hasUserVerification: true, + isUserVerified: true, // auto-satisfy UV so no human touch is needed + automaticPresenceSimulation: true, + }, + }); + return { cdp, authenticatorId }; +} + +export async function removeVirtualAuthenticator(auth: VirtualAuthenticator): Promise { + await auth.cdp.send('WebAuthn.removeVirtualAuthenticator', { authenticatorId: auth.authenticatorId }); +} + +/** + * The credentials the virtual authenticator currently holds. + * + * A virtual authenticator belongs to ONE browser context, so a key enrolled in + * one context cannot sign in another — the same as a real laptop's platform + * key. Exporting and re-importing is what lets a spec enrol in one session and + * assert in the next without pretending the key travelled by magic. + */ +export async function exportCredentials(auth: VirtualAuthenticator): Promise { + const { credentials } = await auth.cdp.send('WebAuthn.getCredentials', { + authenticatorId: auth.authenticatorId, + }); + return credentials as unknown[]; +} + +export async function importCredentials(auth: VirtualAuthenticator, credentials: unknown[]): Promise { + for (const credential of credentials) { + await auth.cdp.send('WebAuthn.addCredential', { + authenticatorId: auth.authenticatorId, + credential: credential as never, + }); + } +} + +export type RegistrationOutcome = { ok: boolean; stage: string; status: number; body: string }; + +/** + * Registers an approver device for the logged-in user, in the page, against + * whatever authenticator is installed. + * + * Runs inside `page.evaluate` because the ceremony must happen in the browser: + * `navigator.credentials.create` is what the virtual authenticator answers. + * The access token is minted the same way the app does (access tokens live in + * memory only; the refresh cookie restores them), so these calls carry a real + * Bearer header. + */ +export async function registerApproverDevice(page: Page, password: string, label: string): Promise { + return page.evaluate( + async ({ adminPassword, deviceLabel }) => { + const csrf = document.cookie + .split('; ') + .find((c) => c.startsWith('breeze_csrf_token=')) + ?.split('=')[1]; + const refreshRes = await fetch('/api/v1/auth/refresh', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...(csrf ? { 'x-breeze-csrf': decodeURIComponent(csrf) } : {}), + }, + credentials: 'include', + body: JSON.stringify({}), + }); + if (!refreshRes.ok) { + return { ok: false, stage: 'refresh', status: refreshRes.status, body: await refreshRes.text() }; + } + const { tokens } = await refreshRes.json(); + const accessToken: string = tokens?.accessToken; + if (!accessToken) return { ok: false, stage: 'refresh', status: 200, body: 'no accessToken in refresh body' }; + + const api = (path: string, body?: unknown) => + fetch(`/api/v1${path}`, { + method: 'POST', + headers: { 'Content-Type': 'application/json', Authorization: `Bearer ${accessToken}` }, + credentials: 'include', + body: body === undefined ? undefined : JSON.stringify(body), + }); + + // #2707: approver-device registration is GRANT-gated, not + // password-per-call. Without a `registerGrantId` the options route + // answers 403 `register_step_up_required`, whatever else the body says. + const grantRes = await api('/authenticator/register-grant', { currentPassword: adminPassword }); + if (!grantRes.ok) return { ok: false, stage: 'register-grant', status: grantRes.status, body: await grantRes.text() }; + const { registerGrantId } = await grantRes.json(); + if (!registerGrantId) return { ok: false, stage: 'register-grant', status: 200, body: 'no registerGrantId in body' }; + + const optRes = await api('/authenticator/devices/webauthn/options', { + currentPassword: adminPassword, + registerGrantId, + }); + if (!optRes.ok) return { ok: false, stage: 'options', status: optRes.status, body: await optRes.text() }; + const optJson = await optRes.json(); + const options = optJson.options ?? optJson.optionsJSON ?? optJson; + + const b64uToBuf = (s: string) => { + const pad = s.replace(/-/g, '+').replace(/_/g, '/'); + const bin = atob(pad + '='.repeat((4 - (pad.length % 4)) % 4)); + return Uint8Array.from(bin, (c) => c.charCodeAt(0)).buffer; + }; + const bufToB64u = (b: ArrayBuffer) => + btoa(String.fromCharCode(...new Uint8Array(b))) + .replace(/\+/g, '-') + .replace(/\//g, '_') + .replace(/=+$/, ''); + + const cred = (await navigator.credentials.create({ + publicKey: { + ...options, + challenge: b64uToBuf(options.challenge), + user: { ...options.user, id: b64uToBuf(options.user.id) }, + excludeCredentials: (options.excludeCredentials ?? []).map((c: { id: string }) => ({ + ...c, + id: b64uToBuf(c.id), + })), + }, + })) as PublicKeyCredential | null; + if (!cred) return { ok: false, stage: 'create', status: 0, body: 'null credential' }; + + const att = cred.response as AuthenticatorAttestationResponse; + const verifyRes = await api('/authenticator/devices/webauthn/verify', { + label: deviceLabel, + registerGrantId, + response: { + id: cred.id, + rawId: bufToB64u(cred.rawId), + type: cred.type, + clientExtensionResults: cred.getClientExtensionResults(), + response: { + clientDataJSON: bufToB64u(att.clientDataJSON), + attestationObject: bufToB64u(att.attestationObject), + }, + }, + }); + return { ok: verifyRes.ok, stage: 'verify', status: verifyRes.status, body: await verifyRes.text() }; + }, + { adminPassword: password, deviceLabel: label }, + ); +} From 510bc88ef0268bf288a1e6e13ce8abc6f3fb4eeb Mon Sep 17 00:00:00 2001 From: Todd Hebebrand Date: Tue, 8 Sep 2026 05:14:38 -0600 Subject: [PATCH 4/4] fix(ai-operator): surface the idempotency invariant break, guard the alert call site, drop an unregistered eslint-disable (W08 of #5205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review findings from the PR #5272 pass. 1. Silent-failure review (HIGH): the ON CONFLICT read-back miss in admitServiceRecoveryTask returned refusal 'invalid_input', which the route turns into a 422 reading to the technician as "your input was wrong" — with nothing logged anywhere. Nothing in a caller's request can cause it (index and read-back are both org+key scoped), so it is a broken invariant: a concurrent erasure racing admission, or a future change desynchronising the index from the lookup. Now throws, matching operationService.ts's dispatch-claim cardinality check, so it surfaces as a 500 the top-level handler logs instead of a consistency break with no breadcrumb. The caller-supplied key is not logged. 2. Test-coverage review: nothing proved the alert call site passes the ALERT's own orgId rather than the globally selected one — a swap would compile, pass every existing test, and aim a live remediation action at the wrong tenant. AlertDetailPage.delegateToOperator.test.tsx now asserts it with the org store deliberately seeded to a DIFFERENT id, plus the status gating (rendered for active/acknowledged, hidden for resolved/dismissed). Red-first verified: reverting the wiring to the store value fails the test with `expected 'org-globally-selected-9999' to be 'org-alert-owner-1111'`. 3. Lint: `// eslint-disable-next-line react-hooks/exhaustive-deps` was itself the lint error — that rule is not registered in this repo's config. Removed; the reason the dep is excluded stays as a plain comment. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01YGPxXGTWpJKUNbEdR3TZEL --- .../src/services/aiOperator/taskService.ts | 22 ++- .../aiOperator/DelegateToOperatorButton.tsx | 5 +- ...lertDetailPage.delegateToOperator.test.tsx | 155 ++++++++++++++++++ 3 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 apps/web/src/components/alerts/AlertDetailPage.delegateToOperator.test.tsx diff --git a/apps/api/src/services/aiOperator/taskService.ts b/apps/api/src/services/aiOperator/taskService.ts index c6b610324f..0afdd4a56a 100644 --- a/apps/api/src/services/aiOperator/taskService.ts +++ b/apps/api/src/services/aiOperator/taskService.ts @@ -247,13 +247,21 @@ export async function admitServiceRecoveryTask( .limit(1); if (!existing) { - // Structurally unreachable: DO NOTHING with a non-null key fires only - // on that index. Refusing beats inventing a task id. - return { - ok: false as const, - refusal: 'invalid_input' as const, - detail: 'admission conflicted but no existing task was found for the idempotency key', - }; + // A conflict fired but the row it must point at is not there. Nothing + // in the caller's request can cause this — the index is org+key scoped + // and so is this read — so it is a broken invariant (a concurrent + // erasure racing admission, or a future change that desynchronises the + // index from this lookup), never a client-format problem. + // + // THROW rather than refuse: a refusal would be reclassified by the + // route as a 422 that reads to the technician as "your input was + // wrong", and would leave no trace anywhere for anyone to investigate + // the actual consistency break. Same convention as + // `operationService.ts`'s dispatch-claim cardinality check. The key + // itself is caller-supplied and never logged. + throw new Error( + `[aiOperator] admission conflicted on the client idempotency key but no existing task was found for org ${input.orgId}`, + ); } return { ok: true as const, taskId: existing.id, replayed: true }; diff --git a/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx b/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx index 3105f7dff4..efea2ce3ef 100644 --- a/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx +++ b/apps/web/src/components/aiOperator/DelegateToOperatorButton.tsx @@ -64,8 +64,9 @@ export function DelegateToOperatorButton({ setTouched(false); } // defaultServiceName intentionally excluded: only re-read when the dialog - // opens, not on every prop change while it's open. - // eslint-disable-next-line react-hooks/exhaustive-deps + // opens, not on every prop change while it's open. (No eslint-disable for + // react-hooks/exhaustive-deps here — the rule is not registered in this + // repo's config, so disabling it IS itself a lint error.) }, [open]); if (!loaded || !enabled) return null; diff --git a/apps/web/src/components/alerts/AlertDetailPage.delegateToOperator.test.tsx b/apps/web/src/components/alerts/AlertDetailPage.delegateToOperator.test.tsx new file mode 100644 index 0000000000..025aa68694 --- /dev/null +++ b/apps/web/src/components/alerts/AlertDetailPage.delegateToOperator.test.tsx @@ -0,0 +1,155 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import '../../lib/i18n'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; + +/** + * Regression test for the DelegateToOperatorButton wiring in + * AlertDetailPage.tsx (W08 of #5205, #5246). + * + * Spec §5.1: "changing global organization context while drafting cannot + * retarget the task" — the button must always target the ALERT's own org + * (`alert.orgId`), never the globally selected org from `useOrgStore`. The + * failure this guards against: someone swaps `alert.orgId` for the org-store + * value at the call site — it would compile and pass every other existing + * test today, since nothing else in the suite exercises this prop. + * + * The real DelegateToOperatorButton is stubbed out so this test reads the + * props the call site actually passed (via data attributes) instead of + * asserting on rendered dialog text, and so it sidesteps the + * useAiOperatorTasksGate feature-flag gate inside the real component. + */ +vi.mock('../remediation/RemediationSuggestionsPanel', () => ({ default: () => null })); + +vi.mock('../aiOperator/DelegateToOperatorButton', () => ({ + DelegateToOperatorButton: (props: { + orgId: string; + deviceId: string; + deviceLabel: string; + source: { kind: string; id: string }; + }) => ( + + ), +})); + +const fetchWithAuth = vi.fn(); + +vi.mock('../../stores/auth', () => ({ + fetchWithAuth: (...args: unknown[]) => fetchWithAuth(...args), + registerOrgIdProvider: vi.fn(), +})); + +import AlertDetailPage from './AlertDetailPage'; +import { useOrgStore } from '@/stores/orgStore'; + +// Deliberately DIFFERENT from every alert's orgId below, so the assertion +// fails if the call site is ever swapped to read the org-store value instead +// of alert.orgId. +const GLOBAL_STORE_ORG_ID = 'org-globally-selected-9999'; +const ALERT_ORG_ID = 'org-alert-owner-1111'; + +type RawAlert = { + id: string; + title: string; + message: string; + severity: string; + status: string; + deviceId: string; + deviceName: string; + orgId: string; + triggeredAt: string; +}; + +const baseAlert: RawAlert = { + id: 'a-delegate-1', + title: 'CPU high', + message: 'CPU over 90%', + severity: 'critical', + status: 'active', + deviceId: 'd-1', + deviceName: 'web-01', + orgId: ALERT_ORG_ID, + triggeredAt: '2026-08-24T16:00:00Z', +}; + +function mockFetch(alert: RawAlert) { + fetchWithAuth.mockImplementation((url: string) => { + if (url.endsWith('/tickets')) { + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve({ data: [] }), + }); + } + return Promise.resolve({ + ok: true, + status: 200, + json: () => Promise.resolve(alert), + }); + }); +} + +function renderPage(alert: RawAlert) { + mockFetch(alert); + return render(); +} + +beforeEach(() => { + fetchWithAuth.mockReset(); + useOrgStore.setState({ + currentOrgId: GLOBAL_STORE_ORG_ID, + serviceManagementMode: 'native', + } as never); +}); + +describe('AlertDetailPage — Delegate to Operator wiring (#5246, spec §5.1)', () => { + it('targets the ALERT\'s own org, not the globally selected org', async () => { + renderPage(baseAlert); + + const stub = await screen.findByTestId('delegate-stub'); + + expect(stub.getAttribute('data-org-id')).toBe(ALERT_ORG_ID); + expect(stub.getAttribute('data-org-id')).not.toBe(GLOBAL_STORE_ORG_ID); + expect(stub.getAttribute('data-device-id')).toBe(baseAlert.deviceId); + }); + + it('renders for an active alert', async () => { + renderPage({ ...baseAlert, status: 'active' }); + await screen.findByTestId('delegate-stub'); + }); + + it('renders for an acknowledged alert', async () => { + renderPage({ ...baseAlert, status: 'acknowledged' }); + await screen.findByTestId('delegate-stub'); + }); + + it('does NOT render for a resolved alert', async () => { + renderPage({ ...baseAlert, status: 'resolved' }); + + // Something else from the page must render first, or a query that + // always finds nothing (because the page never mounted) would pass + // vacuously. + await screen.findByRole('heading', { name: baseAlert.title }); + await waitFor(() => { + expect(screen.queryByTestId('delegate-stub')).toBeNull(); + }); + }); + + it('does NOT render for a dismissed (non-actionable) alert', async () => { + renderPage({ ...baseAlert, status: 'dismissed' }); + + await screen.findByRole('heading', { name: baseAlert.title }); + await waitFor(() => { + expect(screen.queryByTestId('delegate-stub')).toBeNull(); + }); + }); +});