R-621: a held app's logs show the log the hold kept

GetLogs (logs page, /api/stacks/<n>/logs, the operator's remote diagnostics) returned "" for a held
app — no containers — while captureHoldLogs had kept the app's own log under
hold-logs/<ts>/compose-logs.txt. It now serves the newest non-empty kept log, under a header naming
the file, when the live log is empty; a running app's own output always wins. GetLogs gains a test
seam (logsComposeFn) so the test never reaches docker.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0159rPz1ZhFKsS53msqPYxtS

controller/internal/stacks/manager.go

@@ -291,6 +291,8 @@ type Manager struct {
 	// --- guarded update (slice 4, update.go) ---
 	updateGuards    UpdateGuards // init-only, SetUpdateGuards; nil ⇒ every update is REFUSED
 	updateComposeFn func(dir string, env []string, args ...string) (string, error)
+	// logsComposeFn is GetLogs' compose seam (tests only; nil → the real compose). R-621.
+	logsComposeFn func(dir string, args ...string) (string, error)
 	// composeDownFn is RecoverInterruptedInstalls' seam (R-681): nil → the real `compose down`. Tests set a
 	// fake so they never reach real Docker (R-650).
 	composeDownFn  func(dir string) error
@@ -1448,11 +1450,26 @@ func (m *Manager) GetLogs(name string, lines int) (string, error) {
 	m.logger.Printf("[INFO] [stacks] Fetching logs for stack %s (tail=%d)", name, lines)
 
 	dir := filepath.Dir(stack.ComposePath)
-	output, err := m.composeExec(dir, "logs", "--tail", fmt.Sprintf("%d", lines), "--no-color")
+	run := m.composeExec
+	if m.logsComposeFn != nil {
+		run = m.logsComposeFn
+	}
+	output, err := run(dir, "logs", "--tail", fmt.Sprintf("%d", lines), "--no-color")
 	if err != nil {
 		m.logger.Printf("[WARN] [stacks] Failed to fetch logs for %s: %v", name, err)
 		return "", fmt.Errorf("getting logs for %s: %w", name, err)
 	}
+	// R-621: a HELD app has no containers, so its live log is empty — but the hold kept the app's own
+	// log before the `down` (captureHoldLogs). Serve the newest kept log then, under a header naming
+	// the file, so the logs page, the API and the operator's remote diagnostics all show WHY the
+	// update failed instead of nothing. Only when the live log is empty: a running app's own output
+	// always wins. Pinned by TestR621_GetLogsFallsBackToTheHoldLog.
+	if strings.TrimSpace(output) == "" {
+		if kept, ts, ok := latestHoldLog(dir); ok {
+			m.logger.Printf("[INFO] [stacks] Logs for %s: no live output — serving the log kept at the update hold (%s, %d bytes)", name, ts, len(kept))
+			return "--- hold-logs/" + ts + "/compose-logs.txt ---\n" + kept, nil
+		}
+	}
 
 	if len(output) == 0 {
 		m.logger.Printf("[DEBUG] Logs result for %s: 0 bytes returned (empty)", name)
@@ -1462,6 +1479,29 @@ func (m *Manager) GetLogs(name string, lines int) (string, error) {
 	return output, nil
 }
 
+// latestHoldLog returns the newest non-empty hold-logs/<ts>/compose-logs.txt under a stack dir. The
+// directory names are UTC timestamps (20060102T150405Z), so the lexical order is the time order.
+func latestHoldLog(dir string) (content, ts string, ok bool) {
+	entries, err := os.ReadDir(filepath.Join(dir, "hold-logs"))
+	if err != nil {
+		return "", "", false
+	}
+	names := make([]string, 0, len(entries))
+	for _, e := range entries {
+		if e.IsDir() {
+			names = append(names, e.Name())
+		}
+	}
+	sort.Sort(sort.Reverse(sort.StringSlice(names)))
+	for _, n := range names {
+		b, rerr := os.ReadFile(filepath.Join(dir, "hold-logs", n, "compose-logs.txt"))
+		if rerr == nil && strings.TrimSpace(string(b)) != "" {
+			return string(b), n, true
+		}
+	}
+	return "", "", false
+}
+
 // --- Env and compose helpers ---
 
 // stackEnv builds the full OS env slice for a stack, merging app.yaml values.

controller/internal/stacks/r621_hold_log_fallback_test.go

@@ -0,0 +1,53 @@
+package stacks
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// R-621 (the open half): the hold keeps the app's own log, and the logs a person can READ — the logs
+// page, /api/stacks/<n>/logs, the operator's remote diagnostics, all through GetLogs — now show it
+// when the held app has no live output. Before this, GetLogs returned "" for a held app (no
+// containers), so the kept file was reachable only by a shell on the box.
+func TestR621_GetLogsFallsBackToTheHoldLog(t *testing.T) {
+	m, dir := newPinManager(t, pinTplOld, pinTplNew, "deployed: true\nenv: {}\n")
+	live := ""
+	m.logsComposeFn = func(string, ...string) (string, error) { return live, nil }
+
+	// No hold log yet: an empty live log stays empty (nothing invented).
+	if got, err := m.GetLogs("nextcloud", 200); err != nil || got != "" {
+		t.Fatalf("no hold log: got %q, %v", got, err)
+	}
+
+	for ts, body := range map[string]string{
+		"20260921T211700Z": "web-1 | OLDER hold\n",
+		"20260922T080000Z": "web-1 | Applying migration 0009... OK\nweb-1 | never bound :8000\n",
+		"20260923T000000Z": "   \n", // a newer, EMPTY capture must not hide the useful one
+	} {
+		p := filepath.Join(dir, "hold-logs", ts)
+		if err := os.MkdirAll(p, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(p, "compose-logs.txt"), []byte(body), 0o644); err != nil {
+			t.Fatal(err)
+		}
+	}
+	got, err := m.GetLogs("nextcloud", 200)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(got, "never bound :8000") || !strings.HasPrefix(got, "--- hold-logs/20260922T080000Z/compose-logs.txt ---\n") {
+		t.Fatalf("R-621: a held app's logs must show the newest non-empty kept log under its header, got %q", got)
+	}
+	if strings.Contains(got, "OLDER hold") {
+		t.Errorf("only the newest kept log is served, got %q", got)
+	}
+
+	// A running app's own output always wins over a kept log.
+	live = "web-1 | serving\n"
+	if got, _ := m.GetLogs("nextcloud", 200); got != live {
+		t.Errorf("live output must win over the hold log, got %q", got)
+	}
+}