fix: honor explicit fallbacks and preserve finished shell calls

docs/changes/unreleased/2020-explicit-fallbacks-and-finished-calls.md

@@ -0,0 +1,22 @@
+---
+kind: fixed
+title: Explicit fallback lists hold, and finished shell calls keep background processes
+pr: 2020
+surface: [chat, engine]
+invalidates:
+  - "A cancellation watcher could kill background processes after their shell call had already finished. A completed call now takes precedence over a late cancellation."
+  - "A fallback list containing only the failing model could trigger catalog guesses. Only a list with no named model now allows a catalog suggestion."
+---
+
+When a shell call's completion and cancellation are both ready, the cancellation
+watcher checks completion before signalling the process group. Commands that are
+still running retain their existing cancellation behavior.
+
+A fallback list that names only the current model leaves no model to try after
+it. Duplicates and surrounding blanks keep that choice; a list of only blanks
+still allows a catalog suggestion.
+
+Carries forward santoshkumarradha's completed-call cancellation and explicit-list
+fixes and regressions from #1738. Pairing and moving chats already shipped
+separately. The draft's broader task/checker pin and skill import policies remain
+separate design decisions.

internal/exec/bare/promote.go

@@ -263,6 +263,13 @@ func watchCancel(ctx context.Context, call *BashCall, done <-chan struct{}) {
 	guard.Go("exec/bare cancel watch", func() {
 		select {
 		case <-ctx.Done():
+			// Both cases can be ready after a successful call returns. Its
+			// background processes no longer belong to this cancellation.
+			select {
+			case <-done:
+				return
+			default:
+			}
 			if _, _, adopted := call.close(); !adopted {
 				killProcessGroup(call.cmd)
 			}

internal/exec/bare/watchcancel_test.go

@@ -0,0 +1,42 @@
+//go:build !windows
+
+package bare
+
+import (
+	"context"
+	"os/exec"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/Agent-Field/codeaf/internal/processgroup"
+)
+
+// A call that has already returned is not killed by the cancellation that
+// follows it. Both of the watcher's cases are ready when the call's own context
+// is cancelled just behind its return; the group it left running must survive
+// whichever the watcher is handed.
+func TestACallThatReturnedLeavesItsBackgroundProcessAlone(t *testing.T) {
+	cmd := exec.Command("sleep", "30")
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	if err := cmd.Start(); err != nil {
+		t.Fatal(err)
+	}
+	pgid := cmd.Process.Pid
+	// The process is reaped as soon as it dies, so a killed group reads as gone.
+	reaped := make(chan struct{})
+	go func() { _ = cmd.Wait(); close(reaped) }()
+	t.Cleanup(func() { _ = syscall.Kill(-pgid, syscall.SIGKILL); <-reaped })
+
+	for i := 0; i < 200; i++ {
+		ctx, cancel := context.WithCancel(context.Background())
+		over := make(chan struct{})
+		cancel()
+		close(over)
+		watchCancel(ctx, newBashCall("sleep 30", cmd, nil, ctx), over)
+	}
+	time.Sleep(50 * time.Millisecond)
+	if !processgroup.Alive(pgid) {
+		t.Fatal("the cancellation after a finished call killed the group it left running")
+	}
+}

internal/manual/chat/models-and-cost.md

@@ -1,5 +1,13 @@
 # Models, context, and what it costs
 
+## A fallback list naming only the current model — keep the answer on that model
+
+The `fallback models` row is your list of other models a failed request may try.
+If it names only the current model, there is no other model to try: codeaf does
+not replace it with a suggestion from the catalog.
+Repeating the same model or adding blank rows does not change that choice.
+A list containing only blanks leaves the catalog free to suggest another model.
+
 ## Lost internet, Wi-Fi disconnected, DNS errors, and waiting for connection
 
 When DNS or a connection attempt fails before the request is accepted, codeaf

internal/manual/chat/running-from-the-terminal.md

@@ -1233,6 +1233,10 @@ own keypair, and `CODEAF_MODEL_POOL_RELAY_URL` with `models.pool.public_key`
 server, a watcher — one per line, tab separated: the name, the status, how long it has been
 up, what is watched for health, and where its log is.
 
+A shell command that has already finished keeps any background server or watcher
+it left running when the turn ends. Interrupting a command that is still running
+stops the command and its child processes.
+
 ```
 dev-server	running	2h	port:5173	/tmp/dev.log
 ```

internal/provider/endpoints.go

@@ -890,6 +890,18 @@ func (c *Client) FallbackModels(model string) []string {
 	return carried
 }
 
+// fallbacksWritten reports whether the configured list names any model. A list
+// naming only the failing model is still an explicit answer, even though that
+// model is removed from the chain; blank rows leave the catalog free to help.
+func (c *Client) fallbacksWritten() bool {
+	for _, model := range c.config.Fallbacks {
+		if strings.TrimSpace(model) != "" {
+			return true
+		}
+	}
+	return false
+}
+
 // fallbackChain is the models to try after the ladder, in order.
 //
 // The operator's own list wins outright, and the catalog is only asked when
@@ -913,7 +925,7 @@ func (c *Client) fallbackChain(model string) []string {
 		}
 	}
 	add(c.config.Fallbacks)
-	if len(chain) == 0 && c.config.NearestModels != nil {
+	if !c.fallbacksWritten() && c.config.NearestModels != nil {
 		add(c.config.NearestModels(model))
 	}
 	if len(chain) > maxFallbackModels {

internal/provider/fallback_self_test.go

@@ -0,0 +1,26 @@
+package provider
+
+import "testing"
+
+func TestAFallbackListNamingOnlyTheFailingModelIsAPin(t *testing.T) {
+	guessed := func(string) []string { return []string{"guessed/one", "guessed/two"} }
+	for name, written := range map[string][]string{
+		"the pinned model alone":    {"sim/model"},
+		"the pinned model repeated": {"sim/model", " SIM/model "},
+		"blank rows around the pin": {"", "sim/model"},
+	} {
+		client, _ := refusingClient(t, func(map[string]any) bool { return false }, Config{
+			Fallbacks: written, NearestModels: guessed,
+		})
+		if got := client.FallbackModels("sim/model"); len(got) != 0 {
+			t.Fatalf("%s: FallbackModels = %v, want none: a pin sends no call to another model", name, got)
+		}
+	}
+	// A row of blanks is no answer at all, and the catalog still helps.
+	client, _ := refusingClient(t, func(map[string]any) bool { return false }, Config{
+		Fallbacks: []string{" ", ""}, NearestModels: guessed,
+	})
+	if got := client.FallbackModels("sim/model"); len(got) != 2 {
+		t.Fatalf("FallbackModels = %v, want the catalog's two for a row that said nothing", got)
+	}
+}