session, run, cmd: the worker harness is the default belt again, with no worker bound

cmd/codeaf/do.go

@@ -323,11 +323,18 @@ func runDo(args []string) error {
 	completionReserve := flags.Int("completion-reserve", 0,
 		"tokens every call keeps free for its answer and its reasoning "+
 			"(default "+strconv.Itoa(ctxbudget.DefaultCompletionReserveTokens)+")")
+	slotsRaw := flags.String("slots", "",
+		"how many workers may run at once for this run; 0 is no limit "+
+			"(default: your task.parallel setting, which is no limit)")
 	debug := flags.Bool("debug", false, debugFlagHelp())
 	if err := parseCommandFlags(flags, reorder(flags, args)); err != nil {
 		return err
 	}
 	noteRenamedFlags(flags)
+	slots, err := parseSlots(*slotsRaw)
+	if err != nil {
+		return err
+	}
 	// THE RUN ID IS MINTED AT THE DOOR, once per invocation and before anything
 	// can make a call, so that every record this errand leaves names the same
 	// run. The folder is announced on the way out and only when something was
@@ -351,7 +358,7 @@ func runDo(args []string) error {
 		task: task, run: run, database: *database, keep: *keep, workspace: *workspace,
 		timeout: wall.wall, asJSON: *asJSON,
 		yesSpend: *yesSpend, model: *model, planModel: *planModel, checkModel: *checkModel,
-		contextFill: *contextFill, completionReserve: *completionReserve,
+		contextFill: *contextFill, completionReserve: *completionReserve, slots: slots,
 		stdout: os.Stdout, stderr: os.Stderr,
 	})
 }
@@ -398,9 +405,11 @@ type doRequest struct {
 	// a run to a price can say so, and a ceiling of nothing is a run that may
 	// spend nothing: the limit stopped it before a worker did.
 	costCap *float64
-	// slots bounds how many run-engine workers run at once. Zero is the door's
-	// own default ([defaultRunSlots]); a test names one it can watch.
-	slots int
+	// slots bounds how many run-engine workers run at once. Nil is the
+	// person's own `task.parallel` setting, the same row the chat door reads,
+	// and a named 0 is no bound at all (the `--slots` flag); a test names one
+	// it can watch.
+	slots *int
 	// newBeltCompleter scripts the run road's worker, the way newClient scripts
 	// the legacy road's. Nil builds a real provider client per seat model, which
 	// is what a live run does; a test hands back a [session.Completer] that
@@ -3356,17 +3365,36 @@ func errandStatus(outcome headlessOutcome) error {
 
 // ── the run road ────────────────────────────────────────────────────────────
 
-// defaultRunSlots is how many run-engine workers `codeaf do` starts at once
-// when no caller names a number. Four is the same width the resident's own
-// dispatcher runs a job at, and it is a bound rather than a target: a brief
-// that needs one worker uses one.
-const defaultRunSlots = 4
+// slotsFor answers how many run-engine workers this errand may run at once,
+// where 0 is no bound. A caller who named a figure gets it; anyone else gets
+// the profile's `task.parallel`, which is the ONE row that answers this
+// question for the chat door too (internal/session's task_run_belt.go reads
+// the same setting) and is no limit out of the box. This door used to carry
+// a constant of its own, four, beside a setting that promised no limit — two
+// answers to one question, and the person who had set the row found `codeaf
+// do` ignoring it.
+func (r doRequest) slotsFor(profileDir string) int {
+	if r.slots != nil {
+		return *r.slots
+	}
+	return config.TaskParallelAt(profileDir)
+}
 
-func (r doRequest) slotsOrDefault() int {
-	if r.slots > 0 {
-		return r.slots
+// parseSlots reads the `--slots` flag. Blank is the flag unset, which leaves
+// the answer to the profile; anything else is a whole number of workers, and
+// 0 is no bound. The flag is a string rather than an int so that an unset
+// flag and a named 0 are two different things, which an int's zero value
+// cannot say.
+func parseSlots(raw string) (*int, error) {
+	raw = strings.TrimSpace(raw)
+	if raw == "" {
+		return nil, nil
+	}
+	n, err := strconv.Atoi(raw)
+	if err != nil || n < 0 {
+		return nil, fmt.Errorf("--slots wants a whole number of workers, 0 for no limit; got %q", raw)
 	}
-	return defaultRunSlots
+	return &n, nil
 }
 
 // runErrand is `codeaf do` on the run engine: the same errand as the road above
@@ -3440,7 +3468,7 @@ func runErrand(request doRequest, seats config.Seats) (headlessOutcome, error) {
 		Workspace: workspace,
 		Title:     title,
 		Brief:     request.task,
-		Slots:     request.slotsOrDefault(),
+		Slots:     request.slotsFor(settings.ProfileDir),
 		Limits:    limits,
 		Factory: runengine.CrewFactory(store, workspace, settings.ProfileDir, runengine.Seats{
 			Work:  seats.Work.Model,

cmd/codeaf/do_engine_test.go

@@ -272,7 +272,7 @@ func TestDoOnTheRunEngineRootStoreFinishBeforeWorkerReturnNamesRootResult(t *tes
 	var stdout, stderr strings.Builder
 	err := doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1, stdout: &stdout, stderr: &stderr,
+		timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr,
 		newBeltCompleter: func(string) session.Completer { return seat },
 	})
 	if err != nil {
@@ -313,7 +313,7 @@ func TestDoOnTheRunEngineCompletesABriefAndNamesTheRootResult(t *testing.T) {
 	var stdout, stderr strings.Builder
 	err := doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1, stdout: &stdout, stderr: &stderr,
+		timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr,
 		newBeltCompleter: func(string) session.Completer { return seat },
 	})
 	if err != nil {
@@ -419,7 +419,7 @@ func TestDoOnTheRunEngineSeatsEveryLaunchOnTheDoorsModels(t *testing.T) {
 	var stdout, stderr strings.Builder
 	err = doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1, model: workModel, planModel: planModel,
+		timeout: 60 * time.Second, slots: bound(1), model: workModel, planModel: planModel,
 		stdout: &stdout, stderr: &stderr, newBeltCompleter: newBelt,
 	})
 	if err != nil {
@@ -485,7 +485,7 @@ func TestDoOnTheRunEngineChecksALeafAndExitsZeroWhenItHolds(t *testing.T) {
 	var stdout, stderr strings.Builder
 	if err := doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1, stdout: &stdout, stderr: &stderr,
+		timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr,
 		newBeltCompleter: func(string) session.Completer { return seat },
 	}); err != nil {
 		t.Fatalf("a run whose check held left with %v, want 0\nstdout:\n%s\nstderr:\n%s",
@@ -525,7 +525,7 @@ func TestDoOnTheRunEngineChecksASelfFinishedRootAndExitsZeroWhenItHolds(t *testi
 	var stdout, stderr strings.Builder
 	if err := doErrand(doRequest{
 		task: "do the work alone and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1, stdout: &stdout, stderr: &stderr,
+		timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr,
 		newBeltCompleter: func(string) session.Completer { return seat },
 	}); err != nil {
 		t.Fatalf("a self-finished root whose check held left with %v, want 0\nstdout:\n%s\nstderr:\n%s",
@@ -620,7 +620,7 @@ func TestDoOnTheRunEngineLeavesTheUsageLedgerToTheSession(t *testing.T) {
 	var stdout, stderr strings.Builder
 	if err := doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1, stdout: &stdout, stderr: &stderr,
+		timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr,
 		newBeltCompleter: func(string) session.Completer { return seat },
 	}); err != nil {
 		t.Fatalf("errand: %v\n%s", err, stderr.String())
@@ -705,7 +705,7 @@ func TestDoOnTheRunEngineSeatsACheckOnTheCheckModel(t *testing.T) {
 	var stdout, stderr strings.Builder
 	err = doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1,
+		timeout: 60 * time.Second, slots: bound(1),
 		model: workModel, planModel: planModel, checkModel: checkModel,
 		stdout: &stdout, stderr: &stderr, newBeltCompleter: newBelt,
 	})
@@ -796,7 +796,7 @@ func TestDoOnTheRunEngineSeatsAnUnpinnedCheckOnTheCrewsChecker(t *testing.T) {
 	var stdout, stderr strings.Builder
 	err = doErrand(doRequest{
 		task: "write out.txt and say what you did", workspace: workspace, asJSON: true,
-		timeout: 60 * time.Second, slots: 1,
+		timeout: 60 * time.Second, slots: bound(1),
 		stdout: &stdout, stderr: &stderr, newBeltCompleter: newBelt,
 	})
 	if err != nil {
@@ -825,3 +825,6 @@ func TestDoOnTheRunEngineSeatsAnUnpinnedCheckOnTheCrewsChecker(t *testing.T) {
 		t.Fatalf("the completer was never asked for the crew's careful row; the check was seated elsewhere (built %v)", models)
 	}
 }
+
+// bound is a named slot count for a request, the way the flag would name one.
+func bound(n int) *int { return &n }

cmd/codeaf/do_slots_test.go

@@ -0,0 +1,62 @@
+package main
+
+import (
+	"testing"
+
+	"github.com/Agent-Field/codeaf/internal/config"
+)
+
+// ONE ROW ANSWERS HOW MANY WORKERS RUN AT ONCE, and `codeaf do` reads it the
+// way the chat door does. The door used to carry a constant of its own, four,
+// so a person who had set `task.parallel` found this command ignoring it and a
+// person who had set nothing got a bound the setting said did not exist.
+func TestCodeafDoRunsAsManyWorkersAsTaskParallelSays(t *testing.T) {
+	dir := t.TempDir()
+	if got := (doRequest{}).slotsFor(dir); got != config.DefaultTaskParallel {
+		t.Fatalf("an unset row resolves %d slots, want the setting's own default %d (no limit)", got, config.DefaultTaskParallel)
+	}
+	registry := config.NewSettings(config.SettingsOptions{
+		ProfileDir: dir,
+		ModelValue: func(slot string) string { return slot + "/model" },
+		SetModel:   func(string, string) error { return nil },
+		SplitPct:   func() int { return 0 },
+	})
+	row, found := registry.Row(config.KeyTaskParallel)
+	if !found {
+		t.Fatalf("no settings row %q", config.KeyTaskParallel)
+	}
+	if err := row.Apply("3"); err != nil {
+		t.Fatal(err)
+	}
+	if got := (doRequest{}).slotsFor(dir); got != 3 {
+		t.Fatalf("a row of 3 resolves %d slots for codeaf do, want 3", got)
+	}
+	// A figure named on the command outranks the row, and a named 0 is no
+	// bound rather than "unset": the two are different requests.
+	if got := (doRequest{slots: bound(2)}).slotsFor(dir); got != 2 {
+		t.Fatalf("--slots 2 resolves %d, want 2 over a row of 3", got)
+	}
+	if got := (doRequest{slots: bound(0)}).slotsFor(dir); got != 0 {
+		t.Fatalf("--slots 0 resolves %d, want 0 (no bound) over a row of 3", got)
+	}
+}
+
+// The flag is read as text so that a blank and a zero stay two different
+// things; everything that is not a whole number of workers is refused with the
+// value it was handed.
+func TestTheSlotsFlagTellsBlankFromZeroAndRefusesTheRest(t *testing.T) {
+	if got, err := parseSlots(""); err != nil || got != nil {
+		t.Fatalf("blank parses (%v, %v), want (nil, nil): the flag was not given", got, err)
+	}
+	if got, err := parseSlots(" 0 "); err != nil || got == nil || *got != 0 {
+		t.Fatalf("0 parses (%v, %v), want a named zero", got, err)
+	}
+	if got, err := parseSlots("12"); err != nil || got == nil || *got != 12 {
+		t.Fatalf("12 parses (%v, %v), want 12", got, err)
+	}
+	for _, bad := range []string{"-1", "four", "1.5"} {
+		if _, err := parseSlots(bad); err == nil {
+			t.Fatalf("--slots %q was accepted, want a refusal", bad)
+		}
+	}
+}

cmd/codeaf/logs_test.go

@@ -46,6 +46,16 @@ func TestMain(m *testing.M) {
 		os.Setenv("CODEAF_MODEL_POOL_SUBMIT_URL", "http://127.0.0.1:1/v1/rows")
 	}
 	restore := isolateTestEnvironment()
+	// AND THIS BINARY'S SUITE IS THE OLDER BELT'S SUITE, for the reason
+	// internal/session's TestMain gives at length: these tests drive `codeaf do`
+	// and the task doors down the node road they were written against, and they
+	// said which road by saying nothing. The pin is unconditional and it is set
+	// AFTER the isolation above, which clears this variable when the binary was
+	// launched by a plan worker — a suite whose answer depends on what the
+	// person running it exported is the one thing a test may not be, and a pin
+	// that a later unset undoes is not a pin. A test that means the harness sets
+	// "bash" for itself and wins.
+	os.Setenv("CODEAF_TASK_BELT", "node")
 	code := m.Run()
 	restore()
 	os.Exit(code)

cmd/codeaf/runwire.go

@@ -5,6 +5,6 @@ package main
 // what puts the door's second road — a `/task` under the bash belt starting a
 // RUN rather than a node of the session's own tree — into this binary, so the
 // import is a blank one: nothing here calls the package, and the registration is
-// its one effect. With CODEAF_TASK_BELT unset the door answers the road it always
-// had, and this changes nothing a person sees.
+// its one effect. With CODEAF_TASK_BELT naming the older belt the door answers
+// the road it always had, and the import still changes nothing a person sees.
 import _ "github.com/Agent-Field/codeaf/internal/run"

docs/changes/unreleased/1355-the-worker-harness-is-the-default-belt-again.md

@@ -0,0 +1,48 @@
+---
+kind: changed
+title: the worker harness is the default belt again, and a run has no worker bound
+pr: 1355
+surface: [engine, chat]
+invalidates:
+  - "#1340 made the node belt the default again, with the exact word `bash` as the only way onto the harness. It is the other way round once more: unset `CODEAF_TASK_BELT` is the harness, and `node`, `legacy` and `off` are the words that reach the older engine. Anything else, a blank included, is the harness."
+  - "A task the chat put on the harness ran its parts ONE AT A TIME whatever `task.parallel` said, because the run engine read the setting's 0 as 1. It reads 0 as no bound now, which is what the setting has always promised."
+  - "`codeaf do` started four workers at once and read no setting. It reads `task.parallel` like the chat door, and `--slots <n>` names a figure for one run; there is no constant of four anywhere."
+---
+#1340 sent the default back to the node belt on a comparison whose cells were
+launched in two unequal waves, and its own entry said the quality gap was one
+task. The comparison that answers it is a 32-cell shuffled single batch on
+Spark: eight DeepSWE tasks, two replicates, both belts interleaved so they saw
+the same load, one model on every seat, each cell graded by its fixture's own
+suite with the official test patch applied. The harness passed 3 of 14 scored
+cells at a median `$0.239` and 1010 seconds; the node belt passed 2 of 13 at
+`$0.384` and 2924 seconds. Pooled over every scored cell of the week, 61 of
+them, the two belts pass at the same rate and the harness is cheaper. The wall
+gap is the node belt running its parts one after another.
+
+So the default is the harness, as #1335 had it, and every line #1340 changed
+goes back: the predicate, the two `TestMain` pins that name the older engine's
+suite once, the manual's three pages and the truth table in
+`bashbelt_default_test.go`.
+
+**The bound is gone too.** `task.parallel` is 0 out of the box and its hint says
+0 is no limit. The chat door handed that 0 to the run engine, and the engine's
+supervisor read a count below one as one, so a conversation's harness task ran
+one part at a time while the setting beside it promised no limit. `codeaf do`
+never asked the setting and carried a constant of four. Both roads read the one
+row now and a 0 is no bound; `codeaf do --slots <n>` names a figure for one
+run, where `0` is no limit and blank is the setting. The manual says so under
+*How many tasks run at once* and in the `codeaf do` flag table.
+
+**The tmux suite names its belt now.** Every scenario `start` launches says
+`CODEAF_TASK_BELT=node`, the road it was written for, and the launcher drops the
+runner's own value of the variable before the child starts, so a word exported
+in a developer's shell cannot choose what the suite tests. Before this the suite
+relied on the absence of a word, which is exactly the instrument fault that
+would have had a benchmark comparing the harness to itself. One new subtest,
+`TaskOnTheDefaultBelt`, launches with the variable absent and reads the run
+road off the screen, which is the only test of the default there has ever been.
+
+The one piece of machinery that had to move for this: the supervisor's drain
+reads every outstanding return before it waits for the worker goroutines. With
+no bound there can be more workers out than the return channel is deep, and a
+worker blocked on handing in its return never reaches the wait.

docs/changes/unreleased/1359-run-row-opens-its-plan.md

@@ -0,0 +1,15 @@
+---
+kind: fixed
+title: a run's row on the tasks place is the store's task, wears its state and opens its page
+pr: 1359
+surface: [chat, engine]
+invalidates:
+  - "A `/task` on the run engine drew a row that wore `working` and whose `enter` opened an empty room. The row was the one the run's door publishes for work the graph holds no node for, and the tasks place had dropped the store's own row for it because the two share a title. The store's row is drawn now: it wears the word its store status maps to (`running`, `queued`, `done`, `incomplete`, `stopped`, `your call`) and `enter` opens the task's page — the work order, the notes, and the trajectory of every command its worker ran."
+  - "internal/session's TaskNotice carried no way to tell a row the store answers for from a node of the session's own tree, and internal/tui3's planRowShown said so in its own comment: THIS SURFACE CANNOT SEE THE STORE ID. TaskNotice.PlanTask now names the store task a run's row is, in planStoreID's spelling — the same one PlanTaskRow.ID carries and PlanTaskPage is asked for — set by the door that mints both halves, carried forward by publishRunRow, and kept in the checkpoint. The title match is left to the node road, where a plan-born node's store ids are not the row's number and the node is the half with a room behind it."
+---
+
+Found by a real-model tmux drive, not by a unit test: this shipped with
+`internal/tui3` and `internal/session` green, because every piece worked and the
+place drew the wrong one of two rows. The defect was already reachable under
+`CODEAF_TASK_BELT=bash`; making the harness the default belt only promotes it
+from opt-in to default, which is why it blocks that change.

internal/config/settings.go

@@ -1312,18 +1312,15 @@ var OperatorEnvPins = []string{
 	// shape, under `make demo-home`'s terms. A row offering to persist a
 	// fixture would put a demo question in front of a person every morning.
 	"CODEAF_QUESTION_DEMO",
-	// CODEAF_TASK_BELT builds a task worker on the bash belt instead of the
-	// shipped belt (internal/session's bashbelt.go,
-	// docs/design/bash-task-loop/DESIGN.md): the one `bash` tool plus the
-	// hands that cannot be a shell command, so both arms of the comparison
-	// run from one binary. It is plumbing for the reason CODEAF_SWARM and
-	// CODEAF_SPLITGATE are — it picks which belt an experiment runs, not
-	// something the product has an opinion about — and it shares their
-	// lifetime: it disappears when the experiment has won or lost, which is
-	// exactly the lifetime a persisted setting must not have. A row would
-	// also be wrong the way the exit-code hatch is: it would put every future
-	// task worker on an experiment's belt on a machine where the variable is
-	// nowhere in sight. Unset, every worker is where it was.
+	// CODEAF_TASK_BELT sends a task worker BACK to the older node belt
+	// (internal/session's bashbelt.go, docs/design/worker-harness/DESIGN.md).
+	// The bash belt is the shipped default, so the variable is an escape
+	// hatch rather than the way in: `node`, `legacy` and `off` are the only
+	// words that turn it off, and one binary still runs both roads. It stays
+	// plumbing rather than a settings row for the reason the exit-code hatch
+	// is — a persisted row would pin a machine to the older engine long after
+	// whoever set it had forgotten, and an escape hatch must be as easy to
+	// stop using as it was to start. It disappears when the older belt does.
 	"CODEAF_TASK_BELT",
 	// CODEAF_PLANDB_BIN names the binary a bash-belt worker's `plandb` shim
 	// execs (internal/session's plandb_plan.go) when the running program is