Issue #33, Addendums 3 & 4: unattended-recovery diagnostics, a bounded redial budget, and a checked glossary - #37
Merged
Merged
Conversation
Two complaints from the field are really one complaint: being asked to do by hand what the guard exists to do on its own. "I have to turn it on after every reboot" and "every drop needs a manual window" both have several possible causes that look identical from outside, and nothing told them apart. Three checks now do. boot service reads the OS service unit and separates "nothing is registered", "registered but not set to start at boot", and "both fine, and enforcing now". The last is the useful one: it rules enforcement out and leaves the menubar app's login item, which is a different fix entirely. It reads the unit file rather than asking the service manager, because doctor is root-free and on macOS an unprivileged status query cannot see the system domain — it answers "not installed" for a job that is loaded and running, which would have sent people to reinstall a working service. The unit is also the better source for the question being asked: the manager says what runs now, the unit says what happens at the next boot. arm at boot says whether the next reboot arms the guard or opens into standby. vpn.armAtBoot may only override the live probe once a tunnel has been observed up on this host (ADR-0008), and that half fails silently — the setting reads "on" throughout — so the check names which half is missing and how to satisfy it. learned endpoints tells apart the two opposite reasons a drop keeps needing a window: addresses learned and then aged out, or a VPN that rotates its server address. The remedies point in opposite directions, so guessing wrong sends you to the wrong knob — a test pins that an aged-out store is never diagnosed as rotation. Rotation leads with the hostname fix, since vpn.endpointRefresh re-resolves it and follows the rotation instead of chasing it. All three are informational and never move the exit code; none is a guard about to fail closed. printDoctor claimed it had a section for every check but nothing enforced that, so three new ones would have landed in the unformatted leftover printer — sectionedChecks and TestEveryCheckHasASection make the claim true. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The automatic redial window is unbounded across drops (a fresh 30s on every drop, forever) and zero within a flap (redialMinUptime suppresses it outright, on exactly the connection that needs help). ADR-0009 inverts both: a rolling budget of total window-open time, and redialMinUptime demoted from suppressor to backoff seed. This lands the decision half and its config surface. internal/redial is pure and clock-injected like internal/decision: it holds the ledger and the backoff, and answers "may this drop open a window, and for how long". Budget is debited when a window opens and credited back when it closes early, so a redial that succeeded in three seconds costs three seconds — charging the offer instead of the exposure would punish exactly the outcome the window exists to produce, which is why capping the *count* of windows was rejected. vpn.advanced.redialBudget (2m) and .redialBudgetWindow (15m) are declared like every other tunable, so the app's hints, `config schema`, and docs/usage/config.md all derive from one number. Both are live keys, so the run loop must read them per drop rather than capture them. Neither takes the Disabled sentinel, and both refuse a "0" by name rather than normalising it: they are limits, so "off" would have to mean *no limit* — the opposite direction from every other "0" here — and an Off switch that removes a bound rather than a feature reads backwards on a security surface. vpn.redialWindow: "0" stays the one way to turn the automatic window off. The Advanced settings group moves out of SettingsView's body; two more rows pushed it past the type-checker's budget, and the compiler said so by name. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Wires ADR-0009's ledger into maybeAutoWindow. redialMinUptime stops suppressing the window and seeds a backoff instead: a fast drop still gets a window, shorter for each consecutive fast drop and with a growing cooldown, until the budget refuses and the guard holds. Order of the gates is unchanged where it matters. Hold the line still returns first and spends nothing — it removes a relaxation rather than granting one, and must never consume an allowance the next accidental drop is entitled to. The preconditions (never from standby, never from FULL BLOCK, never while a window is open, never for a tunnel not observed up) still short-circuit ahead of the ledger, so a drop that could not have opened a window costs nothing either. Budget.Close is called from both paths that clear windowActive, and unconditionally: it is a no-op when no automatic episode is open, so neither call site needs to know whether this window was an automatic one, and the takeover case still settles — clamped to what the budget granted rather than the operator's longer cap. The early close is the case credit-on-close exists for. The ledger lives in Run's frame, not Options: it is mutable per-episode state, and there it is touched only by the goroutine that owns every Backend.Apply. Settings are read through a closure per drop, never captured, because all four are live keys. A refusal is not silent. The log names which bound refused, the uptime that led there, what remains of the budget, and the instant a window can next open. TestVPNAutoWindowFlapGuard asserted the behaviour this replaces, so it becomes two: a fast drop DOES get a window, and a budget that cannot afford one refuses. Every other runner test that exercises the automatic window now sets a budget explicitly — without one the ledger can afford nothing, so a test expecting no window would have agreed with itself while proving nothing. Verified no ruleset change: print-rules stdout, stderr and exit status are byte-identical to 0e49ec5 across all five example configs x three modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A refused redial window was only a log line. Both surfaces showed "all traffic is cut until your VPN redials" — which is the one thing that is not true after a refusal: nothing relaxes until the budget allows it, however fast the VPN comes back. The user is left unable to tell a wait from a wall, which is the failure mode this project treats as worst: a guard that silently declines to help. state.Snapshot gains an additive `redial` object, present only while a refusal stands (an open window is already reported by `switch`; two fields for one truth is two fields that can disagree). It carries the reason as a stable identifier, the instant a window can next open, the remaining budget in seconds, and the consecutive fast drops. internal/render turns it into the sentence, REPLACING the "…until your VPN redials" clause rather than joining it, and always naming the instant — "the guard is holding" without a time is the wall. An unrecognised reason from a newer daemon falls back to what is true of every refusal and still names the time. The app decodes the field but composes nothing: it renders display.detail, so the CLI, the state file and the menubar say the same words. Verified no ruleset change: print-rules stdout, stderr and exit status still byte-identical to 0e49ec5 across five configs x three modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The glossary has always claimed to be the authority — "when user-facing copy and this page disagree, the copy is wrong" — but nothing verified it, and about forty places had drifted back to "protection", "egress" and "daemon" while the page said not to. internal/vocab parses the banned-word table out of the glossary itself, so there is one list and it is the one a human reads. A hardcoded list would agree with the page until someone edited one of the two; this cannot. Same trick config's docdrift_test already uses on config.md. The Go side uses go/parser, not grep, because the distinction that matters is unavailable to a text search: a literal reaching fmt.Print* is copy, the same literal reaching o.Log.Warn is the technical register where "daemon" and "egress" are the correct words. Multi-line usage blocks are checked per line so a violation names the sentence rather than the help page, and so an exemption cannot silently cover future edits to it. Swift gets a line scan — no parser here, and a false positive costs a rewording while a parser costs a dependency. The table gained two markers, because the register split is real and a single banned list cannot express it: unmarked means wrong in both registers, ‡ means user-facing copy only (correct in logs and in these docs), † means the ban needs judgement no matcher can supply. The four terms the audit named are adjudicated: "relaxation" and "guard is disarmed" are enforced, "peer"/"server" and "utun"/"interface" are † because the recommended replacement contains "server" and because "physical interface" is correct. "Disarmed" is phrased narrowly on purpose — disarm is the right verb for hold the line, which really is an armed flag. Copy fixed accordingly: `status` prints "control socket:" not "daemon control:", block/unblock drop "(via daemon)", refusals read "dezhban refused:", and the app's panic tooltip and block hint lose "daemon" and "egress". status --json keys are untouched; they are identifiers. Exemptions carry a written reason each — a flag's own name, an env var, a shipped ADR's filename. Verified the lint fails on a reintroduced violation and that print-rules is byte-identical to 0e49ec5 across five configs x three modes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Applies every finding, including the nits. Three change behaviour a user can see; the rest close gaps in what the new code checks about itself. A refusal no longer names a time it has gone past. The redial refusal is decided on a tunnel-down edge and re-decided only on the next one, but snapshots keep publishing in between — so a tunnel that stays down carried "it can relax again at 3:19PM" to 3:44PM and beyond, for a moment that came and went with nothing happening. Both surfaces now say "the next time your VPN tries to reconnect" once the instant has passed, which is what is actually being waited for. A past time is worse than no time: it states a commitment that was broken, which inverts the exact wait-versus-wall confusion the sentence was added to end. The backoff no longer deepens on drops it refused. shortRun and coolUntil were mutated before the budget check, so a drop turned away for exhaustion still advanced the streak and pushed the cooldown out — refusals compounding into a wait neither bound asked for, with the guard still holding after the budget had rolled over. Grant now computes the backoff and commits it only past every refusal, which is the rule the cooldown early-return already followed and said so in a comment eleven lines above. The two budget keys refuse a "0" written in the file, not only one typed at `config set`. One path errored by name while the other accepted it and normalised it to 2m, so the same value meant two different things depending on how it was set — the silent-discard half being the failure this project calls its worst. Also in redial: Grant refuses a disabled window outright rather than computing its way to a zero-length episode, and settles an orphan if called with one open. Neither is reachable through the run loop, but expire never ages out an unsettled episode, so an orphan would be charged its full grant for the life of the process — a permanent, silent budget leak guarded only by a caller several hundred lines away. The vocabulary lint was not looking at the settings copy. Tunable.Label/Help and the preset summaries are the text `config schema` prints and the macOS Settings pane shows, but they live in internal/config while the Printf that writes them lives in cmd/ — so a lint that follows the print statement sees a format verb and calls the file clean. Adding the package found seven violations, six of them in the table CLAUDE.md describes as the one place every surface derives its hints from. Copy is where the words are, not where the write call is. Two parser fixes fell out of writing the package its first unit tests. Load latched onto the first |---| in the section, which is now the marker legend rather than the terms table — harmless today only because no legend row's first cell happens to be quoted, so it anchors on the "Don't say" header instead. And the "did we find a table" check read inTable, which a later H2 clears, so the parse silently depended on the vocabulary section being the glossary's last: any section added below it would have failed Load and taken the whole lint down with it. vocab_test.go is new. Every promise in the doc comments — word boundaries, the \s+ relaxation, the ‡ register split, one row yielding several phrases, † rows staying out — was verified only by the absence of failures elsewhere, which is the wrong direction for a matcher: break compile() and the lint reports zero violations, and zero violations reads as success. Nits: Check's `copy` parameter renamed for the register it names rather than the builtin it shadowed; swiftLiterals stops at a trailing `//` without truncating a URL inside a literal; the dead "docs/adr/" exemption removed in favour of saying why docScopes never lists it. Verification: go build, go vet, go test ./... → 683 passed, 26 packages (was 656; +27 new). swift build && swift test → 99 passed, 12 suites. print-rules stdout, stderr and exit status are byte-identical to e1c9596 across all 5 example configs × 3 modes, so none of this reached the ruleset. `config schema` and `preset list` differ in exactly the seven copy lines and nothing else. Not run: the privileged on-host checks, which still need a reboot and a real flapping VPN. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Round two of the PR #37 review, all six findings. A failed window open cost the full budget. maybeAutoWindow debits the grant before openWindow applies rules — the decision has to come first — but openWindow reports an Apply failure by leaving windowActive false and returning, so the debit stood with no window to close it. expire never ages out an open episode (deliberately: a real open window must not be forgotten), so one failed open could spend the whole budget and refuse every later drop, charging for exposure that never happened. It also self-healed wrong: the next Grant settled the orphan at min(now-start, granted), i.e. in full. The grant is now credited back when the open fails, which is what "the ledger measures exposure taken, not exposure offered" was supposed to mean. status --json published an open window and a standing refusal together. Only maybeAutoWindow cleared redialRefused, so a manual switch or a pause opening over a refused drop left state.redial beside state.switch — and cli.md tells scripts to match on .redial.reason while promising an open window is reported "instead, never here". Any window opening now clears it, whatever the trigger. The rendered sentence was never affected: redialRefusal is only reachable from guardDisplay's downed-tunnel branch. A redialBudget under 5s validated clean and turned the automatic window off by arithmetic. 5s is redial.MinGrant, the shortest window worth opening, so a smaller budget can never afford one — the feature was permanently off while the config read as on, and the surfaces covered for it: with an empty ledger nextEligible answers "now", so status told the user it could relax again the next time the VPN reconnected. Refused by name now. The constant is duplicated into config rather than imported, because config depends on nothing but the standard library and nearly everything imports it; a test-only import of redial pins the two together so they cannot drift. The two new liveKeys had no behaviour test, only the three completeness ones — and CLAUDE.md is explicit that a copied field is not the promise. The wiring was in fact correct; the coverage was not. Both directions are pinned now, including the one that matters for a bound: lowering it binds on the next drop, not at the next restart. Nits: Budget.Remaining mutates (it expires the ledger) and now says so; cli.md documents that remainingSeconds goes stale the same way nextEligible does, and only ever understates. Verification: go build, go vet, go test ./... → 692 passed, 26 packages (was 683; +9). swift build && swift test → 99 passed, 12 suites. Both runner regression tests were confirmed to FAIL with the fixes reverted, so neither passes vacuously. print-rules stdout, stderr and exit status are identical to 964b4f6 across all 5 example configs × 3 modes over three full rounds, and `config schema` is byte-identical — none of this reached the ruleset or the copy. (A single first-round mismatch on dezhban.vpn-guard.json was DNS flake: that config resolves vpn.example.com, so its endpoint set varies between any two invocations; 15 targeted retries agreed.) Not run: the privileged on-host checks, which still need a reboot and a real flapping VPN. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A recovered tunnel was still held by the redial backoff's cooldown. The cooldown gate ran before any evidence about the current drop, so a tunnel that redialed, carried a confirmed exit, stayed up past redialMinUptime and then dropped again was refused a window with budget to spare. That refusal is not a short wait: it is only re-decided on the next tunnel-down edge, so it stood until an operator ran `dezhban switch` — the manual interaction ADR-0009 exists to remove. Grant now reads the same evidence that already disqualifies a drop from being "fast", and a non-fast drop clears coolUntil along with the streak, so a disproved flap cannot leave a deadline behind to refuse a later drop. The budget is untouched; it remains the real bound. The budget floor now follows the ledger's floor rather than the constant. redial.floorFor honours a vpn.redialWindow shorter than MinGrant as written, so a 3s window with a 4s budget opens fine — validation rejected it anyway while telling the user a window "could never open", which was not true of that config. state.redial.remainingSeconds is re-read from the live ledger on each snapshot instead of being frozen at the refusal, since episodes roll out of the period while the cut lasts. Published as a copy, never a mutation of the carried record, which Publish may still hold. reason and nextEligible are the decision and still stand as decided; cli.md's paragraph promising the opposite is corrected. Nits: closeWindowRevert and finishCloseProbe take `now` instead of reading the clock mid-function, so one turn of the loop settles the ledger at the instant it opened against; vocab.Check's doc no longer claims to report every occurrence when it reports the first per term. Verified: go build/vet clean, 695 Go tests pass (26 packages), 99 Swift tests pass (12 suites). print-rules stdout, stderr and exit status are identical to the pre-fix binary across three configs x three modes — the only behavioural difference anywhere is the short-window budget config that was previously refused and now validates.
Three of these are the same failure in different clothes: a check that
reports success while enforcing nothing.
**The vocabulary lint had two blind spots and a dead rule set.** Any banned
phrase ending in punctuation compiled to a regex with a trailing \b that
cannot match, so three rows — every one naming a config key — parsed,
counted toward the zero-terms guard, and checked nothing. Anchors are now
applied only where there is a word to anchor against, and
TestEveryTermMatchesItself fails any row that cannot find itself in its own
text. Separately the Swift scanner could not see multi-line (""") literals
at all: the fences carry no content and the content lines carry no quotes,
so two user-facing alerts said "the daemon" while the file read clean.
Teaching the scanner about fences found one of them that visual review had
already missed.
**internal/runner's refusals are copy, and four of them said so badly.**
reply(false, …) lands in control.Response.Error, which the CLI prints after
"dezhban refused:", so `dezhban pause` in standby answered a user with two
retired words in one sentence. The package is in the lint's Go scope now;
its log calls stay exempt through isLogCall, which is the distinction
go/parser buys. The startup lockout refusal no longer says "egress" either.
**A refusal's time is a bound, not an appointment.** Nothing fires at
nextEligible — the decision is retaken only on the next tunnel-down edge, so
a tunnel that cannot come back on its own produces no further edge and no
further decision. "It can relax again at 3:15PM" promised an unattended
recovery that was not coming, worst in exactly the case the window exists
for. cli.md already stated the caveat for scripts; a person deserves it
more, not less.
Two ways the instant was also simply wrong: a cooldown refusal reported only
the cooldown deadline, so a host both backing off and out of budget was told
3:00PM and then 3:15PM — the moving deadline the published refusal exists to
avoid. And an episode exactly one period old was still counted, putting the
promised instant one tick before the ledger could afford a window. Writing
the regression test for the first uncovered the second.
**nextEligible is omitted rather than zeroed.** omitempty does not omit a
zero time.Time, so a writer without an instant would emit
"0001-01-01T00:00:00Z" — which the app's ISO8601 decoder refuses, failing
the whole snapshot decode and reading as "stopped" while dezhban enforces.
omitzero on the Go side, optional on the Swift side.
Verified: go build/vet/test 697 passed, swift build/test 100 passed.
print-rules stdout and exit status byte-identical across 5 configs x 3 modes
— no enforcement change.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A refusal names an instant the guard can relax again. Nothing acted at it. The decision was retaken only on the next tunnel-down edge, so a tunnel that could not come back on its own — a rotated server address the endpoint pass does not cover, which is precisely the case the automatic window exists for — produced no further edge, the refusal stood indefinitely, and the budget refilling changed nothing. The user waited out a time that was never going to be honoured and then ran `dezhban switch` by hand: the manual interaction ADR-0009 exists to remove, reintroduced by the bound meant to be safe. A timer in the run loop now re-takes the decision when the bound lifts. **This is trigger 2 completing, not a fourth trigger.** A trigger is a CAUSE for relaxing the guard and this admits none: the drop already qualified at its own edge — healthy GUARD, a tunnel observed up, not standby, not FULL BLOCK, no window open, hold not armed — and only the budget or cooldown said no. Every rail is unchanged: same Grant, same ledger debit and credit-on-close, same TriggerAuto episode under redialWindowMax. The rails that keep it from becoming one: - At most one automatic window per drop, still. A retry runs only while a refusal stands; a grant clears it and disarms the timer, and nothing re-arms. TestTheRetryStillOpensAtMostOneWindowPerDrop pins it. - The retry re-asks the SAME question. The drop's uptime and confirmed-exit status are captured at its edge, because now-tunnelUpSince keeps growing while the tunnel is down — a retry deriving uptime fresh would report a fast drop as healthy and cancel the backoff at the moment it is working. - Every precondition is re-checked, through the one predicate both callers share (autoWindowPossible), so the retry cannot relax a guard the drop edge would have refused. - Hold the line still wins, and is not spent: an operator arming it mid-cut means "keep me cut", and the flag names the NEXT drop. Hold may only ever subtract a relaxation, so it must be able to subtract this one. - Armed only for an instant in the future, so a bound already lifted schedules nothing rather than spinning. Also, the rest of the Go-side decode nit: NextEligible was not the only time.Time published bare. SwitchState.Until, DropRecord.At and HoldState.At had the same shape, and render.go already treats a zero as reachable for two of them — Go guards it, Swift explodes on it. All three are omitzero now, with the Swift counterparts optional and one SwitchState.timeLeft/leftSuffix helper so the five display sites cannot drift on what a missing deadline means: drop the countdown, keep the label. Verified: go build/vet/test 699 passed, swift build/test 103 passed. The new retry test fails with the exact diagnostic it was written for when the retry is disabled. print-rules stdout and exit status byte-identical to the start of this session across 5 configs x 3 modes — no enforcement change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three of these are the same failure: a surface that states something the
daemon will not do.
**The copy still described an instant nothing acted on.** The previous commit
put a timer in the run loop that re-takes the decision at nextEligible, and
every sentence, comment and doc around it was still written for the era when
the time was inert — render.go's own comment opened "Nothing in the daemon
fires at nextEligible", which had just stopped being true. Wording it as a
bound only ("No window will open before 3:15PM") now UNDERSTATES the guard,
and understating it leaves the user reaching for the manual escape hatch
ADR-0009 exists to remove. Both surfaces read *"dezhban tries again at 3:15PM
— no window opens before then"*: the attempt is the strongest true claim
available, because the re-decision consults the budget afresh and re-checks
every precondition, so it may refuse again — "the guard relaxes at 3:15PM"
would not be true. A passed deadline still drops the instant rather than
reprinting a moment that came and went; that clause's reason changed (the
retry has already run and refused) but its conclusion did not.
**A refusal outlived the setting that justified it.** Reloading
`vpn.redialWindow` to `"0"` mid-cut left the standing refusal published, so
`status --json` and the app went on naming a time for a window that had been
switched off entirely — a promise nothing would keep, which is precisely the
failure publishing the refusal exists to prevent, inverted. The retry cannot
clear it on its own: disabling the window is exactly what makes
autoWindowPossible false, so the re-decision returns before reaching the
ledger. Cleared in both places — in the reload handler, where the reason is
known and can be logged, and on grantAutoWindow's early return, which catches
any other precondition that lapses while a refusal stands.
**`doctor` reported an unreadable boot service as a missing one.** Any read
failure on the launchd plist — a permission problem on it or on
/Library/LaunchDaemons — became Present:false, which renders as "not
registered to start at boot": a user whose guard is installed and enforcing
right now, told to reinstall it. That is the same false negative Boot exists
to avoid, reached from the other side. Only fs.ErrNotExist may mean absent;
anything else is Determinable:false, and the undeterminable summary no longer
names a platform, since it is no longer Windows-only. platformBoot is now a
one-line wrapper over bootFrom(path) because it reads one fixed system
location — on every CI runner and most dev machines only the absent branch is
reachable, so the branch that matters most would go untested for want of a
file no test can put in /Library/LaunchDaemons.
**A granted window that could not be applied said nothing.** It is the one
drop outcome that neither opens a window nor publishes a refusal: the ledger
granted, Backend.Apply failed, the guard held, and nothing anywhere named the
reason. Now logged. Deliberately no retry armed — nothing is waiting on a
bound to lift, the failure is the Backend's, and openWindow has already set
enfErr, so the posture the surfaces show is the enforcement error, which is
the more urgent truth than a missing redial window.
Also a rule that read as load-bearing and was not: `(?s)` on runAtLoad only
changes what `.` matches and the pattern has no `.`. `\s*` is what lets the
key and its value sit on separate lines, which is how kardianos renders them.
The new reload test needed one fix to fail for the right reason: it must
assert on the last LIVE snapshot, not the last one, because shutdown
publishes a terminal posture:"stopped" record carrying no refusal — asserting
on that passes whether or not the refusal was ever dropped.
Verified: go build/vet/test 701 passed, swift build/test 103 passed.
print-rules stdout and exit status byte-identical to HEAD across 5 configs x
3 modes — no enforcement change. (Its stderr is not: an autodetect log line
carries a timestamp, so only stdout is comparable.)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
**Cancelling hold the line mid-cut stranded the drop permanently.** Arming it during a cut correctly suppresses the pending re-decision. Cancelling did not give that re-decision back: the retry fires once, the hold consumes it, the timer disarms itself, and nothing re-armed it — so nothing decided again until the next tunnel-down edge, which cannot arrive while the tunnel is already down. That is exactly the wall ADR-0009 exists to remove, reached by using the feature meant to be the CAUTIOUS choice, and both surfaces went on saying "dezhban re-checks on its own" throughout. This is the "only ever subtracts" rule read backwards: cancelling is the subtraction being taken back, so what it took has to return. Still no fourth trigger — the work is done by the same retryAutoWindow the timer would have run, so every rail applies unchanged. resumeRedialRetry fires only when nothing is armed, because a hold cancelled BEFORE the deadline leaves the original timer running and still governing. **boot_linux kept the false negative boot_darwin had just been fixed for.** Any stat error read as absent, so a permission problem on the unit or on /etc/systemd/system reported "not registered to start at boot" — a user whose service is installed and enforcing, told to reinstall it. The same rule boot.go's own doc comment states, applied on one platform and not the other, which is how it comes back. Only fs.ErrNotExist means absent now. The enablement symlink gets the same split, and it matters more there: an unreadable wants entry used to read as "installed, but not set to start at boot" for a service that IS enabled, sending the user to fix something that was not broken. **Budget.Remaining mutated the ledger.** It expired episodes in place, which was safe only because every caller happened to sit on the run loop's goroutine — and the run loop calls it once per published snapshot. Safety by call-site convention is not safety, and the doc comment had to warn future callers off. spentAsOf excludes rolled-off episodes from the sum instead; Grant stays the only thing that changes the ledger. Both now share one `retired(e, cutoff)` predicate. Having just deleted one copy of that rule, leaving two would be the same mistake in a new place: a read that aged the ledger differently from the write would report a budget no decision will honour — the same class of lie as a nextEligible nothing acts on, which is what this whole branch was built to end. **The hold/retry interaction had no test at all**, which is why the first finding survived a five-line comment describing the exact semantics it got wrong. TestCancellingHoldRestoresTheRetry fails with the fix reverted; TestHoldArmedMidCutSuppressesTheRetry passes either way on purpose — that behaviour was already correct and had simply never been pinned. Both share redialTwoDropScript with the existing control, so the three cannot drift onto different fixtures. boot_linux_test covers the unreadable unit and the unreadable wants link, skipping when systemd is not the running init or when running as root, where mode bits are ignored and the assertions would be vacuous rather than wrong. Verified: gofmt clean; go build/vet clean on darwin, linux and windows; 703 Go tests pass; go test -race clean across runner/redial/svc; swift build/test 103 passed. print-rules stdout and exit status byte-identical across 5 configs x 3 modes — no enforcement change. The three Linux boot tests are cross-compile-checked only (GOOS=linux go vet); they first execute wherever Linux CI runs. Co-Authored-By: Claude Opus 5 <noreply@anthropic.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
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.
Closes the last of #33: the two
addendums filed as issue comments and never scheduled. Addendum 3's item 1 and
Addendum 4's glossary itself already shipped in #36 — what remained was the
redial budget, the diagnosis work, and the enforcement half of Addendum 4.
Three phases, in the order the plan required, because P's findings inform Q's
defaults. They are also pushed as three stacked branches
(
feat/addendum-3-diagnostics→feat/redial-budget→feat/glossary-lint) ifreviewing them separately is easier; this PR is all five commits.
The outcome all three aim at is the product principle behind Addendum 3: keep
the real IP hidden with the minimum possible interaction. A user forced to open
a window by hand after a drop, or to re-arm after a reboot, is a product failure
— so diagnose why it happens (P), remove the cause (Q), and stop the vocabulary
drifting back (R).
Phase P —
doctorsays whether a reboot or a drop will need you againThree checks, for the two complaints that are really the same complaint.
Boot service separates "nothing is registered", "registered but not set to
start at boot", and "both fine, and enforcing right now" — the last matters
because it rules enforcement out and leaves the menubar app's login item, which
has an entirely different fix.
It reads the service unit file rather than asking the service manager. The
plan said to use
kardianos/service'sStatus(), butinternal/svc/launchd_darwin.gofalls back to legacy
launchctl listwhen unprivileged, which cannot see themacOS system domain and reports a running daemon as not installed.
doctorisroot-free by contract, so that would have sent users to reinstall a working
service. A unit file is both root-free and a better match for the question:
"what happens at next boot" rather than "what runs now".
Arm at boot says whether the next reboot arms the guard or opens into
standby.
vpn.armAtBootmay only arm when a tunnel has been observed up at leastonce on this host, and that half fails silently — the setting reads "on" the
whole time — so the check names which half is missing.
Learned endpoints tells apart the two opposite reasons a drop keeps needing a
window by hand: addresses learned and then aged out (retain them longer), or a
VPN that rotates its server address (retaining more only delays it; a hostname
re-resolves and follows the rotation).
All three are informational and never change the exit code. The macOS Diagnostics
pane renders them through the existing
doctorReportcontract.Phase Q — a bounded redial budget with backoff (ADR-0009)
Today's automatic redial window is wrong in two ways, in opposite directions.
Across drops it is unbounded. Every drop earns a fresh 30s, with no ceiling
on how many drops — a link dropping once a minute produces 30s of relaxed guard
every minute, indefinitely. Nothing in the design said how much total exposure a
redial policy may cost, so nothing enforced one.
Within a flap it is zero, on exactly the connection that needs help.
redialMinUptimesuppressed the window outright, pushing the users with theworst connections onto
dezhban switchby hand — a product failure in a toolwhose promise is minimum interaction.
The budget inverts both.
internal/redialis pure and clock-injected likeinternal/decision, so a pathological flap is testable as a sequence of instantsrather than as a wait.
Credit-on-close is the load-bearing part. Budget is debited when a window
opens and credited back when it closes early, so the ledger measures exposure
taken, not exposure offered — a redial that succeeded in three seconds costs
three seconds. Without it, ten successful 3s redials would exhaust the same
allowance as ten 30s failures, punishing exactly the outcome the window exists to
produce. It is also why "cap the count of windows per interval" is rejected as
Alternative 3.
redialMinUptimebecomes the backoff seed: a fast drop still gets a window,halved per consecutive fast drop with a growing cooldown, until the budget
refuses and the guard holds.
A refusal is published, not only logged.
status --jsongainsstate.redial(reason,
nextEligible, remaining budget) and both surfaces read "Your VPN hasdropped often enough to use up its redial budget, so the guard is holding and
traffic stays cut. It can relax again at 3:15PM." — composed once in
internal/render; Swift decodes the field but writes no prose. Without a time,"the guard is holding" leaves a wait indistinguishable from a wall.
What does not change
Still trigger 2 — no fourth trigger.
vpn.redialWindow: "0"still removes itentirely and stays the only way to. Hold the line still suppresses ahead of the
ledger and spends nothing.
redialWindowMaxstill caps any single window. Thepreconditions, the hold-on-unknown rule and the tunnel+destination-scoped geo
pass are untouched.
One deliberate departure from the plan
The plan specified
redialBudgetasDisablable: true. Both budget keys insteadrefuse a
"0"by name. On every other duration"0"means "off"; on alimit it would have to mean no limit — the opposite direction — and a
security surface offering an Off switch that removes a bound rather than a
feature is a misreading waiting to happen. Anyone wanting today's unbounded
behaviour sets a large budget explicitly, which says what it does.
Phase R — the glossary is checked, not just written down
The glossary has always claimed authority — "when user-facing copy and this page
disagree, the copy is wrong" — but nothing verified it, and the copy had drifted
back to "protection", "egress" and "daemon" in 40 places.
internal/vocabparses the banned-word table out of the glossary itself, sothere is one list and it is the one a human reads; editing a row changes what the
build enforces. A hardcoded list would agree with the page until someone edited
one of the two. Same trick
internal/config/docdrift_test.goalready uses onconfig.md.The Go side uses
go/parser, not grep, because the distinction that matters isunavailable to a text search: a literal reaching
fmt.Print*is copy, the sameliteral reaching
o.Log.Warnis the technical register where "daemon" and"egress" are the correct words. Multi-line
usageblocks are checked per line, soa violation names the sentence rather than the help page.
The table needed a register split, which is the substantive change to
Addendum 4's design. A single banned list cannot express "wrong on a button,
right in a log": unmarked rows apply everywhere,
‡to user-facing copy only,†are not linted because the ban needs judgement. Of the four terms Addendum 4named, "relaxation" and "guard is disarmed" are enforced; "peer"/"server" and
"utun"/"interface" are
†and cannot be otherwise — the recommendedreplacement is "VPN server address" and "physical interface" is correct
usage. "Disarmed" is phrased narrowly on purpose: disarm is the right verb for
hold the line, which really is an armed flag.
Copy fixed accordingly.
statusprintscontrol socket:instead ofdaemon control:;block/unblockdrop "(via daemon)"; refusals readdezhban refused:; the app's panic tooltip and block hint lose "daemon" and"egress".
status --jsonkeys are untouched — they are stable identifiers,and the lint does not look at them.
Every exemption carries a written reason (a flag's own name, an env var, a
shipped ADR's filename), so an exception is a recorded decision rather than a
silent dodge.
Verification
No enforcement change, confirmed rather than asserted.
print-rulesstdout,stderr and exit status are byte-identical to
0e49ec5across all five exampleconfigs × three modes — checked after Phase Q and again after Phase R. A
difference would have meant the budget or the copy leaked into the ruleset.
The lint was verified to fail on a reintroduced violation, not merely to pass
when clean.
Not run: the privileged, on-host checks. They need a reboot and a real
flapping VPN, so they are written into
docs/contribute/testing.mdas standingchecklist entries — two new sections, Unattended recovery and Redial budget
and backoff, 16 items covering the reboot cases, the backoff sequence,
exhaustion, refill, hold-the-line spending nothing, and the
"0"refusal.🤖 Generated with Claude Code