Skip to content

feat: extract approval interaction service - #466

Merged
delete-cloud merged 2 commits into
mainfrom
dogfood/approval-interaction-service
Jun 3, 2026
Merged

delete-cloud merged 2 commits into
mainfrom
dogfood/approval-interaction-service

Conversation

@delete-cloud

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

Copy link
Copy Markdown
Owner

Summary

  • add ApprovalInteractionService for durable approval interaction create/resolve persistence
  • route SessionManager approval interaction persistence through the service
  • update ADR-0058 status and add focused service tests

Verification

  • uv run pytest tests/coding_agent/test_approval_interactions.py -v
  • uv run pytest tests/ui/test_session_manager_runtime.py -k "approval_interaction" -v
  • uv run pytest tests/coding_agent/test_approval_runtime_messages.py -k "submit_approval" -v
  • uv run ruff check src/coding_agent/approval src/coding_agent/server/session_manager.py tests/coding_agent/test_approval_interactions.py tests/ui/test_session_manager_runtime.py tests/coding_agent/test_approval_runtime_messages.py

Summary by CodeRabbit

  • Refactor

    • Improved internal architecture for approval interaction handling by introducing a dedicated service component to manage approval persistence and resolution.
  • Tests

    • Added comprehensive tests validating approval interaction lifecycle, including creation, resolution, status tracking, and graceful handling of edge cases.

@coderabbitai

coderabbitai Bot commented Jun 3, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@delete-cloud, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 51 minutes and 36 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 5dc43788-0715-4797-8211-96df5df7d64f

📥 Commits

Reviewing files that changed from the base of the PR and between 7c84015 and fe24bc5.

📒 Files selected for processing (2)
  • coding-agent/src/coding_agent/approval/interactions.py
  • coding-agent/tests/coding_agent/test_approval_interactions.py
📝 Walkthrough

Walkthrough

This PR extracts approval interaction persistence and resolution from SessionManager into a new ApprovalInteractionService. The service creates pending interaction records and resolves them with status/response payloads, while SessionManager retains approval-waiting and runtime-message handling. All approval request/response flows now delegate through the service.

Changes

Approval Interaction Service Extraction

Layer / File(s) Summary
ApprovalInteractionService module implementation
src/coding_agent/approval/interactions.py
New module defines ApprovalInteractionSession protocol, ApprovalInteractionService dataclass with create() and resolve() methods, helper functions for interaction IDs/status mapping/request-response payloads, and _json_compatible_value() serializer supporting dataclasses, enums, datetimes, and sequences.
Approval module re-exports
src/coding_agent/approval/__init__.py
Module imports and re-exports ApprovalInteractionService, ApprovalInteractionSession, and four helper functions, expanding the public approval API surface.
SessionManager delegation to ApprovalInteractionService
src/coding_agent/server/session_manager.py
SessionManager imports the service, removes local approval interaction helpers, adds _approval_interactions() factory, and updates all approval request/response flows (wire consumer, published approval, HTTP approval waiting) to call approval_interactions.create() and approval_interactions.resolve().
ApprovalInteractionService test coverage
tests/coding_agent/test_approval_interactions.py
New test module adds FakeSession and RecordingInteractionStore test utilities, then verifies create() persists pending interactions with correct metadata, resolve() updates status/response/timestamp, explicit status arguments override implicit derivation, and graceful no-ops when store or session turn ID is missing.
Architecture documentation and task tracking
.opencode/prompts/tasks/local-daemon-pr43-approval-interaction-service.md, docs/adr/0058-local-daemon-control-plane-executor-architecture.md
Task prompt documents refactor scope/commands; ADR-0058 marks approval interaction delegation complete and restates remaining gaps for runtime/store boundary extraction.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • delete-cloud/Coding-Agent-Loop#423: Both PRs modify SessionManager approval state handling—this PR delegates interaction create/resolve to the new service, while that PR splits process-local and pending approval runtime state into SessionRuntimeHandle, directly affecting session state projection and runtime data storage.

