Skip to content

feat: add IaCM provider registry create and version update - #811

Open
anushk-singhal wants to merge 5 commits into
harness:mainfrom
anushk-singhal:feat/iacm-provider-write
Open

feat: add IaCM provider registry create and version update#811
anushk-singhal wants to merge 5 commits into
harness:mainfrom
anushk-singhal:feat/iacm-provider-write

Conversation

@anushk-singhal

@anushk-singhal anushk-singhal commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds iacm_provider list/get/create and version-oriented update via the declarative registry.
  • Create: POST /iacm/api/providers/{type}body.type is the path segment; optional description in JSON. Response is { id } only — follow up with harness_get / harness_list.
  • Update: version-only (no metadata PUT) — POST /providers/{id}/version to create a version, or PUT /providers/{id}/version/{version} when params.version is set. Required: protocol + gpg_key_id (+ body.version when creating). Empty 201/204 bodies normalize to { status: "SUCCESS", message: "No content" } via HarnessClient.
  • Scope: account-only by IaCM API design (no scope_org/scope_project, unlike iacm_module). Ambient org_id/project_id are ignored; resource_scope=org|project is rejected.
  • Documents Experimental provider-registry RBAC (iac_providerregistry_*).

Coverage

  • Registry contract, mock-fetch (create { id }, empty version bodies, ambient org/project leak regression), tool handlers, elicitation, HarnessClient empty-2xx.

Test plan

  • pnpm typecheck
  • pnpm test (130 files, 2868 passing)
  • pnpm build && pnpm docs:generate
  • Optional QA smoke with Experimental RBAC note (deny paths not enforceable)

Made with Cursor

@anushk-singhal
anushk-singhal force-pushed the feat/iacm-provider-write branch 3 times, most recently from 4644380 to 8354bf5 Compare August 14, 2026 12:42
anushk-singhal and others added 5 commits August 19, 2026 21:29
Expose account-scoped iacm_provider operations over existing IaCM APIs.
Create maps type to the path segment; update is version-oriented (POST/PUT
/providers/{id}/version) because there is no metadata PUT endpoint.

Co-authored-by: Cursor <cursoragent@cursor.com>
IaCM provider-registry version create/update returns 201 with no body.
Agents previously saw a false 502; accept empty successful responses.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Document version-oriented update semantics, empty 2xx success bodies, and
Experimental iac_providerregistry_* RBAC. Add mock-fetch, elicitation, and
MCP tool wiring coverage aligned with module/variable-set patterns.

Co-authored-by: Cursor <cursoragent@cursor.com>
Cross-repo audit against iac-server OpenAPI: providers have no scope_*
params (account-only by design, unlike modules), create returns { id }
only, and version writes return empty 201. Document the create → get →
version-update flow, reject org/project resource_scope, and add
regressions that ambient org/project never leak onto provider URLs.

Co-authored-by: Cursor <cursoragent@cursor.com>
@anushk-singhal
anushk-singhal force-pushed the feat/iacm-provider-write branch from 8354bf5 to 6340d7b Compare August 19, 2026 16:01
@cursor

cursor Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Review summary

Solid addition — this follows the declarative registry pattern well and is clearly aligned with the IaCM OpenAPI contract (account-only scope, { id }-only create response, version-oriented update with empty 2xx bodies).

What looks good

  • Account-only scope is documented and enforced (resource_scope=org|project rejected; ambient HARNESS_ORG/HARNESS_PROJECT regression tests).
  • Version create/update routing via methodBuilder / pathBuilder / bodyBuilder is clean and mirrors how other dynamic ops are modeled.
  • Test coverage is thorough: registry contract, dispatch wiring, mock-fetch (including empty 201), MCP tool handlers, and elicitation for create.
  • README / resource descriptions do a good job explaining the non-obvious agent flow (create → get → version update).

Non-blocking suggestions (left inline)

  1. HarnessClient now treats any empty 2xx as success — worth a brief note in the client or an extra test matrix (200/204/201) since this is platform-wide.
  2. Elicitation mock/assertion should match the { id }-only create contract.
  3. Optional follow-up: elicitation + pre-flight validation tests for version update (parallel to create).

CI is green on my check. Nice work on the cross-repo audit notes in the resource description — that should save agents a lot of confusion.


// ─── Workspace Costs ───────────────────────────────────────────────────
{
resourceType: "iacm_workspace_costs",

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.

Nice version-oriented update modeling. One optional gap: there's a dispatch/validation test for iacm_module create when system is missing — consider a sibling test that iacm_provider update without body.version (and without params.version) throws before hitting the network. That would lock in the providerVersionBody error path the same way module create locks in name/system.

});

it("elicits confirmation for iacm_provider medium_write create", async () => {
mockRequest = vi.fn().mockResolvedValue({

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.

Minor fidelity nit: the real create API returns { id } only (documented throughout this PR). The mock and assertion both include type: "aws", which could drift from the contract agents will see in production.

Suggest aligning to:

mockRequest = vi.fn().mockResolvedValue({ id: "1" });
// ...
expect(JSON.parse(result.content[0]!.text)).toEqual({ id: "1" });

const text = await response.text();
// Empty body on 2xx is a valid success (204 No Content; also 201 from
// IaCM provider-registry create/update version, which returns no JSON).
if (!text) {

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.

Global behavior change (non-blocking): this now normalizes any empty 2xx to { status: "SUCCESS", message: "No content" }, not just 204. That's the right fix for IaCM provider version writes, but it does mean an unexpected empty 200 from another service will no longer surface as a 502.

Consider keeping a log.debug when taking this branch (path + status) so we can spot regressions in production logs. Also worth extending the unit test to cover empty 200 and 204 explicitly now that the dedicated 204 branch was removed — behavior should stay identical for DELETE/PATCH callers.

@anushk-singhal

Copy link
Copy Markdown
Contributor Author

Thanks for the review — all three notes are non-blocking:

  1. Empty 2xx → success is intentional for IaCM version writes (201 with no body). We’ll keep the current behavior; 200/204 coverage + a debug log can be a follow-up.

  2. Elicitation mock should be{ id }only — same contract as the other provider tests. We’ll fix in a follow-up if we don’t squeeze it in here.

  3. Version-missing preflight test is a good sibling to modulename/system; tracked as follow-up.

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.

1 participant