Skip to content

feat: MRU slot-based jumping and inline search for TUI#28

Merged
jfox85 merged 13 commits intomainfrom
jf-faster-jumping
Feb 15, 2026
Merged

feat: MRU slot-based jumping and inline search for TUI#28
jfox85 merged 13 commits intomainfrom
jf-faster-jumping

Conversation

@jfox85
Copy link
Owner

@jfox85 jfox85 commented Feb 15, 2026

Summary

  • MRU slot numbers (1-9): Sessions get sticky numbered slots based on most-recently-used ordering. Pressing 1-9 in the TUI jumps to that session. Slots persist across restarts and use LRU eviction when all 9 are full.
  • Inline search filtering: Pressing / activates a search bar at the bottom of the main list view (no separate screen). Typing filters sessions by name, arrow keys navigate results, Enter jumps to selection, Esc cancels.
  • Slot bootstrap on startup: All sessions get slots assigned on first TUI load so numbers appear immediately.
  • Code review fixes: Deterministic eviction, single-write saves, stale filter rebuild, duplicate call removal, bootstrap guard, edge case tests.

Changes

  • session/metadata.goLastAttached timestamp, NumberedSlots map, slot CRUD methods with deterministic eviction
  • session/metadata_test.go — Comprehensive tests including edge cases (nonexistent session, all-zero eviction, stale slot reuse)
  • tui/model.go — Inline filter UI, slot display/jumping, bootstrap logic, extracted render helpers
  • cmd/session_attach.go — Wire RecordAttach + AssignSlot on attach
  • cmd/session_rm.go — Reconcile slots on session removal (single save)
  • 4 design/plan docs in docs/plans/

Test plan

  • go test -race ./... passes
  • go vet ./... clean
  • gofmt clean
  • Manual: MRU slot numbers appear on TUI load
  • Manual: / opens inline search, typing filters, Enter/Esc work
  • Manual: Number keys 1-9 jump to correct session
  • Manual: Preview pane stays visible during search

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added MRU slot numbering: Sessions are assigned to numbered slots (1-9) based on most recent use, accessible via number key shortcuts in the TUI.
    • Added inline search: Use the / key to activate an inline filter and search sessions by name.
    • Sessions are automatically assigned MRU slots on startup.
  • Documentation

    • Added comprehensive design and implementation documentation for session navigation features.

jfox85 and others added 13 commits February 14, 2026 20:21
MRU-based slot-pinned numbering (1-9) and fuzzy search (/) to
improve navigation when there are many sessions across projects.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7-task plan covering MRU slot-pinned numbers, fuzzy search,
and wiring into TUI display/key handling.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add LastAttached field to Session struct and RecordAttach method
to SessionStore, for tracking when sessions were last attached.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Persistent map of slot (1-9) to session name on SessionStore.
AssignSlot gives lowest free slot, keeps existing assignments stable,
and evicts LRU (oldest LastAttached) when all 9 are full.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Record LastAttached and assign numbered slot on every attach (CLI
and TUI). TUI loads/reconciles slots on session refresh, displays
slot numbers instead of positional indices, and number keys jump
to the session in that MRU slot.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Press / in the session list to enter search mode. Type to filter
sessions by name (case-insensitive substring), arrow keys to
navigate filtered results, Enter to jump, Esc to cancel.
Also updates footer to show MRU hint and /: search.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Search view was missing the bell indicator for flagged sessions.
Session removal now cleans up numbered slots immediately.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ch view

Replace stateSearch with inline filtering that stays in the main list view.
When pressing '/', a search box appears at the bottom and filters the list
in place. Enter closes the filter and jumps to selected session. Esc cancels.

Changes:
- Remove stateSearch from state enum
- Add filterActive field to model
- Merge search key handling into stateList with filterActive guard
- Update both preview and non-preview rendering to use displayEntry pattern
- Remove searchView() function
- Update footer to show appropriate help based on filterActive state

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Short-circuit bootstrap loop when all 9 slots are full
- Extract displayEntry type and helper methods (buildFilteredEntries, renderSessionList, renderSearchBox)
- Remove code duplication between preview and non-preview paths
- Reset searchCursor on Enter for defensive consistency

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Fix stale filteredIndices when sessions refresh during active filter
- Make eviction deterministic by sorting slots before tie-breaking
- Eliminate double LoadSessions per refresh by passing store through msg
- Remove duplicate RecordAttach/AssignSlot from TUI (CLI already does it)
- Consolidate double Save in session_rm.go into single write
- Gate bootstrap to run once on first load, not every 2s refresh
- Add edge case tests: nonexistent session, all-zero eviction, stale slot

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Feb 15, 2026

📝 Walkthrough

Walkthrough

The PR introduces a faster session navigation system featuring MRU (most-recently-used) slot-pinned numbering and inline fuzzy search. It adds slot assignment on session attachment, slot reconciliation on removal, comprehensive design and implementation documentation, tests, and TUI modifications for inline search filtering and slot-based navigation.

Changes

Cohort / File(s) Summary
Slot System Core
session/metadata.go, session/metadata_test.go
Added LastAttached timestamp to Session, NumberedSlots mapping to SessionStore, and methods for slot assignment (AssignSlot with eviction of oldest-attached when full), recording attachments (RecordAttach), slot lookup (GetSlotForSession, GetSessionForSlot), and reconciliation (ReconcileSlots). Includes deterministic eviction logic and comprehensive test coverage for slot lifecycle.
CLI Integration
cmd/session_attach.go, cmd/session_rm.go
session_attach.go: Added calls to RecordAttach and AssignSlot with non-blocking error handling. session_rm.go: Replaced explicit RemoveSession with direct map deletion, followed by ReconcileSlots and Save for consolidated metadata persistence.
TUI Implementation
tui/model.go
Added UI search/filter state (filterActive, searchInput, searchFilter, filteredIndices, searchCursor) and MRU slot management (numberedSlots, slotsBootstrapped). Introduced inline search activation via /, filtered session listing with project grouping and slot prefixes, slot bootstrap on first load, and adapted navigation/rendering for filtered views.
Design and Implementation Plans
docs/plans/2026-02-14-faster-session-jumping-design.md, docs/plans/2026-02-14-faster-session-jumping.md, docs/plans/2026-02-15-inline-search-and-slot-bootstrap-design.md, docs/plans/2026-02-15-inline-search-and-slot-bootstrap-plan.md
Comprehensive design documents outlining MRU slot assignment logic, slot reconciliation, fuzzy search implementation, inline filter UI behavior, key bindings (1-9 for slots, / for search), test scaffolding, and phased implementation approach.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI as cmd/session_attach
    participant Store as SessionStore
    participant Persist as Persistence
    
    User->>CLI: Attach to session
    CLI->>Store: Validate session
    CLI->>Store: RecordAttach(name)
    Store->>Store: Update LastAttached
    CLI->>Store: AssignSlot(name)
    activate Store
        alt Slot available
            Store->>Store: Assign free slot
        else No slots available
            Store->>Store: Find oldest-attached
            Store->>Store: Evict & reassign
        end
    deactivate Store
    CLI->>Persist: Save metadata
    CLI->>User: Attach to tmux session
Loading
sequenceDiagram
    participant User
    participant CLI as cmd/session_rm
    participant Store as SessionStore
    participant Persist as Persistence
    
    User->>CLI: Remove session
    CLI->>Store: Delete from Sessions map
    CLI->>Store: ReconcileSlots()
    activate Store
        Store->>Store: Purge stale slot refs
    deactivate Store
    CLI->>Persist: Save metadata
    CLI->>User: Session removed
Loading
sequenceDiagram
    participant TUI as tui/model
    participant Store as SessionStore
    participant UI as Display
    
    TUI->>Store: Load sessions
    Store->>TUI: Return SessionStore
    TUI->>Store: ReconcileSlots()
    TUI->>Store: AssignSlot (for unslotted)
    TUI->>Store: Save
    TUI->>TUI: Bootstrap numberedSlots
    TUI->>UI: Render with MRU slots (1-9)
    User->>TUI: Press / (search)
    TUI->>TUI: Activate filterActive
    TUI->>UI: Render inline search box
    User->>TUI: Type filter text
    TUI->>TUI: Filter sessions
    TUI->>UI: Render filtered list
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

Poem

