Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
157 changes: 145 additions & 12 deletions src/registry/toolsets/templates.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>, 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") {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This helper silently ignores invalid scope_level values and falls back to configured defaults. Since scope_level reaches the registry through the generic tool params / filters records, a typo like "acount" on create / update / delete can route the write to the default project instead of failing locally.

Current-head reproduction after pnpm build:

{"path":"/template/api/templates/testTemplate/v2","params":{"orgIdentifier":"default","projectIdentifier":"test-project"}}

from registry.dispatch(..., "template", "delete", { scope_level: "acount", template_id: "testTemplate", version_label: "v2" }).

Given the repo's fail-loud guidance, I'd reject unknown scope selectors here instead of treating them as "use defaults".

return undefined;
}
return value;
}

function resolveTemplateScope(input: Record<string, unknown>, 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)) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Treating global=true as account scope for every template operation regresses template.list semantics. Because applyTemplateScope() clears org_id / project_id and the resource is now scopeOptional, a normal scoped list call like harness_list(resource_type="template", global=true) drops the configured orgIdentifier / projectIdentifier entirely instead of "including global templates" in the active scope.

I reproduced it on this head after pnpm build with registry.dispatch(..., "template", "list", { global: true }), which emitted:

{"path":"/template/api/templates/list-metadata","params":{"isGlobal":true}}

That is a behavior change from origin/main, where global=true only added isGlobal=true and preserved the default project/org scope.

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<string, unknown>, 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<string, unknown>, 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",
Expand All @@ -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,
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
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" },
],
Expand All @@ -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",
Expand All @@ -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: {
Expand All @@ -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<string, unknown>) ?? {};
const templateYaml =
Expand All @@ -75,26 +193,36 @@ 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<string, unknown> = { 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;
},
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)" },
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new update schema now surfaces label as the version label, but the public harness_update path still defaults missing version_label to v1 in src/tools/harness-update.ts. That creates a metadata/runtime contract mismatch for agents using harness_describe.

Current-head reproduction after pnpm build:

{"path":"/v1/orgs/default/projects/test-project/templates/testTemplate/versions/v1","body":{"label":"v2"}}

from harness_update(resource_type="template", resource_id="testTemplate", body={ template_yaml: ..., label: "v2" }).

If update calls still must provide params.version_label, I think this schema should say that explicitly instead of implying label drives version selection.

{ 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<string, unknown>) ?? {};
const templateYaml =
Expand Down Expand Up @@ -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.",
},
},
},
Expand Down
21 changes: 20 additions & 1 deletion tasks/todo.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand All @@ -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`.
Loading
Loading