Fix template updates to require explicit version labels

src/tools/harness-update.ts

@@ -70,11 +70,15 @@ export function registerUpdateTool(server: McpServer, registry: Registry, client
           input[primaryField] = args.resource_id;
         }
         const versionLabel = asString(input.version_label);
+        const bodyRecord = isRecord(args.body) ? args.body : undefined;
+        const bodyVersionLabel = bodyRecord
+          ? asString(bodyRecord.version_label) ?? asString(bodyRecord.versionLabel) ?? asString(bodyRecord.label)
+          : undefined;
         if (versionLabel) { /* already set via params */ }
-        else if (isRecord(args.body) && "version_label" in args.body) {
-          input.version_label = args.body.version_label;
+        else if (bodyVersionLabel) {
+          input.version_label = bodyVersionLabel;
         } else if (args.resource_type === "template") {
-          input.version_label = "v1";
+          return errorResult("version_label is required for template update. Pass params.version_label or body.label to identify the template version to update.");
         }
 
         const result = await registry.dispatch(client, args.resource_type, "update", input, { tool: "harness_update", confirmation: elicit.method, resource_id: args.resource_id });

tasks/todo.md

@@ -340,3 +340,22 @@
 - Split query strings out of `RequestOptions.path` before base-path de-duplication and query assembly.
 - Merged path query params into the generated `URLSearchParams` before applying `options.params`, preserving explicit override behavior.
 - Verified with `pnpm test tests/client/harness-client.test.ts`, `pnpm typecheck`, `pnpm build`, and full `pnpm test`.
+
+## Critical Bug Inspection (2026-05-14)
+- [x] Inspect recent commits for high-severity behavioral regressions
+- [x] Reproduce template update wrong-version fallback
+- [x] Add regression coverage that template updates require an explicit version label
+- [x] Implement minimal fix in the template update handler
+- [x] Run focused tests, typecheck, build, and relevant full tests
+- [x] Commit, push, open PR, and post Slack summary
+
+### Plan
+- Focus the fix on `src/tools/harness-update.ts`, where the MCP handler currently defaults template updates to `version_label="v1"` before dispatch.
+- Add tool-level regression tests in `tests/tools/tool-handlers.test.ts` because the registry already rejects missing `version_label`; the bug is the handler masking that validation with a silent default.
+- Preserve successful template updates when `params.version_label`, `body.version_label`, or body version aliases identify the intended template version.
+
+### Review
+- Found that `harness_update(resource_type="template")` silently filled missing `version_label` with `v1`, so a caller editing another version such as `stable` could mutate `v1` instead of failing loudly.
+- Removed the silent `v1` fallback and resolve the update version only from `params.version_label`, `body.version_label`, `body.versionLabel`, or `body.label`.
+- Added regression coverage for the missing-version error path and for using `body.label` as the intended version.
+- Verified with `pnpm test tests/tools/tool-handlers.test.ts`, `pnpm typecheck`, `pnpm build`, and full `pnpm test`.

tests/tools/tool-handlers.test.ts

@@ -641,6 +641,52 @@ describe("harness_update", () => {
       identifier: "proj1",
     });
   });
+
+  it("rejects template updates without an explicit version label", async () => {
+    registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" }));
+    mockRequest = vi.fn().mockResolvedValue({ data: { identifier: "stepTemplate" } });
+    client = makeClient(mockRequest);
+    const templateServer = makeMcpServer("accept");
+    const { registerUpdateTool } = await import("../../src/tools/harness-update.js");
+    registerUpdateTool(templateServer, registry, client);
+
+    const result = await templateServer.call("harness_update", {
+      resource_type: "template",
+      resource_id: "stepTemplate",
+      body: {
+        template_yaml: "template:\n  name: Step Template\n  identifier: stepTemplate\n",
+      },
+    });
+
+    expect(result.isError).toBe(true);
+    expect(parseResult(result)).toMatchObject({
+      error: expect.stringContaining("version_label is required for template update"),
+    });
+    expect(mockRequest).not.toHaveBeenCalled();
+  });
+
+  it("uses body.label as the template update version when params.version_label is omitted", async () => {
+    registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" }));
+    mockRequest = vi.fn().mockResolvedValue({ data: { identifier: "stepTemplate" } });
+    client = makeClient(mockRequest);
+    const templateServer = makeMcpServer("accept");
+    const { registerUpdateTool } = await import("../../src/tools/harness-update.js");
+    registerUpdateTool(templateServer, registry, client);
+
+    const result = await templateServer.call("harness_update", {
+      resource_type: "template",
+      resource_id: "stepTemplate",
+      body: {
+        label: "stable",
+        template_yaml: "template:\n  name: Step Template\n  identifier: stepTemplate\n",
+      },
+    });
+
+    expect(result.isError).toBeUndefined();
+    const callArgs = mockRequest.mock.calls[0]![0] as { path: string; body: Record<string, unknown> };
+    expect(callArgs.path).toBe("/v1/templates/stepTemplate/versions/stable");
+    expect(callArgs.body.label).toBe("stable");
+  });
 });
 
 describe("harness_delete", () => {