diff --git a/cmd/codeaf/do.go b/cmd/codeaf/do.go index 886f59b1d3..e63ca1a11a 100644 --- a/cmd/codeaf/do.go +++ b/cmd/codeaf/do.go @@ -3736,7 +3736,7 @@ func runErrand(request doRequest, seats config.Seats) (outcome headlessOutcome, Work: seats.Work.Model, Plan: seats.Plan.Model, Check: seats.Check.Model, - }, memorySection, completerFor, settings.Sources), + }, memorySection, spendPreauthorized(request.yesSpend, env.Value), completerFor, settings.Sources), }) errand := runErrandOutcome(summary, workspace, bound) // THE RUN'S OWN CLOCK, read off the context because the summary's word is diff --git a/docs/changes/unreleased/1842-daily-ceiling-every-path.md b/docs/changes/unreleased/1842-daily-ceiling-every-path.md new file mode 100644 index 0000000000..29b4ac7da2 --- /dev/null +++ b/docs/changes/unreleased/1842-daily-ceiling-every-path.md @@ -0,0 +1,15 @@ +--- +kind: fixed +title: the daily spend ceiling binds task and errand turns, not interactive chat alone +pr: 1842 +surface: [engine] +invalidates: + - "The daily spend ceiling (the machine-wide limit over a day) was enforced only on some paths: `Agent.railBlockLocked` skipped the check for an `InTask` task-node runner (including one under `--one-model`) and the home `Errand`, and the crew guard that also enforces the day cap is absent under `--one-model` and for an `InTask` agent with no crew. So a chat `/task` past the day's limit, and a task under `--one-model`, kept spending with no refusal while headless `codeaf do` and standing firings honoured it. The ceiling is now checked on every turn-starting path through `railBlockLocked`; it is a ceiling read of the shared ledger, never a tally, so a path already held to the crew's own day cap is refused in two places and never charged twice. The rail still stops the next turn rather than cutting one in half." + - "A worker is handed no profile path (it would open the settings door), so the daily ceiling it is held to is now RESOLVED by the root door and carried down (`Config.DailyBudgetUSD`), rather than read from a profile at an empty path - which returned the DEFAULT limit, so a worker refused under a higher selected limit and spent past a lower one. The task-child, the task checker, the adaptive-run worker and the plan-run bash worker all carry the resolved figure. A separate resolved bit preserves an explicitly unlimited worker while ordinary roots, including the empty default-profile path and home errands, keep live profile/environment lookup." + - "`--yes-spend` / `CODEAF_PREAUTHORIZE_SPEND=1`, the standing permission to spend past the daily limit, is now carried into workers (`Config.SpendPreauthorized`) and lifts the daily check there the same way it already lifts it from the crew guard. A separate per-conversation or per-task limit stays in force under it." +--- + +The gap left open by #1627, which enabled the chat and crew daily check but +kept the `InTask`/`Errand` carve-out. Removing it is what closes the two roads +that had no guard behind them at all - a task under `--one-model` and an +`InTask` turn. diff --git a/internal/manual/chat/models-and-cost.md b/internal/manual/chat/models-and-cost.md index c9defc015a..b95acfdd59 100644 --- a/internal/manual/chat/models-and-cost.md +++ b/internal/manual/chat/models-and-cost.md @@ -4190,7 +4190,10 @@ estimated cost. Other conversations' completed usage receipts are read on each check, and completed spending resets at the local date boundary. Calls already in flight may finish; this is not an atomic reservation across processes. Use `/budget` to change the limit. Per-conversation and per-task limits still -apply separately. +apply separately. The default profile and home errands follow the saved +or environment daily limit too; `/budget day` changes apply on the next turn. +Task checks use the same selected daily limit and spending permission as the work +they check. ## Which budget takes precedence — daily or per conversation, and why my budget command doesn't work while the conversation is still blocked diff --git a/internal/run/bashworker.go b/internal/run/bashworker.go index 4df984ef94..ed26d9aac9 100644 --- a/internal/run/bashworker.go +++ b/internal/run/bashworker.go @@ -73,6 +73,14 @@ type BashWorker struct { // The web pair uses the run profile and resolves its settings per operation. searchProvider search.Provider searchFetcher search.Fetcher + // dailyBudgetUSD is the machine-wide daily ceiling this run was started under, + // resolved once by the factory from the run's profile (the worker's own + // session carries no ProfileDir, so [Agent.railBlockLocked] cannot read it). + // Zero is no ceiling. spendPreauthorized is the person's `--yes-spend` / + // `CODEAF_PREAUTHORIZE_SPEND=1` standing permission to spend past it, lifting + // that one limit the same way `doSpendGuard` lifts it from the crew guard. + dailyBudgetUSD float64 + spendPreauthorized bool } // NewBashWorker builds the seat the run's factory hands each claimed task to. @@ -126,6 +134,12 @@ func (w *BashWorker) Run(ctx context.Context, task plandb.Task) (rep Report, run AuthKeySource: w.authKeySource, SearchProvider: w.searchProvider, SearchFetcher: w.searchFetcher, + // The run's daily ceiling and the person's standing permission to spend + // past it travel to the worker's session, which carries no ProfileDir to + // read them from (session.go's [Config.DailyBudgetUSD], #1546). + DailyBudgetUSD: w.dailyBudgetUSD, + DailyBudgetResolved: true, + SpendPreauthorized: w.spendPreauthorized, }, w.completer, &task, w.store.Path(), w.store.RootID()) if err != nil { return Report{}, err diff --git a/internal/run/bashworker_web_test.go b/internal/run/bashworker_web_test.go index 2d57233d70..0cdc2f90f1 100644 --- a/internal/run/bashworker_web_test.go +++ b/internal/run/bashworker_web_test.go @@ -122,7 +122,7 @@ func TestCrewFactoryWiresLiveWebFromItsProfile(t *testing.T) { } } write("exa", "fixture-key") - factory := CrewFactory(nil, t.TempDir(), profile, Seats{One: "test/model"}, "", func(string) session.Completer { return &webSeat{} }) + factory := CrewFactory(nil, t.TempDir(), profile, Seats{One: "test/model"}, "", false, func(string) session.Completer { return &webSeat{} }) worker := factory(plandb.Task{}).(*BashWorker) if worker.searchProvider == nil || worker.searchFetcher == nil { t.Fatal("run worker lost the web pair") diff --git a/internal/run/chat_check_seat_test.go b/internal/run/chat_check_seat_test.go index 385a9805a3..cb729c1048 100644 --- a/internal/run/chat_check_seat_test.go +++ b/internal/run/chat_check_seat_test.go @@ -90,7 +90,7 @@ func TestUnderOneModelEveryRunSeatIsTheConversationsModel(t *testing.T) { config.KeyTierMastermindModel: "vendor/profile-thinking", }) recorder := &recordingCompleter{} - factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{One: "vendor/one"}, "", recorder.forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{One: "vendor/one"}, "", false, recorder.forModel) for _, id := range []string{store.RootID(), "one", "review", "discriminate"} { recorder.models = nil factory(*store.Task(id)) diff --git a/internal/run/crew.go b/internal/run/crew.go index 095e3729d3..b733c8ff31 100644 --- a/internal/run/crew.go +++ b/internal/run/crew.go @@ -124,7 +124,7 @@ type Seats struct { // for a tier key the profile has never held and only a row CLEARED on purpose // reads empty, so a fallback means somebody emptied a row rather than that the // profile is old. -func CrewFactory(store *plandb.Store, workspace, profileDir string, seats Seats, always string, completerFor func(model string) session.Completer, sourceSets ...modelsource.Set) WorkerFactory { +func CrewFactory(store *plandb.Store, workspace, profileDir string, seats Seats, always string, preauthorized bool, completerFor func(model string) session.Completer, sourceSets ...modelsource.Set) WorkerFactory { // The admitted source set explains the refused account without resolving // worker requests a second time. The completer still owns every route. var sources modelsource.Set @@ -133,8 +133,16 @@ func CrewFactory(store *plandb.Store, workspace, profileDir string, seats Seats, } else { sources = config.ResolveSources(profileDir, config.APIKeyAt(profileDir), config.DefaultBaseURL) } + // The run's daily ceiling, resolved ONCE from the run's profile: a worker's + // own session is handed no ProfileDir, so it cannot read the limit the person + // selected and would otherwise fall back to the default (#1546). Zero when + // there is none. The person's `--yes-spend` rides beside it, lifting that one + // limit in the worker exactly as it does in the crew guard. + dailyBudgetUSD, _ := config.DailyBudgetUSDAt(profileDir) workerFor := func(model string, completer session.Completer) *BashWorker { worker := NewBashWorker(store, workspace, model, completer) + worker.dailyBudgetUSD = dailyBudgetUSD + worker.spendPreauthorized = preauthorized // The memory section rides on every worker this run seats, read once by // the door that built the factory. worker.always = always diff --git a/internal/run/crew_context_test.go b/internal/run/crew_context_test.go index bccf658bb3..b235619061 100644 --- a/internal/run/crew_context_test.go +++ b/internal/run/crew_context_test.go @@ -53,7 +53,7 @@ func TestCrewFactoryCarriesTheRoleSeatToTheSpendGuard(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{}, "", false, func(model string) session.Completer { p := &crewCallProbe{} probes = append(probes, p) return guard.Wrap(model, p) diff --git a/internal/run/crew_test.go b/internal/run/crew_test.go index 833dd758bb..e856eba991 100644 --- a/internal/run/crew_test.go +++ b/internal/run/crew_test.go @@ -101,7 +101,7 @@ func TestCrewFactorySeatsEachRoleInItsTier(t *testing.T) { config.KeyTierMastermindModel: "vendor/thinking", }) recorder := &recordingCompleter{} - factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", recorder.forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", false, recorder.forModel) for _, test := range []struct { task, want string @@ -148,7 +148,7 @@ func TestCrewFactoryRunsTheDoorsSeatsWhateverTheProfileSays(t *testing.T) { factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{ Work: "vendor/named-work", Plan: "vendor/named-plan", - }, "", recorder.forModel) + }, "", false, recorder.forModel) for _, test := range []struct { task, want string @@ -185,7 +185,7 @@ func TestCrewFactoryFallsBackToTheWorkerRowForAnEmptyTier(t *testing.T) { config.KeyTierWorkerModel: "vendor/worker", }) recorder := &recordingCompleter{} - factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", recorder.forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", false, recorder.forModel) factory(*store.Task("review")) @@ -200,7 +200,7 @@ func TestCrewFactoryFallsBackToTheWorkerRowForAnEmptyTier(t *testing.T) { func TestCrewFactoryRefusesATaskTheCrewCannotSeat(t *testing.T) { store := runOpenStore(t) dir := crewProfile(t, map[string]string{config.KeyTierWorkerModel: ""}) - factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", (&recordingCompleter{}).forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", false, (&recordingCompleter{}).forModel) worker := factory(*store.Task(store.RootID())) _, err := worker.Run(runContext(t), *store.Task(store.RootID())) @@ -221,7 +221,7 @@ func TestCrewFactoryReadsTheCrewAgainAtEachLaunch(t *testing.T) { store := runOpenStore(t) dir := crewProfile(t, map[string]string{config.KeyTierWorkerModel: "vendor/first"}) recorder := &recordingCompleter{} - factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", recorder.forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", false, recorder.forModel) factory(*store.Task(store.RootID())) writeCrew(t, dir, map[string]string{config.KeyTierWorkerModel: "vendor/second-longer"}) @@ -283,7 +283,7 @@ func TestBashWorkerChargesTheSeatModelToTheTaskSpendRow(t *testing.T) { return toolReply(finishCommand("root", "the work is done")), nil }, }} - factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", recorder.forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, run.Seats{}, "", false, recorder.forModel) worker := factory(*store.Task(store.RootID())) if len(recorder.models) != 1 || recorder.models[0] != "vendor/seat-model" { @@ -327,7 +327,7 @@ func TestCrewFactorySeatsACheckOnTheCheckSeat(t *testing.T) { Work: "vendor/named-work", Plan: "vendor/named-plan", Check: "vendor/named-check", - }, "", recorder.forModel) + }, "", false, recorder.forModel) for _, test := range []struct { task, want string @@ -367,7 +367,7 @@ func TestCrewFactorySeatsTheChatDoorsCheckOnTheCrewsChecker(t *testing.T) { {}, } { recorder := &recordingCompleter{} - factory := run.CrewFactory(store, t.TempDir(), dir, seats, "", recorder.forModel) + factory := run.CrewFactory(store, t.TempDir(), dir, seats, "", false, recorder.forModel) recorder.models = nil factory(*store.Task("review")) if len(recorder.models) != 1 || recorder.models[0] != "vendor/profile-careful" { diff --git a/internal/run/daily_authority_test.go b/internal/run/daily_authority_test.go new file mode 100644 index 0000000000..32324440e9 --- /dev/null +++ b/internal/run/daily_authority_test.go @@ -0,0 +1,121 @@ +package run_test + +import ( + "context" + "errors" + "path/filepath" + "sync/atomic" + "testing" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/plandb" + "github.com/Agent-Field/codeaf/internal/run" + "github.com/Agent-Field/codeaf/internal/session" +) + +type dailyAuthoritySeat struct{ calls atomic.Int32 } + +func (s *dailyAuthoritySeat) CompleteWithMessages(context.Context, []ai.Message, ...ai.Option) (*ai.Response, error) { + s.calls.Add(1) + return nil, errors.New("fake provider reached") +} + +func TestRunFactoryCarriesDailyAuthority(t *testing.T) { + for _, tc := range []struct { + name string + cap, spent float64 + allowed bool + }{{"chosen higher allowance", 1000, 600, true}, {"chosen lower allowance", 1, 1.5, false}, {"chosen unlimited", 0, 600, true}} { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + profile := t.TempDir() + t.Setenv("CODEAF_HOME", root) + t.Setenv("CODEAF_PROFILE_DIR", profile) + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv("CODEAF_PLANDB_BIN", filepath.Join(root, "stub-codeaf")) + t.Setenv("CODEAF_DAILY_BUDGET", "") + t.Setenv("CODEAF_PREAUTHORIZE_SPEND", "") + if err := config.WriteDailyBudgetUSD("", 500); err != nil { + t.Fatal(err) + } + if err := config.WriteDailyBudgetUSD(profile, tc.cap); err != nil { + t.Fatal(err) + } + session.RecordUsage(session.UsageLedgerPath(), session.UsageLine{At: time.Now(), USD: tc.spent, Model: "test/model", Calls: 1}) + session.FlushUsage() + store, err := plandb.Open(filepath.Join(root, "plan.db"), "daily-authority", "root", "Work", "Work") + if err != nil { + t.Fatal(err) + } + defer store.Close() + seat := &dailyAuthoritySeat{} + factory := run.CrewFactory(store, root, profile, run.Seats{One: "test/model"}, "", false, func(string) session.Completer { return seat }) + task := *store.Task(store.RootID()) + worker := factory(task) + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + _, err = worker.Run(ctx, task) + gotAllowed := seat.calls.Load() > 0 + t.Logf("profile cap=%g spent=%g provider calls=%d ending=%v", tc.cap, tc.spent, seat.calls.Load(), err) + if !tc.allowed && !errors.Is(err, session.ErrSpendRail) { + t.Fatalf("worker stopped for a reason other than its daily limit: %v", err) + } + if gotAllowed != tc.allowed { + t.Fatalf("run factory's worker used wrong daily cap: reached provider=%t want=%t", gotAllowed, tc.allowed) + } + }) + } +} + +func TestPreauthorizedRunPreservesTaskCaps(t *testing.T) { + for _, limited := range []bool{false, true} { + t.Run(map[bool]string{false: "daily preauthorization", true: "separate task cap"}[limited], func(t *testing.T) { + root := t.TempDir() + profile := t.TempDir() + t.Setenv("CODEAF_HOME", root) + t.Setenv("CODEAF_PROFILE_DIR", profile) + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv("CODEAF_PLANDB_BIN", filepath.Join(root, "stub-codeaf")) + t.Setenv("CODEAF_DAILY_BUDGET", "") + if err := config.WriteDailyBudgetUSD(profile, 1); err != nil { + t.Fatal(err) + } + if err := config.WriteDailyBudgetUSD("", 1); err != nil { + t.Fatal(err) + } + t.Setenv("CODEAF_PREAUTHORIZE_SPEND", "") + session.RecordUsage(session.UsageLedgerPath(), session.UsageLine{At: time.Now(), USD: 1.5, Model: "test/model", Calls: 1}) + session.FlushUsage() + store, err := plandb.Open(filepath.Join(root, "plan.db"), "daily-authority", "root", "Work", "Work") + if err != nil { + t.Fatal(err) + } + defer store.Close() + seat := &dailyAuthoritySeat{} + factory := run.CrewFactory(store, root, profile, run.Seats{One: "test/model"}, "", true, func(model string) session.Completer { + if !limited { + return seat + } + guard := session.TaskSpendGuard(profile) + guard.TaskCap = 0.01 + guard.TaskAction = "separate task cap" + guard.Price = func(string) (float64, float64, float64, bool) { return 1, 1, 0, true } + return guard.Wrap(model, seat) + }) + task := *store.Task(store.RootID()) + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + _, err = factory(task).Run(ctx, task) + t.Logf("limited=%t calls=%d ending=%v", limited, seat.calls.Load(), err) + if !limited && seat.calls.Load() != 1 { + t.Fatalf("preauthorized run stopped before provider: %v", err) + } + var stopped session.ErrSpendStopped + if limited && (!errors.As(err, &stopped) || stopped.Action != "separate task cap" || seat.calls.Load() != 0) { + t.Fatalf("preauthorization lifted separate task limit: calls=%d err=%v", seat.calls.Load(), err) + } + }) + } +} diff --git a/internal/run/enginewire.go b/internal/run/enginewire.go index 97cfdebd47..2fcde96c48 100644 --- a/internal/run/enginewire.go +++ b/internal/run/enginewire.go @@ -47,7 +47,10 @@ func (engine) Start(ctx context.Context, spec session.RunSpec) session.RunSummar // AND UNDER `--one-model` ONE MODEL IS EVERY SEAT, the probe and the // check's environment rung included ([Seats.One]). One: spec.OneModel, - }, spec.Memory, spec.CompleterFor, spec.Sources) + }, spec.Memory, false, spec.CompleterFor, spec.Sources) + // A `/task` run from interactive chat is never spend-preauthorized: `--yes-spend` + // is a headless `codeaf do` flag, so a chat run is held to the daily ceiling + // like every interactive turn (crew.go's CrewFactory, #1546). if spec.Delegate != nil { // A DELEGATED RUN SEATS THE PROGRAM ON ITS ROOT and has no review // round: a check seat is a bash-belt worker, which the belt switch may diff --git a/internal/run/note_once_test.go b/internal/run/note_once_test.go index ef3a62686c..6f52a28881 100644 --- a/internal/run/note_once_test.go +++ b/internal/run/note_once_test.go @@ -104,7 +104,7 @@ func TestAPlanBornWorkerIsHandedThePersonsRules(t *testing.T) { store := runOpenStore(t) const rules = "Rules that always hold here:\n\n- never force-push a shared branch" seat := &seat{ever: echoesUntilTheCap()} - factory := run.CrewFactory(store, filepath.Dir(store.Path()), t.TempDir(), run.Seats{One: "test/model"}, rules, + factory := run.CrewFactory(store, filepath.Dir(store.Path()), t.TempDir(), run.Seats{One: "test/model"}, rules, false, func(string) session.Completer { return seat }) worker := factory(*store.Task(store.RootID())) if worker == nil { diff --git a/internal/session/daily_authority_test.go b/internal/session/daily_authority_test.go new file mode 100644 index 0000000000..bddff31d99 --- /dev/null +++ b/internal/session/daily_authority_test.go @@ -0,0 +1,139 @@ +package session + +import ( + "context" + "errors" + "strconv" + "testing" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/config" +) + +// The default profile has an empty path at the real chat door. Its daily +// setting must remain live for both a conversation and a copied home errand. +func TestDefaultProfileDailyAuthorityStaysLive(t *testing.T) { + for _, errand := range []bool{false, true} { + for _, source := range []string{"saved", "environment"} { + name := source + "/chat" + if errand { + name = source + "/errand" + } + t.Run(name, func(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_PROFILE_DIR", "") + t.Setenv("CODEAF_DAILY_BUDGET", "") + setLimit := func(limit float64) { + t.Helper() + if source == "environment" { + t.Setenv("CODEAF_DAILY_BUDGET", strconv.FormatFloat(limit, 'f', -1, 64)) + } else if err := config.WriteDailyBudgetUSD("", limit); err != nil { + t.Fatal(err) + } + } + setLimit(1) + settings, err := config.LoadKeyless() + if err != nil { + t.Fatal(err) + } + if settings.ProfileDir != "" || settings.DailyBudgetUSD != 1 { + t.Fatalf("unexpected default settings: profile=%q day=%g", settings.ProfileDir, settings.DailyBudgetUSD) + } + c := &scriptedCompleter{steps: []step{func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }}} + // Match v3ConfigFor: it carries ProfileDir but no resolved daily figure. + a, err := New(Config{ + Workspace: t.TempDir(), Model: "test/model", System: "SYSTEM", + Interactive: true, OneModel: true, Errand: errand, + ProfileDir: settings.ProfileDir, completer: c, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(a.SettleWrites) + t.Cleanup(func() { _ = a.Close() }) + recordUsage(t, UsageLedgerPath(), UsageLine{At: time.Now(), USD: 1.5, Calls: 1, Model: "test/model"}) + events := collect(t, mustSubmit(t, a, "work")) + if len(events) != 1 || !errors.Is(events[0].Err, ErrSpendRail) || c.requests() != 0 { + t.Fatalf("daily limit bypassed: events=%v calls=%d", kinds(events), c.requests()) + } + setLimit(2) + events = collect(t, mustSubmit(t, a, "work")) + if c.requests() != 1 || len(events) == 0 || events[len(events)-1].Kind != EventTurnDone { + t.Fatalf("live daily increase did not release the same root: events=%v calls=%d", kinds(events), c.requests()) + } + }) + } + } +} + +// Both production child constructors must carry the selected authority, even +// when it is unlimited, without lending either settings tool to the child. +func TestTaskConstructorsCarryDailyAuthority(t *testing.T) { + for _, checker := range []bool{false, true} { + kind := "worker" + if checker { + kind = "checker" + } + for _, tc := range []struct { + name string + limit, spent float64 + authorized, blocked bool + }{ + {"higher selected limit", 1000, 600, false, false}, + {"lower selected limit", 1, 1.5, false, true}, + {"selected unlimited", 0, 600, false, false}, + {"authorized daily spending", 1, 1.5, true, false}, + } { + t.Run(kind+"/"+tc.name, func(t *testing.T) { + t.Setenv("CODEAF_HOME", t.TempDir()) + t.Setenv("CODEAF_DAILY_BUDGET", "") + if err := config.WriteDailyBudgetUSD("", 500); err != nil { + t.Fatal(err) + } + profile := t.TempDir() + if err := config.WriteDailyBudgetUSD(profile, tc.limit); err != nil { + t.Fatal(err) + } + c := &scriptedCompleter{steps: []step{func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }}} + parent, _ := newTestAgent(t, c, func(cfg *Config) { + cfg.ProfileDir = profile + cfg.OneModel = true + cfg.SpendPreauthorized = tc.authorized + }) + g := parent.graph() + g.run = func(*TaskNode) {} + id := g.reserve() + g.admit(id, taskSpec{title: "work", brief: "answer", acceptance: "done"}) + var child *Agent + var err error + if checker { + child, err = parent.newAuditAgent(t.TempDir(), g.node(id), plainDoor(auditReadCommands), "") + } else { + child, err = parent.newTaskAgent(context.Background(), t.TempDir(), g.node(id), "") + } + if err != nil { + t.Fatal(err) + } + t.Cleanup(child.SettleWrites) + t.Cleanup(func() { _ = child.Close() }) + if child.config.ProfileDir != "" || len(child.settingsTools()) != 0 { + t.Fatal("child gained settings access") + } + // Only the carried value may answer now, including an explicit zero. + // Reading the environment or default profile would fail the high case. + t.Setenv("CODEAF_DAILY_BUDGET", "1") + recordUsage(t, UsageLedgerPath(), UsageLine{At: time.Now(), USD: tc.spent, Calls: 1, Model: "test/model"}) + events := collect(t, mustSubmit(t, child, "work")) + blocked := len(events) == 1 && errors.Is(events[0].Err, ErrSpendRail) + calls := 1 + if tc.blocked { + calls = 0 + } + if blocked != tc.blocked || c.requests() != calls { + t.Fatalf("wrong carried authority: events=%v calls=%d wantBlocked=%t wantCalls=%d", kinds(events), c.requests(), tc.blocked, calls) + } + }) + } + } +} diff --git a/internal/session/orchestrate.go b/internal/session/orchestrate.go index bead3d3591..aaa1dac303 100644 --- a/internal/session/orchestrate.go +++ b/internal/session/orchestrate.go @@ -1099,14 +1099,21 @@ func (e *orchestrateExec) newChild(dir string, node orchestrate.Node) (*Agent, e // and every line this node writes names the conversation the run is rooted // in, so the tree rollup a status line reads ([UsageTree]) sees a run // while it is still running rather than when it folds. - usageLedger: parent.usageLedger, - rootSession: root, - Workspace: dir, - Model: model, - APIKey: parent.APIKey, - BaseURL: parent.BaseURL, - Sources: parent.Sources, - ContextWindow: window, + usageLedger: parent.usageLedger, + rootSession: root, + // AND THE DAILY CEILING IT SPENDS UNDER, resolved and carried for a task + // node's exact reason (task_run.go's newChildAgent): a worker reads no + // profile, so the root's figure travels with it (rail.go's + // [Agent.dailyCeilingUSD]) and the person's `--yes-spend` with it. + DailyBudgetUSD: a.dailyCeilingUSD(), + DailyBudgetResolved: true, + SpendPreauthorized: a.config.SpendPreauthorized, + Workspace: dir, + Model: model, + APIKey: parent.APIKey, + BaseURL: parent.BaseURL, + Sources: parent.Sources, + ContextWindow: window, // And the catalog with it, for the reason newTaskAgent hands it down: // a worker that switches its own model has to be able to learn that // model's window. diff --git a/internal/session/rail.go b/internal/session/rail.go index d88568011c..83986c9d56 100644 --- a/internal/session/rail.go +++ b/internal/session/rail.go @@ -51,6 +51,24 @@ func (a *Agent) spendRailUSD() float64 { return a.config.SpendRailUSD } +// dailyCeilingUSD is the machine-wide daily limit this session is held to, and +// zero when there is none. +// +// A ROOT reads its profile live, including the ordinary empty default-profile +// path, so a `/budget day` change takes effect on the next turn. A WORKER carries +// the value resolved by its parent without gaining the profile's settings door. +// [Config.DailyBudgetResolved] distinguishes that authority from an unset root +// field; neither an empty path nor a zero amount can make that distinction. +func (a *Agent) dailyCeilingUSD() float64 { + if a.config.DailyBudgetResolved { + return a.config.DailyBudgetUSD + } + if daily, err := config.DailyBudgetUSDAt(a.config.ProfileDir); err == nil { + return daily + } + return 0 +} + // ErrSpendRail is what a refused turn carries in its EventError. It is a named // sentinel so a surface can match it with errors.Is and say the one thing worth // saying — the rail, not a provider fault — instead of matching on words. @@ -62,8 +80,24 @@ func (a *Agent) railBlockLocked() error { if err := a.launchBudgetBlockLocked(); err != nil { return err } - if !a.config.InTask && !a.config.Errand { - if daily, err := config.DailyBudgetUSDAt(a.config.ProfileDir); err == nil && daily > 0 { + // THE DAILY CEILING BINDS EVERY TURN-STARTING PATH, not interactive chat + // alone. It is the machine-wide limit over a day (internal/config), so a task + // node's runner (InTask) and the home errand (Errand) are held to it exactly + // as a chat turn is: a conversation that has spent the day's allowance must + // start no new work, however the work was asked for. Checking it here rather + // than only in the crew guard is what closes the two roads that had no guard + // behind them - a task under `--one-model`, which has no crew, and an InTask + // turn, which skipped this block. It is a CEILING READ and never a tally, so + // a path already guarded by the crew's own day cap is refused in two places, + // never charged twice (#1546). + // + // `--yes-spend` / `CODEAF_PREAUTHORIZE_SPEND=1` is the person's standing + // permission to spend past the day, carried into workers as [Config.SpendPreauthorized]; + // it lifts THIS limit and nothing else, the same authority `doSpendGuard` + // lifts from the crew guard (cmd/codeaf's do.go). A per-conversation or + // per-task limit stays in force below. + if !a.config.SpendPreauthorized { + if daily := a.dailyCeilingUSD(); daily > 0 { spentToday := spentTodayOnLedger() if a.crewDayHeld != nil { spentToday = max(spentToday, a.crewDayHeld.Total()) diff --git a/internal/session/rail_test.go b/internal/session/rail_test.go index 7bfe96b4c0..20f5400aa4 100644 --- a/internal/session/rail_test.go +++ b/internal/session/rail_test.go @@ -190,6 +190,128 @@ func TestSpendRailRefusesTurnWhenDailyBudgetExceeded(t *testing.T) { } } +// TestTheDailyCeilingBindsTaskAndErrandTurnsToo guards #1546: the daily budget +// is a machine-wide ceiling, and before this fix [Agent.railBlockLocked] skipped +// it for an InTask runner (a task node, including one under `--one-model`, which +// has no crew guard behind it) and for the home Errand. Each of those is now +// held to the day's allowance exactly as a chat turn is: a conversation past the +// ceiling starts no new turn, however the work was asked for. +func TestTheDailyCeilingBindsTaskAndErrandTurnsToo(t *testing.T) { + for _, posture := range []struct { + what string + config func(*Config) + }{ + {"a task node runner", func(cfg *Config) { cfg.InTask = true }}, + {"a task node under one-model", func(cfg *Config) { cfg.InTask = true; cfg.OneModel = true }}, + {"the home errand", func(cfg *Config) { cfg.Errand = true }}, + } { + t.Run(posture.what, func(t *testing.T) { + profileDir := t.TempDir() + if err := config.WriteDailyBudgetUSD(profileDir, 1.00); err != nil { + t.Fatalf("write daily budget: %v", err) + } + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(cfg *Config) { + cfg.ProfileDir = profileDir + posture.config(cfg) + }) + // The day's spend is over the ceiling, read the way the rail reads it. + agent.crewDayHeld = NewSpendDay(1.50) + + err := agent.railBlockLocked() + if err == nil { + t.Fatalf("%s past the daily ceiling was allowed to start a turn", posture.what) + } + if !errors.Is(err, ErrSpendRail) { + t.Fatalf("the refusal must carry the sentinel: %v", err) + } + if !strings.Contains(err.Error(), "daily limit reached") { + t.Fatalf("error = %v, want it to say the daily limit was reached", err) + } + }) + } + + // And under the ceiling, each posture still starts: the fix refuses only the + // turn the day's allowance cannot pay for. + profileDir := t.TempDir() + if err := config.WriteDailyBudgetUSD(profileDir, 1.00); err != nil { + t.Fatalf("write daily budget: %v", err) + } + agent, _ := newTestAgent(t, &scriptedCompleter{}, func(cfg *Config) { + cfg.ProfileDir = profileDir + cfg.InTask = true + }) + agent.crewDayHeld = NewSpendDay(0.50) + if err := agent.railBlockLocked(); err != nil { + t.Fatalf("a task turn under the daily ceiling was refused: %v", err) + } +} + +// TestAWorkerIsHeldToTheSelectedBudgetItCarriesNotTheDefault covers the #1842 +// review: a worker is handed no ProfileDir (it would open the settings door), so +// the daily check must read the figure the root resolved and carried +// ([Config.DailyBudgetUSD]), never a profile at an empty path - which returns +// the default profile's limit and would refuse a worker under a higher selected limit and +// let it spend past a lower one. +func TestAWorkerIsHeldToTheSelectedBudgetItCarriesNotTheDefault(t *testing.T) { + // A worker: no ProfileDir, the selected limit carried down as $1. + worker := &Agent{} + worker.config.DailyBudgetUSD = 1.00 + worker.config.DailyBudgetResolved = true + worker.crewDayHeld = NewSpendDay(1.50) + if err := worker.railBlockLocked(); err == nil { + t.Fatal("a worker past its carried $1 ceiling was allowed to start") + } else if !strings.Contains(err.Error(), "daily limit reached") { + t.Fatalf("error = %v, want the daily limit", err) + } + + // The same spend ($600) under a HIGHER selected limit ($1000) carried down + // must NOT refuse - the bug was reading the default $500 and refusing here. + high := &Agent{} + high.config.DailyBudgetUSD = 1000.00 + high.config.DailyBudgetResolved = true + high.crewDayHeld = NewSpendDay(600.00) + if err := high.railBlockLocked(); err != nil { + t.Fatalf("a worker under its carried $1000 ceiling was refused: %v", err) + } + + // An explicitly unlimited worker keeps its selected zero ceiling. + none := &Agent{config: Config{DailyBudgetResolved: true}} + none.crewDayHeld = NewSpendDay(1000.00) + if err := none.railBlockLocked(); err != nil { + t.Fatalf("an explicitly unlimited worker was refused: %v", err) + } +} + +// TestSpendPreauthorizationLiftsTheDailyCeiling covers the #1842 review's second +// case: `--yes-spend` / `CODEAF_PREAUTHORIZE_SPEND=1` is the person's standing +// permission to spend past the daily limit, so a preauthorized worker past the +// ceiling runs rather than refusing - the same authority the crew guard already +// honours. A separate conversation limit is NOT lifted by it. +func TestSpendPreauthorizationLiftsTheDailyCeiling(t *testing.T) { + pre := &Agent{} + pre.config.DailyBudgetUSD = 1.00 + pre.config.DailyBudgetResolved = true + pre.config.SpendPreauthorized = true + pre.crewDayHeld = NewSpendDay(1.50) + if err := pre.railBlockLocked(); err != nil { + t.Fatalf("a preauthorized worker past the daily ceiling was refused: %v", err) + } + + // But a conversation limit stands even under preauthorization: --yes-spend + // lifts the day, not a ceiling the person put on this one conversation. + bounded := &Agent{} + bounded.config.DailyBudgetUSD = 1.00 + bounded.config.DailyBudgetResolved = true + bounded.config.SpendPreauthorized = true + bounded.config.SpendRailUSD = 0.50 + bounded.usage.CostUSD = 0.60 + if err := bounded.railBlockLocked(); err == nil { + t.Fatal("preauthorization lifted a conversation limit it should not") + } else if !strings.Contains(err.Error(), "conversation limit reached") { + t.Fatalf("error = %v, want the conversation limit", err) + } +} + // Raising the day's allowance does not remove a conversation's separate cap. // The named conversation setting must release the same live agent on retry. func TestBudgetScopeAndLiveConversationRecovery(t *testing.T) { diff --git a/internal/session/session.go b/internal/session/session.go index 23b367d2ad..dfc0666a25 100644 --- a/internal/session/session.go +++ b/internal/session/session.go @@ -2308,6 +2308,26 @@ type Config struct { // SpendRailUSD remains a separate, adjustable conversation spending limit. Budget Budget + // DailyBudgetUSD is the machine-wide daily ceiling resolved by a parent + // and carried into a worker without lending it the profile's settings door. + // It is read only when DailyBudgetResolved is true; zero then means unlimited. + DailyBudgetUSD float64 + + // DailyBudgetResolved says the daily ceiling was deliberately carried into + // this agent. Roots leave it false and read their profile and environment live, + // including the ordinary default profile whose ProfileDir is empty. A separate + // bit preserves a selected zero limit without mistaking an unset root for an + // unlimited worker. + DailyBudgetResolved bool + + // SpendPreauthorized is the person's `--yes-spend` / `CODEAF_PREAUTHORIZE_SPEND=1` + // standing permission to spend past the daily ceiling, resolved at the door + // and carried into workers the same way [DailyBudgetUSD] is. When it is set + // the daily check in [Agent.railBlockLocked] is lifted - exactly the + // authority `doSpendGuard` already lifts from the crew guard (cmd/codeaf's + // do.go) - while a separate per-task or per-conversation limit stays in force. + SpendPreauthorized bool + // newerBuild is the cheap process-local reading that says this running // codeaf has been replaced on disk. It is private because the session owns // when the reading reaches a turn; tests replace only the reading itself. diff --git a/internal/session/task_audit.go b/internal/session/task_audit.go index f6fbdc73cf..f209c615f9 100644 --- a/internal/session/task_audit.go +++ b/internal/session/task_audit.go @@ -3168,6 +3168,11 @@ func (a *Agent) newAuditAgent(dir string, node *TaskNode, door auditDoor, on str judge = named } auditor, err := a.newChildAgent(Config{ + // Checking spends under the same daily authority as the work it checks, + // without gaining access to the person's profile or settings tools. + DailyBudgetUSD: a.dailyCeilingUSD(), + DailyBudgetResolved: true, + SpendPreauthorized: parent.SpendPreauthorized, // A checker can independently read the source a worker cited, without // gaining the writable memory store or any additional mutation tool. ConversationHistory: parent.conversationHistory(), diff --git a/internal/session/task_run.go b/internal/session/task_run.go index d4454f6c23..23f8d0fce0 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -7621,6 +7621,15 @@ func (a *Agent) newTaskAgentOn(ctx context.Context, dir string, node *TaskNode, // else, and a rollup over the family then found half of it // (usage_ledger.go). usageLedger: parent.usageLedger, + // AND THE DAILY CEILING IT SPENDS UNDER, resolved by the root door and + // carried because a worker is handed no ProfileDir to read it from + // (session.go's [Config.DailyBudgetUSD], rail.go's [Agent.dailyCeilingUSD]). + // Without this a worker read the DEFAULT profile's limit, refusing under a + // higher selected one and spending past a lower one. The person's standing + // `--yes-spend` travels with it, lifting that one limit and no other. + DailyBudgetUSD: a.dailyCeilingUSD(), + DailyBudgetResolved: true, + SpendPreauthorized: a.config.SpendPreauthorized, // AND WHOSE MONEY IT IS. Every line this worker records names the // conversation the work is rooted in, however deep the family goes. rootSession: root,