fix: apply pi manifest opt-out to extension-discovery.ts (#1545)

* fix: apply pi manifest opt-out to extension-discovery.ts (#1537 follow-up)

The cmux fix in #1537 patched resolveExtensionEntries() in
packages/pi-coding-agent/src/core/extensions/loader.ts to honor
"pi": {} as an opt-out from auto-discovery. However, there is a
second copy of resolveExtensionEntries() in src/extension-discovery.ts
that was not updated. This is the version actually used at startup
by loader.js via discoverExtensionEntryPaths().

As a result, cmux/index.js is still discovered and loaded as an
extension on startup, producing:
  Extension does not export a valid factory function: .../cmux/index.js

Fix: Apply the same authoritative-manifest logic to the
extension-discovery.ts copy. When a package.json has a "pi" field,
treat it as authoritative and return early — either with declared
extension paths or an empty array for library opt-out.

Tests: 7 new tests covering resolveExtensionEntries and
discoverExtensionEntryPaths behavior for opt-out, declared
extensions, and fallback discovery.

* fix: apply pi manifest opt-out to package-manager.ts (third copy)

There are THREE copies of resolveExtensionEntries():
1. packages/pi-coding-agent/src/core/extensions/loader.ts (fixed in #1537)
2. src/extension-discovery.ts (fixed in previous commit)
3. packages/pi-coding-agent/src/core/package-manager.ts (THIS commit)

Copy #3 is used by collectAutoExtensionEntries() which is called from
addAutoDiscoveredResources() during DefaultPackageManager.resolve().
This is the actual code path that discovers ~/.gsd/agent/extensions/cmux
and passes it to loadExtensions(), producing the factory function error.

* fix: rewrite pi.extensions .ts paths to .js during resource copy

copy-resources.cjs compiles .ts → .js via tsc but copies package.json
files verbatim. Extensions with pi.extensions: ["./index.ts"] end up
in dist/ pointing to a .ts file that doesn't exist (only .js does).

This causes resolveExtensionEntries() to find no valid entry points,
silently skipping the extension. Affected: gsd, browser-tools, context7,
google-search, universal-config — all extensions with pi manifests.

Fix: When copying package.json files, rewrite .ts/.tsx extensions in
pi.extensions arrays to .js so they match the compiled output.

* fix: add missing commands to /gsd description and rate sub-completions

- Add 9 missing commands to the description string: widget, rate, park,
  unpark, init, setup, logs, inspect, extensions
- Add sub-completions for /gsd rate (over/ok/under)

* feat: grid layout for parallel cmux splits and completion trailing-space fix

CmuxClient.createGridLayout(count) pre-creates a tiled grid of surfaces
before launching parallel agents, instead of the previous approach of
creating splits per-agent with alternating right/down directions.

Grid layout strategy:
  1 agent:  [gsd | A]
  2 agents: [gsd | A]    (A split down)
            [    | B]
  3 agents: [gsd | A]    (2x2 grid)
            [ C  | B]
  4 agents: [gsd | A]    (additional splits from bottom-right)
            [ C  | B]
            [    | D]

Changes:
- Add CmuxClient.createSplitFrom(sourceSurfaceId, direction) to split
  from a specific surface rather than always the gsd surface
- Add CmuxClient.createGridLayout(count) that builds the grid and
  returns surface IDs in order
- Update runSingleAgentInCmuxSplit to accept a pre-created surface ID
  (string) or a direction for backward compatibility
- Parallel dispatch pre-creates grid, assigns each agent a surface
- Fix getArgumentCompletions trailing-space handling so sub-completions
  work (e.g., /gsd cmux <tab> now shows status/on/off/etc.)
- 5 new tests for grid layout logic
This commit is contained in:
Jeremy McSpadden 2026-03-20 09:11:51 -05:00 committed by GitHub
parent 1b6b16f2d5
commit b580f64144
8 changed files with 339 additions and 18 deletions

View file

@ -414,7 +414,13 @@ function resolveExtensionEntries(dir: string): string[] | null {
const packageJsonPath = join(dir, "package.json");
if (existsSync(packageJsonPath)) {
const manifest = readPiManifestFile(packageJsonPath);
if (manifest?.extensions?.length) {
if (manifest) {
// When a pi manifest exists, it is authoritative — don't fall through
// to index.ts/index.js auto-detection. This allows library directories
// (like cmux) to opt out by declaring "pi": {} with no extensions.
if (!manifest.extensions?.length) {
return null;
}
const entries: string[] = [];
for (const extPath of manifest.extensions) {
const resolvedExtPath = resolve(dir, extPath);
@ -422,9 +428,7 @@ function resolveExtensionEntries(dir: string): string[] | null {
entries.push(resolvedExtPath);
}
}
if (entries.length > 0) {
return entries;
}
return entries.length > 0 ? entries : null;
}
}

View file

@ -18,6 +18,23 @@ function copyNonTsFiles(srcDir, destDir) {
}
mkdirSync(dirname(destPath), { recursive: true });
// Rewrite pi.extensions paths from .ts to .js in package.json files
// so they match the compiled output (tsc compiles index.ts → index.js
// but package.json is copied as-is).
if (entry.name === 'package.json') {
try {
const pkg = JSON.parse(require('fs').readFileSync(srcPath, 'utf-8'));
if (Array.isArray(pkg?.pi?.extensions)) {
pkg.pi.extensions = pkg.pi.extensions.map(ext =>
ext.replace(/\.ts$/, '.js').replace(/\.tsx$/, '.js')
);
require('fs').writeFileSync(destPath, JSON.stringify(pkg, null, 2) + '\n');
continue;
}
} catch { /* fall through to plain copy */ }
}
copyFileSync(srcPath, destPath);
}
}

View file

@ -8,24 +8,29 @@ function isExtensionFile(name: string): boolean {
/**
* Resolves the entry-point file(s) for a single extension directory.
*
* 1. If the directory contains a package.json with a `pi.extensions` array,
* each entry is resolved relative to the directory and returned (if it exists).
* 2. Otherwise falls back to `index.ts` `index.js`.
* 1. If the directory contains a package.json with a `pi` manifest object,
* the manifest is authoritative:
* - `pi.extensions` array resolve each entry relative to the directory.
* - `pi: {}` (no extensions) return empty (library opt-out, e.g. cmux).
* 2. Only when no `pi` manifest exists does it fall back to `index.ts` `index.js`.
*/
export function resolveExtensionEntries(dir: string): string[] {
const packageJsonPath = join(dir, 'package.json')
if (existsSync(packageJsonPath)) {
try {
const pkg = JSON.parse(readFileSync(packageJsonPath, 'utf-8'))
const declared = pkg?.pi?.extensions
if (Array.isArray(declared)) {
const resolved = declared
if (pkg?.pi && typeof pkg.pi === 'object') {
// When a pi manifest exists, it is authoritative — don't fall through
// to index.ts/index.js auto-detection. This allows library directories
// (like cmux) to opt out by declaring "pi": {} with no extensions.
const declared = pkg.pi.extensions
if (!Array.isArray(declared) || declared.length === 0) {
return []
}
return declared
.filter((entry: unknown): entry is string => typeof entry === 'string')
.map((entry: string) => resolve(dir, entry))
.filter((entry: string) => existsSync(entry))
if (resolved.length > 0) {
return resolved
}
}
} catch {
// Ignore malformed manifests and fall back to index.ts/index.js discovery.

View file

@ -289,10 +289,17 @@ export class CmuxClient {
}
async createSplit(direction: "right" | "down" | "left" | "up"): Promise<string | null> {
return this.createSplitFrom(this.config.surfaceId, direction);
}
async createSplitFrom(
sourceSurfaceId: string | undefined,
direction: "right" | "down" | "left" | "up",
): Promise<string | null> {
if (!this.config.splits) return null;
const before = new Set(await this.listSurfaceIds());
const args = ["new-split", direction];
const scopedArgs = this.appendSurface(this.appendWorkspace(args), this.config.surfaceId);
const scopedArgs = this.appendSurface(this.appendWorkspace(args), sourceSurfaceId);
await this.runAsync(scopedArgs);
const after = await this.listSurfaceIds();
for (const id of after) {
@ -301,6 +308,55 @@ export class CmuxClient {
return null;
}
/**
* Create a grid of surfaces for parallel agent execution.
*
* Layout strategy (gsd stays in the original surface):
* 1 agent: [gsd | A]
* 2 agents: [gsd | A]
* [ | B]
* 3 agents: [gsd | A]
* [ C | B]
* 4 agents: [gsd | A]
* [ C | B] (D splits from B downward)
* [ | D]
*
* Returns surface IDs in order, or empty array on failure.
*/
async createGridLayout(count: number): Promise<string[]> {
if (!this.config.splits || count <= 0) return [];
const surfaces: string[] = [];
// First split: create right column from the gsd surface
const rightCol = await this.createSplitFrom(this.config.surfaceId, "right");
if (!rightCol) return [];
surfaces.push(rightCol);
if (count === 1) return surfaces;
// Second split: split right column down → bottom-right
const bottomRight = await this.createSplitFrom(rightCol, "down");
if (!bottomRight) return surfaces;
surfaces.push(bottomRight);
if (count === 2) return surfaces;
// Third split: split gsd surface down → bottom-left
const bottomLeft = await this.createSplitFrom(this.config.surfaceId, "down");
if (!bottomLeft) return surfaces;
surfaces.push(bottomLeft);
if (count === 3) return surfaces;
// Fourth+: split subsequent surfaces down from the last created
let lastSurface = bottomRight;
for (let i = 3; i < count; i++) {
const next = await this.createSplitFrom(lastSurface, "down");
if (!next) break;
surfaces.push(next);
lastSurface = next;
}
return surfaces;
}
async sendSurface(surfaceId: string, text: string): Promise<boolean> {
const payload = text.endsWith("\n") ? text : `${text}\n`;
const stdout = await this.runAsync(["send-surface", "--surface", surfaceId, payload]);

View file

@ -159,7 +159,7 @@ async function guardRemoteSession(
export function registerGSDCommand(pi: ExtensionAPI): void {
pi.registerCommand("gsd", {
description: "GSD — Get Shit Done: /gsd help|start|templates|next|auto|stop|pause|status|visualize|queue|quick|capture|triage|dispatch|history|undo|skip|export|cleanup|mode|prefs|config|keys|hooks|run-hook|skill-health|doctor|forensics|changelog|migrate|remote|steer|knowledge|new-milestone|parallel|cmux|update",
description: "GSD — Get Shit Done: /gsd help|start|templates|next|auto|stop|pause|status|widget|visualize|queue|quick|capture|triage|dispatch|history|undo|rate|skip|export|cleanup|mode|prefs|config|keys|hooks|run-hook|skill-health|doctor|logs|forensics|changelog|migrate|remote|steer|knowledge|new-milestone|parallel|cmux|park|unpark|init|setup|inspect|extensions|update",
getArgumentCompletions: (prefix: string) => {
const subcommands = [
{ cmd: "help", desc: "Categorized command reference with descriptions" },
@ -210,7 +210,11 @@ export function registerGSDCommand(pi: ExtensionAPI): void {
{ cmd: "templates", desc: "List available workflow templates" },
{ cmd: "extensions", desc: "Manage extensions (list, enable, disable, info)" },
];
const hasTrailingSpace = prefix.endsWith(" ");
const parts = prefix.trim().split(/\s+/);
if (hasTrailingSpace && parts.length >= 1) {
parts.push("");
}
if (parts.length <= 1) {
return subcommands
@ -536,6 +540,18 @@ export function registerGSDCommand(pi: ExtensionAPI): void {
.map((p) => ({ value: `dispatch ${p.cmd}`, label: p.cmd, description: p.desc }));
}
if (parts[0] === "rate" && parts.length <= 2) {
const tierPrefix = parts[1] ?? "";
const tiers = [
{ cmd: "over", desc: "Model was overqualified for this task" },
{ cmd: "ok", desc: "Model was appropriate for this task" },
{ cmd: "under", desc: "Model was underqualified for this task" },
];
return tiers
.filter((t) => t.cmd.startsWith(tierPrefix))
.map((t) => ({ value: `rate ${t.cmd}`, label: t.cmd, description: t.desc }));
}
return [];
},

View file

@ -100,6 +100,99 @@ test("buildCmuxStatusLabel and progress prefer deepest active unit", () => {
assert.deepEqual(buildCmuxProgress(state), { value: 0.4, label: "2/5 tasks" });
});
describe("createGridLayout", () => {
// Create a mock CmuxClient that tracks createSplitFrom calls
function makeMockClient() {
let nextId = 1;
const calls: Array<{ source: string | undefined; direction: string }> = [];
const client = {
calls,
async createGridLayout(count: number) {
// Simulate the grid layout logic with a fake client
if (count <= 0) return [];
const surfaces: string[] = [];
const createSplitFrom = async (source: string | undefined, direction: string) => {
calls.push({ source, direction });
return `surface-${nextId++}`;
};
const rightCol = await createSplitFrom("gsd-surface", "right");
surfaces.push(rightCol);
if (count === 1) return surfaces;
const bottomRight = await createSplitFrom(rightCol, "down");
surfaces.push(bottomRight);
if (count === 2) return surfaces;
const bottomLeft = await createSplitFrom("gsd-surface", "down");
surfaces.push(bottomLeft);
if (count === 3) return surfaces;
let lastSurface = bottomRight;
for (let i = 3; i < count; i++) {
const next = await createSplitFrom(lastSurface, "down");
surfaces.push(next);
lastSurface = next;
}
return surfaces;
},
};
return client;
}
test("1 agent creates single right split", async () => {
const mock = makeMockClient();
const surfaces = await mock.createGridLayout(1);
assert.equal(surfaces.length, 1);
assert.deepEqual(mock.calls, [
{ source: "gsd-surface", direction: "right" },
]);
});
test("2 agents creates right column then splits it down", async () => {
const mock = makeMockClient();
const surfaces = await mock.createGridLayout(2);
assert.equal(surfaces.length, 2);
assert.deepEqual(mock.calls, [
{ source: "gsd-surface", direction: "right" },
{ source: "surface-1", direction: "down" },
]);
});
test("3 agents creates 2x2 grid (gsd + 3 agent surfaces)", async () => {
const mock = makeMockClient();
const surfaces = await mock.createGridLayout(3);
assert.equal(surfaces.length, 3);
assert.deepEqual(mock.calls, [
{ source: "gsd-surface", direction: "right" },
{ source: "surface-1", direction: "down" },
{ source: "gsd-surface", direction: "down" },
]);
});
test("4 agents creates 2x2 grid with extra split", async () => {
const mock = makeMockClient();
const surfaces = await mock.createGridLayout(4);
assert.equal(surfaces.length, 4);
assert.deepEqual(mock.calls, [
{ source: "gsd-surface", direction: "right" },
{ source: "surface-1", direction: "down" },
{ source: "gsd-surface", direction: "down" },
{ source: "surface-2", direction: "down" },
]);
});
test("0 agents returns empty", async () => {
const mock = makeMockClient();
const surfaces = await mock.createGridLayout(0);
assert.equal(surfaces.length, 0);
assert.equal(mock.calls.length, 0);
});
});
describe("cmux extension discovery opt-out", () => {
test("cmux directory has package.json with pi manifest to prevent auto-discovery as extension", () => {
const cmuxDir = path.resolve(

View file

@ -452,7 +452,7 @@ async function runSingleAgent(
async function runSingleAgentInCmuxSplit(
cmuxClient: CmuxClient,
direction: "right" | "down",
directionOrSurfaceId: "right" | "down" | string,
defaultCwd: string,
agents: AgentConfig[],
agentName: string,
@ -503,7 +503,12 @@ async function runSingleAgentInCmuxSplit(
const stdoutPath = path.join(tmpOutputDir, "stdout.jsonl");
const stderrPath = path.join(tmpOutputDir, "stderr.log");
const exitPath = path.join(tmpOutputDir, "exit.code");
const cmuxSurfaceId = await cmuxClient.createSplit(direction);
// Accept either a pre-created surface ID or a direction to create a new split
const isDirection = directionOrSurfaceId === "right" || directionOrSurfaceId === "down"
|| directionOrSurfaceId === "left" || directionOrSurfaceId === "up";
const cmuxSurfaceId = isDirection
? await cmuxClient.createSplit(directionOrSurfaceId as "right" | "down" | "left" | "up")
: directionOrSurfaceId;
if (!cmuxSurfaceId) {
return runSingleAgent(defaultCwd, agents, agentName, task, cwd, step, signal, onUpdate, makeDetails);
}
@ -806,12 +811,16 @@ export default function (pi: ExtensionAPI) {
const MAX_RETRIES = 1; // Retry failed tasks once
const batchId = crypto.randomUUID();
const batchSize = params.tasks.length;
// Pre-create a grid layout for cmux splits so agents get a clean tiled arrangement
const gridSurfaces = cmuxSplitsEnabled
? await cmuxClient.createGridLayout(Math.min(batchSize, MAX_CONCURRENCY))
: [];
const results = await mapWithConcurrencyLimit(params.tasks, MAX_CONCURRENCY, async (t, index) => {
const workerId = registerWorker(t.agent, t.task, index, batchSize, batchId);
const runTask = () => cmuxSplitsEnabled
? runSingleAgentInCmuxSplit(
cmuxClient,
index % 2 === 0 ? "right" : "down",
gridSurfaces[index] ?? (index % 2 === 0 ? "right" : "down"),
ctx.cwd,
agents,
t.agent,

View file

@ -0,0 +1,121 @@
import test, { describe } from 'node:test'
import assert from 'node:assert/strict'
import { mkdirSync, writeFileSync, rmSync } from 'node:fs'
import { join } from 'node:path'
import { tmpdir } from 'node:os'
import { resolveExtensionEntries, discoverExtensionEntryPaths } from '../extension-discovery.ts'
function makeTempDir(): string {
const dir = join(tmpdir(), `ext-discovery-test-${Date.now()}-${Math.random().toString(36).slice(2)}`)
mkdirSync(dir, { recursive: true })
return dir
}
describe('resolveExtensionEntries', () => {
test('returns index.ts when no package.json exists', () => {
const dir = makeTempDir()
try {
writeFileSync(join(dir, 'index.ts'), 'export default function() {}')
const entries = resolveExtensionEntries(dir)
assert.equal(entries.length, 1)
assert.ok(entries[0].endsWith('index.ts'))
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
test('returns index.js when no package.json and no index.ts', () => {
const dir = makeTempDir()
try {
writeFileSync(join(dir, 'index.js'), 'module.exports = function() {}')
const entries = resolveExtensionEntries(dir)
assert.equal(entries.length, 1)
assert.ok(entries[0].endsWith('index.js'))
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
test('returns declared extensions from pi.extensions array', () => {
const dir = makeTempDir()
try {
writeFileSync(join(dir, 'package.json'), JSON.stringify({
pi: { extensions: ['main.js'] }
}))
writeFileSync(join(dir, 'main.js'), 'module.exports = function() {}')
writeFileSync(join(dir, 'index.js'), 'should not be returned')
const entries = resolveExtensionEntries(dir)
assert.equal(entries.length, 1)
assert.ok(entries[0].endsWith('main.js'))
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
test('returns empty array when pi manifest has no extensions (library opt-out)', () => {
const dir = makeTempDir()
try {
writeFileSync(join(dir, 'package.json'), JSON.stringify({
name: '@gsd/cmux',
pi: {}
}))
writeFileSync(join(dir, 'index.js'), 'export function utility() {}')
const entries = resolveExtensionEntries(dir)
assert.equal(entries.length, 0, 'pi: {} should opt out of extension discovery')
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
test('returns empty array when pi.extensions is an empty array', () => {
const dir = makeTempDir()
try {
writeFileSync(join(dir, 'package.json'), JSON.stringify({
pi: { extensions: [] }
}))
writeFileSync(join(dir, 'index.js'), 'should not be returned')
const entries = resolveExtensionEntries(dir)
assert.equal(entries.length, 0)
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
test('falls back to index.ts when package.json has no pi field', () => {
const dir = makeTempDir()
try {
writeFileSync(join(dir, 'package.json'), JSON.stringify({ name: 'some-pkg' }))
writeFileSync(join(dir, 'index.ts'), 'export default function() {}')
const entries = resolveExtensionEntries(dir)
assert.equal(entries.length, 1)
assert.ok(entries[0].endsWith('index.ts'))
} finally {
rmSync(dir, { recursive: true, force: true })
}
})
})
describe('discoverExtensionEntryPaths', () => {
test('skips library directories with pi: {} opt-out', () => {
const root = makeTempDir()
try {
// Real extension
const extDir = join(root, 'my-ext')
mkdirSync(extDir)
writeFileSync(join(extDir, 'index.js'), 'module.exports = function() {}')
// Library with opt-out (like cmux)
const libDir = join(root, 'cmux')
mkdirSync(libDir)
writeFileSync(join(libDir, 'package.json'), JSON.stringify({ pi: {} }))
writeFileSync(join(libDir, 'index.js'), 'export function utility() {}')
const paths = discoverExtensionEntryPaths(root)
assert.equal(paths.length, 1, 'should discover my-ext but skip cmux')
assert.ok(paths[0].includes('my-ext'))
assert.ok(!paths.some(p => p.includes('cmux')), 'cmux should not be discovered')
} finally {
rmSync(root, { recursive: true, force: true })
}
})
})