| 1 | package acp |
| 2 | |
| 3 | import ( |
| 4 | "context" |
| 5 | "encoding/json" |
| 6 | "io" |
| 7 | "os" |
| 8 | "path/filepath" |
| 9 | "strings" |
| 10 | "testing" |
| 11 | |
| 12 | "reasonix/internal/agent" |
| 13 | "reasonix/internal/control" |
| 14 | "reasonix/internal/event" |
| 15 | "reasonix/internal/provider" |
| 16 | ) |
| 17 | |
| 18 | // divergedACPSession writes a transcript to path whose on-disk content has |
| 19 | // diverged from the returned in-memory session, so the next Snapshot on a |
| 20 | // controller holding the stale session hits a conflict and retargets to a |
| 21 | // recovery branch. |
| 22 | func divergedACPSession(t *testing.T, path string) *agent.Session { |
| 23 | t.Helper() |
| 24 | disk := agent.NewSession("sys prompt") |
| 25 | disk.Add(provider.Message{Role: provider.RoleUser, Content: "first"}) |
| 26 | disk.Add(provider.Message{Role: provider.RoleAssistant, Content: "one"}) |
| 27 | disk.Add(provider.Message{Role: provider.RoleUser, Content: "disk second"}) |
| 28 | if err := disk.Save(path); err != nil { |
| 29 | t.Fatalf("save disk session: %v", err) |
| 30 | } |
| 31 | |
| 32 | stale := agent.NewSession("sys prompt") |
| 33 | stale.Add(provider.Message{Role: provider.RoleUser, Content: "first"}) |
| 34 | stale.Add(provider.Message{Role: provider.RoleAssistant, Content: "one"}) |
| 35 | stale.Add(provider.Message{Role: provider.RoleUser, Content: "local second"}) |
| 36 | return stale |
| 37 | } |
| 38 | |
| 39 | // primaryRecoveryFiles filters a recovery-branch glob down to primary session |
| 40 | // transcripts, dropping lifecycle/diagnostic sidecars that the broad |
| 41 | // *-recovery-*.jsonl pattern also matches. |
| 42 | func primaryRecoveryFiles(t *testing.T, dir string) []string { |
| 43 | t.Helper() |
| 44 | matches, err := filepath.Glob(filepath.Join(dir, "*-recovery-*.jsonl")) |
| 45 | if err != nil { |
| 46 | t.Fatalf("glob recovery branches: %v", err) |
| 47 | } |
| 48 | primary := matches[:0] |
| 49 | for _, path := range matches { |
| 50 | base := filepath.Base(path) |
| 51 | if strings.HasSuffix(base, ".events.jsonl") || |
| 52 | strings.HasSuffix(base, ".guardian.jsonl") || |
| 53 | strings.HasSuffix(base, ".turns.jsonl") { |
| 54 | continue |
| 55 | } |
| 56 | primary = append(primary, path) |
| 57 | } |
| 58 | return primary |
| 59 | } |
| 60 | |
| 61 | func assertACPSessionOnRecoveryPath(t *testing.T, sess *acpSession, originalPath, recoveryPath string) { |
| 62 | t.Helper() |
| 63 | if recoveryPath == "" || recoveryPath == originalPath || !strings.Contains(filepath.Base(recoveryPath), "-recovery-") { |
| 64 | t.Fatalf("session path = %q, want recovery path distinct from %q", recoveryPath, originalPath) |
| 65 | } |
| 66 | sess.mu.Lock() |
| 67 | transcript := sess.transcript |
| 68 | lease := sess.lease |
| 69 | sess.mu.Unlock() |
| 70 | if transcript != recoveryPath { |
| 71 | t.Fatalf("session transcript = %q, want recovery path %q", transcript, recoveryPath) |
| 72 | } |
| 73 | if lease == nil || lease.Path() != agent.CanonicalSessionPath(recoveryPath) { |
| 74 | got := "" |
| 75 | if lease != nil { |
| 76 | got = lease.Path() |
| 77 | } |
| 78 | t.Fatalf("session lease path = %q, want recovery path %q", got, recoveryPath) |
| 79 | } |
| 80 | // The original transcript's lease must have been released by the move so |
| 81 | // another runtime can bind it. |
| 82 | orig, err := agent.TryAcquireSessionLease(originalPath) |
| 83 | if err != nil { |
| 84 | t.Fatalf("original transcript lease should be free after recovery move: %v", err) |
| 85 | } |
| 86 | orig.Release() |
| 87 | } |
| 88 | |
| 89 | // TestACPRebuildSessionContinuesRecoveryPathAfterSnapshotConflict is the ACP |
| 90 | // twin of the desktop rebuild fix: when the pre-rebuild Snapshot hits a |
| 91 | // conflict and retargets the old controller to a recovery branch, the session |
| 92 | // bookkeeping must follow at commit time (sessionRecoveredHandler moves |
| 93 | // sess.transcript and the lease), and AdoptHistory must bind the replacement |
| 94 | // controller to that recovery path. A pre-snapshot capture bound the |
| 95 | // just-recovered transcript back to the original file, so every later save |
| 96 | // re-conflicted and derived yet another recovery branch. |
| 97 | func TestACPRebuildSessionContinuesRecoveryPathAfterSnapshotConflict(t *testing.T) { |
| 98 | t.Setenv(agent.SessionLogSchemaEnv, "v1") |
| 99 | dir := t.TempDir() |
| 100 | originalPath := filepath.Join(dir, "acp-switch-conflict.jsonl") |
| 101 | stale := divergedACPSession(t, originalPath) |
| 102 | |
| 103 | sink := newUpdateSink(&fakeNotifier{}, "sess-recovery") |
| 104 | sess := &acpSession{ |
| 105 | id: "sess-recovery", |
| 106 | sink: sink, |
| 107 | cwd: dir, |
| 108 | model: "fast", |
| 109 | transcript: originalPath, |
| 110 | } |
| 111 | lease, err := agent.TryAcquireSessionLease(originalPath) |
| 112 | if err != nil { |
| 113 | t.Fatalf("acquire original session lease: %v", err) |
| 114 | } |
| 115 | sess.lease = lease |
| 116 | t.Cleanup(sess.releaseSessionLease) |
| 117 | t.Cleanup(func() { |
| 118 | if ctrl := sess.currentCtrl(); ctrl != nil { |
| 119 | ctrl.Close() |
| 120 | } |
| 121 | }) |
| 122 | |
| 123 | svc := &service{ |
| 124 | factory: &configurableFactory{dir: dir}, |
| 125 | sessions: map[string]*acpSession{sess.id: sess}, |
| 126 | } |
| 127 | oldCtrl := control.New(control.Options{ |
| 128 | Executor: agent.New(nil, nil, stale, agent.Options{}, event.Discard), |
| 129 | SessionDir: dir, |
| 130 | SessionPath: originalPath, |
| 131 | Label: "fast", |
| 132 | OnSessionRecovered: svc.sessionRecoveredHandler(sess.id), |
| 133 | }) |
| 134 | sess.ctrl = oldCtrl |
| 135 | |
| 136 | if err := svc.rebuildSession(context.Background(), sess, SessionConfigState{Model: "pro"}, []sessionConfigDelta{{axis: "model", model: "pro"}}); err != nil { |
| 137 | t.Fatalf("rebuildSession: %v", err) |
| 138 | } |
| 139 | if sess.ctrl == oldCtrl { |
| 140 | t.Fatal("session controller was not replaced") |
| 141 | } |
| 142 | |
| 143 | recoveryPath := sess.ctrl.SessionPath() |
| 144 | assertACPSessionOnRecoveryPath(t, sess, originalPath, recoveryPath) |
| 145 | |
| 146 | // The rebuilt controller adopted the recovery file's baseline, so its next |
| 147 | // snapshot must not derive a second recovery branch. |
| 148 | if err := sess.ctrl.Snapshot(); err != nil { |
| 149 | t.Fatalf("Snapshot after rebuild: %v", err) |
| 150 | } |
| 151 | if primary := primaryRecoveryFiles(t, dir); len(primary) != 1 || primary[0] != recoveryPath { |
| 152 | t.Fatalf("recovery branches after follow-up snapshot = %v, want only %q", primary, recoveryPath) |
| 153 | } |
| 154 | } |
| 155 | |
| 156 | // TestACPPersistAfterTurnMovesBookkeepingToRecoveryPath covers the autosave |
| 157 | // path: a turn-end Snapshot in persistAfterTurn that recovers onto a recovery |
| 158 | // branch must move sess.transcript and the session lease with the controller, |
| 159 | // so session/prompt reports the live file, session/delete destroys it, and the |
| 160 | // recovery transcript stays lease-guarded against other runtimes. |
| 161 | func TestACPPersistAfterTurnMovesBookkeepingToRecoveryPath(t *testing.T) { |
| 162 | t.Setenv(agent.SessionLogSchemaEnv, "v1") |
| 163 | dir := t.TempDir() |
| 164 | originalPath := filepath.Join(dir, "acp-autosave-conflict.jsonl") |
| 165 | stale := divergedACPSession(t, originalPath) |
| 166 | |
| 167 | sink := newUpdateSink(&fakeNotifier{}, "sess-autosave") |
| 168 | sess := &acpSession{ |
| 169 | id: "sess-autosave", |
| 170 | sink: sink, |
| 171 | cwd: dir, |
| 172 | model: "fast", |
| 173 | transcript: originalPath, |
| 174 | } |
| 175 | lease, err := agent.TryAcquireSessionLease(originalPath) |
| 176 | if err != nil { |
| 177 | t.Fatalf("acquire original session lease: %v", err) |
| 178 | } |
| 179 | sess.lease = lease |
| 180 | t.Cleanup(sess.releaseSessionLease) |
| 181 | |
| 182 | svc := &service{ |
| 183 | factory: &configurableFactory{dir: dir}, |
| 184 | sessions: map[string]*acpSession{sess.id: sess}, |
| 185 | } |
| 186 | ctrl := control.New(control.Options{ |
| 187 | Executor: agent.New(nil, nil, stale, agent.Options{}, event.Discard), |
| 188 | SessionDir: dir, |
| 189 | SessionPath: originalPath, |
| 190 | Label: "fast", |
| 191 | OnSessionRecovered: svc.sessionRecoveredHandler(sess.id), |
| 192 | }) |
| 193 | sess.ctrl = ctrl |
| 194 | t.Cleanup(ctrl.Close) |
| 195 | |
| 196 | sess.persistAfterTurn("hello") |
| 197 | |
| 198 | recoveryPath := ctrl.SessionPath() |
| 199 | assertACPSessionOnRecoveryPath(t, sess, originalPath, recoveryPath) |
| 200 | if primary := primaryRecoveryFiles(t, dir); len(primary) != 1 || primary[0] != recoveryPath { |
| 201 | t.Fatalf("recovery branches after autosave = %v, want only %q", primary, recoveryPath) |
| 202 | } |
| 203 | // The next turn-end autosave writes the recovery file the session now |
| 204 | // owns; it must not derive a second recovery branch. |
| 205 | sess.persistAfterTurn("again") |
| 206 | if got := ctrl.SessionPath(); got != recoveryPath { |
| 207 | t.Fatalf("controller session path after second autosave = %q, want %q", got, recoveryPath) |
| 208 | } |
| 209 | if primary := primaryRecoveryFiles(t, dir); len(primary) != 1 || primary[0] != recoveryPath { |
| 210 | t.Fatalf("recovery branches after second autosave = %v, want only %q", primary, recoveryPath) |
| 211 | } |
| 212 | } |
| 213 | |
| 214 | // recoverACPSessionAndRestart drives an autosave recovery for session id in |
| 215 | // dir, then simulates a process restart: the live session's lease is released, |
| 216 | // its controller closed, and a fresh service (empty session registry, same |
| 217 | // session dir) is returned alongside the original and recovery paths. |
| 218 | func recoverACPSessionAndRestart(t *testing.T, dir, id string) (originalPath, recoveryPath string, restarted *service) { |
| 219 | t.Helper() |
| 220 | originalPath = transcriptPath(dir, id) |
| 221 | stale := divergedACPSession(t, originalPath) |
| 222 | |
| 223 | svc := &service{ |
| 224 | factory: &configurableFactory{dir: dir}, |
| 225 | sessions: map[string]*acpSession{}, |
| 226 | } |
| 227 | sess := &acpSession{ |
| 228 | id: id, |
| 229 | sink: newUpdateSink(&fakeNotifier{}, id), |
| 230 | cwd: dir, |
| 231 | model: "fast", |
| 232 | title: "recovered title", |
| 233 | transcript: originalPath, |
| 234 | } |
| 235 | lease, err := agent.TryAcquireSessionLease(originalPath) |
| 236 | if err != nil { |
| 237 | t.Fatalf("acquire original session lease: %v", err) |
| 238 | } |
| 239 | sess.lease = lease |
| 240 | svc.sessions[id] = sess |
| 241 | ctrl := control.New(control.Options{ |
| 242 | Executor: agent.New(nil, nil, stale, agent.Options{}, event.Discard), |
| 243 | SessionDir: dir, |
| 244 | SessionPath: originalPath, |
| 245 | Label: "fast", |
| 246 | OnSessionRecovered: svc.sessionRecoveredHandler(id), |
| 247 | }) |
| 248 | sess.ctrl = ctrl |
| 249 | |
| 250 | sess.persistAfterTurn("hello") |
| 251 | recoveryPath = ctrl.SessionPath() |
| 252 | assertACPSessionOnRecoveryPath(t, sess, originalPath, recoveryPath) |
| 253 | |
| 254 | sess.releaseSessionLease() |
| 255 | ctrl.Close() |
| 256 | restarted = &service{ |
| 257 | conn: NewConn(strings.NewReader(""), io.Discard), |
| 258 | factory: &configurableFactory{dir: dir}, |
| 259 | sessions: map[string]*acpSession{}, |
| 260 | } |
| 261 | return originalPath, recoveryPath, restarted |
| 262 | } |
| 263 | |
| 264 | // TestACPLoadAfterRestartFollowsRecoveryTranscript covers the restart half of |
| 265 | // the recovery move: session/load and session/resume resolve the session id to |
| 266 | // the transcript the session actually lives in. Without the id-keyed redirect, |
| 267 | // a restart reopened the pre-recovery file and the user's recovered work |
| 268 | // silently vanished from ACP's view. |
| 269 | func TestACPLoadAfterRestartFollowsRecoveryTranscript(t *testing.T) { |
| 270 | t.Setenv(agent.SessionLogSchemaEnv, "v1") |
| 271 | dir := t.TempDir() |
| 272 | id := "sess-restart" |
| 273 | originalPath, recoveryPath, svc := recoverACPSessionAndRestart(t, dir, id) |
| 274 | |
| 275 | if _, err := svc.openExistingSession(context.Background(), "session/load", id, dir, nil, false); err != nil { |
| 276 | t.Fatalf("openExistingSession after restart: %v", err) |
| 277 | } |
| 278 | loaded := svc.session(id) |
| 279 | if loaded == nil { |
| 280 | t.Fatal("session not registered after load") |
| 281 | } |
| 282 | t.Cleanup(func() { |
| 283 | loaded.releaseSessionLease() |
| 284 | loaded.ctrl.Close() |
| 285 | }) |
| 286 | assertACPSessionOnRecoveryPath(t, loaded, originalPath, recoveryPath) |
| 287 | if got := loaded.ctrl.SessionPath(); got != recoveryPath { |
| 288 | t.Fatalf("loaded controller session path = %q, want recovery path %q", got, recoveryPath) |
| 289 | } |
| 290 | // The test factory's controller has no executor, so prove the content via |
| 291 | // the transcript ACP now points at: it must hold the recovered local line, |
| 292 | // not the pre-recovery disk line. |
| 293 | resumed, err := agent.LoadSession(loaded.transcript) |
| 294 | if err != nil { |
| 295 | t.Fatalf("load resolved transcript: %v", err) |
| 296 | } |
| 297 | msgs := resumed.Snapshot() |
| 298 | if len(msgs) == 0 { |
| 299 | t.Fatal("resolved transcript is empty") |
| 300 | } |
| 301 | if got := msgs[len(msgs)-1].Content; got != "local second" { |
| 302 | t.Fatalf("resolved transcript last message = %q, want recovered local transcript (%q)", got, "local second") |
| 303 | } |
| 304 | } |
| 305 | |
| 306 | // TestACPLoadAfterRestartFollowsIntentionalBranch verifies that the same |
| 307 | // restart redirect used for conflict recovery is written when an ACP session |
| 308 | // intentionally changes paths. Without it, the live process owns the branch |
| 309 | // correctly, but session/load after restart falls back to the stale id-keyed |
| 310 | // parent transcript. |
| 311 | func TestACPLoadAfterRestartFollowsIntentionalBranch(t *testing.T) { |
| 312 | dir := schemaOneTempDir(t) |
| 313 | id := "sess-branch-restart" |
| 314 | originalPath := transcriptPath(dir, id) |
| 315 | original := agent.NewSession("sys prompt") |
| 316 | original.Add(provider.Message{Role: provider.RoleUser, Content: "parent"}) |
| 317 | if err := original.Save(originalPath); err != nil { |
| 318 | t.Fatalf("save original session: %v", err) |
| 319 | } |
| 320 | loaded, err := agent.LoadSession(originalPath) |
| 321 | if err != nil { |
| 322 | t.Fatalf("load original session: %v", err) |
| 323 | } |
| 324 | |
| 325 | svc := &service{ |
| 326 | factory: &configurableFactory{dir: dir}, |
| 327 | sessions: map[string]*acpSession{}, |
| 328 | } |
| 329 | sess := &acpSession{ |
| 330 | id: id, |
| 331 | sink: newUpdateSink(&fakeNotifier{}, id), |
| 332 | cwd: dir, |
| 333 | model: "fast", |
| 334 | transcript: originalPath, |
| 335 | } |
| 336 | lease, err := agent.TryAcquireSessionLease(originalPath) |
| 337 | if err != nil { |
| 338 | t.Fatalf("acquire original session lease: %v", err) |
| 339 | } |
| 340 | sess.lease = lease |
| 341 | svc.sessions[id] = sess |
| 342 | ctrl := control.New(control.Options{ |
| 343 | Executor: agent.New(nil, nil, loaded, agent.Options{}, event.Discard), |
| 344 | SessionDir: dir, |
| 345 | SessionPath: originalPath, |
| 346 | Label: "fast", |
| 347 | OnSessionTransition: svc.sessionTransitionHandler(id), |
| 348 | }) |
| 349 | sess.ctrl = ctrl |
| 350 | if err := bindACPWriteAuthority(ctrl, lease); err != nil { |
| 351 | t.Fatalf("bind original authority: %v", err) |
| 352 | } |
| 353 | |
| 354 | branchPath, err := ctrl.Branch("restart target") |
| 355 | if err != nil { |
| 356 | t.Fatalf("branch session: %v", err) |
| 357 | } |
| 358 | if branchPath == originalPath { |
| 359 | t.Fatalf("branch path = original path %q", originalPath) |
| 360 | } |
| 361 | sess.mu.Lock() |
| 362 | activePath := sess.transcript |
| 363 | activeLease := sess.lease |
| 364 | sess.mu.Unlock() |
| 365 | if activePath != branchPath { |
| 366 | t.Fatalf("ACP transcript = %q, want branch %q", activePath, branchPath) |
| 367 | } |
| 368 | if activeLease == nil || activeLease.Path() != agent.CanonicalSessionPath(branchPath) { |
| 369 | t.Fatalf("ACP lease does not cover branch %q", branchPath) |
| 370 | } |
| 371 | |
| 372 | sess.releaseSessionLease() |
| 373 | ctrl.Close() |
| 374 | if got := resolveTranscriptPath(dir, id); got != branchPath { |
| 375 | t.Fatalf("restart transcript = %q, want branch %q", got, branchPath) |
| 376 | } |
| 377 | |
| 378 | restarted := &service{ |
| 379 | conn: NewConn(strings.NewReader(""), io.Discard), |
| 380 | factory: &configurableFactory{dir: dir}, |
| 381 | sessions: map[string]*acpSession{}, |
| 382 | } |
| 383 | if _, err := restarted.openExistingSession(context.Background(), "session/load", id, dir, nil, false); err != nil { |
| 384 | t.Fatalf("open intentional branch after restart: %v", err) |
| 385 | } |
| 386 | reloaded := restarted.session(id) |
| 387 | if reloaded == nil { |
| 388 | t.Fatal("session not registered after restart") |
| 389 | } |
| 390 | t.Cleanup(func() { |
| 391 | reloaded.releaseSessionLease() |
| 392 | reloaded.ctrl.Close() |
| 393 | }) |
| 394 | if reloaded.transcript != branchPath || reloaded.ctrl.SessionPath() != branchPath { |
| 395 | t.Fatalf("reloaded paths = transcript %q, controller %q; want %q", reloaded.transcript, reloaded.ctrl.SessionPath(), branchPath) |
| 396 | } |
| 397 | } |
| 398 | |
| 399 | // TestACPDeleteAfterRestartRemovesRecoveryAndIDKeyedFiles: session/delete on a |
| 400 | // non-live recovered session must remove both the recovery transcript (the |
| 401 | // session's live file) and the id-keyed original, or the survivor resurfaces |
| 402 | // in session/list as a ghost that can never be deleted by id. |
| 403 | func TestACPDeleteAfterRestartRemovesRecoveryAndIDKeyedFiles(t *testing.T) { |
| 404 | t.Setenv(agent.SessionLogSchemaEnv, "v1") |
| 405 | dir := t.TempDir() |
| 406 | id := "sess-del" |
| 407 | originalPath, recoveryPath, svc := recoverACPSessionAndRestart(t, dir, id) |
| 408 | |
| 409 | raw, err := json.Marshal(SessionDeleteParams{SessionID: id}) |
| 410 | if err != nil { |
| 411 | t.Fatalf("marshal delete params: %v", err) |
| 412 | } |
| 413 | if _, err := svc.sessionDelete(context.Background(), raw); err != nil { |
| 414 | t.Fatalf("sessionDelete after restart: %v", err) |
| 415 | } |
| 416 | for _, path := range []string{originalPath, recoveryPath, acpMetaPath(originalPath), acpMetaPath(recoveryPath)} { |
| 417 | if _, err := os.Stat(path); !os.IsNotExist(err) { |
| 418 | t.Fatalf("%s should be removed by session/delete, stat err = %v", path, err) |
| 419 | } |
| 420 | } |
| 421 | res, err := svc.sessionList(context.Background(), nil) |
| 422 | if err != nil { |
| 423 | t.Fatalf("sessionList after delete: %v", err) |
| 424 | } |
| 425 | if sessions := res.(SessionListResult).Sessions; len(sessions) != 0 { |
| 426 | t.Fatalf("session list after delete = %#v, want empty", sessions) |
| 427 | } |
| 428 | } |
| 429 | |
| 430 | // TestACPSessionListAfterRecoveryShowsSingleActiveEntry: after a recovery the |
| 431 | // id-keyed sidecar becomes a redirect, and session/list must present exactly |
| 432 | // one entry for the id, backed by the active recovery transcript's metadata |
| 433 | // (the live title), never the stale pre-recovery sidecar. |
| 434 | func TestACPSessionListAfterRecoveryShowsSingleActiveEntry(t *testing.T) { |
| 435 | t.Setenv(agent.SessionLogSchemaEnv, "v1") |
| 436 | dir := t.TempDir() |
| 437 | id := "sess-list" |
| 438 | _, _, svc := recoverACPSessionAndRestart(t, dir, id) |
| 439 | |
| 440 | res, err := svc.sessionList(context.Background(), nil) |
| 441 | if err != nil { |
| 442 | t.Fatalf("sessionList after recovery: %v", err) |
| 443 | } |
| 444 | sessions := res.(SessionListResult).Sessions |
| 445 | if len(sessions) != 1 { |
| 446 | t.Fatalf("session list after recovery = %#v, want exactly one entry", sessions) |
| 447 | } |
| 448 | if sessions[0].SessionID != id { |
| 449 | t.Fatalf("session list entry id = %q, want %q", sessions[0].SessionID, id) |
| 450 | } |
| 451 | if sessions[0].Title != "recovered title" { |
| 452 | t.Fatalf("session list entry title = %q, want the active transcript's title %q", sessions[0].Title, "recovered title") |
| 453 | } |
| 454 | } |
| 455 | |
| 456 | // modelSystemPromptFactory builds controllers whose leading system message |
| 457 | // encodes the requested model, so a rebuild test can check that AdoptHistory |
| 458 | // splices in the replacement contract instead of carrying the outgoing one. |
| 459 | type modelSystemPromptFactory struct { |
| 460 | dir string |
| 461 | } |
| 462 | |
| 463 | func (f *modelSystemPromptFactory) NewSession(_ context.Context, p SessionParams) (*control.Controller, error) { |
| 464 | prompt := "system prompt for model " + p.Model |
| 465 | exec := agent.New(nil, nil, agent.NewSession(prompt), agent.Options{}, event.Discard) |
| 466 | return control.New(control.Options{Executor: exec, SessionDir: f.dir, Label: p.Model}), nil |
| 467 | } |
| 468 | |
| 469 | func (f *modelSystemPromptFactory) SessionDir() string { return f.dir } |
| 470 | |
| 471 | // TestACPRebuildSessionRefreshesLeadingSystemPromptForNewModel pins the fix |
| 472 | // for the bug where a model switch rebuilt the controller with the target |
| 473 | // model's own system prompt, only for AdoptHistory to immediately overwrite |
| 474 | // it with the carried history's leading message — the outgoing model's |
| 475 | // contract. |
| 476 | func TestACPRebuildSessionRefreshesLeadingSystemPromptForNewModel(t *testing.T) { |
| 477 | dir := t.TempDir() |
| 478 | path := filepath.Join(dir, "acp-model-switch.jsonl") |
| 479 | |
| 480 | oldSession := agent.NewSession("system prompt for model fast") |
| 481 | oldSession.Add(provider.Message{Role: provider.RoleUser, Content: "hello"}) |
| 482 | oldSession.Add(provider.Message{Role: provider.RoleAssistant, Content: "hi"}) |
| 483 | if err := oldSession.Save(path); err != nil { |
| 484 | t.Fatalf("save base session: %v", err) |
| 485 | } |
| 486 | |
| 487 | sink := newUpdateSink(&fakeNotifier{}, "sess-model-switch") |
| 488 | sess := &acpSession{ |
| 489 | id: "sess-model-switch", |
| 490 | sink: sink, |
| 491 | cwd: dir, |
| 492 | model: "fast", |
| 493 | runtimeProfile: "balanced", |
| 494 | transcript: path, |
| 495 | } |
| 496 | lease, err := agent.TryAcquireSessionLease(path) |
| 497 | if err != nil { |
| 498 | t.Fatalf("acquire session lease: %v", err) |
| 499 | } |
| 500 | sess.lease = lease |
| 501 | t.Cleanup(sess.releaseSessionLease) |
| 502 | t.Cleanup(func() { |
| 503 | if ctrl := sess.currentCtrl(); ctrl != nil { |
| 504 | ctrl.Close() |
| 505 | } |
| 506 | }) |
| 507 | |
| 508 | sess.ctrl = control.New(control.Options{ |
| 509 | Executor: agent.New(nil, nil, oldSession, agent.Options{}, event.Discard), |
| 510 | SessionDir: dir, |
| 511 | SessionPath: path, |
| 512 | Label: "fast", |
| 513 | }) |
| 514 | svc := &service{ |
| 515 | factory: &modelSystemPromptFactory{dir: dir}, |
| 516 | sessions: map[string]*acpSession{sess.id: sess}, |
| 517 | } |
| 518 | |
| 519 | deltas := []sessionConfigDelta{{axis: "model", model: "pro"}} |
| 520 | if err := svc.rebuildSession(context.Background(), sess, SessionConfigState{Model: "pro"}, deltas); err != nil { |
| 521 | t.Fatalf("rebuildSession: %v", err) |
| 522 | } |
| 523 | |
| 524 | history := sess.currentCtrl().History() |
| 525 | if len(history) == 0 || history[0].Role != provider.RoleSystem { |
| 526 | t.Fatalf("history = %+v, want a leading system message", history) |
| 527 | } |
| 528 | if got, want := history[0].Content, "system prompt for model pro"; got != want { |
| 529 | t.Fatalf("leading system message = %q, want %q (stale outgoing-model contract carried forward)", got, want) |
| 530 | } |
| 531 | if len(history) != 3 || history[1].Content != "hello" || history[2].Content != "hi" { |
| 532 | t.Fatalf("history after model switch = %+v, want carried user/assistant turns preserved", history) |
| 533 | } |
| 534 | } |
| 535 | |
| 536 | // TestACPModelSwitchPersistsRefreshedSystemPromptAcrossReload pins the disk |
| 537 | // half of the rebuild-prompt fix: the refreshed leading system prompt must be |
| 538 | // persisted at switch time, because session/close never snapshots and |
| 539 | // session/load resumes the transcript exactly as saved. Before the fix the |
| 540 | // switch refreshed only the new controller's in-memory history, so a |
| 541 | // switch → close → load sequence revived the outgoing model's contract even |
| 542 | // though the session metadata already claimed the new model. |
| 543 | func TestACPModelSwitchPersistsRefreshedSystemPromptAcrossReload(t *testing.T) { |
| 544 | dir := t.TempDir() |
| 545 | id := "sess-model-persist" |
| 546 | path := transcriptPath(dir, id) |
| 547 | |
| 548 | oldSession := agent.NewSession("system prompt for model fast") |
| 549 | oldSession.Add(provider.Message{Role: provider.RoleUser, Content: "hello"}) |
| 550 | oldSession.Add(provider.Message{Role: provider.RoleAssistant, Content: "hi"}) |
| 551 | if err := oldSession.Save(path); err != nil { |
| 552 | t.Fatalf("save base session: %v", err) |
| 553 | } |
| 554 | |
| 555 | sink := newUpdateSink(&fakeNotifier{}, id) |
| 556 | sess := &acpSession{ |
| 557 | id: id, |
| 558 | sink: sink, |
| 559 | cwd: dir, |
| 560 | model: "fast", |
| 561 | runtimeProfile: "balanced", |
| 562 | transcript: path, |
| 563 | } |
| 564 | lease, err := agent.TryAcquireSessionLease(path) |
| 565 | if err != nil { |
| 566 | t.Fatalf("acquire session lease: %v", err) |
| 567 | } |
| 568 | sess.lease = lease |
| 569 | sess.ctrl = control.New(control.Options{ |
| 570 | Executor: agent.New(nil, nil, oldSession, agent.Options{}, event.Discard), |
| 571 | SessionDir: dir, |
| 572 | SessionPath: path, |
| 573 | Label: "fast", |
| 574 | }) |
| 575 | svc := &service{ |
| 576 | conn: NewConn(strings.NewReader(""), io.Discard), |
| 577 | factory: &modelSystemPromptFactory{dir: dir}, |
| 578 | sessions: map[string]*acpSession{sess.id: sess}, |
| 579 | } |
| 580 | |
| 581 | deltas := []sessionConfigDelta{{axis: "model", model: "pro"}} |
| 582 | if err := svc.rebuildSession(context.Background(), sess, SessionConfigState{Model: "pro"}, deltas); err != nil { |
| 583 | t.Fatalf("rebuildSession: %v", err) |
| 584 | } |
| 585 | |
| 586 | // The refreshed contract must be on disk as soon as the switch lands. |
| 587 | onDisk, err := agent.LoadSession(path) |
| 588 | if err != nil { |
| 589 | t.Fatalf("load transcript after switch: %v", err) |
| 590 | } |
| 591 | if msgs := onDisk.Snapshot(); len(msgs) == 0 || msgs[0].Content != "system prompt for model pro" { |
| 592 | t.Fatalf("on-disk leading message after switch = %+v, want the pro model contract", msgs) |
| 593 | } |
| 594 | |
| 595 | raw, err := json.Marshal(SessionCloseParams{SessionID: id}) |
| 596 | if err != nil { |
| 597 | t.Fatalf("marshal close params: %v", err) |
| 598 | } |
| 599 | if _, err := svc.sessionClose(context.Background(), raw); err != nil { |
| 600 | t.Fatalf("sessionClose: %v", err) |
| 601 | } |
| 602 | |
| 603 | if _, err := svc.openExistingSession(context.Background(), "session/load", id, dir, nil, false); err != nil { |
| 604 | t.Fatalf("openExistingSession after close: %v", err) |
| 605 | } |
| 606 | loaded := svc.session(id) |
| 607 | if loaded == nil { |
| 608 | t.Fatal("session not registered after load") |
| 609 | } |
| 610 | t.Cleanup(func() { |
| 611 | loaded.releaseSessionLease() |
| 612 | loaded.currentCtrl().Close() |
| 613 | }) |
| 614 | |
| 615 | history := loaded.currentCtrl().History() |
| 616 | if len(history) == 0 || history[0].Role != provider.RoleSystem { |
| 617 | t.Fatalf("loaded history = %+v, want a leading system message", history) |
| 618 | } |
| 619 | if got, want := history[0].Content, "system prompt for model pro"; got != want { |
| 620 | t.Fatalf("leading system prompt after switch → close → load = %q, want %q (stale outgoing-model contract revived from disk)", got, want) |
| 621 | } |
| 622 | if len(history) != 3 || history[1].Content != "hello" || history[2].Content != "hi" { |
| 623 | t.Fatalf("loaded history = %+v, want carried user/assistant turns preserved", history) |
| 624 | } |
| 625 | } |
| 626 | |
| 627 | // TestACPModelSwitchSnapshotFailureKeepsOutgoingController proves failure |
| 628 | // atomicity for the switch-time persistence step. If the refreshed history |
| 629 | // cannot be written, the service must return an error and leave the outgoing |
| 630 | // controller/config active instead of publishing an in-memory-only switch. |
| 631 | func TestACPModelSwitchSnapshotFailureKeepsOutgoingController(t *testing.T) { |
| 632 | dir := t.TempDir() |
| 633 | invalidPath := filepath.Join(dir, "transcript-is-a-directory") |
| 634 | if err := os.Mkdir(invalidPath, 0o755); err != nil { |
| 635 | t.Fatalf("mkdir invalid transcript path: %v", err) |
| 636 | } |
| 637 | |
| 638 | oldSession := agent.NewSession("system prompt for model fast") |
| 639 | oldSession.Add(provider.Message{Role: provider.RoleUser, Content: "hello"}) |
| 640 | oldSession.Add(provider.Message{Role: provider.RoleAssistant, Content: "hi"}) |
| 641 | oldCtrl := &snapshotLockProbeController{Controller: control.New(control.Options{ |
| 642 | Executor: agent.New(nil, nil, oldSession, agent.Options{}, event.Discard), |
| 643 | SessionDir: dir, |
| 644 | SessionPath: invalidPath, |
| 645 | Label: "fast", |
| 646 | })} |
| 647 | t.Cleanup(oldCtrl.Close) |
| 648 | |
| 649 | sess := &acpSession{ |
| 650 | id: "sess-model-persist-failure", |
| 651 | ctrl: oldCtrl, |
| 652 | sink: newUpdateSink(&fakeNotifier{}, "sess-model-persist-failure"), |
| 653 | cwd: dir, |
| 654 | model: "fast", |
| 655 | runtimeProfile: "balanced", |
| 656 | transcript: invalidPath, |
| 657 | } |
| 658 | svc := &service{ |
| 659 | factory: &modelSystemPromptFactory{dir: dir}, |
| 660 | sessions: map[string]*acpSession{sess.id: sess}, |
| 661 | } |
| 662 | |
| 663 | deltas := []sessionConfigDelta{{axis: "model", model: "pro"}} |
| 664 | err := svc.rebuildSession(context.Background(), sess, SessionConfigState{Model: "pro"}, deltas) |
| 665 | if err == nil || !strings.Contains(err.Error(), "snapshot after switch") { |
| 666 | t.Fatalf("rebuildSession error = %v, want snapshot after switch failure", err) |
| 667 | } |
| 668 | if got := sess.currentCtrl(); got != oldCtrl { |
| 669 | t.Fatalf("controller changed after persistence failure: got %T %p, want outgoing %p", got, got, oldCtrl) |
| 670 | } |
| 671 | if got := sess.model; got != "fast" { |
| 672 | t.Fatalf("model = %q, want outgoing fast after persistence failure", got) |
| 673 | } |
| 674 | } |
| 675 |