Skip to content

Add synchronized pinned worktree index views - #170

Merged
Shengyu Fu (shengyfu) merged 8 commits into
shengyfu-shared-index-generationsfrom
shengyfu-shared-worktree-sync
Oct 5, 2026
Merged

Shengyu Fu (shengyfu) merged 8 commits into
shengyfu-shared-index-generationsfrom
shengyfu-shared-worktree-sync

Conversation

@shengyfu

@shengyfu Shengyu Fu (shengyfu) commented Oct 4, 2026 •

Copy link
Copy Markdown
Member

Summary

Layer 2 of shared-worktree indexing, dependent on #169 and part of native stack 172. Based on parent commit 4a1cdb9f8f98b40e9d9f59c5b2768d1e8dfa3354. Latest head: 2af22f08bfb74d61d11da75d5dc9d96976464f78. Automatic shared serving, native watchers, routing, and content caches belong to layer 3; this layer does not enable shared CLI routing.

Shared-worktree design, API contracts, and cost caveats

  • Add tgrep_core::worktrees::{WorktreeView, WorktreeOptions, WorktreeSnapshot, WorktreeStatus, ReconcileStats, WorktreeError}. Each canonical worktree retains its exact Arc<Generation> and private whole-file overlay relative to that pin, including committed divergence, staging, untracked files, renames/deletions, sparse checkout and eligibility changes.
  • Verify actual checkout bytes with the existing auto decoder and ContentId, rather than trusting Git status or stat data. Reuse matching base/overlay postings, and stream-copy verified content-identical base masks for renames/copies without extraction.
  • Publish overlay, visibility and complete filename membership atomically behind readiness. Bounded file/subtree invalidations close the gate; overflow/unknown changes force full verification. Concurrent invalidation aborts publication with ChangedDuringReconcile; discovery/read errors remain not-ready. Reconciliation makes one attempt, never an unbounded churn loop.
  • Expose guarded candidate/path resolution and files() membership without leaking live IDs or mutable HybridIndex. Final matching reads requesting-root handles through WorktreeSnapshot::open_file, or uses a private versioned cache.
  • Save only the private delta using the existing directory-bound atomic checkpoint publisher. Its single file embeds the exact generation key/root/base binding; restoration always requires reconciliation before readiness. Checkpoint destinations must be directories, validated on construction, restore and save. No sidecar writes, base flushes, GC or generation deletion.
  • Preserve ordinary walker membership for a linked worktree's plain .git pointer file, including hidden filename and content queries. Actual common_dir/git_dir metadata subtrees and generation/checkpoint/index storage remain excluded. The gitfile follows ordinary ignore/hidden/size rules and is not followed into the directory it names.
  • Reject unrepresentable native file/directory names before lossy metadata-path conversion or visibility recording. Non-Unicode names and literal Unix backslashes return incomplete-discovery errors rather than aliasing another, potentially ignored file. Ordinary scans retain native paths.
  • Normalize accepted invalidation hints, preserving subtree semantics for trailing/repeated separators and interior .. Invalid parent/absolute/leading . hints explicitly error and force full repair. Probe normalized paths and ancestor prefixes in a BTreeSet, rather than scanning all hints for every file; retain conservative ASCII case matching and non-ASCII full repair.
  • Extract the existing serving reader into tgrep_core::rooted::RootedDir. Worktree views and ordinary ServerState each retain one reader instead of reopening/re-registering the root for every candidate. Unix uses component-relative no-follow/nonblocking opens and requires a regular final handle; Windows guards ancestors, rejects reparse points, and checks actual handle parent/containment. Revalidation uses rooted opens too.
  • Update README API usage and the linked design's implementation status, safe final-read contract, root lifecycle, freshness and cost limitations.

Layer 3 contract

Construct/restore not-ready, then subscribe to changes before the first refresh(). Forward relative file/subtree hints through invalidate_path (both rename paths), and overflow/Git/ignore configuration or uncertain events through invalidate_all. refresh rewalks membership and handles hints; without hints it verifies all admitted contents. reconcile_full always verifies all content, including same-size/restored-mtime changes. Every reconciliation advances the epoch; successful publication acknowledges earlier invalidation tokens with an equal or later epoch.

with_snapshot holds readiness and overlay guards through candidate ID resolution. Snapshot methods expose root/epoch/visibility, candidates(plan, prefix, include_hidden) and files(prefix, include_hidden) as owned relative paths, plus open_file(relative: &str) -> std::io::Result<std::fs::File> through the retained reader. Do not reenter the view from the callback.

Pinned-root identity is verified automatically before and after every with_snapshot callback, including empty/file-only queries. Root verification or candidate-open failure returns an outer WorktreeError::Io, closes readiness, advances the invalidation epoch, clears hints and queues full repair before releasing the guard. The first candidate-open error is latched even if the callback handles it, performs a later successful open, or tries to return a success-shaped result.

A returned file handle can be read/matched outside the guard; the runtime must buffer results and perform its final snapshot/epoch check before publication. Later errors reading that handle remain caller-owned: report them and call invalidate_all() after leaving the guard, without publishing partial results. Neither handles nor epochs freeze same-inode contents. A refresh acknowledges processed inputs rather than promising an atomic filesystem snapshot; periodic full checks and scan fallback remain necessary.

The additive RootedDir::{open(root), open_file(relative), verify_root()} helper is also available. No extra view reader accessor is needed. Windows directory guards persist until their owners are dropped: detach/release shared registrations and stop ordinary serving before removing or renaming those roots.

Cost evidence

Real fixtures assert three identical tracked files reuse the shared reader with zero trigram extractions. A linked worktree also has a plain .git file absent from the committed base: this costs one additional private extraction initially. Four clean CRLF/smudge-transformed tracked files need four private extractions plus the gitfile, for five total; their next unchanged reconciliation extracts none. Positional/next-byte masks make LF/CRLF normalization unsafe. Widespread transformations may require an all-file private overlay; no cross-view transformed-content cache is claimed.

A hinted pass can avoid rereading unaffected, previously verified contents but still walks metadata and opens regular-file handles safely. ReconcileStats.hint_lookups counts ordered-set probes, bounded by path depth per file (each probe logarithmic in queue size). The regression queues 2,001 hints for two files and performs five probes, finds a same-size/restored-mtime subtree edit, and avoids rereading the unrelated file. Full/non-ASCII repair performs no hint probes. Ordinary serving retains its root registration instead of paying constructor work per file; identity verification still costs filesystem operations.

Validation

Final head 2af22f08bfb74d61d11da75d5dc9d96976464f78 passed locally on Windows and real Linux via WSL:

  • cargo test -p tgrep-core: 361 Windows / 375 Linux tests passed.
  • Entire CLI binary unit suite, including serve::tests and serve::poll_tests: 266 Windows / 288 Linux passed, with one pre-existing ignored test per platform. The serving subset passed 129 / 151.
  • cargo fmt --all -- --check and cargo clippy --workspace --all-targets -- -D warnings passed on both platforms.
  • Final-head CI run passed Windows, Ubuntu and macOS workspace tests, Format and Clippy; CodeQL run passed.
  • The completed final-head automated overview reports no findings, and all three actual inline review threads are resolved. It still recommends final human review of the cross-platform filesystem/concurrency/checkpoint surface.
  • Linux tests were explicitly listed and executed: FIFO, root replacement, native-name collisions, hint normalization and symlink/executable/gitlink coverage are real top-level tests, not nested/unexecuted fixtures.
  • Deterministic hooks cover directory-to-link/junction swaps before/after ancestor opening and root replacement during verification/open. Real-Git tests assert not-ready/full-retry behavior. Unix FIFO regressions use deadline-protected subprocesses so blocking regressions fail instead of hanging.
  • Reproduced snapshot-open readiness failures on Windows and Linux before fixing them. Regressions cover directory-link swaps, missing candidates, swallowed errors, first-error preservation despite later opens, and mandatory full repair after an unrelated hint catches a same-size/restored-mtime edit. A dedicated real Linux snapshot FIFO test also executes under a deadline.
  • Added checkpoint regular-file rejection tests for inside/outside-root destinations and a directory replaced before save, retaining destination bytes. Added final candidate-read link-swap and root-replacement checks, including empty snapshot callbacks and ordinary-serving root retention.
  • Pinned root registration exposed two artificial missing-root polling fixtures. They now use an existing root plus a malformed-ignore discovery error, retaining failed-attempt, one-walk, completion-time and retry-cadence assertions. The previously omitted serve::poll_tests module now runs in the full CLI validation above.

Earlier regressions reproduced lossy native-path aliases on Linux (including ignored literal-U+FFFD/directory-path collisions), lost subtree hints on Windows after same-size/restored-mtime edits, and missing linked .git membership. Other fixtures cover divergent worktrees/current-root reads; scan parity and match removal; empty/short content; ignore/hidden/filename-only binary/size/storage rules; CRLF/smudge and UTF-16; assume-unchanged; sparse/skip-worktree; deterministic concurrent invalidation; read/discovery errors and overflow; checkpoint revalidation/bindings and immutable destination rejection. Windows hidden-attribute/exclusive-read coverage ran locally; Linux executed actual non-UTF-8 collisions that APFS may reject at fixture creation.

No unrelated Python scripts were changed. Parent fixes are inherited by rebase, not duplicated. Native stack membership is preserved; nothing is merged.

Shengyu Fu (shengyfu) and others added 2 commits October 3, 2026 19:40
Verify actual checkout content against shared generations, publish private overlays and membership under epoch-gated readiness, and support bounded hints, full reconciliation, and exact-generation delta checkpoints. Add real Git worktree regressions and core API documentation.

Co-authored-by: Copilot App <[email protected]>
Keep plain .git pointer files eligible under existing hidden, ignore, and size rules while excluding actual Git metadata directories. Cover independent filename/content scan parity and update private-overlay extraction cost expectations.

Co-authored-by: Copilot App <[email protected]>
@shengyfu
Shengyu Fu (shengyfu) force-pushed the shengyfu-shared-worktree-sync branch from bde4557 to 79157bb Compare October 4, 2026 02:42
Copilot AI balanced review requested due to automatic review settings October 4, 2026 02:42

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Path aliasing and noncanonical subtree hints can expose incorrect files or retain stale evidence.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
What changed in this PR

Adds synchronized, generation-pinned worktree views with private overlays, reconciliation, readiness gating, and checkpoint persistence.

Changes:

  • Introduces WorktreeView synchronization and query APIs.
  • Extends overlay checkpoints with generation binding.
  • Adds reconciliation tests and user/design documentation.
File Description
tgrep-core/​src/​worktrees.rs Implements synchronized worktree views.
tgrep-core/​src/​worktrees/​tests.rs Adds synchronization regressions.
tgrep-core/​src/​walker.rs Reports directory-entry errors.
tgrep-core/​src/​shared.rs Binds checkpoints to generations.
tgrep-core/​src/​lib.rs Exports the worktree module.
tgrep-core/​src/​gitignore.rs Adds fallible frozen ignore snapshots.
tgrep-core/​src/​generations/​mod.rs Exposes worktree-root discovery.
tgrep-core/​src/​generations/​git.rs Resolves canonical worktree roots.
SHARED_WORKTREE_INDEXES.md Documents the implemented architecture.
README.md Documents API usage and limitations.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tgrep-core/src/worktrees.rs
Comment thread tgrep-core/src/worktrees.rs
Reject metadata-walk paths that cannot be represented losslessly before recording visibility or opening an aliased path. Rebuild accepted invalidation hints from normal components so alternate subtree spellings still reverify same-stat edits. Add executed Unix collision regressions and update the core API documentation.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 4, 2026 03:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

File opening has a root-containment race, and hinted reconciliation scales multiplicatively with files and queued hints.

Review effort: Balanced
Findings: 1 High severity

Open (1)
Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Hint scanning causes O(files × hints) refresh performance

tgrep-core/​src/​worktrees.rs:489

Each discovered file linearly scans every queued hint, making a hinted refresh O(files × hints). With the default capacity of 4,096, a large checkout can perform billions of prefix comparisons before avoiding any reads. Probe the path's own ancestor prefixes in the BTreeSet instead, which is O(path depth × log hints).

