Add dl <workspace> dotfiles refresh subcommand - #44
Merged
Merged
Conversation
Reviewer's GuideAdds a new Sequence diagram for workspace attach with dotfiles auto-refreshsequenceDiagram
actor Developer
participant dl_cli as dl
participant DevPod
participant Workspace as Workspace_container
Developer->>dl_cli: dl <workspace>
activate dl_cli
dl_cli->>DevPod: get_workspace_state(workspace_id)
DevPod-->>dl_cli: state
alt Fast attach (already Running)
dl_cli->>Workspace: dotfiles_update(workspace_id)
activate Workspace
Workspace-->>dl_cli: exit code (df_ret)
deactivate Workspace
alt df_ret != 0
dl_cli->>dl_cli: log warning Dotfiles update failed
end
dl_cli->>Workspace: workspace_ssh(workspace_id)
Workspace-->>dl_cli: SSH session
else Cold start or not Running
dl_cli->>DevPod: workspace_up(workspace_spec)
DevPod-->>dl_cli: result
dl_cli->>Workspace: dotfiles_update(workspace_id)
activate Workspace
Workspace-->>dl_cli: exit code (df_ret)
deactivate Workspace
alt df_ret != 0
dl_cli->>dl_cli: log warning Dotfiles update failed
end
dl_cli->>Workspace: workspace_ssh(workspace_id)
Workspace-->>dl_cli: SSH session
end
dl_cli-->>Developer: Attached shell in workspace
deactivate dl_cli
Flow diagram for dotfiles_update workspace refresh logicflowchart TD
A[dotfiles_update workspace_id] --> B[get_context_options]
B --> C[Read DOTFILES_URL from context]
C --> D{DOTFILES_URL set?}
D -- Yes --> E[Construct fallback: git clone DOTFILES_URL
into temp dir, run install.sh, rm -rf temp]
D -- No --> F[Construct fallback: echo chezmoi not found and no DOTFILES_URL, exit 1]
E --> G[Build update_cmd
if command -v chezmoi
run chezmoi update --force
run pixi global sync
else run fallback]
F --> G
G --> H[Call workspace_ssh workspace_id, command=update_cmd]
H --> I[Return SSH exit code]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #44 +/- ##
==========================================
+ Coverage 94.67% 94.70% +0.03%
==========================================
Files 21 21
Lines 2591 2607 +16
==========================================
+ Hits 2453 2469 +16
Misses 138 138
🚀 New features to boost your workflow:
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
dotfiles_update,dotfiles_urlis interpolated directly into thegit clonecommand without shell quoting, so consider wrapping it in quotes or otherwise sanitizing to avoid shell injection or breakage with unusual URLs. - The dotfiles auto-refresh logic in
main()is duplicated in both attach paths; consider extracting a small helper to performdotfiles_updateand warning/logging so behavior stays consistent if this flow changes later.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `dotfiles_update`, `dotfiles_url` is interpolated directly into the `git clone` command without shell quoting, so consider wrapping it in quotes or otherwise sanitizing to avoid shell injection or breakage with unusual URLs.
- The dotfiles auto-refresh logic in `main()` is duplicated in both attach paths; consider extracting a small helper to perform `dotfiles_update` and warning/logging so behavior stays consistent if this flow changes later.
## Individual Comments
### Comment 1
<location path="devlaunch/dl.py" line_range="808" />
<code_context>
+ fallback = (
+ f'echo "chezmoi not found, running full install..." && '
+ f"DOTFILES_DIR=$(mktemp -d) && "
+ f"git clone {dotfiles_url} \"$DOTFILES_DIR\" && "
+ f'cd "$DOTFILES_DIR" && bash install.sh && '
+ f'rm -rf "$DOTFILES_DIR" && '
</code_context>
<issue_to_address>
**🚨 issue (security):** Interpolate `dotfiles_url` more safely to avoid shell injection and quoting issues.
`dotfiles_url` is inserted directly into a single-string shell command. If `DOTFILES_URL` contains spaces or shell metacharacters (e.g., from misconfiguration), the command can break or allow command injection. Please either properly quote the URL (e.g., `
</issue_to_address>
### Comment 2
<location path="devlaunch/dl.py" line_range="895" />
<code_context>
dl <user/repo> restart Stop and start (no rebuild)
dl <user/repo> recreate Recreate container
dl <user/repo> reset Clean slate (remove all, recreate)
+ dl <user/repo> dotfiles Refresh dotfiles (chezmoi update)
dl <user/repo> -- <command> Run shell command in workspace
</code_context>
<issue_to_address>
**nitpick:** Help text no longer matches the actual behavior of `dotfiles`.
`dotfiles_update` also runs `pixi global sync` and may clone/run `install.sh` when `chezmoi` is unavailable, so the help text understates what happens. Please update the wording to reflect these additional steps while staying concise.
</issue_to_address>
### Comment 3
<location path="devlaunch/dl.py" line_range="795" />
<code_context>
return result.returncode
+def dotfiles_update(workspace_id: str) -> int:
+ """Refresh dotfiles inside a running workspace.
+
</code_context>
<issue_to_address>
**issue (complexity):** Consider restructuring the dotfiles update command into composable parts and extracting the repeated update-and-warn logic into a helper to simplify and centralize behavior.
You can reduce complexity without changing behavior by:
1. **Structuring the shell command construction**
2. **Deduplicating the “update + warn” pattern**
### 1. Make `dotfiles_update` easier to read/extend
Instead of one long interpolated string, build the command from small, named fragments and then join them. This keeps all behavior but lowers cognitive load:
```python
def dotfiles_update(workspace_id: str) -> int:
"""Refresh dotfiles inside a running workspace."""
ctx = get_context_options()
dotfiles_url = ctx.get("DOTFILES_URL", "")
if dotfiles_url:
fallback_parts = [
'echo "chezmoi not found, running full install..."',
'DOTFILES_DIR=$(mktemp -d)',
f"git clone {dotfiles_url} \"$DOTFILES_DIR\"",
'cd "$DOTFILES_DIR"',
"bash install.sh",
'rm -rf "$DOTFILES_DIR"',
'echo "Dotfiles installed successfully"',
]
else:
fallback_parts = [
'echo "chezmoi not found and no DOTFILES_URL configured"',
"exit 1",
]
chezmoi_parts = [
'echo "Updating dotfiles..."',
"chezmoi update --force",
'echo "Syncing pixi global packages..."',
"pixi global sync",
'echo "Dotfiles updated successfully"',
]
fallback_cmd = " && ".join(fallback_parts)
chezmoi_cmd = " && ".join(chezmoi_parts)
update_cmd = (
"if command -v chezmoi >/dev/null 2>&1; then "
f"{chezmoi_cmd}; "
f"else {fallback_cmd}; "
"fi"
)
return workspace_ssh(workspace_id, command=update_cmd)
```
Behavior is identical, but it’s now obvious where to add/remove steps.
### 2. Deduplicate “update dotfiles if possible” in `main()`
The “call `dotfiles_update`, check return, log warning, continue” pattern appears twice. A tiny helper keeps this in one place:
```python
def refresh_dotfiles_if_possible(workspace_id: str) -> None:
ret = dotfiles_update(workspace_id)
if ret != 0:
logging.warning("Dotfiles update failed, continuing anyway...")
```
Then in `main()`:
```python
# Fast-attach: skip workspace_up() if workspace is already running
if custom_id is None and get_workspace_state(workspace_id) == "Running":
logging.info(f"Workspace {workspace_id} is already running, attaching...")
setup_hostname(workspace_id)
refresh_dotfiles_if_possible(workspace_id)
ret = workspace_ssh(
workspace_id,
shell_command,
workdir=get_container_workdir(workspace_id),
)
update_cache_background()
return ret
# ...
# After ensuring workspace is up
setup_hostname(workspace_id)
refresh_dotfiles_if_possible(workspace_id)
# Attach to workspace
ret = workspace_ssh(
workspace_id,
shell_command,
workdir=get_container_workdir(workspace_id),
)
```
This keeps all functionality intact while addressing the complexity concern and making future changes to the dotfiles refresh behavior centralized.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| dl <user/repo> restart Stop and start (no rebuild) | ||
| dl <user/repo> recreate Recreate container | ||
| dl <user/repo> reset Clean slate (remove all, recreate) | ||
| dl <user/repo> dotfiles Refresh dotfiles (chezmoi update) |
There was a problem hiding this comment.
nitpick: Help text no longer matches the actual behavior of dotfiles.
dotfiles_update also runs pixi global sync and may clone/run install.sh when chezmoi is unavailable, so the help text understates what happens. Please update the wording to reflect these additional steps while staying concise.
| return result.returncode | ||
|
|
||
|
|
||
| def dotfiles_update(workspace_id: str) -> int: |
There was a problem hiding this comment.
issue (complexity): Consider restructuring the dotfiles update command into composable parts and extracting the repeated update-and-warn logic into a helper to simplify and centralize behavior.
You can reduce complexity without changing behavior by:
- Structuring the shell command construction
- Deduplicating the “update + warn” pattern
1. Make dotfiles_update easier to read/extend
Instead of one long interpolated string, build the command from small, named fragments and then join them. This keeps all behavior but lowers cognitive load:
def dotfiles_update(workspace_id: str) -> int:
"""Refresh dotfiles inside a running workspace."""
ctx = get_context_options()
dotfiles_url = ctx.get("DOTFILES_URL", "")
if dotfiles_url:
fallback_parts = [
'echo "chezmoi not found, running full install..."',
'DOTFILES_DIR=$(mktemp -d)',
f"git clone {dotfiles_url} \"$DOTFILES_DIR\"",
'cd "$DOTFILES_DIR"',
"bash install.sh",
'rm -rf "$DOTFILES_DIR"',
'echo "Dotfiles installed successfully"',
]
else:
fallback_parts = [
'echo "chezmoi not found and no DOTFILES_URL configured"',
"exit 1",
]
chezmoi_parts = [
'echo "Updating dotfiles..."',
"chezmoi update --force",
'echo "Syncing pixi global packages..."',
"pixi global sync",
'echo "Dotfiles updated successfully"',
]
fallback_cmd = " && ".join(fallback_parts)
chezmoi_cmd = " && ".join(chezmoi_parts)
update_cmd = (
"if command -v chezmoi >/dev/null 2>&1; then "
f"{chezmoi_cmd}; "
f"else {fallback_cmd}; "
"fi"
)
return workspace_ssh(workspace_id, command=update_cmd)Behavior is identical, but it’s now obvious where to add/remove steps.
2. Deduplicate “update dotfiles if possible” in main()
The “call dotfiles_update, check return, log warning, continue” pattern appears twice. A tiny helper keeps this in one place:
def refresh_dotfiles_if_possible(workspace_id: str) -> None:
ret = dotfiles_update(workspace_id)
if ret != 0:
logging.warning("Dotfiles update failed, continuing anyway...")Then in main():
# Fast-attach: skip workspace_up() if workspace is already running
if custom_id is None and get_workspace_state(workspace_id) == "Running":
logging.info(f"Workspace {workspace_id} is already running, attaching...")
setup_hostname(workspace_id)
refresh_dotfiles_if_possible(workspace_id)
ret = workspace_ssh(
workspace_id,
shell_command,
workdir=get_container_workdir(workspace_id),
)
update_cache_background()
return ret
# ...
# After ensuring workspace is up
setup_hostname(workspace_id)
refresh_dotfiles_if_possible(workspace_id)
# Attach to workspace
ret = workspace_ssh(
workspace_id,
shell_command,
workdir=get_container_workdir(workspace_id),
)This keeps all functionality intact while addressing the complexity concern and making future changes to the dotfiles refresh behavior centralized.
Owner
Author
|
Leaving this open — unlike #32, #35 and #36, this one is not superseded. Main has no So What is blocking it is age, not correctness. The branch is 203 commits behind main and conflicts, and its CI predates the To land it, it needs a rebase onto current main. Two things changed underneath it that the rebase has to account for:
Not doing that rebase here, since it is feature work rather than a CI fix. |
blooop
force-pushed
the
add-dotfiles-refresh-command
branch
from
August 14, 2026 06:37
5f4648f to
960163f
Compare
blooop
changed the title
dl <workspace> dotfiles refresh subcommand
Owner
Author
|
Rebased onto current main (was ~207 commits behind and conflicting). The branch now carries only the This is a scope reduction, not a rejection of the idea — the follow-up issue tracks what an opt-in version would need. Why the auto-attach half came outIts insertion points no longer exist. It patched two call sites that each did It breaks the round-trip budget. I injected the hook into That module's docstring says it exists so "a change that reintroduces a redundant round-trip fails here instead of quietly costing half a second". The failures are the suite doing its job, not incidental breakage. The cost is real. #139 measured a The two specific problems to solve before it returns
What is on the branch now
|
devpod applies dotfiles when it provisions a workspace, and workspace_up already passes --dotfiles/--dotfiles-script to `devpod up`. But attaching to a workspace that is already Running skips `devpod up` entirely, so a long-lived workspace keeps whatever dotfiles it was created with until it is restarted or recreated. This adds an explicit way to refresh them in place: - `dl <workspace> dotfiles` subcommand, which starts the workspace first if it is not already running - dotfiles_update() runs `chezmoi update --force && pixi global sync` over one devpod ssh, falling back to cloning DOTFILES_URL and running install.sh when chezmoi is not on the image - Bash completion, help text, and README updated
The fallback path interpolates DOTFILES_URL straight into a shell command for `git clone`. It comes from local devpod config rather than anywhere hostile, so this is not a security fix, but a URL containing a space split into two arguments and failed with a message about the wrong thing. Quote it with shlex.quote, and cover it with a test. Also normalises string quoting and line length in the surrounding block to what ruff format emits.
blooop
force-pushed
the
add-dotfiles-refresh-command
branch
from
August 14, 2026 06:40
960163f to
3e4f093
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
dl <workspace> dotfiles, a subcommand that refreshes dotfiles inside an already-provisioned workspace.dotfiles_update()runschezmoi update --force && pixi global syncover onedevpod ssh, falling back to cloningDOTFILES_URLand runninginstall.shwhenchezmoiis not on the image.dl <ws> dotfilesstarts the workspace first if it is not Running, then refreshes.--help, and the README command table list the new subcommand.Motivation
devpod applies dotfiles when it provisions a workspace. Main already covers that half:
workspace_up()readsDOTFILES_URL/DOTFILES_SCRIPTfrom the devpod context and passes--dotfiles/--dotfiles-scripttodevpod up(devlaunch/dl.py:1934-1937).What is missing is refreshing dotfiles in a workspace that is already up. Attaching to a Running workspace deliberately skips
devpod upaltogether —test/test_devpod_spawn_counts.py::test_attaching_to_a_running_workspacepins that path to onestatusand twosshcalls, with noup— so a long-lived workspace keeps whatever dotfiles it was born with. This subcommand is the explicit way to refresh them without arestartorrecreate.Scope change on rebase
This branch originally carried a second feature: an automatic dotfiles refresh on every attach. That half has been dropped. The reasoning is in a comment on this PR. Short version: its insertion points no longer exist (main collapsed them into
attach_workspace()), and re-adding the hook fails 5 tests intest/test_devpod_spawn_counts.py, the suite that exists to catch exactly this kind of added round-trip. The idea is not rejected, only descoped — see the follow-up issue for what an opt-in version would have to satisfy.Test plan
pixi run test— 1320 passed, 23 deselectedpixi run lint— ruff and ty clean, pylint 10.00/10pixi run prek— all hooks passpixi run dl --helplistsdl <user/repo> dotfilesNew tests cover the chezmoi path, the fallback path, shell-quoting of
DOTFILES_URL, the missing-DOTFILES_URLcase, and the subcommand against Running, Stopped, and failed-start workspaces.