[mikkelchokolate]

Summary

Audit-hardening wave (zb scope): replaces false-green tests with honest regression coverage and lands the minimal product fixes the audit exposed.

Test hardening (false-green → real assertions):

• [P2] Auth soft: login StatusOK without cookie/csrf; logout without expire; vacuous ConstantTimeCompareCSRF; revoke StatusOK-only #827 auth/session: login now asserts a usable veil_session cookie (non-empty value, positive MaxAge, HttpOnly, SameSite=Lax, path /) plus a non-empty CSRF token; logout asserts an expiration cookie (empty value, negative MaxAge); the vacuous raw subtle.ConstantTimeCompare test now exercises SessionRegistry.ValidateCSRF; persistent-session DELETE asserts the revoked session is actually gone from the registry.
• false-green: panel users CRUD StatusOK-only; missing viewer 403; legacy item-route path #838 viewer authorization: production-mux test drives GET/POST/PUT/DELETE /api/users with a live viewer session and asserts 403 + unchanged state + surviving session; atomic user update/delete handlers reject viewer mutations without state corruption; panel role-tab guard asserts the viewer wiring actually runs.
• false-green: client capability presence-only + quota soft Contains + empty profile password #842 quota enforcement: unknown traffic identities no longer pause quota enforcement — the test injects an unknown identity via a fake Hysteria2 provider, seeds a real pending quota_enforcement target the way the reconciler does, and runs the production enforceQuotaMutation path to completion.
• false-green: empty AcmeChallengeMode planner no-op vs renderer tls-alpn default (Hy2 soft) #844/false-green: Panel+Naive Caddy plans lock owners only (Domains/ACME unasserted) #845/tests: caddyassembly — ServiceName never locked on bind owners #971 Caddy planner/assembly: empty AcmeChallengeMode is a planner no-op while the renderer defaults to tls-alpn-01; plan tests lock Panel/Naive domains, ACME email/domain enrollment, and bind-owner ServiceName: "veil-caddy.service" including ACME challenge binds.
• false-green: Naive apply-plan tests Valid-only (Panel sister locks config/action/runtime) #846 apply plans: Naive apply-plan tests stub Caddy capability probing and lock Configs, Actions (incl. reload veil-caddy.service), and Runtimes.
• false-green: generatedconfig render soft-OR/presence-only (naive/hy2/olcrtc/warp/retries) #857 generatedconfig: exact listen directives, credentials, 64-hex OLC RTC key, exact Warp rule Match/Outbound, exactly-three route-data retries, and real artifact bodies replace soft OR/presence-only assertions.
• tests: renderer/naive — AllowsNaiveTCP err-only + InvalidPanelListen Caddyfile dialect + publicPort soft Contains #895 Naive renderer: requires the forward_proxy handler + expected encoded credentials in Caddy JSON, checks JSON-dialect panel markers (not Caddyfile tokens), and locks the public listen field to :8443.
• false-green: TestServerDispatchesAllOperations omits CaddyLoad + OK-only #900 caddy_load dispatch arm coverage; [P2] Auth tests lock legacy handleLogin/Logout/Status/Sessions; production mux uses revalidation/persist handlers #825/false-green: health probes / healthcheck / wait_health accept empty HTTP 200 (product requires status:ok) #828/false-green: OpenAPI x-roles lists viewer on admin-metadata users/sessions/audit (+ soft role locks) #837/false-green: authz matrix claims OpenAPI-exhaustive but misses rollback/room (+ ghost GET verify) #839/[P2] PUT/DELETE /api/users/{name} revoke sessions before state save — save failure logs user out with no user change #903/[P2] POST /api/v1/clients/bulk all-fail/skip path returns MutationOutcome.success=true (errNoClientChanges) #904 v1 client envelope/audit/capability/bulk/diagnostic/OpenAPI test hardening.

Product fixes (real bugs found by the audit):

• tests: InboundClone/List profile-isolation — ProtocolFields map shared; catalog corruption via List stays green #945 inbounds.Slice/management-state snapshot clones were shallow: ProtocolFields maps and nested containers aliased the source. New model.CloneInbound/CloneProtocolFields/DeepCloneValue deep-copy profiles, runtime credentials, and nested containers; both call sites now use them.
• Health probes (cliflow/status, cliflow/update) now require the actual Veil {"status":"ok"} JSON contract rather than any HTTP 200.
• client_v1_routes.go, user_update_atomic.go, user_delete_atomic.go, endpoint_capabilities.go, docs/openapi.yaml — supporting fixes for the audit findings above.

Deferred: #770 is owned by the sibling fix/wave-zg branch (CI concurrency + install-script scope); the zb edit was reverted. All other candidate issues not listed here are deferred — they are either sibling-owned or not confirmed in this diff.

Verification: gofmt clean on all committed blobs. Docker (golang:1.27-bookworm, Linux) run of ./internal/privileged and ./internal/apply/...: PASS. ./internal/api run CI-style via test-orchestrator.py (4 shards + serial lane, -race): every test root executed; the only failures were three SPA-shell tests that require a built web/dist (repo ships a stub — CI builds the frontend first; all pass with a realistic dist overlay) and TestQuotaEnforcementRetriesUntilDepletedRevisionIsApplied, a pre-existing second-boundary flake in a file untouched by this diff (passes in isolation).

Closes #825
Closes #827
Closes #828
Closes #837
Closes #838
Closes #839
Closes #842
Closes #844
Closes #845
Closes #846
Closes #857
Closes #895
Closes #900
Closes #903
Closes #904
Closes #945
Closes #971

[mikkelchokolate]

Review (head 4ad6ad04fa39)

Verdict: would APPROVE — GitHub blocks APPROVE on own PRs, so this is a COMMENT with the same intent on the product delta. Merge is blocked until rebase: mergeable_state=dirty, behind_by=9.

Scope

Single product commit on fix/wave-zb vs main (c6f24fe60f): honest regression tests (false-green → real assertions) plus the product fixes the audit forced — deep inbound clones (#945), healthz JSON contract, atomic user update/delete ordering, v1 bindings GET, bulk envelope honesty, OpenAPI/capability alignment. 60 files, +2178/−249 (≈10 product paths, ~50 test paths).

Product — OK

• #945 clone — model.CloneInbound / CloneProtocolFields / DeepCloneValue deep-copy nested map[string]any / []any / []string / map[string]string; inbounds.Slice and managementstate.cloneInbounds both call it. Profiles/RuntimeCredentials are scalar structs, so slice append copy is enough. Regression covers nested mutation both ways.
• Health probes — cliflow/status.Probe and cliflow/update.WaitForHealthyAt require {"status":"ok"} via shared HealthzStatusOK instead of any HTTP 200. Fail-closed on bad/partial body.
• Atomic user update/delete — Mutation.UpdateUser/DeleteUser/CreateUser call m.save() immediately, so “persist before revoke, restore on revoke failure” is a real compensating write, not a no-op inside an uncommitted callback. Ordering matches the comment: a failed save never revokes sessions for a change that did not land.
• v1 bindings GET — list + by-id read models; capability + OpenAPI (GET .../bindings/{bindingId}) added. Aligns with viewer read surface.
• Capability/OpenAPI — dropping GET /api/backups/{name}/verify from endpointPolicies matches the handler (POST-only verify) and OpenAPI; leftover GET falls through the path-pattern admin gate then 405. Viewer removed from admin-secret surfaces; viewer kept/added on the documented read/tools paths.
• Test hardening — cookie/CSRF/session, viewer denial on /api/users, quota leftover-identity path that seeds a real pending target and runs enforceQuotaMutation, Caddy planner/domain/ACME locks, generatedconfig exactness, Naive forward_proxy + listen :8443, etc. Assertions match the claimed issue closures.

Merge blockers (process — not product defects)

Simulated git merge origin/main hits 3 content conflicts (main is 9 commits ahead, including #973–#978/#974/#976/#989):

1. internal/api/client_v1_routes.go — zb extracted handleV1ListClientBindings; main inlined the same GET list. Keep the helper (or inline once); behavior is equivalent.
2. internal/api/generated_config_set_test.go — zb locks credentials/forward_proxy in consolidated Caddy JSON; main locks listener ports + “no per-inbound naive artifact”. Keep both assertion sets.
3. internal/caddyassembly/hdd_happy_path_test.go — zb locks panel/naive domain ACME enrollment; main locks DefaultChallengeMode + empty ACMEChallenges on shared :443. Keep both.

Please rebase onto current main, resolve those three by unioning the strengthened assertions, and re-push before merge.

Mild (non-blocking)

1. Unreachable bulk success=false — in handleV1Bulk, when !anyApplied the callback already returns errNoClientChanges, and the outer if err != nil { writeV1ClientError; return } exits before the new if !anyApplied && failed > 0 { resp["success"] = false } (lines 239–241). Honest failure is already on the error path; the envelope override is dead. Either delete it or special-case errNoClientChanges into a 200 envelope if that contract is intentional.
2. DeepCloneValue shape coverage — covers the JSON-decoded containers the panel stores; exotic typed maps/slices outside the switch still alias. Acceptable for #945 as tested; extend if non-JSON shapes appear in ProtocolFields.
3. Health body read errors ignored — io.ReadAll errors are discarded; empty/partial body fails HealthzStatusOK (fail-closed). Fine; optional to surface read I/O distinctly.

CI (4ad6ad04fa39 at review time)

No Actions workflow runs, check-runs, or combined statuses observed on this head yet (PR just opened). Re-check after rebase — required checks should go green before merge.

Bottom line

Product + test hardening look correct and would be APPROVE-worthy. Do not merge until rebase resolves the three conflicts; then own-PR still needs a non-author approve or admin merge.

[mikkelchokolate]

Mild: this success=false override is currently unreachable. When !anyApplied, the mutation callback returns errNoClientChanges and the outer if err != nil { writeV1ClientError(...); return } exits before we build resp. Honest failure already rides the error path — either drop this branch or intentionally map errNoClientChanges into a 200 envelope if that is the desired bulk contract.

[mikkelchokolate]

Review (head 7635702952006511480c6c6e11598938a01ea4a8)

Verdict: would REQUEST_CHANGES — GitHub blocks both APPROVE and REQUEST_CHANGES on own PRs, so this is a COMMENT with the same intent. Product wave zb remains sound (same substance as prior review on 4ad6ad0), but this head is CI-red on a PR-introduced OpenAPI ↔ orval drift that also skips the Go test job (needs: frontend). Fix the codegen before merge.

Delta vs prior review (4ad6ad0 → 7635702)

• Second commit is merge origin/main into fix/wave-zb — resolves the prior dirty state (conflicts in client_v1_routes, generated_config_set_test, hdd_happy_path_test).
• Wave zb product paths remain intact; bulk envelope now also hoists anyApplied and forces success=false when every client fails (addresses prior mild note on dead bulk success path).
• New / newly visible blocker: docs/openapi.yaml adds GET /api/v1/clients/{id}/bindings/{bindingId}, but committed web/src/api/generated/** has no getApiV1ClientsIdBindingsBindingId* (only PATCH/DELETE). Same gap was already on 4ad6ad0; current CI makes it definitive.

Product — OK (re-verified)

• #945 deep clone — model.CloneInbound / CloneProtocolFields / DeepCloneValue deep-copy nested map[string]any / []any / []map[string]any / []string / map[string]string. inbounds.Slice and managementstate.cloneInbounds both call CloneInbound. Profiles/RuntimeCredentials are scalar structs → slice append is sufficient. Nested mutation regression covers both directions.
• Health probes — cliflow/status.Probe and cliflow/update.WaitForHealthyAt require {"status":"ok"} via shared HealthzStatusOK; bare 200 / wrong status fail closed. Regression rejects "", "ok", unhealthy, degraded.
• #838 viewer auth — atomic update/delete gate on requestHasAdminRole; production-mux viewer CRUD test asserts 403 + unchanged state + live session/CSRF.
• #842 quota — unknown identity no longer pauses; test seeds real pending quota_enforcement and runs enforceQuotaMutation to completion.
• v1 bindings GET — handleV1GetClientBinding + capability GET .../bindings/{bindingId} = viewer; OpenAPI documents it. Handler/OpenAPI/capability are aligned; generated TS clients are not.
• Bulk honesty — anyApplied captured outside the mutation closure; all-fail → resp["success"]=false.
• Honest test wave (#827 session/CSRF, #844/#845/#971 Caddy, #846 apply plans, #857 generatedconfig, #895 Naive, #900/#825/#828/#837/#839/#903/#904 client/envelope) reads as real assertions, not presence-only OR soft greens.

Blockers

1. Frontend generated-client drift (CI failure). Job frontend fails: [ci] === generated API client drift === — regenerating with orval 8.33.0 adds GetApiV1ClientsIdBindingsBindingId* to clients.ts / MSW / zod that are absent from the tree. Because test, e2e, package-smoke-matrix, etc. needs: frontend, those jobs are skipped; package-smoke aggregator then fails closed (PACKAGE_SMOKE_MATRIX_RESULT=skipped).
   • Fix: regenerate orval clients from the PR’s docs/openapi.yaml and commit the blobs (handler already exists — regen is the right path).

Mild (non-blocking)

• DeepCloneValue default branch returns unknown typed containers by reference (fine for JSON-decoded map[string]any trees; note if non-JSON shapes ever land in ProtocolFields).
• Branch is behind main by 1 (888df6cb — chore(deps): bump the github-actions-updates group (#991)). mergeable=true, mergeable_state=behind. Update branch after codegen.

CI summary (head 7635702)

Check	Result
frontend	failure (orval drift)
package-smoke	failure (matrix skipped → fail-closed)
test / e2e / browser-e2e / install-acceptance / privilege-boundary / package-smoke-matrix	skipped (needs frontend)
lint, image-build, filesystem-faults, multi-process, sigkill, CodeQL / analyze	success

Mergeability

mergeable=true, mergeable_state=behind, behind_by=1 (no remaining file conflicts). Do not merge until frontend is green and test actually runs.

Prior review

Prior COMMENT on 4ad6ad0 (id 5305498204) would APPROVE but noted dirty/behind_by=9. Conflicts cleared via merge; codegen drift is now the gate.

[mikkelchokolate]

Review (head a7bcfe0241729d85e5e50180d7ea68ee2ffd3135)

Verdict: would APPROVE — GitHub blocks APPROVE on own PRs, so this is a COMMENT with the same intent. Prior codegen blocker is cleared; product wave zb remains sound.

Delta vs prior review (7635702 → a7bcfe0)

• Merge origin/main (888df6cb — github-actions CodeQL bump #991) → behind_by=0.
• chore(web): regen API client for binding GET endpoint adds getApiV1ClientsIdBindingsBindingId* / zod params+response / MSW query hooks to match docs/openapi.yaml GET /api/v1/clients/{id}/bindings/{bindingId}.
• Frontend CI now green (orval drift check passed); test / e2e / package-smoke-matrix / privilege-boundary / install-acceptance / browser-e2e are queued or running (no longer skipped).

Prior blocker — cleared

OpenAPI documented the single-binding GET while committed web/src/api/generated/** lacked the symbols. Regen restores alignment with handler handleV1GetClientBinding and capability GET .../bindings/{bindingId} = viewer.

Product — OK (unchanged substance; re-spot-checked)

• #945 deep clone — CloneInbound / CloneProtocolFields / DeepCloneValue; inbounds.Slice + managementstate snapshot use them.
• Health probes — cliflow/status + cliflow/update require {"status":"ok"} (fail closed on bare 200 / wrong status).
• #838 / #842 / bulk honesty / honest test wave — same as prior review on 7635702 / 4ad6ad0.
• v1 bindings GET — handler + OpenAPI + capability + generated clients now consistent.

Blockers

none

Mild (non-blocking)

• Orval clients.ts emits GET binding as useMutation (same pattern as sibling useGetApiV1Clients / useGetApiV1ClientsIdBindings); MSW correctly emits useQuery. Pre-existing generator quirk, not introduced by this regen.
• DeepCloneValue default branch still returns unknown typed containers by reference (fine for JSON-decoded map[string]any trees).

CI summary (head a7bcfe0, at review time)

Check	Result
frontend	success (prior failure fixed)
lint, image-build, filesystem-faults, multi-process, sigkill, CodeQL, analyze (actions/js-ts)	success
test / e2e / browser-e2e / install-acceptance / privilege-boundary / package-smoke-matrix	queued / in_progress
analyze (go)	in_progress

Mergeability

mergeable=true, mergeable_state=blocked (required checks still settling), behind_by=0. Safe to merge once test (and other required jobs) finish green.

Prior reviews

• COMMENT on 4ad6ad0 (id 5305498204) — would APPROVE; dirty/behind.
• COMMENT on 7635702 (id 5305802979) — would REQUEST_CHANGES for orval drift; resolved by this head.

[mikkelchokolate]

Review (head cb0ac7843c53cb4d3eef45668b4c4212862aadf0)

Verdict: would APPROVE — GitHub blocks APPROVE on own PRs, so this is a COMMENT with the same intent. Go SDK codegen now matches the OpenAPI/frontend binding GET; product wave zb unchanged and still sound.

Delta vs prior review (a7bcfe0 → cb0ac78)

• Single commit: chore(sdk): regen Go client for binding GET endpoint.
• Only file touched: sdk/go/veilclient.gen.go (+210 / −0).
• Adds oapi-codegen surface for GET /api/v1/clients/{id}/bindings/{bindingId} (GetApiV1ClientsIdBindingsBindingId*):
  • ClientInterface + Client method
  • NewGetApiV1ClientsIdBindingsBindingIdRequest (MethodGet, path /api/v1/clients/%s/bindings/%s)
  • ClientWithResponsesInterface + WithResponse wrapper
  • GetApiV1ClientsIdBindingsBindingIdResponse with JSON200 *ClientBinding, 401/403/404 (+ WWW-Authenticate on 401)
  • ParseGetApiV1ClientsIdBindingsBindingIdResponse — same switch pattern as sibling Delete/Patch parsers
• Aligns with docs/openapi.yaml (operationId / responses / ClientBinding) and with the prior web orval regen (web/src/api/generated/clients|msw|zod).

Prior blocker — still cleared

Frontend orval drift (missing GET binding symbols) was fixed on a7bcfe0. This head closes the matching Go SDK gap so server OpenAPI, web clients, and sdk/go stay in lockstep.

Product — OK (unchanged; not re-diffed beyond spot-check)

Wave zb substance from 4ad6ad0 / prior reviews still applies:

• #945 deep clone (CloneInbound / CloneProtocolFields / DeepCloneValue) at inbound slice + management-state snapshot sites
• Health probes require {"status":"ok"} (not bare HTTP 200)
• Honest regression coverage for #825/#827/#828/#837/#838/#839/#842/#844–#846/#857/#895/#900/#903/#904/#971
• v1 single-binding GET handler + capability + OpenAPI already present; generated clients (web + Go) now complete

Blockers

none

Mild (non-blocking)

• CI on this head is still settling (expected right after push). multi-process / sigkill / analyze (actions) already green; frontend / lint / test / CodeQL matrix / image-build / filesystem-faults still pending or in progress at review time.
• Orval GET-as-useMutation quirk on the web clients (noted on a7bcfe0) is unchanged and pre-existing.

CI summary (head cb0ac78, at review time)

Check	Result
multi-process, sigkill, analyze (actions)	success
frontend, lint, image-build, filesystem-faults, analyze (go/js-ts)	pending / in_progress
CodeQL (aggregate)	skipping (matrix jobs running)
test / e2e / browser-e2e / install-acceptance / privilege-boundary / package-smoke-matrix	not yet reported on this head

Mergeability

mergeable=true, mergeable_state=blocked (required checks still settling), behind_by=0. Safe to merge once required jobs finish green.

Prior reviews on this PR

• COMMENT on 4ad6ad0 (id 5305498204) — would APPROVE; then dirty/behind.
• COMMENT on 7635702 (id 5305802979) — would REQUEST_CHANGES (orval drift); resolved.
• COMMENT on a7bcfe0 (id 5306402762) — would APPROVE after web regen; this head adds the Go SDK counterpart.

[mikkelchokolate]

Review — head 97814dcf

Verdict: would APPROVE (own-PR blocks APPROVE).

Delta vs prior review head cb0ac784

Merge-only push: Merge remote-tracking branch 'origin/main' into fix/wave-zb bringing #994 (test: harden false-green assertions across api, serve, and e2e). Compare cb0ac784...97814dc = 2 commits / 13 files; no wave-zb product edits in this push.

Product wave zb sanity (blob-identical vs cb0ac784): internal/model/clone.go, health probes (cliflow/status, cliflow/update), docs/openapi.yaml, Go SDK veilclient.gen.go, orval clients (clients.ts / .zod.ts / .msw.ts), client_v1_routes.go, atomic user update/delete, endpoint_capabilities.go — all unchanged. Prior orval frontend blocker remains cleared; #945 clone/health fixes intact. No product regression vs the prior zb OK assessment.

What #994 adds (already reviewed as would-APPROVE on #994):

• False-green honesty: client audit positive target match (#874), binding patch/rotate success:true (#841), unbound-client bulk envelope, disabled inbound for deterministic apply, metrics/livez/readyz/healthz body contracts (#829), SSE zeroed counters, e2e mieru link + exact generated/mieru/server_config.json users/transports + stage-apply plan.valid/writtenFiles (#899), CI concurrency/cancel-in-progress lock in install_script_test (#770).
• Sole production delta: extract HTTPServer.resolvedHelperSocket() — behavior-preserving refactor so the default-socket test can assert the path directly (#875). Build still wires the same default/HelperSocket value.

Blockers

None.

Mild notes

1. Port-collision timeout (proxy_negative_test.go): 5s → 15s — acceptable flake guard; not a correctness change.
2. E2E apply 409 retry (management_flow_test.go): up to ~5s of 500ms sleeps before requiring 200 — honest vs accepting 200|409, but can stretch wall time under load.
3. Audit rename assertion: after rename, positive check is item.Target == id (not specifically update_client) — intentional identity-survival honesty; action-type coverage is slightly weaker than pre-#994.
4. #770 via merge: zb PR body deferred concurrency/cancel to fix/wave-zg, but #994’s test lock for ci.yml concurrency now sits on this branch. No conflict — just note the issue is partially covered here via main.
5. CodeQL check: completed neutral (“configuration not found” vs main) — known post-#991 workflow noise, not a product signal.

CI snapshot (97814dcf)

Check	Status
filesystem-faults, sigkill, multi-process, image-build, analyze (actions)	success
frontend, lint, analyze (go), analyze (javascript-typescript), codeql workflow	in_progress
CodeQL (advanced-security)	neutral

behind_by vs main (b1a51c8): 0 (PR is 6 commits ahead; mergeable but mergeable_state=blocked pending remaining checks / review policy).

Ship once remaining CI goes green.

[mikkelchokolate]

Review — head b1a6d5494c209a8914ae10e96bad60911ac0e052

Verdict: would APPROVE (own-PR cannot submit APPROVE → COMMENT).

Delta vs last reviewed head 97814dcf

Not a merge-main bump. 1 commit / 1 file:

• test(cli): healthcheck fixture returns the {"status":"ok"} contract body (b1a6d549)
• Only internal/cli/healthcheck_regression_test.go (+3/−1)

TestHealthcheckReadsDerivedContract previously answered with a bare 200 (WriteHeader(StatusOK) and no body). Product probes (cliflow/status.Probe / cliflow/update.WaitForHealthyAt) now require HealthzStatusOK → JSON {"status":"ok"}. The fixture now sets Content-Type: application/json and writes that body — matching the already-correct sibling fixture earlier in the same file (~L18–29). This closes a fail-closed / false-green mismatch introduced by the wave-zb health contract change.

behind_by=0 vs 97814dcf; ahead by that single commit only.

Product wave-zb claims (re-checked)

Blob-identical vs 97814dcf for all key product / SDK / OpenAPI paths:

• internal/model/clone.go, inbound_clone.go, managementstate/snapshot.go (#945 deep clone)
• internal/cliflow/status/container_health.go, internal/cliflow/update/health.go (health JSON contract)
• client_v1_routes.go, user_{update,delete}_atomic.go, endpoint_capabilities.go
• docs/openapi.yaml, sdk/go/veilclient.gen.go, web orval/zod/msw clients

No product regression in this push; wave-zb product surface still holds.

Blockers

none

Mild notes

• Good catch: after tightening the probe, leaving a bare-200 fixture would have turned TestHealthcheckReadsDerivedContract into an honest failure (or, worse, if the test path didn’t exercise the body check, a latent false green). Aligning the fixture is the right fix; no further test scrub needed in this file (other happy-path fixture already returns the contract body).
• Carry-forward from prior reviews (unchanged here): port-collision timeout 5s→15s, e2e notes — not re-litigated.

CI (b1a6d549)

Job	Status
lint	success
image-build	success
multi-process	success
sigkill	success
filesystem-faults	success
analyze (actions)	success
analyze (javascript-typescript)	success
frontend	in_progress
analyze (go)	in_progress
CodeQL	neutral (config-not-found vs main; not a product fail)

No failed checks. Remaining in-progress jobs are the usual long-tail; not blocking the review verdict on this delta.

Summary

Small, correct test-only follow-up that keeps the healthcheck regression suite honest against the wave-zb probe contract. Product / OpenAPI / SDK / clone claims unchanged and still good. Would APPROVE.

[mikkelchokolate]

Review — head f479bcbec5be6be82aff9b85f28d4dc4a9e4dd3a

Verdict: would APPROVE (own-PR cannot submit APPROVE → COMMENT).

Delta vs last reviewed head b1a6d549

Merge-only push: Merge branch 'main' into fix/wave-zb bringing #990 (chore(deps-dev): bump the web-dependencies group in /web with 2 updates). Compare b1a6d549...f479bcbe = 2 commits / 3 files; no wave-zb product edits in this push.

Files from #990:

• web/package.json — @babel/parser 8.0.5→8.0.6, orval ^8.33.0→^8.35.0 (blob-identical to main)
• web/pnpm-lock.yaml — matching lockfile
• web/src/api/generated/zod/clients/clients.zod.ts — PatchApiV1ClientsIdBody.quotaResetPolicy regen style zod.enum([...]).nullish() → zod.union([zod.literal(...), zod.literal(null)]).nullish() (orval 8.35 output)

Product wave zb (blob-identical vs b1a6d549)

Re-checked key product / SDK / OpenAPI / health / clone paths — all identical:

• internal/model/clone.go, inbound_clone.go, managementstate/snapshot.go (#945)
• internal/cliflow/status/container_health.go, internal/cliflow/update/health.go (health JSON contract)
• client_v1_routes.go, user_{update,delete}_atomic.go, endpoint_capabilities.go
• docs/openapi.yaml, sdk/go/veilclient.gen.go
• web orval clients.ts / clients.msw.ts (binding GET symbols intact)
• internal/cli/healthcheck_regression_test.go (prior fixture fix intact)

Only zod differs vs prior head (the #990 quotaResetPolicy line above). Diff vs main zod is still just the wave-zb GetApiV1ClientsIdBindingsBindingId* schemas (+23/−0) — expected until this PR lands.

Blockers

none

Mild notes

1. quotaResetPolicy zod.union + literal(null) + .nullish() — same note as on #990: slightly redundant nullability, but matches committed orval 8.35 output on main. Not a drift risk as long as frontend regenerates against the PR OpenAPI (binding GET schemas remain).
2. Prior test red on older head 97814dcf was package-014 (unrelated to this merge). Head b1a6d549 CI was largely green with test still running at cutover; this head’s CI is freshly queued.

CI (f479bcbe, at review time)

Job	Status
analyze (actions)	success
analyze (go / javascript-typescript)	in_progress
CodeQL	neutral (config-not-found vs main; known post-#991 noise)
ci workflow	pending (jobs not yet started)

behind_by vs main (86fc28b6 / #990): 0. mergeable=true, mergeable_state=blocked pending required checks.

Summary

Merge-main only; wave-zb product / OpenAPI / SDK / clone / health claims unchanged and still good. Would APPROVE once required CI on this head settles green.

Prior reviews on this PR

• COMMENT 4ad6ad0 — would APPROVE; then dirty/behind
• COMMENT 7635702 — would REQUEST_CHANGES (orval drift); resolved
• COMMENT a7bcfe0 / cb0ac78 / 97814dc / b1a6d549 — would APPROVE; this head continues that line after #990 merge

[mikkelchokolate]

Review — head 33fdccf7601befc7e997241bcd094761e0020c45

Verdict: would APPROVE (own-PR cannot submit APPROVE → COMMENT).

Delta vs last reviewed head f479bcbe

Merge-only push: Merge branch 'main' into fix/wave-zb bringing #992 (fix: metrics path labels, audit spool honesty, privileged fencing, and API mutation edge cases). Compare f479bcbe...33fdccf7 = 2 commits / 29 files; no wave-zb product edits in this push beyond the expected clean merge into overlapping API files.

What #992 adds on this branch (already on main):

• Audit spool honesty in health (auditHealth() → audit_spool = spool_active when durable, else durability_unverified) (#981)
• Migrate repair counts (bindingsCreated / credentialsCreated) and treat repair writes as applied (#983)
• Binding enable requires an active password credential (#986)
• Runtime fence, privileged fencing regressions, expiration next-boundary honesty, metrics path labels, etc.

Product wave zb (re-checked)

Original zb commit touched 60 paths; vs prior head 59/60 blob-identical. Sole diff is internal/api/client_v1_routes.go, and the patch is strictly additive #992 (sql import + migrate totals + ActiveCredential gate). Wave-zb symbols still present: handleV1ListClientBindings, handleV1GetClientBinding, routing to both.

Curated product / SDK / OpenAPI / health / clone paths 15/15 blob-identical vs f479bcbe:

• internal/model/clone.go, inbound_clone.go, managementstate/snapshot.go, inbounds/slice.go (#945)
• cliflow/status/container_health.go, cliflow/update/health.go, cli/healthcheck_regression_test.go (JSON {"status":"ok"} contract + fixture)
• user_{update,delete}_atomic.go, endpoint_capabilities.go
• docs/openapi.yaml, sdk/go/veilclient.gen.go
• web orval clients.ts / clients.msw.ts / clients.zod.ts (binding GET + prior #990 zod style intact)

health_routes.go also changed vs prior head, but only with the #992 auditHealth / spool honesty block — wave-zb health probe contracts live in cliflow and are unchanged.

Blockers

none

Mild notes

1. Carry-forward: quotaResetPolicy zod.union + literal(null) + .nullish() from #990/orval 8.35 — redundant nullability, matches main.
2. #992’s binding-enable credential check uses ActiveCredential(..., "password") only — fine for current Naive/password bindings; watch if non-password credential kinds become enableable later.
3. CI long-tail still settling (test / e2e / browser-e2e / install-acceptance / privilege-boundary / package-smoke-matrix in_progress).

CI (33fdccf7, at review time)

Job	Status
frontend, lint, image-build, filesystem-faults, multi-process, sigkill, CodeQL, analyze (go/js-ts/actions)	success
test, e2e, browser-e2e, privilege-boundary, install-acceptance (amd64/arm64), package-smoke-matrix (amd64/arm64)	in_progress

behind_by vs main (c1e3fb9c / #992): 0. mergeable=true, mergeable_state=blocked pending required checks / review policy.

Summary

Merge-main only (#992). Wave-zb product / OpenAPI / SDK / clone / health claims unchanged and still good; overlapping client_v1_routes.go merge kept zb handlers and layered #992 honesty cleanly. Would APPROVE once remaining required CI on this head goes green.

Prior reviews on this PR

• COMMENT 4ad6ad0 — would APPROVE; then dirty/behind
• COMMENT 7635702 — would REQUEST_CHANGES (orval drift); resolved
• COMMENT a7bcfe0 / cb0ac78 / 97814dc / b1a6d549 / f479bcbe — would APPROVE; this head continues that line after #992 merge