Skip to content

Copy uploads into worktree on first message#71

Merged
mcintyre94 merged 3 commits intomainfrom
git-worktree-upload-issue-workaround-e7ed76df
Mar 14, 2026
Merged

Copy uploads into worktree on first message#71
mcintyre94 merged 3 commits intomainfrom
git-worktree-upload-issue-workaround-e7ed76df

Conversation

@mcintyre94
Copy link
Copy Markdown
Owner

Summary

  • When a file is uploaded before the first message in a chat, it lands in the original working directory (e.g. /home/sprite/project/file.txt)
  • On first message send, a git worktree is created from main — which doesn't include the uncommitted upload
  • This PR copies any pre-send attachments into the worktree after it's created, then rebuilds the prompt with the new paths so Claude sees files that are actually inside the worktree's git context

How it works

In sendMessage(), the raw text and attachments are captured before being cleared. After setupWorktree() succeeds, a new copyAttachmentsToWorktree() helper:

  1. Constructs the equivalent path for each attachment inside the worktree
  2. Batches cp commands into a single exec call on the sprite
  3. Returns updated AttachedFile values with the new paths

The prompt sent to Claude is then rebuilt with the worktree paths. The display message stored in messages keeps the original paths (already appended before the task runs) — this is a minor cosmetic inconsistency but doesn't affect functionality since absolute paths remain valid.

If the copy fails, Claude falls back to the original absolute paths, which are still readable.

Test plan

  • Upload a file, send first message to a git-repo sprite → verify file appears in the worktree directory and Claude can git add it
  • Upload a file, send first message to a non-git sprite (no worktree) → verify original flow unchanged
  • Upload a sprite-browsed file from outside the working directory → verify it's left at its original path (skip condition: attachment.path != newPath)

🤖 Generated with Claude Code

@claude
Copy link
Copy Markdown

claude bot commented Mar 13, 2026

Security: Command injection via unsanitized file paths

File paths are interpolated directly into a shell command using single quotes with no escaping:

if attachment.path != newPath {
copyCommands.append("cp '\(attachment.path)' '\(newPath)' 2>/dev/null && echo ok || echo skip")
updated.append(AttachedFile(name: attachment.name, path: newPath))

Single quotes in POSIX shell have no escape sequence — a filename containing a single-quote character (e.g. a file named with an apostrophe) breaks out of the quoting and allows arbitrary command execution on the Sprite. Since attachment.path comes from user-uploaded filenames, this is a shell injection vulnerability.

To fix, escape single quotes in each path before interpolation. The standard POSIX idiom is to end the single-quoted string, insert an escaped literal quote, then reopen: replace each single-quote with the four-character sequence: close-single-quote, backslash, single-quote, open-single-quote.

Then build the command with cp shellEscape(attachment.path) shellEscape(newPath).

@claude
Copy link
Copy Markdown

claude bot commented Mar 13, 2026

Bug: Copy result is discarded so failed copies still return the new (non-existent) path

The updated array is populated with new worktree paths in the loop — before any copies run. The runExec result is then discarded:

if !copyCommands.isEmpty {
let command = copyCommands.joined(separator: "; ")
_ = await apiClient.runExec(spriteName: spriteName, command: command, timeout: 30)
}
return updated

The doc comment promises "Files that fail to copy are left with their original paths", but this is not what happens. When a cp fails (and emits skip), the corresponding entry in updated already points to the new worktree path — a file that does not exist. Claude will then receive a prompt referencing a non-existent path.

To fix, either:

  1. Parse the per-file ok/skip output from runExec and revert to the original path for any skip entries, or
  2. Build a mapping of old to new paths, run the copies, then construct updated based on which copies succeeded.

@claude
Copy link
Copy Markdown

claude bot commented Mar 13, 2026

CLAUDE.md: Missing unit tests for new view model logic

The PR adds a new method copyAttachmentsToWorktree and modifies the sendMessage / streamTask flow in ChatViewModel (a view model), but includes no new tests.

CLAUDE.md requires:
"Add new unit tests when adding or modifying logic (models, parsers, utilities, view models)"

The new path-construction logic, the attachment.path != newPath guard, and the command-batching behaviour are all testable in isolation (e.g. by verifying the returned [AttachedFile] paths). Existing test files like ChatViewModelTests.swift already cover ChatViewModel, so there is a natural home for these tests.

When a file is uploaded and then a worktree is created on the first
message send, the uploaded file only existed in the original working
directory. This copies any pre-send attachments into the worktree after
it's created, and rebuilds the prompt with the new paths so Claude
operates on files that are actually inside the git worktree.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mcintyre94 mcintyre94 force-pushed the git-worktree-upload-issue-workaround-e7ed76df branch from b10c5dd to 24cb257 Compare March 14, 2026 14:29
mcintyre94 and others added 2 commits March 14, 2026 14:45
Three issues flagged in review:

1. Shell injection via unescaped single quotes: file paths were
   interpolated directly into `cp '...' '...'` with no escaping.
   Added `shellEscapePath(_:)` which applies the POSIX idiom of
   closing the quote, inserting `\'`, then reopening.

2. Failed copies returned the new (non-existent) path: `updated`
   was built with new paths before exec ran, so any `skip` still
   resolved to the worktree path. Fixed by building a `copyJobs`
   index first, then updating only entries where the exec echoed `ok`.

3. Missing unit tests: added WorktreeTests.swift covering
   `shellEscapePath` (plain, apostrophe, spaces, empty) and
   `copyAttachmentsToWorktree` (no worktree, already-in-worktree,
   failed-copy fallback, mixed).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The user bubble was added to messages[] with original file paths before
the worktree was created. claudePrompt was rebuilt with worktree paths
but the displayed message never reflected this. Since ChatMessage is a
reference type, update userMessage.content in place so the bubble shows
the same paths Claude actually receives.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@mcintyre94 mcintyre94 merged commit 73e59a4 into main Mar 14, 2026
2 checks passed
@mcintyre94 mcintyre94 deleted the git-worktree-upload-issue-workaround-e7ed76df branch March 14, 2026 18:16
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