Tag built images with the commit's short hash (closes #239)

Builds are tagged upaas-<app>:<short hash>, git's short form of
the commit checked out, instead of the deployment number. The clone
prints the short hash and fails without one.

A redeploy of a commit gives the tag to the new image. The cleanup
after a deploy now also finds the app's untagged images among those
its deployments recorded, and removes them by ID once the app neither
runs them nor would roll back to them. It keeps every image any app
uses, since apps that build the same commit can share one.

commit_sha is now saved on update so manual deploys keep the commit
read from the clone.

Model: opus-5-5

TODO.md

@@ -20,6 +20,11 @@ regress.
 
 # Completed Steps
 
+- 2026-10-01: The hint under the app page's environment variable editor now says
+  changes take effect at the next deploy or rollback, in the page's warning
+  style, instead of asking for a container restart, which keeps the old values
+  (#255).
+
 - 2026-10-01: An app with no volume mounts says on its page, and in the log of
   each deploy, that a deploy or rollback loses the files it writes and a restart
   keeps them; the README's Volume mounts section says a deploy or rollback keeps

internal/docker/validation_test.go

@@ -223,18 +223,7 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 
 			c := &Client{docker: dockerAPI, log: slog.Default()}
 
-			dir := t.TempDir()
+			_, _ = c.performClone(ctx, testCloneConfig(t))
-			cfg := &cloneConfig{
-				repoURL:       "git@example.com:repo.git",
-				branch:        mainBranch,
-				sshPrivateKey: "fake-key",
-				containerDir:  filepath.Join(dir, "repo"),
-				hostDir:       filepath.Join(dir, "repo"),
-				keyFile:       filepath.Join(dir, "deploy_key"),
-				hostKeyFile:   filepath.Join(dir, "deploy_key"),
-			}
-
-			_, _ = c.performClone(ctx, cfg)
 
 			select {
 			case query := <-removeQuery:
@@ -248,6 +237,24 @@ func TestPerformCloneRemovesContainerVolumes(t *testing.T) {
 	}
 }
 
+// testCloneConfig returns the settings of a clone in these tests, with its
+// files in a temporary directory.
+func testCloneConfig(t *testing.T) *cloneConfig {
+	t.Helper()
+
+	dir := t.TempDir()
+
+	return &cloneConfig{
+		repoURL:       "git@example.com:repo.git",
+		branch:        mainBranch,
+		sshPrivateKey: "fake-key",
+		containerDir:  filepath.Join(dir, "repo"),
+		hostDir:       filepath.Join(dir, "repo"),
+		keyFile:       filepath.Join(dir, "deploy_key"),
+		hostKeyFile:   filepath.Join(dir, "deploy_key"),
+	}
+}
+
 // cloneCommit is the commit the fake clones in these tests check out.
 const cloneCommit = "1a2b3c4d5e6f7a8b9c0d1e2f3a4b5c6d7e8f9a0b"
 
@@ -549,18 +556,7 @@ func TestPerformCloneReadsFramedLogs(t *testing.T) {
 
 	c := &Client{docker: dockerAPI, log: slog.Default()}
 
-	dir := t.TempDir()
+	result, err := c.performClone(t.Context(), testCloneConfig(t))
-	cfg := &cloneConfig{
-		repoURL:       "git@example.com:repo.git",
-		branch:        mainBranch,
-		sshPrivateKey: "fake-key",
-		containerDir:  filepath.Join(dir, "repo"),
-		hostDir:       filepath.Join(dir, "repo"),
-		keyFile:       filepath.Join(dir, "deploy_key"),
-		hostKeyFile:   filepath.Join(dir, "deploy_key"),
-	}
-
-	result, err := c.performClone(t.Context(), cfg)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -574,3 +570,43 @@ func TestPerformCloneReadsFramedLogs(t *testing.T) {
 		t.Errorf("got commit %q, short %q", result.CommitSHA, result.ShortSHA)
 	}
 }
+
+// TestPerformCloneFailsWithoutShortSHA runs a clone against a fake Docker API
+// whose clone succeeds but prints no "SHORT_SHA:" line, and checks that the
+// clone fails, since the short hash names the image the deploy builds.
+func TestPerformCloneFailsWithoutShortSHA(t *testing.T) {
+	t.Parallel()
+
+	srv := httptest.NewServer(http.HandlerFunc(
+		func(w http.ResponseWriter, r *http.Request) {
+			w.Header().Set("Content-Type", "application/json")
+
+			switch {
+			case strings.HasSuffix(r.URL.Path, "/containers/create"):
+				_, _ = w.Write([]byte(`{"Id":"gitcontainer"}`))
+			case strings.HasSuffix(r.URL.Path, "/wait"):
+				_, _ = w.Write([]byte(`{"StatusCode":0}`))
+			case strings.HasSuffix(r.URL.Path, "/logs"):
+				_, _ = stdcopy.NewStdWriter(w, stdcopy.Stdout).
+					Write([]byte("COMMIT:" + cloneCommit + "\n"))
+			default:
+				_, _ = w.Write([]byte(`{}`))
+			}
+		},
+	))
+	t.Cleanup(srv.Close)
+
+	dockerAPI, err := client.NewClientWithOpts(
+		client.WithHost("tcp://" + srv.Listener.Addr().String()),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	c := &Client{docker: dockerAPI, log: slog.Default()}
+
+	_, err = c.performClone(t.Context(), testCloneConfig(t))
+	if !errors.Is(err, ErrGitCloneFailed) {
+		t.Errorf("got error %v, want %v", err, ErrGitCloneFailed)
+	}
+}

internal/handlers/app_page_env_var_hint_test.go

@@ -0,0 +1,39 @@
+package handlers_test
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"sneak.berlin/go/upaas/internal/service/app"
+)
+
+// TestAppPageEnvVarHint checks that the environment variable editor says
+// changes take effect at the next deploy or rollback, in the warning style.
+func TestAppPageEnvVarHint(t *testing.T) {
+	t.Parallel()
+
+	testCtx := setupTestHandlers(t)
+
+	createdApp, err := testCtx.appSvc.CreateApp(t.Context(), app.CreateAppInput{
+		Name:    "env-hint-app",
+		RepoURL: "git@example.com:user/env-hint-app.git",
+	})
+	require.NoError(t, err)
+
+	request := httptest.NewRequestWithContext(
+		t.Context(), http.MethodGet, "/apps/"+createdApp.ID, nil,
+	)
+	request = addChiURLParams(request, map[string]string{"id": createdApp.ID})
+	recorder := httptest.NewRecorder()
+
+	testCtx.handlers.HandleAppDetail().ServeHTTP(recorder, request)
+
+	require.Equal(t, http.StatusOK, recorder.Code)
+	assert.Contains(t, recorder.Body.String(),
+		`<p class="alert-warning mt-1">`+
+			"Environment variable changes take effect at the next deploy or rollback.</p>")
+}

internal/models/models_test.go

@@ -564,6 +564,33 @@ func TestDeploymentMarkFinished(t *testing.T) {
 	assert.True(t, found.FinishedAt.Valid)
 }
 
+// TestDeploymentSaveStoresCommitSetAfterInsert checks that a commit set on a
+// deployment after it was first saved, as a manual deploy does once the clone
+// reads it, is stored by the next save.
+func TestDeploymentSaveStoresCommitSetAfterInsert(t *testing.T) {
+	t.Parallel()
+
+	testDB, cleanup := setupTestDB(t)
+	defer cleanup()
+
+	app := createTestApp(t, testDB)
+
+	deployment := models.NewDeployment(testDB)
+	deployment.AppID = app.ID
+
+	err := deployment.Save(context.Background())
+	require.NoError(t, err)
+
+	deployment.CommitSHA = sql.NullString{String: "abc123def456", Valid: true}
+
+	err = deployment.Save(context.Background())
+	require.NoError(t, err)
+
+	found, err := models.FindDeployment(context.Background(), testDB, deployment.ID)
+	require.NoError(t, err)
+	assert.Equal(t, "abc123def456", found.CommitSHA.String)
+}
+
 func TestDeploymentFindByAppID(t *testing.T) {
 	t.Parallel()
 

templates/app_detail.html

@@ -178,7 +178,7 @@
                                     <button type="submit" class="btn-primary text-sm">Save</button>
                                     <button type="button" @click="editIdx = -1" class="text-gray-500 hover:text-gray-700 text-sm">Cancel</button>
                                 </form>
-                                <p class="text-xs text-amber-600 mt-1">⚠ Container restart needed after env var changes.</p>
+                                <p class="alert-warning mt-1">Environment variable changes take effect at the next deploy or rollback.</p>
                             </td>
                         </template>
                     </tr>