Commit Graph

5 Commits

Author SHA1 Message Date
Michael Ramos d2d2dba7fa feat(annotate): configurable extra markdown extensions (#1309)
* feat(annotate): configurable extra markdown extensions (#1307)

Adds a config-only `markdownExtensions` key to ~/.plannotator/config.json,
e.g. { "markdownExtensions": [".livemd"] } for Livebook notebooks. A listed
extension is accepted everywhere .md is on the annotate path: CLI target
resolution, folder discovery and the file browser, /api/doc plus relative and
wiki-link navigation between sibling docs, the 2MB size cap, and per-file
version history. Listed extensions render as markdown with frontmatter
stripped, never as raw HTML, and they only widen the accepted set.

Design:
- packages/core/annotatable.ts stays browser-safe and zero-dep. Its regexes
  and predicates now take an optional, defaulted-empty list of extra
  extensions, plus a normalizer and regex builders.
- packages/shared/markdown-extensions.ts is the node-side seam: it reads
  config.json once per process through the existing loadConfig() and threads
  the normalized list into those pure functions. resolve-file re-exports the
  config-aware predicates so both runtimes pick them up; the Bun server, the
  Pi mirror, the OpenCode plugin and the CLI all go through them.
- The annotate /api/plan payload ships the resolved list so the renderer can
  linkify links to sibling documents (module-level UI registry, empty by
  default, so nothing changes without config).

Validation: entries must be dot-led, lowercase-normalized, and free of path
separators, globs and whitespace. Invalid entries are dropped silently,
built-ins are deduplicated, and `.env` is denylisted so config can never
register it (annotate copies file contents into the data dir).

Deliberately unchanged: the Pi plan-write allowlist (ALLOWED_PLAN_EXTENSIONS
in tool-scope.ts) and Edit Mode source save (SOURCE_SAVE_FILE_REGEX), which
keep their own narrower allowlists.

* fix(annotate): deny the dotenv family and sandbox config-aware tests

Review follow-ups on #1309:
- deny the whole dotenv family (.prod.env, .env.local, ...) in
  normalizeMarkdownExtensions, not just the exact .env name
- resolve config.json path per call instead of at module scope so
  PLANNOTATOR_DATA_DIR sandboxing works in single-process test runs
- stop resolve-file.test.ts reading the real user config: pure
  predicate imports plus pinned empty extras on every resolve call
- add the config.json -> memo -> predicate integration test using
  resetMarkdownExtensionsCache under a temp data dir

* test(call-flow): make the stale-read advert test self-sufficient

The read-only GET only probes the node runtime while Call flow is
enabled. The stale-read test relied on earlier tests' settings POSTs
leaking callFlow=true through the process-frozen config path; with lazy
config resolution each sandbox is genuinely isolated, so the test now
enables Call flow in its own data dir. Locally the dependency was
masked by an fnm-shimmed sem sidecar spawning node coincidentally.
2026-08-13 09:47:18 -07:00
Michael Ramos f9a6c1e39d feat: annotate accepts YAML, JSON, TOML and other plain-text files (#1099)
* feat(annotate): accept common plain-text config formats (.yaml, .json, .toml, …)

Annotate previously rejected every file that wasn't .md/.mdx/.txt (or
.html/.htm), even though the pipeline reads files as UTF-8 text and
renders anything. Widen the accepted set to unambiguously plain-text
config/data formats: .yaml .yml .json .jsonc .json5 .toml .ini .cfg
.conf .properties .csv .tsv .log .xml .env.example. They render exactly
the way .txt renders today.

- New single source of truth: packages/core/annotatable.ts
  (ANNOTATABLE_TEXT_REGEX / ANNOTATABLE_DOC_REGEX + predicates),
  re-exported through @plannotator/shared/resolve-file and vendored into
  the Pi extension.
- .env stays excluded (commonly holds secrets; annotate history copies
  file contents into the data dir). Source-code extensions stay with
  code review.
- Single-file accept + bare-filename fuzzy search widen in
  resolveMarkdownFile; folder discovery and the file-browser listing
  widen in all three runtimes (hook CLI, OpenCode, Pi).
- /api/doc gains a `doc=1` param set by the file browser so extensions
  that overlap CODE_FILE_REGEX (.yaml/.json/.toml/.ini/.xml) render as
  annotatable documents there while code-file links inside documents
  keep the syntax-highlighted popout.
- Error messages now list the wider set; docs updated (AGENTS.md,
  marketing annotate page).

Closes #1029

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk

* fix(annotate): frontmatter, size caps, edit-guard, and skill docs from review

Review fixes for #1099:

- Frontmatter: `--- … ---` stripping is a markdown convention; for
  non-markdown annotatable sources (multi-document YAML, .txt starting
  with ---) the delimiters are real content. parseMarkdownToBlocks gains
  a { frontmatter } option and the editor keys it off the active
  document's path via shouldStripFrontmatter() (strip for .md/.mdx and
  pathless/converted sources; keep raw for other annotatable text).
- Size caps: new shared MAX_ANNOTATABLE_FILE_BYTES (2MB — same limit the
  code-file popout always had) now guards the annotate CLI single-file
  read in all three runtimes and the /api/doc document branches in both
  servers. Also applies to .md/.txt (behavior change for pathological
  inputs; previously unbounded).
- Editing guard: mid-edit file opens gate on isSourceSaveFilePath
  (.md/.mdx/.txt) instead of the wider annotatable set — config files
  are view-only, so switching to one mid-edit no longer silently
  downgrades "Done editing" to feedback-only edits.
- Skill docs: plannotator-annotate SKILL.md (core + Kiro) now mention
  the plain-text config formats.

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk
2026-07-20 15:33:42 -07:00
Michael Ramos 8a6288b9fe fix(annotate): resolve explicit ../ paths that escape the project root
plannotator annotate ../docs/plan.md failed with a misleading 'File type
not supported: .md' because resolveMarkdownFile rejected any relative path
resolving outside the project root, even when the file existed and was a
supported type. The CLI then found the file on disk and mislabeled the
resolver miss as a type error.

An explicit path the user types (one containing a separator, including ../)
is now honored when it exists, matching the trust already given to absolute
paths. Bare filenames stay restricted to the in-root fuzzy search, so a
stray notes.md cannot resolve into a parent directory.

Closes #1085.

Claude-Session: https://claude.ai/code/session_01YXkgsNucxDwAL4GdR4XYRk
2026-07-20 01:55:10 -07:00
Michael Ramos 7f0c36028b fix(server): bound startup file discovery (#1036)
* fix(server): bound startup file discovery (#978)

* test: observe the server's own warm cache key so the ordering tests actually pin the fix

Review finding: both bind-before-warm ordering tests raced against
observeWarmState(projectRoot), but on macOS mkdtempSync returns /var/...
while the chdir'd server warms under the realpath /private/var/... —
a different warmFileListCache key. The tests therefore raced a FRESH
warm (always pending at observation time) and passed on the OLD broken
code too. Observing process.cwd() inside onReady uses the server's real
key. Verified: with old resolve-file/server code checked out, all four
ordering tests now fail; on the fix they pass (17/17 across both files).

Claude-Session: https://claude.ai/code/session_01SFy9fY27SA8g5BtotWPi1G
2026-07-10 10:18:01 -07:00
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