Skip to content

Run executable shell scripts in the terminal instead of opening in the editor#9503

Merged
captainsafia merged 2 commits intowarpdotdev:masterfrom
amriksingh0786:fix/issue-9005-shell-script-default-action
Apr 30, 2026
Merged

Run executable shell scripts in the terminal instead of opening in the editor#9503
captainsafia merged 2 commits intowarpdotdev:masterfrom
amriksingh0786:fix/issue-9005-shell-script-default-action

Conversation

@amriksingh0786
Copy link
Copy Markdown
Contributor

Description

is_file_openable_in_warp returns Some(Text) for any non-binary file, which shadowed the existing executor branch in open_file when handling a file:// URL — so executable shell scripts were opened in the editor instead of being run. The URI handler now classifies the action up front via a new pure helper classify_open_file_action, and routes executable shell scripts to the executor path that the rest of the agent flow already uses.

Detection rules for "runnable shell script":

  • Unix: user-execute bit set AND (extension in {sh, bash, zsh, fish, ksh} OR file starts with a #! shebang).
  • Windows: extension in {ps1, bat, cmd} (no x-bit concept).

Non-executable shell scripts still open in the editor — the routing is gated on the user-execute bit on Unix, so this only changes behavior for files the user marked executable.

Fixes #9005.

Testing

Unit tests added at both layers:

  • is_runnable_shell_script — 7 tests in app/src/util/openable_file_type.rs: executable .sh, non-executable .sh, alternate executable extensions (.bash, .zsh, .fish, .ksh), shebang-with-no-extension (with and without x-bit), plain-text rejection, symlink to executable.
  • classify_open_file_action — 6 tests in app/src/uri/uri_test.rs: executable .sh → execute, non-executable .sh → editor, executable .bash/.zsh/.fish → execute, markdown → notebook, Rust source → editor, directory → session.

./script/presubmit is clean (cargo fmt, cargo clippy --workspace --all-targets --tests -- -D warnings, clang-format, wgslfmt, cargo nextest). The 5 shell_integration_tests::*ssh* failures observed locally are unrelated to this change — they require a Docker SSH container — and are skipped on fork PRs per #9304.

Agent Mode

  • Warp Agent Mode — This PR was created via Warp's AI Agent Mode

CHANGELOG-BUG-FIX: Executable shell scripts opened from a `file://` URL now run in the terminal instead of opening in the editor.

@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Apr 29, 2026

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @amriksingh0786 on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

@oz-for-oss
Copy link
Copy Markdown
Contributor

oz-for-oss Bot commented Apr 29, 2026

@amriksingh0786

I'm starting a first review of this pull request.

I reviewed this pull request and requested human review from: @seemeroland.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

I'm re-reviewing this pull request in response to a review request.

I completed the review and posted feedback on this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @seemeroland.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR changes file:// routing so executable shell scripts bypass the editor and are queued in a terminal session, with unit coverage for URI routing and shell-script detection.

Concerns

  • The Unix executable check uses any execute bit instead of the documented user-execute bit, so files not executable by the owner can be routed to execution.
  • No-extension shebang detection reads the entire file into memory, which can block or exhaust memory for large local files opened through the URI path.

Security

  • The no-extension shebang path performs unbounded file reads from a file://-provided path; limit the read to the prefix needed for detection.

Verdict

Found: 0 critical, 2 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread app/src/util/openable_file_type.rs Outdated
Comment thread app/src/util/openable_file_type.rs Outdated
@oz-for-oss oz-for-oss Bot requested a review from seemeroland April 29, 2026 21:52
@seemeroland
Copy link
Copy Markdown
Contributor

Feel free to re-request review from the oz agent once comments are addressed! Removing myself for now

@seemeroland seemeroland removed their request for review April 29, 2026 22:19
…e editor

`is_file_openable_in_warp` returns `Some(Text)` for any non-binary file,
which shadowed the existing executor branch when opening a `.sh` from a
`file://` URL. Now the URI handler classifies the action up front and
falls through to the executor path when the target is an executable
shell script (x-bit + .sh/bash/zsh/fish/ksh extension or `#!` shebang on
Unix; .ps1/.bat/.cmd on Windows), matching the rest of the agent flow.

Fixes warpdotdev#9005
@amriksingh0786 amriksingh0786 force-pushed the fix/issue-9005-shell-script-default-action branch from 66460f4 to ce4cd2f Compare April 29, 2026 22:36
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Apr 29, 2026

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @amriksingh0786 on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment @cla-bot check to trigger another check.

@amriksingh0786
Copy link
Copy Markdown
Contributor Author

Thanks for the review — both points addressed in ce4cd2f (force-pushed after rebasing onto current master):

  • User-execute bit only. 0o111 → 0o100 so a file with mode 0o070 (group-execute, no user-execute) no longer classifies as runnable. Doc comment updated to match.
  • Bounded shebang read. Replaced std::fs::read(path) with File::open(path) + read_exact(&mut [0u8; 2]); only the first two bytes are pulled in, so a large file://-opened file can't OOM the URI handler.
  • Added regression test test_is_runnable_shell_script_group_only_executable_rejected (mode 0o070) for the first change. The existing helper tests for 0o755 / 0o644 still cover the happy path and the no-execute path.

CLA signing in progress separately.

/oz-review

oz-for-oss[bot]
oz-for-oss Bot previously requested changes Apr 29, 2026
Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR extracts file:// routing into classify_open_file_action and adds shell-script detection so executable shell scripts can be queued in a terminal instead of opened in the editor.

Concerns

  • The URI classifier does not apply the new Unix x-bit gate to extensionless shell scripts that is_file_openable_in_warp treats as non-openable; those still fall through to execution, including non-executable shebang files.

Verdict

Found: 0 critical, 1 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Comment thread app/src/uri/mod.rs Outdated
@amriksingh0786
Copy link
Copy Markdown
Contributor Author

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed label Apr 29, 2026
@cla-bot
Copy link
Copy Markdown

cla-bot Bot commented Apr 29, 2026

The cla-bot has been summoned, and re-checked this pull request!

An extensionless `#!/bin/sh` file without the user-execute bit was falling
through to `ExecuteInSession` because `is_file_openable_in_warp` rejects
extensionless files by extension only, so the `!is_runnable_shell_script`
gate never fired. Such files now route to the editor.

Extracts `starts_with_shebang` from `is_runnable_shell_script` and reuses
it both there and in `classify_open_file_action`. Adds a URI-layer
regression test plus four helper tests for shebang detection.
@amriksingh0786
Copy link
Copy Markdown
Contributor Author

/oz-review please re-review i believe all comments are resolved, thanks.

Copy link
Copy Markdown
Contributor

@oz-for-oss oz-for-oss Bot left a comment

Choose a reason for hiding this comment

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

Overview

This PR refactors file:// URL handling to classify the intended open action before dispatching, allowing executable shell scripts to route through the existing terminal execution path while preserving markdown, editor, and directory behavior. It also adds focused unit coverage for runnable shell-script detection and URI routing.

Concerns

  • None found.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Copy link
Copy Markdown
Contributor

@seemeroland seemeroland left a comment

Choose a reason for hiding this comment

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

Nice fix, thanks for the contribution!

@amriksingh0786
Copy link
Copy Markdown
Contributor Author

@seemeroland the ci is failing, i can't merge. and it feels like its unrelated to my changes, can we try to re-run the CI. I don't see any option from my side to do it.

@amriksingh0786
Copy link
Copy Markdown
Contributor Author

@seemeroland the ci is failing, i can't merge. and it feels like its unrelated to my changes, can we try to re-run the CI. I don't see any option from my side to do it.

Error: Custom { kind: NotFound, error: "Could not find protoc. If protoc is installed, try setting the PROTOC environment variable to the path of the protoc binary. Try installing protobuf-compiler or protobuf using your package manager. It is also available at https://github.com/protocolbuffers/protobuf/releases For more information: https://docs.rs/prost-build/#sourcing-protoc"

@seemeroland
Copy link
Copy Markdown
Contributor

Yea it seems like installing protoc in prepare environment is flakey, rerunning CI

@amriksingh0786
Copy link
Copy Markdown
Contributor Author

seemeroland

Thanks, will you be merging it, because i dont see a merge button on my end.
@seemeroland

@captainsafia captainsafia added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label Apr 30, 2026 — with Warp Dev Github Integration
@amriksingh0786
Copy link
Copy Markdown
Contributor Author

thanks for label @captainsafia , i see "1 workflow awaiting approval, This workflow requires approval from a maintainer." and merge button is still not visible.

@captainsafia captainsafia merged commit edac651 into warpdotdev:master Apr 30, 2026
41 of 43 checks passed
zerx-lab pushed a commit to zerx-lab/warp that referenced this pull request May 3, 2026
…e editor (warpdotdev#9503)

`is_file_openable_in_warp` returns `Some(Text)` for any non-binary file,
which shadowed the existing executor branch in `open_file` when handling
a `file://` URL — so executable shell scripts were opened in the editor
instead of being run. The URI handler now classifies the action up front
via a new pure helper `classify_open_file_action`, and routes executable
shell scripts to the executor path that the rest of the agent flow
already uses.

Detection rules for "runnable shell script":

- **Unix:** user-execute bit set AND (extension in `{sh, bash, zsh,
fish, ksh}` OR file starts with a `#!` shebang).
- **Windows:** extension in `{ps1, bat, cmd}` (no x-bit concept).

Non-executable shell scripts still open in the editor — the routing is
gated on the user-execute bit on Unix, so this only changes behavior for
files the user marked executable.

Fixes warpdotdev#9005.

Unit tests added at both layers:

- **`is_runnable_shell_script`** — 7 tests in
`app/src/util/openable_file_type.rs`: executable `.sh`, non-executable
`.sh`, alternate executable extensions (`.bash`, `.zsh`, `.fish`,
`.ksh`), shebang-with-no-extension (with and without x-bit), plain-text
rejection, symlink to executable.
- **`classify_open_file_action`** — 6 tests in
`app/src/uri/uri_test.rs`: executable `.sh` → execute, non-executable
`.sh` → editor, executable `.bash`/`.zsh`/`.fish` → execute, markdown →
notebook, Rust source → editor, directory → session.

`./script/presubmit` is clean (`cargo fmt`, `cargo clippy --workspace
--all-targets --tests -- -D warnings`, `clang-format`, `wgslfmt`, `cargo
nextest`). The 5 `shell_integration_tests::*ssh*` failures observed
locally are unrelated to this change — they require a Docker SSH
container — and are skipped on fork PRs per warpdotdev#9304.

- [ ] Warp Agent Mode — This PR was created via Warp's AI Agent Mode

CHANGELOG-BUG-FIX: Executable shell scripts opened from a \`file://\`
URL now run in the terminal instead of opening in the editor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed external-contributor Indicates that a PR has been opened by someone outside the Warp team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Instead of running shell scripts, default action is to open in editor.

3 participants