Skip to content

fix(cli): stage discard could not discard an edge deletion - #96

Closed
rennehan wants to merge 1 commit into
mainfrom
fix/stage-discard-needs-the-index
Closed

fix(cli): stage discard could not discard an edge deletion#96
rennehan wants to merge 1 commit into
mainfrom
fix/stage-discard-needs-the-index

Conversation

@rennehan

@rennehan rennehan commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Found by using the product, not by reading it: authoring a graph through the CLI, deleting an edge to see the coherence finding, then trying to put it back.

$ hydrate stage discard
hydrate: a staged edge deletion targets an edge that isn't in the pulled index
$ hydrate status
Staged: 0 nodes, 0 edges, 1 removal.        ← still there

stage discard summarised the stage with summarize(&stage, None) — no index — while status and diff both use summarize_workdir, which loads it. Any delta whose rendering needs a lookup then fails, and a staged edge deletion is exactly that.

So the verb could not undo the thing a reader is most likely to want undone, and left the work staged: the state it exists to escape. The module's own doc already said the summary must come from "the same OpSummary projection status and diff use" — the code called a different one.

Verified against the real service after the fix:

$ hydrate stage discard
- edge Shortener.Links.find -> Shortener.ResolveCode.record
Discarded 1 staged operation on branch 'example': 0 nodes, 0 edges, 1 removal.

A missing index is still fine — there is nothing to look up. A corrupt one still fails loud, which is why an older test's malformed {"version":1} fixture had to become well-formed; that test was passing only because discard never read the file.

Mutation-verified: restoring summarize(&stage, None) fails the new test.

440 tests, fmt and clippy -D warnings clean.

It summarised the stage without the pulled index, while status and diff both go
through summarize_workdir, which loads it. Any delta whose rendering needs a
lookup then failed — an edge deletion above all:

  hydrate: a staged edge deletion targets an edge that isn't in the pulled index

So the verb could not undo the thing a reader is most likely to want undone: the
deletion they made a moment ago. It exited 1 and left the work staged, which is
the state the verb exists to escape.

Found by using the product — authoring the example graph through the CLI,
deleting an edge to see the coherence finding, then trying to put it back. The
module's own doc already said the summary must come from the same projection
status and diff use; the code called a different one.

A missing index is still fine (nothing to look up); a corrupt one still fails
loud, which is why an older test's malformed fixture had to become well-formed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@rennehan rennehan added the bug Something isn't working label Aug 1, 2026
@rennehan rennehan self-assigned this Aug 1, 2026
@rennehan rennehan added the bug Something isn't working label Aug 1, 2026
@rennehan

rennehan commented Aug 2, 2026

Copy link
Copy Markdown
Contributor Author

Superseded — the fix this PR carries is already on main, and merging this branch now would revert it along with everything else that has landed since.

cli/src/cmd/stage.rs:92 on main has the summarize_workdir(&base) call in discard, with a comment naming the same failure this PR found: without the pulled index, a stage referencing anything outside itself made discard fail loud — unable to complete and unable to report what it was destroying. The regression test came across too.

It arrived via #97 (hydrate stage restore), which picked the fix up because discard and restore share that projection and the bug was in the way.

The problem with merging this now is mechanical rather than conceptual: this branch predates #97 and #98, so git diff main... is -1635 lines across stage.rs, walk.rs, error.rs and state/mod.rs. It would take out stage restore, the project verb group, the typed error kinds, and the validate_bare_name extraction. Closing it removes that footgun.

Nothing is lost. If you want the history, the commit is d19eb50.

@rennehan rennehan closed this Aug 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant