Skip to content

Commit 8b987aa

Browse files
andyrewleeandyrewlee
authored
Improve OpenClaw DX validation and summary sanitization (#179)
* Improve OpenClaw DX validation and summary sanitization * Fix openclaw DX/step review regressions
1 parent 812b783 commit 8b987aa

12 files changed

+1560
-161
lines changed

‎internal/cli/openclaw_dx_script_assistant_validation_test.go‎

Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
package cli
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestOpenClawDXStart_InvalidAssistantReturnsCommandError(t *testing.T) {
11+
requireBinary(t, "jq")
12+
requireBinary(t, "bash")
13+
14+
scriptPath := filepath.Join("..", "..", "skills", "amux", "scripts", "openclaw-dx.sh")
15+
fakeBinDir := t.TempDir()
16+
fakeAmuxPath := filepath.Join(fakeBinDir, "amux")
17+
18+
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
19+
set -euo pipefail
20+
if [[ "${1:-}" == "--json" ]]; then
21+
shift
22+
fi
23+
case "${1:-} ${2:-}" in
24+
"workspace list")
25+
printf '%s' '{"ok":true,"data":[{"id":"ws-1","name":"demo","repo":"/tmp/demo"}],"error":null}'
26+
;;
27+
*)
28+
printf '%s' '{"ok":true,"data":{},"error":null}'
29+
;;
30+
esac
31+
`)
32+
33+
env := os.Environ()
34+
env = withEnv(env, "PATH", fakeBinDir+":"+os.Getenv("PATH"))
35+
36+
payload := runScriptJSON(t, scriptPath, env,
37+
"start",
38+
"--workspace", "ws-1",
39+
"--assistant", "not/a-real-assistant",
40+
"--prompt", "hello",
41+
)
42+
43+
if got, _ := payload["command"].(string); got != "start" {
44+
t.Fatalf("command = %q, want %q", got, "start")
45+
}
46+
if got, _ := payload["status"].(string); got != "command_error" {
47+
t.Fatalf("status = %q, want %q", got, "command_error")
48+
}
49+
summary, _ := payload["summary"].(string)
50+
if !strings.Contains(summary, "invalid assistant") {
51+
t.Fatalf("summary = %q, want invalid assistant", summary)
52+
}
53+
}
54+
55+
func TestOpenClawDXReview_InvalidAssistantReturnsCommandError(t *testing.T) {
56+
requireBinary(t, "jq")
57+
requireBinary(t, "bash")
58+
59+
scriptPath := filepath.Join("..", "..", "skills", "amux", "scripts", "openclaw-dx.sh")
60+
fakeBinDir := t.TempDir()
61+
fakeAmuxPath := filepath.Join(fakeBinDir, "amux")
62+
63+
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
64+
set -euo pipefail
65+
if [[ "${1:-}" == "--json" ]]; then
66+
shift
67+
fi
68+
case "${1:-} ${2:-}" in
69+
"workspace list")
70+
printf '%s' '{"ok":true,"data":[{"id":"ws-1","name":"demo","repo":"/tmp/demo"}],"error":null}'
71+
;;
72+
*)
73+
printf '%s' '{"ok":true,"data":{},"error":null}'
74+
;;
75+
esac
76+
`)
77+
78+
env := os.Environ()
79+
env = withEnv(env, "PATH", fakeBinDir+":"+os.Getenv("PATH"))
80+
81+
payload := runScriptJSON(t, scriptPath, env,
82+
"review",
83+
"--workspace", "ws-1",
84+
"--assistant", "not/a-real-assistant",
85+
)
86+
87+
if got, _ := payload["command"].(string); got != "review" {
88+
t.Fatalf("command = %q, want %q", got, "review")
89+
}
90+
if got, _ := payload["status"].(string); got != "command_error" {
91+
t.Fatalf("status = %q, want %q", got, "command_error")
92+
}
93+
summary, _ := payload["summary"].(string)
94+
if !strings.Contains(summary, "invalid assistant") {
95+
t.Fatalf("summary = %q, want invalid assistant", summary)
96+
}
97+
}
98+
99+
func TestOpenClawDXWorkflowKickoff_InvalidAssistantReturnsCommandError(t *testing.T) {
100+
requireBinary(t, "jq")
101+
requireBinary(t, "bash")
102+
103+
scriptPath := filepath.Join("..", "..", "skills", "amux", "scripts", "openclaw-dx.sh")
104+
fakeBinDir := t.TempDir()
105+
fakeAmuxPath := filepath.Join(fakeBinDir, "amux")
106+
107+
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
108+
set -euo pipefail
109+
printf '%s' '{"ok":true,"data":{},"error":null}'
110+
`)
111+
112+
env := os.Environ()
113+
env = withEnv(env, "PATH", fakeBinDir+":"+os.Getenv("PATH"))
114+
115+
payload := runScriptJSON(t, scriptPath, env,
116+
"workflow", "kickoff",
117+
"--name", "ws-new",
118+
"--project", "/tmp/example",
119+
"--assistant", "not/a-real-assistant",
120+
"--prompt", "hello",
121+
)
122+
123+
if got, _ := payload["command"].(string); got != "workflow.kickoff" {
124+
t.Fatalf("command = %q, want %q", got, "workflow.kickoff")
125+
}
126+
if got, _ := payload["status"].(string); got != "command_error" {
127+
t.Fatalf("status = %q, want %q", got, "command_error")
128+
}
129+
summary, _ := payload["summary"].(string)
130+
if !strings.Contains(summary, "invalid assistant") {
131+
t.Fatalf("summary = %q, want invalid assistant", summary)
132+
}
133+
}
134+
135+
func TestOpenClawDXWorkflowDual_InvalidAssistantReturnsCommandError(t *testing.T) {
136+
requireBinary(t, "jq")
137+
requireBinary(t, "bash")
138+
139+
scriptPath := filepath.Join("..", "..", "skills", "amux", "scripts", "openclaw-dx.sh")
140+
fakeBinDir := t.TempDir()
141+
fakeAmuxPath := filepath.Join(fakeBinDir, "amux")
142+
143+
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
144+
set -euo pipefail
145+
if [[ "${1:-}" == "--json" ]]; then
146+
shift
147+
fi
148+
case "${1:-} ${2:-}" in
149+
"workspace list")
150+
printf '%s' '{"ok":true,"data":[{"id":"ws-1","name":"demo","repo":"/tmp/demo"}],"error":null}'
151+
;;
152+
*)
153+
printf '%s' '{"ok":true,"data":{},"error":null}'
154+
;;
155+
esac
156+
`)
157+
158+
env := os.Environ()
159+
env = withEnv(env, "PATH", fakeBinDir+":"+os.Getenv("PATH"))
160+
161+
payload := runScriptJSON(t, scriptPath, env,
162+
"workflow", "dual",
163+
"--workspace", "ws-1",
164+
"--implement-assistant", "fake/impl",
165+
"--review-assistant", "codex",
166+
)
167+
168+
if got, _ := payload["command"].(string); got != "workflow.dual" {
169+
t.Fatalf("command = %q, want %q", got, "workflow.dual")
170+
}
171+
if got, _ := payload["status"].(string); got != "command_error" {
172+
t.Fatalf("status = %q, want %q", got, "command_error")
173+
}
174+
summary, _ := payload["summary"].(string)
175+
if !strings.Contains(summary, "invalid assistant") {
176+
t.Fatalf("summary = %q, want invalid assistant", summary)
177+
}
178+
}
179+
180+
func TestOpenClawDXStart_UnlistedAssistantPassesThrough(t *testing.T) {
181+
requireBinary(t, "jq")
182+
requireBinary(t, "bash")
183+
184+
scriptPath := filepath.Join("..", "..", "skills", "amux", "scripts", "openclaw-dx.sh")
185+
fakeBinDir := t.TempDir()
186+
fakeAmuxPath := filepath.Join(fakeBinDir, "amux")
187+
fakeTurnPath := filepath.Join(fakeBinDir, "openclaw-turn.sh")
188+
189+
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
190+
set -euo pipefail
191+
if [[ "${1:-}" == "--json" ]]; then
192+
shift
193+
fi
194+
case "${1:-} ${2:-}" in
195+
"workspace list")
196+
printf '%s' '{"ok":true,"data":[{"id":"ws-1","name":"demo","repo":"/tmp/demo"}],"error":null}'
197+
;;
198+
*)
199+
printf '%s' '{"ok":true,"data":{},"error":null}'
200+
;;
201+
esac
202+
`)
203+
204+
writeExecutable(t, fakeTurnPath, `#!/usr/bin/env bash
205+
set -euo pipefail
206+
printf '%s' '{"ok":true,"mode":"run","status":"idle","overall_status":"completed","summary":"turn ok","agent_id":"agent-1","workspace_id":"ws-1","assistant":"aider","quick_actions":[],"channel":{"message":"done","chunks":["done"],"chunks_meta":[{"index":1,"total":1,"text":"done"}],"inline_buttons":[]}}'
207+
`)
208+
209+
env := os.Environ()
210+
env = withEnv(env, "PATH", fakeBinDir+":"+os.Getenv("PATH"))
211+
env = withEnv(env, "OPENCLAW_DX_TURN_SCRIPT", fakeTurnPath)
212+
213+
payload := runScriptJSON(t, scriptPath, env,
214+
"start",
215+
"--workspace", "ws-1",
216+
"--assistant", "aider",
217+
"--prompt", "hello",
218+
)
219+
220+
if got, _ := payload["command"].(string); got != "start" {
221+
t.Fatalf("command = %q, want %q", got, "start")
222+
}
223+
if got, _ := payload["status"].(string); got == "command_error" {
224+
t.Fatalf("status = %q, expected non-command_error for unlisted assistant pass-through", got)
225+
}
226+
}

‎internal/cli/openclaw_dx_script_assistants_git_test.go‎

Lines changed: 22 additions & 109 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,17 @@ func TestOpenClawDXAssistants_ReportsMissingFromConfig(t *testing.T) {
2525

2626
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
2727
set -euo pipefail
28-
printf '%s' '{"ok":true,"data":{},"error":null}'
28+
if [[ "${1:-}" == "--json" ]]; then
29+
shift
30+
fi
31+
case "${1:-} ${2:-}" in
32+
"workspace list")
33+
printf '%s' '{"ok":true,"data":[{"id":"ws-1","name":"demo","repo":"/tmp/demo","assistant":"codex"}],"error":null}'
34+
;;
35+
*)
36+
printf '%s' '{"ok":true,"data":{},"error":null}'
37+
;;
38+
esac
2939
`)
3040
writeExecutable(t, readyBotPath, `#!/usr/bin/env bash
3141
set -euo pipefail
@@ -104,7 +114,17 @@ func TestOpenClawDXAssistants_ProbeAggregatesReadiness(t *testing.T) {
104114

105115
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
106116
set -euo pipefail
107-
printf '%s' '{"ok":true,"data":{},"error":null}'
117+
if [[ "${1:-}" == "--json" ]]; then
118+
shift
119+
fi
120+
case "${1:-} ${2:-}" in
121+
"workspace list")
122+
printf '%s' '{"ok":true,"data":[{"id":"ws-1","name":"demo","repo":"/tmp/demo","assistant":"codex"}],"error":null}'
123+
;;
124+
*)
125+
printf '%s' '{"ok":true,"data":{},"error":null}'
126+
;;
127+
esac
108128
`)
109129
writeExecutable(t, passBotPath, `#!/usr/bin/env bash
110130
set -euo pipefail
@@ -196,113 +216,6 @@ printf '%s' '{"ok":true,"status":"needs_input","overall_status":"needs_input","s
196216
}
197217
}
198218

199-
func TestOpenClawDXAssistants_ProbePrefersProbePassedAssistant(t *testing.T) {
200-
requireBinary(t, "jq")
201-
requireBinary(t, "bash")
202-
203-
scriptPath := filepath.Join("..", "..", "skills", "amux", "scripts", "openclaw-dx.sh")
204-
fakeBinDir := t.TempDir()
205-
fakeAmuxPath := filepath.Join(fakeBinDir, "amux")
206-
fakeTurnPath := filepath.Join(fakeBinDir, "fake-turn.sh")
207-
claudeBotPath := filepath.Join(fakeBinDir, "claude-ready-bot")
208-
codexBotPath := filepath.Join(fakeBinDir, "codex-ready-bot")
209-
homeDir := t.TempDir()
210-
amuxHome := filepath.Join(homeDir, ".amux")
211-
if err := os.MkdirAll(amuxHome, 0o755); err != nil {
212-
t.Fatalf("mkdir amux home: %v", err)
213-
}
214-
configPath := filepath.Join(amuxHome, "config.json")
215-
216-
writeExecutable(t, fakeAmuxPath, `#!/usr/bin/env bash
217-
set -euo pipefail
218-
printf '%s' '{"ok":true,"data":{},"error":null}'
219-
`)
220-
writeExecutable(t, claudeBotPath, `#!/usr/bin/env bash
221-
set -euo pipefail
222-
echo ready
223-
`)
224-
writeExecutable(t, codexBotPath, `#!/usr/bin/env bash
225-
set -euo pipefail
226-
echo ready
227-
`)
228-
if err := os.WriteFile(configPath, []byte(`{
229-
"assistants": {
230-
"claude": {"command": "claude-ready-bot"},
231-
"codex": {"command": "codex-ready-bot"}
232-
}
233-
}
234-
`), 0o644); err != nil {
235-
t.Fatalf("write config: %v", err)
236-
}
237-
238-
writeExecutable(t, fakeTurnPath, `#!/usr/bin/env bash
239-
set -euo pipefail
240-
assistant=""
241-
for ((i=1; i<=$#; i++)); do
242-
if [[ "${!i}" == "--assistant" ]]; then
243-
next=$((i+1))
244-
assistant="${!next}"
245-
fi
246-
done
247-
if [[ "$assistant" == "claude" ]]; then
248-
printf '%s' '{"ok":true,"status":"needs_input","overall_status":"needs_input","summary":"Needs local permission choice."}'
249-
exit 0
250-
fi
251-
printf '%s' '{"ok":true,"status":"idle","overall_status":"completed","summary":"READY: codex can proceed."}'
252-
`)
253-
254-
env := os.Environ()
255-
env = withEnv(env, "PATH", fakeBinDir+":"+os.Getenv("PATH"))
256-
env = withEnv(env, "HOME", homeDir)
257-
env = withEnv(env, "OPENCLAW_DX_TURN_SCRIPT", fakeTurnPath)
258-
259-
payload := runScriptJSON(t, scriptPath, env,
260-
"assistants",
261-
"--workspace", "ws-1",
262-
"--probe",
263-
"--limit", "9",
264-
)
265-
266-
if got, _ := payload["command"].(string); got != "assistants" {
267-
t.Fatalf("command = %q, want %q", got, "assistants")
268-
}
269-
if got, _ := payload["status"].(string); got != "needs_input" {
270-
t.Fatalf("status = %q, want %q", got, "needs_input")
271-
}
272-
suggested, _ := payload["suggested_command"].(string)
273-
if !strings.Contains(suggested, "--assistant codex") {
274-
t.Fatalf("suggested_command = %q, want codex start recommendation", suggested)
275-
}
276-
if strings.Contains(suggested, "workflow dual") {
277-
t.Fatalf("suggested_command = %q, expected no dual workflow when claude probe needs input", suggested)
278-
}
279-
quickActions, ok := payload["quick_actions"].([]any)
280-
if !ok || len(quickActions) == 0 {
281-
t.Fatalf("quick_actions missing or empty: %#v", payload["quick_actions"])
282-
}
283-
var sawStartReady bool
284-
var sawDual bool
285-
for _, raw := range quickActions {
286-
action, ok := raw.(map[string]any)
287-
if !ok {
288-
continue
289-
}
290-
id, _ := action["id"].(string)
291-
if id == "start_ready" {
292-
sawStartReady = true
293-
}
294-
if id == "dual" {
295-
sawDual = true
296-
}
297-
}
298-
if !sawStartReady {
299-
t.Fatalf("expected start_ready quick action in %#v", quickActions)
300-
}
301-
if sawDual {
302-
t.Fatalf("did not expect dual quick action when claude probe needs input: %#v", quickActions)
303-
}
304-
}
305-
306219
func TestOpenClawDXGitShip_CommitsWorkspaceChanges(t *testing.T) {
307220
requireBinary(t, "jq")
308221
requireBinary(t, "bash")

0 commit comments

Comments
 (0)