Repository navigation
fix(graph): mark collected only after sealed retirement - #4818
ScriptedAlchemy wants to merge 3 commits into
Conversation
Generation deletion could commit staging-row removal and mark the in-memory locator collected before attempting sealed-artifact retirement. An unlink failure then returned an error with the durable sealed bytes still present, while the in-memory collected set falsely declared the generation gone. Retire the sealed artifact before transitioning the locator to collected. Strengthen the existing real filesystem regression with negative and positive collected-state assertions across failure, retry and success. Refs #4616
|
The successful retry fixture previously created an arbitrary directory with no receipt. The ownership-verified sealed retire path correctly refuses such an artifact, so combining the safe collector with this regression would fail even though collection is correct. Use a self-describing, generation-bound receipt in the retry, preserve an unrelated sibling sentinel, and require typed sealed-retirement failure without hard-coding the former unsafe remover's exact message. Refs #4616
|
Refs #4616 — published Root cause (cross-PR behavioral incompatibility): the real failure/retry regression used a synthetic The repair writes a structurally complete generation-bound receipt with the current graph format and recovered digest, keeps the forced file-as-directory unlink failure assertion, verifies that failed retirement does not mark the locator collected, that successful retry does mark it collected and removes the target, and that an unrelated sibling sentinel survives. It also requires a sealed-retirement error without depending on obsolete wording. No test skip, timeout increase, CI change, or unrelated source edits. Validation: source/diff cross-review of #4804's This remains a partial Refs #4616 update. Do not close until all five acceptance requirements are proven on |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Assert the concrete unavailable error from the sealed-directory unlink boundary, not any error that happens to mention sealed. Accept the two currently reviewable collector implementations until the ownership-safe collector is integrated, without weakening the failure/retry assertions. Refs #4616
|
Refs #4616 — published Review fix: the previous Validation: source+exact-commit-diff review; no local repository checkout or Bazel runner, no Cargo. Prior exact-head CI #42502588004 had graph-db unit tests PASSED (the 174-test lane), while broader graph-db suite failed in unrelated This fixes test discrimination only, not all sealed-retention acceptance. Keep #4616 open. No changes to mixed draft #4776, model PRs, release #4763, or peer branches. |
|
Refs #4616 — superseded by clean-history PR #4819, commit |
Root cause and focused repair
Refs #4616 — partial retention-state correctness fix, not closure of the sealed-generation collection acceptance.
GraphDb::delete_generation_contentsdeleted staging rows, then marked the locatorcollectedand cleared its in-memory retirement state before callingretire_sealed_generation_store. If a real filesystem unlink failed, the method returned an error but the in-memorycollectedset already claimed the on-disk sealed generation was gone. This is an unsafe ordering between physical reclamation and in-memory completion state.Move sealed retirement before the transition to
collected, preserving the prior idempotent error/retry semantics. The existing real filesystem test intentionally places a regular file in the exact sealed-artifact directory slot to force a removal error; its new assertions require the locator not collected after failure, collected only after successful retry, and physical directory removal. The pre-change code fails the first new assertion, so the regression is not vacuous.Scope: one file,
crates/tracedecay-graph-db/src/generation_runtime.rs(21 additions, 1 deletion). No dependency, CI, timeout, release or infrastructure modifications. Basemaster=91c72aba7750a398dd28e83d64bf0c6d6fef4172, isolated source commitd53b9e35017802c04d51f1e3cfd7ae58425a1ecb. Created its issue-specific branch from the exact base and published via expected-old-headforce=falsefast-forward, then verified the remote ref and 1-file compare.Required validation and boundaries
No native Bazel/macOS pass is claimed: this tool environment has no checked-out repository or Bazel installation. Run
bazel test //crates/tracedecay-graph-db:unit_test --test_output=errors, ensure the named regression actually executes (nonzero tests), and review applicable CI before merging. No Cargo was run.The separate #4804 addresses receipt/ownership cleanup, #4809 addresses bounded active-pointer reads, #4813 addresses fair writer admission, and #4799 addresses Doctor reclaim guidance. None is merged into
master; this PR does not import their work or touch their paths. Mixed draft #4776 and Kev/decision-model PRs must remain draft and never merge or auto-merge. Do not close #4616 until complete N>3 sealed-retirement, unsafe-artifact isolation, bounded progress under writers, Doctor guidance, and OOM-safe storage-report acceptance are all proven onmaster. Release #4763 remains read-only; no production/infra/tags/credentials changes.