Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .golangci.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@ linters:
- path: ^internal/sim/
linters:
- wrapcheck
- path: ^cmd/entire/cli/review/tui_model\.go$
linters:
- ireturn
- text: "`strat` is a misspelling"
linters:
- misspell
5 changes: 5 additions & 0 deletions cmd/entire/cli/agent/claudecode/reviewer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,11 @@ func TestReviewer_ArgvShape(t *testing.T) {
if cmd.Args[2] == "" {
t.Error("Args[2] (prompt) is empty")
}
for _, arg := range cmd.Args {
if arg == "--continue" || arg == "-c" || arg == "--resume" || arg == "-r" {
t.Fatalf("Args must start a fresh Claude review, got resume/continue flag in %v", cmd.Args)
}
}
// Stdin must be nil — claude receives prompt via argv, not stdin.
if cmd.Stdin != nil {
t.Errorf("cmd.Stdin = %v, want nil (claude uses argv, not stdin)", cmd.Stdin)
Expand Down
38 changes: 16 additions & 22 deletions cmd/entire/cli/agent/codex/output_filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,8 @@ func Strip(r io.Reader) io.Reader {
scanner := bufio.NewScanner(r)
scanner.Buffer(make([]byte, 1024*1024), 16*1024*1024)
state := codexStripNormal
var currentAssistant []string
for scanner.Scan() {
done, err := collectFinalCodexLine(scanner.Text(), &state, &currentAssistant, pw)
done, err := collectFinalCodexLine(scanner.Text(), &state, pw)
if err != nil {
_ = pw.CloseWithError(err)
return
Expand All @@ -116,12 +115,6 @@ func Strip(r io.Reader) io.Reader {
_ = pw.CloseWithError(err)
return
}
if state != codexStripAfterTokens {
if err := writeCodexAssistantBlock(pw, currentAssistant); err != nil {
_ = pw.CloseWithError(err)
return
}
}
_ = pw.Close()
}()
return pr
Expand All @@ -137,7 +130,7 @@ const (
codexStripAfterTokens
)

func collectFinalCodexLine(raw string, state *codexStripState, currentAssistant *[]string, w io.Writer) (bool, error) {
func collectFinalCodexLine(raw string, state *codexStripState, w io.Writer) (bool, error) {
cleaned := csiRegex.ReplaceAllString(raw, "")
trimmed := strings.TrimSpace(cleaned)
trimmedRight := strings.TrimRight(cleaned, " \t")
Expand All @@ -146,34 +139,34 @@ func collectFinalCodexLine(raw string, state *codexStripState, currentAssistant
return true, nil
}
if isTokensUsedMarker(trimmed) {
return true, writeCodexAssistantBlock(w, *currentAssistant)
return true, nil
}

switch *state {
case codexStripUserBlock:
if isCodexRoleMarker(trimmed) {
*state = codexStripAssistantBlock
*currentAssistant = nil
}
return false, nil
case codexStripAssistantBlock:
if isCodexRoleMarker(trimmed) {
*currentAssistant = nil
return false, nil
}
if isUserRoleMarker(trimmed) {
*state = codexStripUserBlock
return false, nil
}
if trimmedRight == "exec" || execBlockRegex.MatchString(trimmedRight) {
if trimmedRight == codexExecCommand || execBlockRegex.MatchString(trimmedRight) {
*state = codexStripExecBlock
return false, nil
}
if isCodexMetadataLine(trimmed) {
return false, nil
}
if line, ok := FilterLine(raw); ok {
*currentAssistant = append(*currentAssistant, line)
if err := writeCodexLine(w, line); err != nil {
return false, err
}
}
return false, nil
case codexStripExecBlock:
Expand All @@ -183,7 +176,6 @@ func collectFinalCodexLine(raw string, state *codexStripState, currentAssistant
}
if isCodexRoleMarker(trimmed) {
*state = codexStripAssistantBlock
*currentAssistant = nil
return false, nil
}
return false, nil
Expand All @@ -199,28 +191,30 @@ func collectFinalCodexLine(raw string, state *codexStripState, currentAssistant
}
if isCodexRoleMarker(trimmed) {
*state = codexStripAssistantBlock
*currentAssistant = nil
return false, nil
}
if isCodexMetadataLine(trimmed) {
return false, nil
}
if trimmedRight == "exec" {
if trimmedRight == codexExecCommand {
*state = codexStripExecBlock
return false, nil
}
if execBlockRegex.MatchString(trimmedRight) {
return false, nil
}

if line, ok := FilterLine(raw); ok {
if err := writeCodexLine(w, line); err != nil {
return false, err
}
}
return false, nil
}

func writeCodexAssistantBlock(w io.Writer, lines []string) error {
for _, line := range lines {
if _, err := w.Write([]byte(line + "\n")); err != nil {
return fmt.Errorf("write filtered codex output: %w", err)
}
func writeCodexLine(w io.Writer, line string) error {
if _, err := w.Write([]byte(line + "\n")); err != nil {
return fmt.Errorf("write filtered codex output: %w", err)
}
return nil
}
Expand Down
39 changes: 30 additions & 9 deletions cmd/entire/cli/agent/codex/output_filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,31 @@ func TestFilterLine_VersionNarrativeKept(t *testing.T) {
}
}

func TestStrip_PlainExecOutputPassesThrough(t *testing.T) {
t.Parallel()

input := strings.Join([]string{
"Review findings:",
"- Missing regression coverage in review picker.",
"- Codex output should appear in the final dump.",
}, "\n")

data, err := io.ReadAll(Strip(strings.NewReader(input)))
if err != nil {
t.Fatalf("Strip read: %v", err)
}
output := string(data)
for _, want := range []string{
"Review findings:",
"- Missing regression coverage in review picker.",
"- Codex output should appear in the final dump.",
} {
if !strings.Contains(output, want) {
t.Fatalf("plain output missing %q:\n%s", want, output)
}
}
}

func TestStrip_FullFixture(t *testing.T) {
t.Parallel()

Expand Down Expand Up @@ -229,21 +254,15 @@ func TestStrip_FullFixture(t *testing.T) {

// Narrative must survive.
narrativeMustSurvive := []string{
"This is the narrative output from the agent.",
"It spans multiple lines.",
"Final conclusion: no issues found.",
}
for _, want := range narrativeMustSurvive {
if !strings.Contains(output, want) {
t.Errorf("narrative %q missing from filtered output; got:\n%s", want, output)
}
}
for _, unwanted := range []string{
"This is the narrative output from the agent.",
"It spans multiple lines.",
} {
if strings.Contains(output, unwanted) {
t.Errorf("pre-final narrative %q should not appear in filtered output; got:\n%s", unwanted, output)
}
}
}

func TestStrip_DropsExecBlocksAndDuplicateSummary(t *testing.T) {
Expand Down Expand Up @@ -291,7 +310,6 @@ func TestStrip_DropsExecBlocksAndDuplicateSummary(t *testing.T) {
"OpenAI Codex",
"workdir:",
"Please run these review skills",
"I will inspect the code.",
"git status",
"cmd/entire/cli/review.go",
"go test ./...",
Expand All @@ -306,6 +324,9 @@ func TestStrip_DropsExecBlocksAndDuplicateSummary(t *testing.T) {
if strings.Count(output, "No findings.") != 1 {
t.Fatalf("filtered output should contain final response once, got:\n%s", output)
}
if !strings.Contains(output, "I will inspect the code.") {
t.Fatalf("filtered output missing live assistant progress line:\n%s", output)
}
if !strings.Contains(output, "Residual risk: tests were not run in this sandbox.") {
t.Fatalf("filtered output missing final residual-risk line:\n%s", output)
}
Expand Down
151 changes: 144 additions & 7 deletions cmd/entire/cli/agent/codex/reviewer.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,36 @@ func NewReviewer() *reviewtypes.ReviewerTemplate {
// buildCodexReviewCmd builds the exec.Cmd for a codex review run.
// Exposed at package level for test inspection of argv, stdin, and env.
func buildCodexReviewCmd(ctx context.Context, cfg reviewtypes.RunConfig) *exec.Cmd {
prompt := review.ComposeReviewPrompt(cfg)
cmd := exec.CommandContext(ctx, "codex", "exec", "--skip-git-repo-check", "-")
promptCfg := cfg
promptCfg.Skills = expandCodexBuiltinReview(cfg.Skills)
args := []string{codexExecCommand, "--skip-git-repo-check", "-"}
prompt := review.ComposeReviewPrompt(promptCfg)
cmd := exec.CommandContext(ctx, "codex", args...)
cmd.Stdin = strings.NewReader(prompt)
cmd.Env = review.AppendReviewEnv(os.Environ(), "codex", cfg, prompt)
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 {
out := make([]string, 0, len(skills))
for _, skill := range skills {
if skill == "/review" {
out = append(out, codexBuiltinReviewPrompt)
continue
}
out = append(out, skill)
}
return out
}

// parseCodexOutput wraps the reader with the chrome filter and converts
// remaining lines into a stream of Events.
// On clean EOF emits Finished{Success: true}. On a scanner error (including
Expand All @@ -49,14 +72,13 @@ func parseCodexOutput(r io.Reader) <-chan reviewtypes.Event {
go func() {
defer close(out)
out <- reviewtypes.Started{}
scanner := bufio.NewScanner(Strip(r))
scanner := bufio.NewScanner(r)
scanner.Buffer(make([]byte, 1024*1024), 16*1024*1024)
state := codexEventNormal
for scanner.Scan() {
line := scanner.Text()
if line == "" {
continue
for _, ev := range collectCodexEventsLine(scanner.Text(), &state) {
out <- ev
}
out <- reviewtypes.AssistantText{Text: line}
}
if err := scanner.Err(); err != nil {
out <- reviewtypes.RunError{Err: fmt.Errorf("read stdout: %w", err)}
Expand All @@ -67,3 +89,118 @@ func parseCodexOutput(r io.Reader) <-chan reviewtypes.Event {
}()
return out
}

type codexEventState int

const (
codexEventNormal codexEventState = iota
codexEventUserBlock
codexEventAssistantBlock
codexEventExecAwaitCommand
codexEventExecBlock
codexEventAfterTokens
)

func collectCodexEventsLine(raw string, state *codexEventState) []reviewtypes.Event {
cleaned := csiRegex.ReplaceAllString(raw, "")
trimmed := strings.TrimSpace(cleaned)
trimmedRight := strings.TrimRight(cleaned, " \t")

if *state == codexEventAfterTokens {
return nil
}
if isTokensUsedMarker(trimmed) {
*state = codexEventAfterTokens
return nil
}

switch *state {
case codexEventUserBlock:
if isCodexRoleMarker(trimmed) {
*state = codexEventAssistantBlock
}
return nil
case codexEventAssistantBlock:
return collectCodexAssistantLine(raw, trimmed, trimmedRight, state)
case codexEventExecAwaitCommand:
return collectCodexExecCommandLine(trimmed, trimmedRight, state)
case codexEventExecBlock:
switch {
case trimmed == "":
*state = codexEventNormal
case isCodexRoleMarker(trimmed):
*state = codexEventAssistantBlock
case isUserRoleMarker(trimmed):
*state = codexEventUserBlock
}
return nil
case codexEventNormal:
// Continue below.
case codexEventAfterTokens:
return nil
}

if isUserRoleMarker(trimmed) {
*state = codexEventUserBlock
return nil
}
if isCodexRoleMarker(trimmed) {
*state = codexEventAssistantBlock
return nil
}
if isCodexMetadataLine(trimmed) {
return nil
}
if trimmedRight == codexExecCommand {
*state = codexEventExecAwaitCommand
return nil
}
if execBlockRegex.MatchString(trimmedRight) {
*state = codexEventExecBlock
return []reviewtypes.Event{reviewtypes.ToolCall{Name: codexExecCommand, Args: trimmedRight}}
}
if line, ok := FilterLine(raw); ok {
return []reviewtypes.Event{reviewtypes.AssistantText{Text: line}}
}
return nil
}

func collectCodexAssistantLine(raw, trimmed, trimmedRight string, state *codexEventState) []reviewtypes.Event {
switch {
case isCodexRoleMarker(trimmed):
return nil
case isUserRoleMarker(trimmed):
*state = codexEventUserBlock
return nil
case trimmedRight == codexExecCommand:
*state = codexEventExecAwaitCommand
return nil
case execBlockRegex.MatchString(trimmedRight):
*state = codexEventExecBlock
return []reviewtypes.Event{reviewtypes.ToolCall{Name: codexExecCommand, Args: trimmedRight}}
case isCodexMetadataLine(trimmed):
return nil
default:
if line, ok := FilterLine(raw); ok {
return []reviewtypes.Event{reviewtypes.AssistantText{Text: line}}
}
return nil
}
}

func collectCodexExecCommandLine(trimmed, trimmedRight string, state *codexEventState) []reviewtypes.Event {
switch {
case trimmed == "":
*state = codexEventNormal
return nil
case isCodexRoleMarker(trimmed):
*state = codexEventAssistantBlock
return nil
case isUserRoleMarker(trimmed):
*state = codexEventUserBlock
return nil
default:
*state = codexEventExecBlock
return []reviewtypes.Event{reviewtypes.ToolCall{Name: codexExecCommand, Args: trimmedRight}}
}
}
Loading
Loading