Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
42fee12
Extract cmdline builder with proper quoting and add unit tests
yeelam-gordon May 21, 2026
a5be1e8
feat: pass agent config as JSON-encoded --agent-config argument
yeelam-gordon May 21, 2026
221c122
fix: correct misleading comments about JsonCpp usage
yeelam-gordon May 21, 2026
ff5e4e2
chore: trigger re-review
yeelam-gordon May 21, 2026
ef17c67
fix: address Copilot review round 2
yeelam-gordon May 21, 2026
ad520e5
chore: request re-review
yeelam-gordon May 21, 2026
92e663d
fix: make QuoteArgForCommandLine non-throwing (address round 3)
yeelam-gordon May 21, 2026
e519a19
chore: trigger review
yeelam-gordon May 21, 2026
9213afe
fix: address round 4 Copilot review comments
yeelam-gordon May 21, 2026
dfdae64
chore: trigger review
yeelam-gordon May 21, 2026
b44b2e3
fix: quote wta path with QuoteProgramPath in all launch paths
yeelam-gordon May 21, 2026
7b7bcc7
chore: trigger review
yeelam-gordon May 21, 2026
183e02d
fix: make BuildAgentConfigArg return std::optional
yeelam-gordon May 21, 2026
59b62da
chore: trigger review
yeelam-gordon May 21, 2026
9cac907
fix: address round 7 - logging and cross-platform guard
yeelam-gordon May 21, 2026
aefece2
chore: trigger review
yeelam-gordon May 21, 2026
a7460ee
chore: trigger copilot review
yeelam-gordon May 21, 2026
e204334
chore: trigger review after resolving threads
yeelam-gordon May 21, 2026
0f8228d
chore: trigger copilot review
yeelam-gordon May 21, 2026
6d3eaf9
fix: use ../inc/ prefix for QuoteArgForCommandLine.h include
yeelam-gordon May 21, 2026
3b5d2dd
fix: address round 8 - noexcept safety and overlay dedup
yeelam-gordon May 21, 2026
b1dd16c
fix: resolve check-spelling alerts in cmdline tests
yeelam-gordon May 21, 2026
6cf87a5
fix: destructure AgentConfig in overlay blocks
yeelam-gordon May 21, 2026
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
153 changes: 35 additions & 118 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#include "TerminalSettingsCache.h"
#include "TerminalProtocolPipeServer.h"
#include "WtaProcessLauncher.h"
#include "QuoteArgForCommandLine.h"
Comment thread
yeelam-gordon marked this conversation as resolved.
Outdated

#include "LaunchPositionRequest.g.cpp"
#include "RenameWindowRequestedArgs.g.cpp"
Expand Down Expand Up @@ -1163,36 +1164,21 @@ namespace winrt::TerminalApp::implementation
const auto& globals = _settings.GlobalSettings();
const auto agentCliPath = _ResolveEffectiveAgentCliPath(globals, [this]() { return _DetectAgentCli(); });

// Helper: escape and quote an argument for the command line.
auto quoteArg = [](std::wstring_view arg) -> std::wstring {
std::wstring escaped{ arg };
for (size_t pos = 0; (pos = escaped.find(L'"', pos)) != std::wstring::npos; pos += 2)
{
escaped.replace(pos, 1, L"\\\"");
}
return L"\"" + escaped + L"\"";
};
using Microsoft::Terminal::CommandLine::QuoteArgForCommandLine;

// Build: wta delegate --agent <agent> --delegate-agent <delegate> "<prompt>"
std::wstring cmdline = quoteArg(wtaPath) + L" delegate";

if (!agentCliPath.empty())
{
cmdline += L" --agent " + quoteArg(std::wstring_view{ agentCliPath });
}
// Build: wta delegate --agent-config <json> --cwd <cwd> "<prompt>"
std::wstring cmdline = QuoteArgForCommandLine(wtaPath) + L" delegate";

Comment thread
yeelam-gordon marked this conversation as resolved.
const auto delegateAgent = _ResolveEffectiveDelegateAgent(globals);
if (!delegateAgent.empty())
{
cmdline += L" --delegate-agent " + quoteArg(std::wstring_view{ delegateAgent });
}
const auto delegateModel = globals.DelegateModel();
if (!delegateModel.empty())
{
cmdline += L" --delegate-model " + quoteArg(std::wstring_view{ delegateModel });
}


// Pass agent config fields as JSON (same mechanism as the agent pane).
cmdline += Microsoft::Terminal::CommandLine::BuildAgentConfigArg(
std::wstring_view{ agentCliPath },
std::wstring_view{} /* agentId — not needed for delegate */,
std::wstring_view{ delegateAgent },
std::wstring_view{ delegateModel },
std::wstring_view{} /* acpModel */);

// Pass CWD from the active pane.
winrt::hstring activeCwd;
Expand All @@ -1210,16 +1196,11 @@ namespace winrt::TerminalApp::implementation
}
if (!activeCwd.empty())
{
cmdline += L" --cwd " + quoteArg(std::wstring_view{ activeCwd });
cmdline += L" --cwd " + QuoteArgForCommandLine(std::wstring_view{ activeCwd });
}

// Append the prompt as a positional argument.
std::wstring escapedPrompt{ prompt };
for (size_t pos = 0; (pos = escapedPrompt.find(L'"', pos)) != std::wstring::npos; pos += 2)
{
escapedPrompt.replace(pos, 1, L"\"\"");
}
cmdline += fmt::format(FMT_COMPILE(L" \"{}\""), escapedPrompt);
cmdline += L" " + QuoteArgForCommandLine(std::wstring_view{ prompt });
Comment thread
yeelam-gordon marked this conversation as resolved.
Outdated

_agentPaneLog("launching: " + winrt::to_string(winrt::hstring{ cmdline }));

Expand Down Expand Up @@ -1730,60 +1711,25 @@ namespace winrt::TerminalApp::implementation
// _NotifyAgentTabChanged → tab_changed events.
if (const auto stableId = tab->StableId(); !stableId.empty())
{
cmdline += fmt::format(FMT_COMPILE(L" --owner-tab-id \"{}\""), std::wstring_view{ stableId });
cmdline += L" --owner-tab-id " + Microsoft::Terminal::CommandLine::QuoteArgForCommandLine(std::wstring_view{ stableId });
}

const auto agentCliPath = _ResolveEffectiveAgentCliPath(globals, [this]() { return _DetectAgentCli(); });
if (!agentCliPath.empty())
{
std::wstring s{ agentCliPath };
for (size_t pos = 0; (pos = s.find(L'"', pos)) != std::wstring::npos; pos += 2)
s.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --agent \"{}\""), s);
}

// See `_OpenOrReuseAgentPane` for the rationale: wta needs the
// canonical `acpAgent` setting value (not the expanded command
// line) to drive the session-management view's CLI filter.
if (const auto acpAgent = globals.AcpAgent(); !acpAgent.empty())
{
std::wstring s{ acpAgent };
for (size_t pos = 0; (pos = s.find(L'"', pos)) != std::wstring::npos; pos += 2)
s.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --agent-id \"{}\""), s);
}

const auto acpAgent = globals.AcpAgent();
const auto delegateAgent = _ResolveEffectiveDelegateAgent(globals);
if (!delegateAgent.empty())
{
std::wstring s{ delegateAgent };
for (size_t pos = 0; (pos = s.find(L'"', pos)) != std::wstring::npos; pos += 2)
s.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --delegate-agent \"{}\""), s);
}

const auto delegateModel = globals.DelegateModel();
if (!delegateModel.empty())
{
std::wstring s{ delegateModel };
for (size_t pos = 0; (pos = s.find(L'"', pos)) != std::wstring::npos; pos += 2)
s.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --delegate-model \"{}\""), s);
}

// Pass the ACP model selection out-of-band so it works for adapter
// launches (claude/codex via npx) where --model can't be put on the
// adapter cmdline. wta sends this via ACP setSessionModel after
// handshake; for copilot/gemini it's redundant (their --model is
// already on the agent cmdline) but harmless.
const auto acpModel = globals.AcpModel();
if (!acpModel.empty())
{
std::wstring s{ acpModel };
for (size_t pos = 0; (pos = s.find(L'"', pos)) != std::wstring::npos; pos += 2)
s.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --acp-model \"{}\""), s);
}

// Pass all agent-related config as a single JSON-encoded argument.
// This eliminates per-field hand-rolled escaping — BuildAgentConfigArg
// performs RFC 8259 JSON encoding internally, and only one correctly-
// quoted argument boundary is needed (via QuoteArgForCommandLine).
cmdline += Microsoft::Terminal::CommandLine::BuildAgentConfigArg(
std::wstring_view{ agentCliPath },
Comment thread
yeelam-gordon marked this conversation as resolved.
Outdated
std::wstring_view{ acpAgent },
std::wstring_view{ delegateAgent },
std::wstring_view{ delegateModel },
std::wstring_view{ acpModel });

if (!globals.AutoFixEnabled())
{
Expand Down Expand Up @@ -2121,46 +2067,17 @@ namespace winrt::TerminalApp::implementation
cmdline = std::wstring{ wtaPath };

const auto agentCliPath = _ResolveEffectiveAgentCliPath(globals, [this]() { return _DetectAgentCli(); });
if (!agentCliPath.empty())
{
std::wstring agentStr{ agentCliPath };
for (size_t pos = 0; (pos = agentStr.find(L'"', pos)) != std::wstring::npos; pos += 2)
agentStr.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --agent \"{}\""), agentStr);
}

// Tell wta which `acpAgent` setting value produced this launch
// — wta needs the canonical id ("copilot" / "claude" / "codex"
// / "gemini" / "custom:…") to drive the session-management
// view's CLI filter, and parsing it back out of the expanded
// `--agent` command line is fragile (adapter launches expand
// to "npx -y …" and lose the agent's name). Passing it through
// here keeps a single source of truth.
if (const auto acpAgent = globals.AcpAgent(); !acpAgent.empty())
{
std::wstring idStr{ acpAgent };
for (size_t pos = 0; (pos = idStr.find(L'"', pos)) != std::wstring::npos; pos += 2)
idStr.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --agent-id \"{}\""), idStr);
}

const auto acpAgent = globals.AcpAgent();
const auto delegateAgent = _ResolveEffectiveDelegateAgent(globals);
if (!delegateAgent.empty())
{
std::wstring delegateStr{ delegateAgent };
for (size_t pos = 0; (pos = delegateStr.find(L'"', pos)) != std::wstring::npos; pos += 2)
delegateStr.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --delegate-agent \"{}\""), delegateStr);
}

const auto delegateModel = globals.DelegateModel();
if (!delegateModel.empty())
{
std::wstring modelStr{ delegateModel };
for (size_t pos = 0; (pos = modelStr.find(L'"', pos)) != std::wstring::npos; pos += 2)
modelStr.replace(pos, 1, L"\"\"");
cmdline += fmt::format(FMT_COMPILE(L" --delegate-model \"{}\""), modelStr);
}

// Pass all agent-related config as a single JSON-encoded argument.
cmdline += Microsoft::Terminal::CommandLine::BuildAgentConfigArg(
std::wstring_view{ agentCliPath },
Comment thread
yeelam-gordon marked this conversation as resolved.
Outdated
std::wstring_view{ acpAgent },
std::wstring_view{ delegateAgent },
std::wstring_view{ delegateModel },
std::wstring_view{} /* acpModel — not used in this path */);

