refactor(adapter): typed FileOp.Action and an explicit OpCleanup kind (#230)

CHANGELOG.md

@@ -242,6 +242,22 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
   same snapshot. The ways the four surfaces disagreed (mode-only drift,
   symlinked destinations) are resolved in the same release; see Fixed above.
 
+- **Internal: `adapter.FileOp` carries a typed `Action` and an explicit op
+  `Kind`** ([#230](https://github.com/spxrogers/agentsync/issues/230)).
+  `Action` was a string with a `"" == "write"` convention normalized at three
+  pipeline intakes and re-asserted by a comment at every reader; it is now an
+  enum whose **zero value is write**, so the normalization and its comment tax
+  are gone. Orphan-cleanup ops — the empty key-merge write that prunes an
+  emptied section's owned keys — are stamped `OpCleanup` at synthesis (by the
+  single constructor `adapter.NewCleanupOp`, guarded by an AST test against
+  hand-rolled cleanup literals) instead of being detected by three copies of a
+  `{}`+`OwnedKeys` shape sniff. No user-visible behaviour changes: the
+  `apply --dry-run` labels, the `removed: N key(s), M file(s)` headline and
+  every `--json` payload are byte-identical (`FileOp` is never serialized).
+  `MergeStrategy` stays a plain string — typing it changes the published
+  `Adapter` interface and is deferred to
+  [#250](https://github.com/spxrogers/agentsync/issues/250).
+
 - **`.state/targets.json` is now `schema_version: 2`.** The upgrade is automatic
   and requires nothing: every command reads the old keys, and the first command
   that WRITES state (`apply`, `import`, `reconcile`, `migrate`, `agent disable

docs/architecture.md

@@ -171,6 +171,25 @@ runtime (e.g. a scope-gap branch) cannot hide from it. `TestEveryAdapterClassifi
 adapter at both scopes, fails on any unset `Kind`, and pins that both kind values
 are exercised.
 
+**`FileOp` is typed too.** `Action` (`adapter.Action`) is `ActionWrite` or
+`ActionDelete`, and **`ActionWrite` is the zero value** — a `FileOp` built
+without naming an action writes. There is no empty spelling and therefore no
+intake normalization: `Plan`, `Apply` and `PreviewApply` used to rewrite
+`"" → "write"` before their guards ran, and every downstream reader carried a
+comment saying whether its ops were plan-normalized or raw adapter output;
+both are gone. `Kind` (`adapter.OpKind`) says why the op exists — an ordinary
+render (`OpRender`, the zero value) or a synthesized orphan cleanup
+(`OpCleanup`), not which source or plugin produced its content — and is
+orthogonal to `Action`: a cleanup op is an `ActionWrite`
+of `{}` whose only work is pruning an emptied section's owned keys (the merge
+path performs the removal via `OwnedKeys`), so the kind is what lets `apply`
+label and count it as a removal rather than sniffing `{}`+`OwnedKeys`. Cleanup
+ops are built by `adapter.NewCleanupOp` — the only producer of `OpCleanup`,
+called from `render.orphanCleanupOps` and from `agent disable --purge` — and
+`TestEveryCleanupLiteralUsesNewCleanupOp` fails any production `FileOp` literal
+that hand-rolls the cleanup shape or stamps the kind by hand, so a hand-rolled
+cleanup literal cannot ship unstamped or hand-stamped.
+
 **Key-merge strategies and on-disk format.** `KeyMergeStrategy` /
 `FileOp.MergeStrategy` name how an adapter co-owns keys inside a shared config
 file: `merge-json-keys` (Claude's `.claude.json`/`settings.json`, a project's
@@ -192,6 +211,9 @@ widening the accessor to a per-path strategy first. A central guard
 MCP+hook fixture through every registered adapter and pins `KeyMergeStrategy()`
 against the `MergeStrategy` stamped on every key-merge `FileOp` it emits, so the
 accessor can never silently drift from what an adapter actually writes.
+`MergeStrategy` itself stays a plain string: typing it would change the
+published `Adapter` interface (`KeyMergeStrategy() string`) and is deferred to
+[#250](https://github.com/spxrogers/agentsync/issues/250).
 
 **Deep vs breadth-tier adapters.** The nine hand-written packages above are
 *deep* adapters — agent-specific, multi-component, often bidirectional. Beyond

docs/components.md

@@ -142,7 +142,9 @@ shared cross-agent dir it writes into, and MUST return nil at project scope (see
 [architecture § VersionedDirs](architecture.md#versioneddirs-optional)).
 - **Key:** `Adapter` (interface); `DestWriter` (interface);
   `VersionedDirs` (optional interface, `VersionRoots`); `NonEmptyDirs` (helper);
-  `Scope` (`ScopeUser`/`ScopeProject`); `FileOp`; `Skip` (with `SkipKind`);
+  `Scope` (`ScopeUser`/`ScopeProject`); `FileOp` (with typed `Action` — zero
+  value `ActionWrite` — and `OpKind`; `NewCleanupOp` builds the one `OpCleanup`
+  op); `Skip` (with `SkipKind`);
   `Registry` (`NewRegistry`, `Register`, `Lookup`, `Names`). Component support is
   expressed by what `Render` emits — an unsupported component yields a `Skip`,
   not an absent capability flag.
@@ -379,7 +381,9 @@ noop-registered agent unless `AGENTSYNC_ALLOW_UNIMPLEMENTED=1`.
 Orchestrates apply: canonical + registry → per-agent `FileOp`s/`Skip`s, runs
 collision detection and backups, records state, and builds the translation
 report. It reclaims two kinds of orphan: emptied key-merge sections (synthesized
-cleanup ops for orphaned owned keys) and **whole-file components** whose
+cleanup ops for orphaned owned keys — built by `adapter.NewCleanupOp` and stamped
+`adapter.OpCleanup`, so consumers identify them by kind rather than by shape)
+and **whole-file components** whose
 `source_id` is under `skills/`, `subagents/`, `commands/`, or the retired
 `agents/` spelling — a destination whose source no longer renders it is deleted,
 backing up a hand-edit first, and SKIPPED (with the state entry kept, so the next

internal/adapter/action_test.go

@@ -0,0 +1,93 @@
+package adapter_test
+
+import (
+	"fmt"
+	"slices"
+	"testing"
+
+	"github.com/spxrogers/agentsync/internal/adapter"
+)
+
+// TestFileOpEnums_ZeroValues is the one explicit pin of the premise the typed
+// FileOp fields rest on: a FileOp built without naming an action writes, and
+// one built without naming a kind is an ordinary render. The intake
+// normalizations ("" → "write") were deleted on the strength of this, so the
+// bare `var` declaration — never an explicit constant — is the point of each
+// subtest.
+func TestFileOpEnums_ZeroValues(t *testing.T) {
+	t.Run("zero Action is ActionWrite", func(t *testing.T) {
+		var a adapter.Action
+		if a != adapter.ActionWrite {
+			t.Fatalf("zero adapter.Action = %v, want ActionWrite", a)
+		}
+	})
+	t.Run("zero OpKind is OpRender", func(t *testing.T) {
+		var k adapter.OpKind
+		if k != adapter.OpRender {
+			t.Fatalf("zero adapter.OpKind = %v, want OpRender", k)
+		}
+	})
+}
+
+// TestFileOpEnums_String pins the human surface the dry-run op label and
+// DispatchOps' error text depend on: %s and %q both route through Stringer, so
+// an out-of-range value reads "action(<n>)" / "opkind(<n>)" rather than a bare
+// integer.
+func TestFileOpEnums_String(t *testing.T) {
+	tests := []struct {
+		name string
+		v    fmt.Stringer
+		want string
+	}{
+		{name: "ActionWrite", v: adapter.ActionWrite, want: "write"},
+		{name: "ActionDelete", v: adapter.ActionDelete, want: "delete"},
+		{name: "Action(9)", v: adapter.Action(9), want: "action(9)"},
+		{name: "OpRender", v: adapter.OpRender, want: "render"},
+		{name: "OpCleanup", v: adapter.OpCleanup, want: "cleanup"},
+		{name: "OpKind(9)", v: adapter.OpKind(9), want: "opkind(9)"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := tt.v.String(); got != tt.want {
+				t.Errorf("String() = %q, want %q", got, tt.want)
+			}
+			if got, want := fmt.Sprintf("%q", tt.v), fmt.Sprintf("%q", tt.want); got != want {
+				t.Errorf("%%q = %s, want %s", got, want)
+			}
+		})
+	}
+}
+
+// TestNewCleanupOp pins every field the single cleanup-op constructor sets. It
+// is the only producer of OpCleanup — render.orphanCleanupOps and `agent
+// disable --purge` both call it, and TestEveryCleanupLiteralUsesNewCleanupOp
+// keeps it that way — so a field it drops is dropped at every synthesis site
+// at once, and a missing Kind stamp here relabels every key removal as a write
+// in `apply`.
+func TestNewCleanupOp(t *testing.T) {
+	owned := []string{"/mcpServers/a", "/mcpServers/b"}
+	op := adapter.NewCleanupOp("/home/u/.claude.json", "merge-json-keys", owned)
+	tests := []struct {
+		name      string
+		got, want any
+	}{
+		{name: "Action is ActionWrite", got: op.Action, want: adapter.ActionWrite},
+		{name: "Kind is OpCleanup", got: op.Kind, want: adapter.OpCleanup},
+		{name: "Path", got: op.Path, want: "/home/u/.claude.json"},
+		{name: "Content is the empty object", got: string(op.Content), want: "{}"},
+		{name: "Mode", got: op.Mode, want: uint32(0o644)},
+		{name: "MergeStrategy", got: op.MergeStrategy, want: "merge-json-keys"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.got != tt.want {
+				t.Errorf("got %v, want %v", tt.got, tt.want)
+			}
+		})
+	}
+	t.Run("OwnedKeys are the input pointers", func(t *testing.T) {
+		if !slices.Equal(op.OwnedKeys, owned) {
+			t.Errorf("OwnedKeys = %v, want %v", op.OwnedKeys, owned)
+		}
+	})
+}

internal/adapter/adapter.go

@@ -6,6 +6,7 @@ package adapter
 import (
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 
 	"github.com/spxrogers/agentsync/internal/secrets"
@@ -52,14 +53,92 @@ func (s Scope) String() string {
 	}
 }
 
-// FileOp describes one destination-side change. Action is "write" (the default;
-// the empty string is treated as "write") or "delete" — see the Action field and
-// DispatchOps, which both accept "" as write. render.Plan normalizes "" to
-// "write" as it collects each adapter's ops, and render.Apply/PreviewApply
-// re-normalize at intake before any pipeline guard runs (they are exported and
-// accept a caller-built RenderPlan that never went through Plan), so the
-// pipeline guards always see the literal "write"; only code reading raw
-// adapter Render output (or state-derived ops) must still accept "" as write.
+// Action says what a FileOp does to its destination. The zero value IS
+// ActionWrite — a FileOp built without naming an action writes — which is why
+// there is nothing to normalize at any intake: no empty spelling exists that
+// an executor would write while a guard matching "write" let it through.
+// Unlike SkipKind below, whose zero value is invalid by design so an unstamped
+// Skip fails TestEverySkipLiteralSetsKind, the zero values of Action and
+// OpKind are deliberately valid (write, render): the common case must need no
+// stamp.
+type Action int
+
+const (
+	// ActionWrite writes Content to Path — whole-file, or key-merged per
+	// MergeStrategy. It is the zero value.
+	ActionWrite Action = iota
+	// ActionDelete removes Path. No adapter renders one and none enters a
+	// plan's Ops: deletes are synthesized at apply time (orphan reclamation),
+	// by `agent disable --purge`, and as the drift walk's orphan item.
+	ActionDelete
+)
+
+// String renders the action for the dry-run op label and DispatchOps' error
+// text. %q on an Action quotes this, so an out-of-range value reads
+// "action(<n>)" rather than a bare integer.
+func (a Action) String() string {
+	switch a {
+	case ActionWrite:
+		return "write"
+	case ActionDelete:
+		return "delete"
+	default:
+		return fmt.Sprintf("action(%d)", int(a))
+	}
+}
+
+// OpKind says why a FileOp exists — an ordinary render, or a synthesized
+// orphan cleanup — orthogonally to what it does (Action). The zero value is
+// OpRender, an ordinary op an adapter's Render emitted.
+type OpKind int
+
+const (
+	// OpRender is an ordinary rendered op. It is the zero value.
+	OpRender OpKind = iota
+	// OpCleanup marks an orphan-cleanup op: an ActionWrite of "{}" to a
+	// key-merge destination whose only work is pruning OwnedKeys — the merge
+	// path performs the removal, so the op writes nothing new. Consumers
+	// identify it by this kind, never by that shape. NewCleanupOp is its
+	// producer (called from render.orphanCleanupOps when a key-merge section
+	// empties in the source, and from `agent disable --purge`).
+	OpCleanup
+)
+
+// String is the Stringer form — "render" | "cleanup" | "opkind(<n>)" — kept
+// for symmetry with Action and for %v in test failures; nothing in production
+// prints a kind.
+func (k OpKind) String() string {
+	switch k {
+	case OpRender:
+		return "render"
+	case OpCleanup:
+		return "cleanup"
+	default:
+		return fmt.Sprintf("opkind(%d)", int(k))
+	}
+}
+
+// NewCleanupOp builds the op that prunes owned keys from a key-merge
+// destination: an ActionWrite of "{}" whose only work is the OwnedKeys removal
+// the merge path performs. It is the only producer of OpCleanup: every site
+// that synthesizes a cleanup op must call it so the kind is never missed, and
+// TestEveryCleanupLiteralUsesNewCleanupOp fails any production FileOp literal
+// that hand-rolls the cleanup shape or stamps OpCleanup by hand instead.
+func NewCleanupOp(path, strategy string, owned []string) FileOp {
+	return FileOp{
+		Action:        ActionWrite,
+		Kind:          OpCleanup,
+		Path:          path,
+		Content:       []byte("{}"),
+		Mode:          0o644,
+		MergeStrategy: strategy,
+		OwnedKeys:     owned,
+	}
+}
+
+// FileOp describes one destination-side change. Action says what happens to
+// the destination (write — the zero value — or delete); Kind says why the op
+// exists (an ordinary render, or a synthesized orphan cleanup).
 // Path is absolute (after AGENTSYNC_TARGET_ROOT redirection).
 //
 // CONTRACT — Content is ALWAYS JSON for a key-merge op, regardless of the
@@ -80,13 +159,21 @@ func (s Scope) String() string {
 // format-specific merge). A new TOML/YAML-backed agent must keep Content JSON,
 // not emit the on-disk format here.
 type FileOp struct {
-	Action        string // "" | "write" | "delete"  ("" == "write"; render.Plan and render.Apply/PreviewApply rewrite "" → "write" at intake)
+	Action        Action // write (zero value) | delete
+	Kind          OpKind // render (zero value) | cleanup
 	Path          string
 	Content       []byte
 	Mode          uint32
-	SourceID      string   // canonical source path that produced this op
-	MergeStrategy string   // "replace" (default) | "merge-json-keys" | "merge-jsonc-keys" | "merge-toml-keys"
-	OwnedKeys     []string // JSON pointers owned by agentsync; populated by Apply from state, not Render
+	SourceID      string // canonical source path that produced this op
+	MergeStrategy string // "replace" (default) | "merge-json-keys" | "merge-jsonc-keys" | "merge-toml-keys"
+	// OwnedKeys lists the JSON pointers agentsync owns at this key-merge
+	// destination, so the merge can remove any the op no longer carries.
+	// render.Plan populates it from state — scoped to the top-level sections
+	// this op writes — whenever Plan is given a state (plugin explain/poll pass
+	// nil). A value an adapter's Render sets is only a fallback for Apply driven
+	// without the pipeline; Plan overwrites it on every key-merge op whenever it
+	// has state.
+	OwnedKeys []string
 }
 
 // SkipKind classifies how much of a component was lost, so consumers never have

internal/adapter/claude/apply_test.go

@@ -16,7 +16,7 @@ func TestApply_NewSettings_WritesContent(t *testing.T) {
 	a := claude.New(claude.Options{TargetRoot: tmp})
 
 	op := adapter.FileOp{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          filepath.Join(tmp, ".claude.json"),
 		Content:       []byte(`{"mcpServers":{"github":{"command":"npx"}}}`),
 		Mode:          0o644,
@@ -40,7 +40,7 @@ func TestApply_PreservesForeignKeys(t *testing.T) {
 	_ = os.WriteFile(target, []byte(`{"foreign":{"x":1},"mcpServers":{"old":{}}}`), 0o644)
 
 	op := adapter.FileOp{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          target,
 		Content:       []byte(`{"mcpServers":{"new":{"command":"x"}}}`),
 		Mode:          0o644,
@@ -72,7 +72,7 @@ func TestApply_OrphanRemoval(t *testing.T) {
 	_ = os.WriteFile(target, []byte(`{"mcpServers":{"github":{"command":"old"},"stale":{}}}`), 0o644)
 
 	op := adapter.FileOp{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          target,
 		Content:       []byte(`{"mcpServers":{"github":{"command":"new"}}}`),
 		Mode:          0o644,

internal/adapter/claude/command.go

@@ -16,7 +16,7 @@ func (a *Adapter) renderCommands(c source.Canonical, p Paths) ([]adapter.FileOp,
 			return nil, fmt.Errorf("encode command %s: %w", cmd.Name, err)
 		}
 		ops = append(ops, adapter.FileOp{
-			Action:        "write",
+			Action:        adapter.ActionWrite,
 			Path:          filepath.Join(p.CommandsDir, cmd.Name+".md"),
 			Content:       body,
 			Mode:          0o644,

internal/adapter/claude/hook.go

@@ -67,7 +67,7 @@ func (a *Adapter) renderHooks(c source.Canonical, p Paths) ([]adapter.FileOp, []
 		return nil, nil, fmt.Errorf("marshal hooks: %w", err)
 	}
 	return []adapter.FileOp{{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          p.Settings,
 		Content:       append(body, '\n'),
 		Mode:          0o644,

internal/adapter/claude/largeint_test.go

@@ -22,7 +22,7 @@ func TestApply_PreservesForeignLargeInt(t *testing.T) {
 		t.Fatal(err)
 	}
 	op := adapter.FileOp{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          dest,
 		MergeStrategy: "merge-json-keys",
 		Content:       []byte(`{"mcpServers":{"github":{"command":"npx"}}}`),

internal/adapter/claude/memory.go

@@ -13,7 +13,7 @@ func (a *Adapter) renderMemory(c source.Canonical, p Paths) ([]adapter.FileOp, e
 	}
 	body := source.RenderManagedMemory(c.Memory.Body, c.Memory.Fragments, filepath.Base(p.Memory), c.Config.MemoryBannerEnabled())
 	return []adapter.FileOp{{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          p.Memory,
 		Content:       []byte(body),
 		Mode:          0o644,

internal/adapter/claude/render.go

@@ -161,7 +161,7 @@ func (a *Adapter) renderMCP(c source.Canonical, p Paths, scope adapter.Scope) ([
 	}
 
 	return []adapter.FileOp{{
-		Action:        "write",
+		Action:        adapter.ActionWrite,
 		Path:          dest,
 		Content:       append(body, '\n'),
 		Mode:          0o644,

internal/adapter/claude/skill.go

@@ -27,7 +27,7 @@ func SkillFileOps(skills []source.Skill, skillsDir string) ([]adapter.FileOp, er
 			return nil, fmt.Errorf("encode skill %s: %w", s.Name, err)
 		}
 		ops = append(ops, adapter.FileOp{
-			Action:        "write",
+			Action:        adapter.ActionWrite,
 			Path:          filepath.Join(skillsDir, s.Name, "SKILL.md"),
 			Content:       body,
 			Mode:          0o644,
@@ -40,7 +40,7 @@ func SkillFileOps(skills []source.Skill, skillsDir string) ([]adapter.FileOp, er
 				mode = 0o644
 			}
 			ops = append(ops, adapter.FileOp{
-				Action:        "write",
+				Action:        adapter.ActionWrite,
 				Path:          filepath.Join(skillsDir, s.Name, filepath.FromSlash(f.Path)),
 				Content:       f.Content,
 				Mode:          mode,