feat: generate and expose session ID for history isolation#133
feat: generate and expose session ID for history isolation#133
Conversation
Use Reedline::create_history_session_id() to generate a nanosecond- precision timestamp as session ID. Pass it to SqliteBackedHistory and the line editor so history entries are tagged per session and search filters can isolate sessions. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Add history_session_id (i64) to SessionInfo (disk), SessionResult (JSON-RPC), HeadlessInfo (--json output), and SessionMeta (cache). In REPL mode the ID is generated once and shared across R/shell history and IPC. In headless mode the field is null since there is no history backend. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
eitsupi
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces a per-process history session identifier to isolate history entries by session, and exposes that identifier through IPC/session metadata so clients can filter history per run.
Changes:
- Generate a
HistorySessionIdat interactive startup and thread it into both R and shellSqliteBackedHistory/Reedlineinstances. - Plumb
history_session_id: Option<i64>through IPC session metadata (diskSessionInfo, JSON-RPCSessionResult, andarf headless --jsonoutput;nullin headless). - Update IPC server startup APIs/callers to accept and cache the optional history session ID.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/arf-console/src/repl/state.rs | Stores the raw history_session_id in REPL state for IPC/meta-command access. |
| crates/arf-console/src/repl/mod.rs | Adds session_id to Repl, configures Reedline + SQLite history with the session ID, and passes raw ID into meta commands. |
| crates/arf-console/src/repl/meta_command.rs | Threads history_session_id into :ipc start so runtime-started IPC includes the same session ID. |
| crates/arf-console/src/main.rs | Generates the session ID at startup, passes it to IPC startup, and includes it in headless --json output. |
| crates/arf-console/src/ipc/session.rs | Adds history_session_id to on-disk session discovery metadata with backward-compatible deserialization. |
| crates/arf-console/src/ipc/server.rs | Extends server startup to accept/capture history_session_id and includes it in SessionInfo. |
| crates/arf-console/src/ipc/protocol.rs | Adds history_session_id to the JSON-RPC SessionResult schema. |
| crates/arf-console/src/ipc/mod.rs | Plumbs history_session_id through IPC start, caches it in SessionMeta, and returns it in session responses. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Reduce visibility of history_session_id_raw() to fn (not pub) - Update HeadlessInfo doc comment to mention history_session_id - Add test for history_session_id propagation through SessionMeta Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
When history is disabled (--no-history or config), generate None instead of a session ID so IPC does not misleadingly advertise history isolation. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Extract create_session_id() helper and add tests verifying that session ID is generated when history is enabled and None when history is disabled. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Update create_session_id() to also check that a history directory is resolvable (matching Repl::r_history_path logic), so IPC does not advertise history isolation when no backend exists. Also update doc comments on SessionInfo and SessionResult to reflect all cases where history_session_id can be null. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- setup_history() now returns a success flag and logs a warning on failure instead of silently falling back - If neither R nor shell history DB opens, clear the session ID from IPC metadata via clear_history_session_id() - ReplState only exposes session ID when at least one history DB was successfully initialized - Update doc comments to reflect all cases where session ID is null Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Apply the same history-failure handling to run_standalone() that was already in run_with_r_mainloop(): clear the IPC session ID and gate the value passed to process_meta_command. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
clear_history_session_id() now updates both the in-memory SessionMeta and the on-disk session file so clients discovering sessions via the file system also see null for history_session_id when history backends failed to open. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Update history_session_id docstring in SessionResult to mention the "no history directory available" case - Add a NOTE comment explaining the brief window where IPC advertises a session ID before history DBs are confirmed open - Guard clear_session_history_id against PID mismatch by forcing info.pid = pid before rewriting the session file Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- clear_history_session_id: skip disk rewrite when value is already None - clear_session_history_id: log non-NotFound read errors instead of silently swallowing them Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Simplify clear_history_session_id to only attempt disk cleanup when IPC was started and the session ID was actually set. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
HistorySessionId) at startup using reedline's nanosecond-timestamp approachSqliteBackedHistoryand the line editor so history entries are tagged per session and search filters can isolate sessionshistory_session_id(i64) in IPC session info:SessionInfo(disk file),SessionResult(JSON-RPC), andHeadlessInfo(--jsonoutput)nullDesign notes
history_session_idas JSON number (i64)The session ID is a nanosecond-precision timestamp (~1.7×10¹⁸), which exceeds JavaScript's
Number.MAX_SAFE_INTEGER(~9.0×10¹⁵). This means JS/TS clients that parse the JSON withJSON.parse()will lose precision in the lower digits.We chose to keep it as a JSON number (
i64) because:HistorySessionId(i64))BigIntor libraries likejson-bigint, or by using a reviver function inJSON.parse()If a future client ecosystem strongly prefers string-typed IDs, a parallel
history_session_id_strfield can be added in a backwards-compatible way.Test plan
cargo check/cargo clippy/cargo fmtpasscreate_session_id()covering disabled history, missing directory, and normal casessession_idcolumn in SQLite history DB is populated (not NULL)arf ipc session | jq .history_session_idreturns a non-null integerarf headless --json | jq .history_session_idreturnsnull🤖 Generated with Claude Code