Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR ensures subprocesses (MCP Python servers and cloud CLIs) run with a defined HOME (and PATH where needed), which is important in Kubernetes/container environments where HOME may be unset.
Changes:
- Add
HOMEandPATHto the environment passed to MCP Python servers (OpenOps tools, AWS cost tools). - Add
HOMEto the environment passed to Google Cloud and Azure/AWS CLI wrappers. - Update Azure and AWS CLI unit tests to account for the new env var behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/server/api/src/app/ai/mcp/openops-tools.ts | Passes HOME/PATH into the MCP server process environment. |
| packages/server/api/src/app/ai/mcp/cost-tools.ts | Passes HOME/PATH into AWS cost MCP server process environment. |
| packages/openops/src/lib/google-cloud/auth.ts | Ensures HOME is set for gcloud auth execution. |
| packages/blocks/google-cloud/src/lib/google-cloud-cli.ts | Ensures HOME is set for gcloud command execution. |
| packages/blocks/azure/src/lib/azure-cli.ts | Ensures HOME is set for az command execution. |
| packages/blocks/azure/test/azure-cli.test.ts | Adjusts env assertions to be less strict via objectContaining and includes HOME. |
| packages/blocks/aws/src/lib/actions/cli/aws-cli.ts | Ensures HOME is set for aws command execution. |
| packages/blocks/aws/test/cli/aws-cli.test.ts | Updates env expectations to include HOME. |
Comments suppressed due to low confidence (1)
packages/blocks/aws/test/cli/aws-cli.test.ts:45
- The test asserts
HOME: process.env['HOME'], butrunCommandnow setsHOMEtoprocess.env['HOME'] ?? '/tmp'. IfHOMEis missing in the test environment, this assertion will fail. Consider asserting the same fallback expression (or setprocess.env.HOMEexplicitly within the test).
{
HOME: process.env['HOME'],
AWS_ACCESS_KEY_ID: credential.accessKeyId,
AWS_SECRET_ACCESS_KEY: credential.secretAccessKey,
AWS_SESSION_TOKEN: credential.sessionToken,
AWS_DEFAULT_REGION: 'region',
PATH: process.env['PATH'],
},
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| expect.objectContaining({ | ||
| HOME: process.env['HOME'], | ||
| PATH: process.env['PATH'], | ||
| AZURE_CONFIG_DIR: expect.any(String), |
There was a problem hiding this comment.
These assertions expect HOME to equal process.env['HOME'], but the implementation now falls back to '/tmp' when HOME is unset. This makes the test environment-dependent and can fail in CI/containers where HOME is not defined. Update the expectation to match the fallback logic (or explicitly set process.env.HOME to a known value in the test setup and restore it).



Fixes #<issue_number>.
Additional Notes
Testing Checklist
Check all that apply:
I tested the feature thoroughly, including edge cases
I verified all affected areas still work as expected
Automated tests were added/updated if necessary
Changes are backwards compatible with any existing data, otherwise a migration script is provided
Visual Changes (if applicable)
If there are UI/UX changes, include at least one of the following: