fix(session): preserve concurrent toggle targets
- commit
- b910459510f8bc4daf89b911e6ca3ac7dd393cfd
- author
- brice · · 2026-07-11 11:53 UTC
- committer
- brice · · 2026-07-11 11:54 UTC
- parent
- 74cc71c
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
4 files · +115 -9
M README.md +1 -2
| @@ -108,7 +108,6 @@ code.focus-pane-down = FPD | |||
| 108 | 108 | code.next-tab = NXT | |
| 109 | 109 | code.previous-tab = PVT | |
| 110 | 110 | code.back-session = BSK | |
| 111 | - | code.forward-session = FSK | |
| 112 | 111 | code.session-picker = SSP | |
| 113 | 112 | code.visual-mode = VIS | |
| 114 | 113 | code.toggle-floating-pane = FLT | |
| @@ -193,7 +192,7 @@ Successful commands are promoted near the top the next time the palette opens. | |||
| 193 | 192 | | CLP | close focused pane | | |
| 194 | 193 | | FPL / FPR / FPU / FPD | focus pane left / right / up / down | | |
| 195 | 194 | | NXT / PVT | switch to next / previous tab | | |
| 196 | - | | BSK / FSK | move back / forward through recent sessions | | |
| 195 | + | | BSK | toggle the previously active session | | |
| 197 | 196 | | SSP | open session picker | | |
| 198 | 197 | | VIS | enter visual mode | | |
| 199 | 198 | | FLT | show or hide the current tab's floating terminal | | |
| @@ -108,7 +108,6 @@ code.focus-pane-down = FPD | |||||
| 108 | code.next-tab = NXT | 108 | code.next-tab = NXT | ||
| 109 | code.previous-tab = PVT | 109 | code.previous-tab = PVT | ||
| 110 | code.back-session = BSK | 110 | code.back-session = BSK | ||
| 111 | - | code.forward-session = FSK | |||
| 112 | code.session-picker = SSP | 111 | code.session-picker = SSP | ||
| 113 | code.visual-mode = VIS | 112 | code.visual-mode = VIS | ||
| 114 | code.toggle-floating-pane = FLT | 113 | code.toggle-floating-pane = FLT | ||
| @@ -193,7 +192,7 @@ Successful commands are promoted near the top the next time the palette opens. | |||||
| 193 | | CLP | close focused pane | | 192 | | CLP | close focused pane | | ||
| 194 | | FPL / FPR / FPU / FPD | focus pane left / right / up / down | | 193 | | FPL / FPR / FPU / FPD | focus pane left / right / up / down | | ||
| 195 | | NXT / PVT | switch to next / previous tab | | 194 | | NXT / PVT | switch to next / previous tab | | ||
| 196 | - | | BSK / FSK | move back / forward through recent sessions | | 195 | + | | BSK | toggle the previously active session | |
| 197 | | SSP | open session picker | | 196 | | SSP | open session picker | | ||
| 198 | | VIS | enter visual mode | | 197 | | VIS | enter visual mode | | ||
| 199 | | FLT | show or hide the current tab's floating terminal | | 198 | | FLT | show or hide the current tab's floating terminal | | ||
M internal/usecase/daemon/input_test.go +73 -2
| @@ -452,13 +452,84 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||
| 452 | 452 | } | |
| 453 | 453 | } | |
| 454 | 454 | ||
| 455 | + | func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | |
| 456 | + | ac := &attachedClient{} | |
| 457 | + | stale := &session{id: "stale"} | |
| 458 | + | updated := &session{id: "updated"} | |
| 459 | + | ac.previousSession.Set(stale) | |
| 460 | + | ||
| 461 | + | // Model a completed hand-off between observing a stale target and clearing | |
| 462 | + | // it. The conditional clear must not erase the newer toggle destination. | |
| 463 | + | ac.previousSession.Set(updated) | |
| 464 | + | ac.clearPreviousSessionIf(stale) | |
| 465 | + | ||
| 466 | + | require.Same(t, updated, ac.previousSession.Get()) | |
| 467 | + | } | |
| 468 | + | ||
| 469 | + | func TestSwitchSourcePreviousSessionContracts(t *testing.T) { | |
| 470 | + | for _, tc := range []struct { | |
| 471 | + | name string | |
| 472 | + | target func(*session, *session) picker.Target | |
| 473 | + | wantCurrent string | |
| 474 | + | wantPrev string | |
| 475 | + | }{ | |
| 476 | + | { | |
| 477 | + | name: "picker cross-session switch records origin", | |
| 478 | + | target: func(_ *session, recent *session) picker.Target { | |
| 479 | + | return picker.Target{Session: recent.id, TabIndex: -1} | |
| 480 | + | }, | |
| 481 | + | wantCurrent: "recent", | |
| 482 | + | wantPrev: "current", | |
| 483 | + | }, | |
| 484 | + | { | |
| 485 | + | name: "attention cross-session switch records origin", | |
| 486 | + | target: func(_ *session, recent *session) picker.Target { | |
| 487 | + | return picker.Target{Session: recent.id, TabIndex: 0} | |
| 488 | + | }, | |
| 489 | + | wantCurrent: "recent", | |
| 490 | + | wantPrev: "current", | |
| 491 | + | }, | |
| 492 | + | { | |
| 493 | + | name: "same-session tab switch does not record", | |
| 494 | + | target: func(current *session, _ *session) picker.Target { | |
| 495 | + | return picker.Target{Session: current.id, TabIndex: 0} | |
| 496 | + | }, | |
| 497 | + | wantCurrent: "current", | |
| 498 | + | wantPrev: "", | |
| 499 | + | }, | |
| 500 | + | { | |
| 501 | + | name: "missing target does not record", | |
| 502 | + | target: func(_ *session, _ *session) picker.Target { | |
| 503 | + | return picker.Target{Session: "missing", TabIndex: -1} | |
| 504 | + | }, | |
| 505 | + | wantCurrent: "current", | |
| 506 | + | wantPrev: "", | |
| 507 | + | }, | |
| 508 | + | } { | |
| 509 | + | t.Run(tc.name, func(t *testing.T) { | |
| 510 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |
| 511 | + | defer releaseAll(releases) | |
| 512 | + | recent := d.sessions[domain.SessionID("recent")] | |
| 513 | + | ||
| 514 | + | d.switchToTarget(current, ac, tc.target(current, recent)) | |
| 515 | + | require.Equal(t, domain.SessionID(tc.wantCurrent), ac.currentSession().id) | |
| 516 | + | if tc.wantPrev == "" { | |
| 517 | + | require.Nil(t, ac.previousSession.Get()) | |
| 518 | + | return | |
| 519 | + | } | |
| 520 | + | require.Equal(t, domain.SessionID(tc.wantPrev), ac.previousSession.Get().id) | |
| 521 | + | }) | |
| 522 | + | } | |
| 523 | + | } | |
| 524 | + | ||
| 455 | 525 | func TestTerminalTeardownClearsPreviousSession(t *testing.T) { | |
| 456 | 526 | for _, tc := range []struct { | |
| 457 | 527 | name string | |
| 458 | 528 | teardown func(*Daemon, *session, *attachedClient) | |
| 459 | 529 | }{ | |
| 460 | - | {name: "send error", teardown: func(d *Daemon, sess *session, ac *attachedClient) { d.detachOnSendError(sess, ac, ac.transport()) }}, | |
| 461 | - | {name: "session kill", teardown: func(d *Daemon, sess *session, _ *attachedClient) { | |
| 530 | + | {name: "explicit detach", teardown: func(d *Daemon, sess *session, ac *attachedClient) { d.clientGone(sess, ac, ac.transport(), true) }}, | |
| 531 | + | {name: "non-park send error fallback", teardown: func(d *Daemon, sess *session, ac *attachedClient) { d.detachOnSendError(sess, ac, ac.transport()) }}, | |
| 532 | + | {name: "killed attached session", teardown: func(d *Daemon, sess *session, _ *attachedClient) { | |
| 462 | 533 | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | |
| 463 | 534 | }}, | |
| 464 | 535 | } { | |
| @@ -452,13 +452,84 @@ func TestPaletteBackSessionDoesNotMoveWithoutValidTarget(t *testing.T) { | |||||
| 452 | } | 452 | } | ||
| 453 | } | 453 | } | ||
| 454 | 454 | ||||
| 455 | + | func TestStaleBackSessionClearPreservesConcurrentTarget(t *testing.T) { | |||
| 456 | + | ac := &attachedClient{} | |||
| 457 | + | stale := &session{id: "stale"} | |||
| 458 | + | updated := &session{id: "updated"} | |||
| 459 | + | ac.previousSession.Set(stale) | |||
| 460 | + | ||||
| 461 | + | // Model a completed hand-off between observing a stale target and clearing | |||
| 462 | + | // it. The conditional clear must not erase the newer toggle destination. | |||
| 463 | + | ac.previousSession.Set(updated) | |||
| 464 | + | ac.clearPreviousSessionIf(stale) | |||
| 465 | + | ||||
| 466 | + | require.Same(t, updated, ac.previousSession.Get()) | |||
| 467 | + | } | |||
| 468 | + | ||||
| 469 | + | func TestSwitchSourcePreviousSessionContracts(t *testing.T) { | |||
| 470 | + | for _, tc := range []struct { | |||
| 471 | + | name string | |||
| 472 | + | target func(*session, *session) picker.Target | |||
| 473 | + | wantCurrent string | |||
| 474 | + | wantPrev string | |||
| 475 | + | }{ | |||
| 476 | + | { | |||
| 477 | + | name: "picker cross-session switch records origin", | |||
| 478 | + | target: func(_ *session, recent *session) picker.Target { | |||
| 479 | + | return picker.Target{Session: recent.id, TabIndex: -1} | |||
| 480 | + | }, | |||
| 481 | + | wantCurrent: "recent", | |||
| 482 | + | wantPrev: "current", | |||
| 483 | + | }, | |||
| 484 | + | { | |||
| 485 | + | name: "attention cross-session switch records origin", | |||
| 486 | + | target: func(_ *session, recent *session) picker.Target { | |||
| 487 | + | return picker.Target{Session: recent.id, TabIndex: 0} | |||
| 488 | + | }, | |||
| 489 | + | wantCurrent: "recent", | |||
| 490 | + | wantPrev: "current", | |||
| 491 | + | }, | |||
| 492 | + | { | |||
| 493 | + | name: "same-session tab switch does not record", | |||
| 494 | + | target: func(current *session, _ *session) picker.Target { | |||
| 495 | + | return picker.Target{Session: current.id, TabIndex: 0} | |||
| 496 | + | }, | |||
| 497 | + | wantCurrent: "current", | |||
| 498 | + | wantPrev: "", | |||
| 499 | + | }, | |||
| 500 | + | { | |||
| 501 | + | name: "missing target does not record", | |||
| 502 | + | target: func(_ *session, _ *session) picker.Target { | |||
| 503 | + | return picker.Target{Session: "missing", TabIndex: -1} | |||
| 504 | + | }, | |||
| 505 | + | wantCurrent: "current", | |||
| 506 | + | wantPrev: "", | |||
| 507 | + | }, | |||
| 508 | + | } { | |||
| 509 | + | t.Run(tc.name, func(t *testing.T) { | |||
| 510 | + | d, current, ac, _, releases := newRecentNavigationTestSessions(t) | |||
| 511 | + | defer releaseAll(releases) | |||
| 512 | + | recent := d.sessions[domain.SessionID("recent")] | |||
| 513 | + | ||||
| 514 | + | d.switchToTarget(current, ac, tc.target(current, recent)) | |||
| 515 | + | require.Equal(t, domain.SessionID(tc.wantCurrent), ac.currentSession().id) | |||
| 516 | + | if tc.wantPrev == "" { | |||
| 517 | + | require.Nil(t, ac.previousSession.Get()) | |||
| 518 | + | return | |||
| 519 | + | } | |||
| 520 | + | require.Equal(t, domain.SessionID(tc.wantPrev), ac.previousSession.Get().id) | |||
| 521 | + | }) | |||
| 522 | + | } | |||
| 523 | + | } | |||
| 524 | + | ||||
| 455 | func TestTerminalTeardownClearsPreviousSession(t *testing.T) { | 525 | func TestTerminalTeardownClearsPreviousSession(t *testing.T) { | ||
| 456 | for _, tc := range []struct { | 526 | for _, tc := range []struct { | ||
| 457 | name string | 527 | name string | ||
| 458 | teardown func(*Daemon, *session, *attachedClient) | 528 | teardown func(*Daemon, *session, *attachedClient) | ||
| 459 | }{ | 529 | }{ | ||
| 460 | - | {name: "send error", teardown: func(d *Daemon, sess *session, ac *attachedClient) { d.detachOnSendError(sess, ac, ac.transport()) }}, | 530 | + | {name: "explicit detach", teardown: func(d *Daemon, sess *session, ac *attachedClient) { d.clientGone(sess, ac, ac.transport(), true) }}, |
| 461 | - | {name: "session kill", teardown: func(d *Daemon, sess *session, _ *attachedClient) { | 531 | + | {name: "non-park send error fallback", teardown: func(d *Daemon, sess *session, ac *attachedClient) { d.detachOnSendError(sess, ac, ac.transport()) }}, |
| 532 | + | {name: "killed attached session", teardown: func(d *Daemon, sess *session, _ *attachedClient) { | |||
| 462 | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | 533 | require.NoError(t, d.killSession(sess, ports.ReasonSessionKilled, false)) | ||
| 463 | }}, | 534 | }}, | ||
| 464 | } { | 535 | } { | ||
M internal/usecase/daemon/resume_test.go +27 -0
| @@ -322,6 +322,33 @@ func TestParkExpiryAndShutdownCleanup(t *testing.T) { | |||
| 322 | 322 | require.Zero(t, parked) | |
| 323 | 323 | } | |
| 324 | 324 | ||
| 325 | + | func TestLiveParkAndResumeRetainsPreviousSession(t *testing.T) { | |
| 326 | + | pty, release := newBlockingPTY(t) | |
| 327 | + | defer release() | |
| 328 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |
| 329 | + | tr, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | |
| 330 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), tr) | |
| 331 | + | require.NoError(t, err) | |
| 332 | + | ||
| 333 | + | previous := &session{id: "previous"} | |
| 334 | + | ac.previousSession.Set(previous) | |
| 335 | + | d.clientGone(sess, ac, ac.transport(), false) | |
| 336 | + | ||
| 337 | + | token := ac.resumeToken | |
| 338 | + | d.mu.Lock() | |
| 339 | + | parked := d.parked[token] | |
| 340 | + | d.mu.Unlock() | |
| 341 | + | require.NotNil(t, parked) | |
| 342 | + | require.Same(t, previous, ac.previousSession.Get(), "a live parked attachment keeps its toggle") | |
| 343 | + | ||
| 344 | + | tr2, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | |
| 345 | + | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), tr2) | |
| 346 | + | require.NoError(t, err) | |
| 347 | + | require.Same(t, sess, resumedSess) | |
| 348 | + | require.Same(t, ac, resumedAC) | |
| 349 | + | require.Same(t, previous, resumedAC.previousSession.Get(), "resume keeps the live attachment toggle") | |
| 350 | + | } | |
| 351 | + | ||
| 325 | 352 | func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | |
| 326 | 353 | for _, tc := range []struct { | |
| 327 | 354 | name string | |
| @@ -322,6 +322,33 @@ func TestParkExpiryAndShutdownCleanup(t *testing.T) { | |||||
| 322 | require.Zero(t, parked) | 322 | require.Zero(t, parked) | ||
| 323 | } | 323 | } | ||
| 324 | 324 | ||||
| 325 | + | func TestLiveParkAndResumeRetainsPreviousSession(t *testing.T) { | |||
| 326 | + | pty, release := newBlockingPTY(t) | |||
| 327 | + | defer release() | |||
| 328 | + | d := newTestDaemon(t, newFactory(t, pty), stubClock{}) | |||
| 329 | + | tr, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | |||
| 330 | + | sess, ac, err := d.route(helloResumeCapable(ports.IntentNew, "work", 0), tr) | |||
| 331 | + | require.NoError(t, err) | |||
| 332 | + | ||||
| 333 | + | previous := &session{id: "previous"} | |||
| 334 | + | ac.previousSession.Set(previous) | |||
| 335 | + | d.clientGone(sess, ac, ac.transport(), false) | |||
| 336 | + | ||||
| 337 | + | token := ac.resumeToken | |||
| 338 | + | d.mu.Lock() | |||
| 339 | + | parked := d.parked[token] | |||
| 340 | + | d.mu.Unlock() | |||
| 341 | + | require.NotNil(t, parked) | |||
| 342 | + | require.Same(t, previous, ac.previousSession.Get(), "a live parked attachment keeps its toggle") | |||
| 343 | + | ||||
| 344 | + | tr2, _, _ := newConn(t, mustHello(ports.IntentAttach, "unused", domain.Size{})) | |||
| 345 | + | resumedSess, resumedAC, err := d.route(helloResumeCapable(ports.IntentResume, "work", token), tr2) | |||
| 346 | + | require.NoError(t, err) | |||
| 347 | + | require.Same(t, sess, resumedSess) | |||
| 348 | + | require.Same(t, ac, resumedAC) | |||
| 349 | + | require.Same(t, previous, resumedAC.previousSession.Get(), "resume keeps the live attachment toggle") | |||
| 350 | + | } | |||
| 351 | + | ||||
| 325 | func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | 352 | func TestDiscardingParkedAttachmentClearsPreviousSession(t *testing.T) { | ||
| 326 | for _, tc := range []struct { | 353 | for _, tc := range []struct { | ||
| 327 | name string | 354 | name string | ||
M internal/usecase/daemon/session_back.go +14 -5
| @@ -10,17 +10,26 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||
| 10 | 10 | } | |
| 11 | 11 | target := ac.previousSession.Get() | |
| 12 | 12 | if target == nil || target == current || d.sessionByID(target.id) != target { | |
| 13 | - | ac.previousSession.With(func(previous **session) { | |
| 14 | - | if *previous == target { | |
| 15 | - | *previous = nil | |
| 16 | - | } | |
| 17 | - | }) | |
| 13 | + | ac.clearPreviousSessionIf(target) | |
| 18 | 14 | d.paint(current, ac, true) | |
| 19 | 15 | return | |
| 20 | 16 | } | |
| 21 | 17 | d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}) | |
| 22 | 18 | } | |
| 23 | 19 | ||
| 20 | + | // clearPreviousSessionIf clears target only if it has not been replaced since | |
| 21 | + | // the caller observed it. This preserves a concurrent successful hand-off. | |
| 22 | + | func (ac *attachedClient) clearPreviousSessionIf(target *session) { | |
| 23 | + | if ac == nil { | |
| 24 | + | return | |
| 25 | + | } | |
| 26 | + | ac.previousSession.With(func(previous **session) { | |
| 27 | + | if *previous == target { | |
| 28 | + | *previous = nil | |
| 29 | + | } | |
| 30 | + | }) | |
| 31 | + | } | |
| 32 | + | ||
| 24 | 33 | // recordPreviousSession is called only after a cross-session hand-off has | |
| 25 | 34 | // committed. Keeping this after the commit makes failed/displaced switches | |
| 26 | 35 | // leave the toggle pair intact. | |
| @@ -10,17 +10,26 @@ func (d *Daemon) backSession(current *session, ac *attachedClient) { | |||||
| 10 | } | 10 | } | ||
| 11 | target := ac.previousSession.Get() | 11 | target := ac.previousSession.Get() | ||
| 12 | if target == nil || target == current || d.sessionByID(target.id) != target { | 12 | if target == nil || target == current || d.sessionByID(target.id) != target { | ||
| 13 | - | ac.previousSession.With(func(previous **session) { | 13 | + | ac.clearPreviousSessionIf(target) |
| 14 | - | if *previous == target { | |||
| 15 | - | *previous = nil | |||
| 16 | - | } | |||
| 17 | - | }) | |||
| 18 | d.paint(current, ac, true) | 14 | d.paint(current, ac, true) | ||
| 19 | return | 15 | return | ||
| 20 | } | 16 | } | ||
| 21 | d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}) | 17 | d.switchToTarget(current, ac, picker.Target{Session: target.id, TabIndex: -1}) | ||
| 22 | } | 18 | } | ||
| 23 | 19 | ||||
| 20 | + | // clearPreviousSessionIf clears target only if it has not been replaced since | |||
| 21 | + | // the caller observed it. This preserves a concurrent successful hand-off. | |||
| 22 | + | func (ac *attachedClient) clearPreviousSessionIf(target *session) { | |||
| 23 | + | if ac == nil { | |||
| 24 | + | return | |||
| 25 | + | } | |||
| 26 | + | ac.previousSession.With(func(previous **session) { | |||
| 27 | + | if *previous == target { | |||
| 28 | + | *previous = nil | |||
| 29 | + | } | |||
| 30 | + | }) | |||
| 31 | + | } | |||
| 32 | + | ||||
| 24 | // recordPreviousSession is called only after a cross-session hand-off has | 33 | // recordPreviousSession is called only after a cross-session hand-off has | ||
| 25 | // committed. Keeping this after the commit makes failed/displaced switches | 34 | // committed. Keeping this after the commit makes failed/displaced switches | ||
| 26 | // leave the toggle pair intact. | 35 | // leave the toggle pair intact. | ||