Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 — <brief>` 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
from openai.types.shared_params.responses_model import ResponsesModel

from pythinker_core.chat_provider import (
APIStatusError,
ChatProvider,
RetryableChatProvider,
StreamedMessagePart,
Expand Down Expand Up @@ -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
Expand Down
45 changes: 45 additions & 0 deletions packages/pythinker-core/tests/test_openai_responses_auth.py
Original file line number Diff line number Diff line change
@@ -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:
Comment thread
coderabbitai[bot] marked this conversation as resolved.
# 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")])
2 changes: 2 additions & 0 deletions src/pythinker_code/ui/shell/tool_renderers/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ def register_builtin_renderers() -> None:
find,
generic,
grep,
implement_judge,
lsp,
mcp_resource,
memory,
Expand Down Expand Up @@ -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)
Expand Down
114 changes: 114 additions & 0 deletions src/pythinker_code/ui/shell/tool_renderers/implement_judge.py
Original file line number Diff line number Diff line change
@@ -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 — <brief>`` 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,
)
43 changes: 43 additions & 0 deletions tests/ui_and_conv/test_tui_card_tool_renderers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading