Skip to content

fix(minimald): lead the shell-exit prompt with the files changed since activation - #1123

Merged
norrietaylor merged 3 commits into
mainfrom
feat/exit-prompt-delta
Jul 31, 2026
Merged

fix(minimald): lead the shell-exit prompt with the files changed since activation#1123
norrietaylor merged 3 commits into
mainfrom
feat/exit-prompt-delta

Conversation

@norrietaylor

@norrietaylor norrietaylor commented Jul 31, 2026

Copy link
Copy Markdown
Member

Exiting a session asks keep-or-delete without saying what would be lost; the prompt now leads with the files changed since activation and the delete option says when nothing would be lost.

  • Baseline snapshot (relative path -> len+mtime) taken before the session process launches; re-walk and diff at shell exit.
  • Prompt shows N files changed since activation: with up to 10 A/M/D <path> rows, or No files changed since activation. (delete option gains — nothing will be lost).
  • Best-effort throughout: an unavailable delta renders the previous prompt verbatim; the exit path never blocks on change detection.
  • Covered by unit tests on the snapshot/diff module and an end-to-end prompt test in the session harness.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f

Note

Lead the minimald shell-exit prompt with files changed since activation

  • Adds a new session_delta module that snapshots a session workspace at activation time using walkdir and blake3, then diffs it on shell exit to produce sorted A/M/D rows.
  • The shell-exit prompt in session_host now shows up to 10 changed-file rows with A/M/D markers; if no files changed, the Delete option appends '— nothing will be lost'.
  • Snapshot walk is bounded by a 5s timeout and skips symlink traversal; files over 4 MiB are signed by length and mtime only, without a digest.
  • Risk: if the baseline snapshot times out or fails, delta is left as None and the prompt falls back to the plain (no-diff) display.

Changes since #1123 opened

  • Modified snapshot function in session_delta module to exclude the root-level .git directory from baseline and re-walk operations by adding a filter_entry that rejects entries where depth equals 1 and file_name equals ".git", while preserving nested .git directories as ordinary content [90ba487]
  • Refactored shell-exit prompt logic in Binding.run method within session_host module by extracting inline prompt code into a new generic async associated function Binding.shell_exit_prompt that computes changed files via DeltaSource, renders either a no-changes banner or a capped list of changed files, and presents a two-item selection menu for Exit or Delete actions [90ba487]

Macroscope summarized 3767383.

Summary by CodeRabbit

  • New Features

    • Shell-exit prompts now show workspace files added, modified, or deleted during a session.
    • Changes are detected reliably even when file size and timestamps remain unchanged.
    • Changed files are listed consistently, with long lists limited for readability.
    • Cleanup options update based on whether changes are present.
  • Bug Fixes

    • Sessions continue to exit safely when workspace change information cannot be collected.
    • Unchanged workspaces no longer display unnecessary file-change details.
    • Symbolic links are handled safely without scanning their targets.

…e activation

The keep-or-delete prompt asked users to decide without saying what a
delete would lose. The host now baselines the workspace before the
session process launches and the prompt lists the delta: added,
modified, and deleted files (capped at 10 rows), or states that nothing
changed — in which case the delete option says so. Change detection is
best-effort: an unwalkable workspace or a failed re-walk renders the
plain prompt and never blocks exit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f
@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The session host captures a workspace baseline before launch. At shell exit, it detects added, modified, and deleted files and displays the results in the exit prompt. Tests cover snapshot behavior, symlinks, content changes, and session integration.

Changes

Workspace delta reporting

Layer / File(s) Summary
Workspace snapshot and diff detection
crates/minimald/src/lib.rs, crates/minimald/src/session_delta.rs, crates/minimald/Cargo.toml
DeltaSource records bounded file signatures, compares workspace snapshots, handles failures and timeouts, sorts changed paths, and tests regular-file and symlink behavior.
Session baseline and binding wiring
crates/minimald/src/session_host.rs
Host arms tracking before launch and passes the shared delta source to bindings.
Shell-exit change prompt
crates/minimald/src/session_host.rs, crates/minimald/src/session.rs
The exit dialog lists changes, limits rows, updates the delete option, and is covered by an integration test.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Host
  participant DeltaSource
  participant Binding
  participant ExitPrompt
  Host->>DeltaSource: Capture workspace baseline
  Host->>Binding: Provide shared delta source
  Binding->>DeltaSource: Read changes after shell exit
  DeltaSource-->>Binding: Return sorted changed paths
  Binding->>ExitPrompt: Display changes and action options
Loading

Possibly related issues

  • gominimal/inbox#436 — Covers workspace-delta tracking and session-exit prompt behavior implemented by this change.

Suggested labels: needs-human

Suggested reviewers: twitchyliquid64

Poem

A rabbit scans the workspace ground,
And finds each changed file around.
Added, edited, deleted too,
The exit prompt lists them for you.
Keep the session, safe and bright.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change to the minimald shell-exit prompt.
Description check ✅ Passed The description explains the behavior, implementation, fallback cases, and testing coverage, although it omits the template headings and checklist.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/exit-prompt-delta

Comment @coderabbitai help to get the list of available commands.

@norrietaylor
norrietaylor marked this pull request as ready for review July 31, 2026 06:22
@norrietaylor
norrietaylor requested a review from a team as a code owner July 31, 2026 06:22

@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: 3

🤖 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 `@crates/minimald/src/session_delta.rs`:
- Around line 15-19: Update the Sig type and its file-change detection usage to
include an exact content digest, ensuring equal-length edits with preserved
modification times are detected before reporting no files changed or no data was
lost. Compute and compare the digest for each file, preserve deletion detection,
and add a regression test covering same-length content changes with unchanged
mtime.
- Around line 52-60: Bound the blocking filesystem operations in
SessionDelta::changed_files and DeltaSource::arm with the established async
timeout policy, while keeping traversal inside spawn_blocking. Return None or
the existing fallback result when the timeout expires, and preserve normal
results when the task completes before the deadline.
- Around line 72-77: Update the directory traversal loop in the relevant
session-delta walker to inspect each entry’s own file type instead of following
links: only push genuine directories onto the stack, and use symlink metadata
when recording non-directory entries. Add tests covering a directory symlink and
a symlink cycle, verifying traversal stays within the root and terminates.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3f7e7393-4399-4f57-96f7-4116169fb350

📥 Commits

Reviewing files that changed from the base of the PR and between f7dc233 and 44fc12d.

📒 Files selected for processing (4)
  • crates/minimald/src/lib.rs
  • crates/minimald/src/session.rs
  • crates/minimald/src/session_delta.rs
  • crates/minimald/src/session_host.rs

Comment thread crates/minimald/src/session_delta.rs Outdated
Comment thread crates/minimald/src/session_delta.rs Outdated
Comment thread crates/minimald/src/session_delta.rs Outdated
@evanspearman

Copy link
Copy Markdown
Member

Unless there's a reason not to, I would strongly recommend using either the walkdir crate (or ignore if you want a version that respects .gitignore and similar ignore lists) and use WalkParallel for performance.

… exit walk

Review follow-ups: the walk moves to walkdir (no symlink following, no
hand-rolled recursion), file signatures gain a size-capped blake3
content digest so same-length mtime-preserved edits cannot masquerade
as "no files changed", and the exit-time re-walk is bounded by a
timeout that degrades to the plain prompt instead of blocking teardown.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f

@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.

🧹 Nitpick comments (2)
crates/minimald/src/session_delta.rs (2)

99-141: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Consider excluding build/VCS directories from the walk.

The prior review comment about following directory symlinks is resolved: walkdir's default follow_links(false) means entry.file_type().is_dir() at Line 109 never descends into a symlinked directory, and entry.metadata() at Line 114 returns the symlink's own metadata rather than the target's. Verified both behaviors against the walkdir API docs.

One practical concern remains. WalkDir::new(root) at Line 107 has no entry filtering. For a typical Rust or JS workspace, this walks (and, for files under DIGEST_CAP_BYTES, hashes) every entry under target/, .git/, or node_modules/ as well. Those directories can hold tens of thousands of files, which risks exceeding WALK_TIMEOUT (5 seconds) on exactly the active development workspaces this feature targets — degrading it to "detection disabled" most of the time.

Filter out common heavy, non-source directories (or switch to the ignore crate, as suggested in review), so the timeout stays meaningful for real workspaces.

♻️ Example: skip known heavy directories during the walk
 fn snapshot(root: &Path) -> std::io::Result<Snapshot> {
     let mut out = Snapshot::new();
-    for entry in WalkDir::new(root) {
+    for entry in WalkDir::new(root).into_iter().filter_entry(|e| {
+        e.depth() == 0
+            || !matches!(e.file_name().to_str(), Some(".git" | "target" | "node_modules"))
+    }) {
         let entry = entry?;
🤖 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 `@crates/minimald/src/session_delta.rs` around lines 99 - 141, Update snapshot
to prune common heavy non-source directories such as target, .git, and
node_modules while walking, using WalkDir’s directory filtering/pruning
mechanism so their descendants are never visited or hashed. Preserve existing
symlink-safe traversal and snapshot behavior for all remaining entries.

64-97: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Log timeout and error paths for diagnosability.

arm and changed_files both resolve the two prior review comments about bounding the walk with WALK_TIMEOUT. Based on learnings, this correctly keeps the filesystem walk on the blocking pool and bounds the .await.

One gap remains: on timeout, I/O error, or panic, both methods silently return None. Nothing signals which case occurred. If a workspace mount wedges in production, this becomes hard to diagnose from logs alone.

Add a tracing::warn! (or debug!) call at each None-producing path, noting the cause (timeout vs. I/O error vs. panic).

🤖 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 `@crates/minimald/src/session_delta.rs` around lines 64 - 97, Update
DeltaSource::arm and DeltaSource::changed_files to log each failure before
returning None, distinguishing timeout, snapshot I/O errors, and spawned-task
panics. Preserve the existing timeout and Option-based control flow while adding
tracing::warn! or debug! messages with the relevant workspace context and error
details where available.

Source: Learnings

🤖 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.

Nitpick comments:
In `@crates/minimald/src/session_delta.rs`:
- Around line 99-141: Update snapshot to prune common heavy non-source
directories such as target, .git, and node_modules while walking, using
WalkDir’s directory filtering/pruning mechanism so their descendants are never
visited or hashed. Preserve existing symlink-safe traversal and snapshot
behavior for all remaining entries.
- Around line 64-97: Update DeltaSource::arm and DeltaSource::changed_files to
log each failure before returning None, distinguishing timeout, snapshot I/O
errors, and spawned-task panics. Preserve the existing timeout and Option-based
control flow while adding tracing::warn! or debug! messages with the relevant
workspace context and error details where available.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bc95bcb8-8bc8-4f57-8299-be5491fa9d2c

📥 Commits

Reviewing files that changed from the base of the PR and between 44fc12d and 3767383.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • crates/minimald/Cargo.toml
  • crates/minimald/src/session_delta.rs

Comment thread crates/minimald/src/session_host.rs Outdated
// Lead with what a "delete" would lose. An unavailable delta (no
// baseline, or the re-walk failed) renders the plain prompt — the
// exit path never blocks on change detection.
let changed = match &self.delta {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: move the bulk of this to a helper function, the handler here is getting long

Comment thread crates/minimald/src/session_delta.rs Outdated
/// would misreport their contents as added later.
fn snapshot(root: &Path) -> std::io::Result<Snapshot> {
let mut out = Snapshot::new();
for entry in WalkDir::new(root) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

A nice optimization here might be to skip a .git in the root

@norrietaylor

Copy link
Copy Markdown
Member Author

Review follow-ups landed in 3767383 (and merged into feat/exit-prompt-archive as 4cb51be):

  • walkdir adopted for both walks — @evanspearman, ignore was considered and rejected: respecting gitignore would hide changed-but-ignored files from what is fundamentally a data-loss report, and "you're about to lose these files" must include the ignored ones.
  • Exit re-walk is bounded: the spawn_blocking await is wrapped in a 5 s tokio::time::timeout; on expiry the prompt degrades to its plain form and teardown never blocks (the abandoned walk finishes detached on the blocking pool).
  • Symlinks are structurally unfollowed: walkdir's no-follow default means a link out of the root or a link cycle is recorded as an entry, never traversed — covered by a regression test with both an escape link and a cycle.
  • Size-capped blake3 content digest (4 MiB cap) in each file signature, so a same-length mtime-preserved edit can no longer masquerade as "no files changed" — covered by a regression test that rewrites same-length bytes and restores the mtime via File::set_times, then requires an M row.

….git in the delta walk

Review follow-ups: Binding::run's ProcessExited epilogue moves into a
shell_exit_prompt associated fn (byte-identical channel output), and
both workspace walks now skip the root-level .git directory — the delta
reports working-tree files, and git-internal churn (index, refs,
objects) would otherwise flood the capped row list the moment any git
command runs in the session. Nested .git directories (vendored
subrepos) remain ordinary workspace content.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KyZLpkRf9G4A2hUDgDvn5f
@norrietaylor

Copy link
Copy Markdown
Member Author

Thanks @twitchyliquid64 — both addressed in 90ba487 (merged forward to feat/exit-prompt-archive as d6400ee):

  • Prompt handler extracted: Binding::run's ProcessExited epilogue now lives in a shell_exit_prompt associated fn (taking the binding's delta/control capabilities piecewise, since run has already split the channel out of self at that point); channel output is byte-identical and the existing prompt/teardown tests pass unchanged. On the archive branch the helper carries the extra save-then-delete arm.
  • Root .git skipped in the delta walk: both the baseline and exit walks (one shared walk fn) now filter a depth-1 .git entry, so any in-session git operation no longer floods the capped row list with index/refs/objects churn — the documented tradeoff is that the delta (and the save archive) reports working-tree files only, never git-internal state. Root-level only: a nested .git (vendored subrepo) remains ordinary workspace content. Covered by a unit test asserting .git churn is invisible while a working-tree change and a nested .git still report.

@norrietaylor
norrietaylor merged commit 5201c97 into main Jul 31, 2026
30 checks passed
@norrietaylor
norrietaylor deleted the feat/exit-prompt-delta branch July 31, 2026 20:24
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.

3 participants