fix(gsd): isolate /gsd command registration from extension bootstrap failures
On Windows, static imports in register-shortcuts.ts (added in v2.72) can fail at module load time, causing the entire GSD extension to silently fail to register. This makes /gsd unavailable despite the welcome screen suggesting it. Three changes: - index.ts: register /gsd command before importing register-extension.js, wrapped in try-catch so bootstrap failures don't prevent core command - register-extension.ts: remove duplicate registerGSDCommand call, wrap non-critical registrations (tools, shortcuts, hooks) in individual try-catch blocks so one failure doesn't prevent others - Add structural contract tests verifying isolation properties Closes #4168, closes #4172 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
a1607d2309
commit
517e7cf0fe
3 changed files with 188 additions and 10 deletions
|
|
@ -2,7 +2,6 @@
|
|||
|
||||
import type { ExtensionAPI, ExtensionCommandContext } from "@gsd/pi-coding-agent";
|
||||
|
||||
import { registerGSDCommand } from "../commands.js";
|
||||
import { registerExitCommand } from "../exit-command.js";
|
||||
import { registerWorktreeCommand } from "../worktree-command.js";
|
||||
import { registerDbTools } from "./db-tools.js";
|
||||
|
|
@ -58,7 +57,8 @@ function installEpipeGuard(): void {
|
|||
}
|
||||
|
||||
export function registerGsdExtension(pi: ExtensionAPI): void {
|
||||
registerGSDCommand(pi);
|
||||
// Note: registerGSDCommand is called by index.ts before this function,
|
||||
// so we intentionally skip it here to avoid double-registration.
|
||||
registerWorktreeCommand(pi);
|
||||
registerExitCommand(pi);
|
||||
|
||||
|
|
@ -71,10 +71,24 @@ export function registerGsdExtension(pi: ExtensionAPI): void {
|
|||
},
|
||||
});
|
||||
|
||||
registerDynamicTools(pi);
|
||||
registerDbTools(pi);
|
||||
registerJournalTools(pi);
|
||||
registerQueryTools(pi);
|
||||
registerShortcuts(pi);
|
||||
registerHooks(pi);
|
||||
// Wrap non-critical registrations individually so one failure
|
||||
// doesn't prevent the others from loading.
|
||||
const nonCriticalRegistrations: Array<[string, () => void]> = [
|
||||
["dynamic-tools", () => registerDynamicTools(pi)],
|
||||
["db-tools", () => registerDbTools(pi)],
|
||||
["journal-tools", () => registerJournalTools(pi)],
|
||||
["query-tools", () => registerQueryTools(pi)],
|
||||
["shortcuts", () => registerShortcuts(pi)],
|
||||
["hooks", () => registerHooks(pi)],
|
||||
];
|
||||
|
||||
for (const [name, register] of nonCriticalRegistrations) {
|
||||
try {
|
||||
register();
|
||||
} catch (err) {
|
||||
process.stderr.write(
|
||||
`[gsd] Failed to register ${name}: ${err instanceof Error ? err.message : String(err)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,6 +16,20 @@ export {
|
|||
} from "./bootstrap/write-gate.js";
|
||||
|
||||
export default async function registerExtension(pi: ExtensionAPI) {
|
||||
const { registerGsdExtension } = await import("./bootstrap/register-extension.js");
|
||||
registerGsdExtension(pi);
|
||||
// Always register the core /gsd command first, in isolation.
|
||||
// This ensures /gsd is available even if the full bootstrap (shortcuts,
|
||||
// tools, hooks) fails — e.g. due to a Windows-specific import error.
|
||||
const { registerGSDCommand } = await import("./commands/index.js");
|
||||
registerGSDCommand(pi);
|
||||
|
||||
// Full setup (shortcuts, tools, hooks) in a separate try/catch so that
|
||||
// any platform-specific load failure doesn't take out the core command.
|
||||
try {
|
||||
const { registerGsdExtension } = await import("./bootstrap/register-extension.js");
|
||||
registerGsdExtension(pi);
|
||||
} catch (err) {
|
||||
process.stderr.write(
|
||||
`[gsd] Extension setup partially failed — /gsd commands are available but shortcuts/tools may be missing: ${err instanceof Error ? err.message : String(err)}\n`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,150 @@
|
|||
// Structural contracts for GSD extension bootstrap isolation.
|
||||
//
|
||||
// The /gsd command must survive failures in the full extension bootstrap
|
||||
// (register-extension.ts). This guards against the regression where a
|
||||
// Windows-specific import failure in register-shortcuts.ts silently
|
||||
// prevented /gsd from being registered at all (#4168, #4172).
|
||||
|
||||
import { describe, test } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join, dirname } from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const indexSrc = readFileSync(join(__dirname, "../index.ts"), "utf-8");
|
||||
const registerExtSrc = readFileSync(
|
||||
join(__dirname, "../bootstrap/register-extension.ts"),
|
||||
"utf-8",
|
||||
);
|
||||
|
||||
// ─── index.ts: core /gsd command must be registered before full bootstrap ─────
|
||||
|
||||
describe("index.ts bootstrap isolation", () => {
|
||||
test("imports registerGSDCommand from commands/index.js separately", () => {
|
||||
assert.ok(
|
||||
indexSrc.includes('./commands/index.js"') || indexSrc.includes("./commands/index.js'"),
|
||||
"index.ts must import registerGSDCommand from ./commands/index.js",
|
||||
);
|
||||
});
|
||||
|
||||
test("calls registerGSDCommand before importing register-extension.js", () => {
|
||||
const gsdCommandCallPos = indexSrc.indexOf("registerGSDCommand(pi)");
|
||||
const bootstrapImportPos = indexSrc.indexOf(
|
||||
'./bootstrap/register-extension.js"',
|
||||
);
|
||||
|
||||
assert.ok(gsdCommandCallPos >= 0, "must call registerGSDCommand(pi)");
|
||||
assert.ok(bootstrapImportPos >= 0, "must import register-extension.js");
|
||||
assert.ok(
|
||||
gsdCommandCallPos < bootstrapImportPos,
|
||||
"registerGSDCommand(pi) must be called BEFORE importing register-extension.js",
|
||||
);
|
||||
});
|
||||
|
||||
test("wraps register-extension.js import in try-catch", () => {
|
||||
// The dynamic import of register-extension.js must be inside a try block
|
||||
const tryPos = indexSrc.indexOf("try {");
|
||||
const bootstrapImportPos = indexSrc.indexOf(
|
||||
'./bootstrap/register-extension.js"',
|
||||
);
|
||||
const catchPos = indexSrc.indexOf("catch (err)");
|
||||
|
||||
assert.ok(tryPos >= 0, "must have try block");
|
||||
assert.ok(catchPos >= 0, "must have catch block");
|
||||
assert.ok(
|
||||
tryPos < bootstrapImportPos && bootstrapImportPos < catchPos,
|
||||
"register-extension.js import must be wrapped in try-catch",
|
||||
);
|
||||
});
|
||||
|
||||
test("writes to stderr on bootstrap failure", () => {
|
||||
assert.ok(
|
||||
indexSrc.includes("process.stderr.write"),
|
||||
"must write to stderr when bootstrap fails",
|
||||
);
|
||||
assert.ok(
|
||||
indexSrc.includes("[gsd] Extension setup partially failed"),
|
||||
"stderr message must indicate partial failure with /gsd still available",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── register-extension.ts: no double-registration + defensive wrapping ───────
|
||||
|
||||
describe("register-extension.ts defensive registration", () => {
|
||||
test("does NOT import or call registerGSDCommand (avoids double-registration)", () => {
|
||||
// registerGSDCommand is now called by index.ts, not register-extension.ts
|
||||
assert.ok(
|
||||
!registerExtSrc.includes("import { registerGSDCommand }"),
|
||||
"register-extension.ts must NOT import registerGSDCommand",
|
||||
);
|
||||
|
||||
// Check the function body of registerGsdExtension doesn't call it
|
||||
const funcBodyStart = registerExtSrc.indexOf(
|
||||
"export function registerGsdExtension",
|
||||
);
|
||||
const funcBody = registerExtSrc.slice(funcBodyStart);
|
||||
assert.ok(
|
||||
!funcBody.includes("registerGSDCommand(pi)"),
|
||||
"registerGsdExtension must NOT call registerGSDCommand(pi)",
|
||||
);
|
||||
});
|
||||
|
||||
test("still registers worktree, exit, and kill commands", () => {
|
||||
const funcBodyStart = registerExtSrc.indexOf(
|
||||
"export function registerGsdExtension",
|
||||
);
|
||||
const funcBody = registerExtSrc.slice(funcBodyStart);
|
||||
|
||||
assert.ok(
|
||||
funcBody.includes("registerWorktreeCommand(pi)"),
|
||||
"must register worktree command",
|
||||
);
|
||||
assert.ok(
|
||||
funcBody.includes("registerExitCommand(pi)"),
|
||||
"must register exit command",
|
||||
);
|
||||
assert.ok(
|
||||
funcBody.includes('"kill"'),
|
||||
"must register kill command",
|
||||
);
|
||||
});
|
||||
|
||||
test("wraps non-critical registrations in individual try-catch blocks", () => {
|
||||
const funcBodyStart = registerExtSrc.indexOf(
|
||||
"export function registerGsdExtension",
|
||||
);
|
||||
const funcBody = registerExtSrc.slice(funcBodyStart);
|
||||
|
||||
// Each non-critical registration should be wrapped with error handling
|
||||
const registrationNames = [
|
||||
"dynamic-tools",
|
||||
"db-tools",
|
||||
"journal-tools",
|
||||
"query-tools",
|
||||
"shortcuts",
|
||||
"hooks",
|
||||
];
|
||||
|
||||
for (const name of registrationNames) {
|
||||
assert.ok(
|
||||
funcBody.includes(`"${name}"`),
|
||||
`non-critical registration "${name}" must be present`,
|
||||
);
|
||||
}
|
||||
|
||||
// Must have try-catch inside the registration loop
|
||||
assert.ok(
|
||||
funcBody.includes("try {") && funcBody.includes("catch (err)"),
|
||||
"must have try-catch for non-critical registrations",
|
||||
);
|
||||
});
|
||||
|
||||
test("writes to stderr when a non-critical registration fails", () => {
|
||||
assert.ok(
|
||||
registerExtSrc.includes("[gsd] Failed to register"),
|
||||
"must write descriptive error to stderr for individual registration failures",
|
||||
);
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Reference in a new issue