Skip to content

Stop workers running tasks whose query is past its timeout - #150

Merged
maximusunc merged 1 commit into
mainfrom
claude/task-timeout-validation-icx28v
Sep 1, 2026
Merged

Stop workers running tasks whose query is past its timeout#150
maximusunc merged 1 commit into
mainfrom
claude/task-timeout-validation-icx28v

Conversation

@maximusunc

Copy link
Copy Markdown
Collaborator

Workers took whatever the stream handed them and ran it, however long ago the query was submitted. The ARS and the other external callers stop waiting after ~5 minutes and the sync /query endpoint gives up around the same point, so a query that fell behind kept moving from operation to operation producing an answer nobody would receive -- taking worker slots and process-pool children from queries that could still be answered, and leaving its shepherd_brain row non-terminal until the monitor's abandoned-query reaper eventually swept it.

The server now stamps each query with an absolute deadline at intake and that deadline travels with the task through every hop (wrap_up_task, handle_task_failure, _terminate_task). get_tasks checks it as a task is picked up -- on freshly delivered and reclaimed messages alike -- and when the budget is spent it skips the operation, drops the rest of the workflow and routes the query to finish_query with a TIMEOUT status. The query then ends exactly as any completed query does: terminal state in Postgres, callback rows reaped, logs saved, and whatever was gathered POSTed to the callback URL. A sync caller gets a partial response instead of waiting out its own timeout for nothing.

Details:

  • QUERY_TIMEOUT_SEC (default 300) is the budget; 0 restores the old unbounded behavior. A client asking to wait longer via TRAPI parameters.timeout is not cut short -- the larger of the two wins.
  • finish_query is exempt (it is the wrap-up) and so is merge_message, which folds in callbacks upstream has already paid for.
  • Tasks carrying no deadline are never expired, so a rollout with in-flight queries is safe.
  • The explanation is flushed to the query's log list before the hand-off, since finish_query reads those logs into the response it delivers.

Claude-Session: https://claude.ai/code/session_019VD7RfD8K98mhHVqWczWMC

Workers took whatever the stream handed them and ran it, however long ago
the query was submitted. The ARS and the other external callers stop
waiting after ~5 minutes and the sync /query endpoint gives up around the
same point, so a query that fell behind kept moving from operation to
operation producing an answer nobody would receive -- taking worker slots
and process-pool children from queries that could still be answered, and
leaving its shepherd_brain row non-terminal until the monitor's
abandoned-query reaper eventually swept it.

The server now stamps each query with an absolute deadline at intake and
that deadline travels with the task through every hop (wrap_up_task,
handle_task_failure, _terminate_task). get_tasks checks it as a task is
picked up -- on freshly delivered and reclaimed messages alike -- and when
the budget is spent it skips the operation, drops the rest of the
workflow and routes the query to finish_query with a TIMEOUT status. The
query then ends exactly as any completed query does: terminal state in
Postgres, callback rows reaped, logs saved, and whatever was gathered
POSTed to the callback URL. A sync caller gets a partial response instead
of waiting out its own timeout for nothing.

Details:
- QUERY_TIMEOUT_SEC (default 300) is the budget; 0 restores the old
  unbounded behavior. A client asking to wait longer via TRAPI
  parameters.timeout is not cut short -- the larger of the two wins.
- finish_query is exempt (it *is* the wrap-up) and so is merge_message,
  which folds in callbacks upstream has already paid for.
- Tasks carrying no deadline are never expired, so a rollout with
  in-flight queries is safe.
- The explanation is flushed to the query's log list before the hand-off,
  since finish_query reads those logs into the response it delivers.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019VD7RfD8K98mhHVqWczWMC
@codecov

codecov Bot commented Sep 1, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.29630% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 55.31%. Comparing base (4f207ed) to head (541a94a).
⚠️ Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
shepherd_utils/shared.py 90.32% 1 Missing and 2 partials ⚠️
Files with missing lines Coverage Δ
shepherd_server/base_routes.py 62.82% <100.00%> (+9.35%) ⬆️
shepherd_utils/config.py 95.41% <100.00%> (+0.08%) ⬆️
shepherd_utils/task_deadline.py 100.00% <100.00%> (ø)
shepherd_utils/shared.py 79.92% <90.32%> (+2.89%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 013842f...541a94a. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@maximusunc
maximusunc merged commit 5d9879c into main Sep 1, 2026
2 checks passed
@maximusunc
maximusunc deleted the claude/task-timeout-validation-icx28v branch September 1, 2026 19:22
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