Skip to content

project: Use runnable kind to disambiguate between cargo and shell commands - #54011

Merged
SomeoneToIgnore merged 3 commits into
zed-industries:mainfrom
sunshowers:runnable-kind
Apr 27, 2026
Merged

project: Use runnable kind to disambiguate between cargo and shell commands#54011
SomeoneToIgnore merged 3 commits into
zed-industries:mainfrom
sunshowers:runnable-kind

Conversation

@sunshowers

Copy link
Copy Markdown
Contributor

Found this bug while investigating why configuring nextest based on the instructions at rust-lang/rust-analyzer#21137 (comment) wasn't working within Zed.

Previously, we'd use serde(untagged), preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}

would end up getting deserialized as a Cargo command, silently dropping program and args.

With this fix, we now use the provided kind as a tag. We do have to introduce a #[serde(flatten)] unfortunately, which has a few side effects due to internal buffering, but #[serde(untagged)] also does internal buffering so this doesn't make things worse.

I've manually tested this by configuring:

{
    "lsp": {
        "rust-analyzer": {
            "initialization_options": {
                "runnables": {
                    "test": {
                        "overrideCommand": [
                            "cargo",
                            "nextest",
                            "run",
                            "--package",
                            "${package}",
                            "${target_arg}",
                            "${target}",
                            "--",
                            "${test_name}",
                            "${exact}",
                            "${include_ignored}"
                        ]
                    }
                }
            }
        }
    }
}

and ensuring that nextest is correctly invoked.

Self-Review Checklist:

  • I've reviewed my own diff for quality, security, and reliability
  • Unsafe blocks (if any) have justifying comments
  • The content is consistent with the UI/UX checklist
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Release Notes:

  • Fixed deserialization of rust-analyzer shell runnables.

@cla-bot

cla-bot Bot commented Apr 15, 2026

Copy link
Copy Markdown

We require contributors to sign our Contributor License Agreement, and we don't have @sunshowers on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@zed-codeowner-coordinator
zed-codeowner-coordinator Bot requested review from a team, cameron1024, cole-miller and eholk and removed request for a team April 15, 2026 21:01
@zed-community-bot zed-community-bot Bot added the first contribution the author's first pull request to Zed. NOTE: the label application is automated via github actions label Apr 15, 2026
@sunshowers

Copy link
Copy Markdown
Contributor Author

@cla-bot check

@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label Apr 15, 2026
@cla-bot

cla-bot Bot commented Apr 15, 2026

Copy link
Copy Markdown

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

@zed-industries-bot

zed-industries-bot commented Apr 19, 2026

Copy link
Copy Markdown
Contributor
Messages
📖

This PR includes links to the following GitHub Issues: #rust-lang/rust-analyzer#21137
If this PR aims to close an issue, please include a Closes #ISSUE line at the top of the PR body.

Generated by 🚫 dangerJS against 8d60f29

@SomeoneToIgnore SomeoneToIgnore self-assigned this Apr 19, 2026

@SomeoneToIgnore SomeoneToIgnore left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for digging into this.

I guess then

// https://rust-analyzer.github.io/book/contributing/lsp-extensions.html#runnables
// Taken from https://github.com/rust-lang/rust-analyzer/blob/a73a37a757a58b43a796d3eb86a1f7dfd0036659/crates/rust-analyzer/src/lsp/ext.rs#L425-L489
deserves some update/rewrite, can you do that?

I think CI fails will go away with a rebase and are not related.

…mmands

Found this bug while investigating why configuring nextest based on the instructions at rust-lang/rust-analyzer#21137 (comment) wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.
@sunshowers

sunshowers commented Apr 26, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @SomeoneToIgnore. I've rebased this on top of main and added a commit with an expanded comment on top. (Happy to squash this in if desired as well.)

To be honest I'd consider r-a to not be quite correct here, though I think it only does serializing so it doesn't get hit by this.

@SomeoneToIgnore SomeoneToIgnore left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you!
No need to squash, we do that in the end anyway.

Fully agree with you that r-a has to get this fix ideally.

@Veykril Veykril self-assigned this Apr 27, 2026
@SomeoneToIgnore
SomeoneToIgnore added this pull request to the merge queue Apr 27, 2026
Merged via the queue into zed-industries:main with commit 47fb164 Apr 27, 2026
31 checks passed
@Veykril

Veykril commented Apr 27, 2026

Copy link
Copy Markdown
Member

To be honest I'd consider r-a to not be quite correct here, though I think it only does serializing so it doesn't get hit by this.

Curious to hear what the issue is on the r-a side (in your opinion). Note I haven't checked this PR yet to detail yet to catch up with this.

@Veykril

Veykril commented Apr 27, 2026

Copy link
Copy Markdown
Member

Ah, the fact that we have the enums defined as untagged despite clearly having a tag field I suppose?

@sunshowers
sunshowers deleted the runnable-kind branch April 27, 2026 15:27
@sunshowers

Copy link
Copy Markdown
Contributor Author

Ah, the fact that we have the enums defined as untagged despite clearly having a tag field I suppose?

Yeah, exactly -- I think the functional change in this PR can be directly transferred over to r-a. Do you want to do that or should I?

@Veykril

Veykril commented Apr 30, 2026

Copy link
Copy Markdown
Member

If you could do that I'd really appreciate it, sounds like a reasonable change

@sunshowers

Copy link
Copy Markdown
Contributor Author

If you could do that I'd really appreciate it, sounds like a reasonable change

Did this at rust-lang/rust-analyzer#22295.

sunshowers added a commit to sunshowers/rust-analyzer that referenced this pull request May 6, 2026
Found this bug while investigating why configuring nextest based on the instructions at rust-lang#21137 (comment) wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.

See zed-industries/zed#54011 for the original fix to Zed.
sunshowers added a commit to sunshowers/rust-analyzer that referenced this pull request May 6, 2026
Found this bug while investigating why configuring nextest based on the instructions at rust-lang#21137 (comment) wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.

See zed-industries/zed#54011 for the original fix to Zed.
sunshowers added a commit to sunshowers/rust-analyzer that referenced this pull request May 6, 2026
Found this bug while investigating why configuring nextest based on the instructions at rust-lang#21137 (comment) wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.

