Skip to content

fix(tasks): reap stale running tasks on startup + auto-purge loop - #6

Open
catoncat wants to merge 1 commit into
mainfrom
fix/auto-purge-and-reap
Open

fix(tasks): reap stale running tasks on startup + auto-purge loop#6
catoncat wants to merge 1 commit into
mainfrom
fix/auto-purge-and-reap

Conversation

@catoncat

Copy link
Copy Markdown
Owner

Why

Two persistent issues with the on-disk task store, surfaced again today after a server restart:

  1. Zombie running status. Worker threads in ExecutorRegistry are daemon=True and have no try/finally guard around the meta.json write. If the MCP process exits abruptly (launchd kill, OOM, supervisor reload, uncaught exception), the task's status sticks at running forever. get_task / wait_task then lie to consumers indefinitely.
  2. No automatic disk cleanup. purge_tasks was only an RPC tool, never scheduled. The task directory under STATE_DIR/tasks grew unbounded — today there were 38 stale dirs purged manually plus 30 more sitting around, including 2 that had been "running" for ~5 days but the actual subprocess was long dead.

What

tasks.py

  • New TaskStore.reap_stale_running_tasks(reason=...) flips every persisted queued / running entry to "abandoned" with an abandoned_reason field and bumped updated_at. Idempotent and lock-protected.
  • purge_tasks gains an optional statuses whitelist so callers can narrow purges to e.g. cancelled / failed / abandoned. When unset, behavior is identical to before.
  • New NON_TERMINAL_STATUSES = {"queued", "running"} constant for clarity.

executors.py

  • TERMINAL_TASK_STATUSES now includes "abandoned" so wait_task / get_task treat reaped tasks as terminal.

server.py

  • main() now calls store.reap_stale_running_tasks(reason="server_startup") once at startup, before binding uvicorn. Logged.
  • A daemon thread notion-local-ops-auto-purge runs store.purge_tasks(...) on a configurable interval. Knobs:
    • NOTION_LOCAL_OPS_AUTO_PURGE_INTERVAL_SECONDS (default 3600)
    • NOTION_LOCAL_OPS_AUTO_PURGE_OLDER_HOURS (default 168, i.e. 7 days)
    • Set the interval to <= 0 to disable.
  • server_info now reports auto-purge state (interval_seconds, older_than_hours, running).
  • New purge_tasks(statuses=...) argument exposed on the MCP tool.
  • New reap_stale_tasks MCP tool for manual recovery (also called automatically on startup).

Compatibility

  • Happy-path tasks are unaffected: succeeded / failed / cancelled flow exactly as before; same age-based purge by default.
  • Existing on-disk meta.json files with status other than queued / running are untouched by reap.
  • The supervisor's rolling reload triggers the reaper on the new child, which is the desired behavior — if the old child had truly-running tasks, those subprocesses were already terminated by _terminate_process before reload, so flipping them to abandoned is correct.

Manual smoke test plan

  1. Start MCP, run run_command_stream with a long-running command, then kill -9 the MCP pid.
  2. Restart MCP.
  3. get_task(<id>) should report status="abandoned", abandoned_reason="server_startup", completed=true. Previously this hung at running.
  4. After 1h (or with NOTION_LOCAL_OPS_AUTO_PURGE_INTERVAL_SECONDS=10 for fast verification), the task dir disappears once it crosses the age threshold.
  5. purge_tasks(statuses=["abandoned"], older_than_hours=0) deletes only abandoned tasks immediately.

Future work (out of scope for this PR)

  • Optional: persist the executor PID in meta.json so the reaper can verify the process is truly gone instead of trusting the missed final write.
  • Optional: emit a Notion notification (or Slack ping) when reap > 0 on startup.

Two persistent issues with the task store are addressed:

1. Zombie 'running' status. Worker threads are daemon=True with no
   try/finally guarantee that meta.json is updated on abrupt exit
   (launchd kill, OOM, supervisor reload, uncaught exception). Once
   that happens the task's status sticks at 'running' forever and
   get_task / wait_task lie to consumers. We now run
   TaskStore.reap_stale_running_tasks() on every server startup,
   flipping persisted queued/running entries to 'abandoned' with a
   reason field. 'abandoned' is added to TERMINAL_TASK_STATUSES so
   wait_task treats it as a terminal state.

2. No automatic disk cleanup. purge_tasks was only an RPC tool, never
   scheduled. We now spawn a daemon thread on startup that calls
   store.purge_tasks(...) on a configurable interval (env
   NOTION_LOCAL_OPS_AUTO_PURGE_INTERVAL_SECONDS, default 3600s) using
   a configurable retention window (env
   NOTION_LOCAL_OPS_AUTO_PURGE_OLDER_HOURS, default 168h = 7 days).
   The MCP purge_tasks tool also gains an optional 'statuses' filter
   so callers can narrow purges to e.g. cancelled/failed/abandoned.

No behavior changes for the happy path: successful tasks still
report 'succeeded' immediately and are purged on the same age basis.
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.

1 participant