Eliminate redundant rglob I/O pass in discover_tracked_lockfiles

[coderabbitai]

Background

Raised during review of PR #75 (refresh stale Cargo lockfiles before publish preflight).

Problem

discover_tracked_lockfiles currently performs three sequential I/O operations:

1. workspace_root.rglob("Cargo.lock") — filesystem walk to find candidate lockfiles
2. git ls-files — subprocess call to narrow to tracked files
3. (lockfile_path.parent / "Cargo.toml").exists() — per-file existence check

The first pass (rglob) is redundant because git ls-files already enumerates tracked lockfiles. Removing it reduces I/O, eliminates the early-exit guard on empty candidates, and simplifies the function.

Similarly, validate_lockfile_freshness spawns one cargo metadata process per lockfile; consider early-exit on the first stale lockfile to avoid unnecessary subprocess invocations when the caller will abort on any stale result.

Resolution

• Replace rglob with direct reliance on git ls-files output in discover_tracked_lockfiles; keep only the target/ filter and Cargo.toml adjacency check.
• Evaluate whether _validate_lockfile_freshness in publish_preflight.py should short-circuit after the first stale lockfile.
• Update unit tests to reflect the simpler discovery path.

References

• PR: Refresh stale Cargo lockfiles before publish preflight (#61) #75
• Requested by: @leynos

[coderabbitai]

🔗 Related PRs

leynos/shared-actions#263 - Make man-page discovery optional [merged]
leynos/corbusier#79 - Add joint Rust and node audit gate [merged]
leynos/theoremc#44 - Run Rust tests with cargo-nextest [merged]
leynos/repovec-appliance#37 - Serialize qdrant key provisioning (#29) [merged]
leynos/dear-diary#42 - Adopt shared-actions stage-release-artefacts for cargo-binstall [closed]

---

📝 Issue Planner

_{Check the box below or use the @coderabbitai plan command to generate an implementation plan and prompts that you can use with your favorite coding assistant.}

✅ Successfully generated implementation plan.

💡 Use @coderabbitai <feedback> to replan

---

🧪 Issue enrichment is currently in open beta.

You can configure auto-planning by selecting labels in the issue_enrichment configuration.

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

💬 Have feedback or questions? Drop into our discord!

[coderabbitai]

Coding Plan

Summary

• Eliminate the redundant rglob pass by parsing git ls-files output directly, reducing filesystem I/O and simplifying control flow
• Implement early-exit in the validation loop to avoid spawning cargo metadata subprocesses for lockfiles that will never be evaluated
• Update unit tests to remove rglob-specific assertions and add coverage for short-circuit behaviour

Design Choices

Design Choice 1: Early-exit semantics for validation are acceptable

Options Considered:

1. Short-circuit on first stale lockfile, returning immediately
2. Collect all stale lockfiles and report them together for user convenience

Chosen Option: 1

Rationale: The ticket explicitly requests evaluation of early-exit; since the publish workflow aborts on any stale lockfile, gathering a complete list provides no additional value and incurs unnecessary subprocess overhead.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Streamline Lockfile Discovery

Remove the filesystem walk and rely exclusively on git ls-files to enumerate tracked Cargo.lock files, retaining only the post-filtering logic.

Task 1: Replace rglob with git ls-files enumeration

Modify discover_tracked_lockfiles in lading/commands/publish_preflight.py to eliminate the initial workspace_root.rglob("Cargo.lock") call.

• Invoke git ls-files --full-name "Cargo.lock" "**/Cargo.lock" via the injected runner, using workspace_root as the working directory
• Parse stdout (newline-separated relative paths) and resolve each against workspace_root to obtain absolute Path objects
• Remove the early-exit guard that checked for empty rglob candidates; the function now proceeds directly with git output

Task 2: Retain target directory and adjacency filters

Keep the existing post-filtering logic unchanged.

• Exclude any path containing a /target/ segment to omit build artefacts from nested Cargo invocations
• Verify (lockfile_path.parent / "Cargo.toml").exists() for each candidate to confirm it is a genuine Cargo workspace/crate lockfile
• Return the filtered collection of valid lockfile paths

🤖 Prompt for AI agents

Optimise `discover_tracked_lockfiles` in `lading/commands/publish_preflight.py`
by replacing the `rglob` filesystem walk with a `git ls-files` invocation. The
goal is to reduce filesystem I/O and simplify control flow.

- Remove the `workspace_root.rglob("Cargo.lock")` call entirely
- Remove the early-exit guard that checked for empty rglob candidates
- Invoke `git ls-files --full-name "Cargo.lock" "**/Cargo.lock"` via the
injected runner, passing `workspace_root` as the working directory
- Parse the stdout of that invocation (newline-separated relative paths) and
resolve each entry against `workspace_root` to produce absolute `Path` objects
- Retain the existing post-filtering logic exactly as-is:
  - Exclude any path containing a `/target/` segment
  - Verify `(lockfile_path.parent / "Cargo.toml").exists()` for each candidate
  - Return the filtered collection of valid lockfile paths

Phase 2: Optimise Lockfile Freshness Validation

Introduce early-exit semantics so the validation loop terminates immediately upon detecting the first stale lockfile.

Implement short-circuit validation loop

Modify _validate_lockfile_freshness in lading/commands/publish_preflight.py to return as soon as a stale lockfile is detected.

• Within the iteration over discovered lockfiles, invoke cargo metadata --locked --manifest-path <Cargo.toml> (where <Cargo.toml> is adjacent to the lockfile) via the injected runner
• Check the exit code: 0 indicates freshness; 101 (or any non-zero) indicates staleness
• On a non-zero exit, raise PublishPreflightError (or return the stale path, per existing contract) immediately without processing remaining lockfiles
• Log a debug message for each lockfile checked, and an info-level message when short-circuiting

🤖 Prompt for AI agents

Optimise `_validate_lockfile_freshness` in
`lading/commands/publish_preflight.py` to short-circuit on the first stale
lockfile detected. The goal is to avoid spawning unnecessary `cargo metadata`
subprocesses.

- Within the loop iterating over discovered lockfiles, invoke `cargo metadata
--locked --manifest-path <Cargo.toml>` via the injected runner, where
`<Cargo.toml>` is the `Cargo.toml` adjacent to the current lockfile
- Check the exit code after each invocation:
  - Exit code `0` → lockfile is fresh; continue to the next
  - Non-zero exit code (including `101`) → lockfile is stale
- On a non-zero exit code, immediately raise `PublishPreflightError` (or return
the stale path, per the existing contract) — do NOT continue iterating
- Emit a debug-level log message for each lockfile checked
- Emit an info-level log message when short-circuiting out of the loop

Phase 3: Update Unit Tests

Adjust unit tests to reflect the simpler discovery path and verify early-exit behaviour.

Task 1: Update discovery tests

Modify or add tests in tests/unit/publish/test_preflight_checks.py (or a new test_preflight_lockfiles.py in the same directory) to validate the streamlined discovery logic.

• Remove any assertions that verify rglob behaviour or the early-exit guard on empty rglob results
• Add a test that injects a runner returning multiple lockfile paths from git ls-files and asserts the function filters correctly for target/ and Cargo.toml adjacency
• Add a test confirming that the function handles empty git output gracefully (returns an empty collection)

Task 2: Add early-exit validation tests

Add tests verifying that _validate_lockfile_freshness short-circuits on the first stale lockfile.

• Inject a runner that returns exit code 0 for the first lockfile and exit code 101 for the second; assert the function raises/returns after checking two lockfiles, not three
• Inject a runner that returns exit code 101 for the first lockfile; assert the function raises/returns after checking only one lockfile
• Verify that when all lockfiles are fresh, the function completes without raising

🤖 Prompt for AI agents

Update unit tests in `tests/unit/publish/test_preflight_checks.py` (or a new
`test_preflight_lockfiles.py` in the same directory) to cover the streamlined
discovery logic and the early-exit validation behaviour.

For discovery tests:
- Remove all assertions that verify rglob behaviour or the old early-exit guard
on empty rglob results
- Add a test that injects a runner returning multiple lockfile paths from `git
ls-files` and asserts correct filtering for `/target/` segments and `Cargo.toml`
adjacency
- Add a test that injects a runner returning empty git output and asserts the
function returns an empty collection

For early-exit validation tests:
- Inject a runner that returns exit code `0` for the first lockfile and exit
code `101` for the second; assert the function raises/returns after exactly two
lockfile checks, not three
- Inject a runner that returns exit code `101` for the first lockfile; assert
the function raises/returns after checking only one lockfile
- Add a test verifying that when all lockfiles are fresh (all exit code `0`),
the function completes without raising

Research

The lading project is a Rust workspace publication orchestration tool written in Python. Pre-flight validation logic resides in lading/commands/publish_preflight.py, which uses the _invoke function (from publish_execution.py) to execute subprocess calls such as git status --porcelain and cargo check. The codebase employs a _CommandRunner protocol for runner injection, enabling unit tests to substitute recording or failing runners without spawning real processes. The functions discover_tracked_lockfiles and _validate_lockfile_freshness are introduced in PR #75 and are targeted for optimisation by this ticket.

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Optimise `discover_tracked_lockfiles` in `lading/commands/publish_preflight.py`
by replacing the `rglob` filesystem walk with a `git ls-files` invocation. The
goal is to reduce filesystem I/O and simplify control flow.

- Remove the `workspace_root.rglob("Cargo.lock")` call entirely
- Remove the early-exit guard that checked for empty rglob candidates
- Invoke `git ls-files --full-name "Cargo.lock" "**/Cargo.lock"` via the
injected runner, passing `workspace_root` as the working directory
- Parse the stdout of that invocation (newline-separated relative paths) and
resolve each entry against `workspace_root` to produce absolute `Path` objects
- Retain the existing post-filtering logic exactly as-is:
  - Exclude any path containing a `/target/` segment
  - Verify `(lockfile_path.parent / "Cargo.toml").exists()` for each candidate
  - Return the filtered collection of valid lockfile paths
===============================================================================

Task: 2

Optimise `_validate_lockfile_freshness` in
`lading/commands/publish_preflight.py` to short-circuit on the first stale
lockfile detected. The goal is to avoid spawning unnecessary `cargo metadata`
subprocesses.

- Within the loop iterating over discovered lockfiles, invoke `cargo metadata
--locked --manifest-path <Cargo.toml>` via the injected runner, where
`<Cargo.toml>` is the `Cargo.toml` adjacent to the current lockfile
- Check the exit code after each invocation:
  - Exit code `0` → lockfile is fresh; continue to the next
  - Non-zero exit code (including `101`) → lockfile is stale
- On a non-zero exit code, immediately raise `PublishPreflightError` (or return
the stale path, per the existing contract) — do NOT continue iterating
- Emit a debug-level log message for each lockfile checked
- Emit an info-level log message when short-circuiting out of the loop
===============================================================================

Task: 3

Update unit tests in `tests/unit/publish/test_preflight_checks.py` (or a new
`test_preflight_lockfiles.py` in the same directory) to cover the streamlined
discovery logic and the early-exit validation behaviour.

For discovery tests:
- Remove all assertions that verify rglob behaviour or the old early-exit guard
on empty rglob results
- Add a test that injects a runner returning multiple lockfile paths from `git
ls-files` and asserts correct filtering for `/target/` segments and `Cargo.toml`
adjacency
- Add a test that injects a runner returning empty git output and asserts the
function returns an empty collection

For early-exit validation tests:
- Inject a runner that returns exit code `0` for the first lockfile and exit
code `101` for the second; assert the function raises/returns after exactly two
lockfile checks, not three
- Inject a runner that returns exit code `101` for the first lockfile; assert
the function raises/returns after checking only one lockfile
- Add a test verifying that when all lockfiles are fresh (all exit code `0`),
the function completes without raising

💡 Iterate on the plan with: `@coderabbitai` <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!