Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new “Tooling Mounts” UI to quickly toggle common credential/config mounts (AWS, gcloud, kubeconfig, Docker, Helm, Azure) and enhances the mounts editor to support editing existing mounts.
Changes:
- Introduces a
TOOL_MOUNTSpreset list and toggle logic that adds/removes mounts and relatedremoteEnvvariables. - Replaces bash history-specific mount logic with the generic tool toggle mechanism.
- Adds mount editing support in
MountsControl(select to edit, update vs add, highlight active edit row).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/components/MountsSection.vue | Adds tooling mount presets and toggle logic; updates Bash History toggle to use the new mechanism; renders a new Tooling Mounts grid. |
| src/components/MountsControl.vue | Adds “edit existing mount” UX: select row to edit, update in place, improved display for string mounts, and readonly/type helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const toolTargets = tool.mounts.map(getMountTarget); | ||
| return (props.config.mounts || []).some((m: any) => | ||
| toolTargets.includes(getMountTarget(m)), | ||
| ); |
There was a problem hiding this comment.
Tool enable/disable and active detection are based only on matching mount target paths. This can mis-detect a tool as active (or remove mounts) if the user already has a different mount that uses the same target (e.g., a custom /host/.aws mount). Consider matching on a fuller mount signature (source+target+type/options) or otherwise scoping removals to mounts that were added by the tool.
| newMount.value = { | ||
| source: sourcePart ? sourcePart.split("=")[1] : "", | ||
| target: targetPart ? targetPart.split("=")[1] : "", | ||
| type: (typePart ? typePart.split("=")[1] : "bind") as any, | ||
| options: parts |
There was a problem hiding this comment.
When selecting a string mount for editing, the parsed type= value is assigned to newMount.type via as any. If the mount uses an unsupported type (e.g., type=tmpfs or any non-bind/volume value), the form state can become inconsistent with the allowed MOUNT_TYPES options. Consider falling back to a supported default when the parsed type isn't one of the allowed values, or adding an explicit "RAW"/custom type option.
| if (tool.remoteEnv) { | ||
| Object.keys(tool.remoteEnv).forEach((key) => { | ||
| if (newConfig.remoteEnv) delete newConfig.remoteEnv[key]; |
There was a problem hiding this comment.
toggleTool deletes remoteEnv keys unconditionally when disabling a tool. If the user already had the same env var configured (or edited it after enabling), toggling the tool off will silently remove their value. Consider only deleting keys when the current value matches the tool-provided value, or track whether the tool introduced the key before removing it.
| if (tool.remoteEnv) { | |
| Object.keys(tool.remoteEnv).forEach((key) => { | |
| if (newConfig.remoteEnv) delete newConfig.remoteEnv[key]; | |
| if (tool.remoteEnv && newConfig.remoteEnv) { | |
| Object.keys(tool.remoteEnv).forEach((key) => { | |
| if (newConfig.remoteEnv[key] === tool.remoteEnv[key]) { | |
| delete newConfig.remoteEnv[key]; | |
| } |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.