Skip to content

fix: launch standalone Pi children directly - #764

Open
ZacharyQin wants to merge 2 commits into
nicobailon:mainfrom
ZacharyQin:fix/standalone-pi-child-spawn
Open

fix: launch standalone Pi children directly#764
ZacharyQin wants to merge 2 commits into
nicobailon:mainfrom
ZacharyQin:fix/standalone-pi-child-spawn

Conversation

@ZacharyQin

Copy link
Copy Markdown

Problem summary

When pi-subagents is launched from a standalone Pi executable, child spawning can insert the verified Pi CLI script as the first child argument:

pi <verified-cli.js> --mode json -p "<task>"

The standalone Pi executable is itself the runtime and should receive the original Pi arguments directly. With the extra CLI path present, the child Pi process can treat cli.js as task input instead of executing it as a Node entrypoint. In a local reproduction, this produced child runs whose first user message was the CLI path, followed by incorrect work, extra model calls, delays, or failures before the requested task was handled.

This is a child-process launch bug. It occurs when pi-subagents starts a child from a standalone Pi executable.

Mechanism

The standalone Pi child should be launched directly:

pi --mode json -p "Task: review diff"

The current code first resolves the Node CLI file and then treats process.execPath as Node:

return {
	command: deps.execPath ?? process.execPath,
	args: [piCliPath, ...args],
};

In a standalone environment, the values are:

process.execPath = pi
piCliPath        = pi-coding-agent/dist/cli.js

The resulting command is therefore:

pi pi-coding-agent/dist/cli.js --mode json -p "Task: review diff"

A minimal reproduction of the launch logic returns:

{
	"command": "/opt/pi/pi",
	"args": [
		".../pi-coding-agent/dist/cli.js",
		"--mode",
		"json",
		"-p",
		"Task: review diff"
	]
}

The standalone Pi process treats the first positional argument, cli.js, as user task input instead of a Node entrypoint. The requested task therefore does not enter the child as its first task, which can lead to incorrect work, extra model calls, delays, or failures before the requested task is handled.

Issue source

  1. Initial shared helper: 5c9823a introduced the shared spawn helper. Its original non-Windows behavior used plain pi, while the Windows path used Node plus a resolved Pi CLI script.
  2. Regression introduced: 1ca48a8 changed the helper to reuse process.execPath and the resolved Pi CLI on all platforms. This introduced the non-Windows standalone regression.
  3. First fix: The behavior was reported in issue #111 and fixed by a57f4fb, which restored plain pi spawning on non-Windows.
  4. Regression reintroduced: PR #460, committed as bcd63ea, reintroduced process.execPath plus a verified CLI path on POSIX to avoid PATH drift and embedded-host ambiguity. The change preserved Node-host behavior but did not distinguish a standalone Pi executable from Node, so the same regression recurred. The standalone case was not covered by the existing verified-CLI tests.

Solution

The fix checks the effective executable before resolving and inserting a Pi CLI path:

const execPath = deps.execPath ?? process.execPath;
if (isStandalonePiExecutable(execPath)) {
	return { command: execPath, args };
}

isStandalonePiExecutable() matches only an executable basename of pi or pi.exe. For the standalone values shown in the mechanism above, the command now becomes:

pi --mode json -p "Task: review diff"

The CLI path is no longer inserted as the first argument. This lets the standalone Pi executable parse --mode, json, -p, and the task text as its own arguments, so the requested task enters the child correctly.

The existing Node command remains:

node <verified-cli.js> --mode json -p "Task: review diff"

The spawn precedence is:

  1. Explicit override: PI_SUBAGENT_PI_BINARY remains the highest priority and returns the configured command with the original arguments.
  2. Standalone Pi detection: pi and pi.exe execute directly with the original arguments, without a CLI path.
  3. Verified CLI resolution: Node hosts continue to use the verified Pi CLI path. Embedded-host validation and package-bin resolution remain part of this branch.
  4. PATH fallback: When no verified CLI can be resolved, the existing bare pi fallback remains unchanged.

The fix changes only standalone executable detection and argument construction. It does not change task arguments, model selection, or execution lifecycle behavior.

Related

This is the same runtime-assumption class as the pi-intercom broker bug. The related fix is proposed in pi-intercom PR #82, which changes broker startup so a standalone Pi executable is not used as the Node runtime. That PR addresses pi-intercom broker spawning; this PR addresses pi-subagents foreground and background child spawning.

@greptile-apps

greptile-apps Bot commented Aug 4, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes a child-process launch regression where getPiSpawnCommand incorrectly prepended the resolved Pi CLI script path when the parent process was itself a standalone Pi executable, causing the child to treat the path as task input. The fix inserts an isStandalonePiExecutable check before CLI script resolution so that standalone Pi binaries are forwarded the original arguments directly.

  • src/runs/shared/pi-spawn.ts: Adds isStandalonePiExecutable(execPath) which matches a basename of pi or pi.exe (case-insensitive, handles both path separators) and short-circuits getPiSpawnCommand before the CLI path is inserted; the existing PI_SUBAGENT_PI_BINARY override and Node+CLI branch are unaffected.
  • test/unit/pi-spawn.test.ts: Adds a parameterized test over darwin, linux, and win32 that verifies the standalone bypass fires even when a valid CLI script could otherwise be resolved, closing the previously missing Linux coverage gap.

Confidence Score: 4/5

The spawn fix itself is correct and well-tested, but a changelog entry crediting the contributor is absent — a required process gate before landing.

The detection logic and argument construction are sound across all three platforms. The only outstanding item is that no [Unreleased] Fixed entry exists in CHANGELOG.md for this fix; comparable fixes in the same file always carry a credit line with login and PR number, and the repository's merge policy treats that credit as a required gate.

Files Needing Attention: CHANGELOG.md — needs an [Unreleased] Fixed entry for this fix before merging.

Important Files Changed

Filename Overview
src/runs/shared/pi-spawn.ts Adds isStandalonePiExecutable() guard that short-circuits before CLI path insertion when execPath basename is pi or pi.exe; logic is correct, path splitting handles both separators cross-platform
test/unit/pi-spawn.test.ts New parameterized test covers darwin, linux, and win32 for the standalone bypass; verifies the fix holds even when a valid CLI path would otherwise resolve

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[getPiSpawnCommand called] --> B{PI_SUBAGENT_PI_BINARY set?}
    B -- Yes --> C[return piBinary with args]
    B -- No --> D{isStandalonePiExecutable execPath?}
    D -- "Yes - basename is pi or pi.exe" --> E[return execPath with args directly]
    D -- "No - Node host" --> F{resolvePiCliScript succeeds?}
    F -- Yes --> G[return execPath with cliPath prepended to args]
    F -- No --> H[return bare pi with args - PATH fallback]
Loading

Reviews (2): Last reviewed commit: "test: cover Linux standalone Pi spawning" | Re-trigger Greptile

Comment thread test/unit/pi-spawn.test.ts
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.

1 participant