Skip to content

feat: pin sessions in the /resume picker#508

Open
mwmoreno wants to merge 2 commits into
mpfaffenberger:mainfrom
mwmoreno:feature/pin-resume-sessions
Open

feat: pin sessions in the /resume picker#508
mwmoreno wants to merge 2 commits into
mpfaffenberger:mainfrom
mwmoreno:feature/pin-resume-sessions

Conversation

@mwmoreno

Copy link
Copy Markdown

What

Adds the ability to pin sessions in the /resume (a.k.a. /autosave_load) interactive picker.

  • Highlight any session and press p to pin/unpin it.
  • Pinned sessions float to the top of the list (above the named / auto-flavored grouping), sorted most-recent-first.
  • Pinned rows show a magenta * marker, and the preview pane shows a * Pinned badge.
  • Pinned sessions are protected from automatic cleanup (cleanup_sessions skips them) — otherwise a pin you can lose isn't much of a pin.
  • While typing in / search mode, p still types a literal p (the pin binding is registered after the alphabet bindings and defers to search mode).

Why

Easy to lose track of an important conversation among a long list of autosaves. Pinning keeps your go-to sessions one keystroke away.

How it works

Pin state lives in a dedicated pinned_sessions.json registry in the autosaves directory, intentionally decoupled from each session's _meta.json. The meta files get rewritten on every autosave, which would silently clobber a pin flag on the active session — a single small registry is the one source of truth.

New helpers in session_storage.py:

  • load_pins / save_pins / is_pinned / toggle_pin / get_pins_path
  • save_pins writes through a new _atomic_write_text() helper (unique temp file + fsync + atomic replace) for crash-safety and concurrency-safety.
  • toggle_pin prunes pins whose sessions no longer exist and refuses to pin a nonexistent session, so the registry never accumulates ghosts.

In autosave_menu.py:

  • Ordering is factored into a shared _order_entries() helper so the initial load, search filtering, and live pin-toggle all use one rule.
  • Picker key handlers log failures (logger.warning(..., exc_info=True)) instead of swallowing them silently.

Tests

Added unit tests in tests/test_session_storage.py covering pin round-trip, missing-session refusal, stale-pin pruning, corrupt-registry resilience, and cleanup protection. ruff check + ruff format clean.

(Two unrelated pre-existing failures in test_session_storage_edge_cases.py / test_session_storage_extended.py are Windows-only permission/path quirks present on main before this change.)

@mpfaffenberger

Copy link
Copy Markdown
Owner

Wouldn't this interfere with searching?

@mwmoreno

Copy link
Copy Markdown
Author

Wouldn't this interfere with searching?

@mpfaffenberger if a user has a lot of pinned sessions then their first page or two could be all pinned sessions. After that it would default to normal sort for searching.

@mpfaffenberger

Copy link
Copy Markdown
Owner

Please address quality checks. It's weird, the instructions to do so are in the project's AGENTS.md and yet every once in awhile I see P/Rs without linters run. Can you describe your agent setup for this P/R?

@mwmoreno

Copy link
Copy Markdown
Author

Hi @mpfaffenberger — thanks for the review, and fair questions. Let me address both.

Agent setup (full transparency)

This PR was built using Code Puppy itself (dogfooding), human-directed by me end-to-end — I reviewed every diff, and ran the checks locally before each push via uv. No Claude co-author trailers on either commit (I saw rule #7 in AGENTS.md); both are authored under my name.

Quality checks

I did run the linters per AGENTS.md rule #6, and re-verified just now against the exact commands your quality job uses:

  • ruff check .All checks passed!
  • ruff format --check .793 files already formatted

The CI failure on the first run was not a lint issue — it was a real test failure in tests/command_line/test_autosave_menu.py::test_with_permission_denied_access. My change makes _get_session_entries stamp a pinned flag onto every entry's metadata, so that test's assert entries[0][1] == {} needed to become {"pinned": False}. That's fixed in 6b22a15.

Heads-up: the latest commit's checks are currently sitting in action_required (the first-time-contributor workflow-approval gate), so they haven't re-run yet — they'll need an "Approve and run" from a maintainer to show green.

The architectural question I want to raise honestly

Your Golden Rule is clear: new functionality should be a plugin; don't edit code_puppy/command_line/. This PR does edit command_line/autosave_menu.py, and I don't want to paper over that.

The reason: the feature adds a p keybinding + pin marker inside the existing autosave//resume picker, and I couldn't find a hook in callbacks.py that lets a plugin inject a keybinding or column into that TUI. The data-layer additions (session_storage.py pin registry) are clean core helpers, but the picker wiring has no extension seam today.

So I'd like your guidance on how you'd prefer this land:

  1. Accept the targeted core change to autosave_menu.py as-is, or
  2. Add a small extension point (e.g., a picker-keybinding / session-list-decorator hook) and reimplement the UI half as a plugin against it, or
  3. Something else you have in mind.

Happy to do the rework whichever way you prefer — just don't want to add a core seam unilaterally. Thanks!

@mpfaffenberger

mpfaffenberger commented Jun 29, 2026

Copy link
Copy Markdown
Owner

Go with #1.

Edit: #2 should totally be done as a separate P/R

@mwmoreno

Copy link
Copy Markdown
Author

@mpfaffenberger Sounds good. I'll work that as a separate P/R.

@mwmoreno
mwmoreno force-pushed the feature/pin-resume-sessions branch from 6b22a15 to 4e71c33 Compare July 8, 2026 23:51
mwmoreno added 2 commits July 13, 2026 12:22
Add the ability to pin/unpin autosave sessions from the /resume (a.k.a.

/autosave_load) interactive picker by pressing 'p'. Pinned sessions float

to the top of the list (above the named/auto grouping), show a magenta '*'

marker + '* Pinned' preview badge, and are protected from automatic

cleanup. In search mode 'p' still types normally.

Pin state lives in a dedicated pinned_sessions.json registry in the

autosaves dir (not per-session _meta.json, which autosave rewrites and

would clobber). Writes go through an atomic+fsync helper; toggle_pin prunes

ghost pins and refuses to pin nonexistent sessions. Failures in the picker

key handlers are logged rather than silently swallowed. Includes unit tests.
_get_session_entries now stamps a 'pinned' flag on every entry's metadata,

so the permission-error case yields {'pinned': False} rather than {}.
@mwmoreno
mwmoreno force-pushed the feature/pin-resume-sessions branch from 4e71c33 to 6374599 Compare July 13, 2026 17:27
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.

2 participants