From 8f38cf8403b954765aabb8b704a58d58276623b0 Mon Sep 17 00:00:00 2001 From: PJ Doland Date: Mon, 28 Sep 2026 13:51:53 -0400 Subject: [PATCH] fix(mentions): resolve @ mentions followed by punctuation or naming paths with spaces A mention ran to the next whitespace, so `@file:notes.md,` looked up `notes.md,` and a path with a space could not be named at all; either way the token reached the agent unresolved. This follows the decision on plmbr/notebook-intelligence#503 to match Claude Code and Codex, and keeps nui's grammar in step with Chatbook's: - `@file:"data/my notes.md"` (also `dir:` and `ext:`) names a value containing whitespace or `@`. It is taken exactly as written, whitespace included, and cannot hold a quote or a line break; a closing quote glued to a word character keeps the old single token. - Sentence punctuation after an unquoted file or dir mention is not part of the path. The name as written is tried first, then with up to five of those characters removed one at a time, and the first name that exists wins, so a real name ending in punctuation still resolves and a shorter name is never reached past it. The trimmed punctuation is written back after the resolved path, so the sentence is unchanged. `ext:` values are never trimmed. - A resolved path containing whitespace is emitted as `@"/abs/path"`, the form Claude Code reads. - The mention menu inserts the quoted form when a value needs it. Tokens are found with a small rune scanner rather than a regexp, because RE2 has no lookaround and its `\s` is ASCII-only while Chatbook's is Unicode. A mention still has to follow whitespace, now checked by rune rather than by byte, which is also what Claude Code requires of the `@path` mentions nui hands it. --- internal/mentions/builtin_files.go | 10 +- internal/mentions/registry.go | 71 ++++++++---- internal/mentions/tokens.go | 148 ++++++++++++++++++++++++ internal/mentions/tokens_test.go | 180 +++++++++++++++++++++++++++++ ui/src/hooks/useMentionMenu.ts | 3 +- ui/src/lib/mentionToken.test.ts | 25 ++++ ui/src/lib/mentionToken.ts | 26 +++++ 7 files changed, 433 insertions(+), 30 deletions(-) create mode 100644 internal/mentions/tokens.go create mode 100644 internal/mentions/tokens_test.go create mode 100644 ui/src/lib/mentionToken.test.ts create mode 100644 ui/src/lib/mentionToken.ts diff --git a/internal/mentions/builtin_files.go b/internal/mentions/builtin_files.go index 639d1ca..032b7a9 100644 --- a/internal/mentions/builtin_files.go +++ b/internal/mentions/builtin_files.go @@ -56,7 +56,7 @@ func (BuiltinFilesProvider) List(_ context.Context, req ListRequest) (ListRespon } func (BuiltinFilesProvider) Resolve(_ context.Context, req ResolveRequest) (string, error) { - value := strings.TrimSpace(req.Value) + value := req.Value switch { case strings.HasPrefix(value, fileValuePrefix): rel := strings.TrimPrefix(value, fileValuePrefix) @@ -71,7 +71,7 @@ func (BuiltinFilesProvider) Resolve(_ context.Context, req ResolveRequest) (stri if info.IsDir() { return "", fmt.Errorf("mention is a directory, not a file: %s", abs) } - return "@" + abs, nil + return atPath(abs), nil case strings.HasPrefix(value, dirValuePrefix): rel := strings.TrimPrefix(value, dirValuePrefix) abs, err := resolveFilePath(req.WorkingDir, rel) @@ -85,7 +85,7 @@ func (BuiltinFilesProvider) Resolve(_ context.Context, req ResolveRequest) (stri if !info.IsDir() { return "", fmt.Errorf("mention is not a directory: %s", abs) } - return "@" + abs, nil + return atPath(abs), nil default: return "", fmt.Errorf("unsupported mention value %q", value) } @@ -202,7 +202,7 @@ func resolveFilePath(workingDir, rel string) (string, error) { if err != nil { return "", err } - rel = filepath.FromSlash(strings.TrimSpace(rel)) + rel = filepath.FromSlash(rel) if rel == "" || rel == "." { return absWorking, nil } @@ -247,5 +247,5 @@ func resolveAbsoluteFileMention(value string) (string, error) { if info.IsDir() { return "", fmt.Errorf("mention is a directory, not a file: %s", abs) } - return "@" + abs, nil + return atPath(abs), nil } diff --git a/internal/mentions/registry.go b/internal/mentions/registry.go index edd3189..46ac03a 100644 --- a/internal/mentions/registry.go +++ b/internal/mentions/registry.go @@ -4,17 +4,15 @@ package mentions import ( "context" + "errors" "fmt" + "io/fs" "os" "path/filepath" - "regexp" "strings" "sync" - "unicode" ) -var mentionTokenPattern = regexp.MustCompile(`@([^\s@]+)`) - // Registry routes mention list and resolve requests to providers. type Registry struct { mu sync.RWMutex @@ -117,40 +115,65 @@ func (r *Registry) ResolveMessage(ctx context.Context, workingDir, message strin if strings.TrimSpace(message) == "" { return message, nil } - matches := mentionTokenPattern.FindAllStringSubmatchIndex(message, -1) - if len(matches) == 0 { + tokens := findMentionTokens(message) + if len(tokens) == 0 { return message, nil } var b strings.Builder last := 0 - for _, match := range matches { - start, end := match[0], match[1] - valueStart, valueEnd := match[2], match[3] - if start > 0 && !unicode.IsSpace(rune(message[start-1])) { - continue - } - b.WriteString(message[last:start]) - value := message[valueStart:valueEnd] - resolved, err := r.Resolve(ctx, ResolveRequest{ - WorkingDir: workingDir, - Value: value, - AllowedExtensionRoots: allowed, - }) + for _, token := range tokens { + b.WriteString(message[last:token.start]) + resolved, err := r.resolveToken(ctx, workingDir, token, allowed) if err != nil { - fmt.Fprintf(os.Stderr, "[mentions] resolve %q: %v\n", value, err) - b.WriteString(message[start:end]) + fmt.Fprintf(os.Stderr, "[mentions] resolve %q: %v\n", token.value, err) + b.WriteString(message[token.start:token.end]) } else { b.WriteString(resolved) } - last = end + last = token.end } b.WriteString(message[last:]) return b.String(), nil } +// resolveToken resolves one mention. For an unquoted file or dir mention it +// tries the name as written, then with trailing punctuation +// removed, and uses the first name that exists; any other failure, such as a +// path outside the working directory, ends the search. Punctuation trimmed off +// is written back after the resolved path so the sentence around it is +// unchanged. Quoted and extension mentions are resolved exactly as written. +func (r *Registry) resolveToken(ctx context.Context, workingDir string, token mentionToken, allowed map[string]bool) (string, error) { + req := ResolveRequest{WorkingDir: workingDir, Value: token.value, AllowedExtensionRoots: allowed} + prefix, trims := "", false + switch { + case token.quoted: + case strings.HasPrefix(token.value, fileValuePrefix): + prefix, trims = fileValuePrefix, true + case strings.HasPrefix(token.value, dirValuePrefix): + prefix, trims = dirValuePrefix, true + } + if !trims { + return r.Resolve(ctx, req) + } + var err error + for _, candidate := range pathCandidates(prefix, token.value) { + req.Value = candidate + var resolved string + resolved, err = r.Resolve(ctx, req) + if errors.Is(err, fs.ErrNotExist) { + continue + } + if err != nil { + return "", err + } + return resolved + token.value[len(candidate):], nil + } + return "", err +} + func (r *Registry) Resolve(ctx context.Context, req ResolveRequest) (string, error) { - value := strings.TrimSpace(req.Value) - if value == "" { + value := req.Value + if strings.TrimSpace(value) == "" { return "", fmt.Errorf("empty mention value") } switch { diff --git a/internal/mentions/tokens.go b/internal/mentions/tokens.go new file mode 100644 index 0000000..da2b346 --- /dev/null +++ b/internal/mentions/tokens.go @@ -0,0 +1,148 @@ +// Copyright (c) Mehmet Bektas + +package mentions + +import ( + "strings" + "unicode" + "unicode/utf8" +) + +// The quoting and punctuation rules match Chatbook mentions in +// notebook-intelligence (chatbook_mentions.py, plmbr/notebook-intelligence#503), +// which follow Claude Code and Codex; change the two together. Unlike Chatbook, +// a mention here must follow whitespace, as Claude Code requires of the `@path` +// mentions nui hands it (see findMentionTokens). + +// trailingPunctuation is the sentence punctuation that, typed after an unquoted +// file or dir mention (`see @file:notes.md,`), is not part of the path. +const trailingPunctuation = ".,;:!?)]}'\"\u2026\u201d\u2019\u00bb" + + "\u3002\u3001\uff0c\uff1b\uff1a\uff01\uff1f\uff09\u300d\u300f" + +// maxTrailingPunctuation caps how many of those characters are trimmed. +const maxTrailingPunctuation = 5 + +// mentionToken is one `@...` mention found in a message. +type mentionToken struct { + start, end int // byte offsets of the whole token, `@` included + value string // mention value with any quotes removed, e.g. "file:a b.md" + quoted bool +} + +// endsUnquotedToken reports whether r ends an unquoted mention: whitespace, +// the file, group, record, and unit separators Python also treats as +// whitespace, or `@`. +func endsUnquotedToken(r rune) bool { + return r == '@' || unicode.IsSpace(r) || (r >= 0x1c && r <= 0x1f) +} + +// breaksQuotedToken reports whether r cannot appear inside a quoted mention. +func breaksQuotedToken(r rune) bool { + switch r { + case '"', '\n', '\r', '\u0085', '\u2028', '\u2029': + return true + } + return false +} + +// isWordRune matches Python's Unicode \w, which Chatbook's tokenizer uses. +func isWordRune(r rune) bool { + return r == '_' || unicode.IsLetter(r) || unicode.IsNumber(r) +} + +// findMentionTokens returns the mentions in message, in order. A mention starts +// at an `@` at the start of the message or after whitespace, which is also what +// Claude Code requires of the `@path` mentions nui hands it. It is either +// quoted, `@file:"data/my notes.md"`, which is how a value containing +// whitespace or `@` is written, or unquoted and runs to the next whitespace or +// `@`. A closing quote followed directly by a word character or another quote +// does not end a quoted mention; the text is read as one unquoted token, as it +// was before quoting existed. +func findMentionTokens(message string) []mentionToken { + var tokens []mentionToken + for i := 0; i < len(message); { + offset := strings.IndexByte(message[i:], '@') + if offset < 0 { + break + } + start := i + offset + if start > 0 { + prev, _ := utf8.DecodeLastRuneInString(message[:start]) + if !unicode.IsSpace(prev) { + i = start + 1 + continue + } + } + if token, ok := quotedMentionAt(message, start); ok { + tokens = append(tokens, token) + i = token.end + continue + } + end := start + 1 + for end < len(message) { + r, size := utf8.DecodeRuneInString(message[end:]) + if endsUnquotedToken(r) { + break + } + end += size + } + if end > start+1 { + tokens = append(tokens, mentionToken{start: start, end: end, value: message[start+1 : end]}) + } + i = end + } + return tokens +} + +func quotedMentionAt(message string, start int) (mentionToken, bool) { + rest := message[start+1:] + for _, kind := range []string{"file", "dir", "ext"} { + prefix := kind + `:"` + if !strings.HasPrefix(rest, prefix) { + continue + } + body := rest[len(prefix):] + closing := strings.IndexFunc(body, breaksQuotedToken) + if closing <= 0 || body[closing] != '"' { + return mentionToken{}, false + } + end := start + 1 + len(prefix) + closing + 1 + if next, size := utf8.DecodeRuneInString(message[end:]); size > 0 && (next == '"' || isWordRune(next)) { + return mentionToken{}, false + } + return mentionToken{ + start: start, + end: end, + value: kind + ":" + body[:closing], + quoted: true, + }, true + } + return mentionToken{}, false +} + +// pathCandidates returns value, then value with trailing punctuation removed +// one character at a time, at most maxTrailingPunctuation times and never to +// an empty path. The caller stops at the first candidate that exists, so a +// name that really ends in punctuation still resolves and a shorter, unrelated +// name is never reached past it. +func pathCandidates(prefix, value string) []string { + candidates := []string{value} + for len(candidates) <= maxTrailingPunctuation { + last := candidates[len(candidates)-1] + r, size := utf8.DecodeLastRuneInString(last) + if size == 0 || len(last)-size <= len(prefix) || !strings.ContainsRune(trailingPunctuation, r) { + break + } + candidates = append(candidates, last[:len(last)-size]) + } + return candidates +} + +// atPath renders a resolved absolute path as a mention for the agent, quoting +// it the way Claude Code expects when it contains whitespace. +func atPath(abs string) string { + if strings.IndexFunc(abs, unicode.IsSpace) >= 0 { + return `@"` + abs + `"` + } + return "@" + abs +} diff --git a/internal/mentions/tokens_test.go b/internal/mentions/tokens_test.go new file mode 100644 index 0000000..132a2c1 --- /dev/null +++ b/internal/mentions/tokens_test.go @@ -0,0 +1,180 @@ +// Copyright (c) Mehmet Bektas + +package mentions_test + +import ( + "context" + "os" + "path/filepath" + "testing" + + "nui/internal/mentions" +) + +func writeFiles(t *testing.T, dir string, files ...string) { + t.Helper() + for _, name := range files { + path := filepath.Join(dir, name) + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte("x"), 0o644); err != nil { + t.Fatal(err) + } + } +} + +func resolve(t *testing.T, dir, msg string) string { + t.Helper() + got, err := mentions.NewRegistry(nil).ResolveMessage(context.Background(), dir, msg, nil) + if err != nil { + t.Fatal(err) + } + return got +} + +func TestResolveMessageKeepsTrailingPunctuationOutOfThePath(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, "data/README.md") + abs := filepath.Join(dir, "data", "README.md") + for _, punctuation := range []string{",", ".", ";", ":", "!", "?", ")", "),", ".)", "\u2026", ".\u201d", "\u3002"} { + msg := "Summarize @file:data/README.md" + punctuation + " then chart it" + want := "Summarize @" + abs + punctuation + " then chart it" + if got := resolve(t, dir, msg); got != want { + t.Errorf("%q: resolved = %q, want %q", punctuation, got, want) + } + } +} + +func TestResolveMessageTrimsPunctuationOneCharacterAtATime(t *testing.T) { + dir := t.TempDir() + if err := os.Mkdir(filepath.Join(dir, "data(2)"), 0o755); err != nil { + t.Fatal(err) + } + want := "compare @" + filepath.Join(dir, "data(2)") + ")." + if got := resolve(t, dir, "compare @dir:data(2))."); got != want { + t.Fatalf("resolved = %q, want %q", got, want) + } +} + +func TestResolveMessagePrefersTheNameAsWritten(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, "notes.", "notes") + want := "read @" + filepath.Join(dir, "notes.") + " first" + if got := resolve(t, dir, "read @file:notes. first"); got != want { + t.Fatalf("resolved = %q, want %q", got, want) + } +} + +func TestResolveMessageNeverReachesADifferentFile(t *testing.T) { + // Each of these names a file that does not resolve; a looser trim would + // reach its shorter sibling instead. + dir := t.TempDir() + writeFiles(t, dir, "foo.c", "data") + if err := os.Mkdir(filepath.Join(dir, "data."), 0o755); err != nil { + t.Fatal(err) + } + for _, msg := range []string{ + "open @file:foo.c++ now", // `+` is part of the name + "open @file:data. please", // `data.` exists (a folder), so `data` is never tried + "open @file:x.md...... please", // more punctuation than is ever trimmed + } { + if got := resolve(t, dir, msg); got != msg { + t.Errorf("resolved = %q, want it unchanged", got) + } + } +} + +func TestResolveMessageTrimmingStaysInsideTheWorkingDirectory(t *testing.T) { + root := t.TempDir() + dir := filepath.Join(root, "work") + writeFiles(t, root, "outside.md", "work/inside.md") + msg := "see @file:../outside.md, and @file:../outside.md." + if got := resolve(t, dir, msg); got != msg { + t.Fatalf("resolved = %q, want it unchanged", got) + } +} + +func TestResolveMessageQuotedPaths(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, "data/my notes.md", "img/logo@2x.png", "notes.md") + spaced := filepath.Join(dir, "data", "my notes.md") + at := filepath.Join(dir, "img", "logo@2x.png") + msg := `use @file:"data/my notes.md", @file:"img/logo@2x.png". and @file:"notes.md,"` + // A resolved path with whitespace is quoted the way Claude Code reads it. + // Quoting means "exactly this name", so `notes.md,` is not trimmed. + want := `use @"` + spaced + `", @` + at + `. and @file:"notes.md,"` + if got := resolve(t, dir, msg); got != want { + t.Fatalf("resolved = %q, want %q", got, want) + } +} + +func TestResolveMessageOnlyAWellFormedQuoteStartsAQuotedMention(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, "a", "a b.md") + for _, msg := range []string{ + `read @file:"a"b.csv now`, // closing quote glued to a word: one plain token + "read @file:\"a\u2028b.md\"", // a line break cannot sit inside quotes + `read @file:"a b.md`, // never closed + `read @file:"" now`, // empty + } { + if got := resolve(t, dir, msg); got != msg { + t.Errorf("resolved = %q, want it unchanged", got) + } + } +} + +func TestResolveMessageMentionMustFollowWhitespace(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, "a.md") + abs := filepath.Join(dir, "a.md") + for msg, want := range map[string]string{ + "see\u3000@file:a.md": "see\u3000@" + abs, + "see (@file:a.md)": "see (@file:a.md)", + "mail me@file:a.md": "mail me@file:a.md", + } { + if got := resolve(t, dir, msg); got != want { + t.Errorf("%q: resolved = %q, want %q", msg, got, want) + } + } +} + +func TestResolveMessageQuotedValueIsExact(t *testing.T) { + dir := t.TempDir() + writeFiles(t, dir, "notes.md ", "notes.md") + want := `read @"` + filepath.Join(dir, "notes.md ") + `" now` + if got := resolve(t, dir, `read @file:"notes.md " now`); got != want { + t.Fatalf("resolved = %q, want %q", got, want) + } +} + +type fakeExtensions struct{ resolved []string } + +func (f *fakeExtensions) ListExtensionRoots() []mentions.Item { return nil } +func (f *fakeExtensions) ListExtension(context.Context, string, string, mentions.ListRequest) (mentions.ListResponse, error) { + return mentions.ListResponse{}, nil +} +func (f *fakeExtensions) ResolveExtension(_ context.Context, _, _ string, req mentions.ResolveRequest) (string, error) { + f.resolved = append(f.resolved, req.Value) + return "[" + req.Value + "]", nil +} +func (f *fakeExtensions) MatchExtensionValue(value string) (string, string, bool) { + return "demo", "catalog", len(value) > len("ext:demo:catalog:") && value[:len("ext:demo:catalog:")] == "ext:demo:catalog:" +} +func (f *fakeExtensions) MatchExtensionParent(string) (string, string, bool) { return "", "", false } + +func TestResolveMessageExtensionValuesAreQuotedButNeverTrimmed(t *testing.T) { + // Provider values are opaque, so trailing punctuation belongs to them. + ext := &fakeExtensions{} + reg := mentions.NewRegistry(ext) + allowed := map[string]bool{"ext:demo:catalog": true} + msg := `join @ext:"demo:catalog:Q3 orders" with @ext:demo:catalog:refunds, today` + got, err := reg.ResolveMessage(context.Background(), t.TempDir(), msg, allowed) + if err != nil { + t.Fatal(err) + } + want := "join [ext:demo:catalog:Q3 orders] with [ext:demo:catalog:refunds,] today" + if got != want { + t.Fatalf("resolved = %q, want %q", got, want) + } +} diff --git a/ui/src/hooks/useMentionMenu.ts b/ui/src/hooks/useMentionMenu.ts index 60eba8b..aef6a23 100644 --- a/ui/src/hooks/useMentionMenu.ts +++ b/ui/src/hooks/useMentionMenu.ts @@ -3,6 +3,7 @@ import { useCallback, useEffect, useRef, useState } from 'react' import { api } from '@/api' import type { MentionBreadcrumb, MentionItem } from '@/types' +import { mentionToken } from '@/lib/mentionToken' export interface MentionTrigger { triggerStart: number @@ -117,7 +118,7 @@ export function useMentionMenu({ const cursor = el.selectionStart ?? input.length const before = input.slice(0, trigger.triggerStart) const after = input.slice(cursor) - const insertion = `@${item.value} ` + const insertion = `${mentionToken(item.value)} ` const next = `${before}${insertion}${after}` setInput(next) close() diff --git a/ui/src/lib/mentionToken.test.ts b/ui/src/lib/mentionToken.test.ts new file mode 100644 index 0000000..7b6ca0c --- /dev/null +++ b/ui/src/lib/mentionToken.test.ts @@ -0,0 +1,25 @@ +// Copyright (c) Mehmet Bektas + +import { describe, expect, it } from 'vitest' +import { mentionToken } from '@/lib/mentionToken' + +describe('mentionToken', () => { + it('leaves values an unquoted token can hold as they are', () => { + expect(mentionToken('file:docs/readme.md')).toBe('@file:docs/readme.md') + expect(mentionToken('builtin:files')).toBe('@builtin:files') + }) + + it('quotes values that contain whitespace or @', () => { + // Unquoted, the server would read these as `data/my` and `img/logo`. + expect(mentionToken('file:data/my notes.md')).toBe('@file:"data/my notes.md"') + expect(mentionToken('dir:My Folder')).toBe('@dir:"My Folder"') + expect(mentionToken('file:img/logo@2x.png')).toBe('@file:"img/logo@2x.png"') + expect(mentionToken('ext:demo:catalog:Q3 orders')).toBe('@ext:"demo:catalog:Q3 orders"') + expect(mentionToken('file:a\u001cb')).toBe('@file:"a\u001cb"') + }) + + it('leaves values the quoted form cannot hold unquoted', () => { + expect(mentionToken('file:say "hi" now.md')).toBe('@file:say "hi" now.md') + expect(mentionToken('ext:demo:catalog:a\u2028b c')).toBe('@ext:demo:catalog:a\u2028b c') + }) +}) diff --git a/ui/src/lib/mentionToken.ts b/ui/src/lib/mentionToken.ts new file mode 100644 index 0000000..a70adb4 --- /dev/null +++ b/ui/src/lib/mentionToken.ts @@ -0,0 +1,26 @@ +// Copyright (c) Mehmet Bektas + +// Matches the server's mention tokenizer (internal/mentions/tokens.go) and +// Chatbook mentions in notebook-intelligence (plmbr/notebook-intelligence#503). + +/** + * Whether an unquoted token would end partway through `text`: at whitespace, + * at `@`, or at U+001C to U+001F, which the server also treats as separators. + */ +function endsUnquotedToken(text: string): boolean { + return /[\s@]/u.test(text) || [...text].some((ch) => ch >= '\u001c' && ch <= '\u001f') +} + +/** + * The token to insert for a picked mention value, quoted as + * `@file:"my notes.md"` when an unquoted token would cut it short, the way + * Codex quotes a picked path. The quoted form cannot hold a quote or a line + * break, so such a value is left as is. + */ +export function mentionToken(value: string): string { + const match = /^(file|dir|ext):(.*)$/su.exec(value) + if (!match || !endsUnquotedToken(match[2]) || /["\n\r\u0085\u2028\u2029]/u.test(match[2])) { + return `@${value}` + } + return `@${match[1]}:"${match[2]}"` +}