Add cross-directory session resume via global registry#192
Conversation
This commit addresses multiple issues with session resuming: 1. **Fix paste support in file path input**: The EpicLoaderOverlay now accepts multi-character input sequences, allowing users to paste file paths instead of typing them character by character. 2. **Add session registry for cross-directory resume**: Sessions are now registered in a global registry (~/.config/ralph-tui/sessions.json), enabling users to: - List all resumable sessions with `ralph-tui resume --list` - Resume sessions by ID from any directory - Clean up stale entries with `ralph-tui resume --cleanup` 3. **Improve error messages**: When no session is found, the command now shows helpful suggestions including available sessions from other directories. Fixes #175
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #192 +/- ##
==========================================
- Coverage 44.91% 44.85% -0.07%
==========================================
Files 76 78 +2
Lines 22091 22872 +781
==========================================
+ Hits 9923 10260 +337
- Misses 12168 12612 +444
🚀 New features to boost your workflow:
|
Add tests for: - listAllSessions function - getRegistryFilePath function - cleanupStaleRegistryEntries edge case (no stale entries) Increases function coverage to 100%.
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughAdds a durable, locked filesystem-backed session registry (~/.config/ralph-tui/sessions.json) and integrates it into run/resume flows. Exposes registry APIs (register/update/unregister and queries), enables cross-directory resume via session-id with listing/cleanup flags, and adds comprehensive tests and docs. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant RunCmd as Run Command
participant Registry as Session Registry
participant ResumeCmd as Resume Command
User->>RunCmd: start run
RunCmd->>RunCmd: persist initial session state
RunCmd->>Registry: registerSession(sessionId, cwd, status: running)
Registry-->>RunCmd: ack
alt mid-run save
RunCmd->>Registry: updateRegistryStatus(sessionId, status)
Registry-->>RunCmd: ack
else error
RunCmd->>Registry: updateRegistryStatus(sessionId, failed)
Registry-->>RunCmd: ack
else completion
RunCmd->>Registry: unregisterSession(sessionId)
Registry-->>RunCmd: ack
end
User->>ResumeCmd: resume --list
ResumeCmd->>Registry: listResumableSessions()
Registry-->>ResumeCmd: sessions list
ResumeCmd-->>User: display sessions
User->>ResumeCmd: resume <session-id>
ResumeCmd->>Registry: getSessionById(session-id) or findSessionsByPrefix
Registry-->>ResumeCmd: session entry(ies)
ResumeCmd->>ResumeCmd: resolve session path/state
ResumeCmd->>RunCmd: launch engine with resolved session
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/session/registry.ts`:
- Around line 80-116: The registry directory and file are created with default
permissive Unix modes; update ensureRegistryDir to call mkdir with mode: 0o700
(and keep recursive: true) and update saveRegistry to write the file with a
restrictive mode: 0o600 when calling writeFile; additionally, after creation (in
ensureRegistryDir and saveRegistry) ensure existing items are corrected by
calling fs.chmod on REGISTRY_DIR and the path returned by getRegistryPath() to
enforce 0o700 and 0o600 respectively so permissions are correct even if the
directory/file already existed.
- Around line 113-152: The registry mutations (registerSession,
updateRegistryStatus, unregisterSession) must acquire a file-level lock around
the read-modify-write sequence to avoid concurrent clobbering; implement a lock
(advisory flock or lockfile) around loadRegistry -> modify registry.sessions ->
saveRegistry and ensure the lock is always released even on errors. Replace the
current writeFile usage in saveRegistry with an atomic-write pattern: write JSON
to a temp file in the same directory as getRegistryPath(), fsync the temp file
(and its parent dir if possible), then rename the temp file over the target path
to ensure atomic replace and durability; ensure errors during write/rename clean
up the temp file. Keep ensureRegistryDir and getRegistryPath for locating files,
and integrate locking/unlocking with these functions so all registry mutations
are serialized across processes.
🧹 Nitpick comments (3)
src/session/registry.test.ts (2)
22-24: Consider test isolation for registry tests.The comment correctly notes that tests modify the actual registry file. This could cause issues if tests run in parallel or if the developer has real sessions. Consider using dependency injection or a test-specific registry path for better isolation.
276-286: Weak assertion may cause flaky tests.The assertion
expect(cleaned).toBeGreaterThanOrEqual(1)is weak because other sessions from concurrent tests or previous runs might exist. Since you registersessionId2and mock it as non-existent, you should be able to assert exactly what was cleaned.🔧 Suggested fix
- expect(cleaned).toBeGreaterThanOrEqual(1); + // We registered sessionId2 and mocked /tmp/gone as non-existent + // so at minimum 1 entry should be cleaned + expect(cleaned).toBeGreaterThanOrEqual(1); + // Verify sessionId2 was specifically removedThe current assertion is acceptable given the shared test environment, but documenting the rationale would improve clarity.
src/commands/resume.tsx (1)
229-246: Minor inefficiency: duplicate registry lookup.When
hasPersistedSessionreturns true (line 231),getSessionByCwdis called (line 233). When it returns false,getSessionByCwdis called again (line 238). This results in two registry loads for the case where the session file is missing but registry entry exists.This is functionally correct but could be optimised by moving the registry lookup earlier and reusing the result.
- Add restrictive file permissions (0o700 for dir, 0o600 for file) - Implement file-level locking using O_EXCL for cross-process safety - Add atomic write pattern (temp file + fsync + rename) for durability - Ensure permissions are enforced on existing files via chmod - Add comprehensive tests for concurrent operations and data integrity The registry now properly handles: - Multiple processes accessing the registry simultaneously - Crash recovery with stale lock timeout detection - Atomic file updates to prevent corruption - Secure file permissions to protect session data
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/session/registry.ts`:
- Around line 175-192: The validation in loadRegistryInternal accepts null or
arrays because it only checks typeof parsed.sessions === 'object'; update the
condition to ensure parsed.sessions is a plain, non-null object (e.g.,
parsed.version === 1 && parsed.sessions && !Array.isArray(parsed.sessions)
and/or check Object.prototype.toString.call(parsed.sessions) === '[object
Object]') so that loadRegistryInternal returns a clean { version: 1, sessions:
{} } when sessions is null/array/other non-plain types; adjust the parsed
validation and keep the same fallback behavior.
- Around line 122-154: The acquireLock loop currently swallows all errors from
unlink(lockPath) when attempting to remove a stale lock, which allows an
infinite hang if unlink fails (e.g., read‑only FS); modify the stale‑lock branch
inside acquireLock to capture the unlink error and if unlink fails with anything
other than ENOENT (or if unlink fails after LOCK_TIMEOUT_MS) rethrow or return a
clear error so the caller won't loop forever; reference acquireLock, lockPath,
unlink, LOCK_TIMEOUT_MS and LOCK_RETRY_DELAY_MS when making this change.
- Around line 365-381: The current cleanupStaleRegistryEntries holds
withRegistryLock while awaiting checkSessionExists for each registry.sessions
entry; instead, snapshot registry.sessions (map sessionId -> cwd), release the
lock, run checkSessionExists for each cwd and collect stale sessionIds, then
reacquire withRegistryLock and in a short critical section delete only those
sessionIds whose registry.sessions[sessionId] still exists and still has the
same cwd (to avoid race) and increment cleaned; reference functions/values:
cleanupStaleRegistryEntries, withRegistryLock, registry.sessions,
checkSessionExists, and ensure LOCK_TIMEOUT_MS is not hit by long checks being
done outside the lock.
…erage Export formatSessionEntry, listSessions, cleanupRegistry, and resolveSession functions from resume.tsx to enable direct unit testing. Add tests covering all argument parsing, session entry formatting, session listing with mocked registry, cleanup behavior, and session resolution across all code paths (exact match, prefix match, multiple matches, missing sessions, etc.).
There was a problem hiding this comment.
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)
src/commands/resume.tsx (1)
566-635: Update registry status to “running” on resume start.
Without this, listings can show “paused/interrupted” even while a resumed session is active.🔧 Suggested patch
const resumedState = resumePersistedSession(persistedState); await savePersistedSession(resumedState); + if (registryEntry) { + await updateRegistryStatus(registryEntry.sessionId, 'running'); + } const summary = getSessionSummary(resumedState);
🤖 Fix all issues with AI agents
In `@src/commands/resume.tsx`:
- Around line 193-227: When resolving a session by registry ID in
resolveSession, validate that the session's backing file actually exists before
returning the registry entry: after you obtain the SessionRegistryEntry (from
getSessionById or findSessionsByPrefix), check the session file path referenced
by entry (e.g. entry.sessionFile or entry.cwd-derived path) and if the file is
missing log a clear, specific message (same cleanup guidance used in the generic
load failure path) prompting the user to remove the stale registry entry or
recreate the session, then return null instead of returning the stale
registryEntry; update logging to include args.sessionId and reuse the existing
user-facing cleanup guidance text.
Registry improvements (PR feedback): - loadRegistryInternal: validate sessions is non-null, non-array plain object - acquireLock: throw clear error if stale lock unlink fails (prevent hang) - cleanupStaleRegistryEntries: two-phase approach releases lock during I/O checks to avoid LOCK_TIMEOUT_MS being hit by slow existence checks CI fix (root cause of 10% patch coverage): - Add src/session/ test batch to ci.yml workflow so registry.test.ts runs - Add session.lcov to Codecov upload file list - The registry tests were never executed in CI because src/session/ was missing from the test batch list
When resolveSession finds a registry entry by session ID (exact or prefix match), verify the backing session file still exists at entry.cwd before returning. If the file is missing, log a clear error including the session ID and cleanup guidance, then return null instead of a stale entry.
…QkNwJ Add cross-directory session resume via global registry
Summary
This PR implements a global session registry that enables resuming Ralph TUI sessions from any directory using session IDs, rather than requiring users to be in the project directory or use
--cwd. Sessions are now tracked in~/.config/ralph-tui/sessions.jsonand can be discovered, listed, and resumed by ID.Key Changes
New Session Registry System (
src/session/registry.ts)SessionRegistryto maintain a global index of all sessionsregisterSession,updateRegistryStatus,unregisterSession,getSessionById,getSessionByCwdlistResumableSessions,findSessionsByPrefix,cleanupStaleRegistryEntries~/.config/ralph-tui/sessions.jsonwith version 1 schemaEnhanced Resume Command (
src/commands/resume.tsx)--listflag to display all resumable sessions with status, agent, and tracker info--cleanupflag to remove stale registry entriessession-idargument for cross-directory resumeresolveSession()to intelligently find sessions by ID, prefix, or current directoryUpdated Run Command (
src/commands/run.tsx)registerSession()Documentation Updates (
website/content/docs/cli/resume.mdx)--listand--cleanupoptionsTesting (
src/session/registry.test.ts)Notable Implementation Details
--cwdflag still works; registry is additive functionalityUser Experience Improvements
cdinto project directory to resume sessionsralph-tui resume --listprovides overview of all active workSummary by CodeRabbit
New Features
Bug Fixes / Reliability
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.