Conversation
"Ask the agent about this" sent only the file name, so the agent could not tell which of several similar blocks the person selected. The prompt now names the block by blockId, or by its opening tag in the source, with the page position, props, and visible text when one piece of code renders many times. The button waits for the editor to save, so the prompt matches the source the agent reads. The canvas skill asks for a blockId on each editable() use. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 2741091a-93e9-4bc1-965a-c9ddc535ef67
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
|
React Doctor found 1 issue in 1 file · 1 warning. 1 warning
Reviewed by React Doctor for commit |
🤖 CI report
|
HostHog preview —
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthroughCanvas selections now carry normalized visible text and rendered-instance details for repeated source elements. A new prompt builder formats references using block IDs or available source and selection context, with tests for its output. The canvas panel now uses this builder for agent prompts and derives a Priority: ➖ Normal Merge Risk: 🔵 Low · up to The change is mergeable with a small documentation correction: add a blockId to the usage example so copied code follows the guidance. The opening-tag escape concern is fixed. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The change provides more precise block descriptions without automatically starting an edit. Save failures or conflicts can leave those descriptions inconsistent with saved content, weakening the intended single-block scope. No privilege expansion was established, but later execution permissions were not fully assessed. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ✅ 1✅ Passed checks (1 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
products/desktop/packages/core/src/canvas/blockLibrary/blockReference.ts-36-36 (1)
36-36: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winHandle an even number of backslashes before a closing quote.
If a JSX attribute value ends in an escaped backslash, Line 36 treats its closing quote as escaped.
openingTagcan then include content beyond the opening tag, so the agent receives the wrong source reference. Track whether the preceding backslash run has odd length before treating a quote as escaped.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: fccef08e-4e0e-4bfe-98aa-f5533d68ae47
📒 Files selected for processing (6)
products/canvas/skills/building-canvases/SKILL.mdproducts/desktop/packages/core/src/canvas/blockLibrary/blockReference.test.tsproducts/desktop/packages/core/src/canvas/blockLibrary/blockReference.tsproducts/desktop/packages/ui/src/features/canvas/blocks/CanvasBlocksPanel.tsxproducts/desktop/packages/ui/src/features/canvas/blocks/canvasEditRuntime.tsproducts/desktop/packages/ui/src/features/canvas/blocks/canvasSourceStore.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
👀 Auto-assigned reviewersThese soft owners were skipped because they only have minor changes here. Nothing blocks merge, so self-assign if you'd like a look:
Soft owners come from each directory's |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
There was a problem hiding this comment.
Not approved — this change needs a human reviewer.
Re-add the stamphog label to request another review once you have addressed this.
CodeRabbit flagged an unresolved Major correctness issue that I confirmed by reading the code: when a selected block has a blockId, blockReferencePrompt returns early and drops the instance/props details entirely, even though the SKILL.md guidance itself recommends dynamic per-item blockIds (e.g. a template literal built from a list key) that won't appear literally in the source — which is exactly the ambiguous-block scenario this PR exists to fix. This is a real, unaddressed functional gap in the core feature, not a nitpick.
- Author wrote 100% of the modified lines and has 2 merged PRs in these paths (familiarity STRONG).
- coderabbitai[bot] reviewed the current head.
- 5 of the 6 changed files are governed by
products/desktop/AGENT_APPROVALS.md. - Unresolved CodeRabbit 'Major' comment on blockReference.ts: when blockId is set, the function short-circuits and omits instance/props/visibleText, so a repeated block with a computed (non-literal) blockId still gives the agent an ambiguous reference — undermining the PR's stated goal.
- Unresolved CodeRabbit 'Minor' comment on blockReference.ts line 36: quote-escaping check in openingTag doesn't handle an even number of preceding backslashes, so it can occasionally include extra source beyond the real opening tag (lower severity, quick fix).
Gate mechanics and policy version
| Gate | Result | |
|---|---|---|
| prerequisites | ✓ | all clear |
| deny-list | ✓ | no deny categories matched |
| size | ✓ | 153L, 4F substantive, 252L/6F incl. docs/generated/snapshots — within ceiling |
| tier | ✓ | T1-agent / T1c-medium (252L, 6F, single-area, feat) |
| stamphog 2.2.0 | .stamphog/policy.yml @ f521c64 · reviewed head f521c64 |
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| const { label, source, blockId, instance } = input; | ||
| const where = source ? ` in ${source.file}` : ""; | ||
| if (blockId) | ||
| return `Change only this ${label} with blockId ${blockId}${where}: `; |
There was a problem hiding this comment.
Low: Prompt injection through block references
blockId is copied verbatim into the task's first user message, and the fallback path similarly includes runtime props and visible text. Because the skill now recommends deriving IDs from item.id, an attacker who supplies a queried analytics value can embed instructions that run when an editor submits this prefilled prompt; canvas tasks use bypass-permissions mode with full PostHog MCP scopes. Use an opaque, validated identifier containing only safe characters, avoid placing rendered data in the instruction, and pass any necessary display metadata as explicitly untrusted structured context to a least-privileged task.
There was a problem hiding this comment.
Partly fixed in 63373f4: the prompt uses a blockId only when it matches ^[\w.:-]{1,80}$. Other IDs fall back to the source reference. Props and visible text stay in the draft, clipped to short lengths, because they are the only way to find a repeated item. The person sees and edits the draft before they send it.
There was a problem hiding this comment.
Thanks for the detail. I can't automatically confirm this is safe to dismiss, so I'm leaving the thread open for a maintainer to make the call.
PR overviewThis pull request updates canvas agent requests to identify the currently selected block, using block references and fallback block details in the initial prompt. One prompt-injection path remains open because attacker-controlled block identifiers or rendered values can be inserted directly into an agent prompt. If an editor submits that prompt, injected instructions could execute through an agent running with broad PostHog permissions; no issues have yet been addressed. Open issues (1)
Fixed/addressed: 0 · PR risk: 7/10 |
Append the page instance, props, and visible text to a blockId reference, ignore block ids with unsafe characters, handle escaped backslashes before a closing quote, and add blockId to the SignupFunnel example props. Generated-By: PostHog Desktop Task-Id: 5bd38eba-eb1f-4af7-86c2-bc12b26fb6d7
There was a problem hiding this comment.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
products/canvas/skills/building-canvases/SKILL.md-85-85 (1)
85-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd
blockIdto the usage example.Line 84 shows a
SignupFunneluse withoutblockId, but this step requires one for every use. Copying the example omitsdata-ph-block-id, so the component cannot use the block-ID reference path. Add ablockIdto the example.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 38ad638a-7559-4c40-ae04-8cc03bd20120
📒 Files selected for processing (3)
products/canvas/skills/building-canvases/SKILL.mdproducts/desktop/packages/core/src/canvas/blockLibrary/blockReference.test.tsproducts/desktop/packages/core/src/canvas/blockLibrary/blockReference.ts
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review.
This branch has not been deployed
2 participants
Problem
Change the channel kpi card at src/canvas.tsx:.Changes
blockId, when the block has one.Change only this channel kpi card `<ChannelKpiCard channel="Referral" goal={800} />` in src/canvas.tsx:..map(), or markup inside a shared component), the page position, the props, and the visible text.building-canvasesskill now asks for ablockIdon eacheditable()use, so new canvases have stable ids.instanceandvisibleText. The message itself is built in@posthog/core(blockReference.ts).Not rendered: the only visual change is the spinner on the button, and no screenshot was taken.
How did you test this code?
blockReference.test.ts(5 cases): catches a message that loses the block id, cuts the opening tag at an=>inside braces, numbers duplicate tags wrong, or drops the page position and text for code that renders many times.@posthog/uiand@posthog/core, typecheck for both packages, Biome, andhogli lint:skillspass locally..map()cards, a title inside a shared component, the agent's source copy out of date, duplicate tags, a bare heading, andblockIdwith and without an out-of-date copy. Sonnet found the correct block in 8 of 8. Haiku found it in 8 of 8, but sometimes planned the change on the shared component.Release status
Automatic notifications
Docs update
None.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: PostHog Desktop (Claude Code), claude-opus-5-5
/writing-tests,/writing-user-facing-copy,/writing-skills,/writing-pr-descriptions.Created with PostHog Desktop
🤖 Generated with Claude Code