fix: support scoped template v1 paths#183
Conversation
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
|
|
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
There was a problem hiding this comment.
Stale comment
Found 1 blocking issue.
templateV1ScopePathstill re-applies default project/org scope for v1 template writes, so explicit broader-scope create/update calls are not actually supported under the normal MCP configuration whereHARNESS_ORGandHARNESS_PROJECTare set. I reproduced this on the built branch:registry.dispatch(..., 'template', 'update', { org_id: 'org-only', ... })emittedPUT /v1/orgs/org-only/projects/test-project/templates/testTemplate/versions/v2and carriedprojectIdentifier: "test-project"in bothparamsandbody. That means org/account-scoped writes only work when defaults are unset, which is narrower than the PR description claims.Open question:
- Should explicit
org_idwith omittedproject_idsuppress default project fallback for v1 writes, or do create/update need a per-operation way to opt out of the registry's project-scope param/body injection?Verification:
pnpm test tests/registry/registry.test.tspnpm test tests/registry/structural-validation.test.tspnpm typecheckpnpm buildSent by Cursor Automation: Sunil On Demand Architecture Review
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
There was a problem hiding this comment.
Stale comment
Important:
src/registry/toolsets/templates.ts,src/tools/harness-describe.ts,src/tools/harness-create.ts,src/tools/harness-update.ts,src/tools/harness-delete.ts,tasks/todo.md: runtime now supports account/org/project template writes viascopeOptional/scope_level, but the agent-facing contract still exposestemplateasscope: "project"and the generic write/delete tools do not advertisescope_level(or templateversion_label) as first-class inputs. With defaults configured, account-scoped writes only work if callers already know to tunnelparams.scope_level="account"through the generic params bag, so the new task/review note overstates end-to-end support.tests/registry/registry.test.ts,tests/tools/tool-handlers.test.ts: the new regression coverage stops atregistry.dispatch(). The shipped behavior goes throughharness_create/harness_update/harness_delete, which still need to mapresource_id -> template_id, default or forwardversion_label, and carryscope_levelthroughparams. Without at least one tool-handler regression on those public paths, this PR proves helper wiring, not the real agent-facing behavior.Sent by Cursor Automation: Sunil On Demand Architecture Review
There was a problem hiding this comment.
Stale comment
Reviewed against Sunil's architecture standards.
Findings:
src/registry/toolsets/templates.tsnow fixes the dispatcher/path behavior for account/org/project template operations, but the agent-facing contract is still incomplete.harness_describe(resource_type="template")still presents templates asscope: "project", while the shared write tools only expose a genericparamsbag. That means the new account/orgcreate/update/deletepaths, plus delete's requiredversion_label, are still not discoverable in the structured metadata surface that this repo expects agents to use.tests/registry/registry.test.tsadds good dispatch-level regressions, but the public bug report was about generic template operations. There is still no tool-handler coverage provingharness_create/harness_update/harness_deleteactually preserveparams.scope_levelandparams.version_labelthrough the shared tool layer.Assumption:
- I reviewed this as a user-facing generic-tool fix, not just an internal registry refactor, because the PR title/body claim scoped template support rather than a helper-only cleanup.
Change summary:
- The earlier blocking issue where config defaults re-narrowed org/account-scoped v1 template writes looks fixed in the current branch.
Verification:
pnpm install --frozen-lockfilepnpm test -- tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts tests/registry/structural-validation.test.ts(52files /1202tests passed)pnpm typecheckpnpm buildgh pr checks 183Sent by Cursor Automation: Sunil On Demand Architecture Review
There was a problem hiding this comment.
Found 3 findings in src/registry/toolsets/templates.ts.
- Important:
global=truenow widenstemplate.listto account scope and drops the configured org/project filters. - Important: invalid
scope_levelvalues silently fall back to configured defaults, which can mis-target writes instead of failing loudly. - Warning: the new update schema advertises
labelas a version label, but the publicharness_updatepath still routes missingversion_labelto/versions/v1.
Open questions / assumptions:
- I'm assuming
global=trueontemplate.listis meant to include global templates alongside the active scope, matching the current description and the pre-PR behavior. - If template updates are still required to pass
params.version_label, the updated body schema should say that explicitly instead of surfacinglabelas the version selector.
Change summary:
- The earlier scoped-write blocker is fixed on the live head. I verified that org-scoped template updates now emit
/v1/orgs/<org>/templates/...withoutprojectIdentifier.
Verification:
pnpm test tests/registry/registry.test.ts tests/tools/tool-handlers.test.tspnpm typecheckpnpm build- runtime probes against
build/fortemplate.list(global=true), invalidscope_level, andharness_updateversion routing
Sent by Cursor Automation: Sunil On Demand Architecture Review
| 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)) { |
There was a problem hiding this comment.
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.
| }; | ||
|
|
||
| function normalizeTemplateScopeLevel(value: unknown): TemplateScope["level"] | undefined { | ||
| if (value !== "account" && value !== "org" && value !== "project") { |
There was a problem hiding this comment.
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".
| { 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)" }, |
There was a problem hiding this comment.
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.


Description
Fixes template create/update/delete registry behavior for the v1 template API. Create/update now build account-, org-, or project-scoped v1 paths per operation, while preserving configured default org/project scoping for existing template list/get/delete reads. Template delete now sends
version_labelas the required path segment, and template update passes through optional v1 metadata fields.Type of Change
Checklist
Verification:
pnpm test tests/registry/registry.test.tspnpm test tests/registry/structural-validation.test.tspnpm typecheckpnpm test