Refactor web extensions onto a contribution kernel

[ashwin-pc]

Summary

Implements PR 1 from #82 as a behavior-preserving kernel consolidation.

• replaces the four per-runtime browser-surface registries with one canonical contribution registry
• normalizes entries to a versioned discriminated union with explicit slot and kind
• centralizes legacy serialization and broadcasts behind contribution policies
• routes existing setFooter, setHeaderAction, setArtifactAction, and setGitTab wrappers and invokes through the registry
• preserves all existing extension APIs, snapshot fields, wire events, and HTTP endpoints
• reduces the touched core implementation by 87 lines (123 additions, 210 deletions)

This intentionally does not expose contribute() or change transport yet; those remain PR 2/3 work described in the RFC.

Tests

Added a registry contract test covering:

• the same key registered in multiple slots
• unchanged legacy serialization
• rendered invocation through the canonical registry
• clearing one slot without affecting contributions in other slots
• unchanged legacy delta broadcasts

Validation:

• npm run typecheck
• npm run test:unit — 324 passed
• npm test — build, typecheck, unit, auth E2E, and desktop/tablet/mobile E2E passed
  • one unrelated mobile stop-button interaction was flaky because the launcher intercepted the first click; it passed on automatic retry

Closes the first implementation phase of #82.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d0024d9f60

ℹ️ 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 1 acceptance criteria from #82, with independent verification on the PR head (d0024d9): typecheck clean, extensions/web-ui-settings/session-service suites pass (48/48), core LOC math confirmed (123+/210− in webUi.ts, −87 net). The two classic consolidation regressions are both avoided and I verified them directly:

• Per-slot ordering is preserved. The shared Map + per-slot filtering keeps registration order within a slot, and re-set of an existing id keeps its original position — matching legacy per-surface Map semantics exactly. This is user-visible (footer stacking), so worth a regression test eventually.
• Key namespacing by (slot, key) is correct and the contract test covers same-key-across-three-slots plus isolated clearing — exactly the collision class a merged registry invites.

One genuine behavior deviation to fix before merge:

invokeArtifactAction relaxed the match guards from Array.isArray(action.kinds) && action.kinds.length && … to action.kinds?.length && … (same for extensions). For well-typed extensions this is identical, but for malformed input the behavior flips: a string kinds: "markdown" was previously ignored (filter skipped → action matches); now "markdown".includes(kind) evaluates — wrong result — and a string extensions throws TypeError (500) instead of being ignored. Marginal, but this PR's whole claim is behavioral equivalence; recommend restoring the Array.isArray guards.

Minor (non-blocking):

1. path.slice(15) replaced path.slice("/api/artifacts/".length) — correct today, but a magic number in a path-security check. Restore the self-documenting form.
2. setContribution(value, contribution, slot, keyValue) re-cleans a key the callers already cleaned, and the registry id is derived from keyValue while the entry embeds contribution.key — two sources of truth that only agree because cleanContributionKey is idempotent (verified, but nothing enforces it). Suggest cleaning once in setContribution and deriving the embedded key from it, or taking the pre-cleaned key only.
3. Some cosmetic line-joining inside invokeGitTab/invokeArtifactAction is mixed into the refactor. In a behavior-preserving PR, cosmetic churn is where equivalence bugs hide (finding Proposal: optional local voice input via pi-web-voice sidecar package #1 and the slice(15) both came from those hunks). Worth keeping future PR 1-style diffs mechanical.
4. version: 1 is currently write-only — fine for PR 1, but PR 2 should make the transport actually branch on it so it doesn't fossilize as decoration.
5. Policy objects own entries/event/field but normalization and invoke budgets (200_000/500_000) still live in wrappers and the three invoke* functions. That's the right scope for PR 1 — flagging it so PR 2's definition of done includes moving normalization/context-parsing/budgets into the policies per the RFC: unified extension contribution kernel (contributions × slots × kinds) #82 contract.

Test suggestions for follow-up: ordering preservation on re-set; invalid-key no-op on set/clear; a malformed-kinds case pinning whichever guard behavior we decide is canonical.

Verdict: direction and execution match the RFC — registry as discriminated union with explicit slot/kind/version, policies as executable objects, legacy wire fully adapter-preserved, and the net-LOC criterion honestly met. Fix the Array.isArray guard regression, ideally the slice(15), and this is ready to merge as PR 1.

[ashwin-pc]

Addressed the review feedback in ae7f653:

• restored the legacy Array.isArray guards for artifact kinds and extensions
• restored "/api/artifacts/".length in the artifact path validation
• changed setContribution to clean the key once and construct the normalized contribution from that canonical key, removing the two-source-of-truth concern
• restored the original multiline formatting in the invoke paths to keep the behavioral diff mechanical
• added regression coverage for per-slot ordering after re-set
• added invalid-key set/clear no-op coverage
• added malformed non-array artifact-filter coverage

The version transport dispatch and moving normalization/context parsing/budgets fully into policies remain explicitly scoped to PR 2, as noted in the review and RFC.

Validation on the updated head:

• npm run typecheck ✅
• npm run test:unit ✅ — 324 tests

[ashwin-pc]

Re-verified on ae7f653 (typecheck + the three affected suites, all green). All findings addressed:

• Guard regression: Array.isArray restored, and — better than a plain revert — the new malformed-filters test pins fail-open as the current canonical behavior, so when PR 2's policy normalization deliberately changes it to registration-time rejection, that test forces the decision to be explicit rather than accidental. Exactly the right way to close the loop.
• setContribution single-source-of-truth: the create(cleanKey) callback shape is cleaner than what I suggested — the canonical key now flows outward from one cleaning site by construction, instead of relying on idempotency.
• "/api/artifacts/".length restored; cosmetic churn reverted — the behavioral diff is mechanical again.
• New coverage (ordering across re-set, invalid-key no-op incl. no spurious broadcasts, malformed filters) matches the follow-up list from the review.

No remaining concerns. This meets the PR 1 contract from #82: behavior-preserving by inspection and by test, net core LOC still negative, versioned discriminated-union registry with policies in place for PR 2 to build on. Ready to merge.