Skip to content

feat: Plugin API Completeness#23

Open
marcuspocus wants to merge 5 commits into
jmfederico:mainfrom
marcuspocus:feat/plugin-api-completeness
Open

feat: Plugin API Completeness#23
marcuspocus wants to merge 5 commits into
jmfederico:mainfrom
marcuspocus:feat/plugin-api-completeness

Conversation

@marcuspocus

@marcuspocus marcuspocus commented Jun 14, 2026

Copy link
Copy Markdown

Plugin API Completeness

Adds file mutation, prompt editor, and attachment APIs to the pi-web plugin system, completing the stable plugin surface for workspace interaction.

What's new

Workspace Files — read, write, delete, move

interface WorkspaceFiles {
  readFile(path: string): Promise<FileContentResponse>;          // existing
  writeFile(path, content, options?): Promise<WriteWorkspaceFileResponse>;  // NEW
  deleteFile(path: string): Promise<DeleteWorkspaceFileResponse>;            // NEW
  moveFile(fromPath, toPath, options?): Promise<MoveWorkspaceFileResponse>; // NEW
}
  • writeFile — write text or binary content, auto-creates directories, optional overwrite: false
  • deleteFile — idempotent delete, removes symlinks (not their targets) via lstat
  • moveFile — unix mv semantics, defaults overwrite: false (safer than writeFile)
  • All mutations auto-refresh the File Explorer panel
  • Path safety: resolveInsideWorkspace + resolveParentInsideWorkspace + realpath prevent traversal and symlink escape

Prompt Editor — stable CM6 API

interface PluginPromptEditor {
  insertText(text: string): void;                                    // replaces selection
  getText(): string;                                                 // returns "" if no editor
  getSelection(): { start: number; end: number; text: string } | null;
  onPaste(handler): () => void;                                     // CM6 domEventHandlers
  onKeyDown(handler): () => void;                                    // CM6 domEventHandlers
  focus(): void;
}
  • Uses CM6 EditorView.domEventHandlers() via Compartment — no raw DOM access
  • Handlers registered before editor mount are preserved and applied on mount
  • First-to-consume-wins ordering for multi-plugin scenarios
  • Unsubscribe functions prevent memory leaks
  • insertText places the cursor after inserted text

Attachments — file references in the prompt

interface PluginAttachments {
  insertFileReference(path: string): Promise<string>;   // validates file exists, inserts @path
  getAttachedFiles(): string[];                          // extracts @path references
  removeFileReference(path: string): void;               // removes first @path occurrence
}
  • insertFileReference validates file existence before inserting
  • insertFileReference places the cursor after the inserted @path
  • removeFileReference places the cursor at the removal point
  • getAttachedFiles uses a heuristic regex (requires file extension to avoid matching emails)
  • Depends on files (for validation) and prompt (for insertion)

API endpoints

Method Path Description
GET /projects/:pid/workspaces/:wid/file Read file (existing)
PUT /projects/:pid/workspaces/:wid/file Write file (new)
DELETE /projects/:pid/workspaces/:wid/file Delete file (new)
POST /projects/:pid/workspaces/:wid/file/move Move/rename file (new)

All endpoints work for local and federated machines. The federated route contract now explicitly covers the new PUT /file, DELETE /file, and POST /file/move client calls.

Error handling

All file mutations use the same patterns:

  • Path traversal blocked by resolveInsideWorkspace / resolveParentInsideWorkspace + ensureInside
  • Symlink escape prevented by realpath(dirname(target)) check after mkdir
  • deleteFile uses lstat (deletes symlink, not target)
  • Error messages are developer-facing: "Path is a directory", "File already exists: path", "path query parameter is required"
  • deleteFile is idempotent: returns { existed: false } for non-existent files

Null safety

API When context unavailable Pattern
files.writeFile etc. Always has workspace context No guard needed
prompt.insertText Editor not mounted Silent no-op
prompt.getText Editor not mounted Returns ""
prompt.onPaste/onKeyDown Editor not mounted console.warn + no-op unsubscribe
attachments.insertFileReference No workspace Throws Error

Test coverage

  • fileContentService.test.ts: 25 unit tests covering write, delete, move, path safety, and symlink behavior
  • app.test.ts: 3 integration tests covering the HTTP contract for write, delete, and move
  • clients.test.ts: 5 client API tests covering request routing, headers, query params, and response parsing
  • federatedRouteContract.test.ts: covers remote-machine proxy route support for write, delete, and move
  • registry.test.ts: Mock updates for all new plugin context methods

Documentation

  • docs/plugins.md — 3 new sections:
    • "Writing, deleting, and moving workspace files"
    • "Prompt editor API"
    • "Attachment API"

Downstream plugin validation

Two real plugins have already been updated and published against this API surface:

  • @yieldcraft/screenshot-paste@0.3.1
    • Replaces shell-based screenshot saves with files.writeFile
    • Uses attachments.insertFileReference, attachments.removeFileReference, and attachments.getAttachedFiles
  • @yieldcraft/doc-viewer@0.2.2
    • Replaces terminal-command file saves with files.writeFile
    • Uses files.deleteFile for inline deletion of temporary docs files

Both READMEs document that these releases depend on this PR until the pi-web version containing the APIs is merged and released.

@jmfederico jmfederico left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Automated review (AI-assisted)

Disclosure: this review was produced by an AI agent. It checked out the PR in an isolated worktree, installed dependencies, and ran the full verify gate. Results: typecheck, lint, knip, and tests all pass (551 tests, 90 files). A human has read and approved posting this. The notes below are written so that an AI coding agent can act on them directly.

Overall: the core feature (local file write/delete/move + prompt/attachment APIs) is high quality, security-conscious, and well-tested. Two categories need attention before merge.

�blocking� Hard blockers

  1. Federation claim is false for remote machines. The PR description and docs/plugins.md state these APIs "work for local and federated machines," but src/shared/federatedRoutes.ts is not modified. FederatedHttpMethod is only "GET" | "POST" | "DELETE" (no PUT), and the new paths (PUT /file, DELETE /file, POST /file/move) are absent from FEDERATED_HTTP_ROUTES. Against a remote machine these fail at runtime (tests can't catch it). Action: either (a) add PUT to FederatedHttpMethod and register all three routes in FEDERATED_HTTP_ROUTES, or (b) correct the docs and PR text to "local machines only."

🧹 Scope creep (split into separate PRs)

  1. scripts/postinstall.mjs + the postinstall hook in package.json — node-pty macOS spawn-helper chmod fix. Unrelated to plugin APIs; adds a postinstall side effect for every consumer. Action: remove from this PR.
  2. src/server/piWebPluginService.test.ts — adds configProvider: () => ({ plugins: {} }) to 7 constructor calls, but configProvider is already optional on main. Pure churn. Action: revert these edits.
  3. docs/plugins.md 127.0.0.1localhost cosmetic change. Action: drop from this PR.
  4. .gitignore additions .pi-paste/ and docs/tmp/ are not referenced anywhere in src. Action: remove unless a code path actually uses them.

💡 Wished improvements (non-blocking)

  1. Move the global app.addContentTypeParser(...) calls out of registerWorkspaceExplorerRoutes (called twice) and into app setup; reconsider the broad catch-all regex parser (/^([a-z]+\/[a-z0-9.+-]+)$/ → buffer) for interaction with future routes.
  2. writeWorkspaceFile/moveWorkspaceFile have a TOCTOU window between the stat existence check and the write/rename, so overwrite:false is racy. Prefer an atomic guard (e.g., wx open flag / link+unlink) if strict semantics matter.
  3. getAttachedFiles() is a regex heuristic and can match non-attachment @word.ext text. It's documented as best-effort; consider a more robust source of truth if accuracy becomes important.

✅ Strengths worth keeping

Path-safety (resolveParentInsideWorkspace + realpath(dirname) + ensureInside) with explicit traversal/absolute/symlink-escape tests; deleteFile correctly uses lstat/unlink to remove the symlink not its target; safe defaults (moveFile overwrite:false, idempotent delete); CM6 domEventHandlers via Compartment with proper cleanup; clean http.ts content-type override.

@marcuspocus marcuspocus requested a review from jmfederico June 14, 2026 12:38
…tachment APIs

- WorkspaceFiles: writeFile, deleteFile, moveFile with path safety
  - writeFile: text/binary, auto-create dirs, overwrite option
  - deleteFile: idempotent, uses lstat (removes symlinks not targets)
  - moveFile: unix mv semantics, overwrite defaults to false
  - All mutations auto-refreshFiles() in File Explorer
  - Symlink escape prevention via realpath(dirname) check

- PluginPromptEditor: insertText, getText, getSelection, onPaste, onKeyDown, focus
  - Uses CM6 EditorView.domEventHandlers() via Compartment (not raw DOM)
  - Handlers registered before mount are preserved and applied on mount
  - First-to-consume-wins ordering for multi-plugin scenarios
  - insertText replaces selection (not inserts after)

- PluginAttachments: insertFileReference, getAttachedFiles, removeFileReference
  - insertFileReference validates file exists before inserting @path
  - Does not auto-focus editor (unlike prompt.insertText)
  - @file regex requires file extension to avoid matching emails

- Server endpoints: PUT /file, DELETE /file, POST /file/move
  - All work for local and federated machines

- Tests: 31 unit tests, 9 integration tests, 5 client tests
- Docs: 3 new sections in plugins.md
…mpt/attachments API

Ensure prompt.insertText, attachments.insertFileReference, and
attachments.removeFileReference move the cursor to the correct
position after modifying the prompt editor. Previously the cursor
stayed at the start of inserted text, which broke the natural
flow for plugin-driven file attachments like screenshot-paste.
@marcuspocus marcuspocus force-pushed the feat/plugin-api-completeness branch from 487ad9f to dde8675 Compare June 14, 2026 13:04
@jmfederico

Copy link
Copy Markdown
Owner

Thanks again for this @marcus — really solid groundwork. I've opened #25 as a slimmed-down version that builds directly on your commits (they're preserved in that branch's history).

It carries forward the two pieces that expose pi-web capabilities plugins genuinely can't reach today: the workspace file mutations (writeFile/deleteFile/moveFile, federated + path-safe) and read-only prompt editor access (insertText/getText/getSelection). Those are the high-value core of this PR and they go in essentially as you designed them.

I held back a few of the other surfaces — not because anything was wrong with them, but based on where we want the public plugin API to sit long-term:

  • The attachments helper. pi-web already has a real, structured attachment model in main (PromptAttachment — binary payloads with inline/folder delivery). A second @path-string notion of "attachment" layered on top of the prompt text would give us two different things called "attachments," which we'd be locked into under the v1 compatibility promise. insertFileReference / removeFileReference are also composable from the prompt + files helpers we're keeping, so they don't need dedicated API. If we want to give plugins attachment access, we'd rather expose the existing model deliberately later.

  • prompt.onPaste / onKeyDown. These are the start of an editor event/hook system, and we'd prefer to design that surface as a whole — which events, precedence vs. the editor's built-in handling (image paste especially), capture/bubble, consume semantics — rather than commit to two specific events now and have to grow or deprecate around them. The CM6 plumbing here is a great reference for when we build that.

  • prompt.focus. We already ship focusPrompt() on the context, so we're keeping focus there to avoid two names for one action. If we later move everything under prompt.* for ergonomics, that'll be its own intentional naming pass.

So the net is: same core capability, narrower public surface, with the event-hook and attachment surfaces deferred to dedicated designs. Your authorship is retained on all the carried-over commits.

One thing that would really help us here: could you evaluate your actual needs (e.g. what screenshot-paste and doc-viewer are doing) against the existing attachment functionality already in main — the PromptAttachment model and its inline/.pi-web/paste folder delivery? We'd like to understand how much your use cases overlap with what's already there, so we can figure out how best to expose that existing pipeline to plugins rather than build a parallel one. If you can map "here's what I need" to "here's the gap vs. what main already does," that'd give us a concrete basis to design the plugin-facing attachment surface. Happy to discuss any of these calls if you see it differently.

@marcuspocus

Copy link
Copy Markdown
Author

If you can map "here's what I need" to "here's the gap vs. what main already does," that'd give us a concrete basis to design the plugin-facing attachment surface.

Thanks for the thoughtful split — that direction makes sense to me.

I agree with the narrower public surface for #25. The files.* APIs are the part my plugins cannot reproduce cleanly today, and prompt.insertText / getText / getSelection cover a lot of useful prompt integration without committing to a full editor hook system yet.

Here is how my real plugin needs map against the attachment functionality already in main.

doc-viewer

doc-viewer does not need attachment access.

It only needs:

  • files.writeFile() for Markdown edit/save;
  • files.deleteFile() for inline deletion of docs/tmp/* files.

So #25 should fully cover doc-viewer.

screenshot-paste

screenshot-paste is the plugin that overlaps with the existing PromptAttachment model.

The workflow I want is:

  1. paste an image while composing;
  2. save it into the workspace immediately, currently under .pi-paste/;
  3. insert a workspace-relative reference at the cursor, e.g. @.pi-paste/foo.png;
  4. show a pre-send thumbnail strip so the user sees exactly what will be sent;
  5. allow removing one image before send;
  6. keep pasted images visible in a workspace gallery and cleanable later.

The important detail is that the image is deliberately workspace-backed before send, not only an inline multimodal payload.

What already overlaps with main

The existing PromptAttachment model already covers much of the UX:

  • image paste / drag / file input;
  • pending attachment chips before send;
  • remove-before-send UI;
  • inline image delivery;
  • folder delivery that saves to .pi-web/paste and appends references on send.

So I agree that my original attachments helper was probably the wrong long-term boundary. A second public thing called “attachments” based on @path strings would risk competing with the real structured attachment model.

Remaining gaps for screenshot-paste

The gaps are mostly about plugin access to the existing pipeline:

  • plugins cannot add an image to the existing pending PromptAttachment queue;
  • plugins cannot list/remove pending attachments they added;
  • folder delivery happens only on send, while my workflow benefits from having the workspace path before send;
  • folder/name policy is fixed (.pi-web/paste + generated names), while a plugin may want its own folder such as .pi-paste/;
  • folder delivery appends references on send, while screenshot-paste wants to insert at the current cursor position while composing;
  • with only prompt.insertText() and prompt.getText(), I can insert @path, but I cannot cleanly remove a specific inserted reference without either a text mutation API, structured attachment access, or falling back to editor-private DOM/CM access.

Possible future shape

I do not think #25 needs this now, but when plugin attachment access is designed, I would prefer exposing the existing model rather than reviving the string-based helper.

The useful primitives would be something like:

interface PluginPromptAttachments {
  add(attachment: PromptAttachment, options?: {
    delivery?: "inline" | "folder";
    folder?: string;
    insertReference?: "cursor" | "append-on-send" | false;
  }): Promise<{ id: string; path?: string }>;

  list(): PromptAttachmentInfo[];
  remove(id: string): void;
  onChange?(handler: () => void): () => void;
}

Exact names aside, the key is: reuse the existing PromptAttachment chips/delivery pipeline, while giving plugins a supported way to add, observe, and remove their own pending attachments.

Practical consequence

So I’m aligned with holding back the old attachments helper from v1. The missing long-term piece is not a parallel @path attachment helper; it is plugin access to the existing PromptAttachment pipeline, plus a deliberately designed editor hook/mutation surface later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants