Repository navigation
fix: recover canceled Loop coordinators on restart - #353
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughThe change centralizes loop session cancellation, preserves umbrella task progress for canceled coordinator runs, and repairs canceled coordinator tasks during loop reconciliation. Recovery uses the previous coordinator run ID and a dedicated idempotency key. ChangesLoop cancellation recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Daemon
participant SessionController
participant SessionManager
participant LoopReconciliation
participant GlobalDB
Daemon->>SessionController: cancel or stop loop session
SessionController->>SessionManager: CancelPrompt or StopWithCause
SessionManager-->>SessionController: success, missing session, or error
Daemon->>LoopReconciliation: reconcile canceled coordinator
LoopReconciliation->>GlobalDB: retrieve previous coordinator run
LoopReconciliation->>GlobalDB: repair task and reserve replacement run
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
| Filename | Overview |
|---|---|
| internal/store/globaldb/global_db_loop_reconcile_queries.go | Recovery now derives a causal idempotency key from the previous coordinator run and performs repair plus reservation through the existing transactional boundary. |
| internal/store/globaldb/global_db_loop_coordinator_seed.go | Adds guarded reopening of canceled coordinator tasks only for nonterminal Loops and clears the task closure timestamp. |
| internal/task/manager_reconcile_status.go | Aligns task projection so canceled Loop coordinator pulses keep their reusable umbrella task in progress while canceled worker runs remain canceled. |
| internal/store/globaldb/global_db_task_catalog_sql.go | Mirrors the task-manager projection in catalog SQL for canceled coordinator pulses. |
| internal/daemon/loop_runtime_adapters.go | Adds a named cancellation adapter that treats only missing-session errors as an already-completed stop and propagates other failures. |
| internal/store/globaldb/global_db_loop_test.go | Covers repair, event creation, repeated boot idempotency, and a causally new recovery after canceling the first replacement. |
| packages/site/content/docs/loops/running.mdx | Documents missing-session cancellation delivery and nonterminal coordinator recovery behavior. |
| skills/compozy/references/loops.md | Updates the official skill reference with the corrected cancellation and restart-recovery boundaries. |
Sequence Diagram
sequenceDiagram
participant Boot as Daemon boot
participant Store as Global DB
participant Task as Coordinator task
participant Run as Coordinator run
Boot->>Store: Reconcile nonterminal Loops
Store->>Store: Find Loop missing an active coordinator
Store->>Task: Reopen canceled task
Store->>Run: Reserve replacement using prior run identity
Store-->>Boot: Return one queued recovery run
Boot->>Store: Repeat reconciliation
Store-->>Boot: No duplicate reservation
Reviews (2): Last reviewed commit: "test: assert Loop cancellation stop meta..." | Re-trigger Greptile
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
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 `@internal/daemon/loop_runtime_adapters_test.go`:
- Around line 74-78: Update the kill-session assertions in the test’s test.kill
branch to also capture and verify the stop cause and detail passed to
KillLoopSession. Assert session.CauseUserRequested and the exact detail
"operator request", while preserving the existing session ID assertion.
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f9d99dba-6816-4ec0-887f-29da317f74ef
⛔ Files ignored due to path filters (5)
docs/qa/charters/CH-loop-cancel-restart.mdis excluded by!**/*.mddocs/qa/reports/2026-08-12-issue-348-loop-cancel-restart.mdis excluded by!**/*.mddocs/qa/scenarios/LP-cancel-restart-recovers.mdis excluded by!**/*.mdpackages/site/content/docs/loops/running.mdxis excluded by!**/*.mdxskills/compozy/references/loops.mdis excluded by!**/*.md
📒 Files selected for processing (13)
internal/daemon/loop_api_service.gointernal/daemon/loop_goal_managed_runtime_integration_test.gointernal/daemon/loop_runtime_adapters.gointernal/daemon/loop_runtime_adapters_test.gointernal/store/globaldb/global_db_loop_coordinator_seed.gointernal/store/globaldb/global_db_loop_reconcile_queries.gointernal/store/globaldb/global_db_loop_test.gointernal/store/globaldb/global_db_task_catalog_sql.gointernal/store/globaldb/global_db_task_test.gointernal/store/globaldb/queries/loop_runtime.sqlinternal/store/globaldb/sqlcgen/loop_runtime.sql.gointernal/task/manager_reconcile_status.gointernal/task/manager_test.go
Member
Author
|
CI baseline evidence before merge: every scoped check for this PR is green, and the final local make gate-full record remains current at fingerprint 5746e5afb9d77a24c3117bdd7cc530c5ca895ad0. The combined E2E lane failed twice on the PR with exactly the same two assertions already failing on its base SHA 452c3d1 (main run 31564670715, job 94020936703): E2E-023 measured an 86 ms frame on main versus 87 ms on the PR (limit 50 ms), and session-provider-override timed out waiting for the same selector scroll predicate. This PR changes no web files or those surfaces. The failures are pre-existing on main and are not caused by #353; no test was weakened or unrelated production code changed. |
This was referenced Aug 12, 2026
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
This was referenced Aug 20, 2026
Merged
Merged
Merged
Merged
This was referenced Sep 10, 2026
Merged
Merged
Merged
Merged
This was referenced Sep 27, 2026
Merged
Merged
This was referenced Oct 6, 2026
Merged
Merged
Merged
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
Cancellation could leave three durable facts out of agreement: the Loop remained nonterminal, its reusable coordinator task was
canceled, and one or more managed sessions had already disappeared. Boot reconciliation correctly detected that the Loop still needed a coordinator, but reservation reused the closed task and failed withtask: invalid status transitionbefore daemon readiness. The daemon adapter also treatedsession.ErrSessionNotFoundas failed delivery, so the cancellation backstop kept revisiting a stop that had already happened.Changes
in_progress, matching completed coordinator pulses; canceled worker runs still cancel their tasks.closed_at, records the normaltask.status_changedevent, and reserves one replacement run.session.ErrSessionNotFoundas already stopped and propagates every other cancel or kill error.skills/compozy/references/loops.mddocument absent-session delivery and restart repair.Release Notes
🐛 Bug Fixes
QA
Final Status: PASS — missing-session cancellation and repeated restart recovery passed; teardown is clean and the full automated suite is green.
Coverage: Bruno completed one Interrupt Tour across
J-recover-loop-node-failure, including affected and foreign workspace reads through CLI, HTTP, UDS, runtime, scheduler, task, session, Loop, and deterministic ACP subprocess surfaces.Issues: 0 new issues (Blocks-Completion: 0, Data-Loss: 0, Trust-Damage: 0, Friction: 0, Cosmetic: 0).
Blocked / needs human: None.
Full report:
2026-08-12-issue-348-loop-cancel-restart.mdCompozy Impact Audit
task.status_changedhook path, and Loop terminal delivery remains unchanged.workspace_id; canonical store tests and the public foreign-workspace QA read prove no cross-workspace result is exposed through CLI, HTTP/UDS, or SSE-backed state.skills/compozy/references/loops.mdnow tells agents that an absent managed session is already stopped and that only a nonterminal Loop can recover its canceled coordinator task.Web / Docs / Config impact
config.tomlkey, default, apply lifecycle, schema migration, or storage migration changes.Review remediation
2873ffanow asserts the exact stop cause and detail forwarded by the daemon cancellation adapter; the focused race test and the full iteration gate passed.2873ffawith no new findings; the round 1 thread is resolved.Test plan
CGO_ENABLED=1 go test -race ./internal/task -run '^TestTaskStatusFromPolicySnapshotPrecedence$'CGO_ENABLED=1 go test -race ./internal/daemon -run '^TestLoopCancellationSessionControllerShouldTreatMissingSessionsAsStopped$'TestLoopGoalManagedRuntimeIntegrationcancellation paths.make verify: 21,511 tests passed, 2 expected skips, Web/Bun gates, Go lint, Go build, codegen checks, and package boundaries passed. The authoritative final content fingerprint is recorded bymake gate-statusafter the closing gate.teardown.jsonreportsclean: truewith no survivors.Closes #348