Revert "fix(kiloclaw): harden Composio onboarding"#3491
Conversation
This reverts commit 11eb1e3.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| await patchSecretsBackoff(MANAGED_COMPOSIO_PATCH_SECRETS_BACKOFF_MS[attemptIndex]); | ||
| } | ||
| } | ||
| await client.patchSecrets( |
There was a problem hiding this comment.
WARNING: patchSecrets errors are now unhandled — they propagate as exceptions instead of returning false
The original code caught all patchSecrets errors, logged them, and returned false cleanly. This PR removes that try/catch, so any network/5xx error from patchSecrets now throws out of completeManagedComposioGoogleCalendarConnection.
In the popup path the outer catch in the route handler does catch it, but the user receives 'internal_error' (via callbackFailureError) rather than the expected 'connection_failed'. More importantly, the DB writes for google_calendar_connected_account_id and composio_config_source are skipped, so the connection attempt is not persisted even if the Composio side succeeded.
If this is intentional (simplify the flow while the feature is disabled), the trade-off should be documented so it's not missed when re-enabling.
| if (existing) return existing; | ||
|
|
||
| throw new Error('Failed to reserve managed Composio identity'); | ||
| const [inserted] = await db |
There was a problem hiding this comment.
WARNING: Removal of onConflictDoNothing relies entirely on the advisory lock preventing concurrent inserts
The old code used onConflictDoNothing + fallback findCurrentComposioIdentity as a database-level safety net for concurrent reservations. The new code replaces this with a plain INSERT ... RETURNING and relies solely on the withKiloclawProvisionContextLock acquired in ensureManagedComposioIdentity.
This is safe as long as all callers of ensureManagedComposioIdentity for the same scope hold the same lock key. However, the inner lock key (kiloclaw-composio:user:${userId}) differs from the outer router provision lock key (kiloclaw:provision:personal:${userId}). They don't share the same advisory lock integer (via hashtext), so concurrent requests serialized only by the outer lock would not be protected if they somehow bypass ensureManagedComposioIdentity's inner lock. Currently, all callers go through the inner lock, so this should be safe — but the safety net removal makes it brittle if a future caller skips ensureManagedComposioIdentity and calls createPendingComposioIdentityReservation directly.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Executive Summary
Overview
Issue Details (click to expand)WARNING
Files Reviewed (14 files)
Fix these issues in Kilo Cloud Reviewed by claude-4.6-sonnet-20260217 · 2,962,183 tokens Review guidance: REVIEW.md from base branch |
2 participants
Reverts #3400