refactor: route all state mutations through process() via system signals (phases 0-2)

[teng-lin]

Summary

Eliminates all remaining public mutator methods on SessionRuntime that bypassed process(). All session state mutations now flow through the reducer or post-reducer hooks inside handleSystemSignal(), centralizing the mutation boundary established in PR #130.

• Phase 0: Delete 3 dead methods (storePendingPermission, setBackendSessionId, setMessageHistory) — reducer already handled these
• Phase 1a: setAdapterName() → ADAPTER_NAME_SET signal (reducer patches adapterName + emits PERSIST_NOW)
• Phase 1b: addConsumer()/removeConsumer() → CONSUMER_CONNECTED/CONSUMER_DISCONNECTED signals with post-reducer handle hooks
• Phase 1d: enqueuePendingPassthrough() → PASSTHROUGH_ENQUEUED signal with post-reducer hook
• Phase 2a: attachBackendConnection() folded into BACKEND_CONNECTED payload — handle refs set before effects execute so drained pending messages reach the live backend
• Phase 2b: resetBackendConnectionState() folded into BACKEND_DISCONNECTED — reducer resets data fields, hook nulls handles
• Phase 2c: drainPendingMessages() → SEND_TO_BACKEND effects in BACKEND_CONNECTED; drainPendingPermissionIds() → BROADCAST_TO_PARTICIPANTS cancel effects in BACKEND_DISCONNECTED
• Phase 2d: seedSessionState() → SESSION_SEEDED signal + new RESOLVE_GIT_INFO effect type (gitTracker added to EffectExecutorDeps)

Post-review fixes applied:

• closeBackendConnection() now dispatches BACKEND_DISCONNECTED (fixes permission-cancellation gap during session teardown)
• executeEffects() has a per-effect try-catch so a failing effect logs an error and continues rather than aborting subsequent effects
• refreshGitInfo() wrapped in try-catch matching resolveGitInfo() — prevents deleted cwds from crashing the backend message loop
• trySendRawToBackend() uses explicit capability check instead of broad catch that misclassified errors
• Consolidated reduceSystemSignal() into single exhaustive switch; removed lifecycleForSignal helper
• Extracted applyUserMessageReducer() to eliminate duplicated reducer-call-then-IO pattern
• Deleted dead mapSetModelEffects and mapUserMessageEffects (unreachable code)
• Documentation: fixed routing table function names, added three-phase JSDoc to handleSystemSignal(), updated module comments

Net result: 11 imperative methods removed from SessionRuntime public API. -95 lines (563 additions, 711 deletions).

Test Plan

• pnpm typecheck — clean
• pnpm test — all 2900 tests pass
• New tests cover: RESOLVE_GIT_INFO effect, BACKEND_CONNECTED ordering guarantee (handle set before SEND_TO_BACKEND effects), BACKEND_DISCONNECTED lifecycle transition + adapterSupportsSlashPassthrough reset, SESSION_SEEDED empty-params edge case
• Verify no external callers remain for deleted methods: grep -rn "\.setAdapterName\|\.addConsumer\|\.removeConsumer\|\.attachBackendConnection\|\.resetBackendConnectionState\|\.drainPendingMessages\|\.drainPendingPermissionIds\|\.storePendingPermission\|\.setBackendSessionId\|\.setMessageHistory\|\.seedSessionState\|\.enqueuePendingPassthrough" src/ --include="*.ts" | grep -v "test\|\.d\.ts"

[gemini-code-assist]

Summary of Changes

Hello @teng-lin, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the core session management logic by enforcing a strict, signal-based approach for all state mutations. The change centralizes how session data is updated, moving away from direct imperative method calls to a more predictable and auditable system using reducers and system signals. This improves consistency, simplifies the SessionRuntime API, and enhances error handling during effect execution and git information resolution.

Highlights

• Centralized State Mutations: All session state mutations now flow through the reducer or post-reducer hooks inside handleSystemSignal(), eliminating public mutator methods on SessionRuntime that bypassed this centralized mutation boundary.
• Removed Imperative Methods: Eleven imperative methods were removed from the SessionRuntime public API, including storePendingPermission, setBackendSessionId, setMessageHistory, setAdapterName, addConsumer, removeConsumer, enqueuePendingPassthrough, attachBackendConnection, resetBackendConnectionState, drainPendingMessages, and drainPendingPermissionIds.
• System Signal Enhancements: New system signals (ADAPTER_NAME_SET, CONSUMER_CONNECTED, CONSUMER_DISCONNECTED, PASSTHROUGH_ENQUEUED, SESSION_SEEDED) were introduced to route these mutations through the process() method. BACKEND_CONNECTED and BACKEND_DISCONNECTED signals now carry more payload data and trigger effects for draining pending messages and cancelling permissions, respectively.
• Robust Effect Execution: The executeEffects() function now includes a per-effect try-catch block, ensuring that a single failing effect logs an error and allows subsequent effects to continue, preventing complete abortion of the effect chain.
• Git Info Resolution: A new RESOLVE_GIT_INFO effect type was added, triggered by the SESSION_SEEDED signal, to resolve git information for a session. The refreshGitInfo() and resolveGitInfo() methods now include try-catch blocks to prevent crashes from deleted current working directories.
• Improved Error Handling: The trySendRawToBackend() method now uses an explicit capability check instead of a broad catch block, preventing misclassification of network errors as 'unsupported' capabilities.

Changelog

• src/core/backend/backend-connector.adapter-selection.test.ts
  • Removed direct calls to attachBackendConnection, resetBackendConnectionState, drainPendingMessages, and drainPendingPermissionIds from mock runtime.
  • Updated routeSystemSignal mock to directly handle BACKEND_CONNECTED and BACKEND_DISCONNECTED signals.
  • Removed broadcaster from dependencies.
• src/core/backend/backend-connector.failure-injection.test.ts
  • Removed direct calls to attachBackendConnection, resetBackendConnectionState, drainPendingMessages, and drainPendingPermissionIds from mock runtime.
  • Updated routeSystemSignal mock to directly handle BACKEND_CONNECTED and BACKEND_DISCONNECTED signals.
  • Removed broadcaster from buildConnectorDeps.
• src/core/backend/backend-connector.lifecycle.test.ts
  • Removed deprecated session runtime methods from mock implementations.
  • Updated routeSystemSignal mock to handle BACKEND_CONNECTED and BACKEND_DISCONNECTED signals directly.
  • Removed broadcaster from dependencies.
  • Modified expect calls for routeSystemSignal to use expect.objectContaining for more robust testing.
  • Updated test descriptions to reflect signal-based handling of pending messages and permission cancellation.
• src/core/backend/backend-connector.test.ts
  • Removed deprecated attachBackendConnection, resetBackendConnectionState, drainPendingMessages, and drainPendingPermissionIds methods from mock runtime.
  • Removed broadcaster from dependencies.
• src/core/backend/backend-connector.ts
  • Removed ConsumerBroadcaster dependency.
  • Deleted private helper methods applyBackendConnectedState, applyBackendDisconnectedState, drainPendingMessagesQueue, drainPendingPermissionRequestIds, and cancelPendingPermissions.
  • Modified connectBackend to pass backendSession, backendAbort, supportsSlashPassthrough, and slashExecutor directly within the BACKEND_CONNECTED system signal.
  • Removed direct calls to state mutation and permission cancellation in disconnectBackend and startBackendConsumption error handling, relying instead on system signals.
• src/core/consumer/consumer-gateway.test.ts
  • Replaced mocks for addConsumer and removeConsumer with a mock for process to handle CONSUMER_CONNECTED and CONSUMER_DISCONNECTED system signals.
  • Updated test assertions to verify calls to mockRuntime.process with appropriate system signals.
• src/core/consumer/consumer-gateway.ts
  • Replaced direct calls to rt.removeConsumer and rt.addConsumer with rt.process calls dispatching CONSUMER_DISCONNECTED and CONSUMER_CONNECTED system signals.
• src/core/session-coordinator.ts
  • Replaced direct calls to removeConsumer and enqueuePendingPassthrough with process calls dispatching CONSUMER_DISCONNECTED and PASSTHROUGH_ENQUEUED system signals.
  • Removed broadcaster from BackendConnector dependencies.
  • Replaced direct calls to seedSessionState and setAdapterName with process calls dispatching SESSION_SEEDED and ADAPTER_NAME_SET system signals.
• src/core/session/effect-executor.test.ts
  • Added gitTracker to the makeDeps function.
  • Introduced a new test case to verify that the RESOLVE_GIT_INFO effect calls gitTracker.resolveGitInfo.
• src/core/session/effect-executor.ts
  • Updated JSDoc to reflect that effects are returned by sessionReducer.
  • Added gitTracker to the EffectExecutorDeps interface.
  • Implemented a try-catch block around each effect execution to ensure that a single failing effect does not abort subsequent effects, logging errors via emitEvent.
  • Added a case for RESOLVE_GIT_INFO effect type to call deps.gitTracker.resolveGitInfo.
• src/core/session/effect-mapper.test.ts
  • Removed test suites for mapInboundCommandEffects — user_message and mapSetModelEffects.
  • Updated comment for set_model command to indicate it is handled by the reducer.
• src/core/session/effect-mapper.ts
  • Updated module description to clarify that SYSTEM_SIGNAL effects are now produced inline in session-reducer.ts.
  • Removed SessionState import.
  • Deleted mapUserMessageEffects and mapSetModelEffects functions.
  • Modified mapInboundCommandEffects to remove the user_message case and update comments to reflect that commands handled explicitly by the reducer do not reach this mapper.
• src/core/session/effect-types.ts
  • Added a new effect type: RESOLVE_GIT_INFO for resolving git information.
• src/core/session/git-info-tracker.ts
  • Added a try-catch block around gitResolver.resolve to prevent crashes if the current working directory is deleted or inaccessible.
• src/core/session/session-event.ts
  • Expanded the BACKEND_CONNECTED SystemSignal type to include backendSession, backendAbort, supportsSlashPassthrough, and slashExecutor.
  • Added new SystemSignal types: ADAPTER_NAME_SET, PASSTHROUGH_ENQUEUED, and SESSION_SEEDED.
• src/core/session/session-reducer.ts
  • Updated JSDoc to clarify routing for BACKEND_MESSAGE, SYSTEM_SIGNAL, and INBOUND_COMMAND.
  • Consolidated reduceSystemSignal into a single exhaustive switch statement, handling data-patch, effect, and lifecycle signals directly.
  • Removed the lifecycleForSignal helper function.
  • Modified BACKEND_CONNECTED and BACKEND_DISCONNECTED cases to include effects for draining pending messages and cancelling permissions, respectively, and to update adapterSupportsSlashPassthrough.
  • Added cases for ADAPTER_NAME_SET and SESSION_SEEDED signals to update session data and generate effects.
  • Removed comments regarding sessionId injection for effects.
  • Updated logic for permissionMode in buildEffects to handle mode and permissionMode metadata fields.
• src/core/session/session-runtime.test.ts
  • Updated BACKEND_CONNECTED signal payload in tests to match the new type definition.
  • Replaced direct calls to setAdapterName, seedSessionState, addConsumer, removeConsumer, storePendingPermission, setBackendSessionId, setMessageHistory, and enqueuePendingPassthrough with process calls dispatching the corresponding system signals.
  • Removed tests for clearBackendConnection, drainPendingMessages, and drainPendingPermissionIds.
  • Added new test cases for SESSION_SEEDED with no parameters and to verify the ordering guarantee of BACKEND_CONNECTED (handle set before SEND_TO_BACKEND effects).
  • Updated trySendRawToBackend test to explicitly check for sendRaw method presence and propagate errors thrown by sendRaw.
• src/core/session/session-runtime.ts
  • Removed public mutator methods: setAdapterName, setBackendSessionId, setMessageHistory, seedSessionState, addConsumer, removeConsumer, clearBackendConnection, attachBackendConnection, resetBackendConnectionState, drainPendingMessages, drainPendingPermissionIds, storePendingPermission, and enqueuePendingPassthrough.
  • Modified closeAllConsumers to directly clear consumerSockets and consumerRateLimiters maps.
  • Updated closeBackendConnection to dispatch a BACKEND_DISCONNECTED system signal.
  • Modified trySendRawToBackend to explicitly check for the existence and type of the sendRaw method on backendSession and to propagate errors thrown by sendRaw.
  • Extracted shared logic for user message processing into a new private method applyUserMessageReducer.
  • Refactored handleSystemSignal to implement a three-phase execution: reducer first, then BACKEND_CONNECTED handle mutations before effects, and finally other handle mutations after effects, with detailed JSDoc explaining the process.
• src/testing/adapter-test-helpers.ts
  • Replaced direct calls to removeConsumer, enqueuePendingPassthrough, and seedSessionState with process calls dispatching the appropriate system signals.
  • Removed broadcaster from BackendConnector dependencies.

Activity

• The pull request author, teng-lin, implemented several post-review fixes, including ensuring closeBackendConnection() dispatches BACKEND_DISCONNECTED, adding per-effect try-catch in executeEffects(), wrapping refreshGitInfo() in try-catch, using explicit capability checks in trySendRawToBackend(), consolidating reduceSystemSignal(), extracting applyUserMessageReducer(), deleting dead code, and updating documentation.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This is an impressive and well-executed refactoring. Centralizing all state mutations through SessionRuntime.process() via system signals is a significant architectural improvement that will greatly enhance the maintainability and predictability of the session state management. The changes are consistently applied across the codebase, and the test suite has been thoroughly updated to reflect the new design. The addition of error handling in the effect executor and the more precise logic in trySendRawToBackend are also excellent robustness improvements. I have a suggestion to improve the type safety and clarity of the trySendRawToBackend method by using a type guard.

[gemini-code-assist]

To ensure type safety and align with best practices for verifying object capabilities, consider using a type guard to check if backendSession implements the sendRaw method. This approach is more robust than using as any and clearly defines the expected interface, as recommended by the rule for verifying component requirements. The typeof check for the function is a good runtime check, but it should be combined with a type guard to inform TypeScript about the object's shape. You would need to define a helper type guard function (e.g., hasSendRawMethod and its corresponding interface HasSendRaw) elsewhere in the module or file.

interface HasSendRaw { sendRaw: (data: string) => void; } function hasSendRawMethod(obj: any): obj is HasSendRaw { return typeof obj === 'object' && obj !== null && typeof obj.sendRaw === 'function'; } if (!hasSendRawMethod(backendSession)) { return "unsupported"; } backendSession.sendRaw(ndjson); return "sent";

References

1. This rule emphasizes using type guards to verify if an object implements a required interface or capability, promoting type safety over type assertions like as any.