fix: show why deploys fail; 409 instead of silent no-ops while a deploy runs; retry skipped reconciles

docs/architecture/deployer.md

@@ -23,6 +23,12 @@ Each app's compose project is named `simpledeploy-<app-slug>`. This isolates app
 
 The whole sequence is wrapped in a tracker that the API surfaces as live deploy logs over WebSocket and as a "deploying" flag on the app.
 
+## Failure messages
+
+When a tracked `docker compose` command (deploy, rollback, restart, pull) exits non-zero, `Err` is a `*ComposeError` whose `Detail` says why: the last few stderr lines (at most 3, 1000 bytes, keeping the end) that are not progress (`Container x  Started`, `web Pulling`, layer downloads) or warnings, with repeats dropped (a failed pull prints the same error twice). Compose prints its error last, so this is normally the single line that names the problem. `Error()` reads `compose up: <detail>: exit status 1`; that string is what the `deploy_failed` / `rollback` audit row, the activity feed, and the logs show. The full output stays in the deploy log and the `deploy_events` row.
+
+Registry credentials reach docker only through the temporary config file (never args or stdin), so compose output does not contain them. As defense in depth, each registry password and its base64 `user:password` form are replaced with `[redacted]` in `Detail` and in the stored output. Detail and output are visible to the same users who can open the app's deploy log.
+
 ## Compose check before `up`
 
 `Deploy`, `RollbackDeploy`, `Pull`, `Scale` and `Cancel` (which runs a follow-up `up`) all call `checkComposeFile` before running `docker compose`. It re-reads `docker-compose.yml` from disk with `compose.ParseFile`, which interpolates `${VARS}` from the app's `.env` and the server environment the way `docker compose` does and refuses a symlinked compose file or `.env`, then runs `compose.ValidateComposeSecurity`. A file changed after the caller parsed it (git sync, version restore, manual edit) is therefore still checked right before it runs. `Restart` (`compose restart`) and `Stop`/`Start` do not create containers and skip the check.

docs/architecture/metrics.md

@@ -45,4 +45,4 @@ A separate code path in the proxy package (the `simpledeploy_metrics` Caddy modu
 
 ## Pruning
 
-Pruning runs nightly. Manual prune is exposed at `POST /api/system/prune/{metrics,request-stats}` for emergencies.
+Pruning runs nightly. Manual prune is exposed at `POST /api/system/prune/{metrics,request-stats}` for emergencies (super admin). The body is `{"tier": "raw", "days": 30}`: `tier` is one of `raw`, `1m`, `5m`, `1h`, `1d` (default `raw`) and `days` defaults to 30. An unknown tier or an unreadable body returns `400` and deletes nothing.

docs/architecture/reconciler.md

@@ -23,6 +23,8 @@ There is no internal periodic tick. If you want a heartbeat sweep, restart the p
 
 Steps 3 are run as goroutines under a `chan struct{}` semaphore of capacity **3**, so at most 3 deploys execute concurrently. Step 4 runs serially after the wait.
 
+An app whose deploy is already running (an API deploy, rollback, restart or pull) is skipped in step 3, since that deploy may have read an older file. One `Reconcile()` is queued to run when it finishes (at most one per app); the hash check makes it a no-op when nothing changed.
+
 ## Hash detection
 
 Migration 008 added `apps.compose_hash` as a `TEXT` column. After every successful deploy, `deployApp()` writes the SHA-256 of the compose file. On the next reconcile, file hash is recomputed and compared. Match -> skip. Mismatch -> deploy. Failure to read -> log and skip. Hashing reads through `fsutil.ReadRegularFile`, so a symlinked file never hashes.
@@ -57,7 +59,7 @@ After every deploy (success or fail), `apps.status` is updated to `running` or `
 
 ## Compose versions
 
-Migration 009 also added `compose_versions`. After every successful deploy, the current compose file content is inserted with the same hash; the 10 most recent per app are kept. The UI's "Versions" tab lists these, and `RollbackOne(slug, versionID)` writes the saved content back to disk and triggers a redeploy. The version must belong to the app (`ErrVersionNotFound` otherwise), and it is checked with the app's current `.env` against today's rules before it replaces the file, so a refused rollback leaves the current file in place.
+Migration 009 also added `compose_versions`. After every successful deploy, the current compose file content is inserted with the same hash; the 10 most recent per app are kept. The UI's "Versions" tab lists these, and `RollbackOne(slug, versionID)` writes the saved content back to disk and triggers a redeploy. The version must belong to the app (`ErrVersionNotFound` otherwise), and it is checked with the app's current `.env` against today's rules before it replaces the file, so a refused rollback leaves the current file in place. While another tracked operation (deploy, rollback, restart, pull) of the app runs, `RollbackOne` returns `deployer.ErrDeployInProgress` without touching the file (the API answers `409`); if one starts between that check and the deployer's own, the previous file is written back. `RestartOne` and `PullOne` return the same error when skipped, and their API handlers check first and answer `409` instead of a `202` that would do nothing.
 
 ## Registry credential resolution
 

docs/operations/security-audit.md

@@ -24,7 +24,7 @@ The activity log is a persistent, queryable record of everything that changes in
 | `webhook` | Webhooks created/changed/removed |
 | `registry` | Registry credentials added/removed |
 | `access` | User app-access grants and revocations |
-| `deploy` | Deploy succeeded, deploy failed (with error), rollback |
+| `deploy` | Deploy succeeded, deploy failed (with the reason docker compose gave), rollback (or why it failed) |
 | `lifecycle` | App created, stopped, started, restarted, scaled, removed |
 | `auth` | Login success/failure, password change (global only) |
 | `system` | User CRUD, API key CRUD, public-host change, git sync config, retention settings (global only) |

docs/operations/troubleshooting.md

@@ -25,6 +25,20 @@ simpledeploy registry list
 - Network issue: confirm DNS works (`getent hosts registry-1.docker.io`) and outbound 443 is open.
 - Rate limited by Docker Hub: log in to a paid account or use a registry mirror.
 
+## Deploy failed
+
+**Symptom:** The app shows `error` and the activity feed says `Deploy failed: compose up: <reason>: exit status 1` (or `Rollback to version N failed: ...`).
+
+**What it means:** `docker compose up` itself stopped with an error. `<reason>` is the last lines docker compose printed, so it usually names the problem. The full output is in the deploy log and the app's deploy history.
+
+**Common reasons:**
+- `pull access denied` or `unauthorized`: the image name or tag is wrong, or the image is private. Fix the `image:` line or add the registry under **Settings -> Registries**.
+- `port is already allocated`: another app or program uses that host port. Pick a different host port in `ports:`.
+- `manifest unknown` or `not found`: the tag does not exist.
+- `required variable X is missing a value`: add `X` in the **Config** tab.
+
+Fix the cause and redeploy.
+
 ## Deploy reported "Unstable"
 
 **Symptom:** The deploy wizard shows a yellow "Unstable" status. The per-service summary lists one or more services as `restarting`, `unhealthy`, or with `restarted Nx`.

docs/reference/api.md

@@ -82,13 +82,15 @@ Deploy an app by uploading a compose file. `name` becomes the slug and the compo
 // Request
 {"name": "myapp", "compose": "<base64-encoded compose file>"}
 
-// Response 201
-{"name": "myapp", "status": "deployed"}
+// Response 202 (deploy runs in the background)
+{"name": "myapp", "status": "started"}
 
 // Response 400 (compose file fails security checks)
 {"error": "compose file contains disallowed directives", "violations": ["service \"web\": privileged mode not allowed"]}
 ```
 
+A compose file saved while the app is already deploying (or restarting, pulling, rolling back) is deployed as soon as that finishes.
+
 Requires `super_admin`. Rollback and version restore check the stored version with the app's current `.env`: one that fails the checks or does not load gets the same `400` `{error, violations}` body, and one using a domain another app now has gets `409`.
 
 ### `DELETE /api/apps/{slug}`
@@ -139,7 +141,7 @@ Stop, start, scale and rollback wait for `docker compose` to finish before respo
 
 ### `POST /api/apps/{slug}/restart`
 
-Restart the app's existing containers in place (`docker compose restart`). Containers are not recreated, so compose changes and newer images are not picked up (use pull or a deploy for that). Runs in the background and returns `202` with `{"status": "started"}`.
+Restart the app's existing containers in place (`docker compose restart`). Containers are not recreated, so compose changes and newer images are not picked up (use pull or a deploy for that). Runs in the background and returns `202` with `{"status": "started"}`. `409` while a deploy, rollback, restart or pull of the app is still running; try again when it finishes.
 
 ### `POST /api/apps/{slug}/stop`
 
@@ -151,7 +153,7 @@ Start stopped containers. Returns `{"status": "ok"}`.
 
 ### `POST /api/apps/{slug}/pull`
 
-Pull latest images and redeploy. Uses configured registry auth if available. Runs in the background and returns `202` with `{"status": "started"}`.
+Pull latest images and redeploy. Uses configured registry auth if available. Runs in the background and returns `202` with `{"status": "started"}`. `409` while another deploy, rollback, restart or pull of the app is running.
 
 ### `POST /api/apps/{slug}/scale`
 
@@ -197,7 +199,7 @@ Rollback to a previous compose version.
 {"status": "ok"}
 ```
 
-`400` with `{error, violations}` when the version fails today's checks or does not load; `409` when one of its domains now belongs to another app.
+`400` with `{error, violations}` when the version fails today's checks or does not load; `409` when one of its domains now belongs to another app, or while a deploy, restart or pull of the app is running (nothing changes; try again when it finishes). When `docker compose` fails, `500` with `rollback failed: compose up: <reason>: exit status 1`.
 
 ### `GET /api/apps/{slug}/events`
 

internal/api/actions.go

@@ -14,6 +14,7 @@ import (
 
 	"github.com/vazra/simpledeploy/internal/audit"
 	"github.com/vazra/simpledeploy/internal/compose"
+	"github.com/vazra/simpledeploy/internal/deployer"
 	"github.com/vazra/simpledeploy/internal/store"
 )
 
@@ -41,12 +42,26 @@ func detachedCtx(r *http.Request) (context.Context, context.CancelFunc) {
 	return context.WithTimeout(context.WithoutCancel(r.Context()), composeOpTimeout)
 }
 
+// refuseWhileDeploying answers 409 when a deploy of slug is running. Restart
+// and pull run in the background, where the deployer would skip them
+// silently after the client already got 202.
+func refuseWhileDeploying(w http.ResponseWriter, rec reconciler, slug string) bool {
+	if !rec.IsDeploying(slug) {
+		return false
+	}
+	http.Error(w, deployer.ErrDeployInProgress.Error(), http.StatusConflict)
+	return true
+}
+
 func (s *Server) handleRestart(w http.ResponseWriter, r *http.Request) {
 	slug := r.PathValue("slug")
 	if s.reconciler == nil {
 		http.Error(w, "reconciler not configured", http.StatusInternalServerError)
 		return
 	}
+	if refuseWhileDeploying(w, s.reconciler, slug) {
+		return
+	}
 	ctx := r.Context()
 	go func() {
 		if err := s.reconciler.RestartOne(context.Background(), slug); err != nil {
@@ -117,6 +132,9 @@ func (s *Server) handlePull(w http.ResponseWriter, r *http.Request) {
 		http.Error(w, "reconciler not configured", http.StatusInternalServerError)
 		return
 	}
+	if refuseWhileDeploying(w, s.reconciler, slug) {
+		return
+	}
 	go func() {
 		if err := s.reconciler.PullOne(context.Background(), slug); err != nil {
 			fmt.Fprintf(os.Stderr, "pull %s: %v\n", slug, err)
@@ -376,6 +394,18 @@ func (s *Server) handleRollback(w http.ResponseWriter, r *http.Request) {
 			http.Error(w, dotEnvProblem, http.StatusBadRequest)
 			return
 		}
+		if errors.Is(err, deployer.ErrDeployInProgress) {
+			http.Error(w, err.Error(), http.StatusConflict)
+			return
+		}
+		// docker compose ran and failed: say why (secrets are already
+		// removed; the app's deploy log shows the same output).
+		var ce *deployer.ComposeError
+		if errors.As(err, &ce) {
+			log.Printf("[api] rollback %s: %v", slug, err)
+			http.Error(w, "rollback failed: "+ce.Error(), http.StatusInternalServerError)
+			return
+		}
 		httpError(w, err, http.StatusInternalServerError)
 		return
 	}

internal/api/actions_busy_test.go

@@ -0,0 +1,97 @@
+package api
+
+import (
+	"bytes"
+	"context"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/vazra/simpledeploy/internal/deployer"
+)
+
+// busyReconciler has a deploy in flight for every app.
+type busyReconciler struct {
+	mockReconcilerFull
+	rollbackErr error
+}
+
+func (b *busyReconciler) IsDeploying(string) bool { return true }
+func (b *busyReconciler) RollbackOne(ctx context.Context, slug string, id int64) error {
+	b.calls = append(b.calls, fmt.Sprintf("RollbackOne:%s:%d", slug, id))
+	return b.rollbackErr
+}
+
+func postAction(t *testing.T, srv *Server, path string, body any) *httptest.ResponseRecorder {
+	t.Helper()
+	var b []byte
+	if body != nil {
+		b, _ = json.Marshal(body)
+	}
+	req := httptest.NewRequest(http.MethodPost, path, bytes.NewReader(b))
+	req.Header.Set("Content-Type", "application/json")
+	req.AddCookie(superAdminCookie(t, srv.jwt))
+	w := httptest.NewRecorder()
+	srv.Handler().ServeHTTP(w, req)
+	return w
+}
+
+func TestRollbackWhileDeployingReturns409(t *testing.T) {
+	srv, _ := newTestServer(t)
+	srv.SetAppsDir(t.TempDir())
+	srv.SetReconciler(&busyReconciler{rollbackErr: deployer.ErrDeployInProgress})
+	verID := seedVersion(t, srv, "myapp")
+
+	w := postAction(t, srv, "/api/apps/myapp/rollback", map[string]any{"version_id": verID})
+	if w.Code != http.StatusConflict {
+		t.Fatalf("status = %d, want 409; body: %s", w.Code, w.Body.String())
+	}
+	if !strings.Contains(w.Body.String(), "a deploy is already running for this app") {
+		t.Errorf("body = %q, want the busy message", w.Body.String())
+	}
+}
+
+func TestRollbackComposeFailureSaysWhy(t *testing.T) {
+	srv, _ := newTestServer(t)
+	srv.SetAppsDir(t.TempDir())
+	ce := &deployer.ComposeError{Op: "compose up", Detail: "Bind for 127.0.0.1:8080 failed: port is already allocated", Err: errors.New("exit status 1")}
+	rec := &busyReconciler{rollbackErr: fmt.Errorf("redeploy: %w", ce)}
+	srv.SetReconciler(&notBusy{rec})
+	verID := seedVersion(t, srv, "myapp")
+
+	w := postAction(t, srv, "/api/apps/myapp/rollback", map[string]any{"version_id": verID})
+	if w.Code != http.StatusInternalServerError {
+		t.Fatalf("status = %d, want 500; body: %s", w.Code, w.Body.String())
+	}
+	if !strings.Contains(w.Body.String(), "port is already allocated") {
+		t.Errorf("body = %q, want the compose error", w.Body.String())
+	}
+}
+
+type notBusy struct{ *busyReconciler }
+
+func (notBusy) IsDeploying(string) bool { return false }
+
+func TestRestartAndPullWhileDeployingReturn409(t *testing.T) {
+	for _, action := range []string{"restart", "pull"} {
+		srv, _ := newTestServer(t)
+		srv.SetAppsDir(t.TempDir())
+		rec := &busyReconciler{}
+		srv.SetReconciler(rec)
+
+		w := postAction(t, srv, "/api/apps/myapp/"+action, nil)
+		if w.Code != http.StatusConflict {
+			t.Fatalf("%s: status = %d, want 409; body: %s", action, w.Code, w.Body.String())
+		}
+		if !strings.Contains(w.Body.String(), "a deploy is already running for this app") {
+			t.Errorf("%s: body = %q, want the busy message", action, w.Body.String())
+		}
+		if len(rec.calls) != 0 {
+			t.Errorf("%s: calls = %v, want none", action, rec.calls)
+		}
+	}
+}

internal/api/system.go

@@ -4,13 +4,16 @@ import (
 	"context"
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
+	"io"
 	"net"
 	"net/http"
 	"os"
 	"path/filepath"
 	"regexp"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 	"syscall"
@@ -77,8 +80,12 @@ type appSummary struct {
 
 type pruneRequest struct {
 	Days int    `json:"days"`
-	Tier string `json:"tier"` // raw, 1m, 5m, 1h — empty defaults to "raw"
+	Tier string `json:"tier"` // one of pruneTiers; empty defaults to "raw"
 }
+
+// pruneTiers are the tiers a manual prune accepts.
+var pruneTiers = []string{"raw", "1m", "5m", "1h", "1d"}
+
 type pruneResponse struct {
 	Deleted int64  `json:"deleted"`
 	Message string `json:"message"`
@@ -208,21 +215,32 @@ func (s *Server) handleStorageBreakdown(w http.ResponseWriter, r *http.Request)
 	json.NewEncoder(w).Encode(storageBreakdownResponse{Metrics: metrics, RequestStats: rs})
 }
 
-func parsePruneRequest(r *http.Request) pruneRequest {
+// parsePruneRequest reads a prune body. An empty body or tier means raw
+// and 30 days. An unknown tier is an error, so a typo never prunes another
+// tier.
+func parsePruneRequest(r *http.Request) (pruneRequest, error) {
 	var req pruneRequest
-	json.NewDecoder(r.Body).Decode(&req)
+	if err := json.NewDecoder(r.Body).Decode(&req); err != nil && !errors.Is(err, io.EOF) {
+		return req, errors.New("invalid request body")
+	}
 	if req.Days <= 0 {
 		req.Days = 30
 	}
-	validTiers := map[string]bool{"raw": true, "1m": true, "5m": true, "1h": true, "1d": true}
-	if !validTiers[req.Tier] {
+	if req.Tier == "" {
 		req.Tier = "raw"
 	}
-	return req
+	if !slices.Contains(pruneTiers, req.Tier) {
+		return req, fmt.Errorf("unknown tier %q: use one of %s", req.Tier, strings.Join(pruneTiers, ", "))
+	}
+	return req, nil
 }
 
 func (s *Server) handlePruneMetrics(w http.ResponseWriter, r *http.Request) {
-	req := parsePruneRequest(r)
+	req, err := parsePruneRequest(r)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
 	cutoff := time.Now().AddDate(0, 0, -req.Days)
 	n, err := s.store.PruneMetrics(req.Tier, cutoff)
 	if err != nil {
@@ -234,7 +252,11 @@ func (s *Server) handlePruneMetrics(w http.ResponseWriter, r *http.Request) {
 }
 
 func (s *Server) handlePruneRequestMetrics(w http.ResponseWriter, r *http.Request) {
-	req := parsePruneRequest(r)
+	req, err := parsePruneRequest(r)
+	if err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
 	cutoff := time.Now().AddDate(0, 0, -req.Days)
 	n, err := s.store.PruneRequestMetrics(req.Tier, cutoff)
 	if err != nil {