Skip to content

fix(resync): skip and report missing registry seats#328

Merged
EtanHey merged 1 commit into
mainfrom
fix/resync-agents-skip-report
Jul 15, 2026
Merged

fix(resync): skip and report missing registry seats#328
EtanHey merged 1 commit into
mainfrom
fix/resync-agents-skip-report

Conversation

@EtanHey

@EtanHey EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner

Summary

  • isolate per-seat missing-state repair failures during resync_agents
  • continue healthy seat repair and real ghost eviction after a skipped seat
  • report skipped seats structurally in diff.repair_skipped
  • preserve fail-closed behavior for same-text generic errors, corrupt state, and filesystem faults

TDD evidence

  • RED: a missing in-memory seat state aborted the whole resync before healthy repair/ghost eviction
  • RED: a persisted seat throwing the same generic Agent not found text was incorrectly accepted by the initial message-based boundary
  • RED: an ELOOP state path was incorrectly classified as absent by existsSync
  • GREEN: only a typed, strictly proven absent-state sentinel is skipped; every ambiguous/non-absence error aborts with no diff

Verification

  • env -u CMUX_SOCKET_PATH -u CMUX_DAEMON_SOCKET bun run test — 104 files, 2,162 tests passed
  • affected suites — 128 tests passed
  • env -u CMUX_SOCKET_PATH -u CMUX_DAEMON_SOCKET bun run typecheck
  • env -u CMUX_SOCKET_PATH -u CMUX_DAEMON_SOCKET bun run build
  • git diff --check
  • red-team, blue-team, and implementation reviews: zero remaining findings
  • local CodeRabbit CLI: skipped after its bounded three-minute review window; disposition lint passed

No sidebar render or placement code changed. All tests were hermetic; no live cmux/agent session was accessed or mutated.

Do not merge; @cmuxlayerRethink-lead owns review and merge.


Note

Medium Risk
Changes agent registry repair and resync diff behavior during discovery repair; incorrect skip vs abort logic could leave stale seats or block healthy resyncs, though tests enforce fail-closed on non-absence errors.

Overview
resync_agents no longer aborts the whole run when one seat’s on-disk state is missing while the in-memory registry still references it. Per-surface repair failures are isolated: only a typed AgentNotFoundError (proven absent state) is recorded in diff.repair_skipped with surface, agent, seat, and reason; other seats continue to repair and ghost eviction proceeds.

Missing-state detection is tightened so “no state file” is not inferred from unreadable paths. StateManager.hasStateFile reads the state file strictly (ENOENT/ENOTDIR → absent) and can resolve aliased state dirs; getMissingStateSentinel uses that instead of readState, so filesystem faults (e.g. ELOOP) still fail closed and abort resync with no diff. updateManagedSurfaceRegistration maps update failures to the same sentinel only when absence is strictly proven; generic “Agent not found” text when state still exists still aborts.

Reviewed by Cursor Bugbot for commit 18216c0. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Skip and report missing registry seats during resync_agents repair

  • AgentRegistry.repairFromDiscovery now catches AgentNotFoundError per seat, records a skip entry, and continues processing rather than aborting the entire repair. The repair summary gains a skipped array alongside repaired and evicted.
  • AgentRegistry.repairExistingRecord wraps stateMgr.updateRecord failures and reclassifies them as AgentNotFoundError when no state file exists, allowing upstream logic to skip gracefully.
  • StateManager.hasStateFile is added to check agent state existence by scanning state directories, used to distinguish truly missing agents from transient read errors.
  • The resync_agents tool response now includes a repair_skipped field populated from the skipped entries.
  • Risk: missing-state detection now uses file existence rather than a successful readState, so agents with unreadable state files may now be classified as missing and skipped instead of surfacing an error.

Macroscope summarized 18216c0.

@cursor

cursor Bot commented Jul 15, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_b72b0b90-8727-4262-905a-0031e9db5a6b)

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@EtanHey, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 2 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e4b3f055-e43a-4140-8856-167eedf4adb0

📥 Commits

Reviewing files that changed from the base of the PR and between 7f8137b and 18216c0.

📒 Files selected for processing (5)
  • src/agent-registry.ts
  • src/server.ts
  • src/state-manager.ts
  • tests/agent-registry.test.ts
  • tests/resync-tool.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/resync-agents-skip-report

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.

❤️ Share

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

@EtanHey

EtanHey commented Jul 15, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review
@codex review
@cursor review
@greptile review

@coderabbitai

coderabbitai Bot commented Jul 15, 2026

Copy link
Copy Markdown

@EtanHey I'll review the changes now. Note that the other mentions (@codex, @cursor, @greptile) are separate tools and will respond independently on their own.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@EtanHey
EtanHey merged commit 90c98c6 into main Jul 15, 2026
5 checks passed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 18216c0098

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/agent-registry.ts
Comment on lines +1717 to +1721
} catch (error) {
if (!(error instanceof AgentNotFoundError)) {
throw error;
}
skipped.push({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Evict skipped missing-state records before continuing

When updateManagedSurfaceRegistration proves the state file is missing, this catch records repair_skipped but leaves the stale AgentRecord in this.agents. In the common case where that in-memory record already matches the discovered surface/workspace, the later listMerged() call has no metadata patch to trigger evictMissingStateAgent, so the missing seat is still returned as a managed, addressable agent and suppresses orphan/auto-discovery reporting even though it has no persisted state. Remove the missing in-memory record before continuing with the rest of the repair.

Useful? React with 👍 / 👎.

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