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

AGENTS.md

@@ -54,6 +54,16 @@ any self-hosted Forgejo Actions runner because `runs-on` is parameterized via
   short LRU of `(source_event_id, target_object_id)` pairs catches the case
   where the marker was stripped. Both layers must stay — they cover different
   failure modes.
+  - **Plane CE v1.3.1 strips HTML comments from `description_html`** during
+    ProseMirror sanitization (verified against plane.stern.ca — POST body
+    `<p>x</p>\n\n<!-- ... -->` round-trips as `<div><p>x</p>\n\n</div>`).
+    Comment bodies (`comment_html`) preserve the marker; work item
+    descriptions do not. The marker check is therefore non-functional on
+    inbound `work_item.*` echoes. The durable defence is the
+    `external_source` round-trip: the bridge stamps
+    `external_source="forge:owner/repo"` on every forge→plane create, and
+    `handlePlaneWorkItemCreated` skips any inbound `work_item.created`
+    whose `WorkItem.ExternalSource` starts with `"forge:"`. See PFB-27.
 - **HMAC verification happens at the HTTP handler boundary**, before the body
   is parsed. Constant-time compare. Reject on missing header.
 - **Stateless process.** No on-disk state, no DB. Config + env vars in, HTTP

CHANGELOG.md

@@ -4,6 +4,28 @@ All notable changes to plane-forge-bridge are documented here. The format
 follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the
 project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.1.4] — 2026-05-24
+
+### Fixed
+
+- **Plane → forge: drop work_item.created echoes via external_source
+  (PFB-27).** 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
+  supposed to catch the echo, 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). With
+  no marker on the echo, `handlePlaneWorkItemCreated` would create a
+  duplicate forge issue — the failure mode PFB-25 exposed in
+  production. The handler now gates on `WorkItem.ExternalSource`
+  starting with the bridge's `"forge:"` prefix and skips with
+  `reasonPlaneCreatedEchoExternalSource`. Non-bridge external sources
+  (e.g. an operator importing from GitHub) are unaffected. AGENTS.md
+  updated to document the ProseMirror sanitization finding so the next
+  feature work doesn't reach for the marker as the durable
+  description-side defence.
+
 ## [0.1.3] — 2026-05-24
 
 ### Fixed
@@ -78,6 +100,7 @@ Loop-break: HTML-comment marker + in-memory LRU. Single static binary,
 distroless runtime, stateless. CI matrix tests against real Forgejo 15
 and Gitea 1.22 service containers end-to-end on every PR.
 
+[0.1.4]: https://github.com/hstern/plane-forge-bridge/compare/v0.1.3...v0.1.4
 [0.1.3]: https://github.com/hstern/plane-forge-bridge/compare/v0.1.2...v0.1.3
 [0.1.2]: https://github.com/hstern/plane-forge-bridge/compare/v0.1.1...v0.1.2
 [0.1.1]: https://github.com/hstern/plane-forge-bridge/compare/v0.1.0...v0.1.1

internal/sync/planeworkitem.go

@@ -37,8 +37,32 @@ const (
 	// loud so operators can correlate the deferral with the rollout
 	// state.
 	reasonPlaneIssueWriterMissing = "plane → forge issue writes deferred — forge client build does not implement ForgeIssueWriter"
+
+	// reasonPlaneCreatedEchoExternalSource is returned when a plane
+	// work_item.created webhook arrives for a work item that already
+	// carries a bridge-stamped external_source ("forge:owner/repo"). The
+	// bridge sets external_source / external_id on every forge → plane
+	// create, so a created webhook with that pair set is necessarily the
+	// echo of our own write — mirroring it back to forge would duplicate
+	// the original forge issue.
+	//
+	// This is the durable backstop from PFB-27. The HTML-comment marker
+	// is the bridge's primary loop-break defence, but Plane CE v1.3.1
+	// strips HTML comments from description_html during ProseMirror
+	// sanitization (verified against plane.stern.ca), so the marker
+	// never reaches the webhook payload for work item descriptions.
+	// Plane preserves the marker in comment_html, so comment echoes
+	// still rely on the marker.
+	reasonPlaneCreatedEchoExternalSource = "plane work_item.created echoes a bridge-originated work item (external_source matches forge: prefix)"
 )
 
+// externalSourceForgePrefix is the prefix the bridge stamps on every
+// forge → plane create. Matching it on an inbound work_item.created
+// webhook is the durable signal that the work item is our own echo —
+// the marker check can't see it (Plane sanitizes HTML comments out of
+// description_html). See PFB-27.
+const externalSourceForgePrefix = "forge:"
+
 // HandlePlaneWorkItem translates a plane work_item.* event into a forge
 // issue create or update. This is the inverse of HandleForgeIssue and
 // closes the bidirectional loop for issues (step 10 of the build order).
@@ -138,11 +162,34 @@ func (e *Engine) HandlePlaneWorkItem(ctx context.Context, evt *plane.Event) (*Ou
 }
 
 // handlePlaneWorkItemCreated implements the EventWorkItemCreated branch.
-// CreateIssue is unconditional in v1 (no reverse lookup); the caller's
-// loop-break LRU is the only defence against a re-delivered create
-// producing a duplicate forge issue. The LRU is the server's job and
-// fires before we get here, so v1 ships without the redundant lookup.
+//
+// First gate: external_source check. The bridge stamps
+// external_source="forge:owner/repo" on every forge → plane create
+// (see externalRef). A plane work_item.created event whose WorkItem
+// carries that prefix is necessarily our own echo — mirroring it back
+// would duplicate the original forge issue. The check is the durable
+// backstop from PFB-27 (the marker-stripping window the inbound
+// HTML-comment marker can't cover).
+//
+// Second gate (unchanged): forge writer capability check.
+//
+// CreateIssue is otherwise unconditional in v1 — no reverse lookup,
+// no LRU consultation here.
 func (e *Engine) handlePlaneWorkItemCreated(ctx context.Context, evt *plane.Event, link *mapping.Link) (*Outcome, error) {
+	if strings.HasPrefix(evt.WorkItem.ExternalSource, externalSourceForgePrefix) {
+		e.Log.Info("dropping plane work_item.created echo (external_source matches bridge prefix)",
+			"plane_project", evt.WorkItem.Project,
+			"work_item", evt.WorkItem.ID,
+			"external_source", evt.WorkItem.ExternalSource,
+			"external_id", evt.WorkItem.ExternalID,
+			"delivery", evt.DeliveryID)
+		return &Outcome{
+			Action: ActionSkipped,
+			Reason: reasonPlaneCreatedEchoExternalSource,
+			Link:   link,
+		}, nil
+	}
+
 	writer, ok := e.ForgeClient.(ForgeIssueWriter)
 	if !ok {
 		e.Log.Warn("plane work_item.created received but ForgeClient cannot write issues",

internal/sync/planeworkitem_test.go

@@ -170,6 +170,82 @@ func TestHandlePlaneWorkItem_AssigneeResolvedFromForgeEmail(t *testing.T) {
 	}
 }
 
+// TestHandlePlaneWorkItem_CreatedEchoSkipsByExternalSource is the
+// regression test for PFB-27. The bridge stamps external_source on
+// every forge → plane create; Plane echoes that back on its own
+// work_item.created webhook. The HTML-comment marker the bridge also
+// stamps would normally catch this, but Plane CE v1.3.1 strips HTML
+// comments out of description_html during sanitization (preserves
+// them in comment_html — verified against plane.stern.ca). The
+// external_source check is the durable defence.
+//
+// This test fails if anyone removes the external_source gate.
+func TestHandlePlaneWorkItem_CreatedEchoSkipsByExternalSource(t *testing.T) {
+	t.Parallel()
+	e, _, fc := newPlaneWorkItemTestEngine(t)
+
+	evt := mkPlaneWorkItemEvent(plane.EventWorkItemCreated)
+	// The work item carries the bridge's external_source — this is our own
+	// echo. The marker on the description is gone (Plane stripped it during
+	// sanitization), so the external_source check is what saves us.
+	evt.WorkItem.ExternalSource = "forge:acme/widgets"
+	evt.WorkItem.ExternalID = "42"
+
+	out, err := e.HandlePlaneWorkItem(context.Background(), evt)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if out.Action != ActionSkipped {
+		t.Fatalf("Action=%v reason=%q, want ActionSkipped", out.Action, out.Reason)
+	}
+	if out.Reason != reasonPlaneCreatedEchoExternalSource {
+		t.Errorf("Reason=%q, want %q", out.Reason, reasonPlaneCreatedEchoExternalSource)
+	}
+	if out.Link == nil {
+		t.Errorf("Link should be set on the echo skip — the project link was matched")
+	}
+	if len(fc.IssueCreates) != 0 {
+		t.Errorf("IssueCreates=%d on echo path — must be 0 (duplicate avoidance)",
+			len(fc.IssueCreates))
+	}
+}
+
+// TestHandlePlaneWorkItem_CreatedNonBridgeExternalSourceProceeds
+// pins that external_source pointing at a non-bridge system (e.g.
+// "github:foo/bar" if an operator imports issues from GitHub) does
+// NOT trigger the echo skip — only the "forge:" prefix is the bridge
+// signal. Without this guard the bridge would silently drop legitimate
+// non-bridge work items.
+func TestHandlePlaneWorkItem_CreatedNonBridgeExternalSourceProceeds(t *testing.T) {
+	t.Parallel()
+	e, pc, fc := newPlaneWorkItemTestEngine(t)
+	pc.ListProjectLabelsFunc = func(_ context.Context, _ string) ([]plane.Label, error) {
+		return []plane.Label{
+			{ID: "label-uuid-bug", Name: "bug"},
+			{ID: "label-uuid-help", Name: "help wanted"},
+		}, nil
+	}
+	fc.ListRepoLabelsFunc = func(_ context.Context, _, _ string) ([]forge.Label, error) {
+		return []forge.Label{{ID: 11, Name: "bug"}, {ID: 22, Name: "help wanted"}}, nil
+	}
+
+	evt := mkPlaneWorkItemEvent(plane.EventWorkItemCreated)
+	evt.WorkItem.ExternalSource = "github:acme/widgets"
+	evt.WorkItem.ExternalID = "99"
+
+	out, err := e.HandlePlaneWorkItem(context.Background(), evt)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if out.Action != ActionCreated {
+		t.Fatalf("Action=%v reason=%q, want ActionCreated (non-bridge source should mirror)",
+			out.Action, out.Reason)
+	}
+	if len(fc.IssueCreates) != 1 {
+		t.Errorf("IssueCreates=%d, want 1", len(fc.IssueCreates))
+	}
+}
+
 func TestHandlePlaneWorkItem_NoLinkSkips(t *testing.T) {
 	t.Parallel()
 	e, _, fc := newPlaneWorkItemTestEngine(t)