Skip to content

Skip widget, session URL, and page_size prompt for internal clients#240

Merged
RafaelPo merged 2 commits intomainfrom
skip-widget-session-internal-clients
Feb 26, 2026
Merged

Skip widget, session URL, and page_size prompt for internal clients#240
RafaelPo merged 2 commits intomainfrom
skip-widget-session-internal-clients

Conversation

@RafaelPo
Copy link
Contributor

Summary

  • Detect EveryRow CC app via User-Agent: everyrow-cc and skip widget JSON, session URL, and submission widget from all responses (submission, progress, results)
  • Extract task ownership recording from widget builder so it still runs for internal clients (fixes "Access denied: task ownership could not be verified")
  • Add auto_page_size_threshold setting (default 50, env AUTO_PAGE_SIZE_THRESHOLD) — when total rows <= threshold, agent loads all rows directly instead of asking the user for page_size
  • Update system instructions to not mention session_url when absent from response

Test plan

  • All 372 existing tests pass
  • Send request with User-Agent: everyrow-cc/1.0 — no widget JSON, no session_url, no "ask the user" prompt for small results
  • Send request with User-Agent: claude-code/2.1 — no widget (existing behavior), session_url still present
  • Send request with default UA — widget + session_url present (default behavior)
  • Verify progress/results work for internal clients (task ownership recorded correctly)

🤖 Generated with Claude Code

Detect requests from EveryRow CC (via User-Agent) and strip widget JSON,
session URL references, and submission widget from responses. This saves
bandwidth and context tokens for our own app which has its own UI.

Also adds auto_page_size_threshold setting (default 50, configurable via
AUTO_PAGE_SIZE_THRESHOLD env var) — when total rows are at or below the
threshold, the agent loads all rows directly instead of asking the user.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@RafaelPo
Copy link
Contributor Author

@claude code review

@github-actions
Copy link
Contributor

github-actions bot commented Feb 26, 2026

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


Code Review

Reviewed all 7 changed files. The core design — extracting _record_task_ownership from the widget builder and gating on is_internal_client() — is clean and solves the "Access denied" bug nicely. A few issues below, one potential bug and a few improvements.


Bug: page_size=0 when task completes with 0 rows

tool_helpers.py:436-438 — When a task completes with total=0, the condition self.total <= settings.auto_page_size_threshold is True, producing:

Call everyrow_results(task_id='...', page_size=0) to load all rows.

But HttpResultsInput.page_size has ge=1, so the model's call will fail validation. Guard with max(self.total, 1):

if self.total <= settings.auto_page_size_threshold:
    next_call = dedent(f"""\
        Call everyrow_results(task_id='{task_id}', page_size={max(self.total, 1)}) to load all rows.""")

Fix this →


Medium: "via the widget" text still shown to internal clients

Several places reference "the widget" in model-facing text that internal clients will see:

  1. tool_helpers.py:442 — Progress completion for large results: "You will have access to all of them via the widget."
  2. result_store.py:164 — Result pagination: "but they have access to all {total} rows via the widget above."
  3. app.py:141-143 — System instructions: "via the widget and download link" / "via the widget above and the download link"

Since internal clients never see widgets, the downstream model may hallucinate widget references to the user. Consider either:

  • Conditioning these messages on is_internal_client() (more work), or
  • Noting this as a known limitation and addressing in a follow-up

Minor: is_internal_client() called redundantly per request

is_internal_client() is called independently in:

  • _submission_text() (tool_helpers.py:208)
  • create_tool_response() (tool_helpers.py:295)
  • everyrow_results_http() (tools.py:889)

Each call does a lazy import + get_user_agent(). Consider computing it once and passing as a parameter, or at minimum caching the result via a request-scoped mechanism. Not a correctness issue, but it's easy to get out of sync if someone adds a new call site with different logic.


Minor: "everyrow-cc" not added to _NO_WIDGET_UA_SUBSTRINGS

_widgets_from_user_agent() (tool_helpers.py:180) blocklists "claude-code" but not "everyrow-cc". So client_supports_widgets() returns True for everyrow-cc in stateless HTTP mode, and the caller in everyrow_results_http has to override it:

skip_widget = not client_supports_widgets(ctx)  # False (thinks it supports widgets)
if is_internal_client():
    skip_widget = True  # override

This works but is fragile. Adding "everyrow-cc" to _NO_WIDGET_UA_SUBSTRINGS would make client_supports_widgets self-consistent and remove the need for the override in the results handler.

Fix this →


Nit: _build_instructions_http() rebuilds on every call

Previously _INSTRUCTIONS_HTTP was a module-level constant. Now _build_instructions_http() rebuilds the string on every get_instructions(is_http=True) call. Since settings is @lru_cache-d and the threshold never changes at runtime, you could cache this too (e.g. @lru_cache on the function). Very minor — only matters if instructions are fetched frequently.


What looks good

  • Ownership extraction (_record_task_ownership): Clean separation. Running it for all HTTP clients while gating the widget is exactly right — this fixes the root cause of the "Access denied" error.
  • auto_page_size_threshold config: Well integrated — config, system instructions, progress message, and model description all updated consistently.
  • Test update (test_stdio_content.py:703-708): Good that the test was updated to match the new threshold behavior.
  • skip_session threading through result_store.py: Clean — the flag is passed through all layers without leaking into unrelated logic.
  • System instructions update (app.py:104): The conditional "If a session_url appears..." is a good pattern — makes the model self-adapting rather than relying on client-specific instruction variants.

|

- Guard against total=0 producing page_size=0 (fails ge=1 validation)
- Add "everyrow-cc" to _NO_WIDGET_UA_SUBSTRINGS so client_supports_widgets
  is self-consistent without needing an override in the results handler

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@RafaelPo RafaelPo requested a review from jackwildman February 26, 2026 19:15
@RafaelPo RafaelPo merged commit 21e9137 into main Feb 26, 2026
5 checks passed
@RafaelPo RafaelPo deleted the skip-widget-session-internal-clients branch February 26, 2026 19:15
Comment on lines +189 to +193
def is_internal_client() -> bool:
"""Return True if the request comes from EveryRow's own app (CC)."""
from everyrow_mcp.http_config import get_user_agent # noqa: PLC0415

return "everyrow-cc" in get_user_agent().lower()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if you run the MCP in Claude Code now? Is that not also going to have the same problem? Is it possible to do this where such that it's json by default, and it's the HTML-requiring clients (e.g. the connector) that announce themselves?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants