From 1a6b17cac44ab9e74804bb137256f445d68856d8 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:38:46 -0400 Subject: [PATCH 1/2] fix(source-control): hold Pulumi .yml/.json, Terraform JSON and ARM templates as cost The pr-merge cost hold's COST_PATHS matched only Pulumi.yaml, *.tf, *.tfvars and Bicep, so an IaC change in Pulumi.yml, Pulumi.json, *.tf.json, *.tfvars.json or an azuredeploy ARM template merged without the owner's cost decision. Add those globs to babysit_risk.COST_PATHS and its generated copy in merge-holds.mjs, count a directory holding any Pulumi project file as a Pulumi project, and record in the README why a secrets reference moved within one file stays accepted. Closes #9241 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KeBVgauhM9QMc1vRhujf3L --- .../cost-hold-iac-paths-9241a3c7.md | 14 +++++++++++ .../cost-hold-iac-paths-9241a3c7.md | 15 +++++++++++ .github/scripts/pr-merge/merge-holds.mjs | 9 +++++++ docs/conventions/pr-pipeline/README.md | 25 ++++++++++++------- .../scripts/babysit_merge_policy.py | 8 +++--- .../babysit-prs/scripts/babysit_risk.py | 19 +++++++++++--- .../scripts/tests/test_babysit_merge_cost.py | 7 +++++- .../scripts/tests/test_babysit_risk_cost.py | 16 ++++++++++++ .../scripts/tests/test_merge_holds_sync.py | 11 ++++++++ 9 files changed, 108 insertions(+), 16 deletions(-) create mode 100644 .changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md create mode 100644 .changes/source-control/cost-hold-iac-paths-9241a3c7.md diff --git a/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md b/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md new file mode 100644 index 00000000000..d29cb56c1d2 --- /dev/null +++ b/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md @@ -0,0 +1,14 @@ +--- +bump: minor +--- + +### Changed + +- The cost hold section names the infrastructure-as-code files the merge gate now holds: + - Pulumi project and stack files in their `.yml` and `.json` forms. + - `*.tf.json` and `*.tfvars.json`. + - The ARM template names `azuredeploy.json`, `azuredeploy.*.json` and `*.azuredeploy.json`. + - Any file under a directory that holds a Pulumi project file. + + It also says why a `secrets` reference moved within one file is accepted: the move adds no + charge, and where it moved is the review lanes' call. diff --git a/.changes/source-control/cost-hold-iac-paths-9241a3c7.md b/.changes/source-control/cost-hold-iac-paths-9241a3c7.md new file mode 100644 index 00000000000..0570e48c014 --- /dev/null +++ b/.changes/source-control/cost-hold-iac-paths-9241a3c7.md @@ -0,0 +1,15 @@ +--- +bump: patch +--- + +### Changed + +- The `cost-review-required` hold now covers more infrastructure-as-code files: + - Pulumi project and stack files in their `.yml` and `.json` forms (`Pulumi.yml`, `Pulumi.json`, + `Pulumi.*.yml`, `Pulumi.*.json`). + - Terraform's JSON files (`*.tf.json`, `*.tfvars.json`). + - The ARM template names `azuredeploy.json`, `azuredeploy.*.json` and `*.azuredeploy.json`. + + A directory holding any Pulumi project file at the base now counts as a Pulumi project, and + `merge-holds.mjs` carries the same paths. A `secrets` reference moved within one file still + passes, because it adds no charge. diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index 6e6a7a60a6e..bfbfda28c7d 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -202,11 +202,20 @@ export function personWrote(pull) { export const COST_PATHS = Object.freeze([ // BEGIN hold-floor COST_PATHS: generated by scripts/sync-hold-floor.py "**/Pulumi.yaml", + "**/Pulumi.yml", + "**/Pulumi.json", "**/Pulumi.*.yaml", + "**/Pulumi.*.yml", + "**/Pulumi.*.json", "**/*.tf", + "**/*.tf.json", "**/*.tfvars", + "**/*.tfvars.json", "**/*.bicep", "**/*.bicepparam", + "**/azuredeploy.json", + "**/azuredeploy.*.json", + "**/*.azuredeploy.json", // END hold-floor COST_PATHS ]); diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index dafe81cb986..92285e131b4 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1262,9 +1262,15 @@ the rules that only tighten in `cost-review-required` in both lanes, whatever `high-path-floor` says and whatever an adopted author's own paths leave off the floor, and waits for a human like any other HIGH reason. It catches: - - any changed `Pulumi.yaml`, `Pulumi.*.yaml`, `*.tf`, `*.tfvars`, `*.bicep` or `*.bicepparam` - file, and any file under a directory that holds a `Pulumi.yaml` at the base. A base tree the gate cannot - list whole holds every file; + - any changed Pulumi project or stack file (`Pulumi.yaml`, `Pulumi.*.yaml` and their `.yml` + and `.json` forms, which + [Pulumi reads too](https://github.com/pulumi/pulumi/blob/master/sdk/go/common/encoding/marshal.go)), + `*.tf`, `*.tf.json`, `*.tfvars`, `*.tfvars.json`, `*.bicep` or `*.bicepparam` file, any ARM + template under the names + [Azure's quickstart samples use](https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/README.md) + (`azuredeploy.json`, `azuredeploy.*.json`, `*.azuredeploy.json`), and any file under a + directory that holds a Pulumi project file at the base. A base tree the gate cannot list whole + holds every file; - a file renamed or copied into `.github/workflows/` or a `pr-pipeline.yaml` path from outside; - a runner value the workflow's head text gains over its base text, or an added inline `runs-on:` value: on a public repository only one that is not a free literal (GitHub's @@ -1325,11 +1331,12 @@ the rules that only tighten in credential, an agent or a write token can reach, not only spend, so a person clears them and no cost decision does. - Accepted: a `secrets` reference moved within one file is net zero and does not hold. Other new - triggers, a removed `concurrency` or job `if:` guard, matrix growth and `max-parallel`, a - non-Claude model action, and paths outside the ones above (a workflow in a subdirectory of - `.github/workflows/`, `Pulumi.yml`, ARM templates, `*.tf.json`) are left to the review lanes' - cost hold, which holds for a real charge (`pr-review-hosted.yml`). + Accepted: a `secrets` reference moved within one file is net zero, so `secret-exposure-review` + does not see it, and it adds no charge, so the cost hold does not either; where it moved is the + review lanes' call. Other new triggers, a removed `concurrency` or job `if:` guard, matrix growth + and `max-parallel`, a non-Claude model action, and paths outside the ones above (a workflow in + a subdirectory of `.github/workflows/`, an ARM template under another name) are left to the + review lanes' cost hold, which holds for a real charge (`pr-review-hosted.yml`). The hold clears only when the pull request body links an owner decision that covers it: a comment on the pull request's linked issue (its closing issue, or its `Refs:` issue as @@ -1852,7 +1859,7 @@ approval lifts no aliasing path, symlink, submodule or mode change (`high-path:* trust-gate, manifest, file-cap or review-lane hold, and a review list or trusted-actor list that does not read lifts nothing. Both also hold a pull request that changes one of the merge gate's cost paths -(`babysit_risk.COST_PATHS`: Pulumi, Terraform and Bicep files). An +(`babysit_risk.COST_PATHS`: Pulumi, Terraform, Bicep and ARM template files). An approval does not lift that hold. Only the owner's cost decision does, read as the gate's `cost_decision` reads it: an unedited comment by a person on the trusted-actor list whose first line opens `**Decision recorded.** Cost:`, on the pull request or on its one closing issue after diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py index 4276826037d..df32787ad32 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py @@ -5238,8 +5238,9 @@ def merged_since_floor( def pulumi_dirs_at(repo: str, pr: Mapping[str, Any]) -> tuple[str, ...] | None: """`babysit_risk.RiskInput.pulumi_dirs`: each directory holding a - `Pulumi.yaml` at the base ("" for the root); None when the base tree - cannot be listed whole.""" + Pulumi project file (`Pulumi.yaml`, `Pulumi.yml` or `Pulumi.json`) at + the base ("" for the root); None when the base tree cannot be listed + whole.""" owner, name = _split_repo(repo) base = str(json_object(pr.get("base")).get("sha")) try: @@ -5257,7 +5258,8 @@ def pulumi_dirs_at(repo: str, pr: Mapping[str, Any]) -> tuple[str, ...] | None: sorted( path.rpartition("/")[0] for path in modes - if path.rpartition("/")[2].casefold() == "pulumi.yaml" + if path.rpartition("/")[2].casefold() + in ("pulumi.yaml", "pulumi.yml", "pulumi.json") ) ) diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py index dbad33add08..95e5f22b9ec 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -57,7 +57,7 @@ README.md, "When work stops for the owner"). A change that could start one (billed infrastructure or billed runner minutes) rates HIGH as `cost-review-required` in every lane, floor or not and whatever `floor_exempt` -says: any `COST_PATHS` file (Pulumi, Terraform, Bicep) or file +says: any `COST_PATHS` file (Pulumi, Terraform, Bicep, ARM templates) or file under a `RiskInput.pulumi_dirs` directory, and every file when `pulumi_dirs` is None (the base tree was not listed); a file renamed or copied into a workflow or `pr-pipeline.yaml` path from outside one; a workflow or pipeline file with @@ -219,8 +219,9 @@ class RiskInput: # The owner decision the caller verified for a `cost-review-required` # change (its comment URL); None keeps the hold. cost_decision: str | None = None - # Directories holding a Pulumi.yaml at the base ("" for the root): any - # change under one is infrastructure as code. + # Directories holding a Pulumi project file (Pulumi.yaml, .yml or .json) + # at the base ("" for the root): any change under one is infrastructure + # as code. pulumi_dirs: tuple[str, ...] | None = () # Paths only a trusted author's own commits touched: the path floor and # the manifest review leave them out (owner decision 2026-10-22). @@ -971,13 +972,25 @@ def _trust_gate_reasons(files: list[_File], raw_files: list[Any]) -> list[Reason SECRET_EXPOSURE_CODE = "secret-exposure-review" AGENT_SURFACE_CODE = "agent-surface-review" # Infrastructure as code: any change can add or resize a billed resource. +# Pulumi reads a project or stack file in JSON or either YAML extension; +# Terraform's JSON forms and the ARM template names Azure's quickstart +# samples use count with the native ones. COST_PATHS: tuple[str, ...] = ( "**/Pulumi.yaml", + "**/Pulumi.yml", + "**/Pulumi.json", "**/Pulumi.*.yaml", + "**/Pulumi.*.yml", + "**/Pulumi.*.json", "**/*.tf", + "**/*.tf.json", "**/*.tfvars", + "**/*.tfvars.json", "**/*.bicep", "**/*.bicepparam", + "**/azuredeploy.json", + "**/azuredeploy.*.json", + "**/*.azuredeploy.json", ) _WORKFLOWS = (".github/workflows/*.yml", ".github/workflows/*.yaml") _PIPELINE_CONFIG = "**/pr-pipeline.yaml" diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py index fc4d66348a1..4a8cd3299d7 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py @@ -949,10 +949,15 @@ def test_each_directory_holding_a_pulumi_yaml(self) -> None: tree = { "Pulumi.yaml": "100644", "infra/x/pulumi.YAML": "100644", + "stack/Pulumi.yml": "100644", + "web/Pulumi.json": "100644", "a.md": "100644", + "b/Pulumi.prod.yml": "100644", } with mock.patch.object(policy, "_tree_modes", return_value=tree): - self.assertEqual(policy.pulumi_dirs_at(REPO, self.PR), ("", "infra/x")) + self.assertEqual( + policy.pulumi_dirs_at(REPO, self.PR), ("", "infra/x", "stack", "web") + ) def test_an_unlisted_tree_is_unknown(self) -> None: with mock.patch.object(policy, "_tree_modes", return_value=None): diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py index 36163bd8100..50a2f4d4082 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py @@ -80,17 +80,33 @@ class CostHold(_Finding): def test_infrastructure_as_code_holds_floor_or_not(self): for path in ( "infra/Pulumi.yaml", + "infra/Pulumi.yml", + "infra/Pulumi.json", "infra/Pulumi.prod.yaml", + "infra/Pulumi.prod.yml", + "infra/Pulumi.prod.json", "infra/main.tf", + "infra/main.tf.json", "infra/prod.tfvars", + "infra/terraform.tfvars.json", + "infra/prod.auto.tfvars.json", "infra/main.bicep", "infra/main.bicepparam", + "deploy/azuredeploy.json", + "deploy/azuredeploy.parameters.json", + "deploy/azuredeploy.parameters.us.json", + "deploy/prereq.azuredeploy.json", ): with self.subTest(path=path): result = _cost([_file(path)]) self.assertEqual(result.score, Risk.HIGH) self.assertIn(COST_CODE, _codes(result)) + def test_names_near_an_infrastructure_file_do_not_hold(self): + for path in ("infra/main.json", "infra/tf.json", "deploy/azuredeploy.md"): + with self.subTest(path=path): + self.assertNotIn(COST_CODE, _codes(_cost([_file(path)]))) + def test_a_cited_owner_decision_clears_it(self): result = _cost([_file("infra/Pulumi.yaml")], cost_decision=DECISION) self.assertEqual(result.score, Risk.LOW) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index fd5b1e70dd7..9be271e805e 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -194,9 +194,20 @@ def python_closure(js_files: list[str]) -> list[str]: "plugins/multi-agent/reference/defaults.yml", "Pulumi.yaml", "infra/Pulumi.yaml", + "infra/Pulumi.yml", + "infra/Pulumi.json", "infra/pulumi.PROD.yaml", + "infra/Pulumi.prod.yml", + "infra/Pulumi.prod.json", "infra/Pulumi.md", "main.tf", + "infra/main.tf.json", + "infra/terraform.tfvars.json", + "infra/main.json", + "deploy/azuredeploy.json", + "deploy/AzureDeploy.Parameters.json", + "deploy/prereq.azuredeploy.json", + "deploy/azuredeploy.md", ".infra/net/main.TF", "a.tf/inside.txt", ".tf", From 11892cea61e8029843e96443a8a2336d791c0602 Mon Sep 17 00:00:00 2001 From: Kyle Sexton <153232337+kyle-sexton@users.noreply.github.com> Date: Fri, 30 Oct 2026 18:49:17 -0400 Subject: [PATCH 2/2] fix(source-control): hold the prereq ARM template parameters file as cost Azure quickstart samples name a prereq parameters file prereq.azuredeploy.parameters.json, which no COST_PATHS glob matched. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01KeBVgauhM9QMc1vRhujf3L --- .changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md | 2 +- .changes/source-control/cost-hold-iac-paths-9241a3c7.md | 2 +- .github/scripts/pr-merge/merge-holds.mjs | 1 + docs/conventions/pr-pipeline/README.md | 6 +++--- .../skills/babysit-prs/scripts/babysit_risk.py | 1 + .../babysit-prs/scripts/tests/test_babysit_risk_cost.py | 1 + .../babysit-prs/scripts/tests/test_merge_holds_sync.py | 1 + 7 files changed, 9 insertions(+), 5 deletions(-) diff --git a/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md b/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md index d29cb56c1d2..1a3cc296323 100644 --- a/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md +++ b/.changes/pr-pipeline/cost-hold-iac-paths-9241a3c7.md @@ -7,7 +7,7 @@ bump: minor - The cost hold section names the infrastructure-as-code files the merge gate now holds: - Pulumi project and stack files in their `.yml` and `.json` forms. - `*.tf.json` and `*.tfvars.json`. - - The ARM template names `azuredeploy.json`, `azuredeploy.*.json` and `*.azuredeploy.json`. + - The ARM template names `azuredeploy.json`, `azuredeploy.*.json`, `*.azuredeploy.json` and `*.azuredeploy.*.json`. - Any file under a directory that holds a Pulumi project file. It also says why a `secrets` reference moved within one file is accepted: the move adds no diff --git a/.changes/source-control/cost-hold-iac-paths-9241a3c7.md b/.changes/source-control/cost-hold-iac-paths-9241a3c7.md index 0570e48c014..38eba10143c 100644 --- a/.changes/source-control/cost-hold-iac-paths-9241a3c7.md +++ b/.changes/source-control/cost-hold-iac-paths-9241a3c7.md @@ -8,7 +8,7 @@ bump: patch - Pulumi project and stack files in their `.yml` and `.json` forms (`Pulumi.yml`, `Pulumi.json`, `Pulumi.*.yml`, `Pulumi.*.json`). - Terraform's JSON files (`*.tf.json`, `*.tfvars.json`). - - The ARM template names `azuredeploy.json`, `azuredeploy.*.json` and `*.azuredeploy.json`. + - The ARM template names `azuredeploy.json`, `azuredeploy.*.json`, `*.azuredeploy.json` and `*.azuredeploy.*.json`. A directory holding any Pulumi project file at the base now counts as a Pulumi project, and `merge-holds.mjs` carries the same paths. A `secrets` reference moved within one file still diff --git a/.github/scripts/pr-merge/merge-holds.mjs b/.github/scripts/pr-merge/merge-holds.mjs index bfbfda28c7d..de391f554cd 100755 --- a/.github/scripts/pr-merge/merge-holds.mjs +++ b/.github/scripts/pr-merge/merge-holds.mjs @@ -216,6 +216,7 @@ export const COST_PATHS = Object.freeze([ "**/azuredeploy.json", "**/azuredeploy.*.json", "**/*.azuredeploy.json", + "**/*.azuredeploy.*.json", // END hold-floor COST_PATHS ]); diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 92285e131b4..a54e6a99d68 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -1268,9 +1268,9 @@ the rules that only tighten in `*.tf`, `*.tf.json`, `*.tfvars`, `*.tfvars.json`, `*.bicep` or `*.bicepparam` file, any ARM template under the names [Azure's quickstart samples use](https://github.com/Azure/azure-quickstart-templates/blob/master/1-CONTRIBUTION-GUIDE/README.md) - (`azuredeploy.json`, `azuredeploy.*.json`, `*.azuredeploy.json`), and any file under a - directory that holds a Pulumi project file at the base. A base tree the gate cannot list whole - holds every file; + (`azuredeploy.json`, `azuredeploy.*.json`, `*.azuredeploy.json`, `*.azuredeploy.*.json`), and + any file under a directory that holds a Pulumi project file at the base. A base tree the gate + cannot list whole holds every file; - a file renamed or copied into `.github/workflows/` or a `pr-pipeline.yaml` path from outside; - a runner value the workflow's head text gains over its base text, or an added inline `runs-on:` value: on a public repository only one that is not a free literal (GitHub's diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py index 95e5f22b9ec..e1ca51d8572 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -991,6 +991,7 @@ def _trust_gate_reasons(files: list[_File], raw_files: list[Any]) -> list[Reason "**/azuredeploy.json", "**/azuredeploy.*.json", "**/*.azuredeploy.json", + "**/*.azuredeploy.*.json", ) _WORKFLOWS = (".github/workflows/*.yml", ".github/workflows/*.yaml") _PIPELINE_CONFIG = "**/pr-pipeline.yaml" diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py index 50a2f4d4082..8de87e461d4 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_risk_cost.py @@ -96,6 +96,7 @@ def test_infrastructure_as_code_holds_floor_or_not(self): "deploy/azuredeploy.parameters.json", "deploy/azuredeploy.parameters.us.json", "deploy/prereq.azuredeploy.json", + "deploy/prereqs/prereq.azuredeploy.parameters.json", ): with self.subTest(path=path): result = _cost([_file(path)]) diff --git a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py index 9be271e805e..1db3c380a11 100644 --- a/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_merge_holds_sync.py @@ -207,6 +207,7 @@ def python_closure(js_files: list[str]) -> list[str]: "deploy/azuredeploy.json", "deploy/AzureDeploy.Parameters.json", "deploy/prereq.azuredeploy.json", + "deploy/prereqs/prereq.azuredeploy.parameters.json", "deploy/azuredeploy.md", ".infra/net/main.TF", "a.tf/inside.txt",