fix(cli): plugin upgrade re-applies through the real apply pipeline (#231)

CHANGELOG.md

@@ -222,6 +222,16 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
   The user guide, the daily-loop guide and the CLI reference said the same
   wrong thing and are corrected with it.
 
+- **`plugin upgrade` now git-backs-up the destinations it overwrites**
+  ([#231](https://github.com/spxrogers/agentsync/issues/231)). Both upgrade
+  forms end in a re-apply that writes user-scope destination dirs, but that
+  re-apply was a second copy of the apply pipeline with no
+  `[destination_directory_git_backup]` pass at all — so an upgrade overwrote
+  `~/.claude`, `~/.codex`, … with **no pre-apply baseline and no checkpoint**,
+  and `agentsync revert` could not undo it. The re-apply is now `apply` itself,
+  so the baseline/checkpoint (and the mode/prompt/`--no-input` policy) apply
+  identically.
+
 ### Changed
 
 - **`status --json`, `diff` and `reconcile` now list a shared file's merged keys
@@ -235,6 +245,27 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
   runs, or a `reconcile` transcript compared against a previous one, will be
   stable for the first time. `explain` already sorted and is unchanged.
 
+- **`plugin upgrade` prints what `apply` prints**
+  ([#231](https://github.com/spxrogers/agentsync/issues/231)). Its re-apply now
+  goes through the one apply pipeline, so it announces the effective scope,
+  reports removals honestly (an upgrade that re-renders identical bytes says
+  `up to date: N ops, no changes` instead of claiming `applied: N ops`), warns
+  and exits 0 when no agents are enabled instead of printing `applied: 0 ops`,
+  prunes old collision backups, and prints the per-plugin translation report —
+  which is what tells you whether the new version still translates. Two strings
+  changed with it: the foreign-collision warning is now apply's wording (and,
+  as in `apply`, is printed even when the apply then fails partway), and the
+  five `… after upgrade:` error prefixes collapse into one
+  `re-apply after plugin upgrade:`. Interactively, the question `apply` asks
+  before its first write into an untracked destination dir (enable git backup
+  for it?) can now appear during an upgrade, and answering `yes` / `don't ask
+  again` persists the mode to `agentsync.toml` exactly as it does for `apply`.
+  Under the default `[destination_directory_git_backup] mode = "prompt"`, an
+  unattended run (cron, `--no-input`, no TTY) also prints apply's git-backup
+  hint and its `could not take a pre-apply baseline` warning on every run until
+  the mode is set to `on` or `off` — the same two lines an unattended `apply`
+  prints.
+
 - **Internal: `status`, `diff`, `reconcile` and `explain` now share one
   plan→drift walk** ([#229](https://github.com/spxrogers/agentsync/issues/229)).
   `explain` now decodes a key-merged destination once per rendered section
@@ -734,8 +765,10 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
 
   - **`plugin upgrade` now re-applies, in BOTH forms.** `--all` carries over
     `update --apply`'s complete re-apply (scope resolution, secret
-    substitution, plan/apply, state recording), so it is behavior-identical to
-    what `update --apply` did. The single-id `plugin upgrade <id>` gains that
+    substitution, plan/apply, state recording), so it keeps `update --apply`'s
+    ending state; #231 above then made that re-apply `apply`'s own pipeline,
+    which prints more and — see Fixed, above — git-backs-up the destinations
+    it overwrites. The single-id `plugin upgrade <id>` gains that
     same re-apply — a **behavior change**: it used to re-fetch and leave your
     agents stale until the next `apply`. One verb, one ending state.
   - **`--auto-safe` became `--lossless` on the plugin side**, because the name
@@ -816,9 +849,10 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
 
 - **`plugin upgrade`'s one-line help now names `apply` instead of saying
   "re-apply".** Both `upgrade` forms genuinely run the full apply pipeline
-  (`render.Plan` + `render.Apply`, honoring `--scope`/`--project`) — but in the
-  `agentsync plugin` command list, "and re-apply" read as loose jargon rather
-  than as "runs `agentsync apply`", so the one behavior a reader most needs to
+  (`render.Plan` + `render.Apply`, honoring `--scope`/`--project`; since #231
+  above, the one `apply` itself runs) — but in the `agentsync plugin` command
+  list, "and re-apply" read as loose jargon rather than as "runs
+  `agentsync apply`", so the one behavior a reader most needs to
   know about the command — that it writes to your agents' native config, not
   just to the plugin cache — was the one the summary buried. Reworded to "and
   run apply" (and matched in the website CLI reference table). Behavior is

docs/architecture.md

@@ -623,9 +623,10 @@ Key stages:
    foreign-collision backups (`internal/render`, `internal/iox`).
 8. **Record** new hashes in `targets.json` (`internal/state`) and print the
    translation report.
-9. **Git-backup** (issue #118) — for a user-scope apply, checkpoint each destination
-   **directory** into its own **local-only** git repo (`internal/cli/gitbackup.go`
-   → `internal/git`). The unit is the directory, not the agent: every enabled
+9. **Git-backup** (issue #118) — for a user-scope apply (including the apply at the
+   tail of `plugin upgrade`), checkpoint each destination **directory** into its
+   own **local-only** git repo (`internal/cli/gitbackup.go` → `internal/git`).
+   The unit is the directory, not the agent: every enabled
    adapter declares its version roots via the optional `adapter.VersionedDirs`
    extension (its config dir plus any shared cross-agent dir it writes — Codex and
    several breadth agents all target `~/.agents/skills`; OpenCode targets
@@ -670,6 +671,19 @@ Key stages:
 vs `→ write` and preview foreign-collision backups, and prints the plan/report —
 all without writing a byte (and it skips the git-backup step 9 entirely).
 
+**One implementation.** The pipeline above lives in a single callable,
+`runApplyPipeline` (`internal/cli/apply.go`), and `apply` is not its only
+caller: the re-apply tail of `plugin upgrade` and `plugin upgrade --all`
+(`reapplyAfterPluginChange`) runs the same function with the default options —
+a real apply of every enabled agent, with destination git backup governed by
+`[destination_directory_git_backup]` exactly as for `apply`. It used to be a second,
+hand-maintained transcription of steps 1–7 and the state-recording half of
+step 8, and the copy had already fallen behind: no translation report (the rest
+of step 8), no pre-apply baseline or checkpoint (step 9), no removal-aware
+headline, no backup pruning. Every apply-side invariant added from here holds
+on the plugin path by construction
+([#231](https://github.com/spxrogers/agentsync/issues/231)).
+
 ---
 
 ## 5. The capture pipeline (Destination ▶ Source)

docs/components.md

@@ -45,11 +45,16 @@ Wires every cobra subcommand into the root tree and dispatches to handlers; this
 is the only package that depends on nearly all the others.
 - **Key:** `NewRoot() *cobra.Command`, `Execute() int` (returns the process exit
   code and owns the terminal `✗ ERROR` line), `Version`/`Commit`/`Date`;
-  `walkPlanItems` — the single plan→state→destination drift walk 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`
+  `runApplyPipeline` — the single apply pipeline (load-projected → resolve
+  secrets → plan → git baseline → write → record state → checkpoint → report),
+  shared by `apply` and the re-apply tail of `plugin upgrade` so the two cannot
+  diverge (#231); its `applyOpts` zero value is a real, all-agents apply, not
+  opted out of git backup (the `[destination_directory_git_backup]` mode still
+  governs it); `walkPlanItems` — the single plan→state→destination drift walk
+  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`
   (`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`,

docs/concepts.md

@@ -261,7 +261,7 @@ plugin manager. These are deliberate outcomes, so they are never rendered in the
 ### Polling (the networked verb of the daily loop)
 `agentsync plugin outdated` is the command that touches the network in the daily
 loop: it polls marketplaces, refreshes the cache, and recomputes version pins.
-`agentsync plugin upgrade --all` then re-pins every pending bump and re-applies.
+`agentsync plugin upgrade --all` then re-pins every pending bump and runs the full `apply`.
 `apply` itself runs entirely from cache — the split keeps it fast, reproducible,
 and offline-safe. It is not the only networked command (`plugin add`,
 `marketplace add`, `import <agent>:plugin`, and `init <git-url>` all fetch), just
@@ -283,7 +283,7 @@ the one the loop runs.
                                                   (dest ▶ source capture)
 
    agentsync plugin outdated ─network─▶ refresh marketplace cache & pins
-   agentsync plugin upgrade --all      re-pin pending bumps, then re-apply
+   agentsync plugin upgrade --all      re-pin pending bumps, then run apply
                                        (plain apply renders from cache, offline)
 ```
 

docs/user-guide.md

@@ -271,10 +271,11 @@ instead lists each one as left unresolved, so it works as a non-interactive
 
 `apply` can keep each user-scope destination dir (`~/.claude`, `~/.codex`, …) in
 its **own local-only git repo**, recording a checkpoint commit after every apply
-that changes managed files there. **Even the first apply is revertible:** before
-that apply overwrites the dir, agentsync records a **pre-apply baseline** commit of
-the prior content of the files it is about to manage, so the apply checkpoint's parent
-is the genuine pre-apply state — there is no "the first apply can't be undone" gap.
+that changes managed files there — including the apply that ends a `plugin
+upgrade`. **Even the first apply is revertible:** before that apply overwrites the
+dir, agentsync records a **pre-apply baseline** commit of the prior content of the
+files it is about to manage, so the apply checkpoint's parent is the genuine
+pre-apply state — there is no "the first apply can't be undone" gap.
 Pre-existing files agentsync did **not** write (an agent's credentials, conversation
 transcripts, your own scratch files) are deliberately left **out** of the versioned
 history so it never becomes a durable copy of your secrets — they are untracked, so a
@@ -867,8 +868,13 @@ agentsync plugin upgrade --all --lossless      # same, skipping bumps that would
 agentsync plugin upgrade atlassian             # re-fetch one plugin, then re-apply
 ```
 
-Both `upgrade` forms end in a re-apply, so an upgrade lands in your agents in one
-command rather than leaving them stale until the next `apply`.
+Both `upgrade` forms end in a re-apply — and the re-apply **is** `apply`: same
+scope resolution, same destination git backup, same removal counts and
+translation report — so an upgrade lands in your agents in one command rather
+than leaving them stale until the next `apply`. Interactively, that includes
+the question `apply` asks before its first write into a destination dir it has
+not versioned yet (enable git backup for it?); answering `yes` or `don't ask
+again` persists the mode to `agentsync.toml`, just as it does for `apply`.
 
 `plugin outdated` is not a pure read despite the `npm outdated` prior: it uses
 the network and it writes state (each marketplace's fetch timestamp and head
@@ -882,7 +888,10 @@ fetch. It is simply the one the daily loop runs.
 
 Want nightly refreshes? agentsync ships no daemon — wire
 `agentsync plugin upgrade --all --lossless` into your own cron / launchd /
-systemd / Task Scheduler.
+systemd / Task Scheduler. Because the upgrade's re-apply is `apply`, an
+unattended run under the default `prompt` git-backup mode prints the same hint
+and baseline warning `apply` does; set `[destination_directory_git_backup] mode`
+to `on` (or `off`) to silence it.
 
 ---
 
@@ -988,7 +997,7 @@ Beta surface. `agentsync <command> --help` is always authoritative.
 | `migrate subagents` | One-shot move of the retired canonical `agents/` directory to `subagents/`, rewriting that tree's recorded `source_id` values. Run once per tree (`--scope project` / `--project <path>` for a project tree). Refuses, listing the names, if a file exists under both directories. | `--scope --project` |
 | `mcp add\|remove\|list\|enable\|disable <name>` | Manage MCP servers. `enable`/`disable` flip the server's `enabled` bit — keeping the definition but stopping the render (`remove` deletes it). `--header "Name: Value"` (repeatable, http/sse only) sets request headers — the usual remote-auth secret site, e.g. `--header "Authorization: Bearer ${secret:TOKEN}"`. | `--type --command --args --url --env --agents --header` |
 | `marketplace add\|remove\|list <url-or-name>` | Manage marketplaces. | |
-| `plugin add\|upgrade\|enable\|disable\|remove <id[@marketplace]>` / `list` / `outdated` / `explain` | Manage plugins (the lifecycle subcommands all accept the same `id[@marketplace]` ref `add` accepts; the bare id also works, and a qualifier naming a different marketplace than the one the plugin was installed from is refused). `outdated` **(network)** polls the marketplaces and reports pending bumps — it also writes each marketplace's fetch timestamp + head SHA to state. `upgrade` **(network)** re-fetches one plugin, or with `--all` every plugin with a pending bump, and **re-applies** in both cases; `--lossless` skips an upgrade that would introduce a new translation loss, reporting it. `explain` shows per-agent translation coverage. | `outdated` · `upgrade [<id>] --all --lossless --scope --project` · `explain [<id>...] --all --json` |
+| `plugin add\|upgrade\|enable\|disable\|remove <id[@marketplace]>` / `list` / `outdated` / `explain` | Manage plugins (the lifecycle subcommands all accept the same `id[@marketplace]` ref `add` accepts; the bare id also works, and a qualifier naming a different marketplace than the one the plugin was installed from is refused). `outdated` **(network)** polls the marketplaces and reports pending bumps — it also writes each marketplace's fetch timestamp + head SHA to state. `upgrade` **(network)** re-fetches one plugin, or with `--all` every plugin with a pending bump, and **runs the full `apply`** in both cases (git backup, removal counts and translation report included); `--lossless` skips an upgrade that would introduce a new translation loss, reporting it. `explain` shows per-agent translation coverage. | `outdated` · `upgrade [<id>] --all --lossless --scope --project` · `explain [<id>...] --all --json` |
 | `secret set\|get\|list\|remove <key>` / `secret edit` | Manage age-encrypted secrets (`list` prints KEYS only; `edit` opens the whole vault, no `<key>`; `set` refuses an empty value unless `--allow-empty`). All five require `[secrets].backend = "age"` (matched case-insensitively, exactly as `apply` matches it) and `[secrets].identity_file`; the three that re-encrypt — `set`, `edit`, `remove` — additionally require `[secrets].recipient`. | `set --stdin` |
 | `apply` | Render source → write agent configs (offline). Git-versions each user-scope destination dir into a local-only repo (opt-out) so a bad apply is revertible. A delete-only run (a component removed from source) reports `removed: N key(s), M file(s)` — key-removals and file-deletes counted distinctly — and a mixed run `applied: X ops, removed: …`, rather than mislabeling itself `up to date`/`applied: 0 ops`; `--dry-run` previews the same removal counts. | `--agents --dry-run --scope --project --no-git-backup` |
 | `revert <agent>` | Roll a destination dir back to a prior apply checkpoint (append-only). Default undoes the most recent apply; prints an out-of-sync notice. `--to` must name one of the dir's own checkpoints (the current one or an ancestor) — anything else is refused. A dir under which a foreign git repo has appeared (or that isn't an agentsync-managed backup) is an **error** when you name the agent, and a **skip with a warning** under `--all` — strictness follows the invocation; there is no `--strict` flag. | `--agents --to --all --dry-run` |