Skip to content

fix: bulk-delete data loss, resume-cmd injection, time-sort semantics, cold-scan I/O - #58

Merged
subinium merged 7 commits into
mainfrom
fix/audit-functional-perf-timesort
Jul 31, 2026
Merged

fix: bulk-delete data loss, resume-cmd injection, time-sort semantics, cold-scan I/O#58
subinium merged 7 commits into
mainfrom
fix/audit-functional-perf-timesort

Conversation

@subinium

Copy link
Copy Markdown
Owner

Repo-wide audit fixing functional bugs, time-sort correctness, and cold-scan I/O. All 7 commits are independent and logically scoped. cargo test (79 passed), cargo clippy --all-targets -- -D warnings, and cargo fmt --check all pass.

Functional correctness

  • fix(tui): bulk delete could destroy the WRONG sessions. selected_set stored absolute sessions Vec indices captured at Space-toggle time, but ingest_scan_results() runs every render frame and reorders sessions (via merge_agent_sessions + apply_sort) whenever a background scan lands. The stale indices then addressed different sessions, so confirming a bulk delete removed the wrong data — silently. Now keyed by (agent, session_id) and resolved to sessions at delete time.
  • fix(resume): session_id shell injection / broken resume command. Unlike project_path (routed through CommandShell::quote), session_id was wrapped in raw single quotes. An id containing ' broke the eval'd command; a crafted transcript filename could inject shell. Now escaped shell-aware; added POSIX + PowerShell regression tests.
  • fix(scanner/codex): saturate updated_at * 1000 so a corrupt sqlite value can't overflow i64 into a garbage sort key.
  • build: drop panic = "abort" so scan_all's per-thread panic isolation (join → recover) works in release — one malformed session file can't abort the whole listing.

Time-sort semantics (no unit bug — all timestamps are ms; the issue was meaning)

  • fix(scanner): Cursor/pi sorted by creation time, not last activity. Cursor used store.db createdAt and pi used the immutable session-header timestamp, so a session created long ago but used today sorted as old and sank below stale ones. Both transcripts are appended per turn, so now use file mtime (Cursor) / max(header, mtime) (pi) — matching the other agents. Verified end-to-end against on-disk fixtures through the built binary (a recently-used old session now sorts to the top; a freshly-created idle one to the bottom — inverting the pre-fix order).
  • fix(tui): grouped view ordered projects by .first(), which is only the newest under Time sort. Now uses the max timestamp per group (correct under Name/Agent sort too).

Performance (cold-start / large ~/.claude)

  • perf(scanner/claude): shrink recap tail read from 256K → 32K. scan_session_metadata read up to 272K/file to reach the tail, but the tail only feeds the latest away_summary recap (off by default in the list), which is appended last and sits in the final ~1K. Measured on a real 714-file tree: scan read I/O 62.2 MB → 29.0 MB (−53%). Added a regression test asserting the recap still surfaces from a file larger than head+tail.

Notes

🤖 Generated with Claude Code

subinium and others added 7 commits July 31, 2026 14:18
session_id came straight from parsed on-disk files but was wrapped in raw
single quotes when building the resume command (unlike project_path, which
already went through CommandShell::quote). An id containing a single quote
broke the command the shell wrapper eval's, and a crafted transcript filename
could inject shell. Route the id through the shell-aware quote() and thread
the active CommandShell into Agent::resume_cmd.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…time

Cursor used the store.db createdAt and pi used the immutable session-header
timestamp as the sort key, so a session created long ago but used today sorted
as old and sank below stale sessions. Both transcripts are appended on every
turn, so use the file mtime (Cursor) / max(header, mtime) (pi) as the
last-activity time, matching how the other agents' timestamps behave.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scan_session_metadata read up to 272K/file to reach the tail, but the tail is
only used for the latest away_summary recap (off by default in the list) which
is appended last and sits in the final ~1K. On a real ~/.claude/projects tree
(714 files) this cut scan read I/O 62.2MB -> 29.0MB (-53%). Added a regression
test asserting the recap still surfaces from a file larger than head+tail.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
updated_at * 1000 on an untrusted state_*.sqlite value could overflow i64 and
wrap to a garbage timestamp that jumps the session to a list extreme. Saturate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
scanner::scan_all joins per-agent threads and recovers from a panicking one
(join -> Err) so one malformed session file can't take down the whole listing.
panic=abort made that recovery dead code in release builds.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
selected_set stored absolute indices into app.sessions, captured at
Space-toggle time. ingest_scan_results runs every render frame and, when a
background scan lands, merge_agent_sessions + apply_sort reorder app.sessions.
The stored indices then addressed different sessions, so confirming a bulk
delete destroyed the WRONG sessions (silent data loss). Key the set by
(agent, session_id) and resolve to sessions at delete time instead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
build_groups ordered groups by the first session's timestamp, which is only
the newest one under Time sort. In Name/Agent sort the first session is not the
newest, so grouped view (Ctrl+G) ordered projects incorrectly. Use the max
timestamp across each group's sessions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@subinium
subinium merged commit c38e516 into main Jul 31, 2026
4 checks passed
subinium pushed a commit that referenced this pull request Jul 31, 2026
Discover, preview, resume, and delete Yolop sessions. Scans the platform-native
yolop/sessions store, parses workspace.json + bounded head/tail of events.jsonl,
validates session_<32hex> dir names, resumes with 'yolop --session <id>'.

Integrated onto main: rebased onto the shell-escaped resume_cmd signature (#58)
and the (agent, session_id) wiring. Original work by @chaliy (#55).
subinium pushed a commit that referenced this pull request Jul 31, 2026
Register Oh My Pi as a first-class agent. Scans ~/.omp/agent/sessions/*/*.jsonl
reusing the pi JSONL parser (now shared via pi::scan_from), excludes nested
subagent transcripts, and resumes with 'omp --resume <id>' (verified against the
omp CLI: --resume, with --session as an alias).

Integrated onto main: adapted to the shell-escaped resume_cmd signature (#58);
omp inherits the pi last-activity timestamp fix. Original work by @Michelh91 (#56).
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