Hide external threads from browser active thread restore - #2632
henrypark133 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a filtering mechanism for the active thread in the web gateway to ensure that only threads from web-writable channels are exposed to the browser, preventing the UI from defaulting to read-only external conversations. A logic flaw was identified in the browser_active_thread utility where a missing thread reference in the session map would bypass the channel filter; a more robust implementation using functional combinators was suggested to handle this edge case.
| pub fn browser_active_thread(session: &Session) -> Option<Uuid> { | ||
| let active_thread_id = session.active_thread?; | ||
| let channel = session | ||
| .threads | ||
| .get(&active_thread_id) | ||
| .and_then(|thread| thread.source_channel.as_deref()); | ||
| if is_web_writable_thread_channel(channel) { | ||
| Some(active_thread_id) | ||
| } else { | ||
| None | ||
| } | ||
| } |
There was a problem hiding this comment.
The current implementation of browser_active_thread has a logic flaw: if the active_thread_id is not found in the session.threads map, channel becomes None, and is_web_writable_thread_channel(None) returns true. This causes the function to return Some(active_thread_id), effectively bypassing the filter for stale or missing thread references. It should return None if the thread is not present in memory to ensure the channel-based safety check is correctly applied.
pub fn browser_active_thread(session: &Session) -> Option<Uuid> {
let active_thread_id = session.active_thread?;
session
.threads
.get(&active_thread_id)
.filter(|t| is_web_writable_thread_channel(t.source_channel.as_ref().map(|s| s.as_str())))
.map(|_| active_thread_id)
}References
- Use .as_ref().map() on Option fields within a struct to prevent partial moves, making the code more robust against future changes.
|
Closing this as superseded by current staging. The original fix here filtered browser restore at the old server-side active-thread path, but the relevant gateway flow has since moved and the user-visible issue now appears covered by #2751 (merged in Given the current code shape, carrying this branch forward would reintroduce an older integration point rather than land the right fix. If we later decide we want stricter fallback/API parity for no-DB or in-memory-only paths, that should be a small fresh follow-up on top of current staging, not this branch. |
Summary
browser_active_thread()to filter out non-browser-writable threads during restoreVerification
CARGO_TARGET_DIR=/Users/henry/near_ai/near_claw/ironclaw-ci-regression-fix-2/.shared-target cargo test browser_active_thread --libCARGO_TARGET_DIR=/Users/henry/near_ai/near_claw/ironclaw-ci-regression-fix-2/.shared-target cargo test test_chat_threads_handler_hides_external_active_thread --lib