diff --git a/.agents/skills/fleet-queue/SKILL.md b/.agents/skills/fleet-queue/SKILL.md index 4057953..f41d7df 100644 --- a/.agents/skills/fleet-queue/SKILL.md +++ b/.agents/skills/fleet-queue/SKILL.md @@ -89,6 +89,15 @@ thurbox's own exit status, a task left `queued`, and a hand-edited `task.yaml`. A repo this machine cannot read is not asked, so a `--host` task still finds out at dispatch. +**`--title` becomes the worker's session NAME.** thurbox makes a path segment +of that name, so it refuses one carrying `/`, `\` or `..`, one starting `.`, +and one over its 64-byte cap β€” `Rust crate, CI/CD and the profile model` is a +title `add` used to take and `dispatch` could never spawn. `add` refuses it +here for the same reason as `--branch`: the repair afterwards is a hand-edit of +`title` in `task.yaml` and the brief's H1, because nothing retitles a task. +Every character thurbox accepts is still accepted β€” a title is human-facing +text β€” and the check is on the rendered name, glyph and cut included. + ### `--host` β€” running a task on another machine `add --host ` takes a name from thurbox's `hosts.toml` and moves the diff --git a/.agents/skills/thurbox-session/SKILL.md b/.agents/skills/thurbox-session/SKILL.md index 6b2c5f1..e9e274c 100644 --- a/.agents/skills/thurbox-session/SKILL.md +++ b/.agents/skills/thurbox-session/SKILL.md @@ -98,9 +98,9 @@ follow from how the name is used: against thurbox 2.19.5, a 61-character name wearing a 5-byte `πŸš€ ` is accepted at 64 bytes and refused at 65. So a name that fits in characters can still fail at spawn the moment it carries anything non-ASCII, and a spawn that - fails takes its whole dispatch with it. `scripts/lib/queue.py`'s - `session_name()` cuts by byte, on a codepoint boundary, for exactly that - reason. + fails leaves that task `queued` while the rest of the set still goes out. + `scripts/lib/queue.py`'s `session_name()` cuts by byte, on a codepoint + boundary, for exactly that reason. - **fleet's own workers wear a mark.** `queue.sh dispatch` puts `πŸš€ ` in front of the name it builds from the task title, under the one setting in `orchestration/session-glyphs.example.conf` that also decides the lead's. The diff --git a/scripts/lib/queue.py b/scripts/lib/queue.py index 321ef87..bd83edd 100644 --- a/scripts/lib/queue.py +++ b/scripts/lib/queue.py @@ -497,6 +497,47 @@ def session_name(title: str, glyph: str) -> str: return encoded[:SESSION_NAME_BYTES].decode(errors="ignore") +def session_name_refusal(title: str, glyph: str) -> str: + """Why `session create` would refuse this title, asked at `add` time. + + thurbox's rule MIRRORED, never re-invented and never tightened: a session + name becomes a path segment there, so `paths::validate_safe_name` refuses + an empty name, one over the byte cap, one starting `.`, and one holding + `/`, `\\` or `..` β€” the four shapes its own `unsafe_names_are_rejected` + enumerates. Everything else it accepts, and so does this: a title is + human-facing text and narrowing it further would be a defect of its own. + + Asked about the RENDERED name and not the raw title, because the rendered + name is what `dispatch` hands to `session create`: the glyph goes in front, + so a title starting `.` is unsafe exactly when no mark precedes it, and the + title is cut to the cap, so one that is merely long never reaches thurbox + long. `add` took `Rust crate, CI/CD and the profile model`; `dispatch` died + on it with thurbox's bare exit status, and the repair was a hand-edit of + `title` in task.yaml, because there is no retitle verb. + """ + name = session_name(title, glyph) + if not name: + why = "and an empty name is not one it accepts" + elif name.startswith("."): + why = "and a name beginning with '.' is not one it accepts" + else: + why = "" + for bad in ("/", "\\", ".."): + if bad in name: + why = f"and it contains {bad!r}, which thurbox refuses" + break + if not why: + return "" + return ( + f"--title {title!r} cannot become a session name:\n" + f"thurbox is asked to create {name!r}, {why}.\n" + "That name becomes a path there, so it carries no '/', no '\\', no " + "'..' and no\nleading '.'. The spawn fails with thurbox's own refusal " + "and the task stays\nqueued.\n" + "Retitle the task; nothing else about it has to change." + ) + + def queue_root() -> str: """Where the queue lives, as an absolute path. @@ -1414,6 +1455,16 @@ def cmd_add(args) -> int: if refusal: raise QueueError(refusal) + # And one layer down again: the title becomes the worker's session NAME, + # and thurbox refuses a name it could not make a path segment of. Asked + # here for the branch's own reason, only harder β€” a title that gets past + # `add` is repaired by hand-editing task.yaml and the brief's H1, because + # nothing here retitles a task. + title = args.title or args.slug.replace("-", " ") + refusal = session_name_refusal(title, worker_glyph()) + if refusal: + raise QueueError(refusal) + # Resolution, first hit wins and per FIELD. A stated method with no stated # tool drops the operator's global one rather than inheriting it: "run # `/no-mistakes --yes`" is the wrong sentence to hand a `push` task. A @@ -1428,7 +1479,7 @@ def cmd_add(args) -> int: doc = { "id": tid, "topic": args.topic, - "title": args.title or args.slug.replace("-", " "), + "title": title, "state": "queued", "repo": args.repo, # None is a local task, and every path below treats it as today's @@ -2281,6 +2332,39 @@ def spawn_commands(task: Task) -> tuple[list, str]: return create, send +def spawn_failure(exc, proc=None) -> str: + """What thurbox actually said when `session create` failed. + + NOT just stderr. `thurbox-cli` prints its structured failure on STDOUT β€” + `{"error": ...}` under `--json` β€” and exits non-zero, so a dispatch reading + only stderr reported `returned non-zero exit status 1` about a session name + thurbox had already named the fault in. That cost an operator a hand-run of + the printed `session create` to find out what it meant. Read both streams, + unwrap thurbox's own `error` field, and fall back to the exception only + when neither stream said anything β€” a spawn that failed for a reason + nobody here anticipated is exactly the case this is for. + + `proc` carries the streams for the failures that are not a non-zero exit: + a success whose JSON does not parse, or carries no `id`, is thurbox saying + something unexpected on stdout, and that something is the whole answer. + """ + source = exc if isinstance(exc, subprocess.CalledProcessError) else proc + said = [] + for raw in (getattr(source, "stdout", None), getattr(source, "stderr", None)): + text = raw.decode(errors="replace") if isinstance(raw, bytes) else (raw or "") + text = text.strip() + if not text: + continue + try: + loaded = json.loads(text) + except ValueError: + loaded = None + if isinstance(loaded, dict) and loaded.get("error"): + text = str(loaded["error"]) + said.append(text) + return "; ".join(said) or str(exc) + + def shell_quote(argv: list) -> str: return " ".join(shlex.quote(a) for a in argv) @@ -2423,12 +2507,12 @@ def cmd_dispatch(args) -> int: continue create, _send = spawn_commands(t) + proc = None try: - out = subprocess.run(create, capture_output=True, check=True).stdout - session = json.loads(out)["id"] + proc = subprocess.run(create, capture_output=True, check=True) + session = json.loads(proc.stdout)["id"] except (OSError, subprocess.CalledProcessError, ValueError, KeyError) as exc: - detail = getattr(exc, "stderr", b"") or b"" - print(f" {t.ref}: spawn failed: {detail.decode().strip() or exc}", file=sys.stderr) + print(f" {t.ref}: spawn failed: {spawn_failure(exc, proc)}", file=sys.stderr) continue attach(t, session) diff --git a/scripts/queue-selftest.sh b/scripts/queue-selftest.sh index 3e02969..fde21e1 100755 --- a/scripts/queue-selftest.sh +++ b/scripts/queue-selftest.sh @@ -92,6 +92,11 @@ # and rewrite it rather than appending to it; and prose the lead wrote # outside that block survives every later pass. # +# 20. A TITLE IS A SESSION NAME, so `add` refuses one thurbox could not spawn +# β€” judged on the RENDERED name, mirroring thurbox's own rule and +# widening it by nothing β€” and a spawn that fails anyway reports what +# thurbox said rather than its exit status. +# # Test 4 is also the wake proof. The event source is `thurbox-cli watch`, which # this script replaces with a recorded stream through `FLEET_QUEUE_WATCH_CMD` β€” # the same override a different transport would use. What matters is the shape: @@ -5053,6 +5058,219 @@ expect "a repo whose origin cannot be read says THAT, not 'no credentials'" \ "$(probe_says '' 'Welcome to GitLab, @letur!' 2>&1)" +# --- 20. a title `dispatch` cannot spawn, and a spawn failure that says why -- +# +# Both halves of one run on 2026-09-11. `add` took the title `Rust crate, +# CI/CD and the profile model`; `dispatch` then died with nothing but +# thurbox's exit status echoed back, and the cause β€” thurbox refuses a session +# name containing `/` β€” was found by running the printed `session create` by +# hand. The repair was a hand-edit of `title` in task.yaml, because there is +# no retitle verb. +# +# (a) `add` refuses a title that cannot become a session name, names the +# character, and creates nothing β€” the same bargain as the `--branch` +# refusal in 16d(f). +# (b) The check is on the name thurbox is actually SENT, not on the raw +# title: the glyph goes in front and the title is cut to the byte cap, +# so a title that is only made long by the rendering is still fine, and +# a leading `.` is unsafe exactly when no glyph precedes it. +# (c) It mirrors thurbox's rule rather than inventing a stricter one. The +# four cases in thurbox's own `unsafe_names_are_rejected` are refused, +# and every character it accepts is still accepted here β€” a title is +# human-facing text and narrowing it further is its own defect. +# (d) A failing spawn carries thurbox's own words, from whichever stream it +# used, and the three things that were already right stay right: the +# task is left `queued`, the rest of the set still goes out, and a +# re-run does not spawn what already went. + +export FLEET_QUEUE_DIR="$tmp/queue-titles" +ntopic="$($QUEUE topic add unspawnable-titles --title 'A title becomes a session name' \ + --prompt 'add accepted a title dispatch could not spawn' 2>/dev/null)" + +# (a) The title from the run, verbatim. +if out="$($QUEUE add "$ntopic" ci-cd --title 'Rust crate, CI/CD and the profile model' \ + --repo /tmp/repo-a --branch feat/ci-cd --number 01 2>&1)"; then + fail "a title that cannot become a session name is refused at add" "$out" +else + pass "a title that cannot become a session name is refused at add" + expect "and the refusal names the offending character" "'/'" "$out" + expect "and says it is the SESSION NAME that cannot carry it" \ + "session name" "$out" + expect "and quotes the name thurbox would have been asked to create" \ + "Rust crate, CI" "$out" +fi +if [ -e "$FLEET_QUEUE_DIR/$ntopic/01-ci-cd" ]; then + fail "and creates nothing, so the repair is one re-run and not an edit" \ + "$(ls "$FLEET_QUEUE_DIR/$ntopic/01-ci-cd")" +else + pass "and creates nothing, so the repair is one re-run and not an edit" +fi + +# (b) The RENDERED name, and only that. A 61-character title wearing a 5-byte +# glyph is 66 bytes and would fail a check against the raw string, but +# `session_name` cuts it to the cap before thurbox ever sees it. +long="Codify the out-of-band identity and patch settings on the box" +if out="$($QUEUE add "$ntopic" long-title --title "$long" \ + --repo /tmp/repo-a --branch feat/long-title --number 02 2>&1)"; then + pass "a title only made over-long by the glyph and the cut is still accepted" +else + fail "a title only made over-long by the glyph and the cut is still accepted" "$out" +fi +expect "and it really was the rendering that made it long: 66 bytes, cut to 64" \ + "66 64" "$(python3 -c ' +import sys +sys.path.insert(0, "scripts/lib") +import queue as q +title = sys.argv[1] +print(len(("\N{ROCKET} " + title).encode()), len(q.session_name(title, "\N{ROCKET}").encode())) +' "$long")" + +# The other direction: a leading `.` is unsafe exactly when nothing precedes +# it, so the same title is fine with the mark on and refused with it off. +nglyph="$(mktemp -d)" +mkdir -p "$nglyph/orchestration" +printf 'GLYPHS=on\nLEAD_GLYPH_ON=πŸ“‘\nLEAD_GLYPH_OFF=βŒ–\nWORKER_GLYPH_ON=πŸš€\n' \ + >"$nglyph/orchestration/session-glyphs.conf" +if out="$(FLEET_GLYPH_ROOT="$nglyph" $QUEUE add "$ntopic" dot-with-mark \ + --title '.hidden agenda' --repo /tmp/repo-a --branch feat/dot-mark \ + --number 03 2>&1)"; then + pass "a title starting '.' is accepted while a mark goes in front of it" +else + fail "a title starting '.' is accepted while a mark goes in front of it" "$out" +fi +printf 'GLYPHS=off\nLEAD_GLYPH_ON=πŸ“‘\nLEAD_GLYPH_OFF=βŒ–\nWORKER_GLYPH_ON=πŸš€\n' \ + >"$nglyph/orchestration/session-glyphs.conf" +if out="$(FLEET_GLYPH_ROOT="$nglyph" $QUEUE add "$ntopic" dot-no-mark \ + --title '.hidden agenda' --repo /tmp/repo-a --branch feat/dot-no-mark \ + --number 04 2>&1)"; then + fail "and refused with the mark off, where the name really does start '.'" "$out" +else + pass "and refused with the mark off, where the name really does start '.'" + expect "naming the rule it broke and not merely the character" \ + "beginning with '.'" "$out" +fi +rm -rf "$nglyph" + +# (c) thurbox's rule, not a stricter one. The four unsafe names are the case +# list from its own `unsafe_names_are_rejected`; the accepted ones are +# ordinary titles, every character of which thurbox takes. +out="$(python3 -c ' +import sys +sys.path.insert(0, "scripts/lib") +import queue as q +unsafe = [".hidden", "foo/bar", "foo..bar", "foo\\bar"] +safe = [ + "Rust crate, CI-CD and the profile model", + "Fix the parser (again!)", + "Ship v2.1: metrics & alerts @ 99% β€” done?", + "Réécrire le lecteur ~ Γ©tape 1", + "a.b.c and #42 + [brackets] {braces} ", + "trailing dot.", +] +for name in unsafe: + print("REFUSED" if q.session_name_refusal(name, "") else "ACCEPTED", name, sep="\t") +for name in safe: + print("REFUSED" if q.session_name_refusal(name, "") else "ACCEPTED", name, sep="\t") +' 2>&1)" +if [ "$(printf '%s\n' "$out" | grep -c '^REFUSED')" = 4 ]; then + pass "every name thurbox's own unsafe_names_are_rejected lists is refused" +else + fail "every name thurbox's own unsafe_names_are_rejected lists is refused" "$out" +fi +refute "and nothing thurbox accepts is refused alongside them" \ + "$(printf 'REFUSED\tRust')" "$out" +if [ "$(printf '%s\n' "$out" | grep -c '^ACCEPTED')" = 6 ]; then + pass "a title is human-facing text, so no character is narrowed beyond that" +else + fail "a title is human-facing text, so no character is narrowed beyond that" "$out" +fi + +# (d) The spawn failure. A `thurbox-cli` in front of the run's own stub fails +# one named branch with a known string, on whichever stream the test +# chooses, and delegates everything else β€” so the tasks that can be +# spawned are spawned by the same stub as every other section. +boombin="$tmp/boom-bin" +mkdir -p "$boombin" +cat >"$boombin/thurbox-cli" <<'SH' +#!/bin/sh +if [ "$1 $2" = "session create" ]; then + case "$*" in + *"$BOOM_MATCH"*) + if [ "${BOOM_STREAM:-stderr}" = stdout ]; then + printf '%s\n' "$BOOM_MESSAGE" + else + printf '%s\n' "$BOOM_MESSAGE" >&2 + fi + exit 1 + ;; + esac +fi +exec "$TBX_REAL" "$@" +SH +chmod +x "$boombin/thurbox-cli" + +for spec in \ + "10:goes-out:Goes out anyway" \ + "11:boom:Spawn fails on stderr" \ + "12:stdout-boom:Spawn fails on stdout"; do + IFS=: read -r n slug title <<<"$spec" + $QUEUE add "$ntopic" "$slug" --title "$title" --repo /tmp/repo-a \ + --branch "feat/$slug" --number "$n" >/dev/null + printf '# %s\n\nA brief with real content in it.\n' "$title" \ + >"$FLEET_QUEUE_DIR/$ntopic/$n-$slug/BRIEF.md" +done +# The two above that were accepted carry a scaffolded brief, and `dispatch` +# refuses the whole wave over one of those. Hold them out of the ready set. +for held in 02-long-title 03-dot-with-mark; do + $QUEUE block "$ntopic/$held" --on "$ntopic/10-goes-out" \ + --kind other --why 'held out of the spawn test below' >/dev/null +done + +nsession=dddddddd-dddd-dddd-dddd-dddddddddddd +printf '{"id":"%s","created":true}\n' "$nsession" >"$tmp/next-session.json" +session_is "$nsession" idle + +# The stubs are named rather than taken off $PATH: test 7's subshell exported +# its own, which is what SC2031 is about, and this section wants the one the +# run set up. +boom() { + env PATH="$boombin:$ghbin:$tbxbin:$sshbin:$quotabin:$base_path" \ + TBX_REAL="$tbxbin/thurbox-cli" "$@" +} + +out="$(boom BOOM_MATCH=feat/boom \ + BOOM_MESSAGE='Name contains invalid characters' \ + $QUEUE dispatch "$ntopic/10-goes-out" "$ntopic/11-boom" 2>&1)" +expect "a failing spawn reports what thurbox said" \ + "Name contains invalid characters" "$out" +refute "and not only the exit status the code used to echo back" \ + "returned non-zero exit status" "$out" +expect "the task that could be spawned still went out" "10-goes-out" "$out" +expect "and it really got its session" "$nsession" "$out" +expect "the failed one is left queued, so fixing it and re-running sends it" \ + "queued" "$($QUEUE show "$ntopic/11-boom" | grep -F 'state:')" + +# thurbox prints its structured failure on STDOUT, not stderr β€” that is why +# reading only stderr left the operator with an exit status and nothing else. +out="$(boom BOOM_MATCH=feat/stdout-boom BOOM_STREAM=stdout \ + BOOM_MESSAGE='{"error":"Name contains invalid characters","suggestion":"the command ran and failed"}' \ + $QUEUE dispatch "$ntopic/12-stdout-boom" 2>&1)" +expect "a refusal thurbox printed on stdout is read too" \ + "Name contains invalid characters" "$out" +refute "and its JSON wrapping is unwrapped rather than echoed" \ + "suggestion" "$out" + +# A re-run sends the ones that failed and does not touch the one that went. +resent=eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee +printf '{"id":"%s","created":true}\n' "$resent" >"$tmp/next-session.json" +session_is "$resent" idle +out="$(boom BOOM_MATCH=no-such-branch $QUEUE dispatch 2>&1)" +expect "a re-run spawns the one that failed" "11-boom" "$out" +refute "and does not spawn the one already sent" "10-goes-out" "$out" +expect "whose session is still the first one" "$nsession" \ + "$($QUEUE show "$ntopic/10-goes-out")" + + # The fixer above got a real worktree; take it back off the test repo, as # section 13 does with its own. git -C "$glrepo" worktree remove --force \ diff --git a/scripts/queue.sh b/scripts/queue.sh index 0086a22..d829afd 100755 --- a/scripts/queue.sh +++ b/scripts/queue.sh @@ -159,7 +159,11 @@ # # headings all goes into `What to do`. A file that # # leaves any section unwritten is refused HERE, naming # # them, and nothing is created β€” as is a --branch no -# # worktree could be cut for, which includes --base +# # worktree could be cut for, which includes --base, +# # and a --title thurbox could not make a session name +# # of: that name is the title wearing the worker's mark +# # and cut to thurbox's byte cap, and it carries no +# # `/`, no `\`, no `..` and no leading `.` # scripts/queue.sh block --on --kind KIND --why 'reason' # or --clear, # which names the blocker to remove, since a task can # carry several; `block --help` lists the valid kinds