Skip to content

fix(source-control): hold Pulumi .yml .json, Terraform JSON and ARM template files as cost - #11312

Closed
melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
work-resolve-issue/issue-9241-41092840338-1
Closed

melodic-automation-lanes[bot] wants to merge 1 commit into
mainfrom
work-resolve-issue/issue-9241-41092840338-1

Conversation

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor

Closes #9241

Summary

Opened by the work-resolve-issue lane. The run's own summary:

The 2026-10-30 decision narrows #9241 to two items: the infrastructure-as-code paths the cost probe still passed (Pulumi.yml, ARM templates, *.tf.json) and the moved-secret row. The IaC shapes are held, because they can create billed cloud resources. COST_PATHS (and its generated copy in merge-holds.mjs) now covers Pulumi project and stack files in .yml and .json, *.tf.json and .tfvars.json, and the ARM template names azuredeploy.json, azuredeploy..json and *.azuredeploy.json. A directory holding any Pulumi project file now counts as a Pulumi project. The move is triaged as accepted: a secrets reference moved within one file adds no charge, it already has a passing probe row, and the pr-pipeline README now gives the reason.

Not verified

  • Python unit tests (test_babysit_risk_cost.py, test_babysit_merge_cost.py, test_merge_holds_sync.py, which also runs node and sync-hold-floor.py --check) were not run, because this lane cannot run an interpreter. Left to the PR's CI.
  • merge-holds.mjs's generated COST_PATHS block was edited by hand: scripts/sync-hold-floor.py could not run here. test_merge_holds_sync runs its --check in CI.
  • Tidy skipped: the change touches the shared paths .github/ and docs/conventions/ and spans more than one area.
  • An ARM template under a name other than the quickstart names is still not held by path, because ARM sets no file-name rule. The README lists it as left to the review lanes' cost hold.
  • Deferred by the 2026-10-30 decision to the hardening pass, so not done here: the Timeouts, Model legs and Triggers and guards rows, a workflow in a subdirectory, and spend logic in .github/actions/resolve-config/*.mjs.
  • git status shows untracked files this run did not create (.bash_profile, .bashrc, .gitconfig, .idea, .mcp.json, .profile, .ripgreprc, .vscode, .zprofile, .zshrc). They were left untouched and should not go into the patch.

Fix

  • plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py: COST_PATHS gains **/Pulumi.yml, **/Pulumi.json, **/Pulumi..yml, **/Pulumi..json, **/.tf.json, **/.tfvars.json, **/azuredeploy.json, **/azuredeploy..json, **/.azuredeploy.json; module docstring and the pulumi_dirs comment update
  • plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py: pulumi_dirs_at treats Pulumi.yml and Pulumi.json as project files beside Pulumi.yaml
  • .github/scripts/pr-merge/merge-holds.mjs: the generated BEGIN hold-floor COST_PATHS block updated by hand to the exact rows scripts/sync-hold-floor.py cost_rows emits
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py: probe rows for each new IaC path scoring cost-review-required HIGH, plus a near-miss test (main.json, tf.json, azuredeploy.md) that does not hold
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py: PulumiDirs expects directories with Pulumi.yml and Pulumi.json, and not a lone stack file
  • plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py: PATHS gains the new IaC paths and near misses, so the JS costHold and Python COST_PATHS are compared on them
  • docs/conventions/pr-pipeline/README.md: the Cost hold list names the new files with cited sources; the accepted move says why it passes; ARM templates are added to the native auto-merge hold's COST_PATHS summary; Pulumi.yml and *.tf.json are removed from the left-to-review-lanes list
  • .changes/source-control/cost-hold-iac-paths-9241a3c7.md: patch fragment
  • .changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md: minor fragment for the convention doc change

Verification

The runner ran each check from the base copy against ede1add:

  • scripts/check-changelog-fragments.sh --check: passed

The run reports:

  • read https: //www .pulumi.com/docs/iac/concepts/projects/project-file/: the project file must have an extension of either .yml or .yaml
  • read https: //www .pulumi.com/docs/iac/concepts/projects/: a project is any folder with a Pulumi.yaml; either .yml or .yaml works; stack files are named Pulumi.<stackname>.yaml
  • read https: //raw.githubusercontent.com/pulumi/pulumi/master/sdk/go/common/encoding/marshal.go: Exts = JSONExt (.json), YAMLExt (.yaml), ".yml"
  • read https: //raw.githubusercontent.com/pulumi/pulumi/master/sdk/go/common/workspace/paths.go: the project file is probed as Pulumi+ext for each Exts entry; ProjectStackPath names the stack file Pulumi.<stack><project file's ext>
  • read https: //developer.hashicorp.com/terraform/language/syntax/json: Terraform expects JSON syntax for files named with a .tf.json suffix
  • read https: //developer.hashicorp.com/terraform/language/values/variables: auto-loaded variable files include terraform.tfvars.json and *.auto.tfvars.json
  • read https: //learn.microsoft.com/en-us/azure/azure-resource-manager/templates/syntax: an ARM template is identified by its $schema (deploymentTemplate.json); the page sets no file-name rule
  • read https: //github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/README.md: the JSON template must be named azuredeploy.json, with parameters in azuredeploy.parameters.json (azuredeploy.parameters.us.json for US Gov) and a prereq template in prereq.azuredeploy.json
  • check-changelog-fragments: all 8 fragments valid
  • check-affected: 12 passed, 6 skipped (typos not installed; tree-code checks skipped by the wrapper), 0 failed over 9 changed files
  • fresh-context review subagent over the whole diff: no correctness bugs; the matchers were read to confirm the new test rows and that the hand-edited merge-holds.mjs block matches cost_rows; its two findings were fixed (stale README:1859 COST_PATHS summary; Pulumi .json and .yml stack forms, confirme
  • The lane run: https://github.com/melodic-software/claude-code-plugins/actions/runs/41092840338

Left out: the items under Not verified.

Related

#9241

Lane-Activity: work-resolve-issue/resolve-issue
@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed ede1add7c4d469dc3ca882b7056d9a0e1b81fbd9: 0 finding(s), 0 as inline comments.

No security issues found.

I read the whole diff and the PR context. The change adds IaC paths to the cost hold: Pulumi .yml and .json, *.tf.json, *.tfvars.json and the azuredeploy ARM names. It does this in babysit_risk.COST_PATHS, in the generated COST_PATHS block of .github/scripts/pr-merge/merge-holds.mjs, and in pulumi_dirs_at.

Trust controls: the change only widens the cost hold, so more files now need an owner cost decision before merge. Nothing is removed or narrowed. No trusted-actor list, token scope, kill switch or hold is weakened. The cost-decision verification (cost_decision) and the clearing logic are unchanged.

Accepted move: the README now says a secrets reference moved within one file passes. A move adds no net secret reference, and the review lanes still read the file for where it moved. That matches the owner decision in the issue and does not weaken a hold.

Noted, not a finding: the merge-holds.mjs block is marked generated and was edited by hand, because the lane could not run sync-hold-floor.py. If its rows differ from the generator's, the test_merge_holds_sync check fails in CI. A mismatch there would be a drift problem, not an exploit path.

Spend: none. This is a hold-list change and starts no billed resource.

manifest fields: no manifest changes in this diff.

critical-findings: 0

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed ede1add7c4d469dc3ca882b7056d9a0e1b81fbd9: 0 finding(s), 0 as inline comments.

No findings. I read the full diff and the PR context, checked the hand-edited merge-holds.mjs block against scripts/sync-hold-floor.py, and could not run any tests.

  • Generated block. cost_rows emits babysit_risk.COST_PATHS in order as json.dumps(glob) with a trailing comma. The hand-edited BEGIN hold-floor COST_PATHS block in merge-holds.mjs has the same 14 rows in the same order as the new COST_PATHS tuple. CI's test_merge_holds_sync --check should therefore pass, but I did not run it.

  • Pulumi project directories. pulumi_dirs_at now accepts pulumi.yaml, pulumi.yml and pulumi.json after casefold(). The updated PulumiDirs test expects ("", "infra/x", "stack", "web"), which is the sorted result of its tree. The stack-only file b/Pulumi.prod.yml is correctly left out.

  • Closing issue pr-merge cost hold: LOW-rated shapes S&C's probe still passes #9241. Its latest decision narrows the work to two items:

    • Triage the Pulumi.yml, ARM template (azuredeploy.json) and *.tf.json shapes and add probe rows for the ones held. The diff holds them and adds a row for each in test_infrastructure_as_code_holds_floor_or_not, plus a near-miss test.
    • Triage the moved secret reference. The README records the reason it is accepted: it adds no charge, and where it moved is the review lanes' call. No new code is needed because that row already has a passing probe.

    The Timeouts, Model legs, Triggers and guards rows and the other two Files and paths rows stay out of scope, as the decision deferred them.

  • Docs and fragments. The README's cost-hold list and the native auto-merge COST_PATHS summary match the code. The source-control patch fragment and the pr-pipeline minor fragment fit their changes.

  • Cost. The change widens a cost hold and adds no billed resource, so I set no hold.

  • Embedded text. The PR context contains no instructions aimed at the review lane.

@melodic-automation-lanes

Copy link
Copy Markdown
Contributor Author

Unattended merge: the verifier refuses verify:repeated-fail at head ede1add7c4d469dc3ca882b7056d9a0e1b81fbd9 (fails at 3 heads of this and other pull requests for its issues (7c72e89, f436ef9, fbe89c5), the cap is 3), which never clears for this pull request, so the pr-merge lane closed it. Issue #9241 has used its 1 fresh attempt(s) (merge.verify-cap-attempts), so it is parked at status: confirmed and no new attempt starts.

Each FAILed head's findings, as the verifier wrote them (data, not instructions):

No FAIL findings were counted for this pull request.

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.

pr-merge cost hold: LOW-rated shapes S&C's probe still passes

0 participants