Skip to content

Commit d8a70af

Browse files
ndisidorekentonv
authored andcommitted
Pass immutable hook target identifiers to enable()
1 parent 2f3549a commit d8a70af

6 files changed

Lines changed: 102 additions & 7 deletions

File tree

.agents/skills/write-gatekeeper/SKELETON.md

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
Gatekeeper,
1414
HookController, // Remove if no hooks
1515
HookInitiator, // Remove if no hooks
16+
HookTargetMetadata, // Remove if no hooks
1617
ResourceDescription,
1718
ApprovalQueue,
1819
ObservationDescription,
@@ -337,8 +338,9 @@ class MyConfiguratorUI extends RpcTarget {
337338
}
338339

339340
// ---------------------------------------------------------------------------
340-
// Hook type — remove this section (and HookController/HookInitiator imports, the `subscribe`
341-
// method, the `hookTsType` field, and MyHookControllerImpl) if the gatekeeper doesn't push events.
341+
// Hook type — remove this section (and the HookController/HookInitiator/HookTargetMetadata
342+
// imports, the `subscribe` method, the `hookTsType` field, and MyHookControllerImpl) if the
343+
// gatekeeper doesn't push events.
342344
//
343345
// The hook interface from types.d.ts is implemented by the Gadget as an RpcTarget. Intersect it
344346
// with RpcTarget so it satisfies the HookController/HookInitiator generic constraints (Hook
@@ -443,7 +445,11 @@ export class MyHookControllerImpl extends WorkerEntrypoint<Env, MyHookController
443445
// Called when the user enables the hook. Store `initiator` somewhere it can be reached when an
444446
// event arrives — typically an event-source DO. Don't store other state until now; everything
445447
// else is already in `this.ctx.props`. If already enabled, replace the previous initiator.
446-
async enable(initiator: Fetcher<HookInitiator<MyHook>>): Promise<void> {
448+
//
449+
// `target` identifies where the hook delivers (workspace, and gadget when pinned to one). Store
450+
// it too if you display or link to the target; otherwise ignore it, but keep the parameter
451+
// declared — RPC argument validation rejects arguments the receiver doesn't declare.
452+
async enable(initiator: Fetcher<HookInitiator<MyHook>>, target: HookTargetMetadata): Promise<void> {
447453
// TODO: persist `initiator` (e.g. forward it to an event-source DO keyed by props).
448454
}
449455

.agents/skills/write-gatekeeper/SKILL.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ Some services can push events to the Gadget (inbound email, webhooks, chat messa
303303
### Lifecycle
304304

305305
1. **Register.** The Gadget calls your Session method (e.g. `subscribe(callback, filter)`). Inside it, construct a `HookController` whose `props` capture the specifics of *this* registration, then call `approvalQueue.bindHook(controller, callback, description)`. The overseer stores the callback and records the hook (initially **disabled**). Do **not** store the callback yourself — it is bound to the current session and would be revoked when the session ends.
306-
2. **Enable.** When the user approves the hook in the Workshop UI, the overseer calls `controller.enable(initiator)`. Store the `initiator` Fetcher somewhere it can be reached when events arrive (e.g. an event-source DO). Avoid storing any other state until enabled; everything else should already be in the controller's `props`.
306+
2. **Enable.** When the user approves the hook in the Workshop UI, the overseer calls `controller.enable(initiator, target)`. Store the `initiator` Fetcher somewhere it can be reached when events arrive (e.g. an event-source DO). `target` identifies where the hook delivers (workspace, plus gadget when the hook is pinned to one); persist it alongside the initiator if you display or link to the target — the IDs are fixed when the hook is bound, so there is nothing to refresh. A gatekeeper that doesn't need it still has to declare the parameter, since RPC argument validation rejects arguments the receiver doesn't declare. Avoid storing any other state until enabled; everything else should already be in the controller's `props`.
307307
3. **Deliver.** When the event occurs, call `initiator.startHook()`. This returns `{callback, approvalQueue}` bound to a fresh session. Call `authorizeObservation()` (a hook event is almost always an observation; register actions too if the callback's return value triggers side effects), then invoke the `callback` to deliver the event to the Gadget.
308308
4. **Disable / delete.** The overseer calls `controller.disable()`. Forget the stored `initiator` and clean up all related state — `disable()` may never be called again, though the overseer may later call `enable()` afresh.
309309

packages/gatekeeper-email/src/email.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
Gatekeeper,
88
HookController,
99
HookInitiator,
10+
HookTargetMetadata,
1011
ResourceDescription,
1112
ApprovalQueue,
1213
VendorDescription,
@@ -581,7 +582,10 @@ export class EmailGatekeeperImpl extends DurableObject<Env, EmailGatekeeperImplP
581582
@validateRpc()
582583
export class EmailHookControllerImpl extends WorkerEntrypoint<Env, EmailGatekeeperImplProps>
583584
implements HookController<EmailHookTarget> {
584-
async enable(initiator: Fetcher<HookInitiator<EmailHookTarget>>): Promise<void> {
585+
// `_target` is unused -- email doesn't display its hooks -- but must be declared, since RPC
586+
// argument validation is generated from this signature and would reject the extra argument.
587+
async enable(initiator: Fetcher<HookInitiator<EmailHookTarget>>,
588+
_target: HookTargetMetadata): Promise<void> {
585589
return this.#setHook(initiator);
586590
}
587591

packages/workshop-backend/__tests__/overseer-hooks.test.ts

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { describe, expect, it, vi } from "vitest";
2+
import { RpcStub as NativeRpcStub } from "cloudflare:workers";
23
import { DEFAULT_ADMIN_CONFIG, serializeAdminConfig } from "../src/admin-config.js";
34
import { OverseerDurableObject } from "../src/overseer.js";
45

@@ -87,3 +88,63 @@ describe("OverseerDurableObject.startHook", () => {
8788
await expect(overseer.startHook(1)).rejects.toThrow("Hook has been deleted or disabled.");
8889
});
8990
});
91+
92+
async function makeTargetOverseer(gadgetId?: number) {
93+
let controllerEnable = vi.fn(async (_initiator: object, _target: object) => {});
94+
let record = {
95+
id: 4,
96+
actionId: 12,
97+
gatekeeperId: 1,
98+
gadgetId,
99+
controller: {enable: controllerEnable},
100+
callback: {},
101+
description: {title: "Incoming email", description: "Receives email"},
102+
enabled: false,
103+
};
104+
let overseer = {
105+
open: OverseerDurableObject.prototype.open,
106+
impl: {
107+
ownerId: "user-id",
108+
ensureAmbientCapsules: async () => {},
109+
joinPresence: () => () => {},
110+
users: {
111+
idFromString: (id: string) => id,
112+
get: () => ({
113+
whoami: async () => ({id: "profile-id", name: "Test User"}),
114+
}),
115+
},
116+
ctx: {
117+
id: {toString: () => "workspace-id"},
118+
exports: {GatekeeperHookLoopback: ({props}: {props: object}) => props},
119+
},
120+
storage: {
121+
prohibitAllSharing: {get: () => false},
122+
boundHooks: {get: () => record, put: vi.fn()},
123+
actions: {get: () => undefined, put: vi.fn()},
124+
},
125+
},
126+
} satisfies Pick<OverseerDurableObject, "open"> & {impl: object};
127+
let notifyClosed = new NativeRpcStub<() => void>(() => {});
128+
let client = await overseer.open("user-id", "profile-id", notifyClosed);
129+
return {client, controllerEnable};
130+
}
131+
132+
describe("hook target", () => {
133+
134+
it("passes the workspace and gadget IDs to enable()", async () => {
135+
let {client, controllerEnable} = await makeTargetOverseer(17);
136+
137+
await client.enableHook(4);
138+
139+
expect(controllerEnable).toHaveBeenCalledTimes(1);
140+
expect(controllerEnable.mock.calls[0][1]).toEqual({workspaceId: "workspace-id", gadgetId: 17});
141+
});
142+
143+
it("omits the gadget ID for a hook that is not pinned to one", async () => {
144+
let {client, controllerEnable} = await makeTargetOverseer();
145+
146+
await client.enableHook(4);
147+
148+
expect(controllerEnable.mock.calls[0][1]).toEqual({workspaceId: "workspace-id"});
149+
});
150+
});

packages/workshop-backend/src/overseer.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7082,7 +7082,11 @@ class OverseerClientInterface extends RpcTarget implements Overseer {
70827082

70837083
await record.controller.enable(
70847084
this.impl.ctx.exports.GatekeeperHookLoopback({props}) as unknown as
7085-
Fetcher<HookInitiator<RpcTarget>>);
7085+
Fetcher<HookInitiator<RpcTarget>>,
7086+
{
7087+
workspaceId: this.impl.ctx.id.toString(),
7088+
...(record.gadgetId !== undefined ? {gadgetId: record.gadgetId} : {}),
7089+
});
70867090

70877091
record.enabled = true;
70887092
this.impl.storage.boundHooks.put(record);

packages/workshop-shared/src/gatekeeper.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1051,14 +1051,34 @@ export type HookDescription = {
10511051
description: string;
10521052
}
10531053

1054+
// Identifies where a hook delivers its events, for display and navigation by a gatekeeper that
1055+
// surfaces its hooks in a UI. Passed to `HookController.enable()`.
1056+
//
1057+
// Both fields are fixed when the hook is bound, so a gatekeeper may persist them alongside the
1058+
// initiator and never needs to refresh them. They are opaque display/routing identifiers: they
1059+
// must not be used for authorization, identity, or storage scoping.
1060+
export type HookTargetMetadata = {
1061+
// The workspace the hook delivers into.
1062+
workspaceId: string;
1063+
1064+
// The specific gadget within that workspace, when the hook is pinned to one. Absent means the
1065+
// workspace's current default gadget.
1066+
gadgetId?: number;
1067+
}
1068+
10541069
// Object passed to `ApprovalQueue.bindHook()`, providing the overseer with callbacks to enable
10551070
// or disable a hook.
10561071
export interface HookController<Hook extends RpcTarget> extends WorkerEntrypoint {
10571072
// Called to enable this hook. When a hook event is to be delivered, initiator.startHook() must
10581073
// be called first, before actually invoking the hook.
10591074
//
10601075
// If the hook was already enabled, the previously-registered `initiator` should be replaced.
1061-
enable(initiator: Fetcher<HookInitiator<Hook>>): Promise<void>;
1076+
//
1077+
// `target` identifies where the hook delivers, for gatekeepers that display or link to it. A
1078+
// gatekeeper that doesn't need it may ignore the value, but must still *declare* the parameter:
1079+
// RPC argument validation is generated from the declared signature, and a call carrying an
1080+
// argument the receiver does not declare is rejected.
1081+
enable(initiator: Fetcher<HookInitiator<Hook>>, target: HookTargetMetadata): Promise<void>;
10621082

10631083
// Unregister the hook, so that future events stop being delivered. The gatekeeper should forget
10641084
// the `initiator` previously registered by `enable()`.

0 commit comments

Comments
 (0)