Development record
capture.commitTransition writes the destination file before removing the source (workflow.go:255-273); a non-ENOENT os.Remove(src) failure (EPERM/EROFS/EIO — e.g. a read-only remount or restrictive ACL on the source status dir) returns an error after dst already landed, with no rollback of dst. The same issue id is now present in two status directories (e.g. both open/ and resolved/). findIssue (alloc.go:383) then rejects any further transition on that id as ErrDuplicateIssueID, so it can never again be resolved or wontfixed on a healthy filesystem — no repair verb exists; recovery requires manually deleting one copy. scanLedger-based reads (List/Status) do not dedupe either, so the id also double-counts. Confirmed with a real non-ENOENT unlink failure (ext4 immutable attribute on the source dir, which blocks unlink even as root, unlike chmod). Reproducing test: internal/core/capture/workflow_test.go, TestTransitionRemoveFailureDoesNotStrandIssueInTwoDirs.
capture.commitTransition writes the destination file before removing the source (workflow.go:255-273); a non-ENOENT os.Remove(src) failure (EPERM/EROFS/EIO — e.g. a read-only remount or restrictive ACL on the source status dir) returns an error after dst already landed, with no rollback of dst. The same issue id is now present in two status directories (e.g. both open/ and resolved/). findIssue (alloc.go:383) then rejects any further transition on that id as ErrDuplicateIssueID, so it can never again be resolved or wontfixed on a healthy filesystem — no repair verb exists; recovery requires manually deleting one copy. scanLedger-based reads (List/Status) do not dedupe either, so the id also double-counts. Confirmed with a real non-ENOENT unlink failure (ext4 immutable attribute on the source dir, which blocks unlink even as root, unlike chmod). Reproducing test: internal/core/capture/workflow_test.go, TestTransitionRemoveFailureDoesNotStrandIssueInTwoDirs.
Frontmatter
| id | iss-186 |
|---|---|
| type | issue |
| lifecycle | resolved |
| severity | minor |
.abcd/work/issues/resolved/iss-186-capture-transition-remove-failure.md
open on GitHub ↗ · commit history ↗
mentions3
- dcn-26080500000000062026-08-05 — bug-hunt loop round 1: iss-184 (critical, guard tokenizer heredoc misparse) fixed at root cause, in two passes after a pre-PR adversarial security review caught the first pass narrowing the hole rather than closing it. `internal/core/guard/tokenize.go`'s `<<` handling already special-cased the literal-digit arithmetic-shift form (`$((1<<20))` is not a heredoc), but `isDelimStart` accepts any identifier-shaped word, so `$((1<<shift))` — an identifier operand — still read as a heredoc delimiter, and `skipHeredocBodies` then silently consumed every remaining line of the command looking for a "shift" terminator line that never comes, dropping later commands (a force-push, an `rm -rf`, anything) from ever reaching command position — a silent allow, not the loud fail-open the hook's own doc comment promises for anything the tokenizer cannot answer confidently. The first pass made `skipHeredocBodies` report whether it actually found each pending heredoc's terminator, turning a miss into `ErrUnparsableCommand`. The security review then constructed the exploit this missed: an attacker supplying a later line that happens to equal the misread "delimiter" (e.g. appending a bare `shift`) still finds a match, so the swallow still succeeds silently — the fix only closed the *unterminated* half of the class, not the *coincidentally-terminated* half, which is the one an attacker actually controls. The real fix is classification, not detection-of-failure: a heredoc delimiter word immediately followed by a bare `(` or `)` with no separator is never a real heredoc — its body and terminator line have to come first, so nothing legitimate places a paren directly against the delimiter word, and `$((expr<<ident))` produces exactly that shape. `<<` in that position is now read as the arithmetic operator at tokenize time, same as the literal-digit case, so the guarded line is reached and matched normally regardless of what any later line contains. The unterminated-body-to-error fix from the first pass is kept as defense in depth for genuinely malformed heredocs (`cat <<EOF` with no closing `EOF` line), a distinct, narrower gap the same swallow also covered — an existing test that pinned that silent-swallow as intended behaviour is updated to assert the corrected `ErrUnparsableCommand` contract. Detectors: `TestArithmeticShiftByIdentifierIsNotAHeredoc`, `TestArithmeticShiftCoincidentalDelimiterStillBlocks` (the adversarial payload the security review demonstrated), and `TestTokenizeRejectsUnterminatedHeredoc` — each watched failing on pre-fix code for the claimed reason and passing after. Three more bugs surfaced by the same round's multi-angle sweep and independently verified with failing tests are captured but not fixed here — iss-185 (critical, scanner leaves a second back-to-back secret token undetected and unredacted, defeating history's fail-closed residual re-scan), iss-186 (minor, a failed source-unlink in a capture ledger transition can permanently strand an issue id across two status directories), iss-187 (minor, `rules.Merge` panics on a nil-Domains base — currently unreachable through any live caller, so a latent exported-API contract defect) — left open for a future round, highest severity first.
- dcn-26080600000000012026-08-06 — iss-186 (bug-hunt loop, round 4): `commitTransition` (`internal/core/capture/workflow.go`) wrote the destination file before removing the source, so a non-ENOENT `os.Remove(src)` failure (EPERM/EROFS/EIO — e.g. an immutable source status dir, or a read-only remount) returned an error after `dst` had already landed, with no rollback. The issue id was then present in both status dirs at once, and `findIssue` rejects any id present in more than one file as `ErrDuplicateIssueID` — so the stranded copy could never again be resolved or wontfixed without a human manually deleting one file; `List`/`Status` also double-counted it, since `scanLedger` does not dedupe. Fixed by rolling the destination back (best-effort `os.Remove(dst)`) whenever the source remove fails for a reason other than "already gone", restoring the pre-call state so a retry is all that is needed once the underlying failure clears; a failed rollback itself is surfaced in the returned error rather than swallowed. A non-ENOENT remove failure has no portable, cross-platform way to trigger deterministically in a test (immutable attributes and read-only remounts are Linux/ext4- and permission-model-specific, and this repo's CI runs macOS and Linux), so a test-only seam (`removeSourceHook`, nil in production) was added, mirroring the existing `beforeOrphanRemoveHook` pattern in `alloc.go`. Repro: `internal/core/capture/workflow_test.go`, `TestTransitionRemoveFailureDoesNotStrandIssueInTwoDirs`, watched failing on pre-fix code for the claimed reason (destination not rolled back) and passing after.
- itd-107Autonomous Routines Assemble From One Versioned Template