Comment thread tgrep-core/src/worktrees.rs Outdated
Share rooted regular-file opens with serving, reject raced link/FIFO replacements, and keep root identity pinned through reconciliation. Probe normalized hint ancestors instead of scanning the hint queue per file. Add deterministic cross-platform races, bounded Unix FIFO checks, and lookup-count regressions.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 4, 2026 04:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Checkpoint configuration accepts regular files as directories, producing views whose persistence operations cannot succeed.

Review effort: Balanced
Findings: None

Resolved since last review (1)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Reject checkpoint paths that are regular files

tgrep-core/​src/​worktrees.rs:796

Validate that the configured checkpoint path is actually a directory. canonicalize also succeeds for a regular file, so WorktreeView::new currently accepts one, excludes that file from discovery, and only fails later when save/restore tries to access <file>/overlay.json. Rejecting it during construction keeps the documented checkpoint_directory contract and avoids creating an unusable view.

Validate the checkpoint directory type at construction, restoration, and save. Cover regular files inside/outside the worktree and replacement after registration without modifying their bytes.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 4, 2026 04:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Final-read guidance can escape the worktree through raced links, and serving now repeatedly rebuilds expensive rooted handles.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity Reuse rooted handle to avoid repeated root verification

tgrep-cli/​src/​serve.rs:4997

This rebuilds RootedDir for every file open. RootedDir::open canonicalizes and opens/verifies the root, and open_file verifies it again; initial indexing invokes open_within_root three times per file (the initial read plus both checks in file_still_has_bytes), multiplying root path resolution and handle opens across the entire corpus. Retain and reuse one rooted handle per server/indexing pass instead.

Low severity Prevent symlink escapes when reading reconciled paths

README.md:722

This example directs consumers to read snapshot.root().join(path) directly, but a candidate path can be replaced by a symlink after reconciliation; ordinary filesystem reads will follow it and can return bytes outside the registered worktree. Final matching should use a RootedDir retained for the registration (or a private versioned cache) so descendant links remain rejected.

Retain one rooted reader in ordinary ServerState. Expose snapshot candidate opens through the existing worktree reader and verify pinned root identity before and after callbacks, invalidating readiness on failure even for empty results. Cover root replacement and link-swapped final reads; document final matching and Windows root disposal.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 4, 2026 05:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The cross-platform filesystem security and concurrency-sensitive publication logic warrants final human review despite extensive coverage.

Review effort: Balanced
Findings: None

Use an existing root with a malformed ignore rule to preserve the prior failed-attempt and retry-cadence assertions. Run all CLI binary unit modules, including serve::poll_tests, on Windows and native Linux.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 4, 2026 05:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

Snapshot file-open failures leave the view ready despite indicating potentially stale indexed state.

Review effort: Balanced
Findings: None

Previously missed (1)

In code that hasn't changed since last review

Medium severity Read-path failures must invalidate readiness

tgrep-core/​src/​worktrees.rs:718

A descendant read failure does not close readiness: open_file returns the I/O error directly, while with_snapshot only invalidates on root-identity failures. After the tested directory-link/FIFO/file swap, status().ready therefore remains true, so later queries can keep using stale candidates (and may miss new terms that never become candidates), contrary to the documented contract that detected read-path swaps leave the view not-ready. Record snapshot open failures and have with_snapshot invalidate the gate/queue a full repair before returning.

Latch the first snapshot open error and reject callback publication while atomically closing readiness and queuing full repair, even when the callback handles the error. Cover link swaps, deleted candidates, swallowed errors, same-stat missed edits, and deadline-protected Unix FIFO swaps. Keep public signatures and caller-owned handle reads unchanged.

Co-authored-by: Copilot App <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 4, 2026 05:29

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The large cross-platform filesystem, concurrency, and checkpointing surface warrants final human review despite strong test coverage.

Review effort: Balanced
Findings: None

@shengyfu
Shengyu Fu (shengyfu) merged commit 9c4bd17 into main Oct 5, 2026
12 checks passed
@shengyfu
Shengyu Fu (shengyfu) deleted the shengyfu-shared-worktree-sync branch October 5, 2026 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants