Unify contribution transport and add panel/FAB slots

[ashwin-pc]

Summary

Implements PR 2 from #82 on top of the contribution kernel landed in #83.

Unified transport

• session snapshots now carry one versioned webContributions descriptor array
• all contribution changes emit one session-scoped web_contributions_changed event
• interactive hosts invoke through POST /api/web-contributions/invoke
• the shared event envelope carries action, payload, fields, and slot-provided context
• frontend hosts ignore descriptors with unknown versions or slots
• existing HTTP invoke endpoints remain as compatibility aliases during migration
• Git-tab and panel HTML accept the shared data-web-action / data-web-payload grammar while retaining legacy attribute aliases

New slots

• adds a rendered panel slot hosted in the shared right panel
• adds a static fab launcher slot
• panels have no implicit launcher; FAB entries explicitly reference a panel with opens
• header actions can open registered panels through a typed open-panel effect
• FAB geometry adapts to contributed launcher entries

Scope

The recovered notepad prototype was used as implementation evidence, but the 631-line exemplar is deliberately not included. It belongs with public contribute() and invalidation work rather than obscuring this transport/host review.

Assets, invalidation, public contribute(), and webviews remain out of scope.

Compatibility

The extension-facing setFooter, setHeaderAction, setArtifactAction, and setGitTab APIs remain available. This adds setPanel and setFabAction. Legacy invoke routes and legacy interactive HTML attributes remain accepted.

Tests

Added or updated coverage for:

• canonical versioned descriptors for every existing slot
• unified delta events
• unified rendered invocation
• panel/FAB decoupling
• panel form serialization through the shared event envelope
• typed header-action panel opening
• invalid panel references
• snapshots and background session projections using webContributions
• panel/FAB live browser behavior
• existing footer, header action, artifact action, and Git-tab hosts over unified transport

Validation:

• npm run typecheck ✅
• npm run build ✅
• npm run test:unit ✅ — 325 tests
• targeted contribution E2E ✅ — 13 tests
• npm test ✅ — build, auth, unit, desktop, tablet, and mobile suites
  • the existing attachment lifecycle diagnostic test encountered upload races on tablet/mobile and passed on automatic retry; contribution tests passed directly

Part of #82.

Snapshot migration decision

Legacy snapshot fields (webFooters, webHeaderActions, etc.) and their per-slot delta events are intentionally replaced rather than dual-emitted. The browser and server ship as one application, websocket hello performs a fresh snapshot, and the service worker/reload path keeps the stale-open-tab window short. HTTP invoke endpoints and interactive data-* attributes remain aliased because those are independently exercised compatibility boundaries; carrying two parallel state protocols would undermine the consolidation and double reducer complexity.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 385b663e51

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[ashwin-pc]

Reviewed against the PR 2 contract from #82, with independent verification on 385b663: typecheck ✅, full unit suite ✅ (325), production build ✅, targeted e2e ✅ (web-panel, web-footer, git — 11/11).

Contract conformance — all the load-bearing pieces are right:

• Unified webContributions descriptor array with client-side version === 1 + slot filtering (the RFC's graceful-degradation rule, actually implemented, with e2e coverage of unknown-slot/version tolerance).
• One invoke endpoint with the shared Event envelope; legacy endpoints and data-* attributes kept as aliases exactly as contracted (data-web-action primary; git host accepts both selectors).
• Effects as a typed list ({ type: "open-panel", key }), server-validated against the panel registry — the amended RFC: unified extension contribution kernel (contributions × slots × kinds) #82 contract, not the prototype's ad-hoc openPanel field.
• Panel/FAB decoupling preserved end-to-end (server drops fab descriptors with unresolvable opens; launcher re-validates; e2e asserts a launcher-less panel stays out of the FAB).
• Excluding the 631-line notepad exemplar was the right scoping call.

Three fixes requested (small, none architectural):

1. Key confusion in invokeContribution artifact routing — invokeArtifactAction(value, { key: input.key, ...context }) lets a client-supplied event.context.key override the routed key, so the invocation resolves against a different contribution than slot/key addressed. Authenticated-only, so low severity, but it's key confusion by construction. Fix: { ...context, key: input.key }.
2. Error→status mapping drift — two cases fall through to 500 that should be 4xx: Header action returned unknown panel "x" matches neither "returned no" nor "not found" on the unified route; and the legacy /api/web-header-action/invoke route still maps only the old message (returned no markdown) while the bridge now throws returned no result — a 400→500 regression on a route kept explicitly for compatibility. String-matching statuses is fragile; consider a typed error with status, or at minimum add both messages.
3. Unbounded field map in invokePanel — per-field 100 KB and per-array 100 items are capped, but the number of fields is bounded only by the 40 MB readBody ceiling. The prototype capped entries at 128; one .slice(0, 128) restores that.

One decision to make explicit in the PR description: legacy snapshot fields (webFooters …) and delta events (web_footer_changed …) are replaced, not aliased. In-contract (#82 promised endpoint + attribute aliases only), and frontend/server ship together — but the consequence is that a stale open tab against a restarted server shows no extension surfaces until reload. Given PR 1's review floated dual-emission during a window, this deserves a sentence stating it's deliberate and why (sw auto-reload + ws hello resnapshot make the window short).

Non-blocking notes: broadcastContributions has an unused _slot param — either emit per-slot deltas someday or drop the param; view-size budgets still live in the invoke paths rather than the policy objects — fine for PR 2, but that's the remaining piece of the "policies own budgets" contract item, worth tracking for PR 3.

Verdict: the transport consolidation is exactly what #82 specified, and the duplication payoff is visible — six slots now ride one snapshot field, one event, one endpoint, one grammar. Fix 1–3 and this is ready to merge.

[ashwin-pc]

Addressed all requested fixes in 2fcd96d:

• artifact context can no longer override the routed contribution key
• unified and legacy header-action routes classify invalid results/unknown panels as 400
• panel form maps are capped at 128 fields
• panel field values preserve trailing spaces and newlines while retaining control-character and size limits
• icon validation now checks own registry properties, with prototype-name regression coverage
• stale header-action responses are ignored after a session switch
• combined Markdown + open-panel responses now render the Markdown and apply the effect
• removed the unused slot argument from contribution broadcasts
• documented the deliberate replacement (not dual emission) of legacy snapshot fields/events in the PR description

Added regression coverage for key confusion, field-map limits, whitespace preservation, inherited icon names, and combined Markdown/effect behavior.

Validation:

• typecheck ✅
• focused unit tests ✅
• production build ✅
• panel E2E ✅

Policy-owned budgets remain tracked for PR 3 as noted in review.

[ashwin-pc]

Re-verified on 2fcd96d: typecheck ✅, full unit suite ✅ (now 327 incl. new icons/field-cap/key-confusion coverage), build ✅, targeted e2e ✅ (11/11 incl. the new header→panel flow).

All three requested fixes confirmed, each with a pinning test. Beyond that, this commit caught two real bugs I missed in both my prototype and my review — credit where due:

• isIconName prototype-chain false positives: value in iconNodes accepted "toString"/"__proto__", which would have handed Object.prototype members to lucide's createElement. That bug originated in my prototype branch and I carried it into review without noticing; the hasOwnProperty fix plus the registry test is exactly right.
• Trailing-whitespace loss in panel fields: routing field values through cleanFooterText silently trimEnd()-ed user input — a textarea ending in a newline would lose it on save. Also a prototype-inherited bug. The dedicated cleanFieldValue (control-char strip + cap, no trim) is the correct semantics for form data, and the regression test pins it.

The stale-session guard on header-action responses and the markdown+effect combination are both good hardening. One nano-nit on the latter, non-blocking: when markdown re-shows the popover after close(), activeKey and the button's active class stay cleared, so the next click re-invokes instead of toggling the popover closed. If anyone ever notices, restoring the active state after showPopover in that branch fixes it — fine as a follow-up or never.

With the wire-replacement decision now documented in the PR description, everything from the review is closed. Ready to merge. PR 3 (public contribute()/update() + invalidation + notepad exemplar migration) has a clean foundation.