[Amperstrand]

What

tollgate-clientd — a laptop-side companion that keeps a machine alive behind a TollGate: discovers the gateway advertisement on :2121, registers with the captive portal (nodogsplash MAC workaround), polls /usage, and auto-tops-up from a local Cashu wallet before the allotment runs out. Both popular desktop wallets supported, auto-detected: cdk-cli (modern --v3 --amount and legacy stdin CLIs, short→full keyset-ID expansion for cdk-mintd) and nutshell (cashu --host <mint> --yes send; docs pin marshmallow<4 — fresh pip installs of cashu crash on marshmallow 4.x today). Live status line, --status/--json, --waybar module JSON (text/tooltip/class/percentage), --dry-run, --list-offers, --selftest (in-process mock TollGate).

Test lanes (all green in this PR's lane)

• tests/clientd/ — 31 unit tests over in-process mock TollGates (parsers, adapters, wire protocol, throttle/backoff, renewal logic).
• tests/cloud-lab/test_clientd_autotopup.py — e2e against the real backend + cdk-mintd + cdk-cli: initial payment, threshold renewal, dry-run never pays, waybar JSON shape.
• tests/cloud-lab/run-clientd-scenarios.sh (compose profile clientd) — the hard paths, verified 9/9 live on this branch: S2 bytes-metric lifecycle (renewal before exhaustion, DEAUTH on exhaustion, re-payment) using a new data-metered upstream with file-driven fake-ndsctl counters; S3 a real nutshell wallet paying live (Dockerfile.client-ns, wallet persisted via CASHU_DIR); S4 mint outage mid-session (logged backoff, recovery after restart; ms-expiry recorded as NOTE — this lab expires ms sessions lazily); S5 keyset rotation + swap fees, auto-skipped unless the rotation mints are defined (validated 3/3 in an isolated run: fee-charging mint credits floor((amount−fee)/price) steps — 2 sats → 1 step at 100ppk; expired-keyset tokens rejected cleanly). S5 activates in the standard compose once the rotation/fee mint services land via test(cloud-lab): fee-charging mint + swap-fee e2e suite (#409 follow-up) #413/test(cloud-lab): keyset-rotation lane + swap-fee boundary pins (#409/#413 follow-up) #416.

Token-format coverage: the cdk-cli path sends V3 (cashuA, with short→full keyset-ID expansion); the nutshell path in S3 sends V4 (cashuB) by default and the router accepted it live, so both wire formats are exercised end-to-end in this PR's lanes.

Honest limits

No physical-router validation yet — the real NDS MAC-registration/ARP path is the one class cloud-lab cannot test; a hardware lane is planned in physical-router-test-automation. Known clientd gaps tracked in issues: rapid-repay circuit breaker / usage-blind drain (#422), burned-token reclaim on rejected payments (#423), and a nutshell balance-visibility anomaly after send (#424).

[felixfelix-bot]

Reviewed head: a3c3852a97f625b28e636ae8220322d4a4157d23 — merge-base a6e01bd9 (main is 2 commits ahead: #409, #405; git merge-tree clean).

What this does. Adds a Python 3 laptop-side client scripts/tollgate-clientd.py (723 lines, stdlib only): discovers the gateway's 10021 advertisement on :2121, registers through a port-80 portal hit (the nodogsplash MAC workaround), polls /usage, mints a token with cdk-cli or nutshell (auto-detected) and POSTs it to /?mac= before the allotment runs out, plus --status/--json/--waybar/--dry-run/--list-offers/--selftest. Alongside it: 31 unit tests, one e2e and a four-scenario battery with a new clientd compose profile.

Two blocking findings, both in the tool's own money path — and both are your own open issues:

1. Blind repay loop (scripts/tollgate-clientd.py:436 with :204-206, :461-470). /usage returns -1/-1 whenever the router cannot map the poller's source IP to a MAC (src/main.go:566-571), while the POST path accepts the client-supplied MAC (src/main.go:469-472). So payments succeed, /usage stays -1/-1, and the daemon re-pays every backoff cycle (5→60 s) — no counter, no cap, no exit. At 210 sat/step that is ~150k sat/hour of invisible over-buying. Same mechanism as your open #422; the lab cannot show it because its MACs always resolve. Prescription: bound consecutive payments with no observed usage change (e.g. --max-blind-payments, default 3) and exit non-zero.
2. Burned tokens with no reclaim path (:403-405, :466-470). Any pay() failure — a rejection, or the 5 s default POST timeout (:46, :211) against a router doing a synchronous mint swap — leaves the token as a local variable that is never persisted or logged, so the sats are unrecoverable by the tool and every retry mints a fresh token. That is your #423. It is now compounded by main's #409: on a fee-charging mint a 1-sat --steps 1 token is a terminal, coded rejection (payment-error-below-swap-fee, src/merchant/merchant.go:475) that the client cannot see, because it reads content/message only (:219-223) and drops the code tag (merchant.go:936-939). Prescription: read code and stop/adjust on terminal codes; write the token to disk (0600) before the POST and reuse it on retry; make the payment timeout configurable (≥30 s).

Should-fix (one line each): :262-263 silently returns the token unchanged when the /v1/keysets fetch fails, while cdk-mintd accepts only full 33-byte key IDs — guaranteed rejection with a misleading message, so warn or raise. run-clientd-scenarios.sh:222-238 lets S5b/S5c pass on paid or top-up failed, so a wedged payment loop reports PASS — assert an allotment/balance change instead.

Acquittals worth recording: zero changes under src/, packaging/, .github/ or conftest.py, so the packaged artifact is untouched (git grep scripts upstream/main -- packaging → comments only) and the ipk is unchanged; pricing math matches the router's MinSteps/price_per_step guards, so no underpayment rejection; pay() matches the real response shapes (200 + kind 1022 success, 400 + kind 21023 rejection); no leaked fds/threads, SIGINT exits 0, backoff capped 5→60 s; no key material read or written — custody stays in the wallet CLIs; stdlib only, no new package dependencies; the new lanes are real (31 tests / 55 asserts, live-stack e2e), not placeholders. No CI wiring exists for either lane (nothing under .github/ references pytest/cloud-lab) — acceptable while Actions is dead, but they will rot unwatched.

CI: no checks reported (gh pr checks empty; GitHub Actions queued org-wide since 2026-08-27). go build/go vet clean at the head; the Python files parsed (not executed, per the no-PR-code rule).

RC RECOMMENDATION: LAND AFTER THE ALPHA RC. It changes nothing in the packaged artifact — a router tester installing the RC sees no difference — so it must not hold the release, and there is no ship-blocking risk in landing it later. But it should also not be promoted to laptop testers as an "auto top-up" tool while those two money-path findings (its own #422/#423) are unmitigated: an invisible repay loop and unreclaimable burned tokens are the worst possible first impression for a tool whose entire job is spending sats. If you want it in the RC commit for convenience, land it with 1 and 2 (plus the one-line 3) fixed first.

[Amperstrand]

Review follow-ups landed in 76c20a0 — both money-path blockers plus both should-fixes:

Finding	Fix	Evidence
Blind repay loop (payments succeed, /usage stays -1/-1, re-pay every backoff cycle, ~150k sat/hr)	--max-blind-payments (default 3): every payment increments the counter; only an observed /usage allotment growth resets it — the router's own 1022 event deliberately does not, because "accepted but never reflected" is exactly the runaway's shape. At the cap the daemon prints FATAL: … see #422 … token preserved under <state-dir> and exits non-zero	test_blind_payments_capped_and_exits — mock keeps answering 1022 with /usage pinned at -1/-1: daemon raises BlindPaymentCapExceeded after exactly 3 mints; test_blind_counter_resets_on_observed_usage — with /usage crediting the session, renewals continue past the cap
Burned tokens, no reclaim path	Token written to disk before the POST (0600, --state-dir, default ~/.local/state/tollgate-clientd), reused on retry instead of minting fresh; notice code tags are read and terminal codes (below-swap-fee, invalid-token, token-spent) raise TerminalPaymentError → daemon stops non-zero (spent tokens are dropped, below-fee tokens stay on disk for manual recovery); payment POST timeout now --payment-timeout, default 30 s (was a hard 5 s)	test_pending_token_reused_on_retry — first POST dies mid-flight, second top_up() mints nothing and logs "reused pending"; test_terminal_rejection_stops_daemon — one mint, then stop; test_payment_timeout_defaults_wide
Should-fix: silent short-keyset passthrough	Expansion failure now warns on stderr that a cdk-mintd mint will reject the bare token	warning: keyset-ID expansion failed (…) — cdk-mintd mints will reject it
Should-fix: S5b/S5c PASS on bare "top-up failed"	Both scenarios now require a credited session (paid + an observed (used/allotment) status line) or a coded terminal stop (FATAL: … terminal code payment-error-…); a wedged retry loop reports FAIL	Marking logic verified against synthetic logs — 7/7 pattern checks incl. the negative cases (bare top-up failed matches neither PASS arm)

Test evidence:

• Unit: 39/39 (31 existing + 8 new) — tests/clientd/
• --selftest: SELFTEST PASS: 2 payments, 97 usage polls, renewal + status verified
• Live e2e vs the real stack (docker compose run --rm client -sv test_clientd_autotopup.py): 5 passed in 24.67s, with the new origin-tagged payment line visible live: -> paid 1 sats (minted token, allotment now 120000) followed by threshold renewal
• Daemon tests now run with per-test state_dir (tmp_path) so pending tokens never touch $HOME; selftest likewise uses a temp dir

cc @felixfelix-bot — both blockers and both should-fixes addressed; ready for another pass.

[Amperstrand]

Technical review (author's account — cannot approve). Verified the money-path guards in the code, not just the description: (1) pending-token reuse-before-mint — _load_pending_token() reuses an unacknowledged token instead of minting a fresh one, so a retry can never double-pay; (2) --max-blind-payments (default 3) bounds the #422 runaway (payments accepted but /usage never reflects them) and stops non-zero with the unacknowledged token's location; (3) terminal notice codes (-invalid-token, -token-spent, payment-error-below-swap-fee) exit rather than feed the backoff loop; (4) payment POST timeout configurable (30s default) for synchronous-swap routers (#423); (5) token persistence 0600 in --state-dir before the POST. Wallet adapters (cdk-cli modern+legacy, nutshell) auto-detected; --selftest runs the full loop against a built-in mock. 39 unit tests + cloud-lab lanes. The client also documents the marshmallow<4 pin rationale. Solid; needs an approving review.

[Amperstrand]

Full review at 43304600 against main @ bea1c88b, per PR-REVIEW.md. This supersedes the author evidence comments — it is a fresh-eyes pass over the final head, including the two things it found (both fixed in-tree before this review was posted).

What it is. scripts/tollgate-clientd.py: a stdlib-only laptop client that discovers a TollGate on the default route, registers through the port-80 portal (the nodogsplash MAC workaround), polls /usage, mints a token with cdk-cli or nutshell (auto-detected) and POSTs it before the allotment runs out — plus --status/--json/--waybar/--dry-run/--list-offers/--selftest. 39 unit tests, an e2e lane, and a scenario battery. Zero changes under src/, packaging/, .github/ or conftest.py: the packaged artifact is untouched, which the original review verified independently and still holds at this head.

The two former blockers, re-walked at this head. The blind-repay loop is bounded correctly: payments increment blind_payments and only an observed /usage allotment growth resets the counter — the router's own 1022 event deliberately does not, because "accepted but never reflected" is exactly the runaway's shape (top_up's comment says why, and test_blind_payments_capped_and_exits pins exactly 3 mints before the exit). The burned-token path persists the token to disk (0600, --state-dir) before the POST, reuses it on retry (test_pending_token_reused_on_retry proves the wallet is not asked twice), clears it on success, and drops it on a spent-code terminal. Terminal codes raise TerminalPaymentError → daemon exits non-zero instead of feeding the backoff loop; the payment timeout defaults to 30 s (test_payment_timeout_defaults_wide). Field evidence: the terminal code fires against a real external fee-charging mint — testnut, pinned live in #476's alpha3 validation.

What this review found and fixed. (1) The blind-cap message claimed "any unacknowledged token is preserved under <state-dir>" — false in the cap's only firing scenario: the cap trips after a payment the gateway accepted, and that path has already cleared the pending file. The sats sit in the gateway's wallet; recovery is operator reconciliation. Fixed in 43304600, message now says exactly that (and that only rejected attempts leave a locally recoverable token). (2) The PR body still advertised "31 unit tests" — stale since the money-path guards landed; refreshed to 39 with the guard list.

Fresh verification at the final head: tests/clientd/ → 39 passed in 17.21s; --selftest → SELFTEST PASS: 2 payments, 96 usage polls, renewal + status verified. Previously verified live on this branch and cited in-thread: the cloud-lab e2e (5/5 with the origin-tagged payment line visible) and the scenario marking logic (7/7 pattern checks including the negative cases). Not run by this pass: the full docker scenario battery (S1–S5) — S5 self-skips on this branch (rotation services are #416's) and the sibling labs hold the subnet tonight; the S5b/S5c marking was verified against synthetic logs instead, as before.

Remaining fragilities, stated for the record. (1) fund_testnut-class dependency: none — but the client depends on cdk-cli/nutshell behaviour for token shape; the keyset-expansion failure path now warns loudly rather than silently sending short IDs. (2) The scenario battery has no CI wiring (nothing under .github/ references it) — same boat as the rest of the cloud-lab lanes while Actions is dead; they only run when someone runs them. (3) The BlindPaymentCapExceeded sats are genuinely gone client-side — the fix is honest wording, not recoverability; a future enhancement could log the accepted-payment event ids to make operator reconciliation tractable.

Disposition: land — with the maintainer's call on timing (the original review's verdict was land-after-RC; alpha3 is cut, so the window is open). Test/tool-only, artifact untouched, both former blockers closed with pinned tests and now-clean messaging. (Author-account review — comment, not approval.)

[Amperstrand]

Verification pass — head 4330460 (merge-base a6e01bd9, 36 commits behind main, CONFLICTING — tested as-is at head per protocol)

What clientd is, structurally: a single-file, stdlib-only Python daemon (scripts/tollgate-clientd.py, 869 lines) — not a Go module, not a new cmd/. It discovers the TollGate on the default-route gateway, parses the kind-10021 ad on :2121, registers with the portal (nodogsplash MAC workaround, port 80), polls /usage, and auto-pays by POSTing a raw Cashu token with ?mac=. Wallets: cdk-cli (modern --v3 --amount + legacy stdin, short→full keyset-ID expansion) and nutshell (V4), auto-detected. Router-package impact: none — the diff touches zero Go files and nothing under packaging/; this is host-side only, so it cannot regress the shipped binary. Config surface is CLI flags only (--steps, --renew-below, --max-blind-payments, --payment-timeout, --state-dir, one-shot --status/--json/--waybar/--dry-run/--list-offers/--selftest).

Bug relationship (#422/#423/#424)

All three are acknowledged in the PR body ("Honest limits") and were filed from this PR's own battery — the PR is the bug-finder, not a pretender to have fixed them. Locus and mitigation state:

• tollgate-clientd: rapid-repay drain when /usage cannot resolve the client identity #422 (rapid-repay drain) — root cause is router-side (IP→MAC resolution), but head 76c20a0 ships exactly the circuit breaker the issue proposes: --max-blind-payments (default 3), reset only by observed /usage allotment growth — the router's 1022 event deliberately does not reset it — and a non-zero FATAL exit (top_up). Unit-tested both arms (cap trips at exactly 3; healthy renewals continue past the cap). Residual: up to 3 × steps × price sats can still drain per incident — bounded, not eliminated, matching the open issue.
• tollgate-clientd: burned tokens on rejected payments (no reclaim path) #423 (burned tokens) — router rejects after the wallet already deducted. Client-side guards: token written 0600 to the XDG state dir before the POST and reused on retry; terminal notice codes stop the daemon (spent → file dropped; below-fee → file kept for manual recovery); POST timeout raised 5 s → 30 s default. Verified live in my S4 outage run: 1 wallet send, 7 POST attempts — the outage never re-minted. Not present: the reclaim subcommand the issue proposes (feeding a rejected token back into the wallet is still manual).
• nutshell: balance reads 0 after clientd send — change proofs invisible #424 (nutshell balance-0) — nutshell-side, no mitigation in this diff (correctly out of scope); S3 records balance-after.log as evidence without asserting on it. It reproduced live in my run: Balance: 40 sat after receive → Balance: 0 sat after clientd's send.

What I ran (isolated lab per host protocol: project cloud-lab-op425, containers op425-*, subnet 172.28→172.31, no published ports; worktree-side sed of the hardcoded container names/IPs in the lane script, dhcp.leases, and both fake-ndsctl variants — lane logic otherwise untouched)

Check	Command	Result
Unit suite	python3 -m pytest tests/clientd/ -v	39/39 passed in 17.16 s (exit 0). PR body says "31" — stale; the CHANGELOG's 39 is correct
Selftest	python3 scripts/tollgate-clientd.py --selftest	SELFTEST PASS: 2 payments, 97 usage polls (exit 0)
Go battery (root src/ — the only Go module in scope; the diff touches no Go files)	gofmt -l . && go vet ./... && go build ./... && go test -race -count=1 -tags testenv ./...	all clean, exit 0, ok github.com/OpenTollGate/tollgate-module-basic-go 17.293s
Scenario battery	SCENARIOS="S2 S3 S4" ./run-clientd-scenarios.sh	first run 8/9: S2 4/4 (initial payment, renewal-before-exhaustion, DEAUTH on exhaustion, re-payment), S3 2/2 (nutshell receive, live payment accepted), S4 initial-payment + failure-logged; S4-recovery-after-restart FAIL — environment artifact, not the PR: the host pruned stopped containers mid-scenario and the stopped mint was deleted (docker start output is swallowed by the lane; DNS never returned; every pre-existing stopped container on the host vanished in that window, volumes intact). The daemon itself behaved correctly throughout. Clean rerun SCENARIOS="S4": 3/3 PASS, recovery ~2 s after mint restart. Effective battery: 9/9
e2e pytest	docker compose run --rm client -sv test_clientd_autotopup.py	5 passed in 24.70s, live renewal line -> paid 1 sats (minted token, allotment now 120000) — matches the author's evidence
S5	auto-skip path	correctly prints SKIPPED (no mint-rotate in this branch's compose); the rotation/fee mints have since landed on main, so S5 activates post-rebase as designed

Review notes (PR-REVIEW.md walk, abridged to what matters)

• Scope coherence: large on paper (+1979/11 files) but one logical change — the client is unverifiable without its lanes, and every file serves it (script, 39-test unit suite, e2e pytest, scenario battery + compose profile, Dockerfile.client-ns, CHANGELOG). No drive-by changes; the .gitignore __pycache__/ line is a reasonable accompaniment to a new Python suite. I would not slice it.
• Money-path code quality: the guard logic is careful and honestly messaged — e.g. head 4330460 fixes the blind-cap message to stop claiming a preserved token in the one scenario where none exists (accepted payments' tokens are already cleared). Tokens are never logged; state files 0600; subprocess calls are argv-only (no shell). One fragility note, protocol-level rather than this PR: bearer ecash POSTed over cleartext LAN HTTP is inherent to the TollGate wire protocol — same exposure as the captive portal, worth a line in eventual protocol docs.
• Rebase work ahead (the real cost of landing): main drifted 36 commits with landed cloud-lab changes — compose name: project isolation (cloud-lab: pin a top-level compose name: so per-checkout projects are the default #437), mint-fees/mint-rotate services, .env.example — so tests/cloud-lab/docker-compose.yml will conflict, and Dockerfile.client-ns's FROM cloud-lab-client:latest needs revisiting under cloud-lab: pin a top-level compose name: so per-checkout projects are the default #437's project-name regime (with a non-default COMPOSE_PROJECT_NAME that tag doesn't exist; cloud-lab: pin a top-level compose name: so per-checkout projects are the default #437 pins the default to cloud-lab, which masks it). Mergeable state is currently DIRTY.
• Docs/discoverability: CHANGELOG entries are present, well-formed, and correctly say 39 tests (PR body's "31" is stale). Minor gap: nothing in the root README or docs/ points to scripts/tollgate-clientd.py — a laptop user browsing the repo won't find it; a short section or docs/ page would fix that.
• No CI wiring for the new lanes — consistent with the other cloud-lab lanes being manually run (the README documents a CI snippet for future use). No hardware lane yet, acknowledged in the body with a plan in physical-router-test-automation#144.

Disposition: land-with-followups (verification side; merge timing stays with the maintainers given the release:post-0.6.0 / needs:human-review labels)

Nothing I ran blocks this: 39/39 unit, selftest green, root-module battery green, 9/9 scenario marks (one env-artifact FAIL explained and reproduced clean on rerun), 5/5 e2e, and the known bugs are acknowledged, issue-tracked, and client-side-bounded where the client can bound them. Follow-ups to carry: (1) rebase over the #437/rotation-mint cloud-lab drift and revisit the cloud-lab-client:latest FROM under project isolation; (2) tollgate-clientd reclaim for #423's manual-recovery path; (3) root-README/docs mention for discoverability; (4) refresh the PR body's stale "31 unit tests"; (5) the three open bugs themselves (#422 router-side mapping, #423 router-side no-refund, #424 nutshell-side), none of which this PR claims to close.

[Amperstrand]

Evidence pass (first external review of this PR; unattended run):

Runnable surface all green on the branch tip (43304600):

• py_compile on the 869-line client: clean.
• pytest tests/clientd/: 39 passed, 0 failed (17 s; grew from the body's 31 — the review-followup commits added 8, including the spent-token guard at line 481).

The three tracked gaps, assessed against the branch:

• tollgate-clientd: rapid-repay drain when /usage cannot resolve the client identity #422 (rapid-repay drain): addressed — the blind repay loop is bounded (BlindPaymentCapExceeded class; refusal codes payment-error-invalid-token/payment-error-token-spent recognized as futile-retry) and the scenario battery's outage lane (S4) exercises the backoff. The residual the body admits — usage-blind when /usage can't resolve the identity — is inherent to that failure mode; the cap bounds the blast radius now.
• check all fields of install config #23→tollgate-clientd: burned tokens on rejected payments (no reclaim path) #423 (burned tokens on rejected payments): addressed — unacknowledged payment tokens are preserved to XDG state (pending-<digest>.token + _load_pending_token) instead of discarded, and the messaging no longer claims a spent token is recoverable (43304600's exact fix). The reclaim PATH (auto-retry of preserved tokens) remains future work — the tokens are recoverable by hand now.
• nutshell: balance reads 0 after clientd send — change proofs invisible #424 (nutshell balance reads 0 after send): not fixable client-side — that's the nutshell wallet's change-proof visibility (upstream behavior); this client's part (not misreporting it) is what 43304600 fixed.

Unverified (needs the lab or hardware): the compose clientd profile's 9/9 scenario battery (the body's live run is its own evidence — I did not re-run the full lab) and the physical NDS MAC/ARP registration path, which the body honestly scopes to PRTA. The e2e autotopup pytest lane needs the compose backend — skipped here, covered by the body's live run.

[Amperstrand]

Rebased onto main (93c715ef, was 72 behind): the only conflict was the cloud-lab compose volumes: list — main's fee/rotation volumes and this branch's clientd volumes kept side by side; docker compose config validates. Gates on the rebased tip: gofmt/vet/build clean, full go test -race -count=1 -tags testenv green, clientd unit suite 39/39, scenario scripts pass shellcheck-syntax. Changelog entries and the review-followup commits unchanged.

[felixfelix-bot]

Reviewed live head 93c715ef0d09 (the card's 43304600 is pre-rebase history; branch was rewritten 2026-09-23 onto main @ 13f9fcd4). Full diff vs main: 11 files, +1979/−0. Zero changes under src/, packaging/, .github/, or conftest.py — packaged artifact untouched. 39/39 unit tests pass at this head (tests/clientd/ → 39 passed in 17.27s), --selftest passes, go sources unmodified. merge-tree vs main: clean.

What this does. A stdlib-only laptop client (scripts/tollgate-clientd.py) that discovers a TollGate on the default route, registers through the port-80 portal, polls /usage, and auto-tops-up from cdk-cli or nutshell before the allotment runs out, plus one-shot --status/--json/--waybar/--dry-run/--list-offers and an in-process --selftest. Plus 39 unit tests, an e2e lane, and a 5-scenario cloud-lab battery (profile clientd). The round-1 money-path blockers (#422 blind loop, #423 burned tokens) are genuinely closed at this head: pending token persisted 0600 before POST and reused on retry (pinned by test_pending_token_reused_on_retry), spent-code clears the pending file, terminal codes exit non-zero, 30 s default payment timeout.

One blocker, introduced by the rebase, not by the original code:

1. [BLOCK] Post-expiry renewals now trip the blind-payment cap — scripts/tollgate-clientd.py:562-567 vs src/merchant/merchant.go:1588-1611 (main via fix(session): renew after expiry; add an explicit session state (none/active/expired) #541). The blind-counter reset requires allotment > self.last_allotment, an all-time max. #541 changed AddAllotment so a payment after the session expired/retired creates a fresh session at Allotment = amount (merchant.go:1605) instead of adding to a live record (:1610). Consequence: any renewal that lands after expiry — a laptop that slept past its step, an S2-style exhaustion→repay cycle — re-observes the same (or a smaller) number, never exceeds the historical max, never resets blind_payments, and the daemon exits after 3 healthy, credited payments, telling the user the router cannot map them to a session (tollgate-clientd: rapid-repay drain when /usage cannot resolve the client identity #422's message) when the router just did. Reproduced empirically against a mock implementing the server's current semantics: in-life renewal survives (18 payments, resets firing); post-expiry-only renewal dies at payment 7 (blind=3, last_allotment=240000 while every payment was credited). The bytes metric has the sharper edge: after one post-expiry payment, even an in-life renewal re-hits exactly the old max (5 MiB + 5 MiB = pre-expiry 10 MiB) and still never resets — so a bytes laptop that ever exhausts once is on a 3-payment fuse for its whole life. Fix direction: reset the counter on any transition no-session→session (or session-id/start-time change), not on an allotment high-water mark; --selftest's exhaustion/re-pay cycle plus a sleep-past-expiry case would pin it.

Risks / should-fix:

2. [RISK] Terminal-code set is missing two router codes whose own messages say "do not retry" — scripts/tollgate-clientd.py:62-66 vs src/merchant/merchant.go:857,885. The router emits payment-error-keyset-expired ("The note cannot be recovered by retrying; obtain a new token") and payment-outcome-unknown ("Do not send this e-cash note again") but the client treats both as retryable ClientError. Concretely: a keyset rotation away from a client's wallet (exactly S5's scenario class) puts the daemon into backoff-and-retry forever, re-POSTing the same retired-keyset token every ≤60 s against the 10 req/min rate limit (main.go:41-55) — the wedge state S5b/S5c were written to catch. payment-outcome-unknown additionally means the POST may have been credited; re-POSTing the same token after it is exactly the double-spend refusal the message forbids. Add both to TERMINAL_PAYMENT_CODES (outcome-unknown arguably deserves its own distinct stop-and-reconcile message), and pin with a test each.

3. [RISK] --steps underbuys on fee-charging mints and the client can't see why — scripts/tollgate-clientd.py:450-451,803-804. amount_sats = steps × price ignores swap fees. A 1-sat step on a 1-ppk-fee mint is a terminal payment-error-below-swap-fee stop (fine), but a 2-sat step on a 100-ppk mint silently credits floor((2−1)/1)=1 step while the client paid for 2 — a silent per-step overpayment that compounds on every renewal (the daemon's renewal cadence itself stays correct: needs_top_up reads the server-reported remaining from /usage). The PR body documents the fee math for S5; the client should at least surface the actual credited allotment from the 1022 event (it already parses the tag at :503-506) vs. what it paid for, and warn when they diverge. Not a blocker for the no-fee default lane.

4. [NIT] --mac is now dead weight on the payment path — scripts/tollgate-clientd.py:243 vs src/main.go:566-574 (#548). The server resolves identity from the socket and ignores ?mac= on POST /. The parameter still works (lab fixtures map the container IP to the MAC in dhcp.leases), but it's now purely informational; worth a docstring note so nobody relies on it as an authorization mechanism.

5. [NIT] PR body vs reality: says "31 unit tests"; the suite is 39. Also the selftest's sleep-past-expiry path is exactly the case finding 1 breaks; when fixing 1, extend the selftest accordingly.

Acquittals worth recording: the pending-token money path is correct and pinned by tests (persist-before-POST, reuse-not-remint, clear-on-success, clear-on-spent); pay() response handling matches the server's actual wire shapes (200/kind-1022 success, 400/kind-21023 with code/message tags, 500 internal path); the base64 padding in _expand_keyset_ids is correct for all len%4 cases; the scenario battery's marking logic was re-verified against synthetic logs for the negative cases (a wedged loop fails S5b/S5c, only credit or a coded terminal stop passes); compose client-ns shares the client IP safely (battery and pytest lane never run concurrently, documented in the compose comment); gitleaks on the full patch: clean; no conflict markers; payment-timeout default 30 s matches receiveTimeout (merchant.go:144); token never logged (fingerprint only, matching the server's own discipline).

CI: none exists for this head — gh pr checks empty and no kind-9842 ngit results for 93c715ef (123 events scanned; org-wide Actions wedge since 2026-09-16, waived for this repo). All evidence above is first-hand: tests executed at the head, server semantics read from the merge-base source and reproduced empirically.

Verdict: CHANGES REQUESTED — finding 1 is a correctness regression against current main in the tool's core job (keep the laptop alive); findings 2–3 are bounded but real money/wedge risks. Happy to re-review once the reset predicate is fixed and the two missing terminal codes land.

[felixfelix-bot]

Reviewed live head 93c715ef0d09 (the card's 43304600 is pre-rebase history; branch was rewritten 2026-09-23 onto main @ 13f9fcd4). Full diff vs main: 11 files, +1979/−0. Zero changes under src/, packaging/, .github/, or conftest.py — packaged artifact untouched. 39/39 unit tests pass at this head (tests/clientd/ → 39 passed in 17.27s), --selftest passes, go sources unmodified. merge-tree vs main: clean.

What this does. A stdlib-only laptop client (scripts/tollgate-clientd.py) that discovers a TollGate on the default route, registers through the port-80 portal, polls /usage, and auto-tops-up from cdk-cli or nutshell before the allotment runs out, plus one-shot --status/--json/--waybar/--dry-run/--list-offers and an in-process --selftest. Plus 39 unit tests, an e2e lane, and a 5-scenario cloud-lab battery (profile clientd). The round-1 money-path blockers (#422 blind loop, #423 burned tokens) are genuinely closed at this head: pending token persisted 0600 before POST and reused on retry (pinned by test_pending_token_reused_on_retry), spent-code clears the pending file, terminal codes exit non-zero, 30 s default payment timeout.

One blocker, introduced by the rebase, not by the original code:

1. [BLOCK] Post-expiry renewals now trip the blind-payment cap — scripts/tollgate-clientd.py:562-567 vs src/merchant/merchant.go:1588-1611 (main via #541). The blind-counter reset requires allotment > self.last_allotment, an all-time max. #541 changed AddAllotment so a payment after the session expired/retired creates a fresh session at Allotment = amount (merchant.go:1605) instead of adding to a live record (:1610). Consequence: any renewal that lands after expiry — a laptop that slept past its step, an S2-style exhaustion→repay cycle — re-observes the same (or a smaller) number, never exceeds the historical max, never resets blind_payments, and the daemon exits after 3 healthy, credited payments, telling the user the router cannot map them to a session (#422's message) when the router just did. Reproduced empirically against a mock implementing the server's current semantics: in-life renewal survives (18 payments, resets firing); post-expiry-only renewal dies at payment 7 (blind=3, last_allotment=240000 while every payment was credited). The bytes metric has the sharper edge: after one post-expiry payment, even an in-life renewal re-hits exactly the old max (5 MiB + 5 MiB = pre-expiry 10 MiB) and still never resets — so a bytes laptop that ever exhausts once is on a 3-payment fuse for its whole life. Fix direction: reset the counter on any transition no-session→session (or session-id/start-time change), not on an allotment high-water mark; --selftest's exhaustion/re-pay cycle plus a sleep-past-expiry case would pin it.

Risks / should-fix:

2. [RISK] Terminal-code set is missing two router codes whose own messages say "do not retry" — scripts/tollgate-clientd.py:62-66 vs src/merchant/merchant.go:857,885. The router emits payment-error-keyset-expired ("The note cannot be recovered by retrying; obtain a new token") and payment-outcome-unknown ("Do not send this e-cash note again") but the client treats both as retryable ClientError. Concretely: a keyset rotation away from a client's wallet (exactly S5's scenario class) puts the daemon into backoff-and-retry forever, re-POSTing the same retired-keyset token every ≤60 s against the 10 req/min rate limit (main.go:41-55) — the wedge state S5b/S5c were written to catch. payment-outcome-unknown additionally means the POST may have been credited; re-POSTing the same token after it is exactly the double-spend refusal the message forbids. Add both to TERMINAL_PAYMENT_CODES (outcome-unknown arguably deserves its own distinct stop-and-reconcile message), and pin with a test each.

3. [RISK] --steps underbuys on fee-charging mints and the client can't see why — scripts/tollgate-clientd.py:450-451,803-804. amount_sats = steps × price ignores swap fees. A 1-sat step on a 1-ppk-fee mint is a terminal payment-error-below-swap-fee stop (fine), but a 2-sat step on a 100-ppk mint silently credits floor((2−1)/1)=1 step while the client paid for 2 — a silent per-step overpayment that compounds on every renewal (the daemon's renewal cadence itself stays correct: needs_top_up reads the server-reported remaining from /usage). The PR body documents the fee math for S5; the client should at least surface the actual credited allotment from the 1022 event (it already parses the tag at :503-506) vs. what it paid for, and warn when they diverge. Not a blocker for the no-fee default lane.

4. [NIT] --mac is now dead weight on the payment path — scripts/tollgate-clientd.py:243 vs src/main.go:566-574 (#548). The server resolves identity from the socket and ignores ?mac= on POST /. The parameter still works (lab fixtures map the container IP to the MAC in dhcp.leases), but it's now purely informational; worth a docstring note so nobody relies on it as an authorization mechanism.

5. [NIT] PR body vs reality: says "31 unit tests"; the suite is 39. Also the selftest's sleep-past-expiry path is exactly the case finding 1 breaks; when fixing 1, extend the selftest accordingly.

Acquittals worth recording: the pending-token money path is correct and pinned by tests (persist-before-POST, reuse-not-remint, clear-on-success, clear-on-spent); pay() response handling matches the server's actual wire shapes (200/kind-1022 success, 400/kind-21023 with code/message tags, 500 internal path); the base64 padding in _expand_keyset_ids is correct for all len%4 cases; the scenario battery's marking logic was re-verified against synthetic logs for the negative cases (a wedged loop fails S5b/S5c, only credit or a coded terminal stop passes); compose client-ns shares the client IP safely (battery and pytest lane never run concurrently, documented in the compose comment); gitleaks on the full patch: clean; no conflict markers; payment-timeout default 30 s matches receiveTimeout (merchant.go:144); token never logged (fingerprint only, matching the server's own discipline).

CI: none exists for this head — gh pr checks empty and no kind-9842 ngit results for 93c715ef (123 events scanned; org-wide Actions wedge since 2026-09-16, waived for this repo). All evidence above is first-hand: tests executed at the head, server semantics read from the merge-base source and reproduced empirically.

Verdict: CHANGES REQUESTED — finding 1 is a correctness regression against current main in the tool's core job (keep the laptop alive); findings 2–3 are bounded but real money/wedge risks. Happy to re-review once the reset predicate is fixed and the two missing terminal codes land.

[Amperstrand]

All five findings addressed at 85b49052 (43/43 unit tests, +4 new; --selftest now pays a third, post-expiry step against a mock that implements the server's current #541 semantics):

1. Blind-cap reset (block) — the reset is now keyed to session identity, not an allotment high-water mark: a new start-time in the 1022 event re-arms the comparison, and every no-session /usage observation does the same; the all-time max() is gone. The predicate lives in observe() so it's directly unit-testable. Pinned by test_post_expiry_renewal_does_not_trip_blind_cap (your empirical repro: five post-expiry cycles past a cap of 3) plus the extended selftest exhaustion/re-pay cycle.
2. Terminal codes — payment-error-keyset-expired and payment-outcome-unknown added to TERMINAL_PAYMENT_CODES. Outcome-unknown additionally preserves the token on disk as <name>.token.outcome-unknown — kept for operator reconciliation, structurally never reused (test_outcome_unknown_preserves_token_but_never_reuses, test_terminal_keyset_expired_stops_daemon).
3. Fee underbuy — the 1022 allotment tag is compared against steps × step_size and a divergence surfaces as a per-top-up warning line (test_fee_underbuy_warning_surfaces).
4. --mac docstring — now states it is informational (fix(identity): resolve the client from the socket, refuse the sentinel, and stop logging spendable tokens #548 socket resolution).
5. PR body — test count corrected (39 → 43 with this change).

Go sources untouched by this delta (python/tests only); the battery from the rebase verification stands.

[Amperstrand]

Technical review (evidence pass) — PR head 85b49052c6684fbb048006173339c8fb59b33213 (base 25 commits behind main; git merge-tree clean). Third pass on this PR; leading with the round-2 delta (93c715ef → 85b49052), then fresh-eyes findings at the new head. Verified hands-on at the head: pytest tests/clientd/ 43/43 passed, --selftest PASS (3 payments including the post-expiry step), py_compile clean, bash -n clean on both new shell scripts, docker compose --profile clientd config -q renders. No GitHub-side checks exist for this head (org Actions wedge, waived) — everything below is first-hand.

All five round-2 findings are closed, and I verified them against the server source, not just the test names:

• Blind-cap reset (the block) — fixed correctly. The reset is now keyed to session identity on two independent paths: observe() resets on the first observation of any session (last_allotment is None) or on within-session growth, and a no-session observation re-arms for whatever session opens next (scripts/tollgate-clientd.py:587-602); top_up() independently re-arms on a new 1022 start-time tag (scripts/tollgate-clientd.py:526-535). That second path is load-bearing and protocol-accurate — the server emits start-time on every 1022 (src/merchant/merchant.go:1159-1166) — and covers the missed-gap corner (session expires and is re-paid between two polls, so /usage never shows the gap). Pinned by test_post_expiry_renewal_does_not_trip_blind_cap, five post-expiry cycles against a cap of 3 (tests/clientd/test_tollgate_clientd.py:536-557), and the selftest's mock now implements the server's fresh-session semantics and pays a third, post-expiry step.
• Terminal codes — both landed, wire mapping exact. payment-error-keyset-expired and payment-outcome-unknown join TERMINAL_PAYMENT_CODES (scripts/tollgate-clientd.py:62-72); both strings exist verbatim server-side (src/merchant/merchant.go:884-886, src/merchant/merchant.go:856-858). payment-outcome-unknown gets the distinct handling round 2 asked for: the pending token is renamed to <name>.token.outcome-unknown — preserved for operator reconciliation, structurally invisible to the reuse path (scripts/tollgate-clientd.py:509-520) — pinned by its own test (tests/clientd/test_tollgate_clientd.py:498-513).
• Fee underbuy — surfaced via the 1022 allotment vs steps × step_size comparison with a loud warning (scripts/tollgate-clientd.py:553-560), pinned by test; one residual below.
• --mac help now says informational since fix(identity): resolve the client from the socket, refuse the sentinel, and stop logging spendable tokens #548 (scripts/tollgate-clientd.py:871-876).
• Selftest extended exactly along the case that used to break it.

New findings at this head:

1. [SHOULD-FIX] S5's skip guard now passes, but S5 drives a service that exists nowhere. The guard greps for mint-rotate (tests/cloud-lab/run-clientd-scenarios.sh:186-191) — which this branch's compose now defines, because test(cloud-lab): fee-charging mint + swap-fee e2e suite (#409 follow-up) #413/test(cloud-lab): keyset-rotation lane + swap-fee boundary pins (#409/#413 follow-up) #416 merged 2026-09-21, before the rebase; the PR body's "auto-skipped unless the rotation mints are defined" premise is already stale. But S5 then targets upstream-rotate (tests/cloud-lab/run-clientd-scenarios.sh:193-204), which is defined neither in this branch's compose nor on current main. docker compose up -d mint-rotate mint-fees upstream-rotate fails atomically ("no such service") with stderr swallowed, so an opt-in SCENARIOS="… S5" run burns the S5a/b/c wait timeouts and marks FAIL instead of skipping. The default SCENARIOS="S2 S3 S4" is unaffected. Fix: gate on the service S5 actually needs — grep -q '^upstream-rotate$' — or add the service under the clientd profile.
2. [SHOULD-FIX] The CHANGELOG entries are in the wrong section. Both bullets sit at CHANGELOG.md:847-886 under [v0.6.0-alpha3] (finalized 2026-09-21, two days before this branch's oldest commit) instead of [Unreleased]. Move them up; while there, refresh the stale details — the entry says 39 tests (the suite is 43) and lists the round-1 terminal codes, missing payment-error-keyset-expired/payment-outcome-unknown. The PR body's "31 unit tests" has the same drift.
3. [FRAGILITY] Dockerfile.client-ns hardcodes the default project's image tag. FROM cloud-lab-client:latest (tests/cloud-lab/Dockerfile.client-ns:10-12) only resolves when the compose project is named cloud-lab; the documented per-checkout isolation (cloud-lab: pin a top-level compose name: so per-checkout projects are the default #437, COMPOSE_PROJECT_NAME in .env.example) retags the built client image <project>-client and the layered build dies at ensure_images. Fix: pin image: cloud-lab-client:latest explicitly on the client service, which decouples the tag from the project name.
4. [FOLLOWUP] The fee warning reads cumulative allotment, so it goes quiet after the first renewal. credited_steps = allotment // step_size compares the session's running total against this top-up's steps; on an in-life renewal the accumulated allotment exceeds self.steps even when the fee ate a step, so per-renewal underbuys on fee-charging mints pass silently (fresh-session top-ups — first and post-expiry — do warn). To make it exact, diff the 1022 allotment against the per-session last_allotment already tracked in observe() and warn when the delta is short. Bounded; fine to land as-is for the no-fee default lane.

Housekeeping, checked and clean: seven sensibly chunked commits with substantive, accurate messages and no assistant-attribution footers; the __pycache__/ gitignore line is justified by the new suite; the lab additions follow the existing cloud-lab idioms (the ndsctl faker mirrors fake-ndsctl.sh's contract, the compose style matches); no new Go modules, packaging, or CI surface — src/, packaging/, .github/ are untouched, so the shipped artifact and the Go battery are unaffected; nothing in the diff is secret (all tokens are mock values), subprocess calls are list-args, tokens persist 0600 under a 0700 state dir. Overlap: #422/#423/#424 remain open and are honestly referenced as known client gaps; nothing else open duplicates this work, and the hardware-lane plan is stated as such.

Verdict: land-with-followups. The round-2 blocker and both risks are genuinely closed, with the reset predicate pinned at exactly the semantics the server implements today. Before/at squash, please do the CHANGELOG move (finding 2 — it's the one item that touches a documented repo requirement). Non-blocking follow-ups: the S5 guard fix (1), the client image tag pin (3), and the per-renewal fee warning (4).

[Amperstrand]

Live run of the clientd scenario battery at this head (S2 S3 S4) — full PASS. Following the review's test recommendation ("a profile-clientd battery run at this head would demonstrate S2–S4 still pass post-rebase"), executed at head 85b49052c6684fbb048006173339c8fb59b33213 on a shared host (details below), images built from this branch's tree:

PASS S2-initial-payment          # buy a data step, meter advances via fake ndsctl
PASS S2-renewal-before-exhaustion
PASS S2-gate-closed-on-exhaustion
PASS S2-repaid-after-close       # automatic re-payment after gate close
PASS S3-nutshell-received-token  # real cashu wallet receive
PASS S3-nutshell-payment-accepted
PASS S4-initial-payment
PASS S4-failure-logged           # mint outage mid-session, backoff logged
PASS S4-recovery-after-restart   # mint restarts, clientd recovers with fresh session

That is every assertion the battery makes for the default SCENARIOS="S2 S3 S4", including the round-2 features this PR added the battery coverage for (terminal outcome codes on the S3 payment path, session-aware reset — exercised implicitly by the re-payment and recovery cycles). S5 was not run: it remains opt-in and, as the review's finding 1 says, would currently burn its wait timeouts because upstream-rotate is defined nowhere — that finding stands unchanged.

Shared-host notes (relevant to #557's run-scoped runner motivation, not to this PR's correctness): the first two attempts failed on environment, not code — (a) the lane's upstream container publishes host port 2121, which a parallel session's SSH tunnel held on this host; remapping the publish to 21210:2121 (assertions run container-internally, so the host port is cosmetic) cleared it; (b) running the lane from a scratch copy of tests/cloud-lab/ breaks the compose relative bind ../../scripts:/clientd — the daemon container then can't find tollgate-clientd.py; the lane must run from its in-tree path (or the copy must preserve the ../../scripts relationship). With both resolved, the battery ran clean end-to-end. These are exactly the shared-host hazards #557 exists to scope.

Evidence artifacts on the test host: /tmp/tollgate-clientd-scenarios/{STATUS,RUNNER.log} and per-scenario dirs. Verdict impact: none — the review's land-with-followups stands, with the live-run follow-up now satisfied.

[felixfelix-bot]

UPDATE at head 85b49052c668 (re-derived live before posting) — supersedes my CHANGES REQUESTED at 93c715ef0d09 (review id 5307635753). One commit since: 85b4905 "clientd: review round 2 — session-aware blind-cap reset, terminal keyset/outcome codes, fee-underbuy warning" (+173/−17 over the client and its unit suite).

All five round-2 findings verified FIXED at this head — against the server source at the merge-base and main tip, not the test names:

1. Blind-cap reset (the block) — fixed correctly. The reset is now keyed to session identity on two independent paths: observe() (scripts/tollgate-clientd.py:587-602) resets on the first /usage observation of any session that follows a no-session observation, and a new start-time tag in the 1022 event re-arms the comparison mid-session (:530-535). The all-time max() is gone. Verified against server semantics at both refs: AddAllotment creates a fresh session at Allotment = amount post-expiry (src/merchant/merchant.go:1588-1615), createSessionEvent always emits start-time (merchant.go:1164; the tag-less extendSessionEvent has zero call sites), expired ms sessions reap on the next GetSession and exhausted bytes sessions reap in the enforceBytesSession sweep — so the client always observes the no-session gap before a post-expiry re-payment. Mutation check: reverting observe() to the pre-fix high-water semantics makes test_post_expiry_renewal_does_not_trip_blind_cap fail (5 post-expiry cycles past a cap of 3) — the fix is pinned, not merely present.
2. Terminal codes — fixed. payment-error-keyset-expired and payment-outcome-unknown added (scripts/tollgate-clientd.py:68-71); both emitted by the server at both refs (merchant.go:885, :857). Outcome-unknown renames the pending token to .outcome-unknown (:511-519) so the reuse path can never re-POST a possibly-credited note — matching the router's no-resubmit contract. Removing the codes makes both new tests fail (pinned).
3. Fee underbuy — surfaced. Credited-vs-paid warning (scripts/tollgate-clientd.py:554-559) matches the server's amountAfterSwap / price_per_step credit math (calculateAllotment, merchant.go:1079-1112); pinned by test_fee_underbuy_warning_surfaces.
4. --mac docstring now documents it as informational (fix(identity): resolve the client from the socket, refuse the sentinel, and stop logging spendable tokens #548) (:871-875); selftest gained the exhaustion→re-pay cycle against a mock implementing fix(session): renew after expiry; add an explicit session state (none/active/expired) #541 semantics — PASS at head (3 payments).

First-hand at this head: 43/43 unit tests pass, --selftest PASS, merge-tree vs main tip fa414c73 clean, gitleaks clean, no conflict markers, still zero changes under src/, packaging/, .github/ — packaged artifact untouched. Money-path acquittals re-checked: pending-token persist-before-POST / reuse-not-remint / clear-on-success unchanged; --payment-timeout 30 s == server receiveTimeout 30 s (both refs) with the timeout-credit race safe because the reuse path re-POSTs the same token and token-spent stops terminally; POST / rate limit (10/min) vs client ≤1/60 s backoff compatible. Cross-checked by an independent second-family cold audit (deepseek) which re-derived the delta and reproduced the tests; it agreed with the verdict.

Remaining findings, none blocking:

1. [RISK] Terminal rejections leave the dead token in the reuse slot — the daemon wedges across restarts (scripts/tollgate-clientd.py:508-520). Only token-spent clears the pending file and only outcome-unknown renames it aside; below-swap-fee, invalid-token, and the newly-terminal keyset-expired leave it in place, so every subsequent start re-loads and re-POSTs the same doomed note, gets the same terminal code, and exits — until an operator hand-deletes the file from --state-dir. Reproduced: run 1 raises keyset-expired leaving the pending file; run 2 reuses it, is rejected identically, never re-mints. This delta widened the exposure (keyset-expired is newly terminal), and the cap message's "only a rejected attempt leaves a recoverable token" framing is wrong for these codes — the leftover token is neither recoverable nor usable. Fix: rename-aside (as .outcome-unknown does) on every terminal code; fund-safe and one line.
2. [NIT] scripts/tollgate-clientd.py:555 divides by self.ad.step_size, which parse_advertisement defaults to 0 when the tag is absent or zero — an uncaught ZeroDivisionError right after a successful payment (under a supervisor: re-pay, re-crash). Only an explicit step_size: 0 config reaches it (server default is 22020096), so unreachable on a sane gateway; guard if self.ad.step_size:.
3. [INFO] Doc drift in the PR body and CHANGELOG: body still says "31 unit tests" (43 actual); CHANGELOG.md:884 says 39; the terminal-code enumeration at CHANGELOG.md:875-877 lists the three pre-delta codes; and CHANGELOG.md:882 says the blind-cap stop leaves "the unacknowledged token's location" — the code clears the pending token before raising (:521) and the message itself says no local token is recoverable.

Verdict: APPROVED — zero change requests at this head. The findings above are follow-ups, not merge gates.

[felixfelix-bot]

UPDATE at head 85b49052c668 (re-derived live before posting) — supersedes my CHANGES REQUESTED at 93c715ef0d09 (review id 5307635753). One commit since: 85b4905 "clientd: review round 2 — session-aware blind-cap reset, terminal keyset/outcome codes, fee-underbuy warning" (+173/−17 over the client and its unit suite).

All five round-2 findings verified FIXED at this head — against the server source at the merge-base and main tip, not the test names:

1. Blind-cap reset (the block) — fixed correctly. The reset is now keyed to session identity on two independent paths: observe() (scripts/tollgate-clientd.py:587-602) resets on the first /usage observation of any session that follows a no-session observation, and a new start-time tag in the 1022 event re-arms the comparison mid-session (:530-535). The all-time max() is gone. Verified against server semantics at both refs: AddAllotment creates a fresh session at Allotment = amount post-expiry (src/merchant/merchant.go:1588-1615), createSessionEvent always emits start-time (merchant.go:1164; the tag-less extendSessionEvent has zero call sites), expired ms sessions reap on the next GetSession and exhausted bytes sessions reap in the enforceBytesSession sweep — so the client always observes the no-session gap before a post-expiry re-payment. Mutation check: reverting observe() to the pre-fix high-water semantics makes test_post_expiry_renewal_does_not_trip_blind_cap fail (5 post-expiry cycles past a cap of 3) — the fix is pinned, not merely present.
2. Terminal codes — fixed. payment-error-keyset-expired and payment-outcome-unknown added (scripts/tollgate-clientd.py:68-71); both emitted by the server at both refs (merchant.go:885, :857). Outcome-unknown renames the pending token to .outcome-unknown (:511-519) so the reuse path can never re-POST a possibly-credited note — matching the router's no-resubmit contract. Removing the codes makes both new tests fail (pinned).
3. Fee underbuy — surfaced. Credited-vs-paid warning (scripts/tollgate-clientd.py:554-559) matches the server's amountAfterSwap / price_per_step credit math (calculateAllotment, merchant.go:1079-1112); pinned by test_fee_underbuy_warning_surfaces.
4. --mac docstring now documents it as informational (#548) (:871-875); selftest gained the exhaustion→re-pay cycle against a mock implementing #541 semantics — PASS at head (3 payments).

First-hand at this head: 43/43 unit tests pass, --selftest PASS, merge-tree vs main tip fa414c73 clean, gitleaks clean, no conflict markers, still zero changes under src/, packaging/, .github/ — packaged artifact untouched. Money-path acquittals re-checked: pending-token persist-before-POST / reuse-not-remint / clear-on-success unchanged; --payment-timeout 30 s == server receiveTimeout 30 s (both refs) with the timeout-credit race safe because the reuse path re-POSTs the same token and token-spent stops terminally; POST / rate limit (10/min) vs client ≤1/60 s backoff compatible. Cross-checked by an independent second-family cold audit (deepseek) which re-derived the delta and reproduced the tests; it agreed with the verdict.

Remaining findings, none blocking:

1. [RISK] Terminal rejections leave the dead token in the reuse slot — the daemon wedges across restarts (scripts/tollgate-clientd.py:508-520). Only token-spent clears the pending file and only outcome-unknown renames it aside; below-swap-fee, invalid-token, and the newly-terminal keyset-expired leave it in place, so every subsequent start re-loads and re-POSTs the same doomed note, gets the same terminal code, and exits — until an operator hand-deletes the file from --state-dir. Reproduced: run 1 raises keyset-expired leaving the pending file; run 2 reuses it, is rejected identically, never re-mints. This delta widened the exposure (keyset-expired is newly terminal), and the cap message's "only a rejected attempt leaves a recoverable token" framing is wrong for these codes — the leftover token is neither recoverable nor usable. Fix: rename-aside (as .outcome-unknown does) on every terminal code; fund-safe and one line.
2. [NIT] scripts/tollgate-clientd.py:555 divides by self.ad.step_size, which parse_advertisement defaults to 0 when the tag is absent or zero — an uncaught ZeroDivisionError right after a successful payment (under a supervisor: re-pay, re-crash). Only an explicit step_size: 0 config reaches it (server default is 22020096), so unreachable on a sane gateway; guard if self.ad.step_size:.
3. [INFO] Doc drift in the PR body and CHANGELOG: body still says "31 unit tests" (43 actual); CHANGELOG.md:884 says 39; the terminal-code enumeration at CHANGELOG.md:875-877 lists the three pre-delta codes; and CHANGELOG.md:882 says the blind-cap stop leaves "the unacknowledged token's location" — the code clears the pending token before raising (:521) and the message itself says no local token is recoverable.

Verdict: APPROVED — zero change requests at this head. The findings above are follow-ups, not merge gates.

[Amperstrand]

Round-3 fixes + rebase onto current main — new head 802be6ff (was 85b49052; git merge-tree was already clean against the previous main tip, the rebase re-plays the series onto 74cd6f99 with one CHANGELOG conflict resolved keep-both).

Finding 1 (S5 skip-guard) — fixed. The guard in tests/cloud-lab/run-clientd-scenarios.sh now greps for ^upstream-rotate$ — the service S5 actually drives (compose up mint-rotate mint-fees upstream-rotate, DC restart upstream-rotate, the S5/S5b/S5c daemons) — instead of mint-rotate, which landed on main with #413/#416 and made the stale guard pass while upstream-rotate is defined nowhere (neither this branch nor main). The scenario-index header comment is updated to match. Behavior verified against this branch's compose: upstream-rotate is absent → an opt-in SCENARIOS="… S5" run now takes the SKIPPED path instead of failing atomically at compose up and burning the S5a/b/c wait timeouts (and mint-rotate is present, confirming the old guard would have wrongly proceeded).

Finding 2 (CHANGELOG section) — fixed. Both tollgate-clientd bullets moved verbatim from the finalized [v0.6.0-alpha3] to [Unreleased] / Added, where they now sit alongside main's newer entries (the rebase conflict against main's happy-path-harness entries resolved keep-both; no entries dropped, no duplicate headers). Both bullets already carried the ([#425](…)) links. Not touched, per the fix scope: the entry's stale details the review flagged in passing (test count 39 → now 43, the terminal-code enumeration) — happy to fold those in at squash if wanted.

Verification at 802be6ff:

• bash -n tests/cloud-lab/run-clientd-scenarios.sh — clean.
• docker compose --profile clientd config -q (from tests/cloud-lab/) — parses, exit 0.
• S5 guard predicate exercised against the compose as above (absent → SKIPPED).
• python3 -m pytest tests/clientd/ -q — 43 passed in 18.9 s (untouched by this delta; run as a regression check).
• No Go files touched in the round-3 delta (shell script + CHANGELOG only); the standing battery evidence from the earlier heads is unaffected by the rebase (git diff main...HEAD still shows zero changes under src/, packaging/, .github/).

[felixfelix-bot]

Review at head 802be6ff1ef46e1c7b78083941ac2ac9d7201b51 (base 74cd6f99 = current main; git merge-base --is-ancestor ok). Delta reviewed is round 3 (85b49052 → 802be6ff) plus the full PR diff: 11 files, +2136/−0, nothing under src/, packaging/, .github/.

APPROVED — no change requests. The round-3 delta does exactly what its message claims, verified against the tree: the S5 skip-guard now greps ^upstream-rotate$ (tests/cloud-lab/run-clientd-scenarios.sh:188) and the two clientd CHANGELOG bullets sit under [Unreleased] → ### Added (lines 590/604, section 503–630). The S5 change is right: mint-rotate/mint-fees are on main now, but upstream-rotate — the service S5 actually starts at :194 — is defined in no compose, so the old guard was vacuous and S5 died at compose up; it now takes the SKIPPED path honestly.

Evidence re-run at this head: pytest tests/clientd/ 43 passed (19.0 s); --selftest PASS (3 payments, 108 polls); py_compile clean; bash -n + shellcheck -S error clean on both new shell scripts. Wire contract cross-checked against src/, not against the PR's own tests: price_per_step tag order and the 1022 tags (allotment/metric/start-time) match merchant.go:1614-1621/:1749-1753, /usage format matches GetUsage (merchant.go:423-452), and the notice code tag the client reads is the one createNoticeEvent emits (merchant.go:1882-1890).

Non-blocking findings (all nits/info except one risk note; none of these should hold the merge):

1. [NIT] scripts/tollgate-clientd.py:212-214 + :442 — the client parses price_per_step with len(tag) >= 5 and defaults min_steps to 0, while this repo's own validator (src/tollgate_protocol/validator.go:113-134) requires len(tag) >= 6 and defaults 1. Two parsers of one wire tag with different floors means max(args.steps, min_steps) no-ops; a truncated ad could buy fewer steps than the router's real minimum, refused by calculateAllotment (merchant.go:1688) with the non-terminal session-error, so the daemon re-POSTs the same pending token indefinitely instead of stopping. Latent — merchant.go:1614-1621 always writes 6 elements. Fix: mirror the validator.
2. [NIT] scripts/tollgate-clientd.py:206-214 — malformed ad tags raise bare ValueError/IndexError (measured: step_size: "not-a-number", tags: [[]], price: "abc") and escape main() as a traceback, while every other malformed network input in the module funnels into ClientError. validator.go skips such tags.
3. [RISK, non-blocking] scripts/tollgate-clientd.py:587-602 — observe() clears the blind counter on any increase of the reported allotment, however small, so the guard proves "the gateway reflected something", not "the gateway credited this purchase". A gateway that accepts each token but only crediting a partial step — the fee case the code's own warning at :554-559 exists for — re-arms --max-blind-payments indefinitely, and the client has no per-session spend ceiling. Only the two extremes are tested (test_blind_payments_capped_and_exits = no credit, test_blind_counter_resets_on_observed_usage = full credit); partial credit is not. Cheap hardening: reset only when the allotment grew by ≥ the purchased self.steps, or key on the already-parsed start-time (:533).
4. [NIT] scripts/tollgate-clientd.py:395-399 — negative remaining renders as a negative clock (human_ms(-5000) == '-1:59:55', measured); reachable via lazily-expired ms sessions (merchant.go:444-446, and the lab's own note at run-clientd-scenarios.sh:11-13), and the waybar percentage (:689) goes negative too. Clamp at 0.
5. [NIT] CHANGELOG.md:627 says "39 tests" (this head ships 43) and :619-620 names 3 of the 5 codes in TERMINAL_PAYMENT_CODES (-keyset-expired, -outcome-unknown missing). Already self-disclosed — fold in at squash.
6. [NIT] scripts/tollgate-clientd.py:285-316 duplicates tests/cloud-lab/conftest.py:71-108 (_expand_keyset_ids, the docstring says so). Have conftest.py import it from the script so the two cannot drift.
7. [NIT] tests/cloud-lab/run-clientd-scenarios.sh:188 — the guard probes 1 of the 3 services S5 starts (:194); probe all three so the SKIPPED line keeps meaning what it says once the keyset-rotation branch lands.
8. [INFO] scripts/tollgate-clientd.py:18 documents POST /?mac=MAC, which this server ignores (main.go:1072-1084, :1182-1187 resolve identity from the socket). --mac's help text already says so; make the docstring match.

Not verified / honest limits: no live scenario battery run (needs the cloud-lab compose; S2–S4 PASS evidence remains the author's, at 85b49052), and no physical-router run — unchanged from the PR body, the NDS MAC-registration/ARP path cannot be exercised in cloud-lab. Gate 2.5 (cold audit by worker-reviewer-hy4) was attempted and is structurally unavailable right now — every cross-family lane returned 503 (hy4-preview → all providers exhausted, candidates_tried: openrouter, deepinfra), curl localhost:9099/quota shows ours weekly at used_pct=100, locked=true, resets_at=1791285725 (2026-10-06T11:22Z), and delegation.model is unset so a delegated child would inherit this reviewer's family. This is therefore a single-gate review; gate 2.5 should re-run against this head after the reset.

FULL_REVIEW: /home/c03rad0r/reports/reviews/tollgate-module-basic-go-t_28b0b226-deepseek-flash.md

[c03rad0r]

does the makefile put tollgate-clientd.py on the router or is it safe to assume that this file only runs on the computer when testing?

[Amperstrand]

Safe to assume computer-only: the PR touches no packaging surface — scripts/tollgate-clientd.py + tests + cloud-lab fixtures only, nothing in packaging/ or the .ipk/.apk install set — so nothing lands on the router. If clientd ever becomes a shipped product it will need its own packaging decision (and its own burn-down of #422/#423/#424); tracked for then.

[felixfelix-bot]

Conflicts resolved on a rebase of your eight commits onto main (ac5811ff); authorship kept.

Replacement PR: #683 — 11 files, +2136/−0, the same file set and the same insertions-only shape as this PR.

Only tests/cloud-lab/README.md conflicted: main added a conformance/ row (#503) where you add test_clientd_autotopup.py. Both rows kept — union of the two sides, one region, nothing else hand-touched. CHANGELOG.md and .gitignore applied clean.

One thing worth knowing: replaying commit-by-commit dropped your two entries under an ### Added deep inside v0.6.0-rc1 (the position your first commit used). Your last commit moved them to [Unreleased] — that is where they sit now, under [Unreleased] → ### Added. Diff is @@ -32,6 +32,47 @@, purely additive; main loses nothing.

Re-verified on the rebased head:

py_compile (clientd + 2 test files)                        -> ok
bash -n run-clientd-scenarios.sh / fake-ndsctl-filecounters.sh -> ok
docker compose -f tests/cloud-lab/docker-compose.yml config     -> ok
python3 -m pytest tests/clientd/ -q                        -> 43 passed in 18.53s

The cloud-lab battery itself (S2–S5, test_clientd_autotopup.py) is not run here — it needs the mint/tollgate/client-ns images and a live nutshell wallet; that lane is unexercised in this environment.

Two ruff findings left as you wrote them, reported not fixed: F401 scripts/tollgate-clientd.py:713 (import shutil unused in selftest()), RUF059 tests/clientd/test_tollgate_clientd.py:473 (unpacked payments unused). One-line cleanups; say the word and I fold them in.

Merge #683 and this PR closes as superseded, or force-push pr/425-clientd-rebased onto feature/tollgate-clientd.

[Amperstrand]

Rebased in place onto current main (8655e33) — the eight commits (feature + review rounds) squashed into one, diff at its honest size: 10 files, +2134. Both CHANGELOG entries under [Unreleased] → Added with the PR link added; the cloud-lab README table conflict resolved keep-both (main's conformance row + this PR's clientd rows). Shell scripts bash -n clean, Python compiles; no Go files touched, no workflows. @felixfelix-bot skip — done (see #676).

[Amperstrand]

@c03rad0r — verified: tollgate-clientd.py is never packaged. Zero references to it anywhere under packaging/ (Makefile, uci-defaults, file lists — grepped on current main); the PR itself touches nothing outside scripts/, tests/cloud-lab/, docs and the changelog. Safe to assume it runs only on the laptop/dev side — it is a client of the router's public API, exercised by the cloud-lab lanes, never installed on the router.

[Amperstrand]

Skipped per PR-REVIEW.md Step 1 (draft). clientd is a product surface with its own lanes (#423/#702 interplay); full pass belongs with its own cycle's review batch.

[felixfelix-bot]

Re-review — PR #425

tollgate-clientd: laptop auto-top-up client + cloud-lab test lane
head 332319b76a · author Amperstrand · 11 files · +2138/-0 · labels release:post-0.6.0, needs:human-review, tested:e2e
artifact: ~/reports/reviews/tollgate-module-basic-go-425-rereview.md

Reviewer: tier/manager-min (zai) — single-family advisory review.
The cross-family lane was unavailable at review time: the flat router returned
503 all providers exhausted (dispatch_gate: can_dispatch=false — all lanes priced out (+inf)),
which killed the 3 delegated review lanes. This verdict is therefore not the D-115
cross-family review; treat it as advisory and re-run a different family before any merge.
CI: this repo has zero checks configured — ci=no_checks_configured. Nothing here is a CI verdict.
Scope: gh pr diff <N> (head vs base). NOT the compare/<old>...<new> endpoint, which reports
97/50 commits of base churn for these branches and is not the PR's delta.

What is new since our last review

Our review (felixfelix-bot, APPROVED) is anchored at 85b49052c6, 2026-09-30. The head is now
332319b76a and the author pushed again on 2026-10-08, so that approval does not cover this tip.
Reading the current gh pr diff (not the compare endpoint, which reports 97 commits of base churn):

Verdict: APPROVE (advisory) — additive, cannot reach the happy path

Verified from the diff

• Zero files under packaging/, no uci-defaults, no admin-UI, no firmware path. The change set is:
  scripts/tollgate-clientd.py (new), tests/cloud-lab/* (new lane: Dockerfile.client-ns,
  fake-ndsctl-filecounters.sh, run-clientd-scenarios.sh, test_clientd_autotopup.py),
  tests/clientd/test_tollgate_clientd.py, .gitignore, CHANGELOG.md. Purely additive (+2138/-0).
• Money-path handling is the part worth reviewing, and it is careful. The client holds the
  operator's laptop against the router's :2121 payment API (GET / advertisement, GET /usage,
  POST /?mac=MAC with a raw Cashu token). It writes unacknowledged payment tokens to disk
  (0600, --state-dir, default ~/.local/state/tollgate-clientd) before the POST and reuses
  them on retry; terminal mint answers (payment-error-invalid-token, payment-error-token-spent)
  stop the daemon with a non-zero exit instead of feeding the backoff loop; and the POST timeout is
  treated as outcome-unknown rather than a blind re-send. Those are the right three properties —
  they are what stops a retry loop from paying twice.
• It is also the client half of a real money path, so "additive" does not mean "risk-free": it is the
  component that can spend the operator's tokens. The guards above are the reason I am comfortable.

Findings

1. (not blocking) tested:e2e is the author's label and I did not run the cloud-lab lane — I am
   not claiming that evidence. The tests/cloud-lab/ scripts and test_clientd_autotopup.py exist and
   read as genuine scenario tests; whoever can run the lab should confirm the lane actually executes.
2. (not blocking) The token state file is the thing that prevents double-payment; worth one explicit
   note in the daemon's docs that deleting it after a timeout is unsafe. The code comments imply this
   ("re-sending the same note is futile") but the operator-facing doc does not say it.

Tag recommendation

tag	verdict	evidence
unlikely to break the happy path	YES	zero install-path files; cannot affect install, config-ui or the portal
after upcoming release	YES	already release:post-0.6.0; a new client is not a v0.6.0 prerequisite
installer relevant	NO	no packaging/, no uci-defaults in the diff
security	NO	it handles Cashu tokens with care (0600, no blind retry) but it is not a security fix
before upcoming release	NO	nothing gates the tag on it

[c03rad0r]

🏷️ Next action — on-hold

On hold — draft PR. Not in the active merge queue.

[Amperstrand]

Closing as superseded: the tollgate-clientd laptop client + cloud-lab tests landed on main 2026-10-06 (feat(scripts): tollgate-clientd laptop client + cloud-lab b...). Content diff of this PR's head vs main is cosmetic only: an import shutil for a module main's selftest never uses, and three _payments → payments renames in the test file. Nothing functional left to merge.