Skip to content

fix(tasks): respect BACKGROUND_TASK_FOREGROUND_GATE in heartbeat-triggered background task stops#5538

Open
maxmilian wants to merge 1 commit into
odysseus-dev:devfrom
maxmilian:fix/5536-heartbeat-gate
Open

fix(tasks): respect BACKGROUND_TASK_FOREGROUND_GATE in heartbeat-triggered background task stops#5538
maxmilian wants to merge 1 commit into
odysseus-dev:devfrom
maxmilian:fix/5536-heartbeat-gate

Conversation

@maxmilian

Copy link
Copy Markdown
Contributor

Summary

stop_background_tasks_for_foreground() cancelled every executing scheduler task unconditionally, so browser heartbeats aborted scheduled tasks even with BACKGROUND_TASK_FOREGROUND_GATE=false — the /api/activity/heartbeat call site is not gated, unlike the interactive middleware (which already no-ops via should_track_interactive_request). This PR gates the method itself on the env gate so every call site (and any future one) honors the opt-out. It also fixes the two secondary symptoms from the issue: runs cancelled by this path recorded the misleading "Stopped by user" instead of "Paused because Odysseus became active" and missed the 15-minute foreground defer (only the in-task monitor set foreground_cancel["hit"]) — externally stopped task ids are now tracked in _foreground_stops so the CancelledError path applies the correct message + defer, with cleanup at the task-level finally so the flag can't leak from a queued-behind-the-semaphore cancel and misclassify a later genuine user stop. Finally, docker-compose.yml (and both standalone GPU variants) now pass BACKGROUND_TASK_* through to the container, so .env settings actually reach it.

Target branch

  • This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release. If your PR is on main by accident, click "Edit" on this PR and change the base.

Linked Issue

Fixes #5536

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)

Checklist

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.

How to Test

Automated: python -m pytest tests/test_foreground_gate_heartbeat_stop.py — 4 regression tests, all red on unmodified dev (except the gate-on guard) and green here. Full suite: 4619 passed; the 5 remaining failures (docker-socket / workspace-confine) reproduce identically on unmodified dev on the same machine — pre-existing, unrelated.

Live app verification (what I ran, uvicorn app:app with a black-hole LLM endpoint so a task hangs mid-run):

  1. Start with the gate off: BACKGROUND_TASK_FOREGROUND_GATE=false uvicorn app:app --port 7801, create an LLM task whose endpoint_url points at a TCP socket that accepts and never responds, POST /api/tasks/{id}/run, wait for the run to reach running.
  2. Fire POST /api/activity/heartbeat a few times (what an open browser tab does).
    • On unmodified dev: the run flips to aborted with "Stopped by user" — the bug.
    • With this PR: the run stays running; heartbeats no longer touch background tasks while the gate is off.
  3. Restart with the gate on (default), repeat: the run is aborted as designed, but now records "Paused because Odysseus became active" (and the task is deferred ~15 min) instead of pretending the user stopped it.
  4. docker compose config renders the new BACKGROUND_TASK_* env passthroughs; setting BACKGROUND_TASK_FOREGROUND_GATE=false in .env now reaches the container.

User-facing stop is unchanged: stop_task (the /api/tasks/{id}/stop endpoint) still records "Stopped by user" with normal rescheduling — covered by the existing test_task_scheduler_cancel.py plus a new leak-regression test for the queued-cancel path.

Visual / UI changes — REQUIRED if you touched anything that renders

N/A — backend only (src/task_scheduler.py, src/interactive_gate.py, compose files, tests). No rendering surface touched.

  • I am not an LLM agent submitting a bulk PR. If you are, please open an issue describing the problem first — bulk auto-generated PRs that don't match the project's visual style are closed on sight, even when the underlying fix is correct.

Screenshots / clips

N/A — backend only.

…gered background task stops

stop_background_tasks_for_foreground() cancelled every executing scheduler
task unconditionally, so browser heartbeats aborted scheduled tasks even
with BACKGROUND_TASK_FOREGROUND_GATE=false (the /api/activity/heartbeat
call site is not gated, unlike the interactive middleware). Gate the
method itself so all call sites honor the opt-out.

Runs cancelled by this path also recorded the misleading 'Stopped by
user' and missed the 15-minute foreground defer, because only the
in-task monitor set foreground_cancel['hit']. Track externally stopped
task ids in _foreground_stops so the CancelledError path applies the
foreground pause message and defer semantics.

Also pass BACKGROUND_TASK_* through docker-compose so .env settings
reach the container.

Fixes odysseus-dev#5536

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@github-actions github-actions Bot added the ready for review Description complete — ready for maintainer review label Jul 14, 2026
@maxmilian maxmilian marked this pull request as ready for review July 14, 2026 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Description complete — ready for maintainer review

Projects

None yet

1 participant