Skip to content

[PR-limits Phase 3] Wire the admission gate into dev-lead-fix-issue.sh (enforce the org-wide cap) #1003

Closed

Description

@don-petry

Story

Wire the already-built PR-limit admission gate into this repo's dev-lead PR-creation path so the org-wide open-automation-PR cap is actually enforced at the source, per the Pull-Request-Limits ADR (petry-projects/.github docs/initiatives/pull-request-limits-adr.md). This is Phase 3 (the "apply path") of epic combsmith/.github#505.

Background (all delivered, all inert today)

GitHub exposes no native max-open-PRs surface (ADR §2–§3), so enforcement is source-side: before a PR-creating automation opens a PR, it asks a guard whether the standing queue of open, non-draft automation PRs is at the cap. Two pieces already exist on petry-projects/.github main (public repo):

  • Guard library — scripts/lib/pr-limit-gate.sh, function plg_admission_gate <source> → prints decision=allow|defer, returns 0 on allow / 1 on defer. Honors DRY_RUN / DEV_LEAD_DRY_RUN. Exempt actors/labels are never counted nor blocked.
  • Config (single source of truth) — standards/pr-limits.json: signed-off org-wide automation_open_pr_cap: 50 (a soft ceiling; ~44 automation PRs open at baseline, so it blocks nothing today), per_source_caps: {}, exempt actors incl. dependabot[bot] + security label.

Nothing calls plg_admission_gate yet. This story adds the one call site.

Integration point (verified)

scripts/dev-lead-fix-issue.sh is the single choke point where automation PRs are created:

  • L221 git checkout -b "$branch"
  • L277 git push --set-upstream origin "$branch"
  • L279 gh pr create ...

scripts/initiative-driver.sh does not open PRs — it dispatches to dev-lead, which does. So gating dev-lead-fix-issue.sh covers dev-lead and initiative-driven PRs.

Acceptance criteria

  1. Admission check before branch/PR creation. Immediately before git checkout -b (L221) in scripts/dev-lead-fix-issue.sh, source the guard and call plg_admission_gate "dev-lead". On defer, do not create the branch/PR.
  2. Guard + config sourced from the public repo (single source of truth). Fetch scripts/lib/pr-limit-gate.sh and standards/pr-limits.json from petry-projects/.github@main at runtime (e.g. gh api .../contents with the existing $GH_TOKEN, or raw URL) rather than vendoring a copy — the config must stay authoritative in petry-projects/.github. Point the guard at the fetched config via $PR_LIMITS_CONFIG.
  3. Graceful defer — never drop the issue. On defer: leave the issue in its ready/dev-lead state (or re-queue via the existing retry path) so it is re-attempted when the queue drains, and post a single concise issue comment noting deferred: open-automation-PR queue N/50 reached — will retry (idempotent; do not spam on every retry). No PR is opened.
  4. Dry-run honored. When DEV_LEAD_DRY_RUN=true, the gate prints the would-be decision but returns allow (guard already implements this) — so dry-run runs never defer. Verify the engine's existing dry-run path is unchanged.
  5. Fail-open. A guard/fetch error (network, missing config, bad JSON) must allow PR creation and log a warning — never wedge the engine. (The guard's counting is already fail-open; extend the same posture to the fetch/source step.)
  6. Tests. Add coverage mirroring the existing tests/dev-lead/ patterns (integration or e2e with gh stubbed): under-cap → PR created as today; at/over-cap → deferred (no gh pr create, issue left for retry, one comment); dry-run over-cap → PR still created. Register per this repo's bats/test-runner convention.

Non-goals

Scope note

Enforcement is org-wide (this is the central engine serving all petry-projects repos) and the cap counts org-wide. At 50 vs a ~44 baseline it refuses nothing today; it only bounds runaway growth. If a repo-scoped pilot is preferred instead, add a simple target_repo allowlist guard around the gate call and note it — but default is org-wide-soft.

References (verified to exist)

  • petry-projects/.github: scripts/lib/pr-limit-gate.sh, standards/pr-limits.json, docs/initiatives/pull-request-limits-adr.md (§7.2–§7.4)
  • this repo: scripts/dev-lead-fix-issue.sh (L221/L277/L279), scripts/dev-lead-retry.sh, tests/dev-lead/

Phase 3 apply path for epic combsmith/.github#505. Kept deliberately small: one call site + runtime fetch + defer handling + tests. Note this edits the engine's own core PR-creation script — implement carefully and keep the dry-run/fail-open guarantees.

Activity

  1. github-actions commented on Jul 1, 2026

    @github-actions
    Contributor

    Thanks for the thorough write-up and for linking all the upstream references — this is a well-scoped Phase 3 story. Before we scope the implementation, could you clarify the preferred runtime fetch mechanism: should dev-lead-fix-issue.sh use gh api (authenticated, respects $GH_TOKEN) or a raw curl against the public URL for pr-limit-gate.sh and pr-limits.json? That choice affects the fail-open error handling and whether we need to cache the fetched files across retries.

  2. don-petry commented on Jul 1, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead Implementation Plan

    Issue: #1003 — [PR-limits Phase 3] Wire the admission gate into dev-lead-fix-issue.sh (enforce the org-wide cap)

    Scope

    Add a single admission-gate call site in scripts/dev-lead-fix-issue.sh immediately before branch/PR creation: fetch the guard (scripts/lib/pr-limit-gate.sh) + config (standards/pr-limits.json) from petry-projects/.github@main at runtime, call plg_admission_gate "dev-lead", and on defer skip PR creation while leaving the issue in its ready/dev-lead state with one idempotent deferral comment — all fail-open and dry-run-safe.

    Implementation Checklist

    • scripts/dev-lead-fix-issue.sh: add _plg_fetch (runtime fetch of guard + config from the public standards repo via gh api .../contents + base64, with a PLG_STANDARDS_DIR local-override for tests, mirroring seed-repo-template.sh:_fetch_standard)
    • scripts/dev-lead-fix-issue.sh: add admission_gate_allows — sources the fetched guard, points it at the fetched config via $PR_LIMITS_CONFIG, calls plg_admission_gate "dev-lead"; fail-open (return allow + ::warning::) on any fetch/source/undefined-function error
    • scripts/dev-lead-fix-issue.sh: add idempotent post_deferral_comment (marker <!-- dev-lead-issue-deferred -->, checks prior comments so retries don't spam)
    • scripts/dev-lead-fix-issue.sh: call the gate in main() immediately before git checkout -b (L221); on defer, post the comment, leave the issue labeled dev-lead, and exit 0 (no branch, no PR)

    Tests to Write

    • tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats (mirrors test_fix_issue.bats stub patterns; injects a fake guard + config via PLG_STANDARDS_DIR)
      • under-cap → allow → PR created as today (gh pr create invoked, exit 0)
      • at/over-cap → defer → no gh pr create, one deferral comment posted, issue left for retry, exit 0
      • over-cap but retry (deferral comment already present) → not re-posted (idempotent)
      • dry-run over-cap → existing dry-run path unchanged (exit 0, [dry-run], not deferred)
      • fail-open: guard fetch fails → PR still created + ::warning::

    Test Command

    bats tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats (full: bats tests/dev-lead/unit/) and bash .dev-lead/scripts/dev-lead-lint.sh

  3. don-petry commented on Jul 1, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: Implementation Complete

    Wired the PR-limit admission gate into the single dev-lead PR-creation choke point so the org-wide open-automation-PR cap is enforced at the source (Phase 3, epic combsmith/.github#505).

    Plan Execution

    • _plg_fetch — runtime fetch of the guard + config from petry-projects/.github@main (contents API + base64 via $GH_TOKEN; PLG_STANDARDS_DIR local-checkout override for tests/offline, mirroring seed-repo-template.sh). Config stays authoritative in the public repo — not vendored.
    • admission_gate_allows — sources the fetched guard, points it at the fetched config via $PR_LIMITS_CONFIG, calls plg_admission_gate "dev-lead"; fail-open (allow + ::warning::) on any fetch/source/undefined-function error.
    • deferral_comment_exists / post_deferral_comment — single idempotent note (marker <!-- dev-lead-issue-deferred -->), so drain-retries don't spam.
    • Call site in main() immediately before git checkout -b: on defer, open nothing, leave the issue in its ready/dev-lead state, post the note, exit 0.
    • New test file mirroring tests/dev-lead/ stub patterns; auto-registered (CI runs bats tests/dev-lead/unit/).

    Test Results

    Gate tests (bats tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats):

    1..6
    ok 1 pr-limit gate: allow (under cap) → PR is created, no deferral comment
    ok 2 pr-limit gate: defer (at cap) → no PR created, exits 0
    ok 3 pr-limit gate: defer → posts a single deferral comment with the retry note
    ok 4 pr-limit gate: defer is idempotent → no second comment when one already exists
    ok 5 pr-limit gate: dry-run over-cap → existing dry-run path unchanged, not deferred
    ok 6 pr-limit gate: fetch failure → fails open (PR created) with a warning
    

    Regression — existing fix-issue suite: ok 16 fix-issue: lint fails → posts lint-failure comment on the issue (16/16). Full tests/dev-lead/unit/ = 654/655; the one failure (test_engine_mcp.bats #28, "REVIEW_MCP_DEBUG healthy handshake") is pre-existing and unrelated — it fails identically on a clean tree (verified via git stash) and touches engine.sh MCP debug, not this change. Required lint bash .dev-lead/scripts/dev-lead-lint.sh → all checks passed; shellcheck -S warning scripts/dev-lead-fix-issue.sh → clean.

    Files Changed

    • scripts/dev-lead-fix-issue.sh: added the fetch + gate + idempotent-defer helpers and the single call site before branch/PR creation.
    • tests/dev-lead/unit/test_fix_issue_pr_limit_gate.bats (new): under-cap→PR created; at/over-cap→deferred (no gh pr create, one comment, issue left for retry); idempotent (no second comment); dry-run over-cap→existing path unchanged; fetch-failure→fail-open.

    Notes

    • Dry-run (AC4): the gate sits after the existing DEV_LEAD_DRY_RUN early-exit (the point "immediately before git checkout -b"), so a dry-run never reaches the gate and never defers — the existing dry-run path is unchanged, exactly as AC4 requires. AC6's "dry-run over-cap → PR still created" is therefore verified as "dry-run over-cap → not deferred, existing dry-run path runs unchanged" (a real dry-run never opens a PR regardless).
    • Scope: org-wide-soft default per the ADR (cap 50 vs ~44 baseline → refuses nothing today; bounds runaway growth). No cap/exempt-list changes; no per-source sub-caps.
    • Guard is trusted first-party and sourced into the current shell; a genuine return 1 from it means a real defer (its own counting step already fail-opens).
  4. don-petry commented on Jul 1, 2026

    @don-petry
    CollaboratorAuthor

    Dev-Lead: issue #1003 implementation failed — will retry

    The engine failed (engine-error — e.g. a timeout or transient engine error). This was attempt 1 of 3; dev-lead will retry automatically. You can also re-apply the dev-lead label to retry now.

  5. don-petry commented on Jul 1, 2026

    @don-petry
    CollaboratorAuthor

    Superseded by hand-authored PR #1004 (which Closes #1003). The dev-lead engine implemented + validated this design here but hit the 1800s CI timeout before it could push, so removing the dev-lead label to stop futile 30-min retries. Tracking completion via #1004.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions