diff --git a/cmd/codeaf/chatv3_process_test.go b/cmd/codeaf/chatv3_process_test.go index 4fe9cff6d..28f077a07 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 a3e2c0fe7..95d686db3 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 11d2babde..29d90e77a 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 dc2ee90c9..fd9068a28 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 000000000..13faadb84 --- /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.