Three tmp + atomic_replace writers can strand an untracked temp file under .bmad-loop/ when the replace fails, which holds verify.worktree_clean False and blocks the next run's preflight and the epic-boundary auto-sweep's clean-tree precondition — until a human deletes the file by hand.
Surfaced while addressing a review comment on #361, which fixed the same shape in operatoractions._drop_legacy. Filing rather than widening that PR: AGENTS.md says not to refactor untouched code.
The exposure
atomic_replace is os.replace plus a win32 sharing-violation retry (platform_util.py:143). It does no cleanup of its own, so a caller that writes a temp and then fails the replace leaves the temp behind. Whether that matters depends entirely on whether the temp's path is gitignored — and bmad-loop init writes exactly three lines (install.py:881): .bmad-loop/runs/, .bmad-loop/cache/, .bmad-loop/policy.toml.
Exposed — the temp lands outside every ignore rule:
| Site |
Temp path |
Why it escapes the ignore rules |
decisions.py:60 _write_store |
.bmad-loop/decisions.tmp |
nothing ignores it |
policy.py:1321 |
.bmad-loop/policy.toml.tmp |
the ignore line is the anchored literal .bmad-loop/policy.toml, which does not match a .tmp suffix |
runs.py:522 archive_run |
.bmad-loop/archive/…tar.gz.tmp |
.bmad-loop/archive/ is not ignored at all (only runs/ is) |
Not exposed, checked and ruled out: journal.py:64 (<run_dir>/state.json.tmp) and runs.py:432 (the stop file) both write under .bmad-loop/runs/, which is gitignored.
decisions.py is the closest sibling to the one already fixed — same module shape, same .bmad-loop/<name>.tmp result, and decisions is likewise written out of band from a CLI command rather than inside a commit window.
Options
- Per-site guard —
except BaseException: with contextlib.suppress(OSError): tmp.unlink(missing_ok=True); raise, the idiom operatoractions.record_park and now _drop_legacy both use. Three small diffs, each independently ablatable.
- Route the three through
platform_util.atomic_write_text — it already unlinks on any raise (platform_util.py:219), and additionally fsyncs before the replace and preserves mode/xattrs/symlinks. archive_run cannot use it (it writes a tarball, not text), so this is really "two of three plus a guard".
- Ignore
.bmad-loop/*.tmp — cheapest, but it hides half-written state rather than not producing it, and would not have caught the _drop_legacy case in review.
Leaning (1) for runs.py and (2) for decisions.py/policy.py, but the call is whether the guarded write should become the house default — right now it is the exception, not the rule.
Notes
- Probability is low everywhere (needs a real
os.replace failure: ENOSPC, EACCES, a win32 sharing violation outliving the retry budget). The cost is a repository that refuses to run with no obvious cause, so it is worth closing.
- Incidental, spotted while tracing this —
runs.py:522 computes its temp as dest.with_suffix(".tar.gz.tmp") on a <id>.tar.gz path, and with_suffix replaces only the last component: the result is <id>.tar.tar.gz.tmp, not <id>.tar.gz.tmp. Harmless today (the name is only ever used to replace into dest), but it is not the name the docstring implies.
Three
tmp+atomic_replacewriters can strand an untracked temp file under.bmad-loop/when the replace fails, which holdsverify.worktree_cleanFalse and blocks the next run's preflight and the epic-boundary auto-sweep's clean-tree precondition — until a human deletes the file by hand.Surfaced while addressing a review comment on #361, which fixed the same shape in
operatoractions._drop_legacy. Filing rather than widening that PR: AGENTS.md says not to refactor untouched code.The exposure
atomic_replaceisos.replaceplus a win32 sharing-violation retry (platform_util.py:143). It does no cleanup of its own, so a caller that writes a temp and then fails the replace leaves the temp behind. Whether that matters depends entirely on whether the temp's path is gitignored — andbmad-loop initwrites exactly three lines (install.py:881):.bmad-loop/runs/,.bmad-loop/cache/,.bmad-loop/policy.toml.Exposed — the temp lands outside every ignore rule:
decisions.py:60_write_store.bmad-loop/decisions.tmppolicy.py:1321.bmad-loop/policy.toml.tmp.bmad-loop/policy.toml, which does not match a.tmpsuffixruns.py:522archive_run.bmad-loop/archive/…tar.gz.tmp.bmad-loop/archive/is not ignored at all (onlyruns/is)Not exposed, checked and ruled out:
journal.py:64(<run_dir>/state.json.tmp) andruns.py:432(the stop file) both write under.bmad-loop/runs/, which is gitignored.decisions.pyis the closest sibling to the one already fixed — same module shape, same.bmad-loop/<name>.tmpresult, anddecisionsis likewise written out of band from a CLI command rather than inside a commit window.Options
except BaseException: with contextlib.suppress(OSError): tmp.unlink(missing_ok=True); raise, the idiomoperatoractions.record_parkand now_drop_legacyboth use. Three small diffs, each independently ablatable.platform_util.atomic_write_text— it already unlinks on any raise (platform_util.py:219), and additionally fsyncs before the replace and preserves mode/xattrs/symlinks.archive_runcannot use it (it writes a tarball, not text), so this is really "two of three plus a guard"..bmad-loop/*.tmp— cheapest, but it hides half-written state rather than not producing it, and would not have caught the_drop_legacycase in review.Leaning (1) for
runs.pyand (2) fordecisions.py/policy.py, but the call is whether the guarded write should become the house default — right now it is the exception, not the rule.Notes
os.replacefailure: ENOSPC, EACCES, a win32 sharing violation outliving the retry budget). The cost is a repository that refuses to run with no obvious cause, so it is worth closing.runs.py:522computes its temp asdest.with_suffix(".tar.gz.tmp")on a<id>.tar.gzpath, andwith_suffixreplaces only the last component: the result is<id>.tar.tar.gz.tmp, not<id>.tar.gz.tmp. Harmless today (the name is only ever used to replace intodest), but it is not the name the docstring implies.