ADE-95: Pulling main from Git actions pane can kill active sessions#511
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughThis PR introduces session-activity gating for Auto-Rebase, preventing rebase operations on lanes with active chat or PTY sessions. A new optional ChangesLane Activity Gating for Auto-Rebase
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
|
@copilot review but do not make fixes |
c697ae1 to
34b1651
Compare
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 `@apps/desktop/src/main/main.ts`:
- Around line 2341-2351: When autoRebaseActivityReady transitions from false to
true you must requeue any persisted pending head-change (auto-rebase) entries so
they get processed; add logic immediately after the flag flip (also apply the
same after the other flip near the other occurrence) to load the persisted
pending blocks and re-invoke the existing head-change processing path rather
than leaving them stuck. Locate autoRebaseActivityReady and the getLaneActivity
usage and, when setting autoRebaseActivityReady=true, call a new or existing
routine (e.g., requeuePendingAutoRebases or the same head-change handler used
for live events) to iterate persisted pending blocks and enqueue them for
processing; ensure you reference laneTeardownDeps services as needed when
replaying each pending head-change so behavior matches a live event.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3c39d693-9987-4660-b9ca-9bd642621c43
📒 Files selected for processing (4)
apps/ade-cli/src/bootstrap.tsapps/desktop/src/main/main.tsapps/desktop/src/main/services/lanes/autoRebaseService.test.tsapps/desktop/src/main/services/lanes/autoRebaseService.ts
Fixes ADE-95
Summary
Describe the change.
What Changed
Key files and behaviors.
Validation
How you tested.
Risks
Anything to watch.
Linked Linear issues
Summary by CodeRabbit
New Features
Tests
Greptile Summary
This PR gates the auto-rebase cascade on active chat and terminal session counts, preventing a git pull from silently rebasing a lane while a user is interacting with it. When sessions are present, the lane is set to
rebasePendingwith an informational message, and all descendants are similarly blocked.autoRebaseService.ts: AddsgetLaneActivityhook (optional),normalizeActivityCounthelper, andgetAutoRebaseActivityBlockto pause or surface lookup errors; both newblockedMessagereasons (active_session,activity_lookup) propagate the block to descendant lanes.bootstrap.ts/main.ts: Wire the activity providers behind anautoRebaseActivityReadyflag, then callrefreshActiveRebaseNeeds(\"activity_services_ready\")to re-evaluate any lanes that were gated during startup.Confidence Score: 5/5
The change is safe to merge; new session-activity gating is additive and the service falls back gracefully when the activity provider is absent or throws.
The activity-gating logic is well-isolated behind an optional callback, error paths are caught and converted to a safe pending state rather than allowing an unguarded rebase, edge-case inputs (NaN, negative counts) are normalised correctly, and four targeted test cases verify each new code path. No data loss or incorrect rebase can result from this change.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[onHeadChanged / refreshActiveRebaseNeeds] --> B[processRoot: build cascadeOrder] B --> C{For each lane in cascade} C --> D{blocked flag set?} D -- Yes --> E[setStatus rebasePending\nblockedMessage ancestor reason] D -- No --> F{rebaseMode === manual?} F -- Yes --> G[blocked=true, reason=manual] G --> C F -- No --> H{getLaneActivity provided?} H -- No --> K[proceed to rebase] H -- Yes --> I[getAutoRebaseActivityBlock] I --> J{activity lookup} J -- Throws --> L[blocked=true\nreason=activity_lookup\n'could not verify'] J -- activeSessions > 0 --> M[blocked=true\nreason=active_session\n'Auto-rebase paused'] J -- No sessions --> K L --> C M --> C K --> N[laneService.rebaseStart] N --> O{Error?} O -- Yes --> P[blocked=true\nconflict or failed] O -- No --> Q[laneService.rebasePush → autoRebased] P --> C Q --> CReviews (4): Last reviewed commit: "Refs ADE-95: Fix activity refresh promis..." | Re-trigger Greptile