Module state exposed before initial Lua load and store writes are applied #47
Breadway
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
How found: reproduced by running unmodified origin/main at
e4ddf6cunder 32 CPU busy loops with eight concurrent ipc_integration binaries. 7/8 suite runs failed: 2module_with_no_manifest_keeps_full_access_but_is_flagged_ungatedat the store field check, 7rules_toml_malformed_rule_surfaces_doctor_visible_erroratbread.rules present, plus onerules_toml_absent_is_a_no_op. Confidence: high. Exact causal paths: (1) Lua module_store_set directly inserts a fabricatedModuleStatus { status: Loaded, ungated: false }when on_load sets its first store value;load_moduleonly queues final status after on_load returns. Thus a real client polling modules.list/state.get can see a loaded module with an empty/partial store and wrong ungated flag. (2) the IPC socket accepts ping/state requests before initial Lua loading finishes, and status updates are async state-engine commands; tests treat ping as ready.bread.rulescan be absent from health/modules during initial load. Fix under review introduces private staging for store writes merged with final status and an explicitstartup.readyIPC barrier over Lua completion plus FIFO state-engine acknowledgement. Regression exercises store staging/merge deterministically; IPC status tests wait on readiness instead of weakening assertions. Not duplicate of BREAD-30 (module-host child startup timeout), BREAD-42 (hyprland bridge), or BREAD-43 (SIGTERM bind cleanup). PR44module_host_bridge.rsis untouched because it is not causative.Independent review: changes_requested at
a7056d1714, despite exact-head CI success (run152/internal2521). Full criterion mapping and precise requests: #48 (comment) . R1 high: mod.rs:3130-3143 store.get ignores newly staged on_load writes. Independently rebuilt old basee4ddf6cloads identical module successfully (x41/y42); rebuilt head fails read-after-write assertion at startup and reload. R2 mod.rs:207-243 discards all FIFO barrier errors and still acknowledges startup.ready; propagate Result, test channel closure/dropped ack, clarify attempted-vs-healthy semantics. All locked workspace tests/clippy/fmt/docs pass; five fresh loaded IPC suites43/43 each. Staging/publication and clear acknowledgement source/unit checks pass but do not cover R1/R2. No merge/done or BREAD-28 status change. Durable evidence ~/.config/breadboard/reviews/BREAD-48-a7056d1/. Required reviewer action is todo for coordinator rework, overriding requested review placement for an accepted head.Independent re-review PASS at repaired exact head
f37efff325; integrated merged7add44b20verified tree-identical. Persisted full criteria/evidence to PR comment #48 (comment). Exact-head CI run154 success and integrated run156 success, independent workspace tests/clippy/fmt/docs pass, actual startup/reload Lua store probe and barrier error tests pass. BREAD-48 remains in review for coordinator post-merge verification; BREAD-28 untouched.Post-merge sync had prematurely marked this card done before coordinator behavior verification. Independent PASS is now persisted at PR comment5124/card comment2300; exact integration tree confirmed and CI green. Restored card to review per workflow. Coordinator must verify behavior on merge
d7add44before done. BREAD-28 remains unchanged.Coordinator integrated verification on merge
d7add44b20: clean detached worktree cargo test --locked -p breadd --test ipc_integration in_process_module_store_supports_read_your_writes_on_startup_and_reload passed (startup+reload). Independent repaired-head pass PR48 comment5124 verifies same tree, negative ack tests, full locked checks and integrated run156 success. Full acceptance now backed; this is not retroactive waiver of the premature merge.