Correctness & data safety improvements #10
Description
Summary
Several correctness and data safety gaps that could lead to state corruption or undefined behavior under edge conditions.
Items
1. No file locking on state files
Two concurrent dgr processes (e.g., triggered by editor hooks or parallel terminals) can write to state.json simultaneously, causing data corruption. Consider flock (Unix) or advisory file locking.
2. write_atomic TOCTOU bug in store/fs.rs
The current implementation does remove_file then rename. On Unix, rename atomically overwrites the target — the remove_file call is unnecessary and introduces a window where a crash loses the state file entirely. On Windows, the TODO comment in the code already flags this.
Fix: On Unix, just rename. On Windows, use ReplaceFile or a temp-rename-rename dance.
3. No cycle detection in graph traversal
If state.json is corrupted with a circular parent reference (A → B → A), lineage(), active_descendant_ids(), and branch_depth() will loop infinitely. A visited set in traversal functions would prevent this.
4. No state validation on load
load_state deserializes JSON but does not validate referential integrity:
- Parent
node_idreferences that don't exist in the nodes list - Duplicate branch names
- Archived nodes referenced as parents by active nodes
A validate() pass after deserialization would catch these early with clear error messages.
5. No --abort for paused operations
When a restack pauses on a conflict, the only escape is manually running git rebase --abort and hoping dagger's operation.json state is still consistent. A dgr sync --abort (and dgr sync --skip) would provide safe, guided recovery.
6. No schema migration system
DAGGER_STATE_VERSION, DAGGER_CONFIG_VERSION, and DAGGER_OPERATION_VERSION fields exist but there is no migration logic. Bumping from version 1 → 2 will break all existing installations with no upgrade path. Even a simple sequential migration runner would future-proof this.