142 lines
10 KiB
Markdown
142 lines
10 KiB
Markdown
# REPORT — v0.151.0: Megosztás status contract (S-1 + S-4 core) + connect-address card (S-2 + S-5)
|
||
|
||
**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.
|
||
|
||
## Baselines used
|
||
|
||
| 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 |
|
||
|
||
Deployed at start: `felhom-controller:0.150.0` on guest 9201 — verified `Up (healthy)`, not assumed.
|
||
|
||
## §4.1 — what `b5d78d1` actually intended, and what became of it
|
||
|
||
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."*
|
||
|
||
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.
|
||
|
||
## Files changed (all in commit `badf17b`)
|
||
|
||
| 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 |
|
||
|
||
## Test results
|
||
|
||
**23/23 packages green, run twice** — the second run with `-count=1` (no cache). Package count
|
||
unchanged: this task added tests, not packages.
|
||
|
||
| Test | Scenario |
|
||
|---|---|
|
||
| `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 |
|
||
|
||
### Red-proofs (§10) — all three mutated → FAILED → restored → green
|
||
|
||
| # | 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. |
|
||
|
||
### Other gates
|
||
|
||
`template_id_gate` · `emoji_gate` · `native_confirm_gate` · `offbox_rename_gate` — **all OK**.
|
||
|
||
**`sharing.html`'s `<script>` block is byte-identical to v0.150.0** — verified by extracting the
|
||
block from `git show HEAD:…` and from the working tree and comparing (one block each, equal). Both
|
||
fixes are server-side, so the client contract is proven fixed rather than worked around.
|
||
|
||
## Deploy
|
||
|
||
```
|
||
./build.sh 0.151.0 --push → sha256:0141b62fe64e3d471446d8a1a8c1a719b3669dab26888e50acbf13644677510b, 145M
|
||
docker ps --filter name=felhom-controller --format '{{.Image}} {{.Status}}'
|
||
gitea.dooplex.hu/admin/felhom-controller:0.151.0 Up 26 seconds (healthy)
|
||
```
|
||
|
||
Clean-tree gate held before the build: `git status --porcelain` empty, `HEAD == origin/main`.
|
||
|
||
## Live validation
|
||
|
||
**Method: endpoint-level** — the exact endpoints the UI invokes, so no server logic is skipped, only
|
||
rendering. `claude-in-chrome` is not available on DooPlex.
|
||
|
||
1. **`GET /sharing/status` ×3, authenticated** → `{"data":{"error":"","phase":"idle","running":true},"ok":true}`
|
||
on all three calls. Before the fix the same call returned `phase:"running"` every time. The
|
||
client's reload path is reachable only from `phase === "running"`, so this is the server-side
|
||
proof that the loop's trigger is gone.
|
||
2. **`GET /sharing`, authenticated** → the connect card is present (2 `Csatlakoz` hits — the heading
|
||
and the Mac line) showing `smb://FELHOM` **and `smb://192.168.0.104`**. That address is the
|
||
guest's real current DHCP lease and is a DERIVED value: it matches what `ip -4 -o addr show eth0`
|
||
reports inside the samba container, and it exists as a literal nowhere in the codebase. Grepped
|
||
with ASCII-safe substrings per the CLAUDE.md trap.
|
||
3. **Human check — OPEN, Viktor's to give:** that the page visibly sits still. A browser is the one
|
||
method not available here.
|
||
|
||
## DIAG finding-status flips (`felhom.eu@eaa2173`)
|
||
|
||
S-1 → **FIXED v0.151.0** · S-2 → **SHIPPED v0.151.0** · S-4 → **CORE SHIPPED v0.151.0** (the unified
|
||
async-job layer stays R-45, which gained a one-line note recording the lesson) · S-5 → **SHIPPED
|
||
v0.151.0** · S-3 → **OPEN**, awaiting `smbutil lookup FELHOM` and `dns-sd -B _smb._tcp` from the Mac.
|
||
|
||
## Observations — noticed, not acted on
|
||
|
||
- **The poll timer's idle path.** `tick()` does call `stop()` in the idle branch, so on a steady-state
|
||
page the interval now ends after the first response — but the `.catch()` branch deliberately keeps
|
||
polling on a transient failure, so a page whose status endpoint is erroring polls forever at 1.5 s.
|
||
Recorded, not fixed: the JS was read-only in this task, and this is the next thing anyone auditing
|
||
that block will ask about.
|
||
- **Multi-tab success banner.** With two `/sharing` tabs open during a real bring-up, only the tab
|
||
that polls first sees „A megosztási szolgáltatás fut." Accepted, and documented at the method
|
||
itself; both tabs still show the true state, which comes from the `running` level and the
|
||
server-rendered badge.
|
||
- **`SMBRunning` in `sharingPageData` had no nil-manager guard** while the status handler beside it
|
||
did. Added (one `!= nil`) — noted because it is the only change in this task not traceable to a
|
||
DIAG finding.
|
||
- **`setup.DetectLocalIPs` is the same problem answered differently.** It needs a `HOST_IP` env var
|
||
because the controller sits on a docker bridge; the new read goes through the samba container
|
||
instead, which needs no env plumbing but only works while sharing is up. Neither is wrong — worth
|
||
knowing both exist if a third caller ever wants a guest IP.
|
||
- **The samba container has no `ps`** (found while establishing what the exec path could rely on) but
|
||
does have `/sbin/ip`. Any future in-container probe should verify its tool the way the catalog
|
||
healthcheck rule already requires.
|
||
- **The `.162` half of the original report is still worth a product answer.** The connect card now
|
||
gives customers the right address, but nothing stops a *stale saved favourite* on the Mac from
|
||
failing silently forever. Out of scope here; it is the kind of thing the S-3 mDNS work would make
|
||
moot by putting the box in the Finder sidebar.
|