Harden live-start full-pull fallback (fix-forward of #325 P1s)#326
Conversation
Two P1s flagged on #325's startup full-pull fallback (now in main): 1. Unguarded startup runOnce() aborted daemon start. runOnce() calls #readyIssuePaths() unguarded, so a transient pull/listTree failure propagated out of #startLiveSubscription and killed `factory start` — a startup-resilience regression vs. the prior "watermark undefined -> continue". Now wrapped: on failure, increment liveHighWatermarkFullPullErrors, log via #error, and degrade to the live stream instead of going down. 2. Pull-before-subscribe blind spot. The full pull ran before the subscription registered, so an issue going Ready *during* the pull emitted an event with no listener and was lost. Now the subscription registers BEFORE the pull; events buffer via a new #deferLiveEventDrain gate and drain once the pull completes. Batch dedupe suppresses overlap with issues the pull dispatched. Tests: startup-pull-throws keeps the daemon up (start resolves + counter); an issue arriving via a live event mid-pull is captured and dispatched. Full factory-sdk suite green (339), typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 34 minutes and 6 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✨ 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 |
|
pr-reviewer could not complete review for #326 in AgentWorkforce/pear. |
There was a problem hiding this comment.
Code Review
This pull request modifies the FactoryLoop orchestrator to register the live subscription before running the startup full pull, ensuring that events arriving during the pull are buffered and not lost. It also wraps the startup full pull in a try-catch block so that a startup pull failure does not abort the daemon, allowing it to fall back to the live event stream. Corresponding unit tests have been added to verify these behaviors. There are no review comments to address.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
pr-reviewer could not complete review for #326 in AgentWorkforce/pear. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 04d1a85dee
ℹ️ 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".
| this.#subscription = this.#mount.subscribe([LIVE_ISSUE_GLOB], (event) => { | ||
| this.#enqueueLiveEvent(event) | ||
| }, { from: 'now', coalesce: 'none' }) |
There was a problem hiding this comment.
Wait for the stream to open before pulling
In the cloud mount, subscribe() returns before the stream is actually registered: createWorkspaceScopedEventClient.subscribe starts tokenProvider() and only calls sync.start() later (packages/factory-sdk/src/subscriptions/event-client.ts around lines 350-417). With the high-watermark route unavailable, this still leaves a blind spot: if an issue becomes Ready after runOnce() has listed ready paths but before that async stream startup reaches sync.start(), the pull will not see it and the { from: 'now' } stream will not deliver it either. Please gate the startup full pull on the subscription being truly open, or use a cursor/backfill mechanism that covers the async subscription startup window.
Useful? React with 👍 / 👎.
|
pr-reviewer could not complete review for #326 in AgentWorkforce/pear. |
3 similar comments
|
pr-reviewer could not complete review for #326 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #326 in AgentWorkforce/pear. |
|
pr-reviewer could not complete review for #326 in AgentWorkforce/pear. |
Fix-forward of two P1s flagged on #325 (now merged to main).
1. Unguarded startup
runOnce()aborted daemon startrunOnce()calls#readyIssuePaths()unguarded, so a transient pull/listTreefailure propagated out of#startLiveSubscriptionand killedfactory start— a startup-resilience regression vs. the prior "watermark undefined → continue" behavior. Now wrapped in try/catch: on failure it incrementsliveHighWatermarkFullPullErrors, logs via#error, and degrades to the live event stream instead of taking the daemon down.2. Pull-before-subscribe blind spot
The full pull ran before the subscription was registered, so an issue going Ready during the pull emitted an event with no listener and was lost — ironic, since avoiding missed events is the point. Now the subscription registers before the pull; events buffer behind a new
#deferLiveEventDraingate and drain once the pull completes. The existing batch dedupe suppresses any overlap with issues the pull already dispatched (so no double-dispatch).Tests
factory.startresolves (daemon stays up) +liveHighWatermarkFullPullErrorscounter.Refs #325.
🤖 Generated with Claude Code