Skip to content

Share the SSE UiEvent union between server and web - #1016

Merged
selfcontained merged 2 commits into
mainfrom
agt_996ac4b15a6b/job-debt-collector-ec063b34
Aug 28, 2026
Merged

Share the SSE UiEvent union between server and web#1016
selfcontained merged 2 commits into
mainfrom
agt_996ac4b15a6b/job-debt-collector-ec063b34

Conversation

@selfcontained

Copy link
Copy Markdown
Owner

What

apps/server/src/server/ui-events.ts and apps/web/src/hooks/use-sse.ts each declared the same ~21-member UiEvent discriminated union, restated member for member. This was the largest remaining wire-type duplication in the repo (backlog entry from 2026-08-20).

Seventeen of the twenty-one members are now declared once, in a new packages/shared/src/ui-event-types.ts, as SharedUiEvent. Both sides compose it with the handful of members whose payloads genuinely differ.

Also moved into packages/shared: TerminalCopyMode, TerminalUiState, and InjectionHoldState — three tiny wire payloads that two of the shared members reference and that were themselves duplicated verbatim on both sides. apps/server/src/terminal/copy-mode-observer.ts, apps/server/src/terminal/injection-coordinator.ts, and apps/web/src/components/app/types.ts re-export them from @dispatch/shared, so every existing importer is untouched (the #863/#976 mechanic).

Types only. No runtime changepackages/shared is types-only by charter, and import type erases.

+56 / −131 across 7 files.

Why it's tech debt

Two hand-maintained copies of one wire contract, with no mechanism keeping them in sync. The drift was already visible: web's review.updated and review_feedback.updated were missing the reviewId/status/feedbackItemId fields the server has always published (verified at every publish site in mcp-review-handlers.ts and routes/reviews.ts). Web's handler never read them, so nothing broke — but that's luck, not a guarantee. Adopting the shared members closes that gap.

Deliberate exclusions — four members stay declared per side

Each is documented in the SharedUiEvent doc comment, and each is a near-miss that should not be consolidated in this PR:

  1. snapshot / agent.upsert — server publishes AgentRecord; web models the same rows with a deliberately lenient Agent (server-only columns like simulatorUdid/cliSessionId/launchedByAgentId dropped, most fields optional, plus the hasStream flag every publish site attaches via withStreamFlag). Converging these means changing Agent, which is load-bearing for a large number of web test fixtures. Queued as its own backlog item.
  2. agent.diff_state_changed — web's DiffStats is Omit<ServerDiffStats, "excludingTests"> & Partial<Pick<...>>. That divergence is deliberate and already documented in components/app/types.ts (an older server can be pushing stats to a newer bundle).
  3. release.cached_info_changedReleaseInfoSnapshot is declared in apps/server/src/release-info.ts, a runtime module that pulls in pg. It can't move to packages/shared as-is, and web already type-imports it directly (Import ReleaseInfoSnapshot from the server instead of restating it #985).

Applying the #989 route-is-the-wire-type rule: for every member I moved, I read the publish sites rather than trusting either declaration.

Validation

  • pnpm run check — green across shared / server / web / site / browser-extension / scripts
  • pnpm run test — server 171 files passed (1 skipped), web 95 files / 1418 tests, browser-extension 9 files
  • pnpm run finalize:web — type check + production build green
  • pnpm run test:e2e — 184 passed, 12 skipped (live-terminal suite, skipped by default)

Queued next

The AgentRecord ⇄ web Agent divergence from exclusion (1) — that's the real remaining finding here, and it deserves its own run.

selfcontained and others added 2 commits August 28, 2026 03:10
apps/server/src/server/ui-events.ts and apps/web/src/hooks/use-sse.ts each
declared the same ~21-member `UiEvent` discriminated union, restated member
for member. Seventeen of those members were byte-identical modulo two web
copies that had simply dropped fields the server sends.

The seventeen now live in packages/shared/src/ui-event-types.ts as
`SharedUiEvent`, alongside the three small wire payloads two of them need
(TerminalCopyMode, TerminalUiState, InjectionHoldState), which were
themselves duplicated verbatim on both sides. The server's
copy-mode-observer.ts / injection-coordinator.ts and web's
components/app/types.ts re-export those from @dispatch/shared, so every
existing importer is untouched.

Four members stay declared per side because their payloads genuinely
differ; the reason for each is recorded on `SharedUiEvent`.

Types only, no runtime change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Un6ifRaaDB6W5zHK1PTKsn
Review feedback on #1016: the header claimed SharedUiEvent held only members
"identical on both sides", which was inaccurate — `review.updated` and
`review_feedback.updated` had a narrower web copy that omitted fields.

State the actual reason those two are safe to share, and why it does not
apply to the version-skew exclusions listed below it. Comment only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Un6ifRaaDB6W5zHK1PTKsn
@selfcontained
selfcontained merged commit 5cfcaec into main Aug 28, 2026
1 check passed
@selfcontained
selfcontained deleted the agt_996ac4b15a6b/job-debt-collector-ec063b34 branch August 28, 2026 09:22
selfcontained added a commit that referenced this pull request Sep 3, 2026
* Derive web's Agent type from the server's AgentRecord

apps/web/src/components/app/types.ts hand-restated the agent row that
apps/server/src/agents/types.ts already declares as AgentRecord — the last
big server<->web wire-type gap left after the UiEvent consolidation (#1016).
The web copy omitted six columns the server has always sent (simulatorUdid,
archiveCleanupMode, gitContextStale, gitContextUpdatedAt, launchedByAgentId,
cliSessionId), typed `type` as a bare string, and hand-wrote
reviewAgentType's member list.

Agent is now `Omit<AgentRecord, LenientAgentField> &
Partial<Pick<AgentRecord, LenientAgentField>> & { hasStream?: boolean }` —
the same derivation idiom the file already uses for DiffStats. AgentStatus,
AgentPin and PinShortcutVariant are re-exported from the server module
instead of restated, so every existing importer is untouched.

Five test fixtures gained `metadata: {}` on their latestEvent literals: the
producer (agents/manager.ts:1429) COALESCEs the column to '{}'::jsonb, so
the wire never omits that field.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* Move the AgentRecord wire contract into @dispatch/shared

Follow-up to the first commit, per architecture-review 1065: web now takes
`AgentRecord` from `@dispatch/shared` rather than reaching into
`apps/server/src/agents/types.ts`, which reversed the intended dependency
direction and coupled the web build to server-internal file layout.

New shared modules:
  - packages/shared/src/agent-types.ts  — AGENT_TYPES / CLI_AGENT_TYPES
  - packages/shared/src/pin-types.ts    — VALID_PIN_TYPES /
                                          VALID_PIN_SHORTCUT_VARIANTS
  - packages/shared/src/agent-record.ts — AgentRecord and its member unions

The index.ts charter already permits "plain constants that both sides
genuinely have to agree on" (DIFF_IMAGE_MAX_BYTES is the precedent), so the
runtime tables were not a blocker as the first commit's comment claimed.
Validation stays server-side: apps/server/src/shared/agent-types.ts keeps
isAgentType/isCliAgentType/sanitizeEnabledAgentTypes and the server-only
PLUGIN_AGENT_TYPES table; apps/server/src/pins.ts keeps every pin validator.
Both re-export what they moved, so no server importer changed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant