Compare commits

...
Author SHA1 Message Date
Vasanthdev2004 772419f8ba test(tools): cover the explicit session_id: nil case for write_stdin 2026-07-20 13:41:04 +05:30
Vasanthdev2004 5476250061 fix(tools): give write_stdin's invalid-session errors the same recovery guidance
Follow-up to #749 (write_stdin session-id probing) / #702.

write_stdin had three "no valid live session" error paths with three different
messages and signatures: a missing session_id returned "session_id is required",
a zero/negative/non-integer one returned "session_id must be at least 1", and
only a valid-but-unknown id got the rich UnknownExecSessionError recovery guidance
("do not guess or probe; start a session with exec_command, or use edit_file/
write_file"). So a model that called write_stdin with session_id 0 (the common
"I have no id" placeholder) got a terse error with no way forward, and naming the
minimum nudged it toward trying 1, 2, 3... the exact id-probing #749 suppresses.
The three signatures also meant a model moving 0 to 1 to 2 reset the
repeated-failure streak on each class of mistake.

Route the missing / non-integer / <1 cases through the same UnknownExecSessionError
message. Now every "no live session" mistake gives the same recovery guidance and,
because that message is id-invariant, shares one repeated-failure signature, so any
mix of missing/zero/probing accumulates toward the halt.

Updates TestWriteStdinRequiresPositiveSessionID to cover missing/zero/negative/
non-integer and assert the recovery message. Guardrail signature/halt tests
unchanged.
2026-07-20 13:15:15 +05:30
2 changed files with 40 additions and 16 deletions

No files matched your search

+14 -6
View File
@@ -775,12 +775,20 @@ func (tool writeStdinTool) Run(ctx context.Context, args map[string]any) Result
}
func (tool writeStdinTool) RunWithOptions(ctx context.Context, args map[string]any, _ RunOptions) Result {
if value, ok := args["session_id"]; !ok || value == nil {
return errorResult("Error: Invalid arguments for write_stdin: session_id is required")
}
sessionID, err := intArg(args, "session_id", 0, 1, 0)
if err != nil {
return errorResult("Error: Invalid arguments for write_stdin: " + err.Error())
// A missing, non-integer, or < 1 session_id all mean the same thing: the model
// has no live session to write to. Route them to the SAME recovery guidance the
// no-live-session case uses (UnknownExecSessionError) instead of a terse
// "session_id must be at least 1". The terse error gives no way forward and, by
// naming the minimum, nudges the model to try 1, 2, 3... — the exact id-probing
// #749 works to suppress. The recovery message instead tells it to start a
// session or edit files directly, and because that message is id-invariant, the
// missing/zero/non-integer entry points and id-probing collapse to ONE
// repeated-failure signature, so any mix of them accumulates toward the halt
// rather than resetting the streak on each class of mistake.
value, present := args["session_id"]
sessionID, sessionErr := intArg(args, "session_id", 0, 1, 0)
if !present || value == nil || sessionErr != nil {
return errorResult(UnknownExecSessionError(sessionID))
}
chars, err := stringArgWithEmpty(args, "chars", "", false, true)
if err != nil {
+26 -10
View File
@@ -923,19 +923,35 @@ func TestWriteStdinSchemaPinsSessionIDMinimum(t *testing.T) {
}
}
// A missing, zero, negative, or non-integer session_id all mean the model has no
// live session, so write_stdin returns the SAME recovery guidance the no-live-
// session case uses (start a session with exec_command, or edit files directly),
// not a terse "session_id must be at least 1" that gives no way forward and names
// a minimum that nudges the model to probe ids 1, 2, 3... Sharing one id-invariant
// message also keeps a single repeated-failure signature, so any mix of these
// mistakes accumulates toward the halt instead of resetting the streak (#749).
func TestWriteStdinRequiresPositiveSessionID(t *testing.T) {
tool := NewWriteStdinTool(newExecSessionManager())
for _, args := range []map[string]any{
{},
{"session_id": 0},
want := UnknownExecSessionError(0)
for name, args := range map[string]map[string]any{
"missing": {},
"nil": {"session_id": nil},
"zero": {"session_id": 0},
"negative": {"session_id": -3},
"non-integer": {"session_id": "abc"},
} {
result := tool.Run(context.Background(), args)
if result.Status != StatusError {
t.Fatalf("Run(%#v) status = %s, want error", args, result.Status)
}
if !strings.Contains(result.Output, "Invalid arguments for write_stdin") {
t.Fatalf("Run(%#v) output = %q, want invalid arguments", args, result.Output)
}
t.Run(name, func(t *testing.T) {
result := tool.Run(context.Background(), args)
if result.Status != StatusError {
t.Fatalf("status = %s, want error", result.Status)
}
if result.Output != want {
t.Fatalf("output = %q,\n want the recovery message %q", result.Output, want)
}
if strings.Contains(result.Output, "must be at least 1") {
t.Fatalf("still returns the terse minimum error: %q", result.Output)
}
})
}
}