From 10d58131bf3846ac4221bc2bb7bf4d1e57af76ce Mon Sep 17 00:00:00 2001 From: Mason Daugherty <61371264+mdrxy@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:52:12 +0000 Subject: [PATCH 1/4] fix(code): consistent styling and links for thread status messages `AppMessage` dims plain-string messages but leaves pre-built `Content` untouched, so thread notes brightened as soon as their LangSmith link resolved. `_build_thread_message` now carries dim italic spans, the "Already on thread" note resolves a link like its siblings, and hovering an embedded link in any app message switches the mouse pointer. Co-authored-by: open-swe[bot] --- libs/code/deepagents_code/app.py | 20 ++++++-- .../deepagents_code/tui/widgets/messages.py | 8 ++++ .../unit_tests/tui/widgets/test_messages.py | 46 +++++++++++++++++++ .../tui/widgets/test_thread_selector.py | 29 ++++++++++++ 4 files changed, 98 insertions(+), 5 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 5a81852d204..41d4b908703 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -9562,7 +9562,10 @@ async def _build_thread_message( (e.g. `' (Resume with /threads -r)'`). Returns: - `Content` with a clickable thread ID, or a plain string. + `Content` with a clickable thread ID, or a plain string. The + `Content` carries the same dim italic styling `AppMessage` + applies to plain strings so linked and unlinked thread notes + look identical apart from the link. """ from deepagents_code.config import build_langsmith_thread_url @@ -9575,10 +9578,11 @@ async def _build_thread_message( url = None if url: + note_style = TStyle(dim=True, italic=True) return Content.assemble( - f"{prefix}: ", - (thread_id, TStyle(link=url)), - suffix, + (f"{prefix}: ", note_style), + (thread_id, TStyle(dim=True, italic=True, link=url)), + (suffix, note_style), ) return f"{prefix}: {thread_id}{suffix}" @@ -22329,7 +22333,13 @@ async def _resume_thread(self, thread_id: str) -> None: if cwd_choice == "abort": return if await asyncio.to_thread(self._cwd_paths_equal, self._cwd, prev_cwd): - await self._mount_message(AppMessage(f"Already on thread: {thread_id}")) + thread_msg_widget = AppMessage(f"Already on thread: {thread_id}") + await self._mount_message(thread_msg_widget) + self._schedule_thread_message_link( + thread_msg_widget, + prefix="Already on thread", + thread_id=thread_id, + ) else: await self._mount_message( AppMessage(f"Switched to thread directory: {self._cwd}"), diff --git a/libs/code/deepagents_code/tui/widgets/messages.py b/libs/code/deepagents_code/tui/widgets/messages.py index 982a2b61430..0918f06ea33 100644 --- a/libs/code/deepagents_code/tui/widgets/messages.py +++ b/libs/code/deepagents_code/tui/widgets/messages.py @@ -4353,6 +4353,14 @@ def on_click(self, event: Click) -> None: # noqa: PLR6301 # Textual event hand """Open style-embedded hyperlinks on single click.""" open_style_link(event) + def on_mouse_move(self, event: MouseMove) -> None: + """Show a pointer cursor over embedded links, text cursor elsewhere.""" + self.styles.pointer = "pointer" if event_targets_link(event) else "text" + + def on_leave(self) -> None: + """Reset the pointer shape when the mouse leaves the message.""" + self.styles.pointer = "text" + class SummarizationMessage(AppMessage): """Widget displaying a summarization completion notification.""" diff --git a/libs/code/tests/unit_tests/tui/widgets/test_messages.py b/libs/code/tests/unit_tests/tui/widgets/test_messages.py index cda33908d5b..d27436ad2e4 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_messages.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_messages.py @@ -4178,6 +4178,52 @@ def test_click_on_suspicious_url_is_blocked(self) -> None: event.stop.assert_not_called() +class _AppMessageApp(App[None]): + """Minimal app that mounts an `AppMessage` for runtime pointer tests.""" + + def compose(self) -> ComposeResult: + yield AppMessage("Resumed thread: tid-1", id="app-msg") + + +class TestAppMessageLinkPointer: + """Tests for the pointer cursor shown when hovering embedded links.""" + + @staticmethod + def _move_event( + *, link: str | None = None, meta: dict | None = None + ) -> SimpleNamespace: + """Build a minimal mouse-move-like event exposing the hovered style.""" + return SimpleNamespace(style=SimpleNamespace(link=link, meta=meta or {})) + + async def test_hovering_link_sets_pointer_cursor(self) -> None: + """An OSC 8 `Style(link=...)` span switches the pointer to pointer.""" + async with _AppMessageApp().run_test() as pilot: + msg = pilot.app.query_one("#app-msg", AppMessage) + + msg.on_mouse_move(self._move_event(link="https://example.com")) # ty: ignore + + assert msg.styles.pointer == "pointer" + + async def test_hovering_text_keeps_text_pointer(self) -> None: + """Plain message text keeps the text pointer.""" + async with _AppMessageApp().run_test() as pilot: + msg = pilot.app.query_one("#app-msg", AppMessage) + + msg.on_mouse_move(self._move_event()) # ty: ignore + + assert msg.styles.pointer == "text" + + async def test_leave_resets_pointer(self) -> None: + """Leaving the message resets the pointer after a link hover.""" + async with _AppMessageApp().run_test() as pilot: + msg = pilot.app.query_one("#app-msg", AppMessage) + msg.on_mouse_move(self._move_event(link="https://example.com")) # ty: ignore + + msg.on_leave() + + assert msg.styles.pointer == "text" + + class TestMountMessageIdSync: """Tests for widget id sync in `_mount_message`.""" diff --git a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py index b62f7262c84..2b9ce92e0d1 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py @@ -3143,6 +3143,8 @@ async def test_already_on_thread_shows_message(self) -> None: ) offer_cwd_switch = AsyncMock(return_value="continue") _app_test_double(app)._offer_thread_cwd_switch = offer_cwd_switch + schedule_link_mock = MagicMock() + _app_test_double(app)._schedule_thread_message_link = schedule_link_mock app._agent = MagicMock() app._session_state = MagicMock() app._session_state.thread_id = "thread-123" @@ -3156,6 +3158,11 @@ async def test_already_on_thread_shows_message(self) -> None: ) assert len(mounted) == 1 assert "Already on thread" in _get_widget_text(mounted[0]) + schedule_link_mock.assert_called_once_with( + mounted[0], + prefix="Already on thread", + thread_id="thread-123", + ) async def test_already_on_thread_reports_cwd_switch( self, @@ -4120,6 +4127,28 @@ async def test_hyperlinked_when_tracing_configured(self) -> None: assert isinstance(style, TStyle) assert style.link == url + async def test_linked_content_matches_plain_app_message_styling(self) -> None: + """Every span should be dim italic so linked notes match unlinked ones.""" + from textual.content import Content + from textual.style import Style as TStyle + + app = DeepAgentsApp() + url = "https://smith.langchain.com/o/org/projects/p/proj/t/tid-123" + target = "deepagents_code.config.build_langsmith_thread_url" + with patch(target, return_value=url): + result = await app._build_thread_message( + "Previous thread", "tid-123", suffix=" (Resume with /threads -r)" + ) + + assert isinstance(result, Content) + covered = 0 + for span in result._spans: + assert isinstance(span.style, TStyle) + assert span.style.dim is True + assert span.style.italic is True + covered += span.end - span.start + assert covered == len(result.plain) + async def test_fallback_on_timeout(self) -> None: """Returns plain string when URL resolution times out.""" app = DeepAgentsApp() From 3b64f42da615ef303f197f8de9ded81a663cf53c Mon Sep 17 00:00:00 2001 From: Mason Daugherty <61371264+mdrxy@users.noreply.github.com> Date: Tue, 28 Jul 2026 16:59:16 +0000 Subject: [PATCH 2/4] fix(code): toast the same-thread no-op instead of mounting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Re-selecting the thread the session is already on is transient feedback, not part of the conversation — the same reasoning that already makes the same-model no-op a toast. Hoists that path's dedup guard into `_notify_unchanged_once` so both share one implementation and neither can stack duplicate toasts while the previous one is still on-screen. Co-authored-by: open-swe[bot] --- libs/code/deepagents_code/app.py | 70 ++++++++++++------ .../tui/widgets/test_thread_selector.py | 72 ++++++++++++++++--- 2 files changed, 110 insertions(+), 32 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 41d4b908703..7644b01dc23 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -3208,6 +3208,13 @@ def __init__( they cannot drift out of sync. """ + self._last_thread_unchanged: tuple[str, float] | None = None + """Most recent same-thread toast, as `(text, monotonic timestamp)`. + + The same-model counterpart of `_last_model_unchanged`, for re-selecting + the thread the session is already on. + """ + self._model_install_switching = False """True while a provider extra install-then-switch flow is active.""" @@ -22333,12 +22340,9 @@ async def _resume_thread(self, thread_id: str) -> None: if cwd_choice == "abort": return if await asyncio.to_thread(self._cwd_paths_equal, self._cwd, prev_cwd): - thread_msg_widget = AppMessage(f"Already on thread: {thread_id}") - await self._mount_message(thread_msg_widget) - self._schedule_thread_message_link( - thread_msg_widget, - prefix="Already on thread", - thread_id=thread_id, + self._last_thread_unchanged = self._notify_unchanged_once( + f"Already on thread: {thread_id}", + self._last_thread_unchanged, ) else: await self._mount_message( @@ -22412,6 +22416,10 @@ async def _resume_thread(self, thread_id: str) -> None: # thread". Set only after the last statement that can raise, so a # failed switch (handled below) never leaves a stale pointer. self._session_state.previous_thread_id = prev_session_thread + + # Landing on a new thread re-arms the same-thread toast, so stepping + # back to a thread and re-selecting it announces itself again. + self._last_thread_unchanged = None except Exception as exc: if prefetched_payload is None: logger.exception("Failed to prefetch history for thread %s", thread_id) @@ -22482,6 +22490,36 @@ async def _mount_resume_adoption_failure( body += f" {hint}" await self._mount_message(ErrorMessage(body)) + def _notify_unchanged_once( + self, message: str, last: tuple[str, float] | None + ) -> tuple[str, float] | None: + """Toast a no-op notice unless an identical toast is still on-screen. + + Re-selecting the model or thread the session is already on is transient + feedback, not part of the conversation, so it surfaces as a toast rather + than an inline chat message. Suppression lasts only for the toast + lifetime, so a later intentional no-op selection can toast again. + + Args: + message: The notice to toast. Interpolated identifiers are rendered + literally — markup parsing is always disabled. + last: The caller's previous `(text, monotonic timestamp)` record, or + `None` when nothing has been toasted yet. + + Returns: + The record the caller should store: the new toast's text and + timestamp, or `last` unchanged when the toast was suppressed. + """ + now = _monotonic() + if ( + last is not None + and last[0] == message + and (now - last[1]) < self.NOTIFICATION_TIMEOUT + ): + return last + self.notify(message, markup=False) + return (message, now) + async def _switch_model( self, model_spec: str, @@ -22631,22 +22669,10 @@ async def _switch_model( self._sync_status_model() params_suffix = _format_model_params(extra_kwargs) if announce_unchanged: - message = f"Already using {current}{params_suffix}" - # Suppress only while the previous identical toast is - # presumed still on-screen. Once it expires, a later - # intentional no-op selection must be able to toast again. - now = _monotonic() - last = self._last_model_unchanged - if ( - last is None - or last[0] != message - or (now - last[1]) >= self.NOTIFICATION_TIMEOUT - ): - # A no-op re-selection is transient feedback, not part - # of the conversation, so surface it as a toast rather - # than an inline chat message. - self.notify(message, markup=False) - self._last_model_unchanged = (message, now) + self._last_model_unchanged = self._notify_unchanged_once( + f"Already using {current}{params_suffix}", + self._last_model_unchanged, + ) logger.info( "Model unchanged (%s); model_params=%s", current, diff --git a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py index 2b9ce92e0d1..da83a3ddaf2 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py @@ -3135,7 +3135,7 @@ async def test_already_switching_shows_message(self) -> None: assert "already in progress" in _get_widget_text(mounted[0]) async def test_already_on_thread_shows_message(self) -> None: - """_resume_thread when already on the thread should show info message.""" + """_resume_thread when already on the thread should toast, not mount.""" app = DeepAgentsApp() mounted: list[Static] = [] _app_test_double(app)._mount_message = AsyncMock( @@ -3143,8 +3143,8 @@ async def test_already_on_thread_shows_message(self) -> None: ) offer_cwd_switch = AsyncMock(return_value="continue") _app_test_double(app)._offer_thread_cwd_switch = offer_cwd_switch - schedule_link_mock = MagicMock() - _app_test_double(app)._schedule_thread_message_link = schedule_link_mock + notify_mock = MagicMock() + _app_test_double(app).notify = notify_mock app._agent = MagicMock() app._session_state = MagicMock() app._session_state.thread_id = "thread-123" @@ -3156,13 +3156,54 @@ async def test_already_on_thread_shows_message(self) -> None: restart_server=True, abort="thread_switch", ) - assert len(mounted) == 1 - assert "Already on thread" in _get_widget_text(mounted[0]) - schedule_link_mock.assert_called_once_with( - mounted[0], - prefix="Already on thread", - thread_id="thread-123", + assert mounted == [] + notify_mock.assert_called_once_with( + "Already on thread: thread-123", markup=False + ) + + async def test_duplicate_already_on_thread_toast_is_suppressed(self) -> None: + """Repeated no-op resumes within the toast lifetime toast only once.""" + app = DeepAgentsApp() + _app_test_double(app)._mount_message = AsyncMock() + _app_test_double(app)._offer_thread_cwd_switch = AsyncMock( + return_value="continue" ) + notify_mock = MagicMock() + _app_test_double(app).notify = notify_mock + app._agent = MagicMock() + app._session_state = MagicMock() + app._session_state.thread_id = "thread-123" + + clock = {"now": 100.0} + with patch("deepagents_code.app._monotonic", side_effect=lambda: clock["now"]): + await app._resume_thread("thread-123") + clock["now"] = 100.0 + app.NOTIFICATION_TIMEOUT / 2 + await app._resume_thread("thread-123") + + notify_mock.assert_called_once_with( + "Already on thread: thread-123", markup=False + ) + + async def test_expired_already_on_thread_toast_can_reemit(self) -> None: + """Once the toast has expired, a later no-op resume toasts again.""" + app = DeepAgentsApp() + _app_test_double(app)._mount_message = AsyncMock() + _app_test_double(app)._offer_thread_cwd_switch = AsyncMock( + return_value="continue" + ) + notify_mock = MagicMock() + _app_test_double(app).notify = notify_mock + app._agent = MagicMock() + app._session_state = MagicMock() + app._session_state.thread_id = "thread-123" + + clock = {"now": 100.0} + with patch("deepagents_code.app._monotonic", side_effect=lambda: clock["now"]): + await app._resume_thread("thread-123") + clock["now"] = 100.0 + app.NOTIFICATION_TIMEOUT + await app._resume_thread("thread-123") + + assert notify_mock.call_count == 2 async def test_already_on_thread_reports_cwd_switch( self, @@ -3192,6 +3233,8 @@ async def offer_cwd_switch( # noqa: RUF029 # must be async: awaited as _offer_ return "continue" _app_test_double(app)._offer_thread_cwd_switch = offer_cwd_switch + notify_mock = MagicMock() + _app_test_double(app).notify = notify_mock app._agent = MagicMock() app._session_state = MagicMock() app._session_state.thread_id = "thread-123" @@ -3200,7 +3243,7 @@ async def offer_cwd_switch( # noqa: RUF029 # must be async: awaited as _offer_ assert len(mounted) == 1 assert "Switched to thread directory" in _get_widget_text(mounted[0]) - assert "Already on thread" not in _get_widget_text(mounted[0]) + notify_mock.assert_not_called() async def test_successful_switch_updates_ids(self) -> None: """Successful _resume_thread should update thread IDs and load history.""" @@ -3304,6 +3347,15 @@ async def test_successful_switch_records_previous_thread(self) -> None: assert session_state is not None assert session_state.previous_thread_id == "old-thread" + async def test_successful_switch_rearms_already_on_thread_toast(self) -> None: + """Landing on a thread lets a later no-op resume toast again.""" + app = self._switch_app() + app._last_thread_unchanged = ("Already on thread: old-thread", 100.0) + + await app._resume_thread("new-thread") + + assert app._last_thread_unchanged is None + async def test_failure_restores_previous_thread_ids(self) -> None: """If _clear_messages raises, thread IDs should be restored.""" from textual.css.query import NoMatches as _NoMatches From f3c861fecaec97563576c1f0a390e3c9c914e407 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 29 Jul 2026 16:57:45 -0400 Subject: [PATCH 3/4] fix(code): log the same-thread no-op and pin its pointer/styling tests Correct `_last_thread_unchanged`'s docstring, which called the field the same-model counterpart of `_last_model_unchanged` while its own summary line calls it the same-thread toast. Log the same-thread no-op resume. The `Already on thread` notice is a toast, which is transient and suppressible, and `/threads -r` mounts the user's command echo before dispatching -- so a no-op left the transcript showing the command and nothing else, indistinguishable from a dropped command. The same-model path already logs unconditionally; this mirrors it, outside the dedup guard. Document what `_notify_unchanged_once` actually guarantees: suppression is time-based and never inspects live toast state, so "on-screen" is a presumption -- a toast clicked away still suppresses. Its window matches the toast lifetime only because `notify` is called without a `timeout` override, which nothing else records. Note that callers own the record, and give `markup=False` the reason it is load-bearing. The return type drops `| None`: the suppression branch is guarded by `last is not None`, so only the explicit re-arm sites clear a caller's field. Replace two tests that could not fail: - `test_hovering_text_keeps_text_pointer` asserted `pointer == "text"`, already the CSS default on a fresh widget. Deleting the handler's entire `else "text"` arm left it green. The new test covers what that arm exists for -- moving off a link onto plain text without leaving the widget, where `on_leave` cannot help and the hand cursor would otherwise stick. - `test_linked_content_matches_plain_app_message_styling` hardcoded the same `dim`/`italic` literals as the implementation, so a change to `AppMessage` could break the parity it names while staying green. It now reads the expected style off a real `AppMessage`. Make the re-arm test behavioral. It asserted private state directly and passed even with the helper broken; it now pins the clock and asserts two toasts across an A -> B -> A round trip, mirroring the same-model counterpart. --- libs/code/deepagents_code/app.py | 37 ++++++++--- .../deepagents_code/tui/widgets/messages.py | 7 ++- .../unit_tests/tui/widgets/test_messages.py | 16 +++++ .../tui/widgets/test_thread_selector.py | 61 ++++++++++++++++--- 4 files changed, 102 insertions(+), 19 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index f471021ae7c..6b3ad1e0855 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -3257,7 +3257,7 @@ def __init__( self._last_thread_unchanged: tuple[str, float] | None = None """Most recent same-thread toast, as `(text, monotonic timestamp)`. - The same-model counterpart of `_last_model_unchanged`, for re-selecting + The same-thread counterpart of `_last_model_unchanged`, for re-selecting the thread the session is already on. """ @@ -22621,6 +22621,11 @@ async def _resume_thread(self, thread_id: str) -> None: f"Already on thread: {thread_id}", self._last_thread_unchanged, ) + # Log unconditionally, outside the toast dedup: the toast is + # transient and may be suppressed, so this is the only durable + # record that the resume was a deliberate no-op rather than a + # dropped command. Mirrors the same-model path. + logger.info("Thread unchanged (%s); resume was a no-op", thread_id) else: from deepagents_code.hooks.models.domain import ( SessionEndCause, @@ -22796,23 +22801,35 @@ async def _mount_resume_adoption_failure( def _notify_unchanged_once( self, message: str, last: tuple[str, float] | None - ) -> tuple[str, float] | None: - """Toast a no-op notice unless an identical toast is still on-screen. + ) -> tuple[str, float]: + """Toast a no-op notice unless an identical toast is presumed on-screen. - Re-selecting the model or thread the session is already on is transient - feedback, not part of the conversation, so it surfaces as a toast rather - than an inline chat message. Suppression lasts only for the toast - lifetime, so a later intentional no-op selection can toast again. + A no-op re-selection — of the model or thread the session is already on, + for example — is transient feedback, not part of the conversation, so it + surfaces as a toast rather than an inline chat message. + + Suppression is time-based: it lasts one `NOTIFICATION_TIMEOUT`, which + matches the toast lifetime only because `notify` is called without a + `timeout` override. Nothing inspects live toast state, so "on-screen" is + a presumption — a toast the user clicked away still suppresses. Once the + window expires, a later intentional no-op selection can toast again. + + Callers own the record: pass the field you keep it in and assign the + result straight back, or dedup silently stops working. Separate fields + per call site keep unrelated notices from suppressing each other. Args: message: The notice to toast. Interpolated identifiers are rendered - literally — markup parsing is always disabled. + literally — `markup=False` is load-bearing, because identifiers + containing square brackets would otherwise crash Textual's toast + renderer when parsed as Rich markup. last: The caller's previous `(text, monotonic timestamp)` record, or `None` when nothing has been toasted yet. Returns: - The record the caller should store: the new toast's text and - timestamp, or `last` unchanged when the toast was suppressed. + The record the caller should store, never `None`: the new toast's + text and timestamp, or `last` unchanged when suppressed. Only + the explicit re-arm sites clear a caller's field. """ now = _monotonic() if ( diff --git a/libs/code/deepagents_code/tui/widgets/messages.py b/libs/code/deepagents_code/tui/widgets/messages.py index 0918f06ea33..bcd0353eae9 100644 --- a/libs/code/deepagents_code/tui/widgets/messages.py +++ b/libs/code/deepagents_code/tui/widgets/messages.py @@ -4358,7 +4358,12 @@ def on_mouse_move(self, event: MouseMove) -> None: self.styles.pointer = "pointer" if event_targets_link(event) else "text" def on_leave(self) -> None: - """Reset the pointer shape when the mouse leaves the message.""" + """Restore the pointer shape when the mouse leaves the message. + + `"text"` restates this widget's CSS default rather than clearing the + inline style, so a subclass declaring a different `pointer` would be + forced back to `text` on leave. + """ self.styles.pointer = "text" diff --git a/libs/code/tests/unit_tests/tui/widgets/test_messages.py b/libs/code/tests/unit_tests/tui/widgets/test_messages.py index d27436ad2e4..52421261c0e 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_messages.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_messages.py @@ -4223,6 +4223,22 @@ async def test_leave_resets_pointer(self) -> None: assert msg.styles.pointer == "text" + async def test_link_then_text_resets_pointer_without_leaving(self) -> None: + """Moving off a link onto plain text resets the pointer without leaving. + + `on_leave` cannot cover this: the mouse stays inside the widget, so only + the handler's non-link branch clears the inline `pointer` set by the + previous move. Without it the hand cursor sticks over non-link text. + """ + async with _AppMessageApp().run_test() as pilot: + msg = pilot.app.query_one("#app-msg", AppMessage) + msg.on_mouse_move(self._move_event(link="https://example.com")) # ty: ignore + assert msg.styles.pointer == "pointer" + + msg.on_mouse_move(self._move_event()) # ty: ignore + + assert msg.styles.pointer == "text" + class TestMountMessageIdSync: """Tests for widget id sync in `_mount_message`.""" diff --git a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py index fd12e054ea3..42c84a71160 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py @@ -3364,13 +3364,40 @@ async def test_successful_switch_records_previous_thread(self) -> None: assert session_state.previous_thread_id == "old-thread" async def test_successful_switch_rearms_already_on_thread_toast(self) -> None: - """Landing on a thread lets a later no-op resume toast again.""" - app = self._switch_app() - app._last_thread_unchanged = ("Already on thread: old-thread", 100.0) + """A real switch clears suppression so the next no-op toasts again. - await app._resume_thread("new-thread") + Without the reset, re-selecting A after an A -> B -> A round trip would + be swallowed by the stale suppression entry left by the first no-op. + Mirrors the same-model counterpart in `test_model_switch.py`. + """ + app = self._switch_app() + notify_mock = MagicMock() + _app_test_double(app).notify = notify_mock + _app_test_double(app)._offer_thread_cwd_switch = AsyncMock( + return_value="continue" + ) - assert app._last_thread_unchanged is None + # Hold the clock still so a second toast is attributable to the reset + # rather than to the toast lifetime quietly expiring. + with patch("deepagents_code.app._monotonic", return_value=100.0): + # No-op records the suppression entry. + await app._resume_thread("old-thread") + # Real switches away and back must clear it. + await app._resume_thread("new-thread") + await app._resume_thread("old-thread") + # Identical message, same instant on the clock: only the reset can + # let this through. + await app._resume_thread("old-thread") + + unchanged_toasts = [ + call.args[0] + for call in notify_mock.call_args_list + if call.args[0].startswith("Already on thread") + ] + assert unchanged_toasts == [ + "Already on thread: old-thread", + "Already on thread: old-thread", + ] async def test_failure_restores_previous_thread_ids(self) -> None: """If _clear_messages raises, thread IDs should be restored.""" @@ -4191,10 +4218,26 @@ async def test_hyperlinked_when_tracing_configured(self) -> None: assert style.link == url async def test_linked_content_matches_plain_app_message_styling(self) -> None: - """Every span should be dim italic so linked notes match unlinked ones.""" + """Linked notes carry the same styling `AppMessage` gives plain strings. + + The expected style is read off a real `AppMessage` rather than hardcoded, + so that if `AppMessage` stops styling plain strings `dim italic` this + fails instead of silently locking in the divergence it exists to catch. + `AppMessage` records the style as an unresolved spec string, so compare + parsed styles rather than span representations. + """ from textual.content import Content from textual.style import Style as TStyle + from deepagents_code.tui.widgets.messages import AppMessage + + plain_spans = AppMessage("Previous thread: tid-123").render()._spans + assert len(plain_spans) == 1 + plain_style = plain_spans[0].style + expected = ( + TStyle.parse(plain_style) if isinstance(plain_style, str) else plain_style + ) + app = DeepAgentsApp() url = "https://smith.langchain.com/o/org/projects/p/proj/t/tid-123" target = "deepagents_code.config.build_langsmith_thread_url" @@ -4204,11 +4247,13 @@ async def test_linked_content_matches_plain_app_message_styling(self) -> None: ) assert isinstance(result, Content) + # Sum the spans to prove every character is styled, not just that the + # spans present are correct: an unstyled gap is the original regression. covered = 0 for span in result._spans: assert isinstance(span.style, TStyle) - assert span.style.dim is True - assert span.style.italic is True + assert span.style.dim == expected.dim + assert span.style.italic == expected.italic covered += span.end - span.start assert covered == len(result.plain) From 4991e618334d238d14e2d79990beedbde55df943 Mon Sep 17 00:00:00 2001 From: Mason Daugherty Date: Wed, 29 Jul 2026 17:09:28 -0400 Subject: [PATCH 4/4] fix(code): log LangSmith URL failures and restore the thread back-pointer Three fixes in the code paths this branch already touches, each of which was invisible rather than wrong. `_build_thread_message` swallowed every LangSmith URL failure without a trace. Unlinked thread IDs are the only symptom, and they look identical to tracing simply not being configured -- so a rejected `LANGSMITH_API_KEY`, a changed payload shape, or a network failure left the user with no diagnostic and no reason to suspect misconfiguration. Log the cause at debug with `exc_info`. The `except (TimeoutError, Exception)` tuple collapses to `except Exception`, which is all it ever meant (`TimeoutError` inherits from `OSError`); its `noqa: BLE001` is dropped because the rule was flagging the missing log, not the breadth of the catch. `_resume_thread` set `previous_thread_id` once the switch was materially complete, but `_run_session_start_hook` runs after that and can raise -- it awaits `on_session_start` and `_mount_message` with no guard. Rollback restored `thread_id` and `_lc_thread_id` but not the back-pointer, so a raise there left `previous == current`: a later bare `/threads -r` resolved to the thread already active and reported the no-op, with nowhere to step back to. Capture and restore it alongside the other rollback state. The comment claiming the assignment sits after "the last statement that can raise" is corrected -- it never did. Add the one test that verifies Textual actually delivers hover events to these widgets. There are ten `on_mouse_move` handlers across seven widget files and every test for them calls the handler directly with a stand-in event, so nothing anywhere proved `MouseMove` reaches the widget or that `event.style` is populated at the hovered offset. One `pilot.hover` test over a real OSC 8 span covers the assumption the whole family shares. --- libs/code/deepagents_code/app.py | 23 +++++++-- .../unit_tests/tui/widgets/test_messages.py | 48 +++++++++++++++++++ .../tui/widgets/test_thread_selector.py | 27 +++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/libs/code/deepagents_code/app.py b/libs/code/deepagents_code/app.py index 38252b349ba..63f05c81e83 100644 --- a/libs/code/deepagents_code/app.py +++ b/libs/code/deepagents_code/app.py @@ -9759,7 +9759,16 @@ async def _build_thread_message( asyncio.to_thread(build_langsmith_thread_url, thread_id), timeout=2.0, ) - except (TimeoutError, Exception): # noqa: BLE001 # Resilient non-interactive mode error handling + except Exception: # Resilient non-interactive mode error handling + # Unlinked thread IDs are the only symptom, and they look identical + # to tracing simply being unconfigured — so log the cause. Covers a + # rejected `LANGSMITH_API_KEY`, a changed LangSmith payload shape, + # and network failures, none of which the user can otherwise see. + logger.debug( + "Could not resolve LangSmith thread URL for %s; rendering plain text", + thread_id, + exc_info=True, + ) url = None if url: @@ -22662,6 +22671,7 @@ async def _resume_thread(self, thread_id: str) -> None: # Save previous state for rollback on failure prev_thread_id = self._lc_thread_id prev_session_thread = self._session_state.thread_id + prev_previous_thread = self._session_state.previous_thread_id prev_cwd = Path(self._cwd) cwd_choice = await self._offer_thread_cwd_switch( @@ -22730,8 +22740,10 @@ async def _resume_thread(self, thread_id: str) -> None: # The switch succeeded: record the thread we just left so a # subsequent bare `/threads -r` steps back to it rather than # resolving `previous == current` and reporting "Already on - # thread". Set only after the last statement that can raise, so a - # failed switch (handled below) never leaves a stale pointer. + # thread". Set once the switch is materially complete -- the thread + # ID is committed and history is loaded. `_run_session_start_hook` + # below can still raise, so the rollback path restores this pointer + # explicitly rather than relying on statement order. self._session_state.previous_thread_id = prev_session_thread # Landing on a new thread re-arms the same-thread toast, so stepping @@ -22754,6 +22766,11 @@ async def _resume_thread(self, thread_id: str) -> None: # Restore previous thread IDs so the user can retry self._session_state.thread_id = prev_session_thread self._lc_thread_id = prev_thread_id + # Also restore the back-pointer. A raise after it was set (the + # session-start hook) would otherwise leave `previous == current`, + # making a later bare `/threads -r` a no-op with nowhere to step + # back to. + self._session_state.previous_thread_id = prev_previous_thread self._update_welcome_banner( prev_session_thread, missing_message=( diff --git a/libs/code/tests/unit_tests/tui/widgets/test_messages.py b/libs/code/tests/unit_tests/tui/widgets/test_messages.py index e4eb55505e3..ce69c300955 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_messages.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_messages.py @@ -4716,6 +4716,54 @@ async def test_link_then_text_resets_pointer_without_leaving(self) -> None: assert msg.styles.pointer == "text" +class _LinkedAppMessageApp(App[None]): + """Mounts an `AppMessage` whose thread ID is a real OSC 8 link span.""" + + PREFIX = "Resumed thread: " + URL = "https://smith.langchain.com/o/org/projects/p/proj/t/tid-123" + + def compose(self) -> ComposeResult: + from textual.content import Content + from textual.style import Style as TStyle + + note = TStyle(dim=True, italic=True) + yield AppMessage( + Content.assemble( + (self.PREFIX, note), + ("tid-123", TStyle(dim=True, italic=True, link=self.URL)), + ), + id="app-msg", + ) + + +class TestAppMessagePointerEventDelivery: + """Pins that Textual actually delivers hover events to `AppMessage`. + + Every other pointer test in this repo calls `on_mouse_move` directly with a + stand-in event, which cannot catch Textual routing `MouseMove` elsewhere or + leaving `event.style` unpopulated at the hovered offset. This drives a real + `pilot.hover` instead, so the delivery assumption the whole family of + pointer handlers shares is verified in one place. + """ + + async def test_hover_over_real_link_span_toggles_pointer(self) -> None: + """Hovering a real link span sets the pointer and moving off resets it.""" + async with _LinkedAppMessageApp().run_test() as pilot: + msg = pilot.app.query_one("#app-msg", AppMessage) + # `AppMessage` pads by 1 column, so content offset N sits at N + 1. + link_x = len(_LinkedAppMessageApp.PREFIX) + 1 + prefix_x = 1 + + await pilot.hover("#app-msg", offset=(prefix_x, 0)) + assert msg.styles.pointer == "text" + + await pilot.hover("#app-msg", offset=(link_x, 0)) + assert msg.styles.pointer == "pointer" + + await pilot.hover("#app-msg", offset=(prefix_x, 0)) + assert msg.styles.pointer == "text" + + class TestMountMessageIdSync: """Tests for widget id sync in `_mount_message`.""" diff --git a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py index be8b4df91cc..21e4a2b697b 100644 --- a/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py +++ b/libs/code/tests/unit_tests/tui/widgets/test_thread_selector.py @@ -3399,6 +3399,33 @@ async def test_successful_switch_rearms_already_on_thread_toast(self) -> None: "Already on thread: old-thread", ] + async def test_failure_after_switch_restores_previous_thread_pointer(self) -> None: + """A raise after the back-pointer is set must not leave previous == current. + + `previous_thread_id` is recorded once the switch is materially complete, + but `_run_session_start_hook` runs after that and can raise. Without an + explicit restore, rollback would put the session back on the outgoing + thread while the back-pointer still named it, making a later bare + `/threads -r` a no-op with nowhere to step back to. + """ + app = self._switch_app() + session_state = app._session_state + assert session_state is not None + session_state.previous_thread_id = "grandparent-thread" + _app_test_double(app)._offer_thread_cwd_switch = AsyncMock( + return_value="continue" + ) + # Raise on the post-switch call; succeed on the rollback call so the + # rollback path itself completes. + _app_test_double(app)._run_session_start_hook = AsyncMock( + side_effect=[RuntimeError("hook exploded"), True] + ) + + await app._resume_thread("new-thread") + + assert session_state.thread_id == "old-thread" + assert session_state.previous_thread_id == "grandparent-thread" + async def test_failure_restores_previous_thread_ids(self) -> None: """If _clear_messages raises, thread IDs should be restored.""" from textual.css.query import NoMatches as _NoMatches