Merge remote-tracking branch 'origin/feat/review-codex-skill-invocation' into feat/codex-skill-discovery

c18972a→main·

Changes

2

• cmd/entire/cli/agent/codex

• Mreviewer.go+22/-10

  • Mreviewer_test.go+48/-2

31 unmodified lines

// buildCodexReviewCmd builds the exec.Cmd for a codex review run.
// Exposed at package level for test inspection of argv, stdin, and env.
// buildCodexReviewCmd builds the exec.Cmd for a codex review run.

// Configured skills are passed through in codex's native $name form — NOT
// paraphrased. Codex's skill system injects a catalog of installed skills
// into every exec session and loads the matching SKILL.md when the prompt
// names one, so the agent runs the real configured workflow. A previous
// version silently REPLACED /review with a generic 28-word instruction: the
// configured skill never ran (the codex sibling of the claude -p
// slash-expansion bug, where the built-in /review hijacked the prompt).

func buildCodexReviewCmd(ctx context.Context, cfg reviewtypes.RunConfig) *exec.Cmd {
    promptCfg := cfg
    promptCfg.Skills = expandCodexBuiltinReview(cfg.Skills)
    promptCfg.Skills = codexNativeSkillInvocations(cfg.Skills)
    args := []string{codexExecCommand, "--skip-git-repo-check", "--json"}
    args = review.AppendModelFlag(args, cfg.Model)
    args = append(args, "-")

return cmd
}

// Codex's native `exec review --base <branch>` rejects an additional prompt,
// so expand `/review` into text and run normal `codex exec -`. That preserves
// Entire's scoped base clause, per-run instructions, and checkpoint context.
const codexBuiltinReviewPrompt = "Review the current branch changes and report actionable findings. " +
    "Prioritize correctness, regressions, security, and missing test coverage. Do not make code changes."

const codexExecCommand = "exec"

func expandCodexBuiltinReview(skills []string) []string {
// codexNativeSkillInvocations rewrites slash-form skill invocations (the
// agent-portable form profiles are configured with) into codex's native
// $name form. Non-slash entries (plain instruction text) pass verbatim.
func codexNativeSkillInvocations(skills []string) []string {
    out := make([]string, 0, len(skills))
    for _, skill := range skills {
        if skill == "/review" {
            out = append(out, codexBuiltinReviewPrompt)
        }
        if rest, ok := strings.CutPrefix(skill, "/"); ok && rest != "" {
            out = append(out, "$"+rest)
            continue
        }
        out = append(out, skill)
    }
    return out
}

Mcmd/entire/cli/agent/codex/reviewer.go+22/-10

121 unmodified lines

prompt := readCodexCmdStdin(t, cmd) if strings.Contains(prompt, "/review") { t.Fatalf("builtin review prompt should not include raw /review: %s", prompt) } for _, wantText := range []string{ "Review the current branch changes and report actionable findings.", "$review", "Focus on auth regressions.", "Scope: review the commits unique to this branch vs main, plus any uncommitted changes in the working tree. Ignore code outside this scope.", "Commits in scope (newest first):", } return m }

// TestBuildCodexReviewCmd_SkillsPassNativelyNotParaphrased locks the fix for // codex skill invocation: configured skills reach codex in its native $name // form so codex's skill system loads the real SKILL.md, instead of /review // being silently REPLACED with a generic 28-word paraphrase. func TestBuildCodexReviewCmd_SkillsPassNativelyNotParaphrased(t *testing.T) { t.Parallel() cmd := buildCodexReviewCmd(context.Background(), reviewtypes.RunConfig{ Skills: []string{ "/review", "/pr-review-toolkit:review-pr", "plain instruction line"}, }) stdin, err := io.ReadAll(cmd.Stdin) if err != nil { t.Fatal(err) } prompt := string(stdin) for _, want := range []string{"$review", "$pr-review-toolkit:review-pr", "plain instruction line"} { if !strings.Contains(prompt, want) { t.Errorf("prompt missing native skill invocation %q: %s", want, prompt) } } if strings.Contains(prompt, "Review the current branch changes and report actionable findings") { t.Errorf("prompt still contains the generic paraphrase: %s", prompt) } if strings.Contains(prompt, "/review\n") || strings.HasSuffix(prompt, "/review") { t.Errorf("slash-form skill leaked through untransformed: %s", prompt) } }

// TestBuildCodexReviewCmd_PromptOverrideVerbatim ensures the $-form transform // never touches a verbatim prompt override. func TestBuildCodexReviewCmd_PromptOverrideVerbatim(t *testing.T) { t.Parallel() cmd := buildCodexReviewCmd(context.Background(), reviewtypes.RunConfig{ Skills: []string{ "/review"}, PromptOverride: "/review exactly as written", }) stdin, err := io.ReadAll(cmd.Stdin) if err != nil { t.Fatal(err) } if got := string(stdin); got != "/review exactly as written" { t.Errorf("PromptOverride modified: %q", got) } }