Skip to content

feat(history): persist terminal history to SQLite with exit code …#879

Open
shauryagangrade wants to merge 8 commits into
crynta:mainfrom
shauryagangrade:shaurya-g-terminal-history-commit
Open

feat(history): persist terminal history to SQLite with exit code …#879
shauryagangrade wants to merge 8 commits into
crynta:mainfrom
shauryagangrade:shaurya-g-terminal-history-commit

Conversation

@shauryagangrade

@shauryagangrade shauryagangrade commented Jun 26, 2026

Copy link
Copy Markdown

…tracking

What

Add a SQLite-backed persistence layer for block-terminal command history.

- Add rusqlite (bundled) to Cargo.toml
- New db.rs: Db struct with insert, delete, clear, seed, list, trim and full unit test suite including corruption recovery and WAL mode
- Extend HistoryState to hold both in-memory index and Db; seed DB from shell history files on first launch if empty
- history_record now accepts exit_code, session_id, max_entries
- New commands: history_list_full, history_clear, history_delete
- Integration tests in tests/history_persistence.rs
- Frontend history.ts: add HistoryEntry, historyClear, historyDelete, historyListFull; update historyRecord signature
- BlockDecorations calls historyRecord on OSC 133 D (finishBlock) with exit code and leaf id as session id, replacing the old submit-time call
- Add historyMaxEntries (50 000) and historyDbPath to settings store

Why

Closes #853

How

Testing

  • pnpm exec tsc --noEmit clean
  • Manual smoke-test of the affected feature
  • (If you touched src-tauri/) cargo test --locked and cargo clippy --all-targets --locked -- -D warnings clean
  • (If you changed a #[tauri::command] signature) called out below so the FE caller can be updated in lockstep
  • (If UI) tested in pnpm tauri dev
  • Platforms tested: macOS
  • Shells tested (if relevant):

Screenshots / GIFs

Notes for reviewer

Summary by CodeRabbit

  • New Features
    • Added persistent terminal history with full record browsing, search, and pagination.
    • Added actions to clear history and delete individual entries.
    • History now stores exit codes and per-session identifiers.
    • Added settings for configurable history DB path and retention limit.
  • Bug Fixes
    • Improved history persistence, trimming, and corrupted/missing storage recovery.
    • Updated history recording flow to ensure commands are persisted with correct metadata.
  • Tests
    • Added integration tests covering persistence, paging, trimming, clearing, filtering, and recovery.

…cking

    Add a SQLite-backed persistence layer for block-terminal command history.

    - Add rusqlite (bundled) to Cargo.toml
    - New db.rs: Db struct with insert, delete, clear, seed, list, trim and
      full unit test suite including corruption recovery and WAL mode
    - Extend HistoryState to hold both in-memory index and Db; seed DB from
      shell history files on first launch if empty
    - history_record now accepts exit_code, session_id, max_entries
    - New commands: history_list_full, history_clear, history_delete
    - Integration tests in tests/history_persistence.rs
    - Frontend history.ts: add HistoryEntry, historyClear, historyDelete,
      historyListFull; update historyRecord signature
    - BlockDecorations calls historyRecord on OSC 133 D (finishBlock) with
      exit code and leaf id as session id, replacing the old submit-time call
    - Add historyMaxEntries (50 000) and historyDbPath to settings store
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

SQLite-backed terminal history is added in Tauri, with new persistence, query, and mutation commands. Frontend history settings and helpers now carry database path, entry limits, session ids, and full-row history data. Command recording moves from submit time to block completion.

Changes

Terminal history persistence

Layer / File(s) Summary
SQLite history store
src-tauri/Cargo.toml, src-tauri/src/modules/history/db.rs, src-tauri/tests/history_persistence.rs
Adds the SQLite history database wrapper, schema and recovery logic, CRUD/query/trim helpers, and persistence coverage.
History state and Tauri commands
src-tauri/src/modules/history/mod.rs, src-tauri/src/lib.rs
Refactors history state to hold optional index and database handles, seeds the database during initialization, adds DB-backed list/record/clear/delete commands, and registers the new Tauri handlers.
History preferences and client API
src/modules/settings/store.ts, src/modules/terminal/block/lib/history.ts
Adds history storage settings to preferences, reads and writes the new keys, adds setters and change propagation, and expands the frontend history helpers for full-list, clear, delete, and metadata-aware record calls.
Block completion recording
src/modules/terminal/block/ShellInput.tsx, src/modules/terminal/block/lib/blockDecorations.ts, src/modules/terminal/lib/useTerminalSession.ts
Removes direct command recording from shell submit, passes a session id into block decorations, and records completed blocks with exit code, session id, and the configured history limit.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Conventional Commits format is used and the title accurately summarizes the SQLite history persistence change.
Linked Issues check ✅ Passed The PR implements persistent terminal history across sessions and exposes clear/delete behavior as requested in #853.
Out of Scope Changes check ✅ Passed All visible changes support the history persistence feature; no unrelated code changes stand out.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@shauryagangrade shauryagangrade marked this pull request as ready for review June 26, 2026 08:15
@shauryagangrade shauryagangrade requested a review from crynta as a code owner June 26, 2026 08:15

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 5

Caution

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

⚠️ Outside diff range comments (1)
src-tauri/src/modules/history/mod.rs (1)

145-185: 🎯 Functional Correctness | 🟠 Major

Wire history settings into initialization

ensure() still hard-codes ("", 50_000), so historyDbPath never changes the DB location and historyMaxEntries only applies on later history_record calls. The first open can seed an oversized DB and leave it above the cap until a new command is recorded. Thread the prefs through startup, or trim immediately after seeding.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/modules/history/mod.rs` around lines 145 - 185, The history
initialization in ensure() still hard-codes the default config, so the database
path and max-entry limit from user settings are not applied during first open.
Update the startup flow so ensure_with_config() receives the actual
historyDbPath and historyMaxEntries preferences, and either pass them through
ensure() or call trimming logic immediately after seeding in Db::open/seed
handling. Keep the fix localized around ensure(), ensure_with_config(), and the
Db initialization path so the first load honors the configured location and cap.
🧹 Nitpick comments (3)
src-tauri/Cargo.toml (1)

50-50: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

bundled SQLite is a real chunk of the bundle budget.

The bundled feature compiles the full SQLite amalgamation into the binary (statically), which is the right call for cross-platform reliability but adds a non-trivial amount to a product that prides itself on ~7-8 MB. Just confirm this is a deliberate trade vs. linking the system libsqlite3 (features = []) on platforms that ship it. If bundled stays, that's defensible, only wanted it to be a decision rather than a default.

As per coding guidelines: "Question any new crate on bundle-size grounds."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/Cargo.toml` at line 50, The rusqlite dependency in the Tauri Cargo
manifest is using the bundled SQLite build, which increases binary size
significantly. Confirm whether this is an intentional tradeoff in the dependency
declaration and, if not, switch the rusqlite configuration to avoid bundling
SQLite so it can use the system libsqlite3 on supported platforms; keep the
current setup only if the bundle-size impact is explicitly desired.

Source: Coding guidelines

src-tauri/tests/history_persistence.rs (1)

95-113: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

disk_write_failure_graceful asserts almost nothing.

It accepts both Ok and Err and only checks "no panic", but Db::open will happily create history.db under the blocker file path only if the OS rejects it; on some filesystems/permissions this may even succeed. The test's intent (graceful failure) is fine, but consider asserting the specific outcome you actually rely on at the call site (caller treats Err as "no db, in-memory only"), otherwise this passes regardless of regressions in the failure path.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/tests/history_persistence.rs` around lines 95 - 113, The
disk_write_failure_graceful test in history_persistence.rs is too weak because
it only checks that Db::open does not panic and accepts both success and
failure. Tighten the assertion around Db::open so the test verifies the specific
failure behavior the caller depends on (that an invalid path returns Err and is
handled as “no db, in-memory only”), using the existing
disk_write_failure_graceful and Db::open symbols to locate the test.
src/modules/terminal/block/lib/blockDecorations.ts (1)

533-534: 🔒 Security & Privacy | 🔵 Trivial

Every completed block is now persisted verbatim to a plaintext SQLite file, secrets and all.

finishBlock fires for any OSC 133 D, so inline-secret commands (mysql -ppassword, export OPENAI_API_KEY=..., curl -H "Authorization: Bearer ...") land on disk unredacted. This is parity with ~/.bash_history, so it isn't a new hole per se, but it runs against the project's strong "secrets never touch disk / deny-list on every fs boundary" posture, and unlike a shell history file it is not behind shell-level HISTIGNORE/space-prefix conventions. Worth a deliberate decision before merge: a minimal secret-pattern skip (mirroring ai/lib/security.ts's spirit), or at least a documented note that history is plaintext and where the DB lives.

Separately, note the behavior shift this introduces: recording moved from submit-time to completion, so a command that never emits D (tab closed mid-run, killed interactive REPL) is now silently absent from history. Acceptable, but it is a coverage change beyond "same data, new store".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/modules/terminal/block/lib/blockDecorations.ts` around lines 533 - 534,
`finishBlock` currently writes every completed command verbatim via
`historyRecord`, so secret-bearing commands can land unredacted on disk. Before
calling `historyRecord`, add a lightweight secret check/redaction path (aligned
with the security filtering used elsewhere, e.g. `ai/lib/security.ts`) so
obvious credential patterns are skipped or sanitized. Also review the
completion-time persistence change in `blockDecorations.ts` to confirm it’s the
intended behavior and, if not, restore the prior history capture semantics
around `historyRecord`/`usePreferencesStore.getState().historyMaxEntries`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src-tauri/src/modules/history/db.rs`:
- Around line 108-126: The history search in the query path that builds the
`LIKE` pattern with `format!("%{}%", query.to_lowercase())` should escape user
input so `%`, `_`, and `\` are treated literally; add an `ESCAPE` clause in the
`self.conn.prepare(...)` SQL and apply escaping before binding the pattern in
`query_map`. Also review the `idx_history_cmd` indexing strategy for this path,
since `lower(command) LIKE '%...%'` cannot use it; either remove that index or
leave it only if you intentionally accept the full scan.
- Around line 130-140: The dedup logic in seed is ineffective because INSERT OR
IGNORE never triggers without a unique constraint on the history table. Update
the history schema in the Db setup/migration code to enforce uniqueness on
command and timestamp (or add a unique index), so seed genuinely skips
duplicates, and keep the seed method aligned with that behavior; if you choose
not to enforce uniqueness, remove or rewrite the misleading dedup
comment/docstring around seed.
- Around line 23-39: `Db::open` only retries on `Connection::open()`, but a
corrupt SQLite file can still fail later in `init()` and skip the recovery path.
Update `open` so the recovery/rotation logic wraps the full initialization flow
(`Connection::open` plus `db.init()`), and if anything in that path fails,
rename the main database to `.bak` and reopen cleanly. Also handle the SQLite
sidecar files associated with `open`/`rename` by removing or rotating the `-wal`
and `-shm` files alongside the main database before retrying.

In `@src-tauri/src/modules/history/mod.rs`:
- Around line 327-337: `history_delete` removes the row from the database but
leaves `HistoryState`’s in-memory `index` stale, so deleted commands can still
appear in completions. Update `history_delete` to also remove or decrement the
corresponding command entry in `inner.index` after `db.delete(id)` succeeds;
since the index is command-keyed, first look up the deleted row’s command string
from the same state/storage path used by `ensure`/`HistoryState`, then apply the
index update consistently. If the command text can’t be recovered here, adjust
the surrounding history APIs so `history_delete` receives or can resolve the
command before deleting.

In `@src/modules/settings/store.ts`:
- Around line 694-696: The `setHistoryMaxEntries` clamp currently allows `0`,
which later flows through `KEY_HISTORY_MAX_ENTRIES` into `db.trim(max)` and can
wipe history on write. Update the clamping logic in `setHistoryMaxEntries` so
`historyMaxEntries` cannot be stored as `0`—either floor it to at least `1` or
introduce a documented unlimited sentinel and handle it consistently where the
setting is read and applied.

---

Outside diff comments:
In `@src-tauri/src/modules/history/mod.rs`:
- Around line 145-185: The history initialization in ensure() still hard-codes
the default config, so the database path and max-entry limit from user settings
are not applied during first open. Update the startup flow so
ensure_with_config() receives the actual historyDbPath and historyMaxEntries
preferences, and either pass them through ensure() or call trimming logic
immediately after seeding in Db::open/seed handling. Keep the fix localized
around ensure(), ensure_with_config(), and the Db initialization path so the
first load honors the configured location and cap.

---

Nitpick comments:
In `@src-tauri/Cargo.toml`:
- Line 50: The rusqlite dependency in the Tauri Cargo manifest is using the
bundled SQLite build, which increases binary size significantly. Confirm whether
this is an intentional tradeoff in the dependency declaration and, if not,
switch the rusqlite configuration to avoid bundling SQLite so it can use the
system libsqlite3 on supported platforms; keep the current setup only if the
bundle-size impact is explicitly desired.

In `@src-tauri/tests/history_persistence.rs`:
- Around line 95-113: The disk_write_failure_graceful test in
history_persistence.rs is too weak because it only checks that Db::open does not
panic and accepts both success and failure. Tighten the assertion around
Db::open so the test verifies the specific failure behavior the caller depends
on (that an invalid path returns Err and is handled as “no db, in-memory only”),
using the existing disk_write_failure_graceful and Db::open symbols to locate
the test.

In `@src/modules/terminal/block/lib/blockDecorations.ts`:
- Around line 533-534: `finishBlock` currently writes every completed command
verbatim via `historyRecord`, so secret-bearing commands can land unredacted on
disk. Before calling `historyRecord`, add a lightweight secret check/redaction
path (aligned with the security filtering used elsewhere, e.g.
`ai/lib/security.ts`) so obvious credential patterns are skipped or sanitized.
Also review the completion-time persistence change in `blockDecorations.ts` to
confirm it’s the intended behavior and, if not, restore the prior history
capture semantics around
`historyRecord`/`usePreferencesStore.getState().historyMaxEntries`.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: e7c134ec-1b7b-4299-8742-8efc7e98f0a2

📥 Commits

Reviewing files that changed from the base of the PR and between bba1b5f and 255ba66.

📒 Files selected for processing (10)
  • src-tauri/Cargo.toml
  • src-tauri/src/lib.rs
  • src-tauri/src/modules/history/db.rs
  • src-tauri/src/modules/history/mod.rs
  • src-tauri/tests/history_persistence.rs
  • src/modules/settings/store.ts
  • src/modules/terminal/block/ShellInput.tsx
  • src/modules/terminal/block/lib/blockDecorations.ts
  • src/modules/terminal/block/lib/history.ts
  • src/modules/terminal/lib/useTerminalSession.ts
💤 Files with no reviewable changes (1)
  • src/modules/terminal/block/ShellInput.tsx

Comment thread src-tauri/src/modules/history/db.rs
Comment thread src-tauri/src/modules/history/db.rs Outdated
Comment thread src-tauri/src/modules/history/db.rs
Comment thread src-tauri/src/modules/history/mod.rs
Comment thread src/modules/settings/store.ts
@shauryagangrade

Copy link
Copy Markdown
Author

@coderabbitai help

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

CodeRabbit commands

These commands are invoked using PR/Issue comments.

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai rate limit to show your current review rate limit status.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai evaluate custom pre-merge check --instructions <custom-checks-instructions> --name <custom-checks-title> [--mode <error|warning>] to validate the custom pre-merge checks instructions. Defaults to error when --mode is omitted.
  • @coderabbitai ignore pre-merge checks to override pre-merge checks and get an approval on PR.
  • @coderabbitai run pre-merge checks to run pre-merge checks on the pull request.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai emit path instructions to emit generated path-specific review instructions for this repository.
  • @coderabbitai resolve merge conflict to automatically resolve merge conflicts.
  • @coderabbitai autofix to automatically fix issues identified in unresolved review comments.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai generate configuration to create a PR that adds the current resolved configuration as .coderabbit.yaml (or show it if already present).
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore or @coderabbit ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, support, documentation and community

  • Visit our status page to check the current availability of CodeRabbit.
  • Create a ticket on our support page for assistance with any issues or questions.
  • Visit our documentation site for detailed information on how to use CodeRabbit.
  • Join our Discord community to connect with other users and get help from the community.
  • Follow us on X/Twitter for updates and announcements.

@shauryagangrade

Copy link
Copy Markdown
Author

@crynta please review this pr

… and redact credentials

    - SQLite DB open recovery now wraps both open and schema initialization, rotating corrupt databases and removing `-
  wal`/`-shm` sidecar files.
    - Added a UNIQUE constraint on command + timestamp in the history table schema and removed the redundant
  `idx_history_cmd` index.
    - Escaped input characters (`%`, `_`, `\`) in search queries and added an ESCAPE clause to the LIKE query.
    - Updated `ensure()` to load actual `historyDbPath` and `historyMaxEntries` preferences from the Tauri Store on first run
  and run initial trimming.
    - Updated `history_delete` to retrieve the command string and update/decrement it in the in-memory autocomplete index
  after DB deletion succeeds.
    - Enforced a minimum value of `1` for the `historyMaxEntries` preference.
    - Redacted sensitive credential patterns in terminal command history before recording.
    - Tightened `disk_write_failure_graceful` test assertions to verify that opening an invalid path returns an `Err`.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Caution

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

⚠️ Outside diff range comments (3)
src-tauri/src/modules/history/mod.rs (3)

295-310: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Redact before persisting at the backend boundary.

history_record writes the raw IPC command to SQLite and the in-memory index; caller-side redaction would still leave this persistence boundary unsafe if another frontend path invokes it. As per path instructions, "validate every boundary for IPC, filesystem, network, and AI tool surfaces."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/modules/history/mod.rs` around lines 295 - 310,
`history_record` is persisting the raw IPC `command` before any backend-side
sanitization, so the persistence boundary remains unsafe; update this function
to redact or normalize sensitive command content immediately after trimming and
before writing to SQLite or the in-memory index. Keep the fix inside
`history_record` (and any helper it uses, like the `db.insert`/index update
path) so every caller is protected even if a frontend forgets to redact.

Source: Path instructions


304-310: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Avoid second-resolution collisions in persisted history.

now() returns seconds and Db::insert uses INSERT OR IGNORE under UNIQUE(command, timestamp), so repeating the same short command twice in one second silently drops the later row from SQLite.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/modules/history/mod.rs` around lines 304 - 310, The persisted
history write path in the history module is vulnerable to collisions because
history timestamping uses now() at second granularity and Db::insert relies on
UNIQUE(command, timestamp) with INSERT OR IGNORE. Update the persistence flow
around the history insertion logic to use a higher-resolution or otherwise
unique timestamp/source in the code path that feeds Db::insert, or include an
additional disambiguating field in the uniqueness key, so rapid repeated
commands are not silently dropped.

174-185: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make SQLite the source of truth after the first seed.

The index is built from shell files before DB load, and any empty DB is reseeded, so persisted Terax-only commands will not restore into suggestions while history_clear can repopulate from shell history on next launch. Seed only on first DB creation, then build Index from db.load_all().

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/src/modules/history/mod.rs` around lines 174 - 185, The history
startup flow in read_histories/build_index currently treats shell files as the
primary source and reseeds any empty DB, which prevents SQLite from being the
long-term source of truth. Change the initialization in history::mod so Db::open
only seeds from shell history on first DB creation, and for subsequent launches
always populate Index from db.load_all() rather than read_histories(). Keep
scan_path() as-is for path commands, but ensure the Index entries come from the
database after the initial seed so Terax-only commands persist and history_clear
does not get overwritten by shell history.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src-tauri/src/modules/history/mod.rs`:
- Around line 295-310: `history_record` is persisting the raw IPC `command`
before any backend-side sanitization, so the persistence boundary remains
unsafe; update this function to redact or normalize sensitive command content
immediately after trimming and before writing to SQLite or the in-memory index.
Keep the fix inside `history_record` (and any helper it uses, like the
`db.insert`/index update path) so every caller is protected even if a frontend
forgets to redact.
- Around line 304-310: The persisted history write path in the history module is
vulnerable to collisions because history timestamping uses now() at second
granularity and Db::insert relies on UNIQUE(command, timestamp) with INSERT OR
IGNORE. Update the persistence flow around the history insertion logic to use a
higher-resolution or otherwise unique timestamp/source in the code path that
feeds Db::insert, or include an additional disambiguating field in the
uniqueness key, so rapid repeated commands are not silently dropped.
- Around line 174-185: The history startup flow in read_histories/build_index
currently treats shell files as the primary source and reseeds any empty DB,
which prevents SQLite from being the long-term source of truth. Change the
initialization in history::mod so Db::open only seeds from shell history on
first DB creation, and for subsequent launches always populate Index from
db.load_all() rather than read_histories(). Keep scan_path() as-is for path
commands, but ensure the Index entries come from the database after the initial
seed so Terax-only commands persist and history_clear does not get overwritten
by shell history.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f6c2ea65-613b-4354-8609-1ad9cb716c28

📥 Commits

Reviewing files that changed from the base of the PR and between 255ba66 and 192a4dc.

📒 Files selected for processing (5)
  • src-tauri/src/modules/history/db.rs
  • src-tauri/src/modules/history/mod.rs
  • src-tauri/tests/history_persistence.rs
  • src/modules/settings/store.ts
  • src/modules/terminal/block/lib/blockDecorations.ts
🚧 Files skipped from review as they are similar to previous changes (4)
  • src-tauri/tests/history_persistence.rs
  • src/modules/terminal/block/lib/blockDecorations.ts
  • src/modules/settings/store.ts
  • src-tauri/src/modules/history/db.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/modules/settings/store.ts`:
- Around line 751-755: The store helpers are syntactically broken because
setHistoryDbPath never closes, causing setDefaultWorkspaceEnv to be parsed
inside it. Add the missing closing brace after the writePref call in
setHistoryDbPath, and verify both setHistoryDbPath and setDefaultWorkspaceEnv
remain separate top-level functions in store.ts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 93469286-2023-40c6-81fd-e8a58c115822

📥 Commits

Reviewing files that changed from the base of the PR and between 192a4dc and 997397d.

📒 Files selected for processing (3)
  • src-tauri/Cargo.toml
  • src-tauri/src/lib.rs
  • src/modules/settings/store.ts
✅ Files skipped from review due to trivial changes (1)
  • src-tauri/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
  • src-tauri/src/lib.rs

Comment thread src/modules/settings/store.ts
… seed index from DB

    - Added `regex` to `Cargo.toml` dependencies.
    - Implemented a backend-side `redact_command` helper in `mod.rs` to sanitize credentials and tokens from
  commands before inserting them into SQLite or the autocomplete index.
    - Switched the `now()` timestamp helper from second to millisecond resolution to prevent database index
  collisions on rapid sequential commands.
    - Updated `ensure_with_config` to treat the database as the long-term source of truth, loading the
  autocomplete `Index` from `db.load_all()` on subsequent boots and only seeding from shell history if the
  database is brand new.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src-tauri/Cargo.toml (1)

50-50: 🚀 Performance & Scalability | 🔵 Trivial

Keep the bundled SQLite setup, but measure the binary impact. rusqlite is already used in src/modules/history/db.rs, so bundled buys portability and avoids system SQLite drift, but it does statically add SQLite to the release binary. Make sure the final artifact still fits the 7-8 MB target.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src-tauri/Cargo.toml` at line 50, Keep the rusqlite bundled SQLite
configuration, but verify the release artifact size impact introduced by the
bundled feature. Check the build output for the Tauri binary after the rusqlite
dependency in Cargo.toml and ensure the resulting size still stays within the
7-8 MB target; if it exceeds that range, adjust the release build settings or
dependency choices while preserving the SQLite functionality used by
src/modules/history/db.rs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src-tauri/Cargo.toml`:
- Around line 51-55: Resolve the unresolved merge conflict in the Cargo manifest
so the crate parses again: remove the literal conflict markers from Cargo.toml
and keep only the final dependency set. The symbols to inspect are the
dependency entries around which and regex, plus the history-related usage in
src-tauri/src/modules/history/mod.rs; retain regex if it is required there, and
only keep which if it is still referenced elsewhere in the crate. Ensure the
manifest contains a single valid TOML declaration with no HEAD or commit-marker
text.

---

Nitpick comments:
In `@src-tauri/Cargo.toml`:
- Line 50: Keep the rusqlite bundled SQLite configuration, but verify the
release artifact size impact introduced by the bundled feature. Check the build
output for the Tauri binary after the rusqlite dependency in Cargo.toml and
ensure the resulting size still stays within the 7-8 MB target; if it exceeds
that range, adjust the release build settings or dependency choices while
preserving the SQLite functionality used by src/modules/history/db.rs.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 75f542a9-113f-4157-8bad-1ac2eba404bb

📥 Commits

Reviewing files that changed from the base of the PR and between 997397d and 7fcf4dc.

📒 Files selected for processing (2)
  • src-tauri/Cargo.toml
  • src-tauri/src/modules/history/mod.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • src-tauri/src/modules/history/mod.rs

Comment thread src-tauri/Cargo.toml Outdated
    Resolved the merge conflict in `src-tauri/Cargo.toml` by retaining both the `which` and `regex` dependencies,
  as they are both active and required by the LSP and history modules respectively.
@shauryagangrade

shauryagangrade commented Jul 3, 2026

Copy link
Copy Markdown
Author

Hello @crynta

I've worked on this pull request for quite some time, and I'd appreciate it if you or one of your maintainers reviewed it. I completely understand that managing a repo of this scale is no easy job, so I understand if things are taking some time on your end. This is just a gentle reminder.

Thanks,

shaurya

@shauryagangrade

Copy link
Copy Markdown
Author

Hi @crynta ,

Just a quick courtesy update on this PR.

Since opening it, I've incorporated the feedback from CodeRabbit and made a number of follow-up improvements:

  • Added backend-side credential redaction before persisting command history.
  • Switched timestamps to millisecond precision to prevent collisions during rapid command execution.
  • Improved SQLite resilience by recovering from corrupt databases and cleaning up WAL/SHM sidecar files.
  • Applied historyMaxEntries and historyDbPath from settings during startup and performed initial trimming.
  • Escaped special characters in history search queries for correct LIKE matching.
  • Kept the in-memory autocomplete index synchronized when history entries are deleted.
  • Enforced a minimum value of 1 for historyMaxEntries.
  • Rebased the branch onto the latest main and resolved the resulting merge conflicts.

The branch is now up to date with main, and the original functionality of persistent SQLite-backed terminal history remains unchanged. Just wanted to leave a note that the implementation has been kept current while the PR has been open. Thanks!

@crynta

crynta commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Reviewed it in detail, thanks for the work here. The persistence core in db.rs is actually decent, clean,
tested, with corruption recovery and WAL.

A few things hold it back though: it doesn't compile right now (missing brace in store.ts trips tsc), half of
it isn't wired to anything (list_full, clear, delete have no UI or callers), and it pulls in SQLite as a
compiled C dependency which adds 1MB+ to the bundle. That last one is the real blocker for me, the whole point
of Terax is the tiny footprint, and for command history a tauri-store JSON file would do the job without the
weight.

There's also a scope gap: #853 was really asking for the Warp-style thing where you reopen and your previous
blocks are still on screen, and this doesn't restore anything visible.

I might build this out properly later, but not right now. Appreciate the effort though, the db layer itself was
solid work.

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.

terminal history

2 participants