Skip to content

fix: Go-side Lows batched on #399 - #526

Merged
wstein merged 7 commits into
mainfrom
fix/399-go-lows
Oct 9, 2026
Merged

wstein merged 7 commits into
mainfrom
fix/399-go-lows

Conversation

@wstein

@wstein wstein commented Oct 9, 2026

Copy link
Copy Markdown
Owner

Fixes the Go-side Lows from #399 (each with a test that failed first):

  • whr github app create refuses a loopback http:// public_url from the config (command, doctor public-url step and config.Load agree); a non-string public_url no longer breaks every client command; the tailscale serve hint prints only for an all-digit port.
  • workspace-folders Fix declares NeedsSudo; an owner/mode change during the confirm is pinned; the NeedsAdmin message and exit are pinned.
  • workspaceVolumes escapes the roots; the roots steps are pinned to needsRoots.
  • config.Load opens with O_NONBLOCK so a FIFO path cannot hang.
  • Doctor repair lines keep the --prefix that was given (golden files change only by that flag); unused dscl stub and a help example removed.

Refs: #399

Review: Opus CLEAR on 27e36f4 (8 mutations each fail the matching test; full go test ./... and make check-ci exit 0). Lows (batched on #399): isDigits is untested and a named port prints the funnel line without a command; a setup-report path in one test is not sandboxed. Unverified: a live whr github app create, real Tailscale, outside readers of the longer doctor fix strings.

🤖 Generated with Claude Code

wstein and others added 7 commits October 9, 2026 13:33
A loopback http:// name is accepted only from --public-url (a test
double); a configuration value goes through NormalizePublicURL like
config.Load and the doctor. A non-string public_url no longer fails
ReadClientConfig for every client command, and the tailscale serve
hint prints only for an all-digit port.

Refs: #399
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
The fix runs sudo commands but previews none, so a non-admin was asked
"Ready to run this?" first. Also test that an owner or mode change
during the confirm runs nothing.

Refs: #399
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
A mutation of the len(admin) > 0 branch survived because the Fail loop
exits non-zero anyway; assert the administrator message.

Refs: #399
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
workspaceVolumes printed roots and df paths raw; escape them like the
other steps. Also pin that the three roots steps use needsRoots.

Refs: #399
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
Load used os.Open, so a FIFO named as the configuration hung the
process; open with O_NONBLOCK as the secret reader does.

Refs: #399
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
The setup commands already carry --prefix; the doctor's fix lines
aimed at /opt/whr for a user with another prefix. Goldens updated.

Refs: #399
Co-Authored-By: Claude Sonnet 5.5 <[email protected]>
@wstein

wstein commented Oct 9, 2026

Copy link
Copy Markdown
Owner Author

Opus review CLEAR on 27e36f4. Eight mutations each fail the matching test; full go test ./... and make check-ci exit 0 (author and reviewer). Lows (batched on #399): isDigits untested and a named port prints the funnel line without a command; a setup-report path in one test is not sandboxed. Unverified: live whr github app create, real Tailscale.

@wstein
wstein marked this pull request as ready for review October 9, 2026 11:43
@wstein
wstein merged commit fc56edb into main Oct 9, 2026
20 of 21 checks passed
@wstein
wstein deleted the fix/399-go-lows branch October 9, 2026 11:51
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.

1 participant