naps62/maestro
Archived
1
0
Fork 0
Code Issues 9 Pull Requests Actions Packages Projects Releases Wiki Activity

Guard the fourth read-after-Dead test #115

New Issue
Closed
opened 2026-08-24 23:01:31 +01:00 by naps62-yolo · 0 comments
naps62-yolo commented 2026-08-24 23:01:31 +01:00
Collaborator
Copy Link
Copy Source

Found while fixing #113, and left alone there because it was not named in that issue.

natural_exit_without_a_retained_store_destroys_nothing (crates/maestro-daemon/tests/session_manager.rs) asserts wt_path.exists() immediately after observing the session go Dead — the same unguarded read-after-Dead pattern behind #111 and #113. It has not been seen failing, but the window is the same one those three flaked in.

It needs a different fix from its siblings. They assert that something happens (a store write, a tree removal), so waiting for it is enough. This one asserts that something never happens: the tree must survive because there is no retained store to record it. Waiting for a negative is not a fix — the settle step could still remove the tree one instant after the assertion passes.

The honest fix is to wait for the settle step itself to complete and only then assert, rather than to wait for the tree's continued existence. If the daemon exposes no observable for "settle finished", say so on the issue rather than reaching for a sleep — adding an observable is a fair outcome here, and it would let the sibling tests stop inferring settle completion from its side effects.

Found while fixing #113, and left alone there because it was not named in that issue. `natural_exit_without_a_retained_store_destroys_nothing` (`crates/maestro-daemon/tests/session_manager.rs`) asserts `wt_path.exists()` immediately after observing the session go `Dead` — the same unguarded read-after-`Dead` pattern behind #111 and #113. It has not been seen failing, but the window is the same one those three flaked in. It needs a different fix from its siblings. They assert that something *happens* (a store write, a tree removal), so waiting for it is enough. This one asserts that something *never* happens: the tree must survive because there is no retained store to record it. Waiting for a negative is not a fix — the settle step could still remove the tree one instant after the assertion passes. The honest fix is to wait for the settle step itself to complete and only then assert, rather than to wait for the tree's continued existence. If the daemon exposes no observable for "settle finished", say so on the issue rather than reaching for a sleep — adding an observable is a fair outcome here, and it would let the sibling tests stop inferring settle completion from its side effects.
naps62-yolo added this to the Sidebar cards milestone 2026-08-24 23:01:31 +01:00
naps62-yolo added the area/daemontype/fixdifficulty/medium labels 2026-08-24 23:01:31 +01:00
This repo is archived. You cannot comment on issues.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area/android
area/cli
area/config
area/daemon
area/protocol
area/tui
area/web
code-review
difficulty/easy
difficulty/hard
difficulty/medium
epic
Tracks a multi-issue epic / parent work item
 tui
type/docs
type/feat
type/fix
type/refactor
type/test
No labels area/daemon difficulty/medium type/fix
Milestone
No items
No Milestone Sidebar cards
Projects
Clear projects
No projects
Assignees
Clear assignees
hermes (Hermes (agent)) naps62-yolo (naps62-yolo (agent)) resende (Resende) naps62 (Miguel Palhas)
No Assignees
1 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: naps62/maestro#115