Bug: jj backend passes paths as fileset expressions, corrupting or breaking diffs for punctuated filenames

从 app/diff/jj.go 开始阅读 FileDiff 和 totalOldLines，重点关注在 -- 之后传入的两个路径参数。添加回归覆盖，断言包含标点符号的路径对应的 jj argv，而不是只检查是否返回了 diff。完成标准是两个受影响的调用都能安全地只指向一个字面文件，同时 jjblame.go 保持不变。

Summary

(*Jj).FileDiff appends the file path after -- on the jj diff command line. jj parses those arguments as fileset expressions — a query language with globs, set operators and quoting — not as literal paths. A filename only works if it happens to be a valid bare fileset string.

Any filename containing punctuation is therefore at risk, in one of two ways. Of 31 filenames tested through (*Jj).FileDiff, 23 produced a wrong result:

• 19 fail loudly. Characters such as $, (, :, # don't parse. The diff pane shows error loading diff and the file can't be reviewed.
• 5 fail silently. * and ? are globs; &, |, ~ are set operators. jj exits 0, and revdiff renders a diff for the wrong set of files — either empty, or other files' diffs concatenated into this one. Nothing indicates a problem.

The silent class is the more serious one: it shows wrong content under a correct-looking filename, and annotations made there are exported against line numbers that don't exist.

Repro — loud failure

jj git init
printf 'one\ntwo\n' > '$test.txt'
jj describe -m base && jj new
printf 'one\ntwoX\n' > '$test.txt'
revdiff

The file lists in the tree normally (jj diff --summary takes no path arg). Selecting it shows:

error loading diff: get file diff for $test.txt: jj diff --git --context=1000000 -- $test.txt:
Error: Failed to parse fileset: Syntax error
  --> 1:1
  |
1 | $test.txt
  | ^---
  = expected <strict_identifier>, <bare_string>, or <expression>

Works when quoted: jj diff --git -- 'root-file:"$test.txt"'

Repro — silent failure

Two files, where one name globs onto the other:

jj git init
printf 'one\n' > 'a*b.txt'; printf 'one\n' > 'axb.txt'
jj describe -m base && jj new
printf 'two\n' > 'a*b.txt'; printf 'two\n' > 'axb.txt'

FileDiff{Path: "a*b.txt"} returns no error and 8 rows for what is a one-line change:

[0] type="-" old=1 new=0 "one"
[1] type="+" old=0 new=1 "two"
[2] type=" " old=2 new=2 "diff --git a/axb.txt b/axb.txt"     <- other file's header as context
[3] type=" " old=3 new=3 "index 5626abf0f7..f719efd430 100644"
[4] type="-" old=4 new=0 "-- a/axb.txt"                        <- marker mangled into a removal
[5] type="+" old=0 new=4 "++ b/axb.txt"
[6] type="-" old=1 new=0 "one"                                 <- numbering restarts
[7] type="+" old=0 new=1 "two"

axb.txt's content is rendered inside a*b.txt's diff. Because parseUnifiedDiff expects a single-file diff, the embedded diff --git/index headers are absorbed as context lines and the ---/+++ markers become fake add/remove rows with the first character eaten. Line numbers restart per embedded file.

This scales with the number of matches — in a 31-file test repo the same call returned 184 rows.

Annotations key on path + line number, and both are wrong here, so an annotation placed on one of these rows is written to -o output against a line that doesn't exist. That makes this a correctness bug in exported review output, not only a display bug.

Root cause

Two call sites, both app/diff/jj.go:

Site	Call	Behavior
jj.go:149	(*Jj).FileDiff — append(args, "--", req.Path)	the failures above
jj.go:193	(*Jj).totalOldLines — jj file show ... -- <file>	silent; error discarded, returns 0, so compact mode drops the trailing ⋯ N lines ⋯ divider

git and hg take literal pathspecs; jj replaced that with the fileset language. fileset appears nowhere in the codebase — the semantics were never accounted for.

Full character results

31 filenames of the form a<char>b.txt, each driven through (*Jj).FileDiff:

Outcome	n	Characters
Silent glob	2	* ?
Silent empty	3	& | ~
Parse error	19	! " # $ % ' ( ) , : ; < = > [ ^ ` { }
Correct	8	+ - . @ ] _ ␣ (incl. plain baseline; ] is safe, [ is not)

Representative ASCII punctuation, not an exhaustive byte sweep.

Not affected

• Blame — jj file annotate (jjblame.go:24) takes a genuine path. Raw name works; cwd-file:"…" fails there (No such path). Must not be changed.
• --all-files — jj file list (directory.go:51) takes no path args.
• git — verified with $ok.txt and :top.txt.
• hg — hg.go:155 passes literal paths (hg needs explicit glob:/re:). Reasoned from code only — hg not installed, not executed.

Suggested fix

Quote the path as a file pattern at the two affected sites only:

// jj parses post-`--` args as fileset expressions, so $ ! ( ) : , break the
// parse and * ? & | ~ silently resolve to the wrong set of files.
func jjFilesetPath(path string) string {
	esc := strings.NewReplacer(`\`, `\\`, `"`, `\"`).Replace(path)
	return `root-file:"` + esc + `"`
}

Escaping verified for a"b.txt → root-file:"a\"b.txt" and a\b.txt → root-file:"a\\b.txt".

• Prefer root-file: over cwd-file: — NewJj gets vcsRoot (renderer_setup.go:53) so they're equivalent today, but root-file: survives a workDir change.
• Do not apply to jjblame.go.
• Worth adding separately: reject input with more than one diff --git header in parseUnifiedDiff. That's the layer that turned a bad path argument into plausible-looking rows.

Tests

No jj test uses a path with punctuation. A regression test should assert the argv passed to jj — asserting "a diff came back" would pass for a*b.txt, which currently returns wrong rows with no error.

Environment

revdiff master e3eb732 · jj 0.44.0-af45d57 · go 1.27.0 linux/amd64