feat: lvt:error CustomEvent for topic_forbidden envelope (livetemplate#415, V14)

[adnaan]

Phase 4 of livetemplate's broadcast-action-redesign (#415). Adds the TS consumer for the topic-ACL error envelope the livetemplate server emits on an ACL-denied ctx.Subscribe in the WS-connect Mount.

Companion PR: livetemplate/livetemplate#427 — the Option B keep-open server change + V14 Tier-1 regression + canonical phase-4.md learnings.

Change

livetemplate-client.ts handleWebSocketPayload: new first-discriminator early-return branch — a typed local cast errorEnvelope to {type?, code?, topic?}, and on errorEnvelope.type === "error" dispatches a non-bubbling CustomEvent("lvt:error", { detail: { code, topic } }) on this.wrapperElement and returns before the diff path (analyzeStatics/updateDOM never see a treeless payload). Mirrors the existing lvt:updated idiom at the same call site.

V14 contract (byte-for-byte, three-tier agreement)

server:  {"type":"error","code":"topic_forbidden","topic":"<denied>"}
client:  CustomEvent("lvt:error", { detail: { code, topic } }) on wrapper

Asserted in three tiers: livetemplate topic_test.go (Tier 1 server), tests/topic-error-envelope.test.ts here (client logic leg), and the lvt chromedp e2e (Tier 2 user-visible). The server keeps the socket OPEN after emitting (livetemplate Phase 4 keep-open), so no disconnect to handle here.

Name overlap (non-functional, documented inline)

state/form-lifecycle-manager.ts also dispatches an lvt:error event — but on the <form> element with a ResponseMetadata detail. These are two distinct, non-bubbling events disambiguated by target (wrapper vs <form>) and detail shape — they do not collide at any listener. Pinned per spec; livetemplate's Phase 6 docs rewrite will distinguish.

Test (tests/topic-error-envelope.test.ts, new)

4 jest tests gating V14's logic leg:

1. Exact { code, topic } detail dispatched on the wrapper.
2. Target-specific: a bubble-phase document listener does not observe the wrapper-dispatched event (proving bubbles:false), while a direct listener on the wrapper does. (Round 3: corrected — earlier wording mischaracterized the test as a capture-phase assertion.)
3. Diff path NOT entered (updateDOM spied: not called; no lvt:updated; DOM untouched).
4. Over-match guard: a normal UpdateResponse still flows to updateDOM and does not fire lvt:error.

Gate (pre-commit ran on this commit)

npm test: 29 suites, 551 tests, 100% pass. npm run build clean (produces dist/livetemplate-client.browser.js the cross-repo lvt e2e consumes).

Cross-repo coordination

Companion PRs in release order (this + livetemplate#427 are wire-independent; lvt e2e gates last):

• livetemplate#427 — server keep-open change + V14 Tier-1 + canonical learnings.
• lvt (follow-up) — V14 Tier-2 chromedp e2e + committed go.mod replace (Phase-5-resolved cross-repo dependency artifact).

🤖 Generated with Claude Code

[claude]

Review

Critical: node_modules symlink must not be committed

The diff includes a symlink pointing to an absolute path on the developer machine (/home/adnaan/...) that was accidentally committed. This will break every other checkout and CI environment. Before merging: (1) git rm node_modules, (2) ensure node_modules is in .gitignore, (3) amend/force-push to remove from history.

---

handleWebSocketPayload — over-broad type discriminator

The errorEnvelope.type === 'error' check will intercept any future server message with type: 'error', not just topic-ACL envelopes. If the server ever emits other error shapes (rate limits, auth failures, etc.) through the same socket, they would be silently short-circuited and surfaced only as lvt:error on the wrapper with no diff path processing. Consider being more specific by also checking errorEnvelope.code, or add a comment locking the assumption that only topic-ACL errors use this envelope shape on this socket.

---

Rest of the change looks good. The into-view one-shot guard via data-lvt-iv-done is clean, the non-bubbling CustomEvent correctly avoids collision with the form-level lvt:error, and the test coverage across all three scenarios is thorough.

[claude]

Review — PR #121

livetemplate-client.ts — error envelope handling

Silent drop when wrapperElement is null. this.wrapperElement?.dispatchEvent(...) silently swallows the error if the wrapper is not set yet. An error arriving before the element is wired up would be lost with no log or throw. At minimum, add a console.warn so it is visible in production.

Over-broad type match. The guard errorEnvelope.type === 'error' will intercept any future payload shape that carries type: 'error', not just topic-ACL envelopes. Consider narrowing with an additional code check (e.g. ensuring code is present and a string) so other error-typed payloads are not accidentally short-circuited before the diff path.

dom/directives.ts — into-view scroll

The one-shot guard (data-lvt-iv-done) is correct. The behavior of re-scrolling when the server replaces the element entirely (new node = no attribute) versus patching in place appears intentional and matches the comment — just worth confirming that DOM diffing replaces rather than patches when the scroll target changes identity.

Tests

Good coverage of the happy path, behavior flag, and one-shot semantics. The non-bubbling assertion for lvt:error is particularly useful given the potential for confusion with the form-level event.

Summary

Two real issues worth addressing before merge: the silent error-swallow and the over-broad type match. Everything else looks solid.

[claude]

Review

Overall the implementation is clean and the test coverage is solid. Two things worth flagging before merge:

1. data-lvt-iv-done is invisible to the server — the "re-scroll to same element" path may be broken

The one-shot guard (htmlElement.dataset.lvtIvDone = "1") is set client-side, which means the server/template has no direct way to clear it. The comment says the directive fires again on "new element or new value (e.g. jumping to a different comment)", but:

• The attribute value is always the literal string "into-view" — there's no second value to change to.
• If the same DOM node is reused for a different comment (server sends the same element with updated content), the guard won't reset unless the DOM diff explicitly removes the data-lvt-iv-done attribute from that node.

Does livetemplate's diff strip client-added data-* attributes it doesn't recognize, or preserve them? If it preserves them, the re-scroll case silently breaks. Worth a quick integration test or a clarifying comment about the intended reset mechanism.

2. PR description misstates the bubbling test

"capture-phase document listener observes it (capture traverses regardless of bubbles:false)"

The actual test uses a bubble-phase listener (document.addEventListener("lvt:error", onDocument) — no third argument), which correctly doesn't observe the event. The behavior is right, but the PR description implies a capture-phase assertion that isn't there. Minor, but could confuse anyone reading this later.

---

The error envelope short-circuit (type === "error" before the diff path) and the malformed-code guard are well-reasoned. No concerns there. LGTM pending the data-lvt-iv-done lifetime question.

[adnaan]

Round-3 review responses:

1. data-lvt-iv-done re-scroll concern: out of PR scope. The into-view directive shipped in client commit b00ef39 feat: lvt-fx:scroll="into-view" directive for jump-to-element which is already on main (merged before this PR was opened). The PR diff for this V14 PR vs main does NOT include dom/directives.ts — git diff main...HEAD --name-only shows only livetemplate-client.ts, tests/topic-error-envelope.test.ts, and .gitignore. The directive bot caught is genuine, but it's a separate concern that needs a separate PR against main, not a Phase-4 V14 follow-up. Flagging it as a follow-up issue is reasonable; addressing it here would conflate two unrelated changes.

2. PR description misstates the bubble test: ✅ fixed in the PR body — the test uses a bubble-phase document listener and asserts non-observation (proving bubbles:false), not capture-phase observation. Description updated.

No other functional items raised in round 3.