Skip to content

Commit e37504c

Browse files
aksOpsclaude
andcommitted
feat(serve): checkpoint diff viewer (V18)
Lets users see the full unified diff for any checkpoint commit from the Checkpoints tab, without leaving the web UI. Backend: - internal/serve/git/diff.go — DiffAt(workdir, sha) runs `git -C workdir show --unified=3 <sha>` with a 5 s ctx timeout. Rejects workdirs without a .git dir. - internal/serve/api/diff.go — GET /api/sessions/{name}/checkpoints/ {sha}/diff handler. Full-40-char hex SHA guard up front, then CheckpointsCache.IsCheckpoint allowlist (same cache the existing /revert uses), then streams text/plain. - Registered next to /checkpoints and /revert in server.go. UI: - DiffSheet.tsx — slide-out sheet mirroring RevertSheet chrome. classifyLine colours +/−/@@/context lines in the monospace <pre>. - useCheckpointDiff(name, sha) TanStack hook, 60 s staleTime, enabled only when both params are present. - CheckpointRow splits into row + internal revert button; new "View diff" button uses stopPropagation so the existing Revert click target stays intact. - SessionDetail manages diffTarget state independent of the revert flow. Tests: - api/diff_test.go — 6 cases (happy, malformed SHA ×5, non-checkpoint, unknown session, 405 on POST, 500 on git failure). - DiffSheet.test.tsx — 4 vitest cases (classifyLine, coloured render, no fetch without checkpoint, error alert). - e2e/checkpoint-diff.spec.ts — full flow: mock checkpoints + diff, click row, assert sheet opens + coloured lines. `make regression` green: Go all pass, vitest 32, playwright 13. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent c4afe11 commit e37504c

8 files changed

Lines changed: 782 additions & 20 deletions

File tree

internal/serve/api/diff.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
package api
2+
3+
import (
4+
"log/slog"
5+
"net/http"
6+
7+
"github.com/RandomCodeSpace/ctm/internal/serve/git"
8+
)
9+
10+
// diffFn is the seam tests inject; production code wires git.DiffAt.
11+
// Kept package-private — callers wire through Diff().
12+
var diffFn = git.DiffAt
13+
14+
// Diff returns the GET handler for
15+
// /api/sessions/{name}/checkpoints/{sha}/diff.
16+
//
17+
// The handler chains two guards that together prevent arbitrary
18+
// `git show` exposure:
19+
//
20+
// 1. resolveWorkdir maps a session name to its workdir (false → 404).
21+
// 2. cache.IsCheckpoint confirms sha is one of the *full* commit
22+
// SHAs currently listed under the session's checkpoints. A SHA
23+
// that doesn't appear — including abbreviated forms — yields 404.
24+
//
25+
// On success the unified diff is streamed as text/plain so the UI can
26+
// render it in a <pre> without JSON envelope overhead.
27+
func Diff(resolveWorkdir func(name string) (string, bool), cache *CheckpointsCache) http.HandlerFunc {
28+
if cache == nil {
29+
cache = NewCheckpointsCache()
30+
}
31+
return func(w http.ResponseWriter, r *http.Request) {
32+
if r.Method != http.MethodGet && r.Method != http.MethodHead {
33+
http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
34+
return
35+
}
36+
37+
name := r.PathValue("name")
38+
sha := r.PathValue("sha")
39+
if name == "" {
40+
http.NotFound(w, r)
41+
return
42+
}
43+
// Cheap-first: reject obviously malformed SHA before doing any
44+
// lookup work. `git show` accepts abbreviated SHAs but the
45+
// checkpoint allowlist rejects them, so in practice every
46+
// legitimate caller sends a 40-char hex string.
47+
if !isFullSHA(sha) {
48+
http.Error(w, "invalid_sha", http.StatusBadRequest)
49+
return
50+
}
51+
52+
workdir, ok := resolveWorkdir(name)
53+
if !ok {
54+
http.NotFound(w, r)
55+
return
56+
}
57+
58+
// Full-SHA allowlist — same cache the /checkpoints handler and
59+
// the revert handler consult, so a cached list produced in the
60+
// last 5 s covers this call too.
61+
if !cache.IsCheckpoint(workdir, name, sha) {
62+
http.NotFound(w, r)
63+
return
64+
}
65+
66+
out, err := diffFn(workdir, sha)
67+
if err != nil {
68+
slog.Error("checkpoint diff failed",
69+
"session", name,
70+
"sha", sha,
71+
"err", err)
72+
http.Error(w, "git_failed", http.StatusInternalServerError)
73+
return
74+
}
75+
76+
w.Header().Set("Content-Type", "text/plain; charset=utf-8")
77+
w.Header().Set("Cache-Control", "no-store")
78+
_, _ = w.Write([]byte(out))
79+
}
80+
}
81+
82+
// isFullSHA reports whether s is exactly 40 lowercase-hex characters —
83+
// the canonical git SHA-1 form. Any shorter or mixed-case input is
84+
// rejected outright to keep the allowlist's blast radius minimal.
85+
func isFullSHA(s string) bool {
86+
if len(s) != 40 {
87+
return false
88+
}
89+
for i := 0; i < len(s); i++ {
90+
c := s[i]
91+
switch {
92+
case c >= '0' && c <= '9':
93+
case c >= 'a' && c <= 'f':
94+
default:
95+
return false
96+
}
97+
}
98+
return true
99+
}

internal/serve/api/diff_test.go

Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
package api
2+
3+
import (
4+
"io"
5+
"net/http"
6+
"net/http/httptest"
7+
"strings"
8+
"testing"
9+
10+
"github.com/RandomCodeSpace/ctm/internal/serve/git"
11+
)
12+
13+
const diffTestSHA = "3e17c87aabbccddee0011223344556677889900a"
14+
15+
// installDiffStubs wires the package-level seams (`checkpointsLister`
16+
// and `diffFn`) to deterministic in-memory doubles for a single test.
17+
// The prior values are restored via t.Cleanup.
18+
func installDiffStubs(
19+
t *testing.T,
20+
lister func(workdir string, limit int) ([]git.Checkpoint, error),
21+
diff func(workdir, sha string) (string, error),
22+
) {
23+
t.Helper()
24+
prevL := checkpointsLister
25+
prevD := diffFn
26+
t.Cleanup(func() {
27+
checkpointsLister = prevL
28+
diffFn = prevD
29+
})
30+
if lister != nil {
31+
checkpointsLister = lister
32+
}
33+
if diff != nil {
34+
diffFn = diff
35+
}
36+
}
37+
38+
func newDiffRequest(t *testing.T, name, sha string) *http.Request {
39+
t.Helper()
40+
req := httptest.NewRequest(http.MethodGet, "/api/sessions/"+name+"/checkpoints/"+sha+"/diff", nil)
41+
req.SetPathValue("name", name)
42+
req.SetPathValue("sha", sha)
43+
return req
44+
}
45+
46+
func TestDiff_HappyPathReturnsPlainText(t *testing.T) {
47+
wantBody := "commit 3e17c87\ndiff --git a/x b/x\n+added line\n"
48+
installDiffStubs(t,
49+
func(workdir string, limit int) ([]git.Checkpoint, error) {
50+
return []git.Checkpoint{{SHA: diffTestSHA, Subject: "checkpoint: pre-yolo"}}, nil
51+
},
52+
func(workdir, sha string) (string, error) {
53+
if sha != diffTestSHA {
54+
t.Errorf("diffFn called with sha = %q, want %q", sha, diffTestSHA)
55+
}
56+
return wantBody, nil
57+
},
58+
)
59+
60+
cache := NewCheckpointsCache()
61+
h := Diff(func(name string) (string, bool) { return "/wd", true }, cache)
62+
63+
rec := httptest.NewRecorder()
64+
h(rec, newDiffRequest(t, "sess", diffTestSHA))
65+
66+
if rec.Code != http.StatusOK {
67+
t.Fatalf("status = %d, want 200", rec.Code)
68+
}
69+
if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "text/plain") {
70+
t.Errorf("content-type = %q, want text/plain prefix", ct)
71+
}
72+
body, _ := io.ReadAll(rec.Body)
73+
if string(body) != wantBody {
74+
t.Errorf("body = %q, want %q", string(body), wantBody)
75+
}
76+
}
77+
78+
func TestDiff_MalformedSHAReturns400(t *testing.T) {
79+
// No stubs needed — the 400 path short-circuits before lookup.
80+
cache := NewCheckpointsCache()
81+
h := Diff(func(name string) (string, bool) { return "/wd", true }, cache)
82+
83+
cases := []string{
84+
"",
85+
"deadbeef", // too short
86+
"DEADBEEFDEADBEEFDEADBEEFDEADBEEFDEADBEEF", // uppercase hex
87+
"zzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzzz", // non-hex
88+
diffTestSHA + "a", // too long
89+
}
90+
for _, sha := range cases {
91+
rec := httptest.NewRecorder()
92+
h(rec, newDiffRequest(t, "sess", sha))
93+
if rec.Code != http.StatusBadRequest {
94+
t.Errorf("sha=%q: status = %d, want 400", sha, rec.Code)
95+
}
96+
}
97+
}
98+
99+
func TestDiff_NonCheckpointSHAReturns404(t *testing.T) {
100+
// Lister returns a *different* SHA, so the requested one is not
101+
// in the allowlist and the handler must refuse.
102+
otherSHA := "0011223344556677889900aabbccddeeff001122"
103+
installDiffStubs(t,
104+
func(workdir string, limit int) ([]git.Checkpoint, error) {
105+
return []git.Checkpoint{{SHA: otherSHA, Subject: "checkpoint: x"}}, nil
106+
},
107+
func(workdir, sha string) (string, error) {
108+
t.Fatalf("diffFn must not be called for non-checkpoint SHA")
109+
return "", nil
110+
},
111+
)
112+
113+
cache := NewCheckpointsCache()
114+
h := Diff(func(name string) (string, bool) { return "/wd", true }, cache)
115+
116+
rec := httptest.NewRecorder()
117+
h(rec, newDiffRequest(t, "sess", diffTestSHA))
118+
if rec.Code != http.StatusNotFound {
119+
t.Errorf("status = %d, want 404", rec.Code)
120+
}
121+
}
122+
123+
func TestDiff_UnknownSessionReturns404(t *testing.T) {
124+
// resolveWorkdir returns false → handler must 404 before touching
125+
// the cache or the diff seam.
126+
installDiffStubs(t,
127+
func(workdir string, limit int) ([]git.Checkpoint, error) {
128+
t.Fatalf("lister must not be called when session is unknown")
129+
return nil, nil
130+
},
131+
func(workdir, sha string) (string, error) {
132+
t.Fatalf("diffFn must not be called when session is unknown")
133+
return "", nil
134+
},
135+
)
136+
137+
cache := NewCheckpointsCache()
138+
h := Diff(func(name string) (string, bool) { return "", false }, cache)
139+
140+
rec := httptest.NewRecorder()
141+
h(rec, newDiffRequest(t, "missing", diffTestSHA))
142+
if rec.Code != http.StatusNotFound {
143+
t.Errorf("status = %d, want 404", rec.Code)
144+
}
145+
}
146+
147+
func TestDiff_405OnPost(t *testing.T) {
148+
cache := NewCheckpointsCache()
149+
h := Diff(func(name string) (string, bool) { return "/wd", true }, cache)
150+
rec := httptest.NewRecorder()
151+
req := httptest.NewRequest(http.MethodPost, "/api/sessions/x/checkpoints/"+diffTestSHA+"/diff", nil)
152+
req.SetPathValue("name", "x")
153+
req.SetPathValue("sha", diffTestSHA)
154+
h(rec, req)
155+
if rec.Code != http.StatusMethodNotAllowed {
156+
t.Errorf("status = %d, want 405", rec.Code)
157+
}
158+
}
159+
160+
func TestDiff_GitErrorReturns500(t *testing.T) {
161+
installDiffStubs(t,
162+
func(workdir string, limit int) ([]git.Checkpoint, error) {
163+
return []git.Checkpoint{{SHA: diffTestSHA, Subject: "checkpoint: ok"}}, nil
164+
},
165+
func(workdir, sha string) (string, error) {
166+
return "", &diffError{msg: "git show exploded"}
167+
},
168+
)
169+
170+
cache := NewCheckpointsCache()
171+
h := Diff(func(name string) (string, bool) { return "/wd", true }, cache)
172+
173+
rec := httptest.NewRecorder()
174+
h(rec, newDiffRequest(t, "sess", diffTestSHA))
175+
if rec.Code != http.StatusInternalServerError {
176+
t.Errorf("status = %d, want 500", rec.Code)
177+
}
178+
}
179+
180+
// diffError is a minimal error type for the 500-path test.
181+
type diffError struct{ msg string }
182+
183+
func (e *diffError) Error() string { return e.msg }

internal/serve/git/diff.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package git
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"os/exec"
7+
"strings"
8+
"time"
9+
)
10+
11+
// diffTimeout caps every `git show` shell-out. 5 s per V18 spec —
12+
// tighter than the 10 s gitTimeout shared by List/Revert because the
13+
// diff endpoint is called interactively from the UI and we'd rather
14+
// surface a fast error than hang the sheet.
15+
const diffTimeout = 5 * time.Second
16+
17+
// DiffAt returns the unified diff (patch + commit header) for sha in
18+
// workdir, produced by `git -C <workdir> show --unified=3 <sha>`.
19+
//
20+
// A missing workdir or one without a `.git` entry returns an error —
21+
// unlike List, which falls through to (nil, nil). The diff handler's
22+
// 404 semantics are driven by the SHA-allowlist check upstream, so a
23+
// bad workdir here is unexpected and should surface, not silently
24+
// produce an empty string.
25+
//
26+
// The caller is expected to have already validated sha through
27+
// api.CheckpointsCache.IsCheckpoint — this function shells out
28+
// without re-validation. Do not call with arbitrary user input.
29+
func DiffAt(workdir, sha string) (string, error) {
30+
if !hasGitDir(workdir) {
31+
return "", fmt.Errorf("workdir %q is not a git repository", workdir)
32+
}
33+
if strings.TrimSpace(sha) == "" {
34+
return "", fmt.Errorf("sha must not be empty")
35+
}
36+
37+
ctx, cancel := context.WithTimeout(context.Background(), diffTimeout)
38+
defer cancel()
39+
cmd := exec.CommandContext(ctx, "git", "-C", workdir, "show", "--unified=3", sha)
40+
var stderr strings.Builder
41+
cmd.Stderr = &stderr
42+
out, err := cmd.Output()
43+
if err != nil {
44+
msg := strings.TrimSpace(stderr.String())
45+
if msg == "" {
46+
return "", fmt.Errorf("git show %s: %w", sha, err)
47+
}
48+
return "", fmt.Errorf("git show %s: %w: %s", sha, err, msg)
49+
}
50+
return string(out), nil
51+
}

0 commit comments

Comments
 (0)