From fd40b29119f31d44f519d5de6914b07a2038c5da Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 20 Jul 2026 13:55:03 +0200 Subject: [PATCH] docs: v0.152.0 REPORT/CONTEXT + fix an async race in TestFabUpload_GCAndIdleTimeout The fab-upload GC test stat-ed the .part immediately after observing the slot free, but expireIdleUpload unlinks AFTER releasing the mutex. Passed alone, failed in the full package once this release's render tests made web heavier. Not a production defect - a new upload mints a fresh random .part. The test now waits for the outcome it asserts on the same deadline; red-proofed by removing the unlink from production. --- CHANGELOG.md | 12 +- CONTEXT.md | 19 +- REPORT.md | 230 +++++++++--------- .../web/handler_export_upload_test.go | 16 +- 4 files changed, 163 insertions(+), 114 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc81f13..fcebb7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,9 +43,15 @@ browse queries on the wire, but the test Mac's sidebar stayed empty — it had n section shown at all, which is a Finder Settings toggle rather than something the box controls. This is recorded as OPEN in the DIAG, deliberately not as a shipped feature. -`TestRenderSambaCompose` asserted the literal tag `felhom-samba:1.0.0`, so a routine image bump read -as a renderer regression. It now derives from `SambaImage` and separately asserts what actually -matters — that the tag is explicit and never `:latest`. +**Two test bugs surfaced and fixed, neither a production defect.** `TestRenderSambaCompose` asserted +the literal tag `felhom-samba:1.0.0`, so a routine image bump read as a renderer regression; it now +derives from `SambaImage` and separately asserts what actually matters — that the tag is explicit and +never `:latest`. And `TestFabUpload_GCAndIdleTimeout` raced: `expireIdleUpload` nils the slot, +releases the mutex, and only then closes and unlinks the `.part`, so "the slot is free" does not yet +mean "the file is gone" — the test stat-ed immediately and passed only by luck. It failed in the full +package while passing in isolation once this release's new render tests made the `web` package +heavier. Now it waits for the outcome it asserts, on the same 3 s deadline; red-proofed by removing +the unlink from production, which still fails it. ### v0.151.0 — the Megosztás page stops reloading, and says how to connect (2026-07-20) diff --git a/CONTEXT.md b/CONTEXT.md index a1f9723..36cdda8 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,24 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-07-20 (v0.151.0 — Megosztás reload loop fixed + connect-address card) +Last updated: 2026-07-20 (v0.152.0 + samba 1.1.0 — mDNS for macOS; S-3 closed) + +> **2026-07-20 — v0.152.0 + felhom-samba 1.1.0 (Megosztás on a Mac).** Closes **S-3**. **A capture +> on the box overturned the earlier guess:** macOS DOES send a correct NBNS query for `<20>` and +> nmbd DOES answer it correctly in 140 µs (flags `0x8580`, RCODE=0, right address) — macOS simply +> never acts on it. NetBIOS there feeds legacy browsing, not `smb://` URL resolution, so **the bare +> `smb://` can never work from a Mac** and nmbd was never the broken part (it is what serves +> Windows). felhom-samba 1.1.0 adds **avahi + dbus**, templating `avahi-daemon.conf` and the +> `_smb._tcp` service file from `FELHOM_SERVER_NAME` so a rename re-advertises; both daemons are +> non-fatal on failure. v0.151.0's card had offered `smb://` for Mac — the one dead form — now +> `smb://.local`; Windows keeps flat `\\`. Spiked live by hand and confirmed from the +> operator's Mac BEFORE publishing the image (the operator's call, and it chose the design too). +> **STILL OPEN: Finder-sidebar discovery is NOT shipped** — the record is published and answers +> browse queries, but was never observed working; likely a Finder Settings → Sidebar toggle, but +> unverified. **Windows was not retested.** Two test bugs fixed en route, neither a production +> defect: `TestRenderSambaCompose` pinned a literal image tag, and `TestFabUpload_GCAndIdleTimeout` +> asserted an async unlink synchronously (it passed alone, failed in the full package once the new +> render tests made `web` heavier). 23/23 green twice; 2 red-proofs. > **2026-07-20 — v0.151.0 (Megosztás).** Closes **S-1/S-2/S-4-core/S-5** of > `felhom.eu/documentation/audits/DIAG-sharing-2026-07-20.md`; **S-3 (no mDNS/Bonjour) stays OPEN**, diff --git a/REPORT.md b/REPORT.md index ed1bf88..8dc34cc 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,141 +1,153 @@ -# REPORT — v0.151.0: Megosztás status contract (S-1 + S-4 core) + connect-address card (S-2 + S-5) +# REPORT — v0.152.0 + felhom-samba 1.1.0: mDNS for macOS (S-3), and the connect card stops offering a dead form -**Date:** 2026-07-20 · **Repo:** felhom-controller (v0.150.0 → **v0.151.0**) · Trunk, pushed to `main`. -**Origin:** `felhom.eu/documentation/audits/DIAG-sharing-2026-07-20.md`. -**Class:** implementation (code + tests + build + deploy + live verify). **S-3 (no mDNS) deliberately -out of scope** — it awaits Viktor's two Mac probes and is an image change, not a controller one. +**Date:** 2026-07-20 · **Repo:** felhom-controller (v0.151.0 → **v0.152.0**) + **felhom-samba +1.0.0 → 1.1.0** · Trunk, pushed to `main`. +**Origin:** operator report — `smb://FELHOM` still failing from a Mac after v0.151.0; S-3 of +`felhom.eu/documentation/audits/DIAG-sharing-2026-07-20.md`. -## Baselines used +## Baselines -| Repo | start `main` @ | end `main` @ | version | -|---|---|---|---| -| felhom-controller | `8db9232` (== `origin/main`, clean tree) | `badf17b` | v0.150.0 → **v0.151.0** | -| felhom.eu | `0c48869` | `eaa2173` | docs only | +| Repo / artifact | start | end | +|---|---|---| +| felhom-controller | `5c105fb`, v0.151.0 live on 9201 | `37e12c8` (+ this docs commit), **v0.152.0** | +| felhom-samba | `1.0.0` | **`1.1.0`**, digest `sha256:1c17c094…` | +| felhom.eu | `a7d9837` | `1d1d60a` | -Deployed at start: `felhom-controller:0.150.0` on guest 9201 — verified `Up (healthy)`, not assumed. +## The finding that redirected the fix -## §4.1 — what `b5d78d1` actually intended, and what became of it +My earlier conclusion — "macOS no longer does NetBIOS" — **was wrong**, and a packet capture on the +box disproved it. On a bare `smb://FELHOM` from the operator's Mac (`192.168.0.11`): -Read in full before touching the coercion. It was **defensive, not a fix for a named repro**: the -v0.147.0 commit message describes slice 4b as detached-and-polled bring-up feedback and never -mentions a status-contradiction bug, and the code's own comment states the intent outright — -*„A stale `idle`/`running` job must never contradict reality: liveness wins on a fresh page."* +``` +11:18:30.222080 .11:52844 > 192.168.0.255:137 NBNS query, 50 B + name → "FELHOM" + 9 pad + suffix 0x20 (File Server Service — correct for SMB) +11:18:30.222220 .104:137 > .11:52844 NBNS response, 62 B, +140 µs + flags 0x8580 = response, AUTHORITATIVE, RCODE=0 · ANCOUNT 1 + TTL 259200 · NB_FLAGS 0x0000 (unique, B-node) · RDATA 192.168.0.104 +``` -That intent is legitimate and survives; it was simply asserted on the wrong channel. Liveness is -already carried by the `running` field in the same JSON object, which is where a LEVEL belongs. So -instead of a repro-derived regression test there is a named test on that field: -`TestSharingStatus_LivenessStillReportedWithoutAnyJob` asserts that with **no job in memory at all**, -`running` tracks `SambaRunning()` in both directions — exactly the guarantee the coercion was written -to provide, minus the phase-channel side effect that produced the loop. +**macOS asked correctly, was answered correctly in 140 microseconds, and never opened a TCP +connection.** It retried once, was answered again, gave up. Sixteen seconds later the same Mac +queried `FELHOM.local` over mDNS and went straight to port 445. -## Files changed (all in commit `badf17b`) +So NetBIOS on macOS feeds legacy browsing, not `smb://` URL resolution: **the bare `smb://` +cannot be made to work from a Mac by any change on our side**, and nmbd — which the R-6/S4b spike +was right to insist on — was never the broken part. It is precisely what serves Windows. + +Investigated and dismissed: the box answers **twice** per broadcast (nmbd holds `0.0.0.0:137`, +`:137` and `:137`; a broadcast lands on two). Standard Samba, and a duplicated correct +answer is still a correct answer that macOS declined to use. + +### Proven matrix + +| Client | Working form | Served by | +|---|---|---| +| Windows | `\\` | nmbd (+ wsdd for the Network view) | +| macOS | **`smb://.local`** | avahi/mDNS — new in 1.1.0 | +| Any | `smb://` | direct | + +## Method — spike before publish (operator's call, and it paid) + +avahi was installed by hand into the *running* container and configured, then proven from a second +machine before any image was built. The operator confirmed `smb://FELHOM.local` connects. Only then +was 1.1.0 built. The spike also chose the design: a **static avahi service file** beats smbd's own +`multicast dns register`, because `smb.conf` is bind-mounted READ-ONLY and owned by the controller's +renderer, and a static file additionally publishes `_device-info._tcp`. + +## Changes | File | Change | |---|---| -| `controller/internal/web/sharing_handlers.go` | A.1 coercion deleted + the two-channel contract documented; B page data gains `SMBDirectAddress` (enabled-only); `sambaLANAddress()` seam resolver; `SMBRunning` gains the nil-manager guard the status handler already had | -| `controller/internal/web/samba_ensure_job.go` | A.2 `consumeIfRunning()` — serve-once for terminal `running`, and only while the single-flight slot is free | -| `controller/internal/web/server.go` | `sambaAddrFn` seam field | -| `controller/internal/web/templates/sharing.html` | B.2 „Csatlakozás a megosztáshoz" section — **markup only** | -| `controller/internal/stacks/samba.go` | `SambaLANAddress()` + the `sambaLANAddr()` seam + the pure `parseIPv4FromIPAddrOutput` | -| `controller/internal/stacks/manager.go` | `sambaAddrFn` field; `SetSambaRunProbe` (exported seam setter — internal/web's tests need a live-container world from another package, same precedent as `SetMigrationDoneHook`) | -| `controller/internal/infra/samba.go` | `SambaHostInterface` — single source for the nic named by smb.conf's `interfaces =`, the container's `FELHOM_IFACE`, and the address read | -| **new** `web/sharing_status_contract_test.go`, `web/sharing_connect_card_test.go`, `stacks/samba_lanaddr_test.go` | Scenarios A/B/C/D + the parser matrix | -| `CHANGELOG.md`, `REUSE.md`, `controller/README.md`, `REPORT.md`, `CONTEXT.md` | docs | +| `controller/infra-images/samba/Dockerfile` | `avahi` + `dbus` added; `rm` of packaged service files; header documents the captured NBNS proof and that sidebar discovery is NOT claimed | +| `controller/infra-images/samba/entrypoint.sh` | templates `avahi-daemon.conf` + `_smb._tcp`/`_device-info._tcp` service file **from `FELHOM_SERVER_NAME`**; starts dbus + avahi, both **non-fatal** | +| `controller/internal/infra/infra.go` | `SambaImage` pin → `1.1.0` (`Images()` and the golden bake follow automatically) | +| `controller/internal/web/templates/sharing.html` | Mac line `smb://` → **`smb://.local`**; Windows flat name untouched | +| `controller/internal/web/sharing_connect_card_test.go` | assertions updated + new `TestSharingConnectCard_MacLineIsDotLocalNotBareName` | +| `controller/internal/infra/samba_test.go` | tag assertion derives from `SambaImage`; adds an explicit non-`:latest` assertion | +| `controller/internal/web/handler_export_upload_test.go` | async-race fix (below) | +| `CHANGELOG.md`, `controller/README.md`, `REPORT.md` | docs | -## Test results +## Tests -**23/23 packages green, run twice** — the second run with `-count=1` (no cache). Package count -unchanged: this task added tests, not packages. +**23/23 packages green, run twice with `-count=1`.** -| Test | Scenario | +Red-proofs, both mutated → FAILED → restored: + +| Mutation | Result | |---|---| -| `TestSharingStatus_SteadyStateProducesNoEdge` | **A** — two polls, live container, empty slot → `phase:"idle"`, `running:true` both times | -| `TestSharingStatus_LivenessStillReportedWithoutAnyJob` | the `b5d78d1` intent regression (both liveness directions) | -| `TestSharingStatus_RealBringUpServedExactlyOnce` | **B** — first poll `running`, second `idle`, `running:true` on both | -| `TestSharingStatus_RunningNotConsumedWhileInFlight` | the in-flight window: not consumed while the slot is held, served once after `release()` | -| `TestSharingStatus_NonEdgePhasesStaySticky` | **C** — `failed`/`needs_password`/`pulling`/`starting` each persist over 3 polls | -| `TestSharingStatus_EnvelopeShapeUnchanged` | exactly the 3 keys `phase`/`error`/`running` | -| `TestSharingStatus_NilStackManagerIsQuiet` | a nil manager does not panic the poll | -| `TestSharingConnectCard_ShowsNameAndDirectAddress` | **D** — Windows + Mac + direct forms present, seam invoked on this render | -| `TestSharingConnectCard_AddressDerivedFreshEveryRender` | **D** — one derivation per render; a changed address reaches the page | -| `TestSharingConnectCard_NoAddressOmitsTheLine` | **D** — `""` omits the address block, keeps the name lines | -| `TestSharingConnectCard_UsesConfiguredName` | **D** — `OTTHON` renders and `FELHOM` appears nowhere | -| `TestSharingConnectCard_AbsentWhenSharingDisabled` | card gated on the feature; zero derivations when off | -| `TestSharingConnectCard_UnsetNameFallsBackToEffective` | unset name → the effective default, never an empty `smb://` | -| `TestParseIPv4FromIPAddrOutput` | 3 accepted forms (incl. verbatim live output) + 8 rejected (link-local, unspecified, loopback-only, IPv6-only, docker error text, …) | -| `TestSambaLANAddressFailsQuiet` | error → `""`, address → passthrough | +| revert the template's Mac line to the bare `smb://{{.SMBServerName}}` | `TestSharingConnectCard_MacLineIsDotLocalNotBareName` **FAILED** on both the missing `.local` and the present bare form, for both configured names (`FELHOM`, `OTTHON`) | +| delete `os.Remove(job.partPath)` from `expireIdleUpload` | `TestFabUpload_GCAndIdleTimeout` **FAILED** — `idle-expired .part must be deleted` | -### Red-proofs (§10) — all three mutated → FAILED → restored → green +### Two test bugs found, neither a production defect -| # | Mutation | Result | -|---|---|---| -| **A** | reinstated `if phase == idle && running { phase = running }` | `TestSharingStatus_SteadyStateProducesNoEdge` **FAILED** — `call 1: phase = "running", want "idle"`, and again on call 2. Restored → green. | -| **B** | deleted the `s.cur = nil` serve-once clear | `TestSharingStatus_RealBringUpServedExactlyOnce` **FAILED** — `second call: phase = "running", want "idle"`; `TestSharingStatus_RunningNotConsumedWhileInFlight` also failed at its post-release assertion. Restored → green. | -| **D** | replaced fresh derivation with a package-level cached value | `TestSharingConnectCard_AddressDerivedFreshEveryRender` **FAILED** on all three assertions — `total derivations = 1, want 2`, the new address absent, the old address still on the page. Restored → green. | +1. **`TestRenderSambaCompose` pinned the literal tag `1.0.0`**, so a routine image bump read as a + renderer regression. Now derives from `SambaImage`, plus a separate assertion for what actually + matters — the tag is explicit and never `:latest`. +2. **`TestFabUpload_GCAndIdleTimeout` raced.** `expireIdleUpload` nils the slot, releases the mutex, + and only *then* closes and unlinks the `.part` — so "the slot is free" does not yet mean "the file + is gone", and the test stat-ed immediately. It passed in isolation and failed in the full package + once this release's new render tests made `web` heavier. **Not caused by this change and not a + production bug** (a new upload mints a fresh random `.part`, so the gap is harmless); the test was + asserting an async post-condition synchronously. It now waits on the same 3 s deadline with the + assertion unchanged. -### Other gates +Design gates `template_id_gate` / `emoji_gate` / `native_confirm_gate` / `offbox_rename_gate`: **OK**. -`template_id_gate` · `emoji_gate` · `native_confirm_gate` · `offbox_rename_gate` — **all OK**. - -**`sharing.html`'s `