You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Surfaced by CodeRabbit's review of #95 (post-kill reconcile, fixes #61), but pre-existing and independent of it: the two raise sites below are reached from the ordinary Stop and crash paths, not only from #95's new hook. Filing separately so the fix that ships in #95 has a scope of its own.
The read-back decodes artifacts as UTF-8. A spec the CLI was killed mid-write can end inside a multi-byte sequence; read_text(encoding="utf-8") then raises UnicodeDecodeError — which is a ValueError, not an OSError, so it sails through every except OSError guard on the way out. verify.read_frontmatter's own comment already warned about exactly this trap.
devcontract.find_result_artifact (devcontract.py:286-287) — reads each candidate to check it carries a real, non-fenced terminal section ([BUG] find_result_artifact / parse_auto_run_result treat a fence-quoted heading as a real terminal section #52), guarded by except OSError. A torn multi-byte sequence escapes. A candidate that cannot be shown to carry a terminal section does not qualify, so it should be skipped like any other unreadable file.
devcontract.synthesize_result (devcontract.py:211) — spec_path.read_text(encoding="utf-8") if spec_path.is_file() else "" is unguarded entirely. It is reached with a file the finder never read, because the bmad-dev-auto-result-*.md fallback marker is matched by name (it has no ## Auto Run Result heading). The preceding read_frontmatter(spec_path) call swallows the decode error and returns {}, so the raise lands on the very next line.
Why it takes down the whole run, not just the session
Nothing between the read and the top-level handler contains it. adapter.run(spec) is called bare from Engine._run_session (engine.py:2068); task.record_session(...) and _save both sit after it (engine.py:2083+). The only catch is Engine.run()'s broad except Exception (engine.py:265), which sets crashed=True / crash_error (engine.py:293-294).
Verified end-to-end against the real Engine.run() on a pre-fix tree:
Pre-fix: run ends crashed=True, crash_error="UnicodeDecodeError: ... byte 0xff", crash.txt written, the task pinned in DEV_RUNNING with noSessionRecord persisted, and every remaining story abandoned.
Post-fix: the same scenario lands in the ordinary retry-exhausted path — crashed=False, task DEFERRED, session records persisted.
find_result_artifact widens to except (OSError, UnicodeDecodeError) and skips the candidate.
synthesize_result's bare read routes through a _read_text_or_empty helper that degrades to "".
An unreadable spec carries no parseable result section, so it now reads exactly like a spec that has not terminated yet — the caller waits, nudges, or keeps its verdict.
The repair path deliberately keeps the opposite contract.reset_spec_status and strip_auto_run_result still raise on a present-but-unreadable spec: silently skipping a rewrite the caller believes succeeded would leave the re-opened spec carrying its stale terminal section, which is precisely the state that makes the re-driven session's first save read as a result.
Context
Surfaced by CodeRabbit's review of #95 (post-kill reconcile, fixes #61), but pre-existing and independent of it: the two raise sites below are reached from the ordinary Stop and crash paths, not only from #95's new hook. Filing separately so the fix that ships in #95 has a scope of its own.
The read-back decodes artifacts as UTF-8. A spec the CLI was killed mid-write can end inside a multi-byte sequence;
read_text(encoding="utf-8")then raisesUnicodeDecodeError— which is aValueError, not anOSError, so it sails through everyexcept OSErrorguard on the way out.verify.read_frontmatter's own comment already warned about exactly this trap.The two raise sites (at
37a01f1, the #95 base)devcontract.find_result_artifact(devcontract.py:286-287) — reads each candidate to check it carries a real, non-fenced terminal section ([BUG] find_result_artifact / parse_auto_run_result treat a fence-quoted heading as a real terminal section #52), guarded byexcept OSError. A torn multi-byte sequence escapes. A candidate that cannot be shown to carry a terminal section does not qualify, so it should be skipped like any other unreadable file.devcontract.synthesize_result(devcontract.py:211) —spec_path.read_text(encoding="utf-8") if spec_path.is_file() else ""is unguarded entirely. It is reached with a file the finder never read, because thebmad-dev-auto-result-*.mdfallback marker is matched by name (it has no## Auto Run Resultheading). The precedingread_frontmatter(spec_path)call swallows the decode error and returns{}, so the raise lands on the very next line.Why it takes down the whole run, not just the session
Nothing between the read and the top-level handler contains it.
adapter.run(spec)is called bare fromEngine._run_session(engine.py:2068);task.record_session(...)and_saveboth sit after it (engine.py:2083+). The only catch isEngine.run()'s broadexcept Exception(engine.py:265), which setscrashed=True/crash_error(engine.py:293-294).Verified end-to-end against the real
Engine.run()on a pre-fix tree:crashed=True,crash_error="UnicodeDecodeError: ... byte 0xff",crash.txtwritten, the task pinned inDEV_RUNNINGwith noSessionRecordpersisted, and every remaining story abandoned.crashed=False, taskDEFERRED, session records persisted.Fix
Landed in #95 as
1f8918e:find_result_artifactwidens toexcept (OSError, UnicodeDecodeError)and skips the candidate.synthesize_result's bare read routes through a_read_text_or_emptyhelper that degrades to"".An unreadable spec carries no parseable result section, so it now reads exactly like a spec that has not terminated yet — the caller waits, nudges, or keeps its verdict.
The repair path deliberately keeps the opposite contract.
reset_spec_statusandstrip_auto_run_resultstill raise on a present-but-unreadable spec: silently skipping a rewrite the caller believes succeeded would leave the re-opened spec carrying its stale terminal section, which is precisely the state that makes the re-driven session's first save read as a result.Related
read_frontmatterguardsUnicodeDecodeErrorbut notOSError.