if (!globals.AutoFixEnabled())
{
Expand Down
11 changes: 4 additions & 7 deletions src/cascadia/TerminalSettingsEditor/AIAgentsViewModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "../inc/AgentRegistry.h"
#include "../inc/AgentHooksStatus.h"
#include "../inc/WtaProcess.h"
#include "../inc/QuoteArgForCommandLine.h"

#include <json/json.h>

Expand Down Expand Up @@ -966,13 +967,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
std::string stdoutText;
if (!wtaPath.empty())
{
// Quote-escape internal `"` per Windows CRT rules.
std::wstring escaped = agentCmdline;
for (size_t pos = 0; (pos = escaped.find(L'"', pos)) != std::wstring::npos; pos += 2)
{
escaped.replace(pos, 1, L"\"\"");
}
const std::wstring args = L"probe-models --agent \"" + escaped + L"\"";
// Use correct CommandLineToArgvW quoting for the agent argument.
const std::wstring args = L"probe-models --agent " +
::Microsoft::Terminal::CommandLine::QuoteArgForCommandLine(std::wstring_view{ agentCmdline });

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 92e663d — QuoteArgForCommandLine returns std::optional. Probe path skips launch if quoting fails.

// 40s ceiling matches probe.rs's internal limits (npx
// initialize 25s + new_session 10s + slack). Cached
// adapters return in <2s.
Expand Down
147 changes: 147 additions & 0 deletions src/cascadia/inc/QuoteArgForCommandLine.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,147 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.
//
// QuoteArgForCommandLine.h
//
// Correct CommandLineToArgvW-compatible quoting for a single argument.
// Eliminates hand-rolled escaping throughout the codebase. Use this
// whenever building a commandline string for CreateProcess/ShellExecute.
//
// Pure Win32 + STL, no WinRT dependency.

#pragma once

Comment thread
yeelam-gordon marked this conversation as resolved.
#include <string>
#include <string_view>

namespace Microsoft::Terminal::CommandLine
{
// Quote a single argument for use in a Windows commandline string.
// The result is always wrapped in double quotes for unambiguous parsing
// by CommandLineToArgvW. Handles:
// - Backslashes before `"` are doubled (2n+1 backslashes + `"`)
// - Trailing backslashes before the closing `"` are doubled
// - All other characters are passed through literally
//
// NOTE: This is for argv[1..n] only. argv[0] (the program path) has
// different rules — backslashes are always literal, and `"` cannot be
// escaped inside it. For argv[0], simply wrap in quotes if needed and
// reject paths containing `"`.
inline std::wstring QuoteArgForCommandLine(std::wstring_view arg)
{
std::wstring result;
// Reserve: arg size + quotes + some escaping headroom
result.reserve(arg.size() + 8);
result.push_back(L'"');

size_t backslashes = 0;
for (const auto ch : arg)
Comment thread
yeelam-gordon marked this conversation as resolved.
Outdated
{
if (ch == L'\\')
{
++backslashes;
}
else if (ch == L'"')
{
// Double the accumulated backslashes, then emit \"
result.append(backslashes * 2 + 1, L'\\');
result.push_back(L'"');
backslashes = 0;
}
else
{
// Flush any accumulated backslashes as-is
result.append(backslashes, L'\\');
backslashes = 0;
result.push_back(ch);
}
}
// Trailing backslashes must be doubled (they precede the closing `"`)
result.append(backslashes * 2, L'\\');
result.push_back(L'"');

return result;
}

// Build a JSON-encoded `--agent-config` argument from the given fields.
// Returns the full fragment: ` --agent-config "<escaped-json>"`
// Uses manual RFC 8259-compliant JSON construction (no external JSON
// library dependency) and QuoteArgForCommandLine for the single
// argument boundary.
//
// Usage:
// cmdline += BuildAgentConfigArg(agentCli, agentId, delegateAgent,
// delegateModel, acpModel);
//
// Any empty field is omitted from the JSON (the Rust side uses
// Option<String> and falls back to defaults for missing fields).
inline std::wstring BuildAgentConfigArg(
std::wstring_view agent,
std::wstring_view agentId,
std::wstring_view delegateAgent,
std::wstring_view delegateModel,
std::wstring_view acpModel)
{
// Build a compact JSON object with only non-empty fields.
// We use manual JSON construction to avoid pulling in JsonCpp here
// (this header is used in both TerminalApp and TerminalSettingsEditor).
// The JSON spec is simple enough for known-safe field names: only the
// VALUES need escaping, and we do it correctly per RFC 8259.
auto jsonEscapeValue = [](std::wstring_view val) -> std::wstring {
std::wstring out;
out.reserve(val.size() + 4);
for (const auto ch : val)
{
switch (ch)
{
case L'"': out += L"\\\""; break;
case L'\\': out += L"\\\\"; break;
case L'\b': out += L"\\b"; break;
case L'\f': out += L"\\f"; break;
case L'\n': out += L"\\n"; break;
case L'\r': out += L"\\r"; break;
case L'\t': out += L"\\t"; break;
default:
if (ch < 0x20)
{
wchar_t buf[8];
swprintf_s(buf, L"\\u%04x", static_cast<unsigned>(ch));
out += buf;
}
else
{
out.push_back(ch);
}
break;
}
}
return out;
};

std::wstring json = L"{";
bool first = true;

auto appendField = [&](const wchar_t* key, std::wstring_view val) {
if (val.empty())
return;
if (!first)
json += L',';
first = false;
json += L'"';
json += key;
json += L"\":\"";
json += jsonEscapeValue(val);
json += L'"';
};

appendField(L"agent", agent);
appendField(L"agentId", agentId);
appendField(L"delegateAgent", delegateAgent);
appendField(L"delegateModel", delegateModel);
appendField(L"acpModel", acpModel);

json += L'}';

return L" --agent-config " + QuoteArgForCommandLine(json);
}
Comment thread
yeelam-gordon marked this conversation as resolved.
}
1 change: 1 addition & 0 deletions tools/wta/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ windows-sys = { version = "0.61", features = [
"Win32_System_Environment",
"Win32_System_Registry",
"Win32_System_Threading",
"Win32_UI_Shell",
] }
tracing = "0.1"
tracing-subscriber = { version = "0.3", features = ["env-filter", "fmt"] }
Expand Down
Loading
Loading