Skip to content

feat: import existing files on mount + fix path-as-tag#1

Open
Dhravya wants to merge 5 commits intomainfrom
feat/import-existing-on-mount
Open

feat: import existing files on mount + fix path-as-tag#1
Dhravya wants to merge 5 commits intomainfrom
feat/import-existing-on-mount

Conversation

@Dhravya
Copy link
Copy Markdown
Member

@Dhravya Dhravya commented Apr 25, 2026

Summary

  • Import existing files on mount: When mounting to a non-empty directory, detects pre-existing files and prompts the user to import them into the Supermemory container. Adds --import-existing flag to skip the prompt. Files are scanned before the mount (which shadows them), then imported after the initial pull so server-side files take precedence.
  • Fix smfs mount .: The positional arg is now detected as a path when it looks like one (., .., contains /, or is an existing directory). The container tag is derived from the directory name and the path is used as the mount point.
  • Fix smfs unmount .: Canonicalizes the path before walking up to find the .smfs marker, so relative paths like . resolve correctly.

Test plan

  • smfs mount mytag — works as before (tag = mytag, path = ./mytag/)
  • smfs mount . — mounts current directory, derives tag from dir name
  • smfs mount ./somedir — mounts somedir, derives tag from somedir
  • smfs mount mytag --path /some/dir — explicit path still works
  • Mount to non-empty dir in TTY — prompts for import
  • Mount to non-empty dir in non-TTY — prints warning
  • --import-existing skips prompt and imports
  • smfs unmount . — finds .smfs marker and unmounts correctly
  • cargo test — all 143 tests pass

🤖 Generated with Claude Code

Dhravya and others added 5 commits April 24, 2026 22:14
When mounting to a non-empty directory, detect existing files and
prompt the user to import them into the container. Also fixes
`smfs mount .` and `smfs unmount .` which previously failed because
`.` was used as a literal container tag / couldn't walk up to find
the .smfs marker.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown

@vorflux vorflux Bot left a comment

Choose a reason for hiding this comment

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

Reviewed — found 3 medium-severity issues.


Review with Vorflux

}

fn resolve_tag_and_path(
positional: &str,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

looks_like_path misclassifies existing directory names as path-as-tag, breaking --path disambiguation.

looks_like_path returns true for any name that happens to match an existing local directory (std::path::Path::new(s).is_dir()). When that triggers, resolve_tag_and_path immediately errors if --path is also provided:

if explicit_path.is_some() {
    anyhow::bail!("cannot use both a path as the tag and --path");
}

This means smfs mount docs --path /tmp/mnt fails whenever a ./docs directory already exists in the working directory, even though docs is the intended container tag and --path was explicitly supplied to disambiguate. The explicit --path should take priority over the filesystem probe — when --path is present, skip the is_dir() check entirely (or treat the positional as a tag unconditionally).

}
}
out
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

collect_files_recursive reads entire file contents into memory before any size gate is applied — can cause large memory spikes or OOM on startup.

std::fs::read(&path) buffers the full file into a Vec<u8> for every file in the tree. The 100 MB cap only fires later inside SqliteFile::write (called from import_file). So a directory with a single 2 GB file — or many large files — will exhaust memory at startup before any rejection happens.

Fix: check entry.metadata()?.len() before calling std::fs::read, and skip (with a warning) any file that exceeds the size limit. Alternatively, stream the content in chunks rather than buffering the whole tree upfront.

match fs.import_file(rel_path, contents).await {
Ok(true) => imported += 1,
Ok(false) => skipped += 1,
Err(e) => {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

--import-existing combined with --no-sync can silently duplicate remote documents.

The comment on line 158 says "Import after initial pull so we skip files already in the container", but when cfg.no_sync is true there is no initial pull. On a first mount or clean cache, any local file that already mirrors a remote document will have no remote_id after import, and the push worker will POST it as a new document instead of skipping or PATCHing the existing one — creating duplicates in the container.

Either reject the combination at startup:

if cfg.import_existing && cfg.no_sync {
    anyhow::bail!("--import-existing cannot be used with --no-sync: \
                   duplicate detection requires an initial pull");
}

or perform a targeted remote existence check before importing each file when no_sync is set.

@Dhravya Dhravya force-pushed the feat/import-existing-on-mount branch from 0ffc591 to 26c9d85 Compare April 25, 2026 05:35
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.

1 participant