fix(review): pass codex skills natively instead of paraphrasing them

3c75670→main

peyton-alt·1w ago·2 files·+70 added/-12 removed

expandCodexBuiltinReview silently REPLACED a configured /review skill with a generic 28-word instruction before composing the prompt — the configured skill never ran. This is the codex sibling of the claude -p slash-expansion bug: in both cases the child executed something other than what the user configured.

Rewrite slash-form skill invocations (the agent-portable form profiles are configured with) into codex's native $name form, which codex's skill system resolves against its installed-skill catalog and loads the matching SKILL.md. Non-slash entries (plain instruction text) pass verbatim; PromptOverride is untouched.

Extracted from PR #1370 (codex review correctness), which is closed in favor of this narrower fix: its live-token tailing remains queued as a separate follow-up, its per-spawn reasoning_effort was dropped by product decision (entire does not alter how skills run), and its skilldiscovery refactor is superseded.

Co-Authored-By: Claude Fable 5 noreply@anthropic.com

Sessions

281a46a17694View transcript

[?\

Handoff To Claude: entire review RedoClaude Code·53 steps](/content/gh/entireio/cli/session/93833a17-c2c6-4cb0-85b2-663c867b105f#timeline-281a46a17694/index.html)

Changes

2

• cmd/entire/cli/agent/codex

• Mreviewer.go+22/-10

• Mreviewer_test.go+48/-2

31 unmodified lines

32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
37
52
53
54
55
4 unmodified lines

60
61
62
48
49
50
51
52
53
63
64
56
65
66
67
68
69
70
59
60
71
72
73
74
75

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).
//
// Native `codex exec review` is intentionally NOT used: it rejects an extra
// prompt when a scope flag is set, and codex hooks don't fire during it —
// leaving no channel for Entire's scope enumeration, per-run prompt, and
// checkpoint context. Plain `codex exec -` with the composed prompt on stdin
// runs the same skill while carrying our arguments.
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, "-")
    4 unmodified lines

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

122 123 124 125 125 126 127 128 128 129 130 131 331 unmodified lines

463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492 493 494 495 496 497 498 499 500 501 502 503 504 505 506 507 508 509 510 511

121 unmodified lines

prompt := readCodexCmdStdin(t, cmd) if strings.Contains(prompt, "/review") { t.Fatalf("builtin review prompt should not include raw /review:\n%s", prompt) } t.Fatalf("slash-form skill must be rewritten to codex's $ form:\n%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):", 331 unmodified lines } 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 (which meant the // configured skill never ran — the codex sibling of the claude -p // slash-expansion bug). 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:\n%s", want, prompt) } } if strings.Contains(prompt, "Review the current branch changes and report actionable findings") { t.Errorf("prompt still contains the generic paraphrase:\n%s", prompt) } if strings.Contains(prompt, "/review\n") || strings.HasSuffix(prompt, "/review") { t.Errorf("slash-form skill leaked through untransformed:\n%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) } }