mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 08:29:42 +02:00
fix(files): keep attachment markdown refs from resolving into the workspace, and render files without chat line breaks (#503 review)
- A markdown preview opened by attachment id under a bare file name
(attachment cards, history drawer) no longer resolves relative refs
against the workspace root: filePreviewText carries attachmentId, and
the rebase pass turns those images into their alt text and unwraps
those links. Absolute-path and workspace previews are unchanged.
- _renderMarkdown(text, { breaks = true } = {}): the File Viewer passes
breaks: false, so a hard-wrapped paragraph renders as one paragraph;
the Response Viewer keeps a <br> per newline.
- Absolute paths linkified inside a rendered document now carry the
preview's data-session-id.
- CLAUDE.md, architecture-invariants and the Working-With-Files wiki page
now say that only an in-workspace path clicked in the terminal keeps
the tail viewer.
- Tests in test/file-preview-markdown.test.ts for all three fixes,
including an end-to-end run of the shipping app.js + marked + DOMPurify.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -392,12 +392,12 @@ Tests: `test/file-editing-policy.test.ts` (pure policy), `test/routes/file-write
|
||||
|
||||
**Rendered markdown + Lines/Wrap** (`_renderFilePreviewText()` and its helpers in `panels-ui.js`, buttons in the `.file-preview-actions` row): a `.md`/`.markdown` opened in the File Viewer renders as a document by default, with an `MD` pill back to source; the plain-text view has `Lines` (a CSS-counter gutter) and `Wrap` toggles. Codeman already had `marked` + DOMPurify behind `_renderMarkdown()` for the Response Viewer, so the viewer reuses that and the codebase keeps ONE markdown pipeline.
|
||||
|
||||
- ⚠️ **One pipeline, one delegate.** The viewer calls `_renderMarkdown()` (marked + the `sanitize-html.js` allowlist) and binds `_bindResponseViewerInteractions()` on `#filePreviewBody` (container-bound and idempotent, so once per page) for the code-block copy buttons and `a.rv-path` opening. Never a second parser, never a second click handler for the same markup.
|
||||
- ⚠️ **Build inside a `<template>`, then rebase.** A detached div whose `innerHTML` is set starts fetching every `<img src>` at once, so the document's relative image paths would hit the server as `/docs/img.png` 404s before being rewritten. `_rebaseFilePreviewMarkdownRefs()` runs on the template content: relative images go to the workspace-confined `file-raw` under the document's directory, root-relative ones (`/docs/x.png`) under the workspace root as on GitHub (the server refuses escapes, so `..` is forwarded as-is), and one `error` handler per image degrades it to alt text, which covers a remote image the page CSP blocks, a 404 for a document outside the workspace, and an SVG that `file-raw` serves as a download. Never widen a route for this. ⚠️ marked percent-encodes destinations (`my image.png` arrives as `my%20image.png`, CJK names as `%E5…`), so the ref is `decodeURIComponent`ed (a malformed escape is kept as written) and stripped of `#fragment` and `?query` BEFORE the route encodes it again; without that `file-raw` looks for a file literally named `my%20image.png`. Workspace links become `a.rv-path` with `data-path` AND `data-session-id` (the preview's session, which the `app.js` delegate prefers over `activeSessionId`, since a preview opened from another session's attachment card must resolve links against that workspace) and lose the `target`/`rel` that `_renderMarkdown` gives every link, which would otherwise open `<origin>/docs/x.md` in a new tab; fragment, protocol-relative and http(s) links are untouched.
|
||||
- ⚠️ **One pipeline, one delegate.** The viewer calls `_renderMarkdown(text, { breaks: false })` (marked + the `sanitize-html.js` allowlist) and binds `_bindResponseViewerInteractions()` on `#filePreviewBody` (container-bound and idempotent, so once per page) for the code-block copy buttons and `a.rv-path` opening. Never a second parser, never a second click handler for the same markup. `breaks` is the one option that differs: chat keeps the default `true` (a newline is the agent's line break), while a file passes `false`, because a README hard-wrapped at 80 columns would otherwise render every wrap as a `<br>`, unlike GitHub's file view.
|
||||
- ⚠️ **Build inside a `<template>`, then rebase.** A detached div whose `innerHTML` is set starts fetching every `<img src>` at once, so the document's relative image paths would hit the server as `/docs/img.png` 404s before being rewritten. `_rebaseFilePreviewMarkdownRefs()` runs on the template content: relative images go to the workspace-confined `file-raw` under the document's directory, root-relative ones (`/docs/x.png`) under the workspace root as on GitHub (the server refuses escapes, so `..` is forwarded as-is), and one `error` handler per image degrades it to alt text, which covers a remote image the page CSP blocks, a 404 for a document outside the workspace, and an SVG that `file-raw` serves as a download. Never widen a route for this. ⚠️ marked percent-encodes destinations (`my image.png` arrives as `my%20image.png`, CJK names as `%E5…`), so the ref is `decodeURIComponent`ed (a malformed escape is kept as written) and stripped of `#fragment` and `?query` BEFORE the route encodes it again; without that `file-raw` looks for a file literally named `my%20image.png`. Workspace links become `a.rv-path` with `data-path` AND `data-session-id` (the preview's session, which the `app.js` delegate prefers over `activeSessionId`, since a preview opened from another session's attachment card must resolve links against that workspace) and lose the `target`/`rel` that `_renderMarkdown` gives every link, which would otherwise open `<origin>/docs/x.md` in a new tab; fragment, protocol-relative and http(s) links are untouched. The absolute paths `_linkifyFilePaths()` then finds in the prose get the same `data-session-id`, set on every `a.rv-path` still lacking one. ⚠️ **An attachment opened under a bare file name has no directory.** Attachment cards and the history drawer call `openFilePreview(relativePath || fileName, sessionId, attachmentId)`, and a registered attachment's `relativePath` is always `''` (`attachmentRecordToEvent`), so `filePath` is just `report.md`: resolving against it sent `docs/report.md`'s `img/chart.png` to the workspace root's `img/chart.png` (a missing image) and its `CONTRIBUTING.md` link to the root's (a silently different file), and an out-of-workspace attachment's refs all landed in the workspace. `filePreviewText` therefore carries `attachmentId`, and when it is set with a non-absolute `filePath` the rebase degrades every workspace ref instead: images become their alt text (as a text node), links are unwrapped to their text. An absolute-path attachment (one `_registerExternalPreview` minted for a clicked path) still resolves against its own directory, as does every workspace preview.
|
||||
- ⚠️ **`data-i18n-skip` on the container.** The translator's MutationObserver translates inserted headings and paragraphs, and the `.file-preview-content` entry in its skip list matches nothing (no element has that class), so the attribute is what keeps a Chinese UI from rewriting a README.
|
||||
- ⚠️ **Toggles are per-device, in their own localStorage keys** (`codeman:filePreviewMdRendered` / `LineNumbers` / `Wrap`), for the same reason as the Files panel's show-hidden toggle: the app-settings object is rebuilt from the settings modal on every save, and they are display state, not synced settings (`SettingsUpdateSchema` is `.strict()`). MD re-renders from the kept source (`filePreviewContent`, which is also what Copy copies) without a refetch; Lines/Wrap are class flips on the one `<pre>`, whose rules are scoped `.file-preview-body > pre.file-preview-text` so they never leak into the document's own code blocks. Lines are one inline `<span class="fp-line">` per line joined by real newlines, the counter in `::before` with `user-select: none`, so select and copy return the exact text.
|
||||
- ⚠️ **Caps.** Markdown fetches `lines=10000` (the route's `MAX_LINES_LIMIT`), because a rendered document cut at 500 lines reads as the whole document; other text keeps 500, which is what stops a huge log locking the tab in one `<pre>`. The attachment (out-of-workspace) branch keeps its 512 KB Range read and skips the 500-line clip for markdown. Edit mode is unchanged and still re-fetches `edit=1`; the three toggles hide while editing and for images, media and PDFs.
|
||||
- ⚠️ **`md` is NOT in `FILE_PREVIEW_EXTENSIONS`.** A `.md` path printed in the terminal or chat still opens the tail viewer (see File-path links above: in-workspace text keeps live follow, which is what Ralph's `fix_plan.md` needs); the rendered view is reached from the Files panel. `avif` and `ico` were added there (a printed `favicon.ico` used to tail binary noise), with `avif` also in file-content's image set and file-raw's MIME map; out-of-workspace avif/ico stay unregistrable, like svg/bmp.
|
||||
- ⚠️ **`md` is NOT in `FILE_PREVIEW_EXTENSIONS`.** Only an in-workspace `.md` path clicked in the terminal still opens the tail viewer (see File-path links above: in-workspace text keeps live follow, which is what Ralph's `fix_plan.md` needs). Everything else reaches `openFilePreview()` and renders it: the Files panel, a path clicked in the Response Viewer (its delegate calls `openFilePreview()` directly), an out-of-workspace terminal path, and attachment cards. `avif` and `ico` were added to the set (a printed `favicon.ico` used to tail binary noise), with `avif` also in file-content's image set and file-raw's MIME map; out-of-workspace avif/ico stay unregistrable, like svg/bmp.
|
||||
|
||||
Tests: `test/file-preview-markdown.test.ts` (jsdom-in-vm, pins every rule above), `test/routes/file-routes.test.ts` (avif).
|
||||
|
||||
|
||||
Reference in New Issue
Block a user