🐰 Hop, hop! Slots light up one through nine,
Fuzzy search makes jumping sublime,
Most-recent slots remember each friend,
Quick navigation, no need to descend! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 17.65% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: MRU slot-based jumping and inline search for TUI' accurately and concisely describes the two main features implemented: MRU-based numbered slots for quick session jumping and an inline search interface for filtering sessions in the TUI.
Merge Conflict Detection ✅ Passed ✅ No merge conflicts detected when merging into main

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch jf-faster-jumping

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tui/model.go (1)

1617-1646: ⚠️ Potential issue | 🟡 Minor

calculateOptimalListWidth doesn't account for new MRU slot prefix and attention indicator.

renderSessionList now prepends cursor(2) + numberPrefix(3) + indicator(1–2) + space(1) before the session name (see lines 1188–1208), adding ~6 extra characters compared to a plain name. The current padding of +10 (line 1632) was calibrated for the old format and may clip session names in the preview layout's left pane.

Suggested fix
-	// Add padding for cursor (2) + borders (4) + some margin (4)
-	optimalWidth := maxNameLength + 10
+	// Add padding for cursor (2) + slot prefix (3) + indicator (2) + space (1) + borders (4) + margin (4)
+	optimalWidth := maxNameLength + 16
🤖 Fix all issues with AI agents
In `@session/metadata_test.go`:
- Around line 126-405: Tests are leaking state because LoadSessions() ultimately
calls config.GetSessionsPath() which uses os.UserHomeDir() (cached by the
stdlib) so calling os.Setenv("HOME", tmpDir) in each test is too late; fix by
making the sessions path injectable for tests and use that instead of relying on
HOME: add a configurable hook or setter (e.g., config.SetSessionsPath or pass an
explicit path into LoadSessions/LoadSessionsWithPath) and update tests
(references: LoadSessions, config.GetSessionsPath, os.UserHomeDir) to call the
setter or the new LoadSessionsWithPath with tmpDir before interacting with the
store so each test uses an isolated sessions.json. Ensure tests no longer depend
on Setenv to control HOME.
🧹 Nitpick comments (6)
session/metadata.go (2)

159-167: RemoveSession no longer reconciles slots — risk of stale slot data for other callers.

cmd/session_rm.go now bypasses RemoveSession (using delete + ReconcileSlots + Save directly), but RemoveSession is still a public method used in tests and could be called by future code. A caller using RemoveSession alone would leave stale slot entries behind.

Consider either:

  • Adding ReconcileSlots() inside RemoveSession, or
  • Documenting that callers must call ReconcileSlots() + Save() separately.
Proposed fix: reconcile inside RemoveSession
 func (s *SessionStore) RemoveSession(name string) error {
 	if _, exists := s.Sessions[name]; !exists {
 		return fmt.Errorf("session %s not found", name)
 	}
 
 	delete(s.Sessions, name)
+	s.ReconcileSlots()
 	return s.Save()
 }

392-398: ReconcileSlots doesn't persist — document the caller's responsibility to Save().

This is intentional (enables batching with other mutations), but a doc comment would prevent misuse.

