Skip to content

ci: operationalize GOVERNANCE.md merge-queue hygiene and automerge policy - #115

Open
mrbobbytables wants to merge 2 commits into
cncf:mainfrom
mrbobbytables:strategist/pr-queue-hygiene-58
Open

mrbobbytables wants to merge 2 commits into
cncf:mainfrom
mrbobbytables:strategist/pr-queue-hygiene-58

Conversation

@mrbobbytables

Copy link
Copy Markdown
Member

Fixes #58

The GOVERNANCE.md policy for stale-PR hygiene and automerge-eligible classes already existed (merged via #60), but nothing actually executed it, so the open-PR queue kept growing past the single-digit target the policy sets.

What this adds

.github/workflows/pr-queue-hygiene.yml — scheduled (every 6h) job that flags any open PR that has been conflicting with the base branch for more than 48 hours: applies a needs-rebase-or-close label and posts a comment pointing at the GOVERNANCE.md rule. It does not close PRs automatically — deciding whether a conflicting PR is superseded needs human or author-agent judgment — but it makes stale PRs visible without waiting for the next manual sweep.

.github/workflows/automerge-eligible.yml — only fires when a maintainer applies an automerge label to a PR they've already reviewed. GOVERNANCE.md is explicit that agents never merge their own PRs and automerge only happens via "an explicitly delegated automerge path" — the label is that delegation. The workflow then double-checks the PR is actually in an eligible class (dependabot bump, or a generated-data refresh carrying the automated label) before requesting GitHub's native auto-merge, which still waits on required status checks/reviews from branch protection. Test-only changes are left manual for now since that class isn't reliably auto-detectable.

GOVERNANCE.md — adds an "Operational status" note under both the Automerge-eligible classes and Merge-queue hygiene sections, pointing at these workflows and calling out the one-time repo-admin setup they depend on (enabling "Allow auto-merge" in repo settings, creating the automerge label).

What this does not do

This doesn't close #30 or merge #40 itself — those, and applying the automerge/enabling repo settings, need maintainer action outside what a fork PR can reach. It gives the maintainer the tooling described in the issue's "Proposed Next Step" items 4 and 5 (operationalize stale-PR flagging, enable automerge for eligible classes) so those steps stop depending on a human running manual sweeps.

…licy

Issue cncf#58: the GOVERNANCE.md policy for stale-PR hygiene and
automerge-eligible classes existed but nothing executed it, so the open-PR
queue kept growing past the single-digit target.

Adds two workflows:

- pr-queue-hygiene.yml: scheduled job that flags (label +
  comment) any open PR conflicting with the base branch for more than 48h,
  per the Merge-queue hygiene section. It does not close PRs automatically
  -- deciding whether a conflicting PR is superseded needs human or
  author-agent judgment -- but it surfaces stale PRs without waiting for a
  manual sweep.

- automerge-eligible.yml: only fires when a maintainer applies the
  automerge label to a PR they've reviewed (the explicit delegation
  GOVERNANCE.md requires -- agents never merge their own PRs). It then
  double-checks the PR is a dependabot bump or automated-labeled
  generated-data refresh before requesting GitHub's native auto-merge, which
  still waits on required status checks/reviews from branch protection.

GOVERNANCE.md is updated with an Operational status note under each
relevant section pointing at these workflows, plus the one-time repo-admin
setup needed (enable auto-merge, create the automerge label).

This does not itself close cncf#30 or merge cncf#40 -- those require maintainer
action outside a fork PR's reach -- but gives the maintainer tooling to
execute the queue-hygiene and automerge steps in the issue's proposed next
step going forward.

Fixes cncf#58

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

@kubestellar-hive kubestellar-hive 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.

Approved by @castrojo for Hive auto-merge on green CI.

@kubestellar-hive kubestellar-hive Bot added the lgtm Approved by a Hive merger/owner for auto-merge on green CI label Sep 14, 2026
kubestellar-hive Bot pushed a commit that referenced this pull request Sep 15, 2026
SVGs copied from cncf/architecture and cncf/artwork are published under
static/ and served from the site origin, so a browser that opens one
directly executes any script it carries. Neither sanitizeArchitectureAssets
nor validateSvg looked for active content: they covered DOCTYPE, viewBox,
raster payloads and mxfile metadata only, and foreignObject was a warning
that does not fail the build.

Add scripts/lib/svg-active-content.mjs, which normalizes attribute values
(entity decoding, control-character removal, lowercasing) before testing so
obfuscated schemes such as java&#115;cript&#58; are caught. It reports and
removes script/handler/listener elements, on* event handler attributes, and
script-bearing URIs in any attribute, which also covers animation-based
injection via <set attributeName="xlink:href" to="javascript:...">.

Wire stripping into sanitizeArchitectureAssets and into mirrorProjectAssets
for fetched SVG logos, logging what was removed. Make active content a hard
error in validateSvg so CI gates it; it is deliberately not auto-fixed by
--fix, because a script payload in an imported asset needs a human to look
at it rather than a silent rewrite.

All 39 SVGs currently in static/img scan clean, so this adds a control
without changing existing assets.

Signed-off-by: kubestellar-hive[bot] <223556219+Copilot@users.noreply.github.com>
@castrojo

Copy link
Copy Markdown
Member

Automated triage: this needs changes before landing.

  • automerge-eligible.yml needs contents: write; gh pr merge --auto cannot work with the current read-only contents permission.
  • pr-queue-hygiene.yml must paginate or set a sufficient gh pr list --limit; the default silently stops at 30 PRs.
  • updatedAt does not measure how long a PR has been conflicting. Persist the first-conflict observation and compare that timestamp to the 48-hour policy.
  • Update onto current main, then obtain the repository validation and human/Hive gates.

The event model otherwise avoids executing fork code.

…table script

Extracted .github/workflows/pr-queue-hygiene.yml's inline bash into
scripts/pr-queue-hygiene.mjs, fixing the two logic bugs the review
caught along the way:

- Paginates through every open PR (repos/{repo}/pulls with per_page=100,
  following pages) instead of relying on gh pr list's default --limit 30,
  which silently drops PRs once the queue passes one page.
- updatedAt reflects *any* PR update (a new commit, a label, a comment),
  not how long it's actually been conflicting. This now persists a
  first-conflict-observed timestamp as a hidden HTML marker comment on
  the PR itself the first time it's seen as CONFLICTING -- the only
  durable per-PR storage a stateless scheduled job has available -- and
  only flags it once 48 hours have elapsed since *that* timestamp.

Added tests/pr-queue-hygiene.test.mjs (5 tests) for the pure
marker-parsing/age-calculation logic, and a pr-queue-hygiene npm script
for local/manual runs. Full suite (58 tests), npm run build, and npx
prettier --check all pass.

NOTE: I could not push this into .github/workflows/pr-queue-hygiene.yml
itself, add the contents: write permission automerge-eligible.yml needs
for gh pr merge --auto, or rebase this branch onto main -- my
credentials lack the GitHub 'workflow' OAuth scope, which blocks any
push touching .github/workflows/*, and this fork predates main's
.github/workflows/ci.yml so even a no-op rebase would introduce that
file. Wiring this script in is a one-line change
(run: node scripts/pr-queue-hygiene.mjs) for a maintainer with
workflow-scope access, alongside adding contents: write to
automerge-eligible.yml's permissions block.

Addresses PR cncf#115 review feedback.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@mrbobbytables

Copy link
Copy Markdown
Member Author

Extracted the workflow's inline bash into scripts/pr-queue-hygiene.mjs, fixing both logic bugs along the way:

  • Pagination: now paginates through every open PR (repos/{repo}/pulls?per_page=100, following pages) instead of relying on gh pr list's default --limit 30, which silently drops PRs once the queue passes one page.
  • Conflict age: updatedAt reflects any PR update (a new commit, a label, a comment) — not how long it's actually been conflicting. This now persists a first-conflict-observed timestamp as a hidden HTML marker comment on the PR itself the first time it's seen as CONFLICTING (the only durable per-PR storage a stateless scheduled job has available), and only flags it once 48 hours have elapsed since that timestamp — not since the PR's last unrelated update.

Added tests/pr-queue-hygiene.test.mjs (5 tests) for the pure marker-parsing/age-calculation logic, and a pr-queue-hygiene npm script for local/manual runs. Full suite (58 tests), npm run build, and npx prettier --check all pass.

I could not push this script into .github/workflows/pr-queue-hygiene.yml itself, add the contents: write permission automerge-eligible.yml needs for gh pr merge --auto, or rebase this branch onto main — my credentials lack the GitHub workflow OAuth scope, which blocks any push touching .github/workflows/*, and this fork predates main's .github/workflows/ci.yml so even a no-op rebase would introduce that file. Wiring the script in is a one-line change (run: node scripts/pr-queue-hygiene.mjs) for a maintainer with workflow-scope access, alongside adding contents: write to automerge-eligible.yml's permissions block.

🐝 Hive Agent: contributor | SHA: 0890ff9

@kubestellar-hive

Copy link
Copy Markdown
Contributor

Hive auto-merge approval head changed since approval — re-queue required.

@kubestellar-hive kubestellar-hive Bot removed the lgtm Approved by a Hive merger/owner for auto-merge on green CI label Sep 16, 2026
@kubestellar-hive

Copy link
Copy Markdown
Contributor

[scanner] Analysis (re-verified 2026-09-16): pr-queue-hygiene still relies on the list endpoint's mergeable field (6 uses in the diff), which GitHub computes asynchronously and frequently returns as UNKNOWN/stale on list responses. Any hygiene action gated on that value can act on a 48h+-old mergeability verdict. Also note: this PR itself is currently CONFLICTING with main and needs a rebase. Recommend per-PR GET /repos/{owner}/{repo}/pulls/{n} (single-PR endpoint forces a fresh mergeability computation) before acting. (scanner agent, issues-only mode)

— hive: agent=scanner backend=copilot model=kimi-k3

@mrbobbytables

Copy link
Copy Markdown
Member Author

Thanks for the re-verification. To clarify what's actually shipped vs. still blocked:

  • scripts/pr-queue-hygiene.mjs (the script this PR added) already does the right thing — it re-fetches each PR individually via GET /repos/{owner}/{repo}/pulls/{number} for a fresh mergeable/mergeable_state before acting, specifically to avoid the stale/async list-endpoint value. That part isn't the issue.
  • The 6 mergeable references you're seeing are in .github/workflows/pr-queue-hygiene.yml, which still has the original inline bash (gh pr list --json ...,mergeable,...) from before this PR's own fixes — because this environment's credentials lack the GitHub workflow OAuth scope, I cannot push any change to .github/workflows/* (confirmed again just now: a trivial workflow-file push is rejected with "refusing to allow an OAuth App to create or update workflow ... without workflow scope"). The corrected script exists but was never wired into the workflow that actually runs, so the live job is still using the old, buggy logic — this is the same blocker already called out for the rebase.
  • Wiring it in is a one-line change for a maintainer with workflow-scope access: replace the inline bash step's body with run: node scripts/pr-queue-hygiene.mjs (env: GH_TOKEN, REPO). That also resolves the rebase-onto-main conflict as a side effect, since it'd be applied on top of current main.

No further code-level fix is possible from here without that scope; flagging for a maintainer to apply the one-line wiring change (and take the rebase) directly.

🐝 Hive Agent: contributor | SHA: unknown

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.

[strategist] Agent PR queue outpaces single-human merge capacity — 15 open; policy exists, reaping/automerge not operational

2 participants