Graceful shutdown for supervised background tasks (6/13)

[alex-clickhouse]

What

stop_background_task in nerve/utils/aio.py: signal a supervised background task's stop event, wait a bounded moment for it to wind itself down, and cancel only if it doesn't. The external-agents sync service uses it, and the workspace-sync loop in #212 reuses the same discipline.

Why

Every other PR in this stack protects one invariant: the config the daemon is running is a known state. It is validated before it lands (#212), applied as a unit, and reported per subsystem when only part of it took (#215). This PR protects that invariant on the teardown path, which nothing else in the stack covers.

The case that makes it concrete is the external-agents sweep. It renders config-derived instruction bundles to disk — ~/.codex/AGENTS.md, ~/.claude/CLAUDE.md, and so on — one target at a time. Killed halfway down that list it leaves some files regenerated from the current sources and some still holding the previous render, on disk, with nothing recording which is which. The loop waits before it sweeps rather than after, so once the daemon comes back the inconsistency sits there for a full external_agents.sync_interval_minutes (default 15) before anything corrects it. That is exactly the half-old/half-new state the rest of the stack refuses to produce, reached by the one path none of its guarantees cover.

This stack also changes how often that matters. Before it, config changed when someone edited a file on the box and restarted, and a sweep being mid-flight at that moment was a coincidence. After it, config arrives by sync on a one-minute cadence and every merge triggers a reload that re-renders those bundles, so something config-derived is far more often in flight when the process is asked to stop. And because part of the config genuinely cannot be applied in place — that is what #215's restart_required reports — restarting right after a config change becomes the documented workflow rather than a rare event. A stack whose premise is "config changes apply continuously and safely" should not leave "unless you restart at the wrong moment" as an unwritten footnote.

Setting a stop event and cancelling in the same breath makes the event decorative, which is how the bug arises: the CancelledError is delivered before the coroutine next looks at the event, so the task dies wherever it happened to be suspended instead of at the end of the cycle it was in. Cancellation stays as the backstop, so a wedged task still cannot hold shutdown open indefinitely, and it logs when it has to fall back to one.

The helper lives in nerve.utils.aio rather than the gateway lifespan so a service can shut its own background task down without importing the FastAPI app. That is what lets the external-agents service own its stop() instead of having the lifespan reach into it.

Scope change from the original version of this PR

This PR was "Auto-reload cron config on file change" — a watchfiles watcher over the cron directory, gated by cron.auto_reload (default on). The watcher has been dropped. The graceful-shutdown fix it happened to carry is what remains.

Why it was dropped: nothing lands in the cron directory until workspace sync merges it, and at that moment the sync loop is already there and reloads. So for git-delivered config the watcher bought no latency at all, and its only unique coverage was an out-of-band local write. Against that it cost ~980 lines — a 708-line test file, 201 lines in CronService, the only @pytest.mark.slow in the suite plus the CI step and conftest marker that existed to serve it — and a standing maintenance surface of filesystem-event edge cases: parent-directory fallback for a not-yet-created directory, __pycache__ writes re-triggering the reload through the directory's own mtime, editor truncate-then-write, and platform variance in inotify delivery.

What replaces it: workspace sync (#212) applies a merged change on its own cycle, now every minute, and nerve reload (#215) applies a local edit on demand. Both were already in the stack; the watcher was a third path that had to agree with them, and the interlock keeping it from double-reloading with the sync loop is gone too.

The branch is still named config-refactor/06-file-watcher — renaming it would close this PR.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds hot-reload support for cron config by running a resilient filesystem watcher in the gateway lifespan, and improves shutdown semantics for background loops so in-flight work can finish cleanly.

Changes:

• Add CronService.watch_config() using watchfiles.awatch to reload cron jobs/plugins when cron config files/directories change, gated by cron.auto_reload.
• Introduce a shared stop_background_task() helper to prefer cooperative stop-signaling over immediate cancellation (used by cron watcher and external-agents sync).
• Add extensive test coverage for watcher behavior (including real watchfiles integration), document the new config flag, and split CI into fast vs slow test steps.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
nerve/cron/service.py	Implements cron config watcher (dir discovery, filtering, retries, catch-up reload logic).
nerve/gateway/server.py	Starts/stops the cron watcher task during FastAPI lifespan when cron.auto_reload is enabled.
nerve/utils/aio.py	Adds shared shutdown helper for background tasks (stop_background_task).
nerve/external_agents/sync_service.py	Uses cooperative stop + backstop cancellation; adds minimum sweep interval floor.
nerve/config.py	Adds cron.auto_reload config flag (default true).
tests/test_cron_watch.py	New unit + integration tests for watcher semantics and config flag.
tests/test_external_agents_sync_service.py	Adds tests to verify shutdown waits for sweep / cancels on timeout.
tests/conftest.py	Registers slow marker for pytest.
docs/cron.md	Documents auto-reload behavior and watcher wait/narrowing semantics.
docs/config.md	Documents cron.auto_reload option.
README.md	Documents running/omitting slow tests locally.
.github/workflows/ci.yml	Splits CI into non-slow and slow test steps.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

nerve/utils/aio.py:56

• stop_background_task swallows all asyncio.CancelledErrors. If the caller (e.g., lifespan shutdown) is itself cancelled, this will suppress cancellation of the caller task and can lead to shutdown code continuing unexpectedly. Only suppress CancelledError when it comes from the background task you are awaiting; if the current task is being cancelled, re-raise.

    except asyncio.CancelledError:
        # The task's cancellation, not ours: either we asked for it above or
        # something else already had. Nothing left to wait for.
        pass