From 9dac0b56aed67fe45afbab610f7ad4daef01b3b1 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:05:18 -0400 Subject: [PATCH 01/10] fix(standing): preserve shared timer ownership during implicit setup --- cmd/codeaf/chatv3_standing.go | 9 +- cmd/codeaf/chatv3_standing_test.go | 12 +- internal/manual/chat/keeping-an-eye.md | 9 + internal/session/standing_test.go | 26 ++- internal/session/tools_standing.go | 8 +- internal/standing/standing.go | 1 + internal/standing/watch.go | 121 ++++++++++-- internal/standing/watch_ownership_test.go | 224 ++++++++++++++++++++++ 8 files changed, 382 insertions(+), 28 deletions(-) create mode 100644 internal/standing/watch_ownership_test.go diff --git a/cmd/codeaf/chatv3_standing.go b/cmd/codeaf/chatv3_standing.go index 9d0aa1c36b..6cb4a1e854 100644 --- a/cmd/codeaf/chatv3_standing.go +++ b/cmd/codeaf/chatv3_standing.go @@ -460,8 +460,7 @@ func v3StandingSeam(seam *session.Standing) tui3.StandingSeam { // named as an interface so a test can hand it a definition pointing at a dead // path without going anywhere near this machine's launchd. type backgroundTimer interface { - Drift() (standing.WatchDrift, error) - Install(ctx context.Context) error + Repair(ctx context.Context) (standing.WatchDrift, error) } // repairBackgroundChecks puts a drifted timer back, and says one line about it @@ -489,11 +488,11 @@ func repairBackgroundChecks(watch backgroundTimer, wanted bool) string { if watch == nil || !wanted { return "" } - drift, err := watch.Drift() - if err != nil || !drift.Present || !drift.Stale { + drift, err := watch.Repair(context.Background()) + if !drift.Present { return "" } - if err := watch.Install(context.Background()); err != nil { + if err != nil { return "could not put the background check back: " + err.Error() } if drift.Gone && drift.Executable != "" { diff --git a/cmd/codeaf/chatv3_standing_test.go b/cmd/codeaf/chatv3_standing_test.go index 877f44350a..c95259ec9c 100644 --- a/cmd/codeaf/chatv3_standing_test.go +++ b/cmd/codeaf/chatv3_standing_test.go @@ -62,11 +62,15 @@ type driftedTimer struct { fail error } -func (d *driftedTimer) Drift() (standing.WatchDrift, error) { return d.drift, d.err } - -func (d *driftedTimer) Install(context.Context) error { +func (d *driftedTimer) Repair(context.Context) (standing.WatchDrift, error) { + if d.err != nil { + return standing.WatchDrift{}, d.err + } + if !d.drift.Present || !d.drift.Stale { + return standing.WatchDrift{}, nil + } d.installs++ - return d.fail + return d.drift, d.fail } // A TIMER POINTING AT A PROGRAM THAT MOVED RUNS NOTHING, and nothing on screen diff --git a/internal/manual/chat/keeping-an-eye.md b/internal/manual/chat/keeping-an-eye.md index faf713c293..35da2338be 100644 --- a/internal/manual/chat/keeping-an-eye.md +++ b/internal/manual/chat/keeping-an-eye.md @@ -701,6 +701,15 @@ program — and a launch speaks only for its own pair. else's: it neither claims it nor rewrites it, and its `/status` says nothing is checking that home. +Approving the first standing item follows the same ownership rule. It installs a +missing timer or repairs this home's stale timer, but never takes another home's +or live build's timer. If another home owns it, the item is saved and CodeAF says +that the background check could not be installed and the existing timer was left +unchanged. This home is not being checked in the background; use the background +checks row in `/settings` only when you deliberately want to move the shared timer. +Ownership checks and changes are serialized across processes, including that +explicit settings action. + ## Do reminders work over --host — yes, on the far machine Yes, and this is the one ambient thing a connection does not take away. Over diff --git a/internal/session/standing_test.go b/internal/session/standing_test.go index 127d53bdf3..0bdf01e5d4 100644 --- a/internal/session/standing_test.go +++ b/internal/session/standing_test.go @@ -724,12 +724,18 @@ func TestStandingListSpeaksThePersonsWords(t *testing.T) { // fakeWatch is this machine's scheduler, stood in for. Nothing in these tests // goes near launchd. type fakeWatch struct { + ensures int installs int uninstalls int fail error installed bool } +func (w *fakeWatch) Ensure(ctx context.Context) error { + w.ensures++ + return w.Install(ctx) +} + func (w *fakeWatch) Install(context.Context) error { w.installs++ if w.fail != nil { @@ -791,8 +797,8 @@ func TestTheFirstThingThatStandsTurnsBackgroundChecksOnAndSaysSo(t *testing.T) { watch := &fakeWatch{} events := standRatify(t, store, watch, t.TempDir()) - if watch.installs != 1 { - t.Fatalf("the timer was installed %d times, want exactly 1", watch.installs) + if watch.installs != 1 || watch.ensures != 1 { + t.Fatalf("implicit ensures=%d, installs=%d; want one ensure", watch.ensures, watch.installs) } if line := backgroundLine(events); line != standingBackgroundLine { t.Fatalf("the line said %q, want %q", line, standingBackgroundLine) @@ -2002,3 +2008,19 @@ func standingNextUpdate(t *testing.T, lane <-chan Event) Event { } } } + +func TestFirstStandingApprovalPreservesAnotherProfilesTimer(t *testing.T) { + store := newFakeStanding(t) + watch := &fakeWatch{fail: standing.ErrWatchOwned} + events := standRatify(t, store, watch, t.TempDir()) + if watch.ensures != 1 || watch.installed { + t.Fatalf("implicit ownership refusal: ensures=%d installed=%v", watch.ensures, watch.installed) + } + line := backgroundLine(events) + if !strings.HasPrefix(line, standingBackgroundFailed) || !strings.Contains(line, "existing timer was left unchanged") { + t.Fatalf("ownership refusal was not told honestly: %q", line) + } + if len(store.created) != 1 { + t.Fatal("timer refusal must not discard the approved standing item") + } +} diff --git a/internal/session/tools_standing.go b/internal/session/tools_standing.go index fc91dfe966..23746e3499 100644 --- a/internal/session/tools_standing.go +++ b/internal/session/tools_standing.go @@ -105,7 +105,7 @@ const standingPastGrace = 30 * time.Second const standingWatchOffer = "watch-offer.json" // standingWatchAnswer is that marker's whole content. It is journaled BEFORE -// [standing.Watch.Install] is called, so a person whose launchd would not take +// [standing.Watch.Ensure] is called, so a person whose launchd would not take // the file is somebody this build knows it has already spoken to — rather than // somebody it tells again tomorrow. // @@ -1213,8 +1213,8 @@ func (a *Agent) emitStandingNews(update string, item standing.Item, text string) // ── background checks, on by default, said once ───────────────────────────── -// standingBackgroundOn installs this machine's timer the first time anything -// ever stands, and says the one dim line about it. +// standingBackgroundOn ensures background checks the first time anything stands, +// without taking another profile's timer, and says the result in one dim line. // // NOBODY IS ASKED, AND IT HAPPENS ONCE, EVER. There used to be a question here // — keep checking when no window is open? — and it had one sensible answer: @@ -1242,7 +1242,7 @@ func (a *Agent) standingBackgroundOn(store standingStore, item standing.Item) { return } standingRememberWatch(store.Root(), true) - if err := a.config.Standing.Watch.Install(context.Background()); err != nil { + if err := a.config.Standing.Watch.Ensure(context.Background()); err != nil { // SAID HONESTLY AND NOT SWALLOWED. The person is about to walk away from // a machine they think is watching something for them. a.emitStandingUpdate(standingBackgroundUpdate, item, diff --git a/internal/standing/standing.go b/internal/standing/standing.go index 02292e71f1..2fcb2aa8e0 100644 --- a/internal/standing/standing.go +++ b/internal/standing/standing.go @@ -1075,6 +1075,7 @@ type WatchStatus struct { // `codeaf tick` every [Interval]. The core lane builds it on internal/watchdog's // shape with its own unit names, so it can coexist with v1's. type Watch interface { + Ensure(ctx context.Context) error Install(ctx context.Context) error Uninstall(ctx context.Context) error Status() (WatchStatus, error) diff --git a/internal/standing/watch.go b/internal/standing/watch.go index 63299b9188..c8faa264bb 100644 --- a/internal/standing/watch.go +++ b/internal/standing/watch.go @@ -44,6 +44,7 @@ import ( "time" "github.com/Agent-Field/codeaf/internal/env" + "github.com/Agent-Field/codeaf/internal/filelock" "github.com/Agent-Field/codeaf/internal/home" ) @@ -183,12 +184,64 @@ func NewWatch(options WatchOptions) (*Timer, error) { }, nil } -// Install writes the exact definition and asks this user's operating system to -// use it now. Installing over an existing one repairs drift and is safe. +// ErrWatchOwned means implicit setup would take the login's shared timer away +// from another profile or a live build. Only explicit settings may do that. +var ErrWatchOwned = errors.New("background checks belong to another profile or running build; the existing timer was left unchanged") + +// Ensure provides background checks without taking another owner's timer. +// It is the implicit first-approval path; Install is an explicit takeover. +func (w *Timer) Ensure(ctx context.Context) error { + return w.underLock(ctx, func() error { + seen, err := w.read() + if err != nil { + return err + } + if seen.drift.Present { + if !seen.ours { + return ErrWatchOwned + } + if !seen.drift.Stale { + return nil // This profile is already checked, possibly by another build. + } + if w.otherLiveProgram(seen) { + return ErrWatchOwned + } + } + return w.install(ctx) + }) +} + +// Repair only restores an existing, repairable timer belonging to this profile. +// The returned drift names the repair attempted (successful when err is nil), never an earlier reading +// that could race an explicit off or another profile's install. +func (w *Timer) Repair(ctx context.Context) (WatchDrift, error) { + var repaired WatchDrift + err := w.underLock(ctx, func() error { + seen, err := w.read() + if err != nil { + return err + } + if !seen.drift.Present || !seen.drift.Stale || !seen.ours || w.otherLiveProgram(seen) { + return nil + } + repaired = seen.drift + return w.install(ctx) + }) + return repaired, err +} + +func (w *Timer) otherLiveProgram(seen reading) bool { + return seen.drift.Executable != "" && seen.drift.Executable != w.executable && !seen.drift.Gone +} + +// Install is the explicit settings action: write this profile/program pair, +// including taking over an existing timer. Implicit callers must use Ensure +// or Repair, which apply ownership under the same interprocess lock. func (w *Timer) Install(ctx context.Context) error { - if w == nil { - return errors.New("standing: no timer") - } + return w.underLock(ctx, func() error { return w.install(ctx) }) +} + +func (w *Timer) install(ctx context.Context) error { switch w.platform { case "darwin": return w.installDarwin(ctx) @@ -198,19 +251,61 @@ func (w *Timer) Install(ctx context.Context) error { return fmt.Errorf("standing: keeping watch is not available on %s", w.platform) } -// Uninstall stops the timer and removes its definition. A definition that is -// not there is already uninstalled, and says so without touching the host. +// Uninstall is the explicit settings action that stops and removes the timer. func (w *Timer) Uninstall(ctx context.Context) error { + return w.underLock(ctx, func() error { + switch w.platform { + case "darwin": + return w.uninstallDarwin(ctx) + case "linux": + return w.uninstallLinux(ctx) + } + return fmt.Errorf("standing: keeping watch is not available on %s", w.platform) + }) +} + +// One lock per OS timer, not per CODEAF_HOME: ownership checks and all writes +// must serialize across profiles and builds. The lock file is never removed, +// so a waiter cannot keep a lock on an unlinked inode while another replaces it. +func (w *Timer) underLock(ctx context.Context, change func() error) error { if w == nil { return errors.New("standing: no timer") } - switch w.platform { - case "darwin": - return w.uninstallDarwin(ctx) - case "linux": - return w.uninstallLinux(ctx) + if w.platform != "linux" && w.platform != "darwin" { + return fmt.Errorf("standing: keeping watch is not available on %s", w.platform) + } + if err := ctx.Err(); err != nil { + return err + } + path := w.primaryPath() + ".lock" + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + return err + } + lock, err := os.OpenFile(path, os.O_CREATE|os.O_RDWR, 0o600) + if err != nil { + return err + } + defer lock.Close() + retry := time.NewTicker(25 * time.Millisecond) + defer retry.Stop() + for { + if err := ctx.Err(); err != nil { + return err + } + err := filelock.Lock(lock, true, true) + if err == nil { + defer func() { _ = filelock.Unlock(lock) }() + return change() + } + if !filelock.IsBusy(err) { + return err + } + select { + case <-ctx.Done(): + return ctx.Err() + case <-retry.C: + } } - return fmt.Errorf("standing: keeping watch is not available on %s", w.platform) } // Status derives everything: installation from the definition's own bytes, the diff --git a/internal/standing/watch_ownership_test.go b/internal/standing/watch_ownership_test.go new file mode 100644 index 0000000000..9a57593f31 --- /dev/null +++ b/internal/standing/watch_ownership_test.go @@ -0,0 +1,224 @@ +package standing + +import ( + "context" + "errors" + "os" + "path/filepath" + "reflect" + "sync" + "testing" + "time" + + "github.com/Agent-Field/codeaf/internal/filelock" +) + +func timerDefinitions(t *testing.T, w *Timer) map[string]string { + t.Helper() + paths := []string{w.primaryPath()} + if w.platform == "linux" { + paths = append(paths, w.linuxServicePath()) + } + out := map[string]string{} + for _, path := range paths { + b, err := os.ReadFile(path) + if err != nil && !os.IsNotExist(err) { + t.Fatal(err) + } + out[path] = string(b) + } + return out +} + +func TestImplicitTimerSetupRespectsOwnership(t *testing.T) { + for _, platform := range []string{"linux", "darwin"} { + for _, scenario := range []string{"other-home", "other-home-gone", "same-home-live", "same-home-live-drift", "same-home-gone"} { + t.Run(platform+"/"+scenario, func(t *testing.T) { + runner := &recordingRunner{} + owner := newTimer(t, platform, t.TempDir(), "", runner, time.Now()) + if err := owner.Install(context.Background()); err != nil { + t.Fatal(err) + } + contender := *owner + contender.executable = realProgram(t) + if scenario == "other-home" || scenario == "other-home-gone" { + contender.stateRoot = filepath.Join(t.TempDir(), "state") + } + if scenario == "other-home-gone" || scenario == "same-home-gone" { + if err := os.Remove(owner.executable); err != nil { + t.Fatal(err) + } + } + if scenario == "same-home-live-drift" { + f, err := os.OpenFile(owner.primaryPath(), os.O_APPEND|os.O_WRONLY, 0) + if err != nil { + t.Fatal(err) + } + _, err = f.WriteString("\n") + _ = f.Close() + if err != nil { + t.Fatal(err) + } + } + before := timerDefinitions(t, owner) + runner.calls = nil + err := contender.Ensure(context.Background()) + denied := scenario == "other-home" || scenario == "other-home-gone" || scenario == "same-home-live-drift" + if denied && !errors.Is(err, ErrWatchOwned) { + t.Fatalf("want ownership refusal, got %v", err) + } + if !denied && err != nil { + t.Fatal(err) + } + if scenario == "same-home-gone" { + status, err := contender.Status() + if err != nil || !status.Installed || len(runner.calls) == 0 { + t.Fatalf("own stale timer not repaired: %+v %v", status, err) + } + } else { + if len(runner.calls) != 0 || !reflect.DeepEqual(before, timerDefinitions(t, owner)) { + t.Fatal("implicit setup changed another owner or touched its scheduler") + } + } + // Explicit settings still deliberately take ownership. + if err := contender.Install(context.Background()); err != nil { + t.Fatal(err) + } + status, err := contender.Status() + if err != nil || !status.Installed { + t.Fatalf("explicit takeover failed: %+v %v", status, err) + } + }) + } + } +} + +func TestImplicitTimerSetupInstallsOnlyWhenAskedAndRepairKeepsAbsence(t *testing.T) { + for _, platform := range []string{"linux", "darwin"} { + t.Run(platform, func(t *testing.T) { + runner := &recordingRunner{} + timer := newTimer(t, platform, t.TempDir(), "", runner, time.Now()) + drift, err := timer.Repair(context.Background()) + if err != nil || drift.Present || len(runner.calls) != 0 { + t.Fatalf("repair installed an absent timer: %+v %v", drift, err) + } + if err := timer.Ensure(context.Background()); err != nil { + t.Fatal(err) + } + if len(runner.calls) == 0 { + t.Fatal("missing timer was not installed") + } + runner.calls = nil + if err := timer.Ensure(context.Background()); err != nil { + t.Fatal(err) + } + if len(runner.calls) != 0 { + t.Fatal("healthy timer was reinstalled") + } + }) + } +} + +func TestEveryTimerMutationSharesTheCancellableOwnerLock(t *testing.T) { + for _, platform := range []string{"linux", "darwin"} { + t.Run(platform, func(t *testing.T) { + runner := &recordingRunner{} + timer := newTimer(t, platform, t.TempDir(), "", runner, time.Now()) + if err := timer.Install(context.Background()); err != nil { + t.Fatal(err) + } + before := timerDefinitions(t, timer) + runner.calls = nil + lock, err := os.OpenFile(timer.primaryPath()+".lock", os.O_RDWR, 0) + if err != nil { + t.Fatal(err) + } + defer lock.Close() + if err := filelock.Lock(lock, true, false); err != nil { + t.Fatal(err) + } + defer filelock.Unlock(lock) + operations := map[string]func(context.Context) error{"ensure": timer.Ensure, "install": timer.Install, "uninstall": timer.Uninstall, "repair": func(ctx context.Context) error { _, err := timer.Repair(ctx); return err }} + for name, operation := range operations { + ctx, cancel := context.WithTimeout(context.Background(), 20*time.Millisecond) + err := operation(ctx) + cancel() + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("%s bypassed lock: %v", name, err) + } + } + if len(runner.calls) != 0 || !reflect.DeepEqual(before, timerDefinitions(t, timer)) { + t.Fatal("blocked writer touched scheduler or definitions") + } + }) + } +} + +type heldInstallRunner struct { + entered, release chan struct{} + once sync.Once +} + +func (r *heldInstallRunner) Run(ctx context.Context, _ string, _ ...string) error { + r.once.Do(func() { close(r.entered) }) + select { + case <-r.release: + return nil + case <-ctx.Done(): + return ctx.Err() + } +} + +func TestConcurrentProfilesCannotBothImplicitlyClaimTimer(t *testing.T) { + for _, platform := range []string{"linux", "darwin"} { + t.Run(platform, func(t *testing.T) { + runner := &heldInstallRunner{entered: make(chan struct{}), release: make(chan struct{})} + var release sync.Once + defer release.Do(func() { close(runner.release) }) + first := newTimer(t, platform, t.TempDir(), "", runner, time.Now()) + second := *first + second.stateRoot = filepath.Join(t.TempDir(), "second") + second.executable = realProgram(t) + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + a, b := make(chan error, 1), make(chan error, 1) + go func() { a <- first.Ensure(ctx) }() + select { + case <-runner.entered: + case <-ctx.Done(): + t.Fatal("first installer did not start") + } + go func() { b <- second.Ensure(ctx) }() + select { + case err := <-b: + t.Fatalf("second installer bypassed ownership lock: %v", err) + case <-time.After(40 * time.Millisecond): + } + release.Do(func() { close(runner.release) }) + if err := <-a; err != nil { + t.Fatal(err) + } + if err := <-b; !errors.Is(err, ErrWatchOwned) { + t.Fatalf("second profile stole timer: %v", err) + } + status, err := first.Status() + if err != nil || !status.Installed { + t.Fatalf("first owner lost timer: %+v %v", status, err) + } + }) + } +} + +func TestUnsupportedTimerOperationsDoNotCreateHostFiles(t *testing.T) { + dir := t.TempDir() + timer := &Timer{platform: "unsupported", homeDir: dir} + for _, operation := range []func(context.Context) error{timer.Ensure, timer.Install, timer.Uninstall, func(ctx context.Context) error { _, err := timer.Repair(ctx); return err }} { + if err := operation(context.Background()); err == nil { + t.Fatal("unsupported timer accepted") + } + } + entries, err := os.ReadDir(dir) + if err != nil || len(entries) != 0 { + t.Fatalf("unsupported operations touched host: %v %v", entries, err) + } +} From 62ffd71f83999078d0c8d1d2b92fb80396f763ec Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:07:44 -0400 Subject: [PATCH 02/10] docs: record shared timer ownership correction --- docs/changes/unreleased/1622-standing-timer-ownership.md | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 docs/changes/unreleased/1622-standing-timer-ownership.md diff --git a/docs/changes/unreleased/1622-standing-timer-ownership.md b/docs/changes/unreleased/1622-standing-timer-ownership.md new file mode 100644 index 0000000000..a1567e2743 --- /dev/null +++ b/docs/changes/unreleased/1622-standing-timer-ownership.md @@ -0,0 +1,8 @@ +--- +kind: fixed +title: First standing approval preserves the shared timer's owner +pr: 1622 +surface: [engine] +invalidates: + - "Approving the first standing item in a new profile could take another profile's OS timer despite launch-time ownership protection. Implicit setup and repair now check ownership under the same interprocess lock as explicit settings changes. Another owner is retained and a blocked setup is reported honestly." +--- From 66d8120a40cdcb7a6e9a5a69e9164ffb30fc7fd5 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 12:46:51 -0400 Subject: [PATCH 03/10] test: preserve machine gate settings when pinning run models --- cmd/codeaf/do_engine_test.go | 47 ++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 24 deletions(-) diff --git a/cmd/codeaf/do_engine_test.go b/cmd/codeaf/do_engine_test.go index 90df20d0dc..f3384da34d 100644 --- a/cmd/codeaf/do_engine_test.go +++ b/cmd/codeaf/do_engine_test.go @@ -149,6 +149,23 @@ func beltRunEnv(t *testing.T) string { return home } +// beltModelPins preserves the fixture's resource settings while changing the +// seats a test intends to exercise. Replacing the profile wholesale would +// silently re-enable the real machine gate on a busy test host (#1525). +func beltModelPins(t *testing.T, profileDir string, pins map[string]string) { + t.Helper() + settings := config.NewSettings(config.SettingsOptions{ProfileDir: profileDir}) + for key, value := range pins { + row, ok := settings.Row(key) + if !ok { + t.Fatalf("missing setting %s", key) + } + if err := row.Apply(value); err != nil { + t.Fatal(err) + } + } +} + // beltPlandbDoor is the real plandb CLI behind the resolver's override, built // once for the package. THE LOOP ENDS IN THE STORE: a worker's task is done // when `plandb done` marks it so and no other way, so a scripted worker that @@ -376,16 +393,10 @@ func TestDoOnTheRunEngineSeatsEveryLaunchOnTheDoorsModels(t *testing.T) { if err := os.MkdirAll(profileDir, 0o700); err != nil { t.Fatal(err) } - rows, err := json.Marshal(map[string]string{ + beltModelPins(t, profileDir, map[string]string{ config.KeyTierWorkerModel: "vendor/profile-worker", config.KeyTierMastermindModel: "vendor/profile-thinking", }) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(config.BudgetConfigPath(profileDir), rows, 0o600); err != nil { - t.Fatal(err) - } workspace := beltRepoWorkspace(t) const ( @@ -433,7 +444,7 @@ func TestDoOnTheRunEngineSeatsEveryLaunchOnTheDoorsModels(t *testing.T) { } var stdout, stderr strings.Builder - err = doErrand(doRequest{ + err := doErrand(doRequest{ task: "write out.txt and say what you did", workspace: workspace, asJSON: true, timeout: 60 * time.Second, slots: bound(1), model: workModel, planModel: planModel, checkModel: planModel, stdout: &stdout, stderr: &stderr, newBeltCompleter: newBelt, @@ -668,18 +679,12 @@ func TestDoOnTheRunEngineSeatsACheckOnTheCheckModel(t *testing.T) { if err := os.MkdirAll(profileDir, 0o700); err != nil { t.Fatal(err) } - rows, err := json.Marshal(map[string]string{ + beltModelPins(t, profileDir, map[string]string{ config.KeyTierLowModel: "vendor/profile-small", config.KeyTierWorkerModel: "vendor/profile-worker", config.KeyTierHighModel: "vendor/profile-careful", config.KeyTierMastermindModel: "vendor/profile-thinking", }) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(config.BudgetConfigPath(profileDir), rows, 0o600); err != nil { - t.Fatal(err) - } workspace := beltRepoWorkspace(t) const ( @@ -722,7 +727,7 @@ func TestDoOnTheRunEngineSeatsACheckOnTheCheckModel(t *testing.T) { } var stdout, stderr strings.Builder - err = doErrand(doRequest{ + err := doErrand(doRequest{ task: "write out.txt and say what you did", workspace: workspace, asJSON: true, timeout: 60 * time.Second, slots: bound(1), model: workModel, planModel: planModel, checkModel: checkModel, @@ -767,17 +772,11 @@ func TestDoOnTheRunEngineSeatsAnUnpinnedCheckOnTheCrewsChecker(t *testing.T) { if err := os.MkdirAll(profileDir, 0o700); err != nil { t.Fatal(err) } - rows, err := json.Marshal(map[string]string{ + beltModelPins(t, profileDir, map[string]string{ config.KeyTierWorkerModel: "vendor/profile-worker", config.KeyTierHighModel: "vendor/profile-careful", config.KeyTierMastermindModel: "vendor/profile-thinking", }) - if err != nil { - t.Fatal(err) - } - if err := os.WriteFile(config.BudgetConfigPath(profileDir), rows, 0o600); err != nil { - t.Fatal(err) - } workspace := beltRepoWorkspace(t) // The same one-seat shape as the two tests above. @@ -813,7 +812,7 @@ func TestDoOnTheRunEngineSeatsAnUnpinnedCheckOnTheCrewsChecker(t *testing.T) { } var stdout, stderr strings.Builder - err = doErrand(doRequest{ + 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: newBelt, From 0735f8b5ec533a2307f7cef1fd83bb4646357c5f Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:05:57 -0400 Subject: [PATCH 04/10] test(session): await task receipts before asserting claim wait --- internal/session/task_quick_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/session/task_quick_test.go b/internal/session/task_quick_test.go index e605370a19..7c8ebc7ab8 100644 --- a/internal/session/task_quick_test.go +++ b/internal/session/task_quick_test.go @@ -350,6 +350,9 @@ func TestTwoQuickTasksClaimingOneFileRunOneAfterTheOther(t *testing.T) { agent, graph, _ := quickAgent(t, completer) events := mustSubmit(t, agent, "rewrite both sections of notes.md") + // Admission precedes journaling the tool receipt. Finish the parent turn + // while both workers are held before inspecting what the model was told. + collect(t, events) // WHICH OF THE TWO WINS IS THE SCHEDULER'S, and the law is about the pair // rather than about either one: exactly one of them is working, and the @@ -386,7 +389,6 @@ func TestTwoQuickTasksClaimingOneFileRunOneAfterTheOther(t *testing.T) { } close(release) - collect(t, events) waitDoneNode(t, running) waitDoneNode(t, waiting) if state := waiting.stateNow(); state != TaskDone { From 12b0ef75d9bde828a498e46f5c6101420ab4f6d6 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 12:53:23 -0400 Subject: [PATCH 05/10] test(session): join closed program run before fixture cleanup --- internal/session/task_run_orphan_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/internal/session/task_run_orphan_test.go b/internal/session/task_run_orphan_test.go index 6073a1da95..0bf2bdfa8c 100644 --- a/internal/session/task_run_orphan_test.go +++ b/internal/session/task_run_orphan_test.go @@ -210,8 +210,9 @@ func TestClosingUnderAProgramsRunEndsItInItsStoreFirst(t *testing.T) { t.Fatalf("the second run is on root %q with brief %q", spec.Store.RootID(), spec.Store.Task(spec.Store.RootID()).Description) } endBeltRun(t, again, second) - close(first.release) - <-first.finished + // Start returning is not the end of the owner's bookkeeping. Join the + // closed agent's run before the fixture removes its store and workspace. + endBeltRun(t, agent, first) } // THE RESTORE ROAD: a conversation read back from disk whose run row comes From fee04a5a7e212031db97eb32d759abfdbb9136e3 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 12:54:27 -0400 Subject: [PATCH 06/10] test(session): await run completion after its store closes --- internal/session/task_run_belt_test.go | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/internal/session/task_run_belt_test.go b/internal/session/task_run_belt_test.go index cac5f03a75..8a3396dc44 100644 --- a/internal/session/task_run_belt_test.go +++ b/internal/session/task_run_belt_test.go @@ -164,11 +164,22 @@ func registerBeltRunEngine(t *testing.T, engine RunEngine) { // directory's removal, and lost it under load as "directory not empty". func endBeltRun(t *testing.T, agent *Agent, double *beltRunDouble) { t.Helper() + agent.beltMu.Lock() + run := agent.beltRun + agent.beltMu.Unlock() close(double.release) - beltRunWaitFor(t, "the run to end", func() bool { - agent.beltMu.Lock() - defer agent.beltMu.Unlock() - return agent.beltRun == nil + if run == nil { + return + } + // releaseBeltRun clears the agent's pointer before it closes the store. + // The run's completion channel covers that final write/close as well. + beltRunWaitFor(t, "the run and its store to close", func() bool { + select { + case <-run.over: + return true + default: + return false + } }) } From 0a58c334dc1c017e3047f14d6986be706ca5fe93 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:07:21 -0400 Subject: [PATCH 07/10] test(session): synchronize young bash steering without timing window Fixes #1623. --- internal/session/steer_test.go | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/internal/session/steer_test.go b/internal/session/steer_test.go index a58cbd191b..cb237da74d 100644 --- a/internal/session/steer_test.go +++ b/internal/session/steer_test.go @@ -867,13 +867,15 @@ func TestASteerAdoptsAnOldBashAndItsExitArrivesLater(t *testing.T) { func TestASteerWaitsForAYoungBash(t *testing.T) { t.Parallel() + release := filepath.Join(t.TempDir(), "release-young-bash") completer := &scriptedCompleter{steps: []step{ - func(context.Context, []ai.Message) (*ai.Response, error) { - return toolResponse("young-bash", "bash", `{"command":"sleep 0.5; echo young-finished"}`), nil - }, + bashCall("young-bash", "while [ ! -f "+shellQuoted(release)+" ]; do sleep 0.01; done; echo young-finished"), func(context.Context, []ai.Message) (*ai.Response, error) { return textResponse("done"), nil }, }} agent, _ := newTestAgent(t, completer, nil) + // The command stays observable until released; its age is a separate + // input, not a race against a half-second sleep on a busy test machine. + advanceSteerAge(agent, 0) turn := mustSubmit(t, agent, "run the quick check") waitFor(t, "young foreground bash to start", func() bool { return len(agent.inFlightBash.snapshot()) == 1 }) agent.mu.Lock() @@ -882,18 +884,23 @@ func TestASteerWaitsForAYoungBash(t *testing.T) { if generation != nil { t.Fatal("a young bash still had a model generation to cut") } - began := time.Now() steered := mustSteer(t, agent, "then read the result") - collect(t, turn) - events := collect(t, steered) - if elapsed := time.Since(began); elapsed < 200*time.Millisecond { - t.Fatalf("young bash landed in %s, want the batch to finish first", elapsed) + select { + case event := <-steered: + if got := steerLanding([]Event{event}); got != "waiting for the running step" { + t.Fatalf("steer landing while bash is held = %q", got) + } + case <-time.After(10 * time.Second): + t.Fatal("the steer was not accepted while the bash was held") } if list := agent.jobs.list(); list != "No background jobs." { t.Fatalf("young bash became a job: %q", list) } - if got := steerLanding(events); got != "waiting for the running step" { - t.Fatalf("steer landing = %q", got) + writeFile(t, release, "finish now") + collect(t, turn) + collect(t, steered) + if got := roleText(completer.request(1), "tool"); !strings.Contains(got, "young-finished") { + t.Fatalf("the resumed turn did not receive the foreground result: %q", got) } } From dddee8c4ef8f91be704ab158647d707bb00b646a Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:19:38 -0400 Subject: [PATCH 08/10] fix(standing): explain unavailable background checks in approval receipt --- internal/manual/chat/keeping-an-eye.md | 4 +++- internal/session/standing_test.go | 15 +++++++++++++++ internal/session/tools_standing.go | 24 ++++++++++++++++++++---- 3 files changed, 38 insertions(+), 5 deletions(-) diff --git a/internal/manual/chat/keeping-an-eye.md b/internal/manual/chat/keeping-an-eye.md index 35da2338be..3f5ac24988 100644 --- a/internal/manual/chat/keeping-an-eye.md +++ b/internal/manual/chat/keeping-an-eye.md @@ -705,7 +705,9 @@ Approving the first standing item follows the same ownership rule. It installs a missing timer or repairs this home's stale timer, but never takes another home's or live build's timer. If another home owns it, the item is saved and CodeAF says that the background check could not be installed and the existing timer was left -unchanged. This home is not being checked in the background; use the background +unchanged. The confirmation also says the item was saved but needs a codeaf window +open for this home while no background timer checks it. A non-waking permission +rule does not need a timer. This home is not being checked in the background; use the background checks row in `/settings` only when you deliberately want to move the shared timer. Ownership checks and changes are serialized across processes, including that explicit settings action. diff --git a/internal/session/standing_test.go b/internal/session/standing_test.go index 0bdf01e5d4..d5e1624254 100644 --- a/internal/session/standing_test.go +++ b/internal/session/standing_test.go @@ -2023,4 +2023,19 @@ func TestFirstStandingApprovalPreservesAnotherProfilesTimer(t *testing.T) { if len(store.created) != 1 { t.Fatal("timer refusal must not discard the approved standing item") } + if output := toolOutput(t, events, "stand"); !strings.Contains(output, "Background checks are not installed") || !strings.Contains(output, "item was saved") { + t.Fatalf("model receipt hid background unavailability: %q", output) + } + // The first-setup marker suppresses repeated UI notices, not truthful receipts. + events = standRatify(t, store, watch, t.TempDir()) + if output := toolOutput(t, events, "stand"); !strings.Contains(output, "Background checks are not installed") { + t.Fatalf("later receipt claimed background execution: %q", output) + } +} + +func TestNonWakingRuleDoesNotRequireBackgroundTimer(t *testing.T) { + agent := &Agent{config: Config{Standing: &Standing{Watch: &fakeWatch{}}}} + if note := agent.standingBackgroundLimitation(standing.Item{When: standing.When{Kind: standing.WhenHold}}); note != "" { + t.Fatalf("permission rule was said to need a timer: %q", note) + } } diff --git a/internal/session/tools_standing.go b/internal/session/tools_standing.go index 23746e3499..12c9b88950 100644 --- a/internal/session/tools_standing.go +++ b/internal/session/tools_standing.go @@ -520,19 +520,35 @@ func (a *Agent) standPropose(ctx context.Context, parsed standArguments) (string } created = a.standingFileTheExchange(store, created) a.emitStandingUpdate("stood", created, "") - // AND THE FIRST THING THAT EVER STANDS TURNS THE BACKGROUND CHECKS ON. It - // is said to the person and not to the model: the line goes on the screen - // as its own dim row, and the model's whole reply is still the one sentence - // about what now stands ([standingRatifiedLine]). + // Implicit setup reports to the surface once. The tool result also carries + // current availability: saving an item is not a promise that this home + // owns the shared timer, even after the first-setup notice was already sent. a.standingBackgroundOn(store, created) line := fmt.Sprintf("set up %s: %s", created.ID, created.Words) if when := strings.TrimSpace(notice.WhenWords); when != "" { line += "\nit wakes: " + when } line += "\n" + standingRatifiedLine + line += a.standingBackgroundLimitation(created) return line, false, nil } +// Waking items need a truthful timer status in every approval receipt, not just +// the first-setup UI notice. A saved permission rule never needs a timer. +func (a *Agent) standingBackgroundLimitation(item standing.Item) string { + if item.When.Kind == standing.WhenHold || a.config.Standing == nil || a.config.Standing.Watch == nil { + return "" + } + status, err := a.config.Standing.Watch.Status() + if err != nil { + return "\nBackground check status could not be confirmed. Say that the item was saved but do not promise it runs after the window closes." + } + if !status.Installed { + return "\nBackground checks are not installed for this home. Say that the item was saved, but scheduled work needs a codeaf window open for this home; do not promise it runs with the window closed. Do not take another profile's timer or suggest the item failed to save." + } + return "" +} + // standingRatifiedLine is what a model is told the instant something stands, // and it is the WHOLE of what it may say next. // From 7d944ed76a7095846dc50049a17a2e2c0e3e3ad4 Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:20:24 -0400 Subject: [PATCH 09/10] test(session): observe readings in the handover fixture --- internal/session/task_phase_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/internal/session/task_phase_test.go b/internal/session/task_phase_test.go index 67422ef07c..e9e7580d5c 100644 --- a/internal/session/task_phase_test.go +++ b/internal/session/task_phase_test.go @@ -1,7 +1,6 @@ package session import ( - "context" "strings" "testing" "time" @@ -330,7 +329,7 @@ func TestTheHandoverSaysItIsBriefingAWorkerWhileItWritesTheBrief(t *testing.T) { ran := make(ranNodes, 2) stubbedGraph(agent, func(node *TaskNode) { ran <- node }) - events, err := agent.Submit(context.Background(), "work through the four things I listed and report back") + events, err := agent.Submit(watchedContext(agent), "work through the four things I listed and report back") if err != nil { t.Fatalf("Submit: %v", err) } From 7eafed30975b89026f3f699687bb587859d6099b Mon Sep 17 00:00:00 2001 From: santoshkumarradha Date: Sun, 27 Sep 2026 13:20:25 -0400 Subject: [PATCH 10/10] test(session): hold the decision turn until resolution --- internal/session/tools_tasks_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/session/tools_tasks_test.go b/internal/session/tools_tasks_test.go index 7db3f0d701..af133390a6 100644 --- a/internal/session/tools_tasks_test.go +++ b/internal/session/tools_tasks_test.go @@ -572,7 +572,9 @@ func TestHandingAYourCallToTheModelAndItsResolveAreOneRoad(t *testing.T) { // what leaves a your-call landing in the PERSON'S hands: with nobody watching // the model holds every one of them by policy ([Agent.settlePolicy]) and the // card this test is about is never drawn. - agent, _ := newTestAgent(t, &scriptedCompleter{}, func(config *Config) { + // Keep the woken turn open until the test resolves the decision; a finished + // turn correctly hands unresolved work back to the person. + agent, _ := newTestAgent(t, &holdingCompleter{}, func(config *Config) { config.Place, config.AskConsent = Place{Dir: mine}, true }) graph := stubbedGraph(agent, func(node *TaskNode) {