h-dv/code-index

Watch

Fork 0

The accept loop serves between start_accept and record_auto_enable, so activation_offer_e2e can see auto_enabled ABSENT under load #156

New issue

Closed

opened 2026-09-05 15:29:40 +02:00 by buildagent · 1 comment

buildagent commented

2026-09-05 15:29:40 +02:00

Member

Pre-existing, found while implementing #144 and separated from it deliberately — #144's StartupBeacon slightly widened this window but did not create it, and papering over it with beacon placement would hide the real defect.

The window

crates/daemon/src/main.rs:

383  start_accept(...)          <- the accept loop is live from here
386  self-probe                    (a full authenticated round trip)
421  beacon.finish()               (one fsync'd payload write)
...
487  extractors.record_auto_enable(...)

Between 383 and 487 the daemon answers requests, and auto_enabled has not been recorded yet. A client that connects in that window gets a reply in which the field is ABSENT — which, under this project's own doctrine, must never read as "nothing to enable".

Observed: activation_offer_e2e failing 3/7 under load on the daemon leg, clean on three subsequent isolated runs and clean on the no-changes baseline. The suite's own doc already calls this window race-prone.

Why beacon placement is the wrong fix

beacon.finish() at 421 is well-argued and should stay: the beacon stops on the far side of a probe that proved a request/response round trip, and clearing the heartbeat is what re-arms takeover. Moving it past record_auto_enable would narrow this window slightly at the cost of a clear invariant, and would leave the underlying defect — that the daemon serves before its activation state is recorded — untouched.

The actual question

Either:

the daemon should not answer questions about activation before it has recorded auto-enable — i.e. the readiness point for those replies is later than the readiness point for the port; or
the reply must disclose it — an activation block whose state has not been recorded yet is unavailable/not reported, never absent.

(2) is cheaper and is what the disclosure doctrine already demands everywhere else. (1) is more honest but needs a second readiness notion.

Note this is the same family as #152: a field that is correct on one path and collapsed on another, where absence reads as a measured negative.

Test note

A flake that appears only under load is still a defect — the window is real, and load only changes how often a client lands in it. Fixing it by retrying or by serialising the test would be hiding it.

#144 (upgrade safety) — widened this slightly; the beacon placement is deliberate and stays
#155 (post-upgrade degraded serving window) — same root shape: the daemon serves before it is fully ready and does not say so
#152 (disclosure surfaces graded by surface, not by field)

🤖 Generated with Claude Code

https://claude.ai/code/session_01K1zj5VcFJvJt3pQxe9259K

Pre-existing, found while implementing #144 and separated from it deliberately — #144's `StartupBeacon` slightly widened this window but did not create it, and papering over it with beacon placement would hide the real defect. ## The window `crates/daemon/src/main.rs`: ``` 383 start_accept(...) <- the accept loop is live from here 386 self-probe (a full authenticated round trip) 421 beacon.finish() (one fsync'd payload write) ... 487 extractors.record_auto_enable(...) ``` Between **383 and 487** the daemon answers requests, and `auto_enabled` has not been recorded yet. A client that connects in that window gets a reply in which the field is **ABSENT** — which, under this project's own doctrine, must never read as "nothing to enable". Observed: `activation_offer_e2e` failing **3/7** under load on the daemon leg, clean on three subsequent isolated runs and clean on the no-changes baseline. The suite's own doc already calls this window race-prone. ## Why beacon placement is the wrong fix `beacon.finish()` at 421 is well-argued and should stay: the beacon stops on the far side of a probe that **proved** a request/response round trip, and clearing the heartbeat is what re-arms `takeover`. Moving it past `record_auto_enable` would narrow this window slightly at the cost of a clear invariant, and would leave the underlying defect — that the daemon serves before its activation state is recorded — untouched. ## The actual question Either: 1. **the daemon should not answer questions about activation before it has recorded auto-enable** — i.e. the readiness point for *those* replies is later than the readiness point for the port; or 2. **the reply must disclose it** — an activation block whose state has not been recorded yet is `unavailable`/`not reported`, never absent. (2) is cheaper and is what the disclosure doctrine already demands everywhere else. (1) is more honest but needs a second readiness notion. Note this is the same family as #152: a field that is correct on one path and collapsed on another, where absence reads as a measured negative. ## Test note A flake that appears only under load is still a defect — the window is real, and load only changes how often a client lands in it. Fixing it by retrying or by serialising the test would be hiding it. ## Related - #144 (upgrade safety) — widened this slightly; the beacon placement is deliberate and stays - #155 (post-upgrade degraded serving window) — same root shape: the daemon serves before it is fully ready and does not say so - #152 (disclosure surfaces graded by surface, not by field) 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01K1zj5VcFJvJt3pQxe9259K

buildagent referenced this issue

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

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

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:23 +02:00

Author

Member

Triage 2026-09-06: CLOSING. Repair (2) — the one this issue names as the cheaper option the doctrine already demands — is in, and neither forbidden shortcut was taken.

Verified against master; landed in d0279c0.

The fix, and why the placement is the whole thing

AutoEnableState { Recorded, Pending, Unavailable } — crates/daemon/src/eligibility.rs:169-180.
auto_enable_ran (:144) is marked before the let Some(new) = enabled else { return } early return — eligibility.rs:279-289. That single line is the fix: a step that ran and could not tell must not advertise pending forever. Without it the third state would have been a permanent lie rather than a transient truth.
auto_enable_expected (:160) defaults to false — the safe direction. A forgotten declaration degrades to today's behaviour, not to a stuck pending.
expecting_auto_enable() (:333-344) is declared at construction, by the daemon only — crates/daemon/src/main.rs:415-416, rationale at :407-414.

Neither thing this issue forbids was done. No retry, no test serialisation, and beacon.finish() was not moved.

Runs (exit 0)

cargo test -p code-index-daemon --test auto_enable_state_e2e            → 3 passed
cargo test -p code-index-mcp --bins an_unrecorded_auto_enable_says_which_kind_of_unrecorded → 1 passed
cargo test -p code-index-mcp --test activation_offer_e2e                → 7/7
COSI_E2E_LEG=daemon cargo test -p code-index-mcp --test activation_offer_e2e → 7/7

The originally-flaking suite passes on both legs, which matters here — this repo has a recorded scar where e2e ran --no-daemon and left five RPC arms deletable-green.

the_state_reaches_stats_on_every_leg asserts all four readings including the wire-skew one: a pre-#156 daemon's missing key must land on None, not on any of the three. That is the direction field-rename work keeps getting wrong here.

Residuals, named

The window itself remains. Option (1) — a second readiness notion — was not taken; the daemon still serves between start_accept and record_auto_enable. That is this issue's own stated preference, so it is a choice rather than a gap, but it was never recorded as declined on the issue. It is now.
pending is not observed on a real daemon inside the real window. Grading is holder-level (auto_enable_state_e2e builds ProjectExtractors directly) plus a renderer unit test. Nothing catches an actual accept-loop reply carrying activation_auto_enable_state: "pending". That is the one test an e2e could still add — and worth knowing, because on this project pending is a promise, not a neutral state.

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

## Triage 2026-09-06: CLOSING. Repair (2) — the one this issue names as the cheaper option the doctrine already demands — is in, and neither forbidden shortcut was taken. Verified against master; landed in `d0279c0`. ### The fix, and why the placement is the whole thing - `AutoEnableState { Recorded, Pending, Unavailable }` — `crates/daemon/src/eligibility.rs:169-180`. - `auto_enable_ran` (`:144`) is marked **before** the `let Some(new) = enabled else { return }` early return — `eligibility.rs:279-289`. That single line is the fix: a step that ran and could not tell must not advertise `pending` forever. Without it the third state would have been a permanent lie rather than a transient truth. - `auto_enable_expected` (`:160`) defaults to **`false`** — the safe direction. A forgotten declaration degrades to today's behaviour, not to a stuck `pending`. - `expecting_auto_enable()` (`:333-344`) is declared at construction, by the daemon only — `crates/daemon/src/main.rs:415-416`, rationale at `:407-414`. **Neither thing this issue forbids was done.** No retry, no test serialisation, and `beacon.finish()` was not moved. ### Runs (exit 0) ``` cargo test -p code-index-daemon --test auto_enable_state_e2e → 3 passed cargo test -p code-index-mcp --bins an_unrecorded_auto_enable_says_which_kind_of_unrecorded → 1 passed cargo test -p code-index-mcp --test activation_offer_e2e → 7/7 COSI_E2E_LEG=daemon cargo test -p code-index-mcp --test activation_offer_e2e → 7/7 ``` The originally-flaking suite passes on **both legs**, which matters here — this repo has a recorded scar where e2e ran `--no-daemon` and left five RPC arms deletable-green. `the_state_reaches_stats_on_every_leg` asserts all four readings including the **wire-skew** one: a pre-#156 daemon's missing key must land on `None`, not on any of the three. That is the direction field-rename work keeps getting wrong here. ### Residuals, named 1. **The window itself remains.** Option (1) — a second readiness notion — was not taken; the daemon still serves between `start_accept` and `record_auto_enable`. That is this issue's own stated preference, so it is a choice rather than a gap, but it was never recorded as declined on the issue. It is now. 2. **`pending` is not observed on a real daemon inside the real window.** Grading is holder-level (`auto_enable_state_e2e` builds `ProjectExtractors` directly) plus a renderer unit test. Nothing catches an actual accept-loop reply carrying `activation_auto_enable_state: "pending"`. That is the one test an e2e could still add — and worth knowing, because on this project `pending` is a promise, not a neutral state. 🤖 Triage lane, 2026-09-06, master `45cf6e4`

buildagent closed this issue

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

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

h-dv/code-index#156

No description provided.

Rows
Columns