refactor(cli): reconcileRun becomes a reconcileSession (#232)

.agentsync/memory/AGENTS.md

@@ -225,14 +225,15 @@ Capture **refuses the whole write** rather than persist cleartext. It errs towar
 refusing; the user updates the vault or edits the canonical source directly.
 Two narrow **deletion-only** exceptions sit outside the funnel: `reconcile`'s
 `removeDroppedSource` (`internal/cli/reconcile.go`) unlinks a canonical
-`mcp/<id>.toml` when the user explicitly chooses `[w]rite-back` for a
-destination-side server deletion, and `source.RemoveHooks` deletes a stale
-`hooks/<event>.toml` during import's stale-hook retirement. Both are safe
-without Capture because a pure deletion carries no content to re-reference —
-there is no secret material to persist — and both are guarded (the reconcile
-path is keystroke-gated and `withinDir`-bounded to `~/.agentsync`; RemoveHooks
-validates the native-supplied event id before touching a path). Anything that
-*writes content* dest→source still MUST go through `capture.Capture`.
+`mcp/<id>.toml` when the user chooses write-back for a destination-side server
+deletion — a per-item `[w]`, a confirmed bulk `[W]`, or `--auto-writeback` —
+and `source.RemoveHooks` deletes a stale `hooks/<event>.toml` during import's
+stale-hook retirement. Both are safe without Capture because a pure deletion
+carries no content to re-reference — there is no secret material to persist —
+and both are guarded (the reconcile path is `withinDir`-bounded to
+`~/.agentsync`; RemoveHooks validates the native-supplied event id before
+touching a path). Anything that *writes content* dest→source still MUST go
+through `capture.Capture`.
 
 **3. Resolved vs templated types.** `secrets.SubstituteCanonical` returns
 `secrets.Resolved` (a wrapper, NOT assignable to `source.Canonical`); it is the

AGENTS.md

@@ -232,14 +232,15 @@ Capture **refuses the whole write** rather than persist cleartext. It errs towar
 refusing; the user updates the vault or edits the canonical source directly.
 Two narrow **deletion-only** exceptions sit outside the funnel: `reconcile`'s
 `removeDroppedSource` (`internal/cli/reconcile.go`) unlinks a canonical
-`mcp/<id>.toml` when the user explicitly chooses `[w]rite-back` for a
-destination-side server deletion, and `source.RemoveHooks` deletes a stale
-`hooks/<event>.toml` during import's stale-hook retirement. Both are safe
-without Capture because a pure deletion carries no content to re-reference —
-there is no secret material to persist — and both are guarded (the reconcile
-path is keystroke-gated and `withinDir`-bounded to `~/.agentsync`; RemoveHooks
-validates the native-supplied event id before touching a path). Anything that
-*writes content* dest→source still MUST go through `capture.Capture`.
+`mcp/<id>.toml` when the user chooses write-back for a destination-side server
+deletion — a per-item `[w]`, a confirmed bulk `[W]`, or `--auto-writeback` —
+and `source.RemoveHooks` deletes a stale `hooks/<event>.toml` during import's
+stale-hook retirement. Both are safe without Capture because a pure deletion
+carries no content to re-reference — there is no secret material to persist —
+and both are guarded (the reconcile path is `withinDir`-bounded to
+`~/.agentsync`; RemoveHooks validates the native-supplied event id before
+touching a path). Anything that *writes content* dest→source still MUST go
+through `capture.Capture`.
 
 **3. Resolved vs templated types.** `secrets.SubstituteCanonical` returns
 `secrets.Resolved` (a wrapper, NOT assignable to `source.Canonical`); it is the

CHANGELOG.md

@@ -289,6 +289,29 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
   `Adapter` interface and is deferred to
   [#250](https://github.com/spxrogers/agentsync/issues/250).
 
+- **Internal: `reconcile`'s interactive pass is a session type**
+  ([#232](https://github.com/spxrogers/agentsync/issues/232)). `reconcileRun`
+  was 375 lines with five `goto done`s, two labeled loops and seven pieces of
+  run-scoped state travelling as loose locals; it is now a 22-line entry point
+  over a `reconcileSession` whose methods are the two prompts, the `--auto-*`
+  dispatch, the action switch and the run's tail. Every `goto` is a plain
+  return, and the tail runs from exactly one call site — deliberately not a
+  `defer`, since it re-applies the queued `[o]verride` ops and saves state. The
+  bulk-action state machine is a typed enum instead of a `byte` with `ch | 0x20`
+  case folding, which also writes down the two keystrokes the prompt
+  deliberately does **not** fold (`I` is not a bulk ignore; `D` is not a
+  diff). No user-visible behaviour changes: 39 scripted-stdin scenarios —
+  every prompt, bulk confirmation, EOF, `[q]uit` (including a quit with a
+  queued override), `--auto-*` mode, project-scope override, exit code, masked
+  secret value and resulting source/state tree — are byte-identical to the
+  pre-change binary (`main` at `309fde0`). Two smaller things went with it:
+  `printItemDiff` was a pure alias of `renderItemValues`, and inverting a hook
+  pointer's native event spelling now uses the registry the caller already
+  holds instead of rebuilding all 31 adapters (~10 µs) on every resolution.
+  The docs no longer call `removeDroppedSource`'s gate "keystroke-gated"
+  (`--auto-writeback` never was one): SECURITY.md, architecture §5 and the
+  project memory name the three write-back routes that reach it.
+
 - **`.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

CLAUDE.md

@@ -232,14 +232,15 @@ Capture **refuses the whole write** rather than persist cleartext. It errs towar
 refusing; the user updates the vault or edits the canonical source directly.
 Two narrow **deletion-only** exceptions sit outside the funnel: `reconcile`'s
 `removeDroppedSource` (`internal/cli/reconcile.go`) unlinks a canonical
-`mcp/<id>.toml` when the user explicitly chooses `[w]rite-back` for a
-destination-side server deletion, and `source.RemoveHooks` deletes a stale
-`hooks/<event>.toml` during import's stale-hook retirement. Both are safe
-without Capture because a pure deletion carries no content to re-reference —
-there is no secret material to persist — and both are guarded (the reconcile
-path is keystroke-gated and `withinDir`-bounded to `~/.agentsync`; RemoveHooks
-validates the native-supplied event id before touching a path). Anything that
-*writes content* dest→source still MUST go through `capture.Capture`.
+`mcp/<id>.toml` when the user chooses write-back for a destination-side server
+deletion — a per-item `[w]`, a confirmed bulk `[W]`, or `--auto-writeback` —
+and `source.RemoveHooks` deletes a stale `hooks/<event>.toml` during import's
+stale-hook retirement. Both are safe without Capture because a pure deletion
+carries no content to re-reference — there is no secret material to persist —
+and both are guarded (the reconcile path is `withinDir`-bounded to
+`~/.agentsync`; RemoveHooks validates the native-supplied event id before
+touching a path). Anything that *writes content* dest→source still MUST go
+through `capture.Capture`.
 
 **3. Resolved vs templated types.** `secrets.SubstituteCanonical` returns
 `secrets.Resolved` (a wrapper, NOT assignable to `source.Canonical`); it is the

SECURITY.md

@@ -29,10 +29,11 @@ can resolve secrets into native config files. Areas of particular interest:
   cannot verify the source's `${secret:…}` refs (the leak check would be blind),
   so a locked vault never silently degrades into persisting a credential. The
   only write-backs outside this path are two guarded pure *deletions* of
-  canonical files (reconcile's dest-dropped MCP-server removal, keystroke-gated
-  and path-bounded to `~/.agentsync`; import's stale-hook retirement) — a
-  deletion carries no secret content to persist, and anything that writes
-  content back still goes through `capture.Capture`.
+  canonical files (reconcile's dest-dropped MCP-server removal, which runs only
+  for a chosen write-back — per-item `[w]`, confirmed bulk `[W]`, or
+  `--auto-writeback` — and is path-bounded to `~/.agentsync`; import's
+  stale-hook retirement) — a deletion carries no secret content to persist,
+  and anything that writes content back still goes through `capture.Capture`.
   The alternative `backend = "env"` stores nothing: `${secret:…}` resolves from
   the process environment at apply time, so there is no vault, no identity file
   and no decryption — the credential's protection is whatever protects the

docs/architecture.md

@@ -716,7 +716,8 @@ to `int64`/`float64` before writing. No other
 code path writes destination data back into the source. (Two guarded code paths
 *delete* canonical files without going through Capture: `reconcile`'s
 `removeDroppedSource` unlinks `mcp/<id>.toml` when the user writes back a
-destination-side server deletion — keystroke-gated, `withinDir`-bounded to
+destination-side server deletion — gated on that chosen write-back (a per-item
+`[w]`, a confirmed bulk `[W]`, or `--auto-writeback`), `withinDir`-bounded to
 `~/.agentsync` — and import's stale-hook retirement calls `source.RemoveHooks`
 on `hooks/<event>.toml`. A pure deletion carries no content to re-reference, so
 the funnel's secret guarantees are not in play; anything that writes *content*

docs/components.md

@@ -54,7 +54,13 @@ is the only package that depends on nearly all the others.
   behind `status`, `diff`, `reconcile` and `explain` (`planwalk.go`); its
   `planItem` is deliberately unexported field-for-field so it can never become
   a `--json` surface, because a plan built from `secrets.SubstituteCanonical`
-  carries resolved cleartext in `op.Content`; `destReadPath` / `readDestText`
+  carries resolved cleartext in `op.Content`; `reconcileSession` — one
+  `agentsync reconcile` pass (printer, scripted input, registry, loaded state,
+  redaction map) plus its run-scoped bookkeeping, so the two prompts, the
+  `--auto-*` dispatch, the action switch and the run's tail (`finish`) are
+  separately testable methods rather than five `goto`s over seven loose locals
+  (#232); `finish` has exactly one call site and is never deferred, because it
+  writes and it returns the run's error; `destReadPath` / `readDestText`
   (`destread.go`) — the whole-file destination readers that carry the symlink
   policy (`AGENTSYNC_ALLOW_SYMLINK_DEST`), mirroring `iox.AtomicWrite`'s.
 - **Commands:** `init`, `agent {add,remove,list,enable,disable}`, `apply`,

internal/cli/apply_state_order_internal_test.go

@@ -1,9 +1,6 @@
 package cli
 
 import (
-	"os"
-	"path/filepath"
-	"runtime"
 	"strings"
 	"testing"
 )
@@ -39,20 +36,11 @@ import (
 // If this ever gains a genuine observable (a component the re-apply does not
 // re-record), replace half A with a test of that.
 func TestApplyPipelineLoadsStateAfterSourceReload(t *testing.T) {
-	_, thisFile, _, ok := runtime.Caller(0)
-	if !ok {
-		t.Fatal("runtime.Caller failed")
-	}
-	dir := filepath.Dir(thisFile)
-
 	// The two halves are independent contracts; separate subtests so a Fatal
 	// in one never hides a failure in the other.
 	t.Run("pipeline loads state after the source reload", func(t *testing.T) {
-		applySrc, err := os.ReadFile(filepath.Join(dir, "apply.go"))
-		if err != nil {
-			t.Fatal(err)
-		}
-		body := funcBody(string(applySrc), "func runApplyPipeline(")
+		applySrc := readFileForGuard(t, repoRootFromCaller(t), "internal/cli/apply.go")
+		body := funcBody(applySrc, "func runApplyPipeline(")
 		if body == "" {
 			t.Fatal("runApplyPipeline not found in apply.go — update this guard")
 		}
@@ -86,7 +74,7 @@ func TestApplyPipelineLoadsStateAfterSourceReload(t *testing.T) {
 		// The signature must not accept state either: a caller-supplied
 		// *state.Targets is read before this function runs, which has the same
 		// defect and is how the bug originally shipped.
-		sig := funcSignature(string(applySrc), "func runApplyPipeline(")
+		sig := funcSignature(applySrc, "func runApplyPipeline(")
 		if strings.Contains(sig, "*state.Targets") {
 			t.Errorf("runApplyPipeline takes a *state.Targets again: %s\n"+
 				"A caller reads it before the source reload that can rewrite it. Load it inside, after the reload.", sig)
@@ -97,11 +85,7 @@ func TestApplyPipelineLoadsStateAfterSourceReload(t *testing.T) {
 		// The plugin re-apply must BE the pipeline, not a copy of it. The
 		// positive check and the four negatives are Errorf, not Fatal, so a
 		// hand-rolled re-apply reports every way it diverged in one run.
-		pollSrc, err := os.ReadFile(filepath.Join(dir, "plugin_poll.go"))
-		if err != nil {
-			t.Fatal(err)
-		}
-		reapply := funcBody(string(pollSrc), "func reapplyAfterPluginChange(")
+		reapply := funcBody(readFileForGuard(t, repoRootFromCaller(t), "internal/cli/plugin_poll.go"), "func reapplyAfterPluginChange(")
 		if reapply == "" {
 			t.Fatal("reapplyAfterPluginChange not found in plugin_poll.go — update this guard")
 		}

internal/cli/explain.go

@@ -252,6 +252,7 @@ func explainRun(cmd *cobra.Command, rawPath, ptr string, jsonOut bool) error {
 		target:        target,
 		pointer:       ptr,
 		plan:          plan,
+		reg:           reg,
 		agents:        reg.Names(),
 		canonical:     c,
 		state:         s,

internal/cli/explain_model.go

@@ -18,10 +18,13 @@ import (
 // explainInputs bundles everything buildExplainModel needs, so the builder stays
 // a pure-ish function the tests can drive without a cobra command.
 type explainInputs struct {
-	fs            afero.Fs
-	target        string
-	pointer       string
-	plan          render.RenderPlan
+	fs      afero.Fs
+	target  string
+	pointer string
+	plan    render.RenderPlan
+	// reg is the registry the plan was rendered with; it resolves a renaming
+	// adapter's native hook-event spelling back to the canonical one.
+	reg           *adapter.Registry
 	agents        []string // registry order, for stable owner ordering
 	canonical     source.Canonical
 	state         *state.Targets
@@ -173,7 +176,7 @@ func fileItem(in explainInputs, it planItem, skips []adapter.Skip,
 func keyItem(in explainInputs, it planItem, skips []adapter.Skip, origins map[string]explainPluginOrigin,
 	secretRefs map[secrets.RefLocation][]string, hookEvents []string,
 ) explainItem {
-	kind, name := componentFromPointer(it.agent, it.ptr, hookEvents)
+	kind, name := componentFromPointer(in.reg, it.agent, it.ptr, hookEvents)
 
 	item := explainItem{
 		Pointer:    it.ptr,
@@ -245,7 +248,7 @@ func sourceOf(in explainInputs, srcID, kind string) *explainSource {
 // pointer shape (the mcp/lsp/hooks container families), falling back to the
 // op-level SourceID when the pointer names no single source.
 func pointerSource(in explainInputs, agent, ptr, opSourceID string, hookEvents []string) *explainSource {
-	abs := pointerSourceFile(in.srcHome, agent, ptr, hookEvents)
+	abs := pointerSourceFile(in.reg, in.srcHome, agent, ptr, hookEvents)
 	if abs == "" {
 		if opSourceID == "" {
 			return nil
@@ -297,7 +300,7 @@ func componentFromSourceID(srcID string) (kind, name string) {
 
 // componentFromPointer maps a NATIVE key-merge pointer to (kind, name), routing
 // hooks through the same canonical-event inversion reconcile's write-back uses.
-func componentFromPointer(agent, ptr string, hookEvents []string) (kind, name string) {
+func componentFromPointer(reg *adapter.Registry, agent, ptr string, hookEvents []string) (kind, name string) {
 	parts := strings.SplitN(strings.TrimPrefix(ptr, "/"), "/", 3)
 	if len(parts) < 2 || parts[1] == "" {
 		return "", ""
@@ -308,7 +311,7 @@ func componentFromPointer(agent, ptr string, hookEvents []string) (kind, name st
 	case "lspServers", "lsp":
 		return "lsp", parts[1]
 	case "hooks":
-		if event, ok := canonicalHookEvent(agent, parts[1], hookEvents); ok {
+		if event, ok := canonicalHookEvent(reg, agent, parts[1], hookEvents); ok {
 			return "hook", event
 		}
 		return "hook", ""

internal/cli/planwalk_characterization_test.go

@@ -1090,6 +1090,7 @@ func TestPlanWalkCharacterization(t *testing.T) {
 				target:        tc.target(userHome),
 				pointer:       tc.pointer,
 				plan:          plan,
+				reg:           reg,
 				agents:        reg.Names(),
 				state:         s,
 				userHome:      userHome,

internal/cli/planwalk_internal_test.go

@@ -623,6 +623,7 @@ func TestPathFilterFlagsSurviveAZeroItemOp(t *testing.T) {
 		fs:          afero.NewMemMapFs(),
 		target:      d,
 		plan:        plan,
+		reg:         registryFactory(),
 		agents:      names,
 		state:       state.New(),
 		userHome:    userHome,