fix(rpc): stop the owned skill watcher during teardown

[LivXue]

Summary

An RPC stack that built its own engine left the skill watcher running after teardown. Stop that watcher before draining the owned stores, and continue the remaining cleanup if stopping it raises.

A stack mounted on a host-owned engine leaves the host's watcher running. The regression uses a real local watcher and checks repeated teardown and failure isolation.

Type

• Fix
• Feature
• Docs
• CI / tooling
• Refactor
• Other

Verification

• Relevant tests pass locally
• Relevant lint / type checks pass locally
• User-facing docs or screenshots are updated when needed

Native Windows, Python 3.12. PYTHONUTF8=1; the existing environment was selected through UV_PROJECT_ENVIRONMENT, and PYTHONPATH pointed at this checkout.

uv run --no-sync pytest -o addopts= tests/test_rpc_bootstrap.py -k "skill_watcher or failed_watcher" -q --tb=short --show-capture=no

Result: 3 passed in the targeted selection. The broader RPC/serve selection previously produced 136 passed, 2 failed and 26 skipped on Windows. Both failures reproduced on the unmodified base: owner-only POSIX mode bits and the supervisor SIGTERM cleanup test. The relevant-tests box is left unchecked because that broader selection is not all green.

Whole-branch gates:

$changed = @(git diff --name-only origin/main...HEAD -- '*.py')
$production = @($changed | Where-Object { $_ -like 'raven/*' })
uv run --no-sync ruff check @changed
uv run --no-sync ruff format --check @changed
uv run --no-sync ty check @production
uv run --no-sync lint-imports
uv run --no-sync pre-commit run --from-ref origin/main --to-ref HEAD
uv run --no-sync python scripts/check_commit_messages.py origin/main..HEAD
uv run --no-sync python scripts/check_source_language.py origin/main...HEAD
uv run --no-sync python scripts/check_large_files.py origin/main...HEAD
git diff --check origin/main...HEAD

All listed gates passed. The whole-branch review covered callers, ownership, guard boundaries, failure paths, regression strength, architecture and compatibility. No screenshot change applies; directly affected code documentation is updated where needed. The full repository suite and Linux/macOS execution were not run locally; the local Docker Linux engine was unavailable.

Risk

• Security impact considered
• Backward compatibility considered
• Rollback path is clear for risky changes

Cleanup applies only to the engine owned by this RPC stack. Mounted stacks retain host ownership. Stop failures are logged without preventing the remaining resources from closing. Revert the squash commit to roll back.

Related Issues

#490

[gloryfromca]

No blockers; this can merge as far as I am concerned.

Reviewed the refreshed github/main...HEAD diff and the surrounding RPC teardown, skill watcher lifecycle, other host cleanup paths, and relevant history. The owned/mounted boundary is preserved, repeated teardown is safe, and watcher-stop failures do not prevent store or transport cleanup. I also checked the repository rules and domain context, backward compatibility, callers, and that the tests add coverage rather than weakening existing assertions.

Verification: uv sync --locked --all-extras --dev succeeded; uv run pytest tests/test_rpc_bootstrap.py -q passed (24 passed in 6.01s). An initial run before syncing extras failed at fixture setup because raven_everos was absent; no tests executed in that attempt.

[LivXue]

Windows compatibility review: no new issue found in the inspected scope.

Reviewed head ffc62d05b136d2cecb013f3754fcc98a41c1a4ab against base 4acc63cebd0752652194f9e4989628da29a8a25d: both changed files, RPC teardown callers, and the skill watcher's stop/join implementation. Checked a real Windows filesystem watcher stopping for an owned stack, remaining active for a mounted stack, repeated teardown, and cleanup continuing after a stop failure.

Native Windows / Python 3.13.12, in a detached temporary checkout with PYTHONUTF8=1, UV_NO_SYNC=1 and isolated user/Raven homes:

uv run pytest tests/test_rpc_bootstrap.py -n 0 -x --tb=short --basetemp "$reviewRunDir/pr910/tests"

Result: 24 passed. No long-running server, large watched tree, or full repository suite was exercised. This scoped review does not establish full Windows compatibility.

[LivXue]

No blockers in the reviewed scope.

Scope: I reviewed the full PR diff (2 changed files), including tests and documentation, and traced affected callers and contracts at ffc62d05b136. This review covers all changed areas, not only raven/agent.

Checked owned versus mounted loops, producer-before-drain shutdown order, repeated teardown, and failure continuation. A mounted loop returns before the new watcher cleanup; an owned watcher is stopped without preventing later store or transport cleanup if it raises.

Verification (native Windows, Python 3.12, isolated source snapshots; local JUnit-output options omitted below):

• PR head: uv run --no-sync pytest -o addopts= tests/test_rpc_bootstrap.py tests/test_skill_watcher_cleanup.py -q --tb=short -> 28 passed.

The focused suites include the real SkillFileWatcher ownership/stop checks, repeated teardown, and cleanup after a watcher-stop exception. No new finding was identified. I also read the Python 3.13 Windows review. Its conclusions are consistent; the measurements here record a separate Python 3.12 run.

git diff --check over the complete merge-base-to-head range passed. No new inline finding is being filed.