diff --git a/.changeset/v2-p2-execution-approve-separate.md b/.changeset/v2-p2-execution-approve-separate.md new file mode 100644 index 00000000..1c7dc759 --- /dev/null +++ b/.changeset/v2-p2-execution-approve-separate.md @@ -0,0 +1,7 @@ +--- +"@semantask/services": minor +"@semantask/web": patch +"@semantask/task-worker": patch +--- + +Phase 2 PR5 — Explicit manager “Allow AI tools” / request-execution path, distinct from WorkSuggestion accept, reusing TaskAction approvals. diff --git a/apps/task-worker/index.ts b/apps/task-worker/index.ts index e2ed4c93..94e45537 100644 --- a/apps/task-worker/index.ts +++ b/apps/task-worker/index.ts @@ -23,6 +23,10 @@ import { recordSuggestOnlyExecutionEnqueueAttempt, shouldFailClosedOnLeakedExecution, } from "@semantask/services/task-execution-enqueue.service"; +import { + resolveSuggestOnlyPolicyOverride, + shouldSkipSuggestOnlyIngressFailClosed, +} from "./services/suggest-only-execution-gate.js"; import { assertExecutionQuotas, OrgQuotaExceededError } from "@semantask/services/organization-quota.service"; import { getOrganizationById } from "@semantask/services/organization.service"; import { GLOBAL_EXECUTION_CONFIDENCE_BASELINE } from "./services/execution-confidence.js"; @@ -237,6 +241,8 @@ type TaskExecutionRequestedPayload = { confidence?: number; needsApproval?: boolean; semanticType?: MessageSemanticType; + explicitManagerRequest?: boolean; + humanApprovedExecution?: boolean; }; type TaskExecutionApprovedPayload = { @@ -246,6 +252,7 @@ type TaskExecutionApprovedPayload = { approvedByType?: "user" | "agent" | "system"; approvedById?: string | null; reason?: string; + humanApprovedExecution?: boolean; }; type TaskSocketBridgePayload = { @@ -336,6 +343,8 @@ function normalizeTaskExecutionRequestedPayload(payload: Record needsApproval: typeof payload.needsApproval === "boolean" ? payload.needsApproval : false, + explicitManagerRequest: payload.explicitManagerRequest === true, + humanApprovedExecution: payload.humanApprovedExecution === true, }; } @@ -655,7 +664,10 @@ async function processTaskExecutionRequested(payload: NormalizedTaskExecutionReq // Defense-in-depth: with suggestion ingress, refuse tools if a leaked // task.execution.requested arrives under suggest_only + SUGGESTION_BLOCK_EXEC. - if (shouldFailClosedOnLeakedExecution(effectiveExecutionMode)) { + // Explicit manager request / human-approved re-entry are not leaks (S2.4). + const skipSuggestOnlyFailClosed = shouldSkipSuggestOnlyIngressFailClosed(payload); + + if (shouldFailClosedOnLeakedExecution(effectiveExecutionMode) && !skipSuggestOnlyFailClosed) { const blockedReason = "Execution blocked: suggest_only ingress forbids tool execution."; recordSuggestOnlyExecutionEnqueueAttempt({ taskId: payload.taskId, @@ -869,12 +881,28 @@ async function processTaskExecutionRequested(payload: NormalizedTaskExecutionReq || reason.includes("no valid recipients") || reason.includes("No executable action") ); - const requiresApproval = policyDecision.outcome === "approval_required"; + const modeDeniedBySuggestOnly = policyDecision.reasons.some((reason) => + reason.includes("execution_mode:suggest_only") + ); + // Explicit manager request under suggest_only → approval_required (not hard block). + // Human-approved re-entry under suggest_only may proceed past mode denial. + const { forceApprovalForExplicit, bypassSuggestOnlyAfterHumanApproval } = + resolveSuggestOnlyPolicyOverride({ + policyOutcome: policyDecision.outcome, + modeDeniedBySuggestOnly, + explicitManagerRequest: payload.explicitManagerRequest, + needsApproval: payload.needsApproval, + humanApprovedExecution: payload.humanApprovedExecution, + }); - if (policyDecision.outcome === "blocked" || unsafe) { - const modeDenied = policyDecision.reasons.some((reason) => - reason.includes("execution_mode:suggest_only") - ); + const requiresApproval = policyDecision.outcome === "approval_required" || forceApprovalForExplicit; + + // Human-approved suggest_only re-entry may bypass mode denial only — never + // unrelated unsafe policy denials (domains, recipients, etc.). + const bypassSuggestOnlyModeDenial = bypassSuggestOnlyAfterHumanApproval && !unsafe; + + if ((policyDecision.outcome === "blocked" || unsafe) && !forceApprovalForExplicit && !bypassSuggestOnlyModeDenial) { + const modeDenied = modeDeniedBySuggestOnly; const blockedReason = unsafe ? "Execution blocked by policy: action marked unsafe." : (policyDecision.reasons.join(" ") || "Execution blocked by policy."); @@ -1209,6 +1237,9 @@ async function processTaskExecutionApproved(payload: TaskExecutionApprovedPayloa : 1, // Human approval satisfies the approval requirement gate, but policy is still evaluated before execution. needsApproval: false, + // S2.4: only the explicit manager "Allow AI tools" path may bypass suggest_only after approval. + explicitManagerRequest: patchAfter.explicitManagerRequest === true, + humanApprovedExecution: patchAfter.explicitManagerRequest === true, }; await processTaskExecutionRequested(normalizedPayload); diff --git a/apps/task-worker/services/suggest-only-execution-gate.ts b/apps/task-worker/services/suggest-only-execution-gate.ts new file mode 100644 index 00000000..1ce5fc4d --- /dev/null +++ b/apps/task-worker/services/suggest-only-execution-gate.ts @@ -0,0 +1,53 @@ +/** + * S2.4 suggest_only exceptions for explicit manager request / human-approved re-entry. + * Pure helpers so unit tests can cover the gate without booting the full worker. + */ + +export type SuggestOnlyExecutionPayloadFlags = { + humanApprovedExecution?: boolean; + explicitManagerRequest?: boolean; + needsApproval?: boolean; +}; + +/** + * Skip the leaked-ingress fail-closed path only for the explicit manager + * "Allow AI tools" request (or its human-approved re-entry) — not for generic + * needsApproval / silent accept leaks. + */ +export function shouldSkipSuggestOnlyIngressFailClosed( + payload: SuggestOnlyExecutionPayloadFlags +): boolean { + return payload.humanApprovedExecution === true + || payload.explicitManagerRequest === true; +} + +export type SuggestOnlyPolicyOverrideInput = { + policyOutcome: string; + modeDeniedBySuggestOnly: boolean; + explicitManagerRequest?: boolean; + needsApproval?: boolean; + humanApprovedExecution?: boolean; +}; + +/** + * Under suggest_only mode denial: + * - explicit manager request (not yet human-approved) → force approval_required + * - humanApprovedExecution after that explicit path → bypass mode denial + * (grants/policy still apply elsewhere) + */ +export function resolveSuggestOnlyPolicyOverride(input: SuggestOnlyPolicyOverrideInput): { + forceApprovalForExplicit: boolean; + bypassSuggestOnlyAfterHumanApproval: boolean; +} { + const forceApprovalForExplicit = input.policyOutcome === "blocked" + && input.modeDeniedBySuggestOnly + && input.explicitManagerRequest === true + && input.humanApprovedExecution !== true; + + const bypassSuggestOnlyAfterHumanApproval = input.policyOutcome === "blocked" + && input.modeDeniedBySuggestOnly + && input.humanApprovedExecution === true + && input.explicitManagerRequest === true; + + return { forceApprovalForExplicit, bypassSuggestOnlyAfterHumanApproval }; +} diff --git a/apps/task-worker/tests/suggest-only-execution-gate.test.ts b/apps/task-worker/tests/suggest-only-execution-gate.test.ts new file mode 100644 index 00000000..0e6182d7 --- /dev/null +++ b/apps/task-worker/tests/suggest-only-execution-gate.test.ts @@ -0,0 +1,98 @@ +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { + resolveSuggestOnlyPolicyOverride, + shouldSkipSuggestOnlyIngressFailClosed, +} from "../services/suggest-only-execution-gate.js"; + +test("leaked suggest_only events do not skip fail-closed", () => { + assert.equal(shouldSkipSuggestOnlyIngressFailClosed({}), false); + assert.equal( + shouldSkipSuggestOnlyIngressFailClosed({ + explicitManagerRequest: false, + needsApproval: false, + humanApprovedExecution: false, + }), + false + ); +}); + +test("needsApproval alone does not skip ingress fail-closed", () => { + assert.equal( + shouldSkipSuggestOnlyIngressFailClosed({ needsApproval: true }), + false + ); +}); + +test("explicit manager request skips ingress fail-closed", () => { + assert.equal( + shouldSkipSuggestOnlyIngressFailClosed({ explicitManagerRequest: true }), + true + ); +}); + +test("human-approved re-entry skips ingress fail-closed", () => { + assert.equal( + shouldSkipSuggestOnlyIngressFailClosed({ humanApprovedExecution: true }), + true + ); +}); + +test("suggest_only + explicit → force approval_required (pending)", () => { + const result = resolveSuggestOnlyPolicyOverride({ + policyOutcome: "blocked", + modeDeniedBySuggestOnly: true, + explicitManagerRequest: true, + needsApproval: true, + humanApprovedExecution: false, + }); + assert.equal(result.forceApprovalForExplicit, true); + assert.equal(result.bypassSuggestOnlyAfterHumanApproval, false); +}); + +test("suggest_only + needsApproval without explicit does not force approval", () => { + const result = resolveSuggestOnlyPolicyOverride({ + policyOutcome: "blocked", + modeDeniedBySuggestOnly: true, + explicitManagerRequest: false, + needsApproval: true, + humanApprovedExecution: false, + }); + assert.equal(result.forceApprovalForExplicit, false); + assert.equal(result.bypassSuggestOnlyAfterHumanApproval, false); +}); + +test("suggest_only + humanApproved without explicit does not bypass", () => { + const result = resolveSuggestOnlyPolicyOverride({ + policyOutcome: "blocked", + modeDeniedBySuggestOnly: true, + explicitManagerRequest: false, + needsApproval: false, + humanApprovedExecution: true, + }); + assert.equal(result.forceApprovalForExplicit, false); + assert.equal(result.bypassSuggestOnlyAfterHumanApproval, false); +}); + +test("suggest_only + explicit + humanApproved → bypass mode denial", () => { + const result = resolveSuggestOnlyPolicyOverride({ + policyOutcome: "blocked", + modeDeniedBySuggestOnly: true, + explicitManagerRequest: true, + needsApproval: false, + humanApprovedExecution: true, + }); + assert.equal(result.forceApprovalForExplicit, false); + assert.equal(result.bypassSuggestOnlyAfterHumanApproval, true); +}); + +test("non-suggest_only blocks do not force approval or bypass", () => { + const result = resolveSuggestOnlyPolicyOverride({ + policyOutcome: "blocked", + modeDeniedBySuggestOnly: false, + explicitManagerRequest: true, + humanApprovedExecution: true, + }); + assert.equal(result.forceApprovalForExplicit, false); + assert.equal(result.bypassSuggestOnlyAfterHumanApproval, false); +}); diff --git a/apps/web/app/admin/task-approvals/page.tsx b/apps/web/app/admin/task-approvals/page.tsx index 7f709624..3495d237 100644 --- a/apps/web/app/admin/task-approvals/page.tsx +++ b/apps/web/app/admin/task-approvals/page.tsx @@ -42,7 +42,9 @@ export default function AdminTaskApprovalsPage() { setLoading(true); setError(null); try { - const response = await getTaskApprovals(conversationId.trim() || undefined); + const response = await getTaskApprovals({ + conversationId: conversationId.trim() || undefined, + }); setApprovals(response.approvals); setCommentsById((current) => { const next = { ...current }; diff --git a/apps/web/app/api/task-approvals/route.ts b/apps/web/app/api/task-approvals/route.ts index 56029e0e..7c0848b9 100644 --- a/apps/web/app/api/task-approvals/route.ts +++ b/apps/web/app/api/task-approvals/route.ts @@ -2,8 +2,24 @@ import { NextRequest, NextResponse } from "next/server"; import { withRequestCorrelation } from "@/lib/observability/with-correlation"; import { z } from "zod"; import { enqueueOutboxEvent } from "@/lib/services/outbox.service"; -import { getPendingApprovalTaskActions, getTaskActionById, updateTaskActionExecutionState } from "@/lib/services/repositories/task.repo"; -import { requireAdminUser } from "@/lib/utils/auth/requireAdminUser"; +import { + getPendingApprovalTaskActions, + getPendingApprovalTaskActionsForOrganization, + getTaskActionById, + updateTaskActionExecutionState, +} from "@/lib/services/repositories/task.repo"; +import { requireAuthUser } from "@/lib/utils/auth/requireAuthUser"; +import { + AuthorizationError, + assertCanDecideTaskExecutionApproval, +} from "@semantask/services/authorization.service"; +import { + assertOrganizationActive, + canManageMembers, + getMembership, +} from "@semantask/services/organization.service"; +import { Conversation } from "@/models/Conversation"; +import TaskModel from "@/models/Task"; const decisionSchema = z.object({ taskActionId: z.string().min(1), @@ -42,20 +58,85 @@ function asRecord(value: unknown): Record { return {}; } +async function resolveOrganizationIdForAction(action: { + conversationId: { toString(): string }; + taskId: { toString(): string }; +}): Promise { + const conversationId = action.conversationId.toString(); + const conversation = await Conversation.findById(conversationId) + .select("organizationId") + .lean<{ organizationId?: { toString(): string } | null }>(); + if (conversation?.organizationId) { + return conversation.organizationId.toString(); + } + + const task = await TaskModel.findById(action.taskId.toString()) + .select("organizationId") + .lean<{ organizationId?: { toString(): string } | null }>(); + return task?.organizationId ? task.organizationId.toString() : null; +} + export async function GET(req: NextRequest) { - const guard = await requireAdminUser(); + const guard = await requireAuthUser(); if (guard.response) return guard.response; const { searchParams } = new URL(req.url); const conversationId = searchParams.get("conversationId") ?? undefined; + const organizationId = searchParams.get("organizationId") ?? undefined; + const isPlatformAdmin = guard.user.role === "admin"; - const actions = await getPendingApprovalTaskActions(conversationId); - return NextResponse.json({ approvals: actions.map(serializeTaskAction) }, { status: 200 }); + try { + if (isPlatformAdmin && !conversationId && !organizationId) { + const actions = await getPendingApprovalTaskActions(); + return NextResponse.json({ approvals: actions.map(serializeTaskAction) }, { status: 200 }); + } + + if (conversationId) { + const conversation = await Conversation.findById(conversationId) + .select("organizationId") + .lean<{ organizationId?: { toString(): string } | null }>(); + const orgId = conversation?.organizationId?.toString() ?? null; + await assertCanDecideTaskExecutionApproval( + guard.user.id, + { conversationId, organizationId: orgId }, + { userRole: guard.user.role, allowAdminBypass: true } + ); + const actions = await getPendingApprovalTaskActions(conversationId); + return NextResponse.json({ approvals: actions.map(serializeTaskAction) }, { status: 200 }); + } + + if (organizationId) { + await assertOrganizationActive(organizationId); + if (!isPlatformAdmin) { + const membership = await getMembership(organizationId, guard.user.id); + if (!membership || !canManageMembers(membership.role)) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } + } + + const actions = await getPendingApprovalTaskActionsForOrganization(organizationId); + return NextResponse.json({ approvals: actions.map(serializeTaskAction) }, { status: 200 }); + } + + return NextResponse.json( + { error: "organizationId or conversationId is required" }, + { status: 400 } + ); + } catch (error) { + if (error instanceof AuthorizationError) { + return NextResponse.json( + { error: error.code === "NOT_FOUND" ? "Not found" : "Forbidden" }, + { status: error.code === "NOT_FOUND" ? 404 : 403 } + ); + } + console.error("GET /api/task-approvals error", error); + return NextResponse.json({ error: "Failed to load approvals" }, { status: 500 }); + } } export async function POST(req: NextRequest) { return withRequestCorrelation(req, async () => { - const guard = await requireAdminUser(); + const guard = await requireAuthUser(); if (guard.response) return guard.response; const parse = decisionSchema.safeParse(await req.json()); @@ -71,7 +152,27 @@ export async function POST(req: NextRequest) { } if (action.executionState !== "approval_pending") { - return NextResponse.json({ error: `Approval request is not pending (state=${action.executionState ?? "null"})` }, { status: 409 }); + return NextResponse.json( + { error: `Approval request is not pending (state=${action.executionState ?? "null"})` }, + { status: 409 } + ); + } + + try { + const organizationId = await resolveOrganizationIdForAction(action); + await assertCanDecideTaskExecutionApproval( + guard.user.id, + { + conversationId: action.conversationId.toString(), + organizationId, + }, + { userRole: guard.user.role, allowAdminBypass: true } + ); + } catch (error) { + if (error instanceof AuthorizationError) { + return NextResponse.json({ error: "Forbidden" }, { status: 403 }); + } + throw error; } if (body.decision === "reject") { @@ -89,6 +190,10 @@ export async function POST(req: NextRequest) { const approvedParameters = body.parameters ?? action.parameters ?? {}; const reviewerComment = body.reviewerComment ?? body.reason ?? "Approved by reviewer."; + const patchAfter = asRecord(action.patch?.after); + const explicitManagerRequest = patchAfter.explicitManagerRequest === true; + // S2.4: only the explicit manager "Allow AI tools" path bypasses suggest_only. + const humanApprovedExecution = explicitManagerRequest; const updated = await updateTaskActionExecutionState({ taskActionId: body.taskActionId, @@ -100,10 +205,12 @@ export async function POST(req: NextRequest) { patch: { before: action.patch?.before ?? null, after: { - ...asRecord(action.patch?.after), + ...patchAfter, approvedParameters, reviewerComment, approvedAt: new Date().toISOString(), + explicitManagerRequest, + humanApprovedExecution, }, }, }); @@ -118,10 +225,11 @@ export async function POST(req: NextRequest) { approvedByType: guard.user.role === "admin" ? "system" : "user", approvedById: guard.user.id, reason: reviewerComment, + humanApprovedExecution, + explicitManagerRequest, }, }); return NextResponse.json({ approval: updated ? serializeTaskAction(updated) : null }, { status: 200 }); - }); -} \ No newline at end of file +} diff --git a/apps/web/app/api/tasks/[id]/request-execution/route.ts b/apps/web/app/api/tasks/[id]/request-execution/route.ts new file mode 100644 index 00000000..f975806f --- /dev/null +++ b/apps/web/app/api/tasks/[id]/request-execution/route.ts @@ -0,0 +1,120 @@ +import { NextRequest, NextResponse } from "next/server"; +import { z } from "zod"; +import { withRequestCorrelation } from "@/lib/observability/with-correlation"; +import { connectToDatabase } from "@/lib/Db/db"; +import { requireAuthUser } from "@/lib/utils/auth/requireAuthUser"; +import { AuthorizationError } from "@semantask/services/authorization.service"; +import { ValidationError, ConflictError } from "@semantask/services/organization-errors"; +import { requestTaskExecution } from "@semantask/services/task-execution-request.service"; + +type RouteContext = { params: Promise<{ id: string }> }; + +const bodySchema = z.object({ + reason: z.string().trim().max(2000).optional(), +}).strict(); + +function serializeTaskAction(action: { + _id: { toString(): string }; + taskId: { toString(): string }; + conversationId: { toString(): string }; + actorType: string; + actorId?: { toString(): string } | null; + actionType: string; + toolName?: string | null; + messageId?: { toString(): string } | null; + parameters?: Record; + executionState?: string | null; + summary?: string | null; + error?: string | null; + patch: unknown; + reason: string; + idempotencyKey: string; + createdAt: Date; +}) { + return { + _id: action._id.toString(), + taskId: action.taskId.toString(), + conversationId: action.conversationId.toString(), + actorType: action.actorType, + actorId: action.actorId ? action.actorId.toString() : null, + actionType: action.actionType, + toolName: action.toolName ?? null, + messageId: action.messageId ? action.messageId.toString() : null, + parameters: action.parameters ?? {}, + executionState: action.executionState ?? null, + summary: action.summary ?? null, + error: action.error ?? null, + patch: action.patch, + reason: action.reason, + idempotencyKey: action.idempotencyKey, + createdAt: action.createdAt.toISOString(), + }; +} + +export async function POST(req: NextRequest, context: RouteContext) { + return withRequestCorrelation(req, async () => { + const guard = await requireAuthUser(); + if (guard.response) { + return guard.response; + } + + const { id } = await context.params; + + try { + await connectToDatabase(); + + let body: z.infer = {}; + const raw = await req.text(); + if (raw.trim().length > 0) { + body = bodySchema.parse(JSON.parse(raw)); + } + + const result = await requestTaskExecution({ + taskId: id, + actorUserId: guard.user.id, + reason: body.reason, + authOptions: { + userRole: guard.user.role, + allowAdminBypass: true, + }, + }); + + return NextResponse.json({ + success: true, + data: { + taskAction: serializeTaskAction(result.taskAction), + enqueued: result.enqueued, + alreadyPending: result.alreadyPending, + }, + }); + } catch (error) { + if (error instanceof z.ZodError || error instanceof SyntaxError) { + return NextResponse.json( + { success: false, error: "Invalid request-execution payload" }, + { status: 400 } + ); + } + if (error instanceof ValidationError) { + return NextResponse.json({ success: false, error: error.message }, { status: 400 }); + } + if (error instanceof ConflictError) { + return NextResponse.json({ success: false, error: error.message }, { status: 409 }); + } + if (error instanceof AuthorizationError) { + const status = error.code === "NOT_FOUND" ? 404 : 403; + return NextResponse.json( + { + success: false, + error: error.code === "NOT_FOUND" ? "Task not found" : "Forbidden", + }, + { status } + ); + } + console.error("POST /api/tasks/[id]/request-execution error", error); + return NextResponse.json( + { success: false, error: "Failed to request execution" }, + { status: 500 } + ); + } + }); +} diff --git a/apps/web/app/work-suggestions/[id]/page.tsx b/apps/web/app/work-suggestions/[id]/page.tsx index 1e53011e..8724299f 100644 --- a/apps/web/app/work-suggestions/[id]/page.tsx +++ b/apps/web/app/work-suggestions/[id]/page.tsx @@ -9,6 +9,7 @@ import { assignWorkSuggestionApi, dismissWorkSuggestionApi, getWorkSuggestion, + requestTaskExecutionApi, } from "@/lib/utils/api"; import { WorkSuggestionDetailView } from "@/components/work-suggestions/work-suggestion-detail"; @@ -112,6 +113,14 @@ export default function WorkSuggestionDetailPage() { setSuggestion(result.suggestion); }); }} + onAllowAiTools={async () => { + if (!suggestion?.convertedTaskId) return; + await runAction(async () => { + await requestTaskExecutionApi(suggestion.convertedTaskId as string, { + reason: "Manager requested AI tool execution from suggestion detail", + }); + }); + }} /> ); } diff --git a/apps/web/components/chat/task-panel.tsx b/apps/web/components/chat/task-panel.tsx index 21a4c2b2..37e0faa2 100644 --- a/apps/web/components/chat/task-panel.tsx +++ b/apps/web/components/chat/task-panel.tsx @@ -1,6 +1,7 @@ "use client"; import { memo, useCallback, useEffect, useMemo, useRef, useState } from "react"; +import Link from "next/link"; import { AnimatePresence, motion, useReducedMotion } from "framer-motion"; import type { TaskExecutionEventRecord, TaskRecord, TaskStatus } from "@semantask/types"; import { authenticatedFetch } from "@/lib/utils/api"; @@ -280,7 +281,14 @@ function TaskInlineCard({ task, onStatusChange, onCancel }: TaskInlineCardProps)

{executionView.retryStatus}

)} {executionView.approvalPending && ( -

Awaiting human approval

+
+

+ Awaiting human approval to allow AI tools +

+ +
)} {task.cancelRequestedAt && task.status !== "failed" && task.status !== "completed" && (

Cancellation requested…

diff --git a/apps/web/components/work-suggestions/inbox-approvals.tsx b/apps/web/components/work-suggestions/inbox-approvals.tsx index 3d1e85e4..171b933c 100644 --- a/apps/web/components/work-suggestions/inbox-approvals.tsx +++ b/apps/web/components/work-suggestions/inbox-approvals.tsx @@ -36,9 +36,12 @@ function getPolicySummary(item: TaskApprovalRecord) { return reasons.join(" "); } +const STORAGE_KEY = "semantask.activeOrganizationId"; + export function InboxApprovalsView() { const [approvals, setApprovals] = useState([]); const [conversationId, setConversationId] = useState(""); + const [organizationId, setOrganizationId] = useState(null); const [loading, setLoading] = useState(true); const [actingId, setActingId] = useState(null); const [error, setError] = useState(null); @@ -46,12 +49,30 @@ export function InboxApprovalsView() { const [paramsById, setParamsById] = useState>({}); const loadSeqRef = useRef(0); + useEffect(() => { + if (typeof window === "undefined") return; + setOrganizationId(window.localStorage.getItem(STORAGE_KEY)); + }, []); + const loadApprovals = useCallback(async () => { const requestId = ++loadSeqRef.current; setLoading(true); setError(null); try { - const response = await getTaskApprovals(conversationId.trim() || undefined); + const scopedConversation = conversationId.trim() || undefined; + if (!scopedConversation && !organizationId) { + if (requestId !== loadSeqRef.current) return; + setApprovals([]); + setError( + "Select an active organization or enter a conversation id to load execution approvals." + ); + return; + } + + const response = await getTaskApprovals({ + conversationId: scopedConversation, + organizationId: scopedConversation ? undefined : organizationId ?? undefined, + }); if (requestId !== loadSeqRef.current) return; setApprovals(response.approvals); setCommentsById((current) => { @@ -78,7 +99,7 @@ export function InboxApprovalsView() { if (loadError instanceof ApiHttpError) { if (loadError.status === 403) { setError( - "Execution approvals require platform admin access. Your account cannot review this queue." + "You do not have permission to review execution approvals for this scope." ); } else { setError(loadError.message); @@ -91,7 +112,7 @@ export function InboxApprovalsView() { setLoading(false); } } - }, [conversationId]); + }, [conversationId, organizationId]); useEffect(() => { void loadApprovals(); @@ -248,7 +269,7 @@ export function InboxApprovalsView() { onClick={() => void decide(item, "approve")} disabled={actingId === item._id} > - Approve + Allow AI tools + {isConverted && suggestion.convertedTaskId ? ( + + ) : null} {!canAssign && isProposed ? (

Assign is available after Accept converts the suggestion.

) : null} + {isConverted ? ( +

+ Allow AI tools requests execution approval — separate from accepting a suggestion. +

+ ) : null} {actionError ? (

diff --git a/apps/web/components/work-suggestions/work-inbox.tsx b/apps/web/components/work-suggestions/work-inbox.tsx index 4b5e652d..841c22db 100644 --- a/apps/web/components/work-suggestions/work-inbox.tsx +++ b/apps/web/components/work-suggestions/work-inbox.tsx @@ -14,6 +14,7 @@ import { dismissWorkSuggestionApi, getOrganizationMembers, listWorkSuggestions, + requestTaskExecutionApi, type WorkSuggestionListResult, } from "@/lib/utils/api"; import useWorkSuggestionStore from "@/store/work-suggestion-store"; @@ -272,6 +273,28 @@ export function WorkInboxView() { } } + async function handleAllowAiTools(item: WorkSuggestionRecord) { + if (!item.convertedTaskId) return; + setActingId(item._id); + setRowError(item._id, null); + try { + await requestTaskExecutionApi(item.convertedTaskId, { + reason: "Manager requested AI tool execution from work inbox", + }); + } catch (actionError) { + setRowError( + item._id, + actionError instanceof ApiHttpError + ? actionError.message + : actionError instanceof Error + ? actionError.message + : "Allow AI tools failed" + ); + } finally { + setActingId(null); + } + } + async function handleAssign(item: WorkSuggestionRecord, assignees: string[]) { const previousOwners = ownersForSuggestion(item, ownerById); setActingId(item._id); @@ -495,6 +518,7 @@ export function WorkInboxView() { onAccept={(assignees) => handleAccept(item, assignees)} onAssign={(assignees) => handleAssign(item, assignees)} onDismiss={(reason) => handleDismiss(item, reason)} + onAllowAiTools={() => handleAllowAiTools(item)} /> ) : null} diff --git a/apps/web/components/work-suggestions/work-suggestion-detail.tsx b/apps/web/components/work-suggestions/work-suggestion-detail.tsx index 9c185747..a6d950c3 100644 --- a/apps/web/components/work-suggestions/work-suggestion-detail.tsx +++ b/apps/web/components/work-suggestions/work-suggestion-detail.tsx @@ -26,6 +26,7 @@ export type WorkSuggestionDetailViewProps = { dueAt?: string | null; priority?: TaskPriority; }) => void | Promise; + onAllowAiTools?: () => void | Promise; }; function formatTimestamp(iso: string) { @@ -51,6 +52,7 @@ export function WorkSuggestionDetailView({ onAccept, onDismiss, onAssign, + onAllowAiTools, }: WorkSuggestionDetailViewProps) { const [dismissReason, setDismissReason] = useState(""); const [assigneesInput, setAssigneesInput] = useState(""); @@ -303,15 +305,34 @@ export function WorkSuggestionDetailView({ ) : ( - +

+ + {suggestion.convertedTaskId ? ( + + ) : null} +
)} + {isConverted ? ( +

+ Allow AI tools requests execution approval for the converted task. It is + not the same as accepting a suggestion. +

+ ) : null} + {actionError ? (

{actionError} diff --git a/apps/web/jest.config.cjs b/apps/web/jest.config.cjs index 131e2212..07cc9695 100644 --- a/apps/web/jest.config.cjs +++ b/apps/web/jest.config.cjs @@ -20,10 +20,13 @@ module.exports = { moduleNameMapper: { "^@/lib/Db/(.*)$": "/../../packages/db/$1", "^@/lib/services/(.*)$": "/../../packages/services/$1", + "^@/models/(.*)$": "/../../packages/db/models/$1", "^@/(.*)$": "/$1", "^@semantask/types$": "/../../packages/types/dist/index.js", "^@semantask/types/(.*)$": "/../../packages/types/$1", "^@semantask/services/(.*)$": "/../../packages/services/$1", + "^@semantask/db$": "/../../packages/db/dist/db.js", + "^@semantask/db/(.*)$": "/../../packages/db/$1", "^next/link$": "/test/mocks/next-link.tsx", }, clearMocks: true, diff --git a/apps/web/lib/utils/api.ts b/apps/web/lib/utils/api.ts index 427a8cf0..7fc5a647 100644 --- a/apps/web/lib/utils/api.ts +++ b/apps/web/lib/utils/api.ts @@ -355,11 +355,60 @@ export async function revokeAdminToolGrant(grantId: string): Promise { }); } -export async function getTaskApprovals(conversationId?: string): Promise { - const query = conversationId ? `?conversationId=${encodeURIComponent(conversationId)}` : ""; +export async function getTaskApprovals(options?: { + conversationId?: string; + organizationId?: string; +}): Promise { + const params = new URLSearchParams(); + if (options?.conversationId) { + params.set("conversationId", options.conversationId); + } + if (options?.organizationId) { + params.set("organizationId", options.organizationId); + } + const query = params.toString() ? `?${params.toString()}` : ""; return request(`/api/task-approvals${query}`); } +export async function requestTaskExecutionApi( + taskId: string, + input?: { reason?: string } +): Promise<{ + taskAction: TaskApprovalRecord; + enqueued: boolean; + alreadyPending: boolean; +}> { + const response = await authenticatedFetch( + `/api/tasks/${encodeURIComponent(taskId)}/request-execution`, + { + method: "POST", + body: JSON.stringify(input ?? {}), + } + ); + const rawText = await response.text(); + const payload = parseAuthPayload(rawText) as ApiErrorPayload & { + success?: boolean; + data?: { + taskAction: TaskApprovalRecord; + enqueued: boolean; + alreadyPending: boolean; + }; + } | null; + + if (!response.ok) { + throw new ApiHttpError( + response.status, + payload?.error || rawText || `Request failed with status ${response.status}` + ); + } + + if (!payload?.data) { + throw new ApiHttpError(500, "Invalid request-execution response"); + } + + return payload.data; +} + export type WorkSuggestionListResult = { items: WorkSuggestionRecord[]; pagination: { diff --git a/apps/web/test/inbox-approvals.test.tsx b/apps/web/test/inbox-approvals.test.tsx index 934d68cf..9cfc908e 100644 --- a/apps/web/test/inbox-approvals.test.tsx +++ b/apps/web/test/inbox-approvals.test.tsx @@ -55,6 +55,8 @@ describe("InboxApprovalsView", () => { beforeEach(() => { getTaskApprovals.mockReset(); decideTaskApproval.mockReset(); + window.localStorage.clear(); + window.localStorage.setItem("semantask.activeOrganizationId", "507f1f77bcf86cd799439015"); }); it("shows loading then empty state", async () => { @@ -74,9 +76,15 @@ describe("InboxApprovalsView", () => { expect(screen.queryByTestId("suggestion-accept")).not.toBeInTheDocument(); expect(screen.queryByTestId("suggestion-dismiss")).not.toBeInTheDocument(); expect(screen.queryByTestId("suggestion-assign")).not.toBeInTheDocument(); + await waitFor(() => { + expect(getTaskApprovals).toHaveBeenCalledWith({ + conversationId: undefined, + organizationId: "507f1f77bcf86cd799439015", + }); + }); }); - it("lists approvals and calls decideTaskApproval on approve", async () => { + it("lists approvals and calls decideTaskApproval on Allow AI tools", async () => { getTaskApprovals.mockResolvedValue({ approvals: [buildApproval()] }); decideTaskApproval.mockResolvedValue({ approval: buildApproval({ executionState: "approved" }) }); @@ -84,6 +92,7 @@ describe("InboxApprovalsView", () => { expect(await screen.findByTestId("inbox-approvals-list")).toBeInTheDocument(); expect(screen.getByText("send_email")).toBeInTheDocument(); + expect(screen.getByTestId("inbox-approvals-approve")).toHaveTextContent("Allow AI tools"); fireEvent.click(screen.getByTestId("inbox-approvals-approve")); @@ -114,12 +123,12 @@ describe("InboxApprovalsView", () => { expect(await screen.findByTestId("inbox-approvals-empty")).toBeInTheDocument(); }); - it("surfaces forbidden access clearly for non-admin callers", async () => { + it("surfaces forbidden access clearly for unauthorized callers", async () => { getTaskApprovals.mockRejectedValue(new ApiHttpError(403, "Forbidden")); render(); const error = await screen.findByTestId("inbox-approvals-error"); - expect(error).toHaveTextContent(/platform admin/i); + expect(error).toHaveTextContent(/permission/i); }); }); diff --git a/apps/web/test/task-approvals.route.test.ts b/apps/web/test/task-approvals.route.test.ts index 6f2bf7f0..98f5b287 100644 --- a/apps/web/test/task-approvals.route.test.ts +++ b/apps/web/test/task-approvals.route.test.ts @@ -1,11 +1,12 @@ import { NextResponse } from "next/server"; -jest.mock("@/lib/utils/auth/requireAdminUser", () => ({ - requireAdminUser: jest.fn(), +jest.mock("@/lib/utils/auth/requireAuthUser", () => ({ + requireAuthUser: jest.fn(), })); jest.mock("@/lib/services/repositories/task.repo", () => ({ getPendingApprovalTaskActions: jest.fn(), + getPendingApprovalTaskActionsForOrganization: jest.fn(), getTaskActionById: jest.fn(), updateTaskActionExecutionState: jest.fn(), })); @@ -18,13 +19,54 @@ jest.mock("@/lib/observability/with-correlation", () => ({ withRequestCorrelation: async (_req: unknown, handler: () => Promise) => handler(), })); -import { requireAdminUser } from "@/lib/utils/auth/requireAdminUser"; +jest.mock("@semantask/services/authorization.service", () => { + class AuthorizationError extends Error { + code: "FORBIDDEN" | "NOT_FOUND"; + constructor(code: "FORBIDDEN" | "NOT_FOUND", message: string) { + super(message); + this.code = code; + this.name = "AuthorizationError"; + } + } + return { + AuthorizationError, + assertCanDecideTaskExecutionApproval: jest.fn(), + }; +}); + +jest.mock("@semantask/services/organization.service", () => ({ + assertOrganizationActive: jest.fn().mockResolvedValue(undefined), + canManageMembers: jest.fn((role: string) => role === "owner" || role === "admin"), + getMembership: jest.fn(), +})); + +jest.mock("@/models/Conversation", () => ({ + Conversation: { + findById: jest.fn(), + }, +})); + +jest.mock("@/models/Task", () => ({ + __esModule: true, + default: { + findById: jest.fn(), + }, +})); + +import { requireAuthUser } from "@/lib/utils/auth/requireAuthUser"; import { getPendingApprovalTaskActions, + getPendingApprovalTaskActionsForOrganization, getTaskActionById, updateTaskActionExecutionState, } from "@/lib/services/repositories/task.repo"; import { enqueueOutboxEvent } from "@/lib/services/outbox.service"; +import { + AuthorizationError, + assertCanDecideTaskExecutionApproval, +} from "@semantask/services/authorization.service"; +import { getMembership } from "@semantask/services/organization.service"; +import { Conversation } from "@/models/Conversation"; import { GET, POST } from "../app/api/task-approvals/route"; const adminUser = { @@ -33,6 +75,12 @@ const adminUser = { role: "admin" as const, }; +const managerUser = { + id: "507f1f77bcf86cd799439011", + email: "manager@example.com", + role: "user" as const, +}; + function pendingAction(overrides: Record = {}) { return { _id: { toString: () => "action-1" }, @@ -61,7 +109,7 @@ describe("GET /api/task-approvals", () => { }); it("returns 401 when unauthenticated", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ + (requireAuthUser as jest.Mock).mockResolvedValue({ user: null, response: NextResponse.json({ error: "Unauthorized" }, { status: 401 }), }); @@ -71,40 +119,58 @@ describe("GET /api/task-approvals", () => { expect(getPendingApprovalTaskActions).not.toHaveBeenCalled(); }); - it("returns 403 for non-admin users", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ - user: null, - response: NextResponse.json({ error: "Forbidden" }, { status: 403 }), - }); - - const response = await GET(new Request("http://localhost/api/task-approvals") as never); - expect(response.status).toBe(403); - expect(getPendingApprovalTaskActions).not.toHaveBeenCalled(); - }); - - it("returns pending approvals for admin", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); + it("allows platform admin global list", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); (getPendingApprovalTaskActions as jest.Mock).mockResolvedValue([pendingAction()]); const response = await GET(new Request("http://localhost/api/task-approvals") as never); expect(response.status).toBe(200); const body = await response.json(); expect(body.approvals).toHaveLength(1); - expect(body.approvals[0]._id).toBe("action-1"); - expect(getPendingApprovalTaskActions).toHaveBeenCalledWith(undefined); + }); + + it("requires scope for non-admin and allows org manager", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ user: managerUser, response: null }); + (getMembership as jest.Mock).mockResolvedValue({ role: "owner" }); + (getPendingApprovalTaskActionsForOrganization as jest.Mock).mockResolvedValue([pendingAction()]); + + const response = await GET( + new Request("http://localhost/api/task-approvals?organizationId=507f1f77bcf86cd799439015") as never + ); + expect(response.status).toBe(200); + expect(getPendingApprovalTaskActionsForOrganization).toHaveBeenCalledWith( + "507f1f77bcf86cd799439015" + ); + }); + + it("forbids non-manager org member listing another org", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ user: managerUser, response: null }); + (getMembership as jest.Mock).mockResolvedValue({ role: "member" }); + + const response = await GET( + new Request("http://localhost/api/task-approvals?organizationId=507f1f77bcf86cd799439015") as never + ); + expect(response.status).toBe(403); + expect(getPendingApprovalTaskActionsForOrganization).not.toHaveBeenCalled(); }); }); describe("POST /api/task-approvals", () => { beforeEach(() => { jest.clearAllMocks(); + (Conversation.findById as jest.Mock).mockReturnValue({ + select: () => ({ + lean: async () => ({ organizationId: { toString: () => "507f1f77bcf86cd799439015" } }), + }), + }); }); - it("returns 403 for non-admin users and does not mutate", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ - user: null, - response: NextResponse.json({ error: "Forbidden" }, { status: 403 }), - }); + it("forbids unauthorized manager decide", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ user: managerUser, response: null }); + (getTaskActionById as jest.Mock).mockResolvedValue(pendingAction()); + (assertCanDecideTaskExecutionApproval as jest.Mock).mockRejectedValue( + new AuthorizationError("FORBIDDEN", "Forbidden") + ); const response = await POST( new Request("http://localhost/api/task-approvals", { @@ -113,14 +179,13 @@ describe("POST /api/task-approvals", () => { }) as never ); expect(response.status).toBe(403); - expect(getTaskActionById).not.toHaveBeenCalled(); - expect(updateTaskActionExecutionState).not.toHaveBeenCalled(); expect(enqueueOutboxEvent).not.toHaveBeenCalled(); }); it("approves pending action and enqueues task.execution.approved", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); + (requireAuthUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); (getTaskActionById as jest.Mock).mockResolvedValue(pendingAction()); + (assertCanDecideTaskExecutionApproval as jest.Mock).mockResolvedValue(undefined); (updateTaskActionExecutionState as jest.Mock).mockResolvedValue( pendingAction({ executionState: "approved" }) ); @@ -138,25 +203,38 @@ describe("POST /api/task-approvals", () => { ); expect(response.status).toBe(200); - expect(updateTaskActionExecutionState).toHaveBeenCalledWith( - expect.objectContaining({ - taskActionId: "action-1", - executionState: "approved", - }) - ); + // Generic approvals do not set S2.4 suggest_only bypass flags. expect(enqueueOutboxEvent).toHaveBeenCalledWith( expect.objectContaining({ topic: "task.execution.approved", dedupeKey: "task.execution.approved:action-1", + payload: expect.objectContaining({ + humanApprovedExecution: false, + explicitManagerRequest: false, + }), }) ); }); - it("rejects pending action without outbox event", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); - (getTaskActionById as jest.Mock).mockResolvedValue(pendingAction()); + it("sets suggest_only bypass flags only for explicit manager Allow AI tools approvals", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); + (getTaskActionById as jest.Mock).mockResolvedValue( + pendingAction({ + actionType: "none", + toolName: "none", + patch: { + before: null, + after: { + source: "explicit-manager-request", + explicitManagerRequest: true, + needsApproval: true, + }, + }, + }) + ); + (assertCanDecideTaskExecutionApproval as jest.Mock).mockResolvedValue(undefined); (updateTaskActionExecutionState as jest.Mock).mockResolvedValue( - pendingAction({ executionState: "rejected" }) + pendingAction({ executionState: "approved" }) ); const response = await POST( @@ -165,37 +243,56 @@ describe("POST /api/task-approvals", () => { headers: { "content-type": "application/json" }, body: JSON.stringify({ taskActionId: "action-1", - decision: "reject", - reason: "nope", + decision: "approve", + reviewerComment: "allow tools", }), }) as never ); expect(response.status).toBe(200); + expect(enqueueOutboxEvent).toHaveBeenCalledWith( + expect.objectContaining({ + topic: "task.execution.approved", + dedupeKey: "task.execution.approved:action-1", + payload: expect.objectContaining({ + humanApprovedExecution: true, + explicitManagerRequest: true, + }), + }) + ); expect(updateTaskActionExecutionState).toHaveBeenCalledWith( expect.objectContaining({ - taskActionId: "action-1", - executionState: "rejected", + patch: expect.objectContaining({ + after: expect.objectContaining({ + explicitManagerRequest: true, + humanApprovedExecution: true, + }), + }), }) ); - expect(enqueueOutboxEvent).not.toHaveBeenCalled(); }); - it("returns 409 when action is not pending", async () => { - (requireAdminUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); - (getTaskActionById as jest.Mock).mockResolvedValue( - pendingAction({ executionState: "approved" }) + it("rejects pending action without outbox event", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ user: adminUser, response: null }); + (getTaskActionById as jest.Mock).mockResolvedValue(pendingAction()); + (assertCanDecideTaskExecutionApproval as jest.Mock).mockResolvedValue(undefined); + (updateTaskActionExecutionState as jest.Mock).mockResolvedValue( + pendingAction({ executionState: "rejected" }) ); const response = await POST( new Request("http://localhost/api/task-approvals", { method: "POST", headers: { "content-type": "application/json" }, - body: JSON.stringify({ taskActionId: "action-1", decision: "approve" }), + body: JSON.stringify({ + taskActionId: "action-1", + decision: "reject", + reason: "nope", + }), }) as never ); - expect(response.status).toBe(409); + expect(response.status).toBe(200); expect(enqueueOutboxEvent).not.toHaveBeenCalled(); }); }); diff --git a/apps/web/test/task-request-execution.route.test.ts b/apps/web/test/task-request-execution.route.test.ts new file mode 100644 index 00000000..e50386f8 --- /dev/null +++ b/apps/web/test/task-request-execution.route.test.ts @@ -0,0 +1,142 @@ +import { NextResponse } from "next/server"; + +jest.mock("@/lib/Db/db", () => ({ + connectToDatabase: jest.fn().mockResolvedValue(undefined), +})); + +jest.mock("@/lib/utils/auth/requireAuthUser", () => ({ + requireAuthUser: jest.fn(), +})); + +jest.mock("@/lib/observability/with-correlation", () => ({ + withRequestCorrelation: async (_req: unknown, handler: () => Promise) => handler(), +})); + +class MockAuthorizationError extends Error { + code: "FORBIDDEN" | "NOT_FOUND"; + constructor(code: "FORBIDDEN" | "NOT_FOUND", message: string) { + super(message); + this.code = code; + this.name = "AuthorizationError"; + } +} + +jest.mock("@semantask/services/authorization.service", () => ({ + AuthorizationError: MockAuthorizationError, +})); + +jest.mock("@semantask/services/organization-errors", () => { + class ValidationError extends Error { + code = "VALIDATION_ERROR" as const; + constructor(message: string) { + super(message); + this.name = "ValidationError"; + } + } + class ConflictError extends Error { + code = "CONFLICT" as const; + constructor(message: string) { + super(message); + this.name = "ConflictError"; + } + } + return { ValidationError, ConflictError }; +}); + +const requestTaskExecution = jest.fn(); + +jest.mock("@semantask/services/task-execution-request.service", () => ({ + requestTaskExecution: (...args: unknown[]) => requestTaskExecution(...args), +})); + +import { requireAuthUser } from "@/lib/utils/auth/requireAuthUser"; +import { POST } from "../app/api/tasks/[id]/request-execution/route"; + +const user = { + id: "507f1f77bcf86cd799439011", + email: "manager@example.com", + role: "user" as const, +}; + +describe("POST /api/tasks/[id]/request-execution", () => { + beforeEach(() => { + jest.clearAllMocks(); + (requireAuthUser as jest.Mock).mockResolvedValue({ user, response: null }); + }); + + it("returns 401 when unauthenticated", async () => { + (requireAuthUser as jest.Mock).mockResolvedValue({ + user: null, + response: NextResponse.json({ error: "Unauthorized" }, { status: 401 }), + }); + + const response = await POST( + new Request("http://localhost/api/tasks/507f1f77bcf86cd799439012/request-execution", { + method: "POST", + body: "{}", + }) as never, + { params: Promise.resolve({ id: "507f1f77bcf86cd799439012" }) } + ); + expect(response.status).toBe(401); + expect(requestTaskExecution).not.toHaveBeenCalled(); + }); + + it("requests execution for authorized manager", async () => { + requestTaskExecution.mockResolvedValue({ + taskAction: { + _id: { toString: () => "action-1" }, + taskId: { toString: () => "507f1f77bcf86cd799439012" }, + conversationId: { toString: () => "507f1f77bcf86cd799439013" }, + actorType: "user", + actorId: { toString: () => user.id }, + actionType: "none", + toolName: "none", + messageId: null, + parameters: {}, + executionState: "requested", + summary: "Explicit manager request", + error: null, + patch: { before: null, after: { explicitManagerRequest: true } }, + reason: "Manager requested AI tool execution", + idempotencyKey: "idem", + createdAt: new Date("2026-08-09T10:00:00.000Z"), + }, + enqueued: true, + alreadyPending: false, + }); + + const response = await POST( + new Request("http://localhost/api/tasks/507f1f77bcf86cd799439012/request-execution", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ reason: "please run" }), + }) as never, + { params: Promise.resolve({ id: "507f1f77bcf86cd799439012" }) } + ); + + expect(response.status).toBe(200); + const body = await response.json(); + expect(body.success).toBe(true); + expect(body.data.enqueued).toBe(true); + expect(requestTaskExecution).toHaveBeenCalledWith( + expect.objectContaining({ + taskId: "507f1f77bcf86cd799439012", + actorUserId: user.id, + reason: "please run", + }) + ); + }); + + it("returns 403 when unauthorized", async () => { + requestTaskExecution.mockRejectedValue(new MockAuthorizationError("FORBIDDEN", "Forbidden")); + + const response = await POST( + new Request("http://localhost/api/tasks/507f1f77bcf86cd799439012/request-execution", { + method: "POST", + body: "{}", + }) as never, + { params: Promise.resolve({ id: "507f1f77bcf86cd799439012" }) } + ); + expect(response.status).toBe(403); + }); +}); diff --git a/apps/web/test/work-inbox.test.tsx b/apps/web/test/work-inbox.test.tsx index 9fbc692d..8033e9df 100644 --- a/apps/web/test/work-inbox.test.tsx +++ b/apps/web/test/work-inbox.test.tsx @@ -12,6 +12,7 @@ const dismissWorkSuggestionApi = jest.fn(); const assignWorkSuggestionApi = jest.fn(); const getOrganizationMembers = jest.fn(); const decideTaskApproval = jest.fn(); +const requestTaskExecutionApi = jest.fn(); const refreshConversation = jest.fn(async () => undefined); class ApiHttpError extends Error { @@ -31,6 +32,7 @@ jest.mock("@/lib/utils/api", () => ({ assignWorkSuggestionApi: (...args: unknown[]) => assignWorkSuggestionApi(...args), getOrganizationMembers: (...args: unknown[]) => getOrganizationMembers(...args), decideTaskApproval: (...args: unknown[]) => decideTaskApproval(...args), + requestTaskExecutionApi: (...args: unknown[]) => requestTaskExecutionApi(...args), })); jest.mock("@/store/work-suggestion-store", () => { @@ -77,6 +79,7 @@ describe("WorkInboxView", () => { assignWorkSuggestionApi.mockReset(); getOrganizationMembers.mockReset(); decideTaskApproval.mockReset(); + requestTaskExecutionApi.mockReset(); refreshConversation.mockClear(); window.localStorage.clear(); getOrganizationMembers.mockResolvedValue([]); @@ -88,6 +91,8 @@ describe("WorkInboxView", () => { expect(listWorkSuggestions).not.toHaveBeenCalled(); expect(screen.queryByTestId("suggestion-accept")).not.toBeInTheDocument(); expect(screen.queryByTestId("suggestion-dismiss")).not.toBeInTheDocument(); + expect(requestTaskExecutionApi).not.toHaveBeenCalled(); + expect(decideTaskApproval).not.toHaveBeenCalled(); }); it("loads org-scoped suggestions with triage actions and links to detail", async () => { @@ -147,10 +152,45 @@ describe("WorkInboxView", () => { }); }); expect(decideTaskApproval).not.toHaveBeenCalled(); + expect(requestTaskExecutionApi).not.toHaveBeenCalled(); expect(await screen.findByTestId("work-inbox-empty")).toBeInTheDocument(); expect(refreshConversation).toHaveBeenCalledWith("507f1f77bcf86cd799439014"); }); + it("requests execution via Allow AI tools on converted rows only", async () => { + window.localStorage.setItem("semantask.activeOrganizationId", "507f1f77bcf86cd799439015"); + listWorkSuggestions.mockResolvedValue({ + items: [ + buildSuggestion({ + status: "converted", + convertedTaskId: "task-1", + }), + ], + pagination: { page: 1, limit: 20, total: 1, totalPages: 1 }, + }); + requestTaskExecutionApi.mockResolvedValue({ + taskAction: { _id: "action-1", executionState: "requested" }, + enqueued: true, + alreadyPending: false, + }); + + render(); + fireEvent.change(await screen.findByTestId("work-inbox-status"), { + target: { value: "converted" }, + }); + + expect(requestTaskExecutionApi).not.toHaveBeenCalled(); + fireEvent.click(await screen.findByTestId("suggestion-allow-ai-tools")); + + await waitFor(() => { + expect(requestTaskExecutionApi).toHaveBeenCalledWith("task-1", { + reason: "Manager requested AI tool execution from work inbox", + }); + }); + expect(acceptWorkSuggestionApi).not.toHaveBeenCalled(); + expect(decideTaskApproval).not.toHaveBeenCalled(); + }); + it("requires dismiss reason and removes row after dismiss", async () => { window.localStorage.setItem("semantask.activeOrganizationId", "507f1f77bcf86cd799439015"); listWorkSuggestions diff --git a/apps/web/test/work-suggestion-detail.test.tsx b/apps/web/test/work-suggestion-detail.test.tsx index 3e96a95d..c9ebe604 100644 --- a/apps/web/test/work-suggestion-detail.test.tsx +++ b/apps/web/test/work-suggestion-detail.test.tsx @@ -2,8 +2,8 @@ * @jest-environment jsdom */ import React from "react"; -import { describe, expect, it } from "@jest/globals"; -import { render, screen } from "@testing-library/react"; +import { describe, expect, it, jest } from "@jest/globals"; +import { fireEvent, render, screen } from "@testing-library/react"; import type { WorkSuggestionRecord } from "@semantask/types"; import { WorkSuggestionDetailView } from "@/components/work-suggestions/work-suggestion-detail"; @@ -121,4 +121,24 @@ describe("WorkSuggestionDetailView", () => { expect(screen.getByTestId("suggestion-assign")).toBeInTheDocument(); expect(screen.queryByTestId("suggestion-accept")).not.toBeInTheDocument(); }); + + it("shows Allow AI tools for converted suggestions with a task id", () => { + const onAllowAiTools = jest.fn(async () => undefined); + render( + + ); + expect(screen.getByTestId("suggestion-allow-ai-tools")).toHaveTextContent("Allow AI tools"); + expect(screen.getByText(/not the same as accepting a suggestion/i)).toBeInTheDocument(); + fireEvent.click(screen.getByTestId("suggestion-allow-ai-tools")); + expect(onAllowAiTools).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/services/__tests__/authorization.service.test.ts b/packages/services/__tests__/authorization.service.test.ts index 2840a3e5..adde1203 100644 --- a/packages/services/__tests__/authorization.service.test.ts +++ b/packages/services/__tests__/authorization.service.test.ts @@ -7,7 +7,9 @@ import { AuthorizationError, canAccessConversation, canAccessWorkSuggestion, + canDecideTaskExecutionApproval, canMutateWorkSuggestion, + assertCanDecideTaskExecutionApproval, } from "../authorization.service"; jest.mock("@semantask/db", () => ({ @@ -363,4 +365,72 @@ describe("authorization.service", () => { }); }); }); + + describe("task execution approval access", () => { + it("mirrors mutation matrix for conversation participants", async () => { + (Conversation.findById as jest.Mock).mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ + _id: new Types.ObjectId(conversationId), + participants: [new Types.ObjectId(userId)], + organizationId: null, + }), + }), + }); + + await expect( + canDecideTaskExecutionApproval(userId, { + conversationId, + organizationId: null, + }) + ).resolves.toBe(true); + }); + + it("allows org admins without conversation participation", async () => { + (Conversation.findById as jest.Mock).mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ + _id: new Types.ObjectId(conversationId), + participants: [new Types.ObjectId(otherUserId)], + organizationId: new Types.ObjectId(organizationId), + }), + }), + }); + (assertOrganizationActive as jest.Mock).mockResolvedValue({ status: "active" }); + (getMembership as jest.Mock).mockResolvedValue({ + role: "admin", + organizationId: new Types.ObjectId(organizationId), + userId: new Types.ObjectId(userId), + }); + + await expect( + canDecideTaskExecutionApproval(userId, { + conversationId, + organizationId, + }) + ).resolves.toBe(true); + }); + + it("denies unrelated users without leaking existence", async () => { + (Conversation.findById as jest.Mock).mockReturnValue({ + select: jest.fn().mockReturnValue({ + lean: jest.fn().mockResolvedValue({ + _id: new Types.ObjectId(conversationId), + participants: [new Types.ObjectId(otherUserId)], + organizationId: null, + }), + }), + }); + + await expect( + assertCanDecideTaskExecutionApproval(userId, { + conversationId, + organizationId: null, + }) + ).rejects.toMatchObject({ + code: "FORBIDDEN", + message: "Forbidden", + }); + }); + }); }); diff --git a/packages/services/__tests__/task-execution-enqueue.service.test.ts b/packages/services/__tests__/task-execution-enqueue.service.test.ts index a2a4098b..e2b9fc1c 100644 --- a/packages/services/__tests__/task-execution-enqueue.service.test.ts +++ b/packages/services/__tests__/task-execution-enqueue.service.test.ts @@ -135,6 +135,28 @@ describe("enqueueTaskExecutionRequested", () => { expect(result.enqueued).toBe(true); expect(enqueueOutboxEvent).toHaveBeenCalled(); }); + + it("allows explicit manager request under suggest_only + SUGGESTION_BLOCK_EXEC", async () => { + process.env.SUGGESTION_INGRESS = "1"; + process.env.SUGGESTION_BLOCK_EXEC = "1"; + enqueueOutboxEvent.mockResolvedValue({ _id: "evt-1" }); + + const result = await enqueueTaskExecutionRequested({ + dedupeKey: "task.execution.requested:t1:explicit:u1", + payload: { + taskId: "t1", + conversationId: "c1", + explicitManagerRequest: true, + needsApproval: true, + }, + executionMode: "suggest_only", + explicitManagerRequest: true, + }); + + expect(result).toEqual({ enqueued: true, blocked: false }); + expect(enqueueOutboxEvent).toHaveBeenCalled(); + expect(executionEnqueueAttemptedWhileSuggestOnlyCounter.inc).not.toHaveBeenCalled(); + }); }); describe("shouldFailClosedOnLeakedExecution", () => { diff --git a/packages/services/authorization.service.ts b/packages/services/authorization.service.ts index d602f8fb..a8665e92 100644 --- a/packages/services/authorization.service.ts +++ b/packages/services/authorization.service.ts @@ -353,3 +353,34 @@ export async function assertWorkSuggestionMutationAccess( throw new AuthorizationError("FORBIDDEN", "Forbidden"); } } + +export type TaskExecutionApprovalTarget = { + conversationId: string; + organizationId?: string | null; +}; + +/** + * Execution approval AuthZ (Allow AI tools / decide pending TaskAction): + * same matrix as WorkSuggestion mutations — conversation participant OR org owner/admin. + */ +export async function canDecideTaskExecutionApproval( + userId: string, + target: TaskExecutionApprovalTarget, + options?: ConversationAccessOptions +): Promise { + return canMutateWorkSuggestion(userId, { + conversationId: target.conversationId, + organizationId: target.organizationId ?? null, + }, options); +} + +export async function assertCanDecideTaskExecutionApproval( + userId: string, + target: TaskExecutionApprovalTarget, + options?: ConversationAccessOptions +): Promise { + const allowed = await canDecideTaskExecutionApproval(userId, target, options); + if (!allowed) { + throw new AuthorizationError("FORBIDDEN", "Forbidden"); + } +} diff --git a/packages/services/package.json b/packages/services/package.json index 4cbbe46c..761a1ab2 100644 --- a/packages/services/package.json +++ b/packages/services/package.json @@ -50,6 +50,11 @@ "types": "./dist/services/task-execution-enqueue.service.d.ts", "default": "./dist/services/task-execution-enqueue.service.js" }, + "./task-execution-request.service": { + "import": "./dist/services/task-execution-request.service.js", + "types": "./dist/services/task-execution-request.service.d.ts", + "default": "./dist/services/task-execution-request.service.js" + }, "./authorization.service": { "import": "./dist/services/authorization.service.js", "types": "./dist/services/authorization.service.d.ts", diff --git a/packages/services/repositories/task.repo.ts b/packages/services/repositories/task.repo.ts index 62d0e0c0..8211585d 100644 --- a/packages/services/repositories/task.repo.ts +++ b/packages/services/repositories/task.repo.ts @@ -365,6 +365,55 @@ export async function getPendingApprovalTaskActions(conversationId?: string): Pr .exec(); } +export async function getPendingApprovalTaskActionForTask(taskId: string): Promise { + await connectToDatabase(); + return TaskActionModel.findOne({ + taskId: toObjectId(taskId), + executionState: "approval_pending", + }) + .sort({ createdAt: -1 }) + .exec(); +} + +/** + * Find an in-flight explicit manager request action that has not yet reached + * approval_pending (worker may still be processing `requested`). + */ +export async function getInFlightExplicitRequestTaskAction( + taskId: string +): Promise { + await connectToDatabase(); + return TaskActionModel.findOne({ + taskId: toObjectId(taskId), + executionState: { $in: ["requested", "approval_pending"] }, + "patch.after.explicitManagerRequest": true, + }) + .sort({ createdAt: -1 }) + .exec(); +} + +export async function getPendingApprovalTaskActionsForOrganization( + organizationId: string +): Promise { + await connectToDatabase(); + + const conversations = await Conversation.find({ organizationId: toObjectId(organizationId) }) + .select("_id") + .lean<{ _id: Types.ObjectId }[]>(); + + const conversationIds = conversations.map((conversation) => conversation._id); + if (conversationIds.length === 0) { + return []; + } + + return TaskActionModel.find({ + executionState: "approval_pending", + conversationId: { $in: conversationIds }, + }) + .sort({ createdAt: -1 }) + .exec(); +} + export async function updateTaskActionExecutionState(input: { taskActionId: string; executionState: ITaskAction["executionState"]; diff --git a/packages/services/task-execution-enqueue.service.ts b/packages/services/task-execution-enqueue.service.ts index 2e52c30a..b23cc884 100644 --- a/packages/services/task-execution-enqueue.service.ts +++ b/packages/services/task-execution-enqueue.service.ts @@ -16,6 +16,11 @@ export type EnqueueTaskExecutionRequestedInput = { payload: Record; executionMode: ExecutionMode; session?: EnqueueOutboxEventInput["session"]; + /** + * Explicit manager "Allow AI tools" / request-execution path. + * Not a leaked ingress enqueue — allowed under suggest_only + SUGGESTION_BLOCK_EXEC. + */ + explicitManagerRequest?: boolean; }; export type EnqueueTaskExecutionRequestedResult = { @@ -25,14 +30,19 @@ export type EnqueueTaskExecutionRequestedResult = { /** * Enqueue boundary for task.execution.requested. - * Refuse writes when suggestion ingress is on and suggest_only + SUGGESTION_BLOCK_EXEC. + * Refuse writes when suggestion ingress is on and suggest_only + SUGGESTION_BLOCK_EXEC, + * unless this is an explicit manager request (S2.4). * When ingress is disabled, enqueue proceeds (legacy path). */ export async function enqueueTaskExecutionRequested( input: EnqueueTaskExecutionRequestedInput ): Promise { + const explicitManagerRequest = input.explicitManagerRequest === true + || input.payload.explicitManagerRequest === true; + if ( - isSuggestionIngressEnabled() + !explicitManagerRequest + && isSuggestionIngressEnabled() && shouldBlockExecutionEnqueue(input.executionMode) ) { executionEnqueueAttemptedWhileSuggestOnlyCounter.inc(); @@ -54,7 +64,9 @@ export async function enqueueTaskExecutionRequested( await enqueueOutboxEvent({ topic: "task.execution.requested", dedupeKey: input.dedupeKey, - payload: input.payload, + payload: explicitManagerRequest + ? { ...input.payload, explicitManagerRequest: true } + : input.payload, session: input.session, }); diff --git a/packages/services/task-execution-request.service.ts b/packages/services/task-execution-request.service.ts new file mode 100644 index 00000000..f7e67afa --- /dev/null +++ b/packages/services/task-execution-request.service.ts @@ -0,0 +1,173 @@ +import { Types } from "mongoose"; +import { connectToDatabase } from "@semantask/db"; +import TaskModel, { type ITask } from "@semantask/db/models/Task"; +import type { ITaskAction } from "@semantask/db/models/TaskAction"; +import { AuthorizationError } from "./authorization-errors"; +import { + assertCanDecideTaskExecutionApproval, + type ConversationAccessOptions, +} from "./authorization.service"; +import { ValidationError } from "./organization-errors"; +import { + getEffectiveExecutionMode, + getOrganizationPolicy, +} from "./organization-policy.service"; +import { + buildTaskActionIdempotencyKey, + createTaskAction, + getInFlightExplicitRequestTaskAction, +} from "./repositories/task.repo"; +import { enqueueTaskExecutionRequested } from "./task-execution-enqueue.service"; + +const toObjectId = (value: string) => new Types.ObjectId(value); + +function isValidObjectId(value: string): boolean { + return Types.ObjectId.isValid(value) && String(new Types.ObjectId(value)) === value; +} + +export type RequestTaskExecutionInput = { + taskId: string; + actorUserId: string; + reason?: string; + authOptions?: ConversationAccessOptions; +}; + +export type RequestTaskExecutionResult = { + taskAction: ITaskAction; + enqueued: boolean; + alreadyPending: boolean; +}; + +function serializeActionSummary(task: ITask, reason?: string): string { + const base = "Explicit manager request to allow AI tool execution."; + if (reason && reason.trim().length > 0) { + return `${base} ${reason.trim()}`; + } + return `${base} Task: ${task.title}`; +} + +/** + * Explicit S2.4 path: request AI tool execution for an existing coordination Task. + * Distinct from WorkSuggestion accept — never called by accept. + */ +export async function requestTaskExecution( + input: RequestTaskExecutionInput +): Promise { + if (!isValidObjectId(input.taskId) || !isValidObjectId(input.actorUserId)) { + throw new ValidationError("taskId and actorUserId must be valid ObjectIds"); + } + + await connectToDatabase(); + + const task = await TaskModel.findById(toObjectId(input.taskId)).exec(); + if (!task) { + throw new AuthorizationError("NOT_FOUND", "Task not found"); + } + + const conversationId = task.conversationId.toString(); + const organizationId = task.organizationId ? task.organizationId.toString() : null; + + await assertCanDecideTaskExecutionApproval( + input.actorUserId, + { conversationId, organizationId }, + input.authOptions + ); + + const existingInFlight = await getInFlightExplicitRequestTaskAction(input.taskId); + if (existingInFlight) { + return { + taskAction: existingInFlight, + enqueued: false, + alreadyPending: true, + }; + } + + const orgPolicy = organizationId ? await getOrganizationPolicy(organizationId) : null; + const executionMode = getEffectiveExecutionMode({ + organizationId, + executionMode: orgPolicy?.executionMode ?? null, + }); + + const parameters: Record = { + titleHint: task.title, + descriptionHint: task.description ?? "", + content: [task.title, task.description] + .filter((part) => typeof part === "string" && part.trim().length > 0) + .join("\n\n"), + source: "explicit-manager-request", + }; + + const triggerMessageId = task.sourceMessageIds?.[0] + ? task.sourceMessageIds[0].toString() + : input.taskId; + + let taskAction: ITaskAction; + try { + taskAction = await createTaskAction({ + taskId: input.taskId, + conversationId, + actorType: "user", + actorId: input.actorUserId, + actionType: "none", + toolName: "none", + messageId: task.sourceMessageIds?.[0] ? task.sourceMessageIds[0].toString() : null, + parameters, + executionState: "requested", + summary: serializeActionSummary(task, input.reason), + error: null, + patch: { + before: null, + after: { + actionType: "none", + toolName: "none", + source: "explicit-manager-request", + explicitManagerRequest: true, + needsApproval: true, + }, + }, + reason: input.reason?.trim() || "Manager requested AI tool execution", + idempotencyKey: buildTaskActionIdempotencyKey( + input.taskId, + "requested:none", + `explicit-${input.actorUserId}` + ), + }); + } catch (error) { + const maybeMongoError = error as { code?: number }; + if (maybeMongoError?.code === 11000) { + const racedInFlight = await getInFlightExplicitRequestTaskAction(input.taskId); + if (racedInFlight) { + return { + taskAction: racedInFlight, + enqueued: false, + alreadyPending: true, + }; + } + } + throw error; + } + + const enqueueResult = await enqueueTaskExecutionRequested({ + dedupeKey: `task.execution.requested:${input.taskId}:explicit:${input.actorUserId}`, + executionMode, + explicitManagerRequest: true, + payload: { + taskId: input.taskId, + conversationId, + triggerMessageId, + requestedByType: "user", + requestedById: input.actorUserId, + actionType: "none", + parameters, + confidence: 1, + needsApproval: true, + explicitManagerRequest: true, + }, + }); + + return { + taskAction, + enqueued: enqueueResult.enqueued, + alreadyPending: false, + }; +}