Skip to content

fix(with-watch): use metadata snapshots for ls#373

Merged
kdy1 merged 2 commits intomainfrom
kdy1/fix-with-watch-ls-startup-stall
Apr 10, 2026
Merged

fix(with-watch): use metadata snapshots for ls#373
kdy1 merged 2 commits intomainfrom
kdy1/fix-with-watch-ls-startup-stall

Conversation

@kdy1
Copy link
Copy Markdown
Contributor

@kdy1 kdy1 commented Apr 10, 2026

Summary

  • switch ls/dir/vdir to metadata-based listing snapshots so the first run does not recurse through large trees before spawning
  • keep ls -R recursive, keep ls -d scoped to the named path, and add snapshot timing/mode debug logs
  • add regression coverage for ls listing behavior and update with-watch docs/contracts

Testing

  • cargo test
  • manual check in /Users/kdy1/projects/cloud: with-watch ls now prints immediately with snapshot_modes="metadata-children"

@kdy1 kdy1 enabled auto-merge (squash) April 10, 2026 19:46
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 11b4ddf07c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +1916 to +1920
push_inferred_path_with_mode(
&mut inputs,
token,
cwd,
ls_like_snapshot_mode(token, cwd, recursive, directory_mode),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Parse ls flags before mapping operand snapshot modes

analyze_ls_like assigns each operand’s PathSnapshotMode immediately using the current recursive/directory_mode booleans, so options that appear later never update earlier operands. This under-watches valid invocations like ls subdir -R (GNU ls treats it as recursive): subdir is recorded as MetadataChildren, and deep descendant updates will not trigger reruns even though delegated command output changes. Parse all effective flags first (or retroactively update collected operands) before choosing snapshot modes.

Useful? React with 👍 / 👎.

Comment on lines +559 to +561
match fs::metadata(path) {
Ok(metadata) if metadata.is_dir() => PathSnapshotMode::ContentTree,
Ok(_) | Err(_) => PathSnapshotMode::ContentPath,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Avoid freezing missing path inputs to non-recursive mode

default_path_snapshot_mode maps metadata lookup failures to ContentPath, and that snapshot mode is fixed when WatchInput::path is created. If a watched path is missing at startup and later becomes a directory (for example with-watch find src before src/ exists), future snapshots keep treating it as a single path entry, so changes under src/** are missed after the first creation rerun. This regresses prior behavior where directory recursion was determined from the current filesystem state at snapshot time.

Useful? React with 👍 / 👎.

@kdy1 kdy1 merged commit 25e0955 into main Apr 10, 2026
16 checks passed
@kdy1 kdy1 deleted the kdy1/fix-with-watch-ls-startup-stall branch April 10, 2026 19:55
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.

1 participant