Review round 2: a media restart no longer starts an app the household stopped (R-721); the file manager's header name file appears whole (R-1100 race, 44/300 -> 0/300); both red-proved; unreleased

CHANGELOG.md

@@ -22,6 +22,20 @@ brief 2026-10-24, Part 4 (commit only; the release rides the next normal train).
   `…OnlyWhenTheAgentWaitsAndAnAppRuns`, `…QBLimitRuleSkipsAHeldLimit`, `…TheTwoAppsReadAndWriteTheirLimit`,
   `TestLineTestRunOutlastsTheClientTimeout`, `TestStartUpwatchRecoversTheHoldFirst`, `TestUpwatchHoldGetsBothAgentCalls`.
 
+### Review round 2 (2026-10-24, unattended; commit only, no release) — two small fixes, each red-proved
+
+
+- **A media restart no longer starts an app the household stopped (R-721).** „Újraindítás most", „Később" (03:30)
+  and the safe removal recreated every media app with `up -d --force-recreate`, which STARTED a stopped one.
+  `stacks.MediaAppsToRecreate` leaves those out (logged „NOT recreating … the household stopped it"); their override
+  is already written, so their next start sees the drives. Pinned by `TestMediaAppsToRecreate_SkipsAppsTheHouseholdStopped`
+  (stacks) and the wiring pin `TestR2_MediaRecreateLoopsSkipStoppedApps` (web) — each seen failing with the rule off.
+- **The file manager's per-box header name file appears whole (R-1100).** `infra.LoadOrCreateFileBrowserSSOHeader`
+  created the file and then wrote it; a second caller at the same start (the stacks manager and the web server) could
+  read it empty and get an error — 44 of 300 runs of the existing parallel-writers check failed, 4 of 50 under
+  `-race`. Now a 0600 temp file is written, synced and hard-linked into place (the link is the exclusive create):
+  0 of 300, 0 of 50 under `-race`.
+
 ## v0.342.0 (2026-10-24) — RELEASED on the operator's yes (attended release run; image sha256:d002d3b7…, from `42cda07`, CI 2283) — the upload and CPU watch, the tested line and qBittorrent's limit
 
 **MinAgent: 0.158.1** (unchanged). **Hub dependency:** the four new event types need hub 0.169.0 (committed the same

REPORT.md

@@ -1,20 +1,18 @@
-# REPORT — the line test for a box that seeds all day (R-1250), 2026-10-24 (attended)
+# REPORT — review round 2 (2026-10-24, unattended): two small fixes, commit only
 
-Brief: „two DooPlex backup alarms …", Part 4 (commit only; the release rides the next normal train). Design: felhom.eu
-`architecture/08-alarm-ladder.md` §9 item 5. Commit `046b808` (CI 2287 success). Not released, no image built.
+**What:** found by the review round's readers and verified at `file:line` by the main session (felhom.eu
+`documentation/audits/review-round-2-2026-10-24/`). No release; both ride the next normal train (next v0.343.0).
 
-- **What:** when the agent says the weekly test is due and waits only for quiet, and qBittorrent or Transmission runs,
-  the controller records the app's upload limit first, holds the app at 1 KiB/s, asks the agent for a test on request,
-  then puts the exact old limit back. Deferred release; a failed hold is undone first; a household change during the
-  test stays; an unreachable app keeps the record; a killed controller's start-up check puts it back; the 50 % rule
-  waits while a record exists. `agentapi.LineTestRun` (own 2-minute timeout via `postSlow`); Transmission's RPC shared.
-- **Tests:** 13 new (CHANGELOG lists them), each seen failing: seven hold rules broken one by one, the slow POST, the
-  start-up order, the wiring of both agent calls.
-- **Live (demo-hp, endpoint level, probes from this commit and agent `1e087bb`):** a real 10 Mbit/s seed from scratch
-  guest 9202 to DooPlex's qBittorrent. Without the hold the agent refused (`not-quiet`, 11.1 Mbit/s). With it the uplink
-  fell to 0.02–0.16 Mbit/s, the test read 94.2 Mbit/s, the hold lasted 56.8 s, and qBittorrent was back at 1 249 280 B/s
-  exactly. A `kill -9` in the middle left the record and the 1 KiB/s; the start-up check put the limit back. The pinned
-  agent client is unit-tested only (the probes spoke plain HTTP). Evidence: felhom.eu
-  `audits/dooplex-alarms-2026-10-24/part4-r1250-live-proof.txt`.
-- **Teardown:** the probe and its store removed from 9202's controller container; qBittorrent removed through the product.
-- **MinAgent:** 0.158.1 unchanged (an older agent never reports `waiting_for_quiet`, so nothing is held).
+1. **A media restart no longer starts an app the household stopped (R-721).** `restartMediaApps` and
+   `mediaSafeRemoval` recreated every media app (`RestartStack` = `up -d --force-recreate`), skipping only held ones.
+   Now both take `stacks.MediaAppsToRecreate`, which leaves out an app whose recorded intent is „stopped" (logged). Its
+   override is already written, so its next start sees the drives. Tests: `TestMediaAppsToRecreate_SkipsAppsTheHouseholdStopped`
+   (red with the rule off) and `TestR2_MediaRecreateLoopsSkipStoppedApps` (source pin of the two loops; red with
+   `s.mediaApps()` back in the safe removal).
+2. **The file manager's header name file appears whole (R-1100).** `LoadOrCreateFileBrowserSSOHeader` created the file
+   empty and wrote it after; a parallel caller read "" and got an error. Measured on clean `main` `321df1f`: 44 of 300
+   runs of the existing parallel-writers check failed (4 of 50 under `-race`). Now: temp file, sync, hard link into place
+   — 0 of 300, 0 of 50 under `-race`.
+
+`go vet ./...` and `go test ./...` green. Design docs read: `15-media-library.md` §10–11 (the restart), `01` §5 / `02`
+(the file manager sign-in).

controller/README.md

@@ -3518,7 +3518,9 @@ no key into the household's account). Code: `internal/stacks/after_unclean_boot.
 - **Drive events** (`internal/web/media_events.go`, `/api/storage/media/{status,restart,wire}`): added / removed /
   returned are recorded on the drive; the storage page and the dashboard show the notice with „Újraindítás most"
   (stream check first) and „Később" (03:30), and per drive „restarted since". A returned drive recreates the file
-  browser at once; a dead or removed drive is not bound into it. **Safe removal** with media apps: pause that drive's
+  browser at once; a dead or removed drive is not bound into it. „Újraindítás most", „Később" and the safe removal
+  recreate every media app EXCEPT one the household stopped (R-721; it reads the new binds at its next start —
+  `MediaAppsToRecreate`, unreleased, next v0.343.0). **Safe removal** with media apps: pause that drive's
   torrents → recreate the apps and the file browser without it → unmount → keep it registered; its return needs no
   household step, and the torrents resume after the restart.
 - **Plex:** the claim code is an `after_start` field — asked on the Plex page once Plex runs (`POST /apps/plex/plex-claim`);

controller/internal/infra/filebrowser_sso.go

@@ -64,22 +64,36 @@ func LoadOrCreateFileBrowserSSOHeader(dataDir string) (string, error) {
 		return "", err
 	}
 	h := "X-Felhom-User-" + hex.EncodeToString(rb)
-	f, err := os.OpenFile(p, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600)
+	// The name file appears WHOLE or not at all (review round 2, 2026-10-24): it is written to a private temp file
+	// first and then hard-linked into place — the link is the exclusive create, so a second writer never reads an
+	// empty, half-written file (measured: 44 of 300 runs of the parallel-writers check read "" with the old
+	// create-then-write; pinned by TestFileBrowserSSOHeader_R1100_ABoxSecret).
+	tmp, err := os.CreateTemp(dataDir, fileBrowserSSOHeaderFile+".tmp-*")
 	if err != nil {
+		return "", err
+	}
+	defer os.Remove(tmp.Name())
+	if err := tmp.Chmod(0o600); err != nil {
+		tmp.Close()
+		return "", err
+	}
+	if _, err := tmp.WriteString(h + "\n"); err != nil {
+		tmp.Close()
+		return "", err
+	}
+	if err := tmp.Sync(); err != nil {
+		tmp.Close()
+		return "", err
+	}
+	if err := tmp.Close(); err != nil {
+		return "", err
+	}
+	if err := os.Link(tmp.Name(), p); err != nil {
 		if os.IsExist(err) {
 			return LoadOrCreateFileBrowserSSOHeader(dataDir)
 		}
 		return "", err
 	}
-	if _, err := f.WriteString(h + "\n"); err != nil {
-		f.Close()
-		_ = os.Remove(p)
-		return "", err
-	}
-	if err := f.Close(); err != nil {
-		_ = os.Remove(p)
-		return "", err
-	}
 	return h, nil
 }
 

controller/internal/stacks/media_library.go

@@ -283,6 +283,21 @@ func (m *Manager) MediaApps() []string {
 	return out
 }
 
+// MediaAppsToRecreate is MediaApps without the apps the household stopped (R-721, „a Stop holds"): the media restart
+// and the safe removal recreate with `up -d --force-recreate` (RestartStack), which would START a stopped app. Their
+// override is already written for the drives present now, so their next start sees them. `stopped` is returned for the
+// caller's log. Found by review round 2 (2026-10-24); pinned by TestMediaAppsToRecreate_SkipsAppsTheHouseholdStopped.
+func (m *Manager) MediaAppsToRecreate() (recreate, stopped []string) {
+	for _, name := range m.MediaApps() {
+		if m.WantsStopped(name) {
+			stopped = append(stopped, name)
+			continue
+		}
+		recreate = append(recreate, name)
+	}
+	return recreate, stopped
+}
+
 // MediaAppsNeedingRestart renders every media app's override for the drives present NOW and returns the apps whose
 // binds changed — the apps that must restart to see a drive that was added, removed or came back (the notice of
 // decision 238). It writes the files; the restart itself is the household's „restart now" (RestartStack).

controller/internal/stacks/media_library_test.go

@@ -187,3 +187,30 @@ func TestAfterStartField_NotAskedAtInstall(t *testing.T) {
 		t.Fatalf("the template shape must parse as required + after_start: %+v %v", meta.DeployFields, err)
 	}
 }
+
+// Review round 2 (2026-10-24, DOC-D2 U2): the media restart („Újraindítás most", „Később") and the safe removal
+// recreate every media app with `up -d --force-recreate`, which STARTED an app the household had stopped — against
+// R-721 („a Stop holds"). COMPANION RED-PROOF: return every MediaApps() entry from MediaAppsToRecreate → the stopped app
+// is in `recreate` and this test fails. Restored.
+func TestMediaAppsToRecreate_SkipsAppsTheHouseholdStopped(t *testing.T) {
+	for _, tc := range []struct {
+		desired      string
+		wantRecreate bool
+	}{{DesiredStateStopped, false}, {DesiredStateRunning, true}, {"", true}} {
+		m, _ := newInstalledManager(t, sonarrLike, "deployed: true\nenv: {}\ndesired_state: "+tc.desired+"\n")
+		dir := filepath.Dir(m.stacks["bookstack"].ComposePath)
+		if err := os.WriteFile(filepath.Join(dir, ".felhom.yml"), []byte("media_library:\n  read_only: true\n"), 0o644); err != nil {
+			t.Fatal(err)
+		}
+		if got := m.MediaApps(); len(got) != 1 {
+			t.Fatalf("fixture: want one media app, got %v", got)
+		}
+		recreate, stopped := m.MediaAppsToRecreate()
+		if tc.wantRecreate && (len(recreate) != 1 || len(stopped) != 0) {
+			t.Fatalf("desired %q: want recreated, got recreate=%v stopped=%v", tc.desired, recreate, stopped)
+		}
+		if !tc.wantRecreate && (len(recreate) != 0 || len(stopped) != 1) {
+			t.Fatalf("desired %q: a stopped app must NOT be recreated (it would start), got recreate=%v stopped=%v", tc.desired, recreate, stopped)
+		}
+	}
+}

