Skip to content

[codex] Route merge-readiness asks out of plan-pr-batch - #47

Merged
justin808 merged 15 commits into
mainfrom
jg-codex/plan-pr-batch-merge-readiness-route
Jul 4, 2026
Merged

[codex] Route merge-readiness asks out of plan-pr-batch#47
justin808 merged 15 commits into
mainfrom
jg-codex/plan-pr-batch-merge-readiness-route

Conversation

@justin808

@justin808 justin808 commented Jun 30, 2026

Copy link
Copy Markdown
Member

Summary

  • make $plan-pr-batch decline PR readiness, manual testing, and merge sequencing requests as implementation-batch planning
  • route those requests through the target repo’s AGENTS.md Review gate seam and any repo-local readiness workflow instead

Why

$plan-pr-batch is for implementation batch planning. Readiness and merge sequencing questions need the target repo’s readiness/review policy, not a generated worker batch. The wording stays portable by naming the existing AGENTS.md seam instead of a consumer-specific skill.

Validation

  • bin/validate

Summary by CodeRabbit

  • Documentation
    • Improved guidance for assessing whether open pull requests are ready to merge.
    • Added clearer instructions for checking remaining manual testing and merge order.
    • Clarified fallback behavior when readiness guidance can’t be found, reducing guesswork.

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@justin808, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0eae1c1d-e6ad-4933-a522-87831d78fe7c

📥 Commits

Reviewing files that changed from the base of the PR and between 7ca66f3 and 4a710b8.

📒 Files selected for processing (1)
  • skills/plan-pr-batch/SKILL.md

Walkthrough

Added a conditional instruction block to skills/plan-pr-batch/SKILL.md directing the planner to resolve repo-specific PR readiness policy via AGENTS.md and .agents/agent-workflow.yml's review_gate setting, prefer a repo-local pr-processing.md, and report UNKNOWN when unresolved rather than guessing.

Changes

Plan-pr-batch readiness guidance

Layer / File(s) Summary
Readiness policy resolution instructions
skills/plan-pr-batch/SKILL.md
New instruction block for handling PR merge-readiness questions: resolve review_gate via AGENTS.md/.agents/agent-workflow.yml, prefer repo-local pr-processing.md, fall back to shared workflow, and report UNKNOWN when unresolved instead of guessing.

Estimated code review effort: 1 (Trivial) | ~3 minutes

Possibly related PRs

  • shakacode/agent-workflows#52: Also updates skills/plan-pr-batch/SKILL.md, expanding UNKNOWN/readiness handling tied to pr-processing.md semantics.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: routing merge-readiness requests out of plan-pr-batch.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-codex/plan-pr-batch-merge-readiness-route

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jun 30, 2026

Copy link
Copy Markdown

Review Summary

This is a tiny, focused change (4 lines added to skills/plan-pr-batch/SKILL.md) that adds a routing rule sending merge-readiness/manual-testing/sequencing questions to $merge-readiness-audit instead of plan-pr-batch.

Blocking issue (inline comment posted): $merge-readiness-audit does not exist anywhere in this repository — not as a skills/ folder, not as a workflows/*.md file, and not anywhere in git history. The PR routes a class of user requests to a skill that can't be resolved. Given this repo's purpose as a portable "source pack" (per AGENTS.md), if this skill is consumer-repo-local, hardcoding the reference here also breaks portability for any other consumer of this shared skill pack.

No code-quality, security, or performance concerns otherwise — the diff is pure documentation/skill-routing text with no shell/Ruby helper changes in this PR.

Suggestion: either add the merge-readiness-audit skill to this repo so the reference resolves, or route through an AGENTS.md seam key (consistent with how this same file already resolves coordination_backend) instead of a hardcoded skill name.

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review

Reviewed the diff (6 lines added to skills/plan-pr-batch/SKILL.md, no other files touched).

Summary: Clean, well-scoped change. No bugs, security issues, or performance concerns — this is a documentation-only addition to a skill file.

Portability check (AGENTS.md Editing Rules): Passes. The new paragraph routes merge-readiness/manual-testing/sequencing questions to the target repo's AGENTS.mdAgent Workflow ConfigurationReview gate seam, which mirrors this repo's own seam-key naming convention (see AGENTS.md's own Review gate bullet). No consumer-repo commands, labels, branches, or paths are hardcoded. The -> seam-reference notation is consistent with usage elsewhere in the same file (e.g. the File-touch map template).

Consistency check: Verified there's no existing guidance elsewhere in plan-pr-batch/SKILL.md that already covers this case, and no contradiction with the batch-mechanics "sequencing" references later in the file (those are about ordering dependent work items within a batch, not about answering "is this PR ready to merge" questions) — so this is a genuinely new, non-duplicated routing rule.

Minor/non-blocking observations:

  • No CHANGELOG.md entry, but per this repo's update-changelog skill, changelog entries are swept in separately post-merge rather than required per-PR, so this isn't a gap.
  • Placement (inserted right after the $spec routing paragraph, before the "skill picker" fallback paragraph) reads naturally and doesn't disrupt the doc flow.

No blocking findings. LGTM.

@justin808
justin808 marked this pull request as ready for review July 3, 2026 10:06

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 82778f360a

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review

Reviewed the diff (6-line addition to skills/plan-pr-batch/SKILL.md) against the requested focus areas.

Summary: Clean, well-scoped change. No issues found.

  • Portability: The new paragraph routes readiness/manual-testing/merge-sequencing questions to AGENTS.md -> Agent Workflow Configuration -> Review gate, which is an existing, documented seam key (see AGENTS.md:24 and workflows/pr-processing.md:1655-1657). It follows the repo's own Editing Rules ("name the corresponding AGENTS.md seam key instead of embedding an example command") and doesn't hardcode any consumer-repo-specific commands, labels, branches, or paths.
  • Correctness/scope: The decline condition (PR-merge readiness, manual testing remaining, merge sequencing) is narrow and doesn't conflict with the skill's existing scope-planning purpose; it correctly hedges with "any repo-local readiness workflow" for cases the Review-gate seam alone doesn't cover.
  • Shell/Ruby/security/perf: N/A — this change touches only skill documentation prose, no scripts or executable code.
  • Minor style note (non-blocking): The new text uses ASCII -> for the AGENTS.md -> ... -> ... reference chain, while a few other skill docs (e.g. skills/pr-batch/SKILL.md:353, skills/post-merge-audit/SKILL.md:222) use the Unicode for similar "per AGENTS.md → Section" references. Not a functional issue since -> is already used elsewhere in this same file (the File-touch map template), just flagging for awareness if the project wants one consistent style.

No inline comments needed — nothing rises to an actionable finding.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e8b573a929

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread skills/plan-pr-batch/SKILL.md Outdated
Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review summary

Small, well-scoped change: adds a guard to plan-pr-batch/SKILL.md so merge-readiness/manual-testing/merge-sequencing questions get declined as batch-planning work and routed to the target repo's seam instead.

Portability check: clean. It only names the already-established generic seam surfaces (AGENTS.mdAgent Workflow Configuration pointer, .agents/agent-workflow.yml, review_gate policy key — all defined in docs/seam-design.md and used by other skills), no consumer-repo-specific commands, labels, branches, or paths are hardcoded.

Correctness: the mapping makes sense — review_gate in seam-presets.yml/docs/downstream-sync.md is literally defined as the "what counts as merge-ready" policy, so pointing readiness questions at it is accurate.

One suggestion (posted inline): the new paragraph's fallback for "any repo-local readiness workflow" is left undefined and has no UNKNOWN-style fallback, unlike every other seam reference in this file. Worth tightening so an agent in a repo that hasn't adopted the seam doesn't stall or invent a workflow — and worth explicitly tying back to $pr-batch, whose own SKILL.md already claims "merge-readiness item[s]" as in scope.

No security, performance, or shell/Ruby-safety concerns — this PR touches only Markdown skill instructions.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9564dafc7c

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review

Reviewed the diff (single-file change to skills/plan-pr-batch/SKILL.md, +10/-0).

Summary: Small, well-scoped addition that routes merge-readiness/manual-testing/merge-sequencing questions away from plan-pr-batch toward the repo's AGENTS.md seam and pr-processing.md readiness workflow, with an explicit UNKNOWN fallback when the policy key or workflow can't be resolved.

Checks performed:

  • Portability: the new text only references existing generic seam concepts (AGENTS.md Agent Workflow Configuration pointer, .agents/agent-workflow.yml, the review_gate policy key, pr-processing.md) — all of which are already used elsewhere in this repo (docs/adoption.md, docs/seam-design.md, bin/agent-workflow-seam-doctor-test.rb, etc.). No consumer-specific commands, labels, branches, or paths are hardcoded.
  • Consistency: the UNKNOWN-on-unresolvable-state pattern and the "installed/shared vs. repo-local" fallback phrasing match the conventions used throughout the rest of this file and workflows/pr-processing.md.
  • No shell/Ruby helpers touched, so no new attack surface in bin/.
  • Confirmed the addition sits outside the ## Goal Prompt for pr-batch fenced template that scripts/check_goal_prompt_size.rb pins, so the self-check script's phrase/size assertions are unaffected.
  • No conflicting or duplicate guidance elsewhere in the file for merge-readiness/manual-testing requests.

Minor, non-blocking nit: the new paragraph names pr-processing.md's "readiness workflow" generically, whereas the adjacent paragraph above it (lines 12-16) links to a specific anchor (../../workflows/pr-processing.md#generic-pr-batch-continuation-prompt). Consider linking to the "Merge Readiness Gate" section anchor in pr-processing.md for the same directness, though this isn't required since the existing wording is unambiguous.

No correctness, security, or portability issues found.

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review summary

This PR adds a short paragraph to skills/plan-pr-batch/SKILL.md that redirects PR-readiness / manual-testing / merge-sequencing questions to the target repo's AGENTS.md seam and pr-processing.md readiness workflow instead of producing an implementation batch plan. Scope is a single 10-line prose addition, no code/helpers touched.

Portability / conventions: Clean. .agents/agent-workflow.yml, the review_gate policy key, and pr-processing.md are all pre-existing, established seam concepts used elsewhere in this repo (docs/downstream-sync.md, docs/seam-design.md, other skills) — nothing consumer-specific is hardcoded here, and the AGENTS.md "Agent Workflow Configuration" pointer name matches the real section header. No CLAUDE.md/AGENTS.md editing-rule violations found. No impact on check_goal_prompt_size.rb (it only pins the Output-section goal-prompt template, which this diff doesn't touch).

Finding (posted inline on skills/plan-pr-batch/SKILL.md:16): the new paragraph says to "read the review_gate policy key" but never uses that value — the routing decision happens unconditionally either way, unlike this file's other policy-key references (merge_authority, coordination_backend, base_branch), which all have a concrete, traceable consumption point. Worth either wiring real branching logic to review_gate or dropping the dead reference so the instruction doesn't imply logic that isn't there.

No other correctness, security, or performance issues found in this diff.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ce25444d2b

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread skills/plan-pr-batch/SKILL.md Outdated
…h-merge-readiness-route

* origin/main:
  [codex] Make plan-pr-batch goals Codex-pasteable (#50)
  [codex] Add host adapter and Codex plugin metadata
  [codex] Add pause restart prompt skill
  [codex] Add batch titles and audit defaults
  [codex] Fix PR-batch Goal-mode completion contract (#52)
  Add strict trust mode to preflight (#48)
  [codex] Add generic PR-batch continuation prompt (#49)
Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Summary

Small, well-scoped change (+10/-0, single file: skills/plan-pr-batch/SKILL.md). It adds a routing rule so merge-readiness / manual-testing / merge-sequencing questions are declined by $plan-pr-batch and redirected elsewhere, rather than turned into an implementation batch plan.

No code, shell, or Ruby helper changes in this PR, so there's no bin/ attack surface, shell/Ruby safety, or runtime performance angle to review — this is pure skill-prose.

Finding (posted inline, on skills/plan-pr-batch/SKILL.md:18-26): the new text tells agents to resolve readiness policy from "the target repo's AGENTS.md Agent Workflow Configuration Review gate policy" — but this repo's own documented seam contract (docs/seam-design.md, docs/adoption.md, docs/downstream-sync.md, and the test/fixtures/consumer-repo fixture) specifies that a properly-adopted consumer AGENTS.md is only a pointer to .agents/agent-workflow.yml, where the real key is review_gate (snake_case). A fully-adopted consumer repo has no "Review gate" bullet in AGENTS.md at all, so an agent following this instruction literally would wrongly report the policy as unresolvable/UNKNOWN. This is also inconsistent with how the same file resolves base_branch and coordination_backend elsewhere (both via .agents/agent-workflow.yml). Details and a suggested fix are in the inline comment.

I also flagged a secondary, more minor ambiguity in the same block: the UNKNOWN-fallback sentence doesn't state precedence for the case where the Review gate policy is unresolved but pr-processing.md is resolved (both listed conditions can be true at once).

Otherwise the change is portable — it doesn't hardcode any consumer-repo-specific commands, labels, branches, or paths, and the "installed/shared … fallback" phrasing matches this file's existing conventions.

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review summary

Reviewed the scoped change (routing PR-merge-readiness/manual-testing/sequencing questions out of $plan-pr-batch into the target repo's AGENTS.md Review-gate seam + pr-processing.md).

Content/portability: The new paragraph (lines 18-26) is clean — it names the generic AGENTS.md Agent Workflow Configuration Review gate key (which matches this repo's own AGENTS.md and the seam name already used in workflows/pr-processing.md) rather than hardcoding a consumer-specific command, label, branch, or tracker. No portability violations found.

Host-adapter syntax: The <!-- host-allow: codex-only --> / <!-- host-branch: codex-only start/end --> markers added around the pre-existing `/goal` mentions are new but correctly placed per bin/validate-host-adapter-syntax's rules (every line containing /goal is either inside a host-branch block or has an inline host-allow marker on the same line). These look like a pre-existing gap in this file that bin/validate surfaced, not new content added by this PR's actual scope — makes sense given the PR's own validation note.

One formatting regression — see inline comment on skills/plan-pr-batch/SKILL.md lines 168-172: adding the host-allow markers appears to have collapsed two nested bullets from 3-space to 2-space indent, breaking consistency with every sibling bullet in the same list and leaving mixed indentation within the same list item. Under the parent numbered step ("4. Output"), 2-space indent no longer nests correctly under CommonMark rules. Recommend restoring the original 3-space indent for both edited bullets.

No security, bin/ or skills/*/bin/ helper-script changes in this diff (only SKILL.md prose is touched), so no additional shell/Ruby safety concerns.

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review: Route merge-readiness asks out of plan-pr-batch

Reviewed the diff to skills/plan-pr-batch/SKILL.md (the only changed file) against this repo's portability rules (AGENTS.md Editing Rules) and the host-adapter contract.

Portability — looks correct. The new readiness-routing paragraph only names existing, generic seam keys (AGENTS.mdAgent Workflow Configuration.agents/agent-workflow.ymlreview_gate) and the shared pr-processing.md workflow. Verified review_gate is an already-established seam key (docs/seam-design.md, docs/adoption.md, seam-presets.yml, test/fixtures/consumer-repo/.agents/agent-workflow.yml) and that the **Agent Workflow Configuration** pointer phrasing matches the convention used in skills/spec/SKILL.md. No consumer-specific commands, labels, branches, or paths are hardcoded.

Fallback logic — sound. The three-way fallback (review_gate resolved → normal path; review_gate unresolved but workflow resolved → fall back to $pr-batch CI/review checks, report policy as UNKNOWN; workflow unresolved → report workflow state UNKNOWN) avoids guessing and is consistent with the rest of the skill's "record UNKNOWN, don't guess" style.

Host-adapter markers (<!-- host-branch: codex-only start/end -->, <!-- host-allow: codex-only -->). These are needed so bin/validate-host-adapter-syntax doesn't flag the bare /goal mentions this file already contained (that validator requires every /goal token to sit inside a codex-only branch or carry an inline allow marker). I checked the marker placement against bin/host-adapter-syntax-test.rb and the existing usage in skills/pr-batch/SKILL.md (e.g. its "Mode: ... create /goal prompt ..." item) — wrapping a mostly-shared block just because it contains the one Codex-specific /goal line is the established pattern here, not something new to this PR, so I don't think it needs changing.

Ruby/shell in bin/: no helper scripts touched by this PR.

No correctness, security, or portability issues found. LGTM.

…h-merge-readiness-route

* origin/main:
  [codex] Harden preflight review trust checks (#45)
  [codex] Clarify completed-batch audit scope (#72)
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review summary

Reviewed the diff (single file, skills/plan-pr-batch/SKILL.md, +11/-0).

Portability — clean. The new paragraph routes readiness/manual-testing/merge-sequencing asks through the AGENTS.md Agent Workflow Configuration pointer, .agents/agent-workflow.yml's review_gate key, and the repo-local/shared pr-processing.md workflow. All of these are existing seam references used consistently elsewhere in the repo (pr-batch/SKILL.md, autoreview/SKILL.md, update-changelog/SKILL.md, etc.) — no consumer-specific commands, labels, branches, or paths are hardcoded.

Correctness — the fallback chain (review_gatepr-processing.mdUNKNOWN) is well-formed and matches the established "report UNKNOWN rather than guessing" idiom used throughout this skill and pr-processing.md. $pr-batch is correctly the skill tied to pr-processing.md (confirmed in skills/pr-batch/SKILL.md), so "continue with the workflow's $pr-batch CI/review readiness checks" points at the right document.

Scope — the new guardrail sits before the existing Workflow steps, doesn't touch the pinned Output/goal-prompt-template section (so check_goal_prompt_size.rb is unaffected), and doesn't duplicate similar redirect logic already present elsewhere in the file or in pr-batch/SKILL.md.

Shell/Ruby helpers — not applicable; no bin/ files changed in this PR.

No bugs, security issues, or portability violations found. This is a low-risk, well-scoped documentation change consistent with repo conventions.

@justin808

Copy link
Copy Markdown
Member Author

Address-review summary

Scan scope: current closeout review threads through 2026-07-04T10:59Z.

Mattered

  • Plan prompt indentation / Codex marker placement: addressed in 0c97308 by removing the literal Codex goal wording and marker from the non-Codex instruction.
  • Inline AGENTS.md review-gate fallback: addressed in 0c97308 by preserving the inline seam fallback when .agents/agent-workflow.yml is absent.

Skipped

  • None.

Next default scan starts after this comment. Say check all reviews to rescan the full PR.

Comment thread skills/plan-pr-batch/SKILL.md Outdated
Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Reviewed the diff (skills/plan-pr-batch/SKILL.md only, +14/-1). Summary:

Portability: The new paragraph correctly uses the existing seam conventions (AGENTS.md Agent Workflow Configuration, .agents/agent-workflow.yml, review_gate) rather than hardcoding consumer-repo specifics — consistent with AGENTS.md's Editing Rules and how these seams are already used elsewhere in this file (e.g. coordination_backend, merge_ledger). No hardcoded commands/labels/branches introduced.

Two things worth a look (posted as inline comments):

  1. The fallback text references "the workflow's $pr-batch CI/review readiness checks," but workflows/pr-processing.md has no section by that name — the closest is ## Merge Readiness Gate. The rest of this file names exact section headings when pointing into pr-processing.md (e.g. "under Batch QA Lane"), so this is a rigor regression relative to its own sibling references, and the phrasing risks being misread as "invoke $pr-batch" — the opposite of what this paragraph is trying to prevent.
  2. A separate, seemingly unrelated edit changes "omit the /goal line" to "omit the Codex goal invocation" (and drops the host-allow: codex-only marker). This is vaguer than the original and could be misread as also dropping the Use $pr-batch to complete this batch with subagents. line, which the template later says must stay. Worth confirming this was an intentional part of the PR rather than incidental drift from the many review-iteration commits.

Nothing else stood out — no security issues, no bugs in logic/control flow, and no bin/ helper changes to review (none touched by this PR). I was not able to execute bin/validate in this sandbox (command execution required approval that wasn't available), so I couldn't independently confirm the PR description's validation claim, but based on static inspection of check_goal_prompt_size.rb's required-phrase list and bin/validate-host-adapter-syntax's codex-only-token check, the change should pass both.

Comment thread skills/plan-pr-batch/SKILL.md Outdated
Comment thread skills/plan-pr-batch/SKILL.md Outdated
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review summary

Small, focused change: skills/plan-pr-batch/SKILL.md gains a paragraph that routes "is this PR ready to merge / what testing remains / how should I sequence merges" requests away from batch planning and into the target repo's review_gate policy (.agents/agent-workflow.yml or inline AGENTS.md) plus the pr-processing.md Merge Readiness Gate, falling back to UNKNOWN when unresolved. One other line clarifies that only the /goal line (not the whole prompt) is omitted for claude/generic targets.

Portability: Good — verified this doesn't invent new terminology. .agents/agent-workflow.yml, the review_gate key, and pr-processing.md's Merge Readiness Gate section are all pre-existing, documented conventions (docs/seam-design.md, docs/adoption.md, test/fixtures/consumer-repo/.agents/agent-workflow.yml, workflows/pr-processing.md:1634). No consumer-repo-specific commands, labels, or branches are hardcoded here. The UNKNOWN-on-unresolved fallback matches the file's existing convention (e.g. line 63).

Code/shell/Ruby safety: N/A — this diff only touches prose in a SKILL.md; no bin/ or helper scripts changed.

Bugs/logic: No functional bugs found. The fallback ladder (resolve review_gate -> fall back to pr-processing.md alone with policy UNKNOWN -> fall back to reporting workflow state UNKNOWN) is internally consistent and doesn't conflict with existing readiness-related text in skills/pr-batch/SKILL.md (checked, no duplication).

Two minor nits posted inline:

  1. Terminology inconsistency: "review-gate seam" (line 23) doesn't match this repo's established literal Review gate label used elsewhere when referring to the inline AGENTS.md value.
  2. The lines 18-23 sentence is dense enough (two parallel resolution paths in one sentence) that splitting it would improve clarity; not a correctness issue.

Possible gap: This is a behavior change to a shared skill (plan-pr-batch now declines/reroutes a class of requests it previously might have handled as batch planning). CHANGELOG.md's [Unreleased] section doesn't have an entry for it, whereas other skill-behavior changes in this repo are logged there (e.g. the "Default post-merge audits..." and "Clarify completed-batch..." entries). Worth confirming whether this qualifies under the repo's own changelog policy before merge.

No security or performance concerns identified.

@justin808

Copy link
Copy Markdown
Member Author

Address-review checkpoint for PR #47 after f81fa86.

  • Addressed the final two routing-language comments with f81fa86.
  • Validation evidence: git diff --check -- skills/plan-pr-batch/SKILL.md, AGENT_WORKFLOWS_SOURCE_CHECKOUT=1 ruby skills/plan-pr-batch/scripts/check_goal_prompt_size.rb, and bin/validate passed locally before push.
  • Deferred/skipped: none.

@justin808

Copy link
Copy Markdown
Member Author

Address-review checkpoint for PR #47 after 9e33806d8a866902ddcf40f7fefae5a4a37d39db.

  • Addressed the follow-up readiness-gate terminology and sentence-density comments with 9e33806d8a866902ddcf40f7fefae5a4a37d39db.
  • Validation evidence: git diff --check -- skills/plan-pr-batch/SKILL.md, AGENT_WORKFLOWS_SOURCE_CHECKOUT=1 ruby skills/plan-pr-batch/scripts/check_goal_prompt_size.rb, and bin/validate passed locally before push.
  • Deferred/skipped: none.

@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review: Route merge-readiness asks out of plan-pr-batch

Scope: single file, skills/plan-pr-batch/SKILL.md (+15/-1) — a new routing paragraph plus a one-word wording fix for consistency with the existing Codex-only /goal-line wording further down the file.

Portability — Verified against the repo's own conventions rather than taking the new text at face value:

  • AGENTS.mdAgent Workflow Configuration pointer and .agents/agent-workflow.yml are the established seam-resolution pattern (see bin/agent-workflow-seam-doctor's POINTER_SECTION/REQUIRED_POLICY_KEYS).
  • review_gate is a real, tooling-checked policy key (bin/agent-workflow-seam-doctor, bin/push-downstream, test/fixtures/consumer-repo/.agents/agent-workflow.yml, seam-presets.yml), and Review gate is the matching inline-AGENTS.md bullet label.
  • pr-processing.md's ## Merge Readiness Gate section exists and covers CI/review/changelog/testing evidence, so the reference is accurate, not invented.
  • No consumer-repo-specific commands, labels, branches, or paths are hardcoded; everything routes through named seam keys, consistent with AGENTS.md's Editing Rules.

Correctness — The new paragraph's routing logic is internally consistent (pointer-style vs. inline-style AGENTS.md, pr-processing.md fallback, UNKNOWN reporting when unresolved) and doesn't conflict with the existing continuation-handoff paragraph a few lines above it.

Minor, non-blocking observation: the paragraph covers three question types ("ready to merge", "manual testing remaining", "how to sequence merges") but the resolution path it defines centers on review_gate / the Merge Readiness Gate section, which is strong on CI/review/testing evidence but doesn't explicitly address multi-PR merge sequencing (that concept lives more in the "Shape" section of this same skill and in the "merge sequencing" mention around pr-processing.md:989). Not a bug — just something to keep in mind if sequencing questions end up under-served by this routing in practice.

No shell/Ruby helper changes in this PR (bin/ and skills/*/bin/ are untouched), so no shell-safety concerns apply here. I wasn't able to execute bin/validate in this sandboxed review environment, but manual inspection of skills/plan-pr-batch/scripts/check_goal_prompt_size.rb shows the wording tweak ("omit only the /goal line") doesn't collide with any of its pinned required phrases (it checks for "remove only the /goal line", a separate, unmodified line).

Overall: clean, well-grounded, low-risk documentation change. No bugs, security, or portability issues found.

…h-merge-readiness-route

* origin/main:
  Add CONTEXT.md glossary for agent batch coordination (#71)
  [codex] Default post-merge audits to follow-up issues (#62)

# Conflicts:
#	skills/plan-pr-batch/SKILL.md
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review

Reviewed the diff (single-file, +14/-0 in skills/plan-pr-batch/SKILL.md). No blocking issues found.

Portability: The new paragraph only references generic seam mechanisms already established across the repo — AGENTS.md Agent Workflow Configuration, .agents/agent-workflow.yml review_gate key (a documented required key per docs/downstream-sync.md), and the pr-processing.md Merge Readiness Gate section (verified this section exists at workflows/pr-processing.md:1639). No consumer-repo-specific commands, labels, branches, or paths are hardcoded, consistent with AGENTS.md's Editing Rules.

Logic: The fallback chain is sound and follows the file's existing UNKNOWN-over-guessing convention:

  • YAML seam (review_gate) → inline AGENTS.md Review gate → pr-processing.md Merge Readiness Gate with policy reported UNKNOWN → workflow itself unresolved reports state UNKNOWN.
  • Explicitly forbids using $pr-batch as a substitute for the readiness workflow, matching the PR's stated intent of not turning readiness/sequencing questions into implementation-batch plans.

Consistency: Bold-term style (**Agent Workflow Configuration**, **Merge Readiness Gate**) matches existing usage (e.g. **Batch QA Lane** a few lines below). No duplication with the readiness content already in pr-batch/SKILL.md — that skill's readiness logic is for actively-managed batch execution, whereas this addition routes pure readiness/sequencing questions away from batch planning entirely.

No shell/Ruby helper changes in this diff (docs-only), so no additional safety concerns there. bin/validate per the PR description was already run.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8ed0db0317

ℹ️ 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".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread skills/plan-pr-batch/SKILL.md Outdated
@justin808

Copy link
Copy Markdown
Member Author

Address-review checkpoint for PR #47 after 4a710b8.

  • Addressed the inline Agent Workflow Configuration fallback comment with 4a710b8.
  • Validation evidence: git diff --check -- skills/plan-pr-batch/SKILL.md, AGENT_WORKFLOWS_SOURCE_CHECKOUT=1 ruby skills/plan-pr-batch/scripts/check_goal_prompt_size.rb, and bin/validate passed locally before push.
  • Deferred/skipped: none.

Comment thread skills/plan-pr-batch/SKILL.md
@claude

claude Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review

Small, well-scoped doc-only change (skills/plan-pr-batch/SKILL.md, +16/-0). Checked it against the areas in scope:

Portability — Passes. The new paragraph only references identifiers already established elsewhere in this shared pack: the AGENTS.md Agent Workflow Configuration pointer, .agents/agent-workflow.yml, the review_gate/merge_ledger policy keys (documented in docs/seam-design.md's Policy Contract), pr-processing.md, and its existing Merge Readiness Gate section. No consumer-repo commands, labels, branches, release trackers, or paths are hardcoded.

Correctness — The routing logic holds together: readiness/manual-testing/merge-sequencing questions get redirected to the repo's pr-processing.md readiness workflow instead of being treated as a batch-planning trigger, with UNKNOWN fallbacks at both the per-policy-value and whole-workflow level, and an explicit "don't use $pr-batch as a substitute" guard. This doesn't conflict with pr-batch/SKILL.md's own readiness checks, which apply to items it's actively completing rather than to a standalone user readiness question — different contexts, no duplication/contradiction.

Shell/Ruby helpers — N/A, no bin/ scripts touched by this PR.

One minor, non-blocking nit left inline: the added block is fairly verbose relative to the rest of the file and AGENTS.md's "keep SKILL.md concise" rule — left a suggested tightened version, take or leave it.

No bugs, security, or performance concerns found.

@justin808

Copy link
Copy Markdown
Member Author

Address-review checkpoint for PR #47 after hosted review on 4a710b8bcbdea13670228c9ed5f3aa260c20b81f.

  • Closed the final non-blocking concision note with rationale; no code change needed.
  • Current-head hosted checks passed: validate, claude-review, CodeRabbit, with claude skipped.
  • Deferred/skipped: optional future tightening only.

@justin808
justin808 merged commit dea3ea0 into main Jul 4, 2026
9 checks passed
@justin808
justin808 deleted the jg-codex/plan-pr-batch-merge-readiness-route branch July 4, 2026 11:41
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