fix(cli): mode drift measured against what apply writes; symlinked destinations follow apply's policy on every surface (#229 PR-C)

CHANGELOG.md

@@ -11,6 +11,43 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
 
 ### Fixed
 
+- **A symlinked destination under `AGENTSYNC_ALLOW_SYMLINK_DEST=1` is no
+  longer reported as permanently drifted**
+  ([#229](https://github.com/spxrogers/agentsync/issues/229)). In the
+  documented chezmoi/Stow configuration, where `apply` writes THROUGH the link,
+  `status`, `reconcile` and `explain` hashed the link itself and answered a
+  sentinel that can never equal a content hash — so every run reported `drift`
+  no apply could clear and `status --exit-code` failed CI forever, while `diff`
+  read through the link and said `no diff`. The switch now governs the READ
+  side too — `apply`, `status`, `diff`, `reconcile` and `explain` all need it:
+  set, all resolve the link and compare the file it points at; unset, the four
+  drift commands refuse the link (`apply` fails only when the content
+  differs), `diff` prints a `symlink` hunk (`--json` `pointer:
+  "symlink"`, alongside `mode`) naming the switch, and `reconcile`'s
+  `[w]`rite-back refuses to capture through it (with advice that omits
+  `[o]verride`: #248). Once opted in, a link that does not resolve — dangling,
+  loop — is reported as such rather than as "set the switch". A link to a
+  FIFO, device or directory is a shape problem the switch cannot fix and is
+  refused as a bare one is; `diff` now prints a `shape` hunk for both instead
+  of rendering the whole source against an "empty" destination.
+
+- **`status`'s permission check now measures the mode the next `apply`
+  writes** ([#229](https://github.com/spxrogers/agentsync/issues/229)). It
+  compared the destination's permission bits against the mode RECORDED at the
+  last apply, so a file whose recorded mode was unset (state written before
+  modes were recorded) or whose adapter changed the mode it renders was
+  reported `clean` while the next apply would chmod it. It now asks `op.Mode`,
+  the question `diff`'s `mode` hunk already asked, so the two agree — on
+  `converged` destinations as well as `clean` ones.
+
+- **`explain <path>` now reports a mode-only drift instead of `clean`**
+  ([#229](https://github.com/spxrogers/agentsync/issues/229)). A
+  content-identical chmod made `status` say `drift` and `explain` say `clean`
+  about the same file in the same second; both now read the one folded class.
+  `reconcile` still ignores mode entirely — it reports "nothing to reconcile"
+  for a drift the other three surfaces name — and that gap is
+  [#245](https://github.com/spxrogers/agentsync/issues/245), not fixed here.
+
 - **A FIFO at a managed destination no longer hangs `status`, `diff`, `explain`,
   or `reconcile`'s drift walk and write-back.** (A directory there never hung — `os.ReadFile`
   fails it immediately with `EISDIR` — and for `status` and `diff` nothing about
@@ -202,8 +239,8 @@ source layout, CLI surface, and state schema are stabilizing but may still chang
   plan→drift walk** ([#229](https://github.com/spxrogers/agentsync/issues/229)).
   `explain` now decodes a key-merged destination once per rendered section
   rather than once per key, so every key in one file is classified against the
-  same snapshot. The ways the four surfaces still disagree (mode-only drift,
-  symlinked destinations) are unchanged and tracked in #229.
+  same snapshot. The ways the four surfaces disagreed (mode-only drift,
+  symlinked destinations) are resolved in the same release; see Fixed above.
 
 - **`.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

README.md

@@ -206,7 +206,7 @@ If you lose your age private key, you lose access to all encrypted secrets. Reco
 - **Hand-edits to agentsync-owned keys** in shared agent files (e.g. an MCP server entry in `~/.claude.json` that agentsync owns): the next `apply` overwrites them with NO foreign-collision backup, because agentsync considers them its own. Use `agentsync reconcile` (the drift classifier catches the edit and offers `[w]`rite-back) BEFORE the next apply if you want to keep them.
 - **Destination git backup is local-only**: `apply` keeps each managed destination dir (`~/.claude`, `~/.codex`, …) in its own git history so `agentsync revert` can undo a bad apply — but that history is **never pushed** (the rendered files hold `${secret:…}` references resolved to **cleartext**, so its commits may too; the `.git` dir is hardened to `0700`, which is **POSIX-only** — a Windows no-op, where NTFS ACLs are the boundary). A destination dir already under **your own** source control is detected as `foreign source control` and left un-versioned (only `agentsync-versioned` dirs are reverted; an `untracked` dir is a candidate for init). `$HOME`-level strays (Claude's `~/.claude.json`) are never versioned — agentsync never inits a repo at `$HOME`. `revert`'s "nothing is lost" guarantee covers **tracked files only**: untracked / gitignored scratch files in the dir are left untouched, never snapshotted.
 - **Plain-http / git:// plugin sources** are rejected by default to prevent MITM swap. Set `AGENTSYNC_ALLOW_INSECURE_URLS=1` for internal mirrors.
-- **Symlinked destinations** (e.g. `~/.claude.json` is a chezmoi symlink into your dotfiles repo) are rejected by default — a rename onto the path would replace the symlink with a regular file and strand your linked source. Set `AGENTSYNC_ALLOW_SYMLINK_DEST=1` to write through the symlink instead (the underlying file is updated in place; the link survives).
+- **Symlinked destinations** (e.g. `~/.claude.json` is a chezmoi symlink into your dotfiles repo) are rejected by default — a rename onto the path would replace the symlink with a regular file and strand your linked source. Set `AGENTSYNC_ALLOW_SYMLINK_DEST=1` to write through the symlink instead (the underlying file is updated in place; the link survives). The same switch governs how agentsync READS a symlinked whole-file destination (a `CLAUDE.md`, a skill, a subagent), so `apply` and every drift command — `status`, `diff`, `reconcile`, `explain` — need it: without it `status`, `diff`, `reconcile` and `explain` report the file as drifted rather than comparing through the link (and `diff` says so, with a `symlink` hunk naming the switch); with it they resolve the link and compare the file it points at, so a chezmoi setup reports clean after a successful apply. A key-merged file such as `~/.claude.json` is read through the link either way.
 - **Aider** and **Firebender**: deliberately deferred — no faithful generic projection (Aider has no MCP and only an `.aider.conf.yml` `read:` pointer for memory; Firebender's config is unverified).
 
 ## Environment overrides
@@ -215,7 +215,7 @@ If you lose your age private key, you lose access to all encrypted secrets. Reco
 | --- | --- |
 | `AGENTSYNC_HOME` | Override `~/.agentsync/` location (absolute path). |
 | `AGENTSYNC_TARGET_ROOT` | Redirect `$HOME` for testing (used by the hermetic test container). |
-| `AGENTSYNC_ALLOW_SYMLINK_DEST=1` | Permit writes to symlinked destination files (resolves the link first). |
+| `AGENTSYNC_ALLOW_SYMLINK_DEST=1` | Write through symlinked destinations, and compare through them when reading (chezmoi-managed files). Needed by `apply` and by `status`/`diff`/`reconcile`/`explain`. |
 | `AGENTSYNC_ALLOW_INSECURE_URLS=1` | Accept http:// and git:// plugin / marketplace sources. |
 | `AGENTSYNC_ALLOW_UNIMPLEMENTED=1` | Register an agent that has no implemented adapter yet (none today — every valid agent is real). |
 | `AGENTSYNC_ALLOW_PLUGIN_DRIFT=1` | Bypass the plugin-cache manifest-SHA check (after hand-editing). |

SECURITY.md

@@ -94,8 +94,12 @@ can resolve secrets into native config files. Areas of particular interest:
   diagnostics surface — native marketplace ids and source types) stay plain
   strings.
 - **Destination writes**: writes are atomic and refuse to clobber symlinked
-  destinations by default; pre-existing foreign files are backed up before
-  overwrite.
+  destinations by default (one known gap: `Writer.Write`'s convergence chmod
+  still follows a link, #248) — and, since #229, every read-side surface
+  (`status`, `diff`, `reconcile`, `explain`) refuses to read through a
+  symlinked whole-file destination under the same switch,
+  `AGENTSYNC_ALLOW_SYMLINK_DEST`, reporting it as drift instead; pre-existing
+  foreign files are backed up before overwrite.
 
 ## Sensitive files
 

docs/architecture.md

@@ -888,9 +888,45 @@ key-merge ops never are, because one agent emits several of them to one file
 (Codex's `/mcp_servers` and `/hooks` both land in `config.toml`). Merged keys
 are walked in sorted pointer order, so `status --json`, `diff` and `reconcile`
 list them reproducibly. Each surface keeps its own presentation on top: `status`
-re-partitions whole-file rows ahead of key rows and folds permission drift into
-the class, `diff` masks and compares text, `reconcile` excludes an orphan
-another agent still renders, `explain` groups by owner.
+re-partitions whole-file rows ahead of key rows, and both `status` and `explain`
+fold permission drift into the class of a content-clean whole file — measured
+against `op.Mode`, the mode the next apply chmods to, which is the same question
+`diff`'s `mode` hunk asks (`planItem.opModeDrifted`, one predicate behind all
+three, so they cannot disagree). `reconcile` still ignores mode entirely; that gap is
+#245. `diff` masks and compares text, `reconcile` excludes
+an orphan another agent still renders, `explain` groups by owner.
+
+A **symlinked** destination is read through only when
+`AGENTSYNC_ALLOW_SYMLINK_DEST=1` — the same switch under which `iox.AtomicWrite`
+writes through the link. `iox.SymlinkDestAllowed` is the one reading of the
+switch, and `destReadPath` (`internal/cli/destread.go`) is the read-side gate
+that the three whole-file destination facts — the content hash, the permission
+bits and the text — pass through. Unset, all four surfaces answer an opaque
+sentinel that can never equal a content hash, so the classifier sees a changed
+destination (`drift`, or `conflict`/`foreign-collision` by its usual table);
+`diff` prints a `symlink` hunk naming the switch rather than reading through and
+reporting no difference; `reconcile` shows the SHA display instead of a text
+diff, and its `[w]`rite-back refuses to capture through the link with advice
+that omits `[o]verride` (#248: `Writer.Write`'s mode arm chmods through a link
+before the policy is consulted). Set, all four resolve the link and compare the
+file it points at, so a converged chezmoi setup reports `clean`; a link that
+does not resolve answers a second sentinel so the advice is "fix the link", not
+"set the switch". A link to a FIFO, device or directory is a shape problem the
+switch cannot fix: it is refused as a bare one is, and `diff` prints a `shape`
+hunk for both — as it does for a path it cannot stat, which shares the token.
+The mirror is a policy, not a prediction: `apply` itself only fails on a
+symlinked destination when the content differs (its convergence read follows
+the link), so the unset answer
+means "a managed regular file became a link you have not opted into — that is
+drift", not "the next apply would fail". The rule covers **whole-file**
+destinations only; the symlink sentinel is a whole-file-only policy signal. A
+key-merged destination is decoded through the link on every surface, as `apply`
+treats it (a converged symlinked key-merge destination is a no-op plus a chmod
+through the link, a differing one is `ErrSymlinkDest` — exactly as for a
+whole-file destination, so the read-through already has apply-parity): refusing
+it would make every owned pointer of a chezmoi `~/.claude.json` classify against
+an absent value permanently, because the classifier has no per-pointer sentinel
+to carry, with `reconcile`'s `[d]` showing `<absent>` and its `[w]` failing.
 
 ---
 
@@ -907,7 +943,8 @@ All present in v1.0 (`internal/iox`, `internal/render`, `internal/state`):
    bans `os.UserHomeDir()` in `_test.go`.
 4. **First-apply backups** — the `foreign-collision` case copies the pre-existing
    destination into `.state/backups/<ts>/` before writing. Symlinked
-   destinations are refused by default.
+   destinations are refused by default — and, on the read path, classified as
+   drift rather than read through (§6).
 5. **Manifest-SHA pinning** — every plugin records a `tree:v1:` content hash
    over its *entire* cache tree (every projected component body — skills,
    command/subagent markdown — not just `plugin.json`, excluding `.git/`), so a

docs/capability-matrix.md

@@ -585,8 +585,12 @@ in the [README](../README.md#known-limits); the highlights:
 - **Insecure sources** — `http://` and `git://` plugin/marketplace sources are
   rejected by default (MITM protection); override with
   `AGENTSYNC_ALLOW_INSECURE_URLS=1`.
-- **Symlinked destinations** are rejected by default; override with
-  `AGENTSYNC_ALLOW_SYMLINK_DEST=1`.
+- **Symlinked destinations** are rejected by default, for both writing and
+  drift comparison (`status`/`diff`/`reconcile`/`explain` report a symlinked
+  whole-file destination as drifted rather than reading through the link; a
+  key-merged file such as `~/.claude.json` is read through either way);
+  override with `AGENTSYNC_ALLOW_SYMLINK_DEST=1`, which `apply` and the four
+  drift commands (`status`, `diff`, `reconcile`, `explain`) then need.
 - **Planned / deferred**: Aider and Firebender (see [Breadth tier](#breadth-tier)
   § "Deliberate exclusions").
 

docs/components.md

@@ -49,7 +49,9 @@ is the only package that depends on nearly all the others.
   `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`.
+  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`,
   `revert`, `status`, `diff`, `reconcile`, `import`, `doctor`, `check`,
   `mcp {add,remove,list,enable,disable}`,
@@ -59,7 +61,7 @@ is the only package that depends on nearly all the others.
   `migrate subagents`, `explain <path>`,
   `version`.
 - **Depends on:** adapter, source, state, secrets, paths, render, marketplace,
-  project, drift, git, ui, log.
+  project, drift, git, iox, ui, log.
 - **Files:** `root.go` + one file per command group + shared helpers
   (`destread.go`, `planwalk.go`).
 
@@ -498,7 +500,9 @@ cleartext secrets the rendered files already contain).
 ### `internal/iox`
 Atomic file IO and locking.
 - **Key:** `AtomicWrite(dest, data, mode)`; `Lock`/`AcquireLock`/
-  `AcquireLockTimeout`; `ErrSymlinkDest`; `AllowSymlinkDestEnv`.
+  `AcquireLockTimeout`; `ErrSymlinkDest`; `AllowSymlinkDestEnv`;
+  `SymlinkDestAllowed` (the one reading of it, shared with `internal/cli`'s
+  read-side gate).
 - **Files:** `atomic.go`, `lock.go`.
 
 ### `internal/jsonkeys`

docs/concepts.md

@@ -120,7 +120,10 @@ recovery beyond your own source control of the destination.
 **clean** — both mean `apply` has nothing left to do, and the distinction
 above is bookkeeping the classifier and `status --json` need, not something a
 human scanning the report benefits from. `status --legend` prints this table
-(as a CLI reference); `status --json` always reports the real class.
+(as a CLI reference); `status --json` always reports the real class. One fold
+applies to both: a clean or converged whole file whose permission bits differ
+from what `apply` writes is reported as **drift**, because the next apply
+changes it.
 
 Granularity is **per-key** for structured files (JSON/JSONC/TOML, tracked by
 JSON pointer) and **per-file** for everything else. Keys agentsync never wrote

docs/user-guide.md

@@ -1029,8 +1029,13 @@ exclusive. Every `list` accepts `ls`, and every `remove` accepts `rm`. `status
 --json` and `diff [<path>] --json` emit
 the structured report instead of the formatted one, suitable for CI gates and
 dashboards (`status --json` is never collapsed — it carries every tracked file;
-`diff --json` masks the same resolved secrets the formatted diff does). For a
-gate that should **fail the build** on drift, add `--exit-code`: `status
+`diff --json` masks the same resolved secrets the formatted diff does; its
+`pointer` field is an RFC-6901 pointer for a merged key, and one of three
+pseudo-pointers for a whole-file finding that is not a text difference —
+`mode` for a content-identical permission change, `symlink` for a symlinked
+destination agentsync is not comparing through, `shape` for a destination that
+is not a regular file, or cannot be stat'd). For a gate that should **fail the
+build** on drift, add `--exit-code`: `status
 --exit-code` / `diff --exit-code` exit `2` when drift/hunks exist and `0` when
 clean (exit `2` is distinct from the generic error exit `1`, and prints no extra
 error line). Interactive prompts (e.g. the scope menu) always go to **stderr**,
@@ -1058,7 +1063,7 @@ and the complete environment-variable table. The ones you'll reach for most:
 | Env var | Purpose |
 |---|---|
 | `AGENTSYNC_HOME` | Override the `~/.agentsync/` location. |
-| `AGENTSYNC_ALLOW_SYMLINK_DEST=1` | Write through symlinked destinations (e.g. chezmoi-managed files). |
+| `AGENTSYNC_ALLOW_SYMLINK_DEST=1` | Write through symlinked destinations, and compare through them when reading (e.g. chezmoi-managed files). Needed by `apply` and by `status`/`diff`/`reconcile`/`explain`. |
 | `AGENTSYNC_ALLOW_INSECURE_URLS=1` | Accept `http://`/`git://` plugin/marketplace sources. |
 | `AGENTSYNC_ALLOW_OFFLINE_VERIFY=1` | Let `check` validate reference *shape* only, skipping resolution (CI without an age key). |
 | `AGENTSYNC_NO_UPGRADE_NOTICE=1` | Never show the one-time first-run-after-upgrade notice. |