Run panel: auto-approve operator-invoked tools (#1183)

* Run panel: auto-approve operator-invoked tools Invoking a tool through the Run/Test panel paused on approval-gated tools and dead-ended on a "This tool requires approval" message, even though the operator clicking Run is itself the approval. Thread an optional autoApprove through the execute path: the execution engine runs the inline accept-all handler instead of intercepting the first elicitation as a pause, so an approval-gated tool runs to completion. The HTTP /executions endpoint takes autoApprove and the Run panel sends it. block policies still fail before any elicitation, so this never bypasses a hard block; the MCP host path is unchanged and still pauses for the model. * Format files flagged by oxfmt These two files were already unformatted on main (identical to origin/main); oxfmt --check flags them repo-wide. Formatting-only, no behavior change.

Rhys Sullivan committed 4 months ago

Commit a150db91 parent 5422ec39 files · +187 −80Browse files

Unified · Split

9 files

e2e/scenarios/run-panel-auto-approve.test.ts +101 −0
packages/core/api/src/executions/api.ts +4 −0
packages/core/api/src/handlers/executions.ts +3 −1
packages/core/execution/src/engine.ts +27 −2
packages/core/execution/src/promise.ts +5 −2
packages/core/execution/src/tool-invoker.test.ts +20 −0
packages/react/src/components/accounts-section.tsx +19 −59
packages/react/src/components/tool-run-panel.tsx +3 −1
packages/react/src/plugins/oauth-reconnect.ts +5 −15

e2e/scenarios/run-panel-auto-approve.test.ts+101−0

@@ -0,0 +1,101 @@
	1	// Cross-target: the Run/Test panel's backend (`POST /executions`) auto-approves
	2	// approval-gated tools when the operator invokes them, because clicking Run in
	3	// the panel IS the human approval.
	4	//
	5	// The panel sends `autoApprove: true`. Here we drive the same HTTP endpoint the
	6	// panel uses and prove both halves of the contract against ONE tool that gates
	7	// itself: the `policies.create` core tool carries a `requiresApproval`
	8	// annotation, so with no matching policy in play the annotation is the only
	9	// thing that can pause the call.
	10	//
	11	// - without `autoApprove`: the call pauses (the panel would have dead-ended
	12	// on "This tool requires approval"), and the policy is not written.
	13	// - with `autoApprove`: the call runs to completion and the policy is written.
	14	//
	15	// The created policy is a `block` rule on a unique, non-matching pattern, so a
	16	// leak cannot gate another scenario's tools; it is removed in an `ensuring`
	17	// finalizer regardless.
	18	import { randomUUID } from "node:crypto";
	19
	20	import { expect } from "@effect/vitest";
	21	import { Effect } from "effect";
	22	import { composePluginApi } from "@executor-js/api/server";
	23
	24	import { scenario } from "../src/scenario";
	25	import { Api, Target } from "../src/services";
	26
	27	const coreApi = composePluginApi([] as const);
	28
	29	/** Sandbox code that creates a policy through the approval-gated core tool. The
	30	* pattern is unique-per-run and matches no real tool, so the rule is inert. */
	31	const createPolicyCode = (pattern: string) => `
	32	return await tools.executor.coreTools.policies.create({
	33	owner: "user",
	34	pattern: ${JSON.stringify(pattern)},
	35	action: "block",
	36	});
	37	`;
	38
	39	scenario(
	40	"Run panel · autoApprove runs an approval-gated tool that otherwise pauses",
	41	{},
	42	Effect.gen(function* () {
	43	const target = yield* Target;
	44	const apiSurface = yield* Api;
	45	const identity = yield* target.newIdentity();
	46	const client = yield* apiSurface.client(coreApi, identity);
	47	const pattern = `run-auto-approve-${randomUUID().slice(0, 8)}.*`;
	48	const code = createPolicyCode(pattern);
	49
	50	const cleanup = client.policies.list().pipe(
	51	Effect.flatMap((list) =>
	52	Effect.forEach(
	53	list.filter((p) => p.pattern === pattern),
	54	(p) =>
	55	client.policies
	56	.remove({ params: { policyId: p.id }, payload: { owner: "user" } })
	57	.pipe(Effect.ignore),
	58	),
	59	),
	60	Effect.ignore,
	61	);
	62
	63	yield* Effect.gen(function* () {
	64	// Baseline: without autoApprove the gated tool pauses (the panel's old
	65	// dead-end), and the side effect must not have happened.
	66	const gated = yield* client.executions.execute({ payload: { code } });
	67	expect(gated.status, "a gated tool pauses without autoApprove").toBe("paused");
	68
	69	const beforeApproval = yield* client.policies.list();
	70	expect(
	71	beforeApproval.some((p) => p.pattern === pattern),
	72	"the policy is not written while the call is paused for approval",
	73	).toBe(false);
	74
	75	// Release the paused fiber so it does not linger waiting on a response.
	76	if (gated.status === "paused") {
	77	const executionId = (gated.structured as { readonly executionId?: string }).executionId;
	78	if (executionId) {
	79	yield* client.executions
	80	.resume({ params: { executionId }, payload: { action: "cancel" } })
	81	.pipe(Effect.ignore);
	82	}
	83	}
	84
	85	// With autoApprove the operator IS the approver: the same call runs to
	86	// completion and the side effect lands.
	87	const approved = yield* client.executions.execute({
	88	payload: { code, autoApprove: true },
	89	});
	90	expect(approved.status, "autoApprove runs the gated tool to completion").toBe("completed");
	91	if (approved.status !== "completed") return; // narrowing only
	92	expect(approved.isError, "the auto-approved run is not an error").toBe(false);
	93
	94	const afterApproval = yield* client.policies.list();
	95	expect(
	96	afterApproval.some((p) => p.pattern === pattern),
	97	"the policy is written once autoApprove runs the gated tool",
	98	).toBe(true);
	99	}).pipe(Effect.ensuring(cleanup));
	100	}),
	101	);

packages/core/api/src/executions/api.ts+4−0

 const ExecuteRequest = Schema.Struct({
   code: Schema.String,
+  // When true the caller is the human approver: approval-gated tools run to
+  // completion instead of pausing. Set by the operator-facing Run/Test panel,
+  // where clicking Run is itself the approval. `block` policies still apply.
+  autoApprove: Schema.optional(Schema.Boolean),
 });
 const CompletedResult = Schema.Struct({

packages/core/api/src/handlers/executions.ts+3−1

       capture(
         Effect.gen(function* () {
           const engine = yield* ExecutionEngineService;
-          const outcome = yield* captureEngineError(engine.executeWithPause(payload.code));
+          const outcome = yield* captureEngineError(
+            engine.executeWithPause(payload.code, { autoApprove: payload.autoApprove }),
+          );
           if (outcome.status === "completed") {
             const formatted = formatExecuteResult(outcome.result);

packages/core/execution/src/engine.ts+27−2

   readonly content?: Record<string, unknown>;
 };
+// Auto-accept every elicitation. Used by the `autoApprove` path where the
+// caller is itself the human approver (the operator-facing Run/Test panel).
+const acceptAllHandler: ElicitationHandler = () => Effect.succeed({ action: "accept" });
 // ---------------------------------------------------------------------------
 // Result formatting
 // ---------------------------------------------------------------------------
    * Execute code, intercepting the first elicitation as a pause point.
    * Use this when the host doesn't support inline elicitation.
    * Returns either a completed result or a paused execution that can be resumed.
+   *
+   * `options.autoApprove` treats the caller as the human in the loop: every
+   * elicitation is accepted inline, so an approval-gated tool runs to
+   * completion instead of pausing. The operator-facing Run/Test panel sets
+   * this because clicking Run IS the approval. `block` policies still fail
+   * before any elicitation, so this never bypasses a hard block.
    */
-  readonly executeWithPause: (code: string) => Effect.Effect<ExecutionResult, E>;
+  readonly executeWithPause: (
+    code: string,
+    options?: { readonly autoApprove?: boolean },
+  ) => Effect.Effect<ExecutionResult, E>;
   /**
    * Resume a paused execution. Returns a completed result, a new pause, or
    * The sandbox is forked as a daemon because paused executions can outlive the
    * caller scope that returned the first pause, such as an HTTP request handler.
    */
-  const startPausableExecution = Effect.fn("mcp.execute")(function* (code: string) {
+  const startPausableExecution = Effect.fn("mcp.execute")(function* (
+    code: string,
+    options?: { readonly autoApprove?: boolean },
+  ) {
     yield* Effect.annotateCurrentSpan({
       "mcp.execute.mode": "pausable",
       "mcp.execute.code_length": code.length,
     });
+    // Operator-approved invoke: run through the inline path with an accept-all
+    // handler so an approval gate resolves itself instead of pausing. Never
+    // pauses, so the caller always gets a completed result.
+    if (options?.autoApprove) {
+      yield* Effect.annotateCurrentSpan({ "mcp.execute.auto_approve": true });
+      const result = yield* runInlineExecution(code, { onElicitation: acceptAllHandler });
+      return { status: "completed", result } satisfies ExecutionResult;
+    }
     // Queue preserves pauses that arrive before the previous approval has
     // returned to the caller, which can happen with concurrent tool calls.
     const pauseQueue = yield* Queue.unbounded<InternalPausedExecution<E>>();

packages/core/execution/src/promise.ts+5−2

     code: string,
     options: { readonly onElicitation: ElicitationHandler },
   ) => Promise<ExecuteResult>;
-  readonly executeWithPause: (code: string) => Promise<ExecutionResult>;
+  readonly executeWithPause: (
+    code: string,
+    options?: { readonly autoApprove?: boolean },
+  ) => Promise<ExecutionResult>;
   readonly resume: (
     executionId: string,
     response: ResumeResponse,
           Effect.tryPromise(() => options.onElicitation(ctx)).pipe(Effect.orDie),
       }),
     ),
-  executeWithPause: (code) => Effect.runPromise(engine.executeWithPause(code)),
+  executeWithPause: (code, options) => Effect.runPromise(engine.executeWithPause(code, options)),
   resume: (executionId, response) => Effect.runPromise(engine.resume(executionId, response)),
   getPausedExecution: (executionId) => Effect.runPromise(engine.getPausedExecution(executionId)),
   getDescription: () => Effect.runPromise(engine.getDescription),

packages/core/execution/src/tool-invoker.test.ts+20−0

@@ -1346,6 +1346,26 @@ describe("pause/resume with multiple elicitations", () => { · 1345 unmodified lines
1346	{ timeout: 10000 },	1346	{ timeout: 10000 },
1347	);	1347	);
1348		1348
		1349	it.effect(
		1350	"autoApprove runs an eliciting tool to completion instead of pausing",
		1351	() =>
		1352	Effect.gen(function* () {
		1353	const executor = yield* makeElicitingExecutor();
		1354	const engine = createExecutionEngine({ executor, codeExecutor });
		1355	const code = "return await tools.api.org.main.singleApproval({});";
		1356
		1357	// Same tool that pauses without autoApprove (see the tests above) runs
		1358	// straight through when the caller is the approver: no pause, no
		1359	// executionId to resume, just the side effect's result.
		1360	const outcome = yield* engine.executeWithPause(code, { autoApprove: true });
		1361	expect(outcome.status, "autoApprove never pauses").toBe("completed");
		1362	if (outcome.status !== "completed") return;
		1363	expect(outcome.result.error).toBeUndefined();
		1364	expect(outcome.result.result).toMatchObject({ ok: true });
		1365	}),
		1366	{ timeout: 10000 },
		1367	);
		1368
1349	// live clock: the sandbox timeout is a real timer, so the test must	1369	// live clock: the sandbox timeout is a real timer, so the test must
1350	// actually wait for it rather than suspend on the virtual TestClock.	1370	// actually wait for it rather than suspend on the virtual TestClock.
1351	it.live(	1371	it.live(

packages/react/src/components/accounts-section.tsx+19−59

 import { useAtomValue, useAtomSet } from "@effect/atom-react";
 import * as AsyncResult from "effect/unstable/reactivity/AsyncResult";
 import * as Exit from "effect/Exit";
-import {
+import { IntegrationSlug, type Connection, type Owner } from "@executor-js/sdk/shared";
-  IntegrationSlug,
-  type Connection,
-  type Owner,
-} from "@executor-js/sdk/shared";
 import type { IntegrationAccountHandoff } from "@executor-js/sdk/client";
 import { toast } from "sonner";
         <CardStackEntryTitle className="flex min-w-0 items-center gap-2">
           <span className="truncate">{displayLabel}</span>
           {needsReconsent ? (
-            <Badge
+            <Badge variant="outline" className="shrink-0 border-border text-muted-foreground">
-              variant="outline"
-              className="shrink-0 border-border text-muted-foreground"
-            >
               Reconnect to grant access
             </Badge>
           ) : null}
         ) : null}
         {needsReconsent ? (
           <CardStackEntryDescription className="mt-1 text-xs text-muted-foreground">
-            This connection wasn't granted all the access this integration now
+            This connection wasn't granted all the access this integration now needs.
-            needs.
           </CardStackEntryDescription>
         ) : null}
       </CardStackEntryContent>
             <DropdownMenuItem className="text-sm" onClick={props.onReconnect}>
               Reconnect
             </DropdownMenuItem>
-            <DropdownMenuItem
+            <DropdownMenuItem variant="destructive" className="text-sm" onClick={props.onRemove}>
-              variant="destructive"
-              className="text-sm"
-              onClick={props.onRemove}
-            >
               Remove
             </DropdownMenuItem>
           </DropdownMenuContent>
   readonly declaredScopes: readonly string[] | undefined;
 }) {
   const { integration, owner } = props;
-  const connections = useAtomValue(
+  const connections = useAtomValue(connectionsForIntegrationAtom({ integration, owner }));
-    connectionsForIntegrationAtom({ integration, owner }),
-  );
   const doRemove = useAtomSet(removeConnectionOptimistic(owner), {
     mode: "promiseExit",
   });
     startErrorMessage: "Failed to reconnect",
   });
-  const rows: readonly Connection[] = AsyncResult.isSuccess(connections)
+  const rows: readonly Connection[] = AsyncResult.isSuccess(connections) ? connections.value : [];
-    ? connections.value
-    : [];
   if (rows.length === 0) return null;
   const handleReconnect = async (connection: Connection) => {
     if (reconnectMode(connection) === "oauth") {
       const method = props.methods.find(
         (candidate: AuthMethod) =>
-          candidate.kind === "oauth" &&
+          candidate.kind === "oauth" && String(candidate.template) === String(connection.template),
-          String(candidate.template) === String(connection.template),
       );
       if (
         method?.oauth?.supportsDynamicRegistration === true ||
   return (
     <CardStack>
-      {props.showOwnerLabels ? (
+      {props.showOwnerLabels ? <CardStackHeader>{ownerLabel(owner)}</CardStackHeader> : null}
-        <CardStackHeader>{ownerLabel(owner)}</CardStackHeader>
-      ) : null}
       <CardStackContent>
         {rows.map((connection: Connection) => (
           <AccountRow
             key={`${connection.owner}:${connection.integration}:${connection.name}`}
             connection={connection}
-            needsReconsent={connectionNeedsReconsent(
+            needsReconsent={connectionNeedsReconsent(connection, props.declaredScopes)}
-              connection,
-              props.declaredScopes,
-            )}
             showOwnerLabel={props.showOwnerLabels}
             onEdit={() => props.onEdit(connection)}
             onReconnect={() => void handleReconnect(connection)}
     removeCustomMethod,
   } = props;
   const [adding, setAdding] = useState(false);
-  const [editingConnection, setEditingConnection] = useState<Connection | null>(
+  const [editingConnection, setEditingConnection] = useState<Connection | null>(null);
-    null,
+  const [reconnectHandoff, setReconnectHandoff] = useState<IntegrationAccountHandoff | null>(null);
-  );
-  const [reconnectHandoff, setReconnectHandoff] =
-    useState<IntegrationAccountHandoff | null>(null);
   const ownerDisplay = useOwnerDisplay();
-  const canAddConnection =
+  const canAddConnection = methods.length > 0 || createCustomMethod !== undefined;
-    methods.length > 0 || createCustomMethod !== undefined;
   useEffect(() => {
     if (accountHandoff) {
   // Read both owners to decide between the grouped view and the empty CTA. The
   // grouped sub-components re-read these (effect-atom dedupes) and self-hide.
-  const orgConnections = useAtomValue(
+  const orgConnections = useAtomValue(connectionsForIntegrationAtom({ integration, owner: "org" }));
-    connectionsForIntegrationAtom({ integration, owner: "org" }),
-  );
   const userConnections = useAtomValue(
     connectionsForIntegrationAtom({ integration, owner: "user" }),
   );
   useAtomSet(addConnectionOptimistic("user"));
   const totalCount = useMemo(() => {
-    const orgRows = AsyncResult.isSuccess(orgConnections)
+    const orgRows = AsyncResult.isSuccess(orgConnections) ? orgConnections.value.length : 0;
-      ? orgConnections.value.length
+    const userRows = AsyncResult.isSuccess(userConnections) ? userConnections.value.length : 0;
-      : 0;
-    const userRows = AsyncResult.isSuccess(userConnections)
-      ? userConnections.value.length
-      : 0;
     return orgRows + userRows;
   }, [orgConnections, userConnections]);
-  const loading =
+  const loading = !AsyncResult.isSuccess(orgConnections) && !AsyncResult.isSuccess(userConnections);
-    !AsyncResult.isSuccess(orgConnections) &&
-    !AsyncResult.isSuccess(userConnections);
   const modalState = reconnectHandoff ?? accountHandoff;
   const modal = (
           onClick={() => {
             trackEvent("connection_add_opened", {
               integration_slug: String(integration),
-              has_oauth_method: methods.some(
+              has_oauth_method: methods.some((m: AuthMethod) => m.kind === "oauth"),
-                (m: AuthMethod) => m.kind === "oauth",
-              ),
               has_api_key_method: methods.some(
                 (m: AuthMethod) => m.kind !== "oauth" && m.kind !== "none",
               ),
         </div>
       ) : totalCount === 0 ? (
         <div className="rounded-lg border border-dashed border-border/60 px-6 py-8 text-center">
-          <p className="text-sm font-medium text-foreground">
+          <p className="text-sm font-medium text-foreground">No connections yet</p>
-            No connections yet
-          </p>
           <p className="mt-1 text-sm text-muted-foreground">
             Add a connection to make this integration's tools available.
           </p>
             onClick={() => {
               trackEvent("connection_add_opened", {
                 integration_slug: String(integration),
-                has_oauth_method: methods.some(
+                has_oauth_method: methods.some((m: AuthMethod) => m.kind === "oauth"),
-                  (m: AuthMethod) => m.kind === "oauth",
-                ),
                 has_api_key_method: methods.some(
                   (m: AuthMethod) => m.kind !== "oauth" && m.kind !== "none",
                 ),

packages/react/src/components/tool-run-panel.tsx+3−1

     // Read-only invoke: pass the validated args through as JSON. Empty
     // `reactivityKeys` — invoking a tool doesn't mutate `tools.list`.
+    // `autoApprove` because the operator clicked Run: that IS the approval, so
+    // an approval-gated tool should run here instead of dead-ending on a pause.
     const code = `return await tools[${JSON.stringify(addressNoPrefix)}](${JSON.stringify(parsed)});`;
-    const exit = await doExecute({ payload: { code }, reactivityKeys: [] });
+    const exit = await doExecute({ payload: { code, autoApprove: true }, reactivityKeys: [] });
     setRunning(false);
     if (Exit.isFailure(exit)) {

packages/react/src/plugins/oauth-reconnect.ts+5−15

  *  connection are exactly the branded types `oauth.start` expects, so the same
  *  connection (owner/integration/name) is re-minted with a fresh refresh token
  *  and the widened scope union. Returns null for a non-OAuth connection. */
-export function oauthReconnectPayload(
+export function oauthReconnectPayload(connection: Connection): OAuthStartPayload | null {
-  connection: Connection,
-): OAuthStartPayload | null {
   if (connection.oauthClient == null) return null;
   return {
     client: connection.oauthClient,
   "https://www.googleapis.com/auth/userinfo.profile": "profile",
 };
-const canonicalScope = (scope: string): string =>
+const canonicalScope = (scope: string): string => OAUTH_SCOPE_ALIASES[scope] ?? scope;
-  OAUTH_SCOPE_ALIASES[scope] ?? scope;
 /** Normalize a scope list: trim, canonicalize known provider aliases, drop
  *  empties, de-dupe (order-preserving). A scope set is compared as a SET —
 /** The scopes a connection was actually GRANTED, parsed from its space-delimited
  *  `oauthScope` record. Empty for static creds / when the AS omitted scope. */
-export function connectionGrantedScopes(
+export function connectionGrantedScopes(connection: Connection): readonly string[] {
-  connection: Connection,
+  return connection.oauthScope ? connection.oauthScope.split(/\s+/).filter(Boolean) : [];
-): readonly string[] {
-  return connection.oauthScope
-    ? connection.oauthScope.split(/\s+/).filter(Boolean)
-    : [];
 }
 /** Whether an OAuth connection must RECONNECT to grant newly-needed access: the
   declaredScopes: readonly string[] | undefined,
 ): boolean {
   if (connection.oauthClient == null) return false;
-  return (
+  return missingScopes(declaredScopes, connectionGrantedScopes(connection)).length > 0;
-    missingScopes(declaredScopes, connectionGrantedScopes(connection)).length >
-    0
-  );
 }
 /** The scopes that count as REQUIRED for a connection to be considered fully