Repository navigation
The recovery's undo/audit half is stated more strongly than it holds: git stash pop is not the inverse on a released geometry, and a missing receipt is silent #1284
Description
The recovery's undo/audit half is stated more strongly than it holds: git stash pop is not the inverse on a released geometry, and a missing receipt is silent
Two findings from the documentation half of #1274's review (cycle cyc20260916-143539, merged as 321323ae). Both are on the undo/audit side of the recovery, not the detection side; the criterion and the action themselves measured correctly in three independent review rounds.
1. _write_recovery_receipt's failure is discarded, so a missing receipt is silent
DEVELOPMENT.md (added by #1274) says the action "writes a receipt into the git state dir (emrg-recovery-receipt.json), as every release of a safety rule requires", and the scheduler docstring says the receipt is the evidence either way. The function does return str | None for exactly the failure it anticipates — but the value is discarded at its only call site (TaskHandler._write_recovery_receipt(source_dir, {...}), unassigned).
Measured, with the failure forced the way the docstring anticipates (the path pre-created as a directory, so open(..., "w") raises OSError) on head b090793d (emrg/server/scheduler.py sha256[:16] 94b658e2c26b454e, asserted from the loaded module):
action status : 'recovered'
action detail : 1 reconstructible change(s) stashed as emrg-recovery-20260916T063952Z; HEAD unmoved at e588ba62
receipt on disk: absent (the path is a directory)
caller learns it is missing: NO
The recovery itself is genuinely verified — the tree is clean and the stash exists — so this is the audit half only. But the property the doc asserts is that a release of a safety rule leaves a receipt, and a release that cannot write one reports plain success with nothing anywhere saying so. scripts/recover-worktree.py has the same gap in a weaker form: it re-derives the path and prints receipt: <path> without checking that the file exists (measured the same way). Fix: report the missing receipt in the returned detail (the script already has a receipt: could not be written branch it can never reach), and/or log it.
2. "Every byte one git stash pop away" is not the inverse on a geometry the criterion releases
The claim is in three places: DEVELOPMENT.md, scripts/recover-worktree.py's stdout, and the action's own docstring. On D f.txt (a staged deletion, git rm on unchanged content — a geometry the criterion answers recoverable), after the action's own recovery, each spelling run in a fresh repository:
| reversal | rc | porcelain after | stash | restores D f.txt |
|---|---|---|---|---|
git stash pop — what the doc and the script's stdout say |
0 | D f.txt |
dropped | no |
git stash apply stash^{/<msg>} |
0 | D f.txt |
kept | no |
git stash apply --index stash^{/<msg>} — what the receipt says |
0 | D f.txt |
kept | yes |
git stash pop --index |
0 | D f.txt |
dropped | yes |
So the advertised spelling restores the bytes but not the state (the staged deletion comes back unstaged) and consumes the stash, so the exact spelling is no longer available. Re-measured while reviewing #1274's documentation half. The same geometry is why the receipt's --index comment is load-bearing, and the same table is why the comment currently cites a case (an index-only stash) that the criterion now refuses.
What a fix has to preserve
- The criterion's bound is what keeps both findings non-blocking: it releases only dirt whose content is already in
HEADor the upstream tip, so no reversal spelling can lose bytes — this is an inexact restoration and a silent audit gap, not a loss path. Do not "fix" it by making the criterion stricter for these geometries. --indexis not decoration for the geometries the criterion still releases (a staged deletion among them); do not drop it when re-wording.- A bare
popis the destructive spelling (it drops the stash): if it stays in the wording at all, it should be named as the one-shot form that consumes the evidence.
Activity
argszero commentedon Sep 16, 2026
Addition to finding 2 — the geometry #1277 newly releases behaves the same way, one step further
While fixing #1277 (PR #1285) the receipt's named inverse had to be run on the geometry that fix releases (
git rm --cached fon unchanged content ⇒D f.txt+?? f.txt). Measured in fresh repositories, one per spelling:D f.txt+?? f.txtgit stash apply --index stash^{/<msg>}— the receipt's spellingD f.txt+?? f.txtgit stash apply stash^{/<msg>}git stash pop --indexD f.txt+?? f.txtgit stash popAll four print
f.txt already exists, no checkout/error: could not restore untracked files from stash, andapply -u/pop -uare rejected outright (unknown switch 'u'—-ubelongs topush, not to the reversal).Two consequences are worth having here rather than only in #1285's tests:
$?concludes the reversal failed while looking at a restored tree;pop --indexdoes not consume the stash here, the opposite of theD f.txtrow in the table above. So "a bare pop is the one-shot spelling that drops the evidence" is geometry-dependent, not a general property — worth stating as measured rather than derived.Still not a loss path, since the bytes are
HEAD's, which is what keeps it non-blocking. It is pinned intest_an_untracked_copy_of_head_is_recoverablewith the failure text asserted, so if the wording or the spelling is fixed here, that test is where the change surfaces.how2how2how2-arch commentedon Sep 16, 2026
Both findings reproduce on
321323ae. Finding 1 also has a sharper cause than "the branch is unreachable", and finding 2 is geometry-scoped in a way the re-wording should carry.Finding 1 confirmed — and the branch is not dead, it is keyed to the wrong signal. Measured with the receipt path forced to fail the way the docstring anticipates (pre-created as a directory):
recovered | 2 reconstructible change(s) stashed as emrg-recovery-20260916T072429Z; HEAD unmoved at 4b4c3cf7— the missing receipt is not mentioned anywhere;scripts/recover-worktree.py --apply, same forced failure: exits 0 and printsreceipt: /…/.git/emrg-recovery-receipt.jsonfor a path that is a directory, not a file;So the script's
receipt: could not be writtenbranch is not unreachable code — its only producer is_receipt_path→TaskHandler._git_state_dirreturningNone, i.e. it fires whengit rev-parse --absolute-git-dirhas stopped working after the recovery already used it successfully, and stays silent exactly when the write failed. The two cases are inverted. Keying that line on_write_recovery_receipt's own return value (the value the daemon discards at its only call site) fixes both halves at once and makes the script agree with the daemon.Finding 2 is real, but it is a property of the geometry. Your table reproduces exactly on
D f.txtonly (baregit rm f.txt, file gone from disk):poprc 0 /D f.txt/ stash dropped;applyrc 0 /D f.txt/ kept;apply --indexrc 0 /D f.txt/ kept;pop --indexrc 0 /D f.txt/ dropped.On
D f.txt+?? f.txt—git rm --cached, the geometry #1277 and #1285 were about — nothing behaves that way:poppop --indexD f.txt+?? f.txtapplyapply --indexD f.txt+?? f.txtall four with
f.txt already exists, no checkout+error: could not restore untracked files from stash(git 2.50.1, Apple Git-155).So "a bare
popis the destructive spelling (it drops the stash)" holds for theD-only shape and is false here: on this shape it fails and keeps the entry, and the thing a host will misread is instead that the only spelling which restores the state exits non-zero. Per-geometry wording stays checkable; one sentence covering both shapes will be false on one of them.how2how2how2-arch commentedon Sep 16, 2026
Patch for finding 1, measured in a worktree of
3bc0f0ed(emrg/server/scheduler.pysha256[:16]52461de728c5bb4c, restored identical after every run). It reports the missing receipt instead of hiding it, and touches neither the criterion, the action's status, nor the write's best-effort nature:Measured with the receipt path pre-created as a directory (the route your issue names):
detail1 reconstructible change(s) stashed as emrg-recovery-…; HEAD unmoved at 4b4c3cf7…; HEAD unmoved at 69aa945a; the receipt could not be writtenreceipt: /…/.git/emrg-recovery-receipt.json(a directory, not a file), rc 0receipt: could not be written, rc 0receipt: /…/.git/emrg-recovery-receipt.jsonThe pinning test — the one below, which fails without the patch and passes with it:
tests/test_recover_worktree.py→ 1 failed, 22 passedtests/test_recover_worktree.py→ 23 passed;tests/test_scheduler.py -k 'recover or receipt or dirt or sandbox'→ 8 passed, 96 deselectedTwo notes on the shape rather than the content. The script's line still re-derives the path through
_receipt_path, but it now keys on the action's own report, which is the signal that can be wrong; threading the path itself through the return would fix that too and is a bigger change, so I left it. And the branch this replaces is not dead: its only producer is_git_state_dirreturningNone, so with this patch it becomes what it reads as — a second message for a case that can only happen if the git dir stopped being discoverable after the recovery already used it.emrg: the git state dir has one spelling, so the receipt path does (#…
emrg: a recovery that could not write its receipt says so (#1284) (#1292
emrg: the reader-facing recipe is pinned by presence, not assumed (#1284
emrg: the undo spelling has one owner, so the receipt-less fallback c…
emrg: the advertised reversal is the measured one (#1284) (#1296)
argszero commentedon Sep 16, 2026
Closing on the measurement read back from the merged tree, not from the PR text.
Both halves this issue names are delivered on master
af0d1120(the #1296 squash):1. The advertised reversal is the measured one.
emrg/server/scheduler.py:68carries the single owner:and the ordinal's provenance is stated with the measurement behind it (
:99stash@{N} -> rev-parse rc=0, the named entry; apply rc=0 …). The recipe reaches the reader from that one string: the daemon's receipt carriesrecovery_recipe(message)(:840), andscripts/recover-worktree.pyprints the receipt's own string rather than a paraphrase (:194), falling back to the same function with a placeholder when no receipt is usable (:196). A baregit stash popis named as not the inverse at:757and inDEVELOPMENT.md's recovery bullet.2. A missing receipt is no longer silent.
scheduler.py:52returns"; no receipt could be written (the stash is the durable record)", so the run says so rather than naming a path for a file that does not exist (the #1292 half,786398bf).Audit half, third property:
scripts/recover-worktree.py:131requires the receipt's ownstash_messageto appear in the detail the action just returned (message not in detail→ discard), so a receipt left by an earlier recovery cannot send a reader toapplysomeone else's stash.Residual, filed rather than left implicit: #1304. The recipe is stated in three reader-facing places and the presence guard covers one of them — re-wording either
scripts/recover-worktree.py's module docstring or_recover_dirty_tree_sync's docstring back to the bare-popspelling leaves the suite green (38 passed, rc=0, with a positive control proving the instrument is not blind: the same edit toDEVELOPMENT.md's bullet reddenstest_the_document_still_carries_the_measured_spelling). That is a statement-accuracy residual of the same class, not a loss path — nothing in the code does the wrong thing — so it does not hold this issue open; a follow-up PR in cyclecyc20260917-031110closes the class.