Skip to content

Yield between startup integration syncs - #670

Open
Dxy2326 wants to merge 2 commits into
rullerzhou-afk:mainfrom
Dxy2326:agent/yield-startup-integration-sync
Open

Yield between startup integration syncs#670
Dxy2326 wants to merge 2 commits into
rullerzhou-afk:mainfrom
Dxy2326:agent/yield-startup-integration-sync

Conversation

@Dxy2326

@Dxy2326 Dxy2326 commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • queue enabled startup integration syncs in their existing order
  • run each remaining integration in a chained setImmediate turn
  • keep Claude hook sync and watcher startup together

Root cause

The server deferred startup registration once, but syncEnabledStartupIntegrations() still ran every synchronous installer in that single callback. Slow process probes and config I/O could therefore starve incoming hook requests for the cumulative duration of the full sweep.

Impact

The HTTP server gets an event-loop opportunity between agent integrations while preserving the existing enablement gates and sync order. Individual installers remain unchanged.

Validation

  • node --test test/integration-sync.test.js test/server-hook-management.test.js test/server-start-http.test.js (51 passed)
  • git diff --check
  • verified the targeted tests leave the user's Claude settings unchanged

Addresses follow-up 2 in #350. Follow-ups 1 and 3 remain out of scope.

@Dxy2326
Dxy2326 marked this pull request as ready for review July 13, 2026 02:26
…ntegration-sync

# Conflicts:
#	src/integration-sync.js

@rullerzhou-afk rullerzhou-afk left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks — the direction is useful, and I verified that the setImmediate chain lets HTTP work run before the full multi-integration sweep completes. I found one race that needs fixing before merge:

shouldSyncAgentIntegration(agentId) is checked only while building startupSyncs. After the first sync yields, Settings IPC can disable or uninstall a later agent, but runNext still invokes the queued sync function without rechecking the gate. I reproduced this with Gemini + Codex: after Gemini ran, I flipped the Codex gate to false; the queued Codex sync still executed. In production, that can restore hook/plugin files just removed by Uninstall.

Please queue [agentId, sync], re-run shouldSyncAgentIntegration(agentId) immediately before sync(), and add a regression test that disables or uninstalls the second agent between turns.

Scope note: fresh defaults enable only Claude + Codex. Claude is handled separately and Codex is the first and only non-Claude sync, so this patch adds no inner yield on that default path; the benefit starts with at least two enabled non-Claude integrations. Please narrow the impact wording accordingly.

I tested the merge result against current main; the targeted tests pass (75/75). Windows performance validation for #350 can follow after the race is fixed.

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.

2 participants