h-dv/code-index

Watch

Fork 0

A permanently malformed lockfile is indistinguishable from a torn mid-write read, so a corrupt payload reads as "no daemon" forever #157

New issue

Closed

opened 2026-09-05 17:02:37 +02:00 by buildagent · 1 comment

buildagent commented

2026-09-05 17:02:37 +02:00

Member

Found while diagnosing the Windows CI failure at 08a6eed, and it is the more general defect underneath it.

The behaviour

Lockfile::read (crates/daemon/src/lifecycle.rs:190-228) maps a TOML parse failure to Ok(None):

// Treat a parse failure as "file is in transit" — the
// owner is mid-write. Caller polls.
Err(_) => Ok(None),

That is correct for a torn read and should stay. The payload is written by rename, but a reader can still catch a partial file on some filesystems, and treating that as "no payload yet, poll again" is right.

The problem is that it is the only reading. A payload that is permanently malformed — truncated by a full disk, corrupted, hand-edited, or written by a buggy forge — returns exactly the same Ok(None). Every caller therefore concludes there is no daemon, forever, and the polling that is correct for a torn read never terminates.

How it presented, which is why it is worth filing

Windows CI failed on readiness_deadline_tests::a_beating_daemon_extends_the_readiness_deadline with:

a live daemon publishing a fresh heartbeat must extend the wait, however long its migration takes

An assertion about heartbeats, for a file that would never parse. The test's forge had interpolated a Windows root into a TOML basic string (root = "C:\Users\..." — \U is an invalid escape), read returned Ok(None), and daemon_is_still_starting fell to its _ => false arm. A tolerance for a transient state swallowed a permanent one, and the diagnostic pointed at the wrong subsystem.

That is this project's recurring shape — two states rendering identically — in the lockfile reader rather than in a payload. Compare #147 (an unreadable file's index_updating with no reachable success condition) and #155/#156 (a transient absence indistinguishable from a permanent one).

What a repair needs

Three states, not two:

absent — no file. Spawn.
in transit — a parse failure that a retry could plausibly fix. Poll, with a bound.
malformed — a parse failure that persisted across N polls, or one that is structurally unrecoverable. Say so, name the path, and say that waiting will not help.

(3) is the missing one. A cheap version: keep Ok(None) for the first observation and have the polling caller escalate after a bounded number of identical failures — the file's own bytes are stable across the polls, so "the same malformed payload N times" is a measurable, not a guess.

Note the bar this must clear: the operator-facing message should name the file and the fact that it is a cache. #144 added exactly that shape for a wedged migration ("the index is a cache and can always be rebuilt from the working tree — to recover, stop the daemon and delete it"), and a malformed daemon.toml is even cheaper to recover from — it is one file, not the index.

#144 — the stated-exit precedent
#147, #155, #156 — the same transient-vs-permanent collapse on other surfaces
The forge defect that surfaced it is fixed, and crates/daemon/tests/lockfile_forge_registry.rs now gates the class

🤖 Generated with Claude Code

https://claude.ai/code/session_01K1zj5VcFJvJt3pQxe9259K

