mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 08:18:19 +00:00
fix(cron): reset session for stateless cron jobs, not stateful ones (#1262)
The per-run session reset was gated on `!job.Stateless`, inverting the flag: stateless jobs (the token-saving default) skipped the reset and accumulated unbounded history, while stateful jobs were wiped every run. Reset for `job.Stateless` instead, and clear BOTH session layers — the goclaw session store AND the Claude CLI on-disk .jsonl. claude-cli resumes its own session by a deterministic per-key UUID, so without clearing the .jsonl a "stateless" run still replayed the entire accumulated history (and silently grew it run after run).
This commit is contained in:
1 parent
d8cc9542f6
commit
c0531e1e07
2 files changed
+94
-7
No files matched your search
+24
-7
@@ -25,6 +25,11 @@ import (
|
||||
// Safe because cron jobs only fire after Start(), well after this is set.
|
||||
var cronHeartbeatWakeFn func(agentID string)
|
||||
|
||||
// cronCLISessionReset clears the Claude CLI on-disk session (.jsonl + CLAUDE.md)
|
||||
// for a session key, mirroring the sessions.reset RPC. Indirected through a var
|
||||
// so the stateless-reset behavior can be unit-tested without filesystem effects.
|
||||
var cronCLISessionReset = providers.ResetCLISession
|
||||
|
||||
func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg *config.Config, channelMgr *channels.Manager, sessionMgr store.SessionStore, agentStore store.AgentStore, providerStore store.ProviderStore, providerReg *providers.Registry) func(job *store.CronJob) (*store.CronJobResult, error) {
|
||||
return func(job *store.CronJob) (*store.CronJobResult, error) {
|
||||
agentID := job.AgentID
|
||||
@@ -89,13 +94,25 @@ func makeCronJobHandler(sched *scheduler.Scheduler, msgBus *bus.MessageBus, cfg
|
||||
cronCtx = store.WithCredentialUserID(cronCtx, job.Payload.CredentialUserID)
|
||||
}
|
||||
|
||||
// Reset session before each cron run to prevent tool errors from previous
|
||||
// runs from polluting the context and blocking future executions (#294).
|
||||
// Save() persists the empty session to DB so stale data won't reload after restart.
|
||||
// Stateless jobs skip this — they intentionally carry no session history.
|
||||
if !job.Stateless {
|
||||
sessionMgr.Reset(cronCtx, sessionKey)
|
||||
sessionMgr.Save(cronCtx, sessionKey)
|
||||
// Reset the session before each STATELESS cron run so the run starts fresh:
|
||||
// no carried-over history (the whole point of stateless — saves tokens) and
|
||||
// no tool errors from a previous run polluting the context (#294). Clear BOTH
|
||||
// layers — the goclaw session store AND the Claude CLI's own on-disk session —
|
||||
// because a claude-cli agent resumes its .jsonl by a deterministic per-key
|
||||
// UUID and would otherwise replay the full accumulated history every run
|
||||
// regardless of this flag (i.e. "stateless" had no effect on what the model
|
||||
// actually sees). Stateful jobs (stateless=false) intentionally keep their
|
||||
// session across runs.
|
||||
//
|
||||
// NOTE: this condition was previously `!job.Stateless`, which inverted the
|
||||
// flag — stateless jobs accumulated unbounded history while stateful jobs were
|
||||
// wiped each run.
|
||||
if job.Stateless {
|
||||
if sessionMgr != nil {
|
||||
sessionMgr.Reset(cronCtx, sessionKey)
|
||||
sessionMgr.Save(cronCtx, sessionKey)
|
||||
}
|
||||
cronCLISessionReset("", sessionKey)
|
||||
}
|
||||
|
||||
// Resolve per-job provider/model override (mirrors heartbeat). Unset → agent default.
|
||||
|
||||
@@ -181,3 +181,73 @@ func TestCronJobHandlerSuppressesNoReplyDelivery(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// fakeCronSessionStore records Reset calls. The embedded nil SessionStore
|
||||
// satisfies the interface; the cron handler only calls Reset/Save.
|
||||
type fakeCronSessionStore struct {
|
||||
store.SessionStore
|
||||
resetCount int
|
||||
}
|
||||
|
||||
func (f *fakeCronSessionStore) Reset(context.Context, string) { f.resetCount++ }
|
||||
func (f *fakeCronSessionStore) Save(context.Context, string) error { return nil }
|
||||
|
||||
// A stateless cron run must start fresh by clearing BOTH the goclaw session
|
||||
// store and the Claude CLI on-disk session; a stateful run must keep both.
|
||||
func TestCronJobHandler_StatelessResetsSession(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
stateless bool
|
||||
wantReset bool
|
||||
}{
|
||||
{name: "stateless resets both layers", stateless: true, wantReset: true},
|
||||
{name: "stateful keeps session", stateless: false, wantReset: false},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
var cliResetKeys []string
|
||||
orig := cronCLISessionReset
|
||||
cronCLISessionReset = func(_, key string) { cliResetKeys = append(cliResetKeys, key) }
|
||||
defer func() { cronCLISessionReset = orig }()
|
||||
|
||||
fakeStore := &fakeCronSessionStore{}
|
||||
|
||||
sched := scheduler.NewScheduler(
|
||||
scheduler.DefaultLanes(),
|
||||
scheduler.QueueConfig{
|
||||
Mode: scheduler.QueueModeQueue,
|
||||
Cap: 1,
|
||||
Drop: scheduler.DropOld,
|
||||
DebounceMs: 0,
|
||||
MaxConcurrent: 1,
|
||||
},
|
||||
func(context.Context, agent.RunRequest) (*agent.RunResult, error) {
|
||||
return &agent.RunResult{Content: "ok"}, nil
|
||||
},
|
||||
)
|
||||
defer sched.Stop()
|
||||
|
||||
handler := makeCronJobHandler(sched, nil, &config.Config{}, nil, fakeStore, nil, nil, nil)
|
||||
|
||||
if _, err := handler(&store.CronJob{
|
||||
ID: uuid.NewString(),
|
||||
TenantID: uuid.New(),
|
||||
Name: "j",
|
||||
AgentID: "reporter",
|
||||
UserID: "user-1",
|
||||
Stateless: tc.stateless,
|
||||
Payload: store.CronPayload{Kind: "agent_turn", Message: "m"},
|
||||
}); err != nil {
|
||||
t.Fatalf("cron handler error: %v", err)
|
||||
}
|
||||
|
||||
if gotStore := fakeStore.resetCount > 0; gotStore != tc.wantReset {
|
||||
t.Errorf("session store reset called=%v, want %v", gotStore, tc.wantReset)
|
||||
}
|
||||
if gotCLI := len(cliResetKeys) > 0; gotCLI != tc.wantReset {
|
||||
t.Errorf("CLI session reset called=%v, want %v", gotCLI, tc.wantReset)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user