fix(registry): preserve live seats during record repair#330
Conversation
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_b76d04c9-0efd-4a09-9d44-9c6c68c2fd7b) |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAgent reconciliation now validates observer snapshots, repairs managed seat identities in place, and uses fresh discovery proofs when deciding whether crash-recovery ghosts can be evicted. ChangesSeat reconciliation and recovery
Sequence Diagram(s)sequenceDiagram
participant resync_agents
participant repairFromDiscovery
participant discovery.scan
participant createLiveSeatDiscoveryProof
participant listMerged
participant evictSurfaceless
resync_agents->>repairFromDiscovery: repair discovered seats
resync_agents->>discovery.scan: scan after repair
resync_agents->>createLiveSeatDiscoveryProof: build proof with observer id and epoch
resync_agents->>listMerged: reconcile discoveredAfterRepair
resync_agents->>evictSurfaceless: evict with liveSeatProof
evictSurfaceless-->>resync_agents: retain or evict recoverable ghosts
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
| // an absent canonical ghost steal a live drifted record's surface. | ||
| const claimedSurfaceRefs = new Set( | ||
| candidates.flatMap((entry) => { | ||
| const occupied = [...this.agents.values()].some( |
There was a problem hiding this comment.
🟡 Medium src/agent-registry.ts:1248
A terminal (done/error) managed record reserves its surface in claimedSurfaceRefs, so a stale terminal record blocks an eligible live managed record from self-healing onto a live replacement surface that shares identity keys. Because the replacement surface is live, surfaceless/terminal cleanup won't remove the stale row, leaving the stale record bound and the live record unable to relocate. The occupancy predicate at line 1248 omits a TERMINAL_STATES exclusion, causing stale terminal rows to be treated as active occupants. Consider excluding terminal-state records from the surface-claim predicate so only live managed records reserve their surfaces.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/agent-registry.ts around line 1248:
A terminal (done/error) managed record reserves its surface in `claimedSurfaceRefs`, so a stale terminal record blocks an eligible live managed record from self-healing onto a live replacement surface that shares identity keys. Because the replacement surface is live, surfaceless/terminal cleanup won't remove the stale row, leaving the stale record bound and the live record unable to relocate. The occupancy predicate at line 1248 omits a `TERMINAL_STATES` exclusion, causing stale terminal rows to be treated as active occupants. Consider excluding terminal-state records from the surface-claim predicate so only live managed records reserve their surfaces.
Summary
Safety
Verification
Review
Note
Medium Risk
Changes core agent registry reconciliation, eviction, and resync behavior; mistakes could drop live sessions or retain stale ghosts, though observer epoch pinning and fail-closed proof reduce cross-topology corruption.
Overview
Registry repair and eviction now favor in-place metadata fixes and stricter proof before removing rows. Drifted managed seats are updated on the existing
agent_id(sessions, PID, task text preserved) instead of aggressive duplicate eviction;evictDuplicateManagedRegistrationsis removed.Self-heal pre-reserves surfaces already bound to live managed records so iteration order cannot let a ghost steal a seat.
listMergedpins the observer snapshot at the start of ingestion (including reconcile/purge) so stale discovery cannot mutate state after a reconnect epoch.Crash-recovery ghosts skip surfaceless eviction unless a same-cycle
LiveSeatDiscoveryProof(observer owner/epoch + screen-classified seat/surface pairs) proves an active same-seat replacement;resync_agentsbuilds that proof after repair and passes it intoevictSurfaceless.Repair path treats all discovered surface refs as “live” for pending-ghost logic (read errors no longer hide refs) and drops duplicate managed eviction during
repairFromDiscovery.Reviewed by Cursor Bugbot for commit 3302d1d. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix AgentRegistry to preserve live seats during record repair
createLiveSeatDiscoveryProofto build an observer-epoch-pinned proof of live, role-classified seats, used to gate surfaceless eviction inevictSurfaceless.selfHealManagedRegistrationsFromDiscoveryso a live managed record's surface is no longer reassigned to a ghost canonical duplicate due to iteration order.hasLiveManagedSeatSibling).evictDuplicateManagedRegistrationspass that previously auto-evicted duplicate managed registrations by identity precedence.resync_agentsin server.ts now generates a live seat proof and passes the already-scanned discovery snapshot tolistMergedto maintain epoch consistency.📊 Macroscope summarized 3302d1d. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.
Summary by CodeRabbit
Bug Fixes
Tests