From 206d486efc9874ea0bb501e06bf38b41d238e9bb Mon Sep 17 00:00:00 2001 From: LeTuR Date: Fri, 11 Sep 2026 10:17:46 +0200 Subject: [PATCH 1/4] fix(queue): refuse a title dispatch cannot spawn, and say what a spawn failure was MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A title becomes the worker's thurbox session name, and thurbox refuses a name it could not make a path segment of. `add` took `Rust crate, CI/CD and the profile model`; `dispatch` then died with nothing but thurbox's exit status, and the repair was a hand-edit of `title` in task.yaml and the brief's H1. `add` now asks thurbox's own rule — empty, over the byte cap, leading `.`, or holding `/`, `\` or `..` — about the RENDERED name, since that is the string `session create` is handed: the glyph goes in front and the title is cut to the cap first. Nothing thurbox accepts is refused. And `dispatch` reports what thurbox said. It read only stderr; thurbox-cli prints its structured failure on STDOUT (`{"error": ...}` under `--json`), so the reason was being discarded. Both streams are read and the `error` field unwrapped. A failed spawn still leaves that task queued, still lets the rest of the set go out, and still does not double-spawn on a re-run. --- scripts/lib/queue.py | 94 +++++++++++++++- scripts/queue-selftest.sh | 218 ++++++++++++++++++++++++++++++++++++++ scripts/queue.sh | 6 +- 3 files changed, 312 insertions(+), 6 deletions(-) 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 From e20cc7620689d396721858758fd95a7877748162 Mon Sep 17 00:00:00 2001 From: Arthur Cesare-Herriau Date: Fri, 11 Sep 2026 10:36:41 +0200 Subject: [PATCH 2/4] chore: no-mistakes review - keep fixer session names spawnable and report spawn failures --- .agents/skills/fleet-queue/SKILL.md | 9 +++ scripts/lib/queue.py | 16 +++-- scripts/queue-selftest.sh | 106 ++++++++++++++++++++++++++++ 3 files changed, 126 insertions(+), 5 deletions(-) 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/scripts/lib/queue.py b/scripts/lib/queue.py index bd83edd..3b80667 100644 --- a/scripts/lib/queue.py +++ b/scripts/lib/queue.py @@ -4518,7 +4518,13 @@ def base_drift(repo: str, base: str, branch: str) -> str: # --- the fixer's brief ------------------------------------------------------- FIXER_TITLES = { - "conflicting": "Rebase PR #{n} onto {base}", + # A fixer title becomes a session NAME, and nothing upstream can refuse it + # the way `add` refuses an operator's: the shepherd writes it itself. So it + # carries the pull request number and nothing that could hold a `/` — a + # base branch is `release/1.0` as a matter of course, and thurbox refuses a + # name it could not make a path segment of. The brief names the base one + # line down, where it is not also a path. + "conflicting": "Rebase PR #{n}", "checks-failed": "Fix the failing checks on PR #{n}", "changes-requested": "Address the review on PR #{n}", "policy": "Re-open PR #{n} through the pipeline", @@ -4732,13 +4738,13 @@ def spawn_fixer(task: Task, name: str, brief_path: str, branch: str) -> tuple[st if parent: create += ["--parent", parent] create += flags + ["--json"] + proc = None try: - out = subprocess.run(create, capture_output=True, check=True).stdout - doc = json.loads(out) + proc = subprocess.run(create, capture_output=True, check=True) + doc = json.loads(proc.stdout) session = doc["id"] except (OSError, subprocess.CalledProcessError, ValueError, KeyError) as exc: - detail = (getattr(exc, "stderr", b"") or b"").decode().strip() - return "", f"could not spawn a fixer: {detail or exc}" + return "", f"could not spawn a fixer: {spawn_failure(exc, proc)}" if not doc.get("created", True): # Adopted, so it may be mid-turn. The same rule as everywhere else: # only the agent's own word puts it at rest (§4a). diff --git a/scripts/queue-selftest.sh b/scripts/queue-selftest.sh index fde21e1..fff32b2 100755 --- a/scripts/queue-selftest.sh +++ b/scripts/queue-selftest.sh @@ -96,6 +96,10 @@ # — 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. +# 21. THE SHEPHERD NAMES A FIXER'S SESSION ITSELF, so the same rule holds +# where no operator is watching: a conflicting change request onto a +# slashed base branch still gets its fixer, and a fixer spawn that fails +# carries thurbox's own words. # # 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` — @@ -5271,6 +5275,106 @@ expect "whose session is still the first one" "$nsession" \ "$($QUEUE show "$ntopic/10-goes-out")" +# --- 21. the fixer's own session name, and a fixer spawn that failed -------- +# +# §20's two defects again, on the one path where there is no operator to +# refuse: `shepherd` writes a fixer's title ITSELF, so nothing upstream can +# catch one thurbox will not take. `Rebase PR #{n} onto {base}` carried the +# BASE BRANCH into that name, and a base branch holds `/` as a matter of +# course — `release/1.0`, a stacked `feat/...` — so the fixer for the one +# condition that named its base could never be spawned, and the shepherd +# reported thurbox's bare exit status for it. +# +# Driven through section 13's fake forge, whose repository, plugin and topic +# are already standing, with thurbox's own naming rule in front of the run's +# stub: `paths::validate_safe_name` refuses a name it could not make a path +# segment of, and the real CLI prints that refusal on STDOUT as +# `{"error": ...}` and exits non-zero. + +strictbin="$tmp/strict-bin" +mkdir -p "$strictbin" +: >"$tmp/strict-names.log" +cat >"$strictbin/thurbox-cli" <<'SH' +#!/bin/sh +if [ "$1 $2" = "session create" ]; then + name="" + prev="" + for a in "$@"; do + [ "$prev" = --name ] && name="$a" + prev="$a" + done + echo "$name" >>"$STRICT_NAMES" + bad="" + case "$name" in + "" | .* | */* | *\\* | *..*) bad=yes ;; + esac + [ "$(printf '%s' "$name" | wc -c)" -gt 64 ] && bad=yes + if [ -n "$bad" ]; then + printf '{"error":"invalid session name: %s cannot be a path segment"}\n' "$name" + exit 1 + fi +fi +exec "$TBX_REAL" "$@" +SH +chmod +x "$strictbin/thurbox-cli" + +# Section 13's environment, with the PATH left to the caller so each pass can +# put its own `thurbox-cli` in front of the run's stub. +onforge() { + env TBX_REAL="$tbxbin/thurbox-cli" STRICT_NAMES="$tmp/strict-names.log" \ + FLEET_QUEUE_DIR="$tmp/queue-fake" \ + FLEET_FORGE_PLUGINS="$fk/forge_plugin.py" \ + FLEET_AUTO_MERGE_REPOS="forge.test:8443/acme/widgets" \ + "$@" +} + +# The shape the fixer title could not survive: conflicting, onto a base branch +# with a `/` in it. The branch appears after the `add`, as everywhere else. +fq add "$ftopic" slashed-base --title 'A change onto a slashed base' \ + --repo "$frepo" --branch fix/slashed-base --number 07 >/dev/null +git -C "$frepo" branch fix/slashed-base +git -C "$frepo" branch release/1.0 +fake_cr 207 'head_branch="fix/slashed-base"' 'base_branch="release/1.0"' \ + 'mergeable="conflicting"' + +fixsession=ffffffff-0000-0000-0000-000000000207 +printf '{"id":"%s","created":true}\n' "$fixsession" >"$tmp/next-session.json" +session_is "$fixsession" idle + +# (b) first, because a fixer that went out is left alone on the next pass. The +# stub from §20(d), failing this one spawn with thurbox's own JSON on the +# stream thurbox really uses. +out="$(onforge PATH="$boombin:$FPATH" BOOM_MATCH=07-slashed-base BOOM_STREAM=stdout \ + BOOM_MESSAGE='{"error":"invalid session name: it cannot be a path segment"}' \ + "$QUEUE" shepherd --ref "$ftopic/07-slashed-base" --no-merge 2>&1)" +expect "a fixer spawn that fails carries thurbox's own words" \ + "cannot be a path segment" "$out" +refute "and not the exit status the note used to be" \ + "returned non-zero exit status" "$out" +expect "and the change request is left for the next pass" "not-dispatched" "$out" + +# (a) The same pull request, against thurbox's real rule: the fixer goes out, +# which it can only do if the name it was asked for is one thurbox takes. +out="$(onforge PATH="$strictbin:$FPATH" "$QUEUE" shepherd \ + --ref "$ftopic/07-slashed-base" --no-merge 2>&1)" +expect "a conflicting change request onto a slashed base still gets its fixer" \ + "$fixsession" "$out" +refute "and nothing was left undispatched for want of a name" "not-dispatched" "$out" +names="$(cat "$tmp/strict-names.log")" +refute "the name thurbox was asked for carries no path separator" "/" "$names" +expect "and it still says which pull request the fixer is for" \ + "Rebase PR #207" "$names" + +fixbrief="$(find "$tmp/queue-fake/$ftopic/07-slashed-base" -name 'fix-*-conflicting.md' | + sort | tail -1)" +if [ -n "$fixbrief" ]; then + expect "and the base branch is in the brief, where it is prose and not a path" \ + "release/1.0" "$(cat "$fixbrief")" +else + fail "and the base branch is in the brief, where it is prose and not a path" \ + "no fix-*-conflicting.md under 07-slashed-base" +fi + # 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 \ @@ -5280,6 +5384,8 @@ git -C "$glrepo" worktree remove --force \ # temp directory can be removed without leaving a stale registration. git -C "$frepo" worktree remove --force \ "$tmp/queue-fake/.worktrees/${ftopic}__02-conflicting" 2>/dev/null +git -C "$frepo" worktree remove --force \ + "$tmp/queue-fake/.worktrees/${ftopic}__07-slashed-base" 2>/dev/null echo if [ "$failed" -eq 0 ]; then From 10397aca8267961983553f6ec3f7bd62c6696f1c Mon Sep 17 00:00:00 2001 From: LeTuR Date: Fri, 11 Sep 2026 13:12:36 +0200 Subject: [PATCH 3/4] revert(shepherd): put the fixer title and its spawn note back MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The review found that `FIXER_TITLES["conflicting"]` carries the base branch into a session name, and a base branch holds `/` as a matter of course — the same defect this branch fixes for an operator's `--title`, on the one path where nobody can refuse it. It is a real finding and it is out of scope: this task's brief forbids changing `shepherd`, and the operator asked for exactly the two defects they hit. So the fixer title, `spawn_fixer` and the selftest section about them go back to what they were. The finding is reported to the lead in this task's result.md instead, to be queued as its own task. The skill note the same round added stays: it documents `add`'s new refusal, which is this task's own work. --- scripts/lib/queue.py | 16 ++---- scripts/queue-selftest.sh | 106 -------------------------------------- 2 files changed, 5 insertions(+), 117 deletions(-) diff --git a/scripts/lib/queue.py b/scripts/lib/queue.py index 3b80667..bd83edd 100644 --- a/scripts/lib/queue.py +++ b/scripts/lib/queue.py @@ -4518,13 +4518,7 @@ def base_drift(repo: str, base: str, branch: str) -> str: # --- the fixer's brief ------------------------------------------------------- FIXER_TITLES = { - # A fixer title becomes a session NAME, and nothing upstream can refuse it - # the way `add` refuses an operator's: the shepherd writes it itself. So it - # carries the pull request number and nothing that could hold a `/` — a - # base branch is `release/1.0` as a matter of course, and thurbox refuses a - # name it could not make a path segment of. The brief names the base one - # line down, where it is not also a path. - "conflicting": "Rebase PR #{n}", + "conflicting": "Rebase PR #{n} onto {base}", "checks-failed": "Fix the failing checks on PR #{n}", "changes-requested": "Address the review on PR #{n}", "policy": "Re-open PR #{n} through the pipeline", @@ -4738,13 +4732,13 @@ def spawn_fixer(task: Task, name: str, brief_path: str, branch: str) -> tuple[st if parent: create += ["--parent", parent] create += flags + ["--json"] - proc = None try: - proc = subprocess.run(create, capture_output=True, check=True) - doc = json.loads(proc.stdout) + out = subprocess.run(create, capture_output=True, check=True).stdout + doc = json.loads(out) session = doc["id"] except (OSError, subprocess.CalledProcessError, ValueError, KeyError) as exc: - return "", f"could not spawn a fixer: {spawn_failure(exc, proc)}" + detail = (getattr(exc, "stderr", b"") or b"").decode().strip() + return "", f"could not spawn a fixer: {detail or exc}" if not doc.get("created", True): # Adopted, so it may be mid-turn. The same rule as everywhere else: # only the agent's own word puts it at rest (§4a). diff --git a/scripts/queue-selftest.sh b/scripts/queue-selftest.sh index fff32b2..fde21e1 100755 --- a/scripts/queue-selftest.sh +++ b/scripts/queue-selftest.sh @@ -96,10 +96,6 @@ # — 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. -# 21. THE SHEPHERD NAMES A FIXER'S SESSION ITSELF, so the same rule holds -# where no operator is watching: a conflicting change request onto a -# slashed base branch still gets its fixer, and a fixer spawn that fails -# carries thurbox's own words. # # 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` — @@ -5275,106 +5271,6 @@ expect "whose session is still the first one" "$nsession" \ "$($QUEUE show "$ntopic/10-goes-out")" -# --- 21. the fixer's own session name, and a fixer spawn that failed -------- -# -# §20's two defects again, on the one path where there is no operator to -# refuse: `shepherd` writes a fixer's title ITSELF, so nothing upstream can -# catch one thurbox will not take. `Rebase PR #{n} onto {base}` carried the -# BASE BRANCH into that name, and a base branch holds `/` as a matter of -# course — `release/1.0`, a stacked `feat/...` — so the fixer for the one -# condition that named its base could never be spawned, and the shepherd -# reported thurbox's bare exit status for it. -# -# Driven through section 13's fake forge, whose repository, plugin and topic -# are already standing, with thurbox's own naming rule in front of the run's -# stub: `paths::validate_safe_name` refuses a name it could not make a path -# segment of, and the real CLI prints that refusal on STDOUT as -# `{"error": ...}` and exits non-zero. - -strictbin="$tmp/strict-bin" -mkdir -p "$strictbin" -: >"$tmp/strict-names.log" -cat >"$strictbin/thurbox-cli" <<'SH' -#!/bin/sh -if [ "$1 $2" = "session create" ]; then - name="" - prev="" - for a in "$@"; do - [ "$prev" = --name ] && name="$a" - prev="$a" - done - echo "$name" >>"$STRICT_NAMES" - bad="" - case "$name" in - "" | .* | */* | *\\* | *..*) bad=yes ;; - esac - [ "$(printf '%s' "$name" | wc -c)" -gt 64 ] && bad=yes - if [ -n "$bad" ]; then - printf '{"error":"invalid session name: %s cannot be a path segment"}\n' "$name" - exit 1 - fi -fi -exec "$TBX_REAL" "$@" -SH -chmod +x "$strictbin/thurbox-cli" - -# Section 13's environment, with the PATH left to the caller so each pass can -# put its own `thurbox-cli` in front of the run's stub. -onforge() { - env TBX_REAL="$tbxbin/thurbox-cli" STRICT_NAMES="$tmp/strict-names.log" \ - FLEET_QUEUE_DIR="$tmp/queue-fake" \ - FLEET_FORGE_PLUGINS="$fk/forge_plugin.py" \ - FLEET_AUTO_MERGE_REPOS="forge.test:8443/acme/widgets" \ - "$@" -} - -# The shape the fixer title could not survive: conflicting, onto a base branch -# with a `/` in it. The branch appears after the `add`, as everywhere else. -fq add "$ftopic" slashed-base --title 'A change onto a slashed base' \ - --repo "$frepo" --branch fix/slashed-base --number 07 >/dev/null -git -C "$frepo" branch fix/slashed-base -git -C "$frepo" branch release/1.0 -fake_cr 207 'head_branch="fix/slashed-base"' 'base_branch="release/1.0"' \ - 'mergeable="conflicting"' - -fixsession=ffffffff-0000-0000-0000-000000000207 -printf '{"id":"%s","created":true}\n' "$fixsession" >"$tmp/next-session.json" -session_is "$fixsession" idle - -# (b) first, because a fixer that went out is left alone on the next pass. The -# stub from §20(d), failing this one spawn with thurbox's own JSON on the -# stream thurbox really uses. -out="$(onforge PATH="$boombin:$FPATH" BOOM_MATCH=07-slashed-base BOOM_STREAM=stdout \ - BOOM_MESSAGE='{"error":"invalid session name: it cannot be a path segment"}' \ - "$QUEUE" shepherd --ref "$ftopic/07-slashed-base" --no-merge 2>&1)" -expect "a fixer spawn that fails carries thurbox's own words" \ - "cannot be a path segment" "$out" -refute "and not the exit status the note used to be" \ - "returned non-zero exit status" "$out" -expect "and the change request is left for the next pass" "not-dispatched" "$out" - -# (a) The same pull request, against thurbox's real rule: the fixer goes out, -# which it can only do if the name it was asked for is one thurbox takes. -out="$(onforge PATH="$strictbin:$FPATH" "$QUEUE" shepherd \ - --ref "$ftopic/07-slashed-base" --no-merge 2>&1)" -expect "a conflicting change request onto a slashed base still gets its fixer" \ - "$fixsession" "$out" -refute "and nothing was left undispatched for want of a name" "not-dispatched" "$out" -names="$(cat "$tmp/strict-names.log")" -refute "the name thurbox was asked for carries no path separator" "/" "$names" -expect "and it still says which pull request the fixer is for" \ - "Rebase PR #207" "$names" - -fixbrief="$(find "$tmp/queue-fake/$ftopic/07-slashed-base" -name 'fix-*-conflicting.md' | - sort | tail -1)" -if [ -n "$fixbrief" ]; then - expect "and the base branch is in the brief, where it is prose and not a path" \ - "release/1.0" "$(cat "$fixbrief")" -else - fail "and the base branch is in the brief, where it is prose and not a path" \ - "no fix-*-conflicting.md under 07-slashed-base" -fi - # 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 \ @@ -5384,8 +5280,6 @@ git -C "$glrepo" worktree remove --force \ # temp directory can be removed without leaving a stale registration. git -C "$frepo" worktree remove --force \ "$tmp/queue-fake/.worktrees/${ftopic}__02-conflicting" 2>/dev/null -git -C "$frepo" worktree remove --force \ - "$tmp/queue-fake/.worktrees/${ftopic}__07-slashed-base" 2>/dev/null echo if [ "$failed" -eq 0 ]; then From cec3fa2febb3466cb9346316f845e4d6fad6b22d Mon Sep 17 00:00:00 2001 From: Arthur Cesare-Herriau Date: Fri, 11 Sep 2026 13:48:47 +0200 Subject: [PATCH 4/4] chore: no-mistakes document - correct spawn-failure claim in thurbox-session naming reference --- .agents/skills/thurbox-session/SKILL.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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