Expose structured composer input to plugins - #1977
Conversation
|
🚨 SLOP COP 🚨 · I am SlopCop. I am reviewing this pull request for security, code quality, performance, architecture, and end-to-end behavior. |
SawyerHood
left a comment
SawyerHood
left a comment
There was a problem hiding this comment.
Findings
- High — The new public type bypasses the experimental API policy.
packages/plugin-sdk/src/app-contract.ts:8 exports PromptInput with a stable name. app.ts:3 and index.ts:10 expose it from both public paths. The audit entry at docs/api_to_audit.md:362 covers only PluginComposerApi.experimental_getInput. The project policy requires an experimental_ name and an audit entry for each new app export. Rename this export and audit it, or avoid the new public export.
- Medium — The portable app harness loses structured mentions after composer edits.
packages/plugin-sdk/src/testing/app.tsx:1007 rebuilds changed text with mentions: []. At lines 1083-1084, insertMention changes plain text and records only the call. Production adds the structured range at apps/app/src/lib/plugin-sdk-hooks.ts:728. Thus, experimental_getInput() differs between tests and production after mention insertion or an edit around an existing mention. Preserve and reconcile mention ranges in the harness. Add a regression test for both paths.
- Medium — The forwarding guide omits the project boundary and the authoring skill omits the API.
packages/plugin-sdk/README.md:19 says a plugin can pass the snapshot unchanged to another thread. Relative attachment paths belong to one project. The server validates them against the destination project at apps/server/src/services/projects/attachments.ts:125. A spawn in another project can fail with an attachment error. State the same-project limit or describe the attachment-copy step. Also update the hook reference at apps/server/src/services/skills/builtin-skills/bb-plugin-authoring/SKILL.md:1882.
Review summary
Security: I found no new security defect. Plugin frontend code already has full page trust.
Performance: The getter clones data only when a caller invokes it. useComposer already updates with draft text, so this change adds no material render cost.
Architecture: Production uses promptDraftToInput, which is the canonical converter. The small clone helper duplication across the app and portable harness is acceptable.
Validation: HEAD matches 8d538317e32f8a270582911ba944c4db79eff000. git diff --check passed. The app suite passed 41 tests. The selected SDK suite passed 35 tests. The SDK Turbo task stopped twice in its generated-type prerequisite, so I ran the selected Vitest file directly. The worktree remains clean.
- Keep structured mentions aligned in the plugin app harness
![bb-slop-cop[bot]](https://avatars.githubusercontent.com/in/4577281?s=60&v=4){kind=small}
There was a problem hiding this comment.
🚨 SLOP COP 🚨 · review
Plain English summary: This PR lets plugin buttons read the current composer as structured data. It keeps text, mentions, screenshots, and files.
I found two current contract issues.
- High — The new public
PromptInputexport has a stable name. Project policy requires anexperimental_prefix and an audit entry. - Medium — The README says plugins can send the snapshot unchanged to another thread. Relative attachment paths only work in the same project. The plugin author skill also omits this API.
The author pushed 5e3dc82 during this review. That commit fixes the earlier test-harness mention defect.
Security: I found no new defect. Installed plugins already have full computer access.
Performance: I found no material new cost. The host creates the detached snapshot only when a plugin calls the method.
Architecture: Production uses the shared draft converter. The small clone helper is acceptable. The updated harness now keeps mention behavior consistent with production.
Validation:
- The targeted app suite passed 41 tests.
- The selected SDK suite passed 35 tests.
- The browser composer accepted text and a file.
git diff --checkpassed.- Latest CI has unrelated failures in unchanged server and automation tests.
This review uses comment status only.
|
Addressed all three review findings on the current head.
Validation: focused SDK declaration/harness tests 38/38, full SDK tests 106/106, SDK typecheck passed, and the plugin-authoring skill test passed 14/14. No second review was run. |
2 participants
What was wrong
Composer customizations could read only the draft text and attachment count. The host retained the actual screenshot/file attachment paths and structured mention ranges inside its private draft state, so a plugin could not forward the complete composer input to another BB thread without dropping non-text context.
What changed
Adds the additive experimental PluginComposerApi.experimental_getInput method. It snapshots the same PromptInput array the active composer would submit at call time, including trimmed text, structured mentions, and local image/file attachments. The result is detached from live draft state and does not mutate, clear, focus, or submit the composer.
The official frontend test harness accepts structured composer input and mirrors the host's mention preservation, rebasing, removal, and insertion behavior. SDK documentation and the built-in plugin-authoring skill describe the same-project attachment boundary and the existing attachment-copy API for cross-project forwarding.
This is standalone from the Browser PR stack. It keeps the existing unpublished SDK 0.4.9 version on main; there is no compatibility version or protocol change.
How you verified
The new host and harness regressions failed before implementation because experimental_getInput did not exist, then passed after implementation.
Fixes #1978
BB-Thread-ID: thr_bwikffmsvp