From 5e6e05919475fbd2e6fce718c0cc0c8ac95dc4ef Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 00:09:47 -0400 Subject: [PATCH 01/10] session: a read hand-off helper is read-only and leaves no stopped card The read hand-off admitted an ordinary quick task with the full belt (bash, commit), guarded only by a prose line, so a manager's whole directive could be taken by a "reading" helper that wrote, committed and merged, and a request for a task on a named model became in-place edits with no branch. The quick node now carries a durable read-only mark and its worker gets the audit read-only belt at the one place quick workers are built; a request the reader cannot serve hands control back to the conversation. A failed hand-off retires a never-started helper instead of cancelling it into a stopped card, and still stops one that did start. Fixes #1568 Part of #1569 Part of #1554 --- internal/session/cancel.go | 36 ++++++++++++++++ internal/session/readhandoff.go | 21 +++++++--- internal/session/readhandoff_test.go | 63 ++++++++++++++++++++++++++++ internal/session/task_audit.go | 8 ++++ internal/session/task_quick.go | 13 ++++-- internal/session/task_run.go | 22 ++++++++++ internal/session/task_store.go | 21 ++++++---- internal/session/task_test.go | 36 ++++++++++++++++ 8 files changed, 203 insertions(+), 17 deletions(-) diff --git a/internal/session/cancel.go b/internal/session/cancel.go index b5ad644421..aaed6a4eaa 100644 --- a/internal/session/cancel.go +++ b/internal/session/cancel.go @@ -167,6 +167,42 @@ func (a *Agent) cancelTask(id uint64, why string) (string, error) { // slot back by hand ([TaskGraph.handBackSlotLocked]). func (g *TaskGraph) stop(id uint64) (string, error) { return g.stopFor(id, "") } +// retireUnstarted removes a hand-off that never acquired a worker. It is not a +// stop: the read sweep is an internal optimization, so a queued helper that +// falls back to the conversation must leave no person-visible history behind. +// The graph lock makes removal race-free with the frontier; a claimed node is +// left alone because its runner already owns the transition to settlement. +func (g *TaskGraph) retireUnstarted(id uint64) bool { + if g == nil { + return false + } + g.mu.Lock() + node := g.nodes[id] + if node == nil || node.state != TaskQueued || node.claimed { + g.mu.Unlock() + return false + } + delete(g.nodes, id) + for index, ordered := range g.order { + if ordered != id { + continue + } + copy(g.order[index:], g.order[index+1:]) + g.order = g.order[:len(g.order)-1] + break + } + g.releaseChildLocked(node.parent) + cut := node.cancel + close(node.done) + g.mu.Unlock() + if cut != nil { + cut() + } + g.checkpoint() + g.planPulse() + return true +} + // stopFor is [TaskGraph.stop] with the reason whoever pulled it gave, and it is // where that reason is written down: onto the node, so the landing this stop // causes carries it, and into the line, so the hand that pulled it reads back diff --git a/internal/session/readhandoff.go b/internal/session/readhandoff.go index 6b960763a6..0eeb23ae47 100644 --- a/internal/session/readhandoff.go +++ b/internal/session/readhandoff.go @@ -63,6 +63,12 @@ const readSweepWait = 4 * time.Minute // held node is a reason to run the batch inline now, not to freeze the chat. const readSweepStartWait = 30 * time.Second +// sweepNeedsConversation is the hand-off's return road when the person's +// request needs an action belt. It is a fixed protocol marker, not an English +// intent matcher: the reader has no task, team, writing, commit, merge, or +// model-selection tools, so it must give those requests back to the chat. +const sweepNeedsConversation = "[READ_HANDOFF_NEEDS_CONVERSATION]" + // readSweep is one turn's ledger of read-only calls. It lives beside the // turn's warmBatch: same lifetime, same owner, reset by the same events that // break a reading run — any call that is not one of the four readers. @@ -140,6 +146,8 @@ func (s *readSweep) due(calls []ai.ToolCall) bool { // as the first reader's result. Everything else in the batch — the wc that // sized the reading, the write the model already knew it wanted — runs the // ordinary way beside the hand-off, since it was emitted blind to the reads. +// The quick task is read-only; its action refusal returns control through the +// ordinary fallback path. // A nil return is every failure road at once — the caller then runs the whole // batch inline exactly as if the hook had not fired, and the sweep is disabled // so the failure is not re-tried round after round. @@ -154,8 +162,9 @@ func (a *Agent) handoffReadSweep(ctx context.Context, ep *episode, hub *eventHub } } id, _, refusal := a.admitQuick(quickAsk{ - line: sweepBrief(user, sweep.glosses, readers), - title: sweepTitle(user), + line: sweepBrief(user, sweep.glosses, readers), + title: sweepTitle(user), + readOnly: true, }) if refusal.said != "" { return nil @@ -189,10 +198,12 @@ func (a *Agent) handoffReadSweep(ctx context.Context, ep *episode, hub *eventHub restResults = a.runToolsWarm(ctx, ep, rest, hub, warm) } answer, ok := a.awaitQuickAnswer(ctx, id) - if !ok || strings.TrimSpace(answer) == "" { + if !ok || strings.TrimSpace(answer) == "" || strings.TrimSpace(answer) == sweepNeedsConversation { // FALLBACK: The quick task failed, timed out, or returned an empty answer. // Clean up the task node so it does not linger in the graph consuming resources. - a.cancelTask(id, "read handoff failed; running inline") + if !a.graph().retireUnstarted(id) { + a.cancelTask(id, "read handoff failed; running inline") + } // Any non-reader calls in the batch already ran in restResults and must NOT // be run a second time. Run the readers inline and stitch the results back together. @@ -315,7 +326,7 @@ func sweepBrief(user userMessage, glosses []string, calls []ai.ToolCall) string for _, call := range calls { b.WriteString("- " + gloss(call) + "\n") } - b.WriteString("\nReturn ONLY the distilled answer to the person's question — the answer itself, not a narration of what you read. Do not write or edit any file.") + b.WriteString("\nReturn ONLY the distilled answer to the person's question — the answer itself, not a narration of what you read. Do not write or edit any file. If the person asked for work, a team action, writing, a commit, a merge, or a named model, return exactly " + sweepNeedsConversation + " so the conversation can handle it.") return b.String() } diff --git a/internal/session/readhandoff_test.go b/internal/session/readhandoff_test.go index c56e7cd08e..df18b25e89 100644 --- a/internal/session/readhandoff_test.go +++ b/internal/session/readhandoff_test.go @@ -146,6 +146,69 @@ func TestReadSweepDisabledStaysDisabled(t *testing.T) { } } +func TestReadHandoffRetiresUnstartedTask(t *testing.T) { + graph := &TaskGraph{ + nodes: map[uint64]*TaskNode{}, + order: []uint64{7}, + claims: map[uint64]int{3: 1}, + } + node := &TaskNode{ + graph: graph, + id: 7, + parent: 3, + state: TaskQueued, + done: make(chan struct{}), + } + graph.nodes[node.id] = node + if !graph.retireUnstarted(node.id) { + t.Fatal("an unstarted hand-off was not retired") + } + if graph.node(node.id) != nil { + t.Fatal("the retired hand-off remains in the graph") + } + if len(graph.order) != 0 { + t.Fatalf("retired hand-off remains in order: %v", graph.order) + } + if graph.claims[3] != 0 { + t.Fatalf("parent claim was not released: %v", graph.claims) + } + if node.stopped { + t.Fatal("retiring a helper marked it stopped") + } +} + +func TestRetireUnstartedLeavesStartedTask(t *testing.T) { + for _, testCase := range []struct { + name string + state TaskState + claimed bool + }{ + {name: "claimed", state: TaskQueued, claimed: true}, + {name: "running", state: TaskRunning}, + } { + t.Run(testCase.name, func(t *testing.T) { + graph := &TaskGraph{ + nodes: map[uint64]*TaskNode{}, + order: []uint64{8}, + } + node := &TaskNode{ + graph: graph, + id: 8, + state: testCase.state, + claimed: testCase.claimed, + done: make(chan struct{}), + } + graph.nodes[node.id] = node + if graph.retireUnstarted(node.id) { + t.Fatal("a started hand-off was retired") + } + if graph.node(node.id) != node { + t.Fatal("a started hand-off left the graph") + } + }) + } +} + // keys is the test's window into the ledger. func (s *readSweep) keys() []string { keys := make([]string, 0, len(s.targets)) diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index 9b9da009ce..1df1541aa1 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -3285,6 +3285,14 @@ func auditBelt(dir string, door auditDoor, droppings Place) []bare.Tool { return belt } +// quickReadOnlyBelt is the existing audit belt applied to a read hand-off. It +// deliberately reuses the audit allowlist and bash guard: a reader must have +// the same hard refusal boundary as an auditor, not a prose instruction that +// can be ignored by the model. +func quickReadOnlyBelt(dir string, droppings Place) []bare.Tool { + return auditBelt(dir, plainDoor(auditReadCommands), droppings) +} + // boundedResult caps what one tool call may hand back. // // A cut result is filed through [writeStub], the same content-addressed, diff --git a/internal/session/task_quick.go b/internal/session/task_quick.go index 5195812b23..fadbdc9108 100644 --- a/internal/session/task_quick.go +++ b/internal/session/task_quick.go @@ -54,9 +54,9 @@ const ( quickItemsToolName = "items" ) -// quickTaskSpec is what a quick node is, and it is deliberately four fields: -// what to do, the list it works through, what it said it would write, and how -// far down the list it has got. +// quickTaskSpec is what a quick node is: what to do, the list it works through, +// what it said it would write, how far down the list it has got, and whether +// its worker is a reader rather than a writer. // // IT IS MUTATED WHILE THE NODE RUNS, which no other spec in this package is, // and that is the one thing to know about reading it. `items` and `done` grow @@ -78,6 +78,9 @@ type quickTaskSpec struct { // done is parallel to items: done[i] says item i+1 has been ticked. It is // written only by the `items` tool and read only under the graph's lock. done []bool + // readOnly is THE SAFETY BOUND OF A READ HAND-OFF. It is persisted with the + // quick body so a resumed helper cannot silently regain the ordinary belt. + readOnly bool // waits is WHY THIS NODE WAS QUEUED BEHIND ANOTHER, kept from admission so // the receipt the model reads can name the task and the path rather than // making it work the collision out for itself. It is a record of that one @@ -389,6 +392,9 @@ type quickAsk struct { // grow a quick node — the tool and the ceiling's carry-on — ask for it the // same way and neither can build a promotion the other could not. inherit bool + // readOnly is set only by the read hand-off. The ordinary quick-task tool + // never receives this capability from its wire arguments. + readOnly bool } // askOf is the wire form read as an ask. It is a function rather than a @@ -640,6 +646,7 @@ func (a *Agent) newQuickSpec(ask quickAsk) (taskSpec, string) { } else { quick = newQuickTaskSpec(line, kept, scope) } + quick.readOnly = ask.readOnly quick.waits = a.graph().quickClaimsOn(a.config.Workspace, scope) for _, claim := range quick.waits { dependsOn = append(dependsOn, claim.id) diff --git a/internal/session/task_run.go b/internal/session/task_run.go index f8d25498ec..e202d2deae 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -7506,6 +7506,16 @@ func (a *Agent) newTaskAgent(ctx context.Context, dir string, node *TaskNode, su return a.newTaskAgentOn(ctx, dir, node, suffix, "", false) } +// quickBelt narrows only a read hand-off worker. The ordinary quick task keeps +// the belt built from its Config, while a read helper must replace that belt +// with the existing hard read-only allowlist before its first model request. +func quickBelt(node *TaskNode, dir string, droppings Place) []bare.Tool { + if node == nil || node.spec.quick == nil || !node.spec.quick.readOnly { + return nil + } + return quickReadOnlyBelt(dir, droppings) +} + // newTaskAgentOn is [Agent.newTaskAgent] with the model said outright, and it // exists for exactly one caller: the repair round, whose model is the cascade's // answer rather than the node's (repair_role.go, task_audit.go's repairNode). @@ -7867,6 +7877,18 @@ func (a *Agent) newTaskAgentOn(ctx context.Context, dir string, node *TaskNode, if err != nil { return nil, err } + if tools := quickBelt(node, dir, parent.droppingsPlace()); tools != nil { + definitions, err := toolDefinitions(tools) + if err != nil { + _ = child.Close() + return nil, err + } + child.mu.Lock() + child.tools = tools + child.definitions = definitions + child.mu.Unlock() + child.clearShelf() + } // AND WHAT THE PERSON IS REMEMBERED TO WANT, HANDED OVER RATHER THAN WAITED // FOR. The node's brief is routed once, beside the work (memory.go's // [nodeMemory]), and this worker gets the answer now if it has come back and diff --git a/internal/session/task_store.go b/internal/session/task_store.go index fde8313c37..afa45ec9ac 100644 --- a/internal/session/task_store.go +++ b/internal/session/task_store.go @@ -576,17 +576,18 @@ type taskRecord struct { Assignment *assignmentRecord `json:"assignment,omitempty"` } -// quickRecord is a quick node's body on disk: the four fields of +// quickRecord is a quick node's body on disk: the five fields of // [quickTaskSpec] that are facts about the work. `waits` is not among them — it // is a receipt for the moment of admission, and the edge it produced is already // on [taskRecord.DependsOn]. // // Done is parallel to Items, one tick per item, exactly as it is in memory. type quickRecord struct { - Line string `json:"line"` - Items []string `json:"items,omitempty"` - Done []bool `json:"done,omitempty"` - Files []string `json:"files,omitempty"` + Line string `json:"line"` + Items []string `json:"items,omitempty"` + Done []bool `json:"done,omitempty"` + Files []string `json:"files,omitempty"` + ReadOnly bool `json:"read_only,omitempty"` } // quickRecordLocked copies a quick body out, with the graph held — the lock the @@ -597,10 +598,11 @@ func quickRecordLocked(spec *quickTaskSpec) *quickRecord { return nil } return &quickRecord{ - Line: spec.line, - Items: append([]string(nil), spec.items...), - Done: append([]bool(nil), spec.done...), - Files: append([]string(nil), spec.files...), + Line: spec.line, + Items: append([]string(nil), spec.items...), + Done: append([]bool(nil), spec.done...), + Files: append([]string(nil), spec.files...), + ReadOnly: spec.readOnly, } } @@ -617,6 +619,7 @@ func (r *quickRecord) body() *quickTaskSpec { for index := range spec.done { spec.done[index] = index < len(r.Done) && r.Done[index] } + spec.readOnly = r.ReadOnly return spec } diff --git a/internal/session/task_test.go b/internal/session/task_test.go index aa14b6a933..6a17193670 100644 --- a/internal/session/task_test.go +++ b/internal/session/task_test.go @@ -1826,6 +1826,42 @@ func TestAuditBeltIsReadOnly(t *testing.T) { } } +func TestQuickWorkerCarriesReadOnlyBelt(t *testing.T) { + spec := newQuickTaskSpec("read the tree", nil, nil) + spec.readOnly = true + if restored := quickRecordLocked(spec).body(); restored == nil || !restored.readOnly { + t.Fatal("the read-only hand-off marker was not persisted") + } + + workspace := t.TempDir() + node := &TaskNode{spec: taskSpec{quick: spec}} + belt := quickBelt(node, workspace, Place{Dir: t.TempDir()}) + if belt == nil { + t.Fatal("a read-only quick worker got no belt") + } + byName := map[string]bare.Tool{} + for _, tool := range belt { + byName[tool.Name] = tool + } + for _, name := range []string{"read", "grep", "find", "ls", "bash"} { + if _, ok := byName[name]; !ok { + t.Fatalf("the reading helper cannot %q", name) + } + } + for _, name := range []string{"edit", "write", "commit", "merge", "propose_task", "team_start"} { + if _, ok := byName[name]; ok { + t.Fatalf("the reading helper carries %q", name) + } + } + text, isError, err := byName["bash"].Execute(context.Background(), json.RawMessage(`{"command":"touch forbidden.txt"}`)) + if err != nil { + t.Fatalf("bash: %v", err) + } + if !isError || !strings.HasPrefix(text, "refused:") { + t.Fatalf("write-capable bash was not refused: %q", text) + } +} + // A VERDICT NOBODY GAVE IS NOT A REFUTATION. Nothing merges without the word // VERIFIED — the frontier still fails closed — but an answer with neither word // in it is read as the NON-VERDICT it is, and it carries what the auditor From 6ce38a9ac68efa70dee3e42f2451e16dd3fdf703 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 09:56:31 -0400 Subject: [PATCH 02/10] standing: a firing that only reported a sentence comes to said, not landed A task firing that changed no file but reported a sentence was logged as landed. The rule that such a run is not "nothing" stands (its words reached the person); it now comes to said, which the outcome set already has, and only a saved change is landed. Fixes #1582 --- internal/session/standing_nothing_test.go | 8 ++++---- internal/session/standing_run.go | 7 +++++-- internal/standing/tick_test.go | 1 + 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/internal/session/standing_nothing_test.go b/internal/session/standing_nothing_test.go index ef816ce63d..5605c74c10 100644 --- a/internal/session/standing_nothing_test.go +++ b/internal/session/standing_nothing_test.go @@ -62,7 +62,7 @@ func nightly(workspace string) standing.Item { } // A CHILD THAT SAVED NOTHING AND SAID NOTHING CAME TO NOTHING, and the same -// child with one sentence to its name landed. The run folder's own marker is +// child with one sentence to its name said. The run folder's own marker is // written by the pass from this word ([standing.CameTo]), so it is the outcome // and not the folder that has to be right here. func TestAFiringThatLeftNothingBehindComesToNothing(t *testing.T) { @@ -97,8 +97,8 @@ func TestAFiringThatLeftNothingBehindComesToNothing(t *testing.T) { if err != nil { t.Fatalf("Run: %v", err) } - if outcome.Kind != "landed" { - t.Fatalf("a run with a report to give came to %q, wanted landed", outcome.Kind) + if outcome.Kind != "said" { + t.Fatalf("a run with a report to give came to %q, wanted said", outcome.Kind) } if !strings.Contains(outcome.Text, "flaky tests passed") { t.Fatalf("the run lost its own report: %q", outcome.Text) @@ -179,7 +179,7 @@ func TestWhatCountsAsARunThatCameToNothing(t *testing.T) { {false, "", "", standing.OutcomeNothing}, {false, " \n ", "", standing.OutcomeNothing}, {true, "", "", "landed"}, - {false, "the suite is green", "", "landed"}, + {false, "the suite is green", "", "said"}, {true, "the suite is green", "", "landed"}, // A run stopped on something only a person can allow is neither: it is // work waiting for them, and it is waiting whatever else it did. diff --git a/internal/session/standing_run.go b/internal/session/standing_run.go index e0b5869d44..5e9b456e9f 100644 --- a/internal/session/standing_run.go +++ b/internal/session/standing_run.go @@ -788,13 +788,16 @@ func (r *standingRunner) Run(ctx context.Context, item standing.Item, runDir, ev // AND A SENTENCE COUNTS AS SOMETHING. A nightly job that changed no file and // reported "the three flaky tests passed this time" delivered that report to // the person ([standingRunner.deliver]), and a run whose words somebody read is -// not a run that came to nothing however little it touched. +// not a run that came to nothing however little it touched. It is said, while a +// run that saved a file is landed. func standingCameTo(saved bool, report, needs string) string { switch { case needs != "": return "needs-you" - case saved || strings.TrimSpace(report) != "": + case saved: return "landed" + case strings.TrimSpace(report) != "": + return "said" } return standing.OutcomeNothing } diff --git a/internal/standing/tick_test.go b/internal/standing/tick_test.go index ad4694f978..d564528826 100644 --- a/internal/standing/tick_test.go +++ b/internal/standing/tick_test.go @@ -863,6 +863,7 @@ func TestTickWritesWhatEachRunCameTo(t *testing.T) { kind string reaped bool }{ + {"said", false}, {"landed", false}, {"needs-you", false}, {OutcomeNothing, true}, From 2b922bb3ee81b1db2187abd7b98067d4ea2ae94e Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 01:29:03 -0400 Subject: [PATCH 03/10] runs: each check has its own ceiling, unfinished checks fail the run, bad check paths are refused, kept branches reach /land - Every checker in a run drew from one checker-seat tally, so after the first few checks the rest stopped "at its spend ceiling" having spent almost nothing. The ceiling is now kept per check task (the run's task cap is still shared), and a run whose checks did not finish no longer ends done; its landing names the unfinished checks. - A run kept on a protected or moved checkout was invisible to /land. Kept, conflicted and aborted run branches are now listed and landable by /land, and editor .orig backups are classified as droppings so they never reach a task branch. Fixes #1572 Fixes #1574 --- internal/manual/chat/tasks.md | 10 ++-- internal/run/crew.go | 6 ++- internal/run/crew_context_test.go | 53 +++++++++++++++++++- internal/run/enginewire.go | 1 + internal/run/review_test.go | 31 ++++++++++++ internal/run/run.go | 25 ++++++++++ internal/session/land_run_tree_test.go | 21 ++++++++ internal/session/seatcompleter.go | 34 +++++++++++++ internal/session/spendguard.go | 36 +++++++++----- internal/session/standingtree.go | 69 ++++++++++++++++++++++++++ internal/session/standingtree_test.go | 41 +++++++++++++++ internal/session/task_run.go | 6 +++ internal/session/task_run_belt.go | 5 ++ internal/session/task_run_belt_test.go | 16 ++++++ 14 files changed, 337 insertions(+), 17 deletions(-) diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index baa53bac7c..ca0b97a582 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -1728,6 +1728,7 @@ the card, the rail, the roster and in the chat: | its brief no longer described the world | `incomplete · its brief went stale` | | it would not take a step it was asked to | `incomplete · would not take a step it was asked to` | | a check looked and named what is missing | `incomplete · the check found gaps: ` | +| a fan-out check did not finish | `incomplete · unfinished checks: ` | | something broke | `incomplete · a fault: ` | **`incomplete` is not `stopped`.** `stopped` is *you* ending the work and means nothing else @@ -1840,7 +1841,9 @@ on a protected branch, was on a different branch than when the work was cut, mov to a different commit by your own work after the cut, or was detached. The branch named there holds the finished work; the how-tasks-run page explains the exact reason. Inspect that branch and keep the delivery workflow you requested. A task finishing -does not by itself request a merge or a checkout change. +does not by itself request a merge or a checkout change. A retained run is the +exception: `/land` lists its waiting folder and merges that named run branch when +you ask. Click anywhere on the card, or press `ctrl+o` with it selected, to expand it. `enter` on the selected card opens the task's room instead. What the expansion holds, and in what order, is @@ -4681,8 +4684,9 @@ Whenever a task stops for any reason it wears its own word — `stopped` when yo `incomplete · ` otherwise — with `branch kept` and the branch name beside it. Nothing is thrown away: on every ending except a clean merge the branch is kept and named, and what the task made is committed onto that branch before it lands — so the files it -produced are listed under `changed:` and `git merge task/…` brings them over. The merge is -never done for you, because only work that was checked reaches your branch. +produced are listed under `changed:` and `git merge task/…` brings them over. For a +retained run, `/land` is the explicit merge door; an ordinary branch-kept task still +waits for you, because only work that was checked reaches your branch. ## Continue task N — keep going on a failed or finished task, No task 1 in this project diff --git a/internal/run/crew.go b/internal/run/crew.go index d04cdd47b8..1b89dff1ff 100644 --- a/internal/run/crew.go +++ b/internal/run/crew.go @@ -144,7 +144,11 @@ func CrewFactory(store *plandb.Store, workspace, profileDir string, seats Seats, // ceiling by seat, and a crew whose seats share one model would give it // nothing else to tell a check's call from a worker's. seat, _ := config.CrewTierSeat(tier) - return NewBashWorker(store, workspace, model, standing, session.SeatCompleter(seat, completerFor(model))) + completer := session.SeatCompleter(seat, completerFor(model)) + if role == plandb.RoleCheck { + completer = session.SpendScope(task.ID, completer) + } + return NewBashWorker(store, workspace, model, standing, completer) } } diff --git a/internal/run/crew_context_test.go b/internal/run/crew_context_test.go index bccf658bb3..476b9f34a3 100644 --- a/internal/run/crew_context_test.go +++ b/internal/run/crew_context_test.go @@ -16,10 +16,16 @@ import ( ) // crewCallProbe is a provider that answers nothing and counts what it was asked. -type crewCallProbe struct{ calls int } +type crewCallProbe struct { + calls int + cost float64 +} func (p *crewCallProbe) CompleteWithMessages(context.Context, []ai.Message, ...ai.Option) (*ai.Response, error) { p.calls++ + if p.cost > 0 { + return &ai.Response{Usage: &ai.Usage{Cost: &p.cost}}, nil + } return &ai.Response{}, nil } @@ -72,3 +78,48 @@ func TestCrewFactoryCarriesTheRoleSeatToTheSpendGuard(t *testing.T) { t.Fatalf("checker crossed its line: %v, probes %+v", err, probes) } } + +func TestCrewFactoryGivesEachCheckerItsOwnSpendCeiling(t *testing.T) { + dir := t.TempDir() + profile := t.TempDir() + rows, _ := json.Marshal(map[string]string{config.KeyTierWorkerModel: "vendor/shared", config.KeyTierHighModel: "vendor/shared"}) + if err := os.WriteFile(config.BudgetConfigPath(profile), rows, 0o600); err != nil { + t.Fatal(err) + } + store, err := plandb.Open(filepath.Join(dir, "plan.db"), "seat-test", "root", "Root", "check the seat") + if err != nil { + t.Fatal(err) + } + defer store.Close() + if _, err := store.AddMany([]plandb.TaskSpec{ + {ID: "review-one", Title: "Review one", Role: plandb.RoleCheck}, + {ID: "review-two", Title: "Review two", Role: plandb.RoleCheck}, + }); err != nil { + t.Fatal(err) + } + guard := &session.SpendGuard{ + Price: func(string) (float64, float64, float64, bool) { return 0, 1e-6, 0, true }, + SeatCeilings: map[crewroute.Seat]float64{crewroute.Checker: 0.01}, + CeilingAction: "checker ceiling $%.2f", + } + probes := []*crewCallProbe{} + factory := CrewFactory(store, dir, profile, Seats{}, func(model string) session.Completer { + probe := &crewCallProbe{cost: 0.01} + probes = append(probes, probe) + return guard.Wrap(model, probe) + }) + call := func(id string) error { + worker := factory(*store.Task(id)).(*BashWorker) + _, err := worker.completer.CompleteWithMessages(t.Context(), []ai.Message{{Role: "user"}}) + return err + } + if err := call("review-one"); err != nil { + t.Fatalf("first checker call: %v", err) + } + if err := call("review-one"); err == nil { + t.Fatal("second call on one checker crossed no ceiling") + } + if err := call("review-two"); err != nil { + t.Fatalf("first call on a second checker: %v", err) + } +} diff --git a/internal/run/enginewire.go b/internal/run/enginewire.go index a3ebd711d8..8811e4fdb3 100644 --- a/internal/run/enginewire.go +++ b/internal/run/enginewire.go @@ -90,6 +90,7 @@ func (engine) Start(ctx context.Context, spec session.RunSpec) session.RunSummar return session.RunSummary{ Outcome: string(outcome), Result: summary.Result, + Failure: summary.Failure, // WHICH LIMIT FIRED IS A FACT AND NOT A WORD IN THE OUTCOME SENTENCE: // the run's own typed answer crosses the seam here, mapped one for one, // so the session draws the ending out of the fact and never parses the diff --git a/internal/run/review_test.go b/internal/run/review_test.go index aa52505437..f3b8206637 100644 --- a/internal/run/review_test.go +++ b/internal/run/review_test.go @@ -213,6 +213,37 @@ func TestSupervisorLeavesADoesNotHoldFindingAsANoteOnTheLeaf(t *testing.T) { } } +func TestSupervisorDoesNotFinishWhenACheckDoesNotFinish(t *testing.T) { + store := runOpenStore(t) + ctx := runContext(t) + seat := newFakeSeat() + seat.actions["root"] = splitRoot(t, store, leafDone("l1")) + factory := func(task plandb.Task) run.Worker { + if task.Role == plandb.RoleCheck { + return funcWorker(func(context.Context, plandb.Task) (run.Report, error) { + return run.Report{}, fmt.Errorf("checker lost its provider") + }) + } + return seat.workerFor(task) + } + outcome, summary := run.Start(ctx, run.Spec{Store: store, Workspace: t.TempDir(), Slots: 2, + Limits: run.Limits{ReviewRound: true}, Factory: factory}) + if outcome != run.OutcomeIncomplete { + t.Fatalf("outcome = %q, want incomplete", outcome) + } + if !strings.Contains(summary.Failure, "unfinished checks: check: leaf") { + t.Fatalf("failure = %q, want the unfinished check named", summary.Failure) + } + check := tasksWithRole(store, plandb.RoleCheck) + if len(check) != 1 || check[0].Status != plandb.StatusFailed { + t.Fatalf("checks = %+v, want one failed check", check) + } + root := store.Task(store.RootID()) + if root.Status != plandb.StatusFailed || !strings.Contains(root.Error, "unfinished checks: check: leaf") { + t.Fatalf("root = %+v, want failed with the unfinished check named", root) + } +} + // TestSupervisorRootWaitsOnAnOpenCheckTask proves the waiting the round relies // on: the store's own CanFinish refuses the root while a check stands open, // naming the check, which is the shape completeTree reads to hold the root's diff --git a/internal/run/run.go b/internal/run/run.go index d825d91428..f01dabbbb9 100644 --- a/internal/run/run.go +++ b/internal/run/run.go @@ -1237,10 +1237,31 @@ func (s *Supervisor) completeTree() { return } if s.treeTerminal() { + if unfinished := unfinishedCheckNames(s.store.Tasks()); len(unfinished) > 0 { + s.rootFailed = true + s.rootFailure = "unfinished checks: " + strings.Join(unfinished, ", ") + return + } _ = s.store.CompleteRoot(s.rootResult) } } +func unfinishedCheckNames(tasks []*plandb.Task) []string { + var names []string + for _, task := range tasks { + if task.Role != plandb.RoleCheck || task.Status == plandb.StatusDone { + continue + } + name := strings.TrimSpace(task.Title) + if name == "" { + name = task.ID + } + names = append(names, name) + } + sort.Strings(names) + return names +} + // rootAwaitingWake answers whether the root still owes a wake: a landing of its // children it has not been given, or a waking worker of its own still running. func (s *Supervisor) rootAwaitingWake() bool { @@ -1887,6 +1908,9 @@ type Summary struct { // Result is the root's own result: what the run's last worker reported // when the tree finished whole, and empty whenever it did not. Result string + // Failure is the run's own account when it did not finish, including a + // checker that ended without completing its proof. + Failure string // Limit is which bound a person set ended the run, and empty on every // run that did not end on one. The outcome word is the same sentence for // both limits; this is what tells them apart. @@ -2007,6 +2031,7 @@ func Start(ctx context.Context, spec Spec) (Outcome, Summary) { return outcome, Summary{ Outcome: outcome, Result: result, + Failure: supervisor.rootFailure, Limit: supervisor.limitHit, Program: supervisor.rootProgram, Verdict: supervisor.rootVerdict, diff --git a/internal/session/land_run_tree_test.go b/internal/session/land_run_tree_test.go index ad21e1d951..913bfea56c 100644 --- a/internal/session/land_run_tree_test.go +++ b/internal/session/land_run_tree_test.go @@ -43,6 +43,27 @@ func TestLandRunTreeCommitsTheTreesOwnWorkOntoItsBranch(t *testing.T) { } } +func TestLandRunTreeDoesNotCarryOrigBackupsOntoTheTaskBranch(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo := newTestRepo(t) + writeFile(t, filepath.Join(repo, "split", "textkit.go.orig"), "editor backup\n") + writeFile(t, filepath.Join(repo, "split", "textkit.go"), "package split\n") + + branch, changed, refusal, err := LandRunTree(repo, "", "split the text kit", "") + if err != nil || refusal != "" { + t.Fatalf("LandRunTree = branch %q changed %v refusal %q error %v", branch, changed, refusal, err) + } + if strings.Contains(strings.Join(changed, "\n"), ".orig") { + t.Fatalf("changed paths include an editor backup: %v", changed) + } + if got := gitOut(t, repo, "ls-tree", "-r", "--name-only", branch); strings.Contains(got, ".orig") { + t.Fatalf("task branch carries an editor backup:\n%s", got) + } + if !strings.Contains(gitOut(t, repo, "show", "--name-only", "--format=", "HEAD"), "split/textkit.go") { + t.Fatal("task branch omitted the real source file") + } +} + // A TREE WITH NOTHING TO LAND IS A REFUSAL, NOT A FAULT: the branch would // carry what it always carried, so the door names no branch and says nothing // happened. diff --git a/internal/session/seatcompleter.go b/internal/session/seatcompleter.go index 0b4effa5e2..fd23977eeb 100644 --- a/internal/session/seatcompleter.go +++ b/internal/session/seatcompleter.go @@ -23,6 +23,8 @@ import ( // makes, fallbacks included. type crewSeatContextKey struct{} +type spendScopeContextKey struct{} + // crewSeatOf is the seat a call was made for, empty for a call no crew seat // made (an auxiliary call, a probe), which no seat's ceiling holds. func crewSeatOf(ctx context.Context) crewroute.Seat { @@ -30,6 +32,11 @@ func crewSeatOf(ctx context.Context) crewroute.Seat { return seat } +func spendScopeOf(ctx context.Context) string { + scope, _ := ctx.Value(spendScopeContextKey{}).(string) + return scope +} + // SeatCompleter is next with every call marked as the seat's, so a guard // attributes the call's spend to that seat even when another seat runs the // same model or a fallback moves the seat to another. It keeps next's model @@ -43,6 +50,17 @@ func SeatCompleter(seat crewroute.Seat, next Completer) Completer { return marked } +// SpendScope marks a completer with the concrete task whose seat spend it +// owns. The checker ceiling is per checker task, while the run's task cap stays +// shared by the guard that wraps all of its workers. +func SpendScope(scope string, next Completer) Completer { + marked := spendScopeCompleter{scope: scope, next: next} + if chain, ok := next.(modelChain); ok { + return spendScopeChain{spendScopeCompleter: marked, chain: chain} + } + return marked +} + // seatCompleter is one seat's completer with its calls marked. type seatCompleter struct { seat crewroute.Seat @@ -60,3 +78,19 @@ type seatChain struct { } func (c seatChain) FallbackModels(model string) []string { return c.chain.FallbackModels(model) } + +type spendScopeCompleter struct { + scope string + next Completer +} + +func (c spendScopeCompleter) CompleteWithMessages(ctx context.Context, messages []ai.Message, options ...ai.Option) (*ai.Response, error) { + return c.next.CompleteWithMessages(context.WithValue(ctx, spendScopeContextKey{}, c.scope), messages, options...) +} + +type spendScopeChain struct { + spendScopeCompleter + chain modelChain +} + +func (c spendScopeChain) FallbackModels(model string) []string { return c.chain.FallbackModels(model) } diff --git a/internal/session/spendguard.go b/internal/session/spendguard.go index 293f81c32e..0b1db51efa 100644 --- a/internal/session/spendguard.go +++ b/internal/session/spendguard.go @@ -177,7 +177,12 @@ type SpendGuard struct { mu sync.Mutex modelSpent map[string]float64 - seatSpent map[crewroute.Seat]*SpendTask + seatSpent map[spendTallyKey]*SpendTask +} + +type spendTallyKey struct { + seat crewroute.Seat + scope string } // SpendTask is what one task has spent and holds in flight, across every @@ -232,17 +237,24 @@ func (g *SpendGuard) tally() *SpendTask { return g.Task } -// seatTally holds one seat's spend and in-flight estimates across model changes. +// seatTally preserves the direct guard test and auxiliary-call API: no scope +// means the guard's historical one-tally-per-seat behavior. func (g *SpendGuard) seatTally(seat crewroute.Seat) *SpendTask { + return g.seatTallyFor(context.Background(), seat) +} + +// seatTallyFor holds one seat's spend within the marked task scope. +func (g *SpendGuard) seatTallyFor(ctx context.Context, seat crewroute.Seat) *SpendTask { g.mu.Lock() defer g.mu.Unlock() if g.seatSpent == nil { - g.seatSpent = make(map[crewroute.Seat]*SpendTask) + g.seatSpent = make(map[spendTallyKey]*SpendTask) } - if g.seatSpent[seat] == nil { - g.seatSpent[seat] = &SpendTask{} + key := spendTallyKey{seat: seat, scope: spendScopeOf(ctx)} + if g.seatSpent[key] == nil { + g.seatSpent[key] = &SpendTask{} } - return g.seatSpent[seat] + return g.seatSpent[key] } // ErrSpendStopped is a call the guard did not make. Its text is the one @@ -290,7 +302,7 @@ func (g *SpendGuard) before(ctx context.Context, model string, messages []ai.Mes if g.TaskCap > 0 && g.tally().Total() >= g.TaskCap { return 0, ErrSpendStopped{Action: g.TaskAction} } - if seat := crewSeatOf(ctx); g.SeatCeilings[seat] > 0 && g.seatTally(seat).Total() >= g.SeatCeilings[seat] { + if seat := crewSeatOf(ctx); g.SeatCeilings[seat] > 0 && g.seatTallyFor(ctx, seat).Total() >= g.SeatCeilings[seat] { ceiling := g.SeatCeilings[seat] return 0, ErrSpendStopped{Action: fmt.Sprintf(g.CeilingAction, ceiling)} } @@ -312,13 +324,13 @@ func (g *SpendGuard) before(ctx context.Context, model string, messages []ai.Mes } seat := crewSeatOf(ctx) ceiling := g.SeatCeilings[seat] - if ceiling > 0 && !g.seatTally(seat).hold(est, ceiling) { + if ceiling > 0 && !g.seatTallyFor(ctx, seat).hold(est, ceiling) { return 0, ErrSpendStopped{Action: fmt.Sprintf(g.CeilingAction, ceiling)} } task := g.tally() if !task.hold(est, g.TaskCap) { if ceiling > 0 { - g.seatTally(seat).settle(est, 0) + g.seatTallyFor(ctx, seat).settle(est, 0) } return 0, ErrSpendStopped{Action: g.TaskAction} } @@ -326,7 +338,7 @@ func (g *SpendGuard) before(ctx context.Context, model string, messages []ai.Mes if !fits { task.settle(est, 0) if ceiling > 0 { - g.seatTally(seat).settle(est, 0) + g.seatTallyFor(ctx, seat).settle(est, 0) } return 0, ErrSpendStopped{Action: g.CapAction} } @@ -365,7 +377,7 @@ func (g *SpendGuard) after(ctx context.Context, model string, response *ai.Respo g.Day.settle(model, held, 0) task.settle(held, 0) if seat := crewSeatOf(ctx); g.SeatCeilings[seat] > 0 { - g.seatTally(seat).settle(held, 0) + g.seatTallyFor(ctx, seat).settle(held, 0) } return } @@ -382,7 +394,7 @@ func (g *SpendGuard) after(ctx context.Context, model string, response *ai.Respo g.Day.settle(model, held, usd) task.settle(held, usd) if seat := crewSeatOf(ctx); g.SeatCeilings[seat] > 0 { - g.seatTally(seat).settle(held, usd) + g.seatTallyFor(ctx, seat).settle(held, usd) } if usd <= 0 { return diff --git a/internal/session/standingtree.go b/internal/session/standingtree.go index 9437f45435..4c5992ba37 100644 --- a/internal/session/standingtree.go +++ b/internal/session/standingtree.go @@ -295,6 +295,7 @@ func (a *Agent) StandingTrees() []StandingTree { // chip appears exactly when there is something to land. func (a *Agent) UnlandedChanges() []StandingChange { var out []StandingChange + seen := map[string]bool{} for _, tree := range a.StandingTrees() { if len(tree.Wrote) == 0 { continue @@ -304,11 +305,53 @@ func (a *Agent) UnlandedChanges() []StandingChange { Name: filepath.Base(tree.Folder), Files: len(tree.Wrote), }) + seen[tree.Folder] = true + } + for _, row := range a.keptRunRows() { + if row.Copy == nil || seen[row.Copy.Root] { + continue + } + out = append(out, StandingChange{ + Folder: row.Copy.Root, + Name: filepath.Base(row.Copy.Root), + Files: len(row.Changed), + }) + seen[row.Copy.Root] = true } sort.SliceStable(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out } +func (a *Agent) keptRunRows() []TaskNotice { + g := a.tasker() + if g == nil { + return nil + } + g.mu.Lock() + defer g.mu.Unlock() + var out []TaskNotice + for _, row := range g.runRowsLocked() { + if !row.State.settled() || row.Copy == nil || strings.TrimSpace(row.Copy.Root) == "" || row.Branch == "" || len(row.Changed) == 0 { + continue + } + switch row.Merge { + case mergeKept, mergeConflicted, mergeAborted: + out = append(out, row) + } + } + return out +} + +func (a *Agent) keptRunFor(folder string) (TaskNotice, bool) { + folder = canonicalPath(strings.TrimSpace(folder)) + for _, row := range a.keptRunRows() { + if canonicalPath(row.Copy.Root) == folder { + return row, true + } + } + return TaskNotice{}, false +} + // standingTreeFor is the copy this conversation holds of one folder, and // whether it holds one at all. func (a *Agent) standingTreeFor(folder string) (StandingTree, bool) { @@ -691,6 +734,9 @@ func (a *Agent) Land(folder string) (FolderLanding, error) { if name == "" { return FolderLanding{}, errors.New("nothing is waiting to go into a folder") } + if row, ok := a.keptRunFor(name); ok { + return a.landKeptRun(row) + } tree, ok := a.standingTreeFor(name) if !ok { return FolderLanding{}, fmt.Errorf("nothing is waiting for %s", filepath.Base(name)) @@ -746,6 +792,29 @@ func (a *Agent) Land(folder string) (FolderLanding, error) { return landing, nil } +// landKeptRun is the explicit /land road for a run branch that automatic +// landing deliberately left alone. The person has named this action, so the +// protected-branch guard no longer applies; git still refuses dirty or +// conflicting ground and the retained row remains the recovery path then. +func (a *Agent) landKeptRun(row TaskNotice) (FolderLanding, error) { + root, branch := canonicalPath(row.Copy.Root), strings.TrimSpace(row.Branch) + landing := FolderLanding{Folder: root, Name: filepath.Base(root), Files: append([]string(nil), row.Changed...)} + unlock := lockGitRoot(a.placeHere(), root) + defer unlock() + if _, err := git(root, "merge", "--no-edit", branch); err != nil { + _, _ = git(root, "merge", "--abort") + landing.Merged = mergeConflicted + landing.Note = "its branch " + branch + " did not merge cleanly and was kept — inspect the retained branch before deciding what to do next" + return landing, nil + } + landing.Merged = mergeMerged + _, _ = git(root, "branch", "-d", branch) + if graph := a.tasker(); graph != nil { + graph.keepRunRows(row.ID, nil) + } + return landing, nil +} + // retireUntouchedTree takes back a working copy NOTHING HAS BEEN WRITTEN INTO. // // It exists for one moment, and the order of that moment cannot be otherwise: a diff --git a/internal/session/standingtree_test.go b/internal/session/standingtree_test.go index 08acff42b6..0cdf3480af 100644 --- a/internal/session/standingtree_test.go +++ b/internal/session/standingtree_test.go @@ -104,6 +104,47 @@ func TestTheFirstWriteIntoAReferredFolderCutsOneCopyAndTheSecondReusesIt(t *test } } +// A KEPT RUN IS STILL A LANDING, not a private branch somebody must discover +// with git. The standing list is the one road /land reads, so a run row left +// by automatic landing has to enter that list and use the same explicit door. +func TestLandFindsAndMergesAKeptRunBranch(t *testing.T) { + repo := newTestRepo(t) + agent, _, _ := standingLab(t, repo) + mustGit(t, repo, "checkout", "-b", "task/retained") + writeFile(t, filepath.Join(repo, "landed.txt"), "from the kept branch\n") + mustGit(t, repo, "add", "landed.txt") + mustGit(t, repo, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-m", "retained") + mustGit(t, repo, "checkout", "work") + + agent.graph().keepRunRows(71, []TaskNotice{{ + ID: 71, + Title: "retained run", + State: TaskDone, + Changed: []string{"landed.txt"}, + Branch: "task/retained", + Merge: mergeKept, + Copy: &TaskCopyRecord{Root: repo, Branch: "task/retained"}, + }}) + + waiting := agent.UnlandedChanges() + if len(waiting) != 1 || waiting[0].Folder != repo { + t.Fatalf("kept run is not waiting to land: %+v", waiting) + } + landing, err := agent.Land("") + if err != nil { + t.Fatalf("Land: %v", err) + } + if landing.Merged != mergeMerged { + t.Fatalf("kept run landing = %+v, want merged", landing) + } + if got := readFile(t, filepath.Join(repo, "landed.txt")); got != "from the kept branch\n" { + t.Fatalf("landed file = %q", got) + } + if got := gitOut(t, repo, "branch", "--list", "task/retained"); strings.TrimSpace(got) != "" { + t.Fatalf("kept branch survived landing: %q", got) + } +} + // AND THE MODEL SEES ITS OWN WORK. A write followed by a read of the same path // must answer what was written — the copy is that folder's truth for this // conversation — while a file the conversation never touched is still read diff --git a/internal/session/task_run.go b/internal/session/task_run.go index e202d2deae..b8a23672c0 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -7020,6 +7020,9 @@ func readTaskDropping(dir, name string) ([]byte, error) { func isTaskDropping(path string) bool { clean := filepath.ToSlash(filepath.Clean(strings.TrimSpace(path))) + if strings.HasSuffix(clean, ".orig") { + return true + } for _, name := range taskDroppingNames() { if clean == name || strings.HasPrefix(clean, name+"/") { return true @@ -9415,6 +9418,9 @@ func beltTreeWork(dir string) []string { if harnessWrote(path) { continue } + if isTaskDropping(path) { + continue + } // AN UNTRACKED BUILD CACHE IS NOT THE WORK EITHER, and it is a // separate, narrower question ([buildCache]): exact cache names, and // only for a file git has never been told about. A tracked cache that diff --git a/internal/session/task_run_belt.go b/internal/session/task_run_belt.go index 43acbaa7a9..12754867b2 100644 --- a/internal/session/task_run_belt.go +++ b/internal/session/task_run_belt.go @@ -235,6 +235,8 @@ const ( type RunSummary struct { Outcome string Result string + // Failure is the run's own account when it did not finish. + Failure string // Limit is empty on every run that did not end on a bound its person set. Limit RunLimit // Program is how a delegated run's program ended when it did not finish, @@ -2315,6 +2317,9 @@ func runEndingWords(summary RunSummary) (string, string) { if ended := summary.Program; ended != nil && summary.Outcome != beltRunOutcomeDone { return strings.TrimSpace(ended.Reason), strings.TrimSpace(ended.Result) } + if summary.Outcome != beltRunOutcomeDone && strings.TrimSpace(summary.Failure) != "" { + return summary.Outcome, strings.TrimSpace(summary.Failure) + } return summary.Outcome, strings.TrimSpace(summary.Result) } diff --git a/internal/session/task_run_belt_test.go b/internal/session/task_run_belt_test.go index 8ea70add72..270a4aca94 100644 --- a/internal/session/task_run_belt_test.go +++ b/internal/session/task_run_belt_test.go @@ -772,6 +772,22 @@ func TestLandingDigestIsUnchangedWithoutAStoredSummary(t *testing.T) { } } +func TestLandingDigestNamesUnfinishedChecks(t *testing.T) { + store, err := plandb.Open(filepath.Join(t.TempDir(), planStoreFilename), "run", planRootID, "The run", "person ask") + if err != nil { + t.Fatal(err) + } + defer store.Close() + got := beltRunOutcomeNote(store, planRootID, RunSummary{ + Outcome: "incomplete", + Failure: "unfinished checks: check: leaf", + }, RunLanding{}, 0) + want := "incomplete · unfinished checks: check: leaf" + if got != want { + t.Fatalf("landing digest = %q, want %q", got, want) + } +} + // landingRunDouble ends synchronously so the test can observe the exact order: // the summary refresh must have stored its sentence before the outcome note is // composed. From d85f429e4965f35362d1a589ed408d0352c37888 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 10:13:36 -0400 Subject: [PATCH 04/10] tasks: a kept branch says why, and /land lists a single task's kept branch The guard that tasks do not merge into a protected checkout stays. But a single task's kept branch never reached /land (only fan-out run rows did), and the landing line said "branch kept" without the reason the guard knew. Kept branches now come from one source over task nodes and run rows, which the standing chip, its preview and /land all read, and the reason (on main, moved, detached) is carried on the notice and drawn with the line. Fixes #1574 --- internal/manual/chat/tasks.md | 22 ++-- internal/session/standingtree.go | 121 +++++++++++++++------ internal/session/standingtree_test.go | 46 ++++++++ internal/session/task_branch_protection.go | 27 +++-- internal/session/task_contract.go | 4 + internal/session/task_ledger.go | 3 + internal/session/task_run.go | 21 +++- internal/session/task_run_belt.go | 11 +- internal/session/task_status.go | 4 + internal/session/task_store.go | 24 ++-- internal/tui3/task.go | 26 ++++- internal/tui3/taskdone.go | 16 ++- internal/tui3/taskretry.go | 2 +- internal/tui3/taskwords_test.go | 11 +- 14 files changed, 257 insertions(+), 81 deletions(-) diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index ca0b97a582..0998d3883b 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -1836,14 +1836,16 @@ After the name the card carries the span, the file count, and how the branch cam `merged`, `in your own folder`, `conflicted · `, or `branch kept · ` — each its own fact, so a task you ended reads `stopped · branch kept · `. -`branch kept · ` on a **done** task means the work finished but your checkout was -on a protected branch, was on a different branch than when the work was cut, moved -to a different commit by your own work after the cut, or was detached. The branch -named there holds the finished work; the how-tasks-run page explains the exact reason. +`branch kept · · ` on a **done** task means the work finished but your +checkout was on a protected branch, was on a different branch than when the work was +cut, moved to a different commit by your own work after the cut, or was detached. For +example, it can say `branch kept · task/port · your checkout is on main, which tasks do +not merge into automatically`. The branch named there holds the finished work; the +how-tasks-run page explains the exact reason. Inspect that branch and keep the delivery workflow you requested. A task finishing -does not by itself request a merge or a checkout change. A retained run is the -exception: `/land` lists its waiting folder and merges that named run branch when -you ask. +does not by itself request a merge or a checkout change. If you want codeaf to bring +the retained work into the checkout, `/land` lists the waiting folder and merges the +named branch, whether it came from one task or a retained run. Click anywhere on the card, or press `ctrl+o` with it selected, to expand it. `enter` on the selected card opens the task's room instead. What the expansion holds, and in what order, is @@ -4684,9 +4686,9 @@ Whenever a task stops for any reason it wears its own word — `stopped` when yo `incomplete · ` otherwise — with `branch kept` and the branch name beside it. Nothing is thrown away: on every ending except a clean merge the branch is kept and named, and what the task made is committed onto that branch before it lands — so the files it -produced are listed under `changed:` and `git merge task/…` brings them over. For a -retained run, `/land` is the explicit merge door; an ordinary branch-kept task still -waits for you, because only work that was checked reaches your branch. +produced are listed under `changed:` and `git merge task/…` brings them over. A kept +branch from either a single task or a retained run appears in the `/land` waiting list; +`/land now` is the explicit merge door when you want that work in your checkout. ## Continue task N — keep going on a failed or finished task, No task 1 in this project diff --git a/internal/session/standingtree.go b/internal/session/standingtree.go index 4c5992ba37..61819b451c 100644 --- a/internal/session/standingtree.go +++ b/internal/session/standingtree.go @@ -307,49 +307,85 @@ func (a *Agent) UnlandedChanges() []StandingChange { }) seen[tree.Folder] = true } - for _, row := range a.keptRunRows() { - if row.Copy == nil || seen[row.Copy.Root] { + for _, kept := range a.keptBranches() { + if seen[kept.root] { continue } out = append(out, StandingChange{ - Folder: row.Copy.Root, - Name: filepath.Base(row.Copy.Root), - Files: len(row.Changed), + Folder: kept.root, + Name: filepath.Base(kept.root), + Files: len(kept.files), }) - seen[row.Copy.Root] = true + seen[kept.root] = true } sort.SliceStable(out, func(i, j int) bool { return out[i].Name < out[j].Name }) return out } -func (a *Agent) keptRunRows() []TaskNotice { +type keptBranch struct { + root string + branch string + files []string + node *TaskNode + runID uint64 +} + +// keptBranches is the one source for branches that automatic landing left on +// disk. It reads ordinary task nodes and adaptive run rows together so the +// standing chip, its preview, and /land cannot disagree about what is waiting. +func (a *Agent) keptBranches() []keptBranch { g := a.tasker() if g == nil { return nil } g.mu.Lock() defer g.mu.Unlock() - var out []TaskNotice - for _, row := range g.runRowsLocked() { - if !row.State.settled() || row.Copy == nil || strings.TrimSpace(row.Copy.Root) == "" || row.Branch == "" || len(row.Changed) == 0 { + var out []keptBranch + for _, id := range g.order { + node := g.nodes[id] + if node == nil || !keptBranchOutcome(node.merge) || strings.TrimSpace(node.branch) == "" || !node.state.settled() { + continue + } + // A task's Ground is the repository root recorded when its branch was + // cut, so this read stays in memory while the composer redraws it. + root := strings.TrimSpace(node.Ground) + if root == "" { + root = a.config.Workspace + } + root = canonicalPath(root) + if root == "" { continue } - switch row.Merge { - case mergeKept, mergeConflicted, mergeAborted: - out = append(out, row) + out = append(out, keptBranch{root: root, branch: node.branch, + files: append([]string(nil), node.changed...), node: node}) + } + for _, row := range g.runRowsLocked() { + if !row.State.settled() || row.Copy == nil || strings.TrimSpace(row.Copy.Root) == "" || row.Branch == "" || !keptBranchOutcome(row.Merge) { + continue } + out = append(out, keptBranch{root: canonicalPath(row.Copy.Root), branch: row.Branch, + files: append([]string(nil), row.Changed...), runID: row.ID}) } return out } -func (a *Agent) keptRunFor(folder string) (TaskNotice, bool) { +func keptBranchOutcome(merge string) bool { + switch merge { + case mergeKept, mergeConflicted, mergeAborted: + return true + default: + return false + } +} + +func (a *Agent) keptBranchFor(folder string) (keptBranch, bool) { folder = canonicalPath(strings.TrimSpace(folder)) - for _, row := range a.keptRunRows() { - if canonicalPath(row.Copy.Root) == folder { - return row, true + for _, kept := range a.keptBranches() { + if canonicalPath(kept.root) == folder { + return kept, true } } - return TaskNotice{}, false + return keptBranch{}, false } // standingTreeFor is the copy this conversation holds of one folder, and @@ -682,13 +718,18 @@ func (a *Agent) noteStandingWrite(tree StandingTree, aimed string) { // waiting — without doing any of it. It is what the card shows before the // person says yes. func (a *Agent) LandingFor(folder string) (FolderLanding, bool) { - tree, ok := a.standingTreeFor(a.standingName(folder)) - if !ok || len(tree.Wrote) == 0 { + name := a.standingName(folder) + tree, ok := a.standingTreeFor(name) + if ok && len(tree.Wrote) > 0 { + files := make([]string, len(tree.Wrote)) + copy(files, tree.Wrote) + return FolderLanding{Folder: tree.Folder, Name: filepath.Base(tree.Folder), Files: files}, true + } + kept, ok := a.keptBranchFor(name) + if !ok { return FolderLanding{}, false } - files := make([]string, len(tree.Wrote)) - copy(files, tree.Wrote) - return FolderLanding{Folder: tree.Folder, Name: filepath.Base(tree.Folder), Files: files}, true + return FolderLanding{Folder: kept.root, Name: filepath.Base(kept.root), Files: append([]string(nil), kept.files...)}, true } // standingName reads whatever a surface was given — a full path, or the @@ -711,6 +752,11 @@ func (a *Agent) standingName(name string) string { return tree.Folder } } + for _, kept := range a.keptBranches() { + if kept.root == folder || filepath.Base(kept.root) == name { + return kept.root + } + } return folder } @@ -734,8 +780,8 @@ func (a *Agent) Land(folder string) (FolderLanding, error) { if name == "" { return FolderLanding{}, errors.New("nothing is waiting to go into a folder") } - if row, ok := a.keptRunFor(name); ok { - return a.landKeptRun(row) + if kept, ok := a.keptBranchFor(name); ok { + return a.landKeptBranch(kept) } tree, ok := a.standingTreeFor(name) if !ok { @@ -792,13 +838,13 @@ func (a *Agent) Land(folder string) (FolderLanding, error) { return landing, nil } -// landKeptRun is the explicit /land road for a run branch that automatic -// landing deliberately left alone. The person has named this action, so the -// protected-branch guard no longer applies; git still refuses dirty or -// conflicting ground and the retained row remains the recovery path then. -func (a *Agent) landKeptRun(row TaskNotice) (FolderLanding, error) { - root, branch := canonicalPath(row.Copy.Root), strings.TrimSpace(row.Branch) - landing := FolderLanding{Folder: root, Name: filepath.Base(root), Files: append([]string(nil), row.Changed...)} +// landKeptBranch is the explicit /land road for any branch automatic landing +// deliberately left alone. The person has named this action, so the protected +// branch guard no longer applies; git still refuses dirty or conflicting ground +// and the retained source remains the recovery path then. +func (a *Agent) landKeptBranch(kept keptBranch) (FolderLanding, error) { + root, branch := canonicalPath(kept.root), strings.TrimSpace(kept.branch) + landing := FolderLanding{Folder: root, Name: filepath.Base(root), Files: append([]string(nil), kept.files...)} unlock := lockGitRoot(a.placeHere(), root) defer unlock() if _, err := git(root, "merge", "--no-edit", branch); err != nil { @@ -809,8 +855,15 @@ func (a *Agent) landKeptRun(row TaskNotice) (FolderLanding, error) { } landing.Merged = mergeMerged _, _ = git(root, "branch", "-d", branch) - if graph := a.tasker(); graph != nil { - graph.keepRunRows(row.ID, nil) + if kept.node != nil { + kept.node.graph.mu.Lock() + kept.node.merge = mergeMerged + kept.node.keptReason = "" + kept.node.graph.mu.Unlock() + kept.node.graph.checkpoint() + a.emitTaskUpdate(kept.node.notice()) + } else if graph := a.tasker(); graph != nil { + graph.keepRunRows(kept.runID, nil) } return landing, nil } diff --git a/internal/session/standingtree_test.go b/internal/session/standingtree_test.go index 0cdf3480af..fd80947f09 100644 --- a/internal/session/standingtree_test.go +++ b/internal/session/standingtree_test.go @@ -145,6 +145,52 @@ func TestLandFindsAndMergesAKeptRunBranch(t *testing.T) { } } +// A SINGLE TASK'S KEPT BRANCH uses the same standing source as an adaptive +// run's retained row. /land must not depend on a run record to find work that +// the task node itself already records. +func TestLandFindsAndMergesASingleKeptTaskBranch(t *testing.T) { + repo := newTestRepo(t) + mustGit(t, repo, "checkout", "-b", "main") + agent, _, _ := standingLab(t, repo) + tree, err := prepareTaskTree(Place{}, repo, "single-kept", 1, "write the retained file") + if err != nil { + t.Fatal(err) + } + writeFile(t, filepath.Join(tree.dir, "retained.txt"), "from the task branch\n") + merge, detail, _, _ := tree.comeHome("write the retained file", []string{"retained.txt"}, gitSignature{}) + if merge != mergeKept { + t.Fatalf("task landing = %q (%s), want kept", merge, detail) + } + + graph := agent.graph() + graph.mu.Lock() + node := &TaskNode{graph: graph, id: 1, done: make(chan struct{}), state: TaskDone, + spec: taskSpec{title: "write the retained file"}, Ground: repo, Mode: TaskModeWorktree, + Home: tree.home, HomeSha: tree.homeSha, changed: []string{"retained.txt"}, + branch: tree.branch, merge: mergeKept} + graph.nodes[node.id] = node + graph.order = append(graph.order, node.id) + graph.mu.Unlock() + + waiting := agent.UnlandedChanges() + if len(waiting) != 1 || waiting[0].Folder != repo || waiting[0].Files != 1 { + t.Fatalf("single kept task is not waiting to land: %+v", waiting) + } + landing, err := agent.Land("") + if err != nil { + t.Fatalf("Land: %v", err) + } + if landing.Merged != mergeMerged { + t.Fatalf("single kept task landing = %+v, want merged", landing) + } + if got := readFile(t, filepath.Join(repo, "retained.txt")); got != "from the task branch\n" { + t.Fatalf("landed file = %q", got) + } + if got := gitOut(t, repo, "branch", "--list", tree.branch); strings.TrimSpace(got) != "" { + t.Fatalf("kept task branch survived landing: %q", got) + } +} + // AND THE MODEL SEES ITS OWN WORK. A write followed by a read of the same path // must answer what was written — the copy is that folder's truth for this // conversation — while a file the conversation never touched is still read diff --git a/internal/session/task_branch_protection.go b/internal/session/task_branch_protection.go index 6d1d6ae140..55574aa469 100644 --- a/internal/session/task_branch_protection.go +++ b/internal/session/task_branch_protection.go @@ -163,24 +163,35 @@ func (t taskTree) landsInThePersonsRepository() bool { return true } -// keptLandingSentence is the one sentence a completed branch landing owes -// when the checkout is not a safe destination. It only reads the checkout; the -// caller has already put the task branch in the root repository before asking. -func (t taskTree) keptLandingSentence() string { +// keptLandingReason is the one policy explanation automatic landing owes when +// the checkout is not a safe destination. Both task and belt landing lines +// carry this result, so a surface never has to infer why a branch was kept. +func (t taskTree) keptLandingReason() string { current := currentBranch(t.root) switch { case current == "": - return "its branch " + t.branch + " was kept: your checkout is not on a branch — inspect the retained task branch without changing this checkout" + return "your checkout is not on a branch — inspect the retained task branch without changing this checkout" case t.home != "" && current != t.home: - return "its branch " + t.branch + " was kept: your checkout has moved from " + t.home + " to " + current + " since the work was cut — inspect the retained task branch before choosing a destination" + return "your checkout has moved from " + t.home + " to " + current + " since the work was cut — inspect the retained task branch before choosing a destination" case protectedBranch(t.root, current): - return "its branch " + t.branch + " was kept: your checkout is on " + current + ", which tasks do not merge into automatically" + return "your checkout is on " + current + ", which tasks do not merge into automatically" case branchMovedByPerson(t.root, current, t.homeSha): - return "its branch " + t.branch + " was kept: " + current + " has moved on since the work was cut — inspect the retained task branch before choosing a destination" + return current + " has moved on since the work was cut — inspect the retained task branch before choosing a destination" } return "" } +// keptLandingSentence is the one sentence a completed branch landing owes +// when the checkout is not a safe destination. It only reads the checkout; the +// caller has already put the task branch in the root repository before asking. +func (t taskTree) keptLandingSentence() string { + reason := t.keptLandingReason() + if reason == "" { + return "" + } + return "its branch " + t.branch + " was kept: " + reason +} + // branchMovedByPerson reports that the named branch no longer points at the // recorded world and that the movement was not made solely by codeaf's own // landings. A rewrite is always the person's movement; a forward move is theirs diff --git a/internal/session/task_contract.go b/internal/session/task_contract.go index 5873ce7d6e..be6efc51fa 100644 --- a/internal/session/task_contract.go +++ b/internal/session/task_contract.go @@ -661,6 +661,10 @@ type TaskNotice struct { // on its branch), "conflicted" (branch kept), "inplace" (a non-git // workspace ran in the person's tree), or "" while running. Merge string + // KeptReason is why automatic landing left a kept branch alone. It is empty + // for conflicts and for every branch that came home, and lets surfaces name + // the policy boundary without parsing person-facing prose. + KeptReason string // Doing is the PHASE a running node of a named kind is in, in that kind's // own plain words — "designing", "awaiting your look" for a subharness // being written (harness_task.go) — and "" for an ordinary task, which has diff --git a/internal/session/task_ledger.go b/internal/session/task_ledger.go index 836440d9c2..98ed2bd45a 100644 --- a/internal/session/task_ledger.go +++ b/internal/session/task_ledger.go @@ -75,6 +75,9 @@ func landHome(node *TaskNode, tree taskTree, changed []string, sign gitSignature // is decided where the refusal happened, not read back out of the sentence // afterwards (task_land_unsaved.go's [landingRefusal]). merge, detail, clashing, why := tree.comeHome(node.title(), ledger, sign.ranOn(signedModel(node))) + if merge == mergeKept { + node.setKeptReason(tree.keptLandingReason()) + } // AND THE NAMES ARE KEPT ON THE NODE, at the one moment they exist. git's index // held them while the refused merge stood and was made to give them back before // the merge was abandoned (groundcarry.go's [taskTree.refuseMerge]); a row drawn diff --git a/internal/session/task_run.go b/internal/session/task_run.go index b8a23672c0..21be7c8798 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -574,10 +574,11 @@ type TaskNode struct { // ([lastToolReceipts], task_audit.go). It is not in the checkpoint and it is // not drawn anywhere — it is evidence for one question, refreshed by whichever // worker spoke last, and a resumed node simply has none. - receipts []toolReceipt - branch string - worktree string - merge string + receipts []toolReceipt + branch string + worktree string + merge string + keptReason string // clashing names the files that stopped this node's branch fastening onto the // person's, read out of the index while the refused merge still held them and // written here by the landing road ([landHome]). It is what a your-call row @@ -2901,6 +2902,17 @@ func (n *TaskNode) finish(report string, changed []string, branch, merge string) n.graph.checkpoint() } +// setKeptReason records the policy explanation beside the branch outcome so a +// later notice, checkpoint, or explicit /land can use the same fact. +func (n *TaskNode) setKeptReason(reason string) { + if n == nil || n.graph == nil { + return + } + n.graph.mu.Lock() + n.keptReason = strings.TrimSpace(reason) + n.graph.mu.Unlock() +} + // noteWrote records that this node has just written a path, so that this // session's presence can say so while the work is still going. // @@ -3757,6 +3769,7 @@ func (n *TaskNode) noticeLocked(cost float64) TaskNotice { Changed: changed, Branch: n.branch, Merge: n.merge, + KeptReason: n.keptReason, Doing: n.doing, Context: n.context, Mending: n.mend, diff --git a/internal/session/task_run_belt.go b/internal/session/task_run_belt.go index 12754867b2..671bb725c3 100644 --- a/internal/session/task_run_belt.go +++ b/internal/session/task_run_belt.go @@ -262,6 +262,8 @@ type RunLanding struct { Branch string Changed []string Refused string + // KeptReason is why automatic landing left Branch on its own ref. + KeptReason string // Home is how the work came home, in the landing road's own outcome words // ([mergeMerged] and its kin), set by this door once the run's copy has been // brought back to its ground ([Agent.landBeltRun]). Empty is an engine's own @@ -2008,6 +2010,9 @@ func (a *Agent) bringBeltRunHome(run *beltRun, landing RunLanding) RunLanding { if said == "" { said = "its work is kept on " + landing.Branch + " and did not go into " + run.ground } + if merge == mergeKept { + landing.KeptReason = run.tree.keptLandingReason() + } landing.Refused, landing.Home = said, merge return landing } @@ -2264,7 +2269,8 @@ func (a *Agent) beltRunNotice(run *beltRun, summary RunSummary, landing RunLandi // the outcome sentence. Ending: beltRunEnding(summary), Report: report, Result: summary.Result, - Changed: landing.Changed, + Changed: landing.Changed, + KeptReason: landing.KeptReason, // THE CREW THAT DID IT AND WHAT IT COST, beside the estimate it was // picked under, for the card's crew line. Crew: run.crewDecision(), Model: run.crewWorker(), CostUSD: run.crew.taskSpent(summary.USD), @@ -2477,5 +2483,8 @@ func carryRunIdentity(notice, kept TaskNotice) TaskNotice { if notice.Crew == nil && kept.Crew != nil { notice.Crew = kept.Crew } + if notice.KeptReason == "" { + notice.KeptReason = kept.KeptReason + } return notice } diff --git a/internal/session/task_status.go b/internal/session/task_status.go index 7a83c13b5e..62774d5ad3 100644 --- a/internal/session/task_status.go +++ b/internal/session/task_status.go @@ -166,6 +166,9 @@ type TaskFacts struct { // Merge and Branch are the source-control facts (TaskNotice.Merge, .Branch). Merge string Branch string + // KeptReason is why automatic landing left Branch on its own ref. It is + // carried for surfaces that put the reason beside the branch handle. + KeptReason string // Report is the landing's own account of itself (TaskNotice.Report), and it is // read for exactly three things: which incomplete reason a fault or a check's // finding gets ([TaskReasonOf]), the gaps a held landing names, and whether a @@ -675,6 +678,7 @@ func (n TaskNotice) StatusFacts() TaskFacts { Stopped: n.Stopped, Merge: n.Merge, Branch: n.Branch, + KeptReason: n.KeptReason, Report: n.Report, Held: n.ResultHeld, Conflicts: n.Conflicts, diff --git a/internal/session/task_store.go b/internal/session/task_store.go index afa45ec9ac..f2c9523f9b 100644 --- a/internal/session/task_store.go +++ b/internal/session/task_store.go @@ -320,10 +320,11 @@ type taskRecord struct { // back to the report exactly as it did then. Result *taskResultRecord `json:"result,omitempty"` - Changed []string `json:"changed,omitempty"` - Branch string `json:"branch,omitempty"` - Worktree string `json:"worktree,omitempty"` - Merge string `json:"merge,omitempty"` + Changed []string `json:"changed,omitempty"` + Branch string `json:"branch,omitempty"` + Worktree string `json:"worktree,omitempty"` + Merge string `json:"merge,omitempty"` + KeptReason string `json:"keptReason,omitempty"` // Wrote is what a node's own hands have written SO FAR, kept while it runs // rather than only when it lands ([TaskNode.noteWrote]). @@ -785,11 +786,12 @@ type runRecord struct { // ended by a limit its person set lost which limit it was, and a row whose // work was kept on a branch came back naming no branch at all. Each is // omitted when empty, so an older file decodes exactly as it always did. - Ending TaskEnding `json:"ending,omitempty"` - Branch string `json:"branch,omitempty"` - Merge string `json:"merge,omitempty"` - Result string `json:"result,omitempty"` - Changed []string `json:"changed,omitempty"` + Ending TaskEnding `json:"ending,omitempty"` + Branch string `json:"branch,omitempty"` + Merge string `json:"merge,omitempty"` + KeptReason string `json:"keptReason,omitempty"` + Result string `json:"result,omitempty"` + Changed []string `json:"changed,omitempty"` } // taskDocument is the file: a type tag, a version, the id counter, the nodes in @@ -1119,6 +1121,7 @@ func runRowRecord(notice TaskNotice) runRecord { Ending: notice.Ending, Branch: notice.Branch, Merge: notice.Merge, + KeptReason: notice.KeptReason, Result: notice.Result, Changed: append([]string(nil), notice.Changed...), PlanTask: notice.PlanTask, @@ -1166,6 +1169,7 @@ func runRowNotice(record runRecord) TaskNotice { Ending: record.Ending, Branch: record.Branch, Merge: record.Merge, + KeptReason: record.KeptReason, Result: record.Result, Changed: append([]string(nil), record.Changed...), PlanTask: record.PlanTask, @@ -1284,6 +1288,7 @@ func (n *TaskNode) recordLocked() taskRecord { Branch: n.branch, Worktree: n.worktree, Merge: n.merge, + KeptReason: n.keptReason, Journal: n.journal, Beat: beat, Model: n.spec.model, @@ -2186,6 +2191,7 @@ func restoreNode(graph *TaskGraph, record taskRecord) *TaskNode { branch: record.Branch, worktree: record.Worktree, merge: record.Merge, + keptReason: record.KeptReason, journal: record.Journal, elapsed: time.Duration(record.ElapsedMS) * time.Millisecond, started: record.StartedAt, diff --git a/internal/tui3/task.go b/internal/tui3/task.go index 4a6e3431d9..12b6e8d462 100644 --- a/internal/tui3/task.go +++ b/internal/tui3/task.go @@ -287,8 +287,8 @@ type taskNode struct { // claim that a node thought for free: a surface draws nothing at all for it, // the way it draws nothing for an unpublished price (session's // task_contract.go on CostUSD). - tokens int - report, branch, merge string + tokens int + report, branch, merge, keptReason string // produced is WHAT THE WORK MADE, kept apart from the report above because // internal/session keeps the two apart and for its reason (task_result.go): // the report is the CARD — a landing's own sentences, which a late verdict, @@ -739,6 +739,20 @@ const taskStoppedWord = "stopped" // own vocabulary, and the roster, the record and home each had a different one. const taskBranchKept = "branch kept" +// taskBranchKeptLine is the one screen line for a retained branch. The branch +// handle stays before the policy explanation so a narrow rail can preserve the +// actionable part while the full card still names why it was left alone. +func taskBranchKeptLine(branch, reason string) string { + line := taskBranchKept + if branch = strings.TrimSpace(branch); branch != "" { + line += " · " + branch + } + if reason = strings.TrimSpace(reason); reason != "" { + line += " · " + reason + } + return line +} + // taskFinishingWord is what a node says while it is closing a gap in work it has // otherwise finished (session's TaskNotice.Mending, carried on [taskNode.mending]). // @@ -4572,7 +4586,7 @@ func (a *app) railUnder(node *taskNode, width int) []string { case mergeWordKept: // FINISHED AND WAITING ON ITS BRANCH. The state stays done because the // work is complete; the row names the one action left to the person. - text = taskBranchKept + " · " + node.branch + text = taskBranchKeptLine(node.branch, node.keptReason) case mergeWordConflicted: // THE ONE LOUD ROW ON THE RAIL. A branch that did not merge is work // that is finished and not delivered, and its branch is the only @@ -5370,6 +5384,12 @@ func (a *app) taskUpdate(ev session.Event) tea.Cmd { } if notice.Merge != "" { node.merge = notice.Merge + if notice.Merge != mergeWordKept { + node.keptReason = "" + } + } + if notice.KeptReason != "" { + node.keptReason = notice.KeptReason } if where := strings.TrimSpace(notice.Where); where != "" { node.where = where diff --git a/internal/tui3/taskdone.go b/internal/tui3/taskdone.go index 483d1729dc..281bcb0369 100644 --- a/internal/tui3/taskdone.go +++ b/internal/tui3/taskdone.go @@ -62,12 +62,12 @@ type taskDone struct { // resultWhole is where the whole of a cut answer can be read, resultCut says // this is only the beginning of it, and resultHeld is the landing that turned // the work back: no body at all, and a pointer to where it is. - result, resultWhole string - resultCut, resultHeld bool - changed []string - added, removed int - branch, merge string - brief, acceptance string + result, resultWhole string + resultCut, resultHeld bool + changed []string + added, removed int + branch, merge, keptReason string + brief, acceptance string // rung is which copy of the ground the work happened in and mode what was // promised about it, frozen off the node at landing with everything else on // this card (session's TaskNotice.Rung). They are here so the row naming the @@ -219,6 +219,7 @@ func (a *app) landedCard(node *taskNode) { changed: node.changed, branch: node.branch, merge: node.merge, + keptReason: node.keptReason, rung: node.rung, mode: node.mode, brief: node.brief, @@ -585,6 +586,9 @@ func (a *app) doneTail(card *taskDone) string { } if card.status.ChangesUnlanded() { tail += " · " + card.status.Branch + if card.keptReason != "" { + tail += " · " + card.keptReason + } } return tail } diff --git a/internal/tui3/taskretry.go b/internal/tui3/taskretry.go index 5adb9d4894..a44dce7e96 100644 --- a/internal/tui3/taskretry.go +++ b/internal/tui3/taskretry.go @@ -107,7 +107,7 @@ func (a *app) resetRetriedTask(node *taskNode) { } node.retried = true node.stopped, node.restored = false, false - node.ending, node.report, node.merge = "", "", "" + node.ending, node.report, node.merge, node.keptReason = "", "", "", "" node.produced, node.producedWhole = "", "" node.producedCut, node.producedHeld = false, false node.began, node.ended = time.Time{}, time.Time{} diff --git a/internal/tui3/taskwords_test.go b/internal/tui3/taskwords_test.go index 57160c79cc..cf1e2b794c 100644 --- a/internal/tui3/taskwords_test.go +++ b/internal/tui3/taskwords_test.go @@ -611,16 +611,17 @@ func engineMergeWords(t *testing.T) []string { // becomes a person-facing label. func TestC13AKeptLandingSaysBranchKeptEverywhere(t *testing.T) { a, _, _ := taskApp(t) + reason := "your checkout is on main, which tasks do not merge into automatically" drive(t, a, streamEventMsg{gen: a.gen, ev: update(7, "Protect the checkout", session.TaskDone, - session.TaskNotice{Merge: mergeWordKept, Branch: "task/protect"})}) + session.TaskNotice{Merge: mergeWordKept, Branch: "task/protect", KeptReason: reason})}) node := a.tasks[7] - want := taskBranchKept + " · task/protect" + want := taskBranchKept + " · task/protect · " + reason - if got := plain(strings.Join(a.railUnder(node, 60), "\n")); got != want { - t.Fatalf("the rail says %q, want %q", got, want) + if got := plain(strings.Join(a.railUnder(node, 60), "\n")); !strings.Contains(got, taskBranchKept+" · task/protect") || !strings.Contains(strings.Join(strings.Fields(got), " "), reason) { + t.Fatalf("the rail says %q, want the branch and reason", got) } card := &taskDone{ - merge: mergeWordKept, branch: "task/protect", + merge: mergeWordKept, branch: "task/protect", keptReason: reason, status: doneStatus(session.TaskFacts{ State: session.TaskDone, Merge: mergeWordKept, Branch: "task/protect"}), } From e3b96edb5f765a5474cffcdac30b50a78a1c4a03 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 11:17:54 -0400 Subject: [PATCH 05/10] land: /land reaches the engine over the local host, so a kept task branch can land Ordinary chat talks to its engine through the local host, whose adapter had no landing door, so /land always said "nothing is waiting" however many kept branches the engine held. The waiting list now rides the pushed session facts, landing preview and landing cross the host as calls, and both run off the paint loop. The surface-door ledger loses its two landing entries. Fixes #1574 --- internal/manual/chat/choosing-a-folder.md | 12 ++ internal/manual/chat/tasks.md | 6 +- internal/remote/callclass.go | 2 +- internal/remote/landing.go | 81 +++++++++ internal/remote/landing_test.go | 74 +++++++++ internal/remote/places.go | 2 + internal/remote/surfacedoors_law_test.go | 7 +- internal/session/facts.go | 6 + internal/session/task_run_belt.go | 4 +- internal/session/task_run_belt_test.go | 77 +++++++++ internal/tui3/app.go | 6 - .../tui3/async_command_visibility_test.go | 1 - internal/tui3/land_belt_host_test.go | 157 ++++++++++++++++++ internal/tui3/landcmd.go | 53 +++--- internal/tui3/landcmd_test.go | 4 +- 15 files changed, 449 insertions(+), 43 deletions(-) create mode 100644 internal/remote/landing.go create mode 100644 internal/remote/landing_test.go create mode 100644 internal/tui3/land_belt_host_test.go diff --git a/internal/manual/chat/choosing-a-folder.md b/internal/manual/chat/choosing-a-folder.md index 64188321a4..4518a5dc2d 100644 --- a/internal/manual/chat/choosing-a-folder.md +++ b/internal/manual/chat/choosing-a-folder.md @@ -559,6 +559,18 @@ and notes · say which one · /land agentfield`. Then `/land agentfield now`. **A folder lands whole.** There is no way to land some of the files and keep the rest today — you either put the folder's changes in or leave them waiting. +## /land after a task kept its branch in this folder + +A task's retained branch is waiting even when it belongs to the folder the +conversation is standing in. `/land` shows the folder and changed files; +`/land now` explicitly merges that branch into your current checkout, including +`main`. Automatic task landing still leaves protected branches alone. + +This works through the local session host used by ordinary chat, and the waiting +branch remains available after reopening the conversation. A dirty checkout or a +merge conflict can still prevent the merge; the retained branch stays available. +The `--host` landing restriction is unchanged. + ## Work in a folder directly, without keeping the changes aside Say so in your own words — "work in ~/code/notes directly", "edit it in place", "just diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 0998d3883b..44c3bf517d 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -1844,8 +1844,10 @@ not merge into automatically`. The branch named there holds the finished work; t how-tasks-run page explains the exact reason. Inspect that branch and keep the delivery workflow you requested. A task finishing does not by itself request a merge or a checkout change. If you want codeaf to bring -the retained work into the checkout, `/land` lists the waiting folder and merges the -named branch, whether it came from one task or a retained run. +the retained work into the checkout, `/land` lists the waiting folder and `/land now` +merges the named branch, whether it came from one task or a retained run. This also +works in an ordinary local-host conversation, before and after reopening it, including +when the task's repository is the folder the conversation is standing in. Click anywhere on the card, or press `ctrl+o` with it selected, to expand it. `enter` on the selected card opens the task's room instead. What the expansion holds, and in what order, is diff --git a/internal/remote/callclass.go b/internal/remote/callclass.go index c48380e0f8..41ebd037c8 100644 --- a/internal/remote/callclass.go +++ b/internal/remote/callclass.go @@ -143,7 +143,7 @@ func classify(method string) callClass { MethodAttachedSkills, MethodSkillShelf, MethodSessionsRecent, MethodHeldQuestions, MethodStandingItems, MethodStandingWatch, - MethodPlacesWorld, MethodPlacesTask, MethodPlacesLedger, MethodPlacesSearch, + MethodPlacesWorld, MethodPlacesTask, MethodPlacesLedger, MethodPlacesSearch, MethodLandingPreview, MethodMemorySnapshot, MethodMemoryChanged, MethodMemoryList, MethodMemoryProvenance, MethodTaskRoom, MethodTaskPending, MethodTaskEffort, MethodListDir, MethodStatPaths, MethodFetchFile, diff --git a/internal/remote/landing.go b/internal/remote/landing.go new file mode 100644 index 0000000000..50d3749257 --- /dev/null +++ b/internal/remote/landing.go @@ -0,0 +1,81 @@ +package remote + +import ( + "encoding/json" + "errors" + + "github.com/Agent-Field/codeaf/internal/session" +) + +// These additive doors carry explicit landing gestures to the session that +// owns the checkout. Ordinary local chat reaches that session through this wire. +const ( + MethodLandingPreview = "Places.LandingPreview" + MethodLand = "Places.Land" +) + +type folderLander interface { + LandingFor(string) (session.FolderLanding, bool) + Land(string) (session.FolderLanding, error) +} + +type landingPreview struct { + Landing session.FolderLanding `json:"landing"` + Found bool `json:"found"` +} + +func (s *server) landingCall(call Frame) (json.RawMessage, bool, error) { + folder, err := arg[string](call) + if err != nil { + return nil, true, err + } + door, ok := s.session.current().(folderLander) + if !ok { + return nil, true, errors.New("this conversation cannot land changes") + } + if call.Method == MethodLandingPreview { + landing, found := door.LandingFor(folder) + payload, err := json.Marshal(landingPreview{Landing: landing, Found: found}) + return payload, true, err + } + landing, err := door.Land(folder) + if err != nil { + return nil, true, err + } + // Every attached window must lose the waiting row when the work lands. + s.session.announce() + payload, err := json.Marshal(landing) + return payload, true, err +} + +// UnlandedChanges is a memory read because the composer asks on every frame. +func (a *Agent) UnlandedChanges() []session.StandingChange { + return a.c.facts.read().Unlanded +} + +// LandingFor asks the owning session only when the person requests a preview. +func (a *Agent) LandingFor(folder string) (session.FolderLanding, bool) { + payload, err := a.c.call(nil, MethodLandingPreview, folder) + if err != nil { + return session.FolderLanding{}, false + } + var preview landingPreview + if err := json.Unmarshal(payload, &preview); err != nil { + return session.FolderLanding{}, false + } + return preview.Landing, preview.Found +} + +// Land keeps Git and copy operations on the session's machine, including when +// the window and its local session host are separate processes on one machine. +func (a *Agent) Land(folder string) (session.FolderLanding, error) { + payload, err := a.c.call(nil, MethodLand, folder) + if err != nil { + return session.FolderLanding{}, err + } + var landing session.FolderLanding + if err := json.Unmarshal(payload, &landing); err != nil { + return session.FolderLanding{}, err + } + return landing, nil +} diff --git a/internal/remote/landing_test.go b/internal/remote/landing_test.go new file mode 100644 index 0000000000..be0d1374f9 --- /dev/null +++ b/internal/remote/landing_test.go @@ -0,0 +1,74 @@ +package remote + +import ( + "errors" + "reflect" + "testing" + + "github.com/Agent-Field/codeaf/internal/session" +) + +var _ folderLander = (*session.Agent)(nil) + +type landingTestAgent struct { + *fakeAgent + waiting []session.StandingChange + result session.FolderLanding + fail error +} + +func (a *landingTestAgent) UnlandedChanges() []session.StandingChange { return a.waiting } +func (a *landingTestAgent) LandingFor(folder string) (session.FolderLanding, bool) { + return a.result, len(a.waiting) > 0 && folder == a.result.Folder +} +func (a *landingTestAgent) Land(string) (session.FolderLanding, error) { + if a.fail == nil { + a.waiting = nil + } + return a.result, a.fail +} + +func TestLandingCrossesTheHostAndWaitingReadsStayOffTheWire(t *testing.T) { + for _, refuse := range []bool{false, true} { + t.Run(map[bool]string{false: "lands", true: "refuses"}[refuse], func(t *testing.T) { + far := &landingTestAgent{fakeAgent: &fakeAgent{model: "m"}, waiting: []session.StandingChange{{Folder: "/srv/repo", Name: "repo", Files: 1}}, result: session.FolderLanding{Folder: "/srv/repo", Name: "repo", Files: []string{"README.md"}, Merged: "merged"}} + if refuse { + far.fail = errors.New("landing refused") + } + loop := foldersLoop(t, far) + door, ok := any(loop.Client.Agent()).(interface { + UnlandedChanges() []session.StandingChange + LandingFor(string) (session.FolderLanding, bool) + Land(string) (session.FolderLanding, error) + }) + if !ok { + t.Fatal("the local-host adapter has no landing door") + } + if got := door.UnlandedChanges(); !reflect.DeepEqual(got, far.waiting) { + t.Fatalf("welcome waiting = %+v", got) + } + if got, ok := door.LandingFor("/srv/repo"); !ok || !reflect.DeepEqual(got, far.result) { + t.Fatalf("preview = %+v, %t", got, ok) + } + if _, ok := door.LandingFor("/srv/other"); ok { + t.Fatal("preview invented a folder") + } + got, err := door.Land("") + if refuse { + if err == nil || err.Error() != far.fail.Error() { + t.Fatalf("refusal = %v", err) + } + } else if err != nil || !reflect.DeepEqual(got, far.result) { + t.Fatalf("landing = %+v, %v", got, err) + } + if err := loop.Close(); err != nil { + t.Fatal(err) + } + // The last stated waiting set remains readable after disconnection, so + // repainting cannot depend on an RPC or silently erase retained work. + if got := door.UnlandedChanges(); !reflect.DeepEqual(got, far.waiting) { + t.Fatalf("cached waiting = %+v, want %+v", got, far.waiting) + } + }) + } +} diff --git a/internal/remote/places.go b/internal/remote/places.go index da249ac246..8154047cde 100644 --- a/internal/remote/places.go +++ b/internal/remote/places.go @@ -42,6 +42,8 @@ func (s *server) placesCall(call Frame) (json.RawMessage, bool, error) { sess.mu.Unlock() switch call.Method { + case MethodLandingPreview, MethodLand: + return s.landingCall(call) case MethodPlacesArchive: args, err := arg[ArchiveArgs](call) if err != nil { diff --git a/internal/remote/surfacedoors_law_test.go b/internal/remote/surfacedoors_law_test.go index e95fb1998b..afafd5f3f0 100644 --- a/internal/remote/surfacedoors_law_test.go +++ b/internal/remote/surfacedoors_law_test.go @@ -81,10 +81,6 @@ var doorsThatHaveNotCrossed = map[string]absentDoor{ says: "memory is off for this session · turn it on under /settings", loses: "/remember, /forget, /memories, /memory and the memory place — memory is not off, it is unreachable", }, - "folderLander": { - says: "nothing is waiting · what this conversation writes in the folder it is standing in is already there", - loses: "/land; and the `changes for … · /land` row above the box goes quiet too", - }, "subharnessAgent": { says: "no subharnesses here yet — a subharness is a saved program for work that comes round again.", loses: "the subharness list, its intake form and running one", @@ -120,14 +116,13 @@ var doorsThatHaveNotCrossed = map[string]absentDoor{ "taskWeightDoor": {loses: "one task's context tokens (the conversation's own ContextTokens crosses; the task's does not)"}, "turnResumer": {loses: "resuming a turn that was stopped"}, "wakeAgent": {loses: "reading wakes"}, - "interface{ LandingFor/1/2 }": {loses: "the landing a folder already has, beside folderLander"}, "interface{ PendingConsent/0/1 }": {loses: "which approvals are still open when a surface detaches"}, } // surfaceDoorLedger is the ratchet: the ledger above may shrink and may never // grow, and shrinking it without lowering this number in the same commit is a // red as well ([ratchetComplaint]). -const surfaceDoorLedger = 21 +const surfaceDoorLedger = 19 // TestEverySurfaceDoorTheEngineHasCrossesTheWire is the law above. func TestEverySurfaceDoorTheEngineHasCrossesTheWire(t *testing.T) { diff --git a/internal/session/facts.go b/internal/session/facts.go index d1297b2caa..edd56618f3 100644 --- a/internal/session/facts.go +++ b/internal/session/facts.go @@ -91,6 +91,9 @@ type Facts struct { // // Nil is a conversation about nowhere else, which is nearly all of them. Places []PlaceRef `json:"places,omitempty"` + // Unlanded is drawn beside the composer, so local-host windows need it in + // the pushed facts rather than a round trip on every repaint. + Unlanded []StandingChange `json:"unlanded,omitempty"` // Skills is the names a person has put in front of this conversation by // hand, in attachment order ([Agent.AttachedSkills]). // @@ -159,6 +162,9 @@ func FactsOf(source FactSource) Facts { if door, ok := source.(PlaceSource); ok { facts.Places = door.Places() } + if door, ok := source.(interface{ UnlandedChanges() []StandingChange }); ok { + facts.Unlanded = door.UnlandedChanges() + } if door, ok := source.(interface{ NeedsPerson() bool }); ok { facts.NeedsPerson = door.NeedsPerson() } diff --git a/internal/session/task_run_belt.go b/internal/session/task_run_belt.go index 671bb725c3..3386c6e5ee 100644 --- a/internal/session/task_run_belt.go +++ b/internal/session/task_run_belt.go @@ -1588,8 +1588,10 @@ func (a *Agent) publishRunRow(g *TaskGraph, notice TaskNotice) { if notice.State.settled() && notice.CostUSD == 0 { notice.CostUSD = a.beltRunSpent(notice.ID) } - a.emitTaskUpdate(notice) + // A host publishes the frame's facts when this update arrives, so the + // retained branch must already be readable from the graph at that moment. g.keepRunRows(notice.ID, []TaskNotice{notice}) + a.emitTaskUpdate(notice) a.indexRunRow(notice) } diff --git a/internal/session/task_run_belt_test.go b/internal/session/task_run_belt_test.go index 270a4aca94..2e549bbf33 100644 --- a/internal/session/task_run_belt_test.go +++ b/internal/session/task_run_belt_test.go @@ -11,6 +11,7 @@ import ( "bytes" "context" "encoding/json" + "fmt" "math" "os" "path/filepath" @@ -158,6 +159,82 @@ func registerBeltRunEngine(t *testing.T, engine RunEngine) { t.Cleanup(func() { RegisterRunEngine(previous) }) } +// A single belt task keeps its work off main, but the explicit landing door +// must still find that branch in the conversation's own repository after a restart. +func TestStartTaskBashBeltKeptBranchCanLandBeforeAndAfterRestart(t *testing.T) { + for _, restart := range []bool{false, true} { + t.Run(fmt.Sprintf("restart=%t", restart), func(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + repo := newTestRepo(t) + mustGit(t, repo, "branch", "-m", "main") + before := gitOut(t, repo, "rev-parse", "HEAD") + dir := t.TempDir() + double := newBeltRunDouble("added README.md") + double.real = true + double.work = func(workspace string) { + writeFile(t, filepath.Join(workspace, "README.md"), "hello\n") + } + registerBeltRunEngine(t, double) + configure := func(config *Config) { + config.Workspace = repo + config.Place = Place{Dir: dir} + config.SessionFile = filepath.Join(dir, placeTranscript) + config.AskConsent = false + } + agent, _ := newTestAgent(t, beltRunCompleter{text: "added README.md"}, configure) + id, _, _, err := agent.StartTask(context.Background(), "add a README.md with one line", false) + if err != nil { + t.Fatal(err) + } + <-double.entered + agent.beltMu.Lock() + over := agent.beltRun.over + agent.beltMu.Unlock() + close(double.release) + select { + case <-over: + case <-time.After(10 * time.Second): + t.Fatal("the run did not finish") + } + rows := agent.tasker().runRows(id) + if len(rows) != 1 || rows[0].State != TaskDone || rows[0].Merge != mergeKept || rows[0].Copy == nil { + t.Fatalf("belt landing = %+v, want a done run with its branch kept", rows) + } + branch := rows[0].Branch + if got := gitOut(t, repo, "rev-parse", "HEAD"); got != before { + t.Fatal("automatic landing changed main") + } + if got := gitOut(t, repo, "show", branch+":README.md"); strings.TrimSpace(got) != "hello" { + t.Fatalf("retained branch README.md = %q", got) + } + if restart { + if err := agent.Close(); err != nil { + t.Fatal(err) + } + agent, _ = newTestAgent(t, beltRunCompleter{text: "unused"}, configure) + } + waiting := agent.UnlandedChanges() + if len(waiting) != 1 || waiting[0].Folder != canonicalPath(repo) || waiting[0].Files != 1 { + t.Fatalf("/land waiting = %+v, want the kept README.md; copy = %+v", waiting, rows[0].Copy) + } + preview, ok := agent.LandingFor(waiting[0].Folder) + if !ok || !reflect.DeepEqual(preview.Files, []string{"README.md"}) { + t.Fatalf("/land preview = %+v, %t", preview, ok) + } + landing, err := agent.Land("") + if err != nil || landing.Merged != mergeMerged { + t.Fatalf("/land now = %+v, %v", landing, err) + } + if got := readFile(t, filepath.Join(repo, "README.md")); got != "hello\n" { + t.Fatalf("landed README.md = %q", got) + } + if waiting := agent.UnlandedChanges(); len(waiting) != 0 { + t.Fatalf("landed branch still waiting: %+v", waiting) + } + }) + } +} + // endBeltRun lets the double's run finish and WAITS FOR THE RUN TO BE OVER: its // landing committed, its copy given back, its store closed. A test that released // the run and returned used to race that ending against its own temporary diff --git a/internal/tui3/app.go b/internal/tui3/app.go index 5294aaaee7..51c093b1aa 100644 --- a/internal/tui3/app.go +++ b/internal/tui3/app.go @@ -5161,12 +5161,6 @@ func (a *app) route(msg tea.Msg) (tea.Model, tea.Cmd) { } return a, nil - case landNoteMsg: - // A landing's whole answer is one line, the clean one and the one that - // could not go in alike (landcmd.go). - a.toldNote(msg.line) - return a, nil - case cacheNoteMsg: // A cache errand's whole answer is one line, success and refusal alike // (cachecmd.go). diff --git a/internal/tui3/async_command_visibility_test.go b/internal/tui3/async_command_visibility_test.go index 7d8a0c0c8f..8c8ad1dfaa 100644 --- a/internal/tui3/async_command_visibility_test.go +++ b/internal/tui3/async_command_visibility_test.go @@ -18,7 +18,6 @@ func TestAsyncUserCommandResponsesStayVisibleInCleanConversation(t *testing.T) { }{ {"cache status", "cache holds", cacheNoteMsg{line: "the cache holds 12 MB"}}, {"cache confirmation", "type /cache clean now", cacheNoteMsg{line: "type /cache clean now to go ahead"}}, - {"landing", "branch landed", landNoteMsg{line: "branch landed"}}, {"compaction error", "compact failed", compactedMsg{err: errors.New("disk busy")}}, {"export", "exported", exportedMsg{path: "/tmp/conversation.md"}}, {"export refusal", "already there", exportedMsg{path: "/tmp/conversation.md", err: fs.ErrExist}}, diff --git a/internal/tui3/land_belt_host_test.go b/internal/tui3/land_belt_host_test.go new file mode 100644 index 0000000000..27da6d109e --- /dev/null +++ b/internal/tui3/land_belt_host_test.go @@ -0,0 +1,157 @@ +package tui3 + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/plandb" + "github.com/Agent-Field/codeaf/internal/remote" + "github.com/Agent-Field/codeaf/internal/session" +) + +// The worker is scripted, but both halves of its landing use the production +// Git road. The surface talks to the session through the ordinary local host. +type landBeltEngine struct{ release <-chan struct{} } + +func (e landBeltEngine) Start(ctx context.Context, spec session.RunSpec) session.RunSummary { + select { + case <-e.release: + case <-ctx.Done(): + return session.RunSummary{} + } + if err := os.WriteFile(filepath.Join(spec.Workspace, "README.md"), []byte("hello\n"), 0644); err != nil { + return session.RunSummary{Result: err.Error()} + } + if err := spec.Store.CompleteRoot("added README.md"); err != nil { + return session.RunSummary{Result: err.Error()} + } + return session.RunSummary{Outcome: "done", Result: "added README.md", Nodes: 1, Steps: 1} +} +func (e landBeltEngine) Land(_ context.Context, _ *plandb.Store, workspace, base, _ string) (session.RunLanding, error) { + branch, changed, refused, err := session.LandRunTree(workspace, base, "add README.md", "") + return session.RunLanding{Branch: branch, Changed: changed, Refused: refused}, err +} + +func TestLocalHostLandFindsBeltTaskBeforeAndAfterRestart(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv("CODEAF_FURROW", filepath.Join(t.TempDir(), "no-furrow")) + t.Setenv("CODEAF_HOME", t.TempDir()) + // With no provider key, naming and summary requests refuse before touching + // the network; the scripted worker still drives the real task and Git roads. + for _, restart := range []bool{false, true} { + t.Run(fmt.Sprintf("restart=%t", restart), func(t *testing.T) { + repo, place := t.TempDir(), t.TempDir() + git := func(args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = repo + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + return strings.TrimSpace(string(out)) + } + git("init", "-b", "main") + git("-c", "user.name=test", "-c", "user.email=test@example.invalid", "commit", "--allow-empty", "-m", "initial") + before := git("rev-parse", "HEAD") + release := make(chan struct{}) + session.RegisterRunEngine(landBeltEngine{release: release}) + t.Cleanup(func() { session.RegisterRunEngine(nil) }) + home := session.Place{Dir: place} + config := session.Config{ + Workspace: repo, Place: home, SessionFile: home.Transcript(), + Model: "test/model", BaseURL: "http://provider.invalid/v1", System: "Test only.", + } + open := func() (*session.Agent, *remote.Loop) { + t.Helper() + engine, err := session.New(config) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = engine.Close(); engine.SettleWrites() }) + loop, err := remote.Loopback(remote.Hello{Version: remote.Version, Workspace: repo}, remote.Options{Boot: func(remote.Hello) (*remote.Engine, error) { + return &remote.Engine{Agent: engine, Workspace: repo, SessionFile: config.SessionFile}, nil + }}) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = loop.Close() }) + return engine, loop + } + engine, loop := open() + updates, stop := loop.Client.Agent().WatchTaskUpdates() + defer stop() + if _, _, _, err := loop.Client.Agent().StartTask(context.Background(), "add a README.md with one line", false); err != nil { + t.Fatal(err) + } + close(release) + timeout := time.After(15 * time.Second) + var notice *session.TaskNotice + for notice == nil { + select { + case event, open := <-updates: + if !open { + t.Fatal("the task lane closed before the belt task finished") + } + if event.Task != nil && event.Task.State == session.TaskDone { + notice = event.Task + } + case <-timeout: + t.Fatal("no finished belt task reached the local host") + } + } + if notice.Merge != "kept" || notice.Branch == "" { + t.Fatalf("landing = %+v", notice) + } + if got := git("rev-parse", "HEAD"); got != before { + t.Fatal("automatic landing changed main") + } + if got := git("show", notice.Branch+":README.md"); got != "hello" { + t.Fatalf("kept README = %q", got) + } + if restart { + stop() + if err := loop.Close(); err != nil { + t.Fatal(err) + } + if err := engine.Close(); err != nil { + t.Fatal(err) + } + engine.SettleWrites() + engine, loop = open() + } + a := newTestApp(loop.Client.Agent()) + a.width, a.height = 160, 40 + if cmd := a.slash("/land"); cmd != nil { + drive(t, a, cmd()) + } + said := plain(lastNote(t, a)) + for _, want := range []string{"changes for ", "1 file", "README.md", "/land now"} { + if !strings.Contains(said, want) { + t.Fatalf("/land = %q, missing %q", said, want) + } + } + cmd := a.slash("/land now") + if cmd == nil { + t.Fatal("/land now did not start a landing") + } + drive(t, a, cmd()) + if said := plain(lastNote(t, a)); !strings.Contains(said, "now has the changes") { + t.Fatalf("/land now = %q", said) + } + body, err := os.ReadFile(filepath.Join(repo, "README.md")) + if err != nil || string(body) != "hello\n" { + t.Fatalf("landed README = %q, %v", body, err) + } + if row := a.landRow(a.width); row != "" { + t.Fatalf("landing left waiting row: %q", row) + } + }) + } +} diff --git a/internal/tui3/landcmd.go b/internal/tui3/landcmd.go index 07255f3cc7..467308b9ad 100644 --- a/internal/tui3/landcmd.go +++ b/internal/tui3/landcmd.go @@ -57,12 +57,6 @@ const landRemoteWord = "putting changes into a folder is not available over --ho // which folder their edits have been going into all along. const landNothingWord = "nothing is waiting · what this conversation writes in the folder it is standing in is already there" -// landNoteMsg is one finished landing's answer, written as the line to show. -// The landing runs git — a commit, a merge, sometimes a copy of a whole folder -// — so it goes off the loop and the answer arrives here, exactly as a cache -// errand's does. -type landNoteMsg struct{ line string } - // waitingChanges is what has been written and not put in yet, or nothing. It is // the ONE reading — the row above the box, the command's own answer and its // refusals all ask this and never the session twice in two ways. @@ -142,32 +136,43 @@ func (a *app) runLandCommand(rest string) tea.Cmd { a.note("changes are waiting for " + strings.Join(names, " and ") + " · say which one · /land " + names[0]) return nil } - if !now { - a.note(a.landPreview(waiting, folder)) - return nil - } - return func() tea.Msg { return landNoteMsg{line: landed(door.Land(folder))} } -} - -// landPreview is what /land says before anything moves: the folder, the count, -// and the first few files by name. -func (a *app) landPreview(waiting []session.StandingChange, folder string) string { chosen := waiting[0] for _, change := range waiting { if folder != "" && (change.Name == folder || change.Folder == folder) { chosen = change } } - files := "" - if door, ok := a.agent.(interface { - LandingFor(string) (session.FolderLanding, bool) - }); ok { - if landing, held := door.LandingFor(chosen.Folder); held { - files = " · " + namedFiles(landing.Files) + // Both gestures reach the owning session, so they share the ordered door + // line and cannot block repainting or report into a different conversation. + return a.offLoop(func() func(bool) tea.Cmd { + var line string + if now { + line = landed(door.Land(folder)) + } else { + files := "" + if preview, ok := door.(interface { + LandingFor(string) (session.FolderLanding, bool) + }); ok { + if landing, held := preview.LandingFor(chosen.Folder); held { + files = " · " + namedFiles(landing.Files) + } + } + line = landPreview(chosen, files, len(waiting) > 1) } - } + return func(here bool) tea.Cmd { + if here { + a.note(line) + } + return nil + } + }) +} + +// landPreview is what /land says before anything moves: the folder, the count, +// and the first few files by name. +func landPreview(chosen session.StandingChange, files string, several bool) string { say := "now" - if len(waiting) > 1 { + if several { say = chosen.Name + " now" } return "changes for " + chosen.Name + " · " + itoa(chosen.Files) + " " + plural("file", chosen.Files) + diff --git a/internal/tui3/landcmd_test.go b/internal/tui3/landcmd_test.go index b8f2ec0295..0562c90756 100644 --- a/internal/tui3/landcmd_test.go +++ b/internal/tui3/landcmd_test.go @@ -89,7 +89,7 @@ func TestTheWaitingRowRidesTheFrameAboveTheBox(t *testing.T) { func TestLandShowsWhatWouldMoveAndOnlyLandNowMovesIt(t *testing.T) { a, agent := landingApp(session.StandingChange{Folder: "/code/agentfield", Name: "agentfield", Files: 2}) - a.slash("/land") + drive(t, a, a.slash("/land")()) said := plain(lastNote(t, a)) for _, want := range []string{"changes for agentfield", "2 files", "shared.txt", "/land now"} { if !strings.Contains(said, want) { @@ -134,7 +134,7 @@ func TestLandWithTwoFoldersWaitingAsksWhichOne(t *testing.T) { t.Fatal("/land picked a folder for the person") } - a.slash("/land notes") + drive(t, a, a.slash("/land notes")()) if got := plain(lastNote(t, a)); !strings.Contains(got, "changes for notes") || !strings.Contains(got, "/land notes now") { t.Fatalf("/land notes said %q", got) } From 53823cfcacdbabed149182545f7adef5d0e50dca Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 10:21:14 -0400 Subject: [PATCH 06/10] spend: the daily limit holds the chat and chat-started tasks too Only headless runs and standing firings checked the machine-wide daily limit; a chat turn and a chat /task ran past it silently (the crew guard was built with the daily fold off). Today's limit, its day-scoped raise and the refusal sentence now live in one place (session.DailySpendAt) that chat turns, chat-task crews and codeaf do all read. A chat turn past the limit raises a card like the team cap's, "1 Raise to $X" or "2 Stop for today", before any model call; a chat-started task's crew guard folds the same limit. Part of #1546 --- cmd/codeaf/do.go | 33 +-- internal/manual/chat/models-and-cost.md | 29 ++- internal/manual/chat/tasks.md | 5 + internal/run/crew_context_test.go | 2 +- internal/session/agent.go | 4 + internal/session/answers.go | 2 + internal/session/dailyspend.go | 223 +++++++++++++++++++++ internal/session/question.go | 11 + internal/session/rail.go | 13 +- internal/session/rail_test.go | 60 ++++++ internal/session/session.go | 6 +- internal/session/spendguard_test.go | 23 +++ internal/session/taskcrew.go | 18 +- internal/session/taskcrew_record.go | 10 +- internal/session/taskcrew_recovery_test.go | 2 +- internal/tui3/question.go | 4 +- 16 files changed, 387 insertions(+), 58 deletions(-) create mode 100644 internal/session/dailyspend.go diff --git a/cmd/codeaf/do.go b/cmd/codeaf/do.go index 6252ec504d..0f571b0e3c 100644 --- a/cmd/codeaf/do.go +++ b/cmd/codeaf/do.go @@ -3927,44 +3927,31 @@ func runSpendBound(request doRequest, profileDir string, now time.Time) (runSpen if err != nil { return runSpend{}, err } - daily, err := config.DailyBudgetUSDAt(profileDir) - if err != nil { - return runSpend{}, err - } bound := runSpend{} if consent > 0 { bound = runSpend{usd: consent, stop: stopPrice, words: fmt.Sprintf( "the run reached $%.2f, the price above which codeaf asks before it spends more; "+ "rerun with --yes-spend to let it go past that", consent)} } - if daily > 0 { - left := daily - spentToday(now) - if left <= 0 { - return runSpend{refused: true, stop: stopBudget, words: fmt.Sprintf( - "today's spending limit of $%.2f is spent, so nothing was started; "+ - "rerun with --yes-spend to spend past it", daily)}, nil + daily, err := session.DailySpendAt(profileDir, now) + if err != nil { + return runSpend{}, err + } + if daily.Limit > 0 { + left := daily.Limit - daily.Spent + if daily.Reached || left <= 0 { + return runSpend{refused: true, stop: stopBudget, words: session.DailySpendAction(daily.Limit) + + "; rerun with --yes-spend to spend past it"}, nil } if bound.usd == 0 || left < bound.usd { bound = runSpend{usd: left, stop: stopBudget, words: fmt.Sprintf( "the run reached what was left of today's spending limit of $%.2f; "+ - "rerun with --yes-spend to spend past it", daily)} + "rerun with --yes-spend to spend past it", daily.Limit)} } } return bound, nil } -// spentToday is what today has cost on this machine, read off the usage ledger -// every conversation and every run worker writes ([session.SpendToday]). A -// ledger that cannot be read is a day that has spent nothing as far as this -// door can tell; the plan-price rung still bounds the run. -func spentToday(now time.Time) float64 { - lines, err := session.ReadUsage(session.UsageLedgerPath(), now.Add(-48*time.Hour)) - if err != nil { - return 0 - } - return session.SpendToday(lines, now) -} - // crewCompleters turns the run road's provider seam into the per-model // completer [runengine.CrewFactory] asks for. The factory reads the crew at every // launch, so a task's seat model is not known until the task is handed over; diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index a695887562..ae04da28c1 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -3003,8 +3003,11 @@ one it was: row to `none` and it never asks. - **`per conversation` — it stops.** `conversation limit reached · $2.05 spent of $2 · /budget changes it`. The section on that below has the whole of it. -- **`per day` — the day's work waits.** When the day's calls reach the daily limit, new - work waits for midnight or for you to raise it. `/budget 800` raises it where you stand. +- **`per day` — the day's work asks before it starts.** When today's calls reach the daily + limit, a new chat turn or `/task` opens the same two-choice card used for a bounded team: + `1 Raise to $X` or `2 Stop for today`. Raise it to continue with that larger limit for + today's local day; stop refuses the work with `today's spending limit of $X is spent, so + nothing was started`. Headless `codeaf do` keeps its `--yes-spend` escape hatch. - **A task's own cap.** A task started from the composer layer (`alt+enter`) carries the figure on that layer's third line — `it may spend up to $100.00 before it asks` — and stops before its next turn when it reaches it. That figure is set where the task is @@ -3106,21 +3109,13 @@ conversation has spent four fifths of its own limit — the figure leaves the di nothing else changes. With no `per conversation` limit set there is no fraction and no colour. -## I started a task after my dollar limit was spent — why did it still pay for a call - -**A task started after this conversation's dollar limit is already spent still gets -one paid call before it ends.** `/task` is not a turn, so the refusal that stops the -next turn — `conversation limit reached · … · /budget changes it` — is not asked in -front of it. The run is handed the smallest figure above nothing rather than zero, -because zero would mean no limit at all. Its first worker makes one model call, that -call puts the run over the figure, and the run ends there: its row says -`a dollar limit you set stopped it`. The call is small, but it is real money, and it -shows in `/cost`. - -The dollar limit here is the smaller of `per conversation` and `--max-cost`, measured -against what this conversation has already spent. To let the task do its work, raise -`per conversation` first — `/budget conversation 20`, or `/budget conversation none` -to remove it — or relaunch with a larger `--max-cost`, then start the task again. +## I started a task after my daily limit was spent — why did it wait + +**A chat turn and a `/task` both stop before a provider call when today's daily limit +is already spent.** The card offers `Raise to $X` and `Stop for today`, just like a +bounded team. Raising persists the larger limit for today's local day and resumes the +held turn or task; stopping says `today's spending limit of $X is spent, so nothing was +started`. The per-conversation limit and a task's own cap remain separate rails. `codeaf do` has no such call: when today's spending limit is already spent it starts nothing and says, for a $5 limit, diff --git a/internal/manual/chat/tasks.md b/internal/manual/chat/tasks.md index 44c3bf517d..1966145c28 100644 --- a/internal/manual/chat/tasks.md +++ b/internal/manual/chat/tasks.md @@ -4397,6 +4397,11 @@ today's crew spend ($5.01) has reached the daily cap of $5.00 · raise it or tur The day turns over at midnight on this machine's clock, and the spend starts again from nothing. +The machine-wide daily limit in `/settings` → **Spending** and `/budget` is a separate +rail over every chat turn and task. If it is already spent, a task opened from chat waits +on the same card shape: `1 Raise to $X` continues with that amount for today, and `2 Stop +for today` refuses it with `today's spending limit of $X is spent, so nothing was started`. + ## Naming a model for one task You ask in words — "let opus do this one", "run that on gpt-5". There is no key, command or diff --git a/internal/run/crew_context_test.go b/internal/run/crew_context_test.go index 476b9f34a3..4002a56d29 100644 --- a/internal/run/crew_context_test.go +++ b/internal/run/crew_context_test.go @@ -103,7 +103,7 @@ func TestCrewFactoryGivesEachCheckerItsOwnSpendCeiling(t *testing.T) { CeilingAction: "checker ceiling $%.2f", } probes := []*crewCallProbe{} - factory := CrewFactory(store, dir, profile, Seats{}, func(model string) session.Completer { + factory := CrewFactory(store, dir, profile, Seats{}, "", func(model string) session.Completer { probe := &crewCallProbe{cost: 0.01} probes = append(probes, probe) return guard.Wrap(model, probe) diff --git a/internal/session/agent.go b/internal/session/agent.go index 3fccee1d65..1efc0e7bef 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -1041,6 +1041,10 @@ func (a *Agent) submitUser(ctx context.Context, user userMessage) (<-chan Event, // message is theirs to send again once the rail moves (rail.go). if user.bash == "" { if err := a.railBlockLocked(); err != nil { + var daily dailyBudgetReached + if errors.As(err, &daily) { + return a.holdDailyBudgetLocked(ctx, user, daily.spend), nil + } a.mu.Unlock() return refusedStream(err), nil } diff --git a/internal/session/answers.go b/internal/session/answers.go index 205d04b0df..ab3118270b 100644 --- a/internal/session/answers.go +++ b/internal/session/answers.go @@ -133,6 +133,8 @@ const ( QuestionFuel QuestionKind = "fuel" // QuestionAsk is the model's own question, raised through the ask tool. QuestionAsk QuestionKind = "ask" + // QuestionDailyBudget is the machine-wide spending limit holding a chat turn. + QuestionDailyBudget QuestionKind = "daily-budget" ) // AnswerOption is one answer a question will take: the key that gives it and diff --git a/internal/session/dailyspend.go b/internal/session/dailyspend.go new file mode 100644 index 0000000000..d5e3341fd0 --- /dev/null +++ b/internal/session/dailyspend.go @@ -0,0 +1,223 @@ +package session + +import ( + "bufio" + "context" + "encoding/json" + "errors" + "fmt" + "math" + "os" + "path/filepath" + "strings" + "time" + + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/teams" +) + +// DailySpend is the one reading of the machine-wide daily spending limit. The +// limit includes a day-scoped raise, and Spent is the same usage ledger every +// road writes; callers must not re-do either half of this arithmetic. +type DailySpend struct { + Limit float64 + Spent float64 + Reached bool +} + +type dailyBudgetRaise struct { + Day string `json:"day"` + Limit float64 `json:"limit"` + Origin string `json:"origin,omitempty"` +} + +type dailyBudgetWait struct { + question Question + limit float64 + raiseTo float64 + user userMessage + ctx context.Context + stream *eventStream + release func() +} + +type dailyBudgetReached struct{ spend DailySpend } + +func (e dailyBudgetReached) Error() string { return DailySpendAction(e.spend.Limit) } + +const dailyBudgetRaisesName = "daily_budget_raises.jsonl" + +// DailySpendAt reads today's effective limit and spend. A missing or unreadable +// usage ledger means no measured spend, matching the headless door's existing +// conservative ledger fallback; a bad budget setting remains an error. The +// optional ledger path is for private session ledgers; production callers use +// the machine-wide path. +func DailySpendAt(profileDir string, now time.Time, ledgerPath ...string) (DailySpend, error) { + limit, err := dailyBudgetLimitAt(profileDir, now) + if err != nil { + return DailySpend{}, err + } + path := UsageLedgerPath() + if len(ledgerPath) > 0 && strings.TrimSpace(ledgerPath[0]) != "" { + path = ledgerPath[0] + } + lines, err := ReadUsage(path, now.Add(-48*time.Hour)) + if err != nil { + return DailySpend{Limit: limit}, nil + } + spent := SpendToday(lines, now) + return DailySpend{Limit: limit, Spent: spent, Reached: limit > 0 && spent >= limit}, nil +} + +// RaiseDailySpend persists a larger ceiling for today's local day. The base +// setting remains unchanged, so tomorrow starts at the configured amount. +func RaiseDailySpend(profileDir string, now time.Time, limit float64, origin string) error { + base, err := config.DailyBudgetUSDAt(profileDir) + if err != nil { + return err + } + if limit <= base || math.IsNaN(limit) || math.IsInf(limit, 0) || strings.TrimSpace(origin) == "" { + return fmt.Errorf("raise daily spending limit: positive larger limit and origin are required") + } + path := config.ProfilePath(profileDir, dailyBudgetRaisesName) + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return fmt.Errorf("create daily spending limit journal: %w", err) + } + line, err := json.Marshal(dailyBudgetRaise{Day: localDay(now), Limit: limit, Origin: origin}) + if err != nil { + return fmt.Errorf("encode daily spending limit raise: %w", err) + } + file, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o600) + if err != nil { + return fmt.Errorf("open daily spending limit journal: %w", err) + } + if _, err := file.Write(append(line, '\n')); err != nil { + _ = file.Close() + return fmt.Errorf("write daily spending limit raise: %w", err) + } + if err := file.Close(); err != nil { + return fmt.Errorf("close daily spending limit journal: %w", err) + } + return nil +} + +func dailyBudgetLimitAt(profileDir string, now time.Time) (float64, error) { + base, err := config.DailyBudgetUSDAt(profileDir) + if err != nil { + return 0, err + } + if base <= 0 { + return 0, nil + } + file, err := os.Open(config.ProfilePath(profileDir, dailyBudgetRaisesName)) + if err != nil { + if os.IsNotExist(err) { + return base, nil + } + return base, nil + } + defer file.Close() + limit := base + scanner := bufio.NewScanner(file) + for scanner.Scan() { + var raise dailyBudgetRaise + if json.Unmarshal(scanner.Bytes(), &raise) == nil && raise.Day == localDay(now) && raise.Limit > limit { + limit = raise.Limit + } + } + return limit, nil +} + +func localDay(now time.Time) string { return now.Local().Format("2006-01-02") } + +// DailySpendAction is the refusal sentence shared by interactive chat and +// headless runs when today's limit leaves no work to start. +func DailySpendAction(limit float64) string { + return fmt.Sprintf("today's spending limit of $%.2f is spent, so nothing was started", limit) +} + +func dailyBudgetQuestion(id uint64, spend DailySpend) Question { + raiseTo := teams.RaiseTo(spend.Limit) + return Question{ + ID: id, + Kind: QuestionDailyBudget, + Ask: AskChoice, + Form: FormCard, + Asker: Asker{Kind: AskerEngine}, + Head: fmt.Sprintf("today's spending limit of %s is spent", teams.Money(spend.Limit)), + Reason: "new work cannot start until you choose whether to raise it or stop for today", + Options: []AnswerOption{ + {Key: "1", Label: "Raise to " + teams.Money(raiseTo), Consequence: "new work may spend up to " + teams.Money(raiseTo) + " today"}, + {Key: "2", Label: "Stop for today", Consequence: "nothing new starts until tomorrow", Safe: true}, + }, + Stakes: StakesCostly, + Blocking: Blocking{Turn: true}, + } +} + +// holdDailyBudgetLocked raises the same question card used by other bounded +// work and keeps the original Submit stream until the person decides. +func (a *Agent) holdDailyBudgetLocked(ctx context.Context, user userMessage, spend DailySpend) <-chan Event { + if a.dailyBudget != nil { + a.mu.Unlock() + return refusedStream(errors.New("today's spending limit is waiting for your answer")) + } + a.dailyBudgetSeq++ + q := dailyBudgetQuestion(a.dailyBudgetSeq, spend) + q.Asked = time.Now() + stream := newEventStream() + a.dailyBudget = &dailyBudgetWait{question: q, limit: spend.Limit, raiseTo: teams.RaiseTo(spend.Limit), user: user, ctx: ctx, stream: stream} + a.mu.Unlock() + release := a.presenceAskingWhole(q, nil) + a.mu.Lock() + if a.dailyBudget != nil && a.dailyBudget.question.ID == q.ID { + a.dailyBudget.release = release + } else { + release() + } + a.mu.Unlock() + return stream.out +} + +func (a *Agent) resolveDailyBudget(answer Answer) error { + key := answer.FirstKey() + a.mu.Lock() + wait := a.dailyBudget + if wait == nil || wait.question.ID != answer.ID { + a.mu.Unlock() + return errAnswerGone + } + if key == "2" { + a.dailyBudget = nil + release := wait.release + stream := wait.stream + a.mu.Unlock() + if release != nil { + release() + } + refuseOn(stream, errors.New(DailySpendAction(wait.limit))) + return nil + } + if key != "1" { + a.mu.Unlock() + return errAnswerEmpty + } + profile, origin := a.config.ProfileDir, "chat:daily-budget" + a.mu.Unlock() + if err := RaiseDailySpend(profile, time.Now(), wait.raiseTo, origin); err != nil { + return err + } + a.mu.Lock() + if a.dailyBudget != wait { + a.mu.Unlock() + return errAnswerGone + } + a.dailyBudget = nil + a.startTurnLocked(wait.ctx, wait.user, wait.stream) + release := wait.release + a.mu.Unlock() + if release != nil { + release() + } + return nil +} diff --git a/internal/session/question.go b/internal/session/question.go index fb9e396525..5861d925f8 100644 --- a/internal/session/question.go +++ b/internal/session/question.go @@ -1877,6 +1877,8 @@ func questionGoneReason(q Question) string { return "the work settled" case QuestionFuel: return "the run is no longer at its gate" + case QuestionDailyBudget: + return "the spending limit is no longer holding the turn" } // The model's own question and everything else: the turn that raised it // has ended — interrupted, or finished around it — which is the one way a @@ -2273,6 +2275,8 @@ func (a *Agent) applyToLane(answer Answer) error { case QuestionFuel: _, err := a.ResolveOrchestrate(answer.Ref, fuelAnswer(key, words)) return err + case QuestionDailyBudget: + return a.resolveDailyBudget(answer) } return errAnswerUnknownLane } @@ -2427,6 +2431,10 @@ func (a *Agent) OpenQuestions() []Question { a.mu.Lock() modelAsks := a.asked.openLocked() + var dailyBudget Question + if a.dailyBudget != nil { + dailyBudget = a.dailyBudget.question + } consent := make([]uint64, 0, len(a.consent)) for id := range a.consent { consent = append(consent, id) @@ -2469,6 +2477,9 @@ func (a *Agent) OpenQuestions() []Question { runs[id] = live } a.mu.Unlock() + if dailyBudget.Kind != "" { + open = append(open, dailyBudget) + } for _, id := range modelAsks { if q, ok := a.questionSaid(QuestionAsk, strconv.FormatUint(id, 10)); ok { open = append(open, q) diff --git a/internal/session/rail.go b/internal/session/rail.go index b3d3ff0029..703d9bb793 100644 --- a/internal/session/rail.go +++ b/internal/session/rail.go @@ -30,7 +30,7 @@ import ( "fmt" "math" - "github.com/Agent-Field/codeaf/internal/config" + "time" ) // SetSpendRail binds a setting written in an open chat before the next turn @@ -63,15 +63,14 @@ func (a *Agent) railBlockLocked() error { return err } if !a.config.InTask && !a.config.Errand { - if daily, err := config.DailyBudgetUSDAt(a.config.ProfileDir); err == nil && daily > 0 { - spentToday := spentTodayOnLedger() + if daily, err := DailySpendAt(a.config.ProfileDir, time.Now(), a.config.usageLedger); err == nil { + spentToday := daily.Spent if a.crewDayHeld != nil { spentToday = max(spentToday, a.crewDayHeld.Total()) } - if spentToday >= daily { - return spendRailReached{said: fmt.Sprintf( - "daily limit reached · %s spent of %s · /budget changes it", - railMoney(spentToday), railMoney(daily))} + if daily.Limit > 0 && spentToday >= daily.Limit { + daily.Spent, daily.Reached = spentToday, true + return dailyBudgetReached{spend: daily} } } } diff --git a/internal/session/rail_test.go b/internal/session/rail_test.go index 24a96810f7..178b8706ce 100644 --- a/internal/session/rail_test.go +++ b/internal/session/rail_test.go @@ -3,8 +3,10 @@ package session import ( "context" "errors" + "path/filepath" "strings" "testing" + "time" "github.com/Agent-Field/agentfield/sdk/go/ai" "github.com/Agent-Field/codeaf/internal/config" @@ -50,6 +52,64 @@ func TestSpendRailRefusesTheTurnAndDoesNoWork(t *testing.T) { } } +func TestDailySpendRailShowsCardAndStopsOrRaises(t *testing.T) { + for _, tc := range []struct { + name string + key string + want string + }{ + {name: "stop", key: "2", want: "today's spending limit of $1.00 is spent, so nothing was started"}, + {name: "raise", key: "1", want: "turn finished"}, + } { + t.Run(tc.name, func(t *testing.T) { + profile := t.TempDir() + ledger := filepath.Join(t.TempDir(), "usage.jsonl") + if err := config.WriteDailyBudgetUSD(profile, 1); err != nil { + t.Fatal(err) + } + now := time.Now() + recordUsage(t, ledger, UsageLine{At: now, Day: localDay(now), Calls: 1, USD: 1.01}) + completer := &scriptedCompleter{steps: []step{ + func(context.Context, []ai.Message) (*ai.Response, error) { + return textResponse("continued"), nil + }, + }} + agent, _ := newTestAgent(t, completer, func(c *Config) { + c.ProfileDir = profile + c.usageLedger = ledger + }) + + stream := mustSubmit(t, agent, "start work") + question := waitForOneQuestion(t, agent) + if question.Kind != QuestionDailyBudget || len(question.Options) != 2 || question.Options[0].Key != "1" || question.Options[0].Label != "Raise to $2" || question.Options[1].Key != "2" || question.Options[1].Label != "Stop for today" { + t.Fatalf("daily question = %+v, want raise and stop choices", question) + } + if got := completer.requests(); got != 0 { + t.Fatalf("provider calls before the answer = %d, want 0", got) + } + if err := agent.ResolveQuestion(Answer{Kind: question.Kind, ID: question.ID, Key: tc.key}); err != nil { + t.Fatalf("ResolveQuestion: %v", err) + } + events := collect(t, stream) + if tc.key == "2" { + if len(events) != 1 || events[0].Kind != EventError || events[0].Err == nil || events[0].Err.Error() != tc.want { + t.Fatalf("stop events = %v, want the daily refusal", events) + } + if got := completer.requests(); got != 0 { + t.Fatalf("provider calls after stop = %d, want 0", got) + } + return + } + if len(events) == 0 || events[len(events)-1].Kind != EventTurnDone || completer.requests() != 1 { + t.Fatalf("raise events = %v, calls = %d, want a completed turn and one call", kinds(events), completer.requests()) + } + if daily, err := DailySpendAt(profile, now, ledger); err != nil || daily.Limit != 2 { + t.Fatalf("raised daily limit = %+v, err=%v, want $2.00 today", daily, err) + } + }) + } +} + // Under the line the rail is not there. func TestSpendRailUnderTheLineRunsTheTurn(t *testing.T) { completer := &scriptedCompleter{steps: []step{ diff --git a/internal/session/session.go b/internal/session/session.go index 542339016c..5d0c0fa0d1 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2281,7 +2281,11 @@ type Agent struct { // that lock. Atomic publication keeps both roads on the same figure. liveSpendRail atomic.Uint64 liveSpendRailSet atomic.Bool - client Completer + // dailyBudget holds the one chat turn waiting for the person to raise or + // stop today's spending limit. Its stream is adopted by the resumed turn. + dailyBudget *dailyBudgetWait + dailyBudgetSeq uint64 + client Completer // managedClient distinguishes the provider adapter built by New from a test // completer handed to newAgent. clientAccount is the resolved account the // adapter holds, so a service-set change can replace it before another call. diff --git a/internal/session/spendguard_test.go b/internal/session/spendguard_test.go index 4aa535fbbe..a29c097b47 100644 --- a/internal/session/spendguard_test.go +++ b/internal/session/spendguard_test.go @@ -3,6 +3,7 @@ package session import ( "context" "errors" + "path/filepath" "strings" "testing" "time" @@ -146,6 +147,28 @@ func TestTheTaskLimitIsOnEveryGuard(t *testing.T) { } } +func TestTheCrewGuardFoldsInTheDailyLimit(t *testing.T) { + profile := t.TempDir() + ledger := filepath.Join(t.TempDir(), "usage.jsonl") + if err := config.WriteDailyBudgetUSD(profile, 1); err != nil { + t.Fatal(err) + } + now := time.Now() + recordUsage(t, ledger, UsageLine{At: now, Day: localDay(now), Calls: 1, USD: 1.01}) + + guard := crewSpendGuard(profile, crewroute.Decision{}, true) + guard.Day = newLedgerSpendDay(func() float64 { return 1.01 }, time.Now) + if guard.Cap != 1 || guard.CapAction != "today's spending limit of $1.00 is reached · raise it with /budget" { + t.Fatalf("crew daily guard = cap $%v, action %q", guard.Cap, guard.CapAction) + } + worker := &spendingCompleter{usd: 0.01} + _, err := guard.Wrap("test/model", worker).CompleteWithMessages(t.Context(), []ai.Message{textMessage("user", "work")}) + var stopped ErrSpendStopped + if !errors.As(err, &stopped) || stopped.Action != guard.CapAction || worker.calls != 0 { + t.Fatalf("daily cap result = %v after %d calls, want refusal before the provider", err, worker.calls) + } +} + // A SHARED MODEL DOES NOT SHARE A SEAT'S CEILING. The checker also keeps its // tally when its next call goes through another model on the fallback ladder. func TestSharedModelCheckerCeilingFollowsTheSeat(t *testing.T) { diff --git a/internal/session/taskcrew.go b/internal/session/taskcrew.go index a7fa5eb51b..e162ef33b0 100644 --- a/internal/session/taskcrew.go +++ b/internal/session/taskcrew.go @@ -576,7 +576,7 @@ func (e errRouteResting) Error() string { // the per-task limit — which withDaily does not touch — and the checker's // ceiling. func crewSpendGuard(profileDir string, d crewroute.Decision, withDaily bool) *SpendGuard { - capUSD, action := config.CrewSpendCap(profileDir, withDaily) + capUSD, action := crewSpendCap(profileDir, withDaily) taskCap, taskAction := config.CrewTaskSpendCap(profileDir) guard := &SpendGuard{ Price: config.CrewCallPriceAt(profileDir), Cap: capUSD, CapAction: action, @@ -608,7 +608,7 @@ func (a *Agent) helperGuard(crew *taskCrew) *SpendGuard { return nil } guard := &SpendGuard{Price: config.CrewCallPriceAt(a.config.ProfileDir), Day: a.crewDay()} - if capUSD, action := config.CrewSpendCap(a.config.ProfileDir, true); capUSD > 0 { + if capUSD, action := crewSpendCap(a.config.ProfileDir, true); capUSD > 0 { guard.Cap, guard.CapAction = capUSD, action } if crew != nil && crew.guard != nil { @@ -704,6 +704,20 @@ func TaskSpendGuard(profileDir string) *SpendGuard { return &SpendGuard{Price: config.CrewCallPriceAt(profileDir), TaskCap: taskCap, TaskAction: taskAction, Task: &SpendTask{}} } +// crewSpendCap combines the recurring crew cap with the shared machine-wide +// daily limit. A raise changes only the latter for today's local day. +func crewSpendCap(profileDir string, withDaily bool) (float64, string) { + capUSD, action := config.CrewSpendCap(profileDir, false) + if !withDaily { + return capUSD, action + } + limit, err := dailyBudgetLimitAt(profileDir, time.Now()) + if err != nil || limit <= 0 || (capUSD > 0 && capUSD <= limit) { + return capUSD, action + } + return limit, fmt.Sprintf("today's spending limit of $%.2f is reached · raise it with /budget", limit) +} + // spentTodayOnLedger is today's spend as the usage ledger has it; nothing // when the ledger cannot be read. func spentTodayOnLedger() float64 { diff --git a/internal/session/taskcrew_record.go b/internal/session/taskcrew_record.go index 5032f6e6a4..2127a4dcb1 100644 --- a/internal/session/taskcrew_record.go +++ b/internal/session/taskcrew_record.go @@ -82,8 +82,10 @@ func (c *taskCrew) record() *TaskCrewRecord { g.mu.Lock() r.Guard = &crewGuardRecord{Cap: g.Cap, TaskCap: g.TaskCap, CapAction: g.CapAction, TaskAction: g.TaskAction, CeilingAction: g.CeilingAction, SeatCeilings: maps.Clone(g.SeatCeilings), ModelSpent: maps.Clone(g.modelSpent), TaskSpent: g.Task.Total(), SeatSpent: map[crewroute.Seat]float64{}, Day: time.Now().Format("2006-01-02")} - for seat, tally := range g.seatSpent { - r.Guard.SeatSpent[seat] = tally.Total() + // A CHECK'S CEILING IS KEPT PER CHECK TASK, but the saved policy holds + // one figure per seat: the largest any one task of the seat has spent. + for key, tally := range g.seatSpent { + r.Guard.SeatSpent[key.seat] = max(r.Guard.SeatSpent[key.seat], tally.Total()) } if g.Day != nil { g.Day.mu.Lock() @@ -149,9 +151,9 @@ func (a *Agent) restoreTaskCrew(row uint64, r *TaskCrewRecord) (*taskCrew, error restoreCrewDay(day, s) c.day = day c.guard = &SpendGuard{Price: config.CrewCallPriceAt(a.config.ProfileDir), Day: day, Cap: s.Cap, TaskCap: s.TaskCap, CapAction: s.CapAction, TaskAction: s.TaskAction, - CeilingAction: s.CeilingAction, SeatCeilings: s.SeatCeilings, modelSpent: s.ModelSpent, Task: &SpendTask{spent: s.TaskSpent}, seatSpent: map[crewroute.Seat]*SpendTask{}} + CeilingAction: s.CeilingAction, SeatCeilings: s.SeatCeilings, modelSpent: s.ModelSpent, Task: &SpendTask{spent: s.TaskSpent}, seatSpent: map[spendTallyKey]*SpendTask{}} for seat, spent := range s.SeatSpent { - c.guard.seatSpent[seat] = &SpendTask{spent: spent} + c.guard.seatSpent[spendTallyKey{seat: seat}] = &SpendTask{spent: spent} } a.bindCrewCheckpoint(row, c) a.crews.put(row, c) diff --git a/internal/session/taskcrew_recovery_test.go b/internal/session/taskcrew_recovery_test.go index a7b3396117..d1f421c10f 100644 --- a/internal/session/taskcrew_recovery_test.go +++ b/internal/session/taskcrew_recovery_test.go @@ -156,7 +156,7 @@ func TestAdmittedRecoveryRestoresRoutingAndSpendState(t *testing.T) { // Seed an accepted crew with a prior fallback and an exhausted task cap. d := recoveryDecision() worker := d.Seat(crewroute.Worker).Send - c := &taskCrew{call: "recorded-call", title: "saved task", brief: "saved brief", repo: repo, decision: d, original: map[crewroute.Seat]string{}, ladders: d.Ladder, started: map[string]bool{"accepted/replacement": true}, swaps: map[string]string{worker: "accepted/replacement"}, bad: map[string]bool{worker: true}, broke: map[string]bool{"old-account": true}, gone: map[string]bool{"old-provider": true}, freeTried: map[crewroute.Seat]int{crewroute.Worker: 2}, failed: map[string]crewFailure{worker: {kind: provider.RouteQuota, until: time.Now().Add(time.Hour), err: errors.New("resting")}}, guard: &SpendGuard{TaskCap: 2, TaskAction: "accepted task cap reached", Task: &SpendTask{spent: 2}, SeatCeilings: map[crewroute.Seat]float64{crewroute.Checker: 1}, seatSpent: map[crewroute.Seat]*SpendTask{crewroute.Checker: {spent: .5}}, modelSpent: map[string]float64{worker: .25}}} + c := &taskCrew{call: "recorded-call", title: "saved task", brief: "saved brief", repo: repo, decision: d, original: map[crewroute.Seat]string{}, ladders: d.Ladder, started: map[string]bool{"accepted/replacement": true}, swaps: map[string]string{worker: "accepted/replacement"}, bad: map[string]bool{worker: true}, broke: map[string]bool{"old-account": true}, gone: map[string]bool{"old-provider": true}, freeTried: map[crewroute.Seat]int{crewroute.Worker: 2}, failed: map[string]crewFailure{worker: {kind: provider.RouteQuota, until: time.Now().Add(time.Hour), err: errors.New("resting")}}, guard: &SpendGuard{TaskCap: 2, TaskAction: "accepted task cap reached", Task: &SpendTask{spent: 2}, SeatCeilings: map[crewroute.Seat]float64{crewroute.Checker: 1}, seatSpent: map[spendTallyKey]*SpendTask{{seat: crewroute.Checker}: {spent: .5}}, modelSpent: map[string]float64{worker: .25}}} for _, pick := range d.Crew { c.original[pick.Seat] = pick.Send } diff --git a/internal/tui3/question.go b/internal/tui3/question.go index 4fe57be8de..775f361a59 100644 --- a/internal/tui3/question.go +++ b/internal/tui3/question.go @@ -4458,8 +4458,8 @@ func (a *app) questionDrawnHere(q session.Question) bool { // shape of a landing that has since been re-settled offered a person the // wrong answers to the right question (#767, tasksettle.go). return true - case session.QuestionSubharnessAsk, session.QuestionFuel, session.QuestionConflict: - // The three lanes the audit found with a resolver and NOTHING ANYWHERE + case session.QuestionSubharnessAsk, session.QuestionFuel, session.QuestionConflict, session.QuestionDailyBudget: + // The lanes the audit found with a resolver and NOTHING ANYWHERE // that drew them: work stopped on a question no surface in this product // could put to a person. They are drawn here first because there is no // older block to retire — this block is the only one they have ever had. From ca817ba922d12c1e767a5f538215166d98bc63d2 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 11:17:03 -0400 Subject: [PATCH 07/10] spend: a chat task past the daily limit asks first, and the top bar shows the raised limit A /task or an approved proposal past today's limit reported "started" and then sat working, held by the crew guard with nothing said. Task admission now waits on the same daily-budget card as a chat turn: stop refuses with the headless sentence, raise re-checks and admits. The top bar's limit is read through the same DailySpendAt reading, so a raise shows at once. Part of #1546 --- internal/manual/chat/models-and-cost.md | 4 +- internal/session/agent.go | 5 + internal/session/dailyspend.go | 110 +++++++++--- internal/session/dailyspend_task_test.go | 206 +++++++++++++++++++++++ internal/session/session.go | 4 +- internal/session/task.go | 3 + internal/session/task_person.go | 8 +- internal/tui3/homemachine.go | 13 +- internal/tui3/pulsemoney_test.go | 30 ++++ 9 files changed, 351 insertions(+), 32 deletions(-) create mode 100644 internal/session/dailyspend_task_test.go diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index ae04da28c1..9d6f12fda9 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -3114,7 +3114,9 @@ colour. **A chat turn and a `/task` both stop before a provider call when today's daily limit is already spent.** The card offers `Raise to $X` and `Stop for today`, just like a bounded team. Raising persists the larger limit for today's local day and resumes the -held turn or task; stopping says `today's spending limit of $X is spent, so nothing was +held turn or task. `/task` and approved `propose_task` work are not started or +listed as working while this card waits. The top bar shows the raised limit. +Stopping says `today's spending limit of $X is spent, so nothing was started`. The per-conversation limit and a task's own cap remain separate rails. `codeaf do` has no such call: when today's spending limit is already spent it starts diff --git a/internal/session/agent.go b/internal/session/agent.go index 1efc0e7bef..c1d13e944c 100644 --- a/internal/session/agent.go +++ b/internal/session/agent.go @@ -2391,6 +2391,8 @@ func (a *Agent) Close() error { return nil } a.closed = true + budgetWait := a.dailyBudget + a.dailyBudget = nil // Nothing armed by a steer outlives the session that armed it // (steer_grace.go), and nor does a clock armed on a question (asklane.go). a.stopSteerGraceLocked() @@ -2453,6 +2455,9 @@ func (a *Agent) Close() error { // joins from inside its own goroutine. a.cancelOrchestrationsLocked() a.mu.Unlock() + if budgetWait != nil { + budgetWait.finish(errAgentClosed) + } // AND THE PROCESS STOPS SAYING IT HOLDS THIS CONVERSATION, before anything // below can take time: a firing that lands during the quit writes to the // inbox rather than onto a queue that will never be drained again diff --git a/internal/session/dailyspend.go b/internal/session/dailyspend.go index d5e3341fd0..81f477e451 100644 --- a/internal/session/dailyspend.go +++ b/internal/session/dailyspend.go @@ -39,6 +39,8 @@ type dailyBudgetWait struct { ctx context.Context stream *eventStream release func() + // Task admission waits on its existing caller rather than opening a turn. + taskResult chan error } type dailyBudgetReached struct{ spend DailySpend } @@ -155,28 +157,102 @@ func dailyBudgetQuestion(id uint64, spend DailySpend) Question { } } -// holdDailyBudgetLocked raises the same question card used by other bounded -// work and keeps the original Submit stream until the person decides. +// holdDailyBudgetLocked keeps the original Submit stream until the person +// decides, so raising the limit resumes the same turn. func (a *Agent) holdDailyBudgetLocked(ctx context.Context, user userMessage, spend DailySpend) <-chan Event { + stream := newEventStream() + wait := &dailyBudgetWait{user: user, ctx: ctx, stream: stream} + if err := a.openDailyBudgetLocked(spend, wait); err != nil { + refuseOn(stream, err) + } + return stream.out +} + +// openDailyBudgetLocked publishes one budget question for either a turn or a +// task admission. It always releases mu; publication itself needs that lock. +func (a *Agent) openDailyBudgetLocked(spend DailySpend, wait *dailyBudgetWait) error { + if a.closed { + a.mu.Unlock() + return errAgentClosed + } if a.dailyBudget != nil { a.mu.Unlock() - return refusedStream(errors.New("today's spending limit is waiting for your answer")) + return errors.New("today's spending limit is waiting for your answer") } a.dailyBudgetSeq++ q := dailyBudgetQuestion(a.dailyBudgetSeq, spend) q.Asked = time.Now() - stream := newEventStream() - a.dailyBudget = &dailyBudgetWait{question: q, limit: spend.Limit, raiseTo: teams.RaiseTo(spend.Limit), user: user, ctx: ctx, stream: stream} + wait.question, wait.limit, wait.raiseTo = q, spend.Limit, teams.RaiseTo(spend.Limit) + a.dailyBudget = wait a.mu.Unlock() release := a.presenceAskingWhole(q, nil) a.mu.Lock() - if a.dailyBudget != nil && a.dailyBudget.question.ID == q.ID { - a.dailyBudget.release = release + if a.dailyBudget == wait { + wait.release = release + a.mu.Unlock() } else { + a.mu.Unlock() release() } - a.mu.Unlock() - return stream.out + return nil +} + +// awaitTaskDailyBudget gates both typed tasks and approved proposals before +// either engine admits work. A raise is checked again because another window +// may have spent beyond even that limit while the question was standing. +func (a *Agent) awaitTaskDailyBudget(ctx context.Context) error { + for { + if err := ctx.Err(); err != nil { + return err + } + a.mu.Lock() + closed := a.closed + a.mu.Unlock() + if closed { + return errAgentClosed + } + daily, err := DailySpendAt(a.config.ProfileDir, time.Now(), a.config.usageLedger) + if err != nil { + return err + } + if !daily.Reached { + return nil + } + wait := &dailyBudgetWait{taskResult: make(chan error, 1)} + a.mu.Lock() + if err := a.openDailyBudgetLocked(daily, wait); err != nil { + return err + } + select { + case err := <-wait.taskResult: + if err != nil { + return err + } + case <-ctx.Done(): + a.mu.Lock() + if a.dailyBudget == wait { + a.dailyBudget = nil + a.mu.Unlock() + wait.finish(ctx.Err()) + } else { + a.mu.Unlock() + } + return ctx.Err() + } + } +} + +// finish releases the card before its caller can report a refusal or a start. +// The owner detaches the wait under mu first, so exactly one ending reaches it. +func (w *dailyBudgetWait) finish(err error) { + if w.release != nil { + w.release() + } + if w.taskResult != nil { + w.taskResult <- err + } else if err != nil { + refuseOn(w.stream, err) + } } func (a *Agent) resolveDailyBudget(answer Answer) error { @@ -189,13 +265,8 @@ func (a *Agent) resolveDailyBudget(answer Answer) error { } if key == "2" { a.dailyBudget = nil - release := wait.release - stream := wait.stream a.mu.Unlock() - if release != nil { - release() - } - refuseOn(stream, errors.New(DailySpendAction(wait.limit))) + wait.finish(errors.New(DailySpendAction(wait.limit))) return nil } if key != "1" { @@ -213,11 +284,10 @@ func (a *Agent) resolveDailyBudget(answer Answer) error { return errAnswerGone } a.dailyBudget = nil - a.startTurnLocked(wait.ctx, wait.user, wait.stream) - release := wait.release - a.mu.Unlock() - if release != nil { - release() + if wait.taskResult == nil { + a.startTurnLocked(wait.ctx, wait.user, wait.stream) } + a.mu.Unlock() + wait.finish(nil) return nil } diff --git a/internal/session/dailyspend_task_test.go b/internal/session/dailyspend_task_test.go new file mode 100644 index 0000000000..f4f1947a8f --- /dev/null +++ b/internal/session/dailyspend_task_test.go @@ -0,0 +1,206 @@ +package session + +import ( + "context" + "encoding/json" + "path/filepath" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/config" +) + +func TestTaskDailyBudgetAdmission(t *testing.T) { + for _, road := range []string{"typed", "proposal", "bash"} { + for _, choice := range []string{"stop", "raise"} { + t.Run(road+"/"+choice, func(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "") + if road == "bash" { + t.Setenv("CODEAF_TASK_BELT", "bash") + registerBeltRunEngine(t, newBeltRunDouble("done")) + } + profile, place := t.TempDir(), t.TempDir() + ledger := filepath.Join(t.TempDir(), "usage.jsonl") + if err := config.WriteDailyBudgetUSD(profile, 0.01); err != nil { + t.Fatal(err) + } + now := time.Now() + if road != "proposal" { + recordUsage(t, ledger, UsageLine{At: now, Day: localDay(now), Calls: 1, USD: 0.011}) + } + client := &scriptedCompleter{} + agent, _ := newTestAgent(t, client, func(c *Config) { + c.ProfileDir, c.usageLedger = profile, ledger + c.Workspace = newTestRepo(t) + c.Place = Place{Dir: place} + c.SessionFile = filepath.Join(place, placeTranscript) + c.AskConsent = true + c.TaskAutoApproveSeconds = 0 + }) + if road == "proposal" { + // Interactive proposals wait for the answer instead of headless auto-approval. + agent.hub = newEventHub() + } + graph := stubbedGraph(agent, func(n *TaskNode) { n.graph.complete(n, TaskDone) }) + questions, stop := agent.WatchQuestions() + defer stop() + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + type outcome struct { + id uint64 + text string + err error + } + result := make(chan outcome, 1) + go func() { + if road == "proposal" { + args, _ := json.Marshal(taskArguments{Title: "Document main", Summary: "Add the comment", Brief: "add a one-line doc comment to main.go\n" + taskBriefMark, Deliverable: "main.go", Acceptance: "main has a doc comment"}) + text, _, err := agent.proposeTask(ctx, args) + result <- outcome{text: text, err: err} + return + } + id, _, _, err := agent.StartTask(ctx, "add a one-line doc comment to main.go", true) + result <- outcome{id: id, err: err} + }() + var q Question + deadline := time.After(5 * time.Second) + asking: + for { + select { + case event := <-questions: + if event.Kind != EventQuestion || event.Question == nil { + continue + } + if event.Question.Kind == QuestionTask { + // The day can cross its limit while an approval card is open. + recordUsage(t, ledger, UsageLine{At: now, Day: localDay(now), Calls: 1, USD: 0.011}) + agent.ResolveTask(event.Question.ID, TaskAnswer{Approved: true}) + continue + } + if event.Question.Kind == QuestionDailyBudget { + q = *event.Question + break asking + } + case got := <-result: + t.Fatalf("task admission returned before a daily-budget card: %+v", got) + case <-deadline: + t.Fatal("no daily-budget card") + } + } + if admitted(graph) != 0 || len(agent.PlanTasks()) != 0 || client.requests() != 0 { + t.Fatal("work started before the daily-budget answer") + } + if q.Options[0].Label != "Raise to $0.02" || q.Options[1].Label != "Stop for today" { + t.Fatalf("wrong choices: %+v", q.Options) + } + select { + case got := <-result: + t.Fatalf("premature started receipt: %+v", got) + default: + } + key := "2" + if choice == "raise" { + key = "1" + } + if err := agent.ResolveQuestion(Answer{Kind: q.Kind, ID: q.ID, Key: key}); err != nil { + t.Fatal(err) + } + var got outcome + select { + case got = <-result: + case <-deadline: + t.Fatal("task start did not settle") + } + if choice == "stop" { + refusal := got.text + if got.err != nil { + refusal = got.err.Error() + } + if refusal != DailySpendAction(0.01) || got.id != 0 { + t.Fatalf("stop = %+v, want exact daily refusal", got) + } + if admitted(graph) != 0 || len(agent.PlanTasks()) != 0 || client.requests() != 0 { + t.Fatal("stop admitted work") + } + } else { + if got.err != nil { + t.Fatal(got.err) + } + if road == "bash" { + if len(agent.PlanTasks()) != 1 { + t.Fatal("raise did not start the run") + } + } else if admitted(graph) != 1 { + t.Fatal("raise did not admit exactly one task") + } + if daily, err := DailySpendAt(profile, now, ledger); err != nil || daily.Limit != 0.02 { + t.Fatalf("daily = %+v, %v", daily, err) + } + } + }) + } + } +} + +func TestTaskDailyBudgetWaitEndsWithCaller(t *testing.T) { + for _, ending := range []string{"cancel", "close"} { + t.Run(ending, func(t *testing.T) { + profile := t.TempDir() + ledger := filepath.Join(t.TempDir(), "usage.jsonl") + if err := config.WriteDailyBudgetUSD(profile, 1); err != nil { + t.Fatal(err) + } + now := time.Now() + recordUsage(t, ledger, UsageLine{At: now, Day: localDay(now), Calls: 1, USD: 1.01}) + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(c *Config) { c.ProfileDir, c.usageLedger = profile, ledger }) + questions, stop := agent.WatchQuestions() + defer stop() + ctx, cancel := context.WithCancel(t.Context()) + defer cancel() + result := make(chan error, 1) + go func() { _, _, _, err := agent.StartTask(ctx, "add a doc comment", true); result <- err }() + deadline := time.After(5 * time.Second) + var q Question + asking: + for { + select { + case event := <-questions: + if event.Kind == EventQuestion && event.Question != nil && event.Question.Kind == QuestionDailyBudget { + q = *event.Question + break asking + } + case err := <-result: + t.Fatalf("returned before asking: %v", err) + case <-deadline: + t.Fatal("no daily-budget question") + } + } + want := context.Canceled + if ending == "close" { + want = errAgentClosed + if err := agent.Close(); err != nil { + t.Fatal(err) + } + } else { + cancel() + } + select { + case err := <-result: + if err != want { + t.Fatalf("ending = %v, want %v", err, want) + } + case <-deadline: + t.Fatal("task admission outlived its caller") + } + if len(agent.OpenQuestions()) != 0 { + t.Fatal("budget card survived its caller") + } + if err := agent.ResolveQuestion(Answer{Kind: q.Kind, ID: q.ID, Key: "1"}); err == nil { + t.Fatal("stale raise resumed a cancelled task") + } + if daily, err := DailySpendAt(profile, now, ledger); err != nil || daily.Limit != 1 { + t.Fatalf("cancelled task raised the limit: %+v, %v", daily, err) + } + }) + } +} diff --git a/internal/session/session.go b/internal/session/session.go index 5d0c0fa0d1..be93ddef6e 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2281,8 +2281,8 @@ type Agent struct { // that lock. Atomic publication keeps both roads on the same figure. liveSpendRail atomic.Uint64 liveSpendRailSet atomic.Bool - // dailyBudget holds the one chat turn waiting for the person to raise or - // stop today's spending limit. Its stream is adopted by the resumed turn. + // dailyBudget holds the turn or task admission waiting for the person to + // raise or stop today's spending limit. No task exists until it is released. dailyBudget *dailyBudgetWait dailyBudgetSeq uint64 client Completer diff --git a/internal/session/task.go b/internal/session/task.go index d231aeae03..8f5e65cf4a 100644 --- a/internal/session/task.go +++ b/internal/session/task.go @@ -829,6 +829,9 @@ func (p *stagedProposal) Commit(ctx context.Context) (string, bool, error) { } return withElsewhere("the person declined this task", elsewhere), false, nil } + if err := a.awaitTaskDailyBudget(ctx); err != nil { + return err.Error(), true, nil + } if redirect := strings.TrimSpace(answer.Redirect); redirect != "" { // APPENDED, never merged into the brief's prose. The person's words // arrive last and in their own voice, so the node reads them as the diff --git a/internal/session/task_person.go b/internal/session/task_person.go index 4ac9b88239..99e190b505 100644 --- a/internal/session/task_person.go +++ b/internal/session/task_person.go @@ -42,9 +42,10 @@ type taskJudgeVerdict struct { } // StartTask starts one person-authored task without routing it through the chat -// model or presenting the model's proposal card. +// model or presenting the model's proposal card. If today's spending limit is +// spent, the daily-budget card must be answered before either engine admits it. // -// ── NOTHING IS WAITED FOR IN FRONT OF IT ── +// ── NO MODEL READING IS WAITED FOR IN FRONT OF IT ── // // WHAT WAS TRUE: the surface asked the sizing judge (three seconds) and then this // door asked the shaper (twenty-five) before the node was admitted, in series, @@ -76,6 +77,9 @@ func (a *Agent) StartTask(ctx context.Context, brief string, solo bool) (uint64, if brief == "" { return 0, "", "", errors.New("a task needs a brief") } + if err := a.awaitTaskDailyBudget(ctx); err != nil { + return 0, "", "", err + } // WHEN THE BASH BELT IS ASKED FOR this door takes its second road: a run on // the run engine, answered AT ONCE with the id the store knows the work by, // so the conversation stays usable while the run goes (task_run_belt.go). diff --git a/internal/tui3/homemachine.go b/internal/tui3/homemachine.go index 44e04e5fa8..f9d656fb1d 100644 --- a/internal/tui3/homemachine.go +++ b/internal/tui3/homemachine.go @@ -30,7 +30,6 @@ import ( "strings" "time" - "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/session" ) @@ -224,13 +223,13 @@ func machineDayStart(now time.Time) time.Time { // against — the person's own daily budget row, which is the rail a firing is // held to as well (cmd/codeaf's v3StandingDailyRail). // -// IT IS ONE SETTING READ IN ONE PLACE, and the pulse is the one line that draws -// it — as the denominator under what has been spent, and only where a machine -// has an allowance at all (pulse.go's [app.pulseSegments]). +// It includes today's raises through the same reading that admits work. The +// pulse draws it as the denominator under what has been spent, and only where +// a machine has an allowance at all (pulse.go's [app.pulseSegments]). func (a *app) machineAllowance() float64 { - rail, err := config.DailyBudgetUSDAt(a.profileDir) - if err != nil || rail <= 0 { + daily, err := session.DailySpendAt(a.profileDir, a.now()) + if err != nil || daily.Limit <= 0 { return 0 } - return rail + return daily.Limit } diff --git a/internal/tui3/pulsemoney_test.go b/internal/tui3/pulsemoney_test.go index 78226cd8fc..79fd33a1b5 100644 --- a/internal/tui3/pulsemoney_test.go +++ b/internal/tui3/pulsemoney_test.go @@ -264,3 +264,33 @@ func TestTheTopLineSpellsTheLimitTheWayEverySurfaceSpellsIt(t *testing.T) { } } } + +func TestTopBarUsesRaisedDailyLimit(t *testing.T) { + t.Setenv("CODEAF_DAILY_BUDGET", "0.01") + lab := newHomeLab(t) + now := moneyFixtureNoon() + here := lab.workspace("alpha") + mine := lab.session("-tmp-alpha", "aaaa000000000001", "here", here, now) + a := lab.app(mine) + a.clock = func() time.Time { return now } + raw, err := json.Marshal(session.UsageLine{At: now, Session: "aaaa000000000001", Model: "m", Calls: 1, USD: 0.011}) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(lab.root, session.UsageLedgerName), append(raw, '\n'), 0o600); err != nil { + t.Fatal(err) + } + a.readMachineMoney(now) + if a.machine.ceiling != 0.01 { + t.Fatalf("base limit = %v", a.machine.ceiling) + } + if err := session.RaiseDailySpend(a.profileDir, now, 0.02, "test"); err != nil { + t.Fatal(err) + } + a.readMachineMoney(now) + got := plain(a.pulseParts(now, a.pal, pulseWhole).money) + want := "$0.01" + pulseAllowanceGap + "$0.02" + if got != want { + t.Fatalf("top bar money = %q, want %q", got, want) + } +} From cf916c3c018696a5deafb97bc8f4cc9ef22f5b35 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Mon, 28 Sep 2026 20:38:57 -0400 Subject: [PATCH 08/10] changes: the task-run audit entry --- docs/changes/unreleased/1670-task-run-audit.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 docs/changes/unreleased/1670-task-run-audit.md diff --git a/docs/changes/unreleased/1670-task-run-audit.md b/docs/changes/unreleased/1670-task-run-audit.md new file mode 100644 index 0000000000..29c8e4593f --- /dev/null +++ b/docs/changes/unreleased/1670-task-run-audit.md @@ -0,0 +1,12 @@ +--- +kind: fixed +title: Held work, kept branches and spend limits say what is true +pr: 1670 +surface: [chat, engine, remote, docs] +invalidates: + - "A chat turn or a chat task past today's spending limit was refused with a bare line, or a task said it started and then sat held. Both now ask first with a card to raise the limit for today or stop, and the top bar shows a raised limit at once." + - "The read hand-off helper could write, commit and merge. It is now read-only, and a failed hand-off no longer leaves a stopped card for a helper that never started." + - "Every check in a run drew from one shared ceiling, so later checks stopped having spent almost nothing, and a run with unfinished checks could end as done. Each check now has its own ceiling, and unfinished checks fail the run and are named." + - "A kept task or run branch was invisible to /land, and the landing line gave no reason. /land now lists it, reaches the engine over the local host, and says why the branch was kept." + - "A scheduled firing that only reported a sentence was logged as landed. It now comes to said." +--- From f1288387eb273aaee76848b7b875bb964f774425 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Mon, 28 Sep 2026 20:41:38 -0400 Subject: [PATCH 09/10] session: the daily limit reading is built whole, not assigned field by field --- internal/session/rail.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/session/rail.go b/internal/session/rail.go index 703d9bb793..ec10a2fc15 100644 --- a/internal/session/rail.go +++ b/internal/session/rail.go @@ -69,8 +69,7 @@ func (a *Agent) railBlockLocked() error { spentToday = max(spentToday, a.crewDayHeld.Total()) } if daily.Limit > 0 && spentToday >= daily.Limit { - daily.Spent, daily.Reached = spentToday, true - return dailyBudgetReached{spend: daily} + return dailyBudgetReached{spend: DailySpend{Limit: daily.Limit, Spent: spentToday, Reached: true}} } } } From b3b5d7d9c16ded140280ace1d341dcb935c3e7a2 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Mon, 28 Sep 2026 20:58:22 -0400 Subject: [PATCH 10/10] tests: firings that only report say said, and a turn past the daily limit is held for a card --- internal/session/rail_test.go | 28 --------------------- internal/session/standing_isolation_test.go | 4 +-- internal/session/standing_width_test.go | 2 +- 3 files changed, 3 insertions(+), 31 deletions(-) diff --git a/internal/session/rail_test.go b/internal/session/rail_test.go index 178b8706ce..8782b8bbb4 100644 --- a/internal/session/rail_test.go +++ b/internal/session/rail_test.go @@ -221,31 +221,3 @@ func TestTheRefusalNamesTheLimitTheFigureAndTheDoor(t *testing.T) { t.Fatalf("a sub-cent figure reads %q", got) } } - -func TestSpendRailRefusesTurnWhenDailyBudgetExceeded(t *testing.T) { - completer := &scriptedCompleter{steps: []step{ - func(context.Context, []ai.Message) (*ai.Response, error) { - t.Error("a refused turn reached the provider") - return textResponse("should not happen"), nil - }, - }} - profileDir := t.TempDir() - if err := config.WriteDailyBudgetUSD(profileDir, 1.00); err != nil { - t.Fatalf("write daily budget: %v", err) - } - agent, _ := newTestAgent(t, completer, func(cfg *Config) { - cfg.ProfileDir = profileDir - }) - agent.crewDayHeld = NewSpendDay(1.50) - - events := collect(t, mustSubmit(t, agent, "keep going")) - if len(events) != 1 || events[0].Kind != EventError { - t.Fatalf("events = %v, want one EventError", kinds(events)) - } - if !errors.Is(events[0].Err, ErrSpendRail) { - t.Fatalf("error = %v, want it to wrap ErrSpendRail", events[0].Err) - } - if !strings.Contains(events[0].Err.Error(), "daily limit reached") { - t.Fatalf("error = %v, want it to say daily limit reached", events[0].Err) - } -} diff --git a/internal/session/standing_isolation_test.go b/internal/session/standing_isolation_test.go index 8a30670d37..a077ee8dbb 100644 --- a/internal/session/standing_isolation_test.go +++ b/internal/session/standing_isolation_test.go @@ -204,8 +204,8 @@ func TestStandingAmbientPlainFolderWithoutGrant(t *testing.T) { t.Fatalf("unexpected Run error: %v", err) } - if outcome.Kind != "landed" { - t.Fatalf("expected outcome landed, got %q", outcome.Kind) + if outcome.Kind != "said" { + t.Fatalf("expected outcome said, got %q", outcome.Kind) } if !strings.Contains(outcome.Text, "everything looks fine") { t.Fatalf("expected outcome.Text to contain response, got %q", outcome.Text) diff --git a/internal/session/standing_width_test.go b/internal/session/standing_width_test.go index 51e76242dc..1ae40fe203 100644 --- a/internal/session/standing_width_test.go +++ b/internal/session/standing_width_test.go @@ -274,7 +274,7 @@ func TestADivisionUnderAFiringIsWaitedForAndBilledToTheRun(t *testing.T) { if outcome.Text != "both parts are home and folded together" { t.Fatalf("the firing came to %q, want what it said after its parts landed", outcome.Text) } - if outcome.Kind != "landed" { + if outcome.Kind != "said" { t.Fatalf("a firing that divided and folded came to %q", outcome.Kind) }