Skip to content

fix(storage): recover from full disks with bounded memory fallback - #106

Open
danielkov wants to merge 2 commits into
mainfrom
fix/issue-100-storage-fallback
Open

fix(storage): recover from full disks with bounded memory fallback#106
danielkov wants to merge 2 commits into
mainfrom
fix/issue-100-storage-fallback

Conversation

@danielkov

@danielkov danielkov commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Route Kit-owned filesystem reads and writes through a shared, bounded write-back service so disk-full and quota failures do not crash active sessions. Retain accepted changes across session close/reopen, recover them to disk automatically, and explicitly shut down when retention is exhausted.

Closes #100.

Impact

Pending state is process-local and does not survive termination. The default retention limits are 64 MiB and 4,096 queued operations. A final recovery pass runs before normal exit; Kit warns and exits unsuccessfully if accepted changes remain unpersisted.

Explicit edit and shell operations still report real filesystem failures. Native consumers require disk-materialized inputs, and session and credential-refresh locks retain real OS authority rather than process-local substitutes.

Technical details

Shared reads and ordered recovery

Session histories, credentials, configuration, plugin caches, diagnostics, and output artifacts use one coherent view of disk contents and pending changes. Ordered replay retains ownership checks and atomic replacement state across failures; idle retries and subsequent internal operations recover pending work when capacity returns. Non-capacity, identity, and integrity errors remain failures rather than enabling unsafe fallback.

Bounded retention and output access

Retained payloads use fallible allocation and zeroizing storage. Budget exhaustion requests orderly cancellation; an actual fallback allocation failure takes a separate emergency exit path with a static diagnostic and best-effort terminal restoration. The artifact tool provides bounded, session-scoped reads of spilled output, including memory-only artifacts that a shell cannot access.

@danielkov
danielkov marked this pull request as draft September 4, 2026 22:38

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The memory-only fallback hides its durability warning in the default-hidden TUI log pane, allowing users to continue without knowing subsequent transcript changes will be lost when the session closes.

Comment thread src/session.rs Outdated
// prefix, but its crash durability cannot be promised here.
self.memory = Some(crate::storage::MemoryBuffer::default());
let _ = io::stderr().write_all(
b"kit: session storage is full; subsequent transcript changes are memory-only and will be lost when the session closes.\n",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Show the memory-only warning outside the hidden log pane

When an active TUI session encounters a storage-capacity error, this warning is written only to the child process's stderr. The TUI pipes that stream and converts unstructured lines into Update::Log (src/tui/mod.rs:760–789), which only appends to app.logs. Because show_logs defaults to false and the log pane is rendered only when explicitly enabled, the normal session view gives no indication that persistence has stopped. Users can therefore continue working and close or switch sessions without knowing that the subsequent transcript changes will be lost. Emit a structured, session-associated degradation event and display a visible warning or persistent memory-only status independently of the optional log pane, retaining stderr reporting for non-TUI callers.

For agents:
Validate the following issue, address if needed:

<comment>The transition to memory-only transcript storage emits only an stderr warning that the TUI routes to its default-hidden log pane, leaving users unaware that subsequent changes will be lost when the session closes.</comment>
<file_context>
diff --git a/src/session.rs b/src/session.rs
--- a/src/session.rs
+++ b/src/session.rs
+                // Sync itself may be failing: truncation restores the logical
+                // prefix, but its crash durability cannot be promised here.
+                self.memory = Some(crate::storage::MemoryBuffer::default());
+                let _ = io::stderr().write_all(
+                    b"kit: session storage is full; subsequent transcript changes are memory-only and will be lost when the session closes.\n",
+                );
</file_context>

@danielkov
danielkov force-pushed the fix/issue-100-storage-fallback branch from 4ff715b to 9867ad7 Compare September 4, 2026 23:56
@danielkov danielkov changed the title fix(session): tolerate disk-full transcript writes fix(storage): recover from full disks with bounded memory fallback Sep 4, 2026
@danielkov
danielkov marked this pull request as ready for review September 4, 2026 23:56

@kit-code-agent kit-code-agent Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The writeback filesystem can silently discard writes from independently opened handles, bypass restrictive creation permissions, and exceed its operation budget through handle chmod. Normal TUI shutdown also kills the storage-owning child without final recovery. Symlinked MCP configurations regress, and queued migration synchronization prevents retained-session reopening during an outage. The prior finding about storage-degradation warnings remaining hidden in the TUI log pane is still valid.

Comment thread src/resilient_fs/mod.rs
"native identity unavailable",
));
}
Ok(_) => object.path = None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Preserve other live handles across service-owned writes

Independently open two append handles A and B to an existing file on a healthy disk, then append through A followed by B. Clean opens create separate Objects, and A's write publishes a replacement inode. rebase updates only dirty objects, so B still holds the old inode. B's next write reaches this identity-mismatch branch, clears its path, and takes the pathless mutation branch, which returns success without publishing the bytes or creating a recovery obligation. Dropping B then silently discards its accepted append.

Preserve shared logical identity across independently opened handles, or update all matching live objects when an ordinary handle mutation publishes its replacement inode. Keep explicit unlink/replacement and external inode replacement distinct from these service-owned writes.

For agents:
Validate the following issue, address if needed:

<comment>A successful write through one independently opened handle causes another handle's subsequent writes to be silently detached from the named file.</comment>
<file_context>
+                    Ok(_) => object.path = None,
+                    Err(e) if e.kind() == io::ErrorKind::NotFound => object.path = None,
+                    Err(e) => return Err(e),
</file_context>

Comment thread src/resilient_fs/mod.rs
#[cfg(unix)]
{
let _ = (s, path);
Ok(permissions(private, dir))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Honor the process umask when creating files

On Unix, ordinary new files receive fixed 0644 permissions from this function. Replay creates a private temporary file but then applies these permissions with file.set_permissions(...) before publishing it. That chmod bypasses the process umask: with umask 077, a newly created file becomes 0644 rather than 0600, exposing its contents to other users when its parent directory is traversable. This affects healthy-disk creation through write, replace, and creating OpenOptions, not just outage recovery.

Determine and retain creation permissions that respect the process umask, then preserve them during replay. Keep existing-file permission preservation separate, and avoid temporarily changing the process-global umask in concurrent code.

For agents:
Validate the following issue, address if needed:

<comment>Fixed creation permissions are later applied by chmod, making newly created files readable beyond the caller's restrictive umask.</comment>
<file_context>
+        #[cfg(unix)]
+        {
+            let _ = (s, path);
+            Ok(permissions(private, dir))
+        }
</file_context>

Comment thread src/main.rs
let result = run().await;
// Finish synchronously: a detached task can be terminated with the process,
// and timing out spawn_blocking would still make runtime teardown wait.
let recovery = kit::resilient_fs::finish_recovery(kit::resilient_fs::global());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P1] Finalize the TUI child's writeback queue before killing it

Normal TUI exit does not run this final recovery in the process that owns transcript and artifact writes. After acknowledging CloseSession, the TUI signals its watcher, which calls child.kill().await (src/tui/mod.rs:823–826,1663–1664). ACP session closure drops the binding but deliberately leaves pending writes in the process-wide filesystem. Killing that child bypasses its main finalizer; the parent's finalizer can inspect only the parent's queue.

Consequently, accepted memory-only changes are discarded without a final recovery attempt or an undurable-storage error reaching the TUI caller. This can lose recoverable changes even when disk capacity has returned before the next background retry. Request graceful child-process shutdown, including its A2A listener, await final recovery and the exit status, and propagate persistence failure. Reserve forced termination for a bounded fallback that reports potential data loss.

For agents:
Validate the following issue, address if needed:

<comment>Normal TUI exit forcibly kills the storage-owning child, bypassing its final recovery and hiding the loss of pending writes from the parent.</comment>
<file_context>
+    kit::resilient_fs::start_recovery_worker();
+    let result = run().await;
+    // Finish synchronously: a detached task can be terminated with the process,
+    // and timing out spawn_blocking would still make runtime teardown wait.
+    let recovery = kit::resilient_fs::finish_recovery(kit::resilient_fs::global());
+    // Always attempt recovery, but preserve the original command error. The
+    // recovery helper separately warns if accepted data remains undurable.
+    result?;
+    recovery?;
+    Ok(())
</file_context>

Comment thread src/resilient_fs/mod.rs
self.fs.authority(&path)?;
self.fs.secure_path(&s, &path, false)?;
Fs::prepare(&mut s, 0)?;
self.fs.enqueue(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Apply operation-budget admission to handle chmod

File::set_permissions calls enqueue directly after prepare, but prepare only reserves allocator capacity; it does not enforce the configured retention limits. When a capacity-blocked operation remains at the queue front, each handle chmod is appended and returns success, including after max_operations has been reached. Repeated calls can therefore grow the queue beyond its bound without setting the budget-exhaustion flag that triggers cancellation.

Use the checked admission path provided by submit, as the path-based permission operation does, or enforce admission centrally before enqueueing. A rejected chmod must leave both the queue and visible metadata unchanged.

For agents:
Validate the following issue, address if needed:

<comment>Handle chmod bypasses configured operation-budget checks and can grow a capacity-blocked queue without triggering exhaustion shutdown.</comment>
<file_context>
+            Fs::prepare(&mut s, 0)?;
+            self.fs.enqueue(
+                &mut s,
+                Action::Chmod {
+                    path,
+                    permissions: p.clone(),
+                },
+            )
</file_context>

Comment thread src/tools/mcp.rs
async fn read_source(source: &ConfigSource) -> Result<Option<Vec<u8>>, String> {
match tokio::fs::read(&source.path).await {
let path = source.path.clone();
match tokio::task::spawn_blocking(move || crate::resilient_fs::read(path))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P2] Preserve symlinked MCP configuration reads

This changes previously working symlinked .mcp.json files and --mcp-config paths into configuration-load failures. Runtime path resolution makes relative paths absolute without resolving the final symlink. resilient_fs::read then normalizes the parent and calls secure_path(..., false), which rejects the final symlink with PermissionDenied; the previous tokio::fs::read followed it. The optional-project-file exception only suppresses NotFound, so it does not preserve this case either.

Preserve symlink-following semantics for user-supplied configuration reads by resolving the target before reading through the coherent filesystem, while retaining no-follow enforcement for security-sensitive managed paths.

For agents:
Validate the following issue, address if needed:

<comment>Switching MCP configuration reads to the no-follow managed filesystem rejects previously supported symlinked configuration files.</comment>
<file_context>
-    match tokio::fs::read(&source.path).await {
+    let path = source.path.clone();
+    match tokio::task::spawn_blocking(move || crate::resilient_fs::read(path))
+        .await
+        .map_err(|error| format!("MCP config read task failed: {error}"))?
+    {
</file_context>

Comment thread src/resilient_fs/mod.rs
) -> io::Result<Lease> {
let path = self.norm(path.as_ref())?;
let scope = self.norm(scope.as_ref())?;
self.require_disk(&path)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] Reuse retained authority before requiring lock-path durability

A legacy-session migration can retain accepted writes during a capacity outage but then fail to reopen after its observer closes. Creating the scoped migration record queues sync_parent_directory behind the blocked write (src/session.rs:1707–1708). Action::Sync::touches treats that parent-directory synchronization as touching the descendant lock path, so this require_disk call returns the unresolved capacity error before the retained-lease registry is consulted. The real lease is still retained by pending work, but its validated owner handoff is unreachable until recovery succeeds.

Check for a valid retained-authority handoff before imposing the native-materialization barrier required for fresh lease acquisition. Preserve the identity, scope, and live-observer checks during that handoff.

For agents:
Validate the following issue, address if needed:

<comment>A queued migration directory sync blocks lock-path durability before retained lease authority can be handed to a reopened session.</comment>
<file_context>
+        let path = self.norm(path.as_ref())?;
+        let scope = self.norm(scope.as_ref())?;
+        self.require_disk(&path)?;
+        let mut state = lock(&self.service.state);
+        state
+            .leases
+            .retain(|(_, authority, _)| authority.strong_count() > 0);
</file_context>

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.

Crashes when fs full

1 participant