Skip to content

fix: handle missing attachment paths gracefully#3063

Open
chindris-mihai-alexandru wants to merge 3 commits intotailcallhq:mainfrom
chindris-mihai-alexandru:fix/handle-missing-attachment-paths-pr
Open

fix: handle missing attachment paths gracefully#3063
chindris-mihai-alexandru wants to merge 3 commits intotailcallhq:mainfrom
chindris-mihai-alexandru:fix/handle-missing-attachment-paths-pr

Conversation

@chindris-mihai-alexandru
Copy link
Copy Markdown
Contributor

Summary

Handle missing attachment paths gracefully so invalid @[...] tags no longer surface raw filesystem errors.

Context

When Forge receives an attachment tag pointing to a truncated or nonexistent path, attachment loading currently falls through into lower-level file reads and bubbles up generic OS errors such as No such file or directory. This makes the failure confusing, especially for shortened display paths that end in ....

Changes

  • added explicit attachment path resolution and existence validation before reading files or directories
  • introduced a dedicated missing-attachment error with a user-facing message that explains shortened display paths must be pasted in full
  • updated attachment directory/file branching to rely on the validated resolved path
  • added regression tests for nonexistent files and truncated display-style paths

Key Implementation Details

Validation now happens at the start of attachment population, before directory listing or text/image reads. That keeps attachment failures within the attachment service boundary and avoids leaking raw file-open errors from lower layers.

Use Cases

  • Pasting @[/Volumes/.../forgecode-fork/...] now returns a clear attachment-specific error
  • Referencing a missing attachment path no longer fails with a generic OS error message
  • Reviewers can verify the behavior directly from the attachment service tests

Testing

cargo test -p forge_services attachment -- --nocapture
cargo check -p forge_services
cargo clippy -p forge_services --all-targets -- -D warnings

Links

Co-Authored-By: ForgeCode <noreply@forgecode.dev>
Copilot AI review requested due to automatic review settings April 17, 2026 23:08
@github-actions github-actions bot added the type: fix Iterations on existing features or infrastructure. label Apr 17, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR improves Forge’s attachment handling by validating attachment paths early and returning a clearer, attachment-scoped error when a referenced path is missing (including truncated display-style paths ending in ...), rather than leaking raw filesystem errors.

Changes:

  • Added explicit attachment path resolution (relative → cwd-joined) and existence validation prior to any directory listing or file reads.
  • Introduced a dedicated missing-attachment error with a user-facing message explaining truncated display paths must be pasted in full.
  • Added regression tests for missing and truncated attachment paths.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/forge_services/src/attachment.rs Outdated
Co-Authored-By: ForgeCode <noreply@forgecode.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: fix Iterations on existing features or infrastructure.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants