Design: Element-anchored review comments with selective agent feedback routing#2117
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
teleaziz
changed the title
…comments # Conflicts: # templates/design/app/pages/Present.tsx
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Here's a visual recap of what changed: Open the full interactive recap |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Builder reviewed your changes and found 2 potential issues 🟡
Review Details
Incremental Code Review Summary
The latest PR update fixes the previously reported view-screen privacy issue: review data is now redacted through shared identity helpers before being included in the agent-facing response, so that existing comment has been resolved. The broader review architecture remains sound, with action-backed mutations, scoped access checks, transactional routing, and server-side public/private access behavior.
New Findings
🟡 MEDIUM
- Public identity redaction still allows email addresses embedded in display names or mention labels, such as
Alice <[email protected]>, to reach anonymous viewers. - A dropped iframe focus response can leave the pending nonce stuck indefinitely, preventing future retries for the same thread focus request.
🟢 LOW
- In the non-transactional reply-routing fallback, a cleanup failure can mask the original routing error. This is diagnostic-only and non-blocking.
🧪 Browser testing: Skipped — browser executors again lacked Chrome automation tools. The planner verified relevant public/private behavior server-side and seeded grounded test data, but all 14 UI cases remain unverified.
| function safeReviewDisplayName( | ||
| value: string | null | undefined, | ||
| ): string | null { | ||
| const name = value?.trim(); | ||
| if (!name || /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(name)) { | ||
| return null; | ||
| } | ||
| return name; |
There was a problem hiding this comment.
🟡 Redact emails embedded in public display names
safeReviewDisplayName() only rejects a value when the entire string is an email address. Public comments and mentions accept caller-supplied labels, so values such as Alice <[email protected]> or [email protected] (Acme) are returned verbatim to anonymous viewers. Treat any display name containing an email address as unsafe, or derive a validated public display name server-side.
| if (pendingFocusNonceRef.current === nonce) return false; | ||
| const iframe = canvas.querySelector<HTMLIFrameElement>( | ||
| "iframe[data-design-preview-iframe]", | ||
| ); | ||
| if (!iframe?.contentWindow) return false; | ||
| const correlationId = crypto.randomUUID(); | ||
| pendingFocusNonceRef.current = nonce; | ||
| frameCallbacksRef.current.set(correlationId, (payload) => { | ||
| pendingFocusNonceRef.current = null; | ||
| if (payload.focused === true) { | ||
| lastFocusNonceRef.current = nonce; | ||
| setLayoutTick((current) => current + 1); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🟡 Retry iframe focus requests after dropped responses
When the iframe bridge is used, pendingFocusNonceRef is set before posting review-focus and is only cleared by the response callback. If the iframe is still loading or the message/response is dropped, later effect runs return early for the same nonce, leaving jump-to-thread focus permanently disabled until a reload or unrelated screen change. Add timeout/fallback cleanup or re-arm the request when the iframe becomes ready.
2 participants
Reviewers can pin comments directly on design elements — in the editor, Present
mode, or a public share link — and feedback resolves with real edits, not just
chat suggestions.
What's included
fallback), rendered across editor, Present, and public share views. Works in
sandboxed iframes via a postMessage hit-test bridge.
agent-targeted feedback. Editors can selectively route any open thread to the
agent (
send-review-thread-to-agent, new core action), or apply the wholequeue with one "Apply feedback (N)" button. The agent resolves each thread
only after the edit is persisted and verified.
the editor header and share surfaces; agent-readable via
view-screen.transactional reply routing,
resolutionNoteon resolved threads, per-commentcanDeletecapabilities, and identity redaction for anonymous/public viewers.app-state writes fire) and sign in to post.
design template.