fix: warn on silently dropped symlink patch source - #1088
Conversation
A literal (non-glob) loadout patch source that resolves to a symlink is the walk root itself. With follow_symlinks off the walker skips it at the is_file() check and the patch is dropped with no message at any log level, so a session comes up missing the file (e.g. a ~/.gitconfig symlinked into a dotfiles repo) with no signal at the moment it matters. Emit a tracing::warn! for that case, mirroring the existing missing-source warn and scoped to the walk root (depth 0) so glob-enumerated child symlinks, whose no-follow behavior is documented, stay quiet. Fixes #1069 Routing-Key: inbox-route/I_kwDOSUhdos8AAAABKsifNA Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 36 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
norrietaylor
left a comment
There was a problem hiding this comment.
Reviewed: scope, clobber, correctness, CI. Approving.
Scope — one file, 52 added lines, comfortably inside budget. Every hunk traces to the reported defect.
No clobber — the only removals are the lines being changed. Nothing from a recent merge is reverted; enumerate.rs has no competing work in flight.
Correctness — the warn is gated narrowly and correctly: !follow_symlinks && entry.depth() == 0 && entry.file_type().is_symlink(). Depth 0 is the walk root, which is the literal-source case; deeper symlinks come from glob enumeration whose no-follow behaviour is documented, so they stay quiet. The comment explains why rather than restating the condition.
Test — asserts both directions (drops with follow_symlinks off, lands with it on), which pins the cause to the follow decision rather than anything else about the patch. It tests the defect, not its own fixture.
Miri — the test calls tempfile::tempdir(), which is what aborted the Miri job in #1020. Checked: nightly-tests.yml:339 runs cargo miri test -p switch only, so crates/sessions is out of scope and this does not reintroduce that failure.
CI — 23/23 green.
Fixes #1069
Routing-Key: inbox-route/I_kwDOSUhdos8AAAABKsifNA
A literal (non-glob) loadout patch source that resolves to a symlink is the walk root itself, so with
follow_symlinksoff the walker skips it at theis_file()check insessions::core::enumerateand the patch is dropped with no message at any log level — the truly silent case in the report. The missing-source case already warns, at a severity the CLI's defaultinfofilter shows, so this adds a paralleltracing::warn!for the symlink drop, mirroring that warn and scoped to the walk root (depth 0) so glob-enumerated child symlinks — whose no-follow behavior is documented — stay quiet (informed by #713, which defined thefollow_symlinkssemantics). Surfacing every drop on stderr alongsideApplying loadouts:(issue item 1) would require threading dropped-source data back through the compose→loadouts→CLI layers, beyond a minimal patch; left for follow-up.Verification
cargo fmt --all --check --manifest-path target/Cargo.toml — clean, no drift
cargo clippy --workspace --locked --manifest-path target/Cargo.toml -- -D warnings — exit 0, Finished
cargo build --workspace --locked --manifest-path target/Cargo.toml — exit 0, Finished in 3m40s
cargo test --workspace --locked --manifest-path target/Cargo.toml — exit 0, sessions lib 353 passed (incl. new literal_symlink_source_drops_off_and_lands_on)
Note
Warn when a symlink patch source is silently dropped in
enumerate_patch_filesWhen
follow_symlinksis false and a literal patch source is a symlink,enumerate_patch_filespreviously dropped the entry without any indication. It now emits atracing::warnwith the source pattern and walk root so callers can detect misconfigured patch sources. The entry continues to be excluded from results.Macroscope summarized 9acfc8e.