fix: fix race condition in poll loop

[Sladuca]

Sometimes sf buy re-enters its poll loop after the order is already filled. If GET /v0/orders fails when that occurs, sf buy will error out, even though the order successfully filled.

This PR attempts to fix this by doing two things:

1. Use a recursive setTimeout instead of a setInterval. That way, there will only ever be one poll happening at a time, which prevents redundant polls (the call to exit() will happen before the useEffect is re-run)
2. use order?.id as the dependency to the useEffect, not order. This will prevent the useEffect from being run after calling setOrder(o), because the order ID will be the same.

[semanticdiff-com]

Review changes with  

Changed Files

File	Status
src/lib/buy/index.tsx	48% smaller

[greptile-apps]

Greptile Summary

This PR fixes a race condition in the sf buy command's order polling mechanism. The issue occurred when the polling loop would re-enter after an order was already filled, potentially causing API errors if the GET /orders request failed during redundant polling attempts.

The fix involves two key changes to the useEffect hook in src/lib/buy/index.tsx:

1. Replaced setInterval with recursive setTimeout: The original implementation used setInterval which could create multiple overlapping polling requests. The new approach uses a recursive setTimeout pattern where each poll completion schedules the next poll, ensuring only one polling operation is active at any time.

2. Changed dependency from order to order?.id: The useEffect dependency array now uses order?.id instead of order. This prevents the effect from re-running when setOrder(o) updates the order object with the same ID, which was a primary cause of the race condition.

The refactored polling logic maintains the same 200ms polling interval but with cleaner lifecycle management. When an order is successfully retrieved, exit() is called immediately without scheduling another poll, ensuring the polling stops cleanly once the order is filled. This change integrates well with the existing React component architecture and maintains the same user experience while eliminating the timing issues that could cause premature errors.

Confidence score: 4/5

• This PR is safe to merge with minimal risk as it addresses a specific race condition with a well-reasoned solution
• Score reflects solid understanding of the problem and appropriate React patterns, though the timeout-based approach could benefit from additional testing
• Pay close attention to the useEffect dependency change to ensure it doesn't affect other parts of the component lifecycle

_{1 file reviewed, no comments}

_{Edit Code Review Bot Settings | Greptile}