diff --git a/.changes/pr-pipeline/drop-trust-path-hold-apps-5d1e7a20.md b/.changes/pr-pipeline/drop-trust-path-hold-apps-5d1e7a20.md new file mode 100644 index 0000000000..8d6afcb4ea --- /dev/null +++ b/.changes/pr-pipeline/drop-trust-path-hold-apps-5d1e7a20.md @@ -0,0 +1,23 @@ +--- +bump: patch +--- + +### Changed + +- The agent App trust-path hold is off (owner decision Q21, 2026-10-29: path holds stay off until + open issues and pull requests reach zero). `merge.trust-path-hold-apps` names only + `trust-path-hold-off[bot]`, a login no App holds, in place of `melodic-agent[bot]`, so neither + ci-status's inline hold nor ci-workflows' `pr-require-checks-trust-paths` (through the regenerated + `.github/trust-path-hold.json`) holds an agent App pull request for a person, except that both + still fail closed on a read they cannot finish: a pull request past 250 commits or 3000 files, or + a branch history past 2000 entries. The list keeps one entry because the shared workflow fails on + an empty one. Restoring both holds takes three steps: list `melodic-agent[bot]` again, set + `merge.diff-check.path-holds` true and run `scripts/sync-hold-floor.py`. Listing the App alone + restores only ci-workflows' hold, since ci-status's inline hold runs only while `path-holds` is + true. +- ci-status no longer holds a release-App or standards-sync-App pull request for changing a + trust-gate path, and still holds a Dependabot one, whose action-pin bumps bring in third-party + code (owner decisions Q21 and Q24). While `merge.diff-check.path-holds` is false, as this + repository sets the new key, `scripts/sync-hold-floor.py` writes a guard after ci-status's + exempt-author `hold_patterns` that empties the list for every author but Dependabot. Setting + it true holds all three again. These authors still get no review lane. diff --git a/.changes/source-control/drop-trust-path-hold-apps-0c4b9e61.md b/.changes/source-control/drop-trust-path-hold-apps-0c4b9e61.md new file mode 100644 index 0000000000..bf4f6140a4 --- /dev/null +++ b/.changes/source-control/drop-trust-path-hold-apps-0c4b9e61.md @@ -0,0 +1,5 @@ +--- +bump: none +--- + +Test-only: the merge-holds sync test checks that the trust-path hold's `apps` is non-empty instead of naming `melodic-agent[bot]`, and checks ci-status's exempt-author patterns as the generator writes them with path holds on and the Dependabot-only guard it adds with them off; the plugin ships no change. diff --git a/.github/trust-path-hold.json b/.github/trust-path-hold.json index 6e71b17432..f78b86622a 100644 --- a/.github/trust-path-hold.json +++ b/.github/trust-path-hold.json @@ -43,6 +43,6 @@ "docs/standards/*" ], "apps": [ - "melodic-agent[bot]" + "trust-path-hold-off[bot]" ] } diff --git a/.github/workflows/pr-require-checks.yml b/.github/workflows/pr-require-checks.yml index 3749224e80..9acfc5cfd9 100644 --- a/.github/workflows/pr-require-checks.yml +++ b/.github/workflows/pr-require-checks.yml @@ -4931,7 +4931,9 @@ jobs: # standards-sync App, by numeric author id, are exempt: no review lane # reviews them. An exempt author's PR still fails when a changed file # (or a renamed file's old path) is a trust-gate path, since no security - # lane reads it; a person merges it. So does any other same-repository + # lane reads it; a person merges it (with merge.diff-check.path-holds + # false, Dependabot alone: the generated guard after hold_patterns + # empties the list for every other author). So does any other same-repository # PR changing one that an agent App (merge.trust-path-hold-apps in the # base commit's pr-pipeline.yaml) opened, authored or committed a commit # of, or pushed to its head branch (the repository activity API, one @@ -4994,6 +4996,8 @@ jobs: # The trust-gate paths, as bash patterns (`*` crosses `/`). # BEGIN hold-floor hold_patterns: generated by scripts/sync-hold-floor.py hold_patterns=('.github/standards/trusted-actors/*' '.github/actions/check-trusted-trigger/*' 'CODEOWNERS' '*/CODEOWNERS' '.gitmodules' '*/.gitmodules' '.github/actions/resolve-config/*' '.github/actions/check-kill-switch/*' '.github/actions/request-lane-token/*' '.github/actions/log-api-budget/rate-guard.mjs' '.github/actions/log-api-budget/rate-limits.json' '.github/scripts/pr-merge/merge-holds.mjs' '.github/scripts/pr-merge/enable-auto-merge.mjs' '.github/scripts/pr-merge/superseded.mjs' '.github/scripts/pr-merge/clear-escalations.mjs' '.github/scripts/pr-merge/carry-run-cache.mjs' '.github/scripts/pr-merge/gate-check.mjs' '.github/scripts/pr-merge/head-ref-map.mjs' '.github/scripts/pr-merge/lane-records.mjs' '.github/scripts/pr-merge/last-report.mjs' '.github/scripts/pr-merge/rate-stop.bash' '.github/scripts/pr-merge/refire-refused.mjs' '.github/scripts/pr-merge/rotation.mjs' '.github/scripts/pr-merge/sweep-summary.mjs' '.github/scripts/pr-merge/who.mjs' '.github/scripts/pr-merge/post-unarmed-status.mjs' '.github/scripts/pr-merge/verify-answers.mjs' '.github/scripts/pr-merge/listing-carry.mjs' '.github/actions/report-check-run/posting-run.mjs' 'scripts/sync-hold-floor.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_check_inputs.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_checks.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_classify.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_dependabot.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_feedback.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_gh.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_merge_policy.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_postmerge.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_repo_config.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_review_trigger.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_run_cache.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_state.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_util.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_verify_evidence.py' 'plugins/source-control/skills/babysit-prs/scripts/babysit_whole_review.py' 'docs/conventions/pr-pipeline.yaml' '.github/scripts/pr-unblock/address-feedback.mjs' '.github/scripts/pr-unblock/escalation-routes.mjs' '.github/scripts/pr-unblock/escalation-routes.json' '.github/scripts/pr-unblock/gate-source.mjs' '.github/scripts/pr-unblock/thread-scope.mjs' '.github/scripts/pr-unblock/retry-fetch.mjs' '.github/scripts/work-resolve-issue/report-outcome.mjs' '.github/scripts/work-resolve-issue/limits.mjs' '.github/scripts/work-resolve-issue/retry-fetch.mjs' '.github/scripts/pr-unblock/report-escalation.sh' '.github/scripts/pr-unblock/adjudicate-feedback.mjs' '.github/scripts/pr-unblock/publish-feedback.mjs' '.github/scripts/pr-unblock/request-budget.mjs' '.github/scripts/pr-unblock/dependency-classify.mjs' '.github/scripts/apply-patch/apply-patch.mjs' '.github/scripts/apply-patch/predicates.mjs' '.github/scripts/apply-patch/protected-paths.mjs' '.github/scripts/apply-patch/retry-fetch.mjs' '.github/scripts/pr-unblock/fix-ci-context.mjs' '.github/scripts/pr-unblock/feedback-dispatch.mjs' '.github/scripts/pr-unblock/verify-notes.mjs' '.github/scripts/pr-unblock/limit-stop.mjs' '.github/scripts/pr-unblock/dispatch-record.mjs' '.github/scripts/pr-unblock/verify-cap.mjs' '.github/scripts/maintenance-reconcile-lost-runs/infra-failure.mjs' '.github/scripts/work-resolve-issue/stop-routes.json' '.github/scripts/work-resolve-issue/stop-routes.mjs' '.github/scripts/lib/stop-reason.mjs' '.github/scripts/lib/done-evidence.mjs' '.github/scripts/pr-review-main/*' '.claude/*' '*/.claude/*' '.claude-lane/*' '*/.claude-lane/*' 'docs/conventions/instruction-exception-register/*' 'docs/conventions/untrusted-content/*' '.mcp.json' '*/.mcp.json' 'CLAUDE.md' '*/CLAUDE.md' 'CLAUDE.local.md' '*/CLAUDE.local.md' 'AGENTS.md' '*/AGENTS.md' 'REVIEW.md' 'docs/standards/*' '.github/workflows/dependabot-fragments.yml' '.github/workflows/eval-browser-dispatch.yml' '.github/workflows/intake-reenter.yml' '.github/workflows/intake-triage.yml' '.github/workflows/maintenance-audit-lane-runs.yml' '.github/workflows/maintenance-reconcile-lost-runs.yml' '.github/workflows/post-merge-demotion-alert.yml' '.github/workflows/post-merge-test-mutation.yml' '.github/workflows/pr-feedback-dispatch.yml' '.github/workflows/pr-merge.yml' '.github/workflows/pr-refine.yml' '.github/workflows/pr-require-checks.yml' '.github/workflows/pr-review-hosted.yml' '.github/workflows/pr-review-main.yml' '.github/workflows/pr-review-security-hosted.yml' '.github/workflows/pr-run-activity-read.yml' '.github/workflows/pr-run-activity-write.yml' '.github/workflows/pr-unblock-dispatch.yml' '.github/workflows/pr-unblock.yml' '.github/workflows/pr-verify.yml' '.github/workflows/release-publish-plugins.yml' '.github/workflows/work-resolve-issue.yml' '.github/workflows/work-unblock.yml') + # merge.diff-check.path-holds is false: only Dependabot stays held. + [ "$AUTHOR_ID" = 49699333 ] || hold_patterns=() # END hold-floor hold_patterns if [[ ! $PR_NUMBER =~ ^[1-9][0-9]*$ ]]; then echo "::error::the pull request number '$PR_NUMBER' is not a number." diff --git a/docs/conventions/pr-pipeline.yaml b/docs/conventions/pr-pipeline.yaml index fe6def6fe8..0b55ee49dc 100644 --- a/docs/conventions/pr-pipeline.yaml +++ b/docs/conventions/pr-pipeline.yaml @@ -954,12 +954,18 @@ merge: rung: C2 authority: pr-merge agent-identity: melodic-automation-lanes[bot] - # The agent App, installed on every repository on purpose: ci-status holds - # a pull request it opened, wrote a commit of or pushed to when it changes a - # trust-gate path, and a person merges it (README.md, Both lanes review the - # head). + # The Apps whose touch holds a trust-gate path change for a person + # (README.md, Both lanes review the head): ci-status here, and ci-workflows' + # pr-require-checks-trust-paths through .github/trust-path-hold.json. Owner + # decision Q21 (2026-10-29) turned the hold off until open issues and pull + # requests reach zero: the one entry is a login no App holds, because the + # shared workflow fails on an empty list. Restoring both holds takes three + # steps: list melodic-agent[bot] again, set `diff-check.path-holds` true, and run + # scripts/sync-hold-floor.py. + # Listing the App alone restores only ci-workflows' hold; ci-status's inline + # hold also needs `path-holds`. trust-path-hold-apps: - - melodic-agent[bot] + - trust-path-hold-off[bot] # true: report each decision and write nothing. report-only: false # true: post-merge-dispatch starts a full run of main inside its 30-minute @@ -1104,6 +1110,10 @@ merge: # Owner decision 2026-10-17: the lane hold covers structural and untrusted- # provenance work only; scoped lane PRs off the HIGH floor merge on the # verifier verdict. + # `path-holds` also sets whether ci-status holds an exempt author's PR for + # a trust-gate path or an App-authored one (`trust-path-hold-apps`), except + # that ci-status holds a Dependabot PR for one either way (owner decision + # Q24, 2026-10-29: its action-pin bumps bring in third-party code). # `path-holds: false`: no changed path holds a pull request for its path # alone, not the floor above, denied-paths nor `review-path:` (owner # decision Q15, 2026-10-29: "We shouldn't really have any path diff --git a/docs/conventions/pr-pipeline/README.md b/docs/conventions/pr-pipeline/README.md index b47607634b..e7ece3460e 100644 --- a/docs/conventions/pr-pipeline/README.md +++ b/docs/conventions/pr-pipeline/README.md @@ -2509,15 +2509,21 @@ them on every `pull_request` run, contract-only runs included: author id, except that their PR fails when a changed file (or a renamed file's old path) is a trust-gate path (the hold-paths above less the security review's own files, matched case-insensitively), since no lane - reviews it; a person merges it. The same holds a same-repository PR that changes a trust-gate + reviews it; a person merges it. While `merge.diff-check.path-holds` is false (this repository's + setting, owner decision Q21, 2026-10-29), `scripts/sync-hold-floor.py` writes a guard after + `hold_patterns` that empties it for every author but Dependabot, so only a Dependabot PR is held + for its paths: its action-pin bumps bring in third-party code (owner decision Q24). The same holds a same-repository PR that changes a trust-gate path when an App named in `merge.trust-path-hold-apps` (the base commit's `pr-pipeline.yaml`, - `[bot]` logins; this repository names the agent App, `melodic-agent[bot]`) opened it, is the + `[bot]` logins; this repository names only `trust-path-hold-off[bot]`, a login no App holds, so + the hold is off) opened it, is the author or committer of one of its commits by account or noreply address, or pushed to its head branch (the repository activity API), since its head can rewrite the review callers; a person merges it. Such a PR still needs both lanes when no trust-gate path changed. ci-status reads the App list, the PR's commits and the branch's push history only once a trust-gate path matched, and an unreadable one, a commit list shorter than the PR, or a full page of pushes fails closed. - A draft is exempt. + A draft is exempt. Restoring the App hold takes three steps: list `melodic-agent[bot]` again, set + `merge.diff-check.path-holds` true and run `scripts/sync-hold-floor.py`, because ci-status's inline hold runs only while `path-holds` is + true; listing the App alone restores only ci-workflows' `pr-require-checks-trust-paths` hold. - **A stopped lane holds its head.** ci-status fails while the newest `pr-unblock/*` commit status on the head by `github-actions[bot]` is `failure` or `error`, naming each such context; a later `success` on the same head (`report-escalation.sh clear`) supersedes it, and another account's diff --git a/docs/conventions/pr-pipeline/pr-pipeline.schema.json b/docs/conventions/pr-pipeline/pr-pipeline.schema.json index 231cca0972..e2b7b0e5e5 100644 --- a/docs/conventions/pr-pipeline/pr-pipeline.schema.json +++ b/docs/conventions/pr-pipeline/pr-pipeline.schema.json @@ -1147,8 +1147,8 @@ "type": "array", "items": { "type": "string", "pattern": "^[A-Za-z0-9]+(-[A-Za-z0-9]+)*\\[bot\\]$" }, "uniqueItems": true, - "default": [], - "description": "[bot] logins of GitHub Apps whose touch puts a trust-gate path change in a person's hands. ci-status fails a same-repository pull request that changes a trust-gate path when one of them opened it, authored or committed one of its commits, or pushed to its head branch, as it fails an exempt author's (README.md, Both lanes review the head). It is read at the base commit. Empty or absent: no App is held this way." + "minItems": 1, + "description": "[bot] logins of GitHub Apps whose touch puts a trust-gate path change in a person's hands. ci-status fails a same-repository pull request that changes a trust-gate path when one of them opened it, authored or committed one of its commits, or pushed to its head branch, as it fails an exempt author's (README.md, Both lanes review the head). It is read at the base commit. It needs at least one entry, because the shared ci-workflows hold fails on an empty list; a login no App holds turns the hold off." }, "diff-check": { "type": "object", @@ -1177,7 +1177,7 @@ "path-holds": { "type": "boolean", "default": false, - "description": "pr-merge lane only: true holds a pull request for a changed path alone, on the risk scorer's path floor and denied-paths (`high-path:` and `review-path:`), enable-auto-merge's and ci-status's hold paths. Absent or false: no path holds it; the gate report lists each skipped hold. Unsafe or aliasing paths, the manifest review, cost holds and the trusted-actor widening count either way. The local babysit lane always holds paths." + "description": "Controls the pr-merge lane's path holds and ci-status's exempt-author and App holds on trust-gate paths. True holds a pull request for a changed path alone, on the risk scorer's path floor and denied-paths (`high-path:` and `review-path:`), enable-auto-merge's and ci-status's hold paths. Absent or false: no path holds it, except that ci-status still holds a Dependabot pull request that changes a trust-gate path; the gate report lists each skipped hold. Unsafe or aliasing paths, the manifest review, cost holds and the trusted-actor widening count either way. The local babysit lane always holds paths." }, "c2-contradictions": { "type": "boolean", 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 30427cc6a9..9fc13ad272 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 @@ -43,6 +43,7 @@ from __future__ import annotations import ast +import importlib.util import fnmatch import json import pathlib @@ -304,10 +305,7 @@ def test_each_sample_path_is_held_by_all_three_or_none(self) -> None: text=True, check=True, ) - workflow = (ROOT / ".github/workflows/pr-require-checks.yml").read_text( - encoding="utf-8" - ) - line = re.search(r"^ *(hold_patterns=\(.*\))$", workflow, re.MULTILINE)[1] + line = _hold_patterns_line() bash = subprocess.run( [ "bash", @@ -516,7 +514,25 @@ def _hold_paths(workflow: str) -> list[str]: return block[1].split() -def _hold_patterns() -> list[str]: +def _sync_hold_floor() -> Any: + spec = importlib.util.spec_from_file_location( + "sync_hold_floor", ROOT / "scripts/sync-hold-floor.py" + ) + assert spec and spec.loader + loaded = importlib.util.module_from_spec(spec) + spec.loader.exec_module(loaded) + return loaded + + +def _hold_patterns_line() -> str: + """ci-status's exempt-author `hold_patterns` as the generator writes them + with `merge.diff-check.path-holds` true: the hardened list. While the + switch is false a guard line after it empties the list for every author + but Dependabot (test_the_workflow_list_follows_the_path_holds_switch).""" + return _sync_hold_floor().hold_patterns("", holds=True)[0] + + +def _workflow_hold_patterns() -> list[str]: text = (ROOT / ".github/workflows/pr-require-checks.yml").read_text( encoding="utf-8" ) @@ -525,6 +541,12 @@ def _hold_patterns() -> list[str]: return shlex.split(found[1]) +def _hold_patterns() -> list[str]: + found = re.fullmatch(r"hold_patterns=\((.*)\)", _hold_patterns_line()) + assert found + return shlex.split(found[1]) + + def _shared_glob(glob: str, path: str) -> bool: """ci-workflows' check-trust-paths matcher (globToRegExp): whole path, case-insensitive, `*` and `**` cross `/`, `?` is one character.""" @@ -547,11 +569,12 @@ class TheScrutinyListsNameTheFloor(unittest.TestCase): base-branch hold's `.github/trust-path-hold.json` (the same, case-folded) each match every floor entry.""" - def test_the_base_branch_hold_holds_github_and_the_agent_apps(self) -> None: + def test_the_base_branch_hold_holds_github_and_the_configured_apps(self) -> None: config = _trust_path_hold() self.assertEqual(sorted(config), ["apps", "paths"]) self.assertEqual(config["paths"][0], ".github/**") - self.assertIn("melodic-agent[bot]", config["apps"]) + # ci-workflows' shared hold fails on an empty `apps`. + self.assertTrue(config["apps"]) pipeline = (ROOT / "docs/conventions/pr-pipeline.yaml").read_text( encoding="utf-8" ) @@ -567,6 +590,43 @@ def test_the_base_branch_hold_holds_github_and_the_agent_apps(self) -> None: self.assertTrue(any(_shared_glob(g, path) for g in config["paths"])) self.assertFalse(any(_shared_glob(g, "README.md") for g in config["paths"])) + def test_the_workflow_list_follows_the_path_holds_switch(self) -> None: + # Owner decisions Q21 and Q24 (2026-10-29): with + # merge.diff-check.path-holds false a guard empties the list for every + # author but Dependabot, whose action-pin bumps bring in third-party + # code; true holds every exempt author. + sync = _sync_hold_floor() + actors = json.loads( + (ROOT / ".github/standards/trusted-actors/trusted-actors.json").read_text( + encoding="utf-8" + ) + )["actors"] + dependabot = [str(a["id"]) for a in actors if a["login"] == "dependabot[bot]"] + self.assertEqual(dependabot, [sync.DEPENDABOT_ID]) + on = sync.hold_patterns("", holds=True) + self.assertEqual(on, [_hold_patterns_line()]) + off = sync.hold_patterns("", holds=False) + self.assertEqual(off[0], on[0]) + self.assertEqual( + off[-1], f'[ "$AUTHOR_ID" = {sync.DEPENDABOT_ID} ] || hold_patterns=()' + ) + self.assertTrue(all(line.startswith("#") for line in off[1:-1])) + text = (ROOT / ".github/workflows/pr-require-checks.yml").read_text( + encoding="utf-8" + ) + block = re.search( + r"BEGIN hold-floor hold_patterns.*?\n(.*?)\n *# END hold-floor hold_patterns", + text, + re.S, + ) + assert block + self.assertEqual( + [line.strip() for line in block[1].split("\n")], + sync.hold_patterns("", sync.path_holds()), + ) + self.assertEqual(_workflow_hold_patterns(), _hold_patterns()) + self.assertTrue(_hold_patterns()) + def test_each_list_covers_every_floor_entry(self) -> None: lists = { workflow: (_hold_paths(workflow), risk.glob_match) diff --git a/scripts/ci-review-lanes.test.sh b/scripts/ci-review-lanes.test.sh index 71e91b5f8f..fdf114c74c 100755 --- a/scripts/ci-review-lanes.test.sh +++ b/scripts/ci-review-lanes.test.sh @@ -8,7 +8,8 @@ # `Apply the hold` step ran. Pending, failed, skipped or missing reviews fail # with the cause named, a lane the kill switch stopped among them; Dependabot, the # release App and the standards-sync App are exempt by numeric author id, but -# not from a trust-gate path, and neither is a pull request an agent App +# not from a trust-gate path (with merge.diff-check.path-holds false, only +# Dependabot is held for one), and neither is a pull request an agent App # (merge.trust-path-hold-apps at the base) opened, wrote a commit of or pushed # to. It fails closed when it cannot read. # test-scope: .github/workflows/pr-require-checks.yml docs/conventions/pr-pipeline.yaml @@ -111,16 +112,40 @@ else fail "the review callers pin different or no pr-review-cli commits: ${pins//$'\n'/ }" fi -# An exempt author's pull request, which no review lane reads, still fails on -# the trust-gate paths in bash form: `**/X` is X or */X, a trailing `**` is -# `*`. The security review's own files in hold-paths are scrutiny only. +# The trust-gate paths in bash form, as sync-hold-floor.py writes them: +# `**/X` is X or */X, a trailing `**` is `*`. The security review's own files +# in hold-paths are scrutiny only. want_patterns="$(grep -v '^plugins/review/' <<<"$first_paths" | sed -E 's#/\*\*$#/*#; s#^\*\*/(.*)$#\1\n*/\1#' | sort)" got_patterns="$(sed -nE 's/^ *hold_patterns=\((.*)\)$/\1/p' "$TMP_ROOT/body.sh" | tr ' ' '\n' | tr -d "'" | sort)" -if [[ -n "$got_patterns" && "$got_patterns" == "$want_patterns" ]]; then +if [[ -n "$want_patterns" && "$got_patterns" == "$want_patterns" ]]; then ok "the exempt-author hold patterns are the trust-gate paths" else fail "exempt-author hold patterns '$got_patterns' are not the trust-gate paths '$want_patterns'" fi +# Owner decisions Q21 and Q24 (2026-10-29): with merge.diff-check.path-holds +# false a generated guard empties the list for every author but Dependabot, +# whose action-pin bumps bring in third-party code; true holds them all. +dependabot_id="$(jq -r '.actors[] | select(.login == "dependabot[bot]") | .id' "$ACTORS")" +guard="[ \"\$AUTHOR_ID\" = $dependabot_id ] || hold_patterns=()" +guards="$(grep -cxF " $guard" "$TMP_ROOT/body.sh")" +if [[ "$(yq '.merge."diff-check"."path-holds" // false' "$ROOT/docs/conventions/pr-pipeline.yaml")" == false && "$guards" == 1 ]]; then + ok "with path-holds false only Dependabot ($dependabot_id) keeps the exempt-author hold" +else + fail "the Dependabot guard '$guard' appears $guards times; it does not follow merge.diff-check.path-holds false" +fi +# body.live.sh is the step as committed (path-holds false). body.sh, used +# below, has the block as the generator writes it with path-holds true, so +# turning path-holds back on is tested before it is needed. +cp "$TMP_ROOT/body.sh" "$TMP_ROOT/body.live.sh" +hardened="hold_patterns=($(awk -v q="'" '{ printf "%s%s%s%s", (NR > 1 ? " " : ""), q, $0, q }' <<<"$want_patterns"))" +awk -v line="$hardened" '/# BEGIN hold-floor hold_patterns/ { print; match($0, /^ */); print substr($0, 1, RLENGTH) line; skip = 1; next } + /# END hold-floor hold_patterns/ { skip = 0 } !skip { print }' \ + "$TMP_ROOT/body.sh" >"$TMP_ROOT/body.hardened" && mv "$TMP_ROOT/body.hardened" "$TMP_ROOT/body.sh" +if grep -qF 'hold_patterns=()' "$TMP_ROOT/body.sh"; then + fail "the path-holds true body still empties the list" +else + ok "the path-holds true body holds every exempt author" +fi # The base commit's config names the agent App. mkdir -p "$TMP_ROOT/base-config/docs/conventions" "$TMP_ROOT/bare-config/docs/conventions" @@ -229,7 +254,7 @@ expect() { [[ -z "${SHARED_FILES:-}" ]] || printf '%s\n' "$SHARED_FILES" >"$TMP_ROOT/pr-files.json" out="$(PATH="${STUB_PATH:+$STUB_PATH:}$TMP_ROOT/bin:$PATH" STUB_LOG="$TMP_ROOT/log" GITHUB_REPOSITORY=o/r GITHUB_REPOSITORY_ID=1 HEAD_SHA="${HEAD_SHA-$HEAD}" \ AUTHOR_ID="${AUTHOR_ID-153232337}" HEAD_REPO="${HEAD_REPO-o/r}" PR_NUMBER="${PR_NUMBER-77}" HEAD_REF="${HEAD_REF-feature/x}" \ - BASE_CONFIG="${BASE_CONFIG-$TMP_ROOT/base-config}" RUNNER_TEMP="$TMP_ROOT" bash -e "$TMP_ROOT/body.sh" 2>&1)" && rc=0 || rc=$? + BASE_CONFIG="${BASE_CONFIG-$TMP_ROOT/base-config}" RUNNER_TEMP="$TMP_ROOT" bash -e "${BODY-$TMP_ROOT/body.sh}" 2>&1)" && rc=0 || rc=$? if [[ "$rc" -ne "$want_rc" ]]; then fail "$label: expected rc=$want_rc got rc=$rc out='$out'" elif [[ -n "${3:-}" && "$out" != *"$3"* ]]; then @@ -408,6 +433,22 @@ for changed in .github/standards/trusted-actors/trusted-actors.json .github/acti STUB_FILES="$(files README.md "$changed")" AUTHOR_ID=300666570 expect "an exempt author changing $changed fails" 1 "::error::#77 is by an author no review lane reviews and changes trust-gate paths:" done +# With path-holds true, each exempt author changing a trust-gate path is held. +for id in 49699333 337859524 300666570; do + for changed in CODEOWNERS .github/actions/request-lane-token/action.yml .gitmodules; do + STUB_FILES="$(files "$changed")" STUB_CHANGED=1 + AUTHOR_ID=$id expect "with path-holds on author $id changing $changed fails" 1 "::error::#77 is by an author no review lane reviews and changes trust-gate paths: $changed." + done +done +# The step as committed, path-holds false: Dependabot is still held (owner +# decision Q24), the release and standards-sync Apps pass (Q21). +for changed in CODEOWNERS .github/actions/request-lane-token/action.yml .gitmodules; do + STUB_FILES="$(files "$changed")" STUB_CHANGED=1 + BODY="$TMP_ROOT/body.live.sh" AUTHOR_ID=49699333 expect "with path-holds off Dependabot changing $changed fails" 1 "::error::#77 is by an author no review lane reviews and changes trust-gate paths: $changed." + for id in 337859524 300666570; do + BODY="$TMP_ROOT/body.live.sh" AUTHOR_ID=$id expect "with path-holds off author $id changing $changed passes" 0 "no trust-gate path changed" + done +done for changed in .github/standards/other.json CODEOWNERS.md docs/gitmodules.md .github/actions/check-trusted-trigger-x/a.mjs .github/actions/check-wip-cap/action.yml .github/scripts/pr-merge/gate.mjs \ plugins/review/skills/security-review/SKILL.md plugins/review/evals/security-review-kill-switch-narrowed/prompt.md; do STUB_FILES="$(files "$changed")" STUB_CHANGED=1 @@ -456,9 +497,14 @@ STUB_PUSHES="$(list "$(push kyle-sexton $SHA_A)" "$(push "$APP" $SHA_B)")" expect "an App-pushed resolve-config change by a person's commit is held" 1 \ "::error::#77 changes trust-gate paths: .github/actions/resolve-config/resolve.mjs, and melodic-agent[bot] pushed bbbbbbbbbbbb to it, so no review lane decides it. A person merges it." if grep -q '/actions/workflows/' "$TMP_ROOT/log"; then fail "a held App change read the review runs"; else ok "a held App change reads no review run"; fi -BASE_CONFIG="$ROOT" expect "the repository's own pr-pipeline.yaml names the agent App" 1 "melodic-agent[bot] pushed bbbbbbbbbbbb to it" +BASE_CONFIG="$ROOT" expect "the repository's own pr-pipeline.yaml holds no agent App while path holds are off (owner decision Q21)" 0 "No agent App opened, wrote or pushed to #77" STUB_PUSHES="$(list "$(push kyle-sexton $SHA_A)")" STUB_AUTHOR="$APP" expect "an App-authored pull request changing a trust-gate path is held" 1 "and melodic-agent[bot] opened it, so no review lane decides it" +# The committed step (path-holds false): the Dependabot guard empties the +# shared list for an App author too, so no App hold applies and the review +# lanes decide. +STUB_AUTHOR="$APP" BODY="$TMP_ROOT/body.live.sh" expect "with path-holds off an App-authored trust-gate change is not held" 0 +if grep -q '/commits' "$TMP_ROOT/log"; then fail "with path-holds off the App hold read the commits"; else ok "with path-holds off the App hold reads no commit"; fi STUB_COMMITS="$(list "$(commit $SHA_A kyle-sexton kyle-sexton "$PERSON")" "$(commit $SHA_B Melodic-Agent[bot] web-flow '')")" STUB_COMMIT_COUNT=2 \ expect "a commit the App authored, in any letter case, is held" 1 "an agent App authored or committed bbbbbbbbbbbb" STUB_COMMITS="$(list "$(commit $SHA_B '' '' "1234+$APP@users.noreply.github.com")")" \ diff --git a/scripts/sync-hold-floor.py b/scripts/sync-hold-floor.py index 412e77f211..28f1dbb131 100755 --- a/scripts/sync-hold-floor.py +++ b/scripts/sync-hold-floor.py @@ -15,7 +15,9 @@ - the review callers' `hold-paths`: the trusted-actor list, every floor glob, then the security review's own files (scrutiny only). - ci-status's exempt-author `hold_patterns`: the same less those two, as bash patterns - (`**/X` is `X` and `*/X`; a trailing `/**` is `/*`). + (`**/X` is `X` and `*/X`; a trailing `/**` is `/*`). While pr-pipeline.yaml's + `merge.diff-check.path-holds` is absent or false, a guard line after the list empties it for + every author but Dependabot (owner decisions Q21 and Q24, 2026-10-29). - `.github/trust-path-hold.json`, the whole file, which ci-workflows' pr-require-checks-trust-paths reads at the base SHA: `.github/**`, then the `hold_patterns` outside `.github/`, and `merge.trust-path-hold-apps` from pr-pipeline.yaml as `apps`. Its @@ -100,10 +102,38 @@ def hold_paths(indent: str) -> list[str]: return [f"{indent}hold-paths: |", *(f"{indent} {g}" for g in HOLD_PATHS)] -def hold_patterns(indent: str) -> list[str]: +def path_holds() -> bool: + """pr-pipeline.yaml's `merge.diff-check.path-holds`: absent is false.""" + found = subprocess.run( + ["bash", str(ROOT / "lib/parse-concern-value.sh"), "--strict"] + + [str(ROOT / PIPELINE), "merge.diff-check.path-holds", "false"], + capture_output=True, + text=True, + check=False, + ) + value = found.stdout.strip() + if found.returncode != 0 or value not in ("true", "false"): + raise SystemExit( + f"sync-hold-floor: merge.diff-check.path-holds in {PIPELINE} is not true or false: {found.stderr}" + ) + return value == "true" + + +# dependabot[bot] in trusted-actors.json. Its pull requests bump third-party action pins, so +# ci-status holds its trust-gate paths with path holds off too (owner decision Q24, 2026-10-29). +DEPENDABOT_ID = "49699333" + + +def hold_patterns(indent: str, holds: bool | None = None) -> list[str]: globs = [g for g in HOLD_PATHS if g not in SCRUTINY_ONLY] quoted = " ".join(f"'{p}'" for g in globs for p in bash_patterns(g)) - return [f"{indent}hold_patterns=({quoted})"] + lines = [f"{indent}hold_patterns=({quoted})"] + if not (path_holds() if holds is None else holds): + lines += [ + f"{indent}# merge.diff-check.path-holds is false: only Dependabot stays held.", + f'{indent}[ "$AUTHOR_ID" = {DEPENDABOT_ID} ] || hold_patterns=()', + ] + return lines def trust_path_hold() -> str: