fix: Resolve the cloud provider when a resource search omits it - #68
liranfarage89 merged 5 commits into
Conversation
The env0 API rejects a cloud resource search that has neither cloudConfigurationId nor cloudProvider, and agents send neither about eight times a day. When both are missing we now look up the organization's cloud configurations: one provider means we fill it in, several means the agent gets a message naming them, none means the organization has no Cloud Compass data yet. Also dropped cloudProvider.in from the tool schema. The API only reads .eq, so an agent that used .in always got a 400. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alonnoga
left a comment
There was a problem hiding this comment.
Approving. Verified locally on the branch: tests 3/3, type-check, lint and prettier all pass, and the fallback logic looks right.
One thing to fix before merging: the test glob in package.json (inline comment). The rest are nits, take or leave.
tscnow emitsdist/env0-service/env0-service.test.jsand.d.tsinto the Docker image. Harmless, but consider adding"exclude": ["**/*.test.ts"]totsconfig.jsonor a separatetsconfig.build.json.- Every unfiltered search now costs an extra API call to
/mcp/cloud/configurations. Fine at ~8/day, just noting in case volume grows.
| "start": "tsx src/cli.ts", | ||
| "dev": "tsx watch src/cli.ts", | ||
| "test": "npm test", | ||
| "test": "tsx --test src/**/*.test.ts", |
There was a problem hiding this comment.
Please fix before merging: npm runs scripts under sh, which has no globstar, so ** behaves like *. Only tests one directory deep run (src/x/y.test.ts). A future src/mcp/tools/foo.test.ts would be silently skipped.
Quote the glob so Node expands it instead of the shell:
"test": "tsx --test 'src/**/*.test.ts'"There was a problem hiding this comment.
Fixed in ff31aaa. Confirmed with a probe test at src/mcp/tools/: npm test found 3 tests before, 4 after.
| // The API rejects a search that has neither cloudConfigurationId nor cloudProvider, and callers often send neither. | ||
| private async withCloudProvider( | ||
| filters: GetCloudResourcesParams['filters'] | ||
| ): Promise<GetCloudResourcesParams['filters']> { |
There was a problem hiding this comment.
Guard checks cloudConfigurationId?.eq only, but the schema still allows cloudConfigurationId.in. An agent that sends in on a multi-provider org gets the "Set filters.cloudProvider.eq..." error even though it did scope the search.
Either check ?.eq || ?.in?.length, or drop in from cloudConfigurationId like you did for cloudProvider. Nit, your call.
There was a problem hiding this comment.
Fixed in 18a6e8b by dropping in, same as cloudProvider. The env0 API's own guard reads filters.cloudConfigurationId.eq only, so in alone got a 400 there too. Widening the guard would just have moved the 400 one layer down.
| return { service: new Env0Service(config, client), requests }; | ||
| }; | ||
|
|
||
| describe('getCloudResources', () => { |
There was a problem hiding this comment.
Nit: no case for the zero-configurations path ("No cloud configurations found"). One more buildService([]) + assert.rejects would cover it.
|
On the two notes in the review body: dist and the image: the published image is already clean. Extra API call: agreed, and it only happens on searches that used to 400, so it costs a call where we previously returned nothing useful. If volume grows the providers list is a good cache candidate. |
…-ai-agents-that-omit
What
AI agents call
get-cloud-resourceswithout any filter and get a 400 back, about eight times a day in prod. The API needs eithercloudConfigurationIdorcloudProvider, and the tool schema only said so in prose, so agents kept leaving both out and the user saw a failed lookup.Instead of demanding the filter, we now work it out. When a search arrives with neither field, the server reads the organization's cloud configurations. One provider means we fill it in and the search runs. More than one means the agent gets back a message naming them, so it can retry with the right one. None means the organization has no Cloud Compass data, which we now say plainly.
Also removed
infrom bothcloudProviderandcloudConfigurationIdin the schema. The API reads.eqon both, so an agent that usedingot a 400 no matter what it sent.Tests
First tests in this repo.
npm testwasnpm test, an infinite loop, so it now runs Node's own test runner through tsx. No new dependencies. Added anpm teststep to the lint workflow, otherwise nothing would run them.Four cases in
src/env0-service/env0-service.test.ts. Two of them fail on main and pass here.Manual QA
Driven through the tool itself over stdio JSON-RPC against the dev API, not through the service class, so these are the responses an agent actually receives.
Single-provider organization, which is the case the fix exists for:
health: false, so no scan ran.filters: {}returnedisError: falseand{"resources":[],"total":0}. The provider was filled in and the API accepted the search. The result is empty because that bucket was never scanned, the 400 is what mattered.HTTP 400 You must specify at least one of the parameters in the filters.Other paths:
Set filters.cloudProvider.eq to one of: AWS, AzureLAW, or set filters.cloudConfigurationId.eq.instead of the raw 400.cloudConfigurationId.eq: resources come back,isError: false.cloudProvider.eq: 1549 resources.No cloud configurations found for this organization.tools/listshowscloudConfigurationIdandcloudProvidereach exposing onlyeq, both required inside their object, so theintrap is gone from the schema agents read.Docker image, since that is what users actually run:
docker buildsucceeds..dockerignorealready lists**/*.test.ts, so the test file never enters the build context and neithersrcnordistin the image carries it.isError: falsewith{"resources":[],"total":0}, so the fallback works in the shipped artifact and not just in tsx.Fixes APO-704
🤖 Generated with Claude Code