Skip to content

Fix account-scope Harness resource queries#182

Draft
thisrohangupta wants to merge 9 commits into
mainfrom
cursor/account-scope-resources-9f58
Draft

Fix account-scope Harness resource queries#182
thisrohangupta wants to merge 9 commits into
mainfrom
cursor/account-scope-resources-9f58

Conversation

@thisrohangupta
Copy link
Copy Markdown
Collaborator

@thisrohangupta thisrohangupta commented May 13, 2026

Description

Adds explicit account/org/project scope selection for query tools so account-level Harness resources can be listed, searched, or fetched without configured org/project defaults being re-injected. Account-level Harness URLs for known multi-scope entities now set resource_scope: "account", and harness_describe surfaces supported scopes for multi-scope resources.

Covers connectors, services, environments, infrastructure definitions, secrets metadata, and templates. The generic selector is named resource_scope to avoid collisions with resource-specific filters such as GitOps cluster-link scope=ACCOUNT. URL-derived resource_scope is limited to known multi-scope entity resources so account-scoped APIs with org/project UI URLs, such as FME feature flags, keep working.

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other

Checklist

  • Tests pass (pnpm test; 52 files / 1213 tests)
  • Typecheck passes (pnpm typecheck)

Additional verification: pnpm build and focused regression suite pnpm test tests/utils/url-parser.test.ts tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts.

Slack Thread

Open in Web Open in Cursor 

Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
@CLAassistant
Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment
  1. Findings
  • Critical — src/registry/index.ts / Registry.executeSpec(): explicit scope: "org" or scope: "project" does not fail when the matching identifiers are unavailable. The new branch writes params[orgParam] = input.org_id ?? HARNESS_ORG and params[projectParam] = input.project_id ?? HARNESS_PROJECT; when those still resolve to undefined, HarnessClient.buildUrl() drops the params entirely, so an explicitly narrowed request silently widens back to account scope. I verified that on the built branch with connector.list: registry.dispatch(..., { scope: "org" }) and no HARNESS_ORG produced an empty params object instead of an error. Given this PR is about fixing scope correctness, silently widening scope is a blocking regression and violates the repo’s fail-loud standard.
  • Important — src/tools/harness-describe.ts with src/registry/index.ts#describe(): the new multi-scope metadata only shows up on harness_describe(resource_type=...). The toolset branch still returns registry.describe() rows that only expose scope, so harness_describe(toolset="connectors") still reports connector as project-scoped only even though runtime now accepts account/org/project queries. That leaves one of the primary discovery surfaces out of sync with the implemented behavior.
  1. Missing Tests / Verification Gaps
  • There is no regression test for explicit scope: "org" / scope: "project" when HARNESS_ORG / HARNESS_PROJECT are unset. The added tests only cover account scope, org scope with an org_id present, and default project behavior, which is why the silent widening path is still uncovered.
  • There is no test covering harness_describe(toolset=...) (or Registry.describe()) for a multi-scope resource. Current coverage only exercises harness_describe(resource_type="connector"), so the stale toolset-level metadata path slipped through.
  • Verification I ran: pnpm test tests/utils/url-parser.test.ts tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts, pnpm typecheck, pnpm build, plus a built-runtime probe for explicit scope behavior.
  1. Overall Assessment
  • Not ready to merge as-is because the new explicit scope selector can still return broader-than-requested data. Once the fail-loud scope validation is fixed, the rest of the diff looks focused and the targeted validation suite is green.
Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +531 to +537
if (requestedScope) {
// Explicit scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
}
if (shouldUseProject(requestedScope)) {
params[projectParam] = (input.project_id as string) ?? this.config.HARNESS_PROJECT;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If the caller explicitly picks scope: "org" or scope: "project" but neither the request nor config provides the matching identifier, these assignments leave the param undefined. HarnessClient.buildUrl() then strips it, so the request silently widens to account/org scope instead of failing locally. I reproduced that on the built branch with connector.list and no HARNESS_ORG: scope: "org" produced { params: {} }. This needs a fail-loud guard before dispatch.

Comment thread src/tools/harness-describe.ts Outdated
Comment on lines +31 to +42
const supportedScopes = def.supportedScopes ?? (
def.scopeOptional ? ["account", "org", "project"] : undefined
);
return jsonResult({
resource_type: def.resourceType,
displayName: def.displayName,
description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
scopeHint: supportedScopes && supportedScopes.length > 1
? "Set scope='account' for account-level data, scope='org' for org-level data, or scope='project' for project-level data. If scope is omitted, the resource uses its default scope and configured defaults."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This fixes the resource_type detail view, but the toolset branch below still proxies registry.describe(), whose resource rows only expose scope. As a result, harness_describe(toolset="connectors") still tells agents connector is project-scoped only, so one of the main discovery surfaces is still out of sync with the new runtime behavior.

cursoragent and others added 3 commits May 13, 2026 01:31
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>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment

Reviewed the requested diff 274a758..2fd9669 and compared it with the live PR head 83093ce; only tasks/todo.md changed afterward, so the code findings below still apply to the latest head.

  1. Important: explicit resource_scope='org'|'project' still widens instead of failing loudly
    Files: src/registry/index.ts, src/client/harness-client.ts
    Status on latest head: still present.
    executeSpec() writes undefined scope params when the selected narrower scope lacks IDs in both input and config, and HarnessClient.buildUrl() drops them. In a direct repro with HARNESS_ORG/HARNESS_PROJECT unset, connector.list({ resource_scope: "org" }) sent {}, and connector.list({ resource_scope: "project", org_id: "my-org" }) sent only { "orgIdentifier": "my-org" }. That silently broadens the request to account/org scope instead of rejecting it locally.

  2. Important: the toolset discovery surface still advertises the old single-scope contract
    Files: src/tools/harness-describe.ts, src/registry/index.ts
    Status on latest head: still present.
    harness_describe(resource_type="connector") now returns supportedScopes/scopeHint, but the toolset path still delegates to Registry.describe(), whose resource entries only include scope. A direct registry.describe() probe for the connectors toolset still omits supportedScopes, so agents using harness_describe(toolset="connectors") never see the new multi-scope guidance.

Missing tests / verification gaps

  • No regression covering fail-loud behavior for resource_scope="org" without org_id (and unset HARNESS_ORG) or resource_scope="project" without project_id (and unset HARNESS_PROJECT). The new tests only cover happy-path account scope and org scope with explicit IDs.
  • No test covering the toolset-level discovery path. Current coverage only asserts harness_describe(resource_type="connector"); it does not validate harness_describe(toolset="connectors") / Registry.describe().

Verification

  • pnpm vitest tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts tests/utils/url-parser.test.ts (pass, 176 tests)
  • Direct runtime probes confirmed the two behaviors above.

Merge readiness

  • Not ready: the PR still misses the fail-loud scope guard and the full discovery-surface metadata sweep requested by the architecture standards.
Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +531 to +537
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
}
if (shouldUseProject(requestedScope)) {
params[projectParam] = (input.project_id as string) ?? this.config.HARNESS_PROJECT;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If resource_scope='org' or 'project' is selected and the matching identifier is absent from both the call and config, these assignments leave the param undefined, and HarnessClient.buildUrl() drops it entirely. That means the request silently widens instead of failing locally. I reproduced it with HARNESS_ORG/HARNESS_PROJECT unset: connector.list({ resource_scope: "org" }) sent {}, and connector.list({ resource_scope: "project", org_id: "my-org" }) sent only { orgIdentifier: "my-org" }. This needs an explicit guard before dispatch so narrower scopes fail loudly when the required IDs are missing.

Comment on lines +31 to +42
const supportedScopes = def.supportedScopes ?? (
def.scopeOptional ? ["account", "org", "project"] : undefined
);
return jsonResult({
resource_type: def.resourceType,
displayName: def.displayName,
description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
scopeHint: supportedScopes && supportedScopes.length > 1
? "Set resource_scope='account' for account-level data, resource_scope='org' for org-level data, or resource_scope='project' for project-level data. If resource_scope is omitted, the resource uses its default scope and configured defaults."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This fixes the resource_type describe path, but harness_describe(toolset=...) still goes through Registry.describe(), which only emits scope for each resource. As a result, harness_describe(toolset="connectors") still omits supportedScopes/scopeHint, so the toolset discovery surface advertises the old single-scope contract. Please update Registry.describe() (and add a regression test for the toolset path) so both discovery surfaces expose the same scope metadata.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment

Found 2 important issues on the current PR head (ee95b698):

  1. src/registry/index.ts: explicit resource_scope="org"|"project" can silently widen to a broader request when the corresponding identifiers/defaults are absent, instead of failing loudly.
  2. src/tools/harness-describe.ts: the new multi-scope metadata is only exposed for harness_describe(resource_type=...); toolset-based discovery still omits it, so agent-facing metadata is out of sync with runtime behavior.

Assumption: I’m treating explicit org/project scope without the matching identifiers as a fail-loud contract, which matches the repo’s existing scoping guidance.

Verification on the PR head worktree:

  • pnpm test tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts tests/utils/url-parser.test.ts
  • pnpm typecheck
  • pnpm build

Repo CI was green when checked.

Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +531 to +537
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
}
if (shouldUseProject(requestedScope)) {
params[projectParam] = (input.project_id as string) ?? this.config.HARNESS_PROJECT;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Explicit resource_scope should fail loudly when the matching identifiers/defaults are missing.

With resource_scope: "org" and no org_id/HARNESS_ORG, this branch leaves orgIdentifier undefined; HarnessClient.buildUrl() then drops the param entirely, so the request widens to account scope instead of erroring. The same thing happens for "project" when project_id/HARNESS_PROJECT is unset. A small guard here (plus a regression test with config defaults removed) would keep the new scope selector honest.

Comment on lines +31 to +42
const supportedScopes = def.supportedScopes ?? (
def.scopeOptional ? ["account", "org", "project"] : undefined
);
return jsonResult({
resource_type: def.resourceType,
displayName: def.displayName,
description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
scopeHint: supportedScopes && supportedScopes.length > 1
? "Set resource_scope='account' for account-level data, resource_scope='org' for org-level data, or resource_scope='project' for project-level data. If resource_scope is omitted, the resource uses its default scope and configured defaults."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This fixes the single-resource describe path, but harness_describe(toolset=...) still goes through Registry.describe(), and that payload only emits scope.

As a result, agents discovering connector/service/environment/etc. by toolset still see the old project-only contract even though runtime now accepts resource_scope. Please thread supportedScopes/scopeHint through the toolset path as well and add a regression around harness_describe({ toolset: "connectors" }).

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment

Important:

  • src/registry/index.ts: explicit resource_scope='org' / 'project' does not fail loudly when the matching IDs are missing. If HARNESS_ORG / HARNESS_PROJECT are unset, the dispatcher leaves those params undefined and the URL builder drops them, so an explicitly narrower query silently widens instead of honoring the requested scope.
  • src/tools/harness-describe.ts, src/registry/index.ts: the new scope metadata is only exposed through harness_describe(resource_type=...). The toolset= path still serializes Registry.describe(), which omits supportedScopes / scopeHint, so toolset-based discovery still misses the new contract.
  • src/tools/harness-get.ts, src/tools/harness-list.ts: the new resource_scope schema uses .describe(...).optional(). This repo’s Zod 4 standard requires .describe() last, otherwise the generated MCP schema drops the field description and the new parameter becomes effectively undocumented to agents.

Notable test gaps:

  • No negative regression covers resource_scope='org' / 'project' with HARNESS_ORG / HARNESS_PROJECT unset.
  • No harness_describe(toolset=...) regression proves the new scope metadata is visible through the toolset-filtered discovery path.
Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +531 to +537
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
}
if (shouldUseProject(requestedScope)) {
params[projectParam] = (input.project_id as string) ?? this.config.HARNESS_PROJECT;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important: this still doesn't fail closed when a caller explicitly requests resource_scope: "org" or "project" but omits the matching IDs and there are no HARNESS_ORG / HARNESS_PROJECT defaults. In that case these assignments remain undefined, and HarnessClient.buildUrl() drops undefined params, so the request silently widens to account scope instead of honoring the explicit scope selection. Please validate org_id for org scope and both org_id / project_id for project scope before dispatch.

Comment on lines +31 to +42
const supportedScopes = def.supportedScopes ?? (
def.scopeOptional ? ["account", "org", "project"] : undefined
);
return jsonResult({
resource_type: def.resourceType,
displayName: def.displayName,
description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
scopeHint: supportedScopes && supportedScopes.length > 1
? "Set resource_scope='account' for account-level data, resource_scope='org' for org-level data, or resource_scope='project' for project-level data. If resource_scope is omitted, the resource uses its default scope and configured defaults."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important: this only fixes the resource_type= branch. The toolset= branch below still returns registry.describe(), and Registry.describe() does not serialize supportedScopes or scopeHint. That means harness_describe(toolset="connectors") still hides the new multi-scope contract even though the PR summary says harness_describe now surfaces it.

Comment thread src/tools/harness-list.ts
inputSchema: {
resource_type: resourceTypeSchema(listableTypes).optional().describe("Resource type to list. Auto-detected from url."),
url: z.string().describe("Harness UI URL — auto-extracts org, project, and type").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to query. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: the repo’s Zod 4 rules require calling .describe() last. With .describe(...).optional(), the new resource_scope field loses its description in the generated MCP schema, so agents see an undocumented enum instead of the guidance this PR is trying to add. harness_get has the same pattern on its new resource_scope field.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment

Found 3 issues that still miss Sunil's architecture standards before this is ready:

  1. Important: explicit resource_scope='org' / 'project' does not fail loudly when the matching IDs/defaults are absent, so the request can silently widen instead of honoring the caller's explicit scope.
  2. Important: harness_describe(toolset=...) still omits the new scope metadata, so the discovery surface is inconsistent with both the PR description and the repo's structured-guidance rules.
  3. Note: the new resource_scope Zod fields use .describe(...).optional(), which drops the agent-facing description in Zod 4.

Assumptions:

  • I reviewed against the PR goal of making explicit scope selection both correct and discoverable across the agent-facing metadata surface.
  • CI was mostly green while reviewing; the read-only smoke job was still running.

Verification:

  • pnpm install --frozen-lockfile
  • pnpm test tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts tests/utils/url-parser.test.ts
  • pnpm typecheck
  • pnpm build
  • Runtime probes on the built branch confirmed the scope-widening case and the missing toolset describe metadata.
Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +531 to +537
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
}
if (shouldUseProject(requestedScope)) {
params[projectParam] = (input.project_id as string) ?? this.config.HARNESS_PROJECT;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important: explicit resource_scope='org' / 'project' should fail locally when the matching identifiers are missing.

Right now these assignments fall back to HARNESS_ORG / HARNESS_PROJECT, but if those defaults are unset the value stays undefined and the request is still sent. I verified that on the built branch with registry.dispatch(..., { resource_scope: 'org' }) and no defaults: the generated query params were {}. That silently widens an explicit org/project request instead of honoring the caller's selected scope, which conflicts with the repo's fail-loud guidance for explicit scope selectors.

Please reject the call before request construction and add a negative test with HARNESS_ORG / HARNESS_PROJECT unset.

Comment on lines +31 to +42
const supportedScopes = def.supportedScopes ?? (
def.scopeOptional ? ["account", "org", "project"] : undefined
);
return jsonResult({
resource_type: def.resourceType,
displayName: def.displayName,
description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
scopeHint: supportedScopes && supportedScopes.length > 1
? "Set resource_scope='account' for account-level data, resource_scope='org' for org-level data, or resource_scope='project' for project-level data. If resource_scope is omitted, the resource uses its default scope and configured defaults."
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important: this only fixes the resource_type branch of harness_describe.

The toolset branch below still returns registry.describe(), and Registry.describe() does not include supportedScopes or scopeHint. I confirmed the runtime shape on the built branch: registry.describe().toolsets.connectors.resources[0] still returns null for both fields even though registry.getResource('connector').supportedScopes is populated.

That leaves the discovery surface inconsistent with the PR's stated contract (harness_describe surfaces supported scopes) and with the repo rule that agent guidance should be expressed as structured metadata everywhere it is surfaced. Please thread the same metadata through the toolset describe path and add a regression test for harness_describe(toolset='connectors').

Comment thread src/tools/harness-list.ts
inputSchema: {
resource_type: resourceTypeSchema(listableTypes).optional().describe("Resource type to list. Auto-detected from url."),
url: z.string().describe("Harness UI URL — auto-extracts org, project, and type").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to query. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Note: the repo's Zod 4 rule is to call .describe() last. z.enum(...).describe(...).optional() drops the description on the optional wrapper, so agents will not actually see the new resource_scope guidance this PR is adding. A quick REPL on this branch returns null for .describe(...).optional().description and the expected text for .optional().describe(...). The same issue applies to harness_get.

cursoragent and others added 3 commits May 13, 2026 02:22
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>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment
  1. Important — src/registry/index.ts: explicit resource_scope='org' / 'project' silently widens to a broader request when the corresponding default scope is unset. Because undefined params are dropped later, an org-scoped query can turn into an account-scoped API call instead of failing loudly.
  2. Important — src/tools/harness-search.ts: resource_scope was added to the search schema, but the default search path still fans out across every listable resource. That now produces a large errors payload for unsupported-scope types rather than a clean scoped search, and the new tests only cover the narrowed resource_types=['connector'] case.
  3. Suggestion — src/tools/harness-describe.ts: the new supportedScopes metadata is only exposed when resource_type is passed. harness_describe(toolset=...) still returns registry.describe() output without that field, so discovery is inconsistent depending on which describe entrypoint an agent uses.
Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +531 to +534
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important: this branch never verifies that the requested org/project scope can actually be resolved. If resource_scope: 'org' is passed with neither org_id nor HARNESS_ORG set, params[orgParam] becomes undefined, and HarnessClient.buildUrl() drops it entirely. That means an explicit org-scoped request silently degrades into an account-scoped call instead of failing loudly.

I verified this against the built branch with HARNESS_ORG/HARNESS_PROJECT unset: dispatch(..., { resource_scope: 'org' }) for connectors emitted no orgIdentifier at all. Please add an explicit validation error when resource_scope requires an identifier that cannot be resolved.

query: z.string().describe("Search term"),
resource_types: z.array(z.enum(listableTypes)).describe("Types to search (defaults to all listable)").optional(),
url: z.string().describe("Harness UI URL — auto-extracts org and project").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to search. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Important: adding resource_scope to the public search input exposes a noisy regression in the default code path. When resource_types is omitted, harness_search still iterates every listable resource, but Registry.dispatch() now rejects unsupported scopes. In a quick probe against this branch, harness_search({ resource_scope: 'account' }) searched 128 types and returned 91 per-type errors.

The new tests only cover resource_types: ['connector'], so this stays green. Please filter targetTypes to resources that support the requested scope before dispatching (or add a test that exercises the default-all-types path with resource_scope).

Comment on lines +31 to +40
const supportedScopes = def.supportedScopes ?? (
def.scopeOptional ? ["account", "org", "project"] : undefined
);
return jsonResult({
resource_type: def.resourceType,
displayName: def.displayName,
description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: this only fixes the harness_describe(resource_type=...) shape. The toolset branch still delegates to registry.describe(), whose resource summaries only expose scope, so harness_describe(toolset='connectors') still reports connectors as project-scoped with no supportedScopes metadata.

That makes the discovery contract inconsistent depending on which describe entrypoint an agent uses. Consider threading supportedScopes/scopeHint through Registry.describe() as well, and add a toolset-filtered test so the new scope metadata stays visible in both paths.

Co-authored-by: Rohan Gupta <thisrohangupta@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment

Found 3 issues that still miss Sunil's architecture standards:

  1. Important: src/registry/index.ts can silently widen an explicitly narrowed resource_scope.
    When callers set resource_scope='org' or 'project' without matching identifiers/defaults, executeSpec() leaves orgIdentifier / projectIdentifier undefined and HarnessClient.buildUrl() drops them. I confirmed on the built branch that connector.list({ resource_scope: 'org' }) with HARNESS_ORG unset emits {} params instead of failing locally, which violates the repo's fail-loud scoping rule.

  2. Important: harness_describe(toolset=...) still omits the new multi-scope metadata.
    harness_describe(resource_type='connector') now returns supportedScopes / scopeHint, but the toolset path still serializes Registry.describe(), whose resource rows only expose scope. A runtime probe of registry.describe().toolsets.connectors.resources[0] confirmed the omission, so one discovery surface is still out of sync with the runtime contract.

  3. Note: the new resource_scope schema fields in src/tools/harness-list.ts, src/tools/harness-get.ts, and src/tools/harness-search.ts use .describe(...).optional().
    This repo's Zod 4 rule requires .describe() last. I verified that z.enum(...).describe('x').optional().description is undefined on this branch, so the new parameter loses its agent-facing description.

Assumption:

  • I’m treating explicit narrower scopes without the required IDs as a fail-loud contract, consistent with CLAUDE.md and the repo’s earlier scope review patterns.

Verification:

  • pnpm test tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts tests/utils/url-parser.test.ts
  • pnpm typecheck
  • pnpm build
  • Runtime probes for explicit org-scope dispatch and toolset-level describe output

Targeted suite/build are green, but there is still no negative regression for explicit org/project scope without IDs and no coverage for harness_describe(toolset=...) exposing the new scope metadata.

Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
Comment on lines +533 to +537
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
}
if (shouldUseProject(requestedScope)) {
params[projectParam] = (input.project_id as string) ?? this.config.HARNESS_PROJECT;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If resource_scope is explicitly org / project and the matching ID or config default is missing, these assignments leave undefined in params; HarnessClient.buildUrl() strips the key and the request silently widens instead of failing. I confirmed connector.list({ resource_scope: "org" }) with HARNESS_ORG unset emits {} on the built branch. Since this PR is about scope correctness, please reject explicit narrower scopes when the required identifiers are unavailable.

description: def.description,
toolset: def.toolset,
scope: def.scope,
supportedScopes,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This only updates the resource_type branch. The toolset branch still returns registry.describe(), and Registry.describe() does not include supportedScopes / scopeHint, so harness_describe(toolset="connectors") still hides the new multi-scope contract. Sunil's structured-guidance standard wants those discovery surfaces to stay in sync.

Comment thread src/tools/harness-list.ts
inputSchema: {
resource_type: resourceTypeSchema(listableTypes).optional().describe("Resource type to list. Auto-detected from url."),
url: z.string().describe("Harness UI URL — auto-extracts org, project, and type").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to query. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

z.enum(...).describe(...).optional() drops the description on the outer optional wrapper in Zod 4. This repo explicitly requires .describe() last, so resource_scope becomes undocumented here (and the same pattern appears in harness_get and harness_search). Please flip these to .optional().describe(...).

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Stale comment

Found 3 issues that keep this from Sunil-standard approval:

  1. scopeOptional is now being treated as full account/org/project support. That broadens both runtime validation and harness_describe metadata for existing resources that are not actually tri-scope.
  2. Explicit resource_scope="org" / "project" still fails open when the matching identifiers are missing. The dispatcher drops back to an effectively wider request instead of failing loudly.
  3. The new resource_scope tool schemas use .describe().optional(), which drops the agent-facing description in Zod 4.

Assumption:

  • I rechecked the live PR head (508e001); the only post-sync change after the initial diff was tasks/todo.md, so the code findings below still apply to the current patch.

Checks run:

  • pnpm test tests/utils/url-parser.test.ts tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts
  • pnpm typecheck
  • pnpm build
  • ad hoc runtime probes for Zod describe-order and missing-org scope dispatch
Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
return def.supportedScopes;
}
if (def.scopeOptional) {
return RESOURCE_SCOPES;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

scopeOptional isn't equivalent to "supports account/org/project". Existing resources already use it for narrower semantics, e.g. delegate is account-only and scs_component_enrichment uses it for account-vs-project branching. With this fallback, getRequestedScope() will now accept resource_scope="org" for those types, and harness_describe / toolset discovery will advertise scopes they don't actually support. I think supportedScopes needs to stay explicit per resource instead of defaulting every scopeOptional resource to all three scopes.

Comment thread src/registry/index.ts
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This still fails open when the caller selects a narrower scope but we can't resolve the required IDs. If resource_scope="org" and both input.org_id and HARNESS_ORG are unset, we assign undefined here; HarnessClient.buildUrl() drops undefined params, so the request silently widens instead of erroring. I verified the built branch emits {} params for connector.list with resource_scope: "org" and no defaults. Since Sunil's rules prefer fail-loud scoping, this should throw locally when the selected scope can't be satisfied.

Comment thread src/tools/harness-list.ts
inputSchema: {
resource_type: resourceTypeSchema(listableTypes).optional().describe("Resource type to list. Auto-detected from url."),
url: z.string().describe("Harness UI URL — auto-extracts org, project, and type").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to query. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This new field uses .describe(...).optional(). In Zod 4 that drops the description on the outer optional wrapper, so the validator works but agents don't see the resource_scope guidance. The repo standards explicitly call for .optional().describe(...). The same new chain appears in harness_get and harness_search, so all three call sites need the same fix.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Found 3 issues that still miss Sunil's architecture standards on the current head (508e00155947e3398491ed5d94a4c4fd1f4afa6f):

  1. Important — src/registry/index.ts: explicit resource_scope='org' / 'project' still fails open when the matching IDs/defaults are absent. I repro'd registry.dispatch(..., 'connector', 'list', { resource_scope: 'org' }) with HARNESS_ORG / HARNESS_PROJECT unset; it sent an empty params object to the client instead of throwing. That silently widens the request instead of honoring the caller's explicit scope.
  2. Important — src/tools/harness-search.ts: the new resource_scope input is only safe when callers manually narrow resource_types. Broad harness_search({ resource_scope: 'account' }) now fans that scope into every listable type, so mixed registries return predictable unsupported-scope errors for project-only resources like pipeline and execution. The same regression appears when an account-level connector URL auto-populates resource_scope='account'.
  3. Important — src/tools/harness-get.ts, src/tools/harness-list.ts, src/tools/harness-search.ts: the new Zod field is declared as .describe(...).optional(). In Zod 4 that strips the outer description, so the new resource_scope parameter becomes effectively undocumented to agents even though validation still works.

Open questions / assumptions

  • I'm treating explicit org/project scope without matching identifiers as a fail-loud contract, consistent with the repo guidance in CLAUDE.md / AGENTS.md.
  • I'm assuming broad search should either filter to compatible resource types or suppress predictable unsupported-scope failures when resource_scope is provided, rather than surfacing them as avoidable per-type errors.

Change summary

  • The latest head did fix one earlier discovery gap: harness_describe(toolset='connectors') now includes supportedScopes.
  • Verification: pnpm test tests/registry/registry.test.ts tests/tools/tool-handlers.test.ts tests/utils/url-parser.test.ts, pnpm typecheck, pnpm build, plus runtime probes for explicit-scope dispatch, toolset describe output, and broad harness_search scope behavior.

Overall: not ready to merge against Sunil's architecture standards yet.

Open in Web View Automation 

Sent by Cursor Automation: Sunil On Demand Architecture Review

Comment thread src/registry/index.ts
if (requestedScope) {
// Explicit resource scoping: account omits org/project, org injects org only, project injects both.
if (shouldUseOrg(requestedScope)) {
params[orgParam] = (input.org_id as string) ?? this.config.HARNESS_ORG;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If resource_scope is explicitly "org" (or "project") and neither the request nor config provides the matching IDs, this branch just leaves the scope params undefined. I repro'd registry.dispatch(client, "connector", "list", { resource_scope: "org" }) with HARNESS_ORG / HARNESS_PROJECT unset and it sent {} to the client instead of throwing. Because the client later drops undefined query params, the request silently widens instead of failing loudly, which conflicts with the repo's scoping standard.

Can we validate the required IDs before populating params when the caller explicitly asks for a narrower scope?

query: z.string().describe("Search term"),
resource_types: z.array(z.enum(listableTypes)).describe("Types to search (defaults to all listable)").optional(),
url: z.string().describe("Harness UI URL — auto-extracts org and project").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to search. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding resource_scope here makes it part of the public search contract, but the handler still fans the same merged args out to every listable resource. On the current head, harness_search({ query: "abc", resource_scope: "account" }) with a mixed pipelines,connectors registry returns unsupported-scope errors for pipeline and execution, and the same happens with an account-level connectors URL. That makes the new input safe only when callers manually narrow resource_types.

Could we either filter targetTypes to resources that support the requested scope, or avoid propagating URL/explicit scope into broad all-types searches?

Comment thread src/tools/harness-list.ts
inputSchema: {
resource_type: resourceTypeSchema(listableTypes).optional().describe("Resource type to list. Auto-detected from url."),
url: z.string().describe("Harness UI URL — auto-extracts org, project, and type").optional(),
resource_scope: z.enum(["account", "org", "project"]).describe("Scope to query. Use account for account-level resources and to omit org/project defaults; org injects only org; project injects org+project. Auto-detected from url.").optional(),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This repo's Zod 4 guidance requires .describe() last. z.enum(...).describe(...).optional() drops the description on the optional wrapper, so the new resource_scope field becomes effectively undocumented to agents even though validation still works. Quick repro on this head: z.enum([...]).describe("scope desc").optional().description is null, while z.enum([...]).optional().describe("scope desc").description preserves the text.

The same pattern is repeated in harness_get and harness_search, so I think these new fields should be flipped to .optional().describe(...).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants