feat(documents): add interactive widgets to documents - #725
Conversation
A widget is an AI-authored, self-contained HTML fragment that runs in an opaque-origin iframe (allow-scripts, no allow-same-origin) and lives as a block inside a document. A document holding nothing but a widget is the standalone case, so there is no separate item type to read, edit, create and render. Reads elide a widget's source to an empty placeholder carrying its ref and title, because one widget runs to kilobytes and would crowd the prose out of a chunk. workspace_read_items gains mode "block" to fetch that source back in full. Elision alone would be data loss, since overwrite echoes a read back by design, so the parser takes a resolveWidgetSource hook and an empty widget with a known ref keeps what is already there. replace_text joins the document edit vocabulary with a ref, so it targets text inside any block: a widget's source or a paragraph's markup. It fails rather than replacing every occurrence when a find matches more than once. replace_all is renamed overwrite so it cannot be read as "replace all instances of a find". Also here, because the surfaces had drifted apart: - Documents degrade unsupported markup instead of rejecting the write. A probe of realistic model output found 12 of 20 snippets refused outright, each costing a whole document to save formatting we can flatten. ProseMirror's DOMParser already normalises HTML, which removed the hand-rolled allowlist. - Math, chemistry and units are one contract across documents and widgets, both being HTML. Chat keeps the $-delimited Markdown form. sub/sup become math rather than failing, and money is written plainly. - KaTeX is served from the app's own origin and injected only when a widget contains math. Widget source stays out of search and previews: the markdown projection drops it, and a preview shows the widget's title instead.
Return canonical block content with a fresh editRef and use the same representation for replace_text across prose and widgets. Remove widget-specific overwrite restoration and align tool schemas, prompts, evals, and focused coverage with the contract.
Keep the Tiptap extension in a non-component module so the React node view remains compatible with state-preserving Fast Refresh.
Make the DOM environment used by widget and existing component tests an explicit dependency.
Default to prose and tables, reserve Mermaid for requested visual explanations, and create widgets only for explicit, interactive, or document-specific visual requests.
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Capy auto-review is paused for this organization because the usage-cycle auto-review limit has been reached. Increase the limit or turn it off in billing settings to resume automatic reviews. |
|
React Doctor found 2 new issues in 1 file · 2 warnings · score 87 / 100 (Great) · 1 fixed · vs 2 warnings
Reviewed by React Doctor for commit |
|
Warning Review limit reached
Next review available in: 15 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (22)
📝 WalkthroughWalkthroughThe PR adds sandboxed HTML widgets to workspace documents, block-level reads and edits using ChangesWorkspace document blocks and edits
Widget rendering and authoring
AI instructions and evaluations
Workspace wiring and generated assets
Estimated code review effort: 5 (Critical) | ~90 minutes Sequence Diagram(s)sequenceDiagram
participant AIThread
participant WorkspaceContentReader
participant DocumentSession
participant DocumentAiEdits
AIThread->>WorkspaceContentReader: request block read with editRef
WorkspaceContentReader->>DocumentSession: readBlock(editRef)
DocumentSession-->>WorkspaceContentReader: block HTML and current editRef
WorkspaceContentReader-->>AIThread: block content
AIThread->>DocumentAiEdits: submit targeted edit
DocumentAiEdits->>DocumentAiEdits: validate prior-read editRef
DocumentAiEdits-->>AIThread: edit result
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc7136b6a2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "media-src data: blob:", | ||
| "connect-src 'none'", | ||
| "object-src 'none'", | ||
| "base-uri 'none'", |
There was a problem hiding this comment.
When widget HTML contains a same-origin <script src>, stylesheet, @import, or font URL, these source expressions permit it because ${origin} authorizes the entire application origin; sandbox="allow-scripts" isolates the frame's origin but does not disable its subresource requests. This breaks the stated no-network contract for untrusted widgets and lets them request or execute arbitrary same-origin resources rather than only the vendored KaTeX files. Scope each directive to ${origin}/widget-libs/katex/ (or serve the required assets through another narrowly allowlisted origin/path).
Useful? React with 👍 / 👎.
Greptile SummaryThis change adds AI-authored interactive widgets to collaborative documents and updates document reads and edits around Browser execution confirmed that widget code can navigate its sandbox to an external URL despite the generated connection policy. The sandbox must reject untrusted document navigation before this change is merged. Confidence Score: 4/5Not safe to merge until the widget iframe prevents untrusted external navigation. The reproduced external navigation path in the widget sandbox remains an open security issue. Files Needing Attention: src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx and the generated sandbox document CSP.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| <iframe | ||
| ref={iframeRef} | ||
| title="Widget preview" | ||
| sandbox="allow-scripts" |
There was a problem hiding this comment.
An authored widget can assign window.location to an external URL. With sandbox="allow-scripts", the iframe can navigate itself; the generated connect-src 'none' policy restricts connection APIs but not document navigation. This makes a real external request and replaces the widget document, allowing widget-controlled data to be included in a navigation URL despite the intended no-network isolation. Prevent or reject untrusted frame navigation at the embedding boundary and add a browser regression test for window.location assignment.
Artifacts
- Chromium rendered the generated widget iframe without a navigation payload; the widget remains in its initial document, establishing the comparison baseline.
Baseline sandboxed widget before location assignment
- Poster frame from the baseline recording shows the authored widget before any location assignment, establishing the unchanged state.
- Chromium rendered the same generated sandbox with an authored window.location assignment and then requested the external destination, proving navigation bypasses the no-network intent.
Widget immediately before its authored external navigation
- Poster frame from the navigation run shows the exact authored widget loaded with the delayed location payload, documenting the changed execution condition.
Successful Chromium navigation proof output
- Output from the executed Playwright proof records exit code 0, sandbox allow-scripts, connect-src none, and the external GET request; the no-network claim is violated.
Playwright proof source for widget navigation
- The executed source builds the repository's actual widget sandbox document, renders it in Chromium, and records baseline and location-assignment behavior; it reproduces the finding.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| sessionId, | ||
| ...theme, | ||
| }; | ||
| iframe?.contentWindow?.postMessage(message, "*"); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
src/features/workspaces/documents/document-ai-edits.ts (1)
241-277: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBlock-ID inheritance is duplicated across the
replace_textandreplacebranches.Lines 256-264 and lines 270-277 build the same "first node inherits the target block ID" list. Extract one helper to keep both paths identical if the rule changes.
♻️ Proposed refactor
+function withInheritedBlockId(target: ProseMirrorNode, children: ProseMirrorNode[]) { + const targetBlockId = readTiptapNodeBlockId(target); + const firstNode = children[0]; + return targetBlockId && firstNode + ? [withTiptapNodeAiRef(firstNode, targetBlockId), ...children.slice(1)] + : children; +}Then call it from both branches:
- const targetBlockId = readTiptapNodeBlockId(target); - const firstNode = parsed.children[0]; - children.splice( - targetIndex, - 1, - ...(targetBlockId && firstNode - ? [withTiptapNodeAiRef(firstNode, targetBlockId), ...parsed.children.slice(1)] - : parsed.children), - ); + children.splice(targetIndex, 1, ...withInheritedBlockId(target, parsed.children));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/documents/document-ai-edits.ts` around lines 241 - 277, Extract the duplicated block-ID inheritance logic into a shared helper near the edit-processing code, accepting the parsed children and target block ID and returning the adjusted node list. Use this helper in both the replace_text branch and the replace branch, preserving inheritance only when a target block ID and first node exist.src/features/workspaces/documents/document-ai-html.test.ts (1)
42-51: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a pattern-valid
editRefin this fixture.
b_modelchosen1does not matchdocumentBlockIdPattern, so the test passes even if the strip loop andparseHTML: () => nullare both removed;readTiptapNodeBlockIdwould reject the value anyway. Use a well-formed value so the test proves that a model cannot choose a usable ref.💚 Proposed fix
- parseDocumentAiHtml('<p data-edit-ref="b_modelchosen1">Hello</p>'), + parseDocumentAiHtml( + '<p data-edit-ref="b_modelchosen1x.r_0123456789">Hello</p>', + ), ).document, ); - expect(html).not.toContain("b_modelchosen1"); + expect(html).not.toContain("b_modelchosen1x");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/documents/document-ai-html.test.ts` around lines 42 - 51, Update the fixture in “ignores editRefs supplied in model-authored HTML” to use an editRef matching documentBlockIdPattern, while preserving the assertion that the original value is absent and a generated valid ref is present. Ensure the test specifically exercises rejection of a structurally valid model-supplied ref.src/features/workspaces/documents/document-ai-html.ts (1)
225-239: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueNested inline markup inside
<sub>/<sup>becomes literal LaTeX text.
element.textContentflattens child elements. Forx<sup><em>n</em></sup>the value staysn, which is correct, but for content that includes LaTeX-significant characters, such as<sub>a_b</sub>or<sub>{x}</sub>, the generateddata-latexbecomes{}_{a_b}or{}_{{x}}. KaTeX renders those differently from the intended text.throwOnError: falseprevents a hard failure, so the impact is limited to a wrong render.Consider escaping LaTeX control characters (
\,{,},_,^,$,%,&,#,~) before building the attribute.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/documents/document-ai-html.ts` around lines 225 - 239, Update the sub/sup conversion loop around element.textContent and data-latex so latex text is escaped before interpolation. Escape backslashes and LaTeX-significant characters (\ , {, }, _, ^, $, %, &, #, ~) while preserving the existing subscript/superscript wrapper and removal behavior for empty content.src/features/workspaces/components/widget/workspace-widget-sandbox-document.test.ts (2)
45-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the CSP directives that carry the security guarantee.
The test name says the document blocks remote assets, but the assertions cover only
img-srcandmedia-src. The directives that actually contain an untrusted widget are untested:default-src 'none',connect-src 'none',object-src 'none',base-uri 'none', andform-action 'none'.
connect-src 'none'is the control that stops a malicious widget from exfiltrating anything it can read. Pin it, so a later edit togetWidgetSandboxCspcannot quietly relax it.The assertion on line 47 checks for a string the builder never emits in any branch, so it cannot fail. Replace it with the positive directive checks.
♻️ Proposed assertions
expect(document).toContain("img-src data: blob:"); expect(document).toContain("media-src data: blob:"); - expect(document).not.toContain("img-src data: https:"); + expect(document).toContain("default-src 'none'"); + expect(document).toContain("connect-src 'none'"); + expect(document).toContain("object-src 'none'"); + expect(document).toContain("base-uri 'none'"); + expect(document).toContain("form-action 'none'");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/components/widget/workspace-widget-sandbox-document.test.ts` around lines 45 - 47, Update the CSP assertions in the workspace sandbox document test to positively verify default-src 'none', connect-src 'none', object-src 'none', base-uri 'none', and form-action 'none', alongside the existing relevant directives. Remove the ineffective negative img-src https assertion and ensure the test pins the directives emitted by getWidgetSandboxCsp.
99-105: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a mismatched
source.The
sourcecheck is the first guard inisWidgetSandboxFrameMessage, and every existing case passesWIDGET_SANDBOX_FRAME_SOURCE. A message from an unrelated frame or from the host channel is the realistic rejection path, and it has no test.♻️ Proposed case
expect( isWidgetSandboxFrameMessage({ source: WIDGET_SANDBOX_FRAME_SOURCE, kind: "ready", sessionId: 0, }), ).toBe(false); + expect(isWidgetSandboxFrameMessage({ kind: "ready", sessionId: 1, source: "other" })).toBe( + false, + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/components/widget/workspace-widget-sandbox-document.test.ts` around lines 99 - 105, Add a test case for isWidgetSandboxFrameMessage where source is a different, unrelated value while the other message fields remain valid, and assert that it returns false. Keep the existing valid-source rejection cases unchanged.src/features/workspaces/components/widget/workspace-widget-sandbox-document.ts (1)
132-135: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRestrict emitted token names to
WIDGET_SANDBOX_TOKENS.
tokensis typedRecord<string, string>, so any key and value reach the frame<style>block verbatim through${name}: ${value};. The only current caller,readWidgetSandboxTheme, iterates theWIDGET_SANDBOX_TOKENSconstant and reads first-party computed styles, so nothing attacker-influenced reaches this concatenation today. The exported signature does not enforce that, and the test on line 13 already passes an ad-hoc object.Filter against the allowlist here so the guarantee lives with the code that builds the document.
♻️ Proposed allowlist filter
- const tokenDeclarations = Object.entries(tokens) - .filter(([, value]) => value.trim().length > 0) - .map(([name, value]) => `${name}: ${value};`) - .join(""); + let tokenDeclarations = ""; + for (const name of WIDGET_SANDBOX_TOKENS) { + const value = tokens[name]?.trim(); + if (value) { + tokenDeclarations += `${name}: ${value};`; + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/components/widget/workspace-widget-sandbox-document.ts` around lines 132 - 135, Update the tokenDeclarations construction to emit only entries whose names are included in the WIDGET_SANDBOX_TOKENS allowlist, while preserving the existing non-empty value filtering and declaration formatting. Apply this restriction in the document-building code rather than relying on readWidgetSandboxTheme callers.src/features/workspaces/content/workspace-read-references.ts (1)
32-32: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the new
type: "block"result.Both conditions now key off
result.type !== "file". This is the correct narrowing: atype: "block"ready result has nolocationfield, so the previoustype !== "document"check would have reachedresult.location.returnedon line 41 and thrown. The guard is also correctly ordered, becauseassetKindexists only on the file variant.The test file has no case that feeds a
type: "block"result intocreateWorkspaceReadReferencesorcreateWorkspaceReadItemsModelOutput. Add one so a future reordering of these conditions fails loudly instead of dereferencing a missinglocation.Also applies to: 135-135
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/content/workspace-read-references.ts` at line 32, Add a test covering a ready result with type "block" through createWorkspaceReadReferences or createWorkspaceReadItemsModelOutput, asserting it completes without dereferencing location and produces the expected output. Keep the existing file/PDF behavior unchanged, and ensure the test would fail if the type guard were reordered or reverted.src/features/workspaces/content/workspace-content-contract.ts (1)
56-61: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd an upper length bound to
editRef.
cursoron line 51 bounds model-supplied strings with.max(4_096).editRefhas only.min(1). AneditRefis a short block reference, so a small maximum keeps the validation surface consistent and rejects oversized payloads at the schema boundary.♻️ Proposed bound
editRef: z .string() .min(1) + .max(256) .describe( "editRef of one block from an earlier document read. The result returns the block in full with its current editRef.", ),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/workspaces/content/workspace-content-contract.ts` around lines 56 - 61, Update the editRef schema in the workspace content contract to add a small maximum length consistent with its short block-reference purpose and the existing cursor bound. Preserve the current non-empty validation and description.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eval/datasets/workspace-tools.cases.ts`:
- Around line 22-25: Strengthen the validation patterns in
eval/datasets/workspace-tools.cases.ts:22-25, 59-64, and 161-161. Update
DOCUMENT_MATH_MARKUP to require data-type and data-latex on the same math
element; update the widget-block check to require an encoded HTML tag such as
<style> or <script>; and update the currency check to require plain
$30, $60, and $90 while still rejecting escaped dollar signs.
In `@eval/support/scorers.ts`:
- Around line 73-89: Update eval/support/harness.ts lines 55-85 to record each
workspace_read_items fixture result and expose the editRefs it delivered to
scoring; update scoreTargetedEditProvenance in eval/support/scorers.ts lines
73-89 to accept an editRef only when it is both valid and present in the
recorded prior-read results, preserving the existing targeted, fabricated, and
overwrite checks.
In `@src/features/workspaces/components/ai-chat/useConsumeComposerPrompt.ts`:
- Around line 29-49: The useEffect hook can run twice in React Strict Mode with
the same pendingPrompt value, causing the prompt to be appended twice before
being cleared. Move the clearPrompt(workspaceId) call inside the
requestAnimationFrame callback (after the textarea focus and selection logic) to
ensure the prompt is only cleared after the append to setInput is scheduled and
guaranteed to execute, preventing duplicate appends when the effect re-runs with
the same dependencies.
In `@src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx`:
- Around line 96-99: Update the error handling in WorkspaceWidgetSandbox so
errors reported after the iframe has completed its ready handshake are displayed
as a dismissible overlay or banner while keeping the live iframe mounted; only
replace the iframe with the full error panel when the frame never becomes ready.
Use the existing ready state or readySessionIdRef to distinguish these cases,
and clear the displayed runtime error when dismissed.
In `@src/features/workspaces/content/workspace-content-reader.ts`:
- Around line 137-177: In the readWorkspaceItem function, add a validation check
for file items before calling readFile to reject requests with mode: "block".
When input.item.type is "file" and input.request.mode is "block", return a
failure result with code "invalid_selection" (matching the pattern in
readDocumentBlock) instead of proceeding to readFile, which does not support
block mode.
In `@src/features/workspaces/model/use-workspace-item-path.ts`:
- Around line 35-45: Update readWorkspaceItemPath to resolve the requested item
from the page.items map before calling getWorkspaceAiContextItemPath, using the
cached object as the path’s base item and ensuring ancestor traversal resolves
cached entries by parentId. Preserve the null return when no cached page exists,
and handle a missing cached item without using the stale captured item.
In `@src/styles.css`:
- Line 509: Add an empty line immediately after the explanatory comment and
before the padding-inline declaration in the relevant CSS rule, leaving the
declaration unchanged.
---
Nitpick comments:
In
`@src/features/workspaces/components/widget/workspace-widget-sandbox-document.test.ts`:
- Around line 45-47: Update the CSP assertions in the workspace sandbox document
test to positively verify default-src 'none', connect-src 'none', object-src
'none', base-uri 'none', and form-action 'none', alongside the existing relevant
directives. Remove the ineffective negative img-src https assertion and ensure
the test pins the directives emitted by getWidgetSandboxCsp.
- Around line 99-105: Add a test case for isWidgetSandboxFrameMessage where
source is a different, unrelated value while the other message fields remain
valid, and assert that it returns false. Keep the existing valid-source
rejection cases unchanged.
In
`@src/features/workspaces/components/widget/workspace-widget-sandbox-document.ts`:
- Around line 132-135: Update the tokenDeclarations construction to emit only
entries whose names are included in the WIDGET_SANDBOX_TOKENS allowlist, while
preserving the existing non-empty value filtering and declaration formatting.
Apply this restriction in the document-building code rather than relying on
readWidgetSandboxTheme callers.
In `@src/features/workspaces/content/workspace-content-contract.ts`:
- Around line 56-61: Update the editRef schema in the workspace content contract
to add a small maximum length consistent with its short block-reference purpose
and the existing cursor bound. Preserve the current non-empty validation and
description.
In `@src/features/workspaces/content/workspace-read-references.ts`:
- Line 32: Add a test covering a ready result with type "block" through
createWorkspaceReadReferences or createWorkspaceReadItemsModelOutput, asserting
it completes without dereferencing location and produces the expected output.
Keep the existing file/PDF behavior unchanged, and ensure the test would fail if
the type guard were reordered or reverted.
In `@src/features/workspaces/documents/document-ai-edits.ts`:
- Around line 241-277: Extract the duplicated block-ID inheritance logic into a
shared helper near the edit-processing code, accepting the parsed children and
target block ID and returning the adjusted node list. Use this helper in both
the replace_text branch and the replace branch, preserving inheritance only when
a target block ID and first node exist.
In `@src/features/workspaces/documents/document-ai-html.test.ts`:
- Around line 42-51: Update the fixture in “ignores editRefs supplied in
model-authored HTML” to use an editRef matching documentBlockIdPattern, while
preserving the assertion that the original value is absent and a generated valid
ref is present. Ensure the test specifically exercises rejection of a
structurally valid model-supplied ref.
In `@src/features/workspaces/documents/document-ai-html.ts`:
- Around line 225-239: Update the sub/sup conversion loop around
element.textContent and data-latex so latex text is escaped before
interpolation. Escape backslashes and LaTeX-significant characters (\ , {, }, _,
^, $, %, &, #, ~) while preserving the existing subscript/superscript wrapper
and removal behavior for empty content.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a700fb0f-693d-41c4-b01a-3f425163e5d7
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/features/workspaces/operations/__snapshots__/workspace-tool-surface.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (47)
.gitignoreeval/datasets/workspace-tools.cases.tseval/support/harness.tseval/support/scorers.tseval/workspace-tools.eval.tspackage.jsonscripts/copy-widget-libs.mjssrc/features/workspaces/ai/ai-thread-soul-prompt.tssrc/features/workspaces/ai/ai-thread.tssrc/features/workspaces/ai/skills/widget-authoring/SKILL.mdsrc/features/workspaces/ai/workspace-citations.test.tssrc/features/workspaces/components/WorkspaceItemToolbarSlot.tsxsrc/features/workspaces/components/ai-chat/AiChatPromptInput.tsxsrc/features/workspaces/components/ai-chat/ai-chat-tool-receipts.tssrc/features/workspaces/components/ai-chat/useConsumeComposerPrompt.tssrc/features/workspaces/components/document-editor/DocumentEditorSurface.tsxsrc/features/workspaces/components/document-editor/DocumentToolbar.tsxsrc/features/workspaces/components/widget/WorkspaceAddWidgetDialog.tsxsrc/features/workspaces/components/widget/WorkspaceWidgetSandbox.test.tsxsrc/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsxsrc/features/workspaces/components/widget/workspace-widget-sandbox-document.test.tssrc/features/workspaces/components/widget/workspace-widget-sandbox-document.tssrc/features/workspaces/composer/workspace-composer-actions.tssrc/features/workspaces/content/workspace-content-contract.test.tssrc/features/workspaces/content/workspace-content-contract.tssrc/features/workspaces/content/workspace-content-reader.test.tssrc/features/workspaces/content/workspace-content-reader.tssrc/features/workspaces/content/workspace-read-references.test.tssrc/features/workspaces/content/workspace-read-references.tssrc/features/workspaces/documents/document-ai-edits.test.tssrc/features/workspaces/documents/document-ai-edits.tssrc/features/workspaces/documents/document-ai-html.test.tssrc/features/workspaces/documents/document-ai-html.tssrc/features/workspaces/documents/document-ai-html.worker.test.tssrc/features/workspaces/documents/document-preview-text.test.tssrc/features/workspaces/documents/document-preview-text.tssrc/features/workspaces/documents/document-session.tssrc/features/workspaces/documents/document-widget-extension.tssrc/features/workspaces/documents/document-widget-node.tsxsrc/features/workspaces/documents/tiptap-extensions.tssrc/features/workspaces/documents/tiptap-schema.tssrc/features/workspaces/model/use-workspace-item-path.tssrc/features/workspaces/model/workspace-ai-context-reference.tssrc/features/workspaces/operations/workspace-tool-definitions.tssrc/features/workspaces/operations/workspace-tool-schemas.tssrc/features/workspaces/state/workspace-ai-composer-draft-store.tssrc/styles.css
| export function readWorkspaceItemPath( | ||
| queryClient: QueryClient, | ||
| workspaceId: string, | ||
| item: WorkspaceItem, | ||
| ) { | ||
| const page = queryClient.getQueryData(workspacePageQueryOptions(workspaceId).queryKey); | ||
| if (!page) { | ||
| return null; | ||
| } | ||
|
|
||
| return getWorkspaceAiContextItemPath(item, new Map(page.items.map((entry) => [entry.id, entry]))); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the resolver and all imperative helper callers.
rg -n -C 12 \
'\bgetWorkspaceAiContextItemPath\s*\(|\breadWorkspaceItemPath\s*\(' \
--type=ts --type=tsx .Repository: ThinkEx-OSS/thinkex
Length of output: 189
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'Matching files:'
git ls-files | grep -E '\.(ts|tsx)$' | sed -n '1,50p'
echo
echo 'Search for path helpers (if grep supports -E):'
rg -n -C 12 '\bgetWorkspaceAiContextItemPath\s*\(|\breadWorkspaceItemPath\s*\(' \
--glob '*.ts' --glob '*.tsx' . \
|| trueRepository: ThinkEx-OSS/thinkex
Length of output: 13030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'use-workspace-item-path.ts:'
cat -n src/features/workspaces/model/use-workspace-item-path.ts
echo
echo 'workspace-ai-context-reference.ts:'
cat -n src/features/workspaces/model/workspace-ai-context-reference.ts
echo
echo 'WorkspaceItem type references:'
rg -n -C 4 'interface WorkspaceItem|type WorkspaceItem|WorkspaceItem =|interface WorkspaceBreadcrumb' --glob '*.ts' --glob '*.tsx' srcRepository: ThinkEx-OSS/thinkex
Length of output: 25101
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo 'tree.ts:'
cat -n src/features/workspaces/model/tree.ts
echo
echo 'types.ts relevant WorkspaceItem schema/type:'
cat -n src/features/workspaces/model/types.ts | sed -n '1,220p'Repository: ThinkEx-OSS/thinkex
Length of output: 4781
Resolve the item from the cached page before building the path.
getWorkspaceBreadcrumbItems(item, itemsById) walks item.parentId up through itemsById, but both the final item and ancestor paths are built from item’s name and parentId. A stale captured object can therefore produce an out-of-date AI context path. Use the cached item for the base segment, and also resolve any renamed/moved ancestors from page.items.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/features/workspaces/model/use-workspace-item-path.ts` around lines 35 -
45, Update readWorkspaceItemPath to resolve the requested item from the
page.items map before calling getWorkspaceAiContextItemPath, using the cached
object as the path’s base item and ensuring ancestor traversal resolves cached
entries by parentId. Preserve the null return when no cached page exists, and
handle a missing cached item without using the stale captured item.
| * the editor. | ||
| */ | ||
| --workspace-document-measure: 80ch; | ||
| padding-inline: max(1rem, calc((100% - var(--workspace-document-measure)) / 2)); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add the required empty line before padding-inline.
Stylelint reports declaration-empty-line-before at Line 509. Add an empty line after the explanatory comment.
Proposed fix
*/
+
--workspace-document-measure: 80ch;
padding-inline: max(1rem, calc((100% - var(--workspace-document-measure)) / 2));🧰 Tools
🪛 Stylelint (17.14.1)
[error] 509-509: Expected empty line before declaration (declaration-empty-line-before)
(declaration-empty-line-before)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/styles.css` at line 509, Add an empty line immediately after the
explanatory comment and before the padding-inline declaration in the relevant
CSS rule, leaving the declaration unchanged.
Source: Linters/SAST tools
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 49 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/widget/WorkspaceAddWidgetDialog.tsx">
<violation number="1" location="src/features/workspaces/components/widget/WorkspaceAddWidgetDialog.tsx:76">
P2: On mobile, submitting “Add with AI” stages the request but leaves the chat hidden, so the user sees no composer and the new widget flow appears to do nothing until they open Chat manually. Use a mobile-aware reveal action that selects fullscreen on mobile (or update the shared staging helper to handle mobile).</violation>
</file>
<file name="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx">
<violation number="1" location="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx:141">
P3: Documents with multiple widgets are indistinguishable to screen-reader users because every embedded frame is announced as “Widget preview.” Passing the node’s widget label into this component and incorporating it into the iframe title would preserve the accessible context.</violation>
<violation number="2" location="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx:143">
P1: Widgets can still make network requests despite the documented no-network boundary: the sandbox does not restrict resource loading, and the generated CSP permits every app-origin script URL. Narrow the CSP to the exact bundled KaTeX asset paths (and add an explicit navigation restriction) so widget-controlled URLs cannot send data to the app or another server.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ref={iframeRef} | ||
| title="Widget preview" | ||
| sandbox="allow-scripts" | ||
| srcDoc={srcDoc ?? undefined} |
There was a problem hiding this comment.
P1: Widgets can still make network requests despite the documented no-network boundary: the sandbox does not restrict resource loading, and the generated CSP permits every app-origin script URL. Narrow the CSP to the exact bundled KaTeX asset paths (and add an explicit navigation restriction) so widget-controlled URLs cannot send data to the app or another server.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx, line 143:
<comment>Widgets can still make network requests despite the documented no-network boundary: the sandbox does not restrict resource loading, and the generated CSP permits every app-origin script URL. Narrow the CSP to the exact bundled KaTeX asset paths (and add an explicit navigation restriction) so widget-controlled URLs cannot send data to the app or another server.</comment>
<file context>
@@ -0,0 +1,172 @@
+ ref={iframeRef}
+ title="Widget preview"
+ sandbox="allow-scripts"
+ srcDoc={srcDoc ?? undefined}
+ className="h-full w-full border-0 bg-background"
+ />
</file context>
| stageComposerPrompt( | ||
| workspaceId, | ||
| `Add an interactive widget to ${documentPath}: ${description}`, | ||
| { revealChat: true }, |
There was a problem hiding this comment.
P2: On mobile, submitting “Add with AI” stages the request but leaves the chat hidden, so the user sees no composer and the new widget flow appears to do nothing until they open Chat manually. Use a mobile-aware reveal action that selects fullscreen on mobile (or update the shared staging helper to handle mobile).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/widget/WorkspaceAddWidgetDialog.tsx, line 76:
<comment>On mobile, submitting “Add with AI” stages the request but leaves the chat hidden, so the user sees no composer and the new widget flow appears to do nothing until they open Chat manually. Use a mobile-aware reveal action that selects fullscreen on mobile (or update the shared staging helper to handle mobile).</comment>
<file context>
@@ -0,0 +1,111 @@
+ stageComposerPrompt(
+ workspaceId,
+ `Add an interactive widget to ${documentPath}: ${description}`,
+ { revealChat: true },
+ );
+ onOpenChange(false);
</file context>
There was a problem hiding this comment.
2 issues found across 51 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/ai-chat/AiChatPromptInput.tsx">
<violation number="1" location="src/features/workspaces/components/ai-chat/AiChatPromptInput.tsx:161">
P2: Staging with `revealChat: false` can consume the focus request while the composer is hidden: the mounted textarea is focused inside the `invisible` chat wrapper, then `clearFocusRequest` runs. Focus only when the composer is visible, or avoid creating the request when chat will remain hidden, so a later chat open can still focus the staged text.</violation>
</file>
<file name="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.test.tsx">
<violation number="1" location="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.test.tsx:141">
P3: Add coverage for the onAskAiToFix path: render the sandbox with onAskAiToFix, trigger a runtime error, and assert the "Ask AI to fix" button calls the callback with the error message. This error-repair affordance is a headline feature of the PR and is currently the only behavior in the error view with no test.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| textarea.focus(); | ||
| const caret = textarea.value.length; | ||
| textarea.setSelectionRange(caret, caret); | ||
| clearFocusRequest(activeThreadId, focusRequest); |
There was a problem hiding this comment.
P2: Staging with revealChat: false can consume the focus request while the composer is hidden: the mounted textarea is focused inside the invisible chat wrapper, then clearFocusRequest runs. Focus only when the composer is visible, or avoid creating the request when chat will remain hidden, so a later chat open can still focus the staged text.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/ai-chat/AiChatPromptInput.tsx, line 161:
<comment>Staging with `revealChat: false` can consume the focus request while the composer is hidden: the mounted textarea is focused inside the `invisible` chat wrapper, then `clearFocusRequest` runs. Focus only when the composer is visible, or avoid creating the request when chat will remain hidden, so a later chat open can still focus the staged text.</comment>
<file context>
@@ -130,6 +144,25 @@ export default function AiChatPromptInput({
+ textarea.focus();
+ const caret = textarea.value.length;
+ textarea.setSelectionRange(caret, caret);
+ clearFocusRequest(activeThreadId, focusRequest);
+ });
+
</file context>
| expect(frame?.style.height).toBe("120px"); | ||
| }); | ||
|
|
||
| it("lets the error view size itself instead of keeping the widget height", async () => { |
There was a problem hiding this comment.
P3: Add coverage for the onAskAiToFix path: render the sandbox with onAskAiToFix, trigger a runtime error, and assert the "Ask AI to fix" button calls the callback with the error message. This error-repair affordance is a headline feature of the PR and is currently the only behavior in the error view with no test.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/widget/WorkspaceWidgetSandbox.test.tsx, line 141:
<comment>Add coverage for the onAskAiToFix path: render the sandbox with onAskAiToFix, trigger a runtime error, and assert the "Ask AI to fix" button calls the callback with the error message. This error-repair affordance is a headline feature of the PR and is currently the only behavior in the error view with no test.</comment>
<file context>
@@ -0,0 +1,216 @@
+ expect(frame?.style.height).toBe("120px");
+ });
+
+ it("lets the error view size itself instead of keeping the widget height", async () => {
+ await act(async () => root.render(<WorkspaceWidgetSandbox html="<p>broken</p>" />));
+
</file context>
There was a problem hiding this comment.
0 issues found across 5 files (changes from recent commits).
Requires human review: Auto-approval blocked by 10 unresolved issues from previous reviews.
Re-trigger cubic
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="eval/support/harness.ts">
<violation number="1" location="eval/support/harness.ts:205">
P2: A schema-valid edit path such as `toString` can make the targeted-edit evaluator throw instead of reporting a failed provenance check, because the plain object exposes inherited properties. Build the snapshot with a null prototype (or use an own-property lookup) before grading path-keyed refs.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| name: part.toolName, | ||
| known: Boolean(definition), | ||
| input: part.input, | ||
| priorReadEditRefsByPath: Object.fromEntries( |
There was a problem hiding this comment.
P2: A schema-valid edit path such as toString can make the targeted-edit evaluator throw instead of reporting a failed provenance check, because the plain object exposes inherited properties. Build the snapshot with a null prototype (or use an own-property lookup) before grading path-keyed refs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At eval/support/harness.ts, line 205:
<comment>A schema-valid edit path such as `toString` can make the targeted-edit evaluator throw instead of reporting a failed provenance check, because the plain object exposes inherited properties. Build the snapshot with a null prototype (or use an own-property lookup) before grading path-keyed refs.</comment>
<file context>
@@ -178,7 +202,9 @@ export async function runWorkspaceAgent(input: WorkspaceAgentInput): Promise<Wor
name: part.toolName,
input: part.input,
- priorReadEditRefs: [...priorReadEditRefs],
+ priorReadEditRefsByPath: Object.fromEntries(
+ [...priorReadEditRefsByPath].map(([path, refs]) => [path, [...refs]]),
+ ),
</file context>
There was a problem hiding this comment.
1 issue found across 4 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/widget/workspace-widget-sandbox-document.ts">
<violation number="1" location="src/features/workspaces/components/widget/workspace-widget-sandbox-document.ts:248">
P2: Removing `data-latex` leaves the previously rendered formula displayed because the observer handles only targets that still have the attribute. The transition should clear or restore the node's non-math content when the attribute is removed so interactive widgets cannot show stale math.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| new MutationObserver(function(records){ | ||
| records.forEach(function(record){ | ||
| if(record.type==="attributes"){ | ||
| if(record.target.matches&&record.target.matches("[data-latex]")){ |
There was a problem hiding this comment.
P2: Removing data-latex leaves the previously rendered formula displayed because the observer handles only targets that still have the attribute. The transition should clear or restore the node's non-math content when the attribute is removed so interactive widgets cannot show stale math.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/widget/workspace-widget-sandbox-document.ts, line 248:
<comment>Removing `data-latex` leaves the previously rendered formula displayed because the observer handles only targets that still have the attribute. The transition should clear or restore the node's non-math content when the attribute is removed so interactive widgets cannot show stale math.</comment>
<file context>
@@ -222,23 +222,45 @@ body{padding:clamp(10px,2.5%,16px);}
+ new MutationObserver(function(records){
+ records.forEach(function(record){
+ if(record.type==="attributes"){
+ if(record.target.matches&&record.target.matches("[data-latex]")){
+ delete record.target.dataset.widgetMathRendered;
+ try{renderLatexNode(record.target);}catch(_){}
</file context>
There was a problem hiding this comment.
1 issue found across 11 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx">
<violation number="1" location="src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx:160">
P2: Read-only viewers lose recovery for non-ready widget errors: when `preserveFrame` is false and `onAskAiToFix` is omitted, this condition removes the iframe and renders no action. Retaining a retry/remount path for consumers without an AI-fix callback would let transient frame failures recover.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| <div className="max-h-32 w-full max-w-lg overflow-auto whitespace-pre-wrap rounded-md bg-muted px-4 py-3 font-mono text-muted-foreground text-xs leading-relaxed"> | ||
| {error.message} | ||
| </div> | ||
| {error.preserveFrame || onAskAiToFix ? ( |
There was a problem hiding this comment.
P2: Read-only viewers lose recovery for non-ready widget errors: when preserveFrame is false and onAskAiToFix is omitted, this condition removes the iframe and renders no action. Retaining a retry/remount path for consumers without an AI-fix callback would let transient frame failures recover.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsx, line 160:
<comment>Read-only viewers lose recovery for non-ready widget errors: when `preserveFrame` is false and `onAskAiToFix` is omitted, this condition removes the iframe and renders no action. Retaining a retry/remount path for consumers without an AI-fix callback would let transient frame failures recover.</comment>
<file context>
@@ -162,26 +157,25 @@ export function WorkspaceWidgetSandbox({
- </Button>
- ) : null}
- </div>
+ {error.preserveFrame || onAskAiToFix ? (
+ <div className="flex items-center gap-2">
+ {error.preserveFrame ? (
</file context>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const blockId = blockIds[index]; | ||
| if (!blockId) throw new Error("Eval standup fixture has an unexpected block count."); | ||
| const node = withTiptapNodeAiRef(document.child(index), blockId); | ||
| const snapshot = await createDocumentAiBlockSnapshot(node); |
There was a problem hiding this comment.
React Doctor · react-doctor/async-await-in-loop (warning)
This makes the for-loop slow because each await runs one after another, so collect the independent calls & run them together with await Promise.all(items.map(...))
Fix → Collect the items, then use await Promise.all(items.map(...)) so independent work runs at the same time
| execute: async () => evalToolFixture(definition.name), | ||
| }), | ||
| ]), | ||
| workspaceToolDefinitions |
There was a problem hiding this comment.
React Doctor · react-doctor/js-combine-iterations (warning)
This loops over your list twice because .filter().map() makes two passes, so do it in one pass with .reduce() or a for...of loop
Fix → Combine .map().filter() style chains into one pass with .reduce() or a for...of loop, so you only loop over the list once
There was a problem hiding this comment.
0 issues found across 8 files (changes from recent commits).
Requires human review: Auto-approval blocked by 8 unresolved issues from previous reviews.
Re-trigger cubic
2 participants
Summary
editRefcontract.Why
Documents could explain an idea but could not let the reader interact with it. This adds small simulations, calculators, adjustable visuals, quizzes, and similar tools without introducing a separate workspace item type.
The AI guidance stays conservative: ordinary prose and tables remain the default, Mermaid remains available for requested static diagrams in chat, and widgets are reserved for explicit, interactive, or document-specific visual requests.
Changes
allow-scriptsonly, a restrictive CSP, no network access, and no parent storage or DOM access.editRef, return canonical block content and its current target together, and use that same content forreplace_text.Testing
pnpm checkNODE_OPTIONS="--no-experimental-webstorage" pnpm test— 66 files and 293 tests passedThe local shell is on unsupported Node 25, while the repository requires Node 24. Node 25 exposes an experimental broken global
localStorageunless web storage is disabled; the full suite passes with that Node 25-only workaround.Review Notes
Start with:
src/features/workspaces/documents/tiptap-schema.tssrc/features/workspaces/documents/document-widget-extension.tssrc/features/workspaces/components/widget/WorkspaceWidgetSandbox.tsxsrc/features/workspaces/components/widget/workspace-widget-sandbox-document.tssrc/features/workspaces/documents/document-ai-edits.tsThe main security-sensitive area is the iframe sandbox/CSP and its small theme, height, and error message channel. Generated KaTeX browser files are copied during
prepareand intentionally ignored by Git.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by CodeRabbit