Poem

🐰 A service hops in to approve with care,
SessionManager delegates its share,
Pending interactions now persist with grace,
Separation of concerns finds its place,
Tests confirm the workflow's right and tight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 3.45% 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 title accurately describes the main change: extracting approval interaction handling into a dedicated service.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch dogfood/approval-interaction-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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@coding-agent/src/coding_agent/approval/interactions.py`:
- Around line 88-106: In resolve(), ensure the stored interaction key is derived
from the response rather than blindly using the standalone request_id: compare
the incoming request_id param with response.request_id and if they differ raise
an error (or replace request_id with response.request_id), then call
self.store.resolve_agent_interaction(...) using
approval_interaction_id(response.request_id, request_id) — or simply
approval_interaction_id(response.request_id, ...) — so the persisted payload
maps to the same request ID in response; update the resolve() implementation
(function resolve, symbols: ApprovalInteractionSession.current_turn_id,
approval_interaction_id, approval_interaction_status, approval_response_payload)
to perform this validation and use the validated/derived request_id.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 78f3dd29-858f-45c7-bf6c-c6f5f97bf19e

📥 Commits

Reviewing files that changed from the base of the PR and between 8c7a30d and 7c84015.

📒 Files selected for processing (6)
  • coding-agent/.opencode/prompts/tasks/local-daemon-pr43-approval-interaction-service.md
  • coding-agent/docs/adr/0058-local-daemon-control-plane-executor-architecture.md
  • coding-agent/src/coding_agent/approval/__init__.py
  • coding-agent/src/coding_agent/approval/interactions.py
  • coding-agent/src/coding_agent/server/session_manager.py
  • coding-agent/tests/coding_agent/test_approval_interactions.py

@delete-cloud

Copy link
Copy Markdown
Owner Author

Local review fallback because CodeRabbit remained stuck in Review in progress and did not complete a diff review.

Reviewer: Lagrange (019e8adf-ad7f-7b52-96ac-98cf6d9b38f8)
Result: No P1/P2 findings.

Scope reviewed: ApprovalInteractionService, SessionManager approval interaction routing, exports, ADR/task packet, and focused tests.

Verification already run locally by implementer:

  • uv run pytest tests/coding_agent/test_approval_interactions.py -v
  • uv run pytest tests/ui/test_session_manager_runtime.py -k "approval_interaction" -v
  • uv run pytest tests/coding_agent/test_approval_runtime_messages.py -k "submit_approval" -v
  • uv run ruff check src/coding_agent/approval src/coding_agent/server/session_manager.py tests/coding_agent/test_approval_interactions.py tests/ui/test_session_manager_runtime.py tests/coding_agent/test_approval_runtime_messages.py

@delete-cloud

Copy link
Copy Markdown
Owner Author

Final local review fallback after review-fix commit because the new CodeRabbit cycle emitted a rate-limit comment despite a passing status.

Reviewer: Lagrange (019e8adf-ad7f-7b52-96ac-98cf6d9b38f8)
Head reviewed: fe24bc54396a6ba9beb73d23f5b66debe0adc695
Result: No P1/P2 findings.
Residual risk from reviewer: static review only; verification suite was run locally by implementer.

Verification after fix:

  • uv run pytest tests/coding_agent/test_approval_interactions.py -v
  • uv run pytest tests/ui/test_session_manager_runtime.py -k "approval_interaction" -v
  • uv run pytest tests/coding_agent/test_approval_runtime_messages.py -k "submit_approval" -v
  • uv run ruff check src/coding_agent/approval src/coding_agent/server/session_manager.py tests/coding_agent/test_approval_interactions.py tests/ui/test_session_manager_runtime.py tests/coding_agent/test_approval_runtime_messages.py

@delete-cloud
delete-cloud merged commit 49ba81c into main Jun 3, 2026
1 check passed
@delete-cloud
delete-cloud deleted the dogfood/approval-interaction-service branch June 3, 2026 00:36
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