Skip to content
Open
Show file tree
Hide file tree
Changes from 6 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
154 changes: 36 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,22 @@ 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;
using Microsoft::Terminal::CommandLine::QuoteProgramPath;

// 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 = QuoteProgramPath(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 +1197,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 +1712,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 +2068,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
Loading
Loading