fix: let chezmoi apply run unattended on assist, and name which half of pai-git-sync failed

[gwarf]

fix: let chezmoi apply run unattended on assist, and name which half of pai-git-sync failed

Two small changes with one cause.

pai-git-sync.service runs chezmoi apply from a systemd user unit on assist, a
long-running headless VM. Four of the mail configs are templated with rbw calls
that resolve at RENDER time, so applying them needs an unlocked Bitwarden agent
and a working pinentry. Under systemd there is neither:

rbw get: failed to read password from pinentry:
  pinentry error: Inappropriate ioctl for device <Pinentry>

The assist ignore block already excluded three of those four — .config/aerc,
.config/emails and .msmtprc — for exactly this reason. .config/vdirsyncer
was the omission, so it is added alongside its siblings. Ignoring leaves any
already-rendered file in place (ignore means unmanaged, not deleted), and
vdirsyncer's timer is disabled on assist, where it last synced in May.

The durable fix for vdirsyncer is a runtime credential read rather than a
render-time one, as torpedo already does via LoadCredentialEncrypted= on
vdirsyncer.service. That cannot be copied to assist as-is: systemd-creds there
(systemd 255.4) has no --user option and the VM has no TPM, so user-scoped
credential encryption is unavailable. Verified rather than assumed — vdirsyncer's
expand_fetch_params is key-agnostic, so client_id.fetch etc. would work, but
there is nowhere on that host to seal the credential for a user unit. That design
belongs to the assist MUA rollout, not to this gate.

Second change: sync_repo now returns 2 for a post-sync hook failure instead of
collapsing it into the same exit 1 as a git failure, and the caller reports the
two halves separately. This unit is named for git sync, so a bare exit 1 from
chezmoi apply reads as a git problem — on 2026-08-31 that cost a detour into
the git half of a unit whose git half was already clean. The summary lines are
emitted on success too, so the journal states the outcome instead of leaving it
to be inferred from an absence of errors.

shellcheck clean.

[github-actions]

✅MegaLinter analysis: Success

Descriptor	Linter	Files	Fixed	Errors	Max errors	Warnings	Elapsed time
✅ REPOSITORY	betterleaks	yes		no		no	0.78s
✅ REPOSITORY	checkov	yes		no		no	12.29s
✅ REPOSITORY	git_diff	yes		no		no	0.05s
✅ REPOSITORY	osv-scanner	yes		no		no	0.55s
✅ REPOSITORY	secretlint	yes		no		no	2.11s
✅ REPOSITORY	syft	yes		no		no	1.48s
✅ REPOSITORY	trivy-sbom	yes		no		no	0.65s

See detailed reports in MegaLinter artifacts
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

Show us your support by starring ⭐ the repository

[gwarf]

_MREVIEW — Automated Review

Platform: github | Ref: #23 | CI: pass (2 checks, both pinned to c7b7ff0a)
Rules applied: global

Verdict: approved; three points below, none blocking. The stated cause is not
just plausible, it is reproduced — and the ignore entry closes the whole class,
not just the one file.

Premise check

Confirmed on assist itself rather than reasoned from the diff:

• The journal records the failure 12 times in 14 days, always the same single
  path: chezmoi: .config/vdirsyncer/config: template: private_readonly_config.tmpl:65:16 … error calling rbw … exit status 1,
  preceded by pinentry error: Inappropriate ioctl for device. Line 65 sits in
  the template's else branch, which is the branch assist takes.
• It is the only failing path — 12 of 12 occurrences, no other file.
• Sweeping the source tree for render-time credential calls (the chezmoi rbw
  template function, as opposed to a runtime passwordeval / PassCmd /
  passwordCmd string) returns exactly two files: aerc/private_operas.yaml.tmpl
  and vdirsyncer/private_readonly_config.tmpl. The first was already ignored on
  this host; this change adds the second. So after this merge no render-time
  secret call remains for assist.
• The supporting claims hold too: vdirsyncer.timer is disabled/inactive
  here, the already-rendered ~/.config/vdirsyncer/config is present with an
  mtime of 2026-05-19, and the reason for not doing the runtime-credential fix
  checks out — systemd 255.4, systemd-creds encrypt --user returns
  unrecognized option '--user', and there is no /dev/tpm*.

Important

File	Finding	Confidence	Rule
home/private_dot_local/bin/executable_pai-git-sync:100-102	set -e is disarmed inside (sync_repo "$repo") || rc=$?. A command that is the left operand of || runs with errexit suppressed, and that suppression applies inside the subshell. Reproduced: set -euo pipefail; f() { false; echo CONT; return 0; }; rc=0; (f) || rc=$? prints CONT and yields rc=0, while a bare (f) aborts. Two commands in sync_repo are unguarded and therefore fail silently: git add -A (line 54) and git config core.hooksPath (line 46). With the first, nothing is staged, no commit is made, rebase and push are no-ops, the function returns 0 — and this change now prints git=ok for a run that captured nothing. The mechanism is pre-existing; what is new is the line affirming success. The moved comment on line 100 also states the opposite of what happens: || rc=$? is what keeps the loop alive, and the subshell's real effect here is to switch errexit off. Guarding both commands the way the other git calls are guarded fixes it.	95/100	global
home/private_dot_local/bin/executable_pai-git-sync:117-121	post-sync-hook=ok prints even when the hook never ran. If the chezmoi repo fails in its git stage — fetch, commit, rebase conflict, push, or missing .git/ — chezmoi apply at line 83 is never reached, and the journal reads git=FAILED (chezmoi) followed by post-sync-hook=ok. hook_failed has two states, so "did not run" is indistinguishable from "ran clean". That is the worst case to mislabel: git failed and the rendered targets are stale, while the summary says the apply half is healthy. A hook_ran flag and a third state (post-sync-hook=not-run) covers it.	92/100	global

Minor

• home/.chezmoiignore:39-41 — the new comment says "aerc/emails/msmtprc/vdirsyncer
  templates resolve rbw at render time". Only aerc and vdirsyncer do. .msmtprc
  uses passwordeval, .mbsyncrc uses PassCmd, both imapnotify templates use
  passwordCmd — all runtime, all render fine headless — and .config/emails has
  no rbw reference at all. Those entries are ignored here for the ordinary
  headless-box reason, same as kitty and waybar. The reason given is right;
  the list of files it applies to is not. Narrowing it to aerc and vdirsyncer
  keeps it true.

Checked and clean: the return 2 sentinel is unambiguous (no unguarded command
in sync_repo can exit 2 — basename maxes at 1, git add returns 128,
git config can return 2 only for a section-less key and core.hooksPath
carries a section); sync_repo returns 0 for the non-chezmoi repo, since an if
with no true condition and no else has status zero; ${git_failed% } trims
correctly for zero, one and two entries; and the directory-level ignore does
cover the subtree — .config/aerc, same shape in the same host block, has zero
managed entries including children, so no /** is needed.

---

Posted by _MREVIEW v1.0 on 2026-09-01 CEST

[gwarf]

Review addressed — d4f72692

All three confirmed before changing anything. Thanks — finding 1 was a real hole in exactly the property this PR exists to provide.

1. errexit suppressed inside sync_repo — fixed, with a correction to the failure mode

The mechanism reproduces exactly as described:

$ bash -c 'set -euo pipefail; f() { false; echo CONT; return 0; }; rc=0; (f) || rc=$?; echo rc=$rc'
CONT
rc=0
$ bash -c 'set -euo pipefail; f() { false; echo CONT; return 0; }; (f); echo after'
$ echo $?
1

One refinement worth recording, because it changes which scenario the guard actually earns its place on. I first tried a corrupted index — and there the old code already returned 1, because git diff --cached --quiet also fails on a broken index, so if ! … entered the block and the existing git commit guard caught it. So that case was never exposed.

The case that does bite is a partial add. With an unreadable file in the tree:

$ git add -A
error: open("unreadable.txt"): Permission denied
fatal: adding files failed
$ echo $?          # 128
$ git diff --cached --name-only   # empty — nothing staged
$ git diff --cached --quiet; echo $?   # 0 — so the commit block is SKIPPED

Nothing staged means git diff --cached --quiet succeeds, the commit block is skipped, and git rebase / git push both no-op successfully. Measured on a scratch repo with a real remote:

	sync_repo returns	output	readable.txt committed?
before	0	Everything up-to-date	no
after	1	fatal: adding files failed	no

So the old code reported success for a run that silently dropped a modified file, and the git=ok line this PR adds would have asserted it. That is the defect.

git add -A is now guarded and returns 1.

One deliberate deviation from the suggestion: git config core.hooksPath is checked but non-fatal — it notifies and continues. An unset lint gate degrades commit quality without making the sync wrong, and returning 1 there would wedge the sync of every repo over a lint concern. Flagging it rather than silently diverging; happy to make it fatal if you'd rather.

2. post-sync-hook=ok when the hook never ran — fixed

Correct, and it was the worst case to mislabel. Now three states, derived from the chezmoi repo's own return code rather than a new flag — 0 → ok, 2 → FAILED, anything else → not-run:

scenario	output	exit
both ok	git=ok / post-sync-hook=ok	0
hook failed	git=ok / post-sync-hook=FAILED	1
chezmoi git-stage failed	git=FAILED (chezmoi) / post-sync-hook=not-run	1
other repo git failed	git=FAILED (.claude) / post-sync-hook=ok	1
chezmoi missing .git/	git=FAILED (chezmoi) / post-sync-hook=not-run	1

Row 3 is the one you identified; it previously read post-sync-hook=ok.

3. .chezmoiignore comment overstated the file list — fixed

Verified per file: .msmtprc render-time 0 / runtime 2 (passwordeval), .mbsyncrc 0 / 3 (PassCmd), both imapnotify templates use passwordCmd, and .config/emails has no rbw reference at all. Only aerc and vdirsyncer resolve at render time. Comment narrowed to those two, with the rest attributed to the ordinary headless-box reason.

Also

Corrected the loop comment, which claimed the subshell keeps the loop alive — it doesn't. || rc=$? does that, the subshell contains sync_repo's cd, and the pair is what switches errexit off. The comment now says so, since that's the footgun a future editor needs at that line.

shellcheck clean.

[gwarf]

_MREVIEW — Round 2 (d4f72692)

Verdict: approved. All three points addressed, nothing open. One wording
observation below that needs no change.

Re-derived from the merge base (e103dbb4) and read both files in full, not just
the paths named in your reply.

On the deliberate deviation — keep core.hooksPath non-fatal

Agreed, and for the reason you gave. The lint gate is a quality guard on what
gets committed; it is not a correctness property of the sync. Making it fatal
inverts the priority of a script whose whole job is unattended capture — a
failure to capture is strictly worse than a commit that skipped a lint pass, and
it would take out the sync of every repo over a condition affecting only one.
The tg_notify is what makes it not-silent, which is the property that was
actually missing. Right call.

Verified

• git add -A (:63-66) now guarded, returns 1. Your correction to the failure
  mode is the more interesting half and matches what I see: a corrupted index was
  never exposed, because git diff --cached --quiet fails on it too and the
  existing git commit guard caught it. The partial add is the case that bit —
  nothing staged, so --quiet succeeds, commit is skipped, rebase and push
  no-op, and the function returns 0.
• The three-state hook_state (:106, :120-132, :142) is correct. I traced
  every row of your table against the code rather than taking it: .claude
  cannot reach the 2) branch since the hook block is chezmoi-only; a chezmoi
  git-stage failure and a missing .git/ both leave the initial not-run
  standing, which is the case that previously read ok; and not-run on its
  own does not force exit 1, which is right — it is only reachable alongside a
  git failure that already does.
• Deriving the state from the return code rather than a separate flag is better
  than what I suggested. One variable, no way for the flag and the code to
  disagree.
• Remaining unguarded commands in sync_repo, since errexit cannot catch them:
  basename at :33 and date inside the commit message at :71. Neither can
  fail with a well-formed argument, and neither corrupts state if it did. Clean.
• .chezmoiignore comment now claims only what is true.

Minor — no change needed

:115-117 says the subshell and || rc=$? "both together suppress errexit".
Measured: the || alone is sufficient — f || rc=$? with no subshell prints
past a false and yields 0, as does if f; then. The subshell is there for
sync_repo's cd, and it is the || context that disarms errexit. The
operative half of the comment, that every command must check its own status, is
the part a future editor needs and it is correct as written.

---

Posted by _MREVIEW v1.0 on 2026-09-01 CEST

[gwarf]

LGTM

[gwarf]

Round 2 addressed — 26402ba0

Two changes, and one of them you explicitly said needed none.

The errexit attribution — corrected anyway

You were right on the facts, and I verified before changing it:

$ bash -c 'set -euo pipefail; f() { false; echo CONT; return 0; }; rc=0; f || rc=$?; echo rc=$rc'
CONT
rc=0
$ bash -c 'set -euo pipefail; f() { false; echo CONT; return 0; }; (f); echo after'   # subshell, no ||
$ echo $?
1

The || alone suppresses; a bare (f) still aborts. The subshell contributes nothing to it.

I corrected it despite the "no change needed" because of the specific way the misreading fails: a future editor who wants errexit back reads "both together suppress errexit", removes the subshell, and silently loses the cd isolation the loop depends on — sync_repo does cd "$repo", so without it the cwd leaks across iterations. Getting the attribution wrong points the fix at the wrong line. The comment now names the || as the cause and says to keep the subshell.

Comment volume

Baptiste's call, on the same PR: too long. 22 added comment lines → 12.

The main change is structural rather than cosmetic — the errexit fact is stated once, at the call site, instead of repeated at each guard. A guarded command needs no comment explaining that it's guarded; only the non-obvious reasons keep a line:

• why core.hooksPath is non-fatal
• why an unguarded add produces a false success rather than a visible failure
• why 2 is a distinct return code
• why the hook has a third state
• not to drop the subshell

Everything else — the measurements, the before/after table, the reasoning — lives in the commit log and this thread, which is where provenance belongs.

On your other notes

Agreed on basename (:33) and date (:71) being clean: neither can fail with a well-formed argument and neither corrupts state if it did. And agreed that deriving hook_state from the return code beats a separate flag — one variable, no way for the two to disagree. That was your suggestion improved by your own review, which is a good sign for the loop.

shellcheck clean.

[gwarf]

Verdict: approved at 26402ba0. Nothing open.

GitHub refuses an approving review here (own PR, reviews post with your token), so this is the verdict on the record instead.

Rounds 1 and 2 are in the thread above; the short version:

• The stated cause is reproduced, not assumed — 12 occurrences in 14 days in the journal on assist, always private_readonly_config.tmpl:65:16, the branch that host takes. Sweeping for render-time credential calls returns exactly two files, aerc/private_operas.yaml.tmpl (already ignored) and vdirsyncer (added here), so the ignore entry closes the class rather than one file.
• git add -A and git config core.hooksPath are now guarded, which is what errexit could never do for them from inside that || context. Non-fatal on core.hooksPath is the right call: capture failing is worse than a commit that skipped a lint pass.
• The three-state hook_state is correct on every path I traced, including the one that motivated it — a chezmoi git-stage failure now reads not-run instead of ok.
• 26402ba0 and 9a028064 are comment-only: code stripped of comments is byte-identical (98 lines each) to the head I reviewed at round 2, so the verdict carries unchanged.