Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/v2-p2-execution-approve-separate.md
Original file line number Diff line number Diff line change
@@ -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.
43 changes: 37 additions & 6 deletions apps/task-worker/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -237,6 +241,8 @@ type TaskExecutionRequestedPayload = {
confidence?: number;
needsApproval?: boolean;
semanticType?: MessageSemanticType;
explicitManagerRequest?: boolean;
humanApprovedExecution?: boolean;
};

type TaskExecutionApprovedPayload = {
Expand All @@ -246,6 +252,7 @@ type TaskExecutionApprovedPayload = {
approvedByType?: "user" | "agent" | "system";
approvedById?: string | null;
reason?: string;
humanApprovedExecution?: boolean;
};

type TaskSocketBridgePayload = {
Expand Down Expand Up @@ -336,6 +343,8 @@ function normalizeTaskExecutionRequestedPayload(payload: Record<string, unknown>
needsApproval: typeof payload.needsApproval === "boolean"
? payload.needsApproval
: false,
explicitManagerRequest: payload.explicitManagerRequest === true,
humanApprovedExecution: payload.humanApprovedExecution === true,
};
}

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
const blockedReason = unsafe
? "Execution blocked by policy: action marked unsafe."
: (policyDecision.reasons.join(" ") || "Execution blocked by policy.");
Expand Down Expand Up @@ -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);
Expand Down
53 changes: 53 additions & 0 deletions apps/task-worker/services/suggest-only-execution-gate.ts
Original file line number Diff line number Diff line change
@@ -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 };
}
98 changes: 98 additions & 0 deletions apps/task-worker/tests/suggest-only-execution-gate.test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
4 changes: 3 additions & 1 deletion apps/web/app/admin/task-approvals/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
Loading
Loading