feat(broadcast): migrate to Subscribe(SelfTopic())+Publish

[adnaan]

Phase 5 of the broadcast-action-redesign proposal removes ctx.BroadcastAction from livetemplate in v0.10.0. This PR migrates the examples monorepo to the new opt-in peer-fanout pattern:

• Mount: ctx.Subscribe(ctx.SelfTopic())
• Actions: ctx.Publish(ctx.SelfTopic(), "ActionName", data)

Bumps livetemplate pin v0.9.0 → v0.9.2 (Subscribe / Publish / SelfTopic already ship in v0.9.2 via Phase 1).

Migrated apps

App	Sites	Notes
chat/main.go	3	UserJoined / NewMessage / UserLeft
shared-notepad/main.go	1	Refresh
landing-demo/main.go	3	Increment / Decrement / Reset; new Mount method added to establish the Subscribe
todos/controller.go	4	RefreshTodos × 4

Plus comment rewording in landing-demo/landing_demo_test.go (skipped subtest renamed BroadcastAction_Propagates_To_Peer_Tab → Publish_Propagates_To_Peer_Tab).

Verification

• ✅ go build ./... — clean
• ✅ go vet ./... — clean
• ✅ Substring grep across the repo for BroadcastAction returns zero matches

Ordering

This PR is intentionally landed before livetemplate v0.10.0:

• The migrated code uses Subscribe / Publish / SelfTopic which are already in v0.9.2
• Once livetemplate v0.10.0 ships (which removes BroadcastAction), no further changes needed here
• This PR's merge unblocks the cross-repo CI on livetemplate#429 (which fails because the Test Examples against Core Changes job builds the current examples/main against the open Phase-5 livetemplate worktree)

Refs livetemplate#415 (broadcast-action-redesign proposal)
Refs livetemplate#429 (Phase 5 livetemplate-side PR)

🤖 Generated with Claude Code

[claude]

Code Review

Overview: This PR migrates all examples from the deprecated ctx.BroadcastAction() API to the new ctx.Subscribe(ctx.SelfTopic()) + ctx.Publish(ctx.SelfTopic(), ...) pattern, and bumps the livetemplate pin from v0.9.0 → v0.9.2. The migration is mechanically consistent and the PR description is thorough.

---

⚠️ Behavioral Change: Partial Mutation on Publish Failure

This is the main concern. The old BroadcastAction was fire-and-forget (no return value). The new Publish returns an error, and every call site now propagates that error, which changes the failure semantics:

// In todos/controller.go (Add, Toggle, ConfirmDeleteConfirm, ClearCompleted):
// --- DB write already executed above ---
if err := ctx.Publish(ctx.SelfTopic(), "RefreshTodos", nil); err != nil {
    return state, err  // DB mutation committed; framework sees an error
}

If Publish fails after the DB write (or in-memory mutation in chat/shared-notepad) has already succeeded, returning the error may cause the framework to treat the whole action as failed — potentially not applying the returned state — while the underlying data store is already modified. This is a regression from the old behavior where the primary action always succeeded regardless of broadcast outcome.

Multi-tab sync is best-effort by nature; a safer pattern is to log the error without propagating it:

if err := ctx.Publish(ctx.SelfTopic(), "RefreshTodos", nil); err != nil {
    // peer-fanout is best-effort; primary action already succeeded
    _ = err
}
return state, nil

If Publish is guaranteed not to fail under normal conditions (e.g., it's synchronous and in-process), please add a brief comment explaining that so reviewers and future maintainers know propagation is safe here.

---

Minor Issues

landing-demo/main.go — Decrement publishes even when no-op:
The Decrement guard only prevents the count from going below zero, but Publish is still called even when s.Count was already 0 and nothing changed. This is pre-existing behavior, not introduced here, but worth noting.

func (c *CounterController) Decrement(s CounterState, ctx *livetemplate.Context) (CounterState, error) {
    if s.Count > 0 {
        s.Count--
    }
    // Publish still fires even if count didn't change
    if err := ctx.Publish(ctx.SelfTopic(), "Decrement", nil); err != nil {

---

What Looks Good

• Consistent migration — all 11 BroadcastAction call sites are updated; the grep verification confirms zero stragglers.
• Mount additions are correct — landing-demo gets a new Mount that only subscribes (clean); the other three controllers correctly insert Subscribe at the top of their existing Mount before any state reads.
• Error handling is consistent — the pattern if err := ...; err != nil { return state, err } is applied uniformly.
• Test comment update — renaming BroadcastAction_Propagates_To_Peer_Tab → Publish_Propagates_To_Peer_Tab and updating the skip message is accurate and appropriate.
• go.sum updated — both hash lines for v0.9.2 are present.
• Module hash consistency — go.mod h1 for v0.9.2 (zF/mbhXxp5u...) differs from v0.9.0 (6vHNNpZLIY...) as expected for a new release.

---

Summary

The migration is correct and consistent. The one thing worth resolving before merge is whether propagating Publish errors is intentionally safe (in which case a comment explaining why would help), or whether the call sites should treat publish failures as best-effort to preserve the original success semantics of each action.

🤖 Generated with Claude Code

[adnaan]

Thanks for the careful read on the propagation question — this is a real semantic shift from the old BroadcastAction's fire-and-forget behavior and worth pinning down.

Decision: keep propagation. Rationale committed inline at chat/main.go Join (f73d1a4).

The errors that ctx.Publish can actually return are all programmer errors, not transient/network ones:

• Empty topic (only happens when ctx.SelfTopic() resolves to "" — a misconfigured custom Authenticator that returns both empty UserID and empty GroupID)
• Empty action name (caller bug)
• Wildcard pattern as Publish target (caller bug — * is Subscribe-only)
• Per-action publish cap (MaxPublishesPerAction = 100) exceeded (caller bug — runaway Publish loop)

Surfacing these loudly via return state, err is the right contract — log-and-swallow would hide a misconfigured Authenticator that silently breaks every multi-tab demo. The partial-mutation concern is real but cleaner-handled by either (a) restructuring the controller to Publish before the mutation, or (b) accepting that under correct configuration these errors don't fire. The recipe apps here keep their existing mutation-then-Publish order for clarity; the rationale comment at chat/main.go:80-84 covers every Publish site in the repo.

This matches the convention applied in livetemplate#429's own test controllers + e2e/docker/app/main.go, and in docs#24's recipe migrations. Same rationale documented at content/recipes/shared-notepad/_app/controller.go there.

The minor landing-demo/main.go no-op Publish observation is pre-existing as you noted; deliberately not addressed here to keep the diff focused on the migration mechanics.

[claude]

Code Review: feat(broadcast): migrate to Subscribe(SelfTopic())+Publish

Overview

Clean, mechanical migration from the removed ctx.BroadcastAction to the new ctx.Subscribe(ctx.SelfTopic()) + ctx.Publish(ctx.SelfTopic(), ...) peer-fanout pattern. 4 apps migrated consistently, livetemplate bumped v0.9.0 → v0.9.2, and the PR description gives excellent context on ordering/unblocking.

---

Strengths

• Complete migration — the zero-match BroadcastAction grep is a great sanity check to include.
• Consistent pattern — every app follows the same Subscribe in Mount, Publish in actions structure.
• New landing-demo Mount — minimal and correct; it only subscribes and returns, no unnecessary work.
• Error propagation — choosing to surface Publish errors rather than log-and-swallow is a reasonable call; the inline comment in chat/main.go explaining why is one of the clearer "non-obvious WHY" comments in the codebase.

---

Issues & Suggestions

1. Behavior change: mutated state returned alongside Publish error

In every action that mutates state before calling Publish, a Publish error causes the action to return the mutated state with an error:

// landing-demo/main.go
func (c *CounterController) Increment(s CounterState, ctx *livetemplate.Context) (CounterState, error) {
    s.Count++                                           // mutation happens first
    if err := ctx.Publish(ctx.SelfTopic(), "Increment", nil); err != nil {
        return s, err                                   // mutated state + error
    }
    return s, nil
}

The same pattern appears in chat, todos, and shared-notepad. The question is: does the framework apply the returned state when an error is also returned, or does it discard it?

• If the framework discards state on error: the current user's tab silently loses the increment, which is surprising UX for what the comment says are "programmer errors" (misconfigured Authenticator or per-action publish cap exceeded).
• If the framework applies state on error: the current tab updates but peer tabs don't — which may be the correct behaviour for programmer errors.

This is worth a one-line call-out in the PR or a link to the framework docs on error semantics, since the behaviour change from fire-and-forget is non-obvious to future readers.

2. Decrement publishes even on no-op

Pre-existing, but now more visible since Publish has observable failure modes:

func (c *CounterController) Decrement(s CounterState, ...) (CounterState, error) {
    if s.Count > 0 {
        s.Count--
    }
    // Always publishes "Decrement", even when count was already 0
    if err := ctx.Publish(ctx.SelfTopic(), "Decrement", nil); err != nil {
        return s, err
    }
    return s, nil
}

Not a blocker for this PR (not a regression), but could contribute to unnecessary peer-tab round-trips. Worth a follow-up issue if the publish cap is tight.

3. Subscribe error path not covered by any test

Mount now returns an error path that didn't exist before:

if err := ctx.Subscribe(ctx.SelfTopic()); err != nil {
    return state, err
}

As the comment notes, this is a programmer-error path, so it may not need a dedicated test. But since E2E tests are required to exercise every controller method (per CLAUDE.md), and Mount is a controller method, it would be worth confirming the CI requirement is satisfied. The existing tests that navigate to the page implicitly exercise Mount — just calling that out explicitly if anyone asks during review.

4. Minor: long comment block in chat/main.go

The 5-line comment before the Join Publish call is useful for explaining a non-obvious design decision about error semantics. However, it applies equally to every Publish call in the file (and in fact across all migrated apps). Consider either:

• Shortening it to 1–2 lines and referencing the error types, or
• Moving it to the package-level doc comment (or a doc comment on a helper) so it doesn't need to repeat across the three call sites.

Not a blocker.

---

Test Coverage

• Renaming BroadcastAction_Propagates_To_Peer_Tab → Publish_Propagates_To_Peer_Tab is correct.
• The skip rationale is still accurate and the tracking issue (landing-demo: re-enable Sync_Propagates_To_Peer_Tab e2e test #94) is referenced.
• No regression risk to existing test suites since the API surface of controller methods is unchanged.

---

Verdict

Approve with minor comments. The migration is correct, complete, and consistent. The one thing worth confirming before merge is the framework's state-on-error semantics for the mutated-state-plus-error return case (#1 above) — even a one-line note in the PR description would be enough.

[adnaan]

Thanks for the round-2 review. Quick walkthrough of the items, all addressable by reply per CLAUDE.md "decline with a PR reply, never silently":

Item 1 (state-on-error semantics): This was the R1 concern; rationale documented inline at chat/main.go:80-84 (commit f73d1a4) and in my earlier PR-reply. To the bot's specific question about framework semantics: livetemplate's mount.go does discard newState when the action returns an error (see the if actionErr != nil { setError(...) } else { connSt.state = newState } block — confirmed by the advisor in transcript). So under a Publish error, the current tab's increment is also discarded. This is intentional under the new model — the errors Publish can return are all programmer errors (misconfigured Authenticator / cap exceeded), so failing loudly on the originator side is the correct contract. The trade-off is that misconfigured demos break loudly instead of silently desyncing peer tabs.

Item 2 (Decrement no-op publish): Pre-existing, confirmed not a regression here. Will file a follow-up issue for tightening the guard.

Item 3 (Subscribe error path test coverage): Existing E2E tests implicitly exercise Mount (page load → Mount runs → Subscribe runs). The Subscribe error path itself is a programmer-error path; livetemplate's own topic_test.go covers the equivalent contract upstream. Not adding a dedicated examples test for it.

Item 4 (long comment in chat/main.go): Kept at 4 lines deliberately — it explains a non-obvious cross-call-site contract that applies to every Publish site in the repo (per the comment's own last line: "Same pattern applies to every Publish call site in this file."). A package-level doc comment would be even further from the code it explains. Sticking with the placement.

No push needed — convergence by CLAUDE.md heuristic: R2 raised the same item 1 concern from R1, no new functional issues, items 2-4 are non-blocking style/pre-existing observations. Ready for merge pending maintainer signoff.