Skip to content

Commit a5afdd2

Browse files
committed
test(server): sweep every GET route on a fresh and a corrupt workspace
The Studio is opened before the first run far more often than after one — that IS the first-run path — so every read handler meets a workspace with no board, no result, no squad plan, no calibration and no session. A handler that assumes any of those exist takes the whole Studio down, because a panic in one kills the process serving all 47 routes. Both sweeps pass today. They are worth having anyway: the route table is read out of server.go rather than listed in the test, so a route added tomorrow is covered the moment it is registered. A hand-maintained list would drift, and the route it forgot would be exactly the untested one. The second sweep covers a different failure mode from the first: absence is not malformation. A run killed mid-write, a hand-edited board or a file from a bad backup leaves truncated JSON, wrong-typed JSON or YAML that is not a mapping, and a handler that parses without checking crashes on content rather than on emptiness. A 500 there is honest; a panic is not. Also: the refusal block a user reads when their first run does not start now leads with `slmcode configure`. It is the only one of the three offered commands that does not require already knowing the answer — `doctor` explains the problem to somebody who can act on it, and the flags are for somebody who knows where their server is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UiLDxnEu1Gf8Q5hWAFhaAd
1 parent e4fa408 commit a5afdd2

3 files changed

Lines changed: 179 additions & 1 deletion

File tree

‎pkg/cli/probe.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,11 @@ func (p ProbeResult) Block() string {
9595
if p.Remedy != "" {
9696
b.WriteString(Dim(" tip: ") + p.Remedy + "\n")
9797
}
98-
b.WriteString(Dim(" fix: slmcode doctor · slmcode run --endpoint <url> --provider <name>\n"))
98+
// `configure` leads: it is the only one of these that does not require
99+
// already knowing the answer. `doctor` explains what is wrong to somebody
100+
// who can act on it, and the flags are for somebody who knows where their
101+
// server is — both are the second thing to reach for, not the first.
102+
b.WriteString(Dim(" fix: slmcode configure · slmcode doctor · slmcode run --endpoint <url>\n"))
99103
return b.String()
100104
}
101105

‎pkg/cli/probe_test.go‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -219,3 +219,30 @@ func TestAnAuthFailureStillPointsAtAuth(t *testing.T) {
219219
t.Errorf("remedy = %q, want the auth command", remedy)
220220
}
221221
}
222+
223+
// The refusal block is what a user reads when their first run does not start.
224+
// The line their eye goes to has to name the command that does not require
225+
// already knowing the answer.
226+
func TestTheRefusalBlockLeadsWithConfigure(t *testing.T) {
227+
p := ProbeResult{
228+
State: ProbeDown, Provider: "omlx", Model: "some-model",
229+
Endpoint: "http://127.0.0.1:8000/v1", Cause: "connection refused",
230+
Remedy: "nothing is listening",
231+
}
232+
block := p.Block()
233+
ci := strings.Index(block, "slmcode configure")
234+
di := strings.Index(block, "slmcode doctor")
235+
if ci < 0 {
236+
t.Fatalf("the refusal block does not name configure:\n%s", block)
237+
}
238+
if di >= 0 && ci > di {
239+
t.Errorf("doctor comes before configure — configure is the one that does not\n"+
240+
"require knowing the answer already:\n%s", block)
241+
}
242+
// The block still says what is wrong, not just what to run.
243+
for _, want := range []string{"connection refused", "http://127.0.0.1:8000/v1", "omlx"} {
244+
if !strings.Contains(block, want) {
245+
t.Errorf("the refusal block lost %q:\n%s", want, block)
246+
}
247+
}
248+
}

‎pkg/server/routes_sweep_test.go‎

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
package server
2+
3+
import (
4+
"net/http"
5+
"net/http/httptest"
6+
"os"
7+
"path/filepath"
8+
"regexp"
9+
"sort"
10+
"strings"
11+
"testing"
12+
13+
"github.com/UnicoLab/slmcode/pkg/harness"
14+
)
15+
16+
// ── Every GET route, on a server that has never run anything ─────────────
17+
//
18+
// The Studio is opened before the first run far more often than after one:
19+
// that IS the first-run path. Every read handler therefore meets a workspace
20+
// with no board, no result, no squad plan, no calibration and no session —
21+
// and a handler that assumes any of those exist takes the whole Studio down
22+
// with it, because a panic in one handler kills the process serving all of
23+
// them.
24+
//
25+
// The route table is read from server.go rather than listed here on purpose. A
26+
// hand-maintained list would drift the moment somebody adds a route, and the
27+
// route it forgot would be exactly the untested one.
28+
29+
var getRouteRe = regexp.MustCompile(`HandleFunc\("GET (/api/[^"]*)"`)
30+
31+
// registeredGETRoutes reads the route table out of the source.
32+
func registeredGETRoutes(t *testing.T) []string {
33+
t.Helper()
34+
src, err := os.ReadFile("server.go")
35+
if err != nil {
36+
t.Fatalf("read server.go: %v", err)
37+
}
38+
var out []string
39+
for _, m := range getRouteRe.FindAllStringSubmatch(string(src), -1) {
40+
out = append(out, m[1])
41+
}
42+
sort.Strings(out)
43+
if len(out) < 30 {
44+
t.Fatalf("found only %d GET routes; the pattern has drifted from the source", len(out))
45+
}
46+
return out
47+
}
48+
49+
// fillPathParams substitutes something plausible for {id}-style wildcards, so a
50+
// wildcard route is exercised rather than skipped.
51+
func fillPathParams(route string) string {
52+
r := strings.NewReplacer(
53+
"{id}", "does-not-exist",
54+
"{name}", "does-not-exist",
55+
"{kind}", "pipeline",
56+
)
57+
return r.Replace(route)
58+
}
59+
60+
func TestEveryGETRouteSurvivesAFreshWorkspace(t *testing.T) {
61+
root := t.TempDir()
62+
h, err := harness.New(root)
63+
if err != nil {
64+
t.Fatal(err)
65+
}
66+
if err := h.Init(); err != nil {
67+
t.Fatal(err)
68+
}
69+
s := New(h, nil)
70+
handler := s.Handler()
71+
72+
for _, route := range registeredGETRoutes(t) {
73+
path := fillPathParams(route)
74+
t.Run(route, func(t *testing.T) {
75+
// The event stream never returns; it is the one route this cannot
76+
// drive with a recorder.
77+
if strings.HasSuffix(route, "/events") {
78+
t.Skip("SSE stream — covered by the stream tests")
79+
}
80+
rec := httptest.NewRecorder()
81+
// A panic here would take down the process serving every other
82+
// route, so failing loudly on one is the whole point.
83+
defer func() {
84+
if r := recover(); r != nil {
85+
t.Fatalf("GET %s panicked on a fresh workspace: %v", path, r)
86+
}
87+
}()
88+
handler.ServeHTTP(rec, newAPIRequest(http.MethodGet, path, nil))
89+
90+
// 404/400 are legitimate answers for a resource that does not
91+
// exist. A 500 is the handler admitting it did not expect this.
92+
if rec.Code >= 500 {
93+
t.Errorf("GET %s = %d on a fresh workspace: %s",
94+
path, rec.Code, strings.TrimSpace(rec.Body.String()))
95+
}
96+
})
97+
}
98+
}
99+
100+
// A workspace can be left with malformed state — a run killed mid-write, a
101+
// hand-edited board, a file restored from a bad backup. Absence is handled by
102+
// the sweep above; malformed content is a different question, and a handler
103+
// that parses without checking takes the Studio down for every other route.
104+
func TestEveryGETRouteSurvivesACorruptWorkspace(t *testing.T) {
105+
root := t.TempDir()
106+
h, err := harness.New(root)
107+
if err != nil {
108+
t.Fatal(err)
109+
}
110+
if err := h.Init(); err != nil {
111+
t.Fatal(err)
112+
}
113+
114+
// Truncated JSON, wrong-typed JSON, and YAML that is not a mapping: the
115+
// three shapes a half-written or hand-edited file actually takes.
116+
slm := h.Config.SlmDir()
117+
for name, body := range map[string]string{
118+
"board.json": `{"tasks":[{"id":"T1","files":`,
119+
"squads.json": `{"squads":"not-a-list"}`,
120+
"pipeline.yaml": "- this is a sequence, not a mapping\n",
121+
"CONTEXT.md": "\x00\x01\x02 not text\n",
122+
} {
123+
if err := os.WriteFile(filepath.Join(slm, name), []byte(body), 0o600); err != nil {
124+
t.Fatal(err)
125+
}
126+
}
127+
128+
s := New(h, nil)
129+
handler := s.Handler()
130+
for _, route := range registeredGETRoutes(t) {
131+
path := fillPathParams(route)
132+
t.Run(route, func(t *testing.T) {
133+
if strings.HasSuffix(route, "/events") {
134+
t.Skip("SSE stream — covered by the stream tests")
135+
}
136+
rec := httptest.NewRecorder()
137+
defer func() {
138+
if r := recover(); r != nil {
139+
t.Fatalf("GET %s panicked on a corrupt workspace: %v", path, r)
140+
}
141+
}()
142+
handler.ServeHTTP(rec, newAPIRequest(http.MethodGet, path, nil))
143+
// A 500 is acceptable here — the state really is broken and saying
144+
// so is honest. A panic is not: it takes every other route with it.
145+
})
146+
}
147+
}

0 commit comments

Comments
 (0)