Skip to content

feat(pr-pipeline): cut the lane hold-path floor to the merge gate's own - #9541

Merged
kyle-sexton merged 4 commits into
mainfrom
agent-ac72c39ea4f0fbf44
Oct 24, 2026
Merged

kyle-sexton merged 4 commits into
mainfrom
agent-ac72c39ea4f0fbf44

Conversation

@kyle-sexton

@kyle-sexton kyle-sexton commented Oct 24, 2026 •

Copy link
Copy Markdown
Member

No related issue: owner direction 2026-10-24, lowest-friction hold floor

Summary

The lanes' native auto-merge path held more than the merge gate does. Two extra holds are cut here, down to the gate's own floor, which is the takeover set in babysit_risk.HIGH_FLOOR (owner direction 2026-10-23):

  • merge-holds.mjs HOLD_PATH_FLOOR held every **/.claude-plugin/plugin.json and .claude-plugin/marketplace.json path-wide. So enable-auto-merge never armed a lane PR that touched a manifest, even for a userConfig default (the feat(source-control)!: ship the pr_open_state default as ready #9483 shape).
  • resolve-config resolveMerge forced .github/** into merge.diff-check.denied-paths. That held every lane PR touching .github/scripts or .github/actions in report-outcome's Hold: line, in enable-auto-merge and in ci-status.

What still holds a manifest:

  • enable-auto-merge. It already scored manifests through the gate's reader: evidence --lane-reviews runs lane_risk_refusal, which reads both sides (with_contents) and scores manifest-review-required with this repository's high-path-floor: true.
  • ci-status. Its lanes App step had no such scoring. The glob was its only manifest check, and it is what caught a push after arming whose disarm failed. So merge-holds.mjs gets a port of the gate's comparison (manifestChanges, the rule of babysit_risk._manifest_changes), with no blanket glob. Both sides are read at the base commit and the head and compared key by key.

The classes held are the ones #9280 set:

  • an _EXECUTABLE_KEYS value differing (hooks, mcpServers, lspServers, monitors), or experimental.monitors;
  • a marketplace plugin added, removed, renamed or re-sourced, or the marketplace name changed;
  • a manifest removed, renamed or copied;
  • a side that cannot be read, does not parse, or repeats a key.

A userConfig default or a version bump merges.

Fix

  • resolve.mjs resolveMerge drops .github/** and keeps the config path and the trusted-actor list. merge-holds.mjs FORCED_DENIED_PATHS matches it (merge-authority.test.mjs).
  • HOLD_PATH_FLOOR is now the forced paths plus each HIGH_FLOOR glob, listed one by one. Before, .github/** was what covered .github/actions/resolve-config/**, .github/actions/check-trusted-trigger/** and .github/standards/trusted-actors/**. A sync test checks that HOLD_PATH_FLOOR contains babysit_risk.HIGH_FLOOR.
  • merge-holds.mjs adds manifestChanges and manifestHold. The port keeps two Python behaviors:
    • its JSON walk rejects a repeated key, as _no_duplicates does;
    • its canonical form tells apart the values json.dumps(sort_keys=True) tells apart (1 and 1.0, false and 0).
  • A trusted approval of the head lifts a manifest hold, as it lifted the glob before.
  • ci-status's step now writes [filename, previous_filename, status] per file and exports BASE_SHA. A line in the older format has no status, so a manifest on it holds. A module older than this one reads the extra entries as paths that match no hold path.
  • Docs, comments, the enable-auto-merge fixture and the tests that stated the old floor are updated.
  • Workflow files carry no hold. The lanes App token never carries workflows: request-lane-token/action.yml checks the broker's grant is exactly contents, pull-requests, issues and at most actions. That covers lanes App pushes only. Another identity with write access could still push a workflow change after auto-merge is armed.

Review fixes at ready (security and code review of 4f4b0e2):

  • matchesHoldPath folds case, as glob_match and costHold do. Without it, .github/codeowners, .github/actions/Resolve-Config/…, .github/Standards/trusted-actors/… and .github/actions/Check-Trusted-Trigger/… went unheld once .github/** left.
  • HIGH_FLOOR and HOLD_PATH_FLOOR add .github/actions/check-kill-switch/**. check-trusted-trigger and resolve-config both import their output writer (writeOutputs) from it, and .github/** was what held it before.
  • test_merge_holds_sync.py now compares matched paths, case variants included, between pathHold and glob_match. A new test walks the two floor actions' imports and fails on any file the floor does not hold.
  • Manifest contents decode as strict UTF-8 with the BOM kept. An integer past int_max_str_digits holds, since the gate's json.loads refuses it. A path hold and a manifest hold both reach the reason.
  • New manifest cases: hooks true to null and to false, a 20-digit integer differing in its last digit, and -0.0 to 0.0.

Verification

  • nice bash scripts/check-affected.sh --uncommitted: 12 passed, 4 skipped.
  • node --test over .github/scripts/pr-merge/*.test.mjs: 271 pass, 0 fail.
  • node --test over .github/actions/resolve-config/*.test.mjs: 680 pass, 0 fail.
  • node --test over .github/scripts/work-resolve-issue/*.test.mjs: 257 pass, 0 fail.
  • bash scripts/ci-lanes-pr-hold.test.sh: 128 pass, 0 fail.
  • uv run --with pytest python -m pytest plugins/source-control/skills/babysit-prs/scripts/tests: 2738 passed.
  • After the review fixes, at 5fb7023: scripts/check-affected.sh --tests passed 14 of 14, run with PyYAML on PATH. Each new sync test fails with its fix reverted (seven case variants unheld, and check-kill-switch.mjs unheld).
  • The new tests cover each layer:

This PR changes .github/actions/resolve-config/**, which is on the HIGH floor, so a person merges it.

🤖 Generated with Claude Code

kyle-sexton and others added 2 commits October 23, 2026 21:46
resolve-config no longer forces .github/** into
merge.diff-check.denied-paths, and merge-holds.mjs drops its path-wide
plugin.json and marketplace.json globs. Its floor is the forced paths
plus babysit_risk.HIGH_FLOOR, and ci-status compares a changed manifest
base against head as the gate's manifest review does (manifestChanges,
kept equal to babysit_risk._manifest_changes by test_merge_holds_sync).
enable-auto-merge already scores manifests through evidence
--lane-reviews.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

PR body contract — issue linkage

This PR body does not yet satisfy the issue-linkage contract:

  • Missing a "## Related" section. List related PRs, ADRs, or decision-log entries this PR does not close.

Edit the body and this comment updates itself on the next run.

Review fixes for the hold-path floor cut:

- matchesHoldPath folds case, as glob_match and costHold do, so
  .github/codeowners or .github/actions/Resolve-Config/ holds.
- The HIGH floor and HOLD_PATH_FLOOR hold .github/actions/check-kill-switch/**:
  check-trusted-trigger and resolve-config write their verdicts through its
  writeOutputs. A test walks both actions' imports and fails on any file the
  floor does not hold, and the floor test compares matched paths, case
  variants included, instead of glob strings.
- Manifest contents decode as strict UTF-8 with the BOM kept, and an integer
  past int_max_str_digits holds, as the gate's json.loads refuses it.
- A path hold and a manifest hold both reach the reason.
- New manifest cases: hooks true to null and to false, a 20-digit integer,
  -0.0 to 0.0.
- The workflows premise names what it covers: lanes App pushes only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton marked this pull request as ready for review October 24, 2026 02:17
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-24T02:19:28.437178Z 5fb7023 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental review of 5fb7023..818f914 (adds .github/actions/request-lane-token/** to the hold floors): no findings.

  • HOLD_PATH_FLOOR (.github/scripts/pr-merge/merge-holds.mjs) and HIGH_FLOOR (plugins/source-control/skills/babysit-prs/scripts/babysit_risk.py) gain the same glob, so the PR's sync test (JS floor contains HIGH_FLOOR) should still hold. I did not run it.
  • The files in .github/actions/request-lane-token/ import nothing from other directories (the README mentions in-repo paths only as links or uses: examples). The new glob therefore leaves no imported file unheld, which the PR's import-walk test should confirm. This is my read, not a test run.
  • The docs paragraph in docs/conventions/pr-pipeline/README.md matches the code.
  • No hold class applies: the incremental change touches no trusted-actors, check-trusted-trigger, CODEOWNERS or .gitmodules path, and adds no model or API spend.
  • No closing issue, so no acceptance criteria were checked.

Not a finding, but worth a look: the changelog fragment .changes/source-control/agent-ac72c39ea4f0fbf44-57679074.md (line 7) lists the HIGH_FLOOR additions but names only check-kill-switch, not the new request-lane-token entry. The file is not in the diff, so I could not anchor a comment on it. If the plugin changelog should list every floor path, add one line for it.

I ran no tests or commands; everything above is from reading the diff and files.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

claude-security-review: 1 inline finding(s) on 5fb7023524910e8703d46a1d0f12c64a59711890; the summary is in this lane's pull request comment.

claude-lane: claude-security-review

@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

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

Incremental security review of 818f914 against 5fb7023: I found no security issues.

The change adds .github/actions/request-lane-token/** to the hold-path floor in three places: HOLD_PATH_FLOOR in merge-holds.mjs, HIGH_FLOOR in babysit_risk.py, and the pipeline README. This makes the control stricter. A lane PR that touches the action that mints the lanes App token now scores HIGH and is held from unattended merge.

  • The two lists carry the same glob. The PR's sync test (test_merge_holds_sync.py), which checks that HOLD_PATH_FLOOR contains HIGH_FLOOR, therefore still holds. I did not run it.
  • The action's own imports are only Node built-ins in a test file, so no other action file needs the floor to cover it.
  • The change touches no trust-gate path: not trusted-actors/**, check-trusted-trigger/**, a CODEOWNERS file or a .gitmodules file.
  • It touches no plugin manifest and removes no instruction.
  • The PR description contains no embedded instructions.

manifest: not applicable, because the incremental diff touches no plugin.json or marketplace.json.

critical-findings: 0

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 24, 2026
Merged via the queue into main with commit eb8e9f9 Oct 24, 2026
43 checks passed
@kyle-sexton
kyle-sexton deleted the agent-ac72c39ea4f0fbf44 branch October 24, 2026 02:46
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.

1 participant