fix(cli): make waired init on an enrolled device resume instead of fail - #367
Merged
Merged
Conversation
Three linked defects meant `waired init` on an enrolled Windows device failed with "daemon did not return a login session id" — every invocation, plain or elevated, with or without --control. NAVI hands operators that exact command to resume a stuck setup, so setup was unresumable on Windows by any documented means. 1. initStateDirMode had no Windows arm: os.Geteuid() is -1 there, so the euid guard was dead code and even an elevated run resolved %AppData%\waired while the daemon reads %ProgramData%\waired. The old comment deferred to "System via the SCM probe", which only fires for paths.AutoDetect — this decision passes Interactive. It now takes elevation as its Windows fact, table-tested across all three OSes. 2. identity.Load on that empty dir returned nothing, so the CLI sent Reauth=false, and the daemon answered with its designed idempotent no-op: phase active, no session id. 3. The CLI's only no-session special case required reauth==true, so it discarded Phase=active and reported a working daemon as a protocol failure. The model is `tailscale up`: the command is idempotent. An enrolled device resumes; an auth key is not spent while the existing credentials are valid (tailscale#19501) and — unlike tailscale#7995, where the key is dropped in silence — an unused key is said out loud, naming the new --force-reauth flag that would have used it. Re-authentication happens when that flag asks for it, or when the daemon reports the credentials are what is broken (auth_state=reauth_required), which keeps `waired init` the documented recovery for a locked-out device. The daemon is now the authority on enrollment: the CLI's own state dir can be the wrong one or unreadable, so a daemon-reported identity feeds the renew summary the disk used to be the only source of. A resume no longer forces --skip-integration either — the coding-tool step is part of the setup being resumed. Refs #313 Signed-off-by: gen16k <gen16k@users.noreply.github.com>
No harness ran `waired init` a second time — on any OS — so the defect #313 describes had nothing standing in its way: an enrolled device that fails to re-init looks exactly like a healthy one to CI. The new leg is defined by what it does NOT pass. No --state-dir, the way an operator types it and the way NAVI prescribes it to resume a stuck setup; that is precisely the combination that failed on every enrolled Windows box. The auth key IS still passed, because an already-signed-in device must not spend it and must say so. Windows registers 313 in $ContractBlocking as blocking from the start: the fix lands in the same PR, so there is no window where it should warn. ItSoft grew a -Repo parameter — these asserts started as monorepo-only and an agent-repo number rendered as "waired#313" points at an unrelated issue. The asserts are positive (exit 0, "resuming setup", "auth key was not used"). A negative assert on the old error string would be a grep for wording the product no longer prints, which is green forever — the failure mode scripts/ci/harness-failure-strings-guard.sh exists to name. Unix runs the same three asserts in both tier-2 legs, keyed on whether a key was minted rather than on the enrol mode, and the tier-2 assert floor goes to 23 to match. Refs #313 Signed-off-by: gen16k <gen16k@users.noreply.github.com>
Re-running `waired init` on a signed-in computer is now a normal thing to do — it is what NAVI prescribes for a stuck setup — so the reference says so, and names the flag for the case where signing in again is the point. Refs #313 Signed-off-by: gen16k <gen16k@users.noreply.github.com>
|
📘 Docs preview for this PR: https://waired-docs--pr-367-oiqlrgzr.web.app Rebuilt on each push; the preview channel auto-expires in 7 days. |
The wording no longer names a login phase — "unenrolled" / "logging_in"
are this protocol's words, not the operator's, and reaching for them is
how the old message ("no login session id") described a working daemon as
a protocol bug. What is left says what did not happen and what to do
about it, cheapest first: run it again, and reach for `waired doctor`
only if it keeps happening.
Refs #313
Signed-off-by: gen16k <gen16k@users.noreply.github.com>
…tatus Windows reports TokenIsElevated for a filtered/basic token too (runas /trustlevel:0x20000), so "elevated" does not imply "can read the service's ACL'd tree" — and since this branch made an elevated CLI target %ProgramData%\waired, that combination turned `waired status` into exit 1 for a user who previously read their own empty %AppData% and got the informational notice. waired#751's contract is that a status query is informational, not a failure, and the installtest asserts exactly that. Permission denied on the SYSTEM dir now prints the same "enrolled system-wide, needs elevation to read" notice the empty-per-user-dir path prints, and exits 0. An unreadable explicit --state-dir stays an error: nothing about it is system-wide, and saying so would send the operator to elevate a prompt that would still fail. Refs #313, waired#751 Signed-off-by: gen16k <gen16k@users.noreply.github.com>
gen16k
added a commit
that referenced
this pull request
Aug 1, 2026
The EN page picked up both the `--force-reauth` rows from #367 and the peer-only line from this branch; the ja page carries both translations, so the pair hash is recomputed to match. Signed-off-by: gen16k <gen16k@gmail.com>
gen16k
added a commit
that referenced
this pull request
Aug 1, 2026
…ed peer-only mode (#377) * feat(tray): split routing off the Inference menu, and add a fail-closed peer-only mode The rc7 review found the tray's "Inference" submenu unreadable: engine controls, status captions and the routing selector all sat at one level, with the abstract modes (Auto / Local only / Peer preferred) listed alongside concrete per-node pins. Three separate complaints, one shape. Split at the TOP level instead of nesting deeper. "Inference" now holds only this computer's engine — pause/resume, start/stop, install, mesh sharing, engine and model status. A new "Inference routing" parent holds the answer to "which computer answers my requests": the current worker, whether any peer engine is reachable, the automatic modes under a "Choose automatically" header, and the per-peer pins under a "Pin to one peer" header. The pins do NOT move one submenu deeper, as the review first suggested: fyne.io/systray's Windows backend does not render a third nesting level (the same limit that flattened these rows in waired#809), and submenus cannot hold separators. Disabled header rows are the available separation, and they are the existing pattern from the Claude Code submenu. Add "Peer only" alongside "Local only", its mirror image: serve from another computer or fail. Unlike peer-preferred it never falls back to the local engine, because falling back is precisely what the operator excluded — the silent-local-fallback shape #325 is removing from pins. The Claude surface inherits this for free: its non-destructive local retry is gated on pinned mode, so peer-only surfaces the error. That invariant now has a test. peer-only is agent-local state: no proto change, no CP contract change. Signed-off-by: gen16k <gen16k@gmail.com> * docs(tray): rename the stale 'Inference worker' submenu references The submenu the comments point at is called 'Inference routing' since the split. Comment-only; no behaviour change. Signed-off-by: gen16k <gen16k@gmail.com> * docs(i18n): re-record the ja CLI reference hash after the rebase The EN page picked up both the `--force-reauth` rows from #367 and the peer-only line from this branch; the ja page carries both translations, so the pair hash is recomputed to match. Signed-off-by: gen16k <gen16k@gmail.com> --------- Signed-off-by: gen16k <gen16k@gmail.com>
gen16k
added a commit
that referenced
this pull request
Aug 2, 2026
…ing sleeps (#357, #384) TestReconciler_SafetyNetSilencedByRecentDirectEvidence was a coin flip on Windows. It injected direct evidence, slept 35ms, then required that less than FallbackAfter (50ms) had passed -- 15ms of slack against a platform whose default timer granularity is ~15.6ms and whose time.Sleep rounds up to the next tick. One tick of overshoot inverted the assertion. Linux's ~1ms timers hid it, so it only ever failed on the Windows leg, on PRs that had not touched cmd/waired-agent at all (#355, #367, and again here). Both issues asked for the seam rather than a wider window, and CLAUDE.md §Test discipline says the same: put the seam below the behaviour under test. Widening only moves the coin toss. The reconciler turns out to need a very small one. It reaches for the wall clock in exactly two places -- Apply and Tick -- because every disco-driven decision is already stamped from the event's own At (evaluateSwitchLocked takes e.At). So a single `now func() time.Time`, defaulted to time.Now in newReconciler, covers the entire surface. With it, all seven time.Sleep calls in reconcile_test.go are gone: * the five Apply/Tick-timed tests install the fakeClock that setup_desired_test.go already defines for the setup executor, and say the elapsed time exactly rather than approximating it; * TestReconciler_NoFlapWithinDwellTime needed no seam at all -- dwell is measured from lastSwitchAt, which its own event stamped, so it just passes a later At. Of those seven, only the two in the safety-net silencing test were ever at risk: the rest sleep PAST a threshold and want it crossed, so overshoot was harmless. They are converted for determinism, not because they were failing. #357 asked for exactly that sweep. Package tests drop from ~0.5s of real sleeping to 0.27s. Verified the tests did not go green for the wrong reason -- each one still fails with its subject behaviour removed: * silencing rule (reconcile.go:841) neutralised -> SafetyNetSilenced fails * handshake gate (:847) neutralised -> StaysDirectIfHandshake fails * dwell gate (:512) neutralised -> NoFlapWithinDwellTime fails * safety net forced to never fire (:835) -> both firing tests fail TestNewReconcilerHasAClock pins the production wiring: a constructor path that forgets the clock would not fail any timing test (those install their own) -- it would nil-panic in Apply on a real agent. Verified: go build ./..., go vet, gofmt, golangci-lint (0 issues), TestReconciler_* at -count=30. Fixes #357 Fixes #384 Signed-off-by: gen16k <gen16k@gmail.com>
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
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.
Why
From the rc7 review (waired-ai/waired#986, F17). On two enrolled Windows 11 test
hosts, every
waired initfailed identically — plain, elevated, with orwithout
--control:NAVI hands operators that exact command to resume a stuck setup, so setup was
unresumable on Windows by any documented means.
Three linked defects:
initStateDirModehad no Windows arm.os.Geteuid()is-1there, so theeuid guard was dead code and even an elevated run resolved
%AppData%\wairedwhile the daemon reads
%ProgramData%\waired. The old comment deferred to"System via the SCM probe" — true of
paths.AutoDetect, and only of it; thisdecision passes
Interactive. (The installer's own init worked only becauseit passes
--state-direxplicitly.)identity.Loadon that empty dir returned(nil, nil), sorenewingwasfalse and the CLI sent
Reauth=false.no session id (
cmd/waired-agent/login.go) — and the CLI's only no-sessionspecial case required
reauth==true, so it discardedPhase=activeandreported a working daemon as a protocol failure.
The daemon was right the whole time; the CLI could not read "you are already
signed in". The same CLI bug is latent on Linux/macOS for a non-sudo re-init
against a root daemon — Windows is just the only OS where no invocation works.
What changed
The model is
tailscale up: the command is idempotent.waired init--auth-key--force-reauthreauth_requiredtailscale up --auth-key fails on restart if auth key expires, blocking already-authenticated nodes tailscale/tailscale#19501 argues for ("if valid state exists, reuse it").
--force-reauth, mirroringtailscale up --force-reauth, so a scriptedwaired initstays idempotent and does notrotate device tokens on every run.
tailscale up silently ignores authkey argument when already authed tailscale/tailscale#7995; the resume notice names the flag that would have used
the key instead.
reports
auth_state=reauth_required,waired initre-authenticates on its own,with no flag.
The daemon is the authority on enrollment. The CLI's own state dir can be the
wrong one (this issue) or unreadable (a standard user against the ACL'd tree,
waired#751), so
GET /waired/v1/identitynow feeds the renew summary and theaccount name that disk used to be the only source of.
A resume no longer forces
--skip-integration. That was right for anauth-only renew and wrong for a resume: the coding-tool step is part of the setup
being resumed.
CI: the leg that was missing
No harness ran
waired inita second time, on any OS, so this defect had nothingstanding in its way. The new leg is defined by what it does not pass — no
--state-dir, the way an operator types it — and still passes the auth key, sothe "key not spent, and said out loud" contract is covered too.
'313'registeredin
$ContractBlockingas blocking from the start (the fix is in this PR, sothere is no window where it should warn).
ItSoftgained a-Repoparameter —these asserts began as monorepo-only and an agent-repo number rendered as
waired#313points at an unrelated issue.was minted rather than on the enrol mode; tier-2 assert floor 20 → 23.
used"). The issue also asked for a negative assert on the old error string, but
that string is deleted by this PR — a grep for wording the product no longer
prints is green forever, which is the failure mode
scripts/ci/harness-failure-strings-guard.shexists to name.Verification
gofmt -l .,go vet ./...(+ proto),go test ./... -timeout 10m,go build -tags prod ./... && go vet -tags prod ./...,go test -tags prod ./internal/buildflag/...,make verify-cross— all clean.scripts/dev/installtest-windows.ps1parse-checked with pwsh 7(
Parser::ParseFile) from WSL. Note it is not inps-script-lint's$Targets, despite the file header claiming that policy covers it — worthfixing separately.
shellcheck -xon the bash side reports only the sameSC2015notes the file already carries for its existingok || badidiom.with the exact reported error:
TestRunInitViaDaemonResumesAnEnrolledDevice→daemon did not return a login session idTestRunInitViaDaemonSaysTheAuthKeyWentUnused→ same, and the key goes unmentionedTestRunInitViaDaemonNamesAPhaselessStart→ leaks the protocol symptomTestInitStateDirMode's{"windows", -1, paths.Interactive}row pinned this defect. It is replaced bya 3-OS × elevation table.
TestRunInitViaDaemonNamesAnAgentTooOldToReauth(thereauth=true+ activepath, including its "must not leak 'session id'" assert) stays green untouched.
runInitBodystill has no direct test; the re-auth decision was extracted into
reauthWanted(force, view)so at least that core is table-tested.Scope left out (say so rather than silently)
Invoke-AsStandardUserruns ona 60-second scheduled-task budget, which is too tight for an init.
-DaemonEngineleg.Fixes #313
Refs waired-ai/waired#986, waired-ai/waired#751