diff --git a/README.md b/README.md index 40d9444..5ac9ad7 100644 --- a/README.md +++ b/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. diff --git a/main.c b/main.c index 7d50ef0..1cfc2f2 100644 --- a/main.c +++ b/main.c @@ -175,15 +175,13 @@ static void load(void) { json_object_put(SSN); SSN = o; } -static void create(void) { - json_object *a, *o, *f, *p, *pr, *cp, *ms, *sy; char *sb, tb[512]; 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")); +/* Builds the single tool the agent exposes and stores it in the session. Kept + apart from create() because the description quotes the timeout, so it has to + be rebuilt after load(): a session saved under one SOLO_TIMEOUT would + otherwise keep telling the model the old limit while run() enforced the new + one. */ +static void mkschema(void) { + json_object *a, *o, *f, *p, *pr, *cp, *rq; char tb[512]; 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(""); - 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 */ - printf("%s--- tool %s: $ %s%s\n", CLR ? YEL : "", id ? id : "", cmd, CLR ? OF : ""); fflush(stdout); - run(cmd, &out, &rc); + s = fn ? GS(fn, "arguments") : 0; cmd = 0; + 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)); } diff --git a/test.sh b/test.sh index 6f88f3a..e5bada5 100755 --- a/test.sh +++ b/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 ]