@claude code review

Claude finished @RafaelPo's task in 1m 7s —— View job

Code Review: Switch widget UA detection to whitelist

• Gather context and read changed files
• Review the diff for correctness, security, and patterns
• Post review feedback

Overall: Clean, well-documented change. The whitelist approach is the right call — fail-closed is safer than fail-open for widget detection. One real issue to fix; the rest are minor.

Stale docstring on client_supports_widgets (should fix)

The tier 3 description in the client_supports_widgets docstring (tool_helpers.py:117-126) still describes the old blocklist behavior:

    3. **User-Agent fallback** (stateless HTTP mode):
       ...  Clients known to
       NOT support widgets (e.g. Claude Code) are excluded.  If the
       User-Agent is unknown, we default to **showing widgets** because
       HTTP mode traffic is overwhelmingly from Claude.ai/Desktop.

    Unknown clients default to **no widget** in stateful mode (tier 2),
    but to **widget** in stateless HTTP mode (tier 3) where the population
    is predominantly Claude.ai/Desktop.

With the whitelist change, unknown UAs now default to text-only, which is the exact opposite of what this docstring says. This will mislead future readers.

Fix this →

Minor observations

is_internal_client broadening (tool_helpers.py:193): Matching "everyrow" instead of "everyrow-cc" is intentionally broader per the PR description. Just confirming: any future internal client UA starting with everyrow-* will be caught, which seems like the right intent. No action needed.

No unit tests for _widgets_from_user_agent / is_internal_client: Neither function has test coverage. These are small and deterministic — a parameterized test would lock in the whitelist/internal-client behavior and catch regressions if UAs change. Not blocking, but worth adding.

@claude code review

Claude finished @RafaelPo's task in 55s —— View job

Code Review: Switch widget UA detection to whitelist

• Gather context and read changed files
• Review the diff against origin/main
• Post review feedback

Overall: The core change (blocklist → whitelist in _widgets_from_user_agent) is a solid improvement — unknown clients defaulting to text-only is the right call. Two issues to address:

1. Stale docstring in client_supports_widgets (bug)

The docstring at lines 117–126 still describes the old blocklist behavior:

    3. **User-Agent fallback** (stateless HTTP mode):
       When ``client_params`` is ``None`` (stateless HTTP — no MCP initialize
       handshake), we check the HTTP User-Agent header.  Clients known to
       NOT support widgets (e.g. Claude Code) are excluded.  If the
       User-Agent is unknown, we default to **showing widgets** because
       HTTP mode traffic is overwhelmingly from Claude.ai/Desktop.

    Unknown clients default to **no widget** in stateful mode (tier 2),
    but to **widget** in stateless HTTP mode (tier 3) where the population
    is predominantly Claude.ai/Desktop.

This is now wrong — tier 3 also defaults to no widgets for unknown UAs. This should be updated to match the new whitelist logic, otherwise the next person reading this will be misled about the fallback behavior.

Fix this →

2. is_internal_client broadening — intentional?

Changing "everyrow-cc" → "everyrow" is a wider match. The PR body and the UA comment (line 178) say the internal client now sends "everyrow/1.0", so this is consistent. Just confirming: this means any future UA containing the substring "everyrow" (e.g. "everyrow-analytics/1.0") would also be treated as internal. That's likely fine given you control the namespace, but worth being aware of.

Looks good

• Whitelist approach with _WIDGET_UA_SUBSTRINGS = {"claude-user"} and .lower() comparison is clean
• The inline UA documentation is thorough and useful for future maintainers
• Logic is simple and easy to follow

@claude code review

Claude finished @RafaelPo's task in 45s —— View job

Code Review: Switch widget UA detection to whitelist

• Gather context and read changed files
• Review the diff for correctness, security, and patterns
• Post review feedback

Overall: Clean PR, ready to merge. The previous review concerns (stale docstring, missing tests) have both been addressed in follow-up commits. No new issues found.

Whitelist logic (tool_helpers.py:158-185): Correct and straightforward. The _WIDGET_UA_SUBSTRINGS = {"claude-user"} with .lower() comparison is a solid fail-closed default — unknown clients get text-only, which is the safe choice.

Docstring (tool_helpers.py:117-125): Now accurately reflects the whitelist behavior. Fixed in e4a33de.

is_internal_client broadening (tool_helpers.py:192): Matching "everyrow" instead of "everyrow-cc" is consistent with the observed UA "everyrow/1.0" and forward-compatible with any future everyrow-* clients. No issues.

Tests (test_ua_detection.py): Good parameterized coverage for both _widgets_from_user_agent and is_internal_client, including edge cases (empty string, substring matches, case variations). Addresses the gap called out in earlier reviews.

LGTM.