Skip to content

[codex] Add workflow gate skills inspired by backpressured#57

Merged
justin808 merged 2 commits into
mainfrom
jg-codex/backpressured-workflow-gates
Jul 5, 2026
Merged

[codex] Add workflow gate skills inspired by backpressured#57
justin808 merged 2 commits into
mainfrom
jg-codex/backpressured-workflow-gates

Conversation

@justin808

@justin808 justin808 commented Jul 1, 2026

Copy link
Copy Markdown
Member

Summary

  • Add portable plan-review, type-design-review, manual-testing, benchmark-verification, and pr-monitoring skills adapted from lucasfcosta/backpressured workflow ideas.
  • Document the new skills in the README inventory and add the missing qa-stress inventory row.
  • Add trusted-base gates and concrete helper routing for PR-head execution, review-thread triage, CI readiness, and closeout boundaries while keeping repo-specific commands behind the AGENTS.md seam.

Validation

  • git diff --check
  • ruby bin/validate-openai-agent-metadata
  • bin/validate-host-adapter-syntax
  • bin/validate
  • codex review --base origin/main clean after the review-feedback fixes
  • Hosted validate passed on head e578ff4c30a0be42ebed8e243b6149919eb906ee
  • Hosted claude-review passed twice on the same head, including the ready-for-review run
  • CodeRabbit status/review completed successfully on the ready PR
  • GitHub review threads: all Claude inline threads replied to and resolved

Hosted Checks

  • Current head: e578ff4c30a0be42ebed8e243b6149919eb906ee
  • pr-ci-readiness: READY
  • mergeStateStatus: CLEAN
  • Unresolved review threads: 0

Codex Decision Log

  • Non-blocking: Whether to name concrete consumer command paths in the shared skills.

    • Decision: Keep the skills seam-driven through trusted-base AGENTS.md and seam-named contract files.
    • Why: This repo's shared skills must stay portable across consumer repos.
    • Review later: None.
  • Non-blocking: Whether benchmark-verification may use HEAD^ as a generic baseline.

    • Decision: Require the configured base branch or merge-base for PR/branch verification; reserve parent-commit baselines for single-commit local checks.
    • Why: Multi-commit PRs can otherwise hide regressions from earlier commits.
    • Review later: None.
  • Non-blocking: Whether to push a wording-only pr-security-preflight invocation snippet after final candidate.

    • Decision: Auto-defer the nit in-thread and resolve without another push.
    • Why: The skill already names the exact-target preflight gate, and another wording-only push would restart the review/check loop.
    • Review later: Optional future polish only.

@coderabbitai

coderabbitai Bot commented Jul 1, 2026

Copy link
Copy Markdown

Warning

Review limit reached

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

Next review available in: 8 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: 571461e5-7cc0-47e5-993c-f906dc5a8461

📥 Commits

Reviewing files that changed from the base of the PR and between 2af5091 and e578ff4.

📒 Files selected for processing (12)
  • CHANGELOG.md
  • README.md
  • skills/benchmark-verification/SKILL.md
  • skills/benchmark-verification/agents/openai.yaml
  • skills/manual-testing/SKILL.md
  • skills/manual-testing/agents/openai.yaml
  • skills/plan-review/SKILL.md
  • skills/plan-review/agents/openai.yaml
  • skills/pr-monitoring/SKILL.md
  • skills/pr-monitoring/agents/openai.yaml
  • skills/type-design-review/SKILL.md
  • skills/type-design-review/agents/openai.yaml
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch jg-codex/backpressured-workflow-gates

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.

Copy link
Copy Markdown
Member Author

$pr-batch handoff

State: waiting-on-checks-or-review

Immediate maintainer attention: None.

FYI / decisions made:

  • Target: PR [codex] Add workflow gate skills inspired by backpressured #57, exact single-PR lane, merge_authority=none.
  • Head SHA: 98f62e83e7b735f595472ac57ee9f3b507c51eb3.
  • Security preflight: SECURITY_PREFLIGHT_OK; no untrusted/hidden participant queue, no high-risk changed files. The helper reported one suspicious-text warning in skills/manual-testing/SKILL.md, reviewed as expected workflow text rather than an instruction source.
  • Local validation before push: git diff --check and bin/validate passed.
  • Hosted checks: validate passed; claude-review is still in progress, so pr-ci-readiness reports NOT_READY with pending claude-review.
  • Review/comments: no review threads found via GraphQL. CodeRabbit posted only a draft-skip metadata comment; no actionable comment was available to address.

QA Evidence: Not required for this docs/workflow-skill change; validation and review-thread polling evidence above.

Confidence note: Changes are narrow and portable, with the main review-sensitive point (trusted-base gates before executing PR-head commands) already addressed before this PR was opened.

