- 
                Notifications
    You must be signed in to change notification settings 
- Fork 248
fix: return success response from void operations (AI-164) #168
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Add SuccessResponse type for operations that don't return data
- Update all void operations to return { success: true }
- Affected operations: pauseProject, restoreProject, deleteBranch,
  mergeBranch, resetBranch, rebaseBranch, applyMigration, updateStorageConfig
- Prevents LLMs from thinking operations failed and attempting retries
Fixes AI-164
    | On second thought maybe it'd be easier to keep the  pause_project: tool({
      ...
      execute: async ({ project_id }) => {
        ...
        
-       return await account.pauseProject(project_id);
+       await account.pauseProject(project_id);
+.      return SUCCESS_RESPONSE;
      },That way we don't have to make additional PRs to update the hosted and self-hosted remote MCP implementations. I don't imagine the success value needs to vary per-platform. | 
…ayer - Keep Promise<void> return types in platform abstraction - Return SUCCESS_RESPONSE in tool execute functions - Affects: pauseProject, restoreProject, deleteBranch, mergeBranch, resetBranch, rebaseBranch, applyMigration, updateStorageConfig - Avoids need to update hosted/self-hosted remote MCP implementations Implements feedback from Matt's review on PR #168
| @mattrossman I've implemented your suggested changes! The platform abstraction now keeps  
 This approach avoids needing additional PRs for the hosted and self-hosted remote MCP implementations. Ready for another review when you have a chance! | 
| Not necessary for this PR, but in the future maybe we can explore richer responses for the unpause tool to clarify that projects don't unpause immediately (when the tool succeeds, agents tend to say that the project is unpaused and ready to go when often it's pending for several minutes). | 
| 
 Agree. I've created a Linear issue to track that AI-185 | 
* fix: return success response from void operations (AI-164)
- Add SuccessResponse type for operations that don't return data
- Update all void operations to return { success: true }
- Affected operations: pauseProject, restoreProject, deleteBranch,
  mergeBranch, resetBranch, rebaseBranch, applyMigration, updateStorageConfig
- Prevents LLMs from thinking operations failed and attempting retries
Fixes AI-164
* refactor: return SUCCESS_RESPONSE in tool layer instead of platform layer
- Keep Promise<void> return types in platform abstraction
- Return SUCCESS_RESPONSE in tool execute functions
- Affects: pauseProject, restoreProject, deleteBranch, mergeBranch, resetBranch, rebaseBranch, applyMigration, updateStorageConfig
- Avoids need to update hosted/self-hosted remote MCP implementations
Implements feedback from Matt's review on PR #168
    
Problem
Some tool operations (e.g.,
pauseProject,restoreProject,deleteBranch, etc.) currently returnPromise<void>, providing no feedback to LLMs about whether operations succeeded. This causes LLM clients to think operations failed when they actually succeeded, leading to unnecessary retries and additional errors.Solution
SuccessResponsetype:{ success: true }SuccessResponseinstead ofPromise<void>SUCCESS_RESPONSEconstant with documentation explaining error handling behaviorAffected Operations
Account Operations:
pauseProjectrestoreProjectBranching Operations:
deleteBranchmergeBranchresetBranchrebaseBranchDatabase Operations:
applyMigrationStorage Operations:
updateStorageConfigNotes
assertSuccess()throws an error, so the success response only indicates successful completionFixes https://linear.app/supabase/issue/AI-164