diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e7c0edf..6bc15661 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,14 @@ GitHub Releases page; `0.8.0` is the new starting line. ## Unreleased +- Fix a fatal "Could not resolve authentication method" crash when a GPT/ChatGPT + session token is invalidated mid-session (e.g. the OAuth refresh token was + rotated by signing in on another machine). The provider now surfaces a typed + 401 that routes into the standard re-authentication path instead of an + uncaught error — prompting `/login` rather than dumping a traceback. +- Render the `ImplementAndJudge` chain as `Implement & Judge — ` in the + TUI instead of the raw `ImplementAndJudge(4 args: …)` argument dump. + ## 0.52.0 (2026-06-23) - **`InvalidToolError` now names the failing tool and the reason.** A bad diff --git a/packages/pythinker-core/src/pythinker_core/contrib/chat_provider/openai_responses.py b/packages/pythinker-core/src/pythinker_core/contrib/chat_provider/openai_responses.py index 7c8c7c9e..39484d80 100644 --- a/packages/pythinker-core/src/pythinker_core/contrib/chat_provider/openai_responses.py +++ b/packages/pythinker-core/src/pythinker_core/contrib/chat_provider/openai_responses.py @@ -30,6 +30,7 @@ from openai.types.shared_params.responses_model import ResponsesModel from pythinker_core.chat_provider import ( + APIStatusError, ChatProvider, RetryableChatProvider, StreamedMessagePart, @@ -192,6 +193,21 @@ async def generate( return OpenAIResponsesStreamedMessage(response) except (OpenAIError, httpx.HTTPError) as e: raise convert_error(e) from e + except TypeError as e: + # The OpenAI SDK raises a bare TypeError from `_validate_headers` at + # request-build time when no credential can be resolved. This happens + # when a live client's `api_key` is blanked mid-session — e.g. an OAuth + # refresh token rejected server-side after being rotated on another + # machine. Surface it as a typed 401 so the caller's refresh -> + # re-authenticate recovery engages instead of crashing the session. + # Gate on the empty key so unrelated TypeErrors still propagate. + if not self._client.api_key: + raise APIStatusError( + 401, + "OpenAI session credential is missing or expired " + "(no API key or Authorization header); re-authenticate with /login.", + ) from e + raise def on_retryable_error(self, error: BaseException) -> bool: old_client = self._client diff --git a/packages/pythinker-core/tests/test_openai_responses_auth.py b/packages/pythinker-core/tests/test_openai_responses_auth.py new file mode 100644 index 00000000..3901ba30 --- /dev/null +++ b/packages/pythinker-core/tests/test_openai_responses_auth.py @@ -0,0 +1,45 @@ +"""Auth-boundary behavior for the OpenAI Responses chat provider. + +Regression coverage for the mid-session credential blank: when an OAuth refresh +token is rejected server-side (e.g. it was rotated on another machine), the +Pythinker OAuth refresh path blanks the *live* OpenAI client's ``api_key`` to +``""``. The OpenAI SDK then raises a bare ``TypeError`` from ``_validate_headers`` +at request-build time. That ``TypeError`` is neither ``OpenAIError`` nor +``httpx.HTTPError``, so without conversion it escapes every handler as a fatal +"Unexpected error". The provider must convert it to a typed ``APIStatusError`` +(401) so the standard refresh -> ``/login`` recovery engages instead. +""" + +import pytest + +from pythinker_core.chat_provider import APIStatusError +from pythinker_core.contrib.chat_provider.openai_responses import OpenAIResponses +from pythinker_core.message import Message + + +@pytest.mark.parametrize("stream", [True, False]) +async def test_blanked_credential_raises_typed_401(stream: bool) -> None: + # Construct with a valid key (the SDK enforces credentials at construction), + # then simulate the live blank applied by the OAuth refresh path. + provider = OpenAIResponses(model="gpt-5-codex", api_key="sk-valid-dummy", stream=stream) + # Mirror _apply_access_token(runtime, ref, "") blanking the live client. + provider._client.api_key = "" # pyright: ignore[reportPrivateUsage] + + with pytest.raises(APIStatusError) as exc_info: + await provider.generate("You are helpful.", [], [Message(role="user", content="hi")]) + + assert exc_info.value.status_code == 401 + + +async def test_typeerror_with_valid_key_propagates(monkeypatch: pytest.MonkeyPatch) -> None: + # A TypeError that is NOT the missing-credential case must still propagate, + # so the conversion never becomes a bug-swallower. + provider = OpenAIResponses(model="gpt-5-codex", api_key="sk-valid-dummy", stream=True) + + async def boom(*args: object, **kwargs: object) -> None: + raise TypeError("unrelated bug, not an auth failure") + + monkeypatch.setattr(provider._client.responses, "create", boom) # pyright: ignore[reportPrivateUsage] + + with pytest.raises(TypeError, match="unrelated bug"): + await provider.generate("You are helpful.", [], [Message(role="user", content="hi")]) diff --git a/src/pythinker_code/ui/shell/tool_renderers/__init__.py b/src/pythinker_code/ui/shell/tool_renderers/__init__.py index 3b61a0fb..8f260eb5 100644 --- a/src/pythinker_code/ui/shell/tool_renderers/__init__.py +++ b/src/pythinker_code/ui/shell/tool_renderers/__init__.py @@ -157,6 +157,7 @@ def register_builtin_renderers() -> None: find, generic, grep, + implement_judge, lsp, mcp_resource, memory, @@ -188,6 +189,7 @@ def register_builtin_renderers() -> None: register_tool_renderer(mcp_resource.READ_MCP_RESOURCE_RENDERER) register_tool_renderer(agent.AGENT_RENDERER) register_tool_renderer(agent.RUN_AGENTS_RENDERER) + register_tool_renderer(implement_judge.IMPLEMENT_JUDGE_RENDERER) register_tool_renderer(ask_user.ASK_USER_RENDERER) register_tool_renderer(think.THINK_RENDERER) register_tool_renderer(todo.TODO_RENDERER) diff --git a/src/pythinker_code/ui/shell/tool_renderers/implement_judge.py b/src/pythinker_code/ui/shell/tool_renderers/implement_judge.py new file mode 100644 index 00000000..7cea248b --- /dev/null +++ b/src/pythinker_code/ui/shell/tool_renderers/implement_judge.py @@ -0,0 +1,114 @@ +"""Pythinker renderer for the ``ImplementAndJudge`` chain tool. + +The generic fallback renders this as ``ImplementAndJudge(4 args: acceptance, +base_prompt, brief, scope)`` — an unreadable arg dump. This dedicated renderer +shows the chain as ``⏺ Implement & Judge — `` so the card reads as a +single clear action instead of leaking the parameter names. +""" + +from __future__ import annotations + +from rich.console import Group, RenderableType +from rich.text import Text + +from pythinker_code.ui.shell.components.render_utils import sanitize_ansi +from pythinker_code.ui.shell.glyphs import TRANSCRIPT_ASSISTANT_MARKER +from pythinker_code.ui.shell.tool_renderers import ( + ToolRenderContext, + ToolRenderDefinition, + ToolResultPayload, +) +from pythinker_code.ui.shell.tool_renderers._render_utils import ( + as_str, + fg, + format_lines_block, + missing_required_arg, + pending_tool_call_header, + running_spinner, + tool_title, +) +from pythinker_code.ui.theme import tui_rich_style + +# Internal tool name stays ``ImplementAndJudge`` (registry/config/tests); only +# the on-screen label is the friendlier form. Kept in sync with +# ``pythinker_code.tools.agent.IMPLEMENT_JUDGE_NAME`` by a focused test. +_TOOL_NAME = "ImplementAndJudge" +_DISPLAY_NAME = "Implement & Judge" +_BRIEF_MAX_CHARS = 80 +_COLLAPSED_LINES = 8 + + +def _compact(text: str, *, max_chars: int = _BRIEF_MAX_CHARS) -> str: + """Collapse whitespace and ellipsize a possibly multi-line brief.""" + compact = " ".join(text.split()) + if len(compact) <= max_chars: + return compact + if max_chars <= 1: + return "…" + return compact[: max_chars - 1].rstrip() + "…" + + +def _render_call(ctx: ToolRenderContext) -> RenderableType: + args = ctx.args or {} + brief = as_str(args.get("brief")) + style_token = "error" if ctx.is_error else "success" if ctx.has_result else "muted" + + if brief is None: + # Streamed args still incomplete, or the required brief is missing. + if ctx.has_result: + header: RenderableType = Group( + _label_header(style_token), missing_required_arg("brief") + ) + else: + header = pending_tool_call_header(_DISPLAY_NAME) + return running_spinner( + header, + execution_started=ctx.execution_started, + has_result=ctx.has_result, + marker_style_token="muted", + ) + + header = _label_header(style_token) + header.append(" — ", style=tui_rich_style("muted")) + header.append(sanitize_ansi(_compact(brief)), style=tui_rich_style("thinking_text")) + header.no_wrap = True + header.overflow = "ellipsis" + return running_spinner( + header, + execution_started=ctx.execution_started, + has_result=ctx.has_result, + marker_style_token="muted", + ) + + +def _label_header(style_token: str) -> Text: + marker = "✘" if style_token == "error" else TRANSCRIPT_ASSISTANT_MARKER + header = Text() + header.append(f"{marker} ", style=tui_rich_style(style_token)) + header.append_text(tool_title(_DISPLAY_NAME)) + return header + + +def _render_result(ctx: ToolRenderContext, result: ToolResultPayload) -> RenderableType | None: + text = sanitize_ansi(result.text or "").rstrip("\n") + if not text: + return None + body, remaining = format_lines_block( + text, + expanded=ctx.expanded, + collapsed_max_lines=_COLLAPSED_LINES, + style_token="error" if result.is_error else "tool_output", + ) + if remaining > 0: + ctx.state["__suppress_generic_expand_hint__"] = True + return Group(body, fg("muted", f"… ({remaining} more lines, ctrl+o to expand)")) + return body + + +IMPLEMENT_JUDGE_RENDERER = ToolRenderDefinition( + name=_TOOL_NAME, + label=_DISPLAY_NAME, + render_shell="default", + render_call=_render_call, + render_result=_render_result, +) diff --git a/tests/ui_and_conv/test_tui_card_tool_renderers.py b/tests/ui_and_conv/test_tui_card_tool_renderers.py index 6f2e3e02..6d4bbf0e 100644 --- a/tests/ui_and_conv/test_tui_card_tool_renderers.py +++ b/tests/ui_and_conv/test_tui_card_tool_renderers.py @@ -2884,3 +2884,46 @@ def test_successful_non_review_agent_shows_done_subline_not_prose_dump(): assert "Done" in rendered assert "Refactored the auth module" not in rendered assert "Review Findings" not in rendered + + +def test_implement_judge_renders_clean_label_not_arg_dump(): + rendered = _render( + "ImplementAndJudge", + { + "brief": "Rewrite Logo.astro GSAP: matchMedia + scoped context", + "scope": ["apps/web/src/components/Logo.astro"], + "acceptance": ["typecheck passes"], + "base_prompt": "shared context", + }, + output="ACCEPT", + width=120, + ) + # Friendly label + brief, not the generic "Name(N args: ...)" arg dump. + assert "Implement & Judge" in rendered + assert "Rewrite Logo.astro GSAP" in rendered + assert "ImplementAndJudge(" not in rendered + assert "4 args" not in rendered + assert "base_prompt" not in rendered + + +def test_implement_judge_running_brief_visible(): + rendered = _render_running( + "ImplementAndJudge", + {"brief": "Add retry to the uploader"}, + width=120, + ) + assert "Implement & Judge" in rendered + assert "Add retry to the uploader" in rendered + assert "4 args" not in rendered + + +def test_implement_judge_renderer_name_matches_tool_constant(): + # The renderer's registered name must track the tool's canonical name so a + # rename of one without the other can't silently fall back to the generic + # arg-dump renderer. + from pythinker_code.tools.agent import IMPLEMENT_JUDGE_NAME + from pythinker_code.ui.shell.tool_renderers.implement_judge import ( + IMPLEMENT_JUDGE_RENDERER, + ) + + assert IMPLEMENT_JUDGE_RENDERER.name == IMPLEMENT_JUDGE_NAME