controller/internal/web/media_events.go

@@ -50,6 +50,19 @@ func (s *Server) mediaApps() []string {
 	return s.stackMgr.MediaApps()
 }
 
+// mediaAppsToRecreate is the list the media restart and the safe removal recreate: every media app except the ones the
+// household stopped (R-721) — a recreate would start them; their next start reads the override already written.
+func (s *Server) mediaAppsToRecreate(why string) []string {
+	if s.stackMgr == nil {
+		return nil
+	}
+	recreate, stopped := s.stackMgr.MediaAppsToRecreate()
+	for _, name := range stopped {
+		s.logger.Printf("[INFO] [media] %s: NOT recreating %s — the household stopped it; it sees the drives at its next start", why, name)
+	}
+	return recreate
+}
+
 func (s *Server) presentMediaDrives() []mediawire.Drive {
 	if s.stackMgr == nil {
 		return nil
@@ -195,7 +208,7 @@ func (s *Server) MediaNotice(checkStale bool) MediaDriveNotice {
 // recreates the file browser, records the restart on every drive, then wires the present drives in.
 func (s *Server) restartMediaApps(reason string) []string {
 	var failed []string
-	for _, name := range s.mediaApps() {
+	for _, name := range s.mediaAppsToRecreate("restart (" + reason + ")") {
 		if s.appHeld(name) {
 			continue
 		}
@@ -304,7 +317,7 @@ func (s *Server) mediaSafeRemoval(path, durableID string) []string {
 	if err := s.settings.SetEjected(path, durableID, paused); err != nil {
 		s.logger.Printf("[WARN] [media] safe removal %s: recording it: %v", path, err)
 	}
-	for _, name := range s.mediaApps() {
+	for _, name := range s.mediaAppsToRecreate("safe removal " + path) {
 		if s.appHeld(name) {
 			continue
 		}

controller/internal/web/r2_media_recreate_wiring_test.go

@@ -0,0 +1,49 @@
+package web
+
+import (
+	"go/ast"
+	"go/parser"
+	"go/token"
+	"os"
+	"strings"
+	"testing"
+)
+
+// Review round 2 (2026-10-24, DOC-D2 U2): both web paths that recreate the media apps must take their list from
+// mediaAppsToRecreate (which leaves out the apps the household stopped, R-721), never from mediaApps(). A source pin:
+// the decision itself is tested in stacks (TestMediaAppsToRecreate_SkipsAppsTheHouseholdStopped); this pins the
+// WIRING, the half a seam test cannot see. COMPANION RED-PROOF: put `s.mediaApps()` back in either loop → this fails.
+func TestR2_MediaRecreateLoopsSkipStoppedApps(t *testing.T) {
+	src, err := os.ReadFile("media_events.go")
+	if err != nil {
+		t.Fatal(err)
+	}
+	fset := token.NewFileSet()
+	f, err := parser.ParseFile(fset, "media_events.go", src, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := map[string]bool{"restartMediaApps": false, "mediaSafeRemoval": false}
+	for _, d := range f.Decls {
+		fn, ok := d.(*ast.FuncDecl)
+		if !ok {
+			continue
+		}
+		if _, tracked := want[fn.Name.Name]; !tracked {
+			continue
+		}
+		body := string(src[fset.Position(fn.Body.Pos()).Offset:fset.Position(fn.Body.End()).Offset])
+		if strings.Contains(body, "range s.mediaApps()") {
+			t.Errorf("%s ranges over s.mediaApps(): it would recreate (start) an app the household stopped", fn.Name.Name)
+		}
+		if !strings.Contains(body, "s.mediaAppsToRecreate(") || !strings.Contains(body, "RestartStack(") {
+			t.Errorf("%s no longer recreates from s.mediaAppsToRecreate", fn.Name.Name)
+		}
+		want[fn.Name.Name] = true
+	}
+	for name, seen := range want {
+		if !seen {
+			t.Errorf("function %s not found in media_events.go — the pin is stale", name)
+		}
+	}
+}