diff --git a/specs/GH9908/product.md b/specs/GH9908/product.md new file mode 100644 index 000000000..b600a3b82 --- /dev/null +++ b/specs/GH9908/product.md @@ -0,0 +1,87 @@ +# GH9908: Product Spec — Listing-Aware File Links for `ls DIR/` Output +## 1. Summary +Warp should resolve clickable filenames in directory listing output against the directory that was listed, not only against the block's current working directory. When a user runs commands such as `ls subdir/`, `ls -la subdir/`, or `ls /tmp/example/`, clicking `README.md` in that block should open the listed file inside the listed directory. +This spec focuses on the `ls`-style directory listing case where output rows contain bare entry names. The goal is to eliminate silent wrong-file opens, make listed files and directories clickable when they exist under the listed directory, and preserve existing behavior for commands whose output already includes rooted or relative paths. +## 2. Problem +Warp currently treats bare filenames in BlockList output as relative to the block's stored working directory. That is correct for `ls` with no path argument, but incorrect when the command lists another directory. The most harmful result is silent misresolution: if both `CWD/README.md` and `CWD/subdir/README.md` exist, clicking `README.md` in `ls subdir/` output opens `CWD/README.md`. +Users expect clicked filenames in listing output to refer to the files that were just listed. A silent wrong-file open is worse than an obvious failure because users may edit, inspect, or trust the wrong file without noticing. +## 3. Goals +- Resolve bare entry names in supported `ls`-style listing output against the listed directory when the command has exactly one supported directory-listing context. +- Prefer the listed directory over the block CWD when both locations contain an entry with the same name. +- Make files that only exist in the listed directory clickable. +- Make directories listed by `ls DIR/` clickable and open them the same way directories listed by plain `ls` open today. +- Preserve scrollback/session-restore behavior by deriving the listed directory from data stored with the block. +- Avoid regressions for non-listing commands and for output that already includes absolute or explicit relative paths. +- Keep hover/link detection responsive and avoid adding noticeable per-hover latency. +## 4. Non-goals +- Recursive listings such as `ls -R DIR/`; each output section has a different root and requires section-aware resolution. +- Multi-directory listings such as `ls DIR1/ DIR2/`; this requires per-section root tracking and should not be guessed from a single clicked filename. +- Piped or redirected output such as `ls DIR/ > file; cat file`; the original listing command context is no longer attached to the output block. +- `tree` or tree-shaped `eza --tree` output; these are blocked by separate tokenization behavior around box-drawing characters. +- Changing how URLs are detected or opened. +- Adding a new visual affordance, menu, tooltip copy, or confirmation dialog for file links. +- Implementing a broad shell command parser for every program that can print filenames. +## 5. Figma / Design References +Figma: none provided. +No visual design changes are expected. Existing file-link hover styling, cursor behavior, tooltips, and open-file routing should remain unchanged. +## 6. User Experience +### Supported commands +- The primary supported command is `ls`. +- The behavior should also apply to common `ls`-style replacement commands that Warp recognizes as listing commands, including `exa`, `eza`, and `lsd`. +- Aliases whose resolved top-level command is a supported listing command should behave like the supported command when Warp can resolve the alias from the block/session context. +- Commands outside the supported listing-command set, such as `find`, `cat`, `git`, and `tree`, should retain current file-link behavior. +### Single listed directory behavior +When a completed local BlockList block was produced by a supported listing command with one directory argument: +- Bare output entry `NAME` resolves first as `LISTED_DIR/NAME`. +- If `LISTED_DIR/NAME` exists and is a file, hovering/clicking `NAME` behaves as an existing file link and opens that file. +- If `LISTED_DIR/NAME` exists and is a directory, hovering/clicking `NAME` behaves as an existing folder link and opens the folder using the same routing as plain `ls` directory entries. +- If both `CWD/NAME` and `LISTED_DIR/NAME` exist, `LISTED_DIR/NAME` wins. +- If `LISTED_DIR/NAME` does not exist, Warp may fall back to the existing CWD-based resolver so unrelated existing behavior is preserved. +### Directory argument forms +The listed directory may be: +- Relative to the block's stored working directory, such as `ls subdir`, `ls subdir/`, or `ls ./subdir`. +- Absolute, such as `ls /tmp/warp-repro`. +- Tilde-prefixed when Warp can expand it consistently with existing file-link behavior, such as `ls ~/Downloads`. +- Quoted or escaped when the path contains spaces or shell-special characters, such as `ls "project files"` or `ls project\ files`. +- Passed after flags, such as `ls -la subdir/` or `ls --color=always subdir/`. +- Used after leading environment variable assignments, such as `LS_COLORS=auto ls subdir/`. +### Cases that should not become listing-aware +Warp should keep current behavior and avoid guessing a listed root when: +- The supported listing command has no directory argument, such as `ls` or `ls -la`. +- The first path-like positional is not an existing directory. +- The command uses multiple directory operands in one invocation. +- The command is recursive, such as `ls -R subdir/`. +- The output is in a different block from the listing command because of pipes, redirects, scripts, or `cat` of saved output. +- The detected output candidate already contains an absolute path or a rooted relative path such as `subdir/README.md`; Warp should not double-join it as `subdir/subdir/README.md`. +### Scrollback and restored sessions +- Clicking file links in older blocks should use the command and working directory associated with that block, not the terminal's current working directory. +- Blocks restored from a previous session should behave the same as live blocks when their stored command and `pwd` are available. +- Moving to another directory after running `ls subdir/` should not affect clicks in the earlier block. +### Failure behavior +- If Warp cannot parse a safe single listed directory from the command, the user should see no new failure state; existing file-link behavior should apply. +- If a listed entry no longer exists by the time the user hovers/clicks it, Warp should not link it unless the existing resolver would link another valid candidate. +- The fix should reduce silent wrong-file opens; it should not introduce a prompt or warning before opening files. +## 7. Success Criteria +1. In a block with `pwd = /tmp/warp-repro` and command `ls -la subdir/`, clicking `README.md` opens `/tmp/warp-repro/subdir/README.md`. +2. If both `/tmp/warp-repro/README.md` and `/tmp/warp-repro/subdir/README.md` exist, the listed directory file wins. +3. If only `/tmp/warp-repro/subdir/README.md` exists, `README.md` is clickable in `ls subdir/` output. +4. A directory entry listed by `ls subdir/` is clickable and opens as a folder, matching current plain `ls` behavior. +5. `ls`, `ls -la`, and other supported listing commands without a directory argument preserve current CWD-based link behavior. +6. Non-listing command output, including `find subdir -name '*.md'`, preserves current path-link behavior. +7. Output candidates that already include `subdir/README.md`, `/tmp/warp-repro/subdir/README.md`, or another explicit path are not double-prefixed. +8. Scrollback clicks in an older block continue to resolve using that block's command and stored `pwd`, even after the active shell has changed directories. +9. Restored blocks with stored command and `pwd` behave the same as live blocks. +10. The listing-aware path check does not make hover scanning visibly slower in blocks with many output rows. +11. Unsupported or ambiguous cases, including `ls -R subdir/` and `ls dir1/ dir2/`, do not silently choose an incorrect root. +## 8. Validation +- Manual: Create `/tmp/warp-repro/README.md` and `/tmp/warp-repro/subdir/README.md`, run `ls -la subdir/`, click `README.md`, and verify the subdirectory file opens. +- Manual: Remove the root `README.md`, run `ls subdir/`, and verify `README.md` remains clickable and opens the subdirectory file. +- Manual: Add a nested folder inside the listed directory, run `ls -la subdir/`, and verify clicking the folder entry opens it in Finder or the platform-equivalent folder route. +- Manual: Run `ls`, `ls -la`, `find subdir -name '*.md'`, and `cat saved-ls-output.txt` to verify existing behavior is unchanged for non-target cases. +- Manual: Run `ls -la subdir/`, `cd /tmp`, scroll back, and verify the older block still resolves against `/tmp/warp-repro/subdir/`. +- Automated: Add pure unit coverage for command-to-listed-directory parsing, including flags, quotes, escaped spaces, env var prefixes, absolute paths, tilde paths, malformed input, unknown commands, recursive listings, and multi-argument listings. +- Automated: Add link-resolution coverage showing that listed-directory resolution is attempted before CWD resolution and falls back safely when no listed directory applies. +- Performance: Validate that the command parsing and existence checks are bounded to the hovered candidate path and do not scan the whole block output on each hover. +## 9. Open Questions +- Should the supported listing-command set be user-configurable in the first implementation, or should the first implementation ship a conservative built-in set and add settings support later? +- Should multi-directory listings be completely disabled for listing-aware resolution, or is a temporary first-directory fallback acceptable? This spec recommends disabling ambiguous multi-directory listings to avoid new silent misresolution. diff --git a/specs/GH9908/tech.md b/specs/GH9908/tech.md new file mode 100644 index 000000000..f87a4d78a --- /dev/null +++ b/specs/GH9908/tech.md @@ -0,0 +1,122 @@ +# GH9908: Tech Spec — Listing-Aware File Link Resolution +## 1. Problem +File-link hover detection for terminal BlockList output currently resolves relative candidate paths against the block's stored working directory. That loses command context for `ls DIR/` output, where bare filenames in the output are entries under `DIR`, not entries under the block CWD. +The implementation should add a narrow listing-aware resolution root for supported `ls`-style commands. It should prefer the listed directory only when the block command and stored `pwd` make that root unambiguous, then fall back to the existing resolver for all other cases. +## 2. Relevant code +- `app/src/terminal/view/link_detection.rs:438` — `scan_for_file_path` selects the working directory used for file-link resolution. For BlockList links it currently reads `block.pwd()` and does not inspect the command. +- `app/src/terminal/view/link_detection.rs:500` — `compute_valid_paths` validates hovered candidate paths by calling `absolute_path_if_valid` against the selected working directory. +- `app/src/util/file.rs:32` — `absolute_path_if_valid` joins a candidate path against the supplied working directory and validates that the resolved path is a file or a directory without line/column metadata. +- `app/src/terminal/model/grid/grid_handler.rs:91` — `FILE_LINK_SEPARATORS` defines token separators for file-link candidates. +- `app/src/terminal/model/grid/grid_handler.rs:1097` — `possible_file_paths_at_point` returns candidate path fragments around the hovered cell, ordered longest to shortest. +- `app/src/terminal/model/terminal_model.rs:1826` and `app/src/terminal/model/blocks.rs:2439` — model/block wrappers route candidate extraction for AltScreen and BlockList. +- `app/src/terminal/model/block.rs:2172` — `Block::top_level_command` returns the block's top-level command and resolves a single alias level through the session. +- `app/src/terminal/model/block/serialized_block.rs:145` — serialized blocks store `stylized_command` and `pwd`, which allows restored blocks to retain the data needed for listing-aware resolution. +- `app/src/terminal/view/open_in_warp.rs:292` — `check_openable_in_warp` shows an existing pattern for parsing command positionals from completed commands. +- `crates/warp_completer/src/parsers/simple/mod.rs:44` — `top_level_command` parses the command name while skipping leading environment variable assignments. +## 3. Current state +On hover, `TerminalView::maybe_link_hover` queues a file-link scan when the hovered fragment changes. `scan_for_file_path` gathers: +- The resolver root: active local `pwd` for AltScreen, or `block.pwd()` for BlockList. +- Candidate path substrings from `possible_file_paths_at_point`. +- Shell launch data for shell-native path conversion. +The expensive work runs on a background thread via `compute_valid_paths`. For each candidate, Warp calls `absolute_path_if_valid(candidate, ShellPathType::ShellNative(working_directory), shell_launch_data)`. Prefix and suffix cleanup for git-diff-style `a/`, `b/`, and symlink `@` runs only after the first direct validation attempt fails. +This design is already asynchronous and existence-based, but it has only one root. For `ls subdir/` output, the root should be `block.pwd()/subdir`, while the current code always uses `block.pwd()`. Tokenization is not the primary problem for ordinary `ls` output because bare filenames like `README.md` are already valid candidates. +## 4. Proposed changes +### Add a pure listing-command resolver helper +Add a small helper module under `crates/warp_util/src/listing_command.rs` and export it from `crates/warp_util/src/lib.rs`. +Recommended API: +- `pub const DEFAULT_LISTING_COMMANDS: &[&str] = &["ls", "exa", "eza", "lsd"];` +- `pub fn listing_command_argument_dir(command: &str, pwd: &Path, listing_commands: &[&str]) -> Option;` +- Optionally add an internal variant that accepts a resolved top-level command override when the caller already resolved an alias. +The helper should: +- Parse shell-like tokens with existing dependency support where possible. `shlex` is already available in the app crate; if the helper lives in `warp_util`, add `shlex` to `crates/warp_util/Cargo.toml` or use an existing parser that is already available to `warp_util`. +- Skip leading `KEY=VALUE` environment variable assignments before checking the command name. +- Check the command name against `listing_commands`. +- Skip flags and flag values that are known not to be directory operands. +- Reject recursive mode for `ls -R` and `ls --recursive`. +- Return a directory only when there is exactly one usable directory operand for V1. +- Resolve relative operands against `pwd`, preserve absolute operands, and support tilde expansion if implemented consistently with existing path-link expansion. +- Return `None` for malformed input, unknown commands, missing operands, non-directory operands, recursive listings, and multi-directory listings. +The helper should not inspect terminal output. It should be pure command/pwd logic plus bounded metadata checks for candidate directory operands. +### Thread listing root through BlockList scanning +Update `scan_for_file_path` so BlockList scanning can collect both: +- The existing `working_directory` root from `block.pwd()`. +- An optional `listing_directory` root derived from the block command and `pwd`. +For AltScreen scanning, keep the current active-pwd behavior and do not attempt listing-aware resolution. AltScreen content is live screen state rather than durable completed-block output, and it does not have the same completed block command boundary. +For BlockList scanning: +- Skip all file-link detection for remote blocks exactly as the current code does. +- Read the block command using the non-secret display command path already used by the block model, such as `block.command_to_string()` or the same source used by `Block::top_level_command`. +- Use `Block::top_level_command(self.sessions.as_ref(ctx))` to support simple aliases where the user-typed command resolves to `ls`, `exa`, `eza`, or `lsd`. +- Compute `listing_directory` only when `block.pwd()` exists and the helper returns a valid directory. +- Move the optional `listing_directory` into the same background thread as `compute_valid_paths` so the UI thread does not do repeated candidate validation. +### Prefer listed-directory resolution before existing CWD resolution +Change `compute_valid_paths` to accept an optional listed-directory root: +- Existing root: `working_directory: &str`. +- New root: `listing_directory: Option`. +For each candidate path: +1. If `listing_directory` exists and the candidate is not already absolute and does not already include a path component that should resolve correctly against `working_directory`, try `absolute_path_if_valid(candidate, ShellPathType::PlatformNative(listing_directory.clone()), shell_launch_data)`. +2. If that succeeds, create the link with the original candidate range and return it. +3. Otherwise run the existing `ShellNative(working_directory)` validation and prefix/suffix cleanup unchanged. +The double-join guard is important. A candidate such as `subdir/README.md`, `./subdir/README.md`, `../other/file.txt`, or `/tmp/warp-repro/subdir/README.md` should not become `subdir/subdir/README.md`. A conservative V1 rule is: +- Use `listing_directory` only for bare output entry names with no path separators after cleanup. +- Continue using the existing resolver for candidates that include `/`, `\`, `./`, `../`, `~`, drive prefixes, or absolute-path syntax. +This conservative rule matches ordinary `ls DIR/` output and avoids altering rooted output from `find`, `grep`, compiler diagnostics, and tools that already print paths. +### Preserve prefix/suffix behavior +The existing `a/` and `b/` prefix cleanup and `@` symlink suffix cleanup should remain intact. Apply listed-directory resolution only to the same candidate forms that are safe to treat as bare entry names. If suffix cleanup turns `name@` into bare `name`, listed-directory resolution can be attempted before falling back to CWD resolution. +Do not apply listed-directory resolution to `a/name` or `b/name`; those are path-like and should remain covered by existing git-diff behavior. +### Settings and extensibility +V1 can ship with `DEFAULT_LISTING_COMMANDS` as the supported set. If reviewers require user configurability in the first implementation, introduce a settings-backed command list that defaults to the same values and pass that list into the helper from `TerminalView`. +If settings support is deferred, keep the helper signature list-based rather than hard-coding internally. That preserves an easy path to a user-configurable command list without changing link-detection semantics later. +## 5. End-to-end flow +1. User hovers `README.md` in a completed BlockList block produced by `ls -la subdir/`. +2. `maybe_link_hover` queues `FindLinkArg` because the hovered fragment changed. +3. `scan_for_file_path` identifies the hovered block, verifies it is local, reads `block.pwd()`, and reads the block command. +4. `listing_command_argument_dir("ls -la subdir/", Path::new(block_pwd), DEFAULT_LISTING_COMMANDS)` returns `block_pwd/subdir`. +5. `possible_file_paths_at_point` returns candidates such as `README.md`. +6. The background thread calls `compute_valid_paths` with both `working_directory = block_pwd` and `listing_directory = block_pwd/subdir`. +7. `compute_valid_paths` sees that `README.md` is a bare candidate, validates `block_pwd/subdir/README.md`, and creates a `FileLink` for the original output range. +8. `handle_file_link_completed` installs the highlighted file link and cursor state exactly as it does today. +9. Opening the highlighted link uses the existing `open_file_path` or folder route based on the resolved absolute path. +## 6. Risks and mitigations +- Risk: New silent misresolution for multi-directory listings. Mitigation: return `None` when more than one directory operand is present in V1. +- Risk: Recursive listings use changing roots per section. Mitigation: detect `-R` and `--recursive` and opt out. +- Risk: Double-joining paths that already include a directory. Mitigation: only apply listed-directory resolution to bare names and fall back to the current resolver for path-like candidates. +- Risk: Per-hover performance regression from command parsing or filesystem checks. Mitigation: parse once per hover, only check the command operands and hovered candidate, keep work off the UI thread, and avoid scanning block output. +- Risk: Moving command parsing into `warp_util` adds dependencies or duplicates shell parsing behavior. Mitigation: prefer the smallest dependency surface, add focused tests, and keep parsing logic limited to listing-command operands. +- Risk: Alias handling can be incomplete. Mitigation: support the existing single-level alias resolution from `Block::top_level_command`; document transitive or positional alias limitations as follow-ups. +- Risk: Remote sessions may have shell-native path conversion needs that differ from local files. Mitigation: preserve the existing remote-block skip behavior and only apply this to local BlockList scanning. +## 7. Testing and validation +### Unit tests for the helper +Add tests in `crates/warp_util` for: +- `ls subdir`, `ls subdir/`, `ls ./subdir`, `ls /absolute/path`. +- `ls -la subdir`, `ls --color=always subdir`, and flags before operands. +- Quoted and escaped directory names with spaces. +- Leading environment assignments such as `LS_COLORS=auto ls subdir`. +- Tilde-prefixed directory operands if tilde support is implemented. +- Unknown commands such as `find`, `cat`, and `git`. +- Missing operands, non-directory operands, malformed shell input, `ls -R subdir`, `ls --recursive subdir`, and `ls dir1 dir2`. +- Custom listing-command lists if the helper accepts caller-provided command sets. +### Unit tests for resolution order +Add targeted tests around `compute_valid_paths` or a new pure helper extracted from it: +- Listed-directory match beats CWD match for a same-named file. +- Listed-directory-only file becomes linkable. +- Listed-directory-only directory becomes linkable when there is no line/column suffix. +- Existing CWD behavior remains for no listed directory. +- Path-like candidates with slashes are not resolved through the listed directory. +- Symlink suffix cleanup with `name@` can resolve under the listed directory. +### Integration/manual validation +- Run the repro from the issue on macOS and verify the clicked Markdown file is the subdirectory copy. +- Verify directories listed by `ls DIR/` open in Finder. +- Verify scrollback after `cd` and restored sessions still use the original block command and `pwd`. +- Verify `find DIR -name '*.md'`, compiler diagnostics, git diff paths, and plain `ls` still behave as before. +### Repository checks +Recommended checks after implementation: +- `cargo test -p warp_util --lib listing_command` +- Targeted app tests covering link detection if available. +- `cargo fmt --all --check` +- `cargo clippy -p warp --all-targets -- -D warnings` if feasible in the development environment. +## 8. Follow-ups +- Add a user-facing setting for listing-aware commands if it is not included in V1. +- Add section-aware support for `ls -R`. +- Add section-aware support for multi-directory listings. +- Address the separate tokenizer issue for `tree` and tree-shaped output using box-drawing characters. +- Consider caching parsed listing roots per block if profiling shows command parsing is measurable, though the current proposed work should be bounded enough to avoid needing a cache.