fix: convert dragged file paths to @mentions for attachment (#382)

* fix: convert dragged file paths to @mentions for attachment

When non-image files are dragged into the terminal, the file path was
inserted as plain text and never attached. Now detected absolute paths
are converted to @mentions so they get picked up by the attachment system.

* test: add tests for drag-and-drop file path detection

* fix: multi-image drag-and-drop only showing last image

insertTextAtCursor read input and cursorOffset from the React closure,
which is stale when called in a synchronous loop (e.g. onImagePaste for
multiple dragged images). Now uses refs so each insertion chains on the
previous one.

* fix: quote Windows absolute paths to avoid MCP mention collision

Paths containing ':' (e.g. Windows drive letters) are now emitted in
quoted @"..." form so they don't match the MCP resource mention regex.

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

* refactor: decouple dragDropPaths from imagePaste and harden image checks

- Check image extension against the cleaned path (post quote/escape
  stripping) so quoted or backslash-escaped image drops are reliably
  routed to the image paste handler.
- Inline the image extension regex and drop the imagePaste/fsOperations
  imports so the module (and its tests) no longer pull in `bun:bundle`
  and the heavier fs wrapper chain. Use plain `fs.existsSync` for the
  on-disk check.
- Add tests covering quoted image paths, uppercase extensions,
  backslash-escaped image paths, escaped real files with spaces, mixed
  segments containing an image, quoted-nonexistent paths, and leading
  or trailing whitespace.

* test: verify dragged paths with an `@` segment are preserved

Adds a fixture under a scoped-package-style subdir (`@types/index.d.ts`)
so we exercise the realistic `node_modules/@types/...` drag case and
lock in that `extractDraggedFilePaths` returns the raw path unchanged —
the `@` inside the path must not collide with the mention prefix the
caller prepends downstream.

* test: parametrize dragDropPaths cases with test.each

Groups the 21 scenarios into four table-driven describes
(empty-result, single-path, multi-path, backslash-escaped) so that
adding a new case is a one-line row instead of a new `test()` block.
Fixture directories are now created synchronously at describe-load
time so their paths are available to the test.each tables, which are
built before any hook runs.

* test: add contract tests for @-mention extractor boundary

Pins the contract between `extractAtMentionedFiles` and
`extractMcpResourceMentions` so the MCP regex can't silently swallow
quoted file-path mentions.

These tests fail on current HEAD — 3 of 11 cases expose the regression
pointed out in the review on #382: `extractMcpResourceMentions`'s
trailing `\b` backtracks past the closing `"` of a quoted mention and
produces a ghost match for `@"C:\Users\..."`, `@C:\Users\...`, and
`@"/tmp/weird:name.txt"`. The remaining 8 cases lock in the behaviour
that must not change (legitimate `server:resource` mentions and plain
file-path mentions).

Committed failing on purpose as the first half of a test-then-fix
pair; the regex fix follows in a subsequent commit.

* fix: prevent MCP extractor from ghost-matching quoted/Windows paths

The MCP resource regex used `\b` as a trailing anchor with `[^\s]+`
character classes. On any quoted file mention containing a colon
(`@"C:\Users\me\file.txt"`, `@"/tmp/weird:name.txt"`), the engine
backtracked past the closing `"` to satisfy `\b`, producing a ghost
match that collided with `extractAtMentionedFiles`. Unquoted Windows
drive-letter paths (`@C:\Users\me\file.txt`) also matched because a
drive letter is structurally identical to an MCP `server:resource`
token.

Two guards:

1. `(?!")` right after `@` drops quoted tokens entirely, and adding
   `"` to the character classes blocks any mid-match backtracking.
2. A post-match filter discards `^[A-Za-z]:[\\/]` — a single-letter
   server followed by a path separator is always a Windows drive
   prefix, never a real MCP resource.

Legitimate MCP forms (`@server:resource/path`, plugin-scoped like
`@asana-plugin:project-status/123`, inline prose mentions) remain
matched and are pinned by the contract tests added in 04998d5.

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Paulo Reis
2026-04-06 06:49:38 -03:00
committed by GitHub
parent 8724d59d48
commit 112df59117
5 changed files with 294 additions and 7 deletions

View File

@@ -67,6 +67,7 @@ import { isBilledAsExtraUsage } from '../../utils/extraUsage.js';
import { getFastModeUnavailableReason, isFastModeAvailable, isFastModeCooldown, isFastModeEnabled, isFastModeSupportedByModel } from '../../utils/fastMode.js'; import { getFastModeUnavailableReason, isFastModeAvailable, isFastModeCooldown, isFastModeEnabled, isFastModeSupportedByModel } from '../../utils/fastMode.js';
import { isFullscreenEnvEnabled } from '../../utils/fullscreen.js'; import { isFullscreenEnvEnabled } from '../../utils/fullscreen.js';
import type { PromptInputHelpers } from '../../utils/handlePromptSubmit.js'; import type { PromptInputHelpers } from '../../utils/handlePromptSubmit.js';
import { extractDraggedFilePaths } from '../../utils/dragDropPaths.js';
import { getImageFromClipboard, PASTE_THRESHOLD } from '../../utils/imagePaste.js'; import { getImageFromClipboard, PASTE_THRESHOLD } from '../../utils/imagePaste.js';
import type { ImageDimensions } from '../../utils/imageResizer.js'; import type { ImageDimensions } from '../../utils/imageResizer.js';
import { cacheImagePath, storeImage } from '../../utils/imageStore.js'; import { cacheImagePath, storeImage } from '../../utils/imageStore.js';
@@ -1204,6 +1205,22 @@ function PromptInput({
// Clean up pasted text - strip ANSI escape codes and normalize line endings and tabs // Clean up pasted text - strip ANSI escape codes and normalize line endings and tabs
let text = stripAnsi(rawText).replace(/\r/g, '\n').replaceAll('\t', ' '); let text = stripAnsi(rawText).replace(/\r/g, '\n').replaceAll('\t', ' ');
// Detect file paths from drag-and-drop and convert to @mentions.
// When files are dragged into the terminal, the terminal sends their
// absolute paths via bracketed paste. Image files are handled by the
// image paste handler upstream; here we handle non-image files by
// converting them to @mentions so they get attached on submit.
const draggedPaths = extractDraggedFilePaths(text);
if (draggedPaths.length > 0) {
const mentions = draggedPaths
.map(p => (p.includes(' ') || p.includes(':') ? `@"${p}"` : `@${p}`))
.join(' ');
// Ensure spacing around the mention(s) relative to existing input
const charBefore = input[cursorOffset - 1];
const prefix = charBefore && !/\s/.test(charBefore) ? ' ' : '';
text = prefix + mentions + ' ';
}
// Match typed/auto-suggest: `!cmd` pasted into empty input enters bash mode. // Match typed/auto-suggest: `!cmd` pasted into empty input enters bash mode.
if (input.length === 0) { if (input.length === 0) {
const pastedMode = getModeFromInput(text); const pastedMode = getModeFromInput(text);
@@ -1245,12 +1262,23 @@ function PromptInput({
if (isNonSpacePrintable(input, key)) return ' ' + input; if (isNonSpacePrintable(input, key)) return ' ' + input;
return input; return input;
}, []); }, []);
// Ref mirrors cursorOffset for use in synchronous loops (e.g. multi-image
// paste) where React batches state updates and the closure value is stale.
const cursorOffsetRef = useRef(cursorOffset);
cursorOffsetRef.current = cursorOffset;
function insertTextAtCursor(text: string) { function insertTextAtCursor(text: string) {
// Push current state to buffer before inserting // Use refs for input/cursor so back-to-back calls in the same event
pushToBuffer(input, cursorOffset, pastedContents); // (e.g. onImagePaste loop for multiple dragged images) chain correctly
const newInput = input.slice(0, cursorOffset) + text + input.slice(cursorOffset); // instead of each reading the same stale closure values.
const currentInput = lastInternalInputRef.current;
const currentOffset = cursorOffsetRef.current;
pushToBuffer(currentInput, currentOffset, pastedContents);
const newInput = currentInput.slice(0, currentOffset) + text + currentInput.slice(currentOffset);
trackAndSetInput(newInput); trackAndSetInput(newInput);
setCursorOffset(cursorOffset + text.length); const newOffset = currentOffset + text.length;
cursorOffsetRef.current = newOffset;
setCursorOffset(newOffset);
} }
const doublePressEscFromEmpty = useDoublePress(() => {}, () => onShowMessageSelector()); const doublePressEscFromEmpty = useDoublePress(() => {}, () => onShowMessageSelector());

View File

@@ -0,0 +1,85 @@
import { describe, expect, test } from 'bun:test'
import {
extractAtMentionedFiles,
extractMcpResourceMentions,
} from './attachments.js'
// Contract tests for the two @-mention extractors.
//
// Scope: the narrow contract between `extractAtMentionedFiles` and
// `extractMcpResourceMentions` where both are called on the same input
// and must not both claim the same token. The motivating bug is that
// `extractMcpResourceMentions`'s `\b` anchor lets it backtrack over the
// closing quote of a quoted file mention, producing a ghost match for
// `@"C:\Users\..."`. These tests pin the boundary so any regression in
// the MCP regex is caught immediately.
describe('extractor contract', () => {
describe('extractMcpResourceMentions must return empty for', () => {
const cases: Array<[string, string]> = [
// Primary bug: the quoted form that PromptInput emits for Windows
// paths today. `\b` backtracks past the trailing `"` and produces
// a ghost MCP match on current HEAD.
['a quoted Windows drive-letter path', '@"C:\\Users\\me\\file.txt"'],
// Even if the quote layer were stripped, a bare drive letter
// followed by a path separator is never an MCP resource.
['an unquoted Windows drive-letter path', '@C:\\Users\\me\\file.txt'],
// Sanity: quoted POSIX paths with no `:` at all never matched the
// MCP regex and must keep not matching after the fix.
['a quoted POSIX path with a space', '@"/Users/foo/my file.ts"'],
['an unquoted POSIX path', '@/Users/foo/bar.ts'],
// Quoted POSIX path that embeds a `:` in the filename — the quote
// layer must shield it from MCP matching, same as the Windows case.
['a quoted POSIX path with a colon in the name', '@"/tmp/weird:name.txt"'],
]
test.each(cases)('%s', (_label, input) => {
expect(extractMcpResourceMentions(input)).toEqual([])
})
})
describe('extractMcpResourceMentions still matches legitimate MCP mentions', () => {
// Regression guard for the fix. If someone tightens the MCP regex
// too aggressively, these break and the intent is clear.
const cases: Array<[string, string, string[]]> = [
[
'a simple server:resource token',
'@server:resource/path',
['server:resource/path'],
],
[
'a plugin-scoped server name with a dash',
'@asana-plugin:project-status/123',
['asana-plugin:project-status/123'],
],
[
'an MCP mention inline in prose',
'please check @server:res here',
['server:res'],
],
]
test.each(cases)('%s', (_label, input, expected) => {
expect(extractMcpResourceMentions(input)).toEqual(expected)
})
})
describe('extractAtMentionedFiles extracts the file paths it should', () => {
// Asserted separately from the MCP side: the bug is purely in the
// MCP extractor over-matching, so these assertions are the
// "baseline still works" half of the contract.
const cases: Array<[string, string, string[]]> = [
[
'a quoted Windows drive-letter path',
'@"C:\\Users\\me\\file.txt"',
['C:\\Users\\me\\file.txt'],
],
[
'a quoted POSIX path with a space',
'@"/Users/foo/my file.ts"',
['/Users/foo/my file.ts'],
],
['an unquoted POSIX path', '@/Users/foo/bar.ts', ['/Users/foo/bar.ts']],
]
test.each(cases)('%s', (_label, input, expected) => {
expect(extractAtMentionedFiles(input)).toEqual(expected)
})
})
})

View File

@@ -2793,11 +2793,30 @@ export function extractAtMentionedFiles(content: string): string[] {
export function extractMcpResourceMentions(content: string): string[] { export function extractMcpResourceMentions(content: string): string[] {
// Extract MCP resources mentioned with @ symbol in format @server:uri // Extract MCP resources mentioned with @ symbol in format @server:uri
// Example: "@server1:resource/path" would extract "server1:resource/path" // Example: "@server1:resource/path" would extract "server1:resource/path"
const atMentionRegex = /(^|\s)@([^\s]+:[^\s]+)\b/g //
// Two guards against Windows-path / quoted-file collisions (see
// `attachments.extractors.test.ts`):
//
// 1. `(?!")` right after `@` drops quoted tokens entirely. The earlier
// form (without the lookahead and with `[^\s]` character classes)
// backtracked past the closing `"` at the `\b` anchor and produced
// ghost matches like `"C:\Users\...\file.txt` for any quoted file
// mention containing a colon.
// 2. The `"` added to the character classes is belt-and-braces: even
// if the lookahead were later removed or bypassed, the engine can
// no longer consume a quote character mid-match.
const atMentionRegex = /(^|\s)@(?!")([^\s"]+:[^\s"]+)\b/g
const matches = content.match(atMentionRegex) || [] const matches = content.match(atMentionRegex) || []
// Remove the prefix (everything before @) from each match return uniq(
return uniq(matches.map(match => match.slice(match.indexOf('@') + 1))) matches
.map(match => match.slice(match.indexOf('@') + 1))
// Post-match filter: a single-letter "server" followed by `:\` or
// `:/` is always a Windows drive-letter prefix, never a real MCP
// resource. This covers the unquoted `@C:\Users\...` case that
// the regex alone cannot disambiguate from `@server:resource`.
.filter(m => !/^[A-Za-z]:[\\/]/.test(m)),
)
} }
export function extractAgentMentions(content: string): string[] { export function extractAgentMentions(content: string): string[] {

View File

@@ -0,0 +1,100 @@
import { afterAll, describe, expect, test } from 'bun:test'
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'fs'
import { tmpdir } from 'os'
import { join } from 'path'
import { extractDraggedFilePaths } from './dragDropPaths.js'
describe('extractDraggedFilePaths', () => {
// Paths that exist on any system.
const thisFile = import.meta.path
const packageJson = `${process.cwd()}/package.json`
// Fixtures created synchronously at describe-load time (not in
// `beforeAll`) so their paths are available to `test.each` tables,
// which are built before any hook runs.
const tmpDir = mkdtempSync(join(tmpdir(), 'dragdrop-test-'))
const spacedFile = join(tmpDir, 'my file.txt')
writeFileSync(spacedFile, 'test')
const scopedDir = join(tmpDir, '@types')
mkdirSync(scopedDir)
const atSignFile = join(scopedDir, 'index.d.ts')
writeFileSync(atSignFile, 'test')
afterAll(() => {
rmSync(tmpDir, { recursive: true, force: true })
})
describe('returns an empty array', () => {
const emptyCases: Array<[string, string]> = [
['a non-absolute path', 'relative/path/file.ts'],
['a plain image path', '/Users/foo/image.png'],
['an uppercase image extension', '/Users/foo/SHOT.PNG'],
['a double-quoted image path', '"/Users/foo/shot.png"'],
['a single-quoted image path', "'/Users/foo/shot.jpg'"],
['regular prose text', 'hello world this is text'],
['a nonexistent absolute path', '/definitely/nonexistent/file.ts'],
['a single-quoted nonexistent path', "'/definitely/nonexistent.ts'"],
['an empty string', ''],
['whitespace only', ' \n '],
// Mixed-segment cases: all-or-nothing policy means a single bad
// entry disqualifies the whole paste.
['a mix where one path does not exist', `${thisFile}\n/nonexistent/file.ts`],
['a mix where one segment is an image', `${thisFile}\n/Users/foo/shot.png`],
]
test.each(emptyCases)('for %s', (_label, input) => {
expect(extractDraggedFilePaths(input)).toEqual([])
})
})
describe('resolves a single path', () => {
const singleCases: Array<[string, string, string]> = [
['a plain absolute path', thisFile, thisFile],
['a double-quoted path', `"${thisFile}"`, thisFile],
['a single-quoted path', `'${thisFile}'`, thisFile],
['a path with leading/trailing whitespace', ` ${thisFile} `, thisFile],
// Realistic: dragging something under `node_modules/@types/...`.
// `@` inside the path must not collide with the mention prefix
// that the caller prepends downstream.
['a path containing an `@` segment', atSignFile, atSignFile],
]
test.each(singleCases)('from %s', (_label, input, expected) => {
expect(extractDraggedFilePaths(input)).toEqual([expected])
})
})
describe('resolves multiple paths', () => {
const multiCases: Array<[string, string, string[]]> = [
[
'newline-separated',
`${thisFile}\n${packageJson}`,
[thisFile, packageJson],
],
[
'space-separated (Finder drag)',
`${thisFile} ${packageJson}`,
[thisFile, packageJson],
],
]
test.each(multiCases)('when input is %s', (_label, input, expected) => {
expect(extractDraggedFilePaths(input)).toEqual(expected)
})
})
// Backslash-escaped paths are a Finder/macOS + Linux convention — on
// Windows the shell-escape step is skipped, so these cases do not apply.
if (process.platform !== 'win32') {
describe('handles backslash-escaped paths', () => {
test('returns empty for an escaped image path', () => {
// The image check must apply after escape stripping so Finder
// image drags still route to the image paste handler.
expect(extractDraggedFilePaths('/Users/foo/my\\ shot.png')).toEqual([])
})
test('resolves an escaped real file with a space in its name', () => {
// Raw form matches what a terminal delivers on Finder drag.
const escaped = spacedFile.replace(/ /g, '\\ ')
expect(extractDraggedFilePaths(escaped)).toEqual([spacedFile])
})
})
}
})

View File

@@ -0,0 +1,55 @@
import { existsSync } from 'fs'
import { isAbsolute } from 'path'
// Inlined to avoid pulling the full `imagePaste.ts` module (which imports
// `bun:bundle`) into this file's dependency graph. Must stay in sync with
// `IMAGE_EXTENSION_REGEX` in `./imagePaste.ts`.
const IMAGE_EXTENSION_REGEX = /\.(png|jpe?g|gif|webp)$/i
/**
* Detect absolute file paths in pasted text (typically from drag-and-drop).
* Returns the cleaned paths if ALL segments are existing non-image files,
* or an empty array otherwise.
*
* Splitting logic mirrors usePasteHandler: space preceding `/` or a Windows
* drive letter, plus newline separators.
*/
export function extractDraggedFilePaths(text: string): string[] {
const segments = text
.split(/ (?=\/|[A-Za-z]:\\)/)
.flatMap(part => part.split('\n'))
.map(s => s.trim())
.filter(Boolean)
if (segments.length === 0) return []
const cleaned: string[] = []
for (const raw of segments) {
// Strip outer quotes and shell-escape backslashes
let p = raw
if (
(p.startsWith('"') && p.endsWith('"')) ||
(p.startsWith("'") && p.endsWith("'"))
) {
p = p.slice(1, -1)
}
if (process.platform !== 'win32') {
p = p.replace(/\\(.)/g, '$1')
}
// Image files are handled by the upstream image paste handler.
// Check against the cleaned path so quoted/escaped image paths like
// `"/foo/shot.png"` or `/foo/my\ shot.png` are reliably excluded.
if (IMAGE_EXTENSION_REGEX.test(p)) return []
if (!isAbsolute(p)) return []
// Verify the path actually exists on disk. Plain `fs.existsSync` is
// used intentionally here instead of the wrapped `getFsImplementation`
// to keep this module free of the heavy `fsOperations` dependency
// chain — this is a pure existence check with no permission semantics.
if (!existsSync(p)) return []
cleaned.push(p)
}
return cleaned
}