fix(gateway): keep engine threads out of chat sidebar - #2751
Conversation
github-actions
Bot
added
scope: channel/web
![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 removes the logic that merged engine v2 threads into the chat sidebar, ensuring that internal execution threads are not surfaced as standalone conversations. The changes include updated unit and E2E tests to verify that engine threads are hidden from the sidebar while remaining accessible via direct history lookups. A review comment suggests optimizing session lock handling by capturing the active_thread during an earlier lock period to avoid redundant acquisition.
| // ordinary prompts look like standalone `engine` threads. | ||
| // Explicit engine-thread history still works via | ||
| // `chat_history_handler` when the caller already has a thread id. | ||
| let active_thread = session.lock().await.active_thread; |
There was a problem hiding this comment.
The session lock is acquired here to retrieve active_thread, but it was already locked and then dropped earlier in the same function (lines 657-663). While the drop was necessary to avoid holding the lock across the asynchronous database call at line 679, you could capture active_thread during the first lock period to avoid the redundant second lock acquisition.
Two pre-existing staging-merge regressions in the coding-flow e2e suite: 1. Assistant thread now intentionally returns `project: None` (#2751), so after `project_set_active` the chrome still showed "No project". `refreshCurrentThread` now mirrors the backend `resolve_thread_project` precedence — per-thread override → active pointer → none — and fetches the active-project pointer when the thread has no override. Makes the home/assistant thread surface the user's currently-selected coding project instead of appearing disconnected. 2. `currentThreadId` was module-scoped and not reachable from Playwright. Expose a read-only `window.currentThreadId` getter so scenarios can observe thread binding without postMessage wiring. 3. `test_shell_mode_rejects_when_no_project` asserted a 409 contract that was deliberately retired in 0323ead (shell mode now falls back to the per-user `default` project). Renamed to `test_shell_mode_falls_back_to_default_project` and flipped the assertion to 202 so the new contract is pinned; the security invariant (shell runs against a project cwd, not the gateway's) is still covered by `test_shell_mode_roundtrip_persists_across_reload`. All 7 scenarios in `test_coding_project_flow.py` pass; `test_project_detail` still passes alongside. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(gateway): keep engine threads out of chat sidebar * fix: address review findings (iteration 1) * fix: address review findings (iteration 2)
1 participant
Summary
/api/chat/threadsENGINErowsTesting
CARGO_TARGET_DIR=/Users/firatsertgoz/Documents/ironclaw/target cargo test --lib test_chat_threads_handler_hides_engine_threads_from_sidebar --features libsqlCARGO_TARGET_DIR=/Users/firatsertgoz/Documents/ironclaw/target cargo test --lib test_chat_history_returns_engine_v2_messages_for_owner --features libsqlpython3 -m py_compile tests/e2e/scenarios/test_v2_thread_visibility.pyNotes
aiohttp, so I left the scenario updated and syntax-checked only.