fix(tools): give write_stdin's invalid-session errors the same recovery guidance (#749 follow-up) (#768)

* 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.

* test(tools): cover the explicit session_id: nil case for write_stdin

Author: Vasanthdev2004

internal/tools/exec_command.go

@@ -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 {

internal/tools/exec_command_test.go

@@ -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)
+			}
+		})
 	}
 }