From f920d1d7342eef2ef023d696ec0efa2436955698 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Wed, 23 Sep 2026 09:51:51 -0400 Subject: [PATCH 1/5] session, config: tests for the manager's digest, run-row verbs, live-model attribution and written-but-unregistered profile keys Written before the fixes and expected RED on santos/dev2 3f46952d4: - TestTheDigestLeadsWithTheLiveRunInTheRailsOwnWords - TestSayAndForwardOnARowOfTheLiveRunAnswerForTheRow - TestAMarkedDraftOpensOnTheRunsRows, TestAMessageWithPicturesOpensOnTheRunsRows - TestAssistedByFollowsTheModelAfterASwitch - TestProfileKeyLedgerLaw, now driving every settings row's writer --- internal/config/config_test.go | 93 ++++++++++ internal/session/attribution_model_test.go | 83 +++++++++ internal/session/plandigest_manager_test.go | 189 ++++++++++++++++++++ 3 files changed, 365 insertions(+) create mode 100644 internal/session/attribution_model_test.go create mode 100644 internal/session/plandigest_manager_test.go diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e9c4ba883f..09bcbbc126 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -622,9 +622,21 @@ func loadProfileKeyLedger(t *testing.T) map[string]bool { // in the ledger. Half (b): every ledger key is either consumed at head or in // retiredProfileKeys. Adding a settings row forces a ledger line; removing one // leaves its line behind and turns this red until the key is retired on purpose. +// +// THE WRITER SET IS WHAT THE WRITERS ACTUALLY WRITE, and not only the consumed +// set restated. It used to be derived from [consumedProfileKeys] alone, which +// made half (a) a check of the consumed set against itself: a writer that put +// down a key nothing registered was invisible to it. `lane.talk.borrow` was +// exactly that — [WriteLaneRow] writes it on every save of the `provider` row +// and [LaneBorrowAt] reads it, and because no list held it, every launch after +// the row was saved warned that it was ignored. So every settings row is now +// driven through its own writer in an empty profile ([profileKeysWrittenByRows]) +// and every key that lands in the file is held to the same two halves, plus a +// third: a key the product writes must be one the unread check knows. func TestProfileKeyLedgerLaw(t *testing.T) { ledger := loadProfileKeyLedger(t) consumed := consumedProfileKeys(t.TempDir()) + written := profileKeysWrittenByRows(t) for key := range consumed { if key == "" { @@ -634,6 +646,15 @@ func TestProfileKeyLedgerLaw(t *testing.T) { t.Errorf("writer key %q is not in testdata/profile-keys.ledger; add it", key) } } + for key, row := range written { + if !ledger[key] { + t.Errorf("the %q row writes %q, which is not in testdata/profile-keys.ledger; add it", row, key) + } + if !consumed[key] && !retiredProfileKeys[key] { + t.Errorf("the %q row writes %q, which consumedProfileKeys does not hold, so every launch after the row is saved "+ + "reports it unread: register it (a settings row, or nonSettingProfileFields when a reader consumes it)", row, key) + } + } for key := range ledger { if !consumed[key] && !retiredProfileKeys[key] { t.Errorf("ledger key %q is neither consumed at head nor in retiredProfileKeys; if its writer was removed, retire the key on purpose", key) @@ -649,6 +670,78 @@ func TestProfileKeyLedgerLaw(t *testing.T) { } } +// profileKeysWrittenByRows drives every settings row through its own writer, one +// row to one empty profile, and answers every top-level key that landed in the +// file with the row that wrote it. +// +// THE VALUE WRITTEN IS THE ROW'S OWN READING FIRST — what an untouched profile +// shows for it, which every writer must take back, because that is what a +// person saving the row unchanged sends. A row that refuses it is offered the +// few other shapes a row takes (blank, its choices, on and off, a small count), +// and a row that takes none of them fails here rather than being skipped: a row +// this law cannot write is a row whose keys it cannot see. +// +// Only one thing is kept out of it, and by the property rather than by name: +// a row pinned by an environment variable is unpinned for the drive, since a +// pinned row refuses every write and this law is about what the writer does. +func profileKeysWrittenByRows(t *testing.T) map[string]string { + t.Helper() + // writeTenure hands its value to the process environment as well as the + // file; this puts the variable back when the law is done. + t.Setenv("CODEAF_TENURE_AFTER", os.Getenv("CODEAF_TENURE_AFTER")) + written := map[string]string{} + for _, listed := range registry(t, t.TempDir()).Rows() { + if listed.Key == "" { + continue + } + if listed.Env != "" { + t.Setenv(listed.Env, "") + } + dir := t.TempDir() + row := mustRow(t, registry(t, dir), listed.Key) + candidates := append([]string{row.Value(), ""}, row.Choices...) + candidates = append(candidates, "on", "off", "auto", "1", "10", "none") + accepted := false + for _, value := range candidates { + if row.Apply(value) == nil { + accepted = true + break + } + } + if !accepted { + if name, pinned := row.PinnedBy(); pinned { + t.Logf("the %q row is pinned by %s and cannot be driven here", row.Key, name) + continue + } + t.Errorf("the %q row refused every value this law offers it (%q), so the keys its writer puts down are "+ + "invisible here: add a value it takes", row.Key, candidates) + continue + } + data, err := os.ReadFile(BudgetConfigPath(dir)) + if os.IsNotExist(err) { + // A row that keeps its value somewhere else — the chat prefs, a + // callback the surface owns — writes nothing to this file. + continue + } + if err != nil { + t.Fatalf("read what the %q row wrote: %v", row.Key, err) + } + var values map[string]json.RawMessage + if err := json.Unmarshal(data, &values); err != nil { + t.Fatalf("the %q row left a profile that is not a JSON object: %v", row.Key, err) + } + for key := range values { + if _, seen := written[key]; !seen { + written[key] = row.Key + } + } + } + if len(written) == 0 { + t.Fatal("no settings row wrote any key, so this law would pass on nothing") + } + return written +} + // TestRetiredProfileKeysAreNotReportedUnread pins that a profile carrying a // retired key is silent, and that a genuinely unknown key is still named. func TestRetiredProfileKeysAreNotReportedUnread(t *testing.T) { diff --git a/internal/session/attribution_model_test.go b/internal/session/attribution_model_test.go new file mode 100644 index 0000000000..8647815b9b --- /dev/null +++ b/internal/session/attribution_model_test.go @@ -0,0 +1,83 @@ +package session + +import ( + "fmt" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/exec" +) + +// pageNow is message[0] as the next request will carry it. +func pageNow(agent *Agent) string { + agent.mu.Lock() + defer agent.mu.Unlock() + return messageContentText(agent.messages[0]) +} + +// THE `Assisted-by` LINE NAMES THE MODEL THE CONVERSATION IS TALKING TO NOW. +// +// The line was filled once, from the model the conversation was launched on, +// and `/model` never rendered the page again — so every commit after a switch +// credited a model that had not written it. The switch now re-renders the page, +// and so does the clock's own refresh later, from the live model. +// +// AND THE SWITCH COSTS THE CACHE NOTHING IT WAS GOING TO KEEP. A prompt cache +// belongs to one model, so the new model's first request is cold whatever the +// page says; what must hold is that the model's name is the only thing that +// moved, so switching back hands the old model the page it already has cached. +func TestAssistedByFollowsTheModelAfterASwitch(t *testing.T) { + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.System = "" + config.Attribution = true + }) + launched := fmt.Sprintf(exec.AttributionAssistedBy, "test/model") + switched := fmt.Sprintf(exec.AttributionAssistedBy, "other/model") + + first := pageNow(agent) + if !strings.Contains(first, launched) { + t.Fatalf("the launch page does not name the launch model in %q", launched) + } + + agent.SetModel("other/model") + page := pageNow(agent) + if !strings.Contains(page, switched) || strings.Contains(page, launched) { + t.Fatalf("after /model the page still credits the launch model; want %q", switched) + } + if want := strings.ReplaceAll(first, "test/model", "other/model"); page != want { + t.Fatal("the switch moved more of the page than the model's name, so the old model's cached prefix cannot come back") + } + + // AND THE CLOCK'S OWN RE-RENDER, LATER, KEEPS THE LIVE MODEL. + agent.mu.Lock() + agent.rerenderSystemLocked(time.Now().Add(time.Hour)) + agent.mu.Unlock() + if page := pageNow(agent); !strings.Contains(page, switched) { + t.Fatalf("a clock refresh after /model went back to the launch model; want %q", switched) + } +} + +// SWITCHING BACK IS THE PAGE THE OLD MODEL ALREADY HAS, and a page that does not +// name the model is not touched by a switch at all. +func TestASwitchBackRestoresThePageByteForByte(t *testing.T) { + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.System = "" + config.Attribution = true + }) + first := pageNow(agent) + agent.SetModel("other/model") + agent.SetModel("test/model") + if pageNow(agent) != first { + t.Fatal("switching back did not restore the page the launch model already had") + } + + unsigned, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + config.System = "" + }) + before := pageNow(unsigned) + unsigned.SetModel("other/model") + if pageNow(unsigned) != before { + t.Fatal("a page that names no model was re-rendered by a switch") + } +} diff --git a/internal/session/plandigest_manager_test.go b/internal/session/plandigest_manager_test.go new file mode 100644 index 0000000000..3f0c62bfd6 --- /dev/null +++ b/internal/session/plandigest_manager_test.go @@ -0,0 +1,189 @@ +package session + +// The conversation as the manager of a live run: what the digest in front of +// the person's sentence leads with and what words it says, every door the +// person speaks through opening on it, and the verbs of `tasks` reaching the +// rows the digest shows. Every fixture seeds the store through its own API and +// calls no model it did not script. + +import ( + "context" + "encoding/json" + "path/filepath" + "strconv" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/plandb" +) + +// armEndedAndLiveRuns is a conversation with history behind its live run: an +// ended run of eight hand-offs, stopped, in the archive the next run left behind +// (`plan.db.1`, [planArchivePaths]), and a live run holding one open task. It is +// the shape every conversation has from its second run on. +func armEndedAndLiveRuns(t *testing.T) *Agent { + t.Helper() + dir := t.TempDir() + path := filepath.Join(dir, planStoreFilename) + var old []plandb.TaskSpec + for i := 1; i <= 8; i++ { + old = append(old, plandb.TaskSpec{ID: strconv.Itoa(i), Title: "Old work " + strconv.Itoa(i)}) + } + seedPlanStore(t, path+".1", "chat-a", old...) + ended, err := plandb.Open(path+".1", "", "", "", "") + if err != nil { + t.Fatalf("open the ended run: %v", err) + } + if err := ended.StopRoot("stopped"); err != nil { + t.Fatalf("end the old run: %v", err) + } + if err := ended.Close(); err != nil { + t.Fatalf("close the ended run: %v", err) + } + seedPlanStore(t, path, "chat-a", plandb.TaskSpec{ID: "9", Title: "Live work"}) + agent, _ := newTestAgent(t, &scriptedCompleter{}, nil) + registerBeltRunEngine(t, &beltRunDouble{}) + armPlanStore(t, agent, path, "chat-a") + return agent +} + +// THE LIVE RUN LEADS, AND EVERY ROW SAYS THE RAIL'S WORD. +// +// The rows come oldest run first, and the digest took the first eight: with +// eight rows of an ended run behind it, a conversation was handed eight +// `cancelled` rows of work long over and not one line of the run the person was +// talking about. And the words were the store's — `cancelled`, `ready`, +// `claimed` — about rows the person's side list calls `stopped`, `running`, +// `queued`, so the two described one plan in two vocabularies. +func TestTheDigestLeadsWithTheLiveRunInTheRailsOwnWords(t *testing.T) { + agent := armEndedAndLiveRuns(t) + + digest := agent.planDigest() + live := strings.Index(digest, "Live work") + if live < 0 { + t.Fatalf("the live run's open task is not in the digest at all:\n%s", digest) + } + if old := strings.Index(digest, "Old work"); old >= 0 && old < live { + t.Fatalf("an ended run's row comes before the live run's:\n%s", digest) + } + rail := map[string]bool{"queued": true, "running": true, "done": true, "stopped": true, "incomplete": true, "your call": true} + for _, line := range strings.Split(digest, "\n") { + if !strings.HasPrefix(line, "#") { + continue + } + fields := strings.Split(line, " · ") + if len(fields) < 3 { + t.Fatalf("a row carries no state word: %q", line) + } + if !rail[fields[2]] { + t.Fatalf("a row says %q, which is not a word the side list draws: %q\n%s", fields[2], line, digest) + } + } +} + +// `say` ON A ROW OF THE RUN IS A LINE TO ITS WORKER, AND `forward` SAYS WHY IT +// CANNOT BE ONE. +// +// Both used to fall through to the session tree's reader, which cannot find a +// run's rows, and came back `No task "2" in this project` about a row the +// digest had just shown the conversation. `say` is delivered the way a line to +// a run's worker is delivered — as a note — and its answer says so; `forward` +// promises to move what the task is judged by, which nothing the conversation +// holds does for a run's row, so it refuses and points at `note`. +func TestSayAndForwardOnARowOfTheLiveRunAnswerForTheRow(t *testing.T) { + agent, path := armLiveRun(t, plandb.TaskSpec{ID: "2", Title: "Move the schema"}) + tool := agent.tasksTool() + + answer, failed, err := tool.Execute(context.Background(), json.RawMessage(`{"id":"2","say":"use postgres"}`)) + if err != nil || failed { + t.Fatalf("say on a row of the live run failed (%v): %q", err, answer) + } + if strings.Contains(answer, "No task") { + t.Fatalf("say on a row of the live run denied the row exists: %q", answer) + } + if !strings.Contains(answer, "note") { + t.Fatalf("say's answer does not say its line went as a note: %q", answer) + } + store, err := plandb.Open(path, "", "", "", "") + if err != nil { + t.Fatalf("open the store: %v", err) + } + notes := store.Notes("2", 0) + _ = store.Close() + if len(notes) != 1 || notes[0].Body != "use postgres" || notes[0].Agent != plandb.NoteAgentChat { + t.Fatalf("the row carries %+v, want the one line the conversation said, in its own voice", notes) + } + + answer, failed, err = tool.Execute(context.Background(), json.RawMessage(`{"id":"2","forward":true}`)) + if err != nil { + t.Fatalf("forward: %v", err) + } + if !failed { + t.Fatalf("forward on a row of the live run was accepted: %q", answer) + } + if strings.Contains(answer, "No task") || !strings.Contains(answer, "`note`") { + t.Fatalf("forward's refusal does not say what it cannot do and point at note: %q", answer) + } +} + +// armDigestOn puts a live run behind an agent some other fixture built, so a +// door that needs its own shape of agent — a standing store, a sighted model — +// can be asked whether it opens on the rows. +func armDigestOn(t *testing.T, agent *Agent) { + t.Helper() + path := filepath.Join(t.TempDir(), planStoreFilename) + seedPlanStore(t, path, "chat-a", plandb.TaskSpec{ID: "2", Title: "Move the schema"}) + registerBeltRunEngine(t, &beltRunDouble{}) + armPlanStore(t, agent, path, "chat-a") +} + +// A DRAFT MARKED STANDING OPENS ON THE ROWS TOO. It is a sentence the person +// said while work ran, and it can make a running row wrong as surely as any +// other; the digest reached the plain sentence only. +func TestAMarkedDraftOpensOnTheRunsRows(t *testing.T) { + store := newFakeStanding(t) + completer := &scriptedCompleter{steps: []step{finalText("that cannot stand")}} + agent := standingAgent(t, completer, store, nil) + armDigestOn(t, agent) + + events, err := agent.SubmitStanding(context.Background(), "what time is it?") + if err != nil { + t.Fatalf("SubmitStanding: %v", err) + } + drainAnsweringStanding(t, events, nil) + opening := firstUserText(t, completer) + if !strings.Contains(opening, planDigestHeading) || !strings.Contains(opening, "Move the schema") { + t.Fatalf("a marked draft did not open on the run's rows:\n%s", opening) + } + if !strings.Contains(opening, standingMarkInstruction) || !strings.HasSuffix(opening, "what time is it?") { + t.Fatalf("the digest displaced the mark or the sentence:\n%s", opening) + } +} + +// A MESSAGE WITH PICTURES OPENS ON THE ROWS TOO, and its pictures still ride +// with it. A screenshot is often the very thing that changes the plan. +func TestAMessageWithPicturesOpensOnTheRunsRows(t *testing.T) { + completer := &scriptedCompleter{} + agent, workspace := newTestAgent(t, completer, withVision) + armDigestOn(t, agent) + + ctx, cancel := deadline(10 * time.Second) + defer cancel() + events, err := agent.SubmitImage(ctx, "this page is wrong", []Image{{Path: writeImage(t, workspace, "shot.png", "PNG")}}) + if err != nil { + t.Fatalf("SubmitImage: %v", err) + } + collect(t, events) + request := completer.request(0) + if len(request) == 0 { + t.Fatal("the model was never called") + } + sent := request[len(request)-1] + if words := messageContentText(sent); !strings.Contains(words, planDigestHeading) || !strings.Contains(words, "this page is wrong") { + t.Fatalf("a message with pictures did not open on the run's rows: %q", words) + } + if len(imagePartURLs(sent)) != 1 { + t.Fatalf("the picture did not ride with the digested message: %+v", sent.Content) + } +} From bf743ee6025abf0e5a0a2b8d75a2702f1eb733a6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Wed, 23 Sep 2026 09:51:59 -0400 Subject: [PATCH 2/5] session, config: the chat manages a live run in the side list's words and through every door - The plan digest leads with the live run and says each row's state through PlanTaskRow.StateWord, the one mapping the tasks listing now says too; a law holds it to internal/tui3's planStateWord. - tasks say on a row of the run is written as a note and says so; forward on one refuses and points at note, instead of both denying the row exists. - The note schema and receipt stop naming revise_assignment, a worker's verb the chat never carries; the chat-coordination design says the same. - lane.talk.borrow is a consumed key, and the ledger law drives every settings row's writer so a written-but-unregistered key fails it. - /model re-renders the page at its own clock stamp, so Assisted-by names the live model and a switch back is the page the old model has cached. - A picture message and a draft marked standing open on the digest too, and a message's text() is the person's words, never the digest in front of them. - The manual says what a note interrupts, the #1357 entry says stop and note reach split parts, and the #1209 entry records its dropped examples; the half-solved-work rule is back on the page, byte-neutral. --- .../unreleased/1209-adhd-shaped-answers.md | 4 +- .../unreleased/1357-plan-digest-on-speak.md | 2 +- docs/design/chat-coordination/DESIGN.md | 22 ++-- internal/config/config.go | 6 + internal/config/testdata/profile-keys.ledger | 1 + internal/manual/chat/permissions.md | 4 +- internal/manual/chat/worker-harness.md | 56 ++++++-- internal/session/agent.go | 43 ++++++- internal/session/image.go | 8 ++ internal/session/plandb_tasks.go | 46 +++++++ internal/session/plandigest.go | 63 ++++++++- internal/session/planstateword_law_test.go | 112 ++++++++++++++++ internal/session/prompt.go | 55 +++++++- internal/session/prompt_belt_pages_test.go | 10 +- internal/session/prompts/system.md | 12 +- internal/session/standing_mark.go | 11 +- internal/session/tools_tasks.go | 121 +++++++++++++++--- 17 files changed, 513 insertions(+), 63 deletions(-) create mode 100644 internal/session/planstateword_law_test.go diff --git a/docs/changes/unreleased/1209-adhd-shaped-answers.md b/docs/changes/unreleased/1209-adhd-shaped-answers.md index dcb4d3d665..72b50ad857 100644 --- a/docs/changes/unreleased/1209-adhd-shaped-answers.md +++ b/docs/changes/unreleased/1209-adhd-shaped-answers.md @@ -3,7 +3,9 @@ kind: changed title: chat answers number their steps, say where a job stands, and end on one next action pr: 1209 surface: [chat] -invalidates: [] +invalidates: + - "The answer section's bans carried examples: no opener (\"Sure\", \"Great question\", \"You're right\") and no closing offer (\"Want me to…\", \"Say the word and I'll…\"). The rewording kept both bans and cut the examples to \"Want me to…\" alone, to stay inside the prefix budget. A test that used `Say the word and I'll` as a sentence only the chat's page carries went on passing without testing anything; it now uses `no closing offer` and checks its sentinels are still on the page." + - "The rewording also dropped \"never hand back half-solved work\" from the definition of done, which no line of this entry said. That was not intended and #1386 put it back, as \"never a compiling scaffold, a narrowed test or half-solved work\", paid for by rewording the same section." --- The answer section of the system prompt (internal/session/prompts/system.md) diff --git a/docs/changes/unreleased/1357-plan-digest-on-speak.md b/docs/changes/unreleased/1357-plan-digest-on-speak.md index 468944e4b1..31f049bd6f 100644 --- a/docs/changes/unreleased/1357-plan-digest-on-speak.md +++ b/docs/changes/unreleased/1357-plan-digest-on-speak.md @@ -5,7 +5,7 @@ pr: 1357 surface: [chat, engine, docs] invalidates: - "Nothing told the conversation what was in flight when a person spoke. A person who said 'skip the migration' while a task was doing the migration was answered, and the task carried on: the chat could have asked with `tasks`, and the turn where somebody changes direction is exactly the turn where nothing suggests asking. While a run is live, each of the person's messages now opens with a bounded digest of the run's rows — id, title, state, newest note — and the plan-road hand-off facts say to act on the row that the person's sentence just made wrong before answering them." - - "The chat-coordination design's channel table names `revise_assignment` as the door that revises a task's brief. It is a WORKER'S verb and never the conversation's (Config.mayRevise is `InTask && tasker != nil && taskID != 0`), so the chat cannot rewrite what a running task was asked for. What it holds over a live run is `tasks` with `stop`, which stopBeltRow and stopJoinedRow resolve to the run's own row or to one hand-off that joined it, and `tasks` with `note`. It has no door at all onto an internal split part such as #1.2. The prompt and the manual say what it can do and not what the design assumed." + - "The chat-coordination design's channel table names `revise_assignment` as the door that revises a task's brief. It is a WORKER'S verb and never the conversation's (Config.mayRevise is `InTask && tasker != nil && taskID != 0`), so the chat cannot rewrite what a running task was asked for. What it holds over a live run is `tasks` with `stop`, which stopBeltRow and stopJoinedRow resolve to the run's own row or to one hand-off that joined it, and `tasks` with `note`. Both of those reach an internal split part such as #1.2 too, by the label the listing and the digest print: `stop` ends it through the store the way the task page's own stop does, and `note` writes onto it. The prompt and the manual say what it can do and not what the design assumed." --- The row bound is `planDigestRows` in `internal/session/plandigest.go`, with diff --git a/docs/design/chat-coordination/DESIGN.md b/docs/design/chat-coordination/DESIGN.md index 37f6118dc1..ec6a706400 100644 --- a/docs/design/chat-coordination/DESIGN.md +++ b/docs/design/chat-coordination/DESIGN.md @@ -38,7 +38,7 @@ Each of these is a missing reader on a channel that is already written to. | one task's page | store | chat's `tasks #N` (brief, result, checks, last steps) | `planTaskText` | | **notes** | worker `plandb task note`, engine (`AddNote`) | **screen only** (`internal/tui3` via `PlanTaskPage.Notes`); `planTasksText` does not print them | `internal/session/plandb_tasks.go:672` | | steer into a running worker | engine only: the same-step sentence, carried by `noteOwed` so a race with the turn's end cannot drop it | the worker's next round | `internal/run/bashworker.go:140-160`, `sameStepNote` | -| revise a task's brief | `revise_assignment`, written through with a version so a stale direction is refused | store; worker on its next read | `planReviseThrough`, `internal/session/plandb_plan.go:842` | +| revise a task's brief | `revise_assignment`, written through with a version so a stale direction is refused. **A worker's verb only** (`Config.mayRevise` is `InTask && tasker != nil && taskID != 0`): the chat never carries it, so the chat has no door that revises a brief | store; worker on its next read | `planReviseThrough`, `internal/session/plandb_plan.go:842` | | dependencies | `propose_task depends_on` | supervisor; the dependent's brief is given its dependencies' reports | `internal/run` | | the run's money | `CostUSD` on the run spec; a dollar limit holds while a worker is working (#1268) | supervisor | `internal/session/task_run_belt.go:81` | | what a check reads | `"Acceptance: " + leaf.Description` plus the task's declared `Checks` | the check worker | `internal/run/run.go:799`, #1220 | @@ -62,9 +62,12 @@ The chat's `tasks` tool prints each task's notes: the last one on the row, all of them (bounded) on the task's page. The data is already in `PlanTaskPage`. **Who may write.** The person, from the task sheet (exists). The chat, through -`revise_assignment` for a redirection (exists) and through a plain note for -information that is not a redirection (new: a `note` field on the unified door -in change 4, or a small tool until then). Workers, through `plandb task note` +a plain note for information that is not a redirection (new: a `note` field on +the unified door in change 4, or a small tool until then). *Correction, +2026-09-23:* this line first said the chat redirects through +`revise_assignment` "(exists)". It does not exist for the chat — the verb is a +worker's — and the chat's only answer to a row whose work is wrong is `tasks` +with `stop` and a fresh hand-off. Workers, through `plandb task note` (exists). Sibling workers thereby reach each other, which is the case in problem 1. @@ -120,14 +123,17 @@ without a card. The `needs you` count and the cards' wording are unchanged. ### 4. One door for the chat, light doors for workers -**Today.** Four chat verbs over one store: `propose_task`, `revise_assignment`, -`tasks`, cancel. Workers use `plandb add/split/note/done` through their shell. +**Today.** Three chat verbs over one store: `propose_task`, `tasks` (which reads, +stops and, on the plan road, notes a row) and cancel. `revise_assignment` is a +worker's verb and is never on the chat's belt, so the chat cannot revise a +brief today. Workers use `plandb add/split/note/done` through their shell. **Change.** A `plan` tool for the chat with actions `add`, `steer`, `note`, `stop`, `show`. `add` keeps `propose_task`'s five required fields (title, summary, brief, deliverable, acceptance) and its optional ones; that schema is -what forces a good brief and it stays at the chat's door. `steer` is -`revise_assignment`. `note` is change 1's writer. `show` is `tasks`. The old +what forces a good brief and it stays at the chat's door. `steer` would be +the chat's first door onto `revise_assignment`'s road — a new capability, not +a rename, since the chat holds no such verb today. `note` is change 1's writer. `show` is `tasks`. The old names remain as aliases for one release and then go, with the manual and `system.md` updated in the same change. diff --git a/internal/config/config.go b/internal/config/config.go index 5a15b816bb..389705924d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -369,6 +369,12 @@ var nonSettingProfileFields = []string{ KeyResponseLiftAfter, KeyResponseLiftCap, keyModelSources, + // THE TALK LANE'S BORROW FLAG. It is no row of its own — it rides the + // `provider` row's words (`pinned: cloudflare, borrow when slow`) — but + // [WriteLaneRow] writes it on EVERY save of that row, `false` included, and + // [LaneBorrowAt] reads it. Left off this list, every launch after the row + // was saved told the person their profile carried an ignored key. + LaneBorrowKey(LaneSlotTalk), } // retiredProfileKeys are top-level config.json keys that a shipped version once diff --git a/internal/config/testdata/profile-keys.ledger b/internal/config/testdata/profile-keys.ledger index 55d49ad2c6..87685de446 100644 --- a/internal/config/testdata/profile-keys.ledger +++ b/internal/config/testdata/profile-keys.ledger @@ -27,6 +27,7 @@ google_oauth_secret history.enabled lane.guard lane.talk +lane.talk.borrow linear_mode memory.consolidation memory.enabled diff --git a/internal/manual/chat/permissions.md b/internal/manual/chat/permissions.md index b27d6fa852..b5d405d6dc 100644 --- a/internal/manual/chat/permissions.md +++ b/internal/manual/chat/permissions.md @@ -882,7 +882,9 @@ Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> ``` The parenthesised id on the first line is the model that wrote the commit — -the session's configured model, whatever it is that day. +the model the conversation is talking to at that moment. Switch with `/model` +and the next commit names the model you switched to, not the one the +conversation started on. **A pull request or an issue** ends its body with a line holding an em dash, and then one sentence: diff --git a/internal/manual/chat/worker-harness.md b/internal/manual/chat/worker-harness.md index 1eb009cf4b..f0b6c78166 100644 --- a/internal/manual/chat/worker-harness.md +++ b/internal/manual/chat/worker-harness.md @@ -299,16 +299,21 @@ stop it`. ## Does the chat know what is running while I talk to it -It does, while a run is live. Everything you type arrives with the run's rows in -front of it: one line per task, the number you see on the side list, its title, -whether it is running or queued or done, and the newest note left on it. You -never see that block — the conversation reads it, and your own sentence is what -stays on the screen and in the transcript. +It does, while a run is live. Everything you send arrives with the run's rows in +front of it — a plain sentence, a message with pictures attached, and a draft you +marked standing alike: one line per task, the number you see on the side list, +its title, its state in the side list's own words (`queued`, `running`, `done`, +`stopped`, `incomplete`, `your call`), and the newest note left on it. You never +see that block — the conversation reads it, and your own sentence is what stays +on the screen and in the transcript. - **It sees rows, never results.** The block carries no result, no steps and no output; those cost context and it can ask for them with `tasks` when it has a reason to. A run wider than eight rows shows eight and says how many more there are. +- **The live run comes first.** Rows of runs this conversation finished earlier + come after the live run's, so history never pushes the work you are talking + about out of the eight. - **Only while something is open.** A conversation that has handed nothing out, or whose run has finished every row, gets none of this and pays nothing for it. - **You are looking at the same picture.** The rows it reads are the rows on your @@ -341,16 +346,41 @@ rest alone. If it acted on the wrong row, say so — and if it kept going when you meant it to drop something, the plainest fix is to name the row: "drop #2". -## Does a note actually reach the worker, when does it read it, does it have to ask for it +## say and forward on a row of a run, #2 or #2.1 + +The conversation reaches every row of a live run by the name the side list and +its own digest show — `#2` for a task it handed off, `#2.1` for a part the run +made for itself — with `tasks` and `stop` or `note`. Two more verbs of `tasks` +answer for those rows too, in words that say what happened: + +- **`say`** on a row of a run is written onto the row as a note, because on a run + that is what a line to the worker is. The answer opens "A row of the run takes + `say` as a note, so your line went through `note`:", and the worker is handed + it the way it is handed any note. +- **`forward`** on a row of a run is refused. It exists to move what a task is + judged by, and nothing the conversation holds does that for a run's task. The + refusal says so and names what does exist: `note` to put your words on the row + as information, or `stop` and a fresh hand-off when the work itself is now + wrong. -Yes, and it does not have to ask. A note left on a task is handed to that task's -worker between its steps, in the gap after one command has finished and before -it chooses the next. It is not something the worker has to think to look for. +Neither ever answers that the row does not exist. A number no run of this +conversation holds still goes on to the ordinary task reader. + +## Does a note actually reach the worker, when does it read it, does it have to ask for it -- **When.** At the next step boundary. A worker mid-command finishes that - command first. If its turn happens to be ending in that same moment there is - nothing to hand the note to, so it stays unread and the next boundary offers - it again: an unread note is a note nobody has been told. +Yes, and it does not have to ask. The run looks for unread notes each time the +worker finishes a step, and hands them over at once, through the same door your +own typing into a running conversation takes. It is not something the worker has +to think to look for, and it does not wait politely for a pause. + +- **What it interrupts.** If the worker's model is in the middle of writing its + next reply, that reply is cut off and asked again with the note in view. If the + worker is running a command in the foreground that has been going for three + seconds or more, the command is moved to the background and keeps running as a + job the worker can check on; a younger command is let finish first. +- **When it is missed.** If the worker's turn is ending in that same moment there + is nothing to hand the note to, so it stays unread and the next step offers it + again: an unread note is a note nobody has been told. - **When it is not handed over at all.** A task that finishes before its next boundary never reads the note left on it — there is nobody left to tell — and the words stay on its page for you. A worker that has just been told it is diff --git a/internal/session/agent.go b/internal/session/agent.go index cf63c39346..622f1525b8 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -699,6 +699,7 @@ func (a *Agent) setModel(model string) ModelLanding { // with the session (loop.go's [Agent.noteModelWindow]). a.noteModelWindow(model) a.scrubBlindImagePartsLocked(model) + a.followModelOnThePageLocked() a.mu.Unlock() // AND THE BEAT IS TOLD, OUTSIDE THE LOCK. Everything above is about this // session's own state; this is about a fetch somebody else will do, and a @@ -1176,6 +1177,13 @@ type userMessage struct { // the messages this turn reasons from carry the rest. said string + // lead is how many of the message's leading content parts the SESSION put + // there, for a message whose parts are not all words: a picture message the + // plan digest opens (plandigest.go's [planDigestedParts]). [userMessage.said] + // cannot carry that case, because it keeps words alone and the journal of a + // picture message must keep its pictures. Zero on every other message. + lead int + // authored marks a line the SESSION wrote rather than the person: every note // that goes through [Agent.enqueueNote] or the watch-only // [Agent.enqueueAmbient]. It is WHO SAID IT, where wake is WHAT IS OWED, and @@ -1627,7 +1635,35 @@ func (u userMessage) empty() bool { // text is the message's words — what a queued message says, with its parts left // out. It is what a reader of the queue wants: the pictures are not a line of // the conversation, and a data URL rendered into one would be unreadable. -func (u userMessage) text() string { return messageContentText(u.message) } +// +// AND IT IS THE PERSON'S WORDS, never the session's in front of them. A message +// the plan digest or a standing mark opens is read by the model whole, but what +// a reader of the message wants — the recall, the owed answer, the ask a +// `forward` carries into a task — is what the person typed, and a digest read +// back as their ask would forward the run's own rows into a worker as the +// person's sentence. +func (u userMessage) text() string { + if u.said != "" { + return u.said + } + return messageContentText(u.journaled()) +} + +// journaled is the message as the record keeps it: the person's own sentence +// where the session wrote something in front of it ([userMessage.said]), the +// message without the session's leading parts where those are separate parts +// ([userMessage.lead]), and the message itself every other time. +func (u userMessage) journaled() ai.Message { + if u.said != "" { + return textMessage("user", u.said) + } + if u.lead > 0 && u.lead <= len(u.message.Content) { + kept := u.message + kept.Content = append([]ai.ContentPart(nil), u.message.Content[u.lead:]...) + return kept + } + return u.message +} // startTurnLocked begins one turn on a transcript the caller has already // checked, with a.mu held. It is the ONE place a turn starts: Submit reaches it @@ -2486,10 +2522,7 @@ func (a *Agent) recordUserLocked(user userMessage) { // instruction the person never typed and never sees (standing_mark.go); the // turn reasons from it and nothing outlives it, because a replay is a // reading of the conversation and that paragraph was never part of one. - kept := user.message - if user.said != "" { - kept = textMessage("user", user.said) - } + kept := user.journaled() // The store's copy is taken before the journal's early return: a session // with no file still has a conversation worth keeping, and the person's own // words are the last thing that should depend on which layout they opened in. diff --git a/internal/session/image.go b/internal/session/image.go index 67248bc96c..911425ee46 100644 --- a/internal/session/image.go +++ b/internal/session/image.go @@ -121,6 +121,14 @@ func (a *Agent) SubmitImage(ctx context.Context, text string, images []Image) (< if err != nil { return nil, err } + // AND IT OPENS ON WHAT IS RUNNING, the way [Agent.Submit]'s sentence does + // (plandigest.go). A picture is often the very thing that changes the plan + // — a screenshot of the wrong page, the error the run is building on — and a + // digest the person's plain sentence carried and their picture did not + // would leave the conversation blind on exactly that turn. + if digest := a.planDigest(); digest != "" { + user = planDigestedParts(digest, user) + } a.mu.Lock() if a.closed { diff --git a/internal/session/plandb_tasks.go b/internal/session/plandb_tasks.go index 9b94c3c14a..adea7b9e5c 100644 --- a/internal/session/plandb_tasks.go +++ b/internal/session/plandb_tasks.go @@ -83,6 +83,48 @@ type PlanTaskRow struct { // Folder is the run copy this row works in. A surface says it once in the // page head and may omit only a leading change into this exact directory. Folder string + // Archived is true for a row read from an ENDED run's store, one of the + // runs this conversation finished before the one it holds now. The rows + // arrive oldest run first, so a reader that wants the live run first — the + // digest in front of the person's sentence — has to be able to tell them + // apart without reopening a store ([planDigestOrder]). + Archived bool `json:",omitempty"` +} + +// planWordRunning is the rail's word for a row whose work is under way. It is +// not [taskWordWorking], and that is the rail's own ruling rather than a slip: +// the plan list has always said `running` for a claimed row, and the word the +// conversation reads about a row is the word the person reads beside it. +const planWordRunning = "running" + +// StateWord is the row's state IN THE WORDS THE RAIL DRAWS: queued, running, +// done, stopped, incomplete, your call. It is the ONE mapping from the store's +// own words (pending, ready, claimed, failed, cancelled, paused) to the ones a +// person reads, and it lives beside the row so every reader of a row — the +// side list, the `tasks` listing, the digest in front of the person's sentence +// — says the same word about the same row. Two readings of one row were two +// states to the model: the digest said `cancelled` and `claimed` of rows the +// person saw as `stopped` and `running`. +// +// A status this build has never heard of reads as NOTHING rather than a word +// invented for it — the emptiness law, applied to a vocabulary that may grow. +func (row PlanTaskRow) StateWord() string { + if row.Stopped { + return taskWordStopped + } + switch strings.TrimSpace(row.Status) { + case string(plandb.StatusPending): + return taskWordQueued + case string(plandb.StatusReady), string(plandb.StatusClaimed), string(plandb.StatusRunning): + return planWordRunning + case string(plandb.StatusDone): + return taskWordDone + case string(plandb.StatusFailed), string(plandb.StatusCancelled): + return taskWordIncomplete + case "paused": + return taskWordYourCall + } + return "" } // PlanTaskNote is one note on a task's page: what was said, who said it, and @@ -204,6 +246,9 @@ func (a *Agent) PlanTasks() []PlanTaskRow { copies := a.planDisplayRunCopy() for _, store := range stores { dir := filepath.Dir(store.Path()) + // AN ENDED RUN'S STORE IS NAMED FOR ITS PLACE IN THE LINE (`plan.db.1`, + // [planArchivePaths]); the live run's is the plan's own path. + archived := filepath.Clean(store.Path()) != filepath.Clean(plan.path) spend := planSpendByTask(store.Path()) live := store.LiveSteps() tasks := store.Tasks(plandb.Filter{Chat: plan.chat}) @@ -216,6 +261,7 @@ func (a *Agent) PlanTasks() []PlanTaskRow { row := planTaskRow(store, dir, task, spend, live) row.Folder = a.planTaskRunCopy(task.ID) row.LiveParts = planStepDisplayFacts(PlanStep{Command: row.Live.Command}, copies.or(row.Folder), planShimFilename).Parts + row.Archived = archived rows = append(rows, row) if task.ID == root { applyPlanRootProgress(&rows[len(rows)-1], tasks, root) diff --git a/internal/session/plandigest.go b/internal/session/plandigest.go index c0b1e28c94..ac694c0e7a 100644 --- a/internal/session/plandigest.go +++ b/internal/session/plandigest.go @@ -31,14 +31,17 @@ package session // // THE PERSON NEVER SEES IT. The digest rides in the message this turn reasons // from and nowhere else: [Agent.recordUserLocked] journals their own sentence -// through [userMessage.said], so a resume, an export and the transcript on -// screen all show what they typed. That is standing_mark.go's law, applied to -// the second door that needs it. +// through [userMessage.said] (or past [userMessage.lead], on a message with +// pictures), so a resume, an export and the transcript on screen all show +// what they typed. That is standing_mark.go's law, applied to the second door +// that needs it. import ( "fmt" "strings" + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/plandb" ) @@ -91,7 +94,11 @@ func (a *Agent) planDigest() string { if !planAnyOpen(rows) { return "" } + // THE LABELS ARE TAKEN BEFORE THE ROWS ARE REORDERED. A part's `#2.1` is its + // place under its parent in the store's own order, which is the order the + // rows arrive in, so reading them after the reorder would renumber parts. labels := planTaskLabels(rows) + rows = planDigestOrder(rows) var b strings.Builder b.WriteString(planDigestHeading + "\n") shown := 0 @@ -100,7 +107,14 @@ func (a *Agent) planDigest() string { break } shown++ - fmt.Fprintf(&b, "%s · %s · %s", labels[row.ID], cutChars(row.Title, planDigestLineChars), row.Status) + // THE RAIL'S WORD AND NOT THE STORE'S ([PlanTaskRow.StateWord]). The + // person reads `stopped` and `running` beside the row; a digest that said + // `cancelled` and `claimed` of the same rows would have the conversation + // and the person describing one plan in two vocabularies. + fmt.Fprintf(&b, "%s · %s", labels[row.ID], cutChars(row.Title, planDigestLineChars)) + if word := row.StateWord(); word != "" { + fmt.Fprintf(&b, " · %s", word) + } // THE NEWEST NOTE AND NOTHING OLDER. A note is the one thing on a row // that can say the plan has gone wrong — a worker's finding, the // person's own word from the task page — and the newest is the one that @@ -120,6 +134,30 @@ func (a *Agent) planDigest() string { return b.String() } +// planDigestOrder is the rows with THE LIVE RUN FIRST, and every ended run's +// rows after it in the order they came. Store order is kept inside each half. +// +// [Agent.PlanTasks] answers ended runs oldest first and the live run last, +// which is the right order for a list a person scrolls and the wrong one for a +// digest cut at [planDigestRows]: a conversation with eight rows of history +// behind it was handed eight `cancelled` rows of runs long over and not one +// line of the run the person was talking about. The rows the bound leaves out +// are the old ones, and they are still counted. +func planDigestOrder(rows []PlanTaskRow) []PlanTaskRow { + ordered := make([]PlanTaskRow, 0, len(rows)) + for _, row := range rows { + if !row.Archived { + ordered = append(ordered, row) + } + } + for _, row := range rows { + if row.Archived { + ordered = append(ordered, row) + } + } + return ordered +} + // planAnyOpen answers whether anything in the run is still going. A run whose // every row has ended is a run nobody can steer, so its digest would be a // paragraph of history in front of every sentence the person types — which is @@ -145,3 +183,20 @@ func planDigested(digest, text string) userMessage { said: text, } } + +// planDigestedParts is [planDigested] for a message another door has already +// assembled out of parts — the person's words and the pictures they attached +// (image.go). The digest is its own leading part, and [userMessage.lead] +// counts it, so the journal keeps the pictures and the words and not the rows. +// +// EVERY DOOR THE PERSON SPEAKS THROUGH OPENS ON THE ROWS. The digest reached +// the plain sentence only, so a person who changed their mind while attaching a +// screenshot, or while marking a draft standing, was talking to a conversation +// that could not see what was running — the one failure this file exists to end. +func planDigestedParts(digest string, user userMessage) userMessage { + parts := make([]ai.ContentPart, 0, len(user.message.Content)+1) + parts = append(parts, ai.ContentPart{Type: "text", Text: digest}) + user.message.Content = append(parts, user.message.Content...) + user.lead++ + return user +} diff --git a/internal/session/planstateword_law_test.go b/internal/session/planstateword_law_test.go new file mode 100644 index 0000000000..052eebf89b --- /dev/null +++ b/internal/session/planstateword_law_test.go @@ -0,0 +1,112 @@ +package session + +import ( + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "strconv" + "testing" +) + +// THE ROW'S WORD IS ONE VOCABULARY, AND THIS IS THE LAW THAT KEEPS IT ONE. +// +// [PlanTaskRow.StateWord] is the mapping from the store's words to the ones a +// person reads, and the digest and the `tasks` listing say it. The side list +// draws its own word through internal/tui3's planStateWord, which this package +// cannot call. So this reads that function out of the tree and holds the two to +// the same table: every store word it maps, and the stopped row, must read the +// same word here. The day planStateWord simply returns row.StateWord() there is +// nothing left to compare, and the law says so and passes. +func TestTheRowsWordIsTheRailsWord(t *testing.T) { + path := filepath.Join(completerLawRoot(t), "internal", "tui3", "taskplan.go") + file, err := parser.ParseFile(token.NewFileSet(), path, nil, 0) + if err != nil { + t.Fatalf("parse the side list's word: %v", err) + } + var fn *ast.FuncDecl + for _, decl := range file.Decls { + if candidate, ok := decl.(*ast.FuncDecl); ok && candidate.Name.Name == "planStateWord" { + fn = candidate + } + } + if fn == nil { + t.Fatal("internal/tui3/taskplan.go has no planStateWord; point this law at the side list's word") + } + delegates := false + ast.Inspect(fn.Body, func(node ast.Node) bool { + if call, ok := node.(*ast.CallExpr); ok { + if selector, ok := call.Fun.(*ast.SelectorExpr); ok && selector.Sel.Name == "StateWord" { + delegates = true + } + } + return true + }) + if delegates { + t.Log("the side list says row.StateWord() itself: one mapping, nothing to compare") + return + } + + literal := func(expr ast.Expr) (string, bool) { + lit, ok := expr.(*ast.BasicLit) + if !ok || lit.Kind != token.STRING { + return "", false + } + value, err := strconv.Unquote(lit.Value) + return value, err == nil + } + returned := func(body []ast.Stmt) (string, bool) { + for _, stmt := range body { + if ret, ok := stmt.(*ast.ReturnStmt); ok && len(ret.Results) == 1 { + return literal(ret.Results[0]) + } + } + return "", false + } + + compared := 0 + for _, stmt := range fn.Body.List { + switch stmt := stmt.(type) { + case *ast.IfStmt: + // `if row.Stopped { return "stopped" }` + selector, ok := stmt.Cond.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "Stopped" { + continue + } + want, ok := returned(stmt.Body.List) + if !ok { + continue + } + if got := (PlanTaskRow{Stopped: true, Status: "cancelled"}).StateWord(); got != want { + t.Errorf("a stopped row: the side list says %q and the row says %q", want, got) + } + compared++ + case *ast.SwitchStmt: + for _, clause := range stmt.Body.List { + cc := clause.(*ast.CaseClause) + want, ok := returned(cc.Body) + if !ok { + continue + } + for _, expr := range cc.List { + status, ok := literal(expr) + if !ok { + continue + } + if got := (PlanTaskRow{Status: status}).StateWord(); got != want { + t.Errorf("store word %q: the side list says %q and the row says %q", status, want, got) + } + compared++ + } + } + } + } + // A word the side list has never heard of draws nothing there, and must + // draw nothing here. + if got := (PlanTaskRow{Status: "a-word-nobody-wrote"}).StateWord(); got != "" { + t.Errorf("an unknown store word reads %q, want nothing", got) + } + if compared < 8 { + t.Fatalf("the law compared %d words out of the side list's planStateWord; its shape moved, so this law is reading nothing", compared) + } +} diff --git a/internal/session/prompt.go b/internal/session/prompt.go index 14f551ff8c..e8d4090c61 100644 --- a/internal/session/prompt.go +++ b/internal/session/prompt.go @@ -550,11 +550,64 @@ func (a *Agent) rerenderSystemLocked(now time.Time) { if !a.systemOwn { return } - a.system = renderSystemAt(a.config, now) + a.system = renderSystemAt(a.liveModelConfigLocked(), now) a.systemAt = now a.refreshSystemLocked() } +// liveModelConfigLocked is this agent's config with the model it is talking to +// NOW in place of the one it was launched on, which is the config the page is +// rendered from. The two differ after a `/model`: [Agent.setModel] moves +// [Agent.model] and leaves [Config.Model] where the launch put it, and a page +// rendered from the launch model names that model in the one line that is +// supposed to say which model wrote the work — `Assisted-by` +// (beltfacts.go's attribution fact). +// +// The caller holds a.mu. +func (a *Agent) liveModelConfigLocked() Config { + config := a.config + if model := strings.TrimSpace(a.model); model != "" { + config.Model = model + } + return config +} + +// followModelOnThePageLocked re-renders the page after the model changed, and +// touches nothing when the page does not say which model it is. +// +// THE `Assisted-by` LINE NAMES THE MODEL, AND IT WENT STALE AFTER `/model`. The +// page was rendered once from [Config.Model] and a switch never rendered it +// again, so every commit after one still credited the model the conversation +// was launched on. +// +// WHY THIS DOES NOT COST THE PREFIX CACHE ANYTHING IT WAS STILL GOING TO HAVE. +// A prompt cache belongs to one model: the first request on the model just +// picked is written cold whatever the page says, so re-rendering it on the +// switch buys the right name for nothing. Two things keep it that way: +// +// - THE CLOCK IS NOT MOVED. The page is rendered at [Agent.systemAt], the +// moment it was last rendered, so the only bytes that change are the ones +// that follow the model. Switching back to the model before is then the +// page that model already has cached, byte for byte, rather than a second +// cold write for a newer minute. +// - A PAGE THAT DOES NOT NAME THE MODEL IS LEFT ALONE. With attribution off +// the render comes back identical and message[0] is not touched at all. +// +// A prompt this agent did not write is never re-rendered ([Agent.systemOwn]). +// +// The caller holds a.mu. +func (a *Agent) followModelOnThePageLocked() { + if !a.systemOwn { + return + } + page := renderSystemAt(a.liveModelConfigLocked(), a.systemAt) + if page == a.system { + return + } + a.system = page + a.refreshSystemLocked() +} + // readAgentsFile reads at most agentsFileLimit bytes of the workspace's // AGENTS.md and reports whether it stopped early. A missing or unreadable file // is not an error: most workspaces do not have one. diff --git a/internal/session/prompt_belt_pages_test.go b/internal/session/prompt_belt_pages_test.go index ecc928c211..1377f20b9c 100644 --- a/internal/session/prompt_belt_pages_test.go +++ b/internal/session/prompt_belt_pages_test.go @@ -26,10 +26,15 @@ import ( // shapes to pick; this page has none of those three, so these are the same test // read off the page that actually shipped — each is verified unique to // prompts/system.md by construction (measured against every other fragment). +// +// AND EACH MUST STILL BE ON THE CHAT'S PAGE, which the test now asserts. The +// closing-offer sentinel was `Say the word and I'll` until #1209 reworded the +// rule without it, and from then on it was a string no page carried: absent +// from the belt's page by default, so it tested nothing at all. var chatOnlySentences = []string{ "a working colleague in a conversation", // the chat's first line "Regex search", // the `grep` hand this belt does not carry - "Say the word and I'll", // the chat's closing-offer rule + "no closing offer", // the chat's closing-offer rule } func TestABeltWorkerReadsTheBeltsPagesAndNothingElse(t *testing.T) { @@ -49,6 +54,9 @@ func TestABeltWorkerReadsTheBeltsPagesAndNothingElse(t *testing.T) { // AND NOT ONE SENTENCE OF THE CHAT'S PAGE. Each is prompts/system.md's word // for word, and prompts/system.md is not a page this belt reads. for _, foreign := range chatOnlySentences { + if !strings.Contains(systemPrompt, foreign) { + t.Errorf("the sentinel %q is no longer on prompts/system.md, so its absence here proves nothing: pick one the chat's page still says", foreign) + } if strings.Contains(page, foreign) { t.Errorf("a belt worker's page carries %q, which only prompts/system.md has", foreign) } diff --git a/internal/session/prompts/system.md b/internal/session/prompts/system.md index 51afc38460..10bacbdab3 100644 --- a/internal/session/prompts/system.md +++ b/internal/session/prompts/system.md @@ -9,15 +9,15 @@ your tools to ground your answers. Keep continuity while delegated work runs. - Unexpected repo changes are the user's work; adapt. # The answer -Once a turn ends, only its last message stays in view; everything before it -folds into a closed "worked" line. +Once a turn ends, only its last message stays in view; the rest folds into a +closed "worked" line. - The last message carries the whole deliverable; the person cannot see earlier - messages, so never write "as above" or "see my previous message". + ones, so never write "as above" or "see my previous message". - Line one answers the question or states the outcome. Then the thing asked for, in the form asked. Then, if needed, a few lines of why. Evidence and blocking details stay complete. - An answer is a few sentences; a deliverable is as long as the work needs. If you - cannot tell which, it is an answer. "Explain", "why" or "walk me through" lift + cannot tell, it is an answer. "Explain", "why" or "walk me through" lift the limit. - Structure only where the content has it: a table for comparisons, the fewest numbered steps for a sequence, prose otherwise. No headers on short answers, @@ -31,8 +31,8 @@ folds into a closed "worked" line. it, not a permission question. A decision you need goes through `ask`, with your pick. - "Done" means the specified behavior end to end plus every named acceptance - check, never a compiling scaffold or a narrowed test. Say plainly what you - did not or could not verify. + check, never a compiling scaffold, a narrowed test or half-solved work. Say + plainly what you did not or could not verify. # Answer or change - Questions, options, comparisons, tables, plans, reviews and "not yet" are answered in words, in the reply itself and not in a file unless the person asks diff --git a/internal/session/standing_mark.go b/internal/session/standing_mark.go index 48d6ab713b..a3292f4edc 100644 --- a/internal/session/standing_mark.go +++ b/internal/session/standing_mark.go @@ -93,7 +93,16 @@ func (a *Agent) SubmitStanding(ctx context.Context, text string) (<-chan Event, if a.standingItems() == nil { return nil, errors.New(standingMarkAbsent) } - return a.submitUser(ctx, standingMarked(text)) + user := standingMarked(text) + // AND IT OPENS ON WHAT IS RUNNING, the way [Agent.Submit]'s sentence does + // (plandigest.go): a rule the person marked is still a sentence they said + // while work was underway, and it can make a running row wrong as surely as + // any other. The digest goes in front of the instruction, and the journal + // still keeps their sentence alone ([userMessage.said]). + if digest := a.planDigest(); digest != "" { + user.message = textMessage("user", digest+"\n\n"+messageContentText(user.message)) + } + return a.submitUser(ctx, user) } // standingMarked is the message itself: what the model reads, and what the diff --git a/internal/session/tools_tasks.go b/internal/session/tools_tasks.go index 2c8a24a022..677cbc7453 100644 --- a/internal/session/tools_tasks.go +++ b/internal/session/tools_tasks.go @@ -119,12 +119,18 @@ var tasksSchemaJSON = `{"type":"object","properties":{` + var tasksSchemaWithNote = strings.TrimSuffix(tasksSchemaJSON, `},"additionalProperties":false}`) + // A NOTE IS NOT `say`, AND THE DESCRIPTION IS WHERE THAT IS SETTLED. `say` // is a correction aimed at a node of this session's own tree; a note is a - // fact written onto a row of the run's plan, where the worker reads it - // between its steps and every other worker and the person can read it too. - // It says what it cannot do in the same breath, because a model that - // believed a note moved a task's work order would be changing what the work - // is judged by with no version check behind it. - `,"note":{"type":"string","description":"A fact written onto the run row named by id (\"2\", \"2.1\"): its worker reads it between its steps, and it stays on the row for the person and the other workers. It is information, not an order — it cannot change what that task was asked for or what it is judged by; revise_assignment does that. It ends nothing."}` + + // fact written onto a row of the run's plan, which the worker is handed the + // moment its current step ends ([Agent.Steer] carries it, so a reply being + // written is cut and re-asked) and every other worker and the person can + // read too. It says what it cannot do in the same breath, because a model + // that believed a note moved a task's work order would be changing what the + // work is judged by with no version check behind it. + // + // AND IT NAMES NO DOOR THAT DOES. It used to end "revise_assignment does + // that", and `revise_assignment` is a worker's verb that no conversation + // carries ([Config.mayRevise]): the schema was pointing the model at a tool + // it did not have. + `,"note":{"type":"string","description":"A fact written onto the run row named by id (\"2\", \"2.1\"): its worker is handed it when its current step ends, and it stays on the row for the person and the other workers. It is information, not an order: it cannot change what that task was asked for or is judged by. It ends nothing."}` + `},"additionalProperties":false}` // tasksArguments is the wire form. The id is RAW because a model that has just @@ -144,9 +150,10 @@ type tasksArguments struct { Continue bool `json:"continue"` Resolve string `json:"resolve"` // Note is the plan road's own field: a fact written onto one row of the - // live run, which that row's worker reads between its steps. It is decoded - // on every road, because a model that sent it where the schema does not - // offer it is better answered with a sentence than with a parse error. + // live run, which that row's worker is handed when its current step ends. + // It is decoded on every road, because a model that sent it where the schema + // does not offer it is better answered with a sentence than with a parse + // error. Note string `json:"note"` } @@ -298,8 +305,29 @@ func (a *Agent) tasksTool() bare.Tool { if note := strings.TrimSpace(parsed.Note); note != "" { return a.planNoteFromChat(token, note) } + // `say` AND `forward` ON A ROW OF THE RUN ARE ANSWERED HERE, for the + // reason `stop` is: the reader below cannot find these rows at all, so + // both used to come back `No task "2" in this project` about a row the + // digest had just shown the conversation, which is a refusal that + // denies the row exists rather than saying what the door can do. + // + // `say` is a line to the work and a note IS that line on this road, so + // it is delivered as one and the answer says it went that way. `forward` + // is refused: it promises to move what a task is judged by, and nothing + // a conversation holds moves that for a row of the run — so it says so, + // and names the doors that do exist. A `say` that rides with `continue` + // or `resolve` is that verb's reason, not a line to the work, and goes + // on to the reader below as before. + if parsed.Forward || (strings.TrimSpace(parsed.Say) != "" && !parsed.Continue && strings.TrimSpace(parsed.Resolve) == "") { + if row, want, ok := a.planRowNamed(token); ok { + if parsed.Forward { + return fmt.Sprintf(planForwardRefusal, want), true, nil + } + return a.writePlanNote(row, want, strings.TrimSpace(parsed.Say), planSayLead) + } + } // A READ OF A TASK THE RUN'S STORE HOLDS IS ANSWERED FROM THE STORE, by the - // number the rail shows for it. Anything else about it (say, stop, + // number the rail shows for it. Anything else about it (stop, // continue, settle) and any id the store does not hold go on to the // shipped reader below, exactly as before. if reading := !parsed.Continue && !parsed.Forward && !parsed.Stop && strings.TrimSpace(parsed.Say) == "" && strings.TrimSpace(parsed.Resolve) == ""; reading { @@ -1519,10 +1547,18 @@ func (a *Agent) planTasksText(rows []PlanTaskRow, query string) string { var b strings.Builder for _, row := range rows { page, _ := a.PlanTaskPage(row.ID) - if query != "" && !strings.Contains(strings.ToLower(row.Title+" "+row.Status+" "+page.Result+" "+row.Note), query) { + // THE ROW SAYS THE RAIL'S WORD ([PlanTaskRow.StateWord]), the one the + // digest in front of the person's sentence says of the same row. A search + // still matches the store's own word too, because a model that has read + // `claimed` in a worker's own output will search for it. + word := row.StateWord() + if query != "" && !strings.Contains(strings.ToLower(row.Title+" "+word+" "+row.Status+" "+page.Result+" "+row.Note), query) { continue } - fmt.Fprintf(&b, "%s · %s · %s", labels[row.ID], cutChars(row.Title, runAskLineChars), row.Status) + fmt.Fprintf(&b, "%s · %s", labels[row.ID], cutChars(row.Title, runAskLineChars)) + if word != "" { + fmt.Fprintf(&b, " · %s", word) + } if line := summaryFirstLine(page.Result, runAskLineChars); line != "" { fmt.Fprintf(&b, " · %s", line) } @@ -1600,23 +1636,62 @@ func stopReasonClause(why string) string { // A model told only that its words arrived will reach for this field again the // next time it wants a task to do something different, and a note that was read // as a direction is a task quietly working to a contract nobody versioned. So -// the answer names the door that does move a work order. +// the answer says that nothing the conversation holds moves a work order, and +// names what it does hold ([Agent.writePlanNote]). func (a *Agent) planNoteFromChat(token, note string) (string, bool, error) { + row, want, ok := a.planRowNamed(token) + if !ok { + return fmt.Sprintf("No task %q in this run. Call tasks with no arguments to see its rows.", token), true, nil + } + return a.writePlanNote(row, want, note, "") +} + +// writePlanNote puts one note onto a row this conversation has already found, +// and answers the receipt. lead is a sentence said before the receipt — the +// one `say` owes about the door its words actually went through. +// +// THE RECEIPT NAMES NO VERB THIS CONVERSATION HAS NOT GOT. It used to send the +// model to `revise_assignment`, which is a WORKER'S verb and is never on a +// conversation's belt ([Config.mayRevise]): a model told the door was there went +// looking for a tool it did not hold. What the conversation does hold over a row +// whose work is wrong is `stop`, and a fresh hand-off with the right ask. +func (a *Agent) writePlanNote(row PlanTaskRow, want, note, lead string) (string, bool, error) { + if err := a.PlanNoteFromChat(row.ID, note); err != nil { + return capitalized(err.Error()) + ".", true, nil + } + return fmt.Sprintf("%snoted on %s · %s: %s\nIts worker is handed it as soon as the step it is on ends, and it stays on the row for the person and the other workers. It does not change what that task was asked for, and nothing you hold does: if the work itself is wrong, stop it and hand off the right ask.", lead, want, cutChars(row.Title, runAskLineChars), note), false, nil +} + +// planRowNamed finds the row of this conversation's runs that a token names, +// spelled the way the model read it in the listing or the digest — `2`, `#2`, +// `#2.1` — and answers that row, its label, and false for a token that names +// none, so the caller goes on to the session tree's own reader as before. +func (a *Agent) planRowNamed(token string) (PlanTaskRow, string, bool) { rows := a.runPlanTasks() labels := planTaskLabels(rows) want := "#" + strings.TrimPrefix(strings.TrimSpace(token), "#") for _, row := range rows { - if labels[row.ID] != want { - continue - } - if err := a.PlanNoteFromChat(row.ID, note); err != nil { - return capitalized(err.Error()) + ".", true, nil + if labels[row.ID] == want { + return row, want, true } - return fmt.Sprintf("noted on %s · %s: %s\nIts worker reads it between its steps, and it stays on the row for the person and the other workers. It does not change what that task was asked for: revise_assignment is the door that does.", want, cutChars(row.Title, runAskLineChars), note), false, nil } - return fmt.Sprintf("No task %q in this run. Call tasks with no arguments to see its rows.", token), true, nil + return PlanTaskRow{}, "", false } +// planSayLead is what a `say` routed onto a run row answers before its +// receipt. The model asked for one door and was given another, and it must know +// which, because the two promise different things: a line into a node of this +// session's own tree, against a note on a row of the run that its worker is +// handed at its next step. +const planSayLead = "A row of the run takes `say` as a note, so your line went through `note`: " + +// planForwardRefusal is what `forward` answers for a row of the run. It is a +// refusal, and it says why in the words the model needs to choose again: the +// person's words cannot move what a row of the run is judged by, because +// nothing this conversation holds can, and `note` is the door that carries +// their words to the worker as information. +const planForwardRefusal = "%s is a row of the run, and `forward` does not reach one: nothing you hold can move what a run's task is judged by. Put the person's words on it with `note` — its worker is handed them as soon as the step it is on ends, as information and not as an order — or, if the work itself is now wrong, `stop` it and hand off the right ask." + // planTaskText is ONE task of the run, answered from the store: what it was // asked, what came back in full, what the run's checks found, and its last // steps, each bounded the way [Agent.AskRun] bounds a read. It answers false for @@ -1639,7 +1714,11 @@ func (a *Agent) planTaskText(rows []PlanTaskRow, token string) (string, bool) { return "", false } var b strings.Builder - fmt.Fprintf(&b, "%s · %s · %s\n\nbrief:\n%s\n", labels[id], cutChars(page.Row.Title, runAskLineChars), page.Row.Status, cutChars(page.Description, runAskBodyChars)) + fmt.Fprintf(&b, "%s · %s", labels[id], cutChars(page.Row.Title, runAskLineChars)) + if word := page.Row.StateWord(); word != "" { + fmt.Fprintf(&b, " · %s", word) + } + fmt.Fprintf(&b, "\n\nbrief:\n%s\n", cutChars(page.Description, runAskBodyChars)) if page.Result != "" { fmt.Fprintf(&b, "\nresult:\n%s\n", cutChars(page.Result, runAskBodyChars)) } From 5ac91c19051f0161ba1cf02ea5d4377dcc29966e Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Wed, 23 Sep 2026 09:53:02 -0400 Subject: [PATCH 3/5] docs: change entry for #1393 --- .../unreleased/1209-adhd-shaped-answers.md | 2 +- .../1393-chat-manages-a-live-run.md | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 docs/changes/unreleased/1393-chat-manages-a-live-run.md diff --git a/docs/changes/unreleased/1209-adhd-shaped-answers.md b/docs/changes/unreleased/1209-adhd-shaped-answers.md index 72b50ad857..1f889655ff 100644 --- a/docs/changes/unreleased/1209-adhd-shaped-answers.md +++ b/docs/changes/unreleased/1209-adhd-shaped-answers.md @@ -5,7 +5,7 @@ pr: 1209 surface: [chat] invalidates: - "The answer section's bans carried examples: no opener (\"Sure\", \"Great question\", \"You're right\") and no closing offer (\"Want me to…\", \"Say the word and I'll…\"). The rewording kept both bans and cut the examples to \"Want me to…\" alone, to stay inside the prefix budget. A test that used `Say the word and I'll` as a sentence only the chat's page carries went on passing without testing anything; it now uses `no closing offer` and checks its sentinels are still on the page." - - "The rewording also dropped \"never hand back half-solved work\" from the definition of done, which no line of this entry said. That was not intended and #1386 put it back, as \"never a compiling scaffold, a narrowed test or half-solved work\", paid for by rewording the same section." + - "The rewording also dropped \"never hand back half-solved work\" from the definition of done, which no line of this entry said. That was not intended and #1393 put it back, as \"never a compiling scaffold, a narrowed test or half-solved work\", paid for by rewording the same section." --- The answer section of the system prompt (internal/session/prompts/system.md) diff --git a/docs/changes/unreleased/1393-chat-manages-a-live-run.md b/docs/changes/unreleased/1393-chat-manages-a-live-run.md new file mode 100644 index 0000000000..750a206c11 --- /dev/null +++ b/docs/changes/unreleased/1393-chat-manages-a-live-run.md @@ -0,0 +1,19 @@ +--- +kind: fixed +title: the chat's run digest leads with the live run in the side list's words, and every door the person speaks through carries it +pr: 1393 +surface: [chat, engine, docs] +invalidates: + - "The digest in front of the person's sentence took the first eight rows in the order the store read them, oldest ended run first, so a conversation with eight rows of history saw eight `cancelled` rows and none of the live run. It now leads with the live run, and every row says the side list's word (queued, running, done, stopped, incomplete, your call) through `PlanTaskRow.StateWord`, which the `tasks` listing of a run's rows now says too. The store's words (pending, ready, claimed, failed, cancelled, paused) are no longer what the model reads about a row." + - "Only a plain sentence carried the digest. A message with pictures and a draft marked standing now open on it as well, and what a message's words are to the rest of the engine (the recall, the owed answer, the ask a `forward` carries) is the person's sentence, never the digest or the standing instruction in front of it." + - "`tasks` with `say` or `forward` on a row of a run answered `No task \"2\" in this project`. `say` is now written onto the row as a note and its answer says so; `forward` refuses and points at `note`." + - "The `note` field's description and its receipt said `revise_assignment` changes what a task was asked for. The chat never carries that verb; both now say nothing the conversation holds changes a run task's brief, and name `stop` and a fresh hand-off." + - "Saving the `provider` row wrote `lane.talk.borrow`, which no list of consumed keys held, so every later launch warned that it was ignored. It is consumed now, and `TestProfileKeyLedgerLaw` drives every settings row's writer, so a key a writer puts down that nothing registers fails the law." + - "`Assisted-by` named the model the conversation was launched on, even after `/model`. A switch now re-renders the page at the clock stamp it already had, so the line names the live model, and switching back is byte for byte the page the earlier model has cached." + - "The manual said a note reaches its worker between steps, after the command it is running. It is handed over as a steer the moment a step ends: a reply being written is cut and asked again, and a foreground command of three seconds or more is moved to the background." +--- + +The digest's mapping and the side list's are one table by law: +`TestTheRowsWordIsTheRailsWord` reads internal/tui3's `planStateWord` out of +the tree and fails on any word the two say differently, until the side list +calls `StateWord` itself. From dac46618cf545a71e4c4b2a5b896b3c1732104e6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Wed, 23 Sep 2026 09:54:52 -0400 Subject: [PATCH 4/5] session: both prefix waivers fall by the 4 bytes the answer section gave back --- internal/session/prefixbudget_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/session/prefixbudget_test.go b/internal/session/prefixbudget_test.go index 8d5febe864..279c78415b 100644 --- a/internal/session/prefixbudget_test.go +++ b/internal/session/prefixbudget_test.go @@ -476,9 +476,15 @@ const fixedPrefixTarget = 48_000 // lines and the belt-fact row were tightened in the same commit and paid part // of the bill. Fixed is 56,277, over by 94, and lean is 47,891, over by 81; // both waivers rise by that figure here, in this diff, on purpose. +// +// 2026-09-23, #1393. The answer section got back the rule #1209's rewording had +// dropped without saying so — "done" is never half-solved work — and the same +// section paid for it by saying three things in fewer words. The page came out +// four bytes lighter on both arms: fixed is 56,273 and lean is 47,887, so both +// waivers fall by 4 and again sit exactly on the measurement. const ( - fixedPrefixWaiver = 8_277 - leanPrefixWaiver = 16_391 + fixedPrefixWaiver = 8_273 + leanPrefixWaiver = 16_387 ) // THE LEAN PROFILE GETS A BUDGET OF ITS OWN (2026-09-10, the prompt diet's lane From d22b724a6d2c4a057e5a132487c73b01a13b6acf Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Wed, 23 Sep 2026 10:45:00 -0400 Subject: [PATCH 5/5] docs: the #1393 entry's title fits on one line --- docs/changes/unreleased/1393-chat-manages-a-live-run.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changes/unreleased/1393-chat-manages-a-live-run.md b/docs/changes/unreleased/1393-chat-manages-a-live-run.md index 750a206c11..20f1b5eb55 100644 --- a/docs/changes/unreleased/1393-chat-manages-a-live-run.md +++ b/docs/changes/unreleased/1393-chat-manages-a-live-run.md @@ -1,6 +1,6 @@ --- kind: fixed -title: the chat's run digest leads with the live run in the side list's words, and every door the person speaks through carries it +title: the chat's run digest leads with the live run, in the side list's words, on every message pr: 1393 surface: [chat, engine, docs] invalidates: