fix stale timeout, empty tool calls and trailing-space commands

Three defects from a review of the agent loop.

A session's tool schema is built by create(), and load() then replaced the
whole object with the one on disk, so a session saved under SOLO_TIMEOUT=42
went on telling the model the 42 s limit while run() enforced whatever the
environment said now. The schema moves into mkschema() and is rebuilt after
the load, so the description and the deadline always agree.

A tool call whose arguments held no command, which a flaky router or a
truncated stream can produce, was passed to the shell as an empty string.
The shell ran it as [exit 0], the agent asked again, and a peer that always
did this pinned the loop at full speed: a mock measuring 685 requests in
five seconds of the same call. Such a call is now answered with an error
instead of a shell run, command is declared required in the schema, and
three rounds in a row that run nothing end the turn. The same mock now
stops after three requests.

dispatch() read a line without trimming it, so "/help " was taken as the
generic parameter editor and wrote a help key into the session, "/quit "
did not quit, and "/temperature 0.5   " stored trailing spaces. A line
that starts with / or ! is right-trimmed before it is routed; a task keeps
its exact text, leading space included, so the model still sees what was
typed and a leading space still keeps the line out of the history.

Eleven offline regression tests cover the three, including a mock endpoint
that sends nothing but empty tool calls. make test: 42 passed, 0 failed.

README.md

@@ -258,6 +258,11 @@ below, but it only writes a `timeout` field into the saved session, which the
 API ignores and which does not change the command timeout. Restart with the
 environment set instead.
 
+The timeout is not persisted with the session: the tool description, which
+quotes it, is rebuilt from the current environment on every start, so a session
+saved under `SOLO_TIMEOUT=42` and reopened under `SOLO_TIMEOUT=99` describes and
+enforces 99 seconds.
+
 ## Slash commands
 
 | Command            | Effect                                                          |
@@ -347,7 +352,10 @@ The loop, in order:
 
 1. `readline` reads one line. A trailing `\` continues it on the next line.
 2. `dispatch` routes the line: a slash command changes settings, `!` runs a shell
-   command locally, anything else becomes a task.
+   command locally, anything else becomes a task. The line is right-trimmed
+   first, so `/quit ` and `/help ` are still commands and a value such as
+   `/temperature 0.5   ` carries no trailing spaces; a task keeps its exact
+   text, leading space included.
 3. A task is appended to the message list as a `user` message and saved.
 4. `hc` in the forked child performs one HTTP request with libcurl, wrapping the
    transport-level result as `{"status", "success", "content"}` and writing it
@@ -383,6 +391,12 @@ The request is a plain non-streaming `POST /chat/completions` with
 
     bash(command: string)  -> combined stdout/stderr plus an exit marker
 
+`command` is required by the schema. A tool call that arrives without a usable
+one, which happens with a flaky router or a truncated stream, is answered with
+an error instead of being passed to the shell as an empty string, and three
+rounds in a row with nothing to run end the turn rather than loop on a peer that
+never sends a command.
+
 Reasoning fields are handled defensively, because providers disagree on the
 name: `reasoning` and `reasoning_content` are both read, and both are stored
 back into the transcript so that a follow-up turn can reuse them.

main.c

@@ -175,15 +175,13 @@ static void load(void) {
   json_object_put(SSN); SSN = o;
 }
 
-static void create(void) {
+/* Builds the single tool the agent exposes and stores it in the session. Kept
-  json_object *a, *o, *f, *p, *pr, *cp, *ms, *sy; char *sb, tb[512]; size_t nb;
+   apart from create() because the description quotes the timeout, so it has to
-  if(SSN) json_object_put(SSN);
+   be rebuilt after load(): a session saved under one SOLO_TIMEOUT would
-  SSN = json_object_new_object();
+   otherwise keep telling the model the old limit while run() enforced the new
-  A(SSN, "model", S(MOD));
+   one. */
-  A(SSN, "reasoning_effort", S(RSN));
+static void mkschema(void) {
-  A(SSN, "temperature", D(TEMP));
+  json_object *a, *o, *f, *p, *pr, *cp, *rq; char tb[512];
-  A(SSN, "stream", B(0));
-  A(SSN, "tool_choice", S("auto"));
   a = json_object_new_array(); o = json_object_new_object(); f = json_object_new_object();
   p = json_object_new_object(); pr = json_object_new_object(); cp = json_object_new_object();
   A(cp, "type", S("string"));
@@ -191,10 +189,23 @@ static void create(void) {
     "A command still running after %ds is killed and reports exit code 124: start long work in the background instead and poll it, e.g. \"nohup make >build.log 2>&1 &\".", TO);
   A(cp, "description", S(tb));
   A(pr, "command", cp);
-  A(p, "type", S("object")); A(p, "properties", pr); A(p, "required", json_object_new_array());
+  rq = json_object_new_array(); json_object_array_add(rq, S("command")); /* command is required, so an endpoint cannot quietly send an empty call */
+  A(p, "type", S("object")); A(p, "properties", pr); A(p, "required", rq);
   A(f, "name", S("bash")); A(f, "description", S("Run a shell command")); A(f, "parameters", p);
   A(o, "type", S("function")); A(o, "function", f);
   json_object_array_add(a, o); A(SSN, "tools", a);
+}
+
+static void create(void) {
+  json_object *ms, *sy; char *sb; size_t nb;
+  if(SSN) json_object_put(SSN);
+  SSN = json_object_new_object();
+  A(SSN, "model", S(MOD));
+  A(SSN, "reasoning_effort", S(RSN));
+  A(SSN, "temperature", D(TEMP));
+  A(SSN, "stream", B(0));
+  A(SSN, "tool_choice", S("auto"));
+  mkschema();
   ms = json_object_new_array(); sy = json_object_new_object();
   A(sy, "role", S("system"));
   if(SYS && *SYS) A(sy, "content", S(SYS));
@@ -224,16 +235,28 @@ static void create(void) {
   json_object_array_add(ms, sy); A(SSN, "messages", ms);
 }
 
-static int tools(json_object *tc) {
+/* ran is set to the number of commands actually executed, so the caller can see
+   a round that ran nothing at all and stop a peer that keeps sending empty
+   tool calls instead of looping on it forever. */
+static int tools(json_object *tc, int *ran) {
   json_object *fn, *ar, *ms, *tm; const char *id, *cs, *s; char *cmd, *out, *cb, mk[80]; int i, rc; size_t l;
+  *ran = 0;
   if(!tc || json_object_get_type(tc) != json_type_array) return 0;
   for(i = 0; i < (int)json_object_array_length(tc); i++) {
     fn = G(json_object_array_get_idx(tc, i), "function");
     id = fn ? GS(json_object_array_get_idx(tc, i), "id") : 0;
-    s = fn ? GS(fn, "arguments") : 0; cmd = strdup("");
+    s = fn ? GS(fn, "arguments") : 0; cmd = 0;
-    if(s) { ar = json_tokener_parse(s); cs = ar ? GS(ar, "command") : 0; if(cs) { free(cmd); cmd = strdup(cs); } if(ar) json_object_put(ar); } /* copy before the put: cmd must outlive ar */
+    if(s) { ar = json_tokener_parse(s); cs = ar ? GS(ar, "command") : 0; if(cs) cmd = strdup(cs); if(ar) json_object_put(ar); } /* copy before the put: cmd must outlive ar */
+    if(cmd) { char *q = cmd; while(*q && isspace((unsigned char)*q)) q++; if(!*q) { free(cmd); cmd = 0; } } /* a blank command is no command */
+    if(!cmd) {
+      printf("%s--- tool %s: no command in the arguments%s\n", CLR ? YEL : "", id ? id : "", CLR ? OF : "");
+      out = strdup("error: this tool call carried no usable \"command\" argument, so nothing ran. Call bash again with a command, e.g. {\"command\": \"ls -l\"}.");
+      rc = 1;
+    } else {
       printf("%s--- tool %s: $ %s%s\n", CLR ? YEL : "", id ? id : "", cmd, CLR ? OF : ""); fflush(stdout);
       run(cmd, &out, &rc);
+      (*ran)++;
+    }
     printf("%s%s%s\n", CLR ? GRY DIM : "", out, CLR ? OF : ""); fflush(stdout);
     ms = G(SSN, "messages");
     if(ms) {
@@ -252,6 +275,7 @@ static int tools(json_object *tc) {
 
 static int ask(const char *in) {
   json_object *ms, *d, *p, *ch, *m, *as, *tc, *u; const char *raw, *co, *rs, *rc2, *mn;
+  int empty = 0;
   if(!(ms = G(SSN, "messages"))) return 1;
   as = json_object_new_object(); A(as, "role", S("user")); A(as, "content", S(in));
   json_object_array_add(ms, as); sv();
@@ -282,10 +306,16 @@ static int ask(const char *in) {
     if(tc && json_object_get_type(tc) == json_type_array) A(as, "tool_calls", json_object_get(tc)); /* json_object_object_add MOVES the ref and does not refcount: take our own ref or json_object_put(p) below frees a node still living in SSN */
     json_object_array_add(ms, as); sv();
     if(tc && json_object_get_type(tc) == json_type_array && json_object_array_length(tc)) {
-      int r = tools(tc); /* the usage line waits for the turn that ends the loop, so a tool run is not chopped up by stats */
+      int r, ran; /* the usage line waits for the turn that ends the loop, so a tool run is not chopped up by stats */
+      r = tools(tc, &ran);
       if(p) json_object_put(p);
       if(d) json_object_put(d);
       if(r) return r;
+      if(ran) empty = 0;
+      else if(++empty >= 3) { /* three rounds in a row with nothing to run: feed the errors back was useless, so give up rather than spin */
+        puts("no command in three consecutive tool rounds, stopping the turn");
+        return 1;
+      }
       continue;
     }
     u = p ? G(p, "usage") : 0; mn = p ? GS(p, "model") : 0;
@@ -312,7 +342,12 @@ static const char *HELP =
 
 static void setstr(char **slot, const char *v) { free(*slot); *slot = strdup(v); }
 
-static int dispatch(const char *l) { /* runs one line and returns the exit status for it */
+static void rtrim(char *s) { size_t n = strlen(s); while(n && isspace((unsigned char)s[n - 1])) s[--n] = 0; }
+
+/* Runs the command lines (a leading / or !) of one already right-trimmed line.
+   The trim happens in dispatch(), because "/help " and "/quit " must stay
+   commands rather than being read as a parameter named help or quit. */
+static int command(char *l) {
   char *k, *v, *out, mk[80]; const char *s; json_object *ms, *nm; int rc;
   if(!strcmp(l, "/help")) { puts(HELP); return 0; }
   if(!strcmp(l, "/quit")) { QUIT = 1; return 0; }
@@ -334,19 +369,19 @@ static int dispatch(const char *l) { /* runs one line and returns the exit statu
     return 0;
   }
   if(!strncmp(l, "/api_key", 8)) {
-    for(v = (char *)l + 8; *v == ' '; v++);
+    for(v = l + 8; *v == ' '; v++);
     if(*v && (!strcmp(v, "null") || !strcmp(v, "none") || !strcmp(v, "off") || !strcmp(v, "-"))) { free(KEY); KEY = 0; }
     else if(*v) setstr(&KEY, v);
     if(KEY) printf("api_key = set, %d chars\n", (int)strlen(KEY)); else puts("api_key = unset");
     return 0;
   }
   if(!strncmp(l, "/endpoint", 9)) {
-    for(v = (char *)l + 9; *v == ' '; v++);
+    for(v = l + 9; *v == ' '; v++);
     if(*v) { setstr(&EP, v); for(char *e = EP + strlen(EP); e > EP && e[-1] == '/'; e--) *e = 0; }
     printf("endpoint = %s\n", EP); return 0;
   }
   if(*l == '!') {
-    for(v = (char *)l + 1; *v == ' '; v++);
+    for(v = l + 1; *v == ' '; v++);
     printf("%s--- $ %s%s\n", CLR ? YEL : "", v, CLR ? OF : "");
     run(v, &out, &rc);
     if(*out) printf("%s%s%s\n", CLR ? GRY DIM : "", out, CLR ? OF : "");
@@ -382,7 +417,16 @@ static int dispatch(const char *l) { /* runs one line and returns the exit statu
     }
     free(k); return 0;
   }
-  return ask(l);
+  return 0;
+}
+
+/* Runs one line and returns the exit status for it. A task keeps its exact
+   text, including a leading space that keeps it out of the history. */
+static int dispatch(const char *l) {
+  char *cl; int rc;
+  if(*l != '/' && *l != '!') return ask(l);
+  cl = strdup(l); rtrim(cl); rc = command(cl); free(cl);
+  return rc;
 }
 
 int main(int argc, char **argv) {
@@ -413,7 +457,7 @@ int main(int argc, char **argv) {
   snprintf(SH, sizeof SH, "%s", e);
   signal(SIGINT, onint); signal(SIGPIPE, SIG_IGN);
   rl_signal_event_hook = ci_hook;
-  create(); load();
+  create(); load(); mkschema(); /* a restored session carries the schema of the run that saved it: rebuild it, so the timeout in the description matches the one run() enforces */
   if((e = getenv("SOLO_MODEL")) && *e) { setstr(&MOD, e); A(SSN, "model", S(e)); }
   if((e = getenv("SOLO_TEMP")) && *e) { TEMP = atof(e); A(SSN, "temperature", D(TEMP)); }
   if((e = getenv("SOLO_REASON")) && *e) { setstr(&RSN, e); A(SSN, "reasoning_effort", S(e)); }

test.sh

@@ -282,5 +282,88 @@ ok $? 'the subagent line names the endpoint and Solo own path'
 if grep -q 'api///' "$D/.solo_sess.json"; then rc=1; else rc=0; fi
 ok "$rc" 'a trailing slash on SOLO_ENDPOINT is stripped'
 
+# --- a command line keeps its meaning when it ends in spaces -------------------
+# dispatch() right-trims before it looks at the line, so "/help " and "/quit "
+# stay commands instead of being read as a parameter named help or quit, and a
+# value such as "/temperature 0.5   " does not gain trailing spaces. A task, on
+# the other hand, keeps every character, because the model should see what was
+# typed and a leading space still keeps the line out of the history.
+D=$WORK/trim
+printf '/help \n/quit \n' | solo_in "$D" 2>&1 | grep -q '^Commands:'
+ok $? '/help with a trailing space prints help'
+if grep -qE '"(help|quit)"' "$D/.solo_sess.json"; then rc=1; else rc=0; fi
+ok "$rc" '"/help " and "/quit " do not become session parameters'
+if printf '/quit \n!echo SURVIVED\n' | solo_in "$D" 2>&1 | grep -q SURVIVED; then rc=1; else rc=0; fi
+ok "$rc" '/quit with a trailing space exits instead of being ignored'
+D=$WORK/trimval
+printf '/temperature 0.5   \n/quit\n' | solo_in "$D" >/dev/null 2>&1
+perl -0777 -ne 'exit(/"temperature":0\.5,/ ? 0 : 1)' "$D/.solo_sess.json"
+ok $? 'a trailing space does not land in a parameter value'
+
+# --- a resumed session must not keep the old timeout in the tool description ---
+# create() builds the schema, then load() replaced the whole object with the one
+# on disk, so a session saved under SOLO_TIMEOUT=42 went on describing 42 s while
+# run() enforced whatever the environment said now. The schema is rebuilt after
+# the load, so the two always agree.
+D=$WORK/timeoutdesc
+printf '/quit\n' | solo_in "$D" SOLO_TIMEOUT=42 >/dev/null 2>&1
+perl -0777 -ne 'exit(/after 42s is killed/ ? 0 : 1)' "$D/.solo_sess.json"
+ok $? 'SOLO_TIMEOUT=42 reaches the tool description'
+printf '/quit\n' | solo_in "$D" SOLO_TIMEOUT=99 >/dev/null 2>&1
+perl -0777 -ne 'exit(/after 99s is killed/ ? 0 : 1)' "$D/.solo_sess.json"
+ok $? 'a rerun with another SOLO_TIMEOUT rebuilds the description'
+if grep -q 'after 42s' "$D/.solo_sess.json"; then rc=1; else rc=0; fi
+ok "$rc" 'the stale timeout is gone from the resumed session'
+perl -0777 -ne 'exit(/"required":\s*\[\s*"command"/s ? 0 : 1)' "$D/.solo_sess.json"
+ok $? 'the bash tool declares command as required'
+
+# --- an empty tool call must not spin the agent -------------------------------
+# A peer that answers with a function call whose arguments hold no command used
+# to be fed an empty string, which the shell ran as [exit 0]: the agent then
+# asked again, forever. Such a call is now answered with an error, and three
+# rounds in a row with nothing to run end the turn instead of the loop.
+cat >"$WORK/emptycmd.pl" <<'EMPTY_EOF'
+use IO::Socket::INET;
+my $srv = IO::Socket::INET->new(LocalAddr => '127.0.0.1', LocalPort => 0,
+                                Listen => 5, ReuseAddr => 1) or exit 1;
+$| = 1;
+print $srv->sockport, "\n";
+my $n = 0;
+while(my $c = $srv->accept) {
+  $c->autoflush(1);
+  my $req = '';
+  while(my $l = <$c>) { last if $l =~ /^\r?\n$/; $req .= $l; }
+  print $c "HTTP/1.0 100 Continue\r\n\r\n" if $req =~ /^Expect:/mi;
+  my ($len) = $req =~ /Content-Length:\s*(\d+)/i;
+  my $body = ''; read($c, $body, $len) if $len;
+  $n++;
+  open(my $f, '>>', $ARGV[0]); print $f "1\n"; close $f;
+  my $msg = '{"role":"assistant","content":"c","tool_calls":[{"id":"a","type":"function",'
+    . '"function":{"name":"bash","arguments":"not json"}}]}';
+  my $out = '{"model":"mock/free","choices":[{"message":' . $msg . '}]}';
+  print $c "HTTP/1.0 200 OK\r\nContent-Type: application/json\r\nContent-Length: "
+    . length($out) . "\r\n\r\n" . $out;
+  close $c;
+}
+EMPTY_EOF
+COUNT=$WORK/emptycmd.count
+: >"$COUNT"
+perl "$WORK/emptycmd.pl" "$COUNT" >"$WORK/emptycmd.port" 2>/dev/null &
+EMPTY=$!
+i=0
+while [ ! -s "$WORK/emptycmd.port" ] && [ "$i" -lt 50 ]; do sleep 0.1; i=$((i + 1)); done
+EPORT=$(cat "$WORK/emptycmd.port" 2>/dev/null)
+mkdir -p "$WORK/emptycmd"
+( cd "$WORK/emptycmd" && SOLO_ENDPOINT="http://127.0.0.1:$EPORT/api" "$SOLO" 'do it' ) \
+  >"$WORK/emptycmd.out" 2>"$WORK/emptycmd.err"
+ERC=$?
+kill "$EMPTY" 2>/dev/null
+[ "$ERC" -ne 124 ]
+ok $? 'a peer sending only empty tool calls does not hang the turn' "exit $ERC"
+[ "$(wc -l <"$COUNT")" -le 4 ]
+ok $? 'the empty-call loop is cut off after a few rounds' "$(wc -l <"$COUNT") request(s)"
+grep -q 'carried no usable "command" argument' "$WORK/emptycmd.out"
+ok $? 'the empty tool call is answered with an error, not a shell run'
+
 printf '%d passed, %d failed\n' "$PASS" "$FAIL"
 [ "$FAIL" -eq 0 ]