feat(update): support JSON Patch operations for token-efficient edits

[revanth-traceable]

Description

Adds a second mode to harness_update: pass operations (RFC 6902 JSON Patch) to change specific fields without resending the full resource. The tool fetches the current resource, applies the patch server-side, and sends the merged result. body (full replacement) and operations are mutually exclusive; dry_run previews the diff without writing.

This is similar to kubectl patch (one of the supported modes):

kubectl patch deployment web-app -n production --type='json' \
  -p='[{"op":"add","path":"/spec/template/spec/containers/0/env/-","value":{"name":"NEW_VAR","value":"test"}}]'

Patch mode is intentionally limited to YAML-backed resources whose registry metadata declares safe mutable-body extraction: pipeline, pipeline_v1, input_set, template, and template_v1. Unsupported non-YAML resources are rejected because many read shapes differ from write shapes. Git-backed metadata such as lastObjectId, lastCommitId, and storeType is preserved for the update call.

Why — token efficiency

Full-body updates force the agent to regenerate the entire resource on every edit, which consumes a lot of output tokens. With this PR, the agent reads once and can patch with minimal output tokens.

Measured on a 526-line pipeline, a multi-part edit:

Mode	Output tokens (generated)
Full body replacement	~5,000+
JSON Patch operations	~500

~11x less generated text for a single edit, and the gap widens with each additional edit since patch size stays flat while full-body cost scales with resource size.

Example

{
  "resource_type": "pipeline",
  "resource_id": "ces_build_lib",
  "url": "https://app.harness.io/ng/account/SL32ke39QeKMAVwtGMAgow/all/orgs/default/projects/revanthcstesting/pipelines/ces_build_lib/pipeline-studio/?storeType=INLINE",
  "confirm": true,
  "operations": [
    {
      "op": "replace",
      "path": "/pipeline/stages/0/stage/spec/execution/steps/0/step/spec/command",
      "value": "PROJECT_VERSION=$(./mvnw --batch-mode -q help:evaluate -Dexpression=project.version -DforceStdout)\nexport PROJECT_VERSION\necho \"Project Version: ${PROJECT_VERSION}\"\n"
    },
    {
      "op": "add",
      "path": "/pipeline/stages/0/stage/spec/execution/steps/0/step/spec/outputVariables/3",
      "value": { "name": "DOCKER_IMAGE_FULL" }
    },
    {
      "op": "replace",
      "path": "/pipeline/stages/0/stage/spec/execution/steps/3/parallel/0/stepGroup/steps/0/step/spec/command",
      "value": "echo \"Starting Maven Build...\"\n./mvnw --batch-mode clean compile -DskipTests\n"
    }
  ]
}

Array paths use numeric indices per RFC 6901; to guard against index drift, precede a replace/remove on an array element with a test op asserting that element's identifier.

Type of Change

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

Checklist

• Tests pass
• Typecheck passes

[thisrohangupta]

Re-reviewed PR #288 at current head 126195f. I’d still block on these:

• Critical src/tools/harness-update.ts:54, src/tools/harness-update.ts:102
 dry_run is exposed on harness_update, but full-body mode ignores it and proceeds to the real update path. body + dry_run: true can still mutate resources if confirmed. Fix by rejecting dry_run unless operations is provided, or implement true full-body preview.

• Critical src/utils/json-patch.ts:52, src/tools/harness-update.ts:181
 Patch mode treats non-YAML GET output as the update body. That is unsafe for resources whose read shape differs from write shape. Example: pull_request GET is passthrough, while update has special state routing in src/registry/toolsets/pull-requests.ts:11 and src/registry/toolsets/pull-requests.ts:120; patching a PR title can accidentally include state/read-only fields and route/reject incorrectly. Use per-resource mutable-body projectors or start with a narrow YAML-backed allowlist.

• Important src/utils/json-patch.ts:20
 template_v1 is omitted from YAML-backed handling, so nested patches operate on the response envelope/string instead of parsed template_yaml. template_v1 update expects template_yaml via src/registry/toolsets/templates.ts:81.

• Important src/tools/harness-update.ts:152, src/tools/harness-update.ts:202
 Patch-mode audit always records confirmation: "auto_approved" even when the user confirmed or elicitation approved. Pass through the actual elicit.method.

• Test gap
 New tests cover helper functions only. Add handler/registry tests for harness_update patch mode: pipeline/template YAML, template_v1, pull_request or another custom bodyBuilder, dry_run, and audit confirmation.

Verification run:
• pnpm test tests/utils/json-patch.test.ts passed, 32 tests.
• pnpm typecheck passed.
• pnpm test tests/tools/tool-handlers.test.ts tests/registry/registry.test.ts passed, 202 tests.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ revanth-traceable
✅ thisrohangupta
❌ cursoragent
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[cursor]

Stale comment

Found 2 issues against Sunil's architecture standards:

1. src/tools/harness-update.ts: the new JSON Patch path does not preserve the existing v0 template fallback for version_label, so harness_update(..., operations=[...]) can fail unless callers know to pass params.version_label. The full-body path still defaults to v1, so this is a behavior regression in the new patch flow.
2. src/registry/types.ts: the new patchSupport registry metadata never reaches harness_describe / Registry.describe(). That breaks the repo's structured-discovery model, because agents still cannot tell which resource types accept operations without trial-and-error.

Assumption: I’m treating JSON Patch support as part of the agent-facing contract because this PR introduces registry metadata for it rather than keeping the allowlist purely internal.

Verification:

• GitHub CI on d9ceade is green (build-and-test on Node 20/22 plus all smoke-test jobs).
• Local: pnpm typecheck
• Local: pnpm build
• Local: pnpm test tests/utils/json-patch.test.ts tests/tools/tool-handlers.test.ts
• Local: pnpm test (1604 tests passed)

Change summary: this PR adds JSON Patch support to harness_update, introduces registry-level patchSupport metadata for YAML-backed resources, and adds focused handler/unit coverage for the new flow.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

Patch mode no longer mirrors the existing v0 template fallback here. handleFullBodyUpdate() defaults resource_type === "template" to version_label = "v1", but this path never does that before the eventual update dispatch. Because template.update uses /template/api/templates/update/{templateIdentifier}/{versionLabel}, a patch call like harness_update({ resource_type: "template", resource_id: "...", operations: [...] }) will fail unless the caller already knows to pass params.version_label, even though the full-body path still works without it.

[cursor]

Adding patchSupport as registry metadata is the right direction, but Sunil's standards prefer agent guidance to live in structured metadata that harness_describe exposes. Right now Registry.describe() / harness_describe never surface this field, so agents still cannot discover which resource types support operations vs full body without trial-and-error. Can we thread this through the describe surface in the same PR?

[cursor]

Stale comment

Found 3 issues against the requested review axes.

• Important — src/tools/harness-update.ts, tests/tools/tool-handlers.test.ts: JSON Patch updates for template are not backward-compatible with the existing implicit version_label="v1" update path. Trigger: call harness_update(resource_type="template", resource_id="...", operations=[...]) without params.version_label. Result: patch mode fails with Missing required field "version_label", while the legacy body-based update still succeeds. This breaks compatibility with existing update semantics and the new tests never exercise the omitted-version_label case.
• Important — src/registry/types.ts, src/registry/toolsets/pipelines.ts, src/registry/toolsets/templates.ts: the new patchSupport metadata is runtime-only. Trigger: an agent calls harness_describe(resource_type="pipeline") or resource_type="template" to discover whether operations is safe. Result: describe output omits patch capability entirely, so agents cannot tell which resource types support JSON Patch. That violates the repo’s discovery standard to surface agent guidance as structured metadata rather than hidden runtime state.
• Important — src/tools/harness-update.ts, tests/tools/tool-handlers.test.ts, tests/utils/json-patch.test.ts: the new operation schema does not encode RFC 6902 field requirements. Trigger: send replace/add/test without value, or move/copy without from. Result: the request is accepted, confirmation runs, and a GET is made before fast-json-patch throws. That falls short of the project’s Zod-first type-safety standard and the new tests only cover utility-level failures, not handler-level schema rejection.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

Important: the patch path never normalizes version_label for template, while handleFullBodyUpdate() still falls back to "v1" when callers omit it. That means harness_update(resource_type="template", resource_id="my_tpl", operations=[...]) now fails with Missing required field "version_label" even though the legacy body-based call still succeeds. I reproduced this locally against the built handler: the patch path errors, while the full-body path returns success. There's also no regression test for the omitted-version_label case in tests/tools/tool-handlers.test.ts.

[cursor]

Important: adding patchSupport here does not help agents unless the capability is exposed through Registry.describe() / harness_describe. Right now a caller asking harness_describe(resource_type="pipeline") still gets no signal that pipeline/input_set/template are patchable while pull_request is not, so this behavior-changing metadata is effectively hidden. That violates the repo's data over prose discovery rule.

[cursor]

Important: this schema makes value and from optional for every op, so malformed RFC 6902 payloads pass tool validation and only fail after confirmation plus a GET round trip. For example, operations:[{op:"replace", path:"/pipeline/name"}] currently reaches the handler, fetches the pipeline, and then dies inside fast-json-patch. Given the project's Zod-first typing standard, these requirements should be enforced at the tool schema boundary, and the new tests don't cover that handler-level validation path.

[cursor]

Stale comment

Found 3 issues that keep this from meeting Sunil's architecture standards:

1. The new JSON Patch path for template updates does not preserve the existing implicit version_label fallback, so the alternate mutation mode is not behaviorally compatible with the legacy full-body path.
2. patchSupport is added to registry metadata but never reaches harness_describe, which leaves patchability discoverable only through prose instead of structured metadata.
3. The operations input schema is too permissive for RFC 6902, so malformed patch payloads make it past tool validation and only fail after confirmation plus a GET.

Verification run:

• pnpm exec vitest run tests/tools/tool-handlers.test.ts tests/utils/json-patch.test.ts
• pnpm build

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

handleFullBodyUpdate() still preserves the legacy template convenience fallback by setting input.version_label = "v1" when callers omit it, but the new patch path copies getInput straight into updateInput and never applies the same normalization. That means resource_type: "template" operations-based updates can now fail in Registry.dispatch() with Missing required field "version_label" for template, even though the equivalent full-body update still works. The alternate mutation mode should preserve the existing default or recover the version from the GET response before dispatching the update.

[cursor]

Adding patchSupport only in ResourceDefinition leaves the new capability trapped in server-side metadata. harness_describe() / Registry.describe() never expose this, so agents still have no machine-readable way to discover which resource types support operations and which ones require full-body updates. That cuts against the repo's data over prose guidance: the runtime knows the capability, but the discovery surface does not. Please surface patch capability metadata through describe output and cover it with a regression test.

[cursor]

This schema is too permissive for RFC 6902. Because value and from are optional for every operation, malformed payloads like { op: "replace", path: "/pipeline/name" } or { op: "move", path: "/x" } clear tool validation, prompt for confirmation, and even do the GET before fast-json-patch rejects them. That violates the repo's validate-early / fail-loudly standards and makes bad requests more expensive than they need to be. A discriminated union per op would reject these before any confirmation or network work.

[cursor]

Stale comment

Found 3 architecture-standard issues in the current head.

1. Important: the new JSON Patch path round-trips YAML through YAML.parse() / YAML.stringify(), so a one-field patch on a Git-backed pipeline/template/input set still rewrites the full YAML text and drops comments/original formatting. That is not minimal-impact behavior for a feature positioned as targeted edits.
2. Important: JSON Patch operation shape is validated up front, but path / from are still plain strings. Malformed pointers can get through the MCP schema and only fail after the GET/apply path, which misses the repo's fail-loud / fully-typed-input bar.
3. Suggestion: harness_describe advertises patchSupport.dryRun, while the actual public tool input is dry_run. That makes the structured discovery metadata disagree with the runtime contract.

Open question: if preserving YAML comments/format is intentionally out of scope for the first cut, I would at least scope the feature/docs to make it explicit that JSON Patch here is semantic-only and may rewrite the underlying YAML text.

Verification run on 38f2a28adc81aa10bb9b272b92c30479e52ca31a:

• pnpm test
• pnpm typecheck
• pnpm build

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

This patch path always parses then stringifies YAML, so a single field update rewrites the full YAML text and strips comments/original formatting. For Git-backed pipelines/templates/input sets that breaks the repo's Minimal Impact goal and can silently drop human-authored inline docs. Can we preserve the original YAML structure/text for untouched nodes, or explicitly scope the feature away from comment-preserving edits?

[cursor]

jsonPointerSchema is only z.string(), and from below is also an unconstrained string. That means malformed pointers like pipeline/name pass the registered MCP schema and only fail later inside fast-json-patch after the GET/apply path. For this repo's fail-loud / fully-typed-input standard, I'd validate RFC 6901 syntax here and reuse the same schema for from.

[cursor]

patchSupport is part of the structured harness_describe surface, but this advertises dryRun while the actual public tool input is dry_run. Agents following discovery metadata will synthesize the wrong key here, so the metadata should match the runtime contract exactly (or expose both if an alias is intentional).

[cursor]

Stale comment

1. src/tools/harness-update.ts still has one remaining fail-loud gap: operations accepts an empty array, so harness_update(..., operations=[]) gets past handler validation and only errors later inside applyJsonPatch() after the confirmation/GET path. That misses the repo's "validate before dispatch" standard for the new patch mode.

Assumptions:

• Re-reviewed the current head (38f2a28a), not the earlier failing snapshot.
• The earlier JSON-schema/startup regression is fixed on this head and the fresh smoke tests are green.

Change summary:

• The latest fixes brought the PR much closer to Sunil's architecture standards: patch capability is now exposed through harness_describe, op-specific required fields are validated before dispatch, and the MCP schema is exportable again.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

patchOperationsSchema still allows operations: [], so an empty patch survives handler validation and only dies later in applyJsonPatch() after confirmation + the GET round trip. Since this PR explicitly moved malformed-op checks to the tool boundary, I'd either add .min(1) here or treat an empty array as an explicit no-op so the tool fails loud before dispatch.

[cursor]

Stale comment

Findings

1. Important: harness_describe now exposes patchSupport using internal/camelCase metadata instead of the public harness_update contract. In particular, it advertises dryRun instead of dry_run, and it forwards raw bodyFields values that can include extractor-only names such as yaml for pipeline_v1. That creates a discovery/runtime mismatch for agents.
2. Important: the JSON Patch path for YAML-backed resources reparses and re-stringifies the entire document, so even a one-field patch rewrites untouched YAML text. For Git-backed pipelines/templates/input sets, that will drop comments and normalize unrelated formatting, which conflicts with the repo's Minimal Impact guidance for targeted edits.

Open Questions / Assumptions

• Assuming harness_describe is intended to be the source of truth for public tool argument names, not internal projector fields.
• Assuming patch mode is expected to preserve human-authored YAML comments/formatting for Git-backed resources. If canonical rewrites are explicitly acceptable, the second finding becomes a design trade-off rather than a blocker.

Change Summary

• Adds RFC 6902 support to harness_update, registry patchSupport metadata, and focused handler/utility tests.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

Because harness_describe now emits this object verbatim, it needs to match the public harness_update contract rather than raw projector metadata. Two mismatches here:

• the preview flag is dry_run, not dryRun
• bodyFields can leak extractor-only names; for example pipeline_v1 now advertises yaml, but the public update path only accepts pipeline_yaml / yamlPipeline

As written, discovery can teach agents calls that the handler will not honor.

[cursor]

This implementation cannot preserve untouched YAML text: extractMutableBody() parses the source into a plain JS object, and serializeBody() always emits a fresh YAML.stringify(...). For Git-backed pipelines/templates/input sets, that means a one-field patch will still drop comments and normalize unrelated formatting/quoting across the file.

That seems at odds with the repo's Minimal Impact standard for targeted edits. If canonical rewrites are acceptable, I'd expect that trade-off to be explicit in the contract/tests; otherwise this likely needs a text- or AST-preserving update path.

[cursor]

Stale comment

Found 1 architecture issue on the latest PR head (265505a4002817b77833adf3a4801bd104efe59e).

1. Medium: JSON Patch rewrites the full YAML document and drops untouched comments/formatting on REMOTE resources. The implementation parses the current YAML into an object and reserializes it with YAML.stringify(...) before calling update, so a targeted patch can still rewrite the whole file and remove user-authored annotations. That conflicts with the repo's Minimal Impact guidance, especially because this PR explicitly documents/supports git-backed patch flows.

Assumption: I'm treating comments and untouched YAML formatting in git-backed resources as user-visible data that should not be discarded by a targeted edit mode.

Verification:

• gh pr checks 288 --json name,bucket,state,workflow,link shows all CI jobs passing on this head.
• Local repro with the same yaml@2.8.3 dependency used by the PR: YAML.parse() followed by YAML.stringify({ lineWidth: 0 }) removes comments entirely, matching the behavior in this implementation.

I did not find additional architecture-standard violations in the updated metadata/validation path after the follow-up fixes.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

This parse -> patch -> YAML.stringify() round-trip means operations mode rewrites the entire YAML text, not just the targeted fields. For REMOTE/git-backed resources that the PR explicitly supports, that will drop comments and normalize untouched formatting/quoting in the committed file. That feels out of bounds for the repo's Minimal Impact standard: a token-efficient targeted edit can still create a broad textual diff and lose human-authored annotations.

Can we either preserve the YAML document/CST here, or fail closed for Git-backed resources until we can patch without rewriting unrelated text?

[revanth-traceable]

I think there is no way to add comments in harness pipeline yaml. It tried via UI and git and next update from UI removes the comments

cc: @thisrohangupta

[cursor]

Found 2 actionable issues in the new JSON Patch update path.

Focused verification on the PR head:

• pnpm test tests/utils/json-patch.test.ts tests/tools/tool-handlers.test.ts
• pnpm typecheck

Those suites are green, but they do not cover the two edge cases below.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

Medium – patch mode stops failing loudly on primary-ID conflicts

handleFullBodyUpdate() still rejects resource_id/URL-vs-params.<primaryId> mismatches, but patch mode now merges params first and then unconditionally overwrites the primary field here. Because URL IDs are normalized into args.resource_id earlier, a call like resource_id="pipe-from-id" plus params.pipeline_id="pipe-from-params" succeeds against pipe-from-id instead of returning the existing Conflicting identifiers error.

I reproduced this on the built PR head: the handler fetched and updated pipe-from-id while silently discarding the conflicting params.pipeline_id. That can patch the wrong resource while hiding a bad agent call.

[cursor]

Low – the params-only ID path leaks undefined into confirmation/audit metadata

This prompt interpolates args.resource_id before the primary identifier is resolved from params, so a supported call like harness_update({ resource_type: "pipeline", params: { pipeline_id: "my-pipe" }, operations: [...] }) asks to patch pipeline "undefined" even though the request succeeds.

The same unresolved value is later passed into the update audit context, so the params-only path is not wired cleanly end-to-end through the user-facing prompt and audit metadata.

[cursor]

Found 2 issues against the harness_update contract in the new JSON Patch flow.

1. Medium: patch mode no longer fails loudly on conflicting primary identifiers the way full-body update mode does.
2. Low: params-only patch calls lose the resolved resource ID in the confirmation/audit path.

Validation run locally:

• pnpm typecheck
• pnpm test -- tests/utils/json-patch.test.ts tests/tools/tool-handlers.test.ts (this executed the full suite locally: 1604 tests passed)

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

handleFullBodyUpdate() explicitly rejects a mismatch between resource_id/URL and params.<primary-id>, but patch mode doesn't do that anymore. Here we merge params, then applyUpdateInputDefaults() silently overwrites the primary identifier from args.resource_id instead of surfacing a conflict. That violates the repo's "fail loudly" rule and makes caller mistakes hard to spot. Please run the same conflict check in patch mode before the GET so conflicting identifiers fail instead of being silently chosen.

[cursor]

Patch mode never resolves the effective primary ID before this prompt, so callers that use the documented params-only identifier flow get "undefined" in confirmation text. The unresolved args.resource_id is also what gets passed into the later audit metadata, so those requests lose the resource identifier in logs as well. Please resolve the ID from params first, the same way full-body mode does, and reuse that resolved value for both confirmation and audit dispatch.

[cursor]

1 Important finding:

• Patch mode can replace the whole resource with a scalar/array because root JSON Pointers are allowed and the patched document is never validated back to object shape before serialization. That weakens the new write-safety contract for harness_update operations mode.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

Important: RFC 6901 allows path: "" to target the document root, and fast-json-patch accepts root replacement. Because we never validate patched after applyJsonPatch(), an operation like { op: "replace", path: "", value: 1 } will serialize to scalar YAML and get forwarded to update, turning a supposedly targeted edit into an unsafe full-body write. Please reject root-level replacement/removal or enforce that the patched result remains a non-array object before serializeBody(). There’s also no regression test covering this edge case.

[hingerabhinav]

@revanth-traceable Can you check this?

[revanth-traceable]

Addressed this in the latest commit