From 17b1f9c0ba94959182150506f027ca3629911ce1 Mon Sep 17 00:00:00 2001 From: codeaf Date: Thu, 1 Oct 2026 00:14:33 -0400 Subject: [PATCH] chat: a standing firing reaches the window's connected accounts v3StandingPosture built session.Config without Connect, so a firing's session.connect hub was nil and `services`/`use_service` were missing from every standing task even when the account was connected. Live conversations already inject proc.Conns, derived once per process by v3Connect. v3StandingPosture and v3StandingTicker now take the accounts manager as a parameter. The live window's tick loop passes p.Conns, so the firing reaches the very object the surface and every conversation hold and the two never keep separate caches over one connect store. The detached `codeaf tick` has no process to borrow from, so it resolves the profile's manager once before it builds its pass. Nil stays nil: a firing with no manager still carries no account tool rather than one fabricated to fill the gap. Three behavioral tests drive the ticker's Runner probe path: the caller's manager is handed back unchanged, a connected-manager firing reaches `services`, and a nil-manager firing reads "Unknown tool: services". Assisted-by: CodeAF (deepseek-v4.1-flash) Co-Authored-By: CodeAF <267109073+agentfield-bot@users.noreply.github.com> (cherry picked from commit 4c9f0ecae198fd02f297df300501a743b58b5f1b) --- cmd/codeaf/chatv3_process_test.go | 3 +- cmd/codeaf/chatv3_standing.go | 45 +++++-- cmd/codeaf/chatv3_standing_test.go | 127 ++++++++++++++++++ cmd/codeaf/tick.go | 9 +- .../unreleased/1715-v3-standing-accounts.md | 16 +++ 5 files changed, 190 insertions(+), 10 deletions(-) create mode 100644 docs/changes/unreleased/1715-v3-standing-accounts.md diff --git a/cmd/codeaf/chatv3_process_test.go b/cmd/codeaf/chatv3_process_test.go index 4fe9cff6de..28f077a079 100644 --- a/cmd/codeaf/chatv3_process_test.go +++ b/cmd/codeaf/chatv3_process_test.go @@ -11,6 +11,7 @@ import ( "github.com/Agent-Field/codeaf/internal/approval" "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/connect" "github.com/Agent-Field/codeaf/internal/modelsource" "github.com/Agent-Field/codeaf/internal/session" "github.com/Agent-Field/codeaf/internal/standing" @@ -434,7 +435,7 @@ func TestCloseAllCancelsAnInFlightStandingPass(t *testing.T) { sawCancel := make(chan struct{}) var once sync.Once oldPass := standingTickPass - standingTickPass = func(ctx context.Context, _ *standing.Store) { + standingTickPass = func(ctx context.Context, _ *standing.Store, _ *connect.Manager) { once.Do(func() { close(started) }) <-ctx.Done() close(sawCancel) diff --git a/cmd/codeaf/chatv3_standing.go b/cmd/codeaf/chatv3_standing.go index a3e2c0fe73..95d686db3b 100644 --- a/cmd/codeaf/chatv3_standing.go +++ b/cmd/codeaf/chatv3_standing.go @@ -37,6 +37,7 @@ import ( "github.com/Agent-Field/codeaf/internal/catalog" "github.com/Agent-Field/codeaf/internal/config" + "github.com/Agent-Field/codeaf/internal/connect" "github.com/Agent-Field/codeaf/internal/guard" "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/session" @@ -126,7 +127,17 @@ func standingWatch(store *standing.Store) standing.Watch { // over: the posture's model catalog warms in the background and writes a cache // when it lands, and the release cancels and joins it (#1274). It is never nil // when the error is. -func v3StandingTicker(store *standing.Store) (*standing.Ticker, func(), error) { +// +// THE ACCOUNTS MANAGER IS THE CALLER'S AND NEVER ONE BUILT HERE. A live window +// already resolved the process's one manager ([v3Process.Conns]) and a tick it +// runs must reach the same object the conversation's own belt does, or the two +// would hold separate caches and a token one refreshed would be a token the +// other still believed had not moved (chatv3.go's [v3Connect] states the law). +// The detached `codeaf tick` has no process and so resolves the profile's own +// manager before it calls this — one manager in that process, for the one pass +// it makes. Nil is the honest absence the belt reads: no manager means no +// account tools at all, and this must not invent one to fill the gap. +func v3StandingTicker(store *standing.Store, conns *connect.Manager) (*standing.Ticker, func(), error) { if store == nil { return nil, nil, fmt.Errorf("standing: no store") } @@ -134,7 +145,7 @@ func v3StandingTicker(store *standing.Store) (*standing.Ticker, func(), error) { if err != nil { return nil, nil, err } - posture, models, err := v3StandingPosture(settings) + posture, models, err := v3StandingPosture(settings, conns) if err != nil { return nil, nil, err } @@ -178,9 +189,17 @@ func v3MemoryPath(profileDir string) string { // The workspace here is only where the rows are read from. Every firing runs in // its own item's workspace, which the runner sets before it opens anything. // +// conns is the accounts manager the firing's session is given, and it is a +// PARAMETER rather than something resolved here: the live process resolved its +// one manager at the door and the detached tick resolved the profile's before +// it called, and a second manager built in this function would share the store +// with the first while disagreeing about every token either refreshed. Nil is +// left nil — the belt then carries no account tool at all rather than a tool +// that answers "not configured" ([session.Config.Connect]). +// // It answers the lazy catalog it opened as well, which the caller closes when // the pass is over; closing it does not take the rows it has already read. -func v3StandingPosture(settings config.Config) (session.Config, *catalog.Catalog, error) { +func v3StandingPosture(settings config.Config, conns *connect.Manager) (session.Config, *catalog.Catalog, error) { root, err := os.UserHomeDir() if err != nil || root == "" { root = os.TempDir() @@ -216,6 +235,12 @@ func v3StandingPosture(settings config.Config) (session.Config, *catalog.Catalog if err != nil { return session.Config{}, nil, err } + // AND THE ACCOUNTS MANAGER IS THE CALLER'S, wired after governance for the + // reason the live launch wires it there (chatv3.go): governance leaves the + // field empty on purpose, because an account connected on the panel must be + // connected for a firing's belt in the same breath and one manager on one + // store is the only way that is true. + cfg.Connect = conns // The media pair, resolved the way a conversation resolves it (chatv3.go): // a firing briefed to draw a diagram needs the hand that draws it, and the // resolver is what says which model does. The catalog is LAZY and is never @@ -253,7 +278,9 @@ func v3StandingPosture(settings config.Config) (session.Config, *catalog.Catalog var standingTickInterval = standing.Interval // standingTickPass is the seam the ticker calls; tests replace it to hold a -// pass in flight. The default is the real pass. +// pass in flight. The default is the real pass. It carries the process's +// accounts manager so the pass's belt reaches the very one the surface and +// every conversation hold ([v3Process.Conns]). var standingTickPass = runStandingTick func (p *v3Process) startStandingTicks(store *standing.Store) { @@ -285,7 +312,7 @@ func (p *v3Process) startStandingTicks(store *standing.Store) { for { select { case <-ticker.C: - standingTickPass(ctx, store) + standingTickPass(ctx, store, p.Conns) case <-stop: return } @@ -334,13 +361,15 @@ var standingTicks atomic.Bool func standingTicking() bool { return standingTicks.Load() } // runStandingTick is one pass, bounded, with everything it can say written to a -// file. -func runStandingTick(ctx context.Context, store *standing.Store) { +// file. conns is the process's accounts manager, handed straight to the pass so +// a firing's belt holds the same accounts every conversation in this window +// does. +func runStandingTick(ctx context.Context, store *standing.Store, conns *connect.Manager) { // A PANIC HERE MUST NOT END THE TICKING. The loop above is this process's // whole contribution to the ambient side, and a goroutine that unwound out // of it would leave a window that looks like it is keeping watch and is not. defer guard.Recover("standing tick") - pass, release, err := v3StandingTicker(store) + pass, release, err := v3StandingTicker(store, conns) if err != nil { noteStanding("could not start a pass: " + err.Error()) return diff --git a/cmd/codeaf/chatv3_standing_test.go b/cmd/codeaf/chatv3_standing_test.go index 11d2babde4..29d90e77ab 100644 --- a/cmd/codeaf/chatv3_standing_test.go +++ b/cmd/codeaf/chatv3_standing_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" + "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/standing" ) @@ -190,3 +191,129 @@ func TestTheRepairRewritesADefinitionThatNamesADeadPath(t *testing.T) { type quietHost struct{} func (quietHost) Run(context.Context, string, ...string) error { return nil } + +// ── the accounts a firing inherits ────────────────────────────────────────── + +// standingAccountsFixture points the state root and the profile at fresh temp +// directories and silences every key variable, so nothing below touches this +// machine's own accounts and no probe can buy a model call. +func standingAccountsFixture(t *testing.T) string { + t.Helper() + t.Setenv(home.EnvVar, t.TempDir()) + profile := t.TempDir() + t.Setenv(config.ProfileDirEnv, profile) + t.Setenv(config.APIKeyEnv, "") + t.Setenv("OPENAI_API_KEY", "") + return profile +} + +// standingServicesItem is the smallest item whose probe reaches a belt tool: +// asking which accounts are connected. It is exactly the shape the existing +// 15-minute sync fires with, minus the cadence. +func standingServicesItem(t *testing.T) standing.Item { + t.Helper() + return standing.Item{ + Schema: 1, + ID: "test-services", + Words: "which accounts do I have", + Workspace: t.TempDir(), + When: standing.When{ + Kind: standing.WhenProbe, + Probe: standing.Probe{Tool: "services"}, + }, + Does: standing.Action{Kind: standing.ActionSay, Say: "the accounts"}, + Rails: standing.Rails{PerRunUSD: 0.05, MaxPerDay: 3}, + } +} + +// A FIRING'S BELT REACHES THE VERY MANAGER ITS CALLER HOLDS. The live process +// resolved one manager at the door and every conversation's belt reaches it; a +// firing ticked by that same window must too, or the two halves would keep +// separate caches and a token either refreshed would be a token the other still +// believed had not moved. The posture must hand back the caller's object, never +// a second one built over the same store. +func TestStandingPostureUsesTheCallersAccountsManager(t *testing.T) { + profile := standingAccountsFixture(t) + conns := v3Connect(profile) + if conns == nil { + t.Fatal("a fresh profile must still resolve an accounts manager") + } + settings, err := config.LoadKeyless() + if err != nil { + t.Fatal(err) + } + posture, models, err := v3StandingPosture(settings, conns) + if err != nil { + t.Fatal(err) + } + defer models.Close() + if posture.Connect != conns { + t.Fatal("the firing posture built or borrowed a second accounts manager instead of the caller's") + } +} + +// AND THROUGH THE FIRING'S OWN PROBE PATH THE ACCOUNTS TOOLS ARE ACTUALLY +// THERE. The ticker's Runner is what a pass calls after its judgment says yes, +// so a `services` probe that answers the accounts list is the whole behavior +// this fix restores: before it, v3StandingPosture left Connect empty, the hub +// was nil, and the same probe came back "Unknown tool: services". +func TestStandingFiringReachesTheConnectedAccountsTools(t *testing.T) { + profile := standingAccountsFixture(t) + store, err := standing.Open(home.Join("v3", "standing")) + if err != nil { + t.Fatal(err) + } + ticker, release, err := v3StandingTicker(store, v3Connect(profile)) + if err != nil { + t.Fatal(err) + } + defer release() + + out, err := ticker.Runner.Probe(context.Background(), standingServicesItem(t)) + if err != nil { + t.Fatalf("the firing's probe failed: %v", err) + } + if strings.Contains(out, "Unknown tool") { + t.Fatalf("a firing with a connected manager carried no accounts tool: %q", out) + } + if strings.TrimSpace(out) == "" { + t.Fatal("the services probe answered nothing") + } +} + +// AND A FIRING WITH NO MANAGER GETS NO MANAGER INVENTED FOR IT. Nil is the same +// absence the belt reads everywhere: no services tool, no use_service, and a +// probe that names one answered "Unknown tool" rather than a fabricated account +// list. This is the disconnected-profile half of the regression. +func TestStandingFiringInventsNoAccountsManagerWhenThereIsNone(t *testing.T) { + standingAccountsFixture(t) + settings, err := config.LoadKeyless() + if err != nil { + t.Fatal(err) + } + posture, models, err := v3StandingPosture(settings, nil) + if err != nil { + t.Fatal(err) + } + defer models.Close() + if posture.Connect != nil { + t.Fatal("a firing with no accounts manager was handed one anyway") + } + store, err := standing.Open(home.Join("v3", "standing")) + if err != nil { + t.Fatal(err) + } + ticker, release, err := v3StandingTicker(store, nil) + if err != nil { + t.Fatal(err) + } + defer release() + + out, err := ticker.Runner.Probe(context.Background(), standingServicesItem(t)) + if err != nil { + t.Fatalf("the firing's probe failed: %v", err) + } + if !strings.Contains(out, "Unknown tool: services") { + t.Fatalf("a firing with no accounts manager still reached accounts tools: %q", out) + } +} diff --git a/cmd/codeaf/tick.go b/cmd/codeaf/tick.go index dc2ee90c9f..fd9068a28e 100644 --- a/cmd/codeaf/tick.go +++ b/cmd/codeaf/tick.go @@ -20,6 +20,7 @@ import ( "fmt" "os" + "github.com/Agent-Field/codeaf/internal/config" "github.com/Agent-Field/codeaf/internal/home" "github.com/Agent-Field/codeaf/internal/standing" ) @@ -32,7 +33,13 @@ func runTick(args []string) error { if err != nil { return err } - ticker, release, err := v3StandingTicker(store) + // THE PASS RESOLVES THE PROFILE'S OWN ACCOUNTS MANAGER, because this door is + // a whole process of its own: there is no window whose manager it could + // borrow, and a firing the timer starts must reach the same accounts a + // conversation would. A live window passes its existing manager instead + // (chatv3_standing.go's [v3StandingTicker]), so the two paths never hold two + // caches over one store. + ticker, release, err := v3StandingTicker(store, v3Connect(config.ProfileDir())) if err != nil { return err } diff --git a/docs/changes/unreleased/1715-v3-standing-accounts.md b/docs/changes/unreleased/1715-v3-standing-accounts.md new file mode 100644 index 0000000000..13faadb845 --- /dev/null +++ b/docs/changes/unreleased/1715-v3-standing-accounts.md @@ -0,0 +1,16 @@ +--- +kind: fixed +title: a standing firing's belt reaches the accounts its window is connected to +pr: 1715 +surface: [chat] +invalidates: + - "A scheduled firing built through `v3StandingPosture` left `session.Config.Connect` empty, so `services` and `use_service` were missing from every standing task even when the account was connected. The firing now inherits the window's accounts manager, and the detached `codeaf tick` resolves the profile's own." +--- + +A live window already resolves one accounts manager per process, and every +conversation's belt reaches it. The standing tick that window runs now hands the +same manager to its firing posture, so the two never keep separate caches over +one connect store and a token either refreshes is never stale in the other. The +detached `codeaf tick` has no process to borrow from, so it resolves the +profile's manager once for its single pass. Nil stays nil: no manager means no +account tools are fabricated for a disconnected profile.