fix(sync): 120s deadline on git subprocesses — hung remote no longer wedges sync (campaign F3)

runGitInDir had no context: a hanging remote parked the sync goroutine in
cmd.Run(), the doSync defer never ran, `syncing` stayed true, and every manual +
periodic sync was refused with "Szinkronizálás már folyamatban" until a
controller restart. Each git command now runs under exec.CommandContext with a
fresh per-command gitCmdTimeout (120s); the deadline error names the timeout and
the (masked) git args. Debounce and failed-sync-arms-debounce unchanged.

Tests: T-C1 cancelled-context kills the subprocess promptly (red-proof: pre-fix
exec.Command shape runs to completion → test FAILS); T-C2 failed sync releases
`syncing` and a post-debounce retry EXECUTES.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PSK5g6qYLknKj8u3QAFEr6

controller/internal/sync/sync.go

@@ -2,6 +2,7 @@ package sync
 
 import (
 	"bytes"
+	"context"
 	"crypto/sha256"
 	"encoding/hex"
 	"fmt"
@@ -18,6 +19,13 @@ import (
 	"gitea.dooplex.hu/admin/felhom-controller/internal/config"
 )
 
+// gitCmdTimeout bounds each individual git subprocess (campaign finding F3, 2026-07-06):
+// without a deadline, a hung remote parks the sync goroutine inside cmd.Run() — the doSync
+// defer never runs, `syncing` stays true, and every manual + periodic sync is refused with
+// "Szinkronizálás már folyamatban" until a controller restart. 120s is generous for the
+// shallow catalog clone/fetch.
+const gitCmdTimeout = 120 * time.Second
+
 // Syncer handles periodic git sync of the app catalog to the local stacks directory.
 type Syncer struct {
 	cfg          *config.Config
@@ -270,10 +278,10 @@ func (s *Syncer) gitCloneOrPull() error {
 	if s.isDebug() {
 		s.logger.Printf("[DEBUG] [sync] git fetch --depth 1 origin %s in %s", s.cfg.Git.Branch, s.cacheDir)
 	}
-	if err := s.runGitInDir(s.cacheDir, "fetch", "--depth", "1", "origin", s.cfg.Git.Branch); err != nil {
+	if err := s.gitCmd(s.cacheDir, "fetch", "--depth", "1", "origin", s.cfg.Git.Branch); err != nil {
 		return fmt.Errorf("git fetch: %w", err)
 	}
-	if err := s.runGitInDir(s.cacheDir, "reset", "--hard", "origin/"+s.cfg.Git.Branch); err != nil {
+	if err := s.gitCmd(s.cacheDir, "reset", "--hard", "origin/"+s.cfg.Git.Branch); err != nil {
 		return fmt.Errorf("git reset: %w", err)
 	}
 	return nil
@@ -414,14 +422,22 @@ func copyIfChanged(src, dst string) (bool, error) {
 	return true, nil
 }
 
-// runGit executes a git command with the given args.
+// runGit executes a git command with the given args under the standard per-command deadline.
 func (s *Syncer) runGit(args ...string) error {
-	return s.runGitInDir("", args...)
+	return s.gitCmd("", args...)
 }
 
-// runGitInDir executes a git command in the specified directory.
-func (s *Syncer) runGitInDir(dir string, args ...string) error {
-	cmd := exec.Command("git", args...)
+// gitCmd runs one git command in dir under a fresh gitCmdTimeout deadline (per-command,
+// not per-sync — a multi-step sync gets a full budget for each subprocess).
+func (s *Syncer) gitCmd(dir string, args ...string) error {
+	ctx, cancel := context.WithTimeout(context.Background(), gitCmdTimeout)
+	defer cancel()
+	return s.runGitInDir(ctx, dir, args...)
+}
+
+// runGitInDir executes a git command in the specified directory, killed when ctx expires.
+func (s *Syncer) runGitInDir(ctx context.Context, dir string, args ...string) error {
+	cmd := exec.CommandContext(ctx, "git", args...)
 	if dir != "" {
 		cmd.Dir = dir
 	}
@@ -433,7 +449,13 @@ func (s *Syncer) runGitInDir(dir string, args ...string) error {
 	s.logger.Printf("[DEBUG] [sync] Running: git %s", maskRepoURL(strings.Join(args, " ")))
 
 	if err := cmd.Run(); err != nil {
-		return fmt.Errorf("git %s: %w\nstderr: %s", strings.Join(args, " "), err, stderr.String())
+		if ctxErr := ctx.Err(); ctxErr != nil {
+			// Subprocess was killed by the deadline/cancellation — name it explicitly so the
+			// failure is diagnosable from SyncResult/logs (never a silent hang).
+			return fmt.Errorf("git %s: %v (subprocess killed at the %s deadline): %w\nstderr: %s",
+				maskRepoURL(strings.Join(args, " ")), ctxErr, gitCmdTimeout, err, stderr.String())
+		}
+		return fmt.Errorf("git %s: %w\nstderr: %s", maskRepoURL(strings.Join(args, " ")), err, stderr.String())
 	}
 	return nil
 }

controller/internal/sync/sync_test.go

@@ -0,0 +1,105 @@
+package sync
+
+import (
+	"context"
+	"log"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"gitea.dooplex.hu/admin/felhom-controller/internal/config"
+)
+
+func newTestSyncer(t *testing.T, repoURL string) *Syncer {
+	t.Helper()
+	dir := t.TempDir()
+	cfg := &config.Config{}
+	cfg.Git.RepoURL = repoURL
+	cfg.Git.Branch = "main"
+	cfg.Git.SyncInterval = "15m"
+	cfg.Paths.DataDir = filepath.Join(dir, "data")
+	cfg.Paths.StacksDir = filepath.Join(dir, "stacks")
+	if err := os.MkdirAll(cfg.Paths.StacksDir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	logger := log.New(os.Stderr, "", 0)
+	return New(cfg, logger, func() error { return nil }, nil)
+}
+
+// T-C1 (campaign F3): a git subprocess must die with its context — the pre-fix
+// exec.Command shape ignored the context entirely, so a hung remote parked the sync
+// goroutine in cmd.Run() forever, holding `syncing=true` until a controller restart.
+// Effect asserted: the returned error NAMES the context cause (not a generic git error),
+// and the call returns promptly instead of running the command to completion.
+func TestRunGitInDir_CancelledContextKillsSubprocess(t *testing.T) {
+	s := newTestSyncer(t, "https://example.invalid/repo.git")
+
+	ctx, cancel := context.WithCancel(context.Background())
+	cancel() // already cancelled — the subprocess must not be allowed to run to completion
+
+	start := time.Now()
+	err := s.runGitInDir(ctx, "", "--version")
+	elapsed := time.Since(start)
+
+	if err == nil {
+		t.Fatalf("expected an error from a cancelled context, got nil (git --version ran to completion — the pre-fix behavior)")
+	}
+	if !strings.Contains(err.Error(), context.Canceled.Error()) {
+		t.Fatalf("error must name the context cause; got: %v", err)
+	}
+	if !strings.Contains(err.Error(), "deadline") && !strings.Contains(err.Error(), "killed") {
+		t.Fatalf("error must say the subprocess was killed at the deadline; got: %v", err)
+	}
+	if elapsed > 5*time.Second {
+		t.Fatalf("cancelled-context git call took %v — subprocess not killed promptly", elapsed)
+	}
+}
+
+// T-C2 (campaign F3): a failed sync must release the single-flight `syncing` flag and a
+// later TriggerSync (past the debounce) must EXECUTE — the campaign symptom was every
+// subsequent sync refused with "Szinkronizálás már folyamatban". Uses a nonexistent local
+// repo_url so git fails fast with no network.
+func TestTriggerSync_FailureReleasesSyncingAndAllowsRetry(t *testing.T) {
+	s := newTestSyncer(t, filepath.Join(t.TempDir(), "no-such-repo"))
+
+	res := s.TriggerSync()
+	if res.OK {
+		t.Fatalf("sync against a nonexistent repo must fail; got OK with message %q", res.Message)
+	}
+	if !strings.Contains(res.Message, "Git hiba") {
+		t.Fatalf("failure must carry the git error to the caller (no silent swallow); got %q", res.Message)
+	}
+
+	st := s.Status()
+	if st.Syncing {
+		t.Fatalf("`syncing` still true after a failed sync — the single-flight flag leaked (the F3 lockup)")
+	}
+	if st.LastStatus != "error" || st.LastError == "" {
+		t.Fatalf("failed sync must be recorded: LastStatus=%q LastError=%q", st.LastStatus, st.LastError)
+	}
+
+	// Debounce window: an immediate retry is refused (existing behavior, unchanged).
+	res2 := s.TriggerSync()
+	if res2.OK || !strings.Contains(res2.Message, "Túl gyakori") {
+		t.Fatalf("immediate retry should hit the 30s debounce; got OK=%v message=%q", res2.OK, res2.Message)
+	}
+
+	// Past the debounce (rewind lastSync directly — same-package seam), the retry must
+	// EXECUTE: it reaches git again and fails with the git error, not a refusal message.
+	s.mu.Lock()
+	s.lastSync = time.Now().Add(-time.Minute)
+	s.mu.Unlock()
+
+	res3 := s.TriggerSync()
+	if res3.OK {
+		t.Fatalf("retry against the same nonexistent repo should still fail: %q", res3.Message)
+	}
+	if !strings.Contains(res3.Message, "Git hiba") {
+		t.Fatalf("retry past the debounce must EXECUTE (git error expected); got refusal/other: %q", res3.Message)
+	}
+	if s.Status().Syncing {
+		t.Fatal("`syncing` leaked after the retry")
+	}
+}