See zed-industries/zed#54011 for the original fix to Zed.
ebaah46 pushed a commit to ebaah46/zed that referenced this pull request May 6, 2026
…mmands (zed-industries#54011)

Found this bug while investigating why configuring nextest based on the
instructions at
rust-lang/rust-analyzer#21137 (comment)
wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell
commands. The problem is that every instance of a shell command is a
valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping
`program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to
introduce a `#[serde(flatten)]` unfortunately, which has a few side
effects due to internal buffering, but `#[serde(untagged)]` also does
internal buffering so this doesn't make things worse.

I've manually tested this by configuring:

```json
{
    "lsp": {
        "rust-analyzer": {
            "initialization_options": {
                "runnables": {
                    "test": {
                        "overrideCommand": [
                            "cargo",
                            "nextest",
                            "run",
                            "--package",
                            "${package}",
                            "${target_arg}",
                            "${target}",
                            "--",
                            "${test_name}",
                            "${exact}",
                            "${include_ignored}"
                        ]
                    }
                }
            }
        }
    }
}
```

and ensuring that nextest is correctly invoked.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed deserialization of rust-analyzer shell runnables.

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
sunshowers added a commit to sunshowers/rust-analyzer that referenced this pull request May 6, 2026
Found this bug while investigating why configuring nextest based on the instructions at rust-lang#21137 (comment) wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.

See zed-industries/zed#54011 for the original fix to Zed.
kathbigra pushed a commit to kathbigra/zed that referenced this pull request May 10, 2026
…mmands (zed-industries#54011)

Found this bug while investigating why configuring nextest based on the
instructions at
rust-lang/rust-analyzer#21137 (comment)
wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell
commands. The problem is that every instance of a shell command is a
valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping
`program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to
introduce a `#[serde(flatten)]` unfortunately, which has a few side
effects due to internal buffering, but `#[serde(untagged)]` also does
internal buffering so this doesn't make things worse.

I've manually tested this by configuring:

```json
{
    "lsp": {
        "rust-analyzer": {
            "initialization_options": {
                "runnables": {
                    "test": {
                        "overrideCommand": [
                            "cargo",
                            "nextest",
                            "run",
                            "--package",
                            "${package}",
                            "${target_arg}",
                            "${target}",
                            "--",
                            "${test_name}",
                            "${exact}",
                            "${include_ignored}"
                        ]
                    }
                }
            }
        }
    }
}
```

and ensuring that nextest is correctly invoked.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed deserialization of rust-analyzer shell runnables.

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
lnicola pushed a commit to lnicola/rust that referenced this pull request May 11, 2026
Found this bug while investigating why configuring nextest based on the instructions at rust-lang/rust-analyzer#21137 (comment) wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell commands. The problem is that every instance of a shell command is a valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping `program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to introduce a `#[serde(flatten)]` unfortunately, which has a few side effects due to internal buffering, but `#[serde(untagged)]` also does internal buffering so this doesn't make things worse.

See zed-industries/zed#54011 for the original fix to Zed.
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
…mmands (zed-industries#54011)

Found this bug while investigating why configuring nextest based on the
instructions at
rust-lang/rust-analyzer#21137 (comment)
wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell
commands. The problem is that every instance of a shell command is a
valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping
`program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to
introduce a `#[serde(flatten)]` unfortunately, which has a few side
effects due to internal buffering, but `#[serde(untagged)]` also does
internal buffering so this doesn't make things worse.

I've manually tested this by configuring:

```json
{
    "lsp": {
        "rust-analyzer": {
            "initialization_options": {
                "runnables": {
                    "test": {
                        "overrideCommand": [
                            "cargo",
                            "nextest",
                            "run",
                            "--package",
                            "${package}",
                            "${target_arg}",
                            "${target}",
                            "--",
                            "${test_name}",
                            "${exact}",
                            "${include_ignored}"
                        ]
                    }
                }
            }
        }
    }
}
```

and ensuring that nextest is correctly invoked.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed deserialization of rust-analyzer shell runnables.

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
…mmands (zed-industries#54011)

Found this bug while investigating why configuring nextest based on the
instructions at
rust-lang/rust-analyzer#21137 (comment)
wasn't working within Zed.

Previously, we'd use `serde(untagged)`, preferring cargo over shell
commands. The problem is that every instance of a shell command is a
valid instance of a cargo command. For example, the shell command:

```json
{
    "label": "test my_test",
    "kind": "shell",
    "args": {
        "environment": {"RUSTC_TOOLCHAIN": "/path/to/toolchain"},
        "cwd": "/project",
        "program": "cargo",
        "args": ["nextest", "run", "--package", "my-crate", "--lib", "--", "my_test", "--exact", "--include-ignored"]
    }
}
```

would end up getting deserialized as a Cargo command, silently dropping
`program` and `args`.

With this fix, we now use the provided `kind` as a tag. We do have to
introduce a `#[serde(flatten)]` unfortunately, which has a few side
effects due to internal buffering, but `#[serde(untagged)]` also does
internal buffering so this doesn't make things worse.

I've manually tested this by configuring:

```json
{
    "lsp": {
        "rust-analyzer": {
            "initialization_options": {
                "runnables": {
                    "test": {
                        "overrideCommand": [
                            "cargo",
                            "nextest",
                            "run",
                            "--package",
                            "${package}",
                            "${target_arg}",
                            "${target}",
                            "--",
                            "${test_name}",
                            "${exact}",
                            "${include_ignored}"
                        ]
                    }
                }
            }
        }
    }
}
```

and ensuring that nextest is correctly invoked.

Self-Review Checklist:

- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable

Release Notes:

- Fixed deserialization of rust-analyzer shell runnables.

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The user has signed the Contributor License Agreement first contribution the author's first pull request to Zed. NOTE: the label application is automated via github actions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants