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
HEAD or 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.
--index is not decoration for the geometries the criterion still releases (a staged deletion among them); do not drop it when re-wording.
- A bare
pop is 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.
The recovery's undo/audit half is stated more strongly than it holds:
git stash popis not the inverse on a released geometry, and a missing receipt is silentTwo findings from the documentation half of #1274's review (cycle
cyc20260916-143539, merged as321323ae). 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 silentDEVELOPMENT.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 returnstr | Nonefor 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")raisesOSError) on headb090793d(emrg/server/scheduler.pysha256[:16]94b658e2c26b454e, asserted from the loaded module):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.pyhas the same gap in a weaker form: it re-derives the path and printsreceipt: <path>without checking that the file exists (measured the same way). Fix: report the missing receipt in the returned detail (the script already has areceipt: could not be writtenbranch it can never reach), and/or log it.2. "Every byte one
git stash popaway" is not the inverse on a geometry the criterion releasesThe claim is in three places:
DEVELOPMENT.md,scripts/recover-worktree.py's stdout, and the action's own docstring. OnD f.txt(a staged deletion,git rmon unchanged content — a geometry the criterion answers recoverable), after the action's own recovery, each spelling run in a fresh repository:D f.txtgit stash pop— what the doc and the script's stdout sayD f.txtgit stash apply stash^{/<msg>}D f.txtgit stash apply --index stash^{/<msg>}— what the receipt saysD f.txtgit stash pop --indexD f.txtSo 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
--indexcomment 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
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.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.