Skip to content

Close approvals when turns are cancelled - #292

Merged
milind-soni merged 1 commit into
mainfrom
codex/pr-290-fix
Aug 20, 2026
Merged

Close approvals when turns are cancelled#292
milind-soni merged 1 commit into
mainfrom
codex/pr-290-fix

Conversation

@milind-soni

@milind-soni milind-soni commented Aug 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • rebuild Close the approvals a cancelled turn can no longer answer #290 cleanly on current main
  • close unresolved provider approvals when a room or direct turn is interrupted
  • keep cleanup running even if a provider interrupt rejects
  • deny and settle peer-communication approvals so their delegation promises do not wait 15 minutes

Validation

  • pnpm typecheck
  • pnpm vitest run server/peer-approval.test.ts server/index.test.ts (75 passed)
  • pnpm test on the pre-rebase patch (1,300 passed, 12 skipped; broker, updater, and packaged-server smoke passed)

Supersedes #290, which conflicts with current main and leaves direct cleanup vulnerable to an interrupt rejection.

Summary by CodeRabbit

  • Bug Fixes

    • Interrupted conversations now automatically dismiss pending peer approvals.
    • Approval cards are marked unavailable and no longer remain unanswered after a conversation is interrupted.
    • Interrupting a specific conversation thread no longer leaves stale approvals that can block future activity.
  • Tests

    • Added coverage verifying approval cancellation and card dismissal during interrupted conversations.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

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: 585ef7ce-6ac4-41e8-b4b1-dc522913e4f5

📥 Commits

Reviewing files that changed from the base of the PR and between 9b73b24 and 7a49264.

📒 Files selected for processing (4)
  • server/index.test.ts
  • server/index.ts
  • server/peer-approval.test.ts
  • server/peer-approval.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review.


📝 Walkthrough

Walkthrough

The change adds thread-scoped peer-approval cancellation. Group and bot interruption routes close affected approvals, dismiss unanswered cards, and resolve pending approval requests. Tests cover helper behavior and room interruption through the HTTP API.

Changes

Approval cancellation on interruption

Layer / File(s) Summary
Thread-scoped approval cancellation
server/peer-approval.ts, server/peer-approval.test.ts
cancelPeerApprovalsForThread denies pending approvals for a thread, cancels timers, dismisses approval cards, and resolves approval promises.
Interruption route integration
server/index.ts
closeOpenApprovals cleans up thread approvals. Group and bot interruption routes invoke it for affected threads.
Interrupted room validation
server/index.test.ts
The new room fixture and HTTP test verify that an unanswered approval becomes dismissed and unavailable after interruption.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 7a492

The change closes pending approvals when turns are cancelled and preserves cleanup across interruption failures; no actionable merge-blocking risk remains, so it is merge-ready after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant InterruptionRoute
  participant ProviderTurn
  participant closeOpenApprovals
  participant cancelPeerApprovalsForThread
  participant ApprovalCard
  InterruptionRoute->>ProviderTurn: request interruption
  InterruptionRoute->>closeOpenApprovals: close approvals for affected threads
  closeOpenApprovals->>cancelPeerApprovalsForThread: cancel pending approvals
  cancelPeerApprovalsForThread->>ApprovalCard: mark unanswered card dismissed and unavailable
Loading

Possibly related PRs

Suggested reviewers: aivsomkar

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: closing approvals when turns are cancelled.
Description check ✅ Passed The description explains the changes and verification results, but it does not use the template headings or include the checklist.
Docstring Coverage ✅ Passed Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.
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.
✨ 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 codex/pr-290-fix

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

@milind-soni
milind-soni merged commit e604de7 into main Aug 20, 2026
6 checks passed
@milind-soni
milind-soni deleted the codex/pr-290-fix branch August 20, 2026 07:14
kargnas added a commit to kargnas/OpenMausBot that referenced this pull request Aug 20, 2026
main의 milind-soni#292(취소 시 승인 종료), milind-soni#264(리액션 팔레트), milind-soni#291(Apache
라이선스) 병합 충돌을 해결했다.

PATCH의 modelSelection 검증을 관대하게 바꿨다: 미확인 인스턴스도
persist되고(사전 엔진 구성·duplicate 시나리오), startTurn이 전송
시점에 409로 크게 실패한다. catalog 기반 모델 검증은 온라인
엔진에만 적용된다. 이에 따라 관련 e2e 기대치를 갱신했다.

Tested: pnpm typecheck, pnpm vitest run (132 files, 1311 passed, 12 skipped)

Confidence: high
Scope-risk: moderate
Reversability: moderate
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