feat(reader): add opt-in durable document export - #2
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
Add an explicit, durable export/save capability to the public OpenCode rich-document reader. Extend the existing read_rich_document tool with an explicit opt-in export request. When export is omitted, preserve the current ephemeral behavior exactly, including the existing temporary media/index behavior. When export is requested without a destination, write the structure-preserving Markdown and media directory beside the source document by default; the accepted design is an optional export destination with sibling output as the default. Support an optional project-relative destination for callers who want another durable location, while rejecting absolute destinations and any path that escapes the current project, including escaping symlinks. The durable export must contain the structure-preserving Markdown, all extracted media, and a machine-readable manifest or equivalent returned paths so later turns can refer to saved artifacts without relying on the temporary directory. Preserve the source document unchanged. Choose a clear, backward-compatible argument shape for the explicit export request and document it with examples. Define and test collision/overwrite behavior; this implementation rejects an existing export destination and never overwrites it. Add behavioral tests for default sibling export, custom project-relative export, path-boundary rejection, Markdown/media/manifest contents, collision behavior, and the unchanged ephemeral default. Update the README safety and usage sections with the exact durable-export behavior and lifecycle. Do not broaden this feature to arbitrary absolute paths, automatic persistence, or changes to the existing non-export contract.
What Changed
exportrequest toread_rich_document, supporting sibling exports by default or custom project-relative destinations with structure-preserving Markdown, extracted media, and a JSON manifest returned through export metadata.exportis omitted, including temporary media paths and unchanged source documents.Risk Assessment
🚨 High: The required project-boundary guarantee is vulnerable to a symlink change between validation and export creation, allowing durable output outside the project; export mode also needlessly duplicates large Markdown allocation.
Testing
The focused export tests passed after restoring dependencies, and the executable end-to-end tool transcript verified the requested durable files, all media, manifest paths, ephemeral default behavior, path-boundary rejection, collision safety, and unchanged source; no full-suite, lint, or static-analysis commands were run.
Evidence: End-to-end rich-document export transcript
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
src/path-safety.ts:120- The required intent is to reject "any path that escapes the current project, including escaping symlinks." The destination is realpath-checked here, butwriteDurableExportthen calls recursivemkdiratsrc/export.ts:55and writes through the unchecked path. If a checked parent is replaced with an outside symlink between validation and creation, the export follows it outside the project. Enforce the boundary at directory creation with no-follow/atomic component handling rather than relying on this preflight check.src/reader.ts:427- Every export request first builds the complete temporary-path Markdown here, then rebuilds the same converted body with exported paths at lines 431-434 and discards the first string whenoutputis replaced at line 450. This adds unnecessary work and peak memory, with avoidable OOM pressure for large documents; construct only the selected output variant.✅ **Test** - passed
✅ No issues found.
npm cito restore the lockfile dependencies after the initial targeted test reported missing@opencode-ai/pluginnode --experimental-strip-types --test --test-name-pattern='(keeps omitted export reads ephemeral|writes a default sibling export|writes a custom project-relative export|rejects absolute and project-escaping export destinations|rejects export destinations whose symlinked parent escapes the project|rejects an existing export destination without overwriting it|leaves the source document byte-for-byte unchanged)' test/read-rich-document.test.tsManual public-tool verification throughRichDocumentReaderPlugin -> read_rich_document.executecovering ephemeral reads, default sibling export, custom project-relative export, media and manifest persistence, collision preservation, lexical/absolute/symlink escapes, and source immutabilityRemoved the transientnode_modulesdirectory created for local testing; only the pre-existing ignored.roam/directory remains in the worktree✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.