feat(skills): add merge-queue skill

[ralphbean]

Summary

• Adds the merge-queue skill from fullsend-ai/fullsend with four scripts for managing GitHub merge queues via the GraphQL API
• Covers enqueue, dequeue-reason investigation, queue-status listing, and await-and-enqueue (poll until checks pass then enqueue)

Test plan

• Verify enqueue-pr.sh successfully enqueues a PR on a repo with merge queues enabled
• Verify queue-status.sh lists current queue entries
• Verify dequeue-reason.sh shows removal reasons for previously dequeued PRs
• Verify await-and-enqueue.sh polls and enqueues once checks pass

🤖 Generated with Claude Code

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:28 PM UTC · Completed 9:43 PM UTC
Commit: 84c2058 · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Add merge-queue skill with GitHub merge queue management scripts

✨ Enhancement 📝 Documentation 🕐 20-40 Minutes

AI Description

• Introduces a new merge-queue skill for interacting with GitHub merge queues via GraphQL.
• Adds scripts to enqueue PRs, list queue entries, and inspect dequeue/removal reasons.
• Provides an await-and-enqueue helper to poll checks/approvals before enqueuing.

Diagram

graph TD
  U(["User/Agent"]) --> A["await-and-enqueue.sh"] --> E["enqueue-pr.sh"] --> GH["gh CLI"] --> API{{"GitHub API"}}
  U --> S["queue-status.sh"] --> GH --> API
  U --> D["dequeue-reason.sh"] --> GH --> API

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Implement as a `gh` extension (packaged tool)

• ➕ Better discoverability/install/update story via gh extension install
• ➕ Can provide richer UX, flags, and structured output contracts
• ➖ Higher maintenance surface (language/tooling, releases) than Bash scripts
• ➖ More work to bootstrap than copy-pasteable scripts

2. Use only GraphQL for required checks detection

• ➕ Single API surface (GraphQL) and potentially fewer auth/endpoint quirks
• ➖ Determining required checks via GraphQL is not always straightforward across rulesets/branch protection variants
• ➖ Current REST rulesets approach is pragmatic and may be more reliable

3. Provide a single entrypoint script with subcommands

• ➕ One command surface (e.g., merge-queue enqueue|status|reason|await)
• ➕ Centralizes shared parsing and error handling
• ➖ Less composable; larger script becomes harder to test/review incrementally
• ➖ Current small scripts are easy to understand and reuse independently

Recommendation: The current approach (small Bash scripts wrapping gh api + jq) is appropriate for a "skill" repository: it’s portable, easy to audit, and leverages existing gh authentication/context. If adoption grows, consider promoting this into a gh extension or a single subcommand-based entrypoint for better UX and distribution.

Files changed (5) +325 / -0

Enhancement (4) +258 / -0

await-and-enqueue.shPoll PR checks/reviews then enqueue when eligible +95/-0

Poll PR checks/reviews then enqueue when eligible

• Introduces a polling script that waits for required status checks to pass and for review approval before enqueuing. Uses repository rulesets to identify required checks, exits early on failed checks, and delegates the final enqueue operation to 'enqueue-pr.sh'.

skills/merge-queue/scripts/await-and-enqueue.sh

dequeue-reason.shExplain why a PR was removed from the merge queue +64/-0

Explain why a PR was removed from the merge queue

• Adds a script that queries PR timeline events for merge-queue removals and prints timestamps, reasons, and the commit SHA at removal time. Supports PR number (current repo) or full PR URL inputs.

skills/merge-queue/scripts/dequeue-reason.sh

enqueue-pr.shEnqueue a PR into the merge queue via GraphQL +48/-0

Enqueue a PR into the merge queue via GraphQL

• Adds a script that resolves a PR to its node ID and calls the 'enqueuePullRequest' GraphQL mutation. Includes basic GraphQL error detection and prints queue position and ETA when available.

skills/merge-queue/scripts/enqueue-pr.sh

queue-status.shList current merge queue entries for a branch +51/-0

List current merge queue entries for a branch

• Adds a script that queries the repository merge queue for a given branch (default 'main') and prints each entry’s position/state, PR metadata, enqueuer, and ETA. Defaults repo context from 'gh' if not provided.

skills/merge-queue/scripts/queue-status.sh

Documentation (1) +67 / -0

SKILL.mdDocument merge-queue skill usage and prerequisites +67/-0

Document merge-queue skill usage and prerequisites

• Adds a new skill description explaining when to use merge queue helpers and why (no native 'gh' support). Documents input formats, script usage, prerequisites, and common error cases.

skills/merge-queue/SKILL.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Context used

✅ Compliance rules (platform): 55 rules

✅ Skills: 4 invoked

  code-review

  code-implementation

  pr-review

  docs-review

1. Required checks fail-open ✓ Resolved 🐞 Bug ≡ Correctness

Description

In await-and-enqueue.sh, failures fetching required check contexts are silently treated as “no
required checks” (fallback to []). This can enqueue a PR after only the currently-reported checks
are non-pending and reviewDecision is APPROVED, even when required contexts were never retrieved or
are missing.

Code

skills/merge-queue/scripts/await-and-enqueue.sh[R25-31]

+# Extract owner/repo from the PR URL
+repo_nwo="$(echo "$pr_url" | sed -E 's|https://github.com/([^/]+/[^/]+)/pull/.*|\1|')"
+
+# Fetch required status checks from branch rulesets as a JSON array
+required_json="$(gh api "repos/$repo_nwo/rules/branches/$base_branch" \
+  --jq '[.[] | select(.type == "required_status_checks") | .parameters.required_status_checks[].context] | unique' 2>/dev/null || echo '[]')"
+

Relevance

⭐⭐⭐ High

Team accepts fail-closed hardening in bash gates/validation (e.g., numeric validation in PR #10;
fail-closed guard in PR #89).

PR-#10
PR-#89

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The script explicitly suppresses errors from the required-check lookup and substitutes an empty
required-check list, and later the “missing required checks” gate iterates only over that list; if
it’s empty, missing checks can never be detected and the loop will proceed to enqueue once pending
is empty and reviews are approved.

skills/merge-queue/scripts/await-and-enqueue.sh[25-31]
skills/merge-queue/scripts/await-and-enqueue.sh[44-56]
skills/merge-queue/scripts/await-and-enqueue.sh[83-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`await-and-enqueue.sh` suppresses errors when fetching required status check contexts and falls back to `[]`, making “API failure / unsupported endpoint / bad repo parse” indistinguishable from “no required checks”. Because the later gating for missing required checks is driven entirely by `$required`, an empty array disables that protection and can allow premature enqueue.

## Issue Context
The script intends to wait for *required* checks and approvals before enqueuing. It currently:
- derives `repo_nwo` via a `sed` parse of the PR URL
- calls `gh api repos/$repo_nwo/rules/branches/$base_branch` with `2>/dev/null || echo '[]'`

## Fix Focus Areas
- skills/merge-queue/scripts/await-and-enqueue.sh[25-31]
- skills/merge-queue/scripts/await-and-enqueue.sh[44-56]

## Suggested fix
1. Remove the fail-open fallback for required checks retrieval.
  - Capture the `gh api` exit code; if non-zero, print a clear error to stderr and `exit 1` (fail-closed), or explicitly require a user-provided override (env var) before continuing.
2. Make repo resolution robust without URL parsing:
  - Prefer reading `nameWithOwner` from `gh pr view` JSON rather than `sed` on `https://github.com/...`.
3. (Optional but recommended) Support both rulesets and classic branch protection required checks:
  - If the rules endpoint returns an empty set, attempt a second lookup against the branch-protection required contexts endpoint and merge/unique results.
4. Add a warning and stop if required checks cannot be determined (rather than silently continuing).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Protected skills/merge-queue files changed ✗ Dismissed 📜 Skill insight § Compliance

Description

This PR adds a new merge-queue skill and related scripts under the protected skills/
governance/infrastructure path, which requires explicit human review and must not be auto-approved.
It should include explicit authorization (ideally a linked issue/ADR in the PR description) to
satisfy auditability expectations for both protected-path and non-trivial feature changes.

Code

skills/merge-queue/SKILL.md[R1-9]

+---
+name: merge-queue
+description: >-
+  Use when you need to add a PR to a GitHub merge queue, check what's currently
+  queued, or find out why a PR was removed from the queue. The gh CLI has no
+  built-in merge-queue commands, so this skill provides scripts that use the
+  GraphQL API.
+allowed-tools: Bash(bash skills/merge-queue/scripts/*:*)
+---

Relevance

⭐⭐ Medium

Protected-path authorization suggestion seen, but outcome unclear (PR #59) and similar
governance-justification suggestion undetermined (PR #29).

PR-#59
PR-#29

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The compliance checklist calls for raising a finding when protected governance/infrastructure paths
(including skills/) are modified and when non-trivial changes are made without explicit
authorization via a linked issue. The diff adds a new skill definition under skills/merge-queue/
(e.g., skills/merge-queue/SKILL.md) and introduces new scripts (e.g.,
skills/merge-queue/scripts/await-and-enqueue.sh) that represent substantial new behavior, which
together place this change within both the protected-path and non-trivial-change requirements.

skills/merge-queue/SKILL.md[1-9]
skills/merge-queue/scripts/await-and-enqueue.sh[1-12]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR introduces a new `merge-queue` skill and scripts under the protected `skills/` governance/infrastructure path, but it does not include explicit authorization (preferably a linked issue/ADR) in the PR description. Changes under protected paths and non-trivial feature work must be explicitly authorized and receive human review (not auto-approval) to meet auditability requirements.

## Issue Context
The compliance checklist requires raising a finding when protected paths (including `skills/`) are modified and when non-trivial changes are not traceable to an approved/linked issue. This PR adds a new skill definition and supporting scripts that constitute substantial new behavior, so it should be justified with explicit authorization (ideally via a linked issue/ADR) in addition to normal review.

## Fix Focus Areas
- skills/merge-queue/SKILL.md[1-9]
- skills/merge-queue/scripts/await-and-enqueue.sh[1-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. Queue status unhandled errors ✓ Resolved 🐞 Bug ☼ Reliability

Description

queue-status.sh and dequeue-reason.sh assume their GraphQL responses always contain the expected
data shapes (mergeQueue entries and pullRequest fields) and never check for .errors or null
objects before dereferencing deep paths. Under set -euo pipefail, any query failure, missing
permissions, or absent data will cause jq to fail and abort the scripts with an opaque error
instead of a clear diagnostic.

Code

skills/merge-queue/scripts/queue-status.sh[R15-43]

+result="$(gh api graphql -f query='
+  query($owner: String!, $name: String!, $branch: String!) {
+    repository(owner: $owner, name: $name) {
+      mergeQueue(branch: $branch) {
+        entries(first: 50) {
+          nodes {
+            position
+            state
+            estimatedTimeToMerge
+            enqueuedAt
+            enqueuer { login }
+            pullRequest {
+              number
+              title
+              url
+              author { login }
+            }
+          }
+        }
+      }
+    }
+  }
+' -f owner="$owner" -f name="$name" -f branch="$branch")"
+
+count="$(echo "$result" | jq '.data.repository.mergeQueue.entries.nodes | length')"
+
+if [[ "$count" -eq 0 ]]; then
+  echo "Merge queue for ${repo}:${branch} is empty."
+  exit 0

Relevance

⭐⭐⭐ High

Reliability/diagnostic hardening under set -euo pipefail has been accepted/partially accepted (PR
#41).

PR-#41

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

In queue-status.sh, the response is parsed by immediately dereferencing
.data.repository.mergeQueue.entries.nodes without first checking whether the response contains
.errors or whether .data.repository.mergeQueue is null, so a failed query or missing/NULL field
will make jq error and the script exit due to set -e. Similarly, dequeue-reason.sh reads
.data.repository.pullRequest.title, .url, and .timelineItems directly and never validates that
.errors is absent or that .data.repository.pullRequest is non-null, meaning an
inaccessible/missing PR or failed query will also trigger a jq failure and terminate the script
under set -euo pipefail.

skills/merge-queue/scripts/queue-status.sh[15-44]
skills/merge-queue/scripts/dequeue-reason.sh[28-56]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`queue-status.sh` and `dequeue-reason.sh` assume their GraphQL responses always include the expected data (`.data.repository.mergeQueue.entries.nodes` and `.data.repository.pullRequest.*`) and do not check `.errors` or guard against null objects; under `set -euo pipefail`, `jq` errors abort the scripts without a helpful message. Update both scripts to validate GraphQL responses before dereferencing and to emit clear diagnostics for query failures and missing/unauthorized data.

## Issue Context
Both scripts use `jq` to parse GraphQL output but lack the `.errors` handling pattern already used in `enqueue-pr.sh`. `dequeue-reason.sh` already distinguishes a valid “no removals found” case via `count`, but it currently can’t distinguish that from “query failed / pullRequest null”, and `queue-status.sh` similarly can’t distinguish “mergeQueue missing/disabled” from a normal empty queue.

## Fix Focus Areas
- skills/merge-queue/scripts/queue-status.sh[15-44]
- skills/merge-queue/scripts/dequeue-reason.sh[28-56]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

4. No tests added for scripts ✗ Dismissed 📜 Skill insight ▣ Testability

Description

The PR adds new production behavior (merge-queue management scripts) but includes no test changes in
the PR. If there is no existing coverage for these scripts, regressions and API contract changes may
go undetected.

Code

skills/merge-queue/scripts/enqueue-pr.sh[R1-8]

+#!/usr/bin/env bash
+# Adds a pull request to a GitHub merge queue using the GraphQL API.
+# Usage: enqueue-pr.sh [PR_NUMBER_OR_URL]
+#
+# If no argument is given, uses the current branch's PR.
+# Requires: gh CLI authenticated with sufficient permissions, and jq.
+
+set -euo pipefail

Relevance

⭐⭐ Medium

They add tests for scripts in some PRs (PR #89), but no evidence of enforcing tests for every new
script.

PR-#89
PR-#37

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The compliance checklist requires verifying test coverage when production behavior changes; this PR
adds new scripts that implement runtime behavior, but no tests are introduced alongside them in the
change set shown.

skills/merge-queue/scripts/enqueue-pr.sh[1-8]
Skill: pr-review

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
New production behavior was added without corresponding test changes.

## Issue Context
The PR introduces new shell scripts that call GitHub APIs; without tests (or an explicit explanation of existing coverage), failures may not be detected early.

## Fix Focus Areas
- skills/merge-queue/scripts/enqueue-pr.sh[1-48]
- skills/merge-queue/scripts/await-and-enqueue.sh[1-95]
- skills/merge-queue/scripts/queue-status.sh[1-51]
- skills/merge-queue/scripts/dequeue-reason.sh[1-64]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

[fullsend-ai-review]

Review

Re-review of 90e2a01 (prev: 459235d). The fix commit addressed all prior findings: StatusContext/CheckRun handling is now correct (.name // .context for keys, .conclusion // .state // .status for values, case-insensitive test() calls), ETA display uses a conditional to avoid unknowns, and SKILL.md now has a Constraints section documenting the rulesets-only limitation. The prior review's medium-severity logic-error finding is resolved.

No medium or higher findings remain. Two new low-severity edge cases were identified. Approving with follow-up notes.

Findings

Low

• [edge-case] skills/merge-queue/scripts/await-and-enqueue.sh:56 — A CheckRun with conclusion: "STALE" (set by GitHub when a check has been incomplete for 14+ days) is not matched by either the failure regex or the terminal-state regex. It would be classified as "pending," causing the script to poll indefinitely since there is no timeout mechanism. While STALE checks are rare, a stale required check would cause an unbounded hang.
  Remediation: Add STALE to the failure regex (alongside FAILURE|ERROR|CANCELLED|...), since a stale check will never transition to success.

• [error-handling] skills/merge-queue/scripts/enqueue-pr.sh:46 — If the enqueuePullRequest mutation succeeds (no errors field) but mergeQueueEntry is null, the script prints PR added to merge queue at position null (ETA: unknown). This is misleading — the PR was not actually enqueued. This can happen when the PR doesn't meet queue requirements but the mutation doesn't raise a GraphQL-level error.
  Remediation: After extracting position, check if it equals null and emit an error message instead of the success message.

• [code-organization] skills/merge-queue/SKILL.md:8 — The allowed-tools frontmatter field is not used by any other skill in the repo. This introduces a new convention without precedent.

• [architectural-divergence] skills/merge-queue/scripts/ — This is the first skill to include a scripts/ subdirectory with executable shell scripts. All 10 existing skills are markdown-only (SKILL.md with optional sub-agents/ markdown). While nothing prohibits this pattern, it establishes a new precedent for future skills.

---

Resolved from prior review: The medium-severity [logic-error] finding about StatusContext handling in await-and-enqueue.sh has been correctly fixed in commit 90e2a01. The fix properly handles both CheckRun (.name, .conclusion) and StatusContext (.context, .state) objects, adds case-insensitive matching, and the ETA display and Constraints section have also been addressed.

Previous run

Review

Re-review of 459235d (prev: 84c2058). The fix commit added fail-closed error handling on API calls and GraphQL error checks in queue-status.sh and dequeue-reason.sh — good improvements. The prior review's high-severity protected-path finding was a false positive (no protected-path policy exists in this repo; CODEOWNERS uses a wildcard rule only).

One medium finding persists from the prior review.

Findings

Medium

• [logic-error] skills/merge-queue/scripts/await-and-enqueue.sh:50 — The jq expression uses .name as the map key for building the check status map, but gh pr view --json statusCheckRollup returns both CheckRun objects (with .name) and StatusContext objects (with .context). StatusContext objects have .name as null, causing all of them to collide on a single null key and silently drop all but one. The value expression .conclusion // .status also misses .state, which is the field StatusContext uses. This creates a fail-open condition where the script may enqueue a PR whose status checks (reported via the Commit Status API) have not actually passed.
  Remediation: Change the map-building expression to ($checks | map({((.name // .context // "unknown")): (.conclusion // .state // .status // "PENDING")}) | add // {}) as $map |. Note: if applying this fix, also add the case-insensitive flag to the test() calls (e.g., test("FAILURE|..."; "i")) since StatusContext .state values may be lowercase.

Low

• [edge-case] skills/merge-queue/scripts/queue-status.sh:57 — The ETA display unconditionally appends a literal s suffix via the format string ETA: \(.estimatedTimeToMerge // "unknown")s. When estimatedTimeToMerge is null, this produces ETA: unknowns instead of ETA: unknown.
  Remediation: Use a conditional: ETA: \(if .estimatedTimeToMerge then "\(.estimatedTimeToMerge)s" else "unknown" end)

• [edge-case] skills/merge-queue/scripts/await-and-enqueue.sh:29 — The script only queries the rulesets API (repos/{nwo}/rules/branches/{branch}) to discover required checks. Repositories using classic branch protection rules will return no rulesets, causing the script to treat all checks as non-required and potentially enqueue prematurely.
  Remediation: Fall back to the classic branch protection API when rulesets returns no required checks, or document that only ruleset-based protection is supported.

• [architectural-divergence] skills/merge-queue/scripts/ — This is the first skill to include a scripts/ subdirectory with executable shell scripts. All 10 existing skills are markdown-only (SKILL.md with optional sub-agents/ markdown). While nothing in AGENTS.md prohibits this pattern, it is novel and worth noting for future reference.

• [code-organization] skills/merge-queue/SKILL.md:8 — The allowed-tools frontmatter field is not used by any other skill in the repo. This introduces a new convention without precedent.

• [pattern-inconsistency] skills/merge-queue/SKILL.md — The skill lacks a ## Constraints section present in most existing skills.

---

Corrections from prior review: The prior review's high-severity [protected-path] and medium-severity [missing-authorization] findings have been removed. No protected-path policy exists in this repo (CODEOWNERS has only * @fullsend-ai/core). The [missing-authorization] finding (no linked issue) is a process preference, not a code defect — AGENTS.md §3 governs agent behavior, not human contributor PRs.

Previous run

Review

Findings

High

• [protected-path] skills/merge-queue/ — All 5 files in this PR are under the skills/ protected path. The PR has no linked issue providing authorization for modifying governance/infrastructure files. Human approval is always required for protected-path changes, regardless of context.
  Remediation: Link the PR to an issue that authorizes the merge-queue skill addition and explains why these protected files need to be changed.

Medium

• [missing-authorization] — Non-trivial change (5 new files adding a merge-queue skill) with no linked issue. AGENTS.md section 3 requires that every changed line trace directly to issue scope. New skills and multi-file additions are non-trivial and require explicit authorization.
  Remediation: Create an issue proposing the merge-queue skill addition and link it to this PR.

• [architectural-divergence] skills/merge-queue/scripts/ — This skill introduces a scripts/ directory with 4 bash scripts, establishing a new architectural pattern. None of the existing skills in this repository use shell scripts or have scripts/ directories — all existing skills are SKILL.md files with optional sub-agents/ directories containing markdown files.
  Remediation: Either refactor to follow the existing markdown-only skill pattern, or provide architectural documentation (ADR) justifying the introduction of script-based skills.

• [logic-error] skills/merge-queue/scripts/await-and-enqueue.sh:47 — The jq expression uses .name to build the check status map, but gh pr view --json statusCheckRollup returns both CheckRun objects (with .name) and StatusContext objects (with .context). StatusContext objects from the Commit Status API will have .name as null, causing all StatusContexts to collide on a single null key and fail to match required check names. The expression also reads .conclusion // .status but StatusContext objects use .state. This makes status checks reported via the Commit Status API invisible to the polling logic, potentially causing infinite polling or enqueuing a PR with failing checks.
  Remediation: Use (.name // .context) instead of .name for the map key, and (.conclusion // .state // .status // "PENDING") for the value.

Low

• [skill-resolution-coherence] skills/merge-queue/ — The PR description states this skill is "from fullsend-ai/fullsend," suggesting it may exist at the platform tier. If it already exists upstream, adding it at repo-level creates duplication. Clarify whether this is an override, prototype, or repo-specific skill.

• [code-organization] skills/merge-queue/SKILL.md:8 — The allowed-tools frontmatter field is not used by any other skill in this repo, making this a novel pattern without precedent in the existing codebase.

• [edge-case] skills/merge-queue/scripts/queue-status.sh:51 — The ETA display appends a literal s suffix unconditionally, producing ETA: unknowns when estimatedTimeToMerge is null.

• [edge-case] skills/merge-queue/scripts/await-and-enqueue.sh:29 — The script only queries the newer branch rulesets API for required checks, not the classic branch protection API. Repos using classic protection will skip required check verification.

• [pattern-inconsistency] skills/merge-queue/SKILL.md — The skill lacks a "Constraints" section, inconsistent with other skills that invoke external commands. The existing "Common errors" section partially addresses this.

---

Labels: PR adds a new merge-queue skill feature.

---

Protected paths detected — this PR modifies files under one or more
protected paths. The review agent cannot approve PRs that touch these paths.
A human reviewer must approve this PR.

Protected files in this PR:

• skills/merge-queue/SKILL.md
• skills/merge-queue/scripts/await-and-enqueue.sh
• skills/merge-queue/scripts/dequeue-reason.sh
• skills/merge-queue/scripts/enqueue-pr.sh
• skills/merge-queue/scripts/queue-status.sh

Previous run

Review

Re-review of 459235d (prev: 84c2058). The fix commit added fail-closed error handling on API calls and GraphQL error checks in queue-status.sh and dequeue-reason.sh — good improvements. The prior review's high-severity protected-path finding was a false positive (no protected-path policy exists in this repo; CODEOWNERS uses a wildcard rule only).

One medium finding persists from the prior review.

Findings

Medium

• [logic-error] skills/merge-queue/scripts/await-and-enqueue.sh:50 — The jq expression uses .name as the map key for building the check status map, but gh pr view --json statusCheckRollup returns both CheckRun objects (with .name) and StatusContext objects (with .context). StatusContext objects have .name as null, causing all of them to collide on a single null key and silently drop all but one. The value expression .conclusion // .status also misses .state, which is the field StatusContext uses. This creates a fail-open condition where the script may enqueue a PR whose status checks (reported via the Commit Status API) have not actually passed.
  Remediation: Change the map-building expression to ($checks | map({((.name // .context // "unknown")): (.conclusion // .state // .status // "PENDING")}) | add // {}) as $map |. Note: if applying this fix, also add the case-insensitive flag to the test() calls (e.g., test("FAILURE|..."; "i")) since StatusContext .state values may be lowercase.

Low

• [edge-case] skills/merge-queue/scripts/queue-status.sh:57 — The ETA display unconditionally appends a literal s suffix via the format string ETA: \(.estimatedTimeToMerge // "unknown")s. When estimatedTimeToMerge is null, this produces ETA: unknowns instead of ETA: unknown.
  Remediation: Use a conditional: ETA: \(if .estimatedTimeToMerge then "\(.estimatedTimeToMerge)s" else "unknown" end)

• [edge-case] skills/merge-queue/scripts/await-and-enqueue.sh:29 — The script only queries the rulesets API (repos/{nwo}/rules/branches/{branch}) to discover required checks. Repositories using classic branch protection rules will return no rulesets, causing the script to treat all checks as non-required and potentially enqueue prematurely.
  Remediation: Fall back to the classic branch protection API when rulesets returns no required checks, or document that only ruleset-based protection is supported.

• [architectural-divergence] skills/merge-queue/scripts/ — This is the first skill to include a scripts/ subdirectory with executable shell scripts. All 10 existing skills are markdown-only (SKILL.md with optional sub-agents/ markdown). While nothing in AGENTS.md prohibits this pattern, it is novel and worth noting for future reference.

• [code-organization] skills/merge-queue/SKILL.md:8 — The allowed-tools frontmatter field is not used by any other skill in the repo. This introduces a new convention without precedent.

• [pattern-inconsistency] skills/merge-queue/SKILL.md — The skill lacks a ## Constraints section present in most existing skills.

---

Corrections from prior review: The prior review's high-severity [protected-path] and medium-severity [missing-authorization] findings have been removed. No protected-path policy exists in this repo (CODEOWNERS has only * @fullsend-ai/core). The [missing-authorization] finding (no linked issue) is a process preference, not a code defect — AGENTS.md §3 governs agent behavior, not human contributor PRs.

Previous run (2)

Review

Findings

High

• [protected-path] skills/merge-queue/ — All 5 files in this PR are under the skills/ protected path. The PR has no linked issue providing authorization for modifying governance/infrastructure files. Human approval is always required for protected-path changes, regardless of context.
  Remediation: Link the PR to an issue that authorizes the merge-queue skill addition and explains why these protected files need to be changed.

Medium

• [missing-authorization] — Non-trivial change (5 new files adding a merge-queue skill) with no linked issue. AGENTS.md section 3 requires that every changed line trace directly to issue scope. New skills and multi-file additions are non-trivial and require explicit authorization.
  Remediation: Create an issue proposing the merge-queue skill addition and link it to this PR.

• [architectural-divergence] skills/merge-queue/scripts/ — This skill introduces a scripts/ directory with 4 bash scripts, establishing a new architectural pattern. None of the existing skills in this repository use shell scripts or have scripts/ directories — all existing skills are SKILL.md files with optional sub-agents/ directories containing markdown files.
  Remediation: Either refactor to follow the existing markdown-only skill pattern, or provide architectural documentation (ADR) justifying the introduction of script-based skills.

• [logic-error] skills/merge-queue/scripts/await-and-enqueue.sh:47 — The jq expression uses .name to build the check status map, but gh pr view --json statusCheckRollup returns both CheckRun objects (with .name) and StatusContext objects (with .context). StatusContext objects from the Commit Status API will have .name as null, causing all StatusContexts to collide on a single null key and fail to match required check names. The expression also reads .conclusion // .status but StatusContext objects use .state. This makes status checks reported via the Commit Status API invisible to the polling logic, potentially causing infinite polling or enqueuing a PR with failing checks.
  Remediation: Use (.name // .context) instead of .name for the map key, and (.conclusion // .state // .status // "PENDING") for the value.

Low

• [skill-resolution-coherence] skills/merge-queue/ — The PR description states this skill is "from fullsend-ai/fullsend," suggesting it may exist at the platform tier. If it already exists upstream, adding it at repo-level creates duplication. Clarify whether this is an override, prototype, or repo-specific skill.

• [code-organization] skills/merge-queue/SKILL.md:8 — The allowed-tools frontmatter field is not used by any other skill in this repo, making this a novel pattern without precedent in the existing codebase.

• [edge-case] skills/merge-queue/scripts/queue-status.sh:51 — The ETA display appends a literal s suffix unconditionally, producing ETA: unknowns when estimatedTimeToMerge is null.

• [edge-case] skills/merge-queue/scripts/await-and-enqueue.sh:29 — The script only queries the newer branch rulesets API for required checks, not the classic branch protection API. Repos using classic protection will skip required check verification.

• [pattern-inconsistency] skills/merge-queue/SKILL.md — The skill lacks a "Constraints" section, inconsistent with other skills that invoke external commands. The existing "Common errors" section partially addresses this.

---

Labels: PR adds a new merge-queue skill feature.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:45 PM UTC · Completed 9:55 PM UTC
Commit: 459235d · View workflow run →

Superseded by updated review

[ralphbean]

/fs-fix

[fullsend-ai-coder]

🤖 Finished Fix · ✅ Success · Started 11:54 PM UTC · Completed 12:11 AM UTC
Commit: e8381e3 · View workflow run →

[fullsend-ai-coder]

🔧 Fix agent — iteration 1 (human-triggered)

Addressed 4 of 6 review findings. Fixed the medium-severity jq logic error in await-and-enqueue.sh that caused StatusContext objects to collide on a null key (fail-open condition). Fixed the ETA display bug in queue-status.sh. Documented the rulesets-only limitation and added a Constraints section to SKILL.md. Disagreed with 2 informational observations (architectural-divergence, code-organization) that describe novel patterns but not code defects.

Fixed (4):

1. jq logic error: .name key misses StatusContext objects (skills/merge-queue/scripts/await-and-enqueue.sh): Changed jq map key from .name to (.name // .context // "unknown") and value from (.conclusion // .status) to (.conclusion // .state // .status) to handle both CheckRun and StatusContext objects. Added case-insensitive flag to test() calls since StatusContext .state values may be lowercase.
2. ETA display produces 'unknowns' when estimatedTimeToMerge is null (skills/merge-queue/scripts/queue-status.sh): Replaced unconditional 's' suffix with jq conditional: if .estimatedTimeToMerge then append 's', else show 'unknown' without suffix.
3. rulesets-only limitation undocumented (skills/merge-queue/scripts/await-and-enqueue.sh): Added inline comment documenting that only the rulesets API is queried and classic branch protection rules are not supported.
4. missing Constraints section in SKILL.md (skills/merge-queue/SKILL.md): Added a Constraints section documenting the rulesets-only limitation, consistent with other skills in the repo.

Disagreed (2):

1. architectural-divergence: first skill with scripts/ subdirectory: This is an informational observation about a novel pattern, not a code defect. The review itself notes 'nothing in AGENTS.md prohibits this pattern'. No code change is warranted.
2. code-organization: allowed-tools frontmatter field is novel: This is an informational observation. The allowed-tools field is part of the skill's design and is referenced by the runtime. Being novel does not make it incorrect.

Tests: passed

_{Updated by fullsend fix agent}

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:12 AM UTC · Completed 12:25 AM UTC
Commit: 90e2a01 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 12:48 PM UTC · Completed 12:59 PM UTC
Commit: 391f6c2 · View workflow run →

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 12:53 PM UTC · Completed 1:01 PM UTC
Commit: 391f6c2 · View workflow run →

[fullsend-ai-review]

Review skipped — this PR is already merged.

The /fs-review command only reviews open pull requests.

_{Posted by fullsend post-review check}

[fullsend-ai-retro]

PR #94 added a merge-queue skill with 4 shell scripts for GitHub merge queue management. The workflow involved 4 commits across 3 iteration cycles, 4 review agent passes, and 30 inline review comments. The review agents caught several genuine bugs (fail-open fallback, CheckRun/StatusContext collision, ETA formatting bug, STALE check indefinite polling, null enqueue position). The fix agent successfully resolved 4/6 findings in one pass. Two key inefficiencies: (1) the review agent re-raised identical low-severity findings across 3 re-reviews forcing 12+ dismissal comments from the author, and (2) a valid skill spec field was flagged as 'novel pattern' 3 times. These patterns also appeared on PR #89. Existing upstream issues fullsend-ai/fullsend#1013 and #1672 track the deduplication and dismissal-honoring gaps; this PR provides additional evidence for both. One repo-level proposal targets documenting valid skill frontmatter fields to prevent false-positive findings.

Proposals filed

• Evidence for fullsend-ai/fullsend#1013: same 3 findings repeated across 3 review iterations on PR #94 (in fullsend-ai/fullsend)
• Evidence for fullsend-ai/fullsend#1672: review agent re-raised findings after explicit author dismissal on PR #94 (in fullsend-ai/fullsend)
• Document valid skill frontmatter fields in AGENTS.md to prevent false-positive review findings (in fullsend-ai/agents)