-
Notifications
You must be signed in to change notification settings - Fork 0
Closed
Description
Problem
The pr-codex-bot skill defines Step 2 (local pre-PR review via csa review --branch main) as happening BEFORE Step 3 (push + create PR). However, there is no enforcement mechanism to prevent the orchestrator from pushing the branch before the review completes.
In practice, the orchestrator may push the feature branch first (e.g., to get it on the remote as a working backup), then invoke pr-codex-bot, which starts the review. While the review can still catch issues before PR creation, the ordering violation means:
- Unreviewed code is already on the remote — reviewers or CI could act on it prematurely
- The "PRECONDITION: REVIEW_COMPLETED=true" on Step 3 is a documentation constraint but not a runtime gate
Observed Behavior
1. Agent commits on feat/optional-opencc
2. Agent runs `git push -u origin feat/optional-opencc` ← BEFORE review
3. Agent invokes /pr-codex-bot
4. pr-codex-bot Step 2: csa review --branch main ← happens AFTER push
5. Step 3: create PR
Expected Behavior
1. Agent commits on feat/optional-opencc
2. Agent invokes /pr-codex-bot
3. pr-codex-bot Step 2: csa review --branch main ← BEFORE push
4. Step 3: git push + create PR ← AFTER review passes
Suggested Fix
Options (not mutually exclusive):
- Runtime gate in the skill pattern: Step 3 should verify
REVIEW_COMPLETED=truewas set by Step 2 within the current pr-codex-bot invocation, not just check if any prior review exists - PATTERN.md warning: Add explicit "FORBIDDEN: pushing before Step 2 completes" to the pattern
- Orchestrator guardrail: The skill could check
git log origin/main..origin/${WORKFLOW_BRANCH}to detect if the branch was already pushed and warn/abort if review hasn't run yet
Context
- Skill:
pr-codex-bot(SKILL.md + patterns/pr-codex-bot/PATTERN.md) - The push-before-review happened because the orchestrator treated push as a prerequisite for the skill invocation rather than as part of the skill's managed workflow
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels