fix(sdk): support command paths containing spaces in scheduler script tasks (#1794)

[bradygaster]

Closes #1794.

The defect

LocalPollingProvider.execute split task.ref on whitespace with no quote handling:

const argv = entry.task.ref.trim().split(/\s+/);
const command = argv[0]!;

On Windows the default Node install is C:\Program Files\nodejs\node.exe, so command became C:\Program and every script task failed with ENOENT. This is the modal Windows configuration, not an exotic edge case — it breaks for anyone who did not install Node somewhere unusual.

It was also documented in the test suite as a constraint rather than a bug. Two existing tests carry the comment:

// Space-free script — the scheduler tokenizes `task.ref` by whitespace.

Same shape as #1796: correctly observed, worked around, never fixed.

Red, reproducing the actual modal failure

process.execPath on the test machine is literally C:\Program Files\nodejs\node.exe. A full suite run on unmodified dev fails four pre-existing scheduler tests for exactly this reason:

FAIL  test/scheduler.test.ts > Scheduler: LocalPollingProvider > should execute script tasks
FAIL  test/scheduler.test.ts > ... > rich error capture on script failure > captures the exit code on a non-zero exit
FAIL  test/scheduler.test.ts > ... > rich error capture on script failure > captures stderr written by the child
FAIL  test/scheduler.test.ts > ... > rich error capture on script failure > does not block the event loop while a script runs

All four pass after this change. This PR removes test/scheduler.test.ts from the Windows failing set entirely (#1797 baseline: 9 files → 8).

The new tests build their fixture at <tmp>/Program Files/nodejs/node.exe from a real copy of the running Node binary, so the red is the actual spawn failure and not a synthetic string with a space in it. Pre-fix:

FAIL > ... (#1794) > executes an unquoted command path containing a space
  AssertionError: expected false to be true
FAIL > ... (#1794) > executes a double-quoted command path containing a space
  AssertionError: expected false to be true
FAIL > ... (#1794) > passes a quoted ARGUMENT containing a space through as one argv entry
  AssertionError: expected false to be true
FAIL > ... (#1794) > executes via explicit argv with no parsing at all
  AssertionError: expected false to be true
FAIL > ... (#1794) > names the command and surfaces ENOENT when the executable does not exist
  AssertionError: expected undefined to be 'ENOENT'

Tests  5 failed | 1 passed | 67 skipped (73)

The 1 pass is deliberate: the fixture path really does contain a space (guards against a vacuous test). A space-free fixture passes identically before and after and proves nothing, so the suite asserts it cannot silently become vacuous.

The fix

• Quote-aware tokenizer. Backslash is not an escape — Windows paths are full of them.
• Unquoted path resolution. If the first token already resolves on disk it is used unchanged, so every previously-working ref keeps its exact behaviour, including bare PATH commands like node which are not files at all. Only when that fails do we widen across spaces, longest match first — the same way Windows CreateProcess resolves an unquoted path.
• TaskConfig.argv?: string[] as the unambiguous form: ref is the executable verbatim, never parsed.
• TaskResult.spawnError?: string. execFile reports spawn failures with a string code (ENOENT, EACCES) and no stdout/stderr, which fell through every branch and produced code: undefined, stderr: ''. That is the silent-success class again — a failure that reports nothing. The message now names the ref.
• shell: false retained, so the injection-safety property is unchanged.

A backward-compat trap this nearly shipped

Naive shell-style tokenizing strips the inner quotes from node -e console.log('hi'), turning the string literal into an identifier so the child dies with ReferenceError. I found this because the existing suite went red, not by reading it — my first tokenizer broke should execute script tasks and two others.

Quotes now group only when they open at a token boundary; a quote mid-token is a literal. Pinned directly:

it('treats a quote appearing mid-token as a literal character', () => {
  const { tokens } = tokenizeTaskRef(`node -e console.log('hi')`);
  expect(tokens).toEqual(['node', '-e', `console.log('hi')`]);
});

Verification

Check	Result
test/scheduler.test.ts post-fix	81 passed
Negative control (change stashed, full suite)	4 scheduler tests fail
Isolation of the two suites that differed between full runs	340 passed — flakes, not caused by this change (see below)
npm run build	exit 0
git diff --cached --stat	3 files, +313 / -4
git diff --cached --diff-filter=D --name-only	empty
Changeset	.changeset/scheduler-quoted-task-ref.md (patch, @bradygaster/squad-sdk)

⚠️ Finding that affects tomorrow's baseline, not this PR

Running the full suite three times produced three different failing sets: 9 files (the #1797 baseline), 8 (with this change), 7 (control, change stashed). template-sync, consumer-imports, promote-insider-tag and patch-esm-imports move in and out between runs. All pass in isolation.

So the Windows failure count is not stable run-to-run, which weakens the single-number rule in #1797. template-sync's instability is specifically the #1796 race — fixed in #1798, unfixed on this branch. Reported separately; flagging here so the count change from this PR (9 → 8) is not read as the whole story.

[github-actions]

🟡 Impact Analysis — PR #1802

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	3
Files added	1
Files modified	2
Files deleted	0
Modules touched	3

🎯 Risk Factors

• 3 files changed (≤5 → LOW)
• 3 modules touched (2-4 → MEDIUM)

📦 Modules Affected

root (1 file)

• .changeset/scheduler-quoted-task-ref.md

squad-sdk (1 file)

• packages/squad-sdk/src/runtime/scheduler.ts

tests (1 file)

• test/scheduler.test.ts

---

This report is generated automatically for every PR. See #733 for details.

[github-actions]

🛫 PR Readiness Check

ℹ️ This comment updates on each push. Last checked: commit 4c1ce2b

PR Scope: 📦🔧 Mixed (product + infrastructure)

⚠️ 4 item(s) to address before review

Status	Check	Details
❌	Single commit	2 commits — consider squashing before review
✅	Not in draft	Ready for review
❌	Branch up to date	dev is 9 commit(s) ahead — rebase recommended
❌	Copilot review	No Copilot review yet — it may still be processing
✅	Changeset present	Changeset file found
✅	Scope clean	No .squad/ or docs/proposals/ files
✅	No merge conflicts	No merge conflicts
❌	Copilot threads resolved	6 unresolved Copilot thread(s) — fix and resolve before merging
✅	CI passing	All checks passing

Files Changed (3 files, +313 −4)

File	+/−
.changeset/scheduler-quoted-task-ref.md	+32 −0
packages/squad-sdk/src/runtime/scheduler.ts	+127 −3
test/scheduler.test.ts	+154 −1

Total: +313 −4

---

This check runs automatically on every push. Fix any ❌ items and push again.
See CONTRIBUTING.md and PR Requirements for details.

[Copilot]

Pull request overview

This PR fixes SDK scheduler execution for command paths and arguments containing spaces, especially on Windows.

Changes:

• Adds quote-aware parsing, path resolution, explicit argv support, and spawn diagnostics.
• Adds regression tests and an SDK patch changeset.

Show a summary per file

File	Summary
test/scheduler.test.ts	Adds regression coverage for spaced paths, quoting, arguments, and spawn failures.
packages/squad-sdk/src/runtime/scheduler.ts	Implements the scheduler changes. Five moderate findings and one nit remain regarding path resolution, error classification, argv validation, verbatim refs, and JSDoc placement.
.changeset/scheduler-quoted-task-ref.md	Documents the SDK patch release.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Suppressed comments (4)

packages/squad-sdk/src/runtime/scheduler.ts:500

• Tokenization collapses all whitespace and candidate reconstruction always joins with one ASCII space. Therefore an unquoted executable path containing repeated spaces, such as C:\Program Files\node.exe, can never match the existing file and falls back to the first token. Preserve the raw separator positions (or explicitly require quoting) so the advertised unquoted path support does not depend on exactly one space between components.

  for (let i = tokens.length; i >= 2; i--) {
    const candidate = tokens.slice(0, i).join(' ');
    if (exists(candidate)) {

packages/squad-sdk/src/runtime/scheduler.ts:607

• tokenizeTaskRef can reject an unterminated quote, but validateManifest still calls only validateTaskRef, which does not perform that syntax check. As a result, parseSchedule accepts a malformed ref and the error is deferred until the task happens to execute. Invoke the tokenizer during ref validation (without resolving the executable) so invalid schedule files fail at load time.

            const { tokens, firstQuoted } = tokenizeTaskRef(entry.task.ref);
            ({ command, args } = resolveScriptCommand(tokens, firstQuoted));

packages/squad-sdk/src/runtime/scheduler.ts:639

• This puts the entire task.ref, including every command-line argument, into an error that scheduleRun persists in .schedule-state.json and prints. A ref can contain a token or password argument, so a spawn failure can leak it into durable state and logs; use the resolved command (or redact arguments) here rather than recording the full ref.

            result.error = `${e.code}: failed to spawn '${entry.task.ref}' — ${e.message}`;

packages/squad-sdk/src/runtime/scheduler.ts:500

• For any command whose first token is not an existing file, this performs a synchronous existsSync for every token—including every argument—on every scheduled invocation. task.ref has no length/token bound, so a long inline script or malformed ref can block the scheduler event loop for an unbounded number of filesystem calls, undermining the non-blocking execution goal. Limit probing to path-like prefixes or use a targeted/async resolution strategy.

  for (let i = tokens.length; i >= 2; i--) {
    const candidate = tokens.slice(0, i).join(' ');
    if (exists(candidate)) {

• Files reviewed: 3/3 changed files
• Comments generated: 6
• Review effort level: Lite