session: a worker can stop its own job, and a run's checker is told the work is in its copy

docs/changes/unreleased/1515-worker-jobs-and-checker-copy.md

@@ -0,0 +1,9 @@
+---
+kind: fixed
+title: a task's worker can stop its own job, and a run's checker checks its own copy
+pr: 1515
+surface: [chat, engine]
+invalidates:
+  - "A bash-belt worker's one call had to name bash: the envelope refused `jobs`, `read_document` and `manual` with \"is not on this belt\" although the belt carried them and the worker's page named two of them. One call per response may now name any tool the belt carries, so a worker reads and stops its own job with `jobs`; a name the belt does not carry is still refused."
+  - "A run's checker was never told where the work was, and could start in the person's checkout, decoded from the project folder's name. Its instructions now say the work is in its working directory, the run's copy, and that the person's checkout is not the work."
+---

internal/manual/chat/how-tasks-run.md

@@ -1403,7 +1403,9 @@ to the full log, all in the one turn. This is why a task does not `sleep` and `t
 build or test run — the waiting is done for it, and those nine `sleep N && tail` steps above
 are what the counter catches when something is polled that nobody is waiting on. A command
 started with `background: true` is the other case: a server or a sweep the task deliberately
-left running holds nothing up, and the task is asked its next step straight away.
+left running holds nothing up, and the task is asked its next step straight away. Either kind
+of job is the task's own to read and to stop, with the `jobs` tool's `output` and `kill`,
+and a job still running when the task ends is stopped with it.
 
 **A task that repeats itself is told what the work has been doing.** Before it is stopped it
 gets a `[stuck]` note, and that note now carries one more fact than the repetition itself:
@@ -1702,6 +1704,20 @@ How the restore is built depends on your workspace:
 The task's own checkout is untouched by any of this, and the restore is removed as soon as
 the answer is in.
 
+## Which folder the checker reads — did the check look at my checkout or the task's copy
+
+**A task's checker checks the task's own copy, never your checkout.** When a part of the
+work is finished, the checker — the seat that reads that part against what it was asked
+for — stands in the same copy the worker wrote in, and its instructions say so:
+the work is in its working directory, and every check and probe runs there.
+
+Your checkout is not the work while the task runs. It does not hold the result until the
+task lands, and it may hold changes of yours, or of other work, that are not this task's —
+so a check read there could pass or fail the task on the wrong diff.
+
+The checker may still read other folders, as every worker may. It writes only in its copy,
+and a write aimed anywhere else is refused before it runs.
+
 ## The check says my tests fail but they were already failing · red before the task started · my task was refused over somebody else's bug · pre-existing failures
 
 A worker committing its own edits does not move this baseline. A restored task

internal/manual/chat/worker-harness.md

@@ -557,8 +557,10 @@ same code path the tool runs, so the two cannot drift:
 - `codeaf web fetch URL` / `codeaf web search QUERY` — the belt's web verbs.
 - `codeaf image PROMPT --out PATH` — one picture the way `generate_image` makes one.
 
-A few hands a shell cannot be are kept too — the billed `read_document`, `jobs`,
-`manual`, and the web, media and services families.
+A few hands a shell cannot be are kept too, and a worker calls them directly, one call
+per response exactly as it calls `bash` — the billed `read_document`, `jobs` (which
+reads and stops a job the worker started), `manual`, and the web, media and services
+families.
 
 **A worker cannot ask you a question.** `ask` is not on its belt: the loop reaches
 the person through the plan CLI and not a consent gate, so a thing it cannot have

internal/manual/chat_test.go

@@ -799,6 +799,10 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) {
 		{"how do I open my tasks on a phone", "tasks"},
 		{"how do I get back from a task on my phone", "tasks"},
 		{"do tasks touch my working copy", "how-tasks-run"},
+		// The run's checker is told the work is in its own copy; a person who
+		// saw it read their checkout asks in these words.
+		{"did the checker read my checkout instead of the task's copy", "how-tasks-run"},
+		{"can a task stop a job it started", "how-tasks-run"},
 		// C14: repository placement, protected landings and kept dependency
 		// inheritance are reachable in the words a person uses after meeting them.
 		{"why didn't my task merge", "how-tasks-run"},

internal/run/check_copy_test.go

@@ -0,0 +1,62 @@
+package run_test
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/Agent-Field/agentfield/sdk/go/ai"
+	"github.com/Agent-Field/codeaf/internal/plandb"
+	"github.com/Agent-Field/codeaf/internal/run"
+)
+
+// TestCheckWorkerIsToldTheWorkIsInItsOwnCopy is the fresh-install checker that
+// began with `cd /tmp/fxfresh/repo`: nothing in its opening said where the work
+// was, so it decoded the person's checkout out of the project folder's name,
+// read that tree's unrelated diff, and was moved home only when a write there
+// was refused. The opening a check really gets — built by the run's own worker
+// seat, from a check the review round's store door seated — must say the work
+// is in its own working directory and that the person's checkout is not it.
+func TestCheckWorkerIsToldTheWorkIsInItsOwnCopy(t *testing.T) {
+	t.Setenv("CODEAF_TASK_BELT", "bash")
+	t.Setenv("CODEAF_PLANDB_BIN", stubCLI(t))
+	store := runOpenStore(t)
+	copyDir := filepath.Join(t.TempDir(), "h", ".codeaf", "v3", "projects", "-tmp-x-repo", "abc", "trees", "2")
+	if err := os.MkdirAll(copyDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	check, err := store.AddReviewCheck(plandb.TaskSpec{
+		ID:          "chk",
+		Title:       "check: fix the bug",
+		Description: "Acceptance: fix the bug that makes test_calc.py fail\n\nResult: one-line fix in calc.py",
+		Role:        plandb.RoleCheck,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// The opening is all this test reads, so the run is cut the moment the
+	// first request carrying it has been recorded.
+	ctx, cut := context.WithCancel(runContext(t))
+	defer cut()
+	seat := &seat{script: []step{func(context.Context, []ai.Message) (*ai.Response, error) {
+		cut()
+		return textReply("done"), nil
+	}}}
+	_, _ = run.NewBashWorker(store, copyDir, "test/model", seat).Run(run.WithStepsPerTask(ctx, 2), *check)
+
+	opening := seat.opening(t)
+	for _, want := range []string{
+		"The work is in your working directory",
+		"run every check and probe there",
+		"The person's own checkout is not the work",
+	} {
+		if !strings.Contains(opening, want) {
+			t.Errorf("the check's opening does not say %q:\n%s", want, opening)
+		}
+	}
+	if strings.Contains(opening, "/tmp/x/repo") {
+		t.Errorf("the check's opening names the person's checkout as a place:\n%s", opening)
+	}
+}

internal/run/worker_jobs_end_test.go

@@ -0,0 +1,85 @@
+package run_test
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/Agent-Field/agentfield/sdk/go/ai"
+	"github.com/Agent-Field/codeaf/internal/run"
+)
+
+// TestBashWorkerStopsItsJobsWhenItsTaskEnds: a job a worker leaves running is
+// the worker's, and it does not outlive the task. The fresh-install run left a
+// `find /` walking the disk as job 1 for the rest of its task; whatever a worker
+// did not stop itself, the end of its task must stop, process group and all.
+func TestBashWorkerStopsItsJobsWhenItsTaskEnds(t *testing.T) {
+	t.Setenv("CODEAF_TASK_BELT", "bash")
+	t.Setenv("CODEAF_PLANDB_BIN", stubCLI(t))
+	store := runOpenStore(t)
+	workspace := t.TempDir()
+	marker := filepath.Join(workspace, "group.txt")
+	// The run's own wall is generous, because a loaded machine is not what
+	// this test is about; the task is ended by cutting its run instead.
+	ctx, cut := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cut()
+	seat := &seat{script: []step{
+		// The job writes its own process group (bash starts it as the group's
+		// leader, and exec keeps the pid) and then sleeps far past the test.
+		func(context.Context, []ai.Message) (*ai.Response, error) {
+			return toolReply(`{"command":"echo $$ > group.txt; exec sleep 900","background":true}`), nil
+		},
+		// Once the job has said which group it is, the task ends with the job
+		// still running: its run is cut, the way a person's stop cuts it. HOW
+		// the task ends is not the point — every ending passes the same
+		// deferred close of the worker's seat.
+		func(context.Context, []ai.Message) (*ai.Response, error) {
+			waitForFile(t, marker)
+			cut()
+			return textReply(""), nil
+		},
+	}}
+	worker := run.NewBashWorker(store, workspace, "test/model", seat)
+	_, _ = worker.Run(run.WithStepsPerTask(ctx, 9), *store.Task(store.RootID()))
+
+	data, err := os.ReadFile(marker)
+	if err != nil {
+		t.Fatalf("the job never wrote its process group: %v", err)
+	}
+	group, err := strconv.Atoi(strings.TrimSpace(string(data)))
+	if err != nil || group <= 1 {
+		t.Fatalf("the job wrote %q, not a process group", data)
+	}
+	deadline := time.Now().Add(5 * time.Second)
+	for {
+		if err := syscall.Kill(-group, 0); errors.Is(err, syscall.ESRCH) {
+			return
+		}
+		if time.Now().After(deadline) {
+			_ = syscall.Kill(-group, syscall.SIGKILL)
+			t.Fatalf("process group %d outlived the task that started it", group)
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+}
+
+// waitForFile waits for a file a background command is about to write.
+func waitForFile(t *testing.T, path string) {
+	t.Helper()
+	deadline := time.Now().Add(10 * time.Second)
+	for {
+		if _, err := os.Stat(path); err == nil {
+			return
+		}
+		if time.Now().After(deadline) {
+			return
+		}
+		time.Sleep(20 * time.Millisecond)
+	}
+}

internal/session/bashbelt_envelope.go

@@ -13,11 +13,19 @@ import (
 //
 // THE ENVELOPE IS THE EXPERIMENT'S SECOND BET (docs/design/bash-task-loop/
 // DESIGN.md, Decision 2): a bash-belt worker gets the one-action discipline —
-// exactly one tool call per response, and it names bash — in exchange for
-// parallelism moving into the shell, where `&` and `xargs -P` have always
-// lived. The branch belt carries one tool, so a response carrying anything
-// else is not a batch, it is a response the model could not drive the belt
-// with, and running a piece of it would answer a question nobody asked.
+// exactly one tool call per response — in exchange for parallelism moving into
+// the shell, where `&` and `xargs -P` have always lived. A response carrying
+// two calls is not a batch, it is a response the model could not drive the
+// belt with, and running a piece of it would answer a question nobody asked.
+//
+// THE ONE CALL MAY NAME ANY HAND THIS BELT CARRIES, and bash is only the
+// commonest. The belt keeps `jobs`, `read_document` and `manual` because none
+// of them can be a shell command (Decision 6 and the tool table), and the
+// worker's page sends the worker to the first two. An envelope that refused
+// every name but bash made those hands present and broken at once: a worker
+// whose `find /` had become a job called `jobs` to stop it, was told `jobs`
+// was not on this belt, and the walk ran on for the rest of the task. A name
+// the belt does not carry is still refused here, before anything runs.
 //
 // THE REJECT IS THE SAME ON BOTH BRANCHES. A response whose calls are
 // addressable — every call carries a non-empty, unique id — is answered with
@@ -63,7 +71,11 @@ const bashEnvelopeStop = "stopped: four responses in a row carried no valid sing
 // bashEnvelopeFault is what is wrong with one submission under the envelope,
 // and empty for one that may run. A response with no calls is a final answer,
 // not an invalid action — ending the turn in words is how this loop finishes.
-func bashEnvelopeFault(calls []ai.ToolCall) string {
+//
+// WHAT THE BELT CARRIES IS READ OFF THE BELT ITSELF ([Agent.beltTools]), the
+// same list the request's definitions were built from, so the envelope and the
+// wire cannot come to disagree about which names a call may carry.
+func (a *Agent) bashEnvelopeFault(calls []ai.ToolCall) string {
 	if len(calls) == 0 {
 		return ""
 	}
@@ -74,8 +86,8 @@ func bashEnvelopeFault(calls []ai.ToolCall) string {
 	if !bashCallsAddressable(calls) {
 		return bashEnvelopeMark + "no action executed: a tool call carries no id, so its result could never be paired with it — send the bash call again as the provider's tool-call form"
 	}
-	if call.Function.Name != "bash" {
-		return bashEnvelopeMark + "no action executed: `" + call.Function.Name + "` is not on this belt — the one tool is bash, and what it cannot do is spelled in the belt's own page"
+	if !a.beltCarries(call.Function.Name) {
+		return bashEnvelopeMark + "no action executed: `" + call.Function.Name + "` is not on this belt — bash is the hand for files and commands, and what the belt cannot do is spelled in its own page"
 	}
 	// THE ARGUMENTS ARE READ WITH THE ONE DECODER EVERY TOOL USES, so a bash
 	// call is refused in the same words on the branch belt as on today's — a
@@ -86,6 +98,17 @@ func bashEnvelopeFault(calls []ai.ToolCall) string {
 	return ""
 }
 
+// beltCarries answers whether a tool of this name is on the belt the request
+// was built from.
+func (a *Agent) beltCarries(name string) bool {
+	for _, tool := range a.beltTools() {
+		if tool.Name == name {
+			return true
+		}
+	}
+	return false
+}
+
 // rejectBashEnvelope answers one invalid submission without running any of it
 // and without letting the malformed shape re-enter what the model reads next.
 //
@@ -223,7 +246,7 @@ func (a *Agent) enforceBashEnvelope(ctx context.Context, hub *eventHub, calls []
 	if !a.config.mayBashBelt() {
 		return false, false
 	}
-	fault := bashEnvelopeFault(calls)
+	fault := a.bashEnvelopeFault(calls)
 	if fault == "" {
 		return false, false
 	}