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
2 changes: 1 addition & 1 deletion src/registry/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ export class Registry {

if (spec.paramsSchema) {
const missingParams = spec.paramsSchema.fields
.filter(f => f.required && input[f.name] === undefined)
.filter(f => f.required && (input[f.name] === undefined || input[f.name] === ""))
.map(f => f.name);
if (missingParams.length > 0) {
throw new Error(
Expand Down
11 changes: 6 additions & 5 deletions src/registry/toolsets/gitops.ts
Original file line number Diff line number Diff line change
Expand Up @@ -228,13 +228,14 @@ export const gitopsToolset: ToolsetDefinition = {
responseExtractor: passthrough,
description:
"Delete a GitOps agent. The agent identifier in the path is the raw identifier — NOT scope-prefixed.\n\n" +
"SCOPE DETERMINES WHICH ORG/PROJECT PARAMS ARE INJECTED:\n" +
"RESOURCE SCOPE IS REQUIRED because the same raw agent identifier can exist at account, org, and project scope. " +
"Pass resource_scope as a top-level harness_delete argument so the request cannot target the wrong scope:\n" +
" Account-level agent: resource_scope='account' — omit org_id and project_id\n" +
" harness_delete(resource_type='gitops_agent', resource_id='myagent', resource_scope='account')\n" +
" Org-level agent: resource_scope='org' — org_id is injected, no project_id\n" +
" harness_delete(resource_type='gitops_agent', resource_id='myagent', resource_scope='org')\n" +
" Project-level agent: resource_scope='project' — both org_id and project_id are injected\n" +
" harness_delete(resource_type='gitops_agent', resource_id='myagent')\n\n" +
" harness_delete(resource_type='gitops_agent', resource_id='myagent', resource_scope='project')\n\n" +
"NOTE: The path identifier is always the raw agent ID (e.g. 'myagent'), never scope-prefixed\n" +
"(unlike other GitOps APIs where agentIdentifier is prefixed with 'account.', 'org.', etc.).\n" +
"The backend derives the scope from the presence of orgIdentifier and projectIdentifier in the request.",
Expand All @@ -250,12 +251,12 @@ export const gitopsToolset: ToolsetDefinition = {
},
{
name: "resource_scope",
required: false,
required: true,
description:
"Controls which scope params are injected. " +
"Required top-level harness_delete argument that controls which scope params are injected. " +
"'account' — account-level agent (no org/project). " +
"'org' — org-level agent (orgIdentifier injected). " +
"'project' — project-level agent (orgIdentifier + projectIdentifier injected, default when omitted).",
"'project' — project-level agent (orgIdentifier + projectIdentifier injected).",
},
],
} satisfies ParamsSchema,
Expand Down
18 changes: 18 additions & 0 deletions tasks/todo.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,23 @@
# Harness MCP Server — Task Tracking

## Critical Bug Investigation Automation (2026-07-17)
- [x] Baseline branch state and identify recent behavioral commits
- [x] Review high-blast-radius diffs and trace concrete trigger scenarios
- [x] Implement a minimal fix only if a critical bug is proven
- [x] Run focused verification for any fix, or sanity checks for no-fix outcome
- [x] Commit/push/open PR if fixed; otherwise report no critical bugs in Slack

### Plan
- Treat current `origin/main` history since the 2026-07-13 investigation as the primary review window.
- Prioritize the new CCM recommendation filters, GitOps delete mappings, and any recent write/auth behavior with concrete data-loss, crash, security, or major-breakage impact.
- Trace candidates through public handlers and registry/client dispatch before changing code; patch only a proven high-severity trigger.

### Review
- Found a wrong-scope deletion bug in the new `gitops_agent` delete operation. The resource is `scopeOptional`, so an omitted `resource_scope` suppresses configured `HARNESS_ORG` / `HARNESS_PROJECT` defaults and sends an account-scoped DELETE, despite the operation documentation claiming omission defaults to project scope. If the same raw agent ID exists at project and account scope, a normal project-context delete can remove the account agent.
- Required explicit account/org/project scope for GitOps agent deletion and documented all three safe call shapes. Required-param validation now also rejects empty strings so `params: { resource_scope: "" }` cannot bypass the fail-closed guard.
- Added registry regressions for omitted, empty, project, and account scope plus a public `harness_delete` regression proving omitted scope sends no API request.
- Verification passed: focused GitOps agent / delete tests (23), `pnpm typecheck`, `pnpm build`, `pnpm docs:check`, `pnpm standards:check` (80), and `pnpm test` (117 files / 2557 tests).

## Critical Bug Investigation Automation (2026-07-13)
- [x] Baseline branch state and identify recent behavioral commits
- [x] Review high-blast-radius diffs and trace concrete trigger scenarios
Expand Down
46 changes: 45 additions & 1 deletion tests/registry/gitops.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,17 +81,61 @@ describe("gitops_agent", () => {
expect(call.path).toBe("/gitops/api/v1/agents/account.myagent");
});

it("delete: raw agent_id → DELETE /gitops/api/v1/agents/{agentIdentifier}", async () => {
it("delete: rejects omitted resource_scope before sending a request", async () => {
const mockRequest = vi.fn();
const client = makeClient(mockRequest);

await expect(
registry.dispatch(client, "gitops_agent", "delete", {
agent_id: "shared-agent",
}),
).rejects.toThrow(/Missing required param\(s\).*resource_scope/);

expect(mockRequest).not.toHaveBeenCalled();
});

it("delete: rejects an empty resource_scope before sending a request", async () => {
const mockRequest = vi.fn();
const client = makeClient(mockRequest);

await expect(
registry.dispatch(client, "gitops_agent", "delete", {
agent_id: "shared-agent",
resource_scope: "",
}),
).rejects.toThrow(/Missing required param\(s\).*resource_scope/);

expect(mockRequest).not.toHaveBeenCalled();
});

it("delete: explicit project scope injects configured org and project", async () => {
const mockRequest = vi.fn().mockResolvedValue({});
const client = makeClient(mockRequest);

await registry.dispatch(client, "gitops_agent", "delete", {
agent_id: "agent1779094157087",
resource_scope: "project",
});

const call = mockRequest.mock.calls[0][0];
expect(call.method).toBe("DELETE");
expect(call.path).toBe("/gitops/api/v1/agents/agent1779094157087");
expect(call.params.orgIdentifier).toBe("default");
expect(call.params.projectIdentifier).toBe("test-project");
});

it("delete: explicit account scope omits org and project", async () => {
const mockRequest = vi.fn().mockResolvedValue({});
const client = makeClient(mockRequest);

await registry.dispatch(client, "gitops_agent", "delete", {
agent_id: "agent1779094157087",
resource_scope: "account",
});

const call = mockRequest.mock.calls[0][0];
expect(call.params.orgIdentifier).toBeUndefined();
expect(call.params.projectIdentifier).toBeUndefined();
});
});

Expand Down
18 changes: 18 additions & 0 deletions tests/tools/tool-handlers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1345,6 +1345,24 @@ describe("harness_delete", () => {
expect(data.deleted).toBe(true);
});

it("does not delete a GitOps agent when resource_scope is omitted", async () => {
const gitopsRegistry = new Registry(makeConfig({ HARNESS_TOOLSETS: "gitops" }));
const gitopsServer = makeMcpServer("accept");
const { registerDeleteTool } = await import("../../src/tools/harness-delete.js");
registerDeleteTool(gitopsServer, gitopsRegistry, client, makeConfig());

const result = await gitopsServer.call("harness_delete", {
resource_type: "gitops_agent",
resource_id: "shared-agent",
});

expect(result.isError).toBe(true);
expect(parseResult(result)).toMatchObject({
error: expect.stringContaining("resource_scope"),
});
expect(mockRequest).not.toHaveBeenCalled();
});

it("returns structured delete payload without spreading API fields at top level", async () => {
const templateRegistry = new Registry(makeConfig({ HARNESS_TOOLSETS: "templates" }));
const templateServer = makeMcpServer("accept");
Expand Down
Loading