fix(openshell): pass workspace env vars to sandbox create#2249
fix(openshell): pass workspace env vars to sandbox create#2249bmahabirbu wants to merge 3 commits into
Conversation
bmahabirbu
requested review from
fbricon and
jeffmaury
and removed request for
a team
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📜 Recent review details⏰ Context from checks skipped due to timeout. (10)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.spec.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/main/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
packages/{main,renderer,preload}/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (2)📚 Learning: 2026-05-12T17:14:02.153ZApplied to files:
📚 Learning: 2026-03-09T08:47:09.657ZApplied to files:
🔇 Additional comments (5)
📝 WalkthroughWalkthroughAdds environment variable propagation for OpenShell sandboxes. A new ChangesEnvironment Variable Propagation to OpenShell Sandbox
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/main/src/plugin/openshell-cli/openshell-cli.ts`:
- Around line 137-141: The environment variable values in the options.env array
are being added as raw KEY=value pairs to the args array, and these args are
subsequently logged by the runCli() function, which can expose secrets in the
application logs. Modify the code to redact or mask the env values in the args
array that will be logged while ensuring the actual CLI execution receives the
unredacted values. This requires separating the logging representation of the
arguments from the actual arguments passed to the CLI, or implementing a
redaction mechanism that masks sensitive --env argument values before they reach
the logging layer.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 597ce298-7641-441f-821e-596bdd31fb9a
📒 Files selected for processing (5)
packages/api/src/openshell-gateway-info.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (10)
- GitHub Check: linter, formatters
- GitHub Check: typecheck
- GitHub Check: smoke-e2e-tests (prod) / ubuntu-24.04 (ollama)
- GitHub Check: smoke-e2e-tests (dev) / ubuntu-24.04 (ollama)
- GitHub Check: macOS
- GitHub Check: unit tests / ubuntu-24.04
- GitHub Check: unit tests / macos-15
- GitHub Check: unit tests / windows-2022
- GitHub Check: Windows
- GitHub Check: Linux
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use
/@/path aliases instead of relative paths for imports outside the current directory's module group; use relative imports only for sibling modules within the same directory
Files:
packages/main/src/plugin/openshell-cli/openshell-cli.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/api/src/openshell-gateway-info.ts
packages/main/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
packages/main/src/**/*.{ts,tsx}: UseipcHandle()to expose handlers in the main process with naming convention<registry-name>:<action>(e.g.,container-provider-registry:listContainers)
UseapiSender.send()to send events from main process to renderer for real-time updates
Long-running operations should useTaskManager.createTask()with title and action configuration
Files:
packages/main/src/plugin/openshell-cli/openshell-cli.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
packages/{main,renderer,preload}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Container operations must include
engineIdparameter to identify the container engine
Files:
packages/main/src/plugin/openshell-cli/openshell-cli.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
**/*.spec.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.spec.{ts,tsx,js,jsx}: Usetest()instead ofit()for test cases in Vitest unit tests
Usevi.mock(import('...'))for auto-mocking modules in unit tests; avoid manual mock factories when possible
Usevi.resetAllMocks()inbeforeEachhooks instead ofvi.clearAllMocks()for resetting mocks between tests
When an auto-mocked function or class method needs a real implementation, usevi.mocked(...)with the prototype pattern for class methods:vi.mocked(MyClass.prototype.myMethod).mockImplementation(...)
Files:
packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.ts
🧠 Learnings (2)
📚 Learning: 2026-03-09T08:47:09.657Z
Learnt from: benoitf
Repo: kortex-hub/kortex PR: 1077
File: packages/main/src/plugin/skill/skill-manager.ts:80-109
Timestamp: 2026-03-09T08:47:09.657Z
Learning: In the kortex-hub/kortex repository, IPC handlers (via ipcHandle()) may be registered directly inside feature manager/service classes (e.g., SkillManager in packages/main/src/plugin/skill/skill-manager.ts) rather than exclusively in packages/main/src/plugin/index.ts. Treat this as an accepted design pattern for files under the plugin directory. Reviewers should not require centralization in index.ts; allow IPC registration proximity to the feature that owns the handler. When reviewing code, accept direct ipcHandle() registrations inside feature managers and ensure the pattern is consistently applied across similar feature-manager modules.
Applied to files:
packages/main/src/plugin/openshell-cli/openshell-cli.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.ts
📚 Learning: 2026-05-12T17:14:02.153Z
Learnt from: MarsKubeX
Repo: openkaiden/kaiden PR: 1850
File: packages/renderer/src/lib/agent-workspaces/AgentWorkspaceList.svelte:66-70
Timestamp: 2026-05-12T17:14:02.153Z
Learning: When reviewing code that uses `AgentWorkspaceSummaryUI.runtime`, treat it as a required, non-null `string` per the `openkaiden/kdn-api` 0.12.0 schema. Therefore, code like `a.runtime.localeCompare(b.runtime)` is safe and should not trigger warnings about possible `undefined`/`null` values or suggestions to use nullish coalescing/optional chaining for `runtime` (unless the current local types still mark `runtime` as optional, indicating a schema/version mismatch).
Applied to files:
packages/main/src/plugin/openshell-cli/openshell-cli.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tspackages/main/src/plugin/openshell-cli/openshell-cli.spec.tspackages/main/src/plugin/agent-workspace/agent-workspace-manager.tspackages/api/src/openshell-gateway-info.ts
🔇 Additional comments (4)
packages/api/src/openshell-gateway-info.ts (1)
88-88: LGTM!packages/main/src/plugin/openshell-cli/openshell-cli.spec.ts (1)
168-180: LGTM!Also applies to: 250-265
packages/main/src/plugin/agent-workspace/agent-workspace-manager.ts (1)
185-193: LGTM!packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.ts (1)
431-448: LGTM!
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
| cpu?: string; | ||
| memory?: string; | ||
| providers?: string[]; | ||
| env?: string[]; |
There was a problem hiding this comment.
suggestion: should be typed as Record<string, string>
There was a problem hiding this comment.
updated! thanks for the suggesiton
Workspace environment entries now flow into OpenShell sandbox creation. This makes values from workspace.json available from the first process and covers the wiring with focused tests. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: Brian <bmahabir@bu.edu>
…ring[] Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
bmahabirbu
force-pushed
the
env-vars-to-openshell
branch
from
df0251e to
f09bd9b
Compare
MarsKubeX
left a comment
MarsKubeX
left a comment
There was a problem hiding this comment.
LGTM. Only take a look to the codeRabbit comment regarding leaking secrets.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Brian <bmahabir@bu.edu>
3 participants
Fixes #2246.
Summary
--env KEY=VALUEsupport to the OpenShell CLI wrapper used by KaidenTest plan
pnpm exec vitest run packages/main/src/plugin/openshell-cli/openshell-cli.spec.ts packages/main/src/plugin/agent-workspace/agent-workspace-manager.spec.tsopenshell sandbox create --name <temp> --from base --env KAIDEN_ENV_VERIFY=works --no-keep --no-tty -- sh -lc 'printf %s "$KAIDEN_ENV_VERIFY"'Made with Cursor