Skip to content

refactor: delegate runtime maintenance admission - #519

Merged
delete-cloud merged 1 commit into
mainfrom
runtime-maintenance-admission-service
Jun 3, 2026
Merged

delete-cloud merged 1 commit into
mainfrom
runtime-maintenance-admission-service

Conversation

@delete-cloud

@delete-cloud delete-cloud commented Jun 3, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • add RuntimeMaintenanceAdmissionService for exclusive maintenance admission checks
  • delegate runtime config replacement, checkpoint capture, and checkpoint restore admission from SessionManager
  • add focused service tests and update ADR-0058 follow-up status

Tests

  • uv run pytest tests/coding_agent/test_runtime_maintenance_admission_service.py -v
  • uv run pytest tests/ui/test_session_manager_runtime.py -k "checkpoint or runtime_config or turn_in_progress" -v
  • uv run pytest tests/ui/test_http_server_failover.py -k "events or event_queue or owner" -v
  • uv run pytest tests/ui/test_session_manager_public_api.py -k "event_queue or close_session or clear_sessions" -v
  • uv run ruff check src/coding_agent/runs/lifecycle.py src/coding_agent/runs/init.py src/coding_agent/server/session_manager.py tests/coding_agent/test_runtime_maintenance_admission_service.py
  • uv run ruff format --check src/coding_agent/runs/lifecycle.py src/coding_agent/runs/init.py src/coding_agent/server/session_manager.py tests/coding_agent/test_runtime_maintenance_admission_service.py
  • uv run python -c "from coding_agent.runs import RuntimeMaintenanceAdmissionService; print(RuntimeMaintenanceAdmissionService.name)"
  • git diff --check

Notes

  • Preserves existing maintenance admission order: active lock rejection, exclusive lock, owner assertion, session load, active-turn rejection, then operation body.
  • Does not change checkpoint payload shape, runtime replacement internals, HTTP event streams, daemon CLI, or executor selection.

Summary by CodeRabbit

  • Chores

    • Refactored internal session maintenance operation handling to use a dedicated admission service for improved concurrency control.
  • Tests

    • Added comprehensive test coverage for session maintenance admission logic.
  • Documentation

    • Updated architecture documentation to reflect current implementation status.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a2a5361-3bbf-47e2-b2f8-d495aa4a6336

📥 Commits

Reviewing files that changed from the base of the PR and between 5a772a6 and b5cdc3e.

📒 Files selected for processing (6)
  • coding-agent/.opencode/prompts/tasks/runtime-maintenance-admission-service-task-packet.md
  • coding-agent/docs/adr/0058-local-daemon-control-plane-executor-architecture.md
  • coding-agent/src/coding_agent/runs/__init__.py
  • coding-agent/src/coding_agent/runs/lifecycle.py
  • coding-agent/src/coding_agent/server/session_manager.py
  • coding-agent/tests/coding_agent/test_runtime_maintenance_admission_service.py

📝 Walkthrough

Walkthrough

This PR introduces RuntimeMaintenanceAdmissionService to centralize exclusive admission checks for session maintenance operations, refactoring three SessionManager methods to delegate their locking and validation logic through the new service instead of managing locks and state checks inline.

Changes

Runtime Maintenance Admission Service

Layer / File(s) Summary
Task planning and architecture documentation
.opencode/prompts/tasks/runtime-maintenance-admission-service-task-packet.md, docs/adr/0058-local-daemon-control-plane-executor-architecture.md
Task packet documents scope, affected components, test targets, and implementation policy for the admission service. ADR-0058 is updated to mark runtime maintenance admission (config replacement, checkpoint capture/restore) as completed and delegated to the new service.
Maintenance admission service definition and public exports
src/coding_agent/runs/lifecycle.py, src/coding_agent/runs/__init__.py
RuntimeMaintenanceAdmissionSession and RuntimeMaintenanceAdmissionBody type aliases define the admission session and maintenance body callback shape. RuntimeMaintenanceAdmissionService dataclass with run_exclusive() method acquires the per-session turn lock, asserts ownership, rejects in-progress turns (including incomplete tasks), loads the admission session, and executes the body under mutual exclusion. New symbols are exported from runs.
SessionManager refactoring to use maintenance admission service
src/coding_agent/server/session_manager.py
SessionManager.__init__ constructs _runtime_maintenance_admission by wiring existing turn lock, ownership assertion, and session loader helpers. replace_session_runtime_config(), capture_checkpoint(), and restore_checkpoint() are refactored to execute their logic inside run_exclusive() callbacks instead of managing turn locks and state validation inline.
Comprehensive service test coverage
tests/coding_agent/test_runtime_maintenance_admission_service.py
Test module defines FakeTask and FakeSession test doubles and a _service() factory to construct the service with mocked dependencies. Tests verify the service acquires and releases the turn lock correctly, rejects execution when the lock is already held, rejects execution when the session is in progress or has an incomplete task, and permits execution when session state is safe.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • delete-cloud/Coding-Agent-Loop#439: Both PRs modify SessionManager.replace_session_runtime_config()—this one routes it through RuntimeMaintenanceAdmissionService admission checks, while #439 routes the local runtime rebuild through LocalDaemonExecutor.prepare_runtime().

Poem

A rabbit hops through locks with care,
Exclusive turns beyond compare,
Check sessions in, check tasks are done,
Maintenance flows for everyone. 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title 'refactor: delegate runtime maintenance admission' accurately and concisely describes the main change: introducing a new RuntimeMaintenanceAdmissionService and delegating maintenance admission checks from SessionManager to it.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch runtime-maintenance-admission-service

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@delete-cloud

Copy link
Copy Markdown
Owner Author

Local fallback review because CodeRabbit has remained pending with only the auto "review in progress" comment.

Manual P1/P2 review result: no blocking findings.

Checked:

  • RuntimeMaintenanceAdmissionService.run_exclusive() preserves the prior maintenance admission order: active lock rejection, exclusive lock, owner assertion, session load, active-turn rejection, operation body.
  • replace_session_runtime_config(), capture_checkpoint(), and restore_checkpoint() keep their existing operation bodies and only delegate the admission wrapper.
  • capture_checkpoint() still runs ensure_session_runtime() inside the exclusive turn lock and preserves checkpoint metadata serialization from the admitted session.
  • No changes to HTTP event queue attach, event append, disconnect cleanup, daemon CLI, executor selection, or checkpoint payload schema.

Verification already run:

  • uv run pytest tests/coding_agent/test_runtime_maintenance_admission_service.py -v
  • uv run pytest tests/ui/test_session_manager_runtime.py -k "checkpoint or runtime_config or turn_in_progress" -v
  • uv run pytest tests/ui/test_http_server_failover.py -k "events or event_queue or owner" -v
  • uv run pytest tests/ui/test_session_manager_public_api.py -k "event_queue or close_session or clear_sessions" -v
  • uv run ruff check src/coding_agent/runs/lifecycle.py src/coding_agent/runs/__init__.py src/coding_agent/server/session_manager.py tests/coding_agent/test_runtime_maintenance_admission_service.py
  • uv run ruff format --check src/coding_agent/runs/lifecycle.py src/coding_agent/runs/__init__.py src/coding_agent/server/session_manager.py tests/coding_agent/test_runtime_maintenance_admission_service.py
  • uv run python -c "from coding_agent.runs import RuntimeMaintenanceAdmissionService; print(RuntimeMaintenanceAdmissionService.__name__)"
  • git diff --check

@delete-cloud
delete-cloud merged commit 1c52515 into main Jun 3, 2026
1 check passed
@delete-cloud
delete-cloud deleted the runtime-maintenance-admission-service branch June 3, 2026 10:53
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