fix(jj): quote file paths so punctuated filenames diff correctly (#346)

jj parses arguments after `--` as fileset expressions, not literal paths, so `(*Jj).FileDiff` and `(*Jj).totalOldLines` were handing it queries instead of filenames. Names containing `$ ( ) : #` failed to parse and showed `error loading diff`. Two other classes fail silently, with jj exiting 0. `*` and `?` are globs, so jj returns a diff for a *different* set of files, and `parseUnifiedDiff` absorbs the extra headers as bogus context and add/remove rows under a correct-looking filename; annotations placed on those rows export against line numbers that do not exist, which corrupts review output rather than only the display. `&`, `|` and `~` are set operators that resolve to nothing, so the file renders as an empty diff with nothing on screen saying why.

Both call sites now route through `(*Jj).pathArg`, which quotes the path as `cwd-file:"<escaped>"`. `jj file annotate` is deliberately left alone, since it takes a literal path and rejects a pattern with `No such path`.

`cwd-file:` rather than `root-file:`. `root-file:` is workspace-relative and rejects an absolute path at parse time, which would have broken `--only=/abs/path` in a jj repo: `FallbackRenderer` hands the user's original absolute `--only` string to `FileDiff`, and the parse error returns before the context-only disk read that renders the file today. The documented Zed task passes `$ZED_FILE`, which is always absolute. Jujutsu runs with `cmd.Dir = workDir` and emits paths relative to it, so `cwd-file:` resolves what `ChangedFiles` reported, plus absolute paths and `../` siblings.

Quoting is unconditional because revdiff needs jj 0.27 or newer anyway, and 0.27 is where the `ui.allow-filesets` opt-out was removed. Official binaries establish that floor: `jj file annotate` is absent before 0.23 and gains `-T` only at 0.27, and the `\x00` escapes in `jjCommitLogTemplate` do not parse before 0.23. So the blame gutter is broken throughout 0.20 to 0.26 and the commit-info popup additionally before 0.23. README, `site/docs.html` and the gotchas note now state it.

Tests cover the escaping table and exercise punctuated names and the compact-mode `totalOldLines` path against a real jj repo; those cases fail on master. The absolute-path case guards `cwd-file:` against `root-file:`.

Related to #341

Author: umputun

.claude/rules/gotchas.md

@@ -9,6 +9,7 @@
 - `--all-files` mode uses `DirectoryReader` (git ls-files) to list all tracked files; `--include` wraps any renderer with `IncludeFilter` for prefix-based inclusion, `--exclude` wraps with `ExcludeFilter` for prefix-based exclusion (include narrows first, then exclude removes). Those wrappers only filter `ChangedFiles`; untracked files come straight from the VCS `UntrackedFiles` call and bypass the renderer chain, so `filterUntracked` (in `renderer_setup.go`) re-applies the same prefixes via `diff.FilterPaths` at the composition root — a future change to include/exclude filtering must touch both paths or untracked scoping silently re-breaks. `--include` is mutually exclusive with `--only`. `--all-files` is mutually exclusive with refs, `--staged`, and `--only`. `--stdin` is mutually exclusive with refs, `--staged`, `--only`, `--all-files`, `--include`, and `--exclude`. `--compare-old/--compare-new` is mutually exclusive with refs, `--staged`, `--only`, `--all-files`, `--stdin`, `--include`, `--exclude`, and `--annotations`.
 - Rename-aware diffs (git-only): `FileDiff` takes a single `diff.FileDiffRequest` value (`Ref`, `Path`, `OldPath`, `Staged`, `ContextLines`) — not positional args — across all three interfaces (`diff.Renderer`, `ui.Renderer`, `review.FileDiffer`) and every implementation/wrapper. `FileEntry.OldPath` is the rename origin (empty for non-renames), populated only by `(*Git).ChangedFiles` from git's `R<score> old new` / `C<score>` pairs. `(*Git).pathArgs(req)` emits `-M -- <old> <new>` when `OldPath != "" && OldPath != Path` so git pairs the rename into a minimal diff (otherwise `-- <path>`); the old-side probes `totalOldLines`/`binarySizeDesc` read `OldPath` when set. hg and jj never set `OldPath` (jj decomposes renames into delete+add; hg `status` reports `R` as *removed*), so non-git renderers ignore it — no behavior change there. UI: `sidepane.FileTree` exposes `OldPath(path)` (parallel to `FileStatus(path)`, populated in `NewFileTree` + `Rebuild`); `fetchEffectiveFileDiff` threads it into the request, and `view.go` renders `old → new` in the diff-pane header via `loadedFileState.oldName` / `fileLoadedMsg.oldName` (still passed through `truncateHeaderTitle`). Review stats (`ComputeStats`) and the annotations preloader (`lookupLineSet`) also set `OldPath` so their per-file diffs match the displayed rename-aware diff.
 - Untracked renames (git-only): a plain `mv old new` (no `git mv`/staging) leaves `old` as an unstaged deletion and `new` untracked, so `git diff -M` never pairs them — `(*Git).ChangedFiles` reports only `D old`. `(*Git).UntrackedRenames(untracked)` recovers the pairing off a **throwaway index**: `(*Git).tempIndexWithIntentToAdd` copies `.git/index` to a temp file (resolved via `git rev-parse --git-path index`), runs `git add -N -- <untracked>` against the copy with `GIT_INDEX_FILE` set (via `runGitEnv`/`runVCSEnv`), then `git diff --name-status -M` reports the pairs as renames; the real index and working tree are never touched. It returns `FileEntry{Status: FileRenamed, Path: new, OldPath: old}` only for renames whose new side is in the passed (already include/exclude-filtered) untracked set. Wiring: `vcsSetup.untrackedRenamesFn` (git case only → `g.UntrackedRenames`, nil for hg/jj) → `ModelConfig.LoadUntrackedRenames` → `Model.loadUntrackedRenames`. `loadFiles` merges via `detectUntrackedRenames` (gated to unstaged working-tree mode: `m.cfg.ref == "" && !m.cfg.staged`) + `mergeUntrackedEntries`, which drops the standalone `D old` and skips the new path from the plain-untracked append. `FileDiff` renders these via `untrackedRenameDiff` (same throwaway-index trick), selected by `isUntrackedRename(req)` = `Ref == "" && !Staged && OldPath != "" && OldPath != Path` — the only producer of `OldPath` in unstaged mode is `UntrackedRenames`, so that condition uniquely identifies the case. `(*Git).parseNameStatusEntries` is the shared NUL-field name-status parser for both `ChangedFiles` and `UntrackedRenames`. All git calls in this path use `(*Git).renameIndexEnv` which sets `GIT_INDEX_FILE` **and** `GIT_LITERAL_PATHSPECS=1` so a working-tree filename that looks like pathspec magic (e.g. `:(top)x`) is treated literally rather than as a pathspec. A fresh repo with no commits has no `.git/index`; `UntrackedRenames` treats that `fs.ErrNotExist` as "no renames possible" and returns `nil, nil`. The `--annotations` preloader (`app/annotations_load.go`) mirrors this: `preloadAnnotations` takes the same `untrackedRenamesFn`, and `(*preloader).foldUntrackedRenames` upgrades untracked entries to `FileRenamed` + records `OldPath` + drops the origin deletion under the same gate, so `lookupLineSet` resolves the rename-aware diff and `(-)`/context annotations on untracked renames round-trip (without it the preloader read `new` as all-added and dropped them).
+- jj paths are fileset expressions, not literal paths (issue #341): `jj diff` and `jj file show` parse post-`--` arguments as jj's fileset query language, so `$ ( ) : #` fail to parse and `* ? & | ~` silently resolve to a *different* set of files — the latter concatenates other files' diffs into one file's view, which `parseUnifiedDiff` absorbs as bogus context/add/remove rows under a correct-looking filename. Every path handed to those two commands goes through `(*Jj).pathArg`, which wraps it as `cwd-file:"<escaped>"` (backslash and double-quote escaped). This is the jj analogue of git's `GIT_LITERAL_PATHSPECS=1`. Unconditional, because **revdiff's jj floor is 0.27** (README). Measured against official binaries: blame is broken throughout 0.20-0.26 (`jj file annotate` absent before 0.23, present but without `-T` through 0.26), and the commit-info popup additionally fails before 0.23 because the `\x00` escapes in `jjCommitLogTemplate` do not parse. So at least one advertised feature is broken on every jj below 0.27, though the core diff pane itself works further back. 0.27 is also where the `ui.allow-filesets` opt-out was removed, so every *supported* jj parses filesets and the quoting needs no capability probe or version branch. Do not add one unless the floor is deliberately lowered below 0.27. **`cwd-file:` not `root-file:`** — jj runs with `cmd.Dir = workDir` and emits paths relative to it, so `cwd-file:` resolves whatever `ChangedFiles` reported. `root-file:` rejects an absolute path outright, which regressed `--only=/abs/path` (the documented Zed task passes `$ZED_FILE`, always absolute) from a working context-only fallback to `error loading diff`; it also rejects a `../` sibling and silently matches the wrong file for a path below a `workDir` that is not the repo root. **Do not apply it to `jj file annotate`** (`jjblame.go`) — that takes a genuine path and rejects a fileset with `No such path`; likewise `jj file list` (`directory.go`) takes no path args. hg passes literal paths (it needs an explicit `glob:`/`re:` prefix) and needs no equivalent.
 - `diff.readReaderAsContext()` is the shared parser for file-backed and stdin-backed context-only views. Preserve its behavior if you change binary detection, line-length handling, or line numbering.
 - Overlay popups managed by `overlay.Manager`. `Compose()` uses ANSI-aware compositing via `charmbracelet/x/ansi.Cut`. `HandleKey()` returns `Outcome` — Model switches on `OutcomeKind` for side effects (file jumps, theme apply/persist). Overlay kinds: help, annot-list, theme-select, info, file-picker. One overlay at a time — opening any overlay auto-closes whichever was previously open
 - File picker key binding: `jump_file` defaults to `P`, deliberately NOT `ctrl+p` — host terminals claim that chord (agterm binds it to `session_palette` by default, so it never reaches the TUI) and a swallowed key reads to users as a missing feature. Consequence of choosing a printable default: `filePickerOverlay.handleKey` calls `appendPrintableRunes` FIRST, and that consumes every unmodified printable rune (plus `KeySpace`) regardless of which action the key resolves to — so inside an open picker `P` extends the filter and the `action == keymap.ActionJumpFile` close branch below it is unreachable for the default binding. That ordering is required: reversing it would break filtering for whichever *bare printable* keys happen to be bound to `jump_file`, `up` or `down` — `P`/`j`/`k` under the defaults, but equally any bare printable key a user maps to those three actions. Conversely the three action branches are reachable by anything `appendPrintableRunes` does not consume — an alt-modified rune (`map alt+f jump_file`), a function key (`map f1 jump_file`), a ctrl chord — so toggle-close survives through any non-printable binding, not only a modified chord. Any future overlay action bound to a bare printable key inherits the same one-way behavior.

README.md

@@ -43,6 +43,7 @@ Built for a specific use case: reviewing code changes, plans, and documents with
 ## Requirements
 
 - `git`, `hg`, or `jj` (used to generate diffs; optional when using `--only` or `--stdin`)
+- Jujutsu must be 0.27 or newer. Versions 0.23 through 0.26 have no `jj file annotate -T`, so the blame gutter does not work. Versions before 0.23 also reject the commit-log template behind the commit-info popup.
 
 ## Installation
 

app/diff/jj.go

@@ -146,7 +146,7 @@ func (j *Jj) FileDiff(req FileDiffRequest) ([]DiffLine, error) {
 	args := make([]string, 0, 5+len(rangeArgs))
 	args = append(args, "diff", "--git", jjContextArg(req.ContextLines))
 	args = append(args, rangeArgs...)
-	args = append(args, "--", req.Path)
+	args = append(args, "--", j.pathArg(req.Path))
 
 	out, err := j.runJj(args...)
 	if err != nil {
@@ -190,13 +190,27 @@ func (j *Jj) totalOldLines(ref, file string) int {
 	if oldRef == "" {
 		oldRef = "@-"
 	}
-	out, err := j.runJj("file", "show", "-r", oldRef, "--", file)
+	out, err := j.runJj("file", "show", "-r", oldRef, "--", j.pathArg(file))
 	if err != nil {
 		return 0
 	}
 	return countLines(out)
 }
 
+// pathArg quotes a path as a jj fileset pattern. jj parses post-`--` arguments as fileset
+// expressions rather than names, so a bare path is a query: `$ ( ) :` fail to parse and
+// `* ? & | ~` resolve to a different set of files. Unconditional because revdiff needs jj
+// 0.27+ anyway (see README), and 0.27 is where the ui.allow-filesets opt-out was removed —
+// every jj that can run revdiff parses filesets.
+//
+// cwd-file: rather than root-file: because jj runs with cmd.Dir = workDir and emits paths
+// relative to it, so absolute and "../" paths resolve too (see .claude/rules/gotchas.md).
+// Not for `jj file annotate` (jjblame.go), which takes a literal path and rejects a pattern.
+func (j *Jj) pathArg(path string) string {
+	esc := strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(path)
+	return `cwd-file:"` + esc + `"`
+}
+
 // jjContextArg returns the --context argument for jj diff given the caller's
 // requested context size. A non-positive contextLines or one at or above
 // fullContextSentinel returns the full-file arg; any other value returns

app/diff/jj_test.go

@@ -603,3 +603,111 @@ func TestJj_FileDiff_SmallContext(t *testing.T) {
 	}
 	assert.Equal(t, 19, fullCtx, "expected 19 context lines with full-file context")
 }
+
+func TestJj_PathArg(t *testing.T) {
+	j := &Jj{}
+	tests := []struct {
+		name, path, want string
+	}{
+		{"plain", "hello.txt", `cwd-file:"hello.txt"`},
+		{"nested", "app/diff/jj.go", `cwd-file:"app/diff/jj.go"`},
+		{"dollar", "$test.txt", `cwd-file:"$test.txt"`},
+		{"glob star", "a*b.txt", `cwd-file:"a*b.txt"`},
+		{"set operator", "a|b.txt", `cwd-file:"a|b.txt"`},
+		{"double quote", `a"b.txt`, `cwd-file:"a\"b.txt"`},
+		{"backslash", `a\b.txt`, `cwd-file:"a\\b.txt"`},
+		{"backslash then quote", `a\"b.txt`, `cwd-file:"a\\\"b.txt"`},
+		{"absolute", "/tmp/repo/a.txt", `cwd-file:"/tmp/repo/a.txt"`},
+		{"parent relative", "../top.txt", `cwd-file:"../top.txt"`},
+		{"empty", "", `cwd-file:""`},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			assert.Equal(t, tt.want, j.pathArg(tt.path))
+		})
+	}
+}
+
+func TestJj_FileDiff_PunctuatedPaths(t *testing.T) {
+	// issue #341: unquoted paths parsed as filesets, so punctuation broke or globbed
+	dir := setupJjRepo(t)
+	j := NewJj(dir)
+
+	names := []string{"a*b.txt", "axb.txt", "$test.txt", "a(b).txt", "a|b.txt", "a?b.txt"}
+	for _, n := range names {
+		writeFile(t, dir, n, n+" old\n")
+	}
+	jjCmd(t, dir, "describe", "-m", "init", "--quiet")
+	jjCmd(t, dir, "new", "-m", "modify", "--quiet")
+	for _, n := range names {
+		writeFile(t, dir, n, n+" new\n")
+	}
+
+	for _, n := range names {
+		t.Run(n, func(t *testing.T) {
+			lines, err := j.FileDiff(FileDiffRequest{Path: n})
+			require.NoError(t, err)
+			require.Len(t, lines, 2, "expected exactly the one-line change for %q, got %+v", n, lines)
+			assert.Equal(t, ChangeRemove, lines[0].ChangeType)
+			assert.Equal(t, n+" old", lines[0].Content)
+			assert.Equal(t, ChangeAdd, lines[1].ChangeType)
+			assert.Equal(t, n+" new", lines[1].Content)
+		})
+	}
+}
+
+func TestJj_FileDiff_AbsolutePath(t *testing.T) {
+	// issue #341 follow-up: root-file: rejected absolute paths, breaking --only=/abs/path
+	dir := setupJjRepo(t)
+	j := NewJj(dir)
+
+	writeFile(t, dir, "changed.txt", "old\n")
+	writeFile(t, dir, "same.txt", "steady\n")
+	jjCmd(t, dir, "describe", "-m", "init", "--quiet")
+	jjCmd(t, dir, "new", "-m", "modify", "--quiet")
+	writeFile(t, dir, "changed.txt", "new\n")
+
+	resolved, err := filepath.EvalSymlinks(dir)
+	require.NoError(t, err)
+
+	lines, err := j.FileDiff(FileDiffRequest{Path: filepath.Join(resolved, "changed.txt")})
+	require.NoError(t, err)
+	require.Len(t, lines, 2)
+	assert.Equal(t, "old", lines[0].Content)
+	assert.Equal(t, "new", lines[1].Content)
+
+	lines, err = j.FileDiff(FileDiffRequest{Path: filepath.Join(resolved, "same.txt")})
+	require.NoError(t, err, "an unchanged file must return empty, not an error")
+	assert.Empty(t, lines)
+}
+
+func TestJj_TotalOldLines_PunctuatedPath(t *testing.T) {
+	// issue #341: jj file show parses its path as a fileset, so the count came back 0
+	dir := setupJjRepo(t)
+	j := NewJj(dir)
+
+	var sb strings.Builder
+	for i := 1; i <= 20; i++ {
+		fmt.Fprintf(&sb, "line %d\n", i)
+	}
+	writeFile(t, dir, "$big.txt", sb.String())
+	jjCmd(t, dir, "describe", "-m", "init", "--quiet")
+	jjCmd(t, dir, "new", "-m", "modify", "--quiet")
+
+	sb.Reset()
+	for i := 1; i <= 20; i++ {
+		if i == 5 {
+			fmt.Fprintf(&sb, "line %d CHANGED\n", i)
+			continue
+		}
+		fmt.Fprintf(&sb, "line %d\n", i)
+	}
+	writeFile(t, dir, "$big.txt", sb.String())
+
+	assert.Equal(t, 20, j.totalOldLines("", "$big.txt"))
+
+	lines, err := j.FileDiff(FileDiffRequest{Path: "$big.txt", ContextLines: 2})
+	require.NoError(t, err)
+	require.NotEmpty(t, lines)
+	assert.Equal(t, ChangeDivider, lines[len(lines)-1].ChangeType, "expected a trailing divider")
+}

site/docs.html

@@ -112,6 +112,7 @@ <h1>Documentation</h1>
 
         <h2 id="requirements">Requirements</h2>
         <p><code>git</code>, <code>hg</code> (Mercurial), or <code>jj</code> (Jujutsu) is used to generate diffs. VCS is optional when using <code>--only</code> for standalone file review or <code>--stdin</code> for scratch-buffer review. In colocated <code>git</code>+<code>jj</code> repositories, revdiff uses the <code>jj</code> working-copy model.</p>
+        <p>Jujutsu must be 0.27 or newer. Versions 0.23 through 0.26 have no <code>jj file annotate -T</code>, so the blame gutter does not work. Versions before 0.23 also reject the commit-log template behind the commit-info popup.</p>
 
         <h2 id="installation">Installation</h2>
         <h3>Homebrew (macOS/Linux)</h3>