From 2c0caf4d95783137bc83bdfa618425e64d02cb51 Mon Sep 17 00:00:00 2001 From: Volta Date: Tue, 22 Sep 2026 05:49:49 +0000 Subject: [PATCH 1/2] feat(manifest): expose list-builds, update-channel and delete-channel (task #167, #175) Three agent-login manifest actions for surfaces that had a worker endpoint but no action, so an agent had to fall back to the generic CLI escape hatch or could not do the job at all: list-builds GET /api/apps/{app_id}/builds (task #167 second half) update-channel PATCH /api/apps/{app_id}/channels/{channel_id} (task #175) delete-channel DELETE /api/apps/{app_id}/channels/{channel_id} (task #175) All three endpoints already exist and are unchanged; this only declares them. update-channel declares the SIX body fields handleUpdateChannel reads (name, bundle_id, password, git_url, enabled_product_types, metadata) and the "nothing to update" rejection. Declaring fewer would hide settable fields from callers. The actions use `parameters`, the key the Raft-side parser actually reads (slock packages/cli/src/commands/integration/manifest.ts:72,:297). `create-channel` uses a `params` key that the parser ignores, so its slug/name are not resolvable for invoke; that pre-existing defect is left alone here rather than changed in passing. They are registered in NEW_ACTIONS. That set gates the migration nudge prepended to every EXISTING action; without registration a brand-new action would ship labelled "Deprecated", which is exactly the wrong label. Registration says nothing about their stability. Tests assert each action is present, is not prefixed Deprecated, maps to the endpoint the worker serves, declares the fields the handler reads, and uses `parameters` rather than `params`. Mutation-checked: removing a name from NEW_ACTIONS fails the prefix assertion, and switching an action to `params` fails the key assertion. Signed-off-by: Volta --- worker/src/routes/auth.ts | 50 +++++++++++++++++++++++++- worker/test/agent_login_routes.test.ts | 28 +++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/worker/src/routes/auth.ts b/worker/src/routes/auth.ts index 4d872600..8713dcb2 100644 --- a/worker/src/routes/auth.ts +++ b/worker/src/routes/auth.ts @@ -973,7 +973,16 @@ export async function handleAgentManifest(c: Context<{ Bindings: Env }>) { // keep the machine contract unchanged, just point callers at `migration-help`. const service = c.env.RAFT_CLIENT_ID || "hands-4cc7a2"; const DEPRECATION_PREFIX = `Deprecated — run raft integration invoke --service ${service} --action migration-help for Hands CLI installation and migration guidance. `; - const NEW_ACTIONS = new Set(["agent-login", "migration-help"]); + const NEW_ACTIONS = new Set([ + "agent-login", + "migration-help", + // Added 2026-09 (#167 follow-up / #175). These are NEW actions, not pre-migration ones, so the + // migration nudge must not be prepended - otherwise a brand-new action ships labelled + // "Deprecated". Registered for that reason only; it says nothing about their stability. + "list-builds", + "update-channel", + "delete-channel", + ]); const applyDeprecation = ( list: Array<{ name: string; description: string; [k: string]: unknown }>, ) => list.map((a) => (NEW_ACTIONS.has(a.name) ? a : { ...a, description: DEPRECATION_PREFIX + a.description })); @@ -1195,6 +1204,32 @@ export async function handleAgentManifest(c: Context<{ Bindings: Env }>) { description: "Create a release channel on an app (channels are never auto-created by publish). Requires app admin. Creating a channel activates nothing.", }, + { + name: "update-channel", + description: + "Change a release channel's settings. Requires app admin. Fields omitted from the body are left unchanged; a body with none of them is rejected with \"nothing to update\".", + endpoint: { method: "PATCH", path: "/api/apps/{app_id}/channels/{channel_id}" }, + parameters: { + app_id: { type: "string", in: "path", required: true, description: "App UUID." }, + channel_id: { type: "string", in: "path", required: true, description: "Channel UUID." }, + name: { type: "string", in: "body", required: false, description: "New display name." }, + bundle_id: { type: "string", in: "body", required: false, description: "New bundle id; an empty string clears it." }, + password: { type: "string", in: "body", required: false, description: "New shared password; an empty string clears it." }, + git_url: { type: "string", in: "body", required: false, description: "New git url; an empty string clears it." }, + enabled_product_types: { type: "string[]", in: "body", required: false, description: "Replacement product-type allowlist; stored as JSON." }, + metadata: { type: "string", in: "body", required: false, description: "Opaque metadata; stored as JSON." }, + }, + }, + { + name: "delete-channel", + description: + "Delete a release channel. Requires app admin. Refused (not cascaded) while any build or release still references the channel.", + endpoint: { method: "DELETE", path: "/api/apps/{app_id}/channels/{channel_id}" }, + parameters: { + app_id: { type: "string", in: "path", required: true, description: "App UUID." }, + channel_id: { type: "string", in: "path", required: true, description: "Channel UUID." }, + }, + }, { name: "list-device-groups", description: "List app-scoped rollout device groups and their installation device ids. Requires app publisher.", @@ -1611,6 +1646,19 @@ export async function handleAgentManifest(c: Context<{ Bindings: Env }>) { bundle_id: { type: "string", in: "query", required: false, description: "Optional bundle-id assertion; it must match immutable build metadata, and is only a fallback when metadata is absent." }, }, }, + { + name: "list-builds", + description: + "List an app's builds, newest first, with the same product/channel/status filters the CLI uses.", + endpoint: { method: "GET", path: "/api/apps/{app_id}/builds" }, + parameters: { + app_id: { type: "string", in: "path", required: true, description: "App UUID." }, + product_type: { type: "string", in: "query", required: false, description: "Filter by product type (e.g. cli-binary)." }, + channel: { type: "string", in: "query", required: false, description: "Filter by channel id or slug." }, + status: { type: "string", in: "query", required: false, description: "Filter by build status." }, + version_name: { type: "string", in: "query", required: false, description: "Filter by version name." }, + }, + }, { name: "list-build-assets", description: diff --git a/worker/test/agent_login_routes.test.ts b/worker/test/agent_login_routes.test.ts index 678351d5..accca2d2 100644 --- a/worker/test/agent_login_routes.test.ts +++ b/worker/test/agent_login_routes.test.ts @@ -232,6 +232,34 @@ describe("manifest: actions retained + deprecated, migration-help added", () => expect(body.actions.find((a: any) => a.name === "migration-help").description.startsWith("Deprecated")).toBe(false); // machine contract unchanged: help still GET /api/agent/help expect(body.actions.find((a: any) => a.name === "help").endpoint).toEqual({ method: "GET", path: "/api/agent/help" }); + + // 2026-09 (#167 follow-up / #175): three actions added for surfaces that previously had no + // agent action. They are NEW, so the migration nudge must not label them deprecated. + for (const n of ["list-builds", "update-channel", "delete-channel"]) { + expect(names).toContain(n); + expect(body.actions.find((a: any) => a.name === n).description.startsWith("Deprecated")).toBe(false); + } + // Each maps to an endpoint the worker already serves, and declares the fields the handler reads. + const listBuilds = body.actions.find((a: any) => a.name === "list-builds"); + expect(listBuilds.endpoint).toEqual({ method: "GET", path: "/api/apps/{app_id}/builds" }); + expect(Object.keys(listBuilds.parameters).sort()).toEqual( + ["app_id", "channel", "product_type", "status", "version_name"].sort(), + ); + const updateChannel = body.actions.find((a: any) => a.name === "update-channel"); + expect(updateChannel.endpoint).toEqual({ method: "PATCH", path: "/api/apps/{app_id}/channels/{channel_id}" }); + // The handler reads six body fields; listing fewer silently hides settable fields. + expect(Object.keys(updateChannel.parameters).sort()).toEqual( + ["app_id", "bundle_id", "channel_id", "enabled_product_types", "git_url", "metadata", "name", "password"].sort(), + ); + const deleteChannel = body.actions.find((a: any) => a.name === "delete-channel"); + expect(deleteChannel.endpoint).toEqual({ method: "DELETE", path: "/api/apps/{app_id}/channels/{channel_id}" }); + // The Raft-side parser reads `parameters` only (a `params` key is ignored), so actions added + // here must use the key the parser actually reads. + for (const n of ["list-builds", "update-channel", "delete-channel"]) { + const a = body.actions.find((x: any) => x.name === n); + expect(a.parameters).toBeDefined(); + expect(a.params).toBeUndefined(); + } }); it("migration-help endpoint returns install + login guidance", async () => { From 2d24eed93394e42bc4b2b044cfd0c8a6f053941a Mon Sep 17 00:00:00 2001 From: Volta Date: Tue, 22 Sep 2026 06:07:57 +0000 Subject: [PATCH 2/2] fix(manifest): use the structured type labels invoke recognises (review of #545) CHANGES from @XX's contract review of 2c0caf4d: - `update-channel.enabled_product_types`: `string[]` -> `array`. `invoke` treats only `array` and `object` as structured types (slock commands/integration/invoke.ts:308-316), so `string[]` is not recognised and the value is lost. The handler reads `string[]` (channels.ts:81); the manifest label is a separate vocabulary. - `update-channel.metadata`: `string` -> `object`, same failure shape. The handler reads `Record` (channels.ts:82). - Tests assert those two labels, not just the field names. Verified red-first: reverting the labels fails with `expected 'string[]' to be 'array'`, so the assertion catches exactly this defect. Also, non-blocking review points: - `list-builds` description now states the viewer requirement (index.ts:834) and the 200 cap (builds.ts:629). - The `NEW_ACTIONS` comment is written as an explicit decision (provenance, not stability). worker: 655 passed (61 files). Signed-off-by: Volta --- worker/src/routes/auth.ts | 13 +++++++------ worker/test/agent_login_routes.test.ts | 7 +++++++ 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/worker/src/routes/auth.ts b/worker/src/routes/auth.ts index 8713dcb2..10f32705 100644 --- a/worker/src/routes/auth.ts +++ b/worker/src/routes/auth.ts @@ -976,9 +976,10 @@ export async function handleAgentManifest(c: Context<{ Bindings: Env }>) { const NEW_ACTIONS = new Set([ "agent-login", "migration-help", - // Added 2026-09 (#167 follow-up / #175). These are NEW actions, not pre-migration ones, so the - // migration nudge must not be prepended - otherwise a brand-new action ships labelled - // "Deprecated". Registered for that reason only; it says nothing about their stability. + // Decision (2026-09, #167 follow-up / #175): these three are newly added actions, so they are + // registered here deliberately. This set is what stops the migration nudge from being + // prepended; without it a brand-new action would ship labelled "Deprecated". Membership is a + // statement about provenance (added after the migration), not about stability or support. "list-builds", "update-channel", "delete-channel", @@ -1216,8 +1217,8 @@ export async function handleAgentManifest(c: Context<{ Bindings: Env }>) { bundle_id: { type: "string", in: "body", required: false, description: "New bundle id; an empty string clears it." }, password: { type: "string", in: "body", required: false, description: "New shared password; an empty string clears it." }, git_url: { type: "string", in: "body", required: false, description: "New git url; an empty string clears it." }, - enabled_product_types: { type: "string[]", in: "body", required: false, description: "Replacement product-type allowlist; stored as JSON." }, - metadata: { type: "string", in: "body", required: false, description: "Opaque metadata; stored as JSON." }, + enabled_product_types: { type: "array", in: "body", required: false, description: "Replacement product-type allowlist; stored as JSON." }, + metadata: { type: "object", in: "body", required: false, description: "Opaque metadata object; stored as JSON." }, }, }, { @@ -1649,7 +1650,7 @@ export async function handleAgentManifest(c: Context<{ Bindings: Env }>) { { name: "list-builds", description: - "List an app's builds, newest first, with the same product/channel/status filters the CLI uses.", + "List an app's builds, newest first, capped at 200, with the same product/channel/status filters the CLI uses. Requires app viewer.", endpoint: { method: "GET", path: "/api/apps/{app_id}/builds" }, parameters: { app_id: { type: "string", in: "path", required: true, description: "App UUID." }, diff --git a/worker/test/agent_login_routes.test.ts b/worker/test/agent_login_routes.test.ts index accca2d2..fa9bb316 100644 --- a/worker/test/agent_login_routes.test.ts +++ b/worker/test/agent_login_routes.test.ts @@ -260,6 +260,13 @@ describe("manifest: actions retained + deprecated, migration-help added", () => expect(a.parameters).toBeDefined(); expect(a.params).toBeUndefined(); } + + // Structured body fields must use the labels the Raft side recognises. `invoke` treats only + // `array` and `object` as structured types (slock commands/integration/invoke.ts:308-316), so a + // `string[]` or `string` label for a structured field silently loses the value. Assert the + // labels, not just the field names. + expect(updateChannel.parameters.enabled_product_types.type).toBe("array"); + expect(updateChannel.parameters.metadata.type).toBe("object"); }); it("migration-help endpoint returns install + login guidance", async () => {