Conversation
…board"" This reverts commit d600b0b.
| setTimeout(() => { | ||
| clearInterval(pollInterval); | ||
| if (connectingProvider === provider.id) { | ||
| setConnectingProvider(null); | ||
| setError('Connection timed out'); | ||
| } | ||
| }, 300000); |
There was a problem hiding this comment.
🟡 Stale closure in timeout prevents connection timeout cleanup in IntegrationConnect
The 5-minute timeout in handleConnect captures a stale connectingProvider value, so the timeout cleanup logic never executes.
Root Cause
When handleConnect is invoked, connectingProvider in the closure is null (its value from the previous render, before setConnectingProvider(provider.id) at line 131 takes effect). The setTimeout on line 200 captures this stale null value. When the timeout fires 5 minutes later, the check connectingProvider === provider.id (line 202) evaluates to false (null === 'slack' → false), so setConnectingProvider(null) and setError('Connection timed out') are never called.
// Line 200-206: connectingProvider is always null here
setTimeout(() => {
clearInterval(pollInterval);
if (connectingProvider === provider.id) { // always false
setConnectingProvider(null); // never reached
setError('Connection timed out'); // never reached
}
}, 300000);Impact: If the OAuth popup is left open or the polling never completes, the UI stays stuck in a "Connecting..." state indefinitely with no timeout error shown. The pollInterval is still cleared, but the user sees no feedback and connectingProvider is never reset.
Prompt for agents
In packages/dashboard/src/components/IntegrationConnect.tsx, the setTimeout at line 200 captures a stale connectingProvider value from the closure. To fix this, use a ref to track the current connectingProvider value (e.g., connectingProviderRef), update it alongside setConnectingProvider calls, and check the ref inside the setTimeout instead. Alternatively, store the provider.id in a local variable before the setTimeout and compare against the ref. Also consider storing the setTimeout ID so it can be cleared when the connection succeeds or the component unmounts, preventing resource leaks.
Was this helpful? React with 👍 or 👎 to provide feedback.
| const handleClearFilters = () => { | ||
| setFilters({ provider: '', agent: '', startTime: '', endTime: '' }); | ||
| setCursor(null); | ||
| fetchEntries(true); | ||
| setShowFilters(false); | ||
| }; |
There was a problem hiding this comment.
🟡 handleClearFilters fetches audit logs with stale (old) filter values instead of cleared filters
In AuditLogViewer, clicking "Clear" filters sets filters to empty but immediately calls fetchEntries(true) which still uses the old filter values from its closure.
Root Cause
At AuditLogViewer.tsx:142-146, handleClearFilters calls setFilters({...}) (line 143) which queues a state update, then immediately calls fetchEntries(true) (line 145). However, fetchEntries is a useCallback (line 104-127) that captures the current render's filters in its closure. Since setFilters hasn't re-rendered the component yet, fetchEntries still uses the OLD filter values:
// Line 142-146
const handleClearFilters = () => {
setFilters({ provider: '', agent: '', startTime: '', endTime: '' }); // queued, not applied
setCursor(null); // queued, not applied
fetchEntries(true); // runs with OLD filters from closure
setShowFilters(false);
};Inside fetchEntries, lines 109-112 read from the stale filters:
if (filters.provider) params.provider = filters.provider; // old value!
if (filters.agent) params.agent = filters.agent; // old value!The useEffect at line 130-132 only re-runs on workspaceId changes, so the cleared filters never trigger a new fetch.
Impact: When a user clicks "Clear" in the audit log filters, the UI shows filters as cleared but the displayed data is still filtered by the old criteria. The user must refresh the page to see unfiltered data.
| const handleClearFilters = () => { | |
| setFilters({ provider: '', agent: '', startTime: '', endTime: '' }); | |
| setCursor(null); | |
| fetchEntries(true); | |
| setShowFilters(false); | |
| }; | |
| const handleClearFilters = () => { | |
| const emptyFilters = { provider: '', agent: '', startTime: '', endTime: '' }; | |
| setFilters(emptyFilters); | |
| setCursor(null); | |
| // Fetch directly with empty params instead of relying on stale closure | |
| setIsLoading(true); | |
| setError(null); | |
| cloudApi.getAuditLogs(workspaceId, { limit: '50' }).then((result) => { | |
| if (result.success) { | |
| setEntries(result.data.entries); | |
| setCursor(result.data.cursor || null); | |
| setHasMore(result.data.hasMore); | |
| } else { | |
| setError(result.error); | |
| } | |
| setIsLoading(false); | |
| }); | |
| setShowFilters(false); | |
| }; | |
Was this helpful? React with 👍 or 👎 to provide feedback.
Combines PR #51 (workspace settings UI for automated PR reviews) with existing staging content including PR #47 (Slack integration). Resolved conflicts by keeping both feature sets: - Slack integration APIs and UI state - Workspace config APIs and automation settings UI - Combined section types in WorkspaceSettingsPanel Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
.