diff --git a/src/registry/toolsets/templates.ts b/src/registry/toolsets/templates.ts index 0657a0789..49a6687d5 100644 --- a/src/registry/toolsets/templates.ts +++ b/src/registry/toolsets/templates.ts @@ -1,6 +1,111 @@ -import type { ToolsetDefinition } from "../types.js"; +import type { PathBuilderConfig, ToolsetDefinition } from "../types.js"; import { ngExtract, pageExtract } from "../extractors.js"; +function nonEmptyString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +function requiredString(input: Record, key: string, context: string): string { + const value = nonEmptyString(input[key]); + if (!value) { + throw new Error(`${key} is required for ${context}`); + } + return value; +} + +type TemplateScope = { + level: "account" | "org" | "project"; + org?: string; + project?: string; +}; + +function normalizeTemplateScopeLevel(value: unknown): TemplateScope["level"] | undefined { + if (value !== "account" && value !== "org" && value !== "project") { + return undefined; + } + return value; +} + +function resolveTemplateScope(input: Record, config: PathBuilderConfig): TemplateScope { + const explicitScope = normalizeTemplateScopeLevel(input.scope_level); + const orgInput = nonEmptyString(input.org_id); + const projectInput = nonEmptyString(input.project_id); + const isGlobalTemplate = input.global === true || input.global === "true" || input.account_id === "__GLOBAL_TEMPLATES_ACCOUNT_ID__"; + + if (explicitScope === "account" || (!explicitScope && isGlobalTemplate)) { + return { level: "account" }; + } + + if (explicitScope === "org") { + const org = orgInput ?? config.HARNESS_ORG; + if (!org) { + throw new Error("org_id or HARNESS_ORG is required for org-scoped template operations"); + } + return { level: "org", org }; + } + + if (explicitScope === "project") { + const org = orgInput ?? config.HARNESS_ORG; + const project = projectInput ?? config.HARNESS_PROJECT; + if (!org || !project) { + throw new Error("org_id/HARNESS_ORG and project_id/HARNESS_PROJECT are required for project-scoped template operations"); + } + return { level: "project", org, project }; + } + + if (projectInput) { + const org = orgInput ?? config.HARNESS_ORG; + if (!org) { + throw new Error("org_id or HARNESS_ORG is required when project_id is provided for template operations"); + } + return { level: "project", org, project: projectInput }; + } + + if (orgInput) { + return { level: "org", org: orgInput }; + } + + if (config.HARNESS_ORG && config.HARNESS_PROJECT) { + return { level: "project", org: config.HARNESS_ORG, project: config.HARNESS_PROJECT }; + } + + if (config.HARNESS_ORG) { + return { level: "org", org: config.HARNESS_ORG }; + } + + return { level: "account" }; +} + +function applyTemplateScope(input: Record, config: PathBuilderConfig): TemplateScope { + const scope = resolveTemplateScope(input, config); + + // Registry scope injection runs after pathBuilder. Keep only the resolved + // scope fields so account/org operations are not promoted by config defaults. + delete input.org_id; + delete input.project_id; + + if (scope.org) { + input.org_id = scope.org; + } + if (scope.project) { + input.project_id = scope.project; + } + + return scope; +} + +function templateV1ScopePath(input: Record, config: PathBuilderConfig): string { + const scope = applyTemplateScope(input, config); + + if (scope.level === "project") { + return `/v1/orgs/${encodeURIComponent(scope.org!)}/projects/${encodeURIComponent(scope.project!)}`; + } + if (scope.level === "org") { + return `/v1/orgs/${encodeURIComponent(scope.org!)}`; + } + return "/v1"; +} + export const templatesToolset: ToolsetDefinition = { name: "templates", displayName: "Templates", @@ -12,11 +117,13 @@ export const templatesToolset: ToolsetDefinition = { description: "Reusable template definition. Supports list, get, create, update, and delete.", toolset: "templates", scope: "project", + scopeOptional: true, identifierFields: ["template_id"], listFilterFields: [ { name: "search_term", description: "Filter templates by name or keyword" }, { name: "template_type", description: "Template entity type", enum: ["Pipeline", "Stage", "Step", "CustomDeployment", "MonitoredService", "SecretManager", "ArtifactSource"] }, { name: "template_list_type", description: "Template list type", enum: ["Stable", "LastUpdated", "All"] }, + { name: "scope_level", description: "Template scope to target: account, org, or project. Defaults to explicit org/project params, then configured defaults.", enum: ["account", "org", "project"] }, { name: "global", description: "When true, accesses global templates (list: passes isGlobal=true; get: forces accountIdentifier to __GLOBAL_TEMPLATES_ACCOUNT_ID__)", type: "boolean" }, { name: "metadata_only", description: "When true, fetches only template metadata (name, identifier, type, tags) via the list-metadata endpoint — faster and lighter than the full list", type: "boolean" }, ], @@ -25,10 +132,12 @@ export const templatesToolset: ToolsetDefinition = { list: { method: "POST", path: "/template/api/templates/list", - pathBuilder: (input) => - input.metadata_only || input.global + pathBuilder: (input, config) => { + applyTemplateScope(input, config); + return input.metadata_only || input.global ? "/template/api/templates/list-metadata" - : "/template/api/templates/list", + : "/template/api/templates/list"; + }, operationPolicy: { risk: "read", retryPolicy: "safe" }, queryParams: { search_term: "searchTerm", @@ -49,6 +158,11 @@ export const templatesToolset: ToolsetDefinition = { get: { method: "GET", path: "/template/api/templates/{templateIdentifier}", + pathBuilder: (input, config) => { + applyTemplateScope(input, config); + const template = requiredString(input, "template_id", "template get"); + return `/template/api/templates/${encodeURIComponent(template)}`; + }, operationPolicy: { risk: "read", retryPolicy: "safe" }, pathParams: { template_id: "templateIdentifier" }, queryParams: { @@ -62,7 +176,11 @@ export const templatesToolset: ToolsetDefinition = { method: "PUT", path: "/v1/orgs/{org}/projects/{project}/templates/{template}/versions/{version}", operationPolicy: { risk: "low_write", retryPolicy: "safe" }, - pathParams: { org_id: "org", project_id: "project", template_id: "template", version_label: "version" }, + pathBuilder: (input, config) => { + const template = requiredString(input, "template_id", "template update"); + const version = requiredString(input, "version_label", "template update"); + return `${templateV1ScopePath(input, config)}/templates/${encodeURIComponent(template)}/versions/${encodeURIComponent(version)}`; + }, bodyBuilder: (input) => { const b = (input.body as Record) ?? {}; const templateYaml = @@ -75,6 +193,11 @@ export const templatesToolset: ToolsetDefinition = { throw new Error("body.template_yaml (or body.yaml) is required: full template YAML string with your changes"); } const out: Record = { template_yaml: templateYaml }; + if (b.identifier !== undefined) out.identifier = b.identifier; + if (b.name !== undefined) out.name = b.name; + if (b.label !== undefined) out.label = b.label; + if (b.yaml_version !== undefined) out.yaml_version = b.yaml_version; + if (b.git_details !== undefined) out.git_details = b.git_details; if (b.is_stable !== undefined) out.is_stable = b.is_stable; if (b.comments !== undefined) out.comments = b.comments; return out; @@ -82,19 +205,24 @@ export const templatesToolset: ToolsetDefinition = { bodySchema: { description: "Template version update", fields: [ - { name: "template_yaml", type: "yaml", required: true, description: "Full template YAML string with changes" }, + { name: "template_yaml", type: "yaml", required: true, description: "Full template YAML string with changes (name, identifier, etc. are derived from the YAML)" }, + { name: "identifier", type: "string", required: false, description: "Template identifier (derived from YAML if omitted)" }, + { name: "name", type: "string", required: false, description: "Display name (derived from YAML if omitted)" }, + { name: "label", type: "string", required: false, description: "Version label (derived from YAML if omitted)" }, + { name: "yaml_version", type: "string", required: false, description: "YAML version (for example, '1')" }, + { name: "git_details", type: "object", required: false, description: "Git storage details (for example, { store_type: 'INLINE' })" }, { name: "is_stable", type: "boolean", required: false, description: "Mark as stable version" }, { name: "comments", type: "string", required: false, description: "Version update comments" }, ], }, responseExtractor: ngExtract, - description: "Update a template version. Provide full template_yaml (required). Optional: is_stable, comments.", + description: "Update a template version. Provide full template_yaml (required). Name, identifier, and other metadata are derived from the YAML content. Optional: is_stable, comments.", }, create: { method: "POST", path: "/v1/orgs/{org}/projects/{project}/templates", operationPolicy: { risk: "low_write", retryPolicy: "do_not_retry" }, - pathParams: { org_id: "org", project_id: "project" }, + pathBuilder: (input, config) => `${templateV1ScopePath(input, config)}/templates`, bodyBuilder: (input) => { const b = (input.body as Record) ?? {}; const templateYaml = @@ -141,13 +269,18 @@ export const templatesToolset: ToolsetDefinition = { }, delete: { method: "DELETE", - path: "/template/api/templates/{templateIdentifier}", + path: "/template/api/templates/{templateIdentifier}/{versionLabel}", + pathBuilder: (input, config) => { + applyTemplateScope(input, config); + const template = requiredString(input, "template_id", "template delete"); + const version = requiredString(input, "version_label", "template delete"); + return `/template/api/templates/${encodeURIComponent(template)}/${encodeURIComponent(version)}`; + }, operationPolicy: { risk: "destructive", retryPolicy: "do_not_retry" }, - pathParams: { template_id: "templateIdentifier" }, - queryParams: { version_label: "versionLabel" }, + pathParams: { template_id: "templateIdentifier", version_label: "versionLabel" }, responseExtractor: ngExtract, description: - "Delete a template. If version_label is provided, only that version is deleted. If omitted, all versions of the template are deleted.", + "Delete a specific version of a template. Both template_id and version_label are required.", }, }, }, diff --git a/tasks/todo.md b/tasks/todo.md index 080088206..61567a520 100644 --- a/tasks/todo.md +++ b/tasks/todo.md @@ -250,7 +250,7 @@ - [x] Trace hosted and local Harness base URL configuration - [x] Clarify hosted MCP vs local MCP Harness0 routing in docs and manifests - [x] Run docs verification and review the diff -- [ ] Commit, push, open PR, and reply in the Slack thread +- [x] Commit, push, open PR, and reply in the Slack thread ### Plan - Treat this as a documentation/configuration gap unless code evidence shows the server ignores `HARNESS_BASE_URL`. @@ -263,3 +263,22 @@ - Clarified in README that hosted `https://mcp.harness.io/mcp` is managed and cannot be pointed at Harness0 from Claude/Cursor/Cowork client config; Support must configure hosted MCP for that target environment. - Updated MCPB manifest descriptions so `HARNESS_BASE_URL` covers private SaaS hosts such as `https://harness0.harness.io`, not just self-managed installs. - Verified with `pnpm build` and `pnpm docs:check`. + +## Slack PR Review: Template Scoped Paths (2026-05-13) +- [x] Read the Slack thread and PR #179 context +- [x] Review template registry path/scope changes against dispatch behavior +- [x] Add focused registry regression tests +- [x] Implement scoped template create/update/delete fixes without regressing list/get default scope +- [x] Run focused tests and typecheck +- [x] Commit, push, open PR, and reply in the Slack thread + +### Plan +- Add tests in `tests/registry/registry.test.ts` that cover account/org/project v1 template create/update paths, version-specific delete path, update body pass-through fields, and existing default org/project scoping for template list. +- Resolve template scope explicitly for every operation so callers can target account/org/project scope without configured project defaults overriding their intent. +- Use path builders to apply the resolved query scope for list/get/delete, dynamic v1 paths for create/update, and `version_label` as the delete path segment. + +### Review +- Found that the template operations need account/org/project scope support, but explicit org/account intent must not be promoted to project scope when `HARNESS_PROJECT` is configured. +- Added explicit template scope resolution (`scope_level`, explicit ids, then configured defaults) and path builders so list/get/delete/create/update all apply the resolved scope consistently. +- Updated template delete to require `version_label` in the path and added pass-through support for optional v1 update metadata fields. +- Verified with `pnpm test tests/registry/registry.test.ts`, `pnpm test tests/registry/structural-validation.test.ts`, `pnpm typecheck`, and full `pnpm test`. diff --git a/tests/registry/registry.test.ts b/tests/registry/registry.test.ts index 4bfc0b3c1..bd13523be 100644 --- a/tests/registry/registry.test.ts +++ b/tests/registry/registry.test.ts @@ -545,6 +545,159 @@ describe("Registry", () => { }); }); + describe("template scoped operations", () => { + const templateYaml = "template:\n name: Test Template\n identifier: testTemplate\n"; + + it("keeps config default scope on template list", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { content: [] } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "list", {}); + + const call = mockRequest.mock.calls[0][0]; + expect(call.method).toBe("POST"); + expect(call.path).toBe("/template/api/templates/list"); + expect(call.params).toMatchObject({ + orgIdentifier: "default", + projectIdentifier: "test-project", + }); + }); + + it("supports org-scoped template list when project defaults are configured", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { content: [] } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "list", { + scope_level: "org", + org_id: "org-only", + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.params.orgIdentifier).toBe("org-only"); + expect(call.params).not.toHaveProperty("projectIdentifier"); + }); + + it("supports account-scoped template get when scope defaults are configured", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { identifier: "testTemplate" } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "get", { + scope_level: "account", + template_id: "testTemplate", + version_label: "v2", + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.path).toBe("/template/api/templates/testTemplate"); + expect(call.params.versionLabel).toBe("v2"); + expect(call.params).not.toHaveProperty("orgIdentifier"); + expect(call.params).not.toHaveProperty("projectIdentifier"); + }); + + it("uses org-scoped v1 path for template update when project defaults are configured", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { identifier: "testTemplate" } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "update", { + scope_level: "org", + org_id: "org-only", + template_id: "testTemplate", + version_label: "v2", + body: { template_yaml: templateYaml }, + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.method).toBe("PUT"); + expect(call.path).toBe("/v1/orgs/org-only/templates/testTemplate/versions/v2"); + }); + + it("uses account-scoped v1 path for template create when scope defaults are configured", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { identifier: "testTemplate" } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "create", { + scope_level: "account", + body: { + template_yaml: templateYaml, + identifier: "testTemplate", + name: "Test Template", + }, + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.method).toBe("POST"); + expect(call.path).toBe("/v1/templates"); + }); + + it("supports org-scoped template delete when project defaults are configured", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { deleted: true } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "delete", { + scope_level: "org", + org_id: "org-only", + template_id: "testTemplate", + version_label: "v2", + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.path).toBe("/template/api/templates/testTemplate/v2"); + expect(call.params.orgIdentifier).toBe("org-only"); + expect(call.params).not.toHaveProperty("projectIdentifier"); + }); + + it("passes optional v1 template update body fields through", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { identifier: "testTemplate" } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "update", { + template_id: "testTemplate", + version_label: "v2", + body: { + template_yaml: templateYaml, + identifier: "testTemplate", + name: "Test Template", + label: "v2", + yaml_version: "1", + git_details: { store_type: "INLINE" }, + }, + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.body).toMatchObject({ + template_yaml: templateYaml, + identifier: "testTemplate", + name: "Test Template", + label: "v2", + yaml_version: "1", + git_details: { store_type: "INLINE" }, + }); + }); + + it("uses version label as a template delete path parameter", async () => { + const mockRequest = vi.fn().mockResolvedValue({ data: { deleted: true } }); + const client = makeClient(mockRequest); + const registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" })); + + await registry.dispatch(client, "template", "delete", { + template_id: "testTemplate", + version_label: "v2", + }); + + const call = mockRequest.mock.calls[0][0]; + expect(call.method).toBe("DELETE"); + expect(call.path).toBe("/template/api/templates/testTemplate/v2"); + expect(call.params).not.toHaveProperty("versionLabel"); + }); + }); + describe("cost category create — account body injection", () => { let registry: Registry;