Files
Michael Ramos 84a0b434f9 fix(ui): smart resolution + existence-validation for code-file paths (#654)
* fix(ui): smart resolution + existence-validation for code-file paths

The bare-prose / backtick path detector linkifies anything that looks
like a code path. Two failure modes regularly produce dead links: prose
abbreviations like `editor/App.tsx` (real file is
`packages/editor/App.tsx`) and references to files the plan proposes
but hasn't created yet. Both 404 on click with no UX cue.

Resolves abbreviated paths via a case-insensitive suffix-match against
a cached project walk (`resolveCodeFile` in `packages/shared/resolve-file.ts`),
mirroring what `resolveMarkdownFile` already does for markdown. The walk
is pre-warmed when the plan/annotate server boots and on every
`/api/doc` request, with a 30s TTL so newly-created files can resolve
mid-review. Storing the walk as a Promise makes the cache race-safe —
concurrent callers piggyback rather than starting a second walk.

A new `POST /api/doc/exists` endpoint takes a batch of candidate paths
and reports `found` / `ambiguous` / `missing` / `unavailable` per path.
On the frontend, `useValidatedCodePaths` extracts candidates from the
markdown on load and POSTs once. The renderer reads the result via
`CodePathValidationContext`: `found` opens directly with the resolved
absolute path, `ambiguous` opens a `CodeFilePicker` popover listing all
matches (common in monorepos where `App.tsx` exists in several
packages), `missing` demotes the link to plain code, and `unavailable`
falls back to the optimistic linkification we have today. While
validation is in flight, every detected path renders as a link, so
first paint is unchanged.

The detection itself gets a shape filter (`isPlausibleCodeFilePath`)
that hard-rejects shell brace expansion (`{a,b}`), glob wildcards, and
whitespace, while explicitly allowing `[` / `]` so Next.js dynamic
routes (`app/[slug]/page.tsx`) still resolve. The bare-prose regex moves
out of `InlineMarkdown.tsx` into `code-file.ts` so the renderer and the
new server-side extractor use the same source of truth, and the
extractor strips fenced code blocks, HTML comments, and URL ranges
before scanning so it only emits candidates the renderer would actually
paint.

Pi extension mirrors the Bun changes (handler upgrade, pre-warm,
`/api/doc/exists` route). When the popout's `/api/doc` request 404s the
dialog now surfaces "File not found in repo: <path>" instead of
silently swallowing the error.

Tests: `code-file.test.ts` extended with shape-filter and Next.js-route
cases; new `extract-code-paths.test.ts` covers extraction, dedup,
fenced/HTML/URL exclusion, and the URL-with-parens regression; new
`resolve-file.test.ts` covers the suffix-match strategy, leading `./`
handling, ambiguous results, and ignored-dir behavior.

* fix(ui): thread doc-base through code-path validator

Out-of-tree linked docs (and annotate-mode files outside cwd) reference
files relative to themselves. The validator was resolving against cwd
only, so those paths got marked missing and the renderer demoted them
to plain text — even though clicks still resolved correctly via base.

Also tightens the suffix-match's leading-segment strip so `../foo.ts`
no longer silently misresolves to an unrelated `foo.ts` in cwd.

Cleanup: delete unused extract-code-paths import in reference-handlers,
add the export entry to packages/shared so consumers don't rely on
Bun's lenient subpath resolution. Add TODO(security) comments at both
handleDocExists sites flagging that absolute paths bypass project-root
containment.

223 tests pass (3 new resolver cases for baseDir + ../ regression).

* refactor(editor): dedupe activeDocBaseDir; expand security TODO

Self-review fallout:

1. The doc-base expression `linkedDocHook.filepath ? dirname(...) :
   imageBaseDir` lived in two places (click-time URL builder and Viewer
   prop). If they drift, validator and click resolve against different
   bases and we silently re-introduce the demote-correct-link bug.
   Extract to a single useMemo.

2. The handleDocExists security TODO mentioned absolute paths in
   `paths[]` but I just added `base` acceptance, which has the same
   shape of leak (hostile sender supplies base=/secret/dir + relative
   path). Both vectors flagged in one TODO, mirrored Bun + Pi.

223 tests pass; both builds clean.

* fix(ui): code-file popout shows real error; misc consistency

Review fallout:

- `CodeFilePopout` hardcoded "File not found in repo" regardless of
  cause. The hook already captures the server's error string, so an
  ambiguous-path 400 (which can happen if a user clicks an optimistic
  link before validation completes) was surfacing as a misleading
  not-found message. Render the actual `error` and only show the
  planned/future-file caveat when the error matches "file not found".
- `InlineMarkdown` emitted demoted bare-prose paths as raw strings
  while every other plain-text branch in `emitPlainTextWithBareUrls`
  routes through `transformPlainText`. Cosmetic-only today since
  paths rarely contain transformable content, but the divergence
  invites copy-paste rot. Routed through the same helper.
- CLAUDE.md missed the new POST /api/doc/exists endpoint in both
  Plan Server and Annotate Server tables. Added.

223 tests pass; both builds clean.

* fix(ui): demote paths the extractor excluded from validation

When the validator is ready but a candidate path has no entry in the
validated map, the extractor intentionally excluded it — e.g. inside
an HTML comment or fenced code block. The renderer was optimistically
linking these because gateCodePath returned 'link' for missing entries.

Found during manual testing: `<!-- packages/editor/App.tsx -->` inside
a paragraph (parser doesn't recognize HTML comments as block-level)
was rendered as a clickable link. The extractor correctly stripped the
comment, but the renderer's optimistic fallback overrode that.

Also adds manual test harness: tests/manual/path-detection/ with
sandbox setup + three launcher scripts (plan mode, annotate in-tree,
annotate out-of-tree) covering ~30 test cases.

223 tests pass; both builds clean.

* fix(ui): skip HTML comments in InlineMarkdown scanner

The parser doesn't recognize <!-- --> as block-level HTML, so comments
inside paragraphs fall through to InlineMarkdown. The scanner then
finds paths inside the comment text and linkifies them.

The previous gateCodePath fix (demote when not in validated map) didn't
help here because the same path appeared elsewhere in the document —
the map had an entry from the non-comment occurrence.

Fix: match <!-- ... --> at the top of the scanner loop and skip the
entire comment. HTML comments should be invisible per CommonMark spec.

* fix: handle unavailable variant in markdown resolve narrowing

The shared ResolveResult type gained an `unavailable` variant for code
files. The markdown resolver never returns it, but TS can't narrow
past it without an explicit guard. Both Bun and Pi handlers now guard
`not_found || unavailable` before accessing `result.path`.
2026-05-04 14:21:16 -07:00
..

Path Detection Manual Test

Tests code-file path detection, smart resolution, existence validation, baseDir threading, and the ambiguous-match picker.

Quick start

cd tests/manual/path-detection
chmod +x *.sh

# Plan mode (primary in-repo)
./run-plan.sh --keep

# Annotate mode (primary in-repo)
./run-annotate-in-tree.sh --keep

# Annotate mode (primary outside repo — key baseDir test)
./run-annotate-out-of-tree.sh --keep

--keep prevents sandbox cleanup on exit so you can inspect files. Without it, the temp directory is removed when the script exits.

Each script: builds hook, creates a temp sandbox with a fake repo + external fixtures, launches the appropriate server, opens the browser.

What the sandbox contains

$SANDBOX/
├── repo/                               ← fake project root (git init'd)
│   ├── packages/editor/App.tsx         ← ambiguous basename (1/2)
│   ├── packages/review-editor/App.tsx  ← ambiguous basename (2/2)
│   ├── packages/ui/components/Button.tsx ← unique basename
│   ├── packages/ui/index.ts
│   ├── src/utils/helper.ts             ← abbreviated path target
│   ├── src/config.json                 ← non-.ts code file
│   ├── app/[slug]/page.tsx             ← Next.js bracket-route
│   ├── node_modules/junk/App.tsx       ← must be ignored
│   └── test-plan.md                    ← fixture plan
└── external/                           ← outside the repo
    ├── notes.md                        ← annotate-out-of-tree primary
    ├── script.ts, config.yaml, bar.ts, parent.ts
    ├── design.md                       ← linked doc target
    └── sub/subdoc.md, sub/nested.ts    ← nested linked doc

Checklist

Plan mode (run-plan.sh)

§ Case Expected
1A packages/editor/App.tsx (backtick, full) Link → opens file
1B packages/ui/components/Button.tsx (prose, full) Link → opens file
1C src/config.json (backtick, JSON ext) Link → opens file
2A editor/App.tsx (abbreviated) Link → opens packages/editor/App.tsx
2B utils/helper.ts (abbreviated, prose) Link → opens src/utils/helper.ts
2C ./editor/App.tsx (leading ./) Link → opens packages/editor/App.tsx
3A Button.tsx (unique basename) Link → opens packages/ui/components/Button.tsx
3B App.tsx (ambiguous basename) Link with badge → picker → two entries
3C helper.ts (unique basename) Link → opens src/utils/helper.ts
4A packages/ui/shortcuts/core.ts (missing, backtick) Plain <code>, not clickable
4B packages/ui/shortcuts/runtime.ts (missing, prose) Plain text, not a link
5A packages/ui/{core,runtime}.ts (braces) Plain <code>, not a link
5B packages/ui/*.tsx (glob) Plain <code>, not a link
5C some path/with spaces/file.ts (spaces) Plain <code>, not a link
6A app/[slug]/page.tsx (bracket route) Link → opens file
6B [slug]/page.tsx (abbreviated bracket) Link → opens app/[slug]/page.tsx
7A junk/App.tsx (node_modules) Plain text or only real App.tsx matches
8A URL with .ts extension URL link only, no path leak
8B URL + real path on same line Two separate links
8C Wikipedia-style parens URL Single URL link
9 Paths inside fenced code block No links inside the block
10 Paths inside HTML comment Invisible, no links
11A ../script.ts (no baseDir in plan) Plain text (demoted)
12 Click linked doc → external/notes.md Overlay opens; verify paths inside

Annotate out-of-tree (run-annotate-out-of-tree.sh)

Case Expected
notes.md script.ts Link → opens external/script.ts
notes.md config.yaml Link → opens external/config.yaml
notes.md editor/App.tsx (cross-context) Link → opens repo's packages/editor/App.tsx via cwd walk
notes.md packages/ui/shortcuts/core.ts Plain text (missing everywhere)
Click [Open design doc] Overlay; bar.ts → external/bar.ts
Click [Open subdoc] → subdoc.md Overlay; ../parent.ts → external/parent.ts
subdoc.md nested.ts Link → opens external/sub/nested.ts
subdoc.md ../missing.ts Plain text (doesn't exist)

Network tab checks

  • On plan/annotate load: exactly one POST /api/doc/exists
  • When a linked doc overlay opens: one additional POST
  • Plan mode POST body: { paths: [...] } (no base field)
  • Annotate out-of-tree POST body: { paths: [...], base: "<external dir>" }
  • Linked doc POST body: { paths: [...], base: "<linked doc's parent>" }