From af60f9fc6b7d93c82c78bc98e7efb204f1803f62 Mon Sep 17 00:00:00 2001 From: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Date: Mon, 4 May 2026 12:21:07 +0000 Subject: [PATCH] fix(ce-dispatch): address Codex review on EveryInc#762 Phase 4 status refresh missed merged PRs and the dispatch_base_branch default returned a full ref path instead of a branch name. Both bugs were flagged by chatgpt-codex-connector on the upstream PR. P1: gh pr list defaults to open PRs only, so dispatched PRs merged outside this orchestrator (GitHub UI, Conductor, another shell) were invisible to the status refresh, leaving the dependency graph stuck and 'Dispatch newly unblocked units' blocked even after prerequisites merged. Pass --state all so the query returns merged PRs alongside open ones. P2: git symbolic-ref refs/remotes/origin/HEAD prints the full ref path (refs/remotes/origin/main), not the bare branch name. That value got propagated into dispatch metadata where a plain branch name is expected, breaking PR-target instructions in dispatched workspaces. Pass --short. Adds two regression assertions in ce-dispatch-contract.test.ts so the suite catches missing --state all on every gh pr list invocation in Phase 4 and missing --short on every git symbolic-ref invocation in Phase 0. --- .../skills/ce-dispatch/SKILL.md | 4 +- tests/skills/ce-dispatch-contract.test.ts | 51 +++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/plugins/compound-engineering/skills/ce-dispatch/SKILL.md b/plugins/compound-engineering/skills/ce-dispatch/SKILL.md index 3dfccb2f9..17aecb642 100644 --- a/plugins/compound-engineering/skills/ce-dispatch/SKILL.md +++ b/plugins/compound-engineering/skills/ce-dispatch/SKILL.md @@ -49,7 +49,7 @@ Config keys and resolution: |---|---|---| | `dispatch_mode` | `conductor`, or another short identifier | `conductor` | | `dispatch_branch_prefix` | any string (no leading/trailing slashes) | `dispatch/` | -| `dispatch_base_branch` | any branch name | repo's default branch (`git symbolic-ref refs/remotes/origin/HEAD`) | +| `dispatch_base_branch` | any branch name | repo's default branch (`git symbolic-ref --short refs/remotes/origin/HEAD`) | | `dispatch_labels` | comma-separated label list | `ce-dispatch` | | `dispatch_auto_review` | `true` or `false` | `true` | @@ -164,7 +164,7 @@ Dispatch status: / merged. open PRs. < Act on the user's selection — do not just announce it. The bare per-option action lives inline below. Elaborate sub-flows (review tool selection, conflict resolution prose) live further down. -- **Check PR status (1)** — for each dispatched unit, run `gh pr list --search "head:"` (or query by linked issue if the workspace renamed the branch); for each match, run `gh pr view --json state,mergeable,statusCheckRollup,headRefName`. Update `dispatched_units` with the latest PR number, state (`OPEN`, `MERGED`, `CLOSED`), CI rollup, and mergeable flag. Re-render the loop status line and re-render the menu. +- **Check PR status (1)** — for each dispatched unit, run `gh pr list --state all --search "head:"` (or query by linked issue if the workspace renamed the branch); `--state all` is required because `gh pr list` defaults to open PRs only and would otherwise miss PRs merged outside this orchestrator (GitHub UI, Conductor, another shell). For each match, run `gh pr view --json state,mergeable,statusCheckRollup,headRefName`. Update `dispatched_units` with the latest PR number, state (`OPEN`, `MERGED`, `CLOSED`), CI rollup, and mergeable flag. Re-render the loop status line and re-render the menu. - **Review a PR (2)** — ask the user which U-ID's PR to review (blocking tool single-select from open PRs in `dispatched_units`). Then invoke the `ce-code-review` skill via the platform's skill-invocation primitive (`Skill` in Claude Code, `Skill` in Codex, the equivalent on Gemini/Pi), passing the PR URL as the argument. When `dispatch_auto_review: true`, also auto-trigger this for every newly opened PR before the user is asked to merge it (record per-PR `reviewed: true` so it isn't re-run). diff --git a/tests/skills/ce-dispatch-contract.test.ts b/tests/skills/ce-dispatch-contract.test.ts index 459a05d5f..e65425c3a 100644 --- a/tests/skills/ce-dispatch-contract.test.ts +++ b/tests/skills/ce-dispatch-contract.test.ts @@ -314,6 +314,57 @@ describe("ce-plan post-generation menu surfaces dispatch as a fifth option", () }) }) +describe("ce-dispatch SKILL.md regression guards (Codex-flagged bugs)", () => { + // Both guards target real bugs flagged by the upstream's chatgpt-codex-connector + // bot on EveryInc#762. Without these, the original `gh pr list` and + // `git symbolic-ref` invocations silently return the wrong data. + + test("Phase 4 status refresh queries merged PRs, not just open ones", () => { + // `gh pr list` defaults to open PRs only (CLI manual: "only lists open PRs" + // by default). Dispatched PRs merged outside this orchestrator (GitHub UI, + // Conductor, another shell) must still be discovered, otherwise the + // dependency graph never advances and `Dispatch newly unblocked units` + // can stay stuck even after prerequisites are merged. Required: --state all + // (or --state merged on a separate pass). + const phase4Start = SKILL_BODY.indexOf("### Phase 4:") + expect(phase4Start).toBeGreaterThan(-1) + const phase4Region = SKILL_BODY.slice(phase4Start) + // Match `gh pr list` invocations (those that include flags/arguments, + // identified by the `--search` flag we always pass) and require a state + // flag on each. A bare prose mention of `gh pr list` without arguments + // is not an invocation and is exempt. Allow `--state all` or + // `--state merged`. + const ghPrListInvocations = + phase4Region.match(/gh pr list[^\n`]*--search[^\n`]*/g) ?? [] + expect(ghPrListInvocations.length).toBeGreaterThan(0) + for (const inv of ghPrListInvocations) { + expect(inv).toMatch(/--state (all|merged)/) + } + }) + + test("dispatch_base_branch default uses --short to return a bare branch name", () => { + // `git symbolic-ref refs/remotes/origin/HEAD` without --short returns the + // full ref path (refs/remotes/origin/main) rather than the bare branch + // name (main). That value gets propagated into dispatch metadata / agent + // prompt instructions where a plain branch name is expected, breaking + // PR-target instructions in dispatched workspaces. + const phase0Start = SKILL_BODY.indexOf("### Phase 0:") + const phase1Start = SKILL_BODY.indexOf("### Phase 1:") + expect(phase0Start).toBeGreaterThan(-1) + expect(phase1Start).toBeGreaterThan(phase0Start) + const phase0Region = SKILL_BODY.slice(phase0Start, phase1Start) + // Every `git symbolic-ref ... refs/remotes/origin/HEAD` invocation in + // Phase 0 must include the --short flag. + const symbolicRefMatches = + phase0Region.match(/git symbolic-ref[^`\n]*refs\/remotes\/origin\/HEAD/g) ?? + [] + expect(symbolicRefMatches.length).toBeGreaterThan(0) + for (const inv of symbolicRefMatches) { + expect(inv).toContain("--short") + } + }) +}) + describe("conductor-notes.md documents key Conductor behavior", () => { const requiredHeadings = [ "Issue-to-workspace lifecycle",