Repository navigation
fix: type --max-concurrent-uploads as int, validate input and default to 5 - #344
BlueLabelscotch wants to merge 10 commits into
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) · e1674d8 CLI grammar changes (beta) · Advisory only, FYI, not required changes.
Overall: 🟡 Recommend manual review |
| 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.
The new YAML update path silently changes untouched integers that cannot be represented exactly as float64: yamlParse preserves 9007199254740993, but the preceding jsonCopyMap rounds it to 9007199254740992. integralFloatsToInts cannot recover that lost precision, so even a name edit or enable/disable operation can PUT corrupted values elsewhere in the configuration.
Severity: medium
🤖 Was this useful? React with 👍 or 👎
| expr: it.type | ||
| - id: enabled | ||
| expr: it.enabled | ||
| mutable_path: trigger.enabled |
There was a problem hiding this comment.
Exposing enabled through the default scalar field handler makes --set enabled=false write the YAML string enabled: "false", because mutateScalar returns the operand unchanged and yaml.Marshal preserves its string type. This violates the field's boolean representation and differs from trigger:disable, whose body_params expression produces an actual boolean.
Severity: medium
🤖 Was this useful? React with 👍 or 👎
| description: "Include hidden files and directories (names starting with '.') when walking directory inputs" | ||
| - name: max-concurrent-uploads | ||
| description: "Maximum number of files uploaded concurrently (default: 4, use 1 for sequential)" | ||
| is_int: true |
There was a problem hiding this comment.
This adopts the new core is_int capability in HAR in the same PR that introduces it, and the pipeline edits likewise adopt the new core YAML-update capability. Per root AGENTS.md's “PR scope: core vs module” rule, reinforced by the permanent review memory from PR #308, core features cannot be bundled with module adoption unless the module edits are required for compatibility; these edits add module behavior rather than maintain compatibility.
Severity: low
🤖 Was this useful? React with 👍 or 👎
Fixes #337
is_intflag type to the spec/registry so non-numeric input is rejected by the flag parser--max-concurrent-uploadsas int inhar.spec.yaml(generic, maven, python)