Skip to content

Inject absolute resolve-command invocation - #531

Open
vanzue wants to merge 9 commits into
mainfrom
dev/vanzue/inject-resolve-command-runtime
Open

Inject absolute resolve-command invocation#531
vanzue wants to merge 9 commits into
mainfrom
dev/vanzue/inject-resolve-command-runtime

Conversation

@vanzue

@vanzue vanzue commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • inject the current wta.exe as a structured absolute executable + argv contract, with a PowerShell-safe fallback string
  • select independent command-resolution sources from shell context: host PATH for supported host shells and profile-aware resolution for PowerShell
  • merge positive source results while reserving not_found for clean authoritative misses; partial non-PowerShell misses remain indeterminate
  • retain the resolver on subsequent planner turns and omit it when no source applies, such as WSL

Testing

  • cargo test --manifest-path tools/wta/Cargo.toml

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 02:18

Copilot AI 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.

Pull request overview

This PR adds a new planner-only runtime context section that tells the agent how to invoke wta resolve-command via an absolute, PowerShell-safe executable path, and updates tests/docs to expect and use that injected guidance.

Changes:

  • Added a PowerShell invocation template helper (with quoting) for resolve-command.
  • Injected a new “Command Resolver Invocation” context section into planner prompts (excluded from autofix).
  • Updated ACP prompt-building tests and the Terminal Agent persona guidance to reference the injected invocation.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
tools/wta/src/resolve_command.rs Adds PowerShell-safe quoting + an invocation template helper (with unit test).
tools/wta/src/protocol/acp/prompt_context.rs Injects a new planner-only context provider emitting the command-resolver invocation section.
tools/wta/src/protocol/acp/client.rs Updates prompt-building tests to assert the new planner context section is included/omitted appropriately.
tools/wta/prompts/terminal-agent.md Updates agent guidance to use the injected invocation template instead of a bare wta call.

Comment thread tools/wta/src/protocol/acp/prompt_context.rs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 02:53
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tools/wta/src/protocol/acp/prompt_context.rs:165

  • The injected Command Resolver Invocation text says to replace <name> with a single-quoted argument, but the template already includes quotes around <name> (resolve-command '<name>' ...). This can lead to double-quoting (e.g. ''deploy-it'') if followed literally. Reword the instruction to make it explicit that '<name>' is the token being replaced.
                "Replace `<name>` with the command name as one PowerShell \
                 single-quoted argument, doubling any embedded `'`, then run:\n\
                 ```powershell\n{}\n```",

tools/wta/prompts/terminal-agent.md:39

  • This guidance tells the agent to replace <name> with a single-quoted argument, but the injected runtime template already has '<name>' quoted. Clarify that the agent should replace the literal '<name>' token with the desired single-quoted value (including quotes) to avoid producing ''name''.
1. **Identify what the name resolves to first.** Use the exact command template from the runtime context's **Command Resolver Invocation** section, replacing `<name>` with `deploy-it` as one PowerShell single-quoted argument. Do not replace its absolute executable path with bare `wta`: the injected path identifies the packaged WTA binary even when the agent's tool environment does not expose it on `PATH`. **Prefer this resolver over your own `Get-Command`/`Get-Alias` probe**: it loads the user's profile, so it reports profile-defined **aliases and functions** your own `execute_command` probe misses (a plain probe usually runs without the user's profile). It returns a `status`: `exists` (with the command's type + resolved target, e.g. an alias → `where.exe`), `not_found` (with the closest real commands), `indeterminate` (couldn't verify — don't assume it's missing), or `unsupported` (the selected shell is not PowerShell). If the runtime section is absent or its invocation fails to start, probe yourself with `execute_command`: `Get-Command deploy-it -All` — and describe it by its actual type (Application / ExternalScript / Cmdlet / Function / Alias); don't assume "on PATH" when it might be a function or alias.

Copilot AI review requested due to automatic review settings July 31, 2026 02:57

