Fix _submission_ui_json RuntimeError in no-auth HTTP mode

[RafaelPo]

Summary

• _submission_ui_json raised RuntimeError when get_access_token() returned None in --no-auth HTTP mode, because ownership recording was gated only on settings.is_http
• Added require_auth field to Settings (default True), set to False by --no-auth CLI flag
• Changed guard in tool_helpers.py from if settings.is_http to if settings.is_http and settings.require_auth to skip ownership recording in no-auth mode
• Added _noauth_http_state test fixture and test_submit_task_noauth_http e2e test

Test plan

• Existing tests pass (316 passed, 25 skipped)
• New test test_submit_task_noauth_http verifies task submission succeeds without an access token
• Manual: scripts/run-no-auth.sh + scripts/mcp_call.py call everyrow_screen '...' succeeds

🤖 Generated with Claude Code

[RafaelPo]

@claude code review

[github-actions]

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

---

Code Review: Fix _submission_ui_json RuntimeError in no-auth HTTP mode

• Read changed files and PR diff
• Analyze downstream ownership checks
• Provide review feedback

---

Overall: The core fix in tool_helpers.py is correct — guarding with settings.require_auth prevents the RuntimeError during task submission. However, the ownership system has two downstream paths that also need the same guard, otherwise tasks submitted in no-auth mode will become inaccessible.

---

Bug: _check_task_ownership also needs require_auth guard

tools.py:76 currently skips ownership checks only for non-HTTP mode:

async def _check_task_ownership(task_id: str) -> list[TextContent] | None:
    if not settings.is_http:
        return None

    owner = await redis_store.get_task_owner(task_id)
    if not owner:
        # ← This fires in no-auth mode since no owner was stored
        return [TextContent(type="text", text="Access denied: ...")]

Since _submission_ui_json now skips store_task_owner in no-auth mode, any later call to everyrow_progress or everyrow_results will hit the if not owner branch and return "Access denied". The user can submit a task but can never retrieve its progress or results.

Fix: Apply the same guard pattern:

if not settings.is_http or not settings.require_auth:
    return None

Fix this →

---

Bug: _validate_task_owner in REST routes also needs the same guard

routes.py:76-96 validates ownership on the REST api_progress and api_download_csv endpoints:

async def _validate_task_owner(task_id: str) -> JSONResponse | None:
    owner = await redis_store.get_task_owner(task_id)
    if not owner:
        # ← Also fires in no-auth mode → 403
        return JSONResponse({"error": "Task ownership could not be verified"}, status_code=403, ...)

In no-auth mode, the session widget's progress polling will 403 for the same reason — no owner was stored at submission time.

Fix: Skip the entire ownership validation when require_auth is False:

async def _validate_task_owner(task_id: str) -> JSONResponse | None:
    if not settings.require_auth:
        return None
    ...

Fix this →

---

Testing gap: no coverage for downstream paths in no-auth mode

The new test_submit_task_noauth_http test verifies submission succeeds, which is great. But the downstream breakage described above means that progress/results calls would fail in no-auth mode. Consider adding a test that calls everyrow_progress and/or accesses the REST progress endpoint after submission in _noauth_http_state.

---

Minor: test fixture duplication

The _noauth_http_state and _http_state fixtures share ~20 lines of tool-swap boilerplate (remove tool, re-register HTTP variant, yield, restore stdio variant). Consider extracting the tool swap into a shared helper to reduce duplication. Not blocking.

---

Summary

Finding	Severity	Status
_check_task_ownership needs require_auth guard	Bug	Needs fix
_validate_task_owner REST route needs require_auth guard	Bug	Needs fix
Missing test coverage for progress/results in no-auth mode	Testing gap	Recommended
Fixture duplication	Nit	Optional

The submission fix itself is correct and well-motivated. The two downstream bugs should be addressed before merging to avoid a "submit works but nothing else does" experience in --no-auth mode.

[sentry]

Bug: In no-auth HTTP mode, task submission correctly skips storing an owner, but follow-up calls like everyrow_progress incorrectly still perform an ownership check, causing them to fail.
_{Severity: MEDIUM}

Suggested Fix

In tools.py, update the condition in _check_task_ownership() to also check for settings.require_auth. Change if not settings.is_http: to if not settings.is_http or not settings.require_auth:. This will bypass the ownership check when authentication is disabled, aligning it with the task submission logic.

Prompt for AI Agent

Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: everyrow-mcp/src/everyrow_mcp/tool_helpers.py#L142-L145

Potential issue: The new code correctly skips recording task ownership when running in
no-auth HTTP mode (`require_auth=False`). However, the corresponding ownership check in
`_check_task_ownership()` was not updated. This function is called by tools like
`everyrow_progress()` and `everyrow_results_http()`. It still attempts to verify
ownership if `settings.is_http` is true, without considering `settings.require_auth`. As
a result, after submitting a task in no-auth mode, any attempt to check its progress or
retrieve results will fail with an "Access denied" error because no owner was ever
stored.

_{Did we get this right? 👍 / 👎 to inform future reviews.}