fix: pass deps=None in SessionPool.send_message() - #166
Merged
Conversation
send_message() delegates directly to _route_message() bypassing receive_request(), but wasn't passing the deps parameter. This caused integration test assertion failures (test_opencode_delivery_mode_mapping, test_send_message_steer_mode_on_active_session) since dd43821 added deps=None to the expected call args.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request modifies the send_message function in session_pool.py to hardcode deps=None when routing messages. The reviewer pointed out that this change silently ignores the deps parameter passed to the function and suggested forwarding the parameter instead to ensure custom dependencies can be successfully propagated.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Address review feedback: send_message() accepts a deps parameter, so _route_message() should receive deps=deps, not deps=None.
Leoyzen
added a commit
that referenced
this pull request
Jul 16, 2026
* fix: wire ACPSkillBridge to expose skills as ACP slash commands ACPSkillBridge was defined but never instantiated or wired into the ACP session lifecycle. ACP clients never received skills as available_commands_update events. Changes: - Modified ACPSkillBridge to produce executable SlashedCommand objects (reusing create_skill_command() from opencode_server skill_bridge) instead of display-only AvailableCommand - Added _register_skill_commands() to ACPSession that builds SkillCommand objects from the skills registry, feeds them through the bridge, and registers SlashedCommand in command_store - Called _register_skill_commands() in __post_init__ for pool-level skills - Wired init_client_skills() to re-register and send available_commands_update after client skill discovery - Added _watch_skill_changes() background task that subscribes to ExtensionRegistry.merge_change_streams() for dynamic updates - Cancel watcher task on session close() - 37 tests covering unit, integration, E2E, and regression scenarios OpenSpec: fix-acp-skill-commands * fix: address review feedback — stale skill removal + lint/format fixes - Add get_command_names() to ACPSkillBridge for clean stale detection - Fix _register_skill_commands() to remove stale skills via handle_change(name, None) + command_store.unregister_command(name) - Fix D205/D400/D415 docstring lint errors in test files - Add D104 docstring to tests/acp_server/__init__.py - Run ruff format on all changed files - All 37 tests still pass * fix: re-add deps parameter to send_message() after span revert The revert of e0451ba (#165) also removed the deps and input_provider parameters from send_message(), which were added in that same commit. This re-adds deps to send_message() signature and forwards it to _route_message(), restoring the fix from #166 that was lost.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix:
deps=Nonemissing inSessionPool.send_message()send_message()calls_route_message()directly, bypassingreceive_request(). It wasn't passing thedepsparameter that_route_message()expects (added in5e3330c99). This causes 2 integration test failures:One-line fix: add
deps=Noneto the_route_message()call.Should be merged first — PR #164 and #165 both depend on this fix.