From 4cb2c2e6e4709e75e8f91a0c1647da51fc8236ae Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Tue, 4 Aug 2026 19:17:47 +0000 Subject: [PATCH] fix: parent-scoped list deep links and standards guardrail - Add pipelineScopedListExtract for child resources listed under a pipeline - Fix trigger and input_set list openInHarness using child identifier - Fix trigger delete to require pipeline_id as targetIdentifier - Add coding-standards test to block bare pageExtract on parent deep links Co-authored-by: Rohan Gupta --- src/registry/extractors.ts | 42 +++++ src/registry/toolsets/pipelines.ts | 14 +- .../registry-metadata.test.ts | 26 +++ .../parent-scoped-list-deep-links.test.ts | 163 ++++++++++++++++++ 4 files changed, 241 insertions(+), 4 deletions(-) create mode 100644 tests/registry/parent-scoped-list-deep-links.test.ts diff --git a/src/registry/extractors.ts b/src/registry/extractors.ts index 3a0d9c3d4..e8f1d5dc3 100644 --- a/src/registry/extractors.ts +++ b/src/registry/extractors.ts @@ -665,6 +665,48 @@ export const chaosInputSetListExtract = ( return { items, total: page.total }; }; +/** + * For child resources listed under a parent scope (e.g. triggers, input sets under a pipeline), + * list rows often lack the parent id field needed for deep links. Inject from item alternate + * API fields or the required list filter input so openInHarness does not fall back to + * item.identifier (the child id). + */ +export function parentScopedListExtract( + parentField: string, + itemAlternateFields: string[] = [], +): (raw: unknown, input?: Record) => { items: unknown[]; total: number } { + return (raw, input) => { + const page = pageExtract(raw); + const fromInput = input?.[parentField]; + const inputParentId = typeof fromInput === "string" && fromInput !== "" ? fromInput : undefined; + const items = page.items.map((item) => { + if (item && typeof item === "object" && !Array.isArray(item)) { + const rec = item as Record; + if (rec[parentField] !== undefined && typeof rec[parentField] !== "object") { + return item; + } + let fromItem: string | undefined; + for (const alt of itemAlternateFields) { + const altValue = rec[alt]; + if (typeof altValue === "string" && altValue !== "") { + fromItem = altValue; + break; + } + } + const resolved = fromItem ?? inputParentId; + if (resolved !== undefined) { + return { ...rec, [parentField]: resolved }; + } + } + return item; + }); + return { items, total: page.total }; + }; +} + +/** Pipeline-scoped child lists (triggers, input sets) use pipelineIdentifier on rows. */ +export const pipelineScopedListExtract = parentScopedListExtract("pipeline_id", ["pipelineIdentifier"]); + /** * Normalize chaos experiment variables response (RunTimeInputs shape): * { experiment: [...] | null, tasks: { taskName: [...] } | null } diff --git a/src/registry/toolsets/pipelines.ts b/src/registry/toolsets/pipelines.ts index 53cabf530..b6ec25aa2 100644 --- a/src/registry/toolsets/pipelines.ts +++ b/src/registry/toolsets/pipelines.ts @@ -1,5 +1,5 @@ import type { ToolsetDefinition, BodySchema, ParamsSchema } from "../types.js"; -import { ngExtract, pageExtract, passthrough, v1ListExtract, runtimeInputExtract, executionInputsExtract, dynamicExecutionExtract } from "../extractors.js"; +import { ngExtract, pageExtract, passthrough, v1ListExtract, runtimeInputExtract, executionInputsExtract, dynamicExecutionExtract, pipelineScopedListExtract } from "../extractors.js"; import YAML from "yaml"; function coerceTriggerBodyRecord(body: unknown): Record { @@ -743,7 +743,7 @@ export const pipelinesToolset: ToolsetDefinition = { page: "page", size: "size", }, - responseExtractor: pageExtract, + responseExtractor: pipelineScopedListExtract, description: "List triggers for a pipeline", }, get: { @@ -801,8 +801,14 @@ export const pipelinesToolset: ToolsetDefinition = { path: "/pipeline/api/triggers/{triggerIdentifier}", operationPolicy: { risk: "destructive", retryPolicy: "do_not_retry" }, pathParams: { trigger_id: "triggerIdentifier" }, + queryParams: { pipeline_id: "targetIdentifier" }, + paramsSchema: { + fields: [ + { name: "pipeline_id", required: true, description: "Target pipeline identifier" }, + ], + } satisfies ParamsSchema, responseExtractor: ngExtract, - description: "Delete a pipeline trigger", + description: "Delete a pipeline trigger. Requires pipeline_id (target pipeline) and trigger_id.", }, }, }, @@ -846,7 +852,7 @@ export const pipelinesToolset: ToolsetDefinition = { page: "page", size: "size", }, - responseExtractor: pageExtract, + responseExtractor: pipelineScopedListExtract, description: "List input sets for a pipeline", }, get: { diff --git a/tests/coding-standards/registry-metadata.test.ts b/tests/coding-standards/registry-metadata.test.ts index 9fcb09f01..a0a5828e9 100644 --- a/tests/coding-standards/registry-metadata.test.ts +++ b/tests/coding-standards/registry-metadata.test.ts @@ -5,6 +5,7 @@ import { describe, it, expect } from "vitest"; import { readFileSync } from "node:fs"; import { join, relative } from "node:path"; import { Registry } from "../../src/registry/index.js"; +import { pageExtract } from "../../src/registry/extractors.js"; const REPO_ROOT = join(import.meta.dirname, "../.."); @@ -125,6 +126,31 @@ describe("Coding standards — registry metadata", () => { expect(violations, violations.join("\n")).toEqual([]); }); + + it("child list resources with parent deep-link placeholder do not use bare pageExtract", () => { + const violations: string[] = []; + + for (const resourceType of registry.getAllResourceTypes()) { + const def = registry.getResource(resourceType); + const listSpec = def.operations.list; + if (!listSpec || !def.deepLinkTemplate) continue; + if (def.identifierFields.length < 2) continue; + + const parentField = def.identifierFields[0]!; + if (!def.deepLinkTemplate.includes(`{${parentField}}`)) continue; + + if (listSpec.responseExtractor === pageExtract) { + violations.push( + `${resourceType}: list uses pageExtract but deepLinkTemplate needs {${parentField}} — use parentScopedListExtract`, + ); + } + } + + expect( + violations, + `Parent-scoped list deep links will use child identifier:\n${violations.join("\n")}`, + ).toEqual([]); + }); }); describe("Coding standards — write handler safety", () => { diff --git a/tests/registry/parent-scoped-list-deep-links.test.ts b/tests/registry/parent-scoped-list-deep-links.test.ts new file mode 100644 index 000000000..a66331cfe --- /dev/null +++ b/tests/registry/parent-scoped-list-deep-links.test.ts @@ -0,0 +1,163 @@ +/** + * Parent-scoped list deep links must keep the parent id from the request + * (via pipelineScopedListExtract) — not overwrite with item.identifier (the child id). + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; +import { Registry } from "../../src/registry/index.js"; +import type { Config } from "../../src/config.js"; +import type { HarnessClient } from "../../src/client/harness-client.js"; + +function makeConfig(overrides: Partial = {}): Config { + return { + HARNESS_API_KEY: "pat.test", + HARNESS_ACCOUNT_ID: "test-account", + HARNESS_BASE_URL: "https://app.harness.io", + HARNESS_ORG: "default", + HARNESS_PROJECT: "test-project", + HARNESS_API_TIMEOUT_MS: 30000, + HARNESS_MAX_RETRIES: 3, + HARNESS_MAX_BODY_SIZE_MB: 10, + HARNESS_RATE_LIMIT_RPS: 10, + HARNESS_READ_ONLY: false, + HARNESS_SKIP_ELICITATION: false, + HARNESS_ALLOW_HTTP: false, + HARNESS_FME_BASE_URL: "https://api.split.io", + LOG_LEVEL: "info", + ...overrides, + }; +} + +function makeClient(requestFn?: (...args: unknown[]) => unknown): HarnessClient { + return { + request: requestFn ?? vi.fn().mockResolvedValue({}), + account: "test-account", + } as unknown as HarnessClient; +} + +describe("trigger list deep links", () => { + let registry: Registry; + + beforeEach(() => { + registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "pipelines" })); + }); + + it("keeps request pipeline_id in openInHarness (not item.identifier)", async () => { + const mockRequest = vi.fn().mockResolvedValue({ + data: { + content: [ + { identifier: "mcp_yaml_verify_cron", name: "mcp_yaml_verify_cron", type: "Scheduled" }, + ], + totalElements: 1, + }, + }); + const client = makeClient(mockRequest); + + const result = (await registry.dispatch(client, "trigger", "list", { + org_id: "default", + project_id: "test-project", + pipeline_id: "test", + })) as { items: Array> }; + + const link = result.items[0]!.openInHarness as string; + expect(link).toContain("/pipelines/test/triggers"); + expect(link).not.toContain("/pipelines/mcp_yaml_verify_cron/"); + }); + + it("prefers item.pipelineIdentifier over request pipeline_id when present", async () => { + const mockRequest = vi.fn().mockResolvedValue({ + data: { + content: [ + { + identifier: "mcp_yaml_verify_cron", + name: "mcp_yaml_verify_cron", + pipelineIdentifier: "otherPipe", + }, + ], + totalElements: 1, + }, + }); + const client = makeClient(mockRequest); + + const result = (await registry.dispatch(client, "trigger", "list", { + org_id: "default", + project_id: "test-project", + pipeline_id: "test", + })) as { items: Array> }; + + const link = result.items[0]!.openInHarness as string; + expect(link).toContain("/pipelines/otherPipe/triggers"); + expect(link).not.toContain("/pipelines/test/"); + expect(link).not.toContain("/pipelines/mcp_yaml_verify_cron/"); + }); +}); + +describe("trigger delete targetIdentifier", () => { + let registry: Registry; + + beforeEach(() => { + registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "pipelines" })); + }); + + it("sends pipeline_id as targetIdentifier query param", async () => { + const mockRequest = vi.fn().mockResolvedValue({ status: "SUCCESS", data: true }); + const client = makeClient(mockRequest); + + await registry.dispatch(client, "trigger", "delete", { + org_id: "default", + project_id: "test-project", + trigger_id: "mcp_yaml_verify_cron", + pipeline_id: "test", + }); + + expect(mockRequest).toHaveBeenCalledOnce(); + const call = mockRequest.mock.calls[0]![0] as { + method: string; + path: string; + params: Record; + }; + expect(call.method).toBe("DELETE"); + expect(call.path).toBe("/pipeline/api/triggers/mcp_yaml_verify_cron"); + expect(call.params.targetIdentifier).toBe("test"); + }); + + it("throws when pipeline_id is missing", async () => { + const client = makeClient(); + await expect( + registry.dispatch(client, "trigger", "delete", { + org_id: "default", + project_id: "test-project", + trigger_id: "mcp_yaml_verify_cron", + }), + ).rejects.toThrow(/Missing required param\(s\) for trigger\.delete: pipeline_id/); + }); +}); + +describe("input_set list deep links", () => { + let registry: Registry; + + beforeEach(() => { + registry = new Registry(makeConfig({ HARNESS_TOOLSETS: "pipelines" })); + }); + + it("keeps request pipeline_id in openInHarness (not item.identifier)", async () => { + const mockRequest = vi.fn().mockResolvedValue({ + data: { + content: [ + { identifier: "my_input_set", name: "My Input Set" }, + ], + totalElements: 1, + }, + }); + const client = makeClient(mockRequest); + + const result = (await registry.dispatch(client, "input_set", "list", { + org_id: "default", + project_id: "test-project", + pipeline_id: "parent_pipeline", + })) as { items: Array> }; + + const link = result.items[0]!.openInHarness as string; + expect(link).toContain("/pipelines/parent_pipeline/input-sets"); + expect(link).not.toContain("/pipelines/my_input_set/"); + }); +});