Skip to content

plane: drop work_item.created echoes via external_source (PFB-27)#15

Merged
hstern merged 1 commit into
mainfrom
fix/pfb-27-external-source-echo
May 24, 2026
Merged

plane: drop work_item.created echoes via external_source (PFB-27)#15
hstern merged 1 commit into
mainfrom
fix/pfb-27-external-source-echo

Conversation

@hstern

@hstern hstern commented May 24, 2026

Copy link
Copy Markdown
Owner

Summary

  • handlePlaneWorkItemCreated now gates on WorkItem.ExternalSource starting with "forge:" and skips with reasonPlaneCreatedEchoExternalSource. The bridge stamps that prefix on every forge→plane create, so an inbound work_item.created carrying it is necessarily our own echo — mirroring it back duplicates the original forge issue.
  • Non-bridge external_source values (e.g. operator-imported "github:..." work items) are unaffected — only the bridge prefix triggers the skip.
  • AGENTS.md documents the underlying finding: Plane CE v1.3.1 strips HTML comments from description_html during ProseMirror sanitization, so the loop-break marker is non-functional on the description side. Plane preserves the marker in comment_html, so comment echoes still work via the marker.

Why

PFB-25 surfaced the duplicate path in production: a forge→plane create succeeded on the Plane side but the bridge errored decoding the REST response. The LRU was never populated (it's recorded after translator success). Plane's work_item.created echo arrived, the marker had been stripped by Plane during sanitization, and the bridge happily created a duplicate Forgejo issue.

PFB-25 closed the decode regression that triggered this specific instance, but the class of bug — any partial failure between Plane-side success and bridge-side success leaves the loop-break LRU empty and the marker is already gone — was open. This PR closes the structural gap.

Verified the sanitization claim against plane.stern.ca:

POST description_html: "<p>x</p>\n\n<!-- pfb:src=forge,evt=probe123 -->"
GET  description_html: "<div><p>x</p>\n\n</div>"
                       ^ marker dropped, body wrapped in <div>

POST comment_html:     "<p>my comment</p>\n\n<!-- pfb:src=forge,evt=c123 -->"
GET  comment_html:     "<p>my comment</p>\n\n<!-- pfb:src=forge,evt=c123 -->"
                       ^ marker preserved

Trade-offs considered

  • Why not pre-record the LRU before POST returns (Option A from PFB-27 body)? The LRU keys are (side, deliveryID, targetObjID) — the forge-side delivery ID and plane-side delivery ID are unrelated, so pre-recording on one side doesn't help the other. Would require a separate cross-side keying scheme; complexity not justified given external_source covers the same threat.
  • Why not validate external_source against the link's configured forge repo? A multi-link bridge with multiple forge repos still wants to skip ALL bridge-created work items; tying the skip to a specific link risks false-negatives if links change. The "forge:" prefix check is the right strictness.
  • Why not also gate work_item.updated / deleted? Both currently skip with deferral reasons (no reverse lookup yet). When they're implemented, the same gate applies.

Test plan

  • make race — all packages green
  • make lint — 0 issues
  • New regression tests: TestHandlePlaneWorkItem_CreatedEchoSkipsByExternalSource (bridge prefix → skip) and TestHandlePlaneWorkItem_CreatedNonBridgeExternalSourceProceeds (non-bridge prefix → proceed)
  • Verified Plane sanitization behaviour against plane.stern.ca (probe commands captured in PR body)
  • After merge: cut v0.1.4, pull :0 on the bridge host, confirm no duplicates on a clean forge→plane create even if response decode is artificially failed

🤖 Generated with Claude Code

The bridge stamps external_source="forge:owner/repo" on every
forge->plane create; Plane echoes that back on its own
work_item.created webhook. The HTML-comment loop-break marker is the
designed defence against echoes, but Plane CE v1.3.1 strips HTML
comments out of description_html during ProseMirror sanitization
(verified against plane.stern.ca: <p>x</p>\n\n<!-- ... --> round-trips
as <div><p>x</p>\n\n</div>; comment bodies preserve the marker). Without
the marker the bridge created a duplicate forge issue on every echo,
the failure mode PFB-25 exposed in production.

handlePlaneWorkItemCreated now gates on WorkItem.ExternalSource
starting with the bridge's "forge:" prefix and skips with a clear
reason; non-bridge sources ("github:..." imports) are unaffected.
Regression tests cover both paths. AGENTS.md documents the
ProseMirror sanitization finding alongside the marker invariant.

Refs PFB-27.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hstern hstern merged commit 293c7f5 into main May 24, 2026
5 checks passed
@hstern hstern deleted the fix/pfb-27-external-source-echo branch May 24, 2026 08:18
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.

1 participant