chore(release): cut v0.13.0

CHANGELOG.md

@@ -6,6 +6,28 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+## [0.13.0] - 2026-09-09
+
+### Changed
+
+- `unless` is gone from the engine. The parser has refused the keyword for a while, but `engine.Shell` still carried the guard and the agent read it from a step's free-form arguments — reachable by a forged request, by nothing a plan can write. A capability with no way to express it is a trap (#619).
+
+### Fixed
+
+- `archive.extract` checks what its members hold, not only that they are there: their digests are recorded at extract time and compared after, so a file emptied in place is no longer reported converged. It also costs less — an observe no longer opens the archive, where listing it decompressed the whole thing every run (#614).
+
+- `postgres.config` doubles a single quote in the value, which is how postgres escapes one. Written raw it closed the quote early and the cluster refused the file — at startup, so the failure surfaced on the next restart rather than on the run that caused it. Verified against a real cluster, including that the value still converges (#618).
+
+- `sshd.config` checks the name it builds a path from, as `sudo.write` and `systemd.unit` already did. A name carrying a `/` wrote into a subdirectory sshd never reads — a drop-in believed installed that the server never sees. The def called itself the same shape as `sudo.write`, having copied its content check and not its name check (#617).
+
+- `==` refuses a value it cannot compare instead of crashing. Two shell results, or two outcomes, panicked the evaluator. The rule is now a whitelist — strings, ints and booleans compare, the rest is refused by name and says what to write instead. `if r == ok`, silently false before, is refused too (#616).
+
+- A dead `SSH_AUTH_SOCK` no longer discards a working inventory key. A socket outliving its agent failed the run over an agent the host never needed, so the same plan worked in one terminal and not in another. The agent is skipped and named in the trace; with no other method the error stands (#612).
+
+- `shellf status` exits non-zero when a host could not be reached. It asked a helper that only inspected block errors, so a sweep printing `unreachable` on every line still exited 0 — the worst answer for the command a monitor runs on a schedule. Drift stays a success: reporting what differs is what `status` is for (#615).
+
+- `archive.extract-member` extracts to a staged file and renames it. It redirected `tar` straight at the destination, so a member missing from the archive emptied the file that was there — usually an executable, since that is what this def installs. The destination's mode is carried over, which a rename would otherwise drop (#613).
+
 ## [0.12.0] - 2026-09-08
 
 ### Added
@@ -361,7 +383,8 @@ agent that evaluates on the host — "raw shell, but idempotent, previewable, fa
   per-user agent/workdir scoping.
 - Commands: `run`, `status`, `clean`, and `version`.
 
-[Unreleased]: https://github.com/haribo/shellf/compare/v0.12.0...HEAD
+[Unreleased]: https://github.com/haribo/shellf/compare/v0.13.0...HEAD
+[0.13.0]: https://github.com/haribo/shellf/compare/v0.12.0...v0.13.0
 [0.12.0]: https://github.com/haribo/shellf/compare/v0.11.0...v0.12.0
 [0.11.0]: https://github.com/haribo/shellf/compare/v0.10.0...v0.11.0
 [0.10.0]: https://github.com/haribo/shellf/compare/v0.9.1...v0.10.0

cmd/shellf/main.go

@@ -547,25 +547,18 @@ func statusCmd(args []string) {
 	// `status` refuses an unknown target like `run` does. The render stays pure — the
 	// exit code is the caller's call, so a report string keeps one job (#451).
 	reports := orchestrator.Run(plan, inv, self, "status", dial, base, secrets, defsSrc, orchestrator.Options{Parallel: *parallel, Limit: limits, ValidateArgs: validate})
+	// The verdict comes from the renderer that produced the report, as it does for `run`:
+	// asking a second function is how `status` came to exit 0 over a fleet where every host
+	// was unreachable (#615).
 	if *asJSON {
-		out, _ := report.JSON(reports)
+		out, anyErr := report.JSON(reports)
 		fmt.Print(report.RedactJSON(out, secretValues))
-		exitFor(anyBlockError(reports))
+		exitFor(anyErr)
 		return
 	}
-	fmt.Print(report.Redact(report.Status(reports), secretValues))
-	exitFor(anyBlockError(reports))
-}
-
-// anyBlockError reports whether any block failed as a whole (an unknown target), as
-// opposed to a per-host outcome.
-func anyBlockError(reports []orchestrator.BlockReport) bool {
-	for _, blk := range reports {
-		if blk.Err != nil {
-			return true
-		}
-	}
-	return false
+	text, anyErr := report.Status(reports)
+	fmt.Print(report.Redact(text, secretValues))
+	exitFor(anyErr)
 }
 
 // allUnknownTargets reports whether every block failed on an unknown target — the shape

cmd/shellf/main_test.go

@@ -10,7 +10,6 @@ import (
 	"strings"
 	"testing"
 
-	"shellf/internal/orchestrator"
 	"shellf/internal/project"
 )
 
@@ -285,37 +284,6 @@ func TestCheckParallel(t *testing.T) {
 // anyBlockError is what makes `status` exit non-zero on an unknown target (#451). It had
 // no test of its own — the behaviour was only covered end to end, where a change to it
 // would surface as a puzzling exit code rather than a failing assertion.
-// errFake is a minimal error for table cases. internal/report has its own since #491:
-// a test helper does not cross a package boundary.
-type errFake string
-
-func (e errFake) Error() string { return string(e) }
-
-func TestAnyBlockError(t *testing.T) {
-	none := []orchestrator.BlockReport{
-		{Target: "web", Hosts: []orchestrator.HostOutcome{{Host: "h1"}}},
-		{Target: "db"},
-	}
-	if anyBlockError(none) {
-		t.Fatal("no block failed as a whole")
-	}
-	// A per-host failure is not a block failure: the block ran, the host did not.
-	perHost := []orchestrator.BlockReport{{
-		Target: "web",
-		Hosts:  []orchestrator.HostOutcome{{Host: "h1", Err: errFake("unreachable")}},
-	}}
-	if anyBlockError(perHost) {
-		t.Fatal("a host error is not a block error")
-	}
-	blocked := []orchestrator.BlockReport{
-		{Target: "web", Hosts: []orchestrator.HostOutcome{{Host: "h1"}}},
-		{Target: "wbe", Err: &orchestrator.UnknownTargetError{Target: "wbe"}},
-	}
-	if !anyBlockError(blocked) {
-		t.Fatal("a block that could not run must be reported")
-	}
-}
-
 // `-v` must not undo what the report masks: the tracer is where redaction happens,
 // because the CLI is what knows the run's secrets (#461).
 func TestTracer_RedactsAndStaysOffStdout(t *testing.T) {

docs/design/orchestration.md

@@ -10,8 +10,8 @@ orchestration plan maps `def` calls onto the inventory.
 is a singleton group.
 
 ```
-on db  { postgres-install() }
-on web { nginx-install(); nginx-config() }
+on db  { db.postgres-install() }
+on web { web.nginx-install(); web.nginx-config() }
 ```
 
 The `def` inside stay neutral — `on` composes them, never the reverse.
@@ -22,7 +22,7 @@ The `def` inside stay neutral — `on` composes them, never the reverse.
 |---|---|
 | Between `on` blocks | **Sequential**, file order. `on db` completes on all its hosts before `on web` starts. |
 | Hosts within one block | **Parallel** (fan-out) — conflict-free, each host independent. |
-| `def` within one block, per host | **Sequential**. `nginx-install` then `nginx-config` on that host. |
+| `def` within one block, per host | **Sequential**. `web.nginx-install` then `web.nginx-config` on that host. |
 
 One SSH session per host carries the whole block's sequence (the agent is pushed
 once, not per `def`).
@@ -36,16 +36,16 @@ absence of conflict.
 ```
 on web {
   parallel {
-    nginx-install()
-    fetch-assets()
+    web.nginx-install()
+    web.fetch-assets()
   }
-  nginx-config()          // after both branches complete
+  web.nginx-config()      // after both branches complete
 }
 ```
 
 - **Result**: aggregate — `err` if any branch is `err`. Branches run to
   completion before the aggregate; no short-circuit.
-- **Halting**: an aggregate `err` halts the rest of the block (`nginx-config`
+- **Halting**: an aggregate `err` halts the rest of the block (`web.nginx-config`
   skipped), per the halting rule.
 - **Real speedup only without a shared exclusive resource.** shellf does start
   the branches together, but the system may re-serialize them: two `apt` runs

docs/language.md

@@ -350,8 +350,8 @@ So `if dir.exists("/opt/legacy") { … }` still resolves on a host where the dir
 **A `check` phase must not depend on state the plan itself produces.** That is a rule for writing defs, not something the model can enforce: `systemd.unit` shipped validating its content with `systemd-analyze verify`, which refuses a unit whose `ExecStart` is not yet on disk — so a plan delivering a script and the unit calling it could not be previewed. The fix belonged in the def.
 
 ```
-if dir.exists("/opt/app") {   // present → then, absent → else — deterministic even in --dry-run
-  apt.install("nginx")
+if dir.exists("/opt/app") {   // present → then; absent → the branch is previewed as
+  apt.install("nginx")        // undetermined, not taken as else (ADR-0051)
 }
 ```
 

internal/agent/agent.go

@@ -404,7 +404,9 @@ func agentErr(label string, err error) proto.StepResult {
 func dispatch(step proto.Step) (engine.Instruction, error) {
 	switch step.Instruction {
 	case "shell":
-		return engine.Shell{Cmd: step.Args["cmd"], Unless: step.Args["unless"], Env: engine.Env(step.Env)}, nil
+		// No `unless`: it was read from the step's arguments and no parser ever produced
+		// one, so the only way to set it was a hand-forged request (#619).
+		return engine.Shell{Cmd: step.Args["cmd"], Env: engine.Env(step.Env)}, nil
 	default:
 		return nil, fmt.Errorf("unknown instruction: %q", step.Instruction)
 	}

internal/agent/capture_test.go

@@ -6,12 +6,8 @@ import (
 	"shellf/internal/proto"
 )
 
-func capture(cmd, unless, bind string) proto.Step {
-	args := map[string]string{"cmd": cmd}
-	if unless != "" {
-		args["unless"] = unless
-	}
-	return proto.Step{Instruction: "shell", Args: args, Bind: bind}
+func capture(cmd, bind string) proto.Step {
+	return proto.Step{Instruction: "shell", Args: map[string]string{"cmd": cmd}, Bind: bind}
 }
 
 func ifRef(name, test string) proto.Step {
@@ -33,27 +29,39 @@ func TestAgentCapture_ChangedRunsThen(t *testing.T) {
 	f.set("doit", "", 0)
 	f.set("thencmd", "", 0)
 	serve(t, f, proto.Request{Mode: "apply", Steps: []proto.Step{
-		capture("doit", "", "x"),
+		capture("doit", "x"),
 		ifRef("x", "changed"),
 	}})
 	if !f.called("thencmd", "") {
 		t.Fatalf("x.changed true → then should run")
 	}
 }
 
+// A captured result that did not act must not fire `if x.changed { … }`. The producer is a
+// def that observes itself already converged: a raw `shell` always acts, so it cannot make
+// this state — it used to be made with an `unless` guard, which was removed in #619 because
+// no plan could write one.
+//
+// Both directions matter and both fail silently: losing the flag stops every
+// `if x.changed { restart }` downstream, inventing it fires them all for nothing.
 func TestAgentCapture_NotChangedSkipsThen(t *testing.T) {
-	// x = shell { doit } unless { guard-ok } → skipped → not changed → then skipped
 	f := newFake()
-	f.set("guardcmd", "", 0) // guard satisfied → shell skipped
-	serve(t, f, proto.Request{Mode: "apply", Steps: []proto.Step{
-		capture("doit", "guardcmd", "x"),
-		ifRef("x", "changed"),
-	}})
-	if f.called("doit", "") {
-		t.Fatalf("guard satisfied → shell must be skipped")
+	f.set(`test -f "$path"`, "", 0) // observe: already in the desired state
+	serve(t, f, proto.Request{
+		Mode: "apply",
+		Defs: map[string]string{
+			"converged": `def converged(path: str) { observe { return state(there: shell { test -f "$path" }.exit == 0) } apply { shell { touch "$path" } return ok.done } }`,
+		},
+		Steps: []proto.Step{
+			{Instruction: "converged", Args: map[string]string{"path": "/tmp/x"}, Bind: "x"},
+			ifRef("x", "changed"),
+		},
+	})
+	if f.called(`touch "$path"`, "") {
+		t.Fatal("the def was already converged → its apply must not run")
 	}
 	if f.called("thencmd", "") {
-		t.Fatalf("x.changed false → then must NOT run")
+		t.Fatal("x.changed false → then must NOT run")
 	}
 }
 
@@ -63,7 +71,7 @@ func TestAgentCapture_OkSugar(t *testing.T) {
 	f.set("doit", "", 0)
 	f.set("thencmd", "", 0)
 	serve(t, f, proto.Request{Mode: "apply", Steps: []proto.Step{
-		capture("doit", "", "x"),
+		capture("doit", "x"),
 		ifRef("x", "ok"),
 	}})
 	if !f.called("thencmd", "") {
@@ -72,7 +80,7 @@ func TestAgentCapture_OkSugar(t *testing.T) {
 }
 
 func caughtCap(cmd, bind string) proto.Step {
-	s := capture(cmd, "", bind)
+	s := capture(cmd, bind)
 	s.Caught = true
 	return s
 }
@@ -207,7 +215,7 @@ func TestAgentCapture_OutcomePattern(t *testing.T) {
 		return []proto.Step{{Instruction: "shell", Args: map[string]string{"cmd": cmd}}}
 	}
 	serve(t, f, proto.Request{Mode: "apply", Steps: []proto.Step{
-		capture("doit", "", "x"),
+		capture("doit", "x"),
 		{If: &proto.IfBlock{CondRef: &proto.ResultRef{Name: "x", Category: "ok"}, Then: then("yes")}},
 		{If: &proto.IfBlock{CondRef: &proto.ResultRef{Name: "x", Category: "err"}, Then: then("no")}},
 	}})

internal/engine/shell.go

@@ -2,15 +2,19 @@ package engine
 
 import "strings"
 
-// Shell runs a raw shell command on the target — the thesis's first-class
-// citizen. Idempotence and previewability come only from an optional `unless`
-// guard: without it the command always runs (and, like any raw shell, cannot be
-// previewed). The user is responsible for the guard, as in Chef `not_if` /
-// Puppet `unless`.
+// Shell runs a raw shell command on the target — the thesis's first-class citizen. It
+// always runs and, like any raw shell, cannot be previewed: a plan that needs a guard
+// writes `if !shell { <guard> } { shell { <cmd> } }`, which the language can express and
+// preview.
+//
+// It carried an `Unless` guard until #619. Nothing could set it: the parser refuses the
+// keyword by name (`internal/lang/parser.go`), #415 removed the def-side remnant, and the
+// only remaining writer read it out of a step's free-form arguments — reachable by a forged
+// request and by nothing a plan can produce. A capability with no way to express it is a
+// trap for the next reader of this file, not a feature.
 type Shell struct {
-	Cmd    string
-	Unless string // read-only guard; empty = no guard
-	Env    Env    // per-host variables, injected as $name (injection-safe) (#106)
+	Cmd string
+	Env Env // per-host variables, injected as $name (injection-safe) (#106)
 }
 
 func (s Shell) Name() string       { return "shell" }
@@ -24,17 +28,8 @@ func (s Shell) PreCheck() *Result {
 	return nil
 }
 
-// Guard: with an `unless`, exit 0 means the desired state already holds → skip.
-func (s Shell) Guard(ex Executor) *Result {
-	if s.Unless == "" {
-		return nil // no guard → always run
-	}
-	if ex.Shell(s.Unless, s.Env).OK() {
-		r := Ok("alreadySatisfied")
-		return &r
-	}
-	return nil
-}
+// Guard: a raw shell has none — it always runs (#619).
+func (s Shell) Guard(Executor) *Result { return nil }
 
 func (s Shell) Apply(ex Executor) Result {
 	r := ex.Shell(s.Cmd, s.Env)

internal/engine/shell_test.go

@@ -14,20 +14,9 @@ func TestShell_NoGuard_Runs(t *testing.T) {
 	}
 }
 
-func TestShell_UnlessSatisfied_Skips(t *testing.T) {
-	f := &fcFake{responses: map[string]ShellResult{netInspect: {Exit: 0}}} // guard satisfied
-	got := Run(Shell{Cmd: netCreate, Unless: netInspect}, f, Apply).String()
-	if got != "ok.alreadySatisfied" {
-		t.Fatalf("got %s, want ok.alreadySatisfied", got)
-	}
-	if f.calls[netCreate] {
-		t.Fatal("command ran despite the guard being satisfied")
-	}
-}
-
 func TestShell_Check_WouldNotMutate(t *testing.T) {
-	f := &fcFake{responses: map[string]ShellResult{netInspect: {Exit: 1}}} // guard not satisfied
-	got := Run(Shell{Cmd: netCreate, Unless: netInspect}, f, Check).String()
+	f := &fcFake{}
+	got := Run(Shell{Cmd: netCreate}, f, Check).String()
 	if got != "would.ran" {
 		t.Fatalf("got %s, want would.ran", got)
 	}
@@ -53,11 +42,3 @@ func TestShell_Apply_InjectsEnv(t *testing.T) {
 		t.Fatalf("Apply must pass Env to the executor (#106), got %+v", f.gotEnv)
 	}
 }
-
-func TestShell_Guard_InjectsEnv(t *testing.T) {
-	f := &envFake{result: ShellResult{Exit: 1}} // guard fails → command would run
-	Shell{Cmd: "act", Unless: "test -f $path", Env: Env{"path": "/tmp/x"}}.Guard(f)
-	if f.gotEnv["path"] != "/tmp/x" {
-		t.Fatalf("Guard must pass Env to the executor (#106), got %+v", f.gotEnv)
-	}
-}