fix: preserve default scope for template operations#187
Conversation
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
|
|
Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Found 3 issues against Sunil's architecture standards.
- The new account-scope validation does not protect the public
harness_get(..., params: { global: true })path, so a caller can still send mixed global + org scope. scopeOptionalDefaultFromConfigstill forwards an implicit project-scoped template request with onlyproject_idwhen no org can be resolved, instead of failing locally.- The new multi-scope template behavior is not reflected in
harness_describe, so the agent-facing contract is now behind the runtime behavior.
Assumptions:
- I am treating Sunil's standards as requiring fail-loud scope validation and metadata/runtime parity for agent-discoverable behavior.
Verification:
pnpm buildpnpm typecheckpnpm test -- tests/registry/registry.test.ts -t template(local run passed; Vitest reported 53 files / 1276 tests)- Reproduced the two scope gaps with small built-node probes on
180e6cfa1dce1823c45a07b21e5fc1ea8d008b3ebefore posting this review.
Sent by Cursor Automation: Sunil On Demand Architecture Review
| accountIdentifier: "__GLOBAL_TEMPLATES_ACCOUNT_ID__", | ||
| versionLabel: "1.0.8", | ||
| }); | ||
| expect(call.params).not.toHaveProperty("orgIdentifier"); |
There was a problem hiding this comment.
This only exercises the internal account_id path. The public harness_get flow still rewrites params.global=true into account_id after dropping global, so dispatch never sees the account-scope signal and will happily keep an explicit or URL-derived org_id.
I reproduced that on the built branch with harness_get({ resource_type: "template", resource_id: "helmDeployAction", org_id: "default", params: { global: true, version_label: "1.0.8" } }), and the request params came out as:
{
"accountIdentifier": "__GLOBAL_TEMPLATES_ACCOUNT_ID__",
"orgIdentifier": "default",
"versionLabel": "1.0.8"
}Please add coverage through the shared tool path or preserve a scope marker so the new validation actually protects the user-facing API.
| if (input.org_id) { | ||
| params[orgParam] = input.org_id as string; | ||
| } | ||
| if (input.project_id) { |
There was a problem hiding this comment.
There is still a fail-loud gap on the implicit project-scope path. When scopeOptionalDefaultFromConfig is enabled and the caller provides project_id but no org_id, this branch only backfills the org when HARNESS_ORG is set. If it is unset, we still fall through and forward a partial request.
I reproduced registry.dispatch(client, "template", "delete", { template_id: "deploy_step", version_label: "v2", project_id: "proj-only" }) with HARNESS_ORG unset, and the request params were just:
{ "projectIdentifier": "proj-only" }Since the explicit scope: "project" branch already rejects missing orgs, this implicit path should fail the same way instead of punting a malformed scope to the backend.
| { 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", description: "Template scope. Omit to use configured org/project defaults; use account or org to target those scopes explicitly.", enum: ["account", "org", "project"] }, |
There was a problem hiding this comment.
Adding scope only to listFilterFields does not fully expose the behavior this PR introduces. With scopeOptionalDefaultFromConfig enabled, templates now support account/org/project semantics for write flows too, but harness_describe(resource_type="template") still reports only scope: "project", and the create/update/delete metadata exposes no structured scope input.
That leaves the new account/org write paths effectively undiscoverable to agents, which is a contract mismatch under the repo guidance to put agent-facing behavior in structured metadata rather than runtime-only flags or prose. Please surface the supported scopes / write-time selector in the describe output as well.
2 participants
Description
Fixes a critical template CRUD scoping regression from the recent multi-scope template change. When users had
HARNESS_ORG/HARNESS_PROJECTconfigured but omitted explicit template scope params, template create/delete/list/get calls stopped inheriting the configured project scope and could route to account scope instead. This could cause wrong-scope writes/deletes for common MCP usage.The fix adds opt-in default scoping for multi-scope resources and enables it for templates: omitted scope fields use configured org/project defaults, while explicit
scope: "account",scope: "org",scope: "project",global=true, or explicit org/project inputs remain supported.Type of Change
Checklist
Validation:
pnpm test tests/registry/registry.test.ts(86 tests passed)pnpm typecheckpnpm test(53 files, 1274 tests passed)pnpm buildHARNESS_API_KEY=pat.test-account.token.secret HARNESS_ACCOUNT_ID=test-account HARNESS_TOOLSETS=templates timeout 3s node build/index.js stdio(startup succeeded; command exits 124 due expected timeout)