Skip to content

Fix/scan into link mount - #193

Open
marcusrognes wants to merge 3 commits into
EpicGames:mainfrom
marcusrognes:fix/scan-into-link-mount
Open

Fix/scan into link mount#193
marcusrognes wants to merge 3 commits into
EpicGames:mainfrom
marcusrognes:fix/scan-into-link-mount

Conversation

@marcusrognes

@marcusrognes marcusrognes commented Sep 7, 2026

Copy link
Copy Markdown

Summary

In my workflow i use links quite a bit, my structure is usually this:
/assets <- linked repo
/doc <- other linked repo
/engine
/etc...

If I make changes in assets or doc from this repo, and run lore status --scan <path> or lore stage --scan <path> on a path inside one of those mounts that the repository doesn't already track, like a new file, or a new directory, the command fails and the local repository is left unusable:

[Error] computing diff against filesystem: Node not found
  at lore-revision\src\state.rs:6803 - scan add: parent directory node missing for nested add
  at lore-revision\src\repository\status.rs:974 - computing diff against filesystem

On a larger mount it comes out as Invalid block index: N instead.

After that, plain lore status no longer runs, and lore link list reports fewer mounts than the repository has.

lore repository verify still reports "Verified repository state integrity" throughout, so nothing warns you that it is broken.

How to reproduce

With a repository mounted at assets/, add a file the parent doesn't track yet and scan its path:

mkdir assets/untracked
echo added > assets/untracked/added.txt
lore status --scan assets/untracked/added.txt

[Error] computing diff against filesystem: Node not found
  at lore-revision\src\state.rs:6803 - scan add: parent directory node missing for nested add

you can also check out f53ea99 and run the scripts/test/regression/test_link_scan_stage.py test.

The fix:

What it does:

A --scan that targets a path inside a link mount now resolves that path in the repository that owns the mount, instead of against the parent repository's state. ensure_scan_dir_chain crosses a link segment and continues in the linked state, the same crossing the whole-tree walk already performs, and hands the resolved parent node to the add, so the node is created where it belongs. Two supporting guards: an add never uses a NodeID owned by another state, and a failed add or dirty-mark aborts instead of being swallowed and continuing over already-mutated state.

What it prevents:

The local corruption. Before, the scan added under a foreign NodeID, which either named a block the parent state doesn't have (Invalid block index: N) or silently hit an unrelated node at the same coordinates, after mutating.

What it allows:

Adding new content inside a mount from the parent working copy, with a path-targeted scan, status --scan /, stage --scan /, and the change lands in the linked repository. That includes the case that had no workaround at all: a mount whose tracked content sits only at its root, with no already-tracked subdirectory to name, previously reachable only by cloning the linked repository and committing there.

This might fix some #90 style issues, not sure about that exact case tho.

Checklist

  • Linked to an issue above, or explained why none applies
  • Every commit carries a Signed-off-by: line (DCO -- see CONTRIBUTING.md)
  • cargo +nightly fmt --all produces no changes
  • cargo clippy --all-targets -- -D warnings --no-deps passes, same two errors on main, none added (see note)
  • cargo test passes
  • uv run pytest passes, full suite on Windows; test_nodeblock (slow, not in CI's -m smoke) needs --timeout=0 locally (see note)
  • Docs under docs/ updated where behavior changed
  • If this change is large or cross-cutting, an LEP has been filed first

clippy: this command does not pass on main either. On a pristine f4f344d,
clippy 0.1.96 reports exactly two errors, both outside this change:
nonminimal_bool at lore-revision/src/state.rs:8568 and large_enum_variant
on PathListingResult at lore-revision/src/util/fs.rs:630. This branch reports
the same two and no others.

pytest: full uv run pytest scripts/test on Windows, release binaries, -n 8:
everything passes except scripts/test/test_nodeblock.py::test_nodeblock, which
exceeds the 600s per-test timeout under parallel load, it is @pytest.mark.slow
and so outside CI's -m smoke job. It passes serially with --timeout=0, at the
same wall time as on main. No assertion failure anywhere in the run.

@github-actions github-actions Bot added documentation Improvements or additions to documentation area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals labels Sep 7, 2026
A path-filtered scan of a target that is not yet in state resolved it
against the parent state alone, so a target inside a link mount used a
NodeID that means something else there. The test covers a new file under
a tracked directory in the mount, a new directory in the mount, and the
already-working tracked directory, and asserts after each that status
still runs and the link registry still holds the mount.

The linked repository is filled past one node block so the mismatch is a
hard "Invalid block index" rather than a silent hit on a colliding node.

Signed-off-by: Marcus Rognes <marcus@rognes.guru>
A path-filtered scan resolves its target through find_node_link, which
follows link mounts. A target that is not in state yet - a new file, a
new directory - does not resolve, and the walk fell back to the parent
state while the path still pointed inside a mount. ensure_scan_dir_chain
then walked the parent state through the link node, and the add resolved
its parent by path to a NodeID owned by the linked state. A NodeID
encodes a block index, so that id addressed an unrelated node in the
parent, or a block the parent does not have, and it did so after the
state had been mutated: status stopped running and the link registry was
lost, on a store repository verify still called healthy.

Cross the link in ensure_scan_dir_chain and continue the chain in the
state that owns it, as the directory walk already does, and return that
repository and state for the traversal to adopt. The single-file scan
takes the parent node from the chain instead of resolving it by path,
which the state it adds into cannot do below a mount.

Two guards behind that: refuse a parent node owned by another repository
rather than adding under it, and propagate the add and the dirty mark
instead of swallowing them - an add that reaches add_change as
INVALID_NODE has already mutated the state it cannot describe.

Signed-off-by: Marcus Rognes <marcus@rognes.guru>
A fragment the peer is missing used to come back as the server status
NOT_FOUND, whose message the client printed verbatim, so the smoke test
looked for "Missing fragment". The miss now returns AddressNotFound and
the client names its own context instead, and the assertion failed with
"Push failed for unrelated reason" on the error it was written for.

Assert on "peer is missing a fragment", the context the push adds where
it forwards the error.

Signed-off-by: Marcus Rognes <marcus@rognes.guru>
@marcusrognes
marcusrognes force-pushed the fix/scan-into-link-mount branch from 17f7ab5 to 5427c60 Compare September 8, 2026 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core Core library and its interfaces (lib, C API); revision, storage, transport, protocol internals documentation Improvements or additions to documentation

Development

Successfully merging this pull request may close these issues.

1 participant