Skip to content

fix(ln): surface remove failure under -f instead of falling through to symlink#1583

Merged
chaliy merged 1 commit into
mainfrom
claude/fix-1577-ln-f-error-check
May 7, 2026
Merged

fix(ln): surface remove failure under -f instead of falling through to symlink#1583
chaliy merged 1 commit into
mainfrom
claude/fix-1577-ln-f-error-check

Conversation

@chaliy
Copy link
Copy Markdown
Contributor

@chaliy chaliy commented May 7, 2026

Closes #1577.

Summary

ln -f previously swallowed the result of ctx.fs.remove(...) and called ctx.fs.symlink(...) unconditionally. When the destination is a non-empty directory, remove(..., false) fails, but the symlink insert on the in-memory VFS overwrites the directory entry while leaving its children orphaned in the entries map — a corruption of filesystem state.

Fix

  • Propagate the remove error and return exit 1 with a real-shell-style ln: cannot remove '<name>': <reason> message.
  • Add unit-level regression test test_ln_force_over_non_empty_dir_fails (verifies non-zero exit + child file still present).
  • Add a spec case ln_force_dir_dest_fails covering the same scenario from the user-facing CLI.

Notes

The issue also suggests "make VFS symlink reject existing paths defensively". That change has broader implications for other callers (overlay, mountable, cp -R) and warrants its own PR — punting for now.

Test plan

  • cargo test -p bashkit --lib builtins::fileops::tests::test_ln_force_over_non_empty_dir_fails
  • cargo test -p bashkit --test spec_tests bash_spec (passes; new ln_force_dir_dest_fails case included)
  • cargo fmt --all -- --check
  • cargo clippy --all-features --all-targets -- -D warnings

Generated by Claude Code

…o symlink

Previously, ln -f swallowed the result of ctx.fs.remove(...) and called
ctx.fs.symlink unconditionally. If the destination is a non-empty
directory, the non-recursive remove fails, but the symlink insert on
the in-memory VFS overwrites the directory entry while leaving its
children orphaned in the entries map — a corruption of FS state.

Propagate the remove error and return exit 1 with a real-shell-style
"ln: cannot remove '<name>': <reason>" message.

Add a unit-level regression test (test_ln_force_over_non_empty_dir_fails)
and a spec case (ln_force_dir_dest_fails) that ensures the child file
is still present after the failed ln -sf.

Closes #1577
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
bashkit 9ae154e Commit Preview URL

Branch Preview URL
May 07 2026, 04:05 AM

@chaliy chaliy merged commit 5733555 into main May 7, 2026
29 checks passed
@chaliy chaliy deleted the claude/fix-1577-ln-f-error-check branch May 7, 2026 06:50
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.

DeepSec: ln -f continues after failed destination removal

1 participant