From d70a5c0964de61f647506326bb80d0c41206d905 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 4 Sep 2026 18:38:29 -0500 Subject: [PATCH 1/7] feat: add external resource creation --- .../workshop-agent-create-resource.test.ts | 289 +++++++++++++++++ .../gatekeeper-test/src/test-gatekeeper.ts | 100 +++++- .../__tests__/agent-compaction.test.ts | 36 +++ .../workshop-backend/src/agent-compaction.ts | 11 +- packages/workshop-backend/src/agent.ts | 126 +++++++- packages/workshop-backend/src/overseer.ts | 306 +++++++++++++++++- packages/workshop-backend/src/user.ts | 67 ++++ .../workshop-frontend/src/ChatInterface.tsx | 12 + packages/workshop-shared/src/api.ts | 45 +++ packages/workshop-shared/src/gatekeeper.ts | 47 +++ 10 files changed, 1020 insertions(+), 19 deletions(-) create mode 100644 packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts diff --git a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts new file mode 100644 index 0000000000..bf9083b2c9 --- /dev/null +++ b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts @@ -0,0 +1,289 @@ +// createExternalResource end to end against the fixture gatekeeper: tool → binding → action card +// → approve → describe refresh, plus the rejection path, replay across turns, and a vendor that +// fails after queueing its creation action. (Provider depth is covered by per-vendor suites, +// e.g. gatekeeper-google's workerd tests.) + +import { afterAll, beforeAll, expect, it } from "vitest"; +import type { RpcStub } from "capnweb"; +import type { + AiChatAuthorInfo, AiModelConfig, AuthenticatedApi, Overseer, PublicApi, +} from "@gadgets/workshop-shared/api"; +import { startTestGatekeeperHarness, TEST_VENDOR_ID, type Harness } from "../src/harness.js"; +import { scriptedChatCompletions } from "../src/mock-model.js"; +import { NetworkInterceptor } from "../src/network-interceptor.js"; +import { + connect, listConnectedAccounts, nextUsernames, signUp, waitFor, +} from "../src/rpc-client.js"; + +const MODEL_ID = "@cf/zai-org/glm-5.2"; +const MODEL_PROFILE: AiChatAuthorInfo = { type: "agent", id: MODEL_ID, name: "Scripted model" }; +const MODEL_CONFIG: AiModelConfig = { + provider: "cloudflare", + model: MODEL_ID, + accountId: "test-account", + apiToken: "test-token", +}; + +const RESOURCE_URL_PATTERN = "https://gadgets-test.example/things/*"; + +let harness: Harness; +const model = scriptedChatCompletions([ + // --- Test 1, turn 1: a fixable rejection, then a successful creation, used immediately. + { + toolCall: { + id: "create-bad-type", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: "https://gadgets-test.example/nope/*", + title: "My Thing", + bindingName: "NEW_THING", + }, + }, + }, + { + toolCall: { + id: "create-thing", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "My Thing", + bindingName: "NEW_THING", + }, + }, + }, + { + toolCall: { + id: "read-new-thing", + name: "executeCode", + arguments: { + code: "export default async function(self, env) { console.log(await env.NEW_THING.readValue()); }", + }, + }, + }, + { text: "Created the thing and read 42 from it." }, + // --- Test 1, turn 2 (after approval): the replayed binding still works, and the write's + // action snapshot shows the refreshed (created) resource URL. writeValue awaits a decision, + // so this turn deliberately suspends. + { + toolCall: { + id: "write-new-thing", + name: "executeCode", + arguments: { + code: "export default async function(self, env) { console.log(await env.NEW_THING.writeValue(9)); }", + }, + }, + }, + // --- Test 2, turn 1: create a thing whose creation the user will reject. + { + toolCall: { + id: "create-doomed", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "Doomed Thing", + bindingName: "DOOMED", + }, + }, + }, + { text: "Created the doomed thing." }, + // --- Test 2, turn 2 (after rejection): the binding is dead, with an explanation. + { + toolCall: { + id: "read-doomed", + name: "executeCode", + arguments: { + code: "export default async function(self, env) {" + + " try { console.log(await env.DOOMED.readValue()); }" + + " catch (err) { console.log('DEAD: ' + (err && err.message)); } }", + }, + }, + }, + { text: "The doomed thing is gone." }, + // --- Test 3: the vendor fails after durably queueing its creation action; the overseer must + // settle the orphan instead of leaving it pending against a removed gatekeeper. + { + toolCall: { + id: "create-orphan", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "fail-after-queue", + bindingName: "ORPHAN", + }, + }, + }, + { text: "The creation failed." }, +]); +const network = new NetworkInterceptor({ handlers: [model.handler] }); + +beforeAll(async () => { + network.install(); + harness = await startTestGatekeeperHarness({ enableGadgetExecution: true }); +}); + +afterAll(async () => { + try { + await harness?.server.close(); + expect(network.getUnmockedCalls()).toEqual([]); + } finally { + network.uninstall(); + } +}); + +/** Sign up a fresh user configured with the scripted model and an ambient test-vendor account. */ +async function signUpScriptedUser( + publicApi: RpcStub, prefix: string): Promise> { + const [username] = nextUsernames(prefix); + if (username === undefined) throw new Error("Failed to allocate a username"); + const authenticated = await signUp(publicApi, username); + await authenticated.addModel(MODEL_PROFILE, MODEL_CONFIG); + await authenticated.setQuickModel(null); + await authenticated.setPreferredModel(MODEL_ID); + await authenticated.completeOnboarding(); + await authenticated.provisionAmbientAccount(TEST_VENDOR_ID); + await waitFor("the ambient test account to be provisioned", async () => + (await listConnectedAccounts(authenticated)).find(entry => entry.vendorId === TEST_VENDOR_ID) + ?? null); + return authenticated; +} + +/** Wait for the agent's turn to end with `text` as its closing message, failing fast on errors. */ +async function waitForAgentSays( + workspace: RpcStub, chatId: number, text: string): Promise { + await waitFor(`the agent to say "${text}"`, async () => { + const current = await workspace.getChatHistory(chatId); + const error = current.messages.find(message => message.type === "error"); + if (error !== undefined) throw new Error(`The scripted agent failed: ${error.message}`); + return current.messages.some(message => + message.type === "message" && message.author.type === "agent" && + message.message === text) ? current : null; + }); +} + +/** Wait until exactly one action is pending and return it. */ +function onlyPendingAction(workspace: RpcStub, what: string) { + return waitFor(what, async () => { + const entries = (await workspace.listActions({ filter: "pending" })).entries; + return entries.length === 1 ? entries[0] : null; + }); +} + +/** The most recent tool result the mock model was shown for `toolCallId`. */ +function toolResultShownToModel(toolCallId: string): string { + for (let i = model.requests.length - 1; i >= 0; i--) { + const request = model.requests[i] as { + messages?: Array<{ role: string; tool_call_id?: string; content?: string }>; + }; + const result = request.messages?.find( + message => message.role === "tool" && message.tool_call_id === toolCallId); + if (result?.content !== undefined) return result.content; + } + throw new Error(`The model never saw a tool result for ${toolCallId}`); +} + +/** All user-role message contents in the model's most recent request. */ +function userMessagesShownToModel(): string[] { + const request = model.requests[model.requests.length - 1] as { + messages?: Array<{ role: string; content?: string }>; + }; + return (request.messages ?? []) + .filter(message => message.role === "user") + .map(message => message.content ?? ""); +} + +it("creates a resource the agent can use before the user approves it", async () => { + using publicApi = connect(harness.url); + using authenticated = await signUpScriptedUser(publicApi, "createres"); + using workspace = await authenticated.newGadget(); + const chatId = await workspace.newChat("Create a new test thing and read it.", MODEL_ID); + + // The turn runs to completion: creation does not suspend the agent the way requestConnection + // or an awaitDecision action does. + await waitForAgentSays(workspace, chatId, "Created the thing and read 42 from it."); + + // The bad resource type was a fixable rejection: the model retried within the same turn. + expect(toolResultShownToModel("create-bad-type")).toMatch(/can create/i); + // The successful creation told the agent the binding is live and approval is still pending. + expect(toolResultShownToModel("create-thing")).toContain("env.NEW_THING"); + // The binding worked from executeCode before any approval: the read reached the simulated + // resource (the fixture session answers 42). + expect(toolResultShownToModel("read-new-thing")).toContain("42"); + + // The creation action rode the normal approval flow into the chat and the action log. + const pending = await onlyPendingAction(workspace, "the creation action to be pending"); + expect(pending).toMatchObject({ + type: "action", + state: "pending", + description: { title: 'Create test thing "My Thing"' }, + }); + expect(pending.resourceUrl).toContain("/things/provisional-"); + const history = await workspace.getChatHistory(chatId); + expect(history.messages.some(message => + message.type === "action" && message.actionId === pending.id)).toBe(true); + + await workspace.approveAction(pending.id); + + // A second turn proves both replay (the binding is re-established from the recorded tool + // output) and the post-apply describe refresh (the new action's snapshot carries the real, + // no-longer-provisional resource URL). + await workspace.sendChatMessage(chatId, "Now set its value to 9.", MODEL_ID); + const write = await onlyPendingAction(workspace, "the write action to be pending"); + expect(write).toMatchObject({ + type: "action", + state: "pending", + description: { title: "Set the test value to 9" }, + }); + expect(write.resourceUrl).toContain("/things/created-"); + expect(write.resourceUrl).not.toContain("provisional"); + + // Replay told the model about the approval — the recorded tool result permanently says the + // resource doesn't exist yet, so without this the model's context never learns it now does. + const approval = userMessagesShownToModel().find(message => + message.includes("The user approved the creation of env.NEW_THING")); + expect(approval).toContain(write.resourceUrl); +}); + +it("kills the binding when the user rejects the creation", async () => { + using publicApi = connect(harness.url); + using authenticated = await signUpScriptedUser(publicApi, "createrej"); + using workspace = await authenticated.newGadget(); + const chatId = await workspace.newChat("Create a doomed test thing.", MODEL_ID); + await waitForAgentSays(workspace, chatId, "Created the doomed thing."); + + const pending = await onlyPendingAction(workspace, "the creation action to be pending"); + await workspace.rejectAction(pending.id); + + // The next turn's use of the binding fails with the gatekeeper's dead-binding explanation + // rather than silently simulating against nothing. + await workspace.sendChatMessage(chatId, "Read the doomed thing.", MODEL_ID); + await waitForAgentSays(workspace, chatId, "The doomed thing is gone."); + expect(toolResultShownToModel("read-doomed")).toContain("DEAD:"); + expect(toolResultShownToModel("read-doomed")).toMatch(/rejected/); + + // Replay told the model about the rejection too. + expect(userMessagesShownToModel().some(message => + message.includes("The user rejected the creation of env.DOOMED"))).toBe(true); +}); + +it("settles the queued action when the vendor fails after queueing it", async () => { + using publicApi = connect(harness.url); + using authenticated = await signUpScriptedUser(publicApi, "createfail"); + using workspace = await authenticated.newGadget(); + const chatId = await workspace.newChat("Create a failing test thing.", MODEL_ID); + await waitForAgentSays(workspace, chatId, "The creation failed."); + + // The tool reported a fixable rejection carrying the vendor's error... + expect(toolResultShownToModel("create-orphan")).toContain("Simulated post-queue failure"); + + // ...and the action the vendor had already queued was settled with the removed gatekeeper, + // not left pending forever (approve/reject would both fail on the missing facet). + const actions = (await workspace.listActions({ filter: "all" })).entries; + expect(actions.filter(action => action.state === "pending")).toEqual([]); + expect(actions.some(action => action.type === "action" && action.state === "rejected")) + .toBe(true); + expect(model.remainingSteps()).toBe(0); +}); diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index f87633c9bd..999feec8e3 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -39,6 +39,9 @@ const SUPPORTED_RESOURCES: SupportedResource[] = [{ urlPattern: `https://${VENDOR_HOST}/things/*`, title: "Test Thing", description: "A resource that exists only so tests can bind something.", + creatable: { + description: "Creates a new test thing with the given title.", + }, }]; const TYPES_CODE = ` @@ -161,7 +164,12 @@ function control(exports: Cloudflare.Exports): DurableObjectStub { // Vendor type AccountProps = { label: string }; -type BindingProps = AccountProps & { resourceUrl: string; ambient?: true }; +type BindingProps = AccountProps & { + resourceUrl: string; + ambient?: true; + /** Present on bindings minted by createResource(): the thing to create once approved. */ + creation?: { title: string }; +}; @validateRpc() export class GatekeeperVendor extends WorkerEntrypoint { @@ -250,6 +258,31 @@ export class TestAccount }; } + /** + * Mint a NEW test thing (createExternalResource): a provisional resource URL and a gatekeeper + * class that simulates the thing until the creation action is approved. + */ + async createResource(resourceUrlPattern: string, options: { title: string }): Promise<{ + class: DurableObjectClass>; + resource: SupportedResource; + resourceUrl: string; + }> { + if (resourceUrlPattern !== SUPPORTED_RESOURCES[0].urlPattern) { + throw new Error( + `The test gatekeeper cannot create resources of type "${resourceUrlPattern}".`); + } + const resourceUrl = `https://${VENDOR_HOST}/things/provisional-${crypto.randomUUID()}`; + return { + class: this.ctx.exports.TestGatekeeper({ + props: { + label: this.ctx.props.label, resourceUrl, creation: { title: options.title }, + }, + }), + resource: SUPPORTED_RESOURCES[0], + resourceUrl, + }; + } + /** The capability the overseer hands to addObserver() to say "this is the user asking". */ @skipRpcValidation() async getVerifier(): Promise> { @@ -362,6 +395,21 @@ export class TestGatekeeper tsType: "TestThing", }; } + const creation = this.ctx.props.creation; + if (creation) { + // Answered locally in both states: a created thing has no provider to describe from, and + // before approval there is nothing at the provider at all. + const createdUrl = this.ctx.storage.kv.get("createdUrl"); + return { + url: createdUrl ?? this.ctx.props.resourceUrl, + title: creation.title, + snippet: createdUrl + ? `The created test resource ${creation.title}.` + : `Test thing (pending creation): ${creation.title}.`, + suggestedBindingName: "TEST_THING", + tsType: "TestThing", + }; + } const name = decodeURIComponent(new URL(this.ctx.props.resourceUrl).pathname.split("/").pop()!); return { url: this.ctx.props.resourceUrl, @@ -382,10 +430,42 @@ export class TestGatekeeper } async startSession(approvalQueue: RpcStub): Promise { + if (this.ctx.storage.kv.get("creationRejected")) { + throw new Error( + "The user rejected creating this test thing; the binding is dead."); + } return new TestSessionTarget( approvalQueue, control(this.ctx.exports), this.ctx.props.label); } + /** Queue the creation of this test thing (createExternalResource). Idempotent. */ + async submitCreationAction(approvalQueue: RpcStub): Promise { + const creation = this.ctx.props.creation; + if (!creation) { + throw new Error("This test gatekeeper was not minted by createResource()."); + } + if (this.ctx.storage.kv.get("creationActionId") !== undefined) return; + const id = await control(this.ctx.exports).stageAction(this.ctx.props.label, 0); + this.ctx.storage.kv.put("creationActionId", id); + try { + await approvalQueue.submitAction(id, { + title: `Create test thing "${creation.title}"`, + description: `Create a new test thing titled **${creation.title}**.`, + implementsRevert: false, + actionKind: { tag: "create-thing", label: "Create thing" }, + }); + } catch (error) { + this.ctx.storage.kv.delete("creationActionId"); + await control(this.ctx.exports).discardAction(this.ctx.props.label, id); + throw error; + } + // Test knob: this title makes the call reject only after the action was durably queued, + // modeling a vendor that fails post-queue (the overseer must settle the orphaned action). + if (creation.title === "fail-after-queue") { + throw new Error("Simulated post-queue failure."); + } + } + /** * Admit an observer, or refuse on the test's instruction. * @@ -413,11 +493,29 @@ export class TestGatekeeper } async applyAction(action: number): Promise { + // The in-order guard the submitCreationAction contract requires: manual approval can target + // any pending action, so the gatekeeper itself must refuse to apply anything that depends on + // the thing existing until the creation has been applied. + const creationActionId = this.ctx.storage.kv.get("creationActionId"); + if (creationActionId !== undefined && action !== creationActionId && + this.ctx.storage.kv.get("createdUrl") === undefined) { + throw new Error( + "The test thing does not exist yet: approve its creation action before this one."); + } await control(this.ctx.exports).applyAction(this.ctx.props.label, action); + if (action === this.ctx.storage.kv.get("creationActionId")) { + // The thing now "exists": describe() flips from the provisional URL to the real one. + this.ctx.storage.kv.put( + "createdUrl", + this.ctx.props.resourceUrl.replace("/things/provisional-", "/things/created-")); + } } async rejectAction(action: number): Promise { await control(this.ctx.exports).discardAction(this.ctx.props.label, action); + if (action === this.ctx.storage.kv.get("creationActionId")) { + this.ctx.storage.kv.put("creationRejected", true); + } } async revertAction(_action: number): Promise { diff --git a/packages/workshop-backend/__tests__/agent-compaction.test.ts b/packages/workshop-backend/__tests__/agent-compaction.test.ts index d54dda2b4a..958596ab4d 100644 --- a/packages/workshop-backend/__tests__/agent-compaction.test.ts +++ b/packages/workshop-backend/__tests__/agent-compaction.test.ts @@ -361,6 +361,42 @@ describe("compaction checkpoint state", () => { expect(state.nextChangeId).toBe(1); }); + // Replay re-establishes a created resource's binding from the recorded tool output; once + // compaction swallows the creation call, the checkpoint must carry the same binding. + it("folds a created external resource's binding into the checkpoint", () => { + let state = buildState([ + { + ...message(0, agent, "Creating"), + toolCalls: [{ + toolCallId: "call_1", toolName: "createExternalResource", + input: {vendorId: "docs", resourceUrlPattern: "https://example.com/*", + title: "Notes", bindingName: "NOTES"}, + output: {gatekeeperId: 5, resourceUrl: "https://example.com/prov", message: "Created"}, + }], + }, + ], 1); + + expect(state.chatBindings).toContainEqual(["NOTES", {type: "workpiece", id: 5}]); + }); + + // A rejection is recorded as a string output with no `error` set; no binding was made, so none + // may be folded. + it("does not bind a rejected external-resource creation", () => { + let state = buildState([ + { + ...message(0, agent, "Creating"), + toolCalls: [{ + toolCallId: "call_1", toolName: "createExternalResource", + input: {vendorId: "docs", resourceUrlPattern: "https://example.com/*", + title: "Notes", bindingName: "NOTES"}, + output: "Cannot create the resource: no connected account.", + }], + }, + ], 1); + + expect(state.chatBindings.map(([name]) => name)).not.toContain("NOTES"); + }); + it("carries a previous checkpoint's proposed state forward", () => { let previous = { chatId: 1, compactedTo: 3, summary: "earlier", diff --git a/packages/workshop-backend/src/agent-compaction.ts b/packages/workshop-backend/src/agent-compaction.ts index 29d71e66c9..784b30db3f 100644 --- a/packages/workshop-backend/src/agent-compaction.ts +++ b/packages/workshop-backend/src/agent-compaction.ts @@ -1,4 +1,5 @@ -import {SUGGESTED_MODELS, WORKERS_AI_OUTPUT_LIMIT, type AiChatMessage, type AiModelConfig} +import {SUGGESTED_MODELS, WORKERS_AI_OUTPUT_LIMIT, isCreatedResourceSuccess, type AiChatMessage, + type AiModelConfig} from "@gadgets/workshop-shared/api"; import {composeCodeChange, type CodeChange} from "@gadgets/workshop-shared/code-change"; import type {Api, Message, Model} from "@earendil-works/pi-ai"; @@ -201,7 +202,8 @@ export function legacyChatBaseVersion( * Earliest turn a checkpoint cannot absorb, or undefined if none. A pending connection request * carries live accept/deny state that only its own message can answer, so the boundary stays behind * it. Provisional gadget creations and binding additions need no such protection: the checkpoint - * records them, and the registry rows they name are untouched by compaction. + * records them, and the registry rows they name are untouched by compaction. (Nor do pending + * creation actions: their decision arrives as a durable agentNudge, an ordinary message.) */ export function findProtectedFromSequence(messages: AiChatMessage[]): number | undefined { let protectedIndex = messages.findIndex( @@ -393,6 +395,11 @@ export function buildCompactionState( } else if (call.toolName === "createWorktree" && call.output !== undefined) { chatBindings.set(call.input.bindingName, {type: "workpiece", id: call.output.worktreeId}); + } else if (call.toolName === "createExternalResource" && + isCreatedResourceSuccess(call.output)) { + // Mirrors replay's re-establishment of the binding from the recorded output. + chatBindings.set(call.input.bindingName, + {type: "workpiece", id: call.output.gatekeeperId}); } } } else if (message.type === "agentCallback") { diff --git a/packages/workshop-backend/src/agent.ts b/packages/workshop-backend/src/agent.ts index f42e70594c..cd1ef46984 100644 --- a/packages/workshop-backend/src/agent.ts +++ b/packages/workshop-backend/src/agent.ts @@ -1,4 +1,4 @@ -import { AiChatMessage, AiChatAuthorInfo, AiToolCall, AiChatMessageBody, AgentSpawnerConfig, AiChatStreamEvent, BlueprintOutput, ChatGadgetPin, ChatCodeBase, WorkpieceId, type AiModelConfig, isTextLikeAttachmentMimeType, validateBindingName } from '@gadgets/workshop-shared/api'; +import { AiChatMessage, AiChatAuthorInfo, AiToolCall, AiChatMessageBody, AgentSpawnerConfig, AiChatStreamEvent, BlueprintOutput, ChatGadgetPin, ChatCodeBase, WorkpieceId, type AiModelConfig, type CreatedResourceOutput, isCreatedResourceSuccess, isTextLikeAttachmentMimeType, validateBindingName } from '@gadgets/workshop-shared/api'; import { applyCodeChange, codeChangeSerializedSize, replaceSpanChange, type CodeContent, type CodeChange, type FileChange } from '@gadgets/workshop-shared/code-change'; import { PDF_MIME_TYPE, modelApiSupportsPdfAttachments } from './chat-attachment-pdf'; @@ -386,6 +386,15 @@ export function makeStoredAssistantMessage(message: AssistantMessage): StoredAss }; } +/** Input of the createExternalResource tool (see the AgentHooks member for semantics). */ +export type CreateExternalResourceInput = { + vendorId: string; + resourceUrlPattern: string; + title: string; + bindingName: string; + accountId?: number; +}; + /** * Methods of OverseerImpl that runAgent() needs to call, extracted as an interface to avoid cyclic * dependencies. @@ -632,6 +641,21 @@ export interface AgentHooks { */ consumeCapturedConnectionRequests(chatId: number): AiChatMessageBody[]; + /** + * Create a new external resource via a connected account: resolves the vendor + creatable + * resource type, mints a provisional gatekeeper workpiece (GatekeeperUser.createResource + + * addGatekeeper), and submits the creation action attributed to this chat (its card is spliced + * via consumeCapturedActions like any action). Unlike requestConnection, no user action gates + * the binding — the gatekeeper simulates the resource until the creation is approved — so the + * turn does NOT end. A string result is a fixable rejection (unknown vendor, type not + * creatable, no usable account, missing authorization); the agent retries in-turn. Either + * shape is recorded verbatim as the tool call's output (see isCreatedResourceSuccess). + * `initiator` names whose connected accounts create the resource -- the turn's initiator, not + * the workspace owner, so a collaborator-driven turn uses (and enumerates) their own accounts. + */ + createExternalResource(chatId: number, input: CreateExternalResourceInput, + initiator: AiChatAuthorInfo): Promise; + /** * Blueprint hooks for the agent. * @@ -986,6 +1010,12 @@ let REQUEST_CONNECTION_TOOL_DESCRIPTION = ` Ask the user to connect a gatekeeper resource (e.g. a ClickHouse cluster, a GitHub repo). Pre-configure as much as you can: always pass vendorId, and pass resourceUrl when you can infer it (use listConnectableResources to learn the URL patterns). The request must resolve to a specific resource: if you pass a resourceUrl it must match one of the vendor's patterns, and if the vendor offers multiple resource types with no whole-instance option you MUST pass a matching resourceUrl. Otherwise the call is rejected with guidance and no card is shown — fix the request and try again. You also choose \`bindingName\`: the name the resource will have in your env once connected (you know why you want the resource, so pick a name that reflects its role). On success this shows the user an accept/deny card in the chat. It does NOT block: your turn ends after a successful call, and you will be resumed once the user accepts (the resource becomes available as \`env.\`, which you can describeBinding and use from executeCode; wire it into a Gadget with setGadgetBinding only if the Gadget's code needs it) or denies (your turn simply ends; wait for the user's next message). `.trim(); +let CREATE_EXTERNAL_RESOURCE_TOOL_DESCRIPTION = ` +Create a brand-new external resource (e.g. a new Google Doc) through an already-connected account. Only resource types marked "Creatable" by listConnectableResources support this; requestConnection is for binding a resource that already EXISTS. Pass the vendor id, the creatable type's urlPattern, a human-readable title for the new resource, and a bindingName (a JavaScript identifier not already in use; style: ALL_CAPS_WITH_UNDERSCORES). + +This does NOT block: on success the resource is immediately available as \`env.\` (describeBinding it, use it from executeCode) and your turn continues. The resource does not exist at the provider yet — the creation is submitted for the user's approval like any other action, and the gatekeeper simulates it locally until then, so edits you queue apply after the creation is approved, in order. If the call is rejected with guidance (bad name, unknown type, no connected account), fix the request and try again; if multiple accounts are connected the rejection lists their ids so you can retry with accountId or ask the user. +`.trim(); + let GIVE_UP_TOOL_DESCRIPTION = ` Gives up on handling the current callbacks, rejecting all outstanding callbacks with an error. Use this if you cannot fulfill the callbacks after attempting to do so. `.trim(); @@ -1926,6 +1956,22 @@ export async function runAgent( case "requestConnection": toolOutput = {text: toolCall.output ?? ""}; break; + case "createExternalResource": { + // Like createGadget: a creation tool can't re-run, so replay re-establishes + // the binding from the recorded output. + if (toolCall.output === undefined) { + throw new Error( + "createExternalResource tool call in log is missing its result"); + } + if (isCreatedResourceSuccess(toolCall.output)) { + chatBindings.set(toolCall.input.bindingName, + {type: "workpiece", id: toolCall.output.gatekeeperId}); + toolOutput = {text: jsonToolResultText(toolCall.output)}; + } else { + toolOutput = {text: toolCall.output}; + } + break; + } default: toolCall satisfies never; throw new Error("Unknown tool."); @@ -2204,7 +2250,8 @@ export async function runAgent( case "action": case "useGadget": case "error": - // No need to tell the agent about this. + // No need to tell the agent about this. (A creation action's decision reaches the model + // as a durable agentNudge appended when the user decides — see nudgeCreationDecision.) break; default: @@ -2473,7 +2520,10 @@ export async function runAgent( `can; use listConnectableResources to learn a vendor's resource URL patterns first). The ` + `user accepts or denies in the chat. If they accept, you'll be resumed and the resource ` + `becomes available as a binding in your env; if they deny, your turn ends and you wait ` + - `for the user's next message.\n` + + `for the user's next message. Resource types listConnectableResources marks "Creatable" ` + + `can instead be created brand-new with createExternalResource through an ` + + `already-connected account — that binding is usable immediately (the user approves the ` + + `creation as a normal action while you keep working).\n` + `If one of these services likely holds information relevant to the task, consider ` + `requesting a connection and reading from it before you answer, instead of answering from ` + `guesswork — a connection often gives you the real information. Connectable vendors:\n` + @@ -3222,6 +3272,76 @@ export async function runAgent( } } }), + + createExternalResource: defineTool({ + name: "createExternalResource", + label: "Create external resource", + description: CREATE_EXTERNAL_RESOURCE_TOOL_DESCRIPTION, + parameters: Type.Object({ + vendorId: Type.String({ + description: "Vendor id, as listed in the system prompt (e.g. 'google').", + }), + resourceUrlPattern: Type.String({ + description: + "The urlPattern of the resource type to create, exactly as listed by " + + "listConnectableResources. Only types marked Creatable can be created.", + }), + title: Type.String({ + description: + "Human-readable title for the new resource (e.g. the document title). Shown to " + + "the user on the approval card.", + }), + bindingName: Type.String({ + description: + "Name under which the new resource appears in your env immediately. Must be a " + + "JavaScript identifier not already in use; pick a name reflecting the resource's " + + "role. Style: ALL_CAPS_WITH_UNDERSCORES.", + }), + accountId: Type.Optional(Type.Number({ + description: + "Which connected account creates the resource. Only needed when several accounts " + + "of the vendor are connected (a rejection will list the candidate ids).", + })), + }), + execute: async (toolCallId, input) => { + try { + // Validate the chosen name before creating anything; like requestConnection, a bad + // name is a fixable message (not an error) so the agent retries within the same turn. + let nameProblem: string | undefined; + try { + validateBindingName(input.bindingName); + } catch (err) { + nameProblem = `${err instanceof Error ? err.message : err}`; + } + if (nameProblem === undefined && isNameInScope(input.bindingName)) { + nameProblem = `There is already a binding named "${input.bindingName}" in your ` + + `env. Choose a different name.`; + } + if (nameProblem !== undefined) { + let message = `Cannot create the resource: ${nameProblem}`; + return toolResult(message, { output: message }); + } + + let output = await hooks.createExternalResource(chatId, input, initiator); + if (!isCreatedResourceSuccess(output)) { + // Fixable rejection: recorded as the string output, no binding was made. + return toolResult(output, { output }); + } + + // The binding is live immediately — no user gate (contrast requestConnection). The + // creation action's card rides the step's captured actions like any other action. + chatBindings.set(input.bindingName, {type: "workpiece", id: output.gatekeeperId}); + + // Persist the full result as the tool's recorded output: replay can't re-run a + // creation tool, so it re-establishes the binding (and the exact text the model saw) + // from this recorded value instead. + return toolResult(jsonToolResultText(output), {output} as Partial); + } catch (error) { + toolCallNotes.set(toolCallId, { error: toolErrorText(error) }); + throw error; + } + } + }), }; // When the agent was started to handle callbacks, add the giveUp tool so it can bail out. diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 75f2b12f5a..8122240842 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -1,6 +1,6 @@ import { RpcCompatible, RpcStub, RpcTarget } from "capnweb"; import { validateRpc } from "capnweb-validate"; -import { Overseer, GadgetMetadata, UiBundle, WorkpieceId, WorkpieceSummary, WorkpiecesSubscriber, GadgetClient, GadgetBindingInfo, GatekeeperClient, ActionState, ActionLogEntry, ActionsSubscriber, ActionHistoryFilter, ActionHistoryPage, ChatGadgetPin, ChatCodeBase, ChatGadgetPinState, CodeChangeSubmission, CommitIdentity, CommitInfo, MergeChangesResult, AiChatMetadata, AiChatMessage, AiChatHistoryPage, AiChatSubscriber, AiChatAuthorInfo, AiModelConfig, AiChatMessageBody, AgentSpawnerConfig, ConsoleLogSubscriber, ConsoleLogEvent, CapsuleSpecifier, CollaboratorInfo, CollaboratorRole, AffectedCollaborator, ShareLinkInfo, GatekeeperCreationSpec, ObserverConfigCallback, ObserverBindingNeed, ObserverBindingFailure, BlueprintBindingAnnotation, BlueprintBinding, BlueprintMetadata, BlueprintOutput, MessageFormatRef, isOutputIcon, SpawnerEnvTarget, BlueprintGadgetSummary, AiChatStreamEvent, BlueprintScreenshotUpload, BLUEPRINT_SCREENSHOT_R2_PREFIX, blueprintScreenshotUrl, ChatAttachmentUpload, ChatAttachmentHandle, ChatAttachmentRef, BoundHookInfo, PreApprovableAction, PresenceParticipant, PresenceSubscriber, SlashCommandChoice, SlashCommandRequest, validateBindingName, createOpenGadgetError, OPEN_GADGET_ERROR_CODES, resolveSiteName, actionChangeTime } from '@gadgets/workshop-shared/api'; +import { Overseer, GadgetMetadata, UiBundle, WorkpieceId, WorkpieceSummary, WorkpiecesSubscriber, GadgetClient, GadgetBindingInfo, GatekeeperClient, ActionState, ActionLogEntry, ActionsSubscriber, ActionHistoryFilter, ActionHistoryPage, ChatGadgetPin, ChatCodeBase, ChatGadgetPinState, CodeChangeSubmission, CommitIdentity, CommitInfo, MergeChangesResult, AiChatMetadata, AiChatMessage, AiChatHistoryPage, AiChatSubscriber, AiChatAuthorInfo, AiModelConfig, AiChatMessageBody, AgentSpawnerConfig, ConsoleLogSubscriber, ConsoleLogEvent, CapsuleSpecifier, CollaboratorInfo, CollaboratorRole, AffectedCollaborator, ShareLinkInfo, GatekeeperCreationSpec, ObserverConfigCallback, ObserverBindingNeed, ObserverBindingFailure, BlueprintBindingAnnotation, BlueprintBinding, BlueprintMetadata, BlueprintOutput, MessageFormatRef, isOutputIcon, SpawnerEnvTarget, BlueprintGadgetSummary, AiChatStreamEvent, BlueprintScreenshotUpload, BLUEPRINT_SCREENSHOT_R2_PREFIX, blueprintScreenshotUrl, ChatAttachmentUpload, ChatAttachmentHandle, ChatAttachmentRef, BoundHookInfo, PreApprovableAction, PresenceParticipant, PresenceSubscriber, SlashCommandChoice, SlashCommandRequest, type CreatedResourceOutput, isCreatedResourceSuccess, validateBindingName, createOpenGadgetError, OPEN_GADGET_ERROR_CODES, resolveSiteName, actionChangeTime } from '@gadgets/workshop-shared/api'; import { applyCodeChange, changedGadgets, codeChangeSerializedSize, composeCodeChange, diffFiles, transformCodeChange, validateCodeChangeContent, validateCodeChangeSchema, type CodeContent, type CodeChange } from "@gadgets/workshop-shared/code-change"; @@ -30,7 +30,7 @@ import { getAiGatewayLogCost, type AiGatewayLogRoute, } from "./ai-gateway"; -import { AgentGadgetInfo, AgentHooks, AiChatAgentContext, CHAT_CHANGE_MESSAGE_BUDGET, ChatBindingEntry, SeedBindingInfo, runAgent, makeStorableArgs, summarizeArgs, type AgentStepChange, type AiChatMessageBodyWithModelData, type CompactionCheckpoint, type StoredAssistantMessage, type WorktreeTurnAccess } from "./agent"; +import { AgentGadgetInfo, AgentHooks, AiChatAgentContext, CHAT_CHANGE_MESSAGE_BUDGET, ChatBindingEntry, SeedBindingInfo, runAgent, makeStorableArgs, summarizeArgs, type AgentStepChange, type AiChatMessageBodyWithModelData, type CompactionCheckpoint, type CreateExternalResourceInput, type StoredAssistantMessage, type WorktreeTurnAccess } from "./agent"; import { WorktreeSessionImpl } from "./worktree-session"; import WORKTREE_BINDING_TYPES from "./worktree-binding.txt"; import { deploymentOutputForBlueprint, FormatOffer, listFormatOffers, readAdminConfig } from "./admin-config"; @@ -286,6 +286,28 @@ type GatekeeperRecord = { class: GatekeeperClass, hook?: string, // export name to which the gatekeeper's hook is connected + // Present while a createExternalResource-minted gatekeeper's resource exists only locally: + // describe() reports a provisional URL until the creation action (queued first, applied first) + // is applied, after which applyPendingAction re-denormalizes the real description and clears + // this marker. + provisional?: true; + + // Present while a createExternalResource mint is not yet backed by the chat log: set in the + // same put as `provisional`, cleared at the step barrier that records the tool call (see + // addChatMessages). An unstamped marker with no active turn is a mid-step crash orphan -- + // #reapPendingGatekeepers rejects its queued actions and removes it, mirroring + // GadgetRecord.pending's lifecycle (no sequence: creations ride an ordinary message, with no + // merge/revert to compare against). Distinct from `provisional`, which means "URL not real + // yet" and outlives the barrier. + pending?: {chatId: number}; + + // Present on a createExternalResource mint: the agent's env name for the resource (stamped at + // the mint) and the creation action's id (stamped by submitAction on the first queued action -- + // the vendor queues the creation first, and this makes that ordering the definition). Never + // cleared; drives the post-apply describe refresh, the crash reap's keep check, and the + // decision nudges. + creation?: {bindingName: string, actionId?: number}; + // Records how this gatekeeper was originally created, enabling blueprint metadata derivation. creationSpec?: GatekeeperCreationSpec; @@ -2456,6 +2478,44 @@ class OverseerImpl implements AgentHooks { let meta = this.storage.chatMeta.get(chatId); if (meta) this.storage.chatMeta.put(meta); } + + // Gatekeepers minted by createExternalResource follow the same barrier lifecycle and are + // swept on the same schedule. (No chatMeta re-put: gatekeepers don't participate in the + // derived proposedChangeWorkpieces.) + this.#reapPendingGatekeepers(chatId); + } + + // Reap crash-orphaned provisional gatekeepers minted by createExternalResource for the given + // chat (see GatekeeperRecord.pending: set durably at the mint, cleared at the step barrier that + // records the tool call). Runs on reconcilePendingGadgets' schedule -- never mid-step, when an + // unstamped marker legitimately exists -- and on chat deletion. Best-effort per gatekeeper, + // like the gadget sweep. + #reapPendingGatekeepers(chatId: number): void { + for (let record of Array.from(this.storage.gatekeepers.list())) { + if (record.pending?.chatId !== chatId) continue; + try { + // Keep the gatekeeper iff its creation action was actually applied: `provisional` clears + // on the post-apply describe refresh, but that refresh is best-effort, so accept an + // approved creation action as proof too -- reaping on `provisional` alone could sever a + // real provider resource. + let creationId = record.creation?.actionId; + let created = !record.provisional || (creationId !== undefined && + this.storage.actions.get(creationId)?.state === "approved"); + if (created) { + delete record.pending; + this.storage.gatekeepers.put(record); + continue; + } + + // A crash orphan: its step's message is by construction lost, so nothing in the log + // backs the creation (and the resumed turn may have minted a replacement). + this.#rejectPendingActionsAndRemoveGatekeeper(record.id); + } catch (err) { + this.logger.warn("failed to reap pending gatekeeper", { + event: "gatekeeper.pending.reconcile.failed", chatId, error: err, + }); + } + } } // Auto-create the workspace's single gadget and record it as the default gadget. New workspaces @@ -2661,6 +2721,7 @@ class OverseerImpl implements AgentHooks { this.bumpVersion([gadget.id]); } } + this.#reapPendingGatekeepers(chatId); } // Disable (if needed) and delete a bound hook, updating its action-log record to match. @@ -5255,6 +5316,62 @@ class OverseerImpl implements AgentHooks { this.gitCache.convertPushMarksToOnRemote(record.id); this.storage.actions.put(record); }); + + // A gatekeeper minted by createExternalResource was described with a provisional URL; once + // its creation action is approved (this action, or an earlier one whose refresh failed), + // describe() reports the real resource, so refresh the denormalized copy and retire the + // marker. The point read keeps an invalidated edit that applies before the creation from + // clearing the marker early. Best-effort: on failure the marker stays set and the next + // applied action retries. + let gatekeeperRecord = this.storage.gatekeepers.get(record.gatekeeperId); + let creationId = gatekeeperRecord?.creation?.actionId; + if (gatekeeperRecord?.provisional && creationId !== undefined && + this.storage.actions.get(creationId)?.state === "approved") { + try { + let description = await gatekeeper.describe(); + // Re-read after the await: a concurrent removeGatekeeper during the describe() would + // otherwise be resurrected by putting the stale record back. + gatekeeperRecord = this.storage.gatekeepers.get(record.gatekeeperId); + if (gatekeeperRecord?.provisional) { + gatekeeperRecord.resourceTitle = description.title; + gatekeeperRecord.resourceUrl = description.url; + gatekeeperRecord.hasSlashCommands = description.hasSlashCommands; + // Blueprint export's suggestValue reads creationSpec.resourceUrl; retire the + // provisional URL there too or exported blueprints would suggest a dead resource. + if (gatekeeperRecord.creationSpec?.type === "gatekeeper") { + gatekeeperRecord.creationSpec.resourceUrl = description.url; + } + delete gatekeeperRecord.provisional; + this.storage.gatekeepers.put(gatekeeperRecord); + } + } catch (error) { + this.logger.warn("failed to refresh created resource description after apply", { + event: "gatekeeper.created.describe.refresh.failed", + gatekeeperId: record.gatekeeperId, error, + }); + } + } + + if (record.id === creationId && record.caller.from === "agent" && + gatekeeperRecord !== undefined) { + this.nudgeCreationDecision(record.caller.chatId, gatekeeperRecord, "approved", resolvedBy); + } + } + + // Record the user's verdict on a created resource in its chat's log: the creation tool's + // recorded result permanently says the resource doesn't exist yet, so the model only learns + // the decision from this durable nudge (replayed as a user message, invisible in the UI). + nudgeCreationDecision(chatId: number, gatekeeper: GatekeeperRecord, + decision: "approved" | "rejected", author: AiChatAuthorInfo) { + if (gatekeeper.creation === undefined) return; + if (this.storage.chatMeta.get(chatId) === undefined) return; // Chat since deleted. + let name = `env.${gatekeeper.creation.bindingName}`; + let text = decision === "approved" + ? `The user approved the creation of ${name}.` + (gatekeeper.provisional ? `` : + ` The resource now exists at ${gatekeeper.resourceUrl}.`) + : `The user rejected the creation of ${name}; it will not be created at the provider. ` + + `Do not retry; wait for the user to tell you how to proceed.`; + this.addChatMessages(chatId, author, [{type: "agentNudge", text}]); } // Apply all currently-eligible pending actions of the given gatekeeper, in ascending id order. @@ -5358,6 +5475,26 @@ class OverseerImpl implements AgentHooks { return new GatekeeperClientImpl(this, id, facet, undefined, joinAs); } + // Reject a gatekeeper's still-pending actions, then remove it, in one durable step -- so no + // pending record survives pointing at a dead gatekeeper (approve/reject would fail forever on + // the missing facet). appliedAt is required (the byLastChanged resume-replay index keys on it); + // clearPushMarks matches rejectAction (a no-op for pushless actions). Rejecting first empties + // the queue, so removeGatekeeper's own push-mark loop is a no-op. No gatekeeper-side + // rejectAction RPC: the facet is deleted outright and nothing observes its internal pending + // state after removal. + #rejectPendingActionsAndRemoveGatekeeper(id: number) { + this.storage.transaction(() => { + for (let action of Array.from(this.storage.actions.pendingByGatekeeper.get(id))) { + if (action.type !== "action") continue; + action.state = "rejected"; + action.appliedAt = new Date(); + this.gitCache.clearPushMarks(action.id); + this.storage.actions.put(action); + } + this.removeGatekeeper(id); + }); + } + // Destroy a gatekeeper (connection) workpiece. Any binding edges pointing at it are severed so // no gadget's env retains a dangling entry. (This is distinct from merely unbinding it from one // gadget -- GadgetClient.unbind() -- which leaves the gatekeeper alive, possibly orphaned.) @@ -5822,6 +5959,12 @@ class OverseerImpl implements AgentHooks { if (description.pushedCommits !== undefined && description.pushedCommits.length > 0) { this.gitCache.markPushClosure(gatekeeperId, actionId, description.pushedCommits); } + // The first action queued against a createExternalResource mint IS the creation; stamp + // its identity in the same durable step as the action itself. + if (gatekeeper?.creation !== undefined && gatekeeper.creation.actionId === undefined) { + gatekeeper.creation.actionId = actionId; + this.storage.gatekeepers.put(gatekeeper); + } this.storage.actions.put(record); }); this.#associateAction(caller, actionId); @@ -7552,7 +7695,8 @@ class OverseerImpl implements AgentHooks { if (capsule.bindingName !== undefined) taken.add(capsule.bindingName); } for (let call of msg.toolCalls ?? []) { - if ((call.toolName === "createGadget" || call.toolName === "createWorktree") && + if ((call.toolName === "createGadget" || call.toolName === "createWorktree" || + call.toolName === "createExternalResource") && call.input.bindingName !== undefined) { taken.add(call.input.bindingName); } @@ -7779,6 +7923,13 @@ class OverseerImpl implements AgentHooks { if (call.output && !nameByTarget.has(call.output.worktreeId)) { nameByTarget.set(call.output.worktreeId, call.input.bindingName); } + } else if (call.toolName === "createExternalResource") { + // The name is taken even on rejection (matches chatScopeNames' overclaiming). + taken.add(call.input.bindingName); + if (isCreatedResourceSuccess(call.output) && + !nameByTarget.has(call.output.gatekeeperId)) { + nameByTarget.set(call.output.gatekeeperId, call.input.bindingName); + } } } } else if (msg.type === "connectionRequest") { @@ -8452,6 +8603,23 @@ class OverseerImpl implements AgentHooks { } } + // Clear the crash-orphan marker of any gatekeeper whose createExternalResource call this + // message records: the log now backs the creation (see GatekeeperRecord.pending). Same + // synchronous step as the message write, so the log and the registry can never disagree. + // A rejected creation left no gatekeeper to unstamp. + if (msg.type === "message") { + for (let call of msg.toolCalls ?? []) { + if (call.toolName === "createExternalResource" && + isCreatedResourceSuccess(call.output)) { + let gatekeeper = this.storage.gatekeepers.get(call.output.gatekeeperId); + if (gatekeeper?.pending?.chatId === chatId) { + delete gatekeeper.pending; + this.storage.gatekeepers.put(gatekeeper); + } + } + } + } + this.storage.chats.put({ chatId, sequence, @@ -8760,10 +8928,16 @@ class OverseerImpl implements AgentHooks { let lines = [`Resource types offered by "${vendorId}" (${vendor.description.displayName}):`]; for (let r of vendor.supportedResources) { lines.push(`* ${r.title} — urlPattern: ${r.urlPattern}\n ${r.description}`); + if (r.creatable) lines.push(` Creatable: ${r.creatable.description}`); } lines.push( `\nTo request one, call requestConnection with vendorId="${vendorId}" and a resourceUrl ` + `matching one of the patterns above (or omit resourceUrl to let the user pick).`); + if (vendor.supportedResources.some(r => r.creatable)) { + lines.push( + `Types marked "Creatable" can also be created brand-new with createExternalResource ` + + `(requires an already-connected "${vendorId}" account).`); + } return lines.join("\n"); } @@ -8835,6 +9009,95 @@ class OverseerImpl implements AgentHooks { return result; } + // Create a brand-new external resource (createExternalResource tool). Unlike requestConnection, + // no user action gates the binding: the gatekeeper simulates the resource locally, and the + // provider-side creation is an ordinary pending action (captured for this chat, so its card + // lands in the transcript at the step barrier). `created: false` is a fixable rejection — the + // agent should adjust and retry in the same turn. + async createExternalResource(chatId: number, input: CreateExternalResourceInput, + initiator: AiChatAuthorInfo): Promise { + let vendors = await this.#listGatekeeperVendorsCached(); + let vendor = vendors.find(v => v.id === input.vendorId); + if (!vendor) { + return `Cannot create a resource: unknown vendor "${input.vendorId}". ` + + `Available vendors: ${vendors.map(v => v.id).join(", ") || "(none)"}.`; + } + + let resource = vendor.supportedResources.find( + r => r.urlPattern === input.resourceUrlPattern); + if (!resource?.creatable) { + let creatable = vendor.supportedResources.filter(r => r.creatable); + return creatable.length === 0 + ? `"${vendor.description.displayName}" does not support creating new resources.` + : `Cannot create a resource of type "${input.resourceUrlPattern}". ` + + `"${vendor.description.displayName}" can create: ` + + creatable.map(r => `${r.title} (${r.urlPattern})`).join(", ") + `.`; + } + + // Mint the provisional gatekeeper class through the *initiator's* user DO (the admin-check + // chokepoint) -- connected accounts are per-user, so a collaborator-driven turn creates the + // resource under (and enumerates) the collaborator's accounts, not the owner's. The same + // initiator.id resolution as listAvailableBlueprints. Its failures are agent-readable by + // contract: no usable account, ambiguous accounts, missing authorization. + let minted; + try { + let userStub = wrapDoStubForTelemetry( + this.users.get(this.users.idFromName(initiator.id)), this.logger); + minted = await userStub.createResourceGatekeeper( + input.vendorId, input.accountId, input.resourceUrlPattern, {title: input.title}); + } catch (error) { + return `Cannot create the resource: ${stringifyError(error)}`; + } + + let client = await this.addGatekeeper(minted.class, { + type: "gatekeeper", + vendorId: minted.vendorId, + resourceUrl: minted.resourceUrl, + typeUrlPattern: minted.typeUrlPattern, + }); + let gatekeeperId = await client.getId(); + + // Mark the record provisional so the post-apply describe refresh (applyPendingAction) knows + // to re-denormalize once the resource really exists, pending so a mid-step crash before the + // tool call reaches the log leaves a reapable orphan rather than a live duplicate (see + // GatekeeperRecord.pending), and stamp the creation identity for the decision nudges. + // (addGatekeeper just created the record.) + let record = this.storage.gatekeepers.get(gatekeeperId)!; + record.provisional = true; + record.pending = {chatId}; + record.creation = {bindingName: input.bindingName}; + this.storage.gatekeepers.put(record); + + // Have the facet queue its creation action, attributed to this chat so the approval card is + // spliced into the transcript. On failure, no half-created workpiece survives. + // (submitCreationAction is optional on Gatekeeper; a creatable-advertising vendor must + // implement it, so view the facet through the usual Required> stub shape.) + let awaitDecisionBefore = this.#capturedActions.get(chatId)?.awaitDecision ?? false; + try { + let facet = this.getGatekeeperFacet(gatekeeperId) as unknown as + Fetcher & Required, "submitCreationAction">>>; + using queue = new RpcStub( + new ApprovalQueueImpl(this, gatekeeperId, {from: "agent", chatId})); + await facet.submitCreationAction(queue as unknown as ApprovalQueue); + } catch (error) { + // The facet may have queued its action durably before the RPC failed; settle it with the + // gatekeeper or the pending record would be unresolvable (its facet is gone). Any + // awaitDecision latch it set must unwind with it -- the settled action can never be + // decided, and suspending the turn would hide the rejection from the model. + this.#rejectPendingActionsAndRemoveGatekeeper(gatekeeperId); + let captured = this.#capturedActions.get(chatId); + if (captured) captured.awaitDecision = awaitDecisionBefore; + return `Cannot create the resource: ${stringifyError(error)}`; + } + + return { gatekeeperId, resourceUrl: minted.resourceUrl, message: + `Created "${input.title}" (${resource.title}), available as env.${input.bindingName} ` + + `in executeCode immediately — use describeBinding to learn its API. The resource ` + + `does not exist at ${vendor.description.displayName} yet: the user must approve the ` + + `creation action (and any edits you queue) before anything reaches the provider, but ` + + `you can keep working against the simulated resource without waiting.` }; + } + // --- Blueprint hooks for the agent --- // List the blueprints the turn's initiator could instantiate with createGadget: their own @@ -11226,17 +11489,34 @@ class OverseerClientInterface extends RpcTarget implements Overseer { // can't leave the action rejected with the gatekeeper but still "pending" in storage. let profile = await this.#getClientProfile(); - await gatekeeper.rejectAction(action.action); + // Rejecting a creation dooms the gatekeeper's other queued actions (in-order application + // means they could only ever apply after a creation that now never will), so settle them too + // rather than stranding cards the user would have to reject one by one. + let record = this.impl.storage.gatekeepers.get(action.gatekeeperId); + let rejectsCreation = record?.creation?.actionId === id; + let doomed = rejectsCreation + ? Array.from(this.impl.storage.actions.pendingByGatekeeper.get(action.gatekeeperId)) + .filter((a): a is ActionRecord & {type: "action"} => + a.type === "action" && a.id !== id) + : []; + + for (let target of [action, ...doomed]) { + await gatekeeper.rejectAction(target.action); + + target.state = "rejected"; + target.appliedAt = new Date(); + target.resolvedBy = profile; + // A rejected push's pending-push marks are removed in the same durable step as the state + // change (nothing was transmitted, so nothing became proven). No-op for pushless actions. + this.impl.storage.transaction(() => { + this.impl.gitCache.clearPushMarks(target.id); + this.impl.storage.actions.put(target); + }); + } - action.state = "rejected"; - action.appliedAt = new Date(); - action.resolvedBy = profile; - // A rejected push's pending-push marks are removed in the same durable step as the state - // change (nothing was transmitted, so nothing became proven). No-op for pushless actions. - this.impl.storage.transaction(() => { - this.impl.gitCache.clearPushMarks(action.id); - this.impl.storage.actions.put(action); - }); + if (rejectsCreation && action.caller.from === "agent") { + this.impl.nudgeCreationDecision(action.caller.chatId, record!, "rejected", profile); + } // Deny leaves the turn ended, like denyConnectionRequest. The rejected record also prevents a // sibling approval from resuming this turn. diff --git a/packages/workshop-backend/src/user.ts b/packages/workshop-backend/src/user.ts index 5ad4a239f7..93e646e4b0 100644 --- a/packages/workshop-backend/src/user.ts +++ b/packages/workshop-backend/src/user.ts @@ -53,6 +53,7 @@ export type ProvidedAccountInfo = { // usable directly, the way the runtime stub actually behaves. type AccountCreatorStub = Required>; type SingletonAccountStub = Required>; +type ResourceCreatorStub = Required>; function areCredentialsValid(record: ConnectedAccountRecord): boolean { if (record.credentialsExpired) return false; @@ -1690,6 +1691,72 @@ export class UserDurableObject extends DurableObject { return {class: cls, vendorId: account.vendorId, typeUrlPattern: resource.urlPattern}; } + /** + * Mint a gatekeeper for a NEW resource of type `resourceUrlPattern` via + * GatekeeperUser.createResource() — the urlPattern→capability chokepoint for creations, applying + * the same admin disable-set checks as getGatekeeperClassFor(). When `accountId` is omitted and + * exactly one usable account is connected for the vendor, that account is used; otherwise the + * error enumerates the candidates so the agent can retry with an accountId or ask the user. + * Every thrown message here is agent-readable: the overseer surfaces it as a fixable tool result. + */ + async createResourceGatekeeper( + vendorId: string, accountId: number | undefined, resourceUrlPattern: string, + options: {title: string}) + : Promise<{class: DurableObjectClass>, vendorId: string, + typeUrlPattern: string, resourceUrl: string}> { + let account: ConnectedAccountRecord; + if (accountId !== undefined) { + let record = this.storage.connectedAccounts.get(accountId); + if (!record || record.vendorId !== vendorId) { + throw new Error(`There is no connected "${vendorId}" account with id ${accountId}.`); + } + account = record; + } else { + let candidates = [...this.#connectedAccountRecords()] + .filter(rec => rec.vendorId === vendorId && areCredentialsValid(rec)); + if (candidates.length === 0) { + throw new Error( + `No connected "${vendorId}" account is available. Use requestConnection to ask the ` + + `user to connect one first.`); + } + if (candidates.length > 1) { + let names = candidates.map(rec => + `${rec.id} (${rec.description.uniqueName ?? rec.description.displayName ?? "unnamed"})`); + throw new Error( + `Multiple "${vendorId}" accounts are connected: ${names.join(", ")}. Retry with the ` + + `accountId of the one to use, or ask the user which they prefer.`); + } + account = candidates[0]; + } + + // No stub-side probe for the optional method: RPC stubs cannot reliably report whether an + // optional method exists (see the note on GatekeeperUser's singleton section), so we view the + // stub through the Required> shape. The caller gates on SupportedResource.creatable; + // a vendor that advertised it without implementing createResource() surfaces here as an RPC + // error, which the overseer relays to the agent. + let {class: cls, resource, resourceUrl} = + await (account.account as unknown as ResourceCreatorStub) + .createResource(resourceUrlPattern, options); + + // Check the admin disable-set against the pattern the vendor actually resolved, after the + // RPC, exactly like getGatekeeperClassFor -- the vendor is the authority on which resource + // type a request maps to. (createResource mints only the class and a provisional URL; the + // provider-side creation is a separate pending action, so nothing external happened yet.) + let config = await readAdminConfig(this.env); + let vendorIdLower = account.vendorId.toLowerCase(); + if (config.disabledGatekeepers.includes(vendorIdLower)) { + throw new Error( + `The "${account.vendorId}" gatekeeper is disabled on this deployment by an administrator.`); + } + if (isResourceDisabled(config, vendorIdLower, resource.urlPattern)) { + throw new Error( + `The "${resource.title}" resource is disabled on this deployment by an administrator.`); + } + + return {class: cls, vendorId: account.vendorId, typeUrlPattern: resource.urlPattern, + resourceUrl}; + } + /** * Mint a verifier from one of THIS user's connected accounts, identified by accountId. The * overseer passes the returned verifier to a gatekeeper's `addObserver()` so the gatekeeper can diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index f41f8ab0b0..7122779092 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -82,6 +82,7 @@ import { WorkpieceId, BlueprintOutput, MessageFormatRef, + isCreatedResourceSuccess, } from "@gadgets/workshop-shared/api"; import { composeCodeChange, type CodeChange } from "@gadgets/workshop-shared/code-change"; import type { ChatChangeRow } from "./otClient"; @@ -619,6 +620,10 @@ function getToolCallSummary( return { verb: "Listed connectable resources", target: tc.input.vendorId }; case "requestConnection": return { verb: "Requested connection", target: tc.input.vendorId }; + case "createExternalResource": + return isCreatedResourceSuccess(tc.output) + ? { verb: "Created external resource", target: tc.input.title } + : { verb: "Tried to create external resource", target: tc.input.title }; } // Compile-time exhaustiveness check. const _exhaustive: never = tc; @@ -700,6 +705,8 @@ function describeToolCallCount(toolName: AiToolCall["toolName"], count: number): return `Listed connectable resources`; case "requestConnection": return count === 1 ? "Requested a connection" : `Requested ${count} connections`; + case "createExternalResource": + return `Created ${pluralize(count, "external resource")}`; } const _exhaustive: never = toolName; return _exhaustive; @@ -728,6 +735,7 @@ function getToolIcon( case "saveCapsuleAsBinding": return LinkSimple; case "createGadget": + case "createExternalResource": return Plus; case "createWorktree": return GitBranch; @@ -762,6 +770,8 @@ function getProvisionalToolLabel(toolName: AiToolCall["toolName"] | null | undef return "Creating gadget"; case "createWorktree": return "Creating worktree"; + case "createExternalResource": + return "Creating external resource"; case "executeCode": return "Running code"; case "webFetch": @@ -798,6 +808,7 @@ function getProvisionalToolVerb(toolName: AiToolCall["toolName"]): string { case "listBlueprints": return "Listing blueprints"; case "listConnectableResources": return "Listing connectable resources"; case "requestConnection": return "Requesting a connection"; + case "createExternalResource": return "Creating external resource"; } const _exhaustive: never = toolName; return _exhaustive; @@ -823,6 +834,7 @@ function describeProvisionalToolCount(toolName: AiToolCall["toolName"], count: n case "listBlueprints": return "Listing blueprints"; case "listConnectableResources": return "Listing connectable resources"; case "requestConnection": return `Requesting ${pluralize(count, "connection")}`; + case "createExternalResource": return `Creating ${pluralize(count, "external resource")}`; } const _exhaustive: never = toolName; return _exhaustive; diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index 78eff489cf..515a52f6ab 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -3233,8 +3233,53 @@ export type AiToolCall = { bindingName?: string; }; output?: string; +} | { + /** + * Create a brand-new external resource through an already-connected account (contrast + * requestConnection, which binds an EXISTING resource and requires user acceptance before the + * agent proceeds). Non-blocking: the gatekeeper simulates the resource until the user approves + * the creation action (a normal action card), so the binding is usable immediately. + */ + toolName: "createExternalResource"; + input: { + vendorId: string; + + /** urlPattern of the resource type to create (one whose SupportedResource is creatable). */ + resourceUrlPattern: string; + + /** Human-readable title for the new resource (e.g. the document title). */ + title: string; + + /** Name under which the resource appears in the chat's env (see validateBindingName()). */ + bindingName: string; + + /** Which connected account creates the resource; required only when several match. */ + accountId?: number; + }; + + /** + * On success, the full tool result: the created gatekeeper workpiece, its provisional + * resourceUrl, and the message shown to the model — replay re-binds and re-renders from it + * instead of re-creating, like createGadget. A string output is a fixable rejection returned + * to the model (no binding was made), like requestConnection's output. + */ + output?: CreatedResourceOutput | string; }); +/** Success output of a createExternalResource call (see the AiToolCall member). */ +export type CreatedResourceOutput = {gatekeeperId: WorkpieceId, resourceUrl: string, message: string}; + +/** + * True iff a createExternalResource output is the structured success shape — the only shape under + * which a binding was made. A string output is a fixable rejection; it is not an error (no + * `error` is set on the call), so error-based filters do not catch it. Every consumer that scans + * recorded tool calls must gate on this, not on `output !== undefined`. + */ +export function isCreatedResourceSuccess(output: CreatedResourceOutput | string | undefined) + : output is CreatedResourceOutput { + return typeof output === "object"; +} + // TODO: Extend AiToolCall for code-mode tool calls. // - Includes inline audit logs from the action. // - Actions can be approved or rejected inline. diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index 930589c99d..98082e7367 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -255,6 +255,18 @@ export type SupportedResource = { * If omitted/false, the resource type is not separately grantable. */ grantable?: boolean; + + /** + * Present when an agent may create a brand-new resource of this type via the + * createExternalResource tool, without user pre-approval. The vendor's GatekeeperUser must + * implement createResource() for this urlPattern, and the gatekeeper class it returns must + * implement submitCreationAction(). The gatekeeper simulates the new resource locally until + * the user approves the creation action. + */ + creatable?: { + /** What creation does, e.g. "Creates a new, empty Google Doc with the given title." */ + description: string; + }; } /** Removes every trailing slash from a string in linear time. */ @@ -593,6 +605,27 @@ export interface GatekeeperUser extends WorkerEntrypoint { resource: SupportedResource; }>; + /** + * Create a NEW resource of the type identified by `resourceUrlPattern` (a urlPattern from + * getSupportedResources() whose `creatable` is set). Mints a provisional identity for the + * resource — no provider API call, no user interaction — and returns a gatekeeper class imbued + * with it (via `ctx.props`), exactly as getGatekeeperClassFor() does for existing resources, + * plus the provisional `resourceUrl` that names the resource until it really exists. + * + * The returned class MUST implement Gatekeeper.submitCreationAction(). The provider-side + * creation happens only when the user approves that action; until then the gatekeeper simulates + * the resource locally, and describe() must not call the provider. + * + * Throws with an agent-readable message when the account cannot create this resource type + * (e.g. its authorization does not cover the needed scopes); callers surface the message. + */ + createResource?(resourceUrlPattern: string, options: {title: string}): Promise<{ + class: DurableObjectClass>; + resource: SupportedResource; + /** Provisional URL of the new resource; replaced by the real URL once created. */ + resourceUrl: string; + }>; + /** * Get the UI used to choose a specific resource. * `resourceUrlPattern` is the `urlPattern` associated with the supported resource. @@ -738,6 +771,20 @@ export interface Gatekeeper extends DurableObject { */ startSession(approvalQueue: RpcStub): Promise; + /** + * For gatekeepers minted by GatekeeperUser.createResource(): submit the pending "create this + * resource" action to the given approval queue. The Overseer calls this exactly once, + * immediately after adding the workpiece, with a queue scoped to it and attributed to the + * creating agent's chat (so the approval card lands there). Implementations must be idempotent + * (a retried call must not queue a second creation) and must queue the creation FIRST. + * + * Ordering is the gatekeeper's responsibility: the platform applies auto-approvals in order, + * but a manual approval can target any pending action, so the gatekeeper must itself reject + * applyAction() of any action that depends on the resource existing until the creation has + * been applied. Only gatekeepers reachable via createResource() need implement this. + */ + submitCreationAction?(approvalQueue: RpcStub): Promise; + /** * Bounded, user-specific metadata the agent uses to discover entries reachable through this * gatekeeper's session, without paging the full session API. Implemented only by gatekeepers From 192f9c056da795864e0f91f8452c64f1a481db57 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 4 Sep 2026 18:38:29 -0500 Subject: [PATCH 2/7] fix: harden external resource creation lifecycle --- .../workshop-agent-create-resource.test.ts | 248 ++++++++++-------- .../__tests__/chat-changes.test.ts | 41 +++ packages/workshop-backend/src/agent.ts | 3 + packages/workshop-backend/src/overseer.ts | 108 +++++--- packages/workshop-backend/src/user.ts | 4 + .../workshop-frontend/src/ChatInterface.tsx | 21 +- packages/workshop-shared/src/gatekeeper.ts | 4 + 7 files changed, 286 insertions(+), 143 deletions(-) diff --git a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts index bf9083b2c9..90198f80ec 100644 --- a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts +++ b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts @@ -9,7 +9,7 @@ import type { AiChatAuthorInfo, AiModelConfig, AuthenticatedApi, Overseer, PublicApi, } from "@gadgets/workshop-shared/api"; import { startTestGatekeeperHarness, TEST_VENDOR_ID, type Harness } from "../src/harness.js"; -import { scriptedChatCompletions } from "../src/mock-model.js"; +import { scriptedChatCompletions, type ScriptedChatCompletions } from "../src/mock-model.js"; import { NetworkInterceptor } from "../src/network-interceptor.js"; import { connect, listConnectedAccounts, nextUsernames, signUp, waitFor, @@ -27,98 +27,12 @@ const MODEL_CONFIG: AiModelConfig = { const RESOURCE_URL_PATTERN = "https://gadgets-test.example/things/*"; let harness: Harness; -const model = scriptedChatCompletions([ - // --- Test 1, turn 1: a fixable rejection, then a successful creation, used immediately. - { - toolCall: { - id: "create-bad-type", - name: "createExternalResource", - arguments: { - vendorId: TEST_VENDOR_ID, - resourceUrlPattern: "https://gadgets-test.example/nope/*", - title: "My Thing", - bindingName: "NEW_THING", - }, - }, - }, - { - toolCall: { - id: "create-thing", - name: "createExternalResource", - arguments: { - vendorId: TEST_VENDOR_ID, - resourceUrlPattern: RESOURCE_URL_PATTERN, - title: "My Thing", - bindingName: "NEW_THING", - }, - }, - }, - { - toolCall: { - id: "read-new-thing", - name: "executeCode", - arguments: { - code: "export default async function(self, env) { console.log(await env.NEW_THING.readValue()); }", - }, - }, - }, - { text: "Created the thing and read 42 from it." }, - // --- Test 1, turn 2 (after approval): the replayed binding still works, and the write's - // action snapshot shows the refreshed (created) resource URL. writeValue awaits a decision, - // so this turn deliberately suspends. - { - toolCall: { - id: "write-new-thing", - name: "executeCode", - arguments: { - code: "export default async function(self, env) { console.log(await env.NEW_THING.writeValue(9)); }", - }, - }, - }, - // --- Test 2, turn 1: create a thing whose creation the user will reject. - { - toolCall: { - id: "create-doomed", - name: "createExternalResource", - arguments: { - vendorId: TEST_VENDOR_ID, - resourceUrlPattern: RESOURCE_URL_PATTERN, - title: "Doomed Thing", - bindingName: "DOOMED", - }, - }, - }, - { text: "Created the doomed thing." }, - // --- Test 2, turn 2 (after rejection): the binding is dead, with an explanation. - { - toolCall: { - id: "read-doomed", - name: "executeCode", - arguments: { - code: "export default async function(self, env) {" + - " try { console.log(await env.DOOMED.readValue()); }" + - " catch (err) { console.log('DEAD: ' + (err && err.message)); } }", - }, - }, - }, - { text: "The doomed thing is gone." }, - // --- Test 3: the vendor fails after durably queueing its creation action; the overseer must - // settle the orphan instead of leaving it pending against a removed gatekeeper. - { - toolCall: { - id: "create-orphan", - name: "createExternalResource", - arguments: { - vendorId: TEST_VENDOR_ID, - resourceUrlPattern: RESOURCE_URL_PATTERN, - title: "fail-after-queue", - bindingName: "ORPHAN", - }, - }, - }, - { text: "The creation failed." }, -]); -const network = new NetworkInterceptor({ handlers: [model.handler] }); +// Each test owns its script (assigned before its first turn) so tests stay independently +// runnable; the interceptor delegates to whichever script is current. +let model: ScriptedChatCompletions; +const network = new NetworkInterceptor({ + handlers: [(url, method, headers, request) => model.handler(url, method, headers, request)], +}); beforeAll(async () => { network.install(); @@ -196,6 +110,55 @@ function userMessagesShownToModel(): string[] { } it("creates a resource the agent can use before the user approves it", async () => { + model = scriptedChatCompletions([ + // Turn 1: a fixable rejection, then a successful creation, used immediately. + { + toolCall: { + id: "create-bad-type", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: "https://gadgets-test.example/nope/*", + title: "My Thing", + bindingName: "NEW_THING", + }, + }, + }, + { + toolCall: { + id: "create-thing", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "My Thing", + bindingName: "NEW_THING", + }, + }, + }, + { + toolCall: { + id: "read-new-thing", + name: "executeCode", + arguments: { + code: "export default async function(self, env) { console.log(await env.NEW_THING.readValue()); }", + }, + }, + }, + { text: "Created the thing and read 42 from it." }, + // Turn 2 (after approval): the replayed binding still works, and the write's action + // snapshot shows the refreshed (created) resource URL. writeValue awaits a decision, so + // this turn deliberately suspends. + { + toolCall: { + id: "write-new-thing", + name: "executeCode", + arguments: { + code: "export default async function(self, env) { console.log(await env.NEW_THING.writeValue(9)); }", + }, + }, + }, + ]); using publicApi = connect(harness.url); using authenticated = await signUpScriptedUser(publicApi, "createres"); using workspace = await authenticated.newGadget(); @@ -240,22 +203,81 @@ it("creates a resource the agent can use before the user approves it", async () expect(write.resourceUrl).toContain("/things/created-"); expect(write.resourceUrl).not.toContain("provisional"); - // Replay told the model about the approval — the recorded tool result permanently says the - // resource doesn't exist yet, so without this the model's context never learns it now does. + // The creation action itself settled as approved. + const all = (await workspace.listActions({ filter: "all" })).entries; + expect(all.find(action => action.id === pending.id)?.state).toBe("approved"); + + // The decision reached the model as a durable nudge — the recorded tool result permanently + // says the resource doesn't exist yet, so without this the model's context never learns it + // now does. const approval = userMessagesShownToModel().find(message => message.includes("The user approved the creation of env.NEW_THING")); expect(approval).toContain(write.resourceUrl); + expect(model.remainingSteps()).toBe(0); }); -it("kills the binding when the user rejects the creation", async () => { +it("kills the binding and cascades to queued edits when the user rejects the creation", + async () => { + model = scriptedChatCompletions([ + // Turn 1: create a thing, queue an edit against it, then suspend on the edit's + // awaitDecision. The user rejects the creation, which must cascade to the queued edit. + { + toolCall: { + id: "create-doomed", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "Doomed Thing", + bindingName: "DOOMED", + }, + }, + }, + { + toolCall: { + id: "write-doomed", + name: "executeCode", + arguments: { + code: "export default async function(self, env) { console.log(await env.DOOMED.writeValue(5)); }", + }, + }, + }, + // Turn 2 (after rejection): the binding is dead, with an explanation. + { + toolCall: { + id: "read-doomed", + name: "executeCode", + arguments: { + code: "export default async function(self, env) {" + + " try { console.log(await env.DOOMED.readValue()); }" + + " catch (err) { console.log('DEAD: ' + (err && err.message)); } }", + }, + }, + }, + { text: "The doomed thing is gone." }, + ]); using publicApi = connect(harness.url); using authenticated = await signUpScriptedUser(publicApi, "createrej"); using workspace = await authenticated.newGadget(); - const chatId = await workspace.newChat("Create a doomed test thing.", MODEL_ID); - await waitForAgentSays(workspace, chatId, "Created the doomed thing."); + const chatId = await workspace.newChat( + "Create a doomed test thing and write to it.", MODEL_ID); - const pending = await onlyPendingAction(workspace, "the creation action to be pending"); - await workspace.rejectAction(pending.id); + // The write awaits a decision, so the turn suspends holding two pending actions: the + // creation and an edit that depends on it. + const pending = await waitFor("the creation and its edit to be pending", async () => { + const entries = (await workspace.listActions({ filter: "pending" })).entries; + return entries.length === 2 ? entries : null; + }); + const creation = pending.find(action => + action.description.title.startsWith("Create test thing")); + if (creation === undefined) throw new Error("No pending creation action found"); + await workspace.rejectAction(creation.id); + + // Rejecting the creation settled the dependent edit too — nothing left to decide one by one. + const all = (await workspace.listActions({ filter: "all" })).entries; + expect(all.filter(action => action.state === "pending")).toEqual([]); + expect(all.filter(action => action.type === "action" && action.state === "rejected")) + .toHaveLength(2); // The next turn's use of the binding fails with the gatekeeper's dead-binding explanation // rather than silently simulating against nothing. @@ -264,12 +286,30 @@ it("kills the binding when the user rejects the creation", async () => { expect(toolResultShownToModel("read-doomed")).toContain("DEAD:"); expect(toolResultShownToModel("read-doomed")).toMatch(/rejected/); - // Replay told the model about the rejection too. + // The rejection nudge reached the model. expect(userMessagesShownToModel().some(message => message.includes("The user rejected the creation of env.DOOMED"))).toBe(true); + expect(model.remainingSteps()).toBe(0); }); it("settles the queued action when the vendor fails after queueing it", async () => { + model = scriptedChatCompletions([ + // The vendor fails after durably queueing its creation action; the overseer must settle + // the orphan instead of leaving it pending against a removed gatekeeper. + { + toolCall: { + id: "create-orphan", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "fail-after-queue", + bindingName: "ORPHAN", + }, + }, + }, + { text: "The creation failed." }, + ]); using publicApi = connect(harness.url); using authenticated = await signUpScriptedUser(publicApi, "createfail"); using workspace = await authenticated.newGadget(); @@ -283,7 +323,9 @@ it("settles the queued action when the vendor fails after queueing it", async () // not left pending forever (approve/reject would both fail on the missing facet). const actions = (await workspace.listActions({ filter: "all" })).entries; expect(actions.filter(action => action.state === "pending")).toEqual([]); - expect(actions.some(action => action.type === "action" && action.state === "rejected")) - .toBe(true); + const settled = actions.find(action => + action.type === "action" && action.state === "rejected"); + if (settled?.gatekeeperId === undefined) throw new Error("No settled creation action found"); + await expect(workspace.getGatekeeperById(settled.gatekeeperId)).rejects.toThrow(); expect(model.remainingSteps()).toBe(0); }); diff --git a/packages/workshop-backend/__tests__/chat-changes.test.ts b/packages/workshop-backend/__tests__/chat-changes.test.ts index 491c4955fe..e58f938724 100644 --- a/packages/workshop-backend/__tests__/chat-changes.test.ts +++ b/packages/workshop-backend/__tests__/chat-changes.test.ts @@ -1276,6 +1276,47 @@ describe("reconcilePendingGadgets", () => { await impl.reconcilePendingGadgets(1); expect(impl.storage.gadgets.get(created.id)).toBeUndefined(); })); + + // Simulates a createExternalResource mint whose step crashed before the barrier: the + // gatekeeper record and its queued creation action are durable, but no tool call backs them. + it("reaps a crash-orphaned provisional gatekeeper and settles its creation action", + () => withImpl(async impl => { + addChat(impl, 1); + impl.storage.gatekeepers.put({ + id: 500, class: { type: "vendor", vendorId: "test", accountId: 1 }, + provisional: true, pending: { chatId: 1 }, creation: { bindingName: "THING", actionId: 7 }, + }); + impl.storage.actions.put({ + id: 7, gatekeeperId: 500, caller: { from: "agent", chatId: 1 }, action: 1, + createdAt: new Date(), state: "pending", type: "action", + description: { title: "Create thing" }, + }); + + await impl.reconcilePendingGadgets(1); + expect(impl.storage.gatekeepers.get(500)).toBeUndefined(); + expect(impl.storage.actions.get(7)!.state).toBe("rejected"); + })); + + it("keeps a pending-marked gatekeeper whose creation action was approved", + () => withImpl(async impl => { + addChat(impl, 1); + impl.storage.gatekeepers.put({ + id: 501, class: { type: "vendor", vendorId: "test", accountId: 1 }, + provisional: true, pending: { chatId: 1 }, creation: { bindingName: "THING", actionId: 8 }, + }); + impl.storage.actions.put({ + id: 8, gatekeeperId: 501, caller: { from: "agent", chatId: 1 }, action: 1, + createdAt: new Date(), state: "approved", appliedAt: new Date(), type: "action", + description: { title: "Create thing" }, + }); + + // The approved creation is proof the resource may exist at the provider: keep the + // gatekeeper, clear only the crash marker. + await impl.reconcilePendingGadgets(1); + let record = impl.storage.gatekeepers.get(501)!; + expect(record.pending).toBeUndefined(); + expect(impl.storage.actions.get(8)!.state).toBe("approved"); + })); }); describe("chat content reconstruction", () => { diff --git a/packages/workshop-backend/src/agent.ts b/packages/workshop-backend/src/agent.ts index cd1ef46984..194f47ca32 100644 --- a/packages/workshop-backend/src/agent.ts +++ b/packages/workshop-backend/src/agent.ts @@ -3317,6 +3317,9 @@ export async function runAgent( nameProblem = `There is already a binding named "${input.bindingName}" in your ` + `env. Choose a different name.`; } + if (nameProblem === undefined && input.title.trim().length === 0) { + nameProblem = `A resource requires a non-empty title.`; + } if (nameProblem !== undefined) { let message = `Cannot create the resource: ${nameProblem}`; return toolResult(message, { output: message }); diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 8122240842..fec0210c45 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -2499,11 +2499,18 @@ class OverseerImpl implements AgentHooks { // approved creation action as proof too -- reaping on `provisional` alone could sever a // real provider resource. let creationId = record.creation?.actionId; - let created = !record.provisional || (creationId !== undefined && - this.storage.actions.get(creationId)?.state === "approved"); - if (created) { + let creation = creationId !== undefined + ? this.storage.actions.get(creationId) : undefined; + let approvedBy = creation?.type === "action" && creation.state === "approved" + ? creation.resolvedBy : undefined; + if (!record.provisional || approvedBy !== undefined) { delete record.pending; this.storage.gatekeepers.put(record); + // The crashed step's barrier would have delivered the deferred decision nudge (see + // addChatMessages); emit it here so the resumed turn learns the resource is real. + if (approvedBy !== undefined) { + this.nudgeCreationDecision(chatId, record, "approved", approvedBy); + } continue; } @@ -5321,14 +5328,15 @@ class OverseerImpl implements AgentHooks { // its creation action is approved (this action, or an earlier one whose refresh failed), // describe() reports the real resource, so refresh the denormalized copy and retire the // marker. The point read keeps an invalidated edit that applies before the creation from - // clearing the marker early. Best-effort: on failure the marker stays set and the next - // applied action retries. + // clearing the marker early. Best-effort with one retry (a lone approved creation has no + // later apply to retry on): on failure the marker stays set and the next applied action, + // if any, retries. let gatekeeperRecord = this.storage.gatekeepers.get(record.gatekeeperId); let creationId = gatekeeperRecord?.creation?.actionId; if (gatekeeperRecord?.provisional && creationId !== undefined && this.storage.actions.get(creationId)?.state === "approved") { try { - let description = await gatekeeper.describe(); + let description = await gatekeeper.describe().catch(() => gatekeeper.describe()); // Re-read after the await: a concurrent removeGatekeeper during the describe() would // otherwise be resurrected by putting the stale record back. gatekeeperRecord = this.storage.gatekeepers.get(record.gatekeeperId); @@ -5364,6 +5372,10 @@ class OverseerImpl implements AgentHooks { nudgeCreationDecision(chatId: number, gatekeeper: GatekeeperRecord, decision: "approved" | "rejected", author: AiChatAuthorInfo) { if (gatekeeper.creation === undefined) return; + // A mid-step decision precedes the mint's own tool call in the log, so a nudge now would + // replay before the call it answers; the step barrier re-emits it once the call is recorded + // (see addChatMessages' unstamp branch). + if (gatekeeper.pending !== undefined) return; if (this.storage.chatMeta.get(chatId) === undefined) return; // Chat since deleted. let name = `env.${gatekeeper.creation.bindingName}`; let text = decision === "approved" @@ -5480,8 +5492,9 @@ class OverseerImpl implements AgentHooks { // the missing facet). appliedAt is required (the byLastChanged resume-replay index keys on it); // clearPushMarks matches rejectAction (a no-op for pushless actions). Rejecting first empties // the queue, so removeGatekeeper's own push-mark loop is a no-op. No gatekeeper-side - // rejectAction RPC: the facet is deleted outright and nothing observes its internal pending - // state after removal. + // rejectAction RPC: on these paths the facet just failed or its step vanished, and removal + // destroys its storage -- vendor state staged elsewhere must tolerate orphaned entries (a + // documented submitCreationAction contract clause). #rejectPendingActionsAndRemoveGatekeeper(id: number) { this.storage.transaction(() => { for (let action of Array.from(this.storage.actions.pendingByGatekeeper.get(id))) { @@ -8541,6 +8554,11 @@ class OverseerImpl implements AgentHooks { return; } + // Creation decisions deferred by nudgeCreationDecision while the mint was un-barriered; + // emitted after the loop so their nudges sequence after the tool calls they answer. + let decidedCreations: {gatekeeper: GatekeeperRecord, decision: "approved" | "rejected", + author: AiChatAuthorInfo}[] = []; + for (let {modelData, ...msg} of msgs) { if (msg.type === "changes") { // (A message's `pins` need no validation or mirroring here: pins are validated and @@ -8615,6 +8633,15 @@ class OverseerImpl implements AgentHooks { if (gatekeeper?.pending?.chatId === chatId) { delete gatekeeper.pending; this.storage.gatekeepers.put(gatekeeper); + // A decision made before this barrier deferred its nudge (see + // nudgeCreationDecision); deliver it now that the call is in the log. + let creation = gatekeeper.creation?.actionId !== undefined + ? this.storage.actions.get(gatekeeper.creation.actionId) : undefined; + if (creation?.type === "action" && creation.resolvedBy !== undefined && + (creation.state === "approved" || creation.state === "rejected")) { + decidedCreations.push( + {gatekeeper, decision: creation.state, author: creation.resolvedBy}); + } } } } @@ -8643,6 +8670,10 @@ class OverseerImpl implements AgentHooks { meta.lastActive = this.getChatTimestamp(); this.storage.chatMeta.put(meta); + for (let {gatekeeper, decision, author} of decidedCreations) { + this.nudgeCreationDecision(chatId, gatekeeper, decision, author); + } + if (aiGatewayLogId && aiGatewayLogRoute) { // Best-effort UI accounting only. The log ID is not persisted, so a DO restart can lose // this update. Do not use this total as a billing source of truth. @@ -11489,39 +11520,52 @@ class OverseerClientInterface extends RpcTarget implements Overseer { // can't leave the action rejected with the gatekeeper but still "pending" in storage. let profile = await this.#getClientProfile(); + await gatekeeper.rejectAction(action.action); + this.#markActionRejected(action, profile); + // Rejecting a creation dooms the gatekeeper's other queued actions (in-order application // means they could only ever apply after a creation that now never will), so settle them too - // rather than stranding cards the user would have to reject one by one. + // rather than stranding cards the user would have to reject one by one. The marks and the + // nudge land in one synchronous step -- a crash mid-cascade must not strand half of it -- + // ahead of the best-effort gatekeeper notifies. The record itself survives: replay + // re-establishes the chat binding from the recorded tool call (see agent.ts), and the + // vendor's dead-session error explains the rejection better than a missing binding would. let record = this.impl.storage.gatekeepers.get(action.gatekeeperId); - let rejectsCreation = record?.creation?.actionId === id; - let doomed = rejectsCreation - ? Array.from(this.impl.storage.actions.pendingByGatekeeper.get(action.gatekeeperId)) - .filter((a): a is ActionRecord & {type: "action"} => - a.type === "action" && a.id !== id) - : []; - - for (let target of [action, ...doomed]) { - await gatekeeper.rejectAction(target.action); - - target.state = "rejected"; - target.appliedAt = new Date(); - target.resolvedBy = profile; - // A rejected push's pending-push marks are removed in the same durable step as the state - // change (nothing was transmitted, so nothing became proven). No-op for pushless actions. - this.impl.storage.transaction(() => { - this.impl.gitCache.clearPushMarks(target.id); - this.impl.storage.actions.put(target); - }); - } - - if (rejectsCreation && action.caller.from === "agent") { - this.impl.nudgeCreationDecision(action.caller.chatId, record!, "rejected", profile); + if (record?.creation?.actionId === id) { + let siblings = Array.from( + this.impl.storage.actions.pendingByGatekeeper.get(action.gatekeeperId)) + .filter(sibling => sibling.type === "action"); + for (let sibling of siblings) this.#markActionRejected(sibling, profile); + if (action.caller.from === "agent") { + this.impl.nudgeCreationDecision(action.caller.chatId, record, "rejected", profile); + } + for (let sibling of siblings) { + try { + await gatekeeper.rejectAction(sibling.action); + } catch (error) { + this.impl.logger.warn("failed to notify gatekeeper of cascaded rejection", { + event: "gatekeeper.creation.cascade.reject.failed", actionId: sibling.id, error, + }); + } + } } // Deny leaves the turn ended, like denyConnectionRequest. The rejected record also prevents a // sibling approval from resuming this turn. } + // A rejected push's pending-push marks are removed in the same durable step as the state + // change (nothing was transmitted, so nothing became proven). No-op for pushless actions. + #markActionRejected(action: ActionRecord & {type: "action"}, resolvedBy: AiChatAuthorInfo) { + action.state = "rejected"; + action.appliedAt = new Date(); + action.resolvedBy = resolvedBy; + this.impl.storage.transaction(() => { + this.impl.gitCache.clearPushMarks(action.id); + this.impl.storage.actions.put(action); + }); + } + // Enable auto-approval of actions carrying `actionKind` on the given gatekeeper. Stores the // opt-in rule (one of the two gates required to auto-apply -- the action's own `autoApprovable` // verdict is the other) with the kind's display label, and immediately drains any pending diff --git a/packages/workshop-backend/src/user.ts b/packages/workshop-backend/src/user.ts index 93e646e4b0..36742074c5 100644 --- a/packages/workshop-backend/src/user.ts +++ b/packages/workshop-backend/src/user.ts @@ -1710,6 +1710,10 @@ export class UserDurableObject extends DurableObject { if (!record || record.vendorId !== vendorId) { throw new Error(`There is no connected "${vendorId}" account with id ${accountId}.`); } + if (!areCredentialsValid(record)) { + throw new Error(`The connected "${vendorId}" account ${accountId} has expired ` + + `credentials. Ask the user to reconnect it, then retry.`); + } account = record; } else { let candidates = [...this.#connectedAccountRecords()] diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index 7122779092..84aea677c5 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -671,7 +671,9 @@ function describeObservationCount(count: number): string { return count === 1 ? "Read 1 resource" : `${count} resource reads`; } -function describeToolCallCount(toolName: AiToolCall["toolName"], count: number): string { +function describeToolCallCount(calls: AiToolCall[]): string { + const toolName = calls[0].toolName; + const count = calls.length; switch (toolName) { case "readFile": return `Read ${pluralize(count, "file")}`; @@ -705,8 +707,13 @@ function describeToolCallCount(toolName: AiToolCall["toolName"], count: number): return `Listed connectable resources`; case "requestConnection": return count === 1 ? "Requested a connection" : `Requested ${count} connections`; - case "createExternalResource": - return `Created ${pluralize(count, "external resource")}`; + case "createExternalResource": { + const created = calls.filter((tc) => + tc.toolName === "createExternalResource" && isCreatedResourceSuccess(tc.output)).length; + return created === 0 + ? `Tried to create ${pluralize(count, "external resource")}` + : `Created ${pluralize(created, "external resource")}`; + } } const _exhaustive: never = toolName; return _exhaustive; @@ -912,12 +919,10 @@ function buildToolCallGroups( const summary = getToolCallSummary(toolCalls[0], outputOf); labelParts.push(detailLines.length === 1 && summary.target && observations.length === 0 ? `${summary.verb} ${summary.target}` - : describeToolCallCount(toolCalls[0].toolName, toolCalls.length)); + : describeToolCallCount(toolCalls)); } else if (toolCalls.length > 1 && distinctToolNames.length <= 3) { - labelParts.push(...distinctToolNames.map((toolName) => { - const count = toolCalls.filter((tc) => tc.toolName === toolName).length; - return describeToolCallCount(toolName, count); - })); + labelParts.push(...distinctToolNames.map((toolName) => + describeToolCallCount(toolCalls.filter((tc) => tc.toolName === toolName)))); } else if (toolCalls.length > 0) { labelParts.push(`${toolCalls.length} tool calls`); } diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index 98082e7367..d914beeca1 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -782,6 +782,10 @@ export interface Gatekeeper extends DurableObject { * but a manual approval can target any pending action, so the gatekeeper must itself reject * applyAction() of any action that depends on the resource existing until the creation has * been applied. Only gatekeepers reachable via createResource() need implement this. + * + * If this call fails, or a crash orphans the mint, the platform settles the queued actions + * and removes the gatekeeper WITHOUT delivering rejectAction(): the facet's storage is + * destroyed with it, and any state staged outside the facet must tolerate orphaned entries. */ submitCreationAction?(approvalQueue: RpcStub): Promise; From fc26e1026f5da438a0d0da0eb6e3f10a2993664e Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Fri, 4 Sep 2026 18:06:13 -0500 Subject: [PATCH 3/7] fix: defer creation restart to step barrier --- packages/workshop-backend/src/overseer.ts | 33 ++++++++++++++++++----- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index fec0210c45..82969019d0 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -5431,9 +5431,11 @@ class OverseerImpl implements AgentHooks { // `joinAs` counts the returned client toward #hasCollaboratorSession for its lifetime; passed by // the collaborator-facing mints, omitted for the owner's and for internal callers (see - // GadgetClientImpl). + // GadgetClientImpl). `deferRestart` is for the one mid-turn mint (createExternalResource): + // quarantine instead of aborting, and restart at the step barrier -- see the restart block below. async addGatekeeper( - cls: GatekeeperClass, creationSpec?: GatekeeperCreationSpec, joinAs?: SessionKind) + cls: GatekeeperClass, creationSpec?: GatekeeperCreationSpec, joinAs?: SessionKind, + options?: {deferRestart?: boolean}) : Promise> { let id = this.allocateWorkpieceId(); let gatekeeperRecord: GatekeeperRecord = { @@ -5478,7 +5480,16 @@ class OverseerImpl implements AgentHooks { // window. Publish, restart-check, and mark share one synchronous block, so no request can // interleave between the record appearing and the block taking effect. if (creationSpec && "vendorId" in creationSpec) { - if (this.#restartIfSessionsAffected( + if (options?.deferRestart) { + // Deferred: an immediate abort would kill the minting turn before its barrier records + // the tool call, and the resumed replay would re-issue the mint -- an abort/resume loop + // for as long as the collaborator keeps reconnecting. Quarantine in the same synchronous + // block instead (unverified sessions can't reach the id, see assertGatekeeperUsable) and + // restart once the log backs the creation (addChatMessages' unstamp branch). + if (this.#hasCollaboratorSession("build")) { + this.#gatekeepersPendingRestart.add(id); + } + } else if (this.#restartIfSessionsAffected( "Gadget restarted because a new connection was added.", "build")) { this.#gatekeepersPendingRestart.add(id); } @@ -6307,9 +6318,11 @@ class OverseerImpl implements AgentHooks { // session can even guess a brand-new one; a "use" session's gadget reload mints fresh binding // loopbacks). Every client-reachable route to the connection checks this set // (assertGatekeeperUsable/gatekeeperUsable). In-memory and never cleared: the scheduled reset - // is what clears it, by destroying this object. Only ever populated when a restart really was - // scheduled -- marking without one would brick the connection until some unrelated restart came - // along. + // is what clears it, by destroying this object. Populated only when a restart was scheduled -- + // or, for a deferred creation mint (addGatekeeper), committed to fire at the step barrier; + // marking with no restart coming would brick the connection until some unrelated restart came + // along. A deferred mark whose creation never reaches the log stays behind on a reaped, + // never-reused id -- inert until the next restart. #gatekeepersPendingRestart = new Set(); // Whether `id` is NOT blocked pending a scheduled restart (see #gatekeepersPendingRestart). @@ -8633,6 +8646,12 @@ class OverseerImpl implements AgentHooks { if (gatekeeper?.pending?.chatId === chatId) { delete gatekeeper.pending; this.storage.gatekeepers.put(gatekeeper); + // A deferred mint-time restart (see addGatekeeper) fires now that the log backs + // the creation: the resumed turn replays this call instead of re-issuing it. + if (this.#gatekeepersPendingRestart.has(gatekeeper.id)) { + this.scheduleAccessRestart( + "Gadget restarted because a new connection was added."); + } // A decision made before this barrier deferred its nudge (see // nudgeCreationDecision); deliver it now that the call is in the log. let creation = gatekeeper.creation?.actionId !== undefined @@ -9085,7 +9104,7 @@ class OverseerImpl implements AgentHooks { vendorId: minted.vendorId, resourceUrl: minted.resourceUrl, typeUrlPattern: minted.typeUrlPattern, - }); + }, undefined, {deferRestart: true}); let gatekeeperId = await client.getId(); // Mark the record provisional so the post-apply describe refresh (applyPendingAction) knows From aec755ae869f809ad390ea98ba1de6f56665ed16 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 7 Sep 2026 14:36:51 -0500 Subject: [PATCH 4/7] fix: harden external resource creation recovery --- .../workshop-agent-create-resource.test.ts | 179 +++++++++++++++--- .../gatekeeper-test/src/test-gatekeeper.ts | 2 + .../__tests__/chat-changes.test.ts | 21 ++ .../__tests__/git-push-actions.test.ts | 7 +- packages/workshop-backend/src/agent.ts | 12 +- packages/workshop-backend/src/overseer.ts | 152 ++++++++++----- packages/workshop-backend/src/user.ts | 5 +- .../workshop-frontend/src/ChatInterface.tsx | 22 ++- 8 files changed, 314 insertions(+), 86 deletions(-) diff --git a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts index 90198f80ec..91510c876c 100644 --- a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts +++ b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts @@ -1,18 +1,20 @@ // createExternalResource end to end against the fixture gatekeeper: tool → binding → action card -// → approve → describe refresh, plus the rejection path, replay across turns, and a vendor that -// fails after queueing its creation action. (Provider depth is covered by per-vendor suites, -// e.g. gatekeeper-google's workerd tests.) +// → out-of-order refusal → in-order approval → describe refresh, plus the rejection path, replay +// across turns, and a vendor that fails after queueing its creation action. (Provider depth is +// covered by per-vendor suites, e.g. gatekeeper-google's workerd tests.) import { afterAll, beforeAll, expect, it } from "vitest"; import type { RpcStub } from "capnweb"; import type { AiChatAuthorInfo, AiModelConfig, AuthenticatedApi, Overseer, PublicApi, } from "@gadgets/workshop-shared/api"; -import { startTestGatekeeperHarness, TEST_VENDOR_ID, type Harness } from "../src/harness.js"; +import { + startTestGatekeeperHarness, TEST_GATEKEEPER_WORKER, TEST_VENDOR_ID, type Harness, +} from "../src/harness.js"; import { scriptedChatCompletions, type ScriptedChatCompletions } from "../src/mock-model.js"; import { NetworkInterceptor } from "../src/network-interceptor.js"; import { - connect, listConnectedAccounts, nextUsernames, signUp, waitFor, + accountLabel, connect, listConnectedAccounts, nextUsernames, signUp, waitFor, } from "../src/rpc-client.js"; const MODEL_ID = "@cf/zai-org/glm-5.2"; @@ -109,6 +111,27 @@ function userMessagesShownToModel(): string[] { .map(message => message.content ?? ""); } +/** The hydrated action card for `actionId` in the chat history (what the UI renders from). */ +async function actionCard(workspace: RpcStub, chatId: number, actionId: number) { + const history = await workspace.getChatHistory(chatId); + const message = history.messages.find(entry => + entry.type === "action" && entry.actionId === actionId); + if (message?.type !== "action") throw new Error(`No action card for action ${actionId}`); + return message; +} + +/** Vendor-side action bookkeeping for `label` (the fixture control DO's view). */ +async function actionState(label: string) { + const response = await harness.fetchWorker( + TEST_GATEKEEPER_WORKER, "http://gatekeeper-test.test/control/action-state", + { method: "POST", body: JSON.stringify({ label }) }); + if (response.status !== 200) { + throw new Error(`Reading test action state failed with ${response.status}`); + } + // Loose shape: the assertions pin the fields that matter. + return await response.json() as { pending: unknown[]; value?: number; applyCount: number }; +} + it("creates a resource the agent can use before the user approves it", async () => { model = scriptedChatCompletions([ // Turn 1: a fixable rejection, then a successful creation, used immediately. @@ -146,9 +169,8 @@ it("creates a resource the agent can use before the user approves it", async () }, }, { text: "Created the thing and read 42 from it." }, - // Turn 2 (after approval): the replayed binding still works, and the write's action - // snapshot shows the refreshed (created) resource URL. writeValue awaits a decision, so - // this turn deliberately suspends. + // Turn 2: replay re-establishes the binding, and a write is queued against the + // still-provisional resource. writeValue awaits a decision, so this turn suspends. { toolCall: { id: "write-new-thing", @@ -158,6 +180,17 @@ it("creates a resource the agent can use before the user approves it", async () }, }, }, + // Resumed turn (after in-order approval): the write reached the provider path. + { + toolCall: { + id: "read-after-write", + name: "executeCode", + arguments: { + code: "export default async function(self, env) { console.log(await env.NEW_THING.readValue()); }", + }, + }, + }, + { text: "The value is now 9." }, ]); using publicApi = connect(harness.url); using authenticated = await signUpScriptedUser(publicApi, "createres"); @@ -184,35 +217,62 @@ it("creates a resource the agent can use before the user approves it", async () description: { title: 'Create test thing "My Thing"' }, }); expect(pending.resourceUrl).toContain("/things/provisional-"); - const history = await workspace.getChatHistory(chatId); - expect(history.messages.some(message => - message.type === "action" && message.actionId === pending.id)).toBe(true); - - await workspace.approveAction(pending.id); + // The card in the chat history renders from the hydrated actionLog, not just the id. + expect((await actionCard(workspace, chatId, pending.id)).actionLog) + .toMatchObject({ state: "pending" }); - // A second turn proves both replay (the binding is re-established from the recorded tool - // output) and the post-apply describe refresh (the new action's snapshot carries the real, - // no-longer-provisional resource URL). + // Turn 2: the binding is re-established from the recorded tool output, and the write is + // queued while the resource is still provisional. The turn suspends holding both actions. await workspace.sendChatMessage(chatId, "Now set its value to 9.", MODEL_ID); - const write = await onlyPendingAction(workspace, "the write action to be pending"); - expect(write).toMatchObject({ - type: "action", - state: "pending", - description: { title: "Set the test value to 9" }, + const queued = await waitFor("the creation and the write to be pending", async () => { + const entries = (await workspace.listActions({ filter: "pending" })).entries; + return entries.length === 2 ? entries : null; }); - expect(write.resourceUrl).toContain("/things/created-"); - expect(write.resourceUrl).not.toContain("provisional"); + const write = queued.find(action => action.description.title === "Set the test value to 9"); + if (write === undefined) throw new Error("No pending write action found"); + // Queue-time snapshot: the resource was still provisional when the write was queued. + expect(write.resourceUrl).toContain("/things/provisional-"); - // The creation action itself settled as approved. + // Approving the dependent write before the creation is refused (the gatekeeper's in-order + // guard) and the write stays pending. + await expect(workspace.approveAction(write.id)).rejects.toThrow(/does not exist yet/); + expect((await workspace.listActions({ filter: "pending" })).entries + .some(action => action.id === write.id)).toBe(true); + + // In order: creation, then the write. The write approval resolving is the apply proof — the + // platform awaits the vendor's applyAction before marking it approved, and the identical call + // was just refused. The second approval resumes the suspended turn. + await workspace.approveAction(pending.id); + await workspace.approveAction(write.id); + await waitForAgentSays(workspace, chatId, "The value is now 9."); + + // Both actions settled, and the post-apply describe refresh retired the provisional URL: + // the resumed read's observation snapshots the created resource. const all = (await workspace.listActions({ filter: "all" })).entries; expect(all.find(action => action.id === pending.id)?.state).toBe("approved"); + expect(all.find(action => action.id === write.id)?.state).toBe("approved"); + expect(all.some(action => + action.type === "observation" && action.resourceUrl?.includes("/things/created-"))).toBe(true); + + // actionLog hydrates at read time: the same card now carries the decision, and the creation + // card's link was refreshed off the dead provisional URL. + expect((await actionCard(workspace, chatId, pending.id)).actionLog).toMatchObject({ + state: "approved", + resourceUrl: expect.stringContaining("/things/created-"), + }); + + // Vendor-side proof the applies really landed: the creation staged value 0, the write 9. + const account = (await listConnectedAccounts(authenticated)) + .find(entry => entry.vendorId === TEST_VENDOR_ID); + if (!account) throw new Error("No connected test account"); + expect(await actionState(accountLabel(account))) + .toMatchObject({ pending: [], value: 9, applyCount: 2 }); // The decision reached the model as a durable nudge — the recorded tool result permanently // says the resource doesn't exist yet, so without this the model's context never learns it - // now does. - const approval = userMessagesShownToModel().find(message => - message.includes("The user approved the creation of env.NEW_THING")); - expect(approval).toContain(write.resourceUrl); + // now does. (The nudge lands before the describe refresh, so it carries no URL.) + expect(userMessagesShownToModel().some(message => + message.includes("The user approved the creation of env.NEW_THING"))).toBe(true); expect(model.remainingSteps()).toBe(0); }); @@ -329,3 +389,66 @@ it("settles the queued action when the vendor fails after queueing it", async () await expect(workspace.getGatekeeperById(settled.gatekeeperId)).rejects.toThrow(); expect(model.remainingSteps()).toBe(0); }); + +it("fails closed when the vendor never queues its creation action", async () => { + model = scriptedChatCompletions([ + // A contract-breaking vendor returns from submitCreationAction without queueing; the + // overseer must surface the bug instead of leaving a permanently provisional binding. + { + toolCall: { + id: "create-unqueued", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "never-queues", + bindingName: "UNQUEUED", + }, + }, + }, + { text: "The creation failed." }, + ]); + using publicApi = connect(harness.url); + using authenticated = await signUpScriptedUser(publicApi, "createnoop"); + using workspace = await authenticated.newGadget(); + const chatId = await workspace.newChat("Create a never-queued test thing.", MODEL_ID); + await waitForAgentSays(workspace, chatId, "The creation failed."); + + expect(toolResultShownToModel("create-unqueued")).toMatch(/vendor bug/); + expect((await workspace.listActions({ filter: "all" })).entries).toEqual([]); + expect(model.remainingSteps()).toBe(0); +}); + +it("settles an undecided creation when its chat is deleted", async () => { + model = scriptedChatCompletions([ + { + toolCall: { + id: "create-abandoned", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "Abandoned Thing", + bindingName: "ABANDONED", + }, + }, + }, + { text: "Created the abandoned thing." }, + ]); + using publicApi = connect(harness.url); + using authenticated = await signUpScriptedUser(publicApi, "createdel"); + using workspace = await authenticated.newGadget(); + const chatId = await workspace.newChat("Create a thing I will abandon.", MODEL_ID); + await waitForAgentSays(workspace, chatId, "Created the abandoned thing."); + const pending = await onlyPendingAction(workspace, "the creation action to be pending"); + if (pending.gatekeeperId === undefined) throw new Error("Creation action has no gatekeeper"); + + await workspace.deleteChat(chatId); + + // The deleted log was the only thing that could ever bind the resource, so the card must not + // stay approvable: approving it would mint a provider resource nothing can address. + const all = (await workspace.listActions({ filter: "all" })).entries; + expect(all.find(action => action.id === pending.id)?.state).toBe("rejected"); + await expect(workspace.getGatekeeperById(pending.gatekeeperId)).rejects.toThrow(); + expect(model.remainingSteps()).toBe(0); +}); diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index 999feec8e3..83cb6ecc87 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -444,6 +444,8 @@ export class TestGatekeeper if (!creation) { throw new Error("This test gatekeeper was not minted by createResource()."); } + // Test knob: a contract-breaking vendor that returns without queueing its creation. + if (creation.title === "never-queues") return; if (this.ctx.storage.kv.get("creationActionId") !== undefined) return; const id = await control(this.ctx.exports).stageAction(this.ctx.props.label, 0); this.ctx.storage.kv.put("creationActionId", id); diff --git a/packages/workshop-backend/__tests__/chat-changes.test.ts b/packages/workshop-backend/__tests__/chat-changes.test.ts index e58f938724..aadcc0a5ba 100644 --- a/packages/workshop-backend/__tests__/chat-changes.test.ts +++ b/packages/workshop-backend/__tests__/chat-changes.test.ts @@ -1317,6 +1317,27 @@ describe("reconcilePendingGadgets", () => { expect(record.pending).toBeUndefined(); expect(impl.storage.actions.get(8)!.state).toBe("approved"); })); + + it("delivers the deferred approval nudge when keeping an approved creation", + () => withImpl(async impl => { + addChat(impl, 1); + impl.storage.gatekeepers.put({ + id: 502, class: { type: "vendor", vendorId: "test", accountId: 1 }, + provisional: true, pending: { chatId: 1 }, creation: { bindingName: "THING", actionId: 9 }, + }); + impl.storage.actions.put({ + id: 9, gatekeeperId: 502, caller: { from: "agent", chatId: 1 }, action: 1, + createdAt: new Date(), state: "approved", appliedAt: new Date(), type: "action", + resolvedBy: { type: "user", id: "u", name: "User" }, + description: { title: "Create thing" }, + }); + + await impl.reconcilePendingGadgets(1); + expect(impl.storage.gatekeepers.get(502)!.pending).toBeUndefined(); + let nudges = chatMessages(impl, 1).filter(m => m.type === "agentNudge"); + expect(nudges).toHaveLength(1); + expect(nudges[0].text).toContain("The user approved the creation of env.THING"); + })); }); describe("chat content reconstruction", () => { diff --git a/packages/workshop-backend/__tests__/git-push-actions.test.ts b/packages/workshop-backend/__tests__/git-push-actions.test.ts index 732d5fcaf5..b867cc38af 100644 --- a/packages/workshop-backend/__tests__/git-push-actions.test.ts +++ b/packages/workshop-backend/__tests__/git-push-actions.test.ts @@ -52,9 +52,12 @@ async function storeLocal(impl: any, type: string, payload: Uint8Array): Promise return oid; } -// Seeds the standard scenario: the gatekeeper has proven a base commit (empty tree), and a -// locally-authored commit sits on top of it. Returns both oids. +// Seeds the standard scenario: the gatekeeper record exists (submitAction refuses to queue +// against a removed one), it has proven a base commit (empty tree), and a locally-authored +// commit sits on top of it. Returns both oids. async function seedPushableHistory(impl: any): Promise<{ base: string, head: string }> { + impl.storage.gatekeepers.put( + { id: GATEKEEPER, class: { type: "vendor", vendorId: "test", accountId: 1 } }); let treeOid = await impl.gitCache.putFromGatekeeper(GATEKEEPER, "tree", new Uint8Array(0)); let base = await impl.gitCache.putFromGatekeeper( GATEKEEPER, "commit", commitPayload(treeOid, [], "base")); diff --git a/packages/workshop-backend/src/agent.ts b/packages/workshop-backend/src/agent.ts index 194f47ca32..5034519c58 100644 --- a/packages/workshop-backend/src/agent.ts +++ b/packages/workshop-backend/src/agent.ts @@ -2520,10 +2520,14 @@ export async function runAgent( `can; use listConnectableResources to learn a vendor's resource URL patterns first). The ` + `user accepts or denies in the chat. If they accept, you'll be resumed and the resource ` + `becomes available as a binding in your env; if they deny, your turn ends and you wait ` + - `for the user's next message. Resource types listConnectableResources marks "Creatable" ` + - `can instead be created brand-new with createExternalResource through an ` + - `already-connected account — that binding is usable immediately (the user approves the ` + - `creation as a normal action while you keep working).\n` + + `for the user's next message.\n` + + `If you need a brand-new resource instead (a new document, for example), resource types ` + + `listConnectableResources marks "Creatable" can be created with createExternalResource ` + + `through an already-connected account. This applies even when your existing binding for ` + + `that vendor is read-only: creation goes through the account, not the binding. Don't ` + + `assume a type isn't creatable — listConnectableResources answers in one call, no ` + + `connected account required. The new binding is usable immediately (the user approves ` + + `the creation as a normal action while you keep working).\n` + `If one of these services likely holds information relevant to the task, consider ` + `requesting a connection and reading from it before you answer, instead of answering from ` + `guesswork — a connection often gives you the real information. Connectable vendors:\n` + diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 82969019d0..2e97d6f1b7 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -301,12 +301,13 @@ type GatekeeperRecord = { // yet" and outlives the barrier. pending?: {chatId: number}; - // Present on a createExternalResource mint: the agent's env name for the resource (stamped at - // the mint) and the creation action's id (stamped by submitAction on the first queued action -- - // the vendor queues the creation first, and this makes that ordering the definition). Never - // cleared; drives the post-apply describe refresh, the crash reap's keep check, and the - // decision nudges. - creation?: {bindingName: string, actionId?: number}; + // Present on a createExternalResource mint: the creating chat (drives chat deletion's sweep + // of undecided creations), the agent's env name for the resource (stamped at the mint), and + // the creation action's id (stamped by submitAction on the first queued action -- the vendor + // queues the creation first, and this makes that ordering the definition). Never cleared; + // also drives the post-apply describe refresh, the crash reap's keep check, and the decision + // nudges. Records minted before chatId existed lack it and are simply not swept. + creation?: {chatId?: number, bindingName: string, actionId?: number}; // Records how this gatekeeper was originally created, enabling blueprint metadata derivation. creationSpec?: GatekeeperCreationSpec; @@ -2501,15 +2502,15 @@ class OverseerImpl implements AgentHooks { let creationId = record.creation?.actionId; let creation = creationId !== undefined ? this.storage.actions.get(creationId) : undefined; - let approvedBy = creation?.type === "action" && creation.state === "approved" - ? creation.resolvedBy : undefined; - if (!record.provisional || approvedBy !== undefined) { + let approval = creation?.type === "action" && creation.state === "approved" + ? creation : undefined; + if (!record.provisional || approval !== undefined) { delete record.pending; this.storage.gatekeepers.put(record); // The crashed step's barrier would have delivered the deferred decision nudge (see // addChatMessages); emit it here so the resumed turn learns the resource is real. - if (approvedBy !== undefined) { - this.nudgeCreationDecision(chatId, record, "approved", approvedBy); + if (approval?.resolvedBy !== undefined) { + this.nudgeCreationDecision(chatId, record, "approved", approval.resolvedBy); } continue; } @@ -2729,6 +2730,20 @@ class OverseerImpl implements AgentHooks { } } this.#reapPendingGatekeepers(chatId); + + // Undecided creations barriered to this chat outlive the pending marker, but the deleted + // log was the only thing that could ever bind them: settle them rather than leaving an + // approvable card that would mint a provider resource nothing can address. Approved + // creations stay -- the provider resource is real. + for (let record of Array.from(this.storage.gatekeepers.list())) { + if (record.creation?.chatId !== chatId) continue; + let actionId = record.creation.actionId; + if (actionId !== undefined && + this.storage.actions.get(actionId)?.state === "approved") { + continue; + } + this.#rejectPendingActionsAndRemoveGatekeeper(record.id); + } } // Disable (if needed) and delete a bound hook, updating its action-log record to match. @@ -3743,7 +3758,8 @@ class OverseerImpl implements AgentHooks { } try { - return this.storage.transaction(() => { + let changesWritten = false; + let committed = this.storage.transaction(() => { let fresh = this.storage.chatMeta.get(chatId); if (!fresh) return false; // chat deleted during the prefetches @@ -3812,7 +3828,7 @@ class OverseerImpl implements AgentHooks { this.addChatMessages(chatId, author, msgs, totalTokens, aiGatewayLogId, aiGatewayLogRoute, estimatedCost); - return this.materializeChatChanges(chatId, undefined, { + changesWritten = this.materializeChatChanges(chatId, undefined, { author, allowDuringTurn: true, createdGadgets: step.createdGadgets, @@ -3820,7 +3836,27 @@ class OverseerImpl implements AgentHooks { addedBindings: step.addedBindings, worktreeCommits: step.worktreeCommits, }) !== undefined; + return true; }); + // The deferred creation-mint restart (see addGatekeeper) fires only after the barrier + // committed: launched inside the transaction, the abort would survive a rollback and + // restart with the tool call unrecorded -- the remint loop the deferral prevents. An + // uncommitted step (chat deleted) records no call, so there is nothing to restart for. + // `committed` is not the return value: the contract's boolean is "changes message + // written", false for an ordinary creation-only step whose barrier fully committed. + if (committed) { + for (let msg of msgs) { + if (msg.type !== "message") continue; + for (let call of msg.toolCalls ?? []) { + if (call.toolName === "createExternalResource" && + isCreatedResourceSuccess(call.output) && + this.#gatekeepersPendingRestart.has(call.output.gatekeeperId)) { + this.scheduleAccessRestart("Gadget restarted because a new connection was added."); + } + } + } + } + return changesWritten; } catch (err) { // The transaction rolled the rows back, but the append path already advanced the // in-memory caches to reflect them; drop both so later reads rebuild from storage. @@ -5324,6 +5360,16 @@ class OverseerImpl implements AgentHooks { this.storage.actions.put(record); }); + // The decision nudge precedes the best-effort refresh below: a reset during describe() must + // not cost the model the verdict -- the recorded tool result permanently says the resource + // doesn't exist yet, and nothing later re-emits the decision. + let gatekeeperRecord = this.storage.gatekeepers.get(record.gatekeeperId); + let creationId = gatekeeperRecord?.creation?.actionId; + if (record.id === creationId && record.caller.from === "agent" && + gatekeeperRecord !== undefined) { + this.nudgeCreationDecision(record.caller.chatId, gatekeeperRecord, "approved", resolvedBy); + } + // A gatekeeper minted by createExternalResource was described with a provisional URL; once // its creation action is approved (this action, or an earlier one whose refresh failed), // describe() reports the real resource, so refresh the denormalized copy and retire the @@ -5331,8 +5377,6 @@ class OverseerImpl implements AgentHooks { // clearing the marker early. Best-effort with one retry (a lone approved creation has no // later apply to retry on): on failure the marker stays set and the next applied action, // if any, retries. - let gatekeeperRecord = this.storage.gatekeepers.get(record.gatekeeperId); - let creationId = gatekeeperRecord?.creation?.actionId; if (gatekeeperRecord?.provisional && creationId !== undefined && this.storage.actions.get(creationId)?.state === "approved") { try { @@ -5351,6 +5395,15 @@ class OverseerImpl implements AgentHooks { } delete gatekeeperRecord.provisional; this.storage.gatekeepers.put(gatekeeperRecord); + // The creation card's link renders from the action's own snapshot (actionLog), so + // retire its provisional URL too. Point-write only: edit cards keep their queue-time + // snapshots (audit semantics; no settled-by-gatekeeper index to rewrite them). + let creationRecord = this.storage.actions.get(creationId); + if (creationRecord?.type === "action") { + creationRecord.resourceTitle = description.title; + creationRecord.resourceUrl = description.url; + this.storage.actions.put(creationRecord); + } } } catch (error) { this.logger.warn("failed to refresh created resource description after apply", { @@ -5359,11 +5412,6 @@ class OverseerImpl implements AgentHooks { }); } } - - if (record.id === creationId && record.caller.from === "agent" && - gatekeeperRecord !== undefined) { - this.nudgeCreationDecision(record.caller.chatId, gatekeeperRecord, "approved", resolvedBy); - } } // Record the user's verdict on a created resource in its chat's log: the creation tool's @@ -5431,11 +5479,12 @@ class OverseerImpl implements AgentHooks { // `joinAs` counts the returned client toward #hasCollaboratorSession for its lifetime; passed by // the collaborator-facing mints, omitted for the owner's and for internal callers (see - // GadgetClientImpl). `deferRestart` is for the one mid-turn mint (createExternalResource): - // quarantine instead of aborting, and restart at the step barrier -- see the restart block below. + // GadgetClientImpl). `creation` marks a createExternalResource mint: its recovery markers ride + // the initial record put (no window where the record exists unmarked), and the access restart + // is deferred to the step barrier -- see the restart block below. async addGatekeeper( cls: GatekeeperClass, creationSpec?: GatekeeperCreationSpec, joinAs?: SessionKind, - options?: {deferRestart?: boolean}) + creation?: {chatId: number, bindingName: string}) : Promise> { let id = this.allocateWorkpieceId(); let gatekeeperRecord: GatekeeperRecord = { @@ -5443,6 +5492,11 @@ class OverseerImpl implements AgentHooks { class: cls, creationSpec, }; + if (creation) { + gatekeeperRecord.provisional = true; + gatekeeperRecord.pending = {chatId: creation.chatId}; + gatekeeperRecord.creation = {chatId: creation.chatId, bindingName: creation.bindingName}; + } // The record is published only once, below, after describe() resolves -- the facet takes the // class directly so it needs no record to exist yet. Publishing it before the await instead @@ -5457,6 +5511,12 @@ class OverseerImpl implements AgentHooks { gatekeeperRecord.resourceTitle = description.title; gatekeeperRecord.resourceUrl = description.url; gatekeeperRecord.hasSlashCommands = description.hasSlashCommands; + // A creation mint whose chat was deleted during the awaits must not publish: the chat's + // sweep already ran and its aborted turn abandons this continuation, so nothing would ever + // reap the record. Check-and-put share one synchronous block. + if (creation && !this.storage.chatMeta.get(creation.chatId)) { + throw new Error("The chat was deleted while the resource was being created."); + } this.storage.gatekeepers.put(gatekeeperRecord); } catch (error) { // Still the right teardown with nothing published: it deletes the facet we just created, and @@ -5480,7 +5540,7 @@ class OverseerImpl implements AgentHooks { // window. Publish, restart-check, and mark share one synchronous block, so no request can // interleave between the record appearing and the block taking effect. if (creationSpec && "vendorId" in creationSpec) { - if (options?.deferRestart) { + if (creation) { // Deferred: an immediate abort would kill the minting turn before its barrier records // the tool call, and the resumed replay would re-issue the mint -- an abort/resume loop // for as long as the collaborator keeps reconnecting. Quarantine in the same synchronous @@ -5958,17 +6018,23 @@ class OverseerImpl implements AgentHooks { this.gitCache.verifyPushAncestry(gatekeeperId, description.pushedCommits); } + // A pending action against a removed gatekeeper would be permanently undecidable (approve + // and reject both need the facet), so a submit racing removal -- deleteChat's reap can pull + // the record while the facet's call is in flight -- fails here instead. + let gatekeeper = this.storage.gatekeepers.get(gatekeeperId); + if (!gatekeeper) { + throw new Error("The connection this action targets has been removed."); + } + let actionId = this.storage.nextActionId.get(); this.storage.nextActionId.put(actionId + 1); - let gatekeeper = this.storage.gatekeepers.get(gatekeeperId); - let record: ActionRecord = { id: actionId, gatekeeperId, caller, - resourceTitle: gatekeeper?.resourceTitle, - resourceUrl: gatekeeper?.resourceUrl, + resourceTitle: gatekeeper.resourceTitle, + resourceUrl: gatekeeper.resourceUrl, action, createdAt: new Date(), state: "pending", @@ -5985,7 +6051,7 @@ class OverseerImpl implements AgentHooks { } // The first action queued against a createExternalResource mint IS the creation; stamp // its identity in the same durable step as the action itself. - if (gatekeeper?.creation !== undefined && gatekeeper.creation.actionId === undefined) { + if (gatekeeper.creation !== undefined && gatekeeper.creation.actionId === undefined) { gatekeeper.creation.actionId = actionId; this.storage.gatekeepers.put(gatekeeper); } @@ -8646,12 +8712,6 @@ class OverseerImpl implements AgentHooks { if (gatekeeper?.pending?.chatId === chatId) { delete gatekeeper.pending; this.storage.gatekeepers.put(gatekeeper); - // A deferred mint-time restart (see addGatekeeper) fires now that the log backs - // the creation: the resumed turn replays this call instead of re-issuing it. - if (this.#gatekeepersPendingRestart.has(gatekeeper.id)) { - this.scheduleAccessRestart( - "Gadget restarted because a new connection was added."); - } // A decision made before this barrier deferred its nudge (see // nudgeCreationDecision); deliver it now that the call is in the log. let creation = gatekeeper.creation?.actionId !== undefined @@ -9104,20 +9164,9 @@ class OverseerImpl implements AgentHooks { vendorId: minted.vendorId, resourceUrl: minted.resourceUrl, typeUrlPattern: minted.typeUrlPattern, - }, undefined, {deferRestart: true}); + }, undefined, {chatId, bindingName: input.bindingName}); let gatekeeperId = await client.getId(); - // Mark the record provisional so the post-apply describe refresh (applyPendingAction) knows - // to re-denormalize once the resource really exists, pending so a mid-step crash before the - // tool call reaches the log leaves a reapable orphan rather than a live duplicate (see - // GatekeeperRecord.pending), and stamp the creation identity for the decision nudges. - // (addGatekeeper just created the record.) - let record = this.storage.gatekeepers.get(gatekeeperId)!; - record.provisional = true; - record.pending = {chatId}; - record.creation = {bindingName: input.bindingName}; - this.storage.gatekeepers.put(record); - // Have the facet queue its creation action, attributed to this chat so the approval card is // spliced into the transcript. On failure, no half-created workpiece survives. // (submitCreationAction is optional on Gatekeeper; a creatable-advertising vendor must @@ -9140,6 +9189,15 @@ class OverseerImpl implements AgentHooks { return `Cannot create the resource: ${stringifyError(error)}`; } + // Fail closed on the vendor contract: submitCreationAction must queue the creation, which + // stamps creation.actionId (see submitAction). A vendor returning without queueing would + // otherwise leave a permanently provisional binding with no approval card and no error. + if (this.storage.gatekeepers.get(gatekeeperId)?.creation?.actionId === undefined) { + this.removeGatekeeper(gatekeeperId); + return `Cannot create the resource: the "${input.vendorId}" gatekeeper returned without ` + + `submitting its creation action. This is a vendor bug; do not retry.`; + } + return { gatekeeperId, resourceUrl: minted.resourceUrl, message: `Created "${input.title}" (${resource.title}), available as env.${input.bindingName} ` + `in executeCode immediately — use describeBinding to learn its API. The resource ` + diff --git a/packages/workshop-backend/src/user.ts b/packages/workshop-backend/src/user.ts index 36742074c5..49ef50b6d0 100644 --- a/packages/workshop-backend/src/user.ts +++ b/packages/workshop-backend/src/user.ts @@ -1746,9 +1746,12 @@ export class UserDurableObject extends DurableObject { // RPC, exactly like getGatekeeperClassFor -- the vendor is the authority on which resource // type a request maps to. (createResource mints only the class and a provisional URL; the // provider-side creation is a separate pending action, so nothing external happened yet.) + // A dormant auto-provisioning vendor (ambient mode "disabled") blocks here too: existing + // accounts stay unusable, matching startHook's use-time check. let config = await readAdminConfig(this.env); let vendorIdLower = account.vendorId.toLowerCase(); - if (config.disabledGatekeepers.includes(vendorIdLower)) { + if (config.disabledGatekeepers.includes(vendorIdLower) || + ambientGatekeeperMode(config, vendorIdLower) === "disabled") { throw new Error( `The "${account.vendorId}" gatekeeper is disabled on this deployment by an administrator.`); } diff --git a/packages/workshop-frontend/src/ChatInterface.tsx b/packages/workshop-frontend/src/ChatInterface.tsx index 84aea677c5..4b2c0c4898 100644 --- a/packages/workshop-frontend/src/ChatInterface.tsx +++ b/packages/workshop-frontend/src/ChatInterface.tsx @@ -916,7 +916,9 @@ function buildToolCallGroups( const summary = getToolCallSummary(toolCalls[0], outputOf); labelParts.push(`${summary.verb}${summary.target ? ` ${summary.target}` : ""}`); } else if (toolCalls.length > 1 && distinctToolNames.length === 1) { - const summary = getToolCallSummary(toolCalls[0], outputOf); + // Label by the last call: for same-target retries the final outcome wins (a failed create + // retried successfully is "Created", not "Tried"). + const summary = getToolCallSummary(toolCalls[toolCalls.length - 1], outputOf); labelParts.push(detailLines.length === 1 && summary.target && observations.length === 0 ? `${summary.verb} ${summary.target}` : describeToolCallCount(toolCalls)); @@ -1451,9 +1453,21 @@ const ToolCallDetails = memo(function ToolCallDetails( )} ) : ( - 
-          {JSON.stringify(tc.input, null, 2)}
-
+ <> + 
+            {JSON.stringify(tc.input, null, 2)}
+
+ {tc.toolName === "createExternalResource" && typeof tc.output === "string" && ( + <> + + Output + + 
+                {tc.output}
+
+ + )} + )} ); From 34bfa3ecddf21dfdb0370f9b56e5dcad823cf90e Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Mon, 7 Sep 2026 16:28:59 -0500 Subject: [PATCH 5/7] Thread vendor-opaque creation options through createExternalResource: a flat scalar map the kernel carries but never reads, validated by the vendor and reflected on its approval card. --- .../workshop-agent-create-resource.test.ts | 45 +++++++++++++++++++ .../gatekeeper-test/src/test-gatekeeper.ts | 27 ++++++++--- packages/workshop-backend/src/agent.ts | 10 ++++- packages/workshop-backend/src/overseer.ts | 3 +- packages/workshop-backend/src/user.ts | 6 +-- packages/workshop-shared/src/api.ts | 8 +++- packages/workshop-shared/src/gatekeeper.ts | 15 ++++++- 7 files changed, 100 insertions(+), 14 deletions(-) diff --git a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts index 91510c876c..43f70598e9 100644 --- a/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts +++ b/packages/integration-tests/__tests__/workshop-agent-create-resource.test.ts @@ -452,3 +452,48 @@ it("settles an undecided creation when its chat is deleted", async () => { await expect(workspace.getGatekeeperById(pending.gatekeeperId)).rejects.toThrow(); expect(model.remainingSteps()).toBe(0); }); + +it("threads creation options to the vendor and its approval card", async () => { + model = scriptedChatCompletions([ + // An unknown option is an agent-fixable vendor rejection; the retry with the documented + // key succeeds and the approval card reflects the placement. + { + toolCall: { + id: "create-bad-option", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "Shelved Thing", + bindingName: "SHELVED", + options: { bogus: true }, + }, + }, + }, + { + toolCall: { + id: "create-shelved", + name: "createExternalResource", + arguments: { + vendorId: TEST_VENDOR_ID, + resourceUrlPattern: RESOURCE_URL_PATTERN, + title: "Shelved Thing", + bindingName: "SHELVED", + options: { shelf: "top" }, + }, + }, + }, + { text: "Created the shelved thing." }, + ]); + using publicApi = connect(harness.url); + using authenticated = await signUpScriptedUser(publicApi, "createopts"); + using workspace = await authenticated.newGadget(); + const chatId = await workspace.newChat("Create a thing on the top shelf.", MODEL_ID); + await waitForAgentSays(workspace, chatId, "Created the shelved thing."); + + expect(toolResultShownToModel("create-bad-option")).toContain('accept only "shelf"'); + const pending = await onlyPendingAction(workspace, "the creation action to be pending"); + if (pending.type !== "action") throw new Error("Expected an action record"); + expect(pending.description.description).toContain("on shelf top"); + expect(model.remainingSteps()).toBe(0); +}); diff --git a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts index 83cb6ecc87..09b4cad975 100644 --- a/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts +++ b/packages/integration-tests/fixtures/gatekeeper-test/src/test-gatekeeper.ts @@ -24,8 +24,8 @@ import { DurableObject, RpcTarget, WorkerEntrypoint, type RpcStub } from "cloudf import { skipRpcValidation, validateRpc } from "capnweb-validate"; import type { AccountDescription, ActionKind, ApprovalQueue, Gatekeeper, GatekeeperConnectCallback, - GatekeeperUser, GatekeeperUserVerifier, ResourceDescription, ResourceConfiguratorFrame, - SupportedResource, VendorDescription, + GatekeeperUser, GatekeeperUserVerifier, ResourceCreationOptions, ResourceDescription, + ResourceConfiguratorFrame, SupportedResource, VendorDescription, } from "@gadgets/workshop-shared/gatekeeper"; import type { ChatGatewayRpcTarget, GadgetResponse, @@ -168,7 +168,7 @@ type BindingProps = AccountProps & { resourceUrl: string; ambient?: true; /** Present on bindings minted by createResource(): the thing to create once approved. */ - creation?: { title: string }; + creation?: { title: string, shelf?: string }; }; @validateRpc() @@ -262,7 +262,8 @@ export class TestAccount * Mint a NEW test thing (createExternalResource): a provisional resource URL and a gatekeeper * class that simulates the thing until the creation action is approved. */ - async createResource(resourceUrlPattern: string, options: { title: string }): Promise<{ + async createResource(resourceUrlPattern: string, + input: { title: string, options?: ResourceCreationOptions }): Promise<{ class: DurableObjectClass>; resource: SupportedResource; resourceUrl: string; @@ -271,11 +272,24 @@ export class TestAccount throw new Error( `The test gatekeeper cannot create resources of type "${resourceUrlPattern}".`); } + // Per the ResourceCreationOptions contract: unknown/invalid options are agent-fixable + // rejections, and accepted ones surface on the creation card (see submitCreationAction). + const { title, options } = input; + const unknown = Object.keys(options ?? {}).filter((key) => key !== "shelf"); + if (unknown.length > 0) { + throw new Error(`Unknown creation option(s): ${unknown.join(", ")}. ` + + `Test things accept only "shelf" (a string).`); + } + const shelf = options?.shelf; + if (shelf !== undefined && typeof shelf !== "string") { + throw new Error(`The "shelf" creation option must be a string.`); + } const resourceUrl = `https://${VENDOR_HOST}/things/provisional-${crypto.randomUUID()}`; return { class: this.ctx.exports.TestGatekeeper({ props: { - label: this.ctx.props.label, resourceUrl, creation: { title: options.title }, + label: this.ctx.props.label, resourceUrl, + creation: { title, ...(shelf !== undefined ? { shelf } : {}) }, }, }), resource: SUPPORTED_RESOURCES[0], @@ -452,7 +466,8 @@ export class TestGatekeeper try { await approvalQueue.submitAction(id, { title: `Create test thing "${creation.title}"`, - description: `Create a new test thing titled **${creation.title}**.`, + description: `Create a new test thing titled **${creation.title}**` + + `${creation.shelf !== undefined ? ` on shelf ${creation.shelf}` : ""}.`, implementsRevert: false, actionKind: { tag: "create-thing", label: "Create thing" }, }); diff --git a/packages/workshop-backend/src/agent.ts b/packages/workshop-backend/src/agent.ts index 5034519c58..1a2a62eb92 100644 --- a/packages/workshop-backend/src/agent.ts +++ b/packages/workshop-backend/src/agent.ts @@ -2,7 +2,7 @@ import { AiChatMessage, AiChatAuthorInfo, AiToolCall, AiChatMessageBody, AgentSp import { applyCodeChange, codeChangeSerializedSize, replaceSpanChange, type CodeContent, type CodeChange, type FileChange } from '@gadgets/workshop-shared/code-change'; import { PDF_MIME_TYPE, modelApiSupportsPdfAttachments } from './chat-attachment-pdf'; -import { AgentCatalog, ObservationDescription } from '@gadgets/workshop-shared/gatekeeper'; +import { AgentCatalog, ObservationDescription, type ResourceCreationOptions } from '@gadgets/workshop-shared/gatekeeper'; import { createWorkshopLogger } from "./observability"; import { Type } from "@earendil-works/pi-ai"; import type { @@ -393,6 +393,7 @@ export type CreateExternalResourceInput = { title: string; bindingName: string; accountId?: number; + options?: ResourceCreationOptions; }; /** @@ -3306,6 +3307,13 @@ export async function runAgent( "Which connected account creates the resource. Only needed when several accounts " + "of the vendor are connected (a rejection will list the candidate ids).", })), + options: Type.Optional(Type.Record( + Type.String(), Type.Union([Type.String(), Type.Number(), Type.Boolean()]), { + description: + "Vendor-specific creation parameters (flat scalars), e.g. a parent folder id. " + + "The creatable type's description lists the accepted keys; omit unless it names " + + "some. Unknown keys are rejected with guidance.", + })), }), execute: async (toolCallId, input) => { try { diff --git a/packages/workshop-backend/src/overseer.ts b/packages/workshop-backend/src/overseer.ts index 2e97d6f1b7..30b0eac738 100644 --- a/packages/workshop-backend/src/overseer.ts +++ b/packages/workshop-backend/src/overseer.ts @@ -9154,7 +9154,8 @@ class OverseerImpl implements AgentHooks { let userStub = wrapDoStubForTelemetry( this.users.get(this.users.idFromName(initiator.id)), this.logger); minted = await userStub.createResourceGatekeeper( - input.vendorId, input.accountId, input.resourceUrlPattern, {title: input.title}); + input.vendorId, input.accountId, input.resourceUrlPattern, + {title: input.title, options: input.options}); } catch (error) { return `Cannot create the resource: ${stringifyError(error)}`; } diff --git a/packages/workshop-backend/src/user.ts b/packages/workshop-backend/src/user.ts index 49ef50b6d0..984cad2372 100644 --- a/packages/workshop-backend/src/user.ts +++ b/packages/workshop-backend/src/user.ts @@ -1,6 +1,6 @@ import { RpcStub } from "capnweb"; import { GadgetMetadataWithTimestamps, AiChatAuthorInfo, AiModelConfig, SUGGESTED_MODELS, CollaboratorRole, ConnectedAccountsSubscriber, ConnectedAccountsFilter, GatekeeperVendorFilter, GadgetMetadata, BlueprintMetadata, BlueprintLibrarySummary, BlueprintSource, BlueprintUserSummary, BLUEPRINT_SCREENSHOT_R2_PREFIX, GatekeeperVendorInfo, BlueprintOutput, OutputSummary, WorkpieceId, ListOutputsResult, AUTH_ERROR_CODES, createAuthError } from '@gadgets/workshop-shared/api'; -import { Gatekeeper, GatekeeperUser, GatekeeperUserVerifier, GatekeeperVendor, AccountDescription, VendorDescription, GatekeeperConnectCallback, SupportedResource, ResourceConfiguratorFrame, AppUiContext, GatekeeperUiFrame } from "@gadgets/workshop-shared/gatekeeper"; +import { Gatekeeper, GatekeeperUser, GatekeeperUserVerifier, GatekeeperVendor, AccountDescription, VendorDescription, GatekeeperConnectCallback, SupportedResource, ResourceConfiguratorFrame, ResourceCreationOptions, AppUiContext, GatekeeperUiFrame } from "@gadgets/workshop-shared/gatekeeper"; import { shouldAutoProvisionAccount, ambientGatekeeperMode } from "./provisioning-policy.js"; import { CloudflareGatekeeperUser } from "@gadgets/workshop-shared/cloudflare-gatekeeper"; import { DurableObject, WorkerEntrypoint } from "cloudflare:workers"; @@ -1701,7 +1701,7 @@ export class UserDurableObject extends DurableObject { */ async createResourceGatekeeper( vendorId: string, accountId: number | undefined, resourceUrlPattern: string, - options: {title: string}) + input: {title: string, options?: ResourceCreationOptions}) : Promise<{class: DurableObjectClass>, vendorId: string, typeUrlPattern: string, resourceUrl: string}> { let account: ConnectedAccountRecord; @@ -1740,7 +1740,7 @@ export class UserDurableObject extends DurableObject { // error, which the overseer relays to the agent. let {class: cls, resource, resourceUrl} = await (account.account as unknown as ResourceCreatorStub) - .createResource(resourceUrlPattern, options); + .createResource(resourceUrlPattern, input); // Check the admin disable-set against the pattern the vendor actually resolved, after the // RPC, exactly like getGatekeeperClassFor -- the vendor is the authority on which resource diff --git a/packages/workshop-shared/src/api.ts b/packages/workshop-shared/src/api.ts index 515a52f6ab..79fad0233b 100644 --- a/packages/workshop-shared/src/api.ts +++ b/packages/workshop-shared/src/api.ts @@ -24,7 +24,7 @@ // Gadget a stub pointing to the Gadget's server-side Durable Object interface. import { RpcCompatible, RpcStub, RpcTarget } from "capnweb"; -import { AccountDescription, ActionKind, ActionDescription, AvatarImage, GatekeeperUiFrame, ObservationDescription, ResourceDescription, ResourceConfiguratorFrame, SupportedResource, VendorDescription, HookDescription } from "./gatekeeper.js"; +import { AccountDescription, ActionKind, ActionDescription, AvatarImage, GatekeeperUiFrame, ObservationDescription, ResourceDescription, ResourceConfiguratorFrame, ResourceCreationOptions, SupportedResource, VendorDescription, HookDescription } from "./gatekeeper.js"; import type { CodeChange } from "./code-change.js"; import type { UiFeatureFlags } from "./feature-flags.js"; @@ -3255,6 +3255,12 @@ export type AiToolCall = { /** Which connected account creates the resource; required only when several match. */ accountId?: number; + + /** + * Vendor-specific creation parameters (flat scalars), as documented by the creatable + * type's description; see ResourceCreationOptions. + */ + options?: ResourceCreationOptions; }; /** diff --git a/packages/workshop-shared/src/gatekeeper.ts b/packages/workshop-shared/src/gatekeeper.ts index d914beeca1..28f4d50e5a 100644 --- a/packages/workshop-shared/src/gatekeeper.ts +++ b/packages/workshop-shared/src/gatekeeper.ts @@ -568,6 +568,15 @@ export interface GatekeeperConnectCallback extends WorkerEntrypoint { credentialsRestored(expiresAt?: Date): Promise; } +/** + * Vendor-specific creation parameters for GatekeeperUser.createResource(), authored by the agent + * and untrusted like `title`. Flat bounded scalars only (e.g. a parent folder id). The resource's + * `creatable.description` documents the accepted keys; the vendor MUST reject unknown or invalid + * entries with an agent-readable message, and MUST reflect consequential entries (e.g. placement) + * in the creation action's description so the user approves what will actually happen. + */ +export type ResourceCreationOptions = Record; + /** * RPC interface to an Adapter. This is a privileged interface exposed to the Gadget Workshop UI * itself, not to Gadgets nor AI agents. @@ -617,9 +626,11 @@ export interface GatekeeperUser extends WorkerEntrypoint { * the resource locally, and describe() must not call the provider. * * Throws with an agent-readable message when the account cannot create this resource type - * (e.g. its authorization does not cover the needed scopes); callers surface the message. + * (e.g. its authorization does not cover the needed scopes) or when `input.options` carries + * unknown or invalid entries (see ResourceCreationOptions); callers surface the message. */ - createResource?(resourceUrlPattern: string, options: {title: string}): Promise<{ + createResource?(resourceUrlPattern: string, + input: {title: string, options?: ResourceCreationOptions}): Promise<{ class: DurableObjectClass>; resource: SupportedResource; /** Provisional URL of the new resource; replaced by the real URL once created. */ From dc950a46c9a721d67db41084383482901c597a74 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Wed, 2 Sep 2026 14:37:00 -0500 Subject: [PATCH 6/7] Support creating new Google Docs via createExternalResource. GOOGLE_DOC_RESOURCE is the first creatable type. createResource() checks the account holds the doc grant (documents.create needs the write scope that grant already carries -- no new scopes), mints a provisional- document id, and returns a GoogleDocGatekeeperImpl imbued with creation props. getGatekeeperClassFor refuses such an id, so a provisional URL that outlives its creation -- one a rejection stranded, or that a blueprint captured under suggestValue and an install auto-assigned -- cannot mint a binding whose document never resolves. The guard sits there rather than in parseResourceUrl, which must keep parsing these URLs for describe(). It accepts no creation options: assertNoCreationOptions refuses the whole map, quoting the type's own creatable.description, so a placement the agent supplies is a fixable rejection rather than a document silently created somewhere else. The refusal runs before the grant round-trip, because the agent can fix its own call within the turn while a missing grant ends it on a user action. The facet queues a createDocument action as pending action #1, so the existing in-order approval rule applies the creation before any queued edit. Until then the binding simulates over a synthetic empty snapshot with zero provider traffic (describe(), getMetadata(), and getContent() all answer locally); sourceMap/bodyEndIndex are safe to fake because materialization only runs at apply time, when the document exists. Apply calls documents.create and late-binds provisional-to-real via the kit's ProvisionalIds; edit actions resolve the real id before touching the API. Rejecting the creation invalidates every queued edit and marks the binding dead, so session methods explain instead of simulating against nothing. Observers are tracked rather than asserted. A provisional binding has no ACL to consult, and admitting one unchecked has to be paid for later: the overseer re-runs addObserver on every open but never on a session already open, so a collaborator who joined before the creation landed would keep observing the real document, including content written to it outside the workspace. The document is one tracked set in the package's ObserverTracker -- a binding minted against an existing document seeds it, keeping admission the hasDocAccess check it was, while a created one seeds nothing and the first read after the creation forward-excludes whoever joined before it. commit() runs only once the overseer authorizes, so the set is promoted only after every named observer was torn down or provably cannot reach it. Session reads funnel through one #authorizeRead, and removeObserver is real. Settling an action the facet already settled is a no-op, not an error. The overseer marks its record approved only after applyAction returns, so a decision whose reply is lost leaves a pending record with no facet-side counterpart; answering "Unknown pending" stranded a card that could then be neither approved nor rejected, blocking every queued edit behind it under the in-order rule and, for a creation, the binding itself. PendingActionStore.issued() separates a never-submitted id, which still throws, from a settled one -- the at-least-once contract gatekeeper-kit documents. A crash between documents.create and the binding write can still leak one duplicate document on a retried approval (the API has no idempotency key, and the edit write-marker protocol cannot cover creation); accepted for now. The workerd docs suite covers simulation-without-traffic, create-then-edit ordering, retried-approval idempotency, observer withholding across the creation, and the rejection cascade. --- .../__tests__/resources.test.ts | 38 +- .../gatekeeper-google/__tests__/worker.ts | 122 +++++- .../workerd/google-doc-actions.test.ts | 130 +++++- packages/gatekeeper-google/src/docs-api.ts | 22 + packages/gatekeeper-google/src/google.ts | 381 ++++++++++++++++-- .../src/markdown-converter.ts | 18 + packages/gatekeeper-google/src/resources.ts | 34 +- 7 files changed, 675 insertions(+), 70 deletions(-) diff --git a/packages/gatekeeper-google/__tests__/resources.test.ts b/packages/gatekeeper-google/__tests__/resources.test.ts index 8e7c2d51b1..b78ea87082 100644 --- a/packages/gatekeeper-google/__tests__/resources.test.ts +++ b/packages/gatekeeper-google/__tests__/resources.test.ts @@ -2,9 +2,11 @@ import { describe, expect, it } from "vitest"; import { BIGQUERY_RESOURCE, GMAIL_RESOURCE, GOOGLE_CALENDAR_RESOURCE, GOOGLE_DOC_RESOURCE, GOOGLE_DRIVE_FILE_RESOURCE, GOOGLE_DRIVE_RESOURCE, GOOGLE_SHARED_DRIVE_RESOURCE, - GOOGLE_SHEETS_RESOURCE, IDENTITY_SCOPES, LEGACY_GRANTED_RESOURCE_URL_PATTERNS, RESOURCE_BY_KIND, - RESOURCE_SCOPES, SCOPE_DERIVED_RESOURCE_URL_PATTERNS, SUPPORTED_RESOURCES, - grantedResourceUrlPatterns, hasDriveResourceGrant, parseResourceUrl, + GOOGLE_SHEETS_RESOURCE, IDENTITY_SCOPES, LEGACY_GRANTED_RESOURCE_URL_PATTERNS, + PROVISIONAL_DOC_ID_PREFIX, RESOURCE_BY_KIND, RESOURCE_SCOPES, + SCOPE_DERIVED_RESOURCE_URL_PATTERNS, SUPPORTED_RESOURCES, + assertNoCreationOptions, grantedResourceUrlPatterns, hasDriveResourceGrant, isProvisionalDocId, + parseResourceUrl, recordedResourceUrlPatterns, resourceUrlPatternsToOAuthScopes, resourcesCoveredByScopes, validateResourceUrlPatterns, } from "../src/resources"; @@ -57,6 +59,27 @@ describe("resource declarations", () => { expect(new Set(patterns).size).toBe(patterns.length); }); + // The vendor half of the ResourceCreationOptions contract. A dropped option is a placement the + // user asked for and silently did not get, so every key is refused while none is accepted. + describe("creation options", () => { + it("accepts an absent or empty map", () => { + expect(() => assertNoCreationOptions(GOOGLE_DOC_RESOURCE)).not.toThrow(); + expect(() => assertNoCreationOptions(GOOGLE_DOC_RESOURCE, {})).not.toThrow(); + }); + + it("refuses every key, naming them and what creation does instead", () => { + expect(() => assertNoCreationOptions(GOOGLE_DOC_RESOURCE, {parentId: "abc", pinned: true})) + .toThrow(/accepts no options, but received: parentId, pinned\./); + expect(() => assertNoCreationOptions(GOOGLE_DOC_RESOURCE, {parentId: "abc"})) + .toThrow(/in the account's My Drive/); + }); + + it("refuses a key that would otherwise reach vendor code as a prototype write", () => { + expect(() => assertNoCreationOptions(GOOGLE_DOC_RESOURCE, JSON.parse('{"__proto__": "x"}'))) + .toThrow(/accepts no options/); + }); + }); + // Adding an entry short-circuits ensureResources, so a legacy account would be treated as // already holding a grant it never made and would never be re-prompted for consent. it("keeps the legacy granted set frozen at Gmail, Doc and BigQuery", () => { @@ -379,6 +402,15 @@ describe("parseResourceUrl", () => { .toEqual({ kind: "doc", documentId: "DOC123" }); }); + // getGatekeeperClassFor refuses to bind such a URL: the document was never created. The + // placeholder must stay recognisable through the URL grammar for that guard to fire. + it("keeps a provisional document ID recognisable", () => { + let documentId = `${PROVISIONAL_DOC_ID_PREFIX}abc`; + let target = parseResourceUrl(`https://docs.google.com/document/d/${documentId}/edit`); + expect(target).toEqual({ kind: "doc", documentId }); + expect(target.kind === "doc" && isProvisionalDocId(target.documentId)).toBe(true); + }); + it("extracts a spreadsheet ID", () => { expect(parseResourceUrl("https://docs.google.com/spreadsheets/d/SHEET123/edit#gid=0")) .toEqual({ kind: "sheets", spreadsheetId: "SHEET123" }); diff --git a/packages/gatekeeper-google/__tests__/worker.ts b/packages/gatekeeper-google/__tests__/worker.ts index aad8302754..6cb362e465 100644 --- a/packages/gatekeeper-google/__tests__/worker.ts +++ b/packages/gatekeeper-google/__tests__/worker.ts @@ -1,7 +1,7 @@ -import { DurableObject, RpcStub, RpcTarget } from "cloudflare:workers"; +import { DurableObject, RpcStub, RpcTarget, WorkerEntrypoint } from "cloudflare:workers"; import type { - ActionDescription, ApprovalQueue, GitCache, HookController, HookDescription, - ObservationDescription, + ActionDescription, ApprovalQueue, GatekeeperUserVerifier, GitCache, HookController, + HookDescription, ObservationDescription, ResourceDescription, } from "@gadgets/workshop-shared/gatekeeper"; import { TestGitCache } from "./test-git-cache"; import type { GoogleAccessToken } from "../src/google-api"; @@ -16,12 +16,27 @@ export class UserAccount extends DurableObject { } } -type GatekeeperProps = { userObjectId: string; documentId: string }; +/** + * Stands in for the Google verifier an observer is checked against; props fix its verdict. + * + * An entrypoint rather than a local RpcTarget because the tracker persists the stub it is given, + * and only a persistent stub can be stored. + */ +export class TestVerifier extends WorkerEntrypoint { + async hasDocAccess(_documentId: string): Promise { + return this.ctx.props.allowed; + } +} + +type GatekeeperProps = { userObjectId: string; documentId: string; creation?: { title: string } }; class TestApprovalQueue extends RpcTarget implements ApprovalQueue { actionId?: number; + readonly observations: ObservationDescription[] = []; - async authorizeObservation(_description: ObservationDescription): Promise {} + async authorizeObservation(description: ObservationDescription): Promise { + this.observations.push(description); + } async getGitCache(): Promise { throw new Error("Unexpected git cache access"); @@ -41,20 +56,56 @@ class TestApprovalQueue extends RpcTarget implements ApprovalQueue { } export class TestHooks extends DurableObject { - #gatekeeper(facetName: string) { + // `creation` makes the facet a createResource-minted provisional doc gatekeeper. Facets are + // cached by name, so every call addressing one provisional facet must pass the same creation + // (the callback re-runs after a Durable Object restart). + #gatekeeper(facetName: string, creation?: { title: string }) { let userObjectId = this.ctx.exports.UserAccount.idFromName("test-user").toString(); + let props: GatekeeperProps = creation + ? { userObjectId, documentId: `provisional-${facetName}`, creation } + : { userObjectId, documentId: "doc-1" }; return this.ctx.facets.get(facetName, () => ({ - class: this.ctx.exports.GoogleDocGatekeeperImpl({ - props: { userObjectId, documentId: "doc-1" } satisfies GatekeeperProps, - }), + class: this.ctx.exports.GoogleDocGatekeeperImpl({ props }), })); } - async submitAppend(facetName: string, markdown: string): Promise { + /** The error admitting an observer fails with, or null when they are admitted. */ + async addObserver( + facetName: string, id: string, allowed: boolean, creation?: { title: string }, + ): Promise { + // ctx.exports is typed from the production main module, which has no test-only entrypoint. + let testExports = this.ctx.exports as unknown as { + TestVerifier(options: { props: { allowed: boolean } }): Fetcher; + }; + let verifier = testExports.TestVerifier({ props: { allowed } }); + try { + await this.#gatekeeper(facetName, creation).addObserver(id, verifier); + return null; + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + } + + /** Observers a content read withholds the disclosure from. */ + async readContentExclusions( + facetName: string, creation?: { title: string }): Promise { + let queue = new TestApprovalQueue(); + { + using approvalQueue = new RpcStub(queue); + using session = await this.#gatekeeper(facetName, creation).startSession( + approvalQueue as unknown as ApprovalQueue, + ) as GoogleDocSession & Disposable; + await session.getContent(); + } + return queue.observations.flatMap(observation => observation.excludeObservers ?? []); + } + + async submitAppend( + facetName: string, markdown: string, creation?: { title: string }): Promise { let queue = new TestApprovalQueue(); { using approvalQueue = new RpcStub(queue); - using session = await this.#gatekeeper(facetName).startSession( + using session = await this.#gatekeeper(facetName, creation).startSession( approvalQueue as unknown as ApprovalQueue, ) as GoogleDocSession & Disposable; await session.appendText(markdown); @@ -63,10 +114,21 @@ export class TestHooks extends DurableObject { return queue.actionId; } + /** Queue the creation action; null when the call was an idempotent no-op. */ + async submitCreation(facetName: string, title: string): Promise { + let queue = new TestApprovalQueue(); + { + using approvalQueue = new RpcStub(queue); + await this.#gatekeeper(facetName, { title }).submitCreationAction( + approvalQueue as unknown as ApprovalQueue); + } + return queue.actionId ?? null; + } + /** The `lastModified` a metadata read reports, as epoch milliseconds. */ - async readMetadata(facetName: string): Promise { + async readMetadata(facetName: string, creation?: { title: string }): Promise { using approvalQueue = new RpcStub(new TestApprovalQueue()); - using session = await this.#gatekeeper(facetName).startSession( + using session = await this.#gatekeeper(facetName, creation).startSession( approvalQueue as unknown as ApprovalQueue, ) as GoogleDocSession & Disposable; let metadata = await session.getMetadata(); @@ -74,28 +136,50 @@ export class TestHooks extends DurableObject { } /** The simulated document content a read reports. */ - async readContent(facetName: string): Promise { + async readContent(facetName: string, creation?: { title: string }): Promise { using approvalQueue = new RpcStub(new TestApprovalQueue()); - using session = await this.#gatekeeper(facetName).startSession( + using session = await this.#gatekeeper(facetName, creation).startSession( approvalQueue as unknown as ApprovalQueue, ) as GoogleDocSession & Disposable; let content = await session.getContent(); return content; } - async applyAction(facetName: string, actionId: number): Promise { + /** The error message a content read fails with, or null if it succeeds. */ + async readContentError( + facetName: string, creation?: { title: string }): Promise { + try { + await this.readContent(facetName, creation); + return null; + } catch (error) { + return error instanceof Error ? error.message : String(error); + } + } + + async describeDoc( + facetName: string, creation?: { title: string }): Promise { + return this.#gatekeeper(facetName, creation).describe(); + } + + async applyAction( + facetName: string, actionId: number, creation?: { title: string }, + ): Promise { try { // The overseer always passes an action-scoped git cache with the apply call, and the // validator (sharpened by the `Gatekeeper` interface) requires it, so the test passes a // stand-in the same way. - await this.#gatekeeper(facetName).applyAction(actionId, new RpcStub(new TestGitCache())); + await this.#gatekeeper(facetName, creation) + .applyAction(actionId, new RpcStub(new TestGitCache())); return null; } catch (error) { return error instanceof Error ? error.message : String(error); } } - async rejectAction(facetName: string, actionId: number): Promise { - await this.#gatekeeper(facetName).rejectAction(actionId); + /** Whether the gatekeeper asked for a session restart. */ + async rejectAction( + facetName: string, actionId: number, creation?: { title: string }): Promise { + let result = await this.#gatekeeper(facetName, creation).rejectAction(actionId); + return !!(result && result.restart); } } diff --git a/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts b/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts index 9f94e5c519..79dbec57f4 100644 --- a/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts +++ b/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts @@ -12,10 +12,14 @@ type BatchKind = "content" | "cleanup"; class DocsModel { content = ""; + title = "Test document"; cleanupFailures = 0; ambiguousContentResponses = 0; contentBatches = 0; maxMarkerCount = 0; + /** Every provider request, of any kind. Provisional simulation must leave this at zero. */ + requests = 0; + readonly createdTitles: string[] = []; readonly deletedMarkerIds: string[] = []; readonly markers = new Map(); #revision = 1; @@ -61,6 +65,16 @@ class DocsModel { if (url.hostname !== "docs.googleapis.com") { throw new Error(`Unexpected provider request: ${url}`); } + this.requests++; + if (url.pathname === "/v1/documents" && init?.method === "POST") { + // documents.create: mints doc-1, which the model's other routes already serve. + let body = JSON.parse(String(init?.body)) as { title: string }; + this.createdTitles.push(body.title); + this.title = body.title; + return Response.json({ + documentId: "doc-1", title: body.title, revisionId: `revision-${this.#revision}`, + }); + } if (!url.pathname.endsWith(":batchUpdate")) return Response.json(this.#document()); let body = JSON.parse(String(init?.body)) as { @@ -134,7 +148,7 @@ class DocsModel { } return { documentId: "doc-1", - title: "Test document", + title: this.title, revisionId: `revision-${this.#revision}`, tabs: [{ documentTab: { @@ -182,7 +196,9 @@ describe("Google Doc write receipts", () => { expect(docs.contentBatches).toBe(1); expect(docs.deletedMarkerIds).toEqual(["marker-1"]); expect(docs.markers.size).toBe(0); - expect(await hooks().applyAction("normal", actionId)).toMatch(/Unknown pending/); + // A retry after a lost reply settles again rather than stranding the approval. + expect(await hooks().applyAction("normal", actionId)).toBeNull(); + expect(docs.contentBatches).toBe(1); }); it("reconciles a committed write after its response is lost", async () => { @@ -199,7 +215,7 @@ describe("Google Doc write receipts", () => { expect(docs.contentBatches).toBe(1); expect(docs.markers.size).toBe(0); - expect(await hooks().applyAction("ambiguous", actionId)).toMatch(/Unknown pending/); + expect(await hooks().applyAction("ambiguous", actionId)).toBeNull(); }); it("cleans a retained receipt after restart before the next write", async () => { @@ -265,7 +281,7 @@ describe("Google Doc write receipts", () => { expect(docs.contentBatches).toBe(0); expect(docs.markers.size).toBe(0); - expect(await hooks().applyAction("reject", actionId)).toMatch(/Unknown pending/); + expect(await hooks().applyAction("reject", actionId)).toBeNull(); }); // The overseer marks a record approved only after applyAction() returns, so a second approval of @@ -283,7 +299,7 @@ describe("Google Doc write receipts", () => { write.release(); expect(await first).toBeNull(); - expect(await second).toMatch(/Unknown pending/); + expect(await second).toBeNull(); expect(docs.contentBatches).toBe(1); expect(docs.content.match(/first/g)).toHaveLength(1); expect(docs.markers.size).toBe(0); @@ -326,6 +342,110 @@ describe("Google Doc write receipts", () => { }); }); +describe("Google Doc creation (createExternalResource)", () => { + const CREATION = { title: "My New Doc" }; + + it("simulates the uncreated document without touching the provider", async () => { + let docs = new DocsModel(); + docs.install(); + + let creationId = await hooks().submitCreation("create-simulate", CREATION.title); + expect(creationId).toBe(1); + // Idempotent: a retried submitCreationAction queues nothing new. + expect(await hooks().submitCreation("create-simulate", CREATION.title)).toBeNull(); + + expect(await hooks().readContent("create-simulate", CREATION)).toBe(""); + await hooks().submitAppend("create-simulate", "hello", CREATION); + expect(await hooks().readContent("create-simulate", CREATION)).toContain("hello"); + expect(await hooks().readMetadata("create-simulate", CREATION)).toBeGreaterThan(0); + + let description = await hooks().describeDoc("create-simulate", CREATION); + expect(description.title).toBe(CREATION.title); + expect(description.url).toContain("provisional-create-simulate"); + expect(description.snippet).toContain("pending creation"); + + expect(docs.requests).toBe(0); + }); + + it("applies the creation first, then queued edits, against the real document", async () => { + let docs = new DocsModel(); + docs.install(); + let creationId = await hooks().submitCreation("create-apply", CREATION.title); + let editId = await hooks().submitAppend("create-apply", "hello", CREATION); + + // In-order approval: the edit cannot apply before the creation. + expect(await hooks().applyAction("create-apply", editId, CREATION)) + .toMatch(/approved in order/); + expect(docs.createdTitles).toEqual([]); + + expect(await hooks().applyAction("create-apply", creationId!, CREATION)).toBeNull(); + expect(docs.createdTitles).toEqual([CREATION.title]); + + expect(await hooks().applyAction("create-apply", editId, CREATION)).toBeNull(); + expect(docs.content).toContain("hello"); + + // The binding now describes (and reads) the real document. + let description = await hooks().describeDoc("create-apply", CREATION); + expect(description.url).toContain("doc-1"); + expect(description.snippet).not.toContain("pending creation"); + expect(await hooks().readContent("create-apply", CREATION)).toContain("hello"); + }); + + it("does not create a second document on a retried approval", async () => { + let docs = new DocsModel(); + docs.install(); + let creationId = await hooks().submitCreation("create-retry", CREATION.title); + + expect(await hooks().applyAction("create-retry", creationId!, CREATION)).toBeNull(); + expect(await hooks().applyAction("create-retry", 99, CREATION)) + .toMatch(/Unknown Google Doc action/); + expect(docs.createdTitles).toEqual([CREATION.title]); + }); + + it("rejecting the creation cascades to queued edits and kills the binding", async () => { + let docs = new DocsModel(); + docs.install(); + let creationId = await hooks().submitCreation("create-reject", CREATION.title); + let editId = await hooks().submitAppend("create-reject", "hello", CREATION); + + expect(await hooks().rejectAction("create-reject", creationId!, CREATION)).toBe(true); + + // The queued edit was invalidated: approving it settles the record without a provider call. + expect(await hooks().applyAction("create-reject", editId, CREATION)).toBeNull(); + expect(docs.requests).toBe(0); + + // The binding is dead, with an explanation rather than simulation against nothing. + expect(await hooks().readContentError("create-reject", CREATION)).toMatch(/rejected/); + let description = await hooks().describeDoc("create-reject", CREATION); + expect(description.snippet).toContain("creation rejected"); + }); + + it("withholds the created document from an observer admitted before it existed", async () => { + let docs = new DocsModel(); + docs.install(); + let creationId = await hooks().submitCreation("create-observer", CREATION.title); + + // Nothing exists to check an ACL against, so the collaborator is admitted unchecked. + expect(await hooks().addObserver("create-observer", "collab", false, CREATION)).toBeNull(); + expect(await hooks().readContentExclusions("create-observer", CREATION)).toEqual([]); + + expect(await hooks().applyAction("create-observer", creationId!, CREATION)).toBeNull(); + + // The first read of the real document is where that admission is settled. + expect(await hooks().readContentExclusions("create-observer", CREATION)).toEqual(["collab"]); + expect(await hooks().readContentExclusions("create-observer", CREATION)).toEqual([]); + }); + + it("checks the ACL when admitting an observer to an existing document", async () => { + new DocsModel().install(); + + expect(await hooks().addObserver("existing-observer", "collab", false)) + .toMatch(/does not have access/); + expect(await hooks().addObserver("existing-observer", "collab", true)).toBeNull(); + expect(await hooks().readContentExclusions("existing-observer")).toEqual([]); + }); +}); + describe("Google Doc metadata", () => { it("holds the modification time steady while the document is unchanged", async () => { let docs = new DocsModel(); diff --git a/packages/gatekeeper-google/src/docs-api.ts b/packages/gatekeeper-google/src/docs-api.ts index 2f51963e5c..7f7eb7ff91 100644 --- a/packages/gatekeeper-google/src/docs-api.ts +++ b/packages/gatekeeper-google/src/docs-api.ts @@ -152,6 +152,28 @@ export class GoogleDocsApi { }); } + /** + * Create a new, empty document titled `title` in the account's My Drive. Requires the + * `documents` (write) scope. Returns the new document's id/title/revision — `documents.create` + * responds with the full document resource, but only these fields are needed and a fresh doc + * has no tabs content worth normalizing. + */ + async createDocument( + title: string, + ): Promise> { + return await this.#request< + Pick + >( + DOCS_API_BASE, + { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ title }), + }, + "create document", + ); + } + /** Fetch and normalize a single-tab document. */ async getDocument(documentId: string): Promise { let document = await this.#request( diff --git a/packages/gatekeeper-google/src/google.ts b/packages/gatekeeper-google/src/google.ts index 890e6d1658..1ede02b945 100644 --- a/packages/gatekeeper-google/src/google.ts +++ b/packages/gatekeeper-google/src/google.ts @@ -1,6 +1,6 @@ import { WorkerEntrypoint, DurableObject, RpcTarget, RpcStub } from "cloudflare:workers"; import { skipRpcValidation, validateRpc } from "capnweb-validate"; -import { GatekeeperUser, GatekeeperUserVerifier, GatekeeperVendor as GatekeeperVendorIface, Gatekeeper, ResourceDescription, ApprovalQueue, ObservationDescription, VendorDescription, GatekeeperConnectCallback, GatekeeperConnectOptions, AccountDescription, SupportedResource, ResourceConfiguratorFrame, Cursor, ActionKind, GitCache } from '@gadgets/workshop-shared/gatekeeper'; +import { GatekeeperUser, GatekeeperUserVerifier, GatekeeperVendor as GatekeeperVendorIface, Gatekeeper, ResourceDescription, ApprovalQueue, ObservationDescription, ResourceCreationOptions, VendorDescription, GatekeeperConnectCallback, GatekeeperConnectOptions, AccountDescription, SupportedResource, ResourceConfiguratorFrame, Cursor, ActionKind, GitCache } from '@gadgets/workshop-shared/gatekeeper'; import { PreviewOAuth, PreviewOAuthConfigurationError, @@ -14,7 +14,7 @@ import type { GoogleSpreadsheetReadSession, GoogleSpreadsheetSession, SpreadsheetInfo, SpreadsheetRange, SpreadsheetValueMode, } from "./sheets-types"; -import { docToMarkdown, markdownToDocRequests, computeReplaceOperations, DocSnapshot } from "./markdown-converter"; +import { docToMarkdown, emptyDocSnapshot, markdownToDocRequests, computeReplaceOperations, DocSnapshot } from "./markdown-converter"; import { DriveApi } from "./drive-api"; import { driveObserverTracker } from "./drive-observers"; import { @@ -67,10 +67,13 @@ import { AccessTokenCache, AccessTokenRequest, ACCESS_TOKEN_EXPIRY_SAFETY_MS } f import { BIGQUERY_HOST, BIGQUERY_RESOURCE, GMAIL_RESOURCE, GOOGLE_CALENDAR_RESOURCE, GOOGLE_DOC_RESOURCE, GOOGLE_DRIVE_FILE_RESOURCE, GOOGLE_DRIVE_RESOURCE, - GOOGLE_SHARED_DRIVE_RESOURCE, GOOGLE_SHEETS_RESOURCE, RESOURCE_BY_KIND, SUPPORTED_RESOURCES, - grantedResourceUrlPatterns, hasDriveResourceGrant, parseResourceUrl, + GOOGLE_SHARED_DRIVE_RESOURCE, GOOGLE_SHEETS_RESOURCE, PROVISIONAL_DOC_ID_PREFIX, + RESOURCE_BY_KIND, SUPPORTED_RESOURCES, + assertNoCreationOptions, grantedResourceUrlPatterns, hasDriveResourceGrant, isProvisionalDocId, + parseResourceUrl, recordedResourceUrlPatterns, type RecordedResourceGrant, } from "./resources"; +import { ProvisionalIds } from "@gadgets/gatekeeper-kit/simulation"; import { beginStoredOAuthFlow, claimStoredOAuthFlow, mergeGrantedResources, prepareOAuthFlow, shouldDeleteCredentialsOnAlarm, type OAuthFlowMode, @@ -712,6 +715,13 @@ export class GatekeeperUserImpl extends WorkerEntrypoint>; + resource: SupportedResource; + resourceUrl: string; + }> { + if (resourceUrlPattern !== GOOGLE_DOC_RESOURCE.urlPattern) { + throw new Error( + `Google can only create resources of type "${GOOGLE_DOC_RESOURCE.title}" ` + + `(${GOOGLE_DOC_RESOURCE.urlPattern}).`); + } + // Before the grant round-trip: this one the agent can fix and retry within its turn, while a + // missing grant ends the turn on a user action. + assertNoCreationOptions(GOOGLE_DOC_RESOURCE, input.options); + + // Creation needs the Google Doc grant (the write scope). Fail with a readable message rather + // than queuing a creation action that can never apply. + let id = this.ctx.exports.UserAccount.idFromString(this.ctx.props.userObjectId); + let granted = await this.ctx.exports.UserAccount.get(id).getGrantedResourceUrlPatterns(); + if (!granted.includes(GOOGLE_DOC_RESOURCE.urlPattern)) { + throw new Error( + "The connected Google account has not granted Google Doc access, which document " + + "creation requires. The user must expand the connection's access first (reconnect " + + "with Google Doc enabled)."); + } + + // A random UUID rather than a stored sequence: this entrypoint is stateless, and the durable + // provisional→real binding lives in the gatekeeper facet's own storage. + let documentId = `${PROVISIONAL_DOC_ID_PREFIX}${crypto.randomUUID()}`; + let props: GoogleDocGatekeeperImplProps = { + userObjectId: this.ctx.props.userObjectId, + documentId, + creation: {title: input.title}, + }; + return { + class: this.ctx.exports.GoogleDocGatekeeperImpl({props}), + resource: GOOGLE_DOC_RESOURCE, + resourceUrl: `https://docs.google.com/document/d/${documentId}/edit`, + }; + } + async startResourceConfigurator( resourceUrlPattern: string): Promise { let getToken = async (opts?: AccessTokenRequest) => { @@ -1025,6 +1076,11 @@ class PendingActionStore { remove(id: number): void { this.#kv.delete(this.#actionKey(id)); } + + /** Whether `id` was issued by this store: with no record left, the action is settled. */ + issued(id: number): boolean { + return id > 0 && id < (this.#kv.get("pending:nextActionId") ?? 1); + } } @validateRpc() @@ -1080,7 +1136,26 @@ type GoogleDocAppendAction = GoogleDocActionBase & { markdown: string; } -type GoogleDocAction = GoogleDocReplaceAction | GoogleDocAppendAction; +/** + * Creates the document itself (see GatekeeperUser.createResource). Always the first pending + * action, so in-order approval applies it before any queued edit. + */ +type GoogleDocCreateAction = GoogleDocActionBase & { + type: "createDocument"; + title: string; +} + +type GoogleDocAction = GoogleDocReplaceAction | GoogleDocAppendAction | GoogleDocCreateAction; + +/** + * `baseRevisionId` sentinel for actions queued before the document exists. The field is stored + * but never consumed by simulation or materialization, so no real revision can collide with it. + */ +const PROVISIONAL_REVISION = "provisional"; +/** Set once submitCreationAction has queued the creation, making retried calls no-ops. */ +const DOC_CREATION_SUBMITTED_KEY = "docCreationSubmitted"; +/** The rejection reason, once the user rejects the creation action. The binding is then dead. */ +const DOC_CREATION_REJECTED_KEY = "docCreationRejected"; const DOC_WRITE_RECEIPT_KEY = "docWriteReceipt"; const DOC_METADATA_REVISION_KEY = "docMetadataRevision"; @@ -1260,6 +1335,9 @@ function applyGoogleDocActionToMarkdown(markdown: string, action: GoogleDocActio markdown, action.oldMarkdown, action.newMarkdown, "replaceText"); case "appendText": return appendMarkdownForSimulation(markdown, action.markdown); + case "createDocument": + // Simulation starts from the empty snapshot the creation implies; nothing to change. + return markdown; default: action satisfies never; throw new Error(`unknown action type: ${(action as any).type}`); @@ -1334,6 +1412,11 @@ function materializeGoogleDocAction(snapshot: DocSnapshot, action: GoogleDocActi return markdownToDocRequests("\n" + action.markdown, insertAt); } + case "createDocument": + // Creation calls documents.create directly in #applyAction; it never becomes batchUpdate + // requests against an existing document. + throw new Error("createDocument cannot be materialized as document edits"); + default: action satisfies never; throw new Error(`unknown action type: ${(action as any).type}`); @@ -1342,7 +1425,10 @@ function materializeGoogleDocAction(snapshot: DocSnapshot, action: GoogleDocActi type GoogleDocGatekeeperImplProps = { userObjectId: string; + /** Provisional (see PROVISIONAL_DOC_ID_PREFIX) when the binding was minted by createResource. */ documentId: string; + /** Present only on bindings minted by createResource: what to create when the user approves. */ + creation?: {title: string}; } // All Google Doc edits (replaceText, appendText, ...) are grouped under a single action kind @@ -1351,6 +1437,48 @@ const EDIT_DOCUMENT_ACTION: ActionKind = { label: "Document edits", }; +// Creating the document is deliberately its own kind, never auto-approvable. +const CREATE_DOCUMENT_ACTION: ActionKind = { + tag: "createDocument", + label: "Document creation", +}; + +/** The provisional→real documentId binding for a doc gatekeeper minted by createResource. */ +function docProvisionalIds(kv: DurableObjectStorage["kv"]): ProvisionalIds { + return new ProvisionalIds(kv, { + namespace: "doc:", + isProvisional: isProvisionalDocId, + }); +} + +/** Key prefix for the document a doc binding has disclosed data from. */ +const DOC_OBSERVATION_PREFIX = "observedDoc:"; + +/** + * The observer tracker for one doc binding, seeded with the document it was minted against. + * + * The seed is what makes admission an ACL check from the first open, as a binding to an existing + * document warrants. It is withheld for a provisional id, whose document does not exist yet: + * nothing readable through the binding is provider data until the creation is applied, and the + * first read after that tracks the real id, forward-excluding observers admitted before it. + */ +function docObserverTracker(kv: DurableObjectStorage["kv"], mintedDocumentId: string) + : ObserverTracker> { + if (!isProvisionalDocId(mintedDocumentId)) { + let key = `${DOC_OBSERVATION_PREFIX}${encodeURIComponent(mintedDocumentId)}`; + if (kv.get(key) === undefined) kv.put(key, "observed"); + } + return new ObserverTracker>(kv, { + setPrefix: DOC_OBSERVATION_PREFIX, + encode: encodeURIComponent, + decode: decodeURIComponent, + hasAccess: (verifier, documentId) => verifier.hasDocAccess(documentId), + deniedMessage: () => + "This collaborator does not have access to the bound Google Doc, so they cannot be allowed " + + "to observe data this workspace read from it.", + }); +} + @validateRpc() export class GoogleDocGatekeeperImpl extends DurableObject @@ -1385,6 +1513,7 @@ export class GoogleDocGatekeeperImpl async #reconcileDocWriteReceipt( api: GoogleDocsApi, + documentId: string, document: GoogleDocsDocument, ): Promise { let receipt = this.#readDocWriteReceipt(); @@ -1398,9 +1527,9 @@ export class GoogleDocGatekeeperImpl return document; } - await api.deleteNamedRange(this.ctx.props.documentId, receipt.markerId); + await api.deleteNamedRange(documentId, receipt.markerId); this.#clearDocWriteReceipt(receipt.markerId); - return api.getDocument(this.ctx.props.documentId); + return api.getDocument(documentId); } /** @@ -1426,11 +1555,33 @@ export class GoogleDocGatekeeperImpl }); } + /** The Google-issued documentId, or undefined while a created document is still pending. */ + #resolvedDocumentId(): string | undefined { + let id = docProvisionalIds(this.ctx.storage.kv).resolve(this.ctx.props.documentId); + return isProvisionalDocId(id) ? undefined : id; + } + async describe(): Promise { + let documentId = this.#resolvedDocumentId(); + if (documentId === undefined) { + // The document exists only locally; answer from the creation parameters — describe() must + // not call the provider for a resource that isn't there yet. + let title = this.ctx.props.creation?.title ?? "Untitled document"; + let rejected = this.ctx.storage.kv.get(DOC_CREATION_REJECTED_KEY); + return { + url: `https://docs.google.com/document/d/${this.ctx.props.documentId}/edit`, + title, + snippet: rejected + ? `Google Doc (creation rejected): ${title}` + : `Google Doc (pending creation): ${title}`, + suggestedBindingName: "GOOGLE_DOC", + tsType: "GoogleDocSession", + }; + } let api = new GoogleDocsApi(opts => this.#getAccessToken(opts)); - let doc = await api.getDocumentMetadata(this.ctx.props.documentId); + let doc = await api.getDocumentMetadata(documentId); return { - url: `https://docs.google.com/document/d/${this.ctx.props.documentId}/edit`, + url: `https://docs.google.com/document/d/${documentId}/edit`, title: doc.title, snippet: `Google Doc: ${doc.title}`, suggestedBindingName: "GOOGLE_DOC", @@ -1446,6 +1597,40 @@ export class GoogleDocGatekeeperImpl return [EDIT_DOCUMENT_ACTION]; } + async submitCreationAction(approvalQueue: RpcStub): Promise { + let creation = this.ctx.props.creation; + if (!creation) { + throw new Error("This Google Doc gatekeeper was not minted by createResource()."); + } + if (this.ctx.storage.kv.get(DOC_CREATION_SUBMITTED_KEY)) return; + + let pendingActions = new PendingActionStore(this.ctx.storage.kv); + let action: GoogleDocAction = { + type: "createDocument", + documentId: this.ctx.props.documentId, + submittedAt: Date.now(), + baseRevisionId: PROVISIONAL_REVISION, + title: creation.title, + }; + let actionId = pendingActions.submit(action); + this.ctx.storage.kv.put(DOC_CREATION_SUBMITTED_KEY, true); + try { + await approvalQueue.submitAction(actionId, { + title: `Create Google Doc "${creation.title}"`, + description: + `Create a new, empty Google Doc titled "${creation.title}" in the account's ` + + `My Drive. Edits queued before approval apply to it afterward, in order.`, + implementsRevert: false, + actionKind: CREATE_DOCUMENT_ACTION, + autoApprovable: false, + }); + } catch (error) { + pendingActions.remove(actionId); + this.ctx.storage.kv.delete(DOC_CREATION_SUBMITTED_KEY); + throw error; + } + } + async startSession(approvalQueue: RpcStub) : Promise { let api = new GoogleDocsApi(opts => this.#getAccessToken(opts)); @@ -1456,7 +1641,8 @@ export class GoogleDocGatekeeperImpl approvalQueue.dup(), pendingActions, this.ctx.storage, - this.#simulationCache); + this.#simulationCache, + this.ctx.props.creation); } async applyAction(actionId: number, _cache: RpcStub): Promise { @@ -1472,7 +1658,16 @@ export class GoogleDocGatekeeperImpl let pending = pendingActions.list(); let pendingIndex = pending.findIndex(({id}) => id === actionId); if (pendingIndex === -1) { - throw new Error(`Unknown pending Google Doc action: ${actionId}`); + // The overseer retries a decision whose reply was lost, by which time the record is gone. + // Settling again is a no-op, where throwing would strand an approval that can never be + // decided: the card stays pending and every queued edit behind it is blocked. + if (!pendingActions.issued(actionId)) { + throw new Error(`Unknown Google Doc action: ${actionId}`); + } + logger.warn("re-applying an already-settled Google Doc action", { + event: "google.doc.action.apply.settled", actionId, + }); + return; } let action = pending[pendingIndex].action; if (action.invalidatedReason) { @@ -1488,14 +1683,31 @@ export class GoogleDocGatekeeperImpl `${firstPending?.id} before edit ${actionId}.`); } + let api = new GoogleDocsApi(opts => this.#getAccessToken(opts)); + let docIds = docProvisionalIds(this.ctx.storage.kv); + + if (action.type === "createDocument") { + // A crash between documents.create and bind() can leak one duplicate doc at Google — + // documents.create has no idempotency key; accepted for now. + let created = await api.createDocument(action.title); + docIds.bind(action.documentId, created.documentId); + pendingActions.remove(actionId); + this.#simulationCache.current = undefined; + // Simulated content moves from the synthetic empty base to the real (still empty) document. + await this.ctx.storage.delete(DOC_SNAPSHOT_KEY); + return; + } + + // Queued edits recorded the provisional id when they predate the creation; the in-order rule + // means the creation has been applied by now, so this resolves (or throws a clear message). + let documentId = docIds.requireResolved(action.documentId); if (!action.writeId) { action.writeId = crypto.randomUUID(); pendingActions.put(actionId, action); } let writeMarkerName = googleDocWriteMarkerName(action.writeId); - let api = new GoogleDocsApi(opts => this.#getAccessToken(opts)); - let doc = await api.getDocument(action.documentId); - doc = await this.#reconcileDocWriteReceipt(api, doc); + let doc = await api.getDocument(documentId); + doc = await this.#reconcileDocWriteReceipt(api, documentId, doc); let snapshot = googleDocSnapshot(doc); let markerIds = googleDocNamedRangeIds(doc, writeMarkerName); if (markerIds.length > 1) { @@ -1522,7 +1734,7 @@ export class GoogleDocGatekeeperImpl return; } if (requests.length > 0) { - let result = await api.batchUpdate(action.documentId, requests, snapshot.revisionId, { + let result = await api.batchUpdate(documentId, requests, snapshot.revisionId, { name: writeMarkerName, rangeStart: snapshot.bodyEndIndex - 1, }); @@ -1535,7 +1747,7 @@ export class GoogleDocGatekeeperImpl if (writeMarkerId) { this.#handoffDocWriteReceipt(actionId, writeMarkerId, pendingActions); try { - await api.deleteNamedRange(action.documentId, writeMarkerId); + await api.deleteNamedRange(documentId, writeMarkerId); this.#clearDocWriteReceipt(writeMarkerId); } catch (error) { logger.warn("failed to clean up Google Doc write marker", { @@ -1550,7 +1762,7 @@ export class GoogleDocGatekeeperImpl try { let refreshedSnapshot = snapshot; if (writeMarkerId) { - refreshedSnapshot = googleDocSnapshot(await api.getDocument(action.documentId)); + refreshedSnapshot = googleDocSnapshot(await api.getDocument(documentId)); } await this.ctx.storage.put(DOC_SNAPSHOT_KEY, refreshedSnapshot); invalidateUnreplayableGoogleDocActions( @@ -1571,15 +1783,38 @@ export class GoogleDocGatekeeperImpl let pending = pendingActions.list(); let index = pending.findIndex(({id}) => id === actionId); if (index === -1) { - throw new Error(`Unknown pending Google Doc action: ${actionId}`); + // Already settled by a decision whose reply was lost; see #applyAction. + if (!pendingActions.issued(actionId)) { + throw new Error(`Unknown Google Doc action: ${actionId}`); + } + logger.warn("re-rejecting an already-settled Google Doc action", { + event: "google.doc.action.reject.settled", actionId, + }); + return; } - let wasActive = !pending[index].action.invalidatedReason; + let rejected = pending[index].action; + let wasActive = !rejected.invalidatedReason; pendingActions.remove(actionId); this.#simulationCache.current = undefined; await this.ctx.storage.delete(DOC_SNAPSHOT_KEY); + if (rejected.type === "createDocument" && wasActive) { + // Rejecting the creation kills the binding: nothing the queued edits target will ever + // exist. Invalidate them all (the user still sees and clears their cards) and mark the + // binding dead so session methods explain instead of simulating against nothing. + this.ctx.storage.kv.put( + DOC_CREATION_REJECTED_KEY, "The user rejected creating this Google Doc."); + for (let other of pending) { + if (other.id !== actionId) { + invalidateGoogleDocAction( + pendingActions, other, + "The document creation was rejected, so this edit can never be applied."); + } + } + } + if (wasActive && index < pending.length - 1) { return {restart: true}; } @@ -1591,22 +1826,25 @@ export class GoogleDocGatekeeperImpl } /** - * Observer tracking — strategy B (ACL check, single unit). The binding is one document, so we just - * confirm the observer can open it with their own token (hasDocAccess, via the Drive/Docs ACL). - * The document is the atomic unit (everything read through this binding is that one doc), so no - * observers are tracked and removeObserver is a no-op. The overseer re-runs addObserver on every - * open, catching loss of access promptly. + * Observer tracking — strategy C over a single unit, the document itself. + * + * A binding minted against an existing document can always reach it, so the set is seeded and + * admission is the ACL check (hasDocAccess, via the Drive/Docs ACL) from the first open. A + * created document has no ACL to consult until it exists, so nothing is seeded and observers + * join unchecked; the first read of the real document then tracks it and forward-excludes + * whoever cannot reach it, which is what the unchecked admission owes. */ - async addObserver(_id: string, user: Fetcher): Promise { - let verifier = user as unknown as Fetcher; - if (!(await verifier.hasDocAccess(this.ctx.props.documentId))) { - throw new Error( - "This collaborator does not have access to the bound Google Doc, so they cannot be allowed " + - "to observe data this workspace read from it."); - } + get #observers(): ObserverTracker> { + return docObserverTracker(this.ctx.storage.kv, this.ctx.props.documentId); } - async removeObserver(_id: string): Promise {} + async addObserver(id: string, user: Fetcher): Promise { + await this.#observers.addObserver(id, user as unknown as Fetcher); + } + + async removeObserver(id: string): Promise { + this.#observers.removeObserver(id); + } } @validateRpc() @@ -1617,6 +1855,7 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { #pendingActions: PendingActionStore; #storage: DurableObjectStorage; #simulationCache: GoogleDocSimulationCacheHolder; + #creation?: {title: string}; constructor( docsApi: GoogleDocsApi, @@ -1625,6 +1864,7 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { pendingActions: PendingActionStore, storage: DurableObjectStorage, simulationCache: GoogleDocSimulationCacheHolder, + creation?: {title: string}, ) { super(); this.#docsApi = docsApi; @@ -1633,9 +1873,38 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { this.#pendingActions = pendingActions; this.#storage = storage; this.#simulationCache = simulationCache; + this.#creation = creation; + } + + /** + * The Google-issued documentId to use against the API, or undefined while a created document is + * still pending. Resolved per call, not at construction: the creation can be approved while + * this session is live. + */ + #apiDocumentId(): string | undefined { + let id = docProvisionalIds(this.#storage.kv).resolve(this.#documentId); + return isProvisionalDocId(id) ? undefined : id; + } + + /** Throws the dead-binding explanation once the user has rejected creating this document. */ + #checkCreationRejected(): void { + let reason = this.#storage.kv.get(DOC_CREATION_REJECTED_KEY); + if (reason) { + throw new Error( + `${reason} This binding will never work — ask the user how to proceed (they can ` + + `remove the connection, or you can create a new document).`); + } } async #getSnapshot(forceRefresh?: boolean): Promise { + let documentId = this.#apiDocumentId(); + if (documentId === undefined) { + // The document exists only locally. Simulate over an empty base; never stored under + // DOC_SNAPSHOT_KEY so a real fetch replaces it naturally once the creation is applied. + return emptyDocSnapshot( + this.#creation?.title ?? "Untitled document", PROVISIONAL_REVISION); + } + if (!forceRefresh) { let cached = await this.#storage.get(DOC_SNAPSHOT_KEY); if (cached) { @@ -1644,7 +1913,7 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { return cached; } // TTL expired — check if document has changed. - let currentRevisionId = await this.#docsApi.getRevisionId(this.#documentId); + let currentRevisionId = await this.#docsApi.getRevisionId(documentId); if (currentRevisionId === cached.revisionId) { cached.fetchedAt = Date.now(); await this.#storage.put(DOC_SNAPSHOT_KEY, cached); @@ -1654,7 +1923,7 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { } // Fetch full document and build snapshot. - let doc = await this.#docsApi.getDocument(this.#documentId); + let doc = await this.#docsApi.getDocument(documentId); let snapshot = googleDocSnapshot(doc); await this.#storage.put(DOC_SNAPSHOT_KEY, snapshot); return snapshot; @@ -1665,6 +1934,7 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { markdown: string, pendingActions: GoogleDocAction[], }> { + this.#checkCreationRejected(); let snapshot = await this.#getSnapshot(); let pending = this.#pendingActions.list(); let pendingFingerprint = googleDocPendingFingerprint(pending); @@ -1700,6 +1970,26 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { return {snapshot, markdown, pendingActions}; } + /** + * Authorize a read, recording the real document as data this binding has disclosed. + * + * A read of the uncreated document discloses only workspace-authored simulation, so it excludes + * nobody — and must not, since the overseer refuses an exclusion it cannot honour, which would + * block the read outright. + */ + async #authorizeRead(description: ObservationDescription): Promise { + let documentId = this.#apiDocumentId(); + if (documentId === undefined) { + await this.#approvalQueue.authorizeObservation(description); + return; + } + let check = await docObserverTracker(this.#storage.kv, this.#documentId) + .prepareObservation([documentId]); + await this.#approvalQueue.authorizeObservation( + {...description, excludeObservers: check.excludeObservers}); + check.commit(); + } + /** * Current title, and a modification time that only advances when something changed. * @@ -1709,23 +1999,30 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { * `getContent()` already shows them. */ async getMetadata(): Promise { - let metadata = await this.#docsApi.getDocumentMetadata(this.#documentId); - let revisedAt = this.#observeDocRevision(metadata.revisionId); + this.#checkCreationRejected(); + let documentId = this.#apiDocumentId(); let pendingActions = this.#pendingActions.list() .map(({action}) => action) .filter(action => !action.invalidatedReason); - await this.#approvalQueue.authorizeObservation({ + // While the document exists only locally its metadata is the creation parameters, dated by + // the pending actions (the creation itself is among them, so the reduce never yields 0). + let title = this.#creation?.title ?? "Untitled document"; + let revisedAt = 0; + if (documentId !== undefined) { + let metadata = await this.#docsApi.getDocumentMetadata(documentId); + title = metadata.title; + revisedAt = this.#observeDocRevision(metadata.revisionId); + } + + await this.#authorizeRead({ title: "Read Google Doc metadata", description: "Read the title and modification time of the document.", }); let lastModified = pendingActions.reduce( (latest, action) => Math.max(latest, action.submittedAt), revisedAt); - return { - title: metadata.title, - lastModified: new Date(lastModified), - }; + return {title, lastModified: new Date(lastModified)}; } /** @@ -1751,7 +2048,7 @@ class GoogleDocSessionImpl extends RpcTarget implements GoogleDocSession { async getContent(): Promise { let {markdown} = await this.#getSimulatedContent(); - await this.#approvalQueue.authorizeObservation({ + await this.#authorizeRead({ title: "Read Google Doc content", description: "Read the full simulated content of the document as Markdown.", }); diff --git a/packages/gatekeeper-google/src/markdown-converter.ts b/packages/gatekeeper-google/src/markdown-converter.ts index a6089ada39..8e032c5ace 100644 --- a/packages/gatekeeper-google/src/markdown-converter.ts +++ b/packages/gatekeeper-google/src/markdown-converter.ts @@ -61,6 +61,24 @@ export type Segment = // Google Docs → Markdown // --------------------------------------------------------------------------- +/** + * Snapshot of a document that exists only locally (a pending creation): empty content, no source + * map. Safe to simulate over because `sourceMap`/`bodyEndIndex` are consumed only when an action + * is materialized for the provider, which cannot happen before the document really exists. + * `bodyEndIndex` is 2 to match a genuinely empty Google Doc (one empty paragraph). + */ +export function emptyDocSnapshot(title: string, revisionId: string): DocSnapshot { + return { + title, + revisionId, + markdown: "", + sourceMap: { blocks: [] }, + fetchedAt: Date.now(), + committedWriteIds: [], + bodyEndIndex: 2, + }; +} + /** Convert a Google Docs document to Markdown with source map. */ export function docToMarkdown(document: GoogleDocsDocument): DocSnapshot { let md = ""; diff --git a/packages/gatekeeper-google/src/resources.ts b/packages/gatekeeper-google/src/resources.ts index 4c836d697f..05eb70d947 100644 --- a/packages/gatekeeper-google/src/resources.ts +++ b/packages/gatekeeper-google/src/resources.ts @@ -6,7 +6,7 @@ * `typeUrlPattern`s, and recorded grants. Never change one after deploy. */ -import type { SupportedResource } from "@gadgets/workshop-shared/gatekeeper"; +import type { ResourceCreationOptions, SupportedResource } from "@gadgets/workshop-shared/gatekeeper"; import type { CalendarAvailabilityMode } from "./calendar-types"; import { validateGmailLabelName, validateGmailQueryForGrouping } from "./gmail-validate"; @@ -38,8 +38,40 @@ export const GOOGLE_DOC_RESOURCE: SupportedResource = { title: "Google Doc", description: "Read and edit documents you choose.", grantable: true, + creatable: { + description: + "Creates a new, empty Google Doc with the given title in the account's My Drive. " + + "Accepts no creation options.", + }, }; +/** + * Prefix marking a document ID minted locally (by createResource) before the document exists at + * Google. Chosen to be visibly non-Google (real IDs are opaque base64-ish tokens) and stable: it + * appears in persisted resource URLs, so never change it. + */ +export const PROVISIONAL_DOC_ID_PREFIX = "provisional-"; + +/** Whether a document ID is a locally-minted provisional ID rather than a Google-issued one. */ +export function isProvisionalDocId(id: string): boolean { + return id.startsWith(PROVISIONAL_DOC_ID_PREFIX); +} + +/** + * The vendor half of the ResourceCreationOptions contract for a type that accepts no options: + * reject the whole map so a parameter the agent supplied — placement above all — is never + * silently dropped. The refusal quotes the type's own `creatable.description`, the surface that + * documents what creation does and which keys it takes. + */ +export function assertNoCreationOptions( + resource: SupportedResource, options?: ResourceCreationOptions): void { + let keys = Object.keys(options ?? {}); + if (keys.length === 0) return; + throw new Error( + `Creating a ${resource.title} accepts no options, but received: ${keys.join(", ")}. ` + + resource.creatable?.description); +} + /** A single Google Sheet. */ export const GOOGLE_SHEETS_RESOURCE: SupportedResource = { urlPattern: "https://docs.google.com/spreadsheets/d/:spreadsheetId/*", From 07dfdf2a53348c2e478749caa7878fc1b88c2c60 Mon Sep 17 00:00:00 2001 From: Nathan Disidore Date: Wed, 2 Sep 2026 16:00:23 -0500 Subject: [PATCH 7/7] Let users opt into auto-approved Google Doc creation. Document creation keeps its own action kind, separate from the edits one, so enabling hands-free creation is a distinct choice -- the case that wants it is a scheduled task minting a new doc each run, whose queued edits would otherwise stall in-order behind a creation card every time. Nothing is auto-applied without the user enabling the createDocument rule. --- packages/gatekeeper-google/__tests__/worker.ts | 4 ++++ .../__tests__/workerd/google-doc-actions.test.ts | 10 ++++++++++ packages/gatekeeper-google/src/google.ts | 7 ++++--- 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/gatekeeper-google/__tests__/worker.ts b/packages/gatekeeper-google/__tests__/worker.ts index 6cb362e465..ce9d159eba 100644 --- a/packages/gatekeeper-google/__tests__/worker.ts +++ b/packages/gatekeeper-google/__tests__/worker.ts @@ -161,6 +161,10 @@ export class TestHooks extends DurableObject { return this.#gatekeeper(facetName, creation).describe(); } + async autoApprovableActions(facetName: string, creation?: { title: string }) { + return this.#gatekeeper(facetName, creation).getAutoApprovableActions(); + } + async applyAction( facetName: string, actionId: number, creation?: { title: string }, ): Promise { diff --git a/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts b/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts index 79dbec57f4..d8f7727ef5 100644 --- a/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts +++ b/packages/gatekeeper-google/__tests__/workerd/google-doc-actions.test.ts @@ -345,6 +345,16 @@ describe("Google Doc write receipts", () => { describe("Google Doc creation (createExternalResource)", () => { const CREATION = { title: "My New Doc" }; + it("advertises creation as its own auto-approvable kind", async () => { + new DocsModel().install(); + // A separate tag from edits, so hands-free creation (a scheduled task minting a doc each + // day) is its own opt-in rule. + expect(await hooks().autoApprovableActions("create-kinds", CREATION)).toEqual([ + { tag: "editDocument", label: "Document edits" }, + { tag: "createDocument", label: "Document creation" }, + ]); + }); + it("simulates the uncreated document without touching the provider", async () => { let docs = new DocsModel(); docs.install(); diff --git a/packages/gatekeeper-google/src/google.ts b/packages/gatekeeper-google/src/google.ts index 1ede02b945..873b0f2664 100644 --- a/packages/gatekeeper-google/src/google.ts +++ b/packages/gatekeeper-google/src/google.ts @@ -1437,7 +1437,8 @@ const EDIT_DOCUMENT_ACTION: ActionKind = { label: "Document edits", }; -// Creating the document is deliberately its own kind, never auto-approvable. +// Creating the document is deliberately its own kind, so hands-free creation (e.g. a scheduled +// task minting a new doc each day) is a separate opt-in rule from the far more common edits one. const CREATE_DOCUMENT_ACTION: ActionKind = { tag: "createDocument", label: "Document creation", @@ -1594,7 +1595,7 @@ export class GoogleDocGatekeeperImpl } async getAutoApprovableActions(): Promise { - return [EDIT_DOCUMENT_ACTION]; + return [EDIT_DOCUMENT_ACTION, CREATE_DOCUMENT_ACTION]; } async submitCreationAction(approvalQueue: RpcStub): Promise { @@ -1622,7 +1623,7 @@ export class GoogleDocGatekeeperImpl `My Drive. Edits queued before approval apply to it afterward, in order.`, implementsRevert: false, actionKind: CREATE_DOCUMENT_ACTION, - autoApprovable: false, + autoApprovable: true, }); } catch (error) { pendingActions.remove(actionId);