Copilot AI 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.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tools/wta/src/protocol/acp/prompt_context.rs:167

  • The instructions for replacing <name> are ambiguous: the injected template already wraps <name> in single quotes ('<name>'), but the text says to replace it with a full single-quoted argument, which could lead to users/agents producing doubled quotes (e.g. ''deploy-it''). Clarify that <name> should be replaced inside the existing quotes, only escaping embedded ' by doubling.
            body: format!(
                "Replace `<name>` with the command name as one PowerShell \
                 single-quoted argument, doubling any embedded `'`, then run:\n\
                 ```powershell\n{}\n```",

tools/wta/prompts/terminal-agent.md:39

  • This sentence says to replace <name> with a single-quoted argument, but the injected resolver template already includes quotes around <name> ('<name>'). This ambiguity can cause a malformed invocation if the reader adds another set of quotes. Consider clarifying that the replacement happens inside the template’s existing quotes (and only needs ' escaping via doubling).
1. **Identify what the name resolves to first.** Use the exact command template from the runtime context's **Command Resolver Invocation** section, replacing `<name>` with `deploy-it` as one PowerShell single-quoted argument. Do not replace its absolute executable path with bare `wta`: the injected path identifies the packaged WTA binary even when the agent's tool environment does not expose it on `PATH`. **Prefer this resolver over your own `Get-Command`/`Get-Alias` probe**: it loads the user's profile, so it reports profile-defined **aliases and functions** your own `execute_command` probe misses (a plain probe usually runs without the user's profile). It returns a `status`: `exists` (with the command's type + resolved target, e.g. an alias → `where.exe`), `not_found` (with the closest real commands), `indeterminate` (couldn't verify — don't assume it's missing), or `unsupported` (the selected shell is not PowerShell). If the runtime section is absent or its invocation fails to start, probe yourself with `execute_command`: `Get-Command deploy-it -All` — and describe it by its actual type (Application / ExternalScript / Cmdlet / Function / Alias); don't assume "on PATH" when it might be a function or alias.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 03:49

Copilot AI 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.

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

@github-actions

This comment has been minimized.

Bind the permission bypass to the exact per-session resolver contract and revoke it with the active turn.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 05:30

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@github-actions

This comment has been minimized.

Accept Copilot's single parsed command identifier while retaining exact canonical command validation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 06:32
@github-actions

This comment has been minimized.

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comment thread tools/wta/src/protocol/acp/prompt_context.rs Outdated
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 07:12

Copilot AI 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.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Comment thread tools/wta/src/protocol/acp/client.rs Outdated
Comment on lines +629 to +640
if let Some(commands) = raw_input.get("commands") {
let Some(commands) = commands.as_array() else {
return false;
};
if commands.len() != 1
|| commands[0]
.as_str()
.is_none_or(|identifier| identifier.trim().is_empty())
{
return false;
}
}
Comment thread tools/wta/src/resolve_command.rs Fixed
@github-actions

This comment has been minimized.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 08:33

Copilot AI 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.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tools/wta/src/resolve_command.rs:105

  • resolve_host_path maps any which::which error to NotFound. which can also fail due to PATH/env/IO errors (e.g. invalid PATH Unicode, unreadable PATH entries), which should be treated as Indeterminate so callers don’t incorrectly report a clean miss (and so the higher-level note can say the host PATH couldn’t be inspected).
    let Ok(path) = which::which(token) else {
        return ResolveOutcome::NotFound;
    };

Route injected resolve-command requests through the standard permission UI while preserving the runtime resolver contract and source-aware lookup. Add regression coverage for the normal permission flow.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 08da03a2-0eec-4ec2-bcb9-39975b0da11d
Copilot AI review requested due to automatic review settings July 31, 2026 09:17

Copilot AI 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.

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (2)

tools/wta/src/resolve_command.rs:221

  • resolve() awaits each resolver source sequentially. When the PowerShell profile source is applicable, its bounded (4s) profile load can delay even simple host-PATH resolutions, even though the sources are independent. Consider running applicable sources concurrently and then merging results, so PATH-only results aren’t blocked by a slow PowerShell profile probe.
pub async fn resolve(token: &str, shell: &str) -> serde_json::Value {
    let context = ResolutionContext { token, shell };
    let mut results = Vec::new();
    for source in default_sources() {
        if !source.applies(&context) {
            continue;
        }
        results.push(SourceResult {
            source: source.id(),
            miss_is_authoritative: source.miss_is_authoritative(&context),
            outcome: source.resolve(&context).await,
        });
    }

tools/wta/src/cli/args.rs:240

  • The --shell flag is described as the “active shell identity”, but the Clap default is still pwsh.exe. If a caller omits --shell in a non-PowerShell context, this will still attempt the PowerShell profile probe and can downgrade an otherwise-clear PATH miss into indeterminate (e.g. when pwsh.exe is missing or profile load times out). Consider clarifying in the help text that --shell should be set to the active pane’s shell, and that the default exists only for backward compatibility.
        /// Active shell identity; PowerShell hosts also load their user profile
        #[arg(long, default_value = "pwsh.exe", value_parser = resolve_command::parse_non_empty)]
        shell: String,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants