Disable mutating tools in read-only mode #114
Conversation
|
Hey @gregnr it seems to me that for Or to not write full |
There was a problem hiding this comment.
@diksipav I think there is no need for e2e tests in this case, let's just stick to unit. The logic is pretty straightforward and probably not worth the cost of running through a real LLM.
The tests you've added to server.test.ts look good.
| @@ -179,7 +179,6 @@ export const mockManagementApi = [ | |||
| name: z.string(), | |||
| region: z.string(), | |||
| organization_id: z.string(), | |||
| db_pass: z.string(), | |||
There was a problem hiding this comment.
Hm, I deleted db_pass because it is not used anywhere in the mock function. I can put it back?
I will also fix the tests message and remove e2e test I previously wrote.
There was a problem hiding this comment.
You're right that we don't use it in the mock implementation, but we still want to include it during request body validation. We're mocking the real management API with this endpoint which does require db_pass, so it's important we still check for it and throw if the client forgot to pass it.
There was a problem hiding this comment.
Oh yes, great. I already put it back in the last commit : )
| @@ -9,12 +9,14 @@ const DEFAULT_TEST_MODEL = 'claude-3-7-sonnet-20250219'; | |||
|
|
|||
| type SetupOptions = { | |||
| projectId?: string; | |||
| readOnly?: boolean; | |||
| features?: string[]; | |||
There was a problem hiding this comment.
I can leave or remove changes to utils.ts since we are not using them for now.
Edit: I removed them.
|
Looks good @diksipav 👍 you can ignore the failed E2E tests since these are just due to the missing API key. We'll merge this soon. |
What kind of change does this PR introduce?
Improvement: #112
What is the current behavior?
Read-only mode executes SQL as a read-only Postgres user (
execute_sqltool), andapply_migrationtool is disabled in read-only mode.What is the new behavior?
Alongside
execute_sqlandapply_migrationall other mutating tools are disabled in read-only mode.