Skip to content

refactor: simplify web event guard#82

Open
ALX99 wants to merge 1 commit into
masterfrom
refactor/simplify-web-event-guard
Open

refactor: simplify web event guard#82
ALX99 wants to merge 1 commit into
masterfrom
refactor/simplify-web-event-guard

Conversation

@ALX99

@ALX99 ALX99 commented Jul 6, 2026

Copy link
Copy Markdown
Owner

Simplification

Reuse the existing isServerEvent() protocol guard in the pi-web frontend bridge instead of repeating the same server-event validation locally with isObject(msg) && isServerEventType(msg.type) plus a manual cast.

Why the current design is overcomplicated

pi-web/src/protocol.ts already owns and tests the server-event type guard used at the WebSocket protocol boundary. pi-web/src/web/bridge.ts had a second, ad hoc validation path for the same event shape, which required importing the lower-level event-type predicate and then casting the message back to ServerEvent after checking it.

That duplicated a protocol responsibility in the UI bridge and created two places to reason about what counts as a valid server event.

Behavioral contract

The frontend WebSocket bridge must keep the same behavior:

  • JSON parse failures are ignored.
  • { type: "response", ok: ... } messages are still dispatched as responses first.
  • Successful command responses without data still refresh state and sessions.
  • SDK/server events with known event types are still dispatched as event reducer actions.
  • session_start events still trigger a session-list refresh.
  • Unknown event types, commands, arrays, nulls, and non-object messages are ignored.
  • WebSocket connection, reconnect, command send, toast, model, thinking-level, and session behavior are unchanged.

No wire format, public command, response shape, reducer behavior, persistence, logging, metrics, or user-visible behavior changes.

Before and after

Before:

  • pi-web/src/web/bridge.ts imported isServerEventType directly from shared/wire.ts.
  • The message handler checked isObject(msg) && isServerEventType(msg.type).
  • It then cast msg as ServerEvent before dispatching the event.

After:

  • pi-web/src/web/bridge.ts imports isServerEvent from ../protocol.ts.
  • The message handler checks isServerEvent(msg) directly.
  • The successful guard narrows msg to the event type without a local cast.

Specific evidence:

  • Removed a duplicate server-event validation expression from the frontend bridge.
  • Removed the frontend bridge's direct dependency on the lower-level isServerEventType helper.
  • Removed the local ServerEvent import and manual cast from the message handler.
  • Reused the existing protocol guard already covered by pi-web/tests/protocol.test.ts.

Validation

Static validation performed through repository inspection:

  • Confirmed isServerEvent() is implemented in pi-web/src/protocol.ts as isObject(value) && isServerEventType(value.type).
  • Confirmed pi-web/tests/protocol.test.ts covers accepted SDK-shaped events and rejected commands/unknowns/nulls.
  • Confirmed the frontend bridge's previous condition was equivalent to the existing protocol guard for server-event handling.
  • Confirmed response handling still runs before event handling.

Commands not run in this environment:

  • cd pi-web && npm run build && npm run typecheck && npm test

This PR is intentionally narrow because local command execution was unavailable here.

Risk assessment

Risk is low because this replaces a local validation expression with the existing protocol-level guard that performs the same object/type check and is already tested. The response path remains first, so response messages are not reclassified. The event dispatch and session_start refresh behavior are unchanged.

Behavior-sensitive areas examined:

  • response-vs-event classification order,
  • accepted server event type list,
  • reducer action shape,
  • session refresh on session_start,
  • ignored malformed messages.

Scope

Intentionally not simplified:

  • isResponse() remains local because there is no existing shared response guard with identical behavior.
  • isObject() remains local because isResponse() still needs it.
  • Broader protocol consolidation was avoided to keep the review small and behavior-preserving.
  • Existing open subagent simplification work was not touched to avoid overlap.

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