diff --git a/README.md b/README.md index 4c66c86..0a73584 100644 --- a/README.md +++ b/README.md @@ -98,8 +98,11 @@ The codex backend is newer and narrower than the claude one: - `build` and `bypass` work fully. Tools run, and codex asks for nothing. - `ask` and `plan` refuse gated actions rather than granting them, because the approval pane is not wired to codex yet. -- Tool calls and file changes render as cards. Side-by-side diffs, session - replay and the slash-command palette are claude-only so far. +- File changes render as real diff cards, in both the unified and side-by-side + styles. Other tool calls render as plain cards rather than the typed ones + claude gets. +- Session replay, `/compact` and the slash-command palette are claude-only so + far. `CATHODE_CODEX_LIVE=1 go test -run TestCodexLive ./...` exercises the backend against the real CLI. It spends a turn on your subscription, so it is off by diff --git a/codexengine_test.go b/codexengine_test.go index 816ccbc..a75ac03 100644 --- a/codexengine_test.go +++ b/codexengine_test.go @@ -8,6 +8,7 @@ import ( "os" "path/filepath" "runtime" + "strings" "testing" "time" ) @@ -214,3 +215,69 @@ func TestCodexAdapterMapsFramesToEntries(t *testing.T) { t.Error("the echoed user message must not be added again") } } + +// The three file-change kinds, each carrying a different thing in `diff`. All +// three shapes were taken from the live CLI; getting the kind wrong renders a +// deletion as an addition, or a hunk as a wall of new text. +func TestCodexFileDiffsReadEachKindCorrectly(t *testing.T) { + const hunk = "@@ -1,3 +1,3 @@\n line one\n-line two\n+line TWO\n line three\n" + raw := json.RawMessage(`{"changes":[ + {"path":"/tmp/a.txt","kind":{"type":"add"},"diff":"hello\n"}, + {"path":"/tmp/b.txt","kind":{"type":"delete"},"diff":"gone\n"}, + {"path":"/tmp/c.txt","kind":{"type":"update"},"diff":` + mustJSON(hunk) + `}, + {"path":"/tmp/d.txt","kind":{"type":"martian"},"diff":"?"} + ]}`) + + ds := codexFileDiffs(raw, "/tmp") + if len(ds) != 3 { + t.Fatalf("got %d diffs, want 3 (the unknown kind is skipped)", len(ds)) + } + if ds[0].file != "a.txt" { + t.Errorf("path = %q, want it trimmed against the session root", ds[0].file) + } + if ds[0].new != "hello\n" || ds[0].old != "" { + t.Errorf("add: got old=%q new=%q, want the content as the new side", ds[0].old, ds[0].new) + } + if ds[1].old != "gone\n" || ds[1].new != "" { + t.Errorf("delete: got old=%q new=%q, want the content as the old side", ds[1].old, ds[1].new) + } + if ds[2].unified != hunk { + t.Errorf("update: got unified=%q, want the supplied hunk verbatim", ds[2].unified) + } +} + +// A supplied hunk must reach the renderer untouched, and a computed one must +// still be computed. This is the pairing the refactor exists to make safe. +func TestUnifiedTextPrefersASuppliedDiff(t *testing.T) { + const hunk = "@@ -1,1 +1,1 @@\n-a\n+b\n" + if got := (fileDiff{file: "f", unified: hunk}).unifiedText(); got != hunk { + t.Errorf("supplied diff was not used verbatim: %q", got) + } + computed := (fileDiff{file: "f", old: "a\n", new: "b\n"}).unifiedText() + if computed == "" || !strings.Contains(computed, "+b") { + t.Errorf("a before/after pair should still be diffed, got %q", computed) + } +} + +// A codex update renders as a real diff card, not a raw tool card. Without the +// unified path this fell through to addTool and showed the hunk as JSON. +func TestCodexUpdateRendersAsADiffCard(t *testing.T) { + m, _ := newTestModel(t, "") + m.handleCodexEvent(codexFrame{ + Method: "item/started", + Params: json.RawMessage(`{"item":{"type":"fileChange","id":"fc1","changes":[ + {"path":"/tmp/x.go","kind":{"type":"update"},"diff":"@@ -1,1 +1,1 @@\n-a\n+b\n"} + ]}}`), + }) + last := m.entries[len(m.entries)-1] + if last.kind != entDiff { + t.Fatalf("entry kind = %v, want entDiff", last.kind) + } + if len(last.diffs) != 1 || last.diffs[0].unified == "" { + t.Errorf("diff entry = %+v, want the supplied hunk carried through", last.diffs) + } + out := stripANSI(renderDiffFor(diffUnified, last.diffs[0], 80)) + if !strings.Contains(out, "+ b") || !strings.Contains(out, "- a") { + t.Errorf("the card should show the change, got:\n%s", out) + } +} diff --git a/codexitems.go b/codexitems.go index aa18533..f814fa0 100644 --- a/codexitems.go +++ b/codexitems.go @@ -5,6 +5,7 @@ package main import ( "encoding/json" + "path/filepath" "strings" ) @@ -45,6 +46,19 @@ func (m *model) codexItem(f codexFrame, started bool) { if t := codexReasoningText(p.Item); t != "" { m.add(entThinking, t) } + case "fileChange": + if !started { + return + } + if head.ID != "" && !m.noteToolCard(head.ID) { + return + } + if ds := codexFileDiffs(p.Item, m.agentCwd); len(ds) > 0 { + m.addDiffs(ds) + return + } + m.addTool(head.Type, p.Item) + default: // A tool item. Its content is on the opening event, and the id pairs it // with the approval request that may follow, so noteToolCard keeps the @@ -59,6 +73,71 @@ func (m *model) codexItem(f codexFrame, started bool) { } } +// codexFileDiffs turns an item/fileChange into diff cards. +// +// The `diff` field means two different things depending on the kind, which is +// the whole reason this function exists rather than one assignment: +// +// add the new file's CONTENT +// delete the removed file's CONTENT +// update an already-computed unified hunk ("@@ -1,3 +1,3 @@ …") +// +// All three were confirmed against the live CLI. An update carries no copy of +// the whole file, so there is no before/after pair to build — it goes through +// fileDiff.unified, which both renderers accept because they parse unified +// text anyway. +func codexFileDiffs(raw json.RawMessage, root string) []fileDiff { + var it struct { + Changes []struct { + Path string `json:"path"` + Kind struct { + Type string `json:"type"` + } `json:"kind"` + Diff string `json:"diff"` + } `json:"changes"` + } + if json.Unmarshal(raw, &it) != nil { + return nil + } + var out []fileDiff + for _, c := range it.Changes { + if c.Path == "" { + continue + } + d := fileDiff{file: codexShortPath(c.Path, root)} + switch c.Kind.Type { + case "add": + d.new = c.Diff + case "delete": + d.old = c.Diff + case "update": + d.unified = c.Diff + default: + continue // an unknown kind: a plain card says more than a blank diff + } + out = append(out, d) + } + return out +} + +// codexShortPath trims the session's working root off a change path. +// +// codex reports absolute paths. The card title is more readable relative, and +// a screenshot of a session then carries no home directory. root is what the +// agent reported for the thread, NOT os.Getwd: those are equal by convention +// only, and a session rooted elsewhere would render every path in full. +// Falls back to the original when root is unknown or the path sits outside it. +func codexShortPath(p, root string) string { + if root == "" { + return p + } + rel, err := filepath.Rel(root, p) + if err != nil || rel == "" || strings.HasPrefix(rel, "..") { + return p + } + return rel +} + // codexReasoningText flattens a reasoning item. The summary is what the // interactive UI shows, so prefer it and fall back to the full content. func codexReasoningText(raw json.RawMessage) string { diff --git a/codexlive_test.go b/codexlive_test.go index 137cd73..aad286a 100644 --- a/codexlive_test.go +++ b/codexlive_test.go @@ -5,6 +5,8 @@ package main import ( "os" + "path/filepath" + "strings" "testing" "time" ) @@ -137,3 +139,63 @@ func TestCodexLiveGatedActionsAlwaysEndTheTurn(t *testing.T) { }) } } + +// A real edit by the real CLI must reach the screen as a diff card. +// +// The unit tests use a hunk copied from a recorded session, which proves +// cathode parses what it was told to expect. This proves codex still sends it. +func TestCodexLiveEditRendersAsADiffCard(t *testing.T) { + if os.Getenv("CATHODE_CODEX_LIVE") == "" { + t.Skip("set CATHODE_CODEX_LIVE=1 to run against the real codex CLI") + } + dir := t.TempDir() + target := filepath.Join(dir, "target.txt") + if err := os.WriteFile(target, []byte("line one\nline two\nline three\n"), 0o644); err != nil { + t.Fatal(err) + } + + // build mode: codex runs the edit without asking, so the turn completes. + e, err := newCodexEngine(codexEngineConfig{Mode: "build", Cwd: dir}) + if err != nil { + t.Fatalf("spawn: %v", err) + } + defer e.Close() + + frames := make(chan codexFrame, 256) + e.mu.Lock() + e.sink = func(f codexFrame) { frames <- f } + e.mu.Unlock() + + if err := e.Initialize(); err != nil { + t.Fatalf("Initialize: %v", err) + } + if err := e.Send("In target.txt, change the word two to TWO. Edit the file, nothing else."); err != nil { + t.Fatalf("Send: %v", err) + } + + m, _ := newTestModel(t, "") + deadline := time.After(3 * time.Minute) + for { + select { + case f := <-frames: + m.handleCodexEvent(f) + if f.Method == "turn/completed" || f.Method == "turn/failed" { + for _, en := range m.entries { + if en.kind != entDiff { + continue + } + card := stripANSI(renderDiffFor(diffUnified, en.diffs[0], 80)) + t.Logf("diff card:\n%s", card) + if !strings.Contains(card, "TWO") { + t.Errorf("the card should show the edit, got:\n%s", card) + } + return + } + t.Error("the edit never produced a diff entry") + return + } + case <-deadline: + t.Fatal("the turn never ended") + } + } +} diff --git a/codexstream.go b/codexstream.go index 18e217f..c5ca604 100644 --- a/codexstream.go +++ b/codexstream.go @@ -62,6 +62,7 @@ func (m *model) noteCodexThread(f codexFrame) { Thread struct { ID string `json:"id"` Model string `json:"model"` + Cwd string `json:"cwd"` } `json:"thread"` } if json.Unmarshal(f.Params, &p) != nil || p.Thread.ID == "" { @@ -74,6 +75,7 @@ func (m *model) noteCodexThread(f codexFrame) { if p.Thread.Model != "" { m.modelID = p.Thread.Model } + m.agentCwd = p.Thread.Cwd m.add(entInfo, fmt.Sprintf("— thread %s · %s —", short(p.Thread.ID), m.modelID)) } diff --git a/diff.go b/diff.go index 8737b40..758aacd 100644 --- a/diff.go +++ b/diff.go @@ -13,8 +13,27 @@ import ( udiff "github.com/aymanbagabas/go-udiff" ) -// fileDiff is one before/after pair to visualise. A MultiEdit produces several. -type fileDiff struct{ file, old, new string } +// fileDiff is one file change to visualise. A MultiEdit produces several. +// +// Usually it holds the before and after text and the diff is computed here. +// Some backends have already computed it: codex reports an edit as a ready-made +// unified hunk and never sends the whole file, so there is no before/after pair +// to hold. `unified` carries that case, and both renderers work from unified +// text anyway — computing it was only ever the first step. +type fileDiff struct { + file, old, new string + unified string // already-computed diff; old/new are unused when set +} + +// unifiedText is the single answer to "what should this card render". Keeping +// it in one place is what stops the two renderers disagreeing about when a +// supplied diff wins over a computed one. +func (d fileDiff) unifiedText() string { + if d.unified != "" { + return d.unified + } + return udiff.Unified("a/"+d.file, "b/"+d.file, d.old, d.new) +} // edit-tool input shapes from Claude Code. type editInput struct { @@ -43,20 +62,20 @@ func diffsForTool(name string, input json.RawMessage) ([]fileDiff, bool) { case "Edit": var in editInput if json.Unmarshal(input, &in) == nil && in.FilePath != "" { - return []fileDiff{{in.FilePath, in.OldString, in.NewString}}, true + return []fileDiff{{file: in.FilePath, old: in.OldString, new: in.NewString}}, true } case "Write": var in writeInput if json.Unmarshal(input, &in) == nil && in.FilePath != "" { old, _ := os.ReadFile(in.FilePath) // empty if new file - return []fileDiff{{in.FilePath, string(old), in.Content}}, true + return []fileDiff{{file: in.FilePath, old: string(old), new: in.Content}}, true } case "MultiEdit": var in multiEditInput if json.Unmarshal(input, &in) == nil && in.FilePath != "" { var ds []fileDiff for _, e := range in.Edits { - ds = append(ds, fileDiff{in.FilePath, e.OldString, e.NewString}) + ds = append(ds, fileDiff{file: in.FilePath, old: e.OldString, new: e.NewString}) } if len(ds) > 0 { return ds, true @@ -66,12 +85,15 @@ func diffsForTool(name string, input json.RawMessage) ([]fileDiff, bool) { return nil, false } -// renderDiff builds one styled, line-numbered diff card. -func renderDiff(filename, oldText, newText string, width int) string { +// renderDiff builds one styled, line-numbered diff card from unified diff text. +// +// It takes the unified text rather than a before/after pair because that is +// what it always worked on: the old signature computed the unified form on the +// first line and spent the rest of the function parsing it back. +func renderDiff(filename, u string, width int) string { if width < 24 { width = 24 } - u := udiff.Unified("a/"+filename, "b/"+filename, oldText, newText) if strings.TrimSpace(u) == "" { return dBox.Width(width - 2).Render(dTitle.Render(" "+filename+" ") + "\n" + dCtx.Render("(no changes)")) } diff --git a/diff_split.go b/diff_split.go index 0b427d0..b8187c8 100644 --- a/diff_split.go +++ b/diff_split.go @@ -8,7 +8,6 @@ import ( "strconv" "strings" - udiff "github.com/aymanbagabas/go-udiff" "github.com/charmbracelet/lipgloss" ) @@ -57,19 +56,20 @@ func (m *model) commitDiff(id string) { // renderDiffFor dispatches to the configured diff style, falling back to the // unified card when split is selected but the terminal is too narrow for it. -func renderDiffFor(style, filename, oldText, newText string, width int) string { +// The unified text is produced once, here, so both styles render the same diff. +func renderDiffFor(style string, d fileDiff, width int) string { + u := d.unifiedText() if style == diffSplit && width >= splitMinWidth { - return renderDiffSplit(filename, oldText, newText, width) + return renderDiffSplit(d.file, u, width) } - return renderDiff(filename, oldText, newText, width) + return renderDiff(d.file, u, width) } // renderDiffSplit builds a side-by-side diff card: deletions (with old line // numbers) on the left, additions (new line numbers) on the right, context on // both. A change is shown as a removed line beside its added line; unpaired // adds/dels leave the opposite column blank. -func renderDiffSplit(filename, oldText, newText string, width int) string { - u := udiff.Unified("a/"+filename, "b/"+filename, oldText, newText) +func renderDiffSplit(filename, u string, width int) string { if strings.TrimSpace(u) == "" { return dBox.Width(width - 2).Render(dTitle.Render(" "+filename+" ") + "\n" + dCtx.Render("(no changes)")) } diff --git a/diff_split_test.go b/diff_split_test.go index c9440ec..8f70285 100644 --- a/diff_split_test.go +++ b/diff_split_test.go @@ -18,7 +18,7 @@ func stripANSI(s string) string { return ansiRe.ReplaceAllString(s, "") } func TestRenderDiffSplit(t *testing.T) { old := "func add(a, b int) int {\n\treturn a + b\n}\n" neu := "func add(a, b, c int) int {\n\treturn a + b + c\n}\n" - out := stripANSI(renderDiffSplit("math.go", old, neu, 100)) + out := stripANSI(renderDiffSplit("math.go", fileDiff{file: "math.go", old: old, new: neu}.unifiedText(), 100)) if !strings.Contains(out, "math.go") || !strings.Contains(out, "+2") || !strings.Contains(out, "-2") { t.Fatalf("missing title / counts:\n%s", out) @@ -57,13 +57,14 @@ func TestDiffCommand(t *testing.T) { // renderDiffFor honors the style and falls back to unified when too narrow. func TestRenderDiffForFallback(t *testing.T) { old, neu := "a\nb\n", "a\nc\n" - if renderDiffFor(diffSplit, "f", old, neu, 60) != renderDiff("f", old, neu, 60) { + d := fileDiff{file: "f", old: old, new: neu} + if renderDiffFor(diffSplit, d, 60) != renderDiff("f", d.unifiedText(), 60) { t.Error("split below splitMinWidth should fall back to unified") } - if renderDiffFor(diffSplit, "f", old, neu, 120) != renderDiffSplit("f", old, neu, 120) { + if renderDiffFor(diffSplit, d, 120) != renderDiffSplit("f", d.unifiedText(), 120) { t.Error("split at a wide width should render side-by-side") } - if renderDiffFor(diffUnified, "f", old, neu, 120) != renderDiff("f", old, neu, 120) { + if renderDiffFor(diffUnified, d, 120) != renderDiff("f", d.unifiedText(), 120) { t.Error("unified style should always render the single-column card") } } diff --git a/model.go b/model.go index bdd55af..317044a 100644 --- a/model.go +++ b/model.go @@ -154,6 +154,11 @@ type model struct { frameBody string bodyKey bodyKey contentVer int + // agentCwd is the working root the agent reported for this session. Used to + // shorten paths for display. Read from the protocol rather than os.Getwd + // because the two are only equal by convention: the agent is told its root + // explicitly, and nothing stops it differing. + agentCwd string toolUses map[string]string // tool_use_id -> tool name, so tool_result events can show what they're answering shownTools map[string]bool // tool_use_ids already drawn as a card/diff, so the stream and approval paths don't both draw one (toolcard.go) busy bool diff --git a/render.go b/render.go index fc84344..557f15f 100644 --- a/render.go +++ b/render.go @@ -117,7 +117,7 @@ func (m *model) renderEntry(e entry) string { case entDiff: parts := make([]string, 0, len(e.diffs)) for _, d := range e.diffs { - parts = append(parts, renderDiffFor(m.settings.Diff, d.file, d.old, d.new, m.vp.Width)) + parts = append(parts, renderDiffFor(m.settings.Diff, d, m.vp.Width)) } return strings.Join(parts, "\n") case entInfo: diff --git a/render_test.go b/render_test.go index 0346089..e0af205 100644 --- a/render_test.go +++ b/render_test.go @@ -36,7 +36,7 @@ func TestDiffRendering(t *testing.T) { if !ok || len(ds) != 1 { t.Fatalf("Edit not detected: ok=%v n=%d", ok, len(ds)) } - out := renderDiff(ds[0].file, ds[0].old, ds[0].new, 80) + out := renderDiff(ds[0].file, ds[0].unifiedText(), 80) for _, want := range []string{"main.go", "+", "-", "│"} { if !strings.Contains(out, want) { t.Fatalf("diff missing %q in:\n%s", want, out)