Found while diagnosing the Windows CI failure at `08a6eed`, and it is the more general defect underneath it. ## The behaviour `Lockfile::read` (`crates/daemon/src/lifecycle.rs:190-228`) maps a TOML parse failure to `Ok(None)`: ```rust // Treat a parse failure as "file is in transit" — the // owner is mid-write. Caller polls. Err(_) => Ok(None), ``` **That is correct for a torn read** and should stay. The payload is written by rename, but a reader can still catch a partial file on some filesystems, and treating that as "no payload yet, poll again" is right. The problem is that it is the *only* reading. A payload that is **permanently** malformed — truncated by a full disk, corrupted, hand-edited, or written by a buggy forge — returns exactly the same `Ok(None)`. Every caller therefore concludes *there is no daemon*, forever, and the polling that is correct for a torn read never terminates. ## How it presented, which is why it is worth filing Windows CI failed on `readiness_deadline_tests::a_beating_daemon_extends_the_readiness_deadline` with: ``` a live daemon publishing a fresh heartbeat must extend the wait, however long its migration takes ``` An assertion about **heartbeats**, for a file that would never parse. The test's forge had interpolated a Windows root into a TOML basic string (`root = "C:\Users\..."` — `\U` is an invalid escape), `read` returned `Ok(None)`, and `daemon_is_still_starting` fell to its `_ => false` arm. **A tolerance for a transient state swallowed a permanent one**, and the diagnostic pointed at the wrong subsystem. That is this project's recurring shape — two states rendering identically — in the lockfile reader rather than in a payload. Compare #147 (an unreadable file's `index_updating` with no reachable success condition) and #155/#156 (a transient absence indistinguishable from a permanent one). ## What a repair needs Three states, not two: 1. **absent** — no file. Spawn. 2. **in transit** — a parse failure that a retry could plausibly fix. Poll, with a bound. 3. **malformed** — a parse failure that persisted across N polls, or one that is structurally unrecoverable. **Say so, name the path, and say that waiting will not help.** (3) is the missing one. A cheap version: keep `Ok(None)` for the first observation and have the polling caller escalate after a bounded number of identical failures — the file's own bytes are stable across the polls, so "the same malformed payload N times" is a measurable, not a guess. Note the bar this must clear: the operator-facing message should name the file and the fact that it is a cache. `#144` added exactly that shape for a wedged migration (*"the index is a cache and can always be rebuilt from the working tree — to recover, stop the daemon and delete it"*), and a malformed `daemon.toml` is even cheaper to recover from — it is one file, not the index. ## Related - #144 — the stated-exit precedent - #147, #155, #156 — the same transient-vs-permanent collapse on other surfaces - The forge defect that surfaced it is fixed, and `crates/daemon/tests/lockfile_forge_registry.rs` now gates the class 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01K1zj5VcFJvJt3pQxe9259K

buildagent referenced this issue

2026-09-05 20:44:12 +02:00

Nothing notices if the scheduler stops firing: a cron that silently stops looks identical to a cron with nothing to report #162

dhoyer referenced this issue from a commit

2026-09-06 00:09:43 +02:00

feat: six silent-loss surfaces render the fact the process already held

buildagent commented

2026-09-06 09:38:06 +02:00

Author

Member

Triage 2026-09-06: CLOSING. Three states became four, the malformed verdict is a measurement rather than a guess, and the operator message names the file and says waiting will not help.

Verified against master; landed in d0279c0.

Against the stated acceptance

Three states, not two — in fact four: PayloadState { Absent, Present, Malformed, ForeignRoot }, crates/daemon/src/lifecycle.rs:227-240, rationale at :194-226.
The malformed verdict is measured. MalformedPayload { path, len, digest, reason } (lifecycle.rs:268-280) carries an FNV digest over the exact bytes, and STABLE_MALFORMED_OBSERVATIONS = 5 (:305) with MalformedWatch (:317) escalates only when the same digest returns five times, clearing on any non-malformed state. So a torn mid-write read can never escalate — it is structurally impossible, not merely unlikely.
The message names the file and forecloses waiting. MalformedPayload::recovery_hint() (lifecycle.rs:286-297) gives the path, the parser's reason and the byte count, says re-reading will not change it, and says "delete it and the next tool call starts a fresh daemon" — #144's "it is a cache, delete it" shape, as asked.
It is wired into the real poller, not just available: crates/mcp-server/src/main.rs:1016 (MalformedWatch::new()) and :1021-1027, which bail!s with the hint plus how many times and over how long it was observed. First-observation warning at main.rs:832-841.
Lockfile::read still collapses to Option deliberately, so existing callers are untouched — the widening is additive.

Runs (exit 0)

cargo test -p code-index-daemon --test payload_state_e2e        → 5 passed
cargo test -p code-index-daemon --test lockfile_forge_registry  → 1 passed (62.9s)

The load-bearing one is only_stable_bytes_escalate (payload_state_e2e.rs:172) and it is strong: 3N polls with changing bytes asserting escalation never fires, then 2N with stable bytes asserting it fires exactly once at observation N, then a successful read asserting the watch clears. That grades both directions of the distinction this issue is about.

no_test_forges_a_daemon_payload_by_hand is the anti-vacuity companion — it stops the suite drifting into grading hand-built structs.

Residuals, named

The mcp-server bail itself is ungraded. MalformedWatch is graded at unit level; nothing drives the real MCP server against a permanently malformed daemon.toml and asserts the operator sees the recovery hint rather than "the daemon did not answer". That is the exact user-visible behaviour this issue was filed about, so it is the natural next test — but the mechanism it would grade is present and unit-graded.
rpc_index.rs:864's Malformed → DaemonBuild::Unknown arm has no test. The renderer is graded (server.rs:18685); the producer mapping is not, so deleting that arm would be green.

Both are test-coverage gaps on a shipped mechanism, not the silent-loss defect this issue names. Closing on that basis; if either is wanted as tracked work it should be a fresh issue rather than holding this one open.

🤖 Triage lane, 2026-09-06, master 45cf6e4

## Triage 2026-09-06: CLOSING. Three states became four, the malformed verdict is a **measurement** rather than a guess, and the operator message names the file and says waiting will not help. Verified against master; landed in `d0279c0`. ### Against the stated acceptance - **Three states, not two** — in fact four: `PayloadState { Absent, Present, Malformed, ForeignRoot }`, `crates/daemon/src/lifecycle.rs:227-240`, rationale at `:194-226`. - **The malformed verdict is measured.** `MalformedPayload { path, len, digest, reason }` (`lifecycle.rs:268-280`) carries an FNV digest over the exact bytes, and `STABLE_MALFORMED_OBSERVATIONS = 5` (`:305`) with `MalformedWatch` (`:317`) escalates **only when the same digest returns five times**, clearing on any non-malformed state. So a torn mid-write read can never escalate — it is structurally impossible, not merely unlikely. - **The message names the file and forecloses waiting.** `MalformedPayload::recovery_hint()` (`lifecycle.rs:286-297`) gives the path, the parser's reason and the byte count, says re-reading will not change it, and says *"delete it and the next tool call starts a fresh daemon"* — #144's "it is a cache, delete it" shape, as asked. - **It is wired into the real poller**, not just available: `crates/mcp-server/src/main.rs:1016` (`MalformedWatch::new()`) and `:1021-1027`, which `bail!`s with the hint plus how many times and over how long it was observed. First-observation warning at `main.rs:832-841`. - `Lockfile::read` still collapses to `Option` deliberately, so existing callers are untouched — the widening is additive. ### Runs (exit 0) ``` cargo test -p code-index-daemon --test payload_state_e2e → 5 passed cargo test -p code-index-daemon --test lockfile_forge_registry → 1 passed (62.9s) ``` The load-bearing one is `only_stable_bytes_escalate` (`payload_state_e2e.rs:172`) and it is strong: `3N` polls with **changing** bytes asserting escalation never fires, then `2N` with stable bytes asserting it fires **exactly once at observation N**, then a successful read asserting the watch **clears**. That grades both directions of the distinction this issue is about. `no_test_forges_a_daemon_payload_by_hand` is the anti-vacuity companion — it stops the suite drifting into grading hand-built structs. ### Residuals, named 1. **The `mcp-server` bail itself is ungraded.** `MalformedWatch` is graded at unit level; nothing drives the real MCP server against a permanently malformed `daemon.toml` and asserts the operator sees the recovery hint rather than "the daemon did not answer". That is the exact user-visible behaviour this issue was filed about, so it is the natural next test — but the mechanism it would grade is present and unit-graded. 2. **`rpc_index.rs:864`'s `Malformed → DaemonBuild::Unknown` arm has no test.** The *renderer* is graded (`server.rs:18685`); the producer mapping is not, so deleting that arm would be green. Both are test-coverage gaps on a shipped mechanism, not the silent-loss defect this issue names. Closing on that basis; if either is wanted as tracked work it should be a fresh issue rather than holding this one open. 🤖 Triage lane, 2026-09-06, master `45cf6e4`

buildagent closed this issue

2026-09-06 09:38:08 +02:00

buildagent referenced this issue

2026-09-06 09:54:11 +02:00

code-index://docs/reason-codes is at 3,979 of its 4,000-token cap, so the next reason code this project mints cannot be documented #184