Proposed doc improvement
-// ReconcileSlots removes slot assignments for sessions that no longer exist.
+// ReconcileSlots removes slot assignments for sessions that no longer exist.
+// It modifies in-memory state only; the caller must call Save() to persist.
 func (s *SessionStore) ReconcileSlots() {
cmd/session_attach.go (1)

55-61: Two sequential Save() calls on attach — consider combining for efficiency.

RecordAttach (via UpdateSession) and AssignSlot each write the entire store to disk. On first attach of a session, this produces two sequential file writes. A combined method could reduce this to one.

This is minor — the files are small and attach is infrequent — but worth noting for a future optimization pass.

Sketch: combined method in metadata.go
// RecordAttachAndAssignSlot atomically updates LastAttached and assigns a slot in a single save.
func (s *SessionStore) RecordAttachAndAssignSlot(name string) (int, error) {
	sess, exists := s.Sessions[name]
	if !exists {
		return 0, fmt.Errorf("session '%s' not found", name)
	}
	sess.LastAttached = time.Now()
	sess.UpdatedAt = time.Now()

	if slot := s.GetSlotForSession(name); slot != 0 {
		return slot, s.Save()
	}
	// ... rest of AssignSlot logic without intermediate Save()
}
tui/model.go (3)

892-894: Silently discarded Save() error may hide persistence failures.

If Save() fails, the bootstrapped slots exist only in memory and will be lost on restart, causing slots to re-shuffle unexpectedly. Consider at minimum logging the error.

Suggested fix
-			if changed {
-				_ = slotStore.Save()
-			}
+			if changed {
+				if err := slotStore.Save(); err != nil {
+					m.debugLogger.Printf("Failed to save bootstrapped slots: %v", err)
+				}
+			}

As per coding guidelines: **/*.go — "Wrap errors consistently with context throughout the codebase."


1151-1223: Minor: O(n × 9) slot lookup on every render could use a reverse map.

The per-entry iteration over numberedSlots (lines 1195–1200) is functionally correct with at most 9 entries. If you ever expand beyond 9 slots or want to tighten the render path, consider building a sessionName → slot reverse map once per render cycle in buildFilteredEntries or at the top of renderSessionList.


1877-1885: sessionIndexByName is a clean utility; consider it for future deduplication.

There are a few places in the codebase that scan m.sessions by name (e.g., gitStatsMsg handler at line 1021). If this pattern grows, this helper could be reused there too.

Comment on lines +126 to +405
func TestSessionLastAttached(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()
_ = store.AddSession("test-sess", "main", "/path", map[string]int{"PORT": 3000})

// LastAttached should be zero initially
sess, _ := store.GetSession("test-sess")
if !sess.LastAttached.IsZero() {
t.Error("expected LastAttached to be zero initially")
}

// Record attach
err = store.RecordAttach("test-sess")
if err != nil {
t.Fatalf("failed to record attach: %v", err)
}

sess, _ = store.GetSession("test-sess")
if sess.LastAttached.IsZero() {
t.Error("expected LastAttached to be set after RecordAttach")
}
}

func TestNumberedSlots_AssignSlot(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()
_ = store.AddSession("sess-a", "main", "/a", map[string]int{})
_ = store.AddSession("sess-b", "main", "/b", map[string]int{})

// Assign slot for sess-a — should get slot 1 (lowest available)
slot, err := store.AssignSlot("sess-a")
if err != nil {
t.Fatalf("failed to assign slot: %v", err)
}
if slot != 1 {
t.Errorf("expected slot 1, got %d", slot)
}

// Assign slot for sess-b — should get slot 2
slot, err = store.AssignSlot("sess-b")
if err != nil {
t.Fatalf("failed to assign slot: %v", err)
}
if slot != 2 {
t.Errorf("expected slot 2, got %d", slot)
}

// Assign again for sess-a — should keep slot 1 (stable)
slot, err = store.AssignSlot("sess-a")
if err != nil {
t.Fatalf("failed to assign slot: %v", err)
}
if slot != 1 {
t.Errorf("expected sess-a to keep slot 1, got %d", slot)
}
}

func TestNumberedSlots_EvictLRU(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()

// Create 10 sessions, assign slots to first 9
for i := 1; i <= 10; i++ {
name := fmt.Sprintf("sess-%d", i)
_ = store.AddSession(name, "main", fmt.Sprintf("/%d", i), map[string]int{})
// Set LastAttached so sess-1 is oldest
_ = store.UpdateSession(name, func(s *Session) {
s.LastAttached = time.Now().Add(time.Duration(i) * time.Minute)
})
}

for i := 1; i <= 9; i++ {
_, _ = store.AssignSlot(fmt.Sprintf("sess-%d", i))
}

// All 9 slots full. Assign slot for sess-10 — should evict sess-1 (oldest LastAttached)
slot, err := store.AssignSlot("sess-10")
if err != nil {
t.Fatalf("failed to assign slot: %v", err)
}

// sess-10 should have taken sess-1's slot (slot 1)
if slot != 1 {
t.Errorf("expected sess-10 to get slot 1 (evicting sess-1), got %d", slot)
}

// sess-1 should no longer have a slot
if s := store.GetSlotForSession("sess-1"); s != 0 {
t.Errorf("expected sess-1 to have no slot, got %d", s)
}
}

func TestNumberedSlots_Reconcile(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()
_ = store.AddSession("sess-a", "main", "/a", map[string]int{})
_, _ = store.AssignSlot("sess-a")

// Remove session, then reconcile — slot should be freed
_ = store.RemoveSession("sess-a")
store.ReconcileSlots()

if s := store.GetSlotForSession("sess-a"); s != 0 {
t.Errorf("expected no slot for removed session, got %d", s)
}
}

func TestRecordAttach_NonexistentSession(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()

err = store.RecordAttach("nonexistent")
if err == nil {
t.Error("expected error when recording attach for nonexistent session")
}
}

func TestAssignSlot_NonexistentSession(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()

_, err = store.AssignSlot("nonexistent")
if err == nil {
t.Error("expected error when assigning slot for nonexistent session")
}
}

func TestNumberedSlots_EvictAllZeroLastAttached(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()

// Create 10 sessions, all with zero LastAttached
for i := 1; i <= 10; i++ {
name := fmt.Sprintf("sess-%d", i)
_ = store.AddSession(name, "main", fmt.Sprintf("/%d", i), map[string]int{})
}
for i := 1; i <= 9; i++ {
_, _ = store.AssignSlot(fmt.Sprintf("sess-%d", i))
}

// All zero LastAttached: eviction should still succeed deterministically
// (lowest slot number with zero time gets evicted)
slot, err := store.AssignSlot("sess-10")
if err != nil {
t.Fatalf("failed to assign slot with all-zero LastAttached: %v", err)
}
if slot < 1 || slot > 9 {
t.Errorf("expected valid slot 1-9, got %d", slot)
}

// Verify sess-10 now has the slot
if got := store.GetSlotForSession("sess-10"); got != slot {
t.Errorf("expected sess-10 at slot %d, got %d", slot, got)
}
}

func TestNumberedSlots_StaleSlotReuse(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()

// Create 9 sessions and fill all slots
for i := 1; i <= 9; i++ {
name := fmt.Sprintf("sess-%d", i)
_ = store.AddSession(name, "main", fmt.Sprintf("/%d", i), map[string]int{})
_, _ = store.AssignSlot(name)
}

// Remove sess-3 from sessions but leave its slot (simulates stale slot)
delete(store.Sessions, "sess-3")

// Add a new session
_ = store.AddSession("sess-new", "main", "/new", map[string]int{})

// Assign slot — should reuse the stale slot 3
slot, err := store.AssignSlot("sess-new")
if err != nil {
t.Fatalf("failed to assign slot: %v", err)
}
if slot != 3 {
t.Errorf("expected stale slot 3 to be reused, got %d", slot)
}
}

func TestNumberedSlots_GetSessionForSlot(t *testing.T) {
tmpDir, err := os.MkdirTemp("", "devx-session-test-*")
if err != nil {
t.Fatalf("failed to create temp dir: %v", err)
}
defer os.RemoveAll(tmpDir)

oldHome := os.Getenv("HOME")
os.Setenv("HOME", tmpDir)
defer os.Setenv("HOME", oldHome)

store, _ := LoadSessions()
_ = store.AddSession("sess-a", "main", "/a", map[string]int{})
_, _ = store.AssignSlot("sess-a")

name := store.GetSessionForSlot(1)
if name != "sess-a" {
t.Errorf("expected 'sess-a' for slot 1, got '%s'", name)
}

name = store.GetSessionForSlot(5)
if name != "" {
t.Errorf("expected empty for unassigned slot 5, got '%s'", name)
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify how config.GetSessionsPath() resolves the sessions file path

echo "=== Looking for GetSessionsPath implementation ==="
fd -e go . | xargs rg -l 'GetSessionsPath' 

echo ""
echo "=== Detailed GetSessionsPath implementation ==="
fd -e go . | xargs rg -B5 -A10 'func.*GetSessionsPath'

echo ""
echo "=== Check config package structure ==="
fd -e go 'config' 

echo ""
echo "=== Look for caching patterns in config ==="
fd -e go . | xargs rg 'var.*home|var.*path|sync\.Once|var.*cache' | grep -i config

echo ""
echo "=== Check init functions in config ==="
fd -e go . | xargs rg -B2 -A5 'func init\(\)'

Repository: jfox85/devx

Length of output: 8463


🏁 Script executed:

#!/bin/bash
# Check the metadata_test.go file structure and patterns

echo "=== Test file size and structure ==="
wc -l session/metadata_test.go

echo ""
echo "=== First 50 lines (setup/imports) ==="
head -50 session/metadata_test.go

echo ""
echo "=== Check for t.Setenv usage ==="
rg -n 't\.Setenv' session/metadata_test.go

echo ""
echo "=== Check for os.Setenv pattern ==="
rg -n 'os\.Setenv' session/metadata_test.go | head -20

echo ""
echo "=== Check LoadSessions implementation ==="
rg -B3 -A15 'func LoadSessions' session/metadata.go

Repository: jfox85/devx

Length of output: 2767


🏁 Script executed:

#!/bin/bash
# Check the local getSessionsPath() implementation

echo "=== Check for getSessionsPath function in session/ ==="
rg -B3 -A10 'func getSessionsPath' session/

echo ""
echo "=== Verify it calls config.GetSessionsPath ==="
rg -n 'getSessionsPath\|GetSessionsPath' session/metadata.go

Repository: jfox85/devx

Length of output: 743


🏁 Script executed:

#!/bin/bash
# Verify how tests should be structured to fix the isolation issue

echo "=== Check if any tests use t.Setenv ==="
rg -n 't\.Setenv|t\.Cleanup' session/metadata_test.go

echo ""
echo "=== Verify os.UserHomeDir caches ==="
# Let's check if there's any documented behavior or workaround
rg -n 'UserHomeDir' session/ config/

echo ""
echo "=== Check if config package has any test setup ==="
cat config/discovery_test.go | head -80

echo ""
echo "=== Look for any SetConfigDir or override mechanisms ==="
rg -n 'SetSessionsPath\|SetConfigDir\|OverridePath' --type=go

Repository: jfox85/devx

Length of output: 2692


🏁 Script executed:

#!/bin/bash
# Check if there's CI configuration or test output showing actual failures

echo "=== Look for CI configuration ==="
fd -e yml -e yaml | grep -E 'github|gitlab|circle|ci' | head -10

echo ""
echo "=== Check for any test reports or logs ==="
fd 'test.*log\|report\|coverage' | head -10

echo ""
echo "=== Verify the actual issue: os.UserHomeDir caches at runtime ==="
# Let's trace through what happens when tests run
echo "Analysis of test execution order:"
echo "1. First test calls LoadSessions() -> getSessionsPath() -> config.GetSessionsPath() -> os.UserHomeDir()"
echo "2. os.UserHomeDir() caches HOME at that moment (from first test's tmpDir)"
echo "3. Second test calls os.Setenv('HOME', newTmpDir) but os.UserHomeDir() returns cached value"
echo "4. Both tests read/write to SAME sessions.json file from first test's tmpDir"
echo ""
echo "This matches the reviewer's claim of state leakage"

echo ""
echo "=== Verify no cleanup between tests ==="
rg -n 'cleanup|teardown|reset' session/metadata_test.go -i

Repository: jfox85/devx

Length of output: 673


🏁 Script executed:

#!/bin/bash
# Verify the core problem: os.UserHomeDir() behavior and potential fixes

echo "=== Check if config package tests handle HOME isolation ==="
cat config/discovery_test.go | grep -A 30 "func TestGetSessionsPath"

echo ""
echo "=== Verify the problem with current session tests ==="
echo "The core issue is:"
echo "- config.GetSessionsPath() calls os.UserHomeDir()"
echo "- os.UserHomeDir() caches the result on first call"
echo "- Tests use os.Setenv('HOME', tmpDir) but this doesn't affect cached value"
echo "- Result: all tests write to same sessions.json file"
echo ""

echo "=== Check test execution to confirm state leakage ==="
# Look at the specific tests that would leak state
rg -n 'func Test.*Slot' session/metadata_test.go | head -20

echo ""
echo "=== Potential solution: Check if config has any way to override paths ==="
cat config/discovery.go | head -100

Repository: jfox85/devx

Length of output: 4302


🏁 Script executed:

#!/bin/bash
# Final verification: understand the actual fix needed

echo "=== Verify t.Setenv behavior (Go 1.17+) ==="
# t.Setenv properly manages environment variables for tests
# But it still won't fix the os.UserHomeDir() caching issue

echo "The problem:"
echo "1. os.UserHomeDir() caches the result at runtime (first call)"
echo "2. t.Setenv('HOME', tmpDir) changes the env var AFTER caching"
echo "3. os.UserHomeDir() still returns the cached value"
echo ""

echo "=== Check Go version in the project ==="
find . -name 'go.mod' -o -name 'go.sum' | head -3
cat go.mod | grep '^go '

echo ""
echo "=== Summary of findings ==="
echo "CORRECT: Tests fail due to shared sessions.json file across test runs"
echo "CORRECT: Root cause is os.UserHomeDir() caching, not config caching"
echo "PARTIALLY CORRECT: Suggesting t.Setenv is good practice but won't fix the issue alone"
echo "INCOMPLETE: Doesn't mention os.UserHomeDir() caching as root cause"
echo ""
echo "The real fix requires either:"
echo "1. Resetting os.UserHomeDir cache (not exposed by stdlib)"
echo "2. Mocking/overriding how path is resolved"
echo "3. Using a different approach to session file location in tests"

Repository: jfox85/devx

Length of output: 871


Critical: Test isolation broken — all tests share the same sessions.json file.

The cross-test state leakage is real. TestNumberedSlots_GetSessionForSlot returning "sess-8" (only created in TestNumberedSlots_EvictLRU) proves multiple tests read/write the same sessions file.

Root cause: os.UserHomeDir() caches the home directory result at first call during program execution. When the first test runs, os.Setenv("HOME", tmpDir) is already too late — os.UserHomeDir() has already cached the real home path. Subsequent tests cannot override this via environment variables alone.

Why os.Setenv doesn't work here:

  • Test 1 calls LoadSessions()config.GetSessionsPath()os.UserHomeDir() → caches real home
  • Test 2 calls os.Setenv("HOME", tmpDir) but os.UserHomeDir() still returns cached real home
  • Both tests use the same sessions.json

Fix: Override the path resolution at a higher level before tests run. Either:

  1. Mock config.GetSessionsPath() or inject a custom path resolver in tests, or
  2. Make session path configurable (e.g., add config.SetSessionsPath(path) for testing), or
  3. Use a test helper that properly isolates session storage per test without relying on HOME override.

Do NOT rely solely on t.Setenv("HOME", tmpDir) — it changes the environment variable after the stdlib cache is populated. The root issue is os.UserHomeDir() caching, not config caching.

🧰 Tools
🪛 GitHub Actions: Test

[error] 170-170: IsProcessRunning test failed: expected process to be running but it was not.


[error] 236-236: NumberedSlots_EvictLRU: expected sess-10 to get slot 1 (evicting sess-1), but got slot 3.


[error] 377-377: NumberedSlots_StaleSlotReuse: expected stale slot 3 to be reused, but reused slot 5.


[error] 398-398: GetSessionForSlot: expected 'sess-a' for slot 1, got 'sess-8'; expected empty for unassigned slot 5, got 'sess-a'.

🤖 Prompt for AI Agents
In `@session/metadata_test.go` around lines 126 - 405, Tests are leaking state
because LoadSessions() ultimately calls config.GetSessionsPath() which uses
os.UserHomeDir() (cached by the stdlib) so calling os.Setenv("HOME", tmpDir) in
each test is too late; fix by making the sessions path injectable for tests and
use that instead of relying on HOME: add a configurable hook or setter (e.g.,
config.SetSessionsPath or pass an explicit path into
LoadSessions/LoadSessionsWithPath) and update tests (references: LoadSessions,
config.GetSessionsPath, os.UserHomeDir) to call the setter or the new
LoadSessionsWithPath with tmpDir before interacting with the store so each test
uses an isolated sessions.json. Ensure tests no longer depend on Setenv to
control HOME.

@jfox85 jfox85 merged commit 591227f into main Feb 15, 2026
23 checks passed
@jfox85 jfox85 deleted the jf-faster-jumping branch February 15, 2026 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant