feat(watch): external capability loading + PID tracker (#918, #921) by tamirdresher · Pull Request #922 · github/squad

tamirdresher · 2026-04-08T06:33:57Z

Summary

Two features that improve watch reliability and extensibility:

1. External WatchCapability loading (closes #918)

Users can now define custom watch capabilities as .js files in .squad/capabilities/. Each file default-exports a WatchCapability object. The loader scans the directory at watch startup, validates required fields, and registers them alongside built-in capabilities.

Files:

watch/external-loader.ts — loader function (78 lines)
watch/index.ts — wired after createDefaultRegistry()
templates/capabilities/issue-pipeline.sample.js — sample capability
6 tests, all passing

2. PID tracker for orphan cleanup (closes #921)

Prevents MCP server process leaks during long-running watch sessions. Tracks PIDs of spawned copilot sessions in .squad/.watch-pids, kills them on exit/crash, and cleans up stale orphans from previous crashed runs on startup.

Problem: After ~12h of watch running on Windows, we found 85 orphaned enghub-mcp processes, 4 orphaned copilot.exe, and 37 orphaned pwsh.exe — all leaked because child processes aren't cleaned up when parents exit.

Files:

watch/pid-tracker.ts — PidTracker class with cross-platform kill
watch/index.ts — stale cleanup on startup + exit handlers
watch/types.ts — pidTracker added to WatchContext
capabilities/execute.ts — tracks copilot child PIDs
11 tests, all passing

Testing

17 new tests (6 external loader + 11 PID tracker), all passing
Type check: 0 errors in new/modified files
No changes to existing built-in capabilities

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

github#918) Add loadExternalCapabilities() that scans .squad/capabilities/ for .js files, validates WatchCapability interface conformance, and registers them in the CapabilityRegistry after built-in capabilities. - external-loader.ts: loader with graceful error handling per file - Wire into runWatch() after createDefaultRegistry() - Export from watch barrel for downstream consumers - Sample capability template: issue-pipeline.sample.js - 6 vitest tests covering all edge cases - Changeset for minor bump

Add PidTracker class that tracks child process PIDs spawned during watch rounds, kills them on exit/crash, and cleans up stale orphans from previous crashed runs on startup. - New pid-tracker.ts with cross-platform process tree kill - Wire into watch/index.ts for startup cleanup and exit handlers - Wire into execute.ts to track/untrack copilot session PIDs - Add pidTracker to WatchContext interface - Export from barrel for external consumers - Add .watch-pids to .gitignore (runtime state) - 11 vitest tests covering track/untrack, cleanup, and stale handling Closes github#921

github-actions · 2026-04-08T06:34:09Z

🟡 Impact Analysis — PR #922

Risk tier: 🟡 MEDIUM

📊 Summary

Metric	Count
Files changed	12
Files added	8
Files modified	4
Files deleted	0
Modules touched	3
Critical files	1

🎯 Risk Factors

12 files changed (6-20 → MEDIUM)
3 modules touched (2-4 → MEDIUM)
Critical files touched: packages/squad-cli/src/cli/commands/watch/index.ts

📦 Modules Affected

root (4 files)

.changeset/external-capability-loading.md
.changeset/pid-tracker-cleanup.md
.gitignore
test-fixtures/init-test/.gitignore

squad-cli (6 files)

packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts
packages/squad-cli/src/cli/commands/watch/external-loader.ts
packages/squad-cli/src/cli/commands/watch/index.ts
packages/squad-cli/src/cli/commands/watch/pid-tracker.ts
packages/squad-cli/src/cli/commands/watch/types.ts
packages/squad-cli/src/cli/templates/capabilities/issue-pipeline.sample.js

tests (2 files)

test/cli/watch-external-loader.test.ts
test/cli/watch-pid-tracker.test.ts

⚠️ Critical Files

packages/squad-cli/src/cli/commands/watch/index.ts

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

Copilot

Pull request overview

Adds two watch-mode enhancements: loading user-defined watch capabilities from .squad/capabilities/*.js, and tracking/killing spawned Copilot session PIDs to reduce orphaned-process leaks across watch exits/restarts.

Changes:

Add external capability loader that dynamically imports .js modules from .squad/capabilities/ and registers them into the capability registry.
Add PidTracker to record spawned child PIDs in .squad/.watch-pids, clean up stale PIDs on startup, and register exit/crash handlers to clean up on shutdown.
Wire PID tracking into the execute capability so Copilot child processes are tracked/untracked.

Reviewed changes

Copilot reviewed 10 out of 11 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
`packages/squad-cli/src/cli/commands/watch/external-loader.ts`	Implements dynamic discovery/import/registration of external capabilities.
`packages/squad-cli/src/cli/commands/watch/index.ts`	Wires external loader + PID tracker into `runWatch()` startup/shutdown flow.
`packages/squad-cli/src/cli/commands/watch/pid-tracker.ts`	New PID tracking + cleanup logic with cross-platform kill behavior.
`packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts`	Tracks Copilot child PIDs for cleanup on exit/crash.
`packages/squad-cli/src/cli/commands/watch/types.ts`	Extends `WatchContext` with optional `pidTracker`.
`packages/squad-cli/src/cli/templates/capabilities/issue-pipeline.sample.js`	Adds sample external capability template for users.
`test/cli/watch-external-loader.test.ts`	Adds tests for external loader behavior and failure handling.
`test/cli/watch-pid-tracker.test.ts`	Adds tests for PID file lifecycle, cleanup, and stale cleanup behavior.
`.gitignore`	Ignores the new `.squad/.watch-pids` tracking file.
`.changeset/pid-tracker-cleanup.md`	Changeset for PID tracking/orphan cleanup fix.
`.changeset/external-capability-loading.md`	Changeset for external capability loading feature.

Comments suppressed due to low confidence (1)

packages/squad-cli/src/cli/commands/watch/capabilities/execute.ts:159

The copilot child process is spawned without detached: true, so on POSIX PidTracker.killTree()'s process.kill(-pid, ...) will usually not kill a process group (because the child is not a group leader). If the goal is to reliably kill descendants (MCP servers) on non-Windows too, consider spawning the child in a new process group and tracking/killing that group.

    const cp: ChildProcess = execFile(
      cmd,
      args,
      { cwd: context.teamRoot, timeout: timeoutMs, maxBuffer: 50 * 1024 * 1024 },
      (err) => {

Copilot · 2026-04-08T06:37:37Z

+      // Validate required fields
+      const missing = REQUIRED_FIELDS.filter(f => !(f in cap));
+      if (missing.length > 0) {
+        console.log(
+          `${YELLOW}⚠️ Failed to load capability from ${filename}: missing fields: ${missing.join(', ')}${RESET}`,
+        );
+        continue;
+      }


Validation currently only checks property presence via f in cap. This will treat non-callable preflight/execute values as valid and will accept any string for phase. To prevent runtime crashes later in the round loop, validate field types (e.g., typeof cap.preflight === 'function', typeof cap.execute === 'function', cap.phase is one of the supported phases).

Fixed in commit 0bcd04d.

Copilot · 2026-04-08T06:37:38Z

+      } else {
+        // Try process group kill first, fall back to single PID
+        try {
+          process.kill(-pid, 'SIGKILL'); // negative PID = process group
+        } catch {
+          process.kill(pid, 'SIGKILL');
+        }
+      }


killTree() attempts process.kill(-pid, 'SIGKILL'), but the execFile() call that spawns the copilot process is not started in its own process group (detached: true), so -pid will typically not correspond to a process group. As a result, on non-Windows this often only kills the direct child PID and may leave grandchildren (e.g., MCP servers) running. Consider spawning the child in a new process group (e.g., detached: true) and then killing the group, or implement a real process-tree kill strategy for POSIX.

Fixed in commit 0bcd04d.

Copilot · 2026-04-08T06:37:39Z

+    it('kills alive processes and reports count', () => {
+      const tracker = new PidTracker(teamRoot);
+      tracker.track(111, 'alive-proc');
+      tracker.track(222, 'dead-proc');
+
+      // Mock process.kill: 111 is alive, 222 is dead
+      const killSpy = vi.spyOn(process, 'kill').mockImplementation(((pid: number, signal?: string | number) => {
+        if (signal === 0) {
+          // Existence check — 111 is alive, 222 is dead
+          if (Math.abs(pid) === 111) return true;
+          throw new Error('ESRCH');
+        }
+        // Actual kill — succeed for alive process
+        return true;
+      }) as typeof process.kill);
+
+      const result = tracker.cleanup();


On Windows, PidTracker.cleanup() will call taskkill via execSync for “alive” PIDs. These tests mock process.kill but do not mock execSync, so running the suite on Windows could execute real taskkill /PID 111 /T /F etc and potentially kill unrelated processes if those PIDs exist. Mock node:child_process (or refactor PidTracker to inject the killer) so tests never execute real OS kill commands.

Fixed in commit 0bcd04d.

Copilot · 2026-04-08T06:37:39Z

+  const entries = await readdir(capDir);
+  const jsFiles = entries.filter(f => f.endsWith('.js'));
+
+  let loaded = 0;
+
+  for (const filename of jsFiles) {
+    const filePath = join(capDir, filename);


readdir() order is filesystem-dependent. If load order matters (or to keep logs deterministic), consider sorting entries/jsFiles before iterating so capability load/override behavior is reproducible across platforms and filesystems.

Fixed in commit 0bcd04d.

P0 mitigations from Worf's security review: 1. Print security warning listing all external .js files before importing 2. Block external capabilities from overwriting built-in names 3. New test: hijack attempt is rejected, built-in preserved Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

…922) * feat(watch): load external WatchCapabilities from .squad/capabilities/ (#918) Add loadExternalCapabilities() that scans .squad/capabilities/ for .js files, validates WatchCapability interface conformance, and registers them in the CapabilityRegistry after built-in capabilities. - external-loader.ts: loader with graceful error handling per file - Wire into runWatch() after createDefaultRegistry() - Export from watch barrel for downstream consumers - Sample capability template: issue-pipeline.sample.js - 6 vitest tests covering all edge cases - Changeset for minor bump * fix(watch): track child PIDs and cleanup orphans on exit (#921) Add PidTracker class that tracks child process PIDs spawned during watch rounds, kills them on exit/crash, and cleans up stale orphans from previous crashed runs on startup. - New pid-tracker.ts with cross-platform process tree kill - Wire into watch/index.ts for startup cleanup and exit handlers - Wire into execute.ts to track/untrack copilot session PIDs - Add pidTracker to WatchContext interface - Export from barrel for external consumers - Add .watch-pids to .gitignore (runtime state) - 11 vitest tests covering track/untrack, cleanup, and stale handling Closes #921 * fix: address Copilot review comments on PR #922 * security: add startup warning banner + block built-in name hijacking P0 mitigations from Worf's security review: 1. Print security warning listing all external .js files before importing 2. Block external capabilities from overwriting built-in names 3. New test: hijack attempt is rejected, built-in preserved Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <copilot@github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot added 2 commits April 8, 2026 08:52

Copilot AI review requested due to automatic review settings April 8, 2026 06:33

Copilot started reviewing on behalf of tamirdresher April 8, 2026 06:34

View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

Copilot and others added 2 commits April 8, 2026 10:08

fix: address Copilot review comments on PR github#922

0bcd04d

bradygaster merged commit b41dde6 into github:dev Apr 12, 2026
11 checks passed

Navigation Menu

Repository navigation

feat(watch): external capability loading + PID tracker (#918, #921) - #922

feat(watch): external capability loading + PID tracker (#918, #921)#922
bradygaster merged 4 commits into
github:devfrom
tamirdresher:squad/918-external-capability-loading

tamirdresher commented Apr 8, 2026

Uh oh!

github-actions Bot commented Apr 8, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

tamirdresher Apr 8, 2026

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

tamirdresher Apr 8, 2026

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

tamirdresher Apr 8, 2026

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Uh oh!

tamirdresher Apr 8, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

tamirdresher commented Apr 8, 2026

Summary

1. External WatchCapability loading (closes #918)

2. PID tracker for orphan cleanup (closes #921)

Testing

Uh oh!

github-actions Bot commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟡 Impact Analysis — PR #922

📊 Summary

🎯 Risk Factors

📦 Modules Affected

⚠️ Critical Files

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

tamirdresher Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

tamirdresher Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

tamirdresher Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

tamirdresher Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Apr 8, 2026 •

edited

Loading