diff --git a/.changes/pr-pipeline/feat-pr-merge-cost-hold-9b3f2d61.md b/.changes/pr-pipeline/feat-pr-merge-cost-hold-9b3f2d61.md new file mode 100644 index 00000000000..b29585a6c36 --- /dev/null +++ b/.changes/pr-pipeline/feat-pr-merge-cost-hold-9b3f2d61.md @@ -0,0 +1,10 @@ +--- +bump: minor +--- + +### Added + +- pr-merge holds a pull request that could add or raise a charge (IaC files, new workflow + schedules, billed runner labels, new secrets references, new model legs, raised model-leg + timeouts, pipeline config changes) for a human unless its body links an owner + `**Decision recorded.** Cost:` comment on its closing issue or the pull request itself. diff --git a/.changes/source-control/feat-pr-merge-cost-hold-5c1e8a27.md b/.changes/source-control/feat-pr-merge-cost-hold-5c1e8a27.md new file mode 100644 index 00000000000..9e4f364c80e --- /dev/null +++ b/.changes/source-control/feat-pr-merge-cost-hold-5c1e8a27.md @@ -0,0 +1,16 @@ +--- +bump: minor +--- + +### Added + +- The unattended merge scorer holds a pull request that could add or raise a charge as HIGH + `cost-review-required`, in both lanes, whatever `high-path-floor` says and whatever an adopted + author's own paths leave off the floor: Pulumi, Terraform or Bicep files, the multi-agent role + map and anything under a Pulumi project; a file renamed into a workflow or `pr-pipeline.yaml`; + a workflow that adds a schedule, a `--model` or `--max-turns` flag, any new `secrets` + reference, a new Claude Code action, image or package run, a raised timeout on a model leg, or + a runner that is not a free standard label (any runner off a public repository); and any + `pr-pipeline.yaml` change beyond comments, blank lines, `description:` and `title:`. The hold + clears only when the body links an owner's unedited `**Decision recorded.** Cost:` comment on + the pull request, or on the closing issue when created after the pull request. diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index 9455e9a5ffe..e51f9b8966c 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -711,6 +711,52 @@ and its queued entries leave; two different identities, or an unreadable head co `plugins/**/scripts/**` entries, `floor-paths` globs that end in a `scripts` directory (such as `plugins/x/scripts/**`), or `floor-script-scopes`; every other floor entry, every other `floor-paths` glob and `denied-paths` still hold it. +- **Cost hold.** A change that could add or raise a charge needs the owner (owner rule, + 2026-10-22). It scores HIGH as `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, `plugins/multi-agent/reference/defaults.yaml` (the model and effort per role), 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; + - a file renamed or copied into `.github/workflows/` or a `pr-pipeline.yaml` path from outside; + - a workflow patch that adds a `cron:` key in any YAML or JSON spelling (a schedule can spend + model credits) or a `--model` or `--max-turns` flag; or that gains any `secrets` reference + (`secrets.NAME`, `secrets['NAME']`, `toJSON(secrets)`, `secrets: inherit`), a `uses:` of + `run-claude-cli` or any `claude-code*` action or image (matched ignoring case), or a run line + invoking `@anthropic-ai/claude-code`, each counted against the lines it removes so a moved + line or a repin is not new; + - a `timeout-minutes` or `leg-timeout-minutes` the patch adds above every one it removes, in a + workflow whose head text uses one of those actions or could not be read; + - 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 + standard `ubuntu-*`, `windows-*` and `macos-*` labels, the self-hosted managed labels, or a + list holding `self-hosted`), so an expression, a `group:` and a larger runner hold, since + [GitHub bills larger runners even on public repositories](https://docs.github.com/en/billing/concepts/product-billing/github-actions); + on any other repository every one, and any added `windows` or `macos` line; + - any `pr-pipeline.yaml` change except one whose added and removed lines are all blank, + comment-only, or a `description:` or `title:` value, or that only edits trailing comments + on the same lines in place. + Flow style, quoted keys and new list items hold; + - a workflow or pipeline file with no patch. + + The hold clears only when the pull request body links an owner decision that covers it: a + comment on the pull request's closing issue or on the pull request itself, in this repository, + whose first line starts `**Decision recorded.** Cost:`, written by a User with admin permission + who is not the agent identity or one of the gate's own logins, not posted through an App, and + never edited; one on the closing issue must also be created after the pull request was. A link + inside an HTML comment in the body counts for nothing. A push after the decision is an accepted + residual: the decision is not bound to a head. The gate reads the linked comments only for a + pull request the hold catches, and a failed read leaves the hold. A decision + clears only this reason: a workflow or `docs/conventions/pr-pipeline.yaml` change still sits on + the HIGH path floor here and waits for a person to merge it. Cite the decision before the + human-gated label lands; a body edit does not remove that label (only a new head or a person + does). Not detected: budgets, billing settings and new paid services outside these files, and + the shapes [#9241](https://github.com/melodic-software/claude-code-plugins/issues/9241) lists. + azure-iac's Pulumi program is C#, and its deploy gate + ([azure-iac#75](https://github.com/melodic-software/azure-iac/issues/75)) stops any Azure change + for the owner at deploy time. The review lanes and the verifier judge the rest. Dependabot pull + requests are not scored, so the hold does not apply to them. - **Merge queue.** The wrapper enqueues with `--match-head-commit`, never `--auto`, and only when every required check is green. An armed auto-merge request on the pull request is a refusal, and the wrapper disarms it. diff --git a/plugins/source-control/skills/babysit-prs/reference/safety.md b/plugins/source-control/skills/babysit-prs/reference/safety.md index fc6f86ddf39..db2ab249f92 100644 --- a/plugins/source-control/skills/babysit-prs/reference/safety.md +++ b/plugins/source-control/skills/babysit-prs/reference/safety.md @@ -1044,7 +1044,10 @@ any refusal is the blocker `unattended merge refused: `: `manifest-review-required`, as does a listed key whose value has the wrong type. A test file under a plugin `scripts/` directory, a `floor-paths` glob ending in a `scripts` directory or a `floor-script-scopes` glob, named by no other changed file, takes no HIGH path floor hold for - that reason alone; the rule is in `babysit_risk.py`'s docstring. A C3 pull request also needs a + that reason alone; the rule is in `babysit_risk.py`'s docstring. A change that could add or + raise a charge scores HIGH as `cost-review-required` in both lanes, floor or not, until the body + links an owner `**Decision recorded.** Cost:` comment on the closing issue or the pull request + (`cost_decision`); which changes count is in that docstring. A C3 pull request also needs a LOW blast-radius rating, which nothing supplies yet, so it scores at least MED (`blast-radius-missing`) until a C3 rung adds that source. diff --git a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py index c901faeadaf..a44b7d26a5a 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py @@ -3057,8 +3057,11 @@ def score_unattended( if eligible.adoption else frozenset() ) - if config.high_path_floor: - # The capability check compares base and head text (the floor only). + if config.high_path_floor or any( + babysit_risk.is_workflow_path(f.path) for f in files + ): + # The capability check compares base and head text (the floor only); + # the cost hold reads a workflow's head text for its model legs. files = merge_policy.with_capability_contents(repo, pr, files) # The pr-merge lane's switches (owner decision Q624); the local lane # keeps HIGH_FLOOR, both caps and the C2 contradictions. @@ -3099,27 +3102,42 @@ def score_unattended( and any(p and babysit_risk.eval_case_dirs(p) for p in changed) else None ) - risk = babysit_risk.score( - babysit_risk.RiskInput( - work_class=eligible.work_class, - rung=babysit_risk.Rung[inputs["rung"].upper()], - files=files, - files_complete=complete, - max_changed_lines=config.max_changed_lines, - high_paths=config.denied_paths, - floor_paths=config.floor_paths, - floor_script_scopes=config.floor_script_scopes, - # The floor additions follow the switch in every lane. - floor_extensions=config.high_path_floor, - declarations=declarations, - agents=agents, - eval_cases=eval_cases, - lanes=lanes, - blast_radius=None, - floor_exempt=own, - **lane_switches, - ) + private = json_object(json_object(pr.get("base")).get("repo")).get("private") + risk_input = babysit_risk.RiskInput( + work_class=eligible.work_class, + rung=babysit_risk.Rung[inputs["rung"].upper()], + files=files, + files_complete=complete, + max_changed_lines=config.max_changed_lines, + high_paths=config.denied_paths, + floor_paths=config.floor_paths, + floor_script_scopes=config.floor_script_scopes, + # The floor additions follow the switch in every lane. + floor_extensions=config.high_path_floor, + declarations=declarations, + agents=agents, + eval_cases=eval_cases, + lanes=lanes, + blast_radius=None, + floor_exempt=own, + public=True if private is False else None, + pulumi_dirs=merge_policy.pulumi_dirs_at(repo, pr), + **lane_switches, ) + risk = babysit_risk.score(risk_input) + if any(r.code == babysit_risk.COST_CODE for r in risk.reasons): + # Read only for a cost hold: an owner decision the body cites clears it. + decision = merge_policy.cost_decision( + repo, + pr, + eligible.issue, + permission=inputs["permission"], + agent_identity=inputs.get("identity"), + self_logins=self_logins, + ) + if decision: + details["costDecision"] = decision + risk = babysit_risk.score(replace(risk_input, cost_decision=decision)) details |= { "risk": { "score": risk.score.name, 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 b217bf44371..540a569adef 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 @@ -66,6 +66,7 @@ glob_match, is_capability_path, is_case_file, + is_workflow_path, is_manifest_path, ) from babysit_util import ( @@ -73,6 +74,7 @@ is_json_object, json_array, json_object, + parse_timestamp, run_command, ) @@ -1835,6 +1837,92 @@ def _closing_issue(pr: dict[str, Any], repo: str) -> tuple[dict[str, Any], str | return issue, None +# --- Cost decision ------------------------------------------------------------------------ + +# A comment link in a pull request body, and the first line that makes a +# comment a decision (as select-trusted-text.mjs `DECISION` reads it). +COST_DECISION_LINK = re.compile( + r"https://github\.com/([A-Za-z0-9-]+)/([A-Za-z0-9._-]+)/(?:issues|pull)/" + r"([0-9]+)#issuecomment-([0-9]+)" +) +# A cost decision's first line: the decision marker, then `Cost:`, so a +# decision recorded for something else clears no cost hold. +DECISION_LINE = re.compile(r"^\*\*Decision recorded\.\*\*[ \t]+Cost:") +# An HTML comment in the body, or an unclosed one to the end: a link inside +# one is hidden from a reader, so it cites nothing. +HTML_COMMENT = re.compile(r"|\Z)", re.DOTALL) +COMMENT_EDITED = "query PolicyCommentEdited($id:ID!){node(id:$id){... on IssueComment{lastEditedAt}}}" + + +def _after(value: Any, opened: datetime | None) -> bool: + made = parse_timestamp(value) + return made is not None and opened is not None and made > opened + + +def cost_decision( + repo: str, + pr: Mapping[str, Any], + issue: int | None, + *, + permission: Permission, + agent_identity: str | None, + self_logins: Collection[str] = frozenset(), +) -> str | None: + """The first comment link in the body that is an owner decision covering + this pull request, else None: a comment on its closing issue or on the + pull request itself in `repo`, by a User with admin permission who is not + the agent identity, not posted through an App, never edited, and whose + first line starts `**Decision recorded.** Cost:`. One on the closing issue + counts only when created after the pull request was, so an older decision + on the issue cannot cover new work (an author can forge commit dates, not + these); one on the pull request postdates it already. A link inside an HTML + comment, and a comment by a `self_logins` account, count for nothing. A + read that fails clears nothing.""" + owner, name = _split_repo(repo) + covered = {pr.get("number"), issue} - {None} + opened = parse_timestamp(pr.get("created_at")) + excluded = {_login_key(agent_identity)} | {_login_key(x) for x in self_logins} + body = HTML_COMMENT.sub("", str(pr.get("body") or "")) + for link in COST_DECISION_LINK.finditer(body): + if f"{link[1]}/{link[2]}".casefold() != repo.casefold(): + continue + if int(link[3]) not in covered: + continue + status, comment = _get(f"repos/{owner}/{name}/issues/comments/{link[4]}") + if status != 200 or not is_json_object(comment): + continue + user = json_object(comment.get("user")) + login = user.get("login") + if ( + not str(comment.get("issue_url") or "").endswith(f"/issues/{link[3]}") + or user.get("type") != "User" + or comment.get("performed_via_github_app") is not None + or not isinstance(login, str) + or _login_key(login) in excluded + or not DECISION_LINE.match(str(comment.get("body") or "").lstrip()) + or ( + int(link[3]) != pr.get("number") + and not _after(comment.get("created_at"), opened) + ) + ): + continue + try: + if permission(login) != "admin": + continue + node = _graphql( + COMMENT_EDITED, "cost decision", id=str(comment.get("node_id")) + ).get("node") + except PolicyReadError: + continue + if ( + is_json_object(node) + and "lastEditedAt" in node + and node["lastEditedAt"] is None + ): + return link[0] + return None + + def _foreign_refusal(pr: dict[str, Any], owner: str) -> str | None: """`fork` for a head outside this repository, `dependency-manager` for a dependency manager's pull request, else None.""" @@ -3204,6 +3292,32 @@ def merged_since_floor( # --- Changed files with git modes ------------------------------------------------------------- +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.""" + owner, name = _split_repo(repo) + base = str(json_object(pr.get("base")).get("sha")) + try: + if not SHA_RE.fullmatch(base): + raise PolicyReadError("the pull request record names no base SHA") + modes = _tree_modes(owner, name, base) + except (PolicyReadError, ValueError) as exc: + print( + f"Pulumi projects at the base: {type(exc).__name__}: {exc}", file=sys.stderr + ) + return None + if modes is None: + return None + return tuple( + sorted( + path.rpartition("/")[0] + for path in modes + if path.rpartition("/")[2].casefold() == "pulumi.yaml" + ) + ) + + def _tree_modes(owner: str, name: str, sha: str) -> dict[str, str] | None: """Each path's git mode at `sha`, or None when the listing is truncated: from a local clone holding `sha` (`_clones_holding`), else the trees API.""" @@ -3419,7 +3533,7 @@ def _text_at(owner: str, name: str, path: str, sha: str) -> str | None: def with_capability_contents( repo: str, pr: Mapping[str, Any], files: tuple[ChangedFile, ...] ) -> tuple[ChangedFile, ...]: - """Each changed capability file, manifest or eval case.yaml (not removed) with its base + """Each changed capability file, manifest, eval case.yaml or workflow (not removed) with its base text at the base SHA, from its previous path on a rename or copy, and its head text at the head SHA, or the reason either could not be read.""" owner, name = _split_repo(repo) @@ -3431,6 +3545,7 @@ def with_capability_contents( is_capability_path(changed.path) or is_manifest_path(changed.path) or is_case_file(changed.path) + or is_workflow_path(changed.path) ): read.append(changed) continue 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 6d179373955..8b86c349167 100755 --- a/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py +++ b/plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py @@ -71,6 +71,26 @@ differ, and a listed key clears only with the type `_CLEAR_TYPES` names (a string otherwise). +A change that could add or raise a charge rates HIGH as `cost-review-required` +in every lane, floor or not and whatever `floor_exempt` says (owner rule, +2026-10-22): any `COST_PATHS` file (Pulumi, Terraform, Bicep, the multi-agent +role map) 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 +`.github/workflows/` file whose patch adds a `_COST_WORKFLOW_LINE` (a `cron` +key in any spelling, `--model`, `--max-turns`), or, unless `RiskInput.public` +is True, a `runs-on:`, `windows` or `macos` line; that gains a `_SECRET_REF`, +a `_MODEL_ACTION` or a `_MODEL_PACKAGE` run, counted against the lines it +removes so a move or repin is not new; that adds a `timeout-minutes` or +`leg-timeout-minutes` above every one it removes, when its head text +(`contents`) names a model leg or could not be read; or that gains a runner +value (head text against base text, and added inline `runs-on:` lines) that +is not `_FREE_RUNNER` on a public repository, or any on another; and a +`pr-pipeline.yaml` change with any added or removed line off +`_SAFE_PIPELINE_LINE`, unless it only edits trailing comments. A workflow or +pipeline file with no patch holds. Only `RiskInput.cost_decision`, an owner +decision the caller verified, clears it. + Paths are normalized before matching (`\\` to `/`, empty and `.` segments dropped). A path is safe only when every `/`-separated segment is non-empty, uses only `[A-Za-z0-9._-]` and does not end in `.` (Windows strips it); every @@ -190,6 +210,15 @@ class RiskInput: # a changed path, as (path, text), None when unreadable; None when not # read, so every changed path under plugins//evals/ holds. eval_cases: tuple[tuple[str, str | None], ...] | None = None + # The repository is public, where standard hosted runners are free; any + # other value reads every runs-on and macOS line as a charge. + public: bool | None = None + # 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. + 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). floor_exempt: frozenset[str] = frozenset() @@ -1433,6 +1462,289 @@ def _high_path_reasons( return reasons +COST_CODE = "cost-review-required" +# Infrastructure as code: any change can add or resize a billed resource. +COST_PATHS: tuple[str, ...] = ( + # The multi-agent role map: model and effort per role for every fan-out. + "plugins/multi-agent/reference/defaults.yaml", + "**/Pulumi.yaml", + "**/Pulumi.*.yaml", + "**/*.tf", + "**/*.tfvars", + "**/*.bicep", + "**/*.bicepparam", +) +_WORKFLOWS = (".github/workflows/*.yml", ".github/workflows/*.yaml") +_PIPELINE_CONFIG = "**/pr-pipeline.yaml" +# Added workflow lines that can charge in any repository: a schedule in any +# YAML or JSON spelling (a scheduled lane spends model credits) and a model or +# turn flag. +_COST_WORKFLOW_LINE = re.compile( + r"\bcron\s*:|[\"']cron[\"']\s*:|--model\b|--max-turns\b", re.IGNORECASE +) +# Standard hosted runners are free only on public repositories, so on any +# other a runs-on line or a Windows or macOS label (a matrix value) charges. +_COST_PRIVATE_LINE = re.compile(r"\bruns-on\s*:|\bmacos\b|\bwindows\b", re.IGNORECASE) +# Runner labels free on a public repository: GitHub's standard hosted labels +# and the self-hosted managed labels (`.github/standards/runner-policy`). A +# list holding `self-hosted` targets a self-hosted runner, which is free. +_FREE_RUNNER = re.compile( + r"(?:ubuntu|windows|macos)-(?:latest|[0-9]+(?:\.[0-9]+)?)(?:-arm)?|windows-11-arm" + r"|(?:melodic|kyle)-(?:[a-z0-9]+-)*ubuntu-24\.04-x64(?:-[a-z0-9-]+)?", + re.IGNORECASE, +) +_RUNS_ON = re.compile(r"^(\s*)-?\s*runs-on\s*:\s*(.*)$") + + +def _scalar(value: str) -> str: + return value.split(" #", 1)[0].strip().strip("'\"") + + +def _labels(value: str) -> list[str]: + value = _scalar(value) + if value.startswith("[") and value.endswith("]"): + return [_scalar(v) for v in value[1:-1].split(",") if v.strip()] + return [value] if value else [] + + +def _runner_values(text: str) -> list[str]: + """Each runs-on value in workflow text: an inline label or list, and a + block's list items and `group:`/`labels:` entries (as `group:`).""" + values: list[str] = [] + lines = text.split("\n") + for i, line in enumerate(lines): + m = _RUNS_ON.match(line) + if not m: + continue + if _scalar(m[2]): + values.append(_scalar(m[2])) + continue + indent = len(m[1]) + for inner in lines[i + 1 :]: + if inner.strip() and len(inner) - len(inner.lstrip()) <= indent: + break + item = inner.strip() + if item.startswith("- "): + values.append(_scalar(item[2:])) + elif item.startswith("group:"): + values.append("group:" + _scalar(item[6:])) + elif item.startswith("labels:"): + values.append(_scalar(item[7:])) + return values + + +def _free(value: str) -> bool: + labels = _labels(value) + if "self-hosted" in (label.casefold() for label in labels): + return True + return bool(labels) and all(_FREE_RUNNER.fullmatch(label) for label in labels) + + +def _new_runners(raw: Any, added: list[str], public: bool) -> list[str]: + """Runner values the change adds that are not free: from the head text + against the base text when both were read, and from added inline runs-on + lines. Unread text and an added block holds.""" + contents = getattr(raw, "contents", None) + pair = contents if isinstance(contents, tuple) and len(contents) == 2 else None + base, head = pair if pair else (None, None) + gained: Counter[str] = Counter() + if isinstance(head, str): + gained.update(_runner_values(head)) + gained.subtract(_runner_values(base if isinstance(base, str) else "")) + found = [v for v, n in gained.items() if n > 0] + for line in added: + m = _RUNS_ON.match(line) + if m and _scalar(m[2]): + found.append(_scalar(m[2])) + elif m and not isinstance(head, str): + found.append("runs-on block in unread text") + return sorted({v for v in found if not (public and _free(v))}) + + +# The pr-pipeline.yaml lines a change may touch without moving spend: blank and +# comment lines and a `description:` or `title:` value. Any other added or +# removed line holds, unless the change only edits trailing comments (the same +# lines with the comments stripped). +_SAFE_PIPELINE_LINE = re.compile( + r"\s*(?:#.*)?|\s*(?:description|title)\s*:(?:\s.*)?", re.IGNORECASE +) +# Any secrets reference: `secrets.NAME`, `secrets['NAME']`, the whole +# `secrets` context, and `secrets: inherit`. +_SECRET_REF = re.compile( + r"\bsecrets\s*(?:\.\s*[A-Za-z0-9_]+|\[[^\]]*\])|\bsecrets\b(?=\s*\))" + r"|\bsecrets\s*:\s*inherit\b", + re.IGNORECASE, +) +# A model leg: a claude-code-* action or image, run-claude-cli, or the +# Claude Code npm package on a run line. +_MODEL_ACTION_NAME = re.compile( + r"run-claude-cli|claude-code[\w-]*|@anthropic-ai/claude-code", re.IGNORECASE +) +_MODEL_ACTION = re.compile( + r"\buses\s*:\s*['\"]?([^@\s'\"]*(?:run-claude-cli|claude-code[\w-]*)[^@\s'\"]*)", + re.IGNORECASE, +) +_MODEL_PACKAGE = re.compile(r"@anthropic-ai/claude-code\b", re.IGNORECASE) +_TIMEOUT = re.compile(r"^\s*(?:leg-)?timeout-minutes\s*:\s*['\"]?([0-9]+)") + + +def is_workflow_path(path: str) -> bool: + return any(glob_match(g, path) for g in _WORKFLOWS) + + +def _patch_lines(patch: Any) -> tuple[list[str], list[str]] | None: + """(added, removed) line texts of a unified patch, None when unreadable.""" + if not isinstance(patch, str): + return None + added, removed = [], [] + for line in patch.split("\n"): + if line.startswith("+") and not line.startswith("+++"): + added.append(line[1:]) + elif line.startswith("-") and not line.startswith("---"): + removed.append(line[1:]) + return added, removed + + +def _unsafe_pipeline_lines(patch: str) -> list[str]: + """The changed pr-pipeline.yaml lines off `_SAFE_PIPELINE_LINE`. A run of + removed lines and the added lines that follow it pass together only when + they are the same lines, in order, once trailing comments are stripped, so + a value moved between two places in the file still holds.""" + + def unsafe(lines: list[str]) -> list[str]: + return [ + line.split(" #", 1)[0].rstrip() + for line in lines + if not _SAFE_PIPELINE_LINE.fullmatch(line) + ] + + found: list[str] = [] + removed: list[str] = [] + added: list[str] = [] + + def close() -> None: + lost, gained = unsafe(removed), unsafe(added) + if lost != gained: + found.extend(line.strip() for line in lost + gained) + removed.clear() + added.clear() + + for line in patch.split("\n"): + if line.startswith("-") and not line.startswith("---"): + if added: + close() + removed.append(line[1:]) + elif line.startswith("+") and not line.startswith("+++"): + added.append(line[1:]) + else: + close() + close() + return found + + +def _net_added( + pattern: re.Pattern[str], added: list[str], removed: list[str] +) -> list[str]: + """Each match `added` gains over `removed`, so a moved or repinned line + is not new.""" + gained = Counter( + re.sub(r"\s+", "", m).casefold() + for line in added + for m in pattern.findall(line) + ) + gained.subtract( + re.sub(r"\s+", "", m).casefold() + for line in removed + for m in pattern.findall(line) + ) + return sorted(m for m, n in gained.items() if n > 0) + + +def _raised_timeouts( + f: _File, raw: Any, added: list[str], removed: list[str] +) -> list[str]: + """Timeouts on a workflow that starts a model leg (or whose head text could + not be read) that the patch adds above every value it removes.""" + new = [int(m[1]) for line in added if (m := _TIMEOUT.match(line))] + if not new: + return [] + contents = getattr(raw, "contents", None) + head = contents[1] if isinstance(contents, tuple) and len(contents) == 2 else None + if isinstance(head, str) and not _MODEL_ACTION_NAME.search(head): + return [] + old = [int(m[1]) for line in removed if (m := _TIMEOUT.match(line))] + return [f"timeout raised to {n}" for n in new if not old or n > max(old)] + + +def _cost_reasons( + inp: RiskInput, files: list[_File], raw_files: list[Any] +) -> list[Reason]: + """`cost-review-required` for a change that could add or raise a charge, + unless `inp.cost_decision` names the owner decision that clears it. It + holds in every lane, whatever the floor switch says.""" + hits = [] + for f, raw in zip(files, raw_files): + if any(glob_match(g, p) for g in COST_PATHS for p in f.paths): + hits.append(f"{f.path}: infrastructure as code") + continue + if inp.pulumi_dirs is None: + hits.append(f"{f.path}: the base tree was not listed for Pulumi projects") + continue + if any( + p == d or not d or p.casefold().startswith(d.casefold() + "/") + for d in inp.pulumi_dirs or () + for p in f.paths + ): + hits.append(f"{f.path}: in a Pulumi project") + continue + workflow = any(is_workflow_path(p) for p in f.paths) + pipeline = any(glob_match(_PIPELINE_CONFIG, p) for p in f.paths) + if not (workflow or pipeline): + continue + new, old = f.paths[0], f.paths[-1] + if new != old and not ( + is_workflow_path(old) or glob_match(_PIPELINE_CONFIG, old) + ): + hits.append(f"{f.path}: renamed or copied from {old}") + continue + lines = _patch_lines(getattr(raw, "patch", None)) + if lines is None: + hits.append(f"{f.path}: no patch to read") + continue + added, removed = lines + if workflow: + patterns = [_COST_WORKFLOW_LINE] + ( + [] if inp.public is True else [_COST_PRIVATE_LINE] + ) + found = [s.strip() for s in added if any(p.search(s) for p in patterns)] + found += [ + f"new secrets reference {n}" + for n in _net_added(_SECRET_REF, added, removed) + ] + found += [ + f"new model leg {n}" for n in _net_added(_MODEL_ACTION, added, removed) + ] + found += [ + f"new model package run {n}" + for n in _net_added(_MODEL_PACKAGE, added, removed) + ] + found += _raised_timeouts(f, raw, added, removed) + found += [ + f"runner {v}" for v in _new_runners(raw, added, inp.public is True) + ] + else: + found = _unsafe_pipeline_lines(getattr(raw, "patch", "")) + if found: + hits.append(f"{f.path}: {'; '.join(found)}") + if not hits or inp.cost_decision: + return [] + detail = "; ".join(hits) + ( + "; cite the owner's `**Decision recorded.**` comment on the closing issue or " + "this pull request in the body to clear it" + ) + return [Reason(COST_CODE, Risk.HIGH, detail)] + + def _c2_contradictions(files: list[_File]) -> list[str]: """The diff contradicts C2 ("deterministic and trivially reversible").""" found = [] @@ -1575,6 +1887,7 @@ def _score(inp: RiskInput) -> RiskResult: *_size_reasons(inp, files), *_lane_reasons(inp.lanes), *_high_path_reasons(inp, files or [], list(raw_files or ())), + *_cost_reasons(inp, files or [], list(raw_files or ())), ] reasons.sort(key=lambda reason: (-reason.severity, reason.code)) worst = max((reason.severity for reason in reasons), default=Risk.LOW) 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 new file mode 100644 index 00000000000..758bf4aada2 --- /dev/null +++ b/plugins/source-control/skills/babysit-prs/scripts/tests/test_babysit_merge_cost.py @@ -0,0 +1,190 @@ +"""`cost_decision`: only an owner `**Decision recorded.**` comment on the +closing issue or the pull request, linked from the body, clears a cost hold.""" + +from __future__ import annotations + +import pathlib +import sys +import unittest +from typing import Any +from unittest import mock + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent)) + +import babysit_merge_policy as policy + +REPO = "owner/repo" +LINK = "https://github.com/owner/repo/issues/40#issuecomment-7" +IDENTITY = "agent-lanes[bot]" +PR_CREATED = "2026-10-22T10:00:00Z" + + +def _comment(**changes: Any) -> dict[str, Any]: + return { + "issue_url": "https://api.github.com/repos/owner/repo/issues/40", + "user": {"login": "owner-person", "type": "User"}, + "performed_via_github_app": None, + "body": "**Decision recorded.** Cost: approve the larger runner.", + "node_id": "IC_7", + "created_at": "2026-10-22T12:00:00Z", + } | changes + + +class CostDecision(unittest.TestCase): + def decide( + self, + *, + body: str = f"Closes #40\n\nCost decision: {LINK}\n", + comment: dict[str, Any] | None = None, + status: int = 200, + permission: str = "admin", + edited: Any = None, + issue: int | None = 40, + node: Any = ..., + created: Any = None, + ) -> str | None: + created = PR_CREATED if created is None else created + self.reads: list[str] = [] + + def get(endpoint: str) -> tuple[int, Any]: + self.reads.append(endpoint) + return status, comment if comment is not None else _comment() + + answer = {"lastEditedAt": edited} if node is ... else node + graphql = mock.Mock(return_value={"node": answer}) + with ( + mock.patch.object(policy, "_get", side_effect=get), + mock.patch.object(policy, "_graphql", graphql), + ): + return policy.cost_decision( + REPO, + {"number": 99, "body": body, "created_at": created}, + issue, + permission=lambda login: permission, + agent_identity=IDENTITY, + self_logins=frozenset({"lane-operator"}), + ) + + def test_an_owner_decision_on_the_closing_issue_clears(self) -> None: + self.assertEqual(self.decide(), LINK) + self.assertEqual(self.reads, ["repos/owner/repo/issues/comments/7"]) + + def test_a_decision_on_the_pull_request_itself_clears(self) -> None: + link = "https://github.com/owner/repo/pull/99#issuecomment-7" + comment = _comment( + issue_url="https://api.github.com/repos/owner/repo/issues/99" + ) + self.assertEqual(self.decide(body=link, comment=comment), link) + + def test_a_decision_on_the_pull_request_needs_no_time_check(self) -> None: + link = "https://github.com/owner/repo/pull/99#issuecomment-7" + comment = _comment( + issue_url="https://api.github.com/repos/owner/repo/issues/99", + created_at=None, + ) + self.assertEqual(self.decide(body=link, comment=comment), link) + + def test_a_decision_on_another_issue_does_not_cover_it(self) -> None: + other = "https://github.com/owner/repo/issues/12#issuecomment-7" + self.assertIsNone(self.decide(body=other)) + self.assertEqual(self.reads, []) + + def test_a_link_into_another_repository_is_ignored(self) -> None: + self.assertIsNone(self.decide(body=LINK.replace("owner/repo", "evil/repo"))) + + def test_a_comment_that_is_not_on_the_linked_issue_is_refused(self) -> None: + moved = _comment(issue_url="https://api.github.com/repos/owner/repo/issues/41") + self.assertIsNone(self.decide(comment=moved)) + + def test_each_failing_condition_holds(self) -> None: + cases = { + "write, not admin": dict(permission="write"), + "edited": dict(edited="2026-10-22T10:00:00Z"), + "unreadable": dict(status=404), + "no decision line": dict(comment=_comment(body="Sounds fine to me.")), + "quoted decision": dict( + comment=_comment(body="> **Decision recorded.** quoted") + ), + "posted through an App": dict( + comment=_comment(performed_via_github_app={"slug": "cursor"}) + ), + "a bot": dict( + comment=_comment(user={"login": "agent-lanes[bot]", "type": "Bot"}) + ), + "the agent identity": dict( + comment=_comment(user={"login": "agent-lanes", "type": "User"}) + ), + "no body link": dict(body="Closes #40"), + "no cost marker": dict( + comment=_comment(body="**Decision recorded.** Option 2.") + ), + "a cost marker on the next line": dict( + comment=_comment(body="**Decision recorded.**\nCost: approve it.") + ), + "a cost marker on a later line": dict( + comment=_comment(body="**Decision recorded.** Option 2.\n\nCost: n/a") + ), + "a self login": dict( + comment=_comment(user={"login": "lane-operator", "type": "User"}) + ), + "a link inside an HTML comment": dict( + body=f"Closes #40\n\n" + ), + "a link after an unclosed HTML comment": dict( + body=f"Closes #40\n