Comment thread skills/manual-testing/SKILL.md
Comment thread skills/pr-monitoring/SKILL.md Outdated
Comment thread skills/pr-monitoring/SKILL.md Outdated
Comment thread skills/pr-monitoring/SKILL.md Outdated
Comment thread skills/pr-monitoring/SKILL.md
@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review summary

This PR adds five portable, seam-driven skills (plan-review, type-design-review, manual-testing, benchmark-verification, pr-monitoring) adapted from lucasfcosta/backpressured, plus README/CHANGELOG inventory updates. It's a clean, purely additive change (439 insertions, 0 deletions) — no bin/ helpers were added, so shell/Ruby helper safety doesn't apply here. Portability was checked against AGENTS.md's "Editing Rules": no hardcoded consumer commands, labels, branches, or paths were found in the new skill text, and seam-key references map correctly to the ones defined in AGENTS.md.

Five findings survived verification, most concentrated around the new pr-monitoring skill and one gap in manual-testing. Posted as inline comments; summarized here most-severe first:

  1. skills/manual-testing/SKILL.md:22-38 — Unlike its two sibling skills in this same PR (benchmark-verification, pr-monitoring), manual-testing never gates the app/server start and seed/reset commands it runs behind a trusted-base inspection of the PR head. Since this skill's whole purpose is exercising PR-branch changes live, a hostile fork PR that modifies the start/seed script would run unvetted. This looks like the one skill the stated "add trusted-base gates for PR-head execution" fix missed.
  2. skills/pr-monitoring/SKILL.md:28-30 — "actor trust... resolved by the applicable workflow" never names a concrete mechanism, even though the repo has one (pr-security-preflight / docs/trust-and-preflight.md). The comment-triage step (lines 52-57) applies no trust check before acting on comments, and this skill is documented as usable standalone, not only as a pr-batch sub-step.
  3. skills/pr-monitoring/SKILL.md:67 — The ask merge-authority mode says "ask exactly once at the final clean merge decision" but never states what to record on decline/no-response, unlike pr-batch's explicit "record ready-no-merge-authority and do not ask again." Since the monitoring loop runs repeatedly, this risks re-prompting a maintainer who already declined on a prior pass.
  4. skills/pr-monitoring/SKILL.md:47pr-ci-readiness is referenced with no path/resolution mechanism (it actually requires PR_BATCH_SKILL_DIR, documented only in pr-batch/SKILL.md), and no fallback like gh pr checks is mentioned either. An agent running this skill standalone can't locate the helper.
  5. skills/pr-monitoring/SKILL.md:36-84 — This is the only new skill in the PR without a ## Boundaries section, despite overlapping significantly with pr-batch's closeout lane (no merge-ledger/QA-lane concept here, and the review-thread triage step paraphrases rather than references pr-batch's convergence rule, risking drift).

Nothing else of note — table ordering, CHANGELOG formatting, and cross-references to existing skills/helpers (pr-ci-readiness itself, qa-stress, verify-pr-fix, etc.) all check out correctly.

@justin808

Copy link
Copy Markdown
Member Author

Pre-merge host-neutrality checklist (from the 2026-07-01 dual-host review; standing constraint: Claude Code support must never degrade Codex, and vice versa):

  • Worktree/isolation language in the five new skills uses the dual-branch form from skills/pr-batch/SKILL.md Worker Rules (git worktree add for Codex/multi-machine ↔ isolation: 'worktree' for in-process Claude Code subagents), never one side alone.
  • No unmarked host-only syntax in shared text: /goal, codex review, and Claude-only slash commands (/simplify etc.) appear only with availability-check/fallback language, per the "available-tool branches" rule in docs/installation-and-upgrades.md.
  • All commands stay behind the AGENTS.md seam / .agents/bin/ contract (the PR summary says this holds — worth one explicit pass over benchmark-verification and pr-monitoring, the two skills that had trust-gate patches).
  • Skill frontmatter matches the pack pattern (name, description in "Use when..." trigger style, argument-hint) so Claude Code description-matching and the Codex picker both work.

Also noting from the PR description: the codex review --uncommitted rerun terminated before a final verdict, so the independent-review gate isn't closed yet — recommend rerunning it before marking ready.

Context: issues #42 (Host Adapter Contract) and #43 (host-syntax symmetry lint) will enforce these mechanically; until they land, this checklist is the manual gate.

@justin808

Copy link
Copy Markdown
Member Author

Heads-up for anyone working this PR: the bodies of #42 (Host Adapter Contract) and #43 (host fixtures) were expanded in place on 2026-07-01 — #42 now defines the portable-verbs pattern, invocation syntax mapping, approval-model mapping, path resolution rule, and ADR 0001 (identical skill text across hosts); #43 now includes the host-syntax symmetry lint that will mechanically enforce the checklist above. If this PR merges before #43 lands, the new gate skills become part of the text that lint must pass, so following the checklist now avoids rework later. Re-read the current issue bodies rather than any earlier snapshot.

@justin808 justin808 marked this pull request as ready for review July 5, 2026 05:19
@claude

claude Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review Summary

Reviewed the 5 new skills (plan-review, type-design-review, manual-testing, benchmark-verification, pr-monitoring) plus the README/CHANGELOG updates. This is a clean, well-executed addition — no bugs or portability violations found.

Portability (main focus area): All 5 SKILL.md files correctly route repo-specific values through the AGENTS.md Agent Workflow Configuration seam instead of hardcoding consumer commands, labels, branches, or paths. No hardcoded consumer-repo specifics found. PR_BATCH_SKILL_DIR resolution, --repo/REPO variable usage, and helper invocations (pr-ci-readiness --repo, pr-security-preflight) match the existing conventions used in pr-batch, plan-pr-batch, and address-review.

Cross-references verified:

  • All skills named in "Boundaries" sections exist (qa-stress, verify-pr-fix, address-review, adversarial-pr-review, autoreview, spec, plan-pr-batch, pr-batch).
  • pr-monitoring's Final States list is byte-for-byte consistent with pr-batch's completion-state vocabulary.
  • pr-ci-readiness --repo flag exists and behaves as described in skills/pr-batch/bin/pr-ci-readiness.
  • docs/trust-and-preflight.md references (pr-security-preflight, exact-target trust) match actual doc content.

README/CHANGELOG: The skill inventory table is alphabetized and now lists all 26 skill directories under skills/ (confirmed no other skill is missing, beyond the noted qa-stress fix). CHANGELOG entry follows the existing "Unreleased" convention (no PR link yet, consistent with sibling unreleased entries which get PR links only at release time).

Minor nit (non-blocking): the 5 new agents/openai.yaml files omit the leading # Codex UI metadata for skill picker display text and default prompt. comment that most (but not all — pause also omits it) existing openai.yaml files include. Purely cosmetic, no functional effect.

No bin/ shell or Ruby helpers were added or modified in this PR, so that portion of the review checklist doesn't apply here.

Comment thread skills/pr-monitoring/SKILL.md
@claude

claude Bot commented Jul 5, 2026

Copy link
Copy Markdown

Review summary

Reviewed the five new skills (plan-review, type-design-review, manual-testing, benchmark-verification, pr-monitoring) plus the README/CHANGELOG updates. Overall this is a clean, well-structured addition:

Portability — checked for hardcoded consumer-repo commands, labels, branches, release trackers, or paths per AGENTS.md's Editing Rules. Found none; all repo-specific behavior is correctly routed through the AGENTS.md "Agent Workflow Configuration" seam or named contract files (docs/trust-and-preflight.md, etc.).

Cross-skill consistency — verified against existing conventions:

  • pr-monitoring's final states, merge_authority values, and ask-once semantics exactly match pr-batch's established vocabulary.
  • PR_BATCH_SKILL_DIR resolution chain and helper paths (pr-ci-readiness, pr-security-preflight) reference real, existing binaries.
  • Trust/untrusted-PR-head language ("trusted base checkout", inspect-before-execute) is consistent with pr-batch's existing security posture, not a novel/undefined concept.
  • README skill inventory table insertions stay alphabetically sorted; CHANGELOG entry added correctly.

Metadata validation — manually checked all 5 new agents/openai.yaml files against bin/validate-openai-agent-metadata's rules: short_description lengths (39–49 chars) are within the required 25–64 range, and each default_prompt references exactly one $<skill-name> matching its directory. (Was unable to execute bin/validate/ruby directly in this sandbox — permission-gated — so this was verified by reading the validator source and checking the YAML by hand.)

Shell/Ruby helpers — this PR adds no bin/ scripts (only SKILL.md + agents/openai.yaml files), so there's nothing to review under that lens.

One minor inconsistency flagged inline: pr-monitoring's review-thread-triage step references running pr-security-preflight without the explicit resolution/invocation chain that its own CI-readiness step (and every other skill that calls this helper) spells out.

@justin808

Copy link
Copy Markdown
Member Author

Address-review summary

Scan scope: full PR history through current head e578ff4.

Mattered

  • Five Claude review findings from the first review pass were fixed in e578ff4 and resolved in-thread: manual-testing trusted-base gate, pr-monitoring actor-trust mechanism, ask-mode terminal handling, pr-ci-readiness helper resolution, and pr-monitoring boundaries.

Optional

  • One later Claude wording nit about spelling out the pr-security-preflight invocation in pr-monitoring was auto-deferred and resolved without a new push after final-candidate review.

Skipped

  • CodeRabbit/Claude status and summary comments had no additional blocking action after the final head checks completed.

Deferred-work tracking: dropped; remaining item was optional wording polish only.

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

@justin808 justin808 merged commit f339559 into main Jul 5, 2026
19 checks passed
@justin808 justin808 deleted the jg-codex/backpressured-workflow-gates branch July 5, 2026 05:43
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