Repository navigation
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 3 remain after this review. 📝 WalkthroughWalkthroughThe end-to-end test now expects the completed worker recovery record to have state Merge Risk: ⚪ Minimal · up to The completed-worker recovery test now expects both renderer and persisted recovery state to remain done after completion. The assertion matches the existing completed-worker setup, with no production behavior changed. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Description checkExplanation The description explains the change and validation, but it does not follow the required template. It omits the required section headings, linked issue, visual proof statement, checklist status, and several review details. Resolution Rewrite the description using the repository template. Add the ELI5, What Changed, Why, Linked Issue with an issue number, Visual Proof with N/A and a reason, Testing details, AI Disclosure if applicable, Review, Agent skill upstream boundary, Notes, and Checklist sections. Mark each checklist item accurately and include the known CI limitation and dependency.
✨ Finishing Touches📝 Generate docstrings
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 |
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
A single-line test expectation update in the completed-worker retirement E2E spec.
- Recovery state expectation —
expectedRecovery.statemoves from'working'to'done'intests/e2e/completed-worker-retirement-resume.spec.ts, aligning the assertion with the reducer's now-correct behavior of preserving the completed recovery checkpoint.
I verified the underlying behavior: reduceAgentStatusLiveUpdate writes the live recovery record into sleepingAgentSessionsByPaneKey on the completion transition, and sleepingRecordFromEntry stamps state from the entry (agent-status-sleeping-records.ts:51), so the fixture's final setAgentStatus(..., 'done') call yields state: 'done' in both the renderer store and the persisted record. The assertion remains an exact .toEqual, so it still fails if the reducer ever regresses to 'working'.
DeepSeek Pro (free via Pullfrog for OSS) ｜ 𝕏
|
Closing as superseded after checking current The completed-worker recovery expectation is already present on main: the recovered status remains done. The test was also updated through merged #23686. This PR's one-line expectation change is therefore already covered. Closing this PR only; its branch is being retained. |
The completed-worker fixture reports working then done, so the renderer and persisted recovery assertions must expect done. This corrects the saved-state expectation while preserving session identity, retirement, first activation, and restart checks. No retries or timeouts change.
Validation: 18 provider-session unit tests pass; formatting and lint pass. Both complete retirement modes passed in the combined pending-application-fixes CI run: https://github.com/stablyai/orca/actions/runs/34005529764. Main-based validation passed the corrected expectation but failed later while waiting for PaneManager to mount: https://github.com/stablyai/orca/actions/runs/34006268416. The empty-workspace reseeding fix #18886 is a likely dependency; the combined pass does not isolate that dependency. Keep this test PR unmerged until the full spec passes against main. Application PRs remain open for user review.