diff --git a/cmd/codeaf/do.go b/cmd/codeaf/do.go index 63fb6813a..d413abecf 100644 --- a/cmd/codeaf/do.go +++ b/cmd/codeaf/do.go @@ -301,8 +301,10 @@ func (o headlessOutcome) status() exitStatus { func runDo(args []string) error { flags := commandFlags("do") - database := flags.String("db", "", "work in this durable store instead of a private one") - keep := flags.Bool("keep", false, "keep the private store instead of deleting it on the way out") + database := flags.String("db", "", "work in this durable store instead of a private one "+ + "(older engine only; the run engine refuses it)") + keep := flags.Bool("keep", false, "keep the run's store instead of deleting it on the way out, "+ + "and say where it is") workspace := flags.String("dir", "", "the directory to work in, edited in place (default: the current directory)") shorthandFlag(flags, "w", "dir") wall := wallFlag{wall: defaultDoWall} @@ -550,11 +552,12 @@ func errandRun(request doRequest, seats config.Seats, started time.Time) (outcom if err := applyContextLaw(request.contextFill, request.completionReserve); err != nil { return headlessOutcome{}, err } - // THE SECOND ROAD, BEHIND THE SAME SWITCH AS THE BASH BELT. When the belt is - // asked for, the errand is dispatched by the run engine over the project's - // own plan store — the same worker, the same store and the same exit ladder — - // rather than by the resident's reconciler below. Unset, not one byte of the - // road below moves, and the legacy errand stays the default. + // THE RUN ENGINE IS THE DEFAULT ROAD, BEHIND THE SAME SWITCH AS THE BASH + // BELT. With the belt on — every machine that has set nothing — the errand + // is dispatched by the run engine over the project's own plan store rather + // than by the resident's reconciler below. CODEAF_TASK_BELT set to one of + // the words that turn the belt off is the only way onto the road below, and + // with it set not one byte of that road moves. if session.BashBeltAsked() { return runErrand(request, seats) } @@ -3400,9 +3403,16 @@ func parseSlots(raw string) (*int, error) { // runErrand is `codeaf do` on the run engine: the same errand as the road above // — the same store, the same worker, the same exit ladder and the same JSON // envelope — dispatched by [internal/run]'s supervisor over the project's own -// plan store instead of by the resident's reconciler. It is taken only when the -// bash belt is asked for ([session.BashBeltAsked]), because the worker it -// dispatches is the belt's and the landing it makes is the belt's. +// plan store instead of by the resident's reconciler. It is taken whenever the +// bash belt is on ([session.BashBeltAsked]), which it is unless the person set +// CODEAF_TASK_BELT to one of the words that turn it off, because the worker it +// dispatches is the belt's. +// +// IT KEEPS THE OLDER ROAD'S CONTRACT WITH THE DIRECTORY: the run edits it in +// place and commits nothing. A landing here once staged the directory's whole +// `git status` and committed it on the checked-out branch — the person's own +// uncommitted edits and untracked files with it — which no `--dir` help line +// ever promised. The files the envelope names are the ones this run changed. // // THE STORE'S OWN ROOT IS THE RUN. Its description is the ask, verbatim, and // its result is the answer: [runengine.Start] puts the brief on it and the root @@ -3411,23 +3421,50 @@ func parseSlots(raw string) (*int, error) { // door was handed is the whole assignment, which is the same verbatim contract // the resident road keeps. func runErrand(request doRequest, seats config.Seats) (headlessOutcome, error) { - // A CEILING OF NOTHING IS A RUN THAT MAY SPEND NOTHING. Refused here, before - // anything is opened or built, because a limit of zero is not a limit that a - // worker crosses — it is a run that was stopped before one began, and the - // promise of exit 3 is that raising the limit and running it again is the - // remedy. - if request.costCap != nil && *request.costCap <= 0 { + // A FLAG THIS ROAD CANNOT HONOUR IS REFUSED IN WORDS, NEVER DROPPED. `--db` + // names a store the older engine works in; a run keeps its plan in the + // working copy's own store instead, and a run that quietly worked somewhere + // other than the store it was pointed at would leave the person reading an + // untouched file for the answer. + if strings.TrimSpace(request.database) != "" { + return headlessOutcome{}, errors.New(runRoadRefusesStore) + } + settings, err := config.Load() + if err != nil { + return headlessOutcome{}, err + } + applySeats(&settings, seats) + // THE SPENDING CONTRACT IS DECIDED BEFORE ANYTHING IS OPENED. A run nobody is + // watching is bounded unless the person said otherwise, the way the older + // road asked its plan-price question before it bought a step. + bound, err := runSpendBound(request, settings.ProfileDir, time.Now()) + if err != nil { + return headlessOutcome{}, err + } + if bound.refused { + // A CEILING OF NOTHING IS A RUN THAT MAY SPEND NOTHING. Refused here, + // before anything is opened or built, because a limit of zero is not a + // limit that a worker crosses — it is a run that was stopped before one + // began, and the promise of exit 3 is that raising the limit and running + // it again is the remedy. return headlessOutcome{ Artifacts: []string{}, Settled: true, - stop: stopBudget, - BlockedOn: fmt.Sprintf("this run's cost cap is $%.2f, so nothing was started", *request.costCap), + stop: bound.stop, + BlockedOn: bound.words, }, nil } workspace, err := errandWorkspace(request.workspace) if err != nil { return headlessOutcome{}, err } + // THE RUN WORKS IN PLACE AND COMMITS NOTHING, which is what `--dir` has + // always promised: "the directory to work in, edited in place". The copy is + // read before the run starts so that, afterwards, the files this run names + // are the ones IT changed — the person's own uncommitted edits and untracked + // files were there first, still hold what they held, and are none of the + // run's business ([session.RunTreeSnapshot]). + before := session.SnapshotRunTree(workspace) title := topicTitle(request.task) store, err := session.OpenRunPlan(workspace, title, request.task) if err != nil { @@ -3435,11 +3472,6 @@ func runErrand(request doRequest, seats config.Seats) (headlessOutcome, error) { } defer store.Close() - settings, err := config.Load() - if err != nil { - return headlessOutcome{}, err - } - applySeats(&settings, seats) completerFor := request.newBeltCompleter if completerFor == nil { newClient := request.newClient @@ -3451,10 +3483,7 @@ func runErrand(request doRequest, seats config.Seats) (headlessOutcome, error) { // THE REVIEW ROUND IS ON for every `do` run: a leaf that lands done is // checked against its acceptance, and a check that does not hold becomes a // fix task under the leaf's parent the run waits on. - limits := runengine.Limits{ReviewRound: true} - if request.costCap != nil { - limits.CostUSD = *request.costCap - } + limits := runengine.Limits{ReviewRound: true, CostUSD: bound.usd} // AN INTERRUPT MUST LAND THE RUN, NOT VANISH IT, the same way it must on the // resident road: routed through the context, the supervisor stops launching, // drains what is in flight, and what it reached is composed and printed. @@ -3486,8 +3515,11 @@ func runErrand(request doRequest, seats config.Seats) (headlessOutcome, error) { case runengine.OutcomeDone: errand.stop, errand.Settled, errand.Deliverable = stopDone, true, strings.TrimSpace(summary.Result) case runengine.OutcomeLimit: - errand.stop, errand.Settled = stopBudget, true - errand.BlockedOn = runLimitWords(request.costCap) + // The only limit this road sets is the spending bound, so a run the + // engine stopped on a limit is one that reached it, and the sentence + // is the bound's own: the figure, and what to pass to go past it. + errand.stop, errand.Settled = bound.stop, true + errand.BlockedOn = bound.words case runengine.OutcomeCannotRun: errand.stop = stopError errand.Error = "the run could not be started" @@ -3502,38 +3534,112 @@ func runErrand(request doRequest, seats config.Seats) (headlessOutcome, error) { if ctx.Err() == context.DeadlineExceeded { errand.stop, errand.wall, errand.Settled = stopDeadline, true, false } - // THE LANDING IS THE RUN'S OWN HALF. A run that finished commits its working - // copy onto its branch, and the branch, the paths it carried and its own - // sentence reach the caller: the paths are what the envelope calls artifacts, - // and the sentence names the branch where a person reads the answer. A run - // stopped short lands nothing, and a working copy that is not a repository - // says so on stderr without costing the work that did land on disk. - if summary.Outcome == runengine.OutcomeDone { - landing, err := runengine.Land(ctx, store, workspace, store.RootID()) - switch { - case err != nil: - fmt.Fprintf(request.stderr, "the run's work is not on a branch: %v\n", err) - default: - errand.Artifacts = landedPaths(workspace, landing.Changed) - if landing.Branch != "" { - if errand.Deliverable != "" { - errand.Deliverable += "\n\n" + runengine.LandingNote(landing) - } else { - errand.Deliverable = runengine.LandingNote(landing) - } - } - } + // WHAT THE RUN CHANGED IS WHERE IT STANDS: in the directory it was handed, + // uncommitted, on whatever branch was checked out there. The envelope's + // files are those paths and no others, on every ending — a run stopped short + // still left its edits on disk, and a caller has to be able to find them. + errand.Artifacts = landedPaths(workspace, before.Changed()) + // `--keep` ASKED FOR THE RECORD BY NAME. On this road the record is the + // working copy's own plan store, which is never deleted, so the flag's + // promise is kept by saying where it is. + if request.keep && request.stderr != nil { + fmt.Fprintf(request.stderr, "record kept at %s\n", session.PlanStorePath(workspace)) } return errand, nil } -// runLimitWords is the sentence a run stopped by its ceiling owes `blocked_on`: -// the price it reached, so a caller knows what to raise. -func runLimitWords(cap *float64) string { - if cap != nil { - return fmt.Sprintf("the run reached the cost cap of $%.2f", *cap) +// runRoadRefusesStore is the sentence `codeaf do --db` answers on the run +// engine. The flag names a store the older engine works in, and a run keeps its +// plan in the directory it works in, so there is nothing for the flag to point +// at; the sentence says where the plan is instead and how to reach the engine +// that takes the flag. +const runRoadRefusesStore = "--db names a store only the older engine works in; " + + "a run keeps its plan in .codeaf/plandb.db inside the directory it works in. " + + "Drop --db, or set CODEAF_TASK_BELT=node to run this on the older engine" + +// runSpend is the spending bound a run on this road is held to: the dollars +// it may spend (0 is no bound), which rung of the exit ladder reaching it is, +// the sentence `blocked_on` carries when it is reached, and whether the run +// may not start at all. +type runSpend struct { + usd float64 + stop stopReason + words string + refused bool +} + +// runSpendBound is THE SPENDING CONTRACT `--yes-spend` promises +// ([yesSpendFlagHelp]): without it, a run stops at the plan-price question's +// figure and at what is left of today's limit, whichever is nearer; with it, +// or with CODEAF_PREAUTHORIZE_SPEND=1, neither stops it. +// +// THE OLDER ROAD ASKED BEFORE IT BOUGHT, and this one cannot: a run has no +// estimate before its workers start, because nothing plans the whole of it up +// front. So the question becomes a ceiling. The run spends up to the figure +// the person set as the point where codeaf asks first (CODEAF_PLAN_CONSENT, or +// the profile's row for it), stops there with exit 3 and `stop` `price`, and +// says what to pass to go further. A figure of 0 is "never ask", and that rung +// does not bound the run. +// +// TODAY'S LIMIT IS THE SECOND RUNG, measured against the usage ledger the +// workers write, so a run started late in an expensive day stops where the day +// does. A day already spent starts nothing. A limit of 0 is no daily limit. +// +// A CAP HANDED IN BY A CALLER (request.costCap) is its own contract and wins +// over both, which is how a test holds a run to a price. +func runSpendBound(request doRequest, profileDir string, now time.Time) (runSpend, error) { + if request.costCap != nil { + limit := *request.costCap + if limit <= 0 { + return runSpend{refused: true, stop: stopBudget, + words: fmt.Sprintf("this run's cost cap is $%.2f, so nothing was started", limit)}, nil + } + return runSpend{usd: limit, stop: stopBudget, + words: fmt.Sprintf("the run reached the cost cap of $%.2f", limit)}, nil + } + if spendPreauthorized(request.yesSpend, env.Value) { + return runSpend{}, nil + } + consent, err := config.PlanConsentUSDAt(profileDir) + 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 + } + 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)} + } + } + 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 "a limit stopped the run" + return session.SpendToday(lines, now) } // crewCompleters turns the run road's provider seam into the per-model diff --git a/cmd/codeaf/do_engine_contract_test.go b/cmd/codeaf/do_engine_contract_test.go new file mode 100644 index 000000000..e6bffd63f --- /dev/null +++ b/cmd/codeaf/do_engine_contract_test.go @@ -0,0 +1,277 @@ +package main + +// `codeaf do` ON THE RUN ENGINE KEEPS THE DOOR'S CONTRACT. The run engine is +// the road every `codeaf do` takes unless CODEAF_TASK_BELT turns the belt off, +// so the promises the door's help makes are this road's to keep: +// +// - `--dir` is "the directory to work in, edited in place". The run edits it +// and commits nothing, and the files it names are the ones it changed — +// never the person's own uncommitted edits or untracked files. +// - `--yes-spend` is "spend past today's limit and past the plan-price +// question, without stopping to ask". Without it an unattended run is +// bounded: by the plan-price figure, and by what is left of today's limit. +// - A flag the road cannot honour is refused in words, and one it can is +// honoured; none is dropped without a sentence. + +import ( + "context" + "encoding/json" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/Agent-Field/agentfield/sdk/go/ai" + "github.com/Agent-Field/codeaf/internal/session" +) + +// finishingSeat is the scripted worker that writes out.txt, finishes the root +// in the store and answers `holds` to the review check, which is a run that +// ends done. Every reply costs usd dollars, so a spending bound can see it. +func finishingSeat(usd float64) *beltSeat { + costed := func(reply *ai.Response) *ai.Response { + if usd > 0 { + cost := usd + reply.Usage.Cost = &cost + } + return reply + } + return &beltSeat{ + script: []func(context.Context, []ai.Message) (*ai.Response, error){ + func(context.Context, []ai.Message) (*ai.Response, error) { + return costed(beltToolReply("printf 'written by the run' > out.txt")), nil + }, + func(context.Context, []ai.Message) (*ai.Response, error) { + return costed(beltToolReply(beltFinish(beltAnswer))), nil + }, + }, + ever: func(_ context.Context, msgs []ai.Message) (*ai.Response, error) { + if doc := beltDocument(msgs); strings.Contains(doc, "## Who checks this work") { + id := briefTaskID(doc) + return costed(beltToolReply("plandb done " + id + " --agent " + id + " --result 'holds: the acceptance is met'")), nil + } + return costed(beltTextReply(beltAnswer)), nil + }, + } +} + +// spendingSeat is a worker that never finishes: every reply is one more shell +// command costing usd dollars. Only a bound stops it before its wall. +func spendingSeat(usd float64) *beltSeat { + return &beltSeat{ever: func(context.Context, []ai.Message) (*ai.Response, error) { + reply := beltToolReply("true") + cost := usd + reply.Usage.Cost = &cost + return reply, nil + }} +} + +// doEnvelopeFields is the --json object as a map, for the words the typed +// outcome does not decode (`stop`). +func doEnvelopeFields(t *testing.T, stdout string) map[string]any { + t.Helper() + fields := map[string]any{} + if err := json.Unmarshal([]byte(stdout), &fields); err != nil { + t.Fatalf("stdout is not one JSON object: %v\n%s", err, stdout) + } + return fields +} + +// doGitIn runs one command of the version-control tool in dir. +func doGitIn(t *testing.T, dir string, args ...string) string { + t.Helper() + cmd := exec.Command("git", args...) + cmd.Dir = dir + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("git %s: %v\n%s", strings.Join(args, " "), err, out) + } + return strings.TrimRight(string(out), "\n") +} + +// THE PERSON'S OWN WORK IS NEVER SWEPT INTO A COMMIT. +// +// The copy holds an edit the person has not committed and an untracked +// secrets file. The run writes out.txt and finishes. Nothing is committed — +// the branch stands on the commit it stood on — the person's edit is still an +// uncommitted edit, the secrets file is still untracked, and the files the +// envelope names are the run's one file and nothing of the person's. +func TestDoOnTheRunEngineNeverCommitsThePersonsOwnWork(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + workspace := beltRepoWorkspace(t) + if err := os.WriteFile(filepath.Join(workspace, "README.md"), []byte("the person's own edit\n"), 0o644); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, "secret.env"), []byte("TOKEN=mine\n"), 0o600); err != nil { + t.Fatal(err) + } + head := doGitIn(t, workspace, "rev-parse", "HEAD") + + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "write out.txt and say what you did", workspace: workspace, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return finishingSeat(0) }, + }) + if err != nil { + t.Fatalf("a brief the model completed left with %v, want 0\nstdout:\n%s\nstderr:\n%s", + err, stdout.String(), stderr.String()) + } + if now := doGitIn(t, workspace, "rev-parse", "HEAD"); now != head { + t.Fatalf("the run committed on the person's branch: HEAD moved %s -> %s\n%s", + head, now, doGitIn(t, workspace, "show", "--stat", "HEAD")) + } + status := doGitIn(t, workspace, "status", "--porcelain", "--untracked-files=all") + for _, want := range []string{" M README.md", "?? secret.env", "?? out.txt"} { + if !strings.Contains(status, want) { + t.Errorf("status after the run lacks %q — the copy was not left as edited in place:\n%s", want, status) + } + } + outcome := decodeErrand(t, stdout.String()) + want := filepath.Join(workspace, "out.txt") + if len(outcome.Artifacts) != 1 || outcome.Artifacts[0] != want { + t.Fatalf("files = %v, want only the run's own %s", outcome.Artifacts, want) + } + if strings.Contains(outcome.Deliverable, "landed on") { + t.Fatalf("the answer claims a landing the run never made:\n%s", outcome.Deliverable) + } +} + +// WITHOUT --yes-spend, AN UNATTENDED RUN STOPS AT THE PLAN PRICE. +// +// The worker never finishes and every call costs a dollar; the plan-price +// figure is fifty cents. The run stops on its first call's bill with exit 3, +// `stop` is `price`, and `blocked_on` says what to pass to go past it — the +// same contract the older road kept by asking before it bought. +func TestDoOnTheRunEngineStopsAtThePlanPriceWithoutYesSpend(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLAN_CONSENT", "0.5") + t.Setenv("CODEAF_DAILY_BUDGET", "0") + t.Setenv("CODEAF_PREAUTHORIZE_SPEND", "") + workspace := beltRepoWorkspace(t) + + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "keep working", workspace: workspace, asJSON: true, + timeout: 30 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return spendingSeat(1) }, + }) + var status exitStatus + if !asExitStatus(err, &status) || status != exitLimit { + t.Fatalf("an unattended run past the plan price left with %v, want exit 3\nstdout:\n%s\nstderr:\n%s", + err, stdout.String(), stderr.String()) + } + fields := doEnvelopeFields(t, stdout.String()) + if fields["stop"] != string(stopPrice) { + t.Fatalf("stop = %v, want %q", fields["stop"], stopPrice) + } + if blocked, _ := fields["blocked_on"].(string); !strings.Contains(blocked, "--yes-spend") || !strings.Contains(blocked, "$0.50") { + t.Fatalf("blocked_on does not name the price and the way past it: %q", blocked) + } +} + +// AND TODAY'S LIMIT IS THE OTHER RUNG. With the plan-price question off, the +// day's limit still bounds a run nobody is watching. +func TestDoOnTheRunEngineStopsAtTodaysLimitWithoutYesSpend(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLAN_CONSENT", "0") + t.Setenv("CODEAF_DAILY_BUDGET", "0.5") + t.Setenv("CODEAF_PREAUTHORIZE_SPEND", "") + workspace := beltRepoWorkspace(t) + + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "keep working", workspace: workspace, asJSON: true, + timeout: 30 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return spendingSeat(1) }, + }) + var status exitStatus + if !asExitStatus(err, &status) || status != exitLimit { + t.Fatalf("an unattended run past today's limit left with %v, want exit 3\nstdout:\n%s\nstderr:\n%s", + err, stdout.String(), stderr.String()) + } + fields := doEnvelopeFields(t, stdout.String()) + if fields["stop"] != string(stopBudget) { + t.Fatalf("stop = %v, want %q", fields["stop"], stopBudget) + } + if blocked, _ := fields["blocked_on"].(string); !strings.Contains(blocked, "today's spending limit") { + t.Fatalf("blocked_on does not name today's limit: %q", blocked) + } +} + +// --yes-spend IS THE PERSON SAYING OTHERWISE. The same run, the same price, and +// the flag: it spends past the figure and finishes. +func TestDoOnTheRunEngineYesSpendRunsPastThePlanPrice(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + t.Setenv("CODEAF_PLAN_CONSENT", "0.5") + t.Setenv("CODEAF_DAILY_BUDGET", "0.5") + workspace := beltRepoWorkspace(t) + + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "write out.txt and say what you did", workspace: workspace, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), yesSpend: true, stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return finishingSeat(1) }, + }) + if err != nil { + t.Fatalf("a run with --yes-spend left with %v, want 0\nstdout:\n%s\nstderr:\n%s", + err, stdout.String(), stderr.String()) + } +} + +// --db IS REFUSED IN WORDS. The run keeps its plan in the directory it works in, +// so a store named on the command line is one it would never touch; the door +// says so and starts nothing, rather than working somewhere else in silence. +func TestDoOnTheRunEngineRefusesDbInWords(t *testing.T) { + beltRunEnv(t) + workspace := beltRepoWorkspace(t) + named := filepath.Join(t.TempDir(), "graph.db") + + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "write out.txt", workspace: workspace, database: named, asJSON: true, + timeout: 10 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return spendingSeat(0) }, + }) + var status exitStatus + if !asExitStatus(err, &status) || status != exitCannotRun { + t.Fatalf("--db on the run engine left with %v, want exit 1\nstdout:\n%s", err, stdout.String()) + } + outcome := decodeErrand(t, stdout.String()) + if !strings.Contains(outcome.Error, "--db") || !strings.Contains(outcome.Error, "CODEAF_TASK_BELT") { + t.Fatalf("the refusal does not say what --db cannot do here and how to reach it: %q", outcome.Error) + } + if _, err := os.Stat(session.PlanStorePath(workspace)); err == nil { + t.Fatal("a refused run opened a plan store anyway") + } +} + +// --keep IS HONOURED BY SAYING WHERE THE RECORD IS. The run's store is the +// working copy's own and is never deleted, so what the flag asks for is kept; +// the door says where, the way the older road does. +func TestDoOnTheRunEngineKeepSaysWhereTheRecordIs(t *testing.T) { + beltRunEnv(t) + t.Setenv("CODEAF_PLANDB_BIN", beltPlandbDoor(t)) + workspace := beltRepoWorkspace(t) + + var stdout, stderr strings.Builder + err := doErrand(doRequest{ + task: "write out.txt and say what you did", workspace: workspace, keep: true, asJSON: true, + timeout: 60 * time.Second, slots: bound(1), stdout: &stdout, stderr: &stderr, + newBeltCompleter: func(string) session.Completer { return finishingSeat(0) }, + }) + if err != nil { + t.Fatalf("errand: %v\n%s", err, stderr.String()) + } + want := "record kept at " + session.PlanStorePath(workspace) + if !strings.Contains(stderr.String(), want) { + t.Fatalf("--keep never said where the record is; want %q in:\n%s", want, stderr.String()) + } + if _, err := os.Stat(session.PlanStorePath(workspace)); err != nil { + t.Fatalf("the record --keep named is not there: %v", err) + } +} diff --git a/cmd/codeaf/do_engine_test.go b/cmd/codeaf/do_engine_test.go index 16cf436dc..02d6407b8 100644 --- a/cmd/codeaf/do_engine_test.go +++ b/cmd/codeaf/do_engine_test.go @@ -9,8 +9,9 @@ package main // bashworker tests make. // // THREE FACTS ARE UNDER TEST. A brief the scripted model completes leaves with -// exit 0 and an envelope naming the root's result, its landed file and the -// branch the landing answered. A ceiling of nothing leaves with exit 3 and +// exit 0 and an envelope naming the root's result and the file it wrote, left +// in place and uncommitted (do_engine_contract_test.go holds the rest of that +// contract). A ceiling of nothing leaves with exit 3 and // `blocked_on` naming the price it was held to. And the usage ledger is the // session's own — the worker the run hosts writes it, so the door adds no // second accounting. @@ -237,9 +238,8 @@ func beltGit(t *testing.T, dir string, args ...string) { // THE RUN ROAD COMPLETES A BRIEF AND NAMES THE ROOT'S RESULT. // // The scripted worker writes one file through bash and then finishes its task -// in the store with the result as its words; the run lands that file on the copy's branch, and the caller reads on -// stdout the root's own result, the landed path, and the branch the landing -// answered — the whole of what the run road owes an envelope. +// in the store with the result as its words; the caller reads on stdout the +// root's own result and the path the run wrote. // A ROOT FINISH THAT LANDS IN THE STORE BEFORE ITS WORKER RETURNS STILL NAMES THE ROOT RESULT. // // The finish command writes the root done row before its shell exits. Holding that @@ -330,14 +330,11 @@ func TestDoOnTheRunEngineCompletesABriefAndNamesTheRootResult(t *testing.T) { if outcome.Seconds <= 0 { t.Fatal("the run reported no elapsed time") } - // The landing committed the worker's file, and both the file and the - // branch it went to are on the object. + // The worker's file is on the object, where the run left it: in the + // directory it was handed, edited in place and not committed. want := filepath.Join(workspace, "out.txt") if len(outcome.Artifacts) != 1 || outcome.Artifacts[0] != want { - t.Fatalf("artifacts = %v, want the one landed path %s", outcome.Artifacts, want) - } - if !strings.Contains(outcome.Deliverable, "landed on work") { - t.Fatalf("the answer never named the branch the landing answered:\n%s", outcome.Deliverable) + t.Fatalf("artifacts = %v, want the one path the run wrote %s", outcome.Artifacts, want) } } diff --git a/docs/changes/unreleased/1416-do-door-keeps-its-contract.md b/docs/changes/unreleased/1416-do-door-keeps-its-contract.md new file mode 100644 index 000000000..b6d676e73 --- /dev/null +++ b/docs/changes/unreleased/1416-do-door-keeps-its-contract.md @@ -0,0 +1,21 @@ +--- +kind: fixed +title: codeaf do on the run engine edits in place, commits nothing, and stops at a price +pr: 1416 +surface: [engine, chat] +invalidates: + - "`codeaf do` on the run engine committed the directory's whole `git status` as `task: ` on the checked-out branch, the person's own uncommitted edits and untracked files included. It edits the directory in place and commits nothing, as `--dir` says, and its files are only the ones the run changed." + - "A `codeaf do` run on the run engine had no spending bound and `--yes-spend` did nothing. Without the flag it stops at the plan price (`CODEAF_PLAN_CONSENT`) or at what is left of today's limit, with exit 3; `--yes-spend` or `CODEAF_PREAUTHORIZE_SPEND=1` lets it spend past both." + - "`codeaf do --db` and `--keep` were silently ignored on the run engine. `--db` is refused with exit 1 and a sentence naming the older engine, and `--keep` says where the run's store is." + - "A belt landing left out every path ending in `.lock`, so a run's change to `yarn.lock`, `Cargo.lock`, `poetry.lock` or `flake.lock` never landed, and a `bench-results/` folder never did either. Only the paths the harness itself writes are left out now." + - "`CODEAF_CHECK_MODEL` seated checks for `codeaf do` only. A chat `/task` run reads it too." + - "An approved hand-off the run engine could not start fell back to the older engine with a receipt identical to the run road's. The receipt now says it runs on the older task engine and why." + - "The v0.4.0 changelog's #1109 entry says the node engine is deleted, that `CODEAF_TASK_BELT` does not exist, and that `propose_task`, `quick_task`, `divide_work` and `revise_assignment` are gone. None of that is true: the node engine is in the binary beside the run engine, `CODEAF_TASK_BELT` switches between them (unset is the run engine; `node`, `legacy` and `off` reach the node engine, see #1355), and those four tools are still the node engine's doors." +--- +The run engine became the road every `codeaf do` takes (#1355), and it kept +none of the older road's promises about the directory or the money. The older +road edits in place and never commits, and the help for `--dir` says exactly +that, so the run road now keeps the same contract: the folder is read before +the run starts and afterwards, and the files the run names are the ones it +changed. The plan-price question cannot be asked before a run starts, because +nothing prices a run up front, so the same figure is a ceiling instead. diff --git a/internal/e2e/do_run_engine_e2e_test.go b/internal/e2e/do_run_engine_e2e_test.go index 0204105bd..06686c6c9 100644 --- a/internal/e2e/do_run_engine_e2e_test.go +++ b/internal/e2e/do_run_engine_e2e_test.go @@ -1,8 +1,8 @@ //go:build e2e // do_run_engine_e2e_test.go drives `codeaf do` ON THE RUN ENGINE end to end: -// the built binary, a real provider, the bash belt, and a run that lands a file -// on a branch. +// the built binary, a real provider, the bash belt, and a run that writes a file +// in the directory it was handed, in place and uncommitted. // // where the column of this lane comes from // @@ -12,14 +12,14 @@ // environment ([session.BashBeltAsked]), and reads back the one machine object // `do --json` promises ([resultEnvelope]). That is the only way to prove the // run road is reachable by a person at all — that the switch survives the door, -// that the run engine dispatches the belt worker, and that the landing commits -// onto the branch the envelope names. +// that the run engine dispatches the belt worker, and that the run keeps the +// door's contract with the directory: edited in place, nothing committed. // -// THE WHOLE RUN IS ONE FILE AND ONE BRANCH. A throwaway repository is made with -// one committed file; the run works in it in place and lands `HELLO.md` on its -// branch; and the lane reads that file back with `git show <branch>:HELLO.md` -// rather than off the working tree, because the branch is the one thing the -// envelope promises and the working tree proves nothing about it. +// THE WHOLE RUN IS ONE FILE AND NO COMMIT. A throwaway repository is made with +// one committed file; the run works in it in place and writes `HELLO.md`; the +// lane reads that file off the working tree, finds it among the envelope's +// files, and checks the branch still stands on the commit it stood on, because +// `--dir` promises the directory "edited in place" and never a commit. // // THE SEATS ARE THE PROFILE'S, AND THE PROFILE IS WHERE THEY ARE PINNED. The // run engine seats each task from the PROFILE's tier rows — [run.CrewFactory] @@ -56,6 +56,7 @@ import ( "encoding/json" "os" "os/exec" + "path/filepath" "strings" "testing" "time" @@ -65,8 +66,8 @@ import ( ) // doBrief is the one errand this lane runs: write a file with one known word in -// it and stop. The word is the needle `git show <branch>:HELLO.md` looks for, -// and HELLO.md is the path the landing note names. +// it and stop. The word is the needle the lane reads HELLO.md for, and HELLO.md +// is the path the envelope's files name. const doBrief = "write HELLO.md containing the word hello, then stop" // doWall is the wall this lane gives the run, and it is the run's own clock. It @@ -95,8 +96,8 @@ type doEnvelope struct { } // TestDoOnTheRunEngine is the run road's completion lane: a real provider -// finishes a trivial errand, the run lands the file it wrote, and the envelope -// names the branch the landing answered. +// finishes a trivial errand, the file it wrote is in the working tree and among +// the envelope's files, and nothing was committed. func TestDoOnTheRunEngine(t *testing.T) { key := liveKey(t) product := binary(t) @@ -112,9 +113,13 @@ func TestDoOnTheRunEngine(t *testing.T) { config.KeyTierLowModel: e2eModel, config.KeyTierReflexModel: e2eModel, }) - // The working copy is a real repository with one committed file: the - // landing needs a branch to commit onto and the envelope a branch to name. + // The working copy is a real repository with one committed file, so a + // commit the run should not have made would move its HEAD. workspace := newWorkspace(t, "do-run-engine", false) + headBefore, err := exec.Command("git", "-C", workspace, "rev-parse", "HEAD").CombinedOutput() + if err != nil { + t.Fatalf("git rev-parse HEAD: %v\n%s", err, headBefore) + } ctx, cancel := context.WithTimeout(context.Background(), doRunEngineLead) defer cancel() @@ -153,41 +158,35 @@ func TestDoOnTheRunEngine(t *testing.T) { t.Fatalf("the envelope carries no deliverable, so nothing answered the brief:\n%s", stdout.String()) } - branch, named := landedBranch(envelope.Deliverable) - if !named { - t.Fatalf("the deliverable never named the branch the run landed on:\n%s", - envelope.Deliverable) - } - // THE BRANCH IS THE ANSWER, NOT THE TREE. The file is read back through - // git at the branch the envelope named, so a run that wrote HELLO.md to the - // working copy but landed nothing would fail here rather than pass. - out, err := exec.Command("git", "-C", workspace, "show", branch+":HELLO.md").CombinedOutput() + // THE DIRECTORY IS THE ANSWER, EDITED IN PLACE. The file is read off the + // working tree the run was handed, it is among the files the envelope + // names, and the branch stands where it stood: a run that committed on the + // person's branch fails here. + out, err := os.ReadFile(filepath.Join(workspace, "HELLO.md")) if err != nil { - t.Fatalf("git show %s:HELLO.md: %v\n%s", branch, err, out) + t.Fatalf("the run left no HELLO.md in the directory it was handed: %v", err) } if !strings.Contains(string(out), "hello") { - t.Fatalf("HELLO.md on %s does not contain the word hello:\n%s", branch, out) + t.Fatalf("HELLO.md does not contain the word hello:\n%s", out) } - t.Logf("landed HELLO.md on %s:\n%s", branch, out) -} - -// landedBranch reads the branch out of the landing note the deliverable ends -// with — the one sentence [run.LandingNote] writes, "landed on <branch>: N -// files". The note is the only place a caller learns the branch, so the parse -// is written against its exact words rather than against anything looser. -func landedBranch(deliverable string) (string, bool) { - const marker = "landed on " - at := strings.LastIndex(deliverable, marker) - if at < 0 { - return "", false + named := false + for _, file := range envelope.Files { + if filepath.Base(file) == "HELLO.md" { + named = true + } + } + if !named { + t.Fatalf("the envelope's files %v do not name HELLO.md", envelope.Files) + } + headAfter, err := exec.Command("git", "-C", workspace, "rev-parse", "HEAD").CombinedOutput() + if err != nil { + t.Fatalf("git rev-parse HEAD: %v\n%s", err, headAfter) } - rest := deliverable[at+len(marker):] - colon := strings.Index(rest, ":") - if colon <= 0 { - return "", false + if strings.TrimSpace(string(headAfter)) != strings.TrimSpace(string(headBefore)) { + t.Fatalf("the run committed on the person's branch: HEAD moved %s -> %s", + strings.TrimSpace(string(headBefore)), strings.TrimSpace(string(headAfter))) } - branch := strings.TrimSpace(rest[:colon]) - return branch, branch != "" + t.Logf("HELLO.md left in place:\n%s", out) } // doRunEngineEnv is the environment the run rides: the throwaway home, the diff --git a/internal/manual/chat/adaptive-runs.md b/internal/manual/chat/adaptive-runs.md index 7dfa7a3d3..f9ab332e0 100644 --- a/internal/manual/chat/adaptive-runs.md +++ b/internal/manual/chat/adaptive-runs.md @@ -151,12 +151,12 @@ keyboard decides for itself and says on the record that it decided. | flag | what it does | | --- | --- | -| `--db <path>` | work in this durable store instead of a private one | -| `--keep` | keep the private store instead of deleting it on the way out | -| `-w <dir>` | the directory to work in, edited in place — the current directory by default | +| `--db <path>` | work in this durable store instead of a private one (older engine only) | +| `--keep` | keep the store instead of deleting it on the way out | +| `-w <dir>` | the directory to work in, edited in place, never committed — the current directory by default | | `--timeout` | a hard wall on the whole run | | `--json` | print one machine-readable object instead of the deliverable | -| `--yes-spend` | approve a plan whose price crosses the consent threshold | +| `--yes-spend` | spend past today's limit and the plan price without stopping | | `--slots <n>` | how many workers may run at once for this run; `0` is no limit. Unset, it is your `task.parallel` setting, which is no limit out of the box | | `--model <slug>` | the work model for this run | | `--plan-model <slug>` | the model that plans, when it should differ from the work model | @@ -727,8 +727,12 @@ check what it did, not a run that ran out of time. ## My headless run failed — where is its record, why is there a folder left behind after `codeaf do`, how do I keep the run's files with `--keep` -`codeaf do` works in a private store of its own unless you point it somewhere durable with -`--db`. What becomes of that store depends on how the run ended: +**This is the older engine's store** (`CODEAF_TASK_BELT=node`). On the run engine, the +default, a run keeps its plan in `.codeaf/plandb.db` inside the directory it works in, never +deletes it, refuses `--db`, and with `--keep` says where the store is. + +On the older engine, `codeaf do` works in a private store of its own unless you point it +somewhere durable with `--db`. What becomes of that store depends on how the run ended: - **It worked** — exit 0 — and the store is deleted on the way out. Nothing is left behind, which is the point of a one-shot. diff --git a/internal/manual/chat/worker-harness.md b/internal/manual/chat/worker-harness.md index 1eb009cf4..091591950 100644 --- a/internal/manual/chat/worker-harness.md +++ b/internal/manual/chat/worker-harness.md @@ -589,7 +589,8 @@ named on a door or in the profile: adds reads a finished leaf against. `codeaf do` resolves it from `--check-model`, then the `CODEAF_CHECK_MODEL` environment value, then a plan seat pinned by `--plan-model` or `CODEAF_PLAN_MODEL`. A run pinned to two models checks on - the plan seat and no third model appears from the profile. Without those pins, + the plan seat and no third model appears from the profile. A `/task` has no flags, so + its check reads `CODEAF_CHECK_MODEL` alone. Without those pins, the check takes the crew's careful row, the same row a conversation's checker rides. The **probe** seat is the one the profile's own `low` row answers alone: nothing on a door names it, so a probe runs on the crew you set in `/crew`. @@ -622,6 +623,52 @@ it was (`done`, `error`, `incomplete`, `unchecked`, `budget`, `turn-cap`, `deadline`, `price`, `question`), and `ok` is true on exactly the runs that leave with 0. +## Does codeaf do commit my changes? It edits the folder in place and commits nothing + +`codeaf do` works in the directory you hand it with `-w` / `--dir` (the current +directory by default), **edited in place, on whatever branch is checked out there, +and nothing is committed**. The run's files are left uncommitted for you to read, +commit or throw away, exactly as the older engine left them. + +Your own work is never touched by the run's accounting: an edit you had not +committed, or an untracked file such as a secrets file, is still yours after the +run, still uncommitted and still untracked. The files the run names — `files:` on +standard output, `artifacts` in `--json` — are the ones **this run** changed, read +by comparing the folder before and after: a file it wrote, a file of yours it edited +further, and anything it committed itself. A folder that is not a git repository +names no files, though the run's edits are still on disk. + +The run's plan is kept in `.codeaf/plandb.db` inside that directory, and that file +is never named among the run's files. + +## How much can a codeaf do run spend — --yes-spend, the plan price and today's limit + +A `codeaf do` run has nobody watching, so **it stops at a price unless you said +otherwise**. Without `--yes-spend` it may spend up to the nearer of two figures: + +- the **plan price** — `CODEAF_PLAN_CONSENT`, or the same row in `/settings`, + **$100** out of the box — the point above which codeaf asks before it spends. + Reaching it ends the run with exit **3**, `stop` `price`, and `blocked_on` + saying the figure and to rerun with `--yes-spend`. `0` means never ask, and then + this figure does not stop the run; +- what is left of **today's spending limit** (`CODEAF_DAILY_BUDGET`, `0` for no + limit). Reaching it ends the run with exit **3** and `stop` `budget`; a day + already spent starts nothing. + +`--yes-spend`, or `CODEAF_PREAUTHORIZE_SPEND=1`, lets the run spend past both +without stopping. The older engine asked the plan-price question before it bought +anything; the run engine cannot price a run before its workers start, so the same +figure is a ceiling instead. + +## codeaf do --db on the run engine, and where a run's store is + +- **`--db` is refused**, with exit 1 and a sentence saying why: it names a store + only the older engine works in, and a run holds its plan in `.codeaf/plandb.db` + inside the directory it works in. Drop the flag, or set `CODEAF_TASK_BELT=node` + to run on the older engine, which takes it. +- **That store is never deleted.** Pass `--keep` and the run names it on the error + stream: `record kept at <dir>/.codeaf/plandb.db`. + ## How do I tell the check what to run? Declare each proof command when the task is created: add `--check '<the command @@ -685,6 +732,8 @@ words set, not one byte of any prompt, belt or landing moves from the older road**. Everything behind the switch is a seam. A build with no run engine linked answers -the older road, and every refusal on the run road falls back to it rather than -inventing a sentence of its own — so a conversation the run road cannot serve gets -exactly the door it always had. +the older road, and a task the run road cannot start falls back to it — so a +conversation the run road cannot serve gets exactly the door it always had. When a +task you approved falls back that way, its receipt says so: `It runs on the older +task engine, because the run engine could not start it:` and the run road's own +reason. diff --git a/internal/manual/chat_test.go b/internal/manual/chat_test.go index 587647ebd..55ccc00a5 100644 --- a/internal/manual/chat_test.go +++ b/internal/manual/chat_test.go @@ -2728,6 +2728,8 @@ func TestTheChatManualAnswersTheQuestionsPeopleAsk(t *testing.T) { {"can I add a note to a running task", "worker-harness"}, {"why did the task refuse my cancel", "worker-harness"}, {"how do I stop a run", "worker-harness"}, + {"does codeaf do commit my changes", "worker-harness"}, + {"how much can a codeaf do run spend without yes-spend", "worker-harness"}, {"stop it did nothing and the task kept running", "worker-harness"}, {"what happens to a run's branch after I stop it", "worker-harness"}, {"what can the task worker actually run", "worker-harness"}, diff --git a/internal/manual/truth_test.go b/internal/manual/truth_test.go index 748954a17..3a8975eff 100644 --- a/internal/manual/truth_test.go +++ b/internal/manual/truth_test.go @@ -245,8 +245,13 @@ func quotedFacts(t *testing.T) []quotedFact { }, }, { fact: "the figure a plan asks above", owner: "config.DefaultPlanConsentUSD", - value: dollarsOwed(config.DefaultPlanConsentUSD), - quotes: []quotedIn{{"models-and-cost", "| **per plan** | `asks first above $%s` |"}}, + value: dollarsOwed(config.DefaultPlanConsentUSD), + quotes: []quotedIn{ + {"models-and-cost", "| **per plan** | `asks first above $%s` |"}, + // `codeaf do` on the run engine stops at the same figure unless + // --yes-spend said otherwise, and the page that says so quotes it. + {"worker-harness", "**$%s** out of the box"}, + }, }, { fact: "what one firing may spend", owner: "standing.DefaultPerRunUSD", value: dollarsOwed(standing.DefaultPerRunUSD), diff --git a/internal/run/chat_check_seat_test.go b/internal/run/chat_check_seat_test.go new file mode 100644 index 000000000..e385ace8f --- /dev/null +++ b/internal/run/chat_check_seat_test.go @@ -0,0 +1,68 @@ +package run_test + +import ( + "context" + "errors" + "slices" + "sync" + "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" +) + +// CODEAF_CHECK_MODEL REACHES A CHAT'S RUN. The manual names the environment +// value as the check seat's rung after the flag, and a conversation has no +// flag, so the environment is the whole of the person's say over which model +// checks a `/task`'s work. The chat's door hands the engine its work and plan +// seats; the check seat is read at the engine's end of the seam, so a run the +// chat opens seats its check on the environment's model and never on the +// profile's careful row while the variable is set. +func TestTheChatDoorsCheckRidesTheCheckModelVariable(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + t.Setenv("CODEAF_PLANDB_BIN", stubCLI(t)) + t.Setenv(config.CheckModelEnv, "vendor/env-check") + store := runOpenStore(t) + if _, err := store.AddMany([]plandb.TaskSpec{{ID: "review", Title: "Review", Role: plandb.RoleCheck}}); err != nil { + t.Fatal(err) + } + dir := crewProfile(t, map[string]string{ + config.KeyTierHighModel: "vendor/profile-careful", + config.KeyTierWorkerModel: "vendor/profile-worker", + config.KeyTierMastermindModel: "vendor/profile-thinking", + }) + var mu sync.Mutex + var asked []string + refuse := &seat{ever: func(context.Context, []ai.Message) (*ai.Response, error) { + return nil, errors.New("scripted: no provider behind this seat") + }} + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Second) + defer cancel() + run.ChatEngine.Start(ctx, session.RunSpec{ + Store: store, + Workspace: t.TempDir(), + ProfileDir: dir, + WorkModel: "vendor/chat-work", + PlanModel: "vendor/chat-plan", + CompleterFor: func(model string) session.Completer { + mu.Lock() + asked = append(asked, model) + mu.Unlock() + return refuse + }, + }) + mu.Lock() + defer mu.Unlock() + if !slices.Contains(asked, "vendor/env-check") { + t.Fatalf("the chat's run seated its launches on %v; the check never rode %s=vendor/env-check", + asked, config.CheckModelEnv) + } + if slices.Contains(asked, "vendor/profile-careful") { + t.Fatalf("the chat's run seated a check on the profile's careful row %v while %s was set", + asked, config.CheckModelEnv) + } +} diff --git a/internal/run/enginewire.go b/internal/run/enginewire.go index 59c248f87..bc8b50718 100644 --- a/internal/run/enginewire.go +++ b/internal/run/enginewire.go @@ -12,6 +12,7 @@ package run import ( "context" + "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/plandb" "github.com/Agent-Field/codeaf/internal/session" ) @@ -40,9 +41,14 @@ func (engine) Start(ctx context.Context, spec session.RunSpec) session.RunSummar // them itself (the enginewire spec's WorkModel and PlanModel), so the // factory seats the work and plan roles on the door's answer rather than // asking the profile again for a row the door already moved. + // + // AND THE CHECK SEAT CLIMBS THE SAME LADDER `codeaf do` CLIMBS, minus + // the flag no chat has ([chatCheckSeat]), so CODEAF_CHECK_MODEL reaches + // a `/task` run the way the manual says it reaches a headless one. Factory: CrewFactory(spec.Store, spec.Workspace, spec.ProfileDir, Seats{ - Work: spec.WorkModel, - Plan: spec.PlanModel, + Work: spec.WorkModel, + Plan: spec.PlanModel, + Check: chatCheckSeat(), }, spec.CompleterFor), OnSpend: spec.OnSpend, }) @@ -65,6 +71,20 @@ func (engine) Start(ctx context.Context, spec session.RunSpec) session.RunSummar } } +// chatCheckSeat is the check seat a chat's run rides: the check seat's own +// ladder ([config.CheckSeat]) with no flag, because a conversation has none, +// and with no pinned plan seat, because a conversation's plan seat is its own +// mastermind row rather than a pin — so CODEAF_CHECK_MODEL, and empty +// otherwise, which the crew factory fills from the profile's careful row. +// +// ONE LADDER, TWO DOORS. `codeaf do` resolves the same seat through the same +// function with its `--check-model` flag in front, so the environment rung the +// manual documents is one rung and not a promise one door kept and the other +// did not. +func chatCheckSeat() string { + return config.CheckSeat("", config.Seat{}).Model +} + // runLimitOf is the seam's one mapping of the limit fact: the run's words and // the session's are spelled apart because neither package may reach the other, // and a limit this build does not know reads as none rather than as a guess. diff --git a/internal/run/export_engine_test.go b/internal/run/export_engine_test.go new file mode 100644 index 000000000..2be70cb2a --- /dev/null +++ b/internal/run/export_engine_test.go @@ -0,0 +1,7 @@ +package run + +import "github.com/Agent-Field/codeaf/internal/session" + +// ChatEngine is the engine this package installs into the chat's task door, +// reached by a test the way the door reaches it: through [session.RunEngine]. +var ChatEngine session.RunEngine = engine{} diff --git a/internal/session/belt_tree_lockfiles_test.go b/internal/session/belt_tree_lockfiles_test.go new file mode 100644 index 000000000..699ed516f --- /dev/null +++ b/internal/session/belt_tree_lockfiles_test.go @@ -0,0 +1,50 @@ +package session + +import ( + "path/filepath" + "strings" + "testing" +) + +// A PROJECT'S OWN LOCKFILES ARE THE PROJECT'S WORK, AND A BELT LANDING CARRIES +// THEM. Every package manager keeps one beside its manifest, and a run that +// changed a dependency changed that file; a landing that dropped every path +// ending in `.lock` left the run's dependency change behind while the manifest +// beside it landed. Only the paths the harness itself writes stay out — its own +// folder, its plan store and the files beside it, and the shim it arms — and a +// project folder that merely has a familiar name is the project's. +func TestABeltLandingCarriesTheProjectsOwnLockfiles(t *testing.T) { + repo := newTestRepo(t) + for _, name := range []string{"yarn.lock", "Cargo.lock"} { + writeFile(t, filepath.Join(repo, name), "pinned 1.0\n") + } + mustGit(t, repo, "add", "-A") + mustGit(t, repo, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-m", "lockfiles") + + // The run's own changes: two lockfiles moved, two new ones, and a project + // folder whose name a benchmark rig also uses. + project := []string{"yarn.lock", "Cargo.lock", "poetry.lock", "flake.lock", "bench-results/table.md"} + for _, name := range project { + writeFile(t, filepath.Join(repo, filepath.FromSlash(name)), "pinned 2.0\n") + } + // The harness's own writes inside the copy. + machinery := []string{".codeaf/plandb.db", planStoreFilename, planStoreFilename + "-wal", "bin/" + planShimFilename} + for _, name := range machinery { + writeFile(t, filepath.Join(repo, filepath.FromSlash(name)), "harness\n") + } + + staged := map[string]bool{} + for _, spec := range beltTreeWork(repo) { + staged[strings.TrimPrefix(spec, literalPathspec)] = true + } + for _, name := range project { + if !staged[name] { + t.Errorf("the landing left the project's own %s behind; it staged %v", name, staged) + } + } + for _, name := range machinery { + if staged[name] { + t.Errorf("the landing staged the harness's own %s as the run's work", name) + } + } +} diff --git a/internal/session/run_tree_changes.go b/internal/session/run_tree_changes.go new file mode 100644 index 000000000..f15b05a6b --- /dev/null +++ b/internal/session/run_tree_changes.go @@ -0,0 +1,217 @@ +package session + +import ( + "crypto/sha256" + "encoding/hex" + "os" + "path/filepath" + "sort" + "strings" +) + +// RunTreeSnapshot is what a working copy held before a run touched it: the +// commit it stood on and, for every path git already saw as changed, what +// that path held. A door that runs IN PLACE — `codeaf do`, which edits the +// directory it was handed and commits nothing — takes one before the run and +// asks it afterwards which paths the RUN changed, so the files it names are +// the run's and never the person's own edits that were sitting there first. +// +// THE PERSON'S WORK IS NOT THE RUN'S WORK. A copy's `git status` after a run is +// the run's changes AND whatever the person had not committed yet, and the +// landing that staged the whole of it committed somebody's half-finished edit +// and an untracked secrets file as `task: <title>` on their own branch. The +// snapshot is how the two are told apart without any ledger: a path the run +// did not touch holds exactly what it held before. +// +// A directory that is not inside a git work tree has no snapshot to take, and +// [RunTreeSnapshot.Changed] answers nothing for it: there is no status to read +// the run's work off, and a walk of an arbitrary folder is not this door's +// business. +type RunTreeSnapshot struct { + dir string + root string + head string + // held is every path git saw as changed before the run, by absolute path, + // and what it held then: a digest of its bytes, or empty when it was gone. + held map[string]string +} + +// SnapshotRunTree reads dir's working copy as it stands now. +func SnapshotRunTree(dir string) RunTreeSnapshot { + snapshot := RunTreeSnapshot{dir: dir} + root, ok := runTreeRoot(dir) + if !ok { + return snapshot + } + snapshot.root = root + snapshot.head = runTreeHead(root) + snapshot.held = make(map[string]string) + for _, path := range runTreeStatus(root) { + snapshot.held[path] = runTreeDigest(path) + } + return snapshot +} + +// Changed answers the paths the run changed since the snapshot was taken, as +// absolute paths, sorted: a path git sees as changed now that was clean before, +// a path that was already changed and now holds something else, a path that +// was changed before and is clean now (the run put it back), and every path a +// commit the run made itself carried. The harness's own files are never in it +// ([harnessWrote]). +func (s RunTreeSnapshot) Changed() []string { + if s.root == "" { + return nil + } + seen := make(map[string]bool) + var changed []string + add := func(path string) { + if seen[path] || s.harnessOwns(path) { + return + } + seen[path] = true + changed = append(changed, path) + } + now := runTreeStatus(s.root) + still := make(map[string]bool, len(now)) + for _, path := range now { + still[path] = true + before, was := s.held[path] + if !was || before != runTreeDigest(path) { + add(path) + } + } + for path := range s.held { + if !still[path] { + add(path) + } + } + // A RUN THAT COMMITTED ITS OWN WORK moved HEAD, and what it committed is + // clean in the status above. Those paths are the run's too. + if head := runTreeHead(s.root); s.head != "" && head != "" && head != s.head { + if out, err := git(s.root, "diff", "--name-only", "-z", s.head, head); err == nil { + for _, name := range strings.Split(out, "\x00") { + if name = strings.TrimSpace(name); name != "" { + add(filepath.Join(s.root, filepath.FromSlash(name))) + } + } + } + } + sort.Strings(changed) + return changed +} + +// harnessOwns says whether an absolute path is one the harness itself wrote +// under the directory the run was handed, read relative to that directory +// because the harness's own folder sits there and not at the repository's top. +// A path outside the directory is the run's: a worker that edited above the +// folder it was handed still edited it. +func (s RunTreeSnapshot) harnessOwns(path string) bool { + base := canonicalPath(s.dir) + relative, err := filepath.Rel(base, path) + if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { + return false + } + return harnessWrote(filepath.ToSlash(relative)) +} + +// harnessWrote is THE ONE ANSWER to which paths inside a working copy are the +// harness's own rather than the work: its folder of droppings, its plan store +// under the name every road agrees on ([planStoreFilename], and the files the +// store's engine keeps beside it), and the shim it arms. It is read by the belt +// landing ([beltTreeWork]) and by an in-place run's account of what it changed +// ([RunTreeSnapshot.Changed]), so the two cannot disagree about it. +// +// A PATH IS THE HARNESS'S ONLY BECAUSE THE HARNESS WROTE IT THERE, never +// because of what a file of that kind is usually called. A suffix such as +// `.lock` is the name a project's own lockfile carries — the one every package +// manager keeps beside the manifest — and a rule that dropped every such file +// dropped the run's dependency changes from every landing while the run said +// it had made them. +func harnessWrote(path string) bool { + switch { + case isTaskDropping(path): + return true + case path == planStoreFilename, + strings.HasPrefix(path, planStoreFilename+"."), + strings.HasPrefix(path, planStoreFilename+"-"): + return true + case path == "bin/"+planShimFilename: + return true + } + return false +} + +// runTreeRoot is the canonical top of the work tree dir sits in, and false +// when dir is not inside one. It asks through [repositoryRoot], the one asker +// of that question, so a scratch folder that happens to sit inside somebody +// else's checkout reads as no repository rather than as theirs. +func runTreeRoot(dir string) (string, bool) { + if strings.TrimSpace(dir) == "" { + return "", false + } + return repositoryRoot(dir) +} + +// runTreeHead is the commit the copy stands on, and empty on a repository with +// no commit yet. +func runTreeHead(root string) string { + out, err := git(root, "rev-parse", "--verify", "-q", "HEAD") + if err != nil { + return "" + } + return strings.TrimSpace(out) +} + +// runTreeStatus is every path git sees as changed in the copy — modified, +// added, deleted and untracked alike — as absolute paths. It reads the NUL +// form so a name with a space, a quote or a newline in it is itself; a rename +// carries both names and both are answered, because both moved. +func runTreeStatus(root string) []string { + out, err := git(root, "status", "--porcelain", "-z", "--untracked-files=all") + if err != nil { + return nil + } + var paths []string + fields := strings.Split(out, "\x00") + for i := 0; i < len(fields); i++ { + entry := fields[i] + if len(entry) < 4 { + continue + } + code, name := entry[:2], entry[3:] + paths = append(paths, filepath.Join(root, filepath.FromSlash(name))) + if code[0] == 'R' || code[0] == 'C' { + // The next field is the name it came from. + if i+1 < len(fields) && fields[i+1] != "" { + paths = append(paths, filepath.Join(root, filepath.FromSlash(fields[i+1]))) + } + i++ + } + } + return paths +} + +// runTreeDigest is what one path holds: a digest of a file's bytes, the target +// of a link, a word for a directory, and empty for a path that is not there. +func runTreeDigest(path string) string { + info, err := os.Lstat(path) + if err != nil { + return "" + } + switch { + case info.Mode()&os.ModeSymlink != 0: + target, err := os.Readlink(path) + if err != nil { + return "link" + } + return "link:" + target + case info.IsDir(): + return "dir" + } + contents, err := os.ReadFile(path) + if err != nil { + return "unreadable" + } + sum := sha256.Sum256(contents) + return info.Mode().Perm().String() + ":" + hex.EncodeToString(sum[:]) +} diff --git a/internal/session/run_tree_changes_test.go b/internal/session/run_tree_changes_test.go new file mode 100644 index 000000000..a468b2c72 --- /dev/null +++ b/internal/session/run_tree_changes_test.go @@ -0,0 +1,55 @@ +package session + +import ( + "os" + "path/filepath" + "sort" + "strings" + "testing" +) + +// AN IN-PLACE RUN NAMES ONLY WHAT IT CHANGED. The person's own uncommitted edit +// and their untracked file were there before the run and still hold what they +// held, so they are not the run's; a file the run wrote, a file of the +// person's the run edited further, and a file the run committed itself are. +func TestARunTreeSnapshotNamesOnlyTheRunsChanges(t *testing.T) { + repo := newTestRepo(t) + writeFile(t, filepath.Join(repo, "shared.txt"), "the person's own edit\n") + writeFile(t, filepath.Join(repo, "secret.env"), "TOKEN=mine\n") + writeFile(t, filepath.Join(repo, "draft.md"), "the person's draft\n") + + snapshot := SnapshotRunTree(repo) + + // What the run does. + writeFile(t, filepath.Join(repo, "out.txt"), "written by the run\n") + writeFile(t, filepath.Join(repo, "draft.md"), "the run finished the draft\n") + writeFile(t, filepath.Join(repo, ".codeaf", "plandb.db"), "harness\n") + writeFile(t, filepath.Join(repo, "committed.txt"), "the run committed this\n") + mustGit(t, repo, "add", "committed.txt") + mustGit(t, repo, "-c", "user.name=t", "-c", "user.email=t@t", "commit", "-m", "the run's own commit") + + root := canonicalPath(repo) + want := []string{ + filepath.Join(root, "committed.txt"), + filepath.Join(root, "draft.md"), + filepath.Join(root, "out.txt"), + } + sort.Strings(want) + got := snapshot.Changed() + if strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Fatalf("the run's changes = %v, want %v", got, want) + } +} + +// A FOLDER THAT IS NOT A REPOSITORY HAS NO STATUS TO READ, and the snapshot +// answers nothing rather than guessing. +func TestARunTreeSnapshotOutsideARepositoryNamesNothing(t *testing.T) { + dir := t.TempDir() + snapshot := SnapshotRunTree(dir) + if err := os.WriteFile(filepath.Join(dir, "out.txt"), []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + if got := snapshot.Changed(); len(got) != 0 { + t.Fatalf("a folder with no repository named %v", got) + } +} diff --git a/internal/session/task.go b/internal/session/task.go index 457392cc0..ddce0407f 100644 --- a/internal/session/task.go +++ b/internal/session/task.go @@ -812,8 +812,9 @@ func (p *stagedProposal) Commit(ctx context.Context) (string, bool, error) { // ask with it when this turn owes one (CHAT-ROLE.md, "A landing speaks only // when an answer is owed"). A task about ANOTHER FOLDER than the work // already underway is refused here ([standsElsewhereError]); any other - // failure of the run road falls through to the shipped engine, exactly as a - // typed /task does, and that engine cuts its own copy from the same stand. + // failure of the run road falls through to the older engine, which cuts its + // own copy from the same stand, and the receipt says so and why + // ([runRoadFellBack]). if bashBeltAsked() && chatRunEngine != nil && !a.config.InTask { a.mu.Lock() question := questionAtTaskHandoff(a.owedAsks) @@ -842,12 +843,28 @@ func (p *stagedProposal) Commit(ctx context.Context) (string, bool, error) { } return receipt, false, nil } + // THE FALLBACK IS SAID, NOT SILENT. The work still starts — on the older + // engine, which cuts its own copy from the same stand — but a receipt + // identical to the run road's hid which engine took it and why, so the + // conversation described a run that did not exist. The reason rides the + // receipt in the run road's own words. + state := graph.admit(p.id, spec) + admitted = true + return withReport(taskReceipt(p.id, spec, state, p.stand, elsewhere), runRoadFellBack(err)), false, nil } state := graph.admit(p.id, spec) admitted = true return taskReceipt(p.id, spec, state, p.stand, elsewhere), false, nil } +// runRoadFellBack is the line an approved hand-off's receipt carries when the +// run engine could not start it and the older engine took it instead: which +// engine the work is on, and the run road's own reason. +func runRoadFellBack(err error) string { + return "It runs on the older task engine, because the run engine could not start it: " + + strings.TrimRight(strings.TrimSpace(err.Error()), ".") + "." +} + // taskReceipt is what an admitted proposal hands back to the model. // // THE MODEL IS NAMED BACK ONLY WHEN IT WAS ASKED FOR. A word resolves to an id diff --git a/internal/session/task_proposal_fallback_test.go b/internal/session/task_proposal_fallback_test.go new file mode 100644 index 000000000..386cd35de --- /dev/null +++ b/internal/session/task_proposal_fallback_test.go @@ -0,0 +1,41 @@ +package session + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// A HAND-OFF THE RUN ENGINE COULD NOT START SAYS WHICH ENGINE TOOK IT, AND WHY. +// +// The run road's store will not open — a directory stands where the store file +// goes — so the approved task falls back to the older engine. The work still +// starts, but the receipt the conversation reads must not be the run road's +// receipt word for word: it names the engine the work is on and the run road's +// own reason, so the chat never describes a run that does not exist. +func TestAnApprovedHandoffTheRunEngineCouldNotStartSaysSo(t *testing.T) { + t.Setenv("CODEAF_TASK_BELT", "bash") + double := newBeltRunDouble("never reached") + registerBeltRunEngine(t, double) + dir := t.TempDir() + // The store's own name, taken by a directory: the run road cannot open it. + if err := os.MkdirAll(filepath.Join(dir, planStoreFilename, "in-the-way"), 0o755); err != nil { + t.Fatal(err) + } + agent, _ := newTestAgent(t, beltRunCompleter{text: "never reached"}, func(config *Config) { + config.Workspace = newTestRepo(t) + config.Place = Place{Dir: dir} + config.AskConsent = false + config.TaskAutoApproveSeconds = 0 + }) + agent.graph().run = func(*TaskNode) {} + + answer, failed, err := approveBeltProposal(t, agent, beltProposalArgs("Change the fallback road", "the focused proof passes")) + if err != nil || failed { + t.Fatalf("propose_task: failed=%v err=%v answer=%q", failed, err, answer) + } + if !strings.Contains(answer, "It runs on the older task engine, because the run engine could not start it:") { + t.Fatalf("the receipt hides that the run engine could not start the task:\n%s", answer) + } +} diff --git a/internal/session/task_run.go b/internal/session/task_run.go index e91044eb2..ae1b40875 100644 --- a/internal/session/task_run.go +++ b/internal/session/task_run.go @@ -9353,15 +9353,14 @@ func beltTreeWork(dir string) []string { } var paths []string for _, path := range porcelainPaths(out) { - switch { - case isTaskDropping(path): - case path == "bench-results" || strings.HasPrefix(path, "bench-results/"): - case path == planStoreFilename || strings.HasPrefix(path, planStoreFilename+"."): - case path == "bin/plandb": - case strings.HasSuffix(path, ".lock"): - default: - paths = append(paths, literalPathspec+path) + // WHAT IS MACHINERY IS ANSWERED IN ONE PLACE ([harnessWrote]), by where + // the harness itself writes, and never by a name project files share: a + // `.lock` suffix here once kept every lockfile a run changed off the + // branch, and a `bench-results` directory is a project's own folder. + if harnessWrote(path) { + continue } + paths = append(paths, literalPathspec+path) } return paths }