Skip to content

Commit b881c50

Browse files
RonCodes88RonCodes88
authored
Merge pull request #73 from openagentlock/fix/post-tool-use-verdict
fix(daemon): record successful tool calls as ran instead of errored
2 parents 649ba9b + 05172c8 commit b881c50

9 files changed

Lines changed: 189 additions & 85 deletions

File tree

‎control-plane/internal/api/hooks_claude.go‎

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -148,9 +148,13 @@ type claudePostToolInput struct {
148148
ToolUseID string `json:"tool_use_id"`
149149
// Claude Code populates `tool_response` with whatever the tool
150150
// returned. We don't mirror the full payload into the ledger
151-
// (potential secret-leak), just its success flag + a size hint.
152-
ToolResponse any `json:"tool_response"`
153-
Success bool `json:"success"`
151+
// (potential secret-leak), just a derived success flag + a size
152+
// hint. The success flag is derived from `tool_response.is_error`
153+
// because Claude Code's hook spec doesn't emit a top-level
154+
// success bool — a stray `Success bool json:"success"` field here
155+
// used to default to false on every call and tag every completed
156+
// tool as a failure in the ledger.
157+
ToolResponse any `json:"tool_response"`
154158
}
155159

156160
// claudePostToolUseHandler records the completion outcome of each tool
@@ -189,21 +193,14 @@ func claudePostToolUseHandler(d Deps) http.HandlerFunc {
189193
// to distinguish "tool ran" from "tool timed out returning
190194
// nothing" without risking that a curl response with a secret
191195
// in it lands in the Merkle-hashed payload.
192-
respSize := 0
193-
if in.ToolResponse != nil {
194-
if s, ok := in.ToolResponse.(string); ok {
195-
respSize = len(s)
196-
} else if b, mErr := json.Marshal(in.ToolResponse); mErr == nil {
197-
respSize = len(b)
198-
}
199-
}
196+
respSize, success := summarizeToolResponse(in.ToolResponse)
200197

201198
toolUseID := in.ToolUseID
202199
if toolUseID == "" {
203200
toolUseID = "claude.post-tool-use"
204201
}
205202
verdict := "complete"
206-
if !in.Success {
203+
if !success {
207204
verdict = "failure"
208205
}
209206

@@ -213,7 +210,7 @@ func claudePostToolUseHandler(d Deps) http.HandlerFunc {
213210
"tool": in.ToolName,
214211
"tool_use_id": toolUseID,
215212
"response_size": respSize,
216-
"success": in.Success,
213+
"success": success,
217214
})
218215
if err != nil {
219216
log.Printf("claude post-tool-use: marshal: %v", err)

‎control-plane/internal/api/hooks_claude_test.go‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,14 +167,16 @@ func TestClaudeStop_EndsSession(t *testing.T) {
167167

168168
func TestClaudePostToolUse_RecordsOutcome(t *testing.T) {
169169
fx := newGateFixture(t, enforcePolicyYAML)
170+
// tool_response as a plain string is the success shape Claude Code
171+
// emits for read-style tools and well-behaved Bash. No is_error
172+
// member, no error member -> verdict=complete.
170173
body := `{
171174
"session_id": "claude-post-001",
172175
"hook_event_name": "PostToolUse",
173176
"tool_name": "Bash",
174177
"tool_use_id": "toolu_post_001",
175178
"tool_input": {"command": "ls"},
176-
"tool_response": "total 0",
177-
"success": true
179+
"tool_response": "total 0"
178180
}`
179181
res, err := http.Post(
180182
fx.srv.URL+"/v1/hooks/claude-code/post-tool-use",
@@ -206,14 +208,15 @@ func TestClaudePostToolUse_RecordsOutcome(t *testing.T) {
206208

207209
func TestClaudePostToolUse_RecordsFailure(t *testing.T) {
208210
fx := newGateFixture(t, enforcePolicyYAML)
211+
// Anthropic-canonical tool_result failure shape: is_error: true
212+
// inside tool_response. The handler reads it via summarizeToolResponse.
209213
body := `{
210214
"session_id": "claude-post-002",
211215
"hook_event_name": "PostToolUse",
212216
"tool_name": "Bash",
213217
"tool_use_id": "toolu_post_002",
214218
"tool_input": {"command": "false"},
215-
"tool_response": "",
216-
"success": false
219+
"tool_response": {"content": "exit 1", "is_error": true}
217220
}`
218221
res, err := http.Post(
219222
fx.srv.URL+"/v1/hooks/claude-code/post-tool-use",

‎control-plane/internal/api/hooks_codex.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,10 @@ type codexPostToolInput struct {
136136
ToolInput map[string]any `json:"tool_input"`
137137
ToolUseID string `json:"tool_use_id"`
138138
TurnID string `json:"turn_id"`
139-
ToolResponse any `json:"tool_response"`
140-
Success bool `json:"success"`
139+
// Codex hook payloads don't carry a reliable top-level success
140+
// boolean; we derive it from tool_response so the ledger reflects
141+
// what the tool actually did. See hooks_common.go.
142+
ToolResponse any `json:"tool_response"`
141143
}
142144

143145
func codexPostToolUseHandler(d Deps) http.HandlerFunc {
@@ -162,21 +164,14 @@ func codexPostToolUseHandler(d Deps) http.HandlerFunc {
162164
return
163165
}
164166

165-
respSize := 0
166-
if in.ToolResponse != nil {
167-
if s, ok := in.ToolResponse.(string); ok {
168-
respSize = len(s)
169-
} else if b, mErr := json.Marshal(in.ToolResponse); mErr == nil {
170-
respSize = len(b)
171-
}
172-
}
167+
respSize, success := summarizeToolResponse(in.ToolResponse)
173168

174169
toolUseID := in.ToolUseID
175170
if toolUseID == "" {
176171
toolUseID = "codex.post-tool-use"
177172
}
178173
verdict := "complete"
179-
if !in.Success {
174+
if !success {
180175
verdict = "failure"
181176
}
182177

@@ -187,7 +182,7 @@ func codexPostToolUseHandler(d Deps) http.HandlerFunc {
187182
"tool_use_id": toolUseID,
188183
"turn_id": in.TurnID,
189184
"response_size": respSize,
190-
"success": in.Success,
185+
"success": success,
191186
})
192187
if err != nil {
193188
log.Printf("codex post-tool-use: marshal: %v", err)

‎control-plane/internal/api/hooks_codex_test.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,8 +190,7 @@ func TestCodexPostToolUse_RecordsOutcome(t *testing.T) {
190190
"tool_use_id": "t_post_001",
191191
"turn_id": "turn_p1",
192192
"tool_input": {"command": "ls"},
193-
"tool_response": "total 0",
194-
"success": true
193+
"tool_response": "total 0"
195194
}`
196195
res, err := http.Post(
197196
fx.srv.URL+"/v1/hooks/codex/post-tool-use",

‎control-plane/internal/api/hooks_common.go‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
// Helpers shared across the per-harness PostToolUse handlers.
2+
//
3+
// Each harness (Claude Code, Codex, Cursor, Gemini) emits a `tool_response`
4+
// blob whose shape varies by tool. Different harness specs encode failure
5+
// differently — Anthropic's canonical tool_result uses `is_error: true`,
6+
// Gemini and some Codex flows use a non-empty `error` member. None of the
7+
// harnesses emit a reliable top-level `success` boolean (the daemon used
8+
// to read one anyway, which left every Claude tool call mis-labeled as a
9+
// failure). summarizeToolResponse derives the success flag from the actual
10+
// payload so the ledger reflects what the tool did, not what the harness
11+
// did or didn't bother to forward.
12+
13+
package api
14+
15+
import "encoding/json"
16+
17+
// summarizeToolResponse returns (response_size, success) for a heterogeneous
18+
// PostToolUse tool_response payload. Failure signals (any one is enough):
19+
//
20+
// is_error: true — Anthropic canonical (Claude Code)
21+
// error: <non-empty> — Gemini / some Codex tools
22+
//
23+
// Anything else is treated as a successful run. We never hash the response
24+
// body itself; only its byte length feeds the Merkle chain, so large
25+
// outputs and any secrets they might contain stay out of the audit log.
26+
func summarizeToolResponse(resp any) (int, bool) {
27+
if resp == nil {
28+
return 0, true
29+
}
30+
if s, ok := resp.(string); ok {
31+
return len(s), true
32+
}
33+
if m, ok := resp.(map[string]any); ok {
34+
size := 0
35+
if b, err := json.Marshal(m); err == nil {
36+
size = len(b)
37+
}
38+
if v, ok := m["is_error"].(bool); ok && v {
39+
return size, false
40+
}
41+
if errVal, present := m["error"]; present {
42+
switch v := errVal.(type) {
43+
case nil:
44+
case string:
45+
if v != "" {
46+
return size, false
47+
}
48+
case bool:
49+
if v {
50+
return size, false
51+
}
52+
default:
53+
return size, false
54+
}
55+
}
56+
return size, true
57+
}
58+
if b, err := json.Marshal(resp); err == nil {
59+
return len(b), true
60+
}
61+
return 0, true
62+
}

‎control-plane/internal/api/hooks_common_test.go‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package api
2+
3+
import (
4+
"encoding/json"
5+
"testing"
6+
)
7+
8+
func TestSummarizeToolResponse(t *testing.T) {
9+
cases := []struct {
10+
name string
11+
raw string
12+
wantSize int
13+
wantSuccess bool
14+
}{
15+
{name: "nil", raw: `null`, wantSize: 0, wantSuccess: true},
16+
{name: "empty string", raw: `""`, wantSize: 0, wantSuccess: true},
17+
{name: "non-empty string", raw: `"total 0"`, wantSize: 7, wantSuccess: true},
18+
{
19+
name: "anthropic tool_result is_error true",
20+
raw: `{"content": "ENOENT", "is_error": true}`,
21+
wantSize: -1,
22+
wantSuccess: false,
23+
},
24+
{
25+
name: "anthropic tool_result is_error false",
26+
raw: `{"content": "ok", "is_error": false}`,
27+
wantSize: -1,
28+
wantSuccess: true,
29+
},
30+
{
31+
name: "gemini error string non-empty",
32+
raw: `{"error": "boom"}`,
33+
wantSize: -1,
34+
wantSuccess: false,
35+
},
36+
{
37+
name: "gemini error string empty",
38+
raw: `{"error": ""}`,
39+
wantSize: -1,
40+
wantSuccess: true,
41+
},
42+
{
43+
name: "gemini error nested object",
44+
raw: `{"error": {"code": 500}}`,
45+
wantSize: -1,
46+
wantSuccess: false,
47+
},
48+
{
49+
name: "gemini error null is treated as no error",
50+
raw: `{"error": null}`,
51+
wantSize: -1,
52+
wantSuccess: true,
53+
},
54+
{
55+
name: "ordinary object with no failure marker",
56+
raw: `{"stdout": "hi", "exit_code": 0}`,
57+
wantSize: -1,
58+
wantSuccess: true,
59+
},
60+
{
61+
name: "array payload",
62+
raw: `[1, 2, 3]`,
63+
wantSize: -1,
64+
wantSuccess: true,
65+
},
66+
}
67+
for _, tc := range cases {
68+
t.Run(tc.name, func(t *testing.T) {
69+
var resp any
70+
if err := json.Unmarshal([]byte(tc.raw), &resp); err != nil {
71+
t.Fatalf("unmarshal: %v", err)
72+
}
73+
gotSize, gotSuccess := summarizeToolResponse(resp)
74+
if gotSuccess != tc.wantSuccess {
75+
t.Fatalf("success: got %v, want %v", gotSuccess, tc.wantSuccess)
76+
}
77+
// wantSize == -1 means "size depends on JSON marshalling and
78+
// isn't worth pinning; just confirm it's non-zero".
79+
switch tc.wantSize {
80+
case -1:
81+
if gotSize <= 0 {
82+
t.Fatalf("size: got %d, want > 0", gotSize)
83+
}
84+
default:
85+
if gotSize != tc.wantSize {
86+
t.Fatalf("size: got %d, want %d", gotSize, tc.wantSize)
87+
}
88+
}
89+
})
90+
}
91+
}

‎control-plane/internal/api/hooks_cursor.go‎

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -146,10 +146,12 @@ type cursorPostToolInput struct {
146146
ToolName string `json:"tool_name"`
147147
ToolInput map[string]any `json:"tool_input"`
148148
ToolUseID string `json:"tool_use_id"`
149-
ToolResponse any `json:"tool_response"`
150-
Success bool `json:"success"`
151-
MCPServerName string `json:"mcp_server_name,omitempty"`
152-
MCPToolName string `json:"mcp_tool_name,omitempty"`
149+
// Cursor hook payloads don't carry a reliable top-level success
150+
// boolean; we derive it from tool_response so the ledger reflects
151+
// what the tool actually did. See hooks_common.go.
152+
ToolResponse any `json:"tool_response"`
153+
MCPServerName string `json:"mcp_server_name,omitempty"`
154+
MCPToolName string `json:"mcp_tool_name,omitempty"`
153155
}
154156

155157
type cursorSessionStartInput struct {
@@ -331,21 +333,14 @@ func cursorOutcomeHandler(d Deps, eventName string) http.HandlerFunc {
331333
return
332334
}
333335

334-
respSize := 0
335-
if in.ToolResponse != nil {
336-
if s, ok := in.ToolResponse.(string); ok {
337-
respSize = len(s)
338-
} else if b, mErr := json.Marshal(in.ToolResponse); mErr == nil {
339-
respSize = len(b)
340-
}
341-
}
336+
respSize, success := summarizeToolResponse(in.ToolResponse)
342337

343338
toolUseID := in.ToolUseID
344339
if toolUseID == "" {
345340
toolUseID = "cursor.post-tool-use"
346341
}
347342
verdict := "complete"
348-
if !in.Success {
343+
if !success {
349344
verdict = "failure"
350345
}
351346

@@ -356,7 +351,7 @@ func cursorOutcomeHandler(d Deps, eventName string) http.HandlerFunc {
356351
"tool_use_id": toolUseID,
357352
"generation_id": in.GenerationID,
358353
"response_size": respSize,
359-
"success": in.Success,
354+
"success": success,
360355
"mcp_server_name": in.MCPServerName,
361356
"mcp_tool_name": in.MCPToolName,
362357
})

‎control-plane/internal/api/hooks_cursor_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,6 @@ func TestCursorAfterMCPExecution_DedupesAgainstPostToolUse(t *testing.T) {
240240
"tool_use_id": "ct_mcp_post_1",
241241
"tool_input": {"foo": "bar"},
242242
"tool_response": "ok",
243-
"success": true,
244243
"mcp_server_name": "server",
245244
"mcp_tool_name": "tool"
246245
}`
@@ -305,8 +304,7 @@ func TestCursorPostToolUse_RecordsOutcome(t *testing.T) {
305304
"tool_name": "Shell",
306305
"tool_use_id": "ct_post_001",
307306
"tool_input": {"command": "ls"},
308-
"tool_response": "total 0",
309-
"success": true
307+
"tool_response": "total 0"
310308
}`
311309
res, err := http.Post(
312310
fx.srv.URL+"/v1/hooks/cursor/post-tool-use",

0 commit comments

Comments
 (0)