Skip to content

The project root is re-canonicalized on every path validation, on a path with ~45 call sites #220

Description

@iceteaSA

path_restriction_context canonicalizes the configured project root on every call:

// context.rs:6095
let resolved_root = std::fs::canonicalize(&root).unwrap_or(root);

That's a full symlink-following syscall chain for a value that changes only when config.project_root changes. It runs before the path-specific canonicalize that follows it, so a validation pays two chains rather than one.

path_restriction_context sits at the top of validate_path, validate_write_location, and validate_read_path — roughly 45 call sites across crates/aft/src/commands/ alone (add_import.rs, apply_patch.rs, edit_match.rs, edit_symbol.rs, callers.rs, call_tree.rs, batch.rs, …). Every file-touching command goes through it.

Where it gets worse: when canonicalize fails — a not-yet-existing write target, a broken symlink — the fallback walks every path component with symlink_metadata, and iterative_follow_chain follows up to 40 read_link hops. On a symlink-heavy tree (node_modules, macOS /var/private/var) that's a meaningful syscall count per request, repeated for a root that hasn't moved.

  • Severity: P2. Not a correctness problem and not visible on a single call; it's a constant multiplier on every file operation, and the root half of it is pure redundancy.
  • Fix direction: memoize the resolved root, invalidated when config.project_root changes — that alone removes one of the two chains with no cache-invalidation risk, since the input is config state rather than filesystem state. A bounded path→canonical cache invalidated on watcher events would cover the second, but that one needs more care and is a separate decision.

I'd treat the root memoization and the path cache as independent: the first is unambiguous, the second trades correctness risk for speed and you may not want it.


Provenance and limits. Found in a read-only sweep; I verified the call site and the caller count myself against 83fe3bd5 before filing. Not measured — no profile, no syscall count, no before/after. The claim is that the work is repeated and its input is stable, both readable from source. Whether it registers against the rest of a command's cost is exactly the thing I can't tell you from a static read.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions