Skip to content

feat(hogli): add devbox:setup --reset-* + Y/n gate#59294

Merged
rnegron merged 11 commits into
masterfrom
feat/hogli-devbox-setup-reset-flags
May 21, 2026
Merged

feat(hogli): add devbox:setup --reset-* + Y/n gate#59294
rnegron merged 11 commits into
masterfrom
feat/hogli-devbox-setup-reset-flags

Conversation

@rnegron
Copy link
Copy Markdown
Member

@rnegron rnegron commented May 20, 2026

Problem

hogli devbox:setup had no way to undo a setting. The dotfiles prompt prefilled the saved URL as the default, so pressing Enter just re-saved it; even if you submitted blank, the config helper had no clear_* counterpart and existing workspaces kept the baked-in dotfiles_uri parameter — meaning a once-saved repo would keep re-cloning its .zshrc on every boot until you wiped it via the Coder UI. Re-runs also printed two lines of "Using saved X / Run --configure-X to change" per option, which got noisy.

Changes

  • New --reset-{git-identity,git-signing,dotfiles,claude} flags on devbox:setup. --reset-dotfiles also pushes dotfiles_uri="" to every existing workspace so the template stops re-cloning the repo.
  • New clear_dotfiles_uri() / clear_git_identity() helpers in config.py.
  • A single compact "Currently configured:" status block replaces the per-option "Using saved X" lines. Each maybe_configure_* helper now returns silently on the already-set path — the status block is the source of truth.
  • Y/n confirmation gate at the top of devbox:setup, bypassed when stdin is non-TTY or any per-option flag is passed, so CI and scripted callers keep working.
  • All existing flags (--configure-X, --skip-configure-X, -v) keep their previous semantics. Purely additive.

How did you test this code?

I'm an agent. I added two new test classes (TestDevboxSetupResets, TestDevboxSetupGate) covering each reset flag, the empty-param push to existing workspaces, the gate-bypass paths (explicit flag, non-TTY), and the "aborted on no" path. Three existing tests had assertions updated to match the new compact status output. 167/167 tests pass in tools/hogli-commands. No manual run of hogli devbox:setup against a real Coder deployment.

Publish to changelog?

no

🤖 Agent context

Investigation started from a user report: someone with a saved dotfiles URL whose .zshrc was conflicting wanted to back out, and the only escape hatch was the Coder web UI. Initial inspection found three stacked gaps (prompt couldn't be cleared, config helper didn't support clearing, existing workspaces kept the baked param). Considered a Setting protocol refactor that would have collapsed the four maybe_configure_* functions into one loop, but chose the smaller additive shape per "keep it simple, backwards-compatible" guidance — the protocol refactor stays available as a follow-up. Tool: Claude Code (Opus 4.7).

@rnegron rnegron marked this pull request as ready for review May 21, 2026 13:08
@rnegron rnegron added the stamphog Request AI review from stamphog label May 21, 2026
@assign-reviewers-posthog assign-reviewers-posthog Bot requested a review from a team May 21, 2026 13:08
github-actions[bot]
github-actions Bot previously approved these changes May 21, 2026
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clean developer-tooling addition by the owning team. New flags are purely additive, the Y/n gate correctly short-circuits on non-TTY stdin, the SSH double-quoting fix is correct, and all new behaviour is covered by tests. No production code, data models, or API contracts touched.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
tools/hogli-commands/hogli_commands/devbox/cli.py:776-798
**Reset flags re-trigger the configure prompt they just cleared**

After `_apply_resets(reset_dotfiles=True)` fires, `clear_dotfiles_uri()` removes `dotfiles_uri` from config. Then `maybe_configure_dotfiles(configure_dotfiles=None)` is called: because `existing_uri` is now `None`, the early-exit guard `configure_dotfiles is None and existing_uri` evaluates to `False`, and the user is immediately re-prompted to enter a dotfiles URL — the opposite of what `--reset-dotfiles` is supposed to accomplish. The same issue affects `--reset-git-identity``maybe_configure_git_identity(None)`. A minimal fix is to pass `configure_dotfiles=False` when `reset_dotfiles` and `configure_git_identity=False` when `reset_git_identity` before calling the `maybe_configure_*` helpers.

### Issue 2 of 2
tools/hogli-commands/hogli_commands/tests/test_devbox.py:1538-1566
**Near-identical secret-reset tests could be a single parameterised case**

`test_reset_git_signing_deletes_user_secret` and `test_reset_claude_deletes_user_secret` have identical structure: patch `delete_user_secret`, invoke with one flag, assert the expected secret name was deleted. Per the project's convention of preferring parameterised tests, these two cases can be collapsed into one `@pytest.mark.parametrize("flag,expected_secret", [...])` test, eliminating the duplication.

Reviews (1): Last reviewed commit: "test(hogli): mock list_user_secrets in d..." | Re-trigger Greptile

Comment thread tools/hogli-commands/hogli_commands/devbox/cli.py Outdated
Comment thread tools/hogli-commands/hogli_commands/tests/test_devbox.py Outdated
@github-actions github-actions Bot dismissed their stale review May 21, 2026 18:44

New commits pushed (delta classified non_trivial_delta) — stamphog approval dismissed; re-review running automatically.

Copy link
Copy Markdown

@stamphog stamphog Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR was denied by the size gate (778 lines across 5 files). The changes are pure developer tooling (hogli devbox commands) by the owning team with a human approval, no production code or data models touched. Needs a human to override the size gate.

@stamphog stamphog Bot removed the stamphog Request AI review from stamphog label May 21, 2026
@rnegron rnegron merged commit 49d016d into master May 21, 2026
156 of 157 checks passed
@rnegron rnegron deleted the feat/hogli-devbox-setup-reset-flags branch May 21, 2026 19:00
inkeep Bot added a commit that referenced this pull request May 21, 2026
- Update devbox:setup description with compact status block and Y/n gate
- Add "Managing devbox configuration" section with devbox:config:show
  and devbox:config:rm usage examples
- Note that clearing dotfiles pushes empty param to existing workspaces
@deployment-status-posthog
Copy link
Copy Markdown

deployment-status-posthog Bot commented May 21, 2026

Deploy status

Environment Status Deployed At Workflow
dev ✅ Deployed 2026-05-21 19:26 UTC Run
prod-us ✅ Deployed 2026-05-21 19:39 UTC Run
prod-eu ✅ Deployed 2026-05-21 19:42 UTC Run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants