Repository navigation
feat: support --set/--del on update trigger - #308
Conversation
Drop the YAML-node toggle handler and its commands; they are re-added as plain endpoint commands on the generic update flow.
Re-add update trigger:enable and trigger:disable as endpoint commands that set trigger.enabled with body_params. Allow content_type on endpoints with an update_strategy.
|
CLI Grammar Review · session (Harness team members only) · cab8b4f CLI grammar changes (beta) · Advisory only, FYI, not required changes.
Overall: 🟡 Recommend manual review |
![augmentcode[bot]](https://avatars.githubusercontent.com/in/1027498?s=60&v=4){kind=small}
| func YamlParse(s string) map[string]any { | ||
| var out map[string]any | ||
| if err := yaml.Unmarshal([]byte(s), &out); err != nil { | ||
| return nil |
There was a problem hiding this comment.
YamlParse returns a typed nil map for invalid or empty YAML, which passes EvalExprAny's out == nil check and is copied as null by jsonCopyMap. Consequently, update trigger:enable can proceed to PUT a document containing only trigger.enabled instead of aborting when the GET response's YAML cannot be parsed.
Severity: medium
🤖 Was this useful? React with 👍 or 👎
| contentType := resolveContentType(ep, method) | ||
| var updateBody any = body | ||
| if contentType == "application/yaml" { | ||
| out, err := yaml.Marshal(integralFloatsToInts(body)) |
There was a problem hiding this comment.
An untouched YAML integer such as 9007199254740993 has already been rounded to 9007199254740992 by jsonCopyMap before integralFloatsToInts runs, so this conversion cannot preserve the original value. An otherwise unrelated field edit can therefore silently change numeric configuration in the full YAML PUT.
Severity: medium
🤖 Was this useful? React with 👍 or 👎
| path: /pipeline/api/triggers/{{lastPart(ctx.id)}} | ||
| file_body: required | ||
| update_strategy: get-then-put | ||
| update_body_pick: yamlParse(it.data.yaml) |
There was a problem hiding this comment.
This bundles the new core yamlParse/YAML-update feature with pipeline-module adoption and workflow removal, contrary to the root AGENTS.md “PR scope: core vs module” rule. These are new module behaviors rather than compatibility edits, so the stated exception does not apply and the required separate core/module review boundary is lost.
Severity: low
🤖 Was this useful? React with 👍 or 👎
|
cosmos review |
What
yamlParseexpr function;get-then-putnow sends the mutated object as YAML whencontent_typeisapplication/yaml.update triggersupports--set/--add/--delonname,descriptionandtags(-fstill works).update trigger:enable|disablereimplemented as endpoint commands (body_params); the Go workflow is removed.content_typeon endpoints with anupdate_strategy.Notes
Test
go test ./pkg/... ./modules/pipeline/...Resolves: #299