fix(sharedproxy,plugin,templates): lock across detach+down, leave retry, source-fallback refusal, tables.yaml only, err-to-zero (D-0929, D-0931, D-0938, D-1415, D-0917, D-0061)

.github/wiki/Multi-App-Isolation.md

@@ -9,7 +9,7 @@ nself init --template multi-app . --force   # overlay the template files
 
 The overlay replaces the `.env.example` and `nself.yml` that `nself init` wrote with the template's own. Keep your own values in `.env` and `.env.local`.
 
-`--dry-run` lists what it would write: `README.md`, `manifest.json`, `nself.yml`, `.env.example`, `migrations/001_multi_app.sql`, `hasura/metadata.json` and `hasura/metadata/databases/default/tables/tables.yaml` (the layout `nself db reconcile` reads; both hold the same tables). No plugins are required, and there is no UI starter.
+`--dry-run` lists what it would write: `README.md`, `manifest.json`, `nself.yml`, `.env.example`, `migrations/001_multi_app.sql` and `hasura/metadata/databases/default/tables/tables.yaml` (the layout `nself db reconcile` reads). No plugins are required, and there is no UI starter.
 
 ## The model
 

.github/wiki/Shared-Proxy.md

@@ -61,8 +61,8 @@ A fronted project publishes no port 80 or 443 and runs no Nginx of its own. The
 |---|---|
 | `nself start` in a fronted project | After the stack is healthy: connect the owner's Nginx to `<project>_network` (no change when already connected), run `nginx -t`, then `nginx -s reload`. |
 | `nself start` in the owner | Does the same for every registration whose network exists. A registered project that is stopped gives one warning per project; the owner still starts. |
-| `nself stop`, `restart`, `start --fresh`, `reset`, `uninstall` in a fronted project | Before compose takes the project down, the owner's Nginx is disconnected from `<project>_network`. Compose cannot remove a network while a foreign container is attached, so without this step `nself stop` would fail. |
-| `nself uninstall` in a fronted project | Also removes the project's files and registration from the owner and reloads the owner's Nginx. If that cannot be done (for example a registration cannot be read, `nginx/shared/quarantine` or a certificate directory is a symlink, or the owner's Nginx cannot be looked up), uninstall returns `E506` and does not report the files as removed. |
+| `nself stop`, `restart`, `start --fresh`, `reset`, `uninstall` in a fronted project | Before compose takes the project down, the owner's Nginx is disconnected from `<project>_network`. Compose cannot remove a network while a foreign container is attached, so without this step `nself stop` would fail. The owner lock is held from the disconnect until compose has finished, so an owner `nself start` cannot reattach the network in between. |
+| `nself uninstall` in a fronted project | Also removes the project's files and registration from the owner and reloads the owner's Nginx. If that cannot be done (for example a registration cannot be read, `nginx/shared/quarantine` or a certificate directory is a symlink, or the owner's Nginx cannot be looked up), uninstall returns `E506` and does not report the files as removed. If only the reload failed, the debt is recorded in `nginx/shared/.reload-pending`; run `nself uninstall` again or `nself start` in the owner and the reload is retried. |
 
 A fronted `nself start` returns `E506` when the registrations under the owner cannot be read (one invalid registration file of any project is enough), because its domains would not be served. The owner's own start only warns. Before a compose down, the same condition is a warning that names the cause; the down continues.
 
@@ -72,7 +72,7 @@ If the owner's Nginx is not running, the fronted project starts and stops anyway
 
 ### Restarting the owner's Nginx
 
-A restarted or recreated Nginx container loses its extra networks. Run `nself start` in the owner afterwards: it reattaches every registered, running project.
+A restarted or recreated Nginx container loses its extra networks. `nself restart nginx` in the owner reattaches every registered, running project once the container is back; for a container you recreated by other means, run `nself start` in the owner. If a start attach fails after it stopped a crash-looping Nginx, the owner's Nginx is started again.
 
 ### A failed `nginx -t` (E506)
 

cmd/commands/debt_d0353_test.go

@@ -107,7 +107,7 @@ func TestD0353ComposeCascadeDoesNotChangeHash(t *testing.T) {
 			}
 			_ = os.Setenv("D0353_VALUE", "dev-conflict")
 			if operation == "restart" {
-				err = restartServices(context.Background(), comp, dir, []string{"hasura"}, false)
+				err = restartServices(context.Background(), comp, dir, nil, []string{"hasura"}, false)
 			} else {
 				err = startRemainingServicesPhase(context.Background(), comp, dir, cfg)
 			}

cmd/commands/restart.go

@@ -140,7 +140,7 @@ func runRestartFlow(cmd *cobra.Command, args []string) error {
 		}
 	} else if len(services) > 0 {
 		// Specific services: restart only those
-		if err := restartServices(ctx, comp, workdir, services, verbose); err != nil {
+		if err := restartServices(ctx, comp, workdir, cfg, services, verbose); err != nil {
 			return err
 		}
 	} else if smart {

cmd/commands/restart_strategies.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"slices"
 	"time"
 
 	"github.com/nself-org/cli/internal/build"
@@ -76,7 +77,7 @@ func restartFull(ctx context.Context, comp *docker.Compose, workdir string, cfg
 }
 
 // restartServices restarts only the specified services.
-func restartServices(ctx context.Context, comp *docker.Compose, workdir string, services []string, verbose bool) error {
+func restartServices(ctx context.Context, comp *docker.Compose, workdir string, cfg *config.Config, services []string, verbose bool) error {
 	if verbose {
 		ui.Info(fmt.Sprintf("Restarting services: %v", services))
 	}
@@ -101,6 +102,11 @@ func restartServices(ctx context.Context, comp *docker.Compose, workdir string,
 	}
 	ui.Success(fmt.Sprintf("Restarted: %v", services))
 
+	// A recreated nginx container loses the networks the shared proxy attached
+	// to it (D-0931): attach again, as `nself start` does.
+	if slices.Contains(services, "nginx") {
+		return afterStartSharedProxy(ctx, cfg, workdir)
+	}
 	return nil
 }
 

cmd/commands/start_shared_proxy.go

@@ -24,6 +24,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"path/filepath"
 	"slices"
 
 	"github.com/nself-org/cli/internal/config"
@@ -36,8 +37,9 @@ import (
 var (
 	spLocate = sharedproxy.Locate
 	spAttach = sharedproxy.Attach
-	spDetach = sharedproxy.Detach
+	spDetach = sharedproxy.DetachHolding
 	spLeave  = sharedproxy.Leave
+	spFlush  = sharedproxy.ReloadIfPending
 )
 
 func init() {
@@ -61,6 +63,7 @@ func afterStartSharedProxy(ctx context.Context, cfg *config.Config, workdir stri
 	}
 	switch site.Role {
 	case sharedproxy.RoleOwner:
+		flushPendingReload(ctx, workdir)
 		attachAllRegistered(ctx, workdir, site)
 		return nil
 	case sharedproxy.RoleFronted:
@@ -72,6 +75,14 @@ func afterStartSharedProxy(ctx context.Context, cfg *config.Config, workdir stri
 	return nil
 }
 
+// flushPendingReload pays the owner reload that a fronted project's failed
+// uninstall left owed (D-0931). A failure is a warning: the owner still starts.
+func flushPendingReload(ctx context.Context, ownerDir string) {
+	if _, err := spFlush(ctx, ownerDir); err != nil {
+		ui.Warn(fmt.Sprintf("Shared proxy: a removed project's files may still be served; the owner's nginx could not be reloaded: %v", err))
+	}
+}
+
 func attachAllRegistered(ctx context.Context, ownerDir string, site sharedproxy.Site) {
 	if len(site.Set.Projects) == 0 {
 		return
@@ -111,22 +122,26 @@ func warnOutcome(out sharedproxy.Outcome, err error) {
 
 // beforeStopSharedProxy is the compose pre-down hook: a fronted project
 // disconnects the owner's nginx from its network before its network is removed.
-func beforeStopSharedProxy(ctx context.Context, workdir string) {
+// The owner lock is held until the returned function runs after the down, so a
+// concurrent owner start cannot reattach the network in between (D-0929).
+func beforeStopSharedProxy(ctx context.Context, workdir string) func() {
 	site, err := spLocate(workdir)
 	if err != nil {
 		ui.Warn(fmt.Sprintf("Shared proxy: registrations are not readable, so the owner's nginx was not disconnected and the network may stay in use: %v", err))
-		return
+		return nil
 	}
 	if site.Role != sharedproxy.RoleFronted {
-		return
+		return nil
 	}
-	switch err := spDetach(ctx, site.OwnerDir, *site.Reg); {
+	release, err := spDetach(ctx, site.OwnerDir, *site.Reg)
+	switch {
 	case err == nil:
 	case errors.Is(err, sharedproxy.ErrOwnerNotRunning):
 		ui.Warn("Shared proxy: the owner's nginx is not running; nothing to disconnect")
 	default:
 		ui.Warn(fmt.Sprintf("Shared proxy: %v", err))
 	}
+	return release
 }
 
 // leaveSharedProxy is `nself uninstall` in a fronted project (review R2b): it
@@ -140,6 +155,14 @@ func leaveSharedProxy(ctx context.Context, workdir string) error {
 		return errs.New("E506", "cannot leave the shared proxy: its registrations are not readable").WithWhy(err.Error())
 	}
 	if site.Role != sharedproxy.RoleFronted {
+		// A retry after a failed reload: the registration is already gone, but
+		// the parent owner may still owe a reload (D-0931).
+		if parent := filepath.Dir(workdir); sharedproxy.ReloadPending(parent) {
+			if _, err := spFlush(ctx, parent); err != nil {
+				ui.Warn(fmt.Sprintf("Shared proxy: reloading %s: %v", parent, err))
+				return fmt.Errorf("reloading the shared proxy at %s: %w", parent, err)
+			}
+		}
 		return nil
 	}
 	if _, err := spLeave(ctx, site.OwnerDir, site.Reg.Project); err != nil {

cmd/commands/start_shared_proxy_pending_test.go

@@ -0,0 +1,108 @@
+package commands
+
+// Tests for D-0931 in start_shared_proxy.go and restart_strategies.go: a
+// targeted `restart nginx` reattaches, the owner's start pays a reload a
+// failed uninstall left owed, and an uninstall retry pays it too.
+
+import (
+	"context"
+	"errors"
+	"os"
+	"path/filepath"
+	"runtime"
+	"strings"
+	"testing"
+
+	"github.com/nself-org/cli/internal/docker"
+	"github.com/nself-org/cli/internal/sharedproxy"
+)
+
+func stubFlush(t *testing.T, err error) *[]string {
+	t.Helper()
+	var calls []string
+	orig := spFlush
+	t.Cleanup(func() { spFlush = orig })
+	spFlush = func(_ context.Context, ownerDir string) (bool, error) {
+		calls = append(calls, ownerDir)
+		return true, err
+	}
+	return &calls
+}
+
+func TestRestartServicesReattachesNginx(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("fake docker is a POSIX shell script")
+	}
+	ctx := context.Background()
+	owner, _, _ := proxyLayout(t)
+	bin := t.TempDir()
+	if err := os.WriteFile(filepath.Join(bin, "docker"), []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+	comp := &docker.Compose{DockerPath: filepath.Join(bin, "docker")}
+	stubFlush(t, nil)
+
+	calls := stubAttach(t, sharedproxy.Outcome{}, nil)
+	if err := restartServices(ctx, comp, owner, nil, []string{"hasura"}, false); err != nil || len(*calls) != 0 {
+		t.Fatalf("restarting another service must not attach: err %v calls %v", err, *calls)
+	}
+	if err := restartServices(ctx, comp, owner, nil, []string{"nginx"}, false); err != nil {
+		t.Fatal(err)
+	}
+	if len(*calls) != 1 || (*calls)[0].owner != owner || strings.Join((*calls)[0].projects, ",") != "b,c" {
+		t.Errorf("restart nginx must reattach every registration, got %+v", *calls)
+	}
+}
+
+func TestOwnerStartPaysAPendingReload(t *testing.T) {
+	ctx := context.Background()
+	owner, _, _ := proxyLayout(t)
+	stubAttach(t, sharedproxy.Outcome{}, nil)
+	flushed := stubFlush(t, nil)
+	if err := afterStartSharedProxy(ctx, nil, owner); err != nil {
+		t.Fatal(err)
+	}
+	if len(*flushed) != 1 || (*flushed)[0] != owner {
+		t.Errorf("the owner's start must flush the owed reload, got %v", *flushed)
+	}
+	// A failed flush warns; the start still succeeds.
+	stubFlush(t, errors.New("boom"))
+	out := captureStderr(t, func() { _ = afterStartSharedProxy(ctx, nil, owner) })
+	if !strings.Contains(out, "could not be reloaded") {
+		t.Errorf("want a warning, got %q", out)
+	}
+}
+
+func TestUninstallRetryPaysAPendingReload(t *testing.T) {
+	ctx := context.Background()
+	owner, b, _ := proxyLayout(t)
+	pending := filepath.Join(sharedproxy.DirOf(owner), sharedproxy.ReloadPendingFile)
+
+	// b's registration is gone, as after a leave whose reload failed.
+	if err := os.Remove(filepath.Join(sharedproxy.DirOf(owner), sharedproxy.FileName(b.Project))); err != nil {
+		t.Fatal(err)
+	}
+	t.Run("nothing owed", func(t *testing.T) {
+		flushed := stubFlush(t, nil)
+		if err := leaveSharedProxy(ctx, b.ProjectDir); err != nil || len(*flushed) != 0 {
+			t.Fatalf("err %v flushed %v", err, *flushed)
+		}
+	})
+	if err := os.WriteFile(pending, nil, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	t.Run("owed and paid", func(t *testing.T) {
+		flushed := stubFlush(t, nil)
+		if err := leaveSharedProxy(ctx, b.ProjectDir); err != nil || len(*flushed) != 1 || (*flushed)[0] != owner {
+			t.Fatalf("err %v flushed %v", err, *flushed)
+		}
+	})
+	t.Run("owed and still failing", func(t *testing.T) {
+		stubFlush(t, errors.New("reload boom"))
+		var err error
+		captureStderr(t, func() { err = leaveSharedProxy(ctx, b.ProjectDir) })
+		if err == nil {
+			t.Fatal("a retry that cannot reload must report the incomplete removal")
+		}
+	})
+}

cmd/commands/start_shared_proxy_test.go

@@ -149,16 +149,24 @@ func TestSharedProxyDetachOnDown(t *testing.T) {
 
 	var detached []string
 	var detachErr error
+	held := 0 // locks taken by the stub and not yet released
 	orig := spDetach
 	t.Cleanup(func() { spDetach = orig })
-	spDetach = func(_ context.Context, ownerDir string, regs ...sharedproxy.Registration) error {
+	spDetach = func(_ context.Context, ownerDir string, regs ...sharedproxy.Registration) (func(), error) {
 		if data, _ := os.ReadFile(order); len(data) != 0 {
 			t.Errorf("detach ran after docker: %q", data)
 		}
 		for _, r := range regs {
 			detached = append(detached, ownerDir+"|"+r.Project)
 		}
-		return detachErr
+		held++
+		return func() {
+			held--
+			// D-0929: the lock outlives the compose down.
+			if data, _ := os.ReadFile(order); !strings.HasPrefix(string(data), "docker compose down") {
+				t.Errorf("the owner lock was released before docker down ran: %q", data)
+			}
+		}, detachErr
 	}
 
 	// Every compose-down path of a fronted project detaches first.
@@ -174,6 +182,9 @@ func TestSharedProxyDetachOnDown(t *testing.T) {
 		if data, _ := os.ReadFile(order); !strings.HasPrefix(string(data), "docker compose down") {
 			t.Errorf("docker down did not run after the detach: %q", data)
 		}
+		if held != 0 {
+			t.Errorf("down %+v left the owner lock held (%d)", opts, held)
+		}
 	}
 
 	// The owner's own down, and a project outside any proxy, detach nothing.
@@ -195,6 +206,9 @@ func TestSharedProxyDetachOnDown(t *testing.T) {
 		if err := comp.ComposeDown(context.Background(), b.ProjectDir, docker.DownOptions{}); err != nil {
 			t.Errorf("detach error %v must not fail the down: %v", e, err)
 		}
+		if held != 0 {
+			t.Errorf("detach error %v left the owner lock held (%d)", e, held)
+		}
 	}
 }
 

cmd/commands/start_shared_proxy_unreadable_test.go

@@ -64,8 +64,11 @@ func TestUnreadableRegistrations(t *testing.T) {
 		detached := false
 		orig := spDetach
 		t.Cleanup(func() { spDetach = orig })
-		spDetach = func(context.Context, string, ...sharedproxy.Registration) error { detached = true; return nil }
-		out := captureStderr(t, func() { beforeStopSharedProxy(ctx, b.ProjectDir) })
+		spDetach = func(context.Context, string, ...sharedproxy.Registration) (func(), error) {
+			detached = true
+			return func() {}, nil
+		}
+		out := captureStderr(t, func() { _ = beforeStopSharedProxy(ctx, b.ProjectDir) })
 		if detached || !strings.Contains(out, "not readable") || !strings.Contains(out, "zzz.json") {
 			t.Fatalf("detached=%v, stderr %q: want a warning naming the unreadable file", detached, out)
 		}

internal/docker/compose.go

@@ -74,7 +74,9 @@ func (c *Compose) ComposeUp(ctx context.Context, workdir string, services ...str
 
 // ComposeDown runs `docker compose down` with the given options in the given workdir.
 func (c *Compose) ComposeDown(ctx context.Context, workdir string, opts DownOptions) error {
-	RunPreDownHooks(ctx, workdir) // the shared proxy detaches here (compose_hooks.go)
+	// The shared proxy detaches here and keeps its lock until the down is done
+	// (compose_hooks.go), so an owner attach cannot slip in between.
+	defer RunPreDownHooks(ctx, workdir)()
 	args := c.buildBaseArgs()
 	args = append(args, "down")
 	if opts.RemoveVolumes {

internal/docker/compose_hooks.go

@@ -11,15 +11,18 @@ package docker
 // Outputs: RegisterPreDownHook returns the function that unregisters the hook.
 // Constraints: a hook cannot fail the down (it reports through its own
 // channel); hooks run in registration order, once per ComposeDown call, before
-// docker is invoked; safe for concurrent use.
+// docker is invoked; safe for concurrent use. A hook may return a release
+// function: ComposeDown calls it after docker returns, so a hook can hold a
+// lock across its own work and the down (D-0929). Releases run in reverse order.
 
 import (
 	"context"
 	"sync"
 )
 
-// PreDownHook runs before `docker compose down` in workdir.
-type PreDownHook func(ctx context.Context, workdir string)
+// PreDownHook runs before `docker compose down` in workdir. The function it
+// returns, if not nil, runs once the down has finished.
+type PreDownHook func(ctx context.Context, workdir string) (release func())
 
 type preDownEntry struct {
 	id   int
@@ -52,15 +55,24 @@ func RegisterPreDownHook(h PreDownHook) (unregister func()) {
 	}
 }
 
-// RunPreDownHooks runs every registered hook for workdir.
-func RunPreDownHooks(ctx context.Context, workdir string) {
+// RunPreDownHooks runs every registered hook for workdir and returns a
+// function that releases whatever they hold. Call it once the down is done.
+func RunPreDownHooks(ctx context.Context, workdir string) (release func()) {
 	preDownMu.Lock()
 	hooks := make([]PreDownHook, 0, len(preDownHooks))
 	for _, e := range preDownHooks {
 		hooks = append(hooks, e.hook)
 	}
 	preDownMu.Unlock()
+	var releases []func()
 	for _, h := range hooks {
-		h(ctx, workdir)
+		if r := h(ctx, workdir); r != nil {
+			releases = append(releases, r)
+		}
+	}
+	return func() {
+		for i := len(releases) - 1; i >= 0; i-- {
+			releases[i]()
+		}
 	}
 }