refactor: extract an internal session broker boundary

[benvinegar]

Summary

• extract the generic broker infrastructure into src/session-broker/*
• move Hunk-specific session projections, wire parsing, and command bridge code into src/hunk-session/*
• generalize broker registration/snapshot envelopes and command transport so the broker routes opaque app commands
• narrow the session CLI layer to broker orchestration plus Hunk-specific formatting/helpers
• split test coverage so broker-core behavior and Hunk adapter behavior are exercised as separate layers

Testing

• bun run typecheck
• bun test

This PR description was generated by Pi using OpenAI GPT-5

[greptile-apps]

Greptile Summary

This PR extracts a generic session broker namespace (src/session-broker/*) from the Hunk-specific adapter code (src/hunk-session/*), establishes cleaner wire and type layers, and splits test coverage across the two layers. The structural and test separation is well-executed. However, SessionBrokerState still imports Hunk-specific parsers and projections from hunk-session/, crossing the boundary the PR is trying to establish.

Confidence Score: 5/5

Safe to merge — all findings are P2 style/architecture suggestions with no functional impact.

No P0 or P1 issues found. The refactor is functionally correct: wire parsing, command dispatch, snapshot lifecycle, and test coverage all work as intended. The P2 comments flag that SessionBrokerState and SessionBrokerClient still import Hunk-specific types despite living in the generic broker namespace — a clean-up for a follow-up PR rather than a blocker here.

src/session-broker/brokerState.ts warrants the most attention: it drives all the broker/Hunk coupling concerns called out in the review.

Important Files Changed

Filename	Overview
src/session-broker/brokerState.ts	Contains the central session registry and command dispatch — functionally correct, but still imports Hunk-specific parsers and projections from hunk-session/, and the dispatchCommand type parameters are constrained to Hunk types despite the generic-transport intent.
src/session-broker/brokerWire.ts	Cleanly generalized: envelope parsers are now generic callbacks (parseSessionRegistrationEnvelope, parseSessionSnapshotEnvelope), and brokerWireParsers is a well-contained utility export used by hunk-session/wire.ts.
src/session-broker/brokerClient.ts	Broker client logic is sound; SessionAppBridge interface and constructor types are still Hunk-specific (HunkSessionRegistration, HunkSessionServerMessage) despite the generic broker namespace.
src/session-broker/brokerServer.ts	Correctly routes WebSocket broker traffic; imports several Hunk result types only to cast an opaque result field — those imports could be replaced with unknown.
src/hunk-session/bridge.ts	Clean adapter: createHunkSessionBridge dispatches all six Hunk commands through named handlers with no broker concerns leaking in.
src/hunk-session/wire.ts	Hunk-specific wire parsers correctly delegate to generic broker envelope helpers; liveCommentCount is intentionally re-derived from filtered array length (documented by tests).
src/hunk-session/projections.ts	Well-scoped Hunk projection functions; correctly moved from the daemon layer into the hunk-session adapter namespace.
src/hunk-session/types.ts	Hunk-specific types are properly scoped here; HunkSessionClientMessage and HunkSessionServerMessage correctly specialize the generic broker types.
src/session/commands.ts	Session CLI command routing correctly delegates formatting/helpers to hunk-session/cli.ts and broker orchestration to session-broker; separation is clean.
test/helpers/session-daemon-fixtures.ts	Test fixtures updated to reflect new broker envelope structure; private summarizeReviewFile helper duplicates the production version but is unexported so the duplication is contained.

Sequence Diagram

sequenceDiagram
    participant App as Hunk App (UI)
    participant Reg as hunk-session/sessionRegistration
    participant Client as session-broker/brokerClient
    participant Server as session-broker/brokerServer
    participant State as session-broker/brokerState
    participant Wire as hunk-session/wire
    participant Bridge as hunk-session/bridge

    App->>Reg: createSessionRegistration(bootstrap)
    Reg-->>App: HunkSessionRegistration
    App->>Client: new SessionBrokerClient(registration, snapshot)
    App->>Client: start()
    Client->>Server: WebSocket connect + register
    Server->>Wire: parseSessionRegistration(payload)
    Wire-->>Server: HunkSessionRegistration
    Server->>State: registerSession(socket, reg, snapshot)

    Note over Client,Server: Snapshot updates
    App->>Client: updateSnapshot(snapshot)
    Client->>Server: snapshot message
    Server->>State: updateSnapshot(sessionId, snapshot)

    Note over Server,Bridge: Inbound agent command
    Server->>Client: command message
    Client->>Bridge: dispatchCommand(message)
    Bridge->>App: handler call
    App-->>Bridge: HunkSessionCommandResult
    Bridge-->>Client: result
    Client->>Server: command-result message
    Server->>State: handleCommandResult()
    State-->>State: resolve pending promise

Loading

Comments Outside Diff (1)

1. src/session-broker/brokerServer.ts, line 455-467 (link)

   Broker server imports Hunk result types to pass them through opaquely

   brokerServer.ts imports HunkSessionCommandResult (and five specific result types) only to pass parsed.result through as HunkSessionCommandResult | undefined to handleCommandResult. Since the broker doesn't inspect the result payload — it just forwards it to the pending promise — unknown would be sufficient here and would keep the server free of Hunk dependencies.

   // Instead of:
   result: parsed.result as HunkSessionCommandResult | undefined,
   
   // The server can stay opaque:
   result: parsed.result as unknown,

   The six named result-type imports at lines 8–15 would then become unused.

   Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

_{Reviews (1): Last reviewed commit: "test(session-broker): separate broker an..." | Re-trigger Greptile}