Add Pi extensions and same-run add_tool to CUA CLI#41
Open
rgarcia wants to merge 18 commits into
Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: sendUserMessage lacks screenshot
sendUserMessagenow routes through a host prompt path that attaches an initial screenshot on first-turn sessions via{ images }before callingharness.prompt.
- ✅ Fixed: Reapply reactivates extension tools
reapplyToolsnow preserves the existing active-tool set and only auto-activates newly introduced extension tools instead of force-enabling all extension tools.
- ✅ Fixed: Reload races async dispose
- Shutdown requests raised during reload are now latched and handled inside reload before rebuilding, preventing async dispose from tearing down a freshly rebuilt runner/bridge.
- ✅ Fixed: Skipped reapply while applying tools
- Nested
reapplyToolscalls now set a queue flag and trigger a follow-up pass after the currentsetToolscall completes, so refresh requests are not dropped.
- Nested
Or push these changes by commenting:
@cursor push ee06abc4e1
Preview (ee06abc4e1)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -1,4 +1,5 @@
import type { AgentHarness, AgentTool, Session } from "@onkernel/cua-agent";
+import type { ImageContent } from "@onkernel/cua-ai";
import {
AuthStorage,
discoverAndLoadExtensions,
@@ -23,6 +24,8 @@
harness: AgentHarness;
/** The same `Session` the harness was constructed with; used for entry writes. */
session: Session;
+ /** Capture a screenshot attachment for first-turn user messages. */
+ initialScreenshot: () => Promise<ImageContent[] | undefined>;
cwd: string;
/** Extension paths passed straight to `discoverAndLoadExtensions`. */
configuredPaths: string[];
@@ -48,6 +51,7 @@
export class HarnessExtensionHost {
private readonly harness: AgentHarness;
private readonly session: Session;
+ private readonly initialScreenshot: () => Promise<ImageContent[] | undefined>;
private readonly cwd: string;
private readonly configuredPaths: string[];
private readonly agentDir?: string;
@@ -66,6 +70,12 @@
private extensionTools: AgentTool[] = [];
/** Guards `harness.setTools` so a tools_update never re-enters reapply. */
private applyingTools = false;
+ /** Follow-up pass requested while `harness.setTools` is in flight. */
+ private reapplyQueued = false;
+ /** Marks reload critical sections where shutdown requests must not race. */
+ private reloading = false;
+ /** Sticky shutdown request raised by `ctx.shutdown()` or owner disposal. */
+ private shutdownRequested = false;
/** Guards `dispose` so `ctx.shutdown()` and an owner call don't double-tear-down. */
private disposed = false;
private sessionName: string | undefined;
@@ -76,6 +86,7 @@
constructor(options: HarnessExtensionHostOptions) {
this.harness = options.harness;
this.session = options.session;
+ this.initialScreenshot = options.initialScreenshot;
this.cwd = options.cwd;
this.configuredPaths = options.configuredPaths;
this.agentDir = options.agentDir;
@@ -84,6 +95,7 @@
this.actions = makeExtensionActions(this.harness, this.session, {
refreshTools: () => void this.reapplyTools(),
+ sendUserMessage: (text) => this.promptUserMessage(text),
getSessionName: () => this.sessionName,
setSessionName: (name) => {
this.sessionName = name;
@@ -112,17 +124,26 @@
* the loader imports each extension fresh from disk.
*/
async reload(): Promise<void> {
+ if (this.disposed) return;
const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
- await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
- this.teardownBridge?.();
- this.teardownBridge = undefined;
+ this.reloading = true;
+ try {
+ await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
+ if (await this.disposeIfShutdownRequested()) return;
+ this.teardownBridge?.();
+ this.teardownBridge = undefined;
- await this.buildRunner();
- for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
+ await this.buildRunner();
+ if (await this.disposeIfShutdownRequested()) return;
+ for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
- await this.reapplyTools();
- this.installBridge();
- await this.runner?.emit({ type: "session_start", reason: "reload" });
+ await this.reapplyTools();
+ if (await this.disposeIfShutdownRequested()) return;
+ this.installBridge();
+ await this.runner?.emit({ type: "session_start", reason: "reload" });
+ } finally {
+ this.reloading = false;
+ }
}
/**
@@ -135,6 +156,7 @@
*/
async dispose(): Promise<void> {
if (this.disposed) return;
+ this.shutdownRequested = true;
this.disposed = true;
this.teardownBridge?.();
this.teardownBridge = undefined;
@@ -159,28 +181,45 @@
/**
* Rebuild the extension-tool union and apply it to the harness as the
- * authoritative tool list. Extension tools are de-duped by name (the harness
- * rejects duplicates) and kept active alongside the base tools. The
- * re-entrancy guard makes a stray `tools_update` subscriber safe; reapply is
- * only triggered from `load`/`reload`/`model_update`/`refreshTools`, none of
- * which run concurrently.
+ * authoritative tool list. Existing active-tool choices are preserved for
+ * both base and extension tools, while newly introduced extension tools start
+ * active by default. A queued follow-up pass handles refresh requests that
+ * arrive while `harness.setTools` is still in flight.
*/
private async reapplyTools(): Promise<void> {
- if (!this.runner || this.applyingTools) return;
- this.extensionTools = wrapRegisteredTools(this.runner.getAllRegisteredTools(), this.runner);
- const extensionNames = new Set(this.extensionTools.map((tool) => tool.name));
- const base = this.harness.getTools().filter((tool) => !extensionNames.has(tool.name));
- const final = [...base, ...this.extensionTools];
- const activeNames = [
- ...this.harness.getActiveTools().map((tool) => tool.name),
- ...extensionNames,
- ];
- this.applyingTools = true;
- try {
- await this.harness.setTools(final, [...new Set(activeNames)]);
- } finally {
- this.applyingTools = false;
+ if (!this.runner) return;
+ if (this.applyingTools) {
+ this.reapplyQueued = true;
+ return;
}
+ do {
+ this.reapplyQueued = false;
+ if (!this.runner) return;
+
+ const previousExtensionNames = new Set(this.extensionTools.map((tool) => tool.name));
+ const nextExtensionTools = wrapRegisteredTools(this.runner.getAllRegisteredTools(), this.runner);
+ const extensionNames = new Set(nextExtensionTools.map((tool) => tool.name));
+ const base = this.harness.getTools().filter((tool) => !extensionNames.has(tool.name));
+ const final = [...base, ...nextExtensionTools];
+ const finalNames = new Set(final.map((tool) => tool.name));
+ const activeNames = new Set(
+ this.harness
+ .getActiveTools()
+ .map((tool) => tool.name)
+ .filter((name) => finalNames.has(name)),
+ );
+ for (const name of extensionNames) {
+ if (!previousExtensionNames.has(name)) activeNames.add(name);
+ }
+
+ this.extensionTools = nextExtensionTools;
+ this.applyingTools = true;
+ try {
+ await this.harness.setTools(final, [...activeNames]);
+ } finally {
+ this.applyingTools = false;
+ }
+ } while (this.reapplyQueued);
}
private installBridge(): void {
@@ -191,6 +230,35 @@
}
private requestShutdown(): void {
+ this.shutdownRequested = true;
+ if (this.reloading) return;
void this.dispose();
}
+
+ private async promptUserMessage(text: string): Promise<void> {
+ const images = await this.maybeInitialScreenshot();
+ await this.harness.prompt(text, images ? { images } : undefined);
+ }
+
+ private async maybeInitialScreenshot(): Promise<ImageContent[] | undefined> {
+ const hasPriorTurn = await sessionHasPriorTurn(this.session);
+ if (hasPriorTurn) return undefined;
+ return this.initialScreenshot();
+ }
+
+ private async disposeIfShutdownRequested(): Promise<boolean> {
+ if (!this.shutdownRequested && !this.disposed) return false;
+ await this.dispose();
+ return true;
+ }
}
+
+async function sessionHasPriorTurn(session: Session): Promise<boolean> {
+ const entries = await session.getBranch();
+ for (const entry of entries) {
+ if (entry.type === "message" && (entry.message.role === "user" || entry.message.role === "assistant")) {
+ return true;
+ }
+ }
+ return false;
+}
diff --git a/packages/cli/src/extensions/seams.ts b/packages/cli/src/extensions/seams.ts
--- a/packages/cli/src/extensions/seams.ts
+++ b/packages/cli/src/extensions/seams.ts
@@ -19,6 +19,8 @@
export interface SeamHooks {
/** Re-apply the authoritative base+extension tool union to the harness. */
refreshTools: () => void;
+ /** Forward user text through the host's first-turn image attachment path. */
+ sendUserMessage: (text: string) => Promise<void>;
/** Synchronous mirror of the session name (kept because the action getter is sync). */
getSessionName: () => string | undefined;
/** Record the latest session name set through the action surface. */
@@ -41,7 +43,7 @@
},
sendUserMessage(content): void {
const text = typeof content === "string" ? content : textPartsOf(content);
- void harness.prompt(text);
+ void hooks.sendUserMessage(text);
},
appendEntry(customType, data): void {
void session.appendCustomEntry(customType, data);
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -36,6 +36,7 @@
const created = new HarnessExtensionHost({
harness: fx.harness,
session: fx.session,
+ initialScreenshot: async () => undefined,
cwd: fx.cwd,
configuredPaths: [makeExtensionDir()],
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Load after dispose breaks lifecycle
load()now returns immediately when the host is disposed, preventing runner/bridge reconstruction after terminal shutdown.
- ✅ Fixed: Repeated load stacks bridge listeners
load()now no-ops once a runner already exists, so duplicate bridge installations and stacked harness listeners cannot occur.
- ✅ Fixed: Nested reload double teardown
reload()now exits early whenreloadingis already true, preventing reentrant reload passes from tearing down each other’s newly built state.
- ✅ Fixed: Reload skips idle wait
reload()now awaitsharness.waitForIdle()before shutdown/bridge teardown, so in-flight runs finish before listeners are detached.
Or push these changes by commenting:
@cursor push a135464efc
Preview (a135464efc)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -118,6 +118,7 @@
}
async load(): Promise<void> {
+ if (this.disposed || this.runner) return;
await this.buildRunner();
await this.reapplyTools();
this.installBridge();
@@ -132,14 +133,16 @@
* the loader imports each extension fresh from disk.
*/
async reload(): Promise<void> {
- if (this.disposed) return;
- const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
+ if (this.disposed || this.reloading) return;
// `reloading` defers any `ctx.shutdown()` raised by an extension's
// session_shutdown handler so an unawaited dispose can't tear down the
- // runner/bridge mid-rebuild. Each await boundary then honors a pending
- // request before continuing.
+ // runner/bridge mid-rebuild (including while waiting for the harness to go
+ // idle). Each await boundary then honors a pending request before continuing.
this.reloading = true;
try {
+ await this.harness.waitForIdle();
+ if (await this.disposeIfShutdownRequested()) return;
+ const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
if (await this.disposeIfShutdownRequested()) return;
this.teardownBridge?.();You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 5 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Reload keeps removed extension tools
reapplyToolsnow excludes both newly loaded and previously registered extension tool names from the base harness list, so removed extensions are dropped on reload, and a regression test covers this case.
Or push these changes by commenting:
@cursor push 9153a337fb
Preview (9153a337fb)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -218,7 +218,12 @@
this.runner,
);
const extensionNames = new Set(nextExtensionTools.map((tool) => tool.name));
- const base = this.harness.getTools().filter((tool) => !extensionNames.has(tool.name));
+ const previousExtensionNames = new Set(this.extensionTools.map((tool) => tool.name));
+ const base = this.harness
+ .getTools()
+ .filter(
+ (tool) => !extensionNames.has(tool.name) && !previousExtensionNames.has(tool.name),
+ );
const final = [...base, ...nextExtensionTools];
const finalNames = new Set(final.map((tool) => tool.name));
const activeNames = new Set(
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -1,5 +1,5 @@
import { afterEach, describe, expect, it } from "vitest";
-import { cpSync, mkdtempSync } from "node:fs";
+import { cpSync, mkdtempSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
@@ -104,4 +104,29 @@
expect(fx!.harness.getTools().map((tool) => tool.name)).toContain("click_visual");
});
+
+ it("drops removed extension tools after reload", async () => {
+ fx = await buildTestHarness({
+ turns: [
+ { steps: [{ type: "tool_call", toolName: "click_visual", args: { description: "the button" } }] },
+ { steps: [{ type: "text", text: "done" }] },
+ ],
+ });
+ const extDir = makeExtensionDir();
+ const created = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ });
+ await created.load();
+ host = created;
+ expect(fx.harness.getTools().map((tool) => tool.name)).toContain("click_visual");
+
+ rmSync(join(extDir, "click-visual.ts"));
+ await created.reload();
+
+ expect(fx.harness.getTools().map((tool) => tool.name)).not.toContain("click_visual");
+ });
});You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Base tools lost after name clash
reapplyTools()now rebuilds base tools fromCuaAgentHarness.getRuntimeTools()when available so a removed shadowing extension no longer deletes the underlying built-in tool.
- ✅ Fixed: Load succeeds after startup shutdown
load()now checks for a latched shutdown after startupsession_start, disposes, and throws so startup does not report success after extension-triggered shutdown.
Or push these changes by commenting:
@cursor push 83fff7ee80
Preview (83fff7ee80)
diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts
--- a/packages/agent/src/agent.ts
+++ b/packages/agent/src/agent.ts
@@ -372,6 +372,10 @@
});
}
+ getRuntimeTools(): AgentTool[] {
+ return this.runtime.tools();
+ }
+
/**
* Mirror pi `AgentHarness.setModel()` while accepting CUA model refs.
*
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -20,6 +20,10 @@
makeExtensionContextActions,
} from "./seams";
+type RuntimeToolAwareHarness = AgentHarness & {
+ getRuntimeTools?: () => AgentTool[];
+};
+
export interface HarnessExtensionHostOptions {
harness: AgentHarness;
/** The same `Session` the harness was constructed with; used for entry writes. */
@@ -122,6 +126,9 @@
await this.reapplyTools();
this.installBridge();
await this.runner?.emit({ type: "session_start", reason: "startup" });
+ if (await this.disposeIfShutdownRequested()) {
+ throw new Error("HarnessExtensionHost shut down during startup");
+ }
}
/**
@@ -224,9 +231,12 @@
this.runner,
);
const extensionNames = new Set(nextExtensionTools.map((tool) => tool.name));
- const base = this.harness
- .getTools()
- .filter((tool) => !extensionNames.has(tool.name) && !priorExtensionNames.has(tool.name));
+ const { tools: runtimeBaseTools, authoritative } = this.getCurrentBaseTools();
+ const base = runtimeBaseTools.filter((tool) => {
+ if (extensionNames.has(tool.name)) return false;
+ if (authoritative) return true;
+ return !priorExtensionNames.has(tool.name);
+ });
const final = [...base, ...nextExtensionTools];
const finalNames = new Set(final.map((tool) => tool.name));
const activeNames = new Set(
@@ -248,6 +258,12 @@
} while (this.reapplyQueued);
}
+ private getCurrentBaseTools(): { tools: AgentTool[]; authoritative: boolean } {
+ const runtimeTools = (this.harness as RuntimeToolAwareHarness).getRuntimeTools?.();
+ if (runtimeTools) return { tools: runtimeTools, authoritative: true };
+ return { tools: this.harness.getTools(), authoritative: false };
+ }
+
/**
* Apply an extension-requested active-tool set, recording which extension
* tools were turned off so `reapplyTools` won't silently re-enable them.
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -132,6 +132,47 @@
expect(toolNames).toContain("beta_tool");
expect(toolNames).not.toContain("alpha_tool");
});
+
+ it("restores colliding base tools when an extension stops registering them", async () => {
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ const extFile = join(extDir, "shadow.ts");
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const collidingToolName = fx.harness.getTools()[0]?.name;
+ expect(collidingToolName).toBeDefined();
+ writeFileSync(extFile, makeToolExtension(collidingToolName!));
+
+ const created = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ });
+ host = created;
+ await created.load();
+
+ writeFileSync(extFile, makeNoopExtension());
+ await created.reload();
+
+ expect(fx.harness.getTools().map((tool) => tool.name)).toContain(collidingToolName);
+ });
+
+ it("fails startup when an extension requests shutdown during session_start", async () => {
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ const extFile = join(extDir, "shutdown.ts");
+ writeFileSync(extFile, makeShutdownOnStartupExtension());
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const created = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ });
+ host = created;
+
+ await expect(created.load()).rejects.toThrow("HarnessExtensionHost shut down during startup");
+ });
});
/** A minimal, import-free extension that registers a single named tool. */
@@ -149,3 +190,16 @@
"",
].join("\n");
}
+
+function makeNoopExtension(): string {
+ return ["export default function () {}", ""].join("\n");
+}
+
+function makeShutdownOnStartupExtension(): string {
+ return [
+ "export default function (pi) {",
+ ' pi.on("session_start", (_event, ctx) => ctx.shutdown());',
+ "}",
+ "",
+ ].join("\n");
+}You can send follow-ups to the cloud agent here.
rgarcia
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.
There are 10 total unresolved issues (including 6 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: Project extensions skip trust gating
- Project-local extension loading is now trust-gated via persisted project trust or explicit
--trust-extensions, while global agent-dir extensions still load.
- Project-local extension loading is now trust-gated via persisted project trust or explicit
- ✅ Fixed: Reload drops bridge on failure
- Reload now rebuilds and reapplies tools before bridge teardown and restores prior host state on errors so the bridge remains installed.
- ✅ Fixed: /reload runs during active agent
/reloadnow waits forharness.waitForIdle()before callinghost.reload()to prevent mid-turn bridge or runner swaps.
- ✅ Fixed: Aborted reload reports success
- Reload command feedback now checks host disposal state and reports shutdown instead of showing a false 'extensions reloaded' success notice.
Or push these changes by commenting:
@cursor push 4fd32a09e4
Preview (4fd32a09e4)
diff --git a/packages/cli/src/cli-harness.ts b/packages/cli/src/cli-harness.ts
--- a/packages/cli/src/cli-harness.ts
+++ b/packages/cli/src/cli-harness.ts
@@ -177,6 +177,7 @@
noSession: boolean;
noSkills: boolean;
noExtensions: boolean;
+ trustExtensions: boolean;
debugTui: boolean;
jsonlIncludeDeltas: boolean;
jsonlIncludeImages: boolean;
@@ -441,6 +442,7 @@
session,
cwd,
noExtensions: flags.noExtensions,
+ trustExtensions: flags.trustExtensions,
initialScreenshot,
});
} catch (err) {
diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts
--- a/packages/cli/src/cli.ts
+++ b/packages/cli/src/cli.ts
@@ -68,6 +68,8 @@
--no-extensions Disable pi extensions, which otherwise load from
<cwd>/.agents/extensions, <cwd>/.pi/extensions,
and the pi agent dir (~/.pi/agent/extensions/)
+ --trust-extensions Trust project-local extension directories for this
+ run (<cwd>/.agents/extensions and <cwd>/.pi/extensions)
--debug-tui Enable TUI render diagnostics for manual repros
-v, --verbose Verbose progress output to stderr
-h, --help Show this help
@@ -101,6 +103,7 @@
noSession: boolean;
noSkills: boolean;
noExtensions: boolean;
+ trustExtensions: boolean;
debugTui: boolean;
jsonlIncludeDeltas: boolean;
jsonlIncludeImages: boolean;
@@ -150,6 +153,7 @@
skill: { type: "string", multiple: true, default: [] },
"no-skills": { type: "boolean", default: false },
"no-extensions": { type: "boolean", default: false },
+ "trust-extensions": { type: "boolean", default: false },
"debug-tui": { type: "boolean", default: false },
output: { type: "string", short: "o" },
"jsonl-include-deltas": { type: "boolean", default: false },
@@ -187,6 +191,7 @@
noSession: !!parsed.values["no-session"],
noSkills: !!parsed.values["no-skills"],
noExtensions: !!parsed.values["no-extensions"],
+ trustExtensions: !!parsed.values["trust-extensions"],
debugTui: !!parsed.values["debug-tui"],
model: parsed.values.model as string | undefined,
thinking: parsed.values.thinking as string | undefined,
@@ -216,6 +221,7 @@
noSession: flags.noSession,
noSkills: flags.noSkills,
noExtensions: flags.noExtensions,
+ trustExtensions: flags.trustExtensions,
debugTui: flags.debugTui,
jsonlIncludeDeltas: flags.jsonlIncludeDeltas,
jsonlIncludeImages: flags.jsonlIncludeImages,
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -2,9 +2,12 @@
import type { ImageContent } from "@onkernel/cua-ai";
import {
AuthStorage,
+ DefaultResourceLoader,
discoverAndLoadExtensions,
ExtensionRunner,
+ getAgentDir,
ModelRegistry,
+ SettingsManager,
SessionManager,
wrapRegisteredTools,
} from "@earendil-works/pi-coding-agent";
@@ -13,6 +16,7 @@
ExtensionCommandContextActions,
ExtensionContextActions,
} from "@earendil-works/pi-coding-agent";
+import { isAbsolute, relative, resolve } from "node:path";
import { installBridge, type BridgeState } from "./bridge";
import {
makeExtensionActions,
@@ -27,6 +31,8 @@
cwd: string;
/** Extension paths passed straight to `discoverAndLoadExtensions`. */
configuredPaths: string[];
+ /** Whether project-local extension sources under `cwd` may be executed. */
+ projectTrusted: boolean;
/** Agent config dir searched for `extensions/`. Pass a temp dir to isolate from `~/.agents`. */
agentDir?: string;
/**
@@ -58,6 +64,7 @@
private readonly session: Session;
private readonly cwd: string;
private readonly configuredPaths: string[];
+ private readonly projectTrusted: boolean;
private readonly agentDir?: string;
private readonly initialScreenshot?: () => Promise<ImageContent[] | undefined>;
private readonly sessionManager: SessionManager;
@@ -95,6 +102,7 @@
this.session = options.session;
this.cwd = options.cwd;
this.configuredPaths = options.configuredPaths;
+ this.projectTrusted = options.projectTrusted;
this.agentDir = options.agentDir;
this.initialScreenshot = options.initialScreenshot;
this.sessionManager = SessionManager.inMemory(this.cwd);
@@ -111,6 +119,7 @@
});
this.contextActions = makeExtensionContextActions(this.harness, {
isIdle: () => this.bridgeState.isIdle,
+ isProjectTrusted: () => this.projectTrusted,
getSignal: () => undefined,
shutdown: () => this.requestShutdown(),
});
@@ -118,21 +127,30 @@
}
async load(): Promise<void> {
- await this.buildRunner();
+ const { runner, loadErrors } = await this.buildRunner();
+ this.runner = runner;
+ this.loadErrors = loadErrors;
await this.reapplyTools();
this.installBridge();
await this.runner?.emit({ type: "session_start", reason: "startup" });
}
+ isDisposed(): boolean {
+ return this.disposed;
+ }
+
/**
- * Mirror `AgentSession.reload`: carry over flag values, tear down the old
- * runner's bridge, re-discover extensions from disk, build a fresh runner over
- * the same in-memory services, restore flags, rebind, re-apply tools, reinstall
- * the bridge, then emit `session_start`. No extension cache is cleared because
- * the loader imports each extension fresh from disk.
+ * Mirror `AgentSession.reload`: carry over flag values, re-discover
+ * extensions from disk, build a fresh runner over the same in-memory
+ * services, restore flags, re-apply tools, swap bridges, then emit
+ * `session_start`. No extension cache is cleared because the loader imports
+ * each extension fresh from disk.
*/
async reload(): Promise<void> {
if (this.disposed) return;
+ const previousRunner = this.runner;
+ const previousLoadErrors = this.loadErrors;
+ const previousExtensionTools = this.extensionTools;
const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
// `reloading` defers any `ctx.shutdown()` raised by an extension's
// session_shutdown handler so an unawaited dispose can't tear down the
@@ -140,19 +158,33 @@
// request before continuing.
this.reloading = true;
try {
- await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
+ await previousRunner?.emit({ type: "session_shutdown", reason: "reload" });
if (await this.disposeIfShutdownRequested()) return;
- this.teardownBridge?.();
- this.teardownBridge = undefined;
-
- await this.buildRunner();
+ const { runner, loadErrors } = await this.buildRunner();
if (await this.disposeIfShutdownRequested()) return;
+ this.runner = runner;
+ this.loadErrors = loadErrors;
for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
await this.reapplyTools();
if (await this.disposeIfShutdownRequested()) return;
+ this.teardownBridge?.();
+ this.teardownBridge = undefined;
this.installBridge();
await this.runner?.emit({ type: "session_start", reason: "reload" });
+ } catch (error) {
+ if (!this.disposed) {
+ this.runner = previousRunner;
+ this.loadErrors = previousLoadErrors;
+ this.extensionTools = previousExtensionTools;
+ try {
+ await this.reapplyTools();
+ } catch {
+ // Preserve the original reload error.
+ }
+ if (!this.teardownBridge && this.runner) this.installBridge();
+ }
+ throw error;
} finally {
this.reloading = false;
}
@@ -178,21 +210,46 @@
this.runner = undefined;
}
- private async buildRunner(): Promise<void> {
- const result = await discoverAndLoadExtensions(this.configuredPaths, this.cwd, this.agentDir);
- this.loadErrors = result.errors;
- this.runner = new ExtensionRunner(
+ private async buildRunner(): Promise<{
+ runner: ExtensionRunner;
+ loadErrors: Array<{ path: string; error: string }>;
+ }> {
+ const result = await this.discoverExtensions();
+ const runner = new ExtensionRunner(
result.extensions,
result.runtime,
this.cwd,
this.sessionManager,
this.modelRegistry,
);
- this.runner.bindCore(this.actions, this.contextActions);
- this.runner.bindCommandContext(this.commandActions);
- this.runner.setUIContext(undefined, "print");
+ runner.bindCore(this.actions, this.contextActions);
+ runner.bindCommandContext(this.commandActions);
+ runner.setUIContext(undefined, "print");
+ return { runner, loadErrors: result.errors };
}
+ private async discoverExtensions() {
+ if (this.projectTrusted) {
+ return discoverAndLoadExtensions(this.configuredPaths, this.cwd, this.agentDir);
+ }
+ const agentDir = this.agentDir ?? getAgentDir();
+ const settingsManager = SettingsManager.create(this.cwd, agentDir, { projectTrusted: false });
+ const loader = new DefaultResourceLoader({
+ cwd: this.cwd,
+ agentDir,
+ settingsManager,
+ additionalExtensionPaths: this.configuredPaths
+ .map((path) => resolve(this.cwd, path))
+ .filter((path) => !isUnderPath(path, this.cwd)),
+ noSkills: true,
+ noPromptTemplates: true,
+ noThemes: true,
+ noContextFiles: true,
+ });
+ await loader.reload();
+ return loader.getExtensions();
+ }
+
/**
* Rebuild the extension-tool union and apply it to the harness as the
* authoritative tool list. Extension tools are de-duped by name (the harness
@@ -308,3 +365,8 @@
(entry.message.role === "user" || entry.message.role === "assistant"),
);
}
+
+function isUnderPath(target: string, root: string): boolean {
+ const rel = relative(resolve(root), resolve(target));
+ return rel === "" || (!rel.startsWith("..") && !isAbsolute(rel));
+}
diff --git a/packages/cli/src/extensions/seams.ts b/packages/cli/src/extensions/seams.ts
--- a/packages/cli/src/extensions/seams.ts
+++ b/packages/cli/src/extensions/seams.ts
@@ -98,7 +98,12 @@
export function makeExtensionContextActions(
harness: AgentHarness,
- state: { isIdle: () => boolean; getSignal: () => AbortSignal | undefined; shutdown: () => void },
+ state: {
+ isIdle: () => boolean;
+ isProjectTrusted: () => boolean;
+ getSignal: () => AbortSignal | undefined;
+ shutdown: () => void;
+ },
): ExtensionContextActions {
return {
getModel() {
@@ -107,9 +112,8 @@
isIdle() {
return state.isIdle();
},
- // Headless host trusts its cwd; project-trust prompts are a TUI concern.
isProjectTrusted(): boolean {
- return true;
+ return state.isProjectTrusted();
},
getSignal() {
return state.getSignal();
diff --git a/packages/cli/src/extensions/setup.ts b/packages/cli/src/extensions/setup.ts
--- a/packages/cli/src/extensions/setup.ts
+++ b/packages/cli/src/extensions/setup.ts
@@ -1,17 +1,22 @@
import type { CuaAgentHarness, Session } from "@onkernel/cua-agent";
import type { ImageContent } from "@onkernel/cua-ai";
-import { getAgentDir } from "@earendil-works/pi-coding-agent";
+import {
+ getAgentDir,
+ hasProjectTrustInputs,
+ ProjectTrustStore,
+ SettingsManager,
+} from "@earendil-works/pi-coding-agent";
+import { existsSync } from "node:fs";
import { join } from "node:path";
import { HarnessExtensionHost } from "./host";
/**
* Resolve extension directories and construct + load a {@link HarnessExtensionHost}.
*
- * Global extensions (`<getAgentDir()>/extensions`) and project-local extensions
- * (`<cwd>/.agents/extensions` plus the loader's implicit `<cwd>/.pi/extensions`
- * scan) all load on every run; `--no-extensions` opts out entirely. This is the
- * substrate for the self-improve loop: an agent writes a learned tool into the
- * project extension dir and it loads on the next run.
+ * Global extensions (`<getAgentDir()>/extensions`) always load; project-local
+ * extensions (`<cwd>/.agents/extensions` plus `<cwd>/.pi/extensions`) only load
+ * when project trust resolves true or `--trust-extensions` is set. `--no-extensions`
+ * opts out entirely.
*
* No browser/auth/provisioning happens here, so a test can drive the exact load
* path the CLI uses with a `buildTestHarness` fixture and temp dirs.
@@ -21,21 +26,45 @@
session: Session;
cwd: string;
noExtensions: boolean;
+ trustExtensions?: boolean;
agentDir?: string;
configuredPaths?: string[];
initialScreenshot?: () => Promise<ImageContent[] | undefined>;
}): Promise<HarnessExtensionHost | undefined> {
if (args.noExtensions) return undefined;
const agentDir = args.agentDir ?? getAgentDir();
+ const projectTrusted = resolveProjectExtensionTrust({
+ cwd: args.cwd,
+ agentDir,
+ trustExtensions: args.trustExtensions === true,
+ });
const configuredPaths = args.configuredPaths ?? [join(args.cwd, ".agents", "extensions")];
const host = new HarnessExtensionHost({
harness: args.harness,
session: args.session,
cwd: args.cwd,
configuredPaths,
+ projectTrusted,
agentDir,
initialScreenshot: args.initialScreenshot,
});
await host.load();
return host;
}
+
+function resolveProjectExtensionTrust(args: {
+ cwd: string;
+ agentDir: string;
+ trustExtensions: boolean;
+}): boolean {
+ if (args.trustExtensions) return true;
+ if (!hasProjectExtensionInputs(args.cwd)) return true;
+ const trustDecision = new ProjectTrustStore(args.agentDir).get(args.cwd);
+ if (trustDecision !== null) return trustDecision;
+ const settings = SettingsManager.create(args.cwd, args.agentDir, { projectTrusted: false });
+ return settings.getDefaultProjectTrust() === "always";
+}
+
+function hasProjectExtensionInputs(cwd: string): boolean {
+ return hasProjectTrustInputs(cwd) || existsSync(join(cwd, ".agents", "extensions"));
+}
diff --git a/packages/cli/src/tui/main.ts b/packages/cli/src/tui/main.ts
--- a/packages/cli/src/tui/main.ts
+++ b/packages/cli/src/tui/main.ts
@@ -531,16 +531,25 @@
}
export async function applyReloadCommand(opts: InteractiveOptions, messages: MessageList): Promise<void> {
- if (!opts.host) {
+ if (!opts.host || opts.host.isDisposed()) {
messages.addNotice("extensions are disabled");
return;
}
messages.addNotice("reloading extensions…");
try {
+ await opts.harness.waitForIdle();
+ if (opts.host.isDisposed()) {
+ messages.addNotice("extensions are disabled");
+ return;
+ }
// reload() emits no harness event, so this helper is the only source of
// feedback; surface loadErrors so a broken edited extension isn't silently
// dropped with its tool missing.
await opts.host.reload();
+ if (opts.host.isDisposed()) {
+ messages.addNotice("extensions were shut down");
+ return;
+ }
if (opts.host.loadErrors.length > 0) {
for (const { path, error } of opts.host.loadErrors) messages.addError(`${path}: ${error}`);
} else {
diff --git a/packages/cli/test/e2e/agent-start-counter-shortcut.test.ts b/packages/cli/test/e2e/agent-start-counter-shortcut.test.ts
--- a/packages/cli/test/e2e/agent-start-counter-shortcut.test.ts
+++ b/packages/cli/test/e2e/agent-start-counter-shortcut.test.ts
@@ -120,6 +120,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [extDir],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
await host.load();
diff --git a/packages/cli/test/e2e/dom-table-extraction.test.ts b/packages/cli/test/e2e/dom-table-extraction.test.ts
--- a/packages/cli/test/e2e/dom-table-extraction.test.ts
+++ b/packages/cli/test/e2e/dom-table-extraction.test.ts
@@ -87,6 +87,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [extDir],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
await host.load();
diff --git a/packages/cli/test/e2e/form-fill-macro.test.ts b/packages/cli/test/e2e/form-fill-macro.test.ts
--- a/packages/cli/test/e2e/form-fill-macro.test.ts
+++ b/packages/cli/test/e2e/form-fill-macro.test.ts
@@ -90,6 +90,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [extDir],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
await host.load();
diff --git a/packages/cli/test/e2e/nav-shortcut-tool.test.ts b/packages/cli/test/e2e/nav-shortcut-tool.test.ts
--- a/packages/cli/test/e2e/nav-shortcut-tool.test.ts
+++ b/packages/cli/test/e2e/nav-shortcut-tool.test.ts
@@ -85,6 +85,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [extDir],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
await host.load();
diff --git a/packages/cli/test/e2e/template-match-click.test.ts b/packages/cli/test/e2e/template-match-click.test.ts
--- a/packages/cli/test/e2e/template-match-click.test.ts
+++ b/packages/cli/test/e2e/template-match-click.test.ts
@@ -98,6 +98,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [extDir],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
await host.load();
diff --git a/packages/cli/test/extension-loader.test.ts b/packages/cli/test/extension-loader.test.ts
--- a/packages/cli/test/extension-loader.test.ts
+++ b/packages/cli/test/extension-loader.test.ts
@@ -74,7 +74,7 @@
expect(fx.harness.getTools().map((t) => t.name)).not.toContain("loader_probe");
});
- it("loads the implicit project <cwd>/.pi/extensions scan by default", async () => {
+ it("does not load the implicit project <cwd>/.pi/extensions scan when untrusted", async () => {
fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
// Unique per run so the whole-harness tool assertion can't collide with
// another worker registering the same name under pool concurrency.
@@ -92,10 +92,10 @@
});
expect(host).toBeDefined();
- expect(fx.harness.getTools().map((t) => t.name)).toContain(probe);
+ expect(fx.harness.getTools().map((t) => t.name)).not.toContain(probe);
});
- it("loads project <cwd>/.agents/extensions by default", async () => {
+ it("does not load project <cwd>/.agents/extensions when untrusted", async () => {
fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
const probe = `agents_probe_${randomUUID().replace(/-/g, "")}`;
const projectExtDir = join(fx.cwd, ".agents", "extensions");
@@ -111,6 +111,31 @@
});
expect(host).toBeDefined();
- expect(fx.harness.getTools().map((t) => t.name)).toContain(probe);
+ expect(fx.harness.getTools().map((t) => t.name)).not.toContain(probe);
});
+
+ it("loads project-local extension directories with --trust-extensions", async () => {
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const agentsProbe = `agents_probe_${randomUUID().replace(/-/g, "")}`;
+ const piProbe = `pi_probe_${randomUUID().replace(/-/g, "")}`;
+ const agentsExtDir = join(fx.cwd, ".agents", "extensions");
+ mkdirSync(agentsExtDir, { recursive: true });
+ writeFileSync(join(agentsExtDir, "agents-probe.ts"), makeToolExtension(agentsProbe));
+ const piExtDir = join(fx.cwd, ".pi", "extensions");
+ mkdirSync(piExtDir, { recursive: true });
+ writeFileSync(join(piExtDir, "pi-probe.ts"), makeToolExtension(piProbe));
+
+ host = await loadHarnessExtensions({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ noExtensions: false,
+ trustExtensions: true,
+ agentDir: tempAgentDir(),
+ });
+
+ expect(host).toBeDefined();
+ expect(fx.harness.getTools().map((t) => t.name)).toContain(piProbe);
+ expect(fx.harness.getTools().map((t) => t.name)).toContain(agentsProbe);
+ });
});
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -38,6 +38,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [makeExtensionDir()],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
await created.load();
@@ -119,6 +120,7 @@
session: fx.session,
cwd: fx.cwd,
configuredPaths: [extDir],
+ projectTrusted: true,
agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
});
host = created;
diff --git a/packages/cli/test/reload-command.test.ts b/packages/cli/test/reload-command.test.ts
--- a/packages/cli/test/reload-command.test.ts
+++ b/packages/cli/test/reload-command.test.ts
@@ -15,23 +15,39 @@
messages: MessageList;
notices: string[];
errors: string[];
+ waitForIdle: ReturnType<typeof vi.fn>;
} {
const messages = new MessageList();
const notices: string[] = [];
const errors: string[] = [];
+ const waitForIdle = vi.fn(async () => {});
vi.spyOn(messages, "addNotice").mockImplementation((text) => void notices.push(text));
vi.spyOn(messages, "addError").mockImplementation((text) => void errors.push(text));
- return { opts: { host } as InteractiveOptions, messages, notices, errors };
+ return {
+ opts: {
+ host,
+ harness: { waitForIdle } as unknown as InteractiveOptions["harness"],
+ } as InteractiveOptions,
+ messages,
+ notices,
+ errors,
+ waitForIdle,
+ };
}
describe("applyReloadCommand (/reload glue)", () => {
it("invokes host.reload() and reports a clean reload", async () => {
const reload = vi.fn(async () => {});
- const host = { reload, loadErrors: [] } as unknown as HarnessExtensionHost;
- const { opts, messages, notices, errors } = setup(host);
+ const host = {
+ reload,
+ loadErrors: [],
+ isDisposed: () => false,
+ } as unknown as HarnessExtensionHost;
+ const { opts, messages, notices, errors, waitForIdle } = setup(host);
await applyReloadCommand(opts, messages);
+ expect(waitForIdle).toHaveBeenCalledTimes(1);
expect(reload).toHaveBeenCalledTimes(1);
expect(notices).toContain("extensions reloaded");
expect(errors).toHaveLength(0);
@@ -42,16 +58,37 @@
const host = {
reload,
loadErrors: [{ path: "/ext/broken.ts", error: "boom" }],
+ isDisposed: () => false,
} as unknown as HarnessExtensionHost;
- const { opts, messages, errors, notices } = setup(host);
+ const { opts, messages, errors, notices, waitForIdle } = setup(host);
await applyReloadCommand(opts, messages);
+ expect(waitForIdle).toHaveBeenCalledTimes(1);
expect(reload).toHaveBeenCalledTimes(1);
expect(errors).toContain("/ext/broken.ts: boom");
expect(notices).not.toContain("extensions reloaded");
});
+ it("reports disabled when reload disposes the host", async () => {
+ let disposed = false;
+ const reload = vi.fn(async () => {
+ disposed = true;
+ });
+ const host = {
+ reload,
+ loadErrors: [],
+ isDisposed: () => disposed,
+ } as unknown as HarnessExtensionHost;
+ const { opts, messages, notices } = setup(host);
+
+ await applyReloadCommand(opts, messages);
+
+ expect(reload).toHaveBeenCalledTimes(1);
+ expect(notices).toContain("extensions were shut down");
+ expect(notices).not.toContain("extensions reloaded");
+ });
+
it("no-ops with a notice when no host is loaded", async () => {
const { opts, messages, notices } = setup(undefined);You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 12 total unresolved issues (including 10 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Extension sendUserMessage rejects unhandled
- Extension sendUserMessage now catches prompt failures and records them as extension_error entries so rejections are handled instead of becoming unhandled promises.
- ✅ Fixed: Extension startup steals first screenshot
- Initial screenshot attachment now keys only off the run-mode first-prompt/resume flags, so extension-startup turns no longer prevent the first user-driven prompt from receiving images.
Or push these changes by commenting:
@cursor push b64dd7022d
Preview (b64dd7022d)
diff --git a/packages/cli/src/action/harness-runner.ts b/packages/cli/src/action/harness-runner.ts
--- a/packages/cli/src/action/harness-runner.ts
+++ b/packages/cli/src/action/harness-runner.ts
@@ -140,23 +140,11 @@
async function maybeInitialScreenshot(opts: HarnessRunOptions): Promise<ImageContent[] | undefined> {
if (opts.skipInitialScreenshot) return undefined;
- const hasPriorTurn = await sessionHasPriorTurn(opts.session);
- if (hasPriorTurn) return undefined;
const png = await captureScreenshot(opts.browserHandle.client, opts.browserHandle.browser.session_id);
if (!png) return undefined;
return [{ type: "image", data: png.toString("base64"), mimeType: "image/png" }];
}
-async function sessionHasPriorTurn(session: Session): Promise<boolean> {
- const entries = await session.getBranch();
- for (const entry of entries) {
- if (entry.type === "message" && (entry.message.role === "user" || entry.message.role === "assistant")) {
- return true;
- }
- }
- return false;
-}
-
function textFromAssistant(message: AssistantMessage): string {
const parts: string[] = [];
for (const block of message.content) {
diff --git a/packages/cli/src/extensions/seams.ts b/packages/cli/src/extensions/seams.ts
--- a/packages/cli/src/extensions/seams.ts
+++ b/packages/cli/src/extensions/seams.ts
@@ -45,7 +45,16 @@
},
sendUserMessage(content): void {
const text = typeof content === "string" ? content : textPartsOf(content);
- void hooks.sendUserMessage(text);
+ void hooks.sendUserMessage(text).catch((error: unknown) => {
+ void session
+ .appendCustomMessageEntry(
+ "extension_error",
+ `sendUserMessage failed: ${errorMessage(error)}`,
+ true,
+ { action: "sendUserMessage" },
+ )
+ .catch(() => {});
+ });
},
appendEntry(customType, data): void {
void session.appendCustomEntry(customType, data);
@@ -166,3 +175,9 @@
.map((part) => part.text ?? "")
.join("");
}
+
+function errorMessage(error: unknown): string {
+ if (error instanceof Error && error.message.trim().length > 0) return error.message;
+ if (typeof error === "string" && error.trim().length > 0) return error;
+ return "unknown error";
+}
diff --git a/packages/cli/src/print.ts b/packages/cli/src/print.ts
--- a/packages/cli/src/print.ts
+++ b/packages/cli/src/print.ts
@@ -94,8 +94,6 @@
async function maybeInitialScreenshot(opts: RunPrintOptions): Promise<ImageContent[] | undefined> {
if (opts.skipInitialScreenshot) return undefined;
- const hasPriorTurn = await sessionHasPriorTurn(opts.session);
- if (hasPriorTurn) return undefined;
const png = await captureScreenshot(opts.browserHandle.client, opts.browserHandle.browser.session_id);
if (!png) return undefined;
return [
@@ -106,13 +104,3 @@
},
];
}
-
-async function sessionHasPriorTurn(session: Session): Promise<boolean> {
- const entries = await session.getBranch();
- for (const entry of entries) {
- if (entry.type === "message" && (entry.message.role === "user" || entry.message.role === "assistant")) {
- return true;
- }
- }
- return false;
-}
diff --git a/packages/cli/src/tui/main.ts b/packages/cli/src/tui/main.ts
--- a/packages/cli/src/tui/main.ts
+++ b/packages/cli/src/tui/main.ts
@@ -451,22 +451,11 @@
): Promise<ImageContent[] | undefined> {
if (firstPromptSent) return undefined;
if (opts.skipInitialScreenshot) return undefined;
- if (await sessionHasPriorTurn(opts.session)) return undefined;
const png = await captureScreenshot(opts.browserHandle.client, opts.browserHandle.browser.session_id);
if (!png) return undefined;
return [{ type: "image", data: png.toString("base64"), mimeType: "image/png" }];
}
-async function sessionHasPriorTurn(session: Session): Promise<boolean> {
- const entries = await session.getBranch();
- for (const entry of entries) {
- if (entry.type === "message" && (entry.message.role === "user" || entry.message.role === "assistant")) {
- return true;
- }
- }
- return false;
-}
-
async function applyModelCommand(
opts: InteractiveOptions,
footer: TelemetryFooter,You can send follow-ups to the cloud agent here.
rgarcia
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 7 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: /reload succeeds during in-flight reload
HarnessExtensionHost.reload()now waits on the current in-flight reload and drains any queued follow-up reload before resolving concurrent callers like/reload.
- ✅ Fixed: Cleanup races idle extension reload
- CLI command cleanup now awaits
host.drainPendingReload()beforehost.dispose()and browser close, preventing teardown from racing a bridge-started idle reload.
- CLI command cleanup now awaits
- ✅ Fixed: Queued reload skipped after await
drainPendingReload()now loops after awaitingpendingReloadand re-checksreloadRequested, so reloads latched during an in-flight pass are processed immediately.
Or push these changes by commenting:
@cursor push ec23eaae08
Preview (ec23eaae08)
diff --git a/packages/cli/src/cli-harness.ts b/packages/cli/src/cli-harness.ts
--- a/packages/cli/src/cli-harness.ts
+++ b/packages/cli/src/cli-harness.ts
@@ -524,8 +524,14 @@
} finally {
// Dispose before closing the handle: extensions receive session_shutdown
// during dispose and may call back into the harness while the browser lives.
- // Separate try blocks so a throwing extension shutdown never skips close().
+ // Drain queued reloads first so teardown can't race a bridge-started reload.
+ // Separate try blocks so one cleanup failure never skips the rest.
try {
+ await runtime.host?.drainPendingReload();
+ } catch (err) {
+ stderr.write(`[cua] cleanup warning: ${(err as Error).message}\n`);
+ }
+ try {
await runtime.host?.dispose();
} catch (err) {
stderr.write(`[cua] cleanup warning: ${(err as Error).message}\n`);
@@ -565,6 +571,11 @@
});
} finally {
try {
+ await runtime.host?.drainPendingReload();
+ } catch (err) {
+ stderr.write(`[cua] cleanup warning: ${(err as Error).message}\n`);
+ }
+ try {
await runtime.host?.dispose();
} catch (err) {
stderr.write(`[cua] cleanup warning: ${(err as Error).message}\n`);
@@ -601,6 +612,11 @@
return emitCompact(res);
} finally {
try {
+ await runtime.host?.drainPendingReload();
+ } catch (err) {
+ stderr.write(`[cua] cleanup warning: ${(err as Error).message}\n`);
+ }
+ try {
await runtime.host?.dispose();
} catch (err) {
stderr.write(`[cua] cleanup warning: ${(err as Error).message}\n`);
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -145,6 +145,8 @@
private reloading = false;
/** Set by `write_extension`; drained into a single reload at the next idle boundary. */
private reloadRequested = false;
+ /** Current reload invocation, so concurrent callers can await completion. */
+ private inFlightReload: Promise<void> | undefined;
/** The in-flight queued reload, so a drain can await one already running. */
private pendingReload: Promise<void> | undefined;
/** Sticky shutdown request raised by `ctx.shutdown()` or owner disposal. */
@@ -219,43 +221,53 @@
if (this.disposed) return;
// Reentrancy guard: a reload triggered (e.g. via ctx.reload()) while one is
// in flight must not run concurrently and double-tear-down the bridge. Re-arm
- // the latch so the in-flight reload's drain picks up the newer request.
+ // the latch and wait for the in-flight reload before draining the follow-up.
if (this.reloading) {
this.reloadRequested = true;
+ if (this.inFlightReload) await this.inFlightReload;
+ await this.drainPendingReload();
return;
}
- this.reloading = true;
- try {
- // Don't swap the runner/bridge mid-turn: wait for the agent loop to be
- // idle first. All callers reach here at or after an idle point (the
- // /reload command runs between prompts; the self-extend drain is scheduled
- // off-stack at agent_end), so this resolves promptly and cannot deadlock
- // on an awaited-in-loop reload.
- await this.harness.waitForIdle();
- if (this.disposed) return;
- const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
- await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
- if (await this.disposeIfShutdownRequested()) return;
- this.teardownBridge?.();
- this.teardownBridge = undefined;
+ const inFlight = (async () => {
+ this.reloading = true;
try {
- await this.buildRunner();
+ // Don't swap the runner/bridge mid-turn: wait for the agent loop to be
+ // idle first. All callers reach here at or after an idle point (the
+ // /reload command runs between prompts; the self-extend drain is scheduled
+ // off-stack at agent_end), so this resolves promptly and cannot deadlock
+ // on an awaited-in-loop reload.
+ await this.harness.waitForIdle();
+ if (this.disposed) return;
+ const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
+ await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
if (await this.disposeIfShutdownRequested()) return;
- for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
+ this.teardownBridge?.();
+ this.teardownBridge = undefined;
+ try {
+ await this.buildRunner();
+ if (await this.disposeIfShutdownRequested()) return;
+ for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
- await this.reapplyTools();
- if (await this.disposeIfShutdownRequested()) return;
- this.installBridge();
- await this.runner?.emit({ type: "session_start", reason: "reload" });
- } catch (error) {
- // A failed rebuild left the bridge torn down; reinstall it over the
- // current runner so extension events keep flowing rather than going
- // silently dark for the rest of the session.
- if (this.runner && !this.teardownBridge) this.installBridge();
- throw error;
+ await this.reapplyTools();
+ if (await this.disposeIfShutdownRequested()) return;
+ this.installBridge();
+ await this.runner?.emit({ type: "session_start", reason: "reload" });
+ } catch (error) {
+ // A failed rebuild left the bridge torn down; reinstall it over the
+ // current runner so extension events keep flowing rather than going
+ // silently dark for the rest of the session.
+ if (this.runner && !this.teardownBridge) this.installBridge();
+ throw error;
+ }
+ } finally {
+ this.reloading = false;
}
+ })();
+ this.inFlightReload = inFlight;
+ try {
+ await inFlight;
} finally {
- this.reloading = false;
+ if (this.inFlightReload === inFlight) this.inFlightReload = undefined;
}
// Honor a shutdown requested during the final emit, after `reloading` cleared.
if (this.shutdownRequested) await this.dispose();
@@ -419,21 +431,24 @@
* latch for the next boundary. `disposed` makes this a no-op during teardown.
*/
async drainPendingReload(): Promise<void> {
- // Await a reload already running (the bridge fires this fire-and-forget, so
- // a caller that awaits the drain — e.g. a test asserting the new tool is
- // live — must observe that reload settle).
- if (this.pendingReload) {
- await this.pendingReload;
- return;
+ while (true) {
+ // Await a reload already running (the bridge fires this fire-and-forget,
+ // so a caller that awaits the drain — e.g. a test asserting the new tool
+ // is live — must observe that reload settle).
+ if (this.pendingReload) {
+ await this.pendingReload;
+ continue;
+ }
+ if (!this.reloadRequested || this.reloading || this.disposed) return;
+ this.reloadRequested = false;
+ const queuedReload = this.reload();
+ this.pendingReload = queuedReload;
+ try {
+ await queuedReload;
+ } finally {
+ if (this.pendingReload === queuedReload) this.pendingReload = undefined;
+ }
}
- if (!this.reloadRequested || this.reloading || this.disposed) return;
- this.reloadRequested = false;
- this.pendingReload = this.reload();
- try {
- await this.pendingReload;
- } finally {
- this.pendingReload = undefined;
- }
}
/**
diff --git a/packages/cli/test/extension-reload.test.ts b/packages/cli/test/extension-reload.test.ts
--- a/packages/cli/test/extension-reload.test.ts
+++ b/packages/cli/test/extension-reload.test.ts
@@ -1,4 +1,4 @@
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
import { mkdtempSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
@@ -80,4 +80,48 @@
expect(host!.loadErrors.length).toBeGreaterThan(0);
});
+
+ it("waits for an in-flight reload and drains the queued follow-up", async () => {
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ const extFile = join(extDir, "learned.ts");
+ writeFileSync(extFile, makeToolExtension("alpha_tool"));
+
+ host = await loadHarnessExtensions({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ noExtensions: false,
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ configuredPaths: [extDir],
+ });
+ expect(host).toBeDefined();
+
+ const realWaitForIdle = fx.harness.waitForIdle.bind(fx.harness);
+ let releaseFirstWait!: () => void;
+ const firstWait = new Promise<void>((resolve) => {
+ releaseFirstWait = resolve;
+ });
+ let waitCalls = 0;
+ const waitSpy = vi.spyOn(fx.harness, "waitForIdle").mockImplementation(async () => {
+ waitCalls += 1;
+ if (waitCalls === 1) {
+ await firstWait;
+ return;
+ }
+ await realWaitForIdle();
+ });
+
+ const firstReload = host!.reload();
+ let secondResolved = false;
+ const secondReload = host!.reload().then(() => {
+ secondResolved = true;
+ });
+ await Promise.resolve();
+ expect(secondResolved).toBe(false);
+
+ releaseFirstWait();
+ await Promise.all([firstReload, secondReload]);
+ expect(waitSpy).toHaveBeenCalledTimes(2);
+ });
});
diff --git a/packages/cli/test/self-extend.test.ts b/packages/cli/test/self-extend.test.ts
--- a/packages/cli/test/self-extend.test.ts
+++ b/packages/cli/test/self-extend.test.ts
@@ -1,4 +1,4 @@
-import { afterEach, describe, expect, it } from "vitest";
+import { afterEach, describe, expect, it, vi } from "vitest";
import { existsSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
@@ -129,6 +129,31 @@
expect(toolNames(fx.harness)).not.toContain("write_extension");
});
+ it("drains follow-up reload requests queued during an in-flight drain", async () => {
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ host = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ selfExtend: true,
+ });
+ await host.load();
+
+ const internals = host as unknown as { reloadRequested: boolean };
+ internals.reloadRequested = true;
+ let reloadCalls = 0;
+ const reloadSpy = vi.spyOn(host, "reload").mockImplementation(async () => {
+ reloadCalls += 1;
+ if (reloadCalls === 1) internals.reloadRequested = true;
+ });
+
+ await host.drainPendingReload();
+ expect(reloadSpy).toHaveBeenCalledTimes(2);
+ });
+
it("writes the file and reports a valid trial load without a mid-turn swap", async () => {
const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
fx = await buildTestHarness({You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
There are 6 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Load latch set before completion
load()now resetsloadedin a failure path so a startup error no longer leaves the same host instance permanently latched as initialized.
- ✅ Fixed: Queued reload dropped on failure
drainPendingReload()now re-armsreloadRequestedwhenreload()throws, allowing the next idle drain to retry queued reloads.
Or push these changes by commenting:
@cursor push 8ed4232155
Preview (8ed4232155)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -199,13 +199,20 @@
if (this.disposed) throw new Error("cannot load a disposed extension host");
if (this.loaded) return;
this.loaded = true;
- await this.buildRunner();
- await this.reapplyTools();
- this.installBridge();
- await this.runner?.emit({ type: "session_start", reason: "startup" });
- // An extension that calls ctx.shutdown() during session_start disposes via
- // requestShutdown; honor it so load doesn't resolve a torn-down host as ready.
- if (this.shutdownRequested) await this.dispose();
+ try {
+ await this.buildRunner();
+ await this.reapplyTools();
+ this.installBridge();
+ await this.runner?.emit({ type: "session_start", reason: "startup" });
+ // An extension that calls ctx.shutdown() during session_start disposes via
+ // requestShutdown; honor it so load doesn't resolve a torn-down host as ready.
+ if (this.shutdownRequested) await this.dispose();
+ } catch (error) {
+ // Keep load() retryable on the same host when startup fails before the
+ // host is disposed.
+ if (!this.disposed) this.loaded = false;
+ throw error;
+ }
}
/**
@@ -435,7 +442,11 @@
}
if (!this.reloadRequested || this.reloading || this.disposed) return;
this.reloadRequested = false;
- this.pendingReload = this.reload();
+ this.pendingReload = this.reload().catch((error) => {
+ // Retry on the next idle drain when a queued reload fails.
+ if (!this.disposed) this.reloadRequested = true;
+ throw error;
+ });
try {
await this.pendingReload;
} finally {You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 5 potential issues.
There are 9 total unresolved issues (including 4 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 5 issues found in the latest run.
- ✅ Fixed: Browser leak before extension load
setupHarnessRuntime()now wraps all post-provision setup in a catch path that always closes the provisioned handle on any throw before returning.
- ✅ Fixed: Idle reload skipped on emit error
- The
agent_endbridge path now queuesdrainPendingReloadin afinallyblock so reload scheduling still happens even ifrunner.emitthrows.
- The
- ✅ Fixed: Failed reload desyncs tool list
- Tool bookkeeping now updates
extensionToolsonly afterharness.setToolssucceeds and reload restores the prior runner state when failure occurs before bridge swap.
- Tool bookkeeping now updates
- ✅ Fixed: Duplicate extension tools crash load
reapplyTools()now performs an extension-to-extension name dedupe pass and drops duplicate tool registrations before applying the final tool list.
- ✅ Fixed: Reload overlaps next prompt
- Reload now keeps the old bridge active until the new generation is fully prepared, adds idle waits around rebuild/reapply, and swaps listeners without a bridge-down window.
Or push these changes by commenting:
@cursor push a32702768c
Preview (a32702768c)
diff --git a/packages/cli/src/cli-harness.ts b/packages/cli/src/cli-harness.ts
--- a/packages/cli/src/cli-harness.ts
+++ b/packages/cli/src/cli-harness.ts
@@ -387,69 +387,65 @@
});
const provisioned = await provisionForFlags(flags, auth);
- const repo = createSessionRepo(flags.sessionDir);
+ const handle = provisioned.handle;
+ try {
+ const repo = createSessionRepo(flags.sessionDir);
- const skipDisk = opts.skipDiskSession === true && !hasExplicitSessionFlag(flags);
- const resolved = skipDisk ? undefined : await resolveSession(repo, cwd, flags, provisioned.named);
+ const skipDisk = opts.skipDiskSession === true && !hasExplicitSessionFlag(flags);
+ const resolved = skipDisk ? undefined : await resolveSession(repo, cwd, flags, provisioned.named);
- let inMemorySession: Session | undefined;
- if (!resolved) {
- const memRepo = new InMemorySessionRepo();
- inMemorySession = await memRepo.create();
- }
+ let inMemorySession: Session | undefined;
+ if (!resolved) {
+ const memRepo = new InMemorySessionRepo();
+ inMemorySession = await memRepo.create();
+ }
- const session = resolved?.session ?? inMemorySession!;
- const { provider } = parseCuaModelRef(auth.modelRef);
+ const session = resolved?.session ?? inMemorySession!;
+ const { provider } = parseCuaModelRef(auth.modelRef);
- if (resolved) {
- await appendBrowserEntry(session, {
- sessionId: provisioned.handle.browser.session_id,
- liveUrl: provisioned.handle.browser.browser_live_view_url,
- profileId: provisioned.handle.profileId,
- createdAt: Date.now(),
- });
- if (provisioned.named) {
- await recordTranscriptPath(provisioned.named.name, resolved.transcriptPath);
+ if (resolved) {
+ await appendBrowserEntry(session, {
+ sessionId: provisioned.handle.browser.session_id,
+ liveUrl: provisioned.handle.browser.browser_live_view_url,
+ profileId: provisioned.handle.profileId,
+ createdAt: Date.now(),
+ });
+ if (provisioned.named) {
+ await recordTranscriptPath(provisioned.named.name, resolved.transcriptPath);
+ }
+ if (flags.verbose) {
+ stderr.write(`[cua] session=${resolved.transcriptPath}\n`);
+ if (resolved.resumed) stderr.write("[cua] resumed prior session into fresh browser\n");
+ }
}
- if (flags.verbose) {
- stderr.write(`[cua] session=${resolved.transcriptPath}\n`);
- if (resolved.resumed) stderr.write("[cua] resumed prior session into fresh browser\n");
- }
- }
- const thinkingLevel = mapThinkingLevel(flags.thinking);
- const baseUrlOverride = providerBaseUrlOverride(provider);
- const harness = buildCuaHarness({
- cwd,
- client: provisioned.handle.client,
- browser: provisioned.handle.browser,
- session,
- model: auth.modelRef,
- skills,
- contextFiles,
- thinkingLevel,
- playwright: flags.playwright,
- modelBaseUrl: baseUrlOverride,
- });
+ const thinkingLevel = mapThinkingLevel(flags.thinking);
+ const baseUrlOverride = providerBaseUrlOverride(provider);
+ const harness = buildCuaHarness({
+ cwd,
+ client: provisioned.handle.client,
+ browser: provisioned.handle.browser,
+ session,
+ model: auth.modelRef,
+ skills,
+ contextFiles,
+ thinkingLevel,
+ playwright: flags.playwright,
+ modelBaseUrl: baseUrlOverride,
+ });
- const handle = provisioned.handle;
- // Raw capture: the host gates this behind the session's prior-turn state, so
- // it must not re-apply the CLI's own first-prompt guard.
- const initialScreenshot = async (): Promise<ImageContent[] | undefined> => {
- const png = await captureScreenshot(handle.client, handle.browser.session_id);
- return png ? [{ type: "image", data: png.toString("base64"), mimeType: "image/png" }] : undefined;
- };
- // Decide the first-turn screenshot before extensions load: a resumed session
- // already has turns, and capturing it now (rather than re-reading the live
- // transcript at prompt time) keeps an extension's startup sendUserMessage from
- // flipping the check and leaving the user's real first prompt without it.
- const skipInitialScreenshot = resolved?.resumed === true || (await sessionHasPriorTurn(session));
- // A throwing extension load (e.g. a tool name colliding with a base tool)
- // must not leak the already-provisioned browser handle: the caller's finally
- // only runs once this returns, so close the handle here before rethrowing.
- let host: HarnessExtensionHost | undefined;
- try {
- host = await loadHarnessExtensions({
+ // Raw capture: the host gates this behind the session's prior-turn state, so
+ // it must not re-apply the CLI's own first-prompt guard.
+ const initialScreenshot = async (): Promise<ImageContent[] | undefined> => {
+ const png = await captureScreenshot(handle.client, handle.browser.session_id);
+ return png ? [{ type: "image", data: png.toString("base64"), mimeType: "image/png" }] : undefined;
+ };
+ // Decide the first-turn screenshot before extensions load: a resumed session
+ // already has turns, and capturing it now (rather than re-reading the live
+ // transcript at prompt time) keeps an extension's startup sendUserMessage from
+ // flipping the check and leaving the user's real first prompt without it.
+ const skipInitialScreenshot = resolved?.resumed === true || (await sessionHasPriorTurn(session));
+ const host = await loadHarnessExtensions({
harness,
session,
cwd,
@@ -457,23 +453,23 @@
initialScreenshot,
selfExtend: flags.selfExtend,
});
+
+ return {
+ handle: provisioned.handle,
+ resolved,
+ session,
+ skills,
+ contextFiles,
+ harness,
+ provider,
+ modelRef: auth.modelRef,
+ skipInitialScreenshot,
+ host,
+ };
} catch (err) {
- await provisioned.handle.close().catch(() => {});
+ await handle.close().catch(() => {});
throw err;
}
-
- return {
- handle: provisioned.handle,
- resolved,
- session,
- skills,
- contextFiles,
- harness,
- provider,
- modelRef: auth.modelRef,
- skipInitialScreenshot,
- host,
- };
}
async function sessionHasPriorTurn(session: Session): Promise<boolean> {
diff --git a/packages/cli/src/extensions/bridge.ts b/packages/cli/src/extensions/bridge.ts
--- a/packages/cli/src/extensions/bridge.ts
+++ b/packages/cli/src/extensions/bridge.ts
@@ -42,17 +42,20 @@
break;
case "agent_end":
state.isIdle = true;
- await runner.emit({ type: "agent_end", messages: event.messages });
- // A reload requested by a tool during this run runs here, at the
- // only true idle boundary (agent_end is where the harness sets
- // phase=idle). It is scheduled off-stack rather than awaited: this
- // listener runs inside the harness's synchronous event dispatch, so
- // awaiting reload() here would tear down this very listener
- // mid-dispatch and swap the runner out from under the in-flight loop.
- // Deferring is safe because reload() immediately awaits async I/O
- // (re-discovering extensions), yielding off the dispatch before any
- // teardown or runner swap.
- queueMicrotask(() => drainPendingReload());
+ try {
+ await runner.emit({ type: "agent_end", messages: event.messages });
+ } finally {
+ // A reload requested by a tool during this run runs here, at the
+ // only true idle boundary (agent_end is where the harness sets
+ // phase=idle). It is scheduled off-stack rather than awaited: this
+ // listener runs inside the harness's synchronous event dispatch, so
+ // awaiting reload() here would tear down this very listener
+ // mid-dispatch and swap the runner out from under the in-flight loop.
+ // Deferring is safe because reload() immediately awaits async I/O
+ // (re-discovering extensions), yielding off the dispatch before any
+ // teardown or runner swap.
+ queueMicrotask(() => drainPendingReload());
+ }
break;
case "turn_start":
await runner.emit({ type: "turn_start", turnIndex: state.turnIndex, timestamp: Date.now() });
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -265,24 +265,38 @@
// cannot deadlock on an awaited-in-loop reload.
await this.harness.waitForIdle();
if (this.disposed) return "disposed";
- const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
- await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
+ const previousRunner = this.runner;
+ const previousBridgeTeardown = this.teardownBridge;
+ const flags = previousRunner?.getFlagValues() ?? new Map<string, boolean | string>();
+ await previousRunner?.emit({ type: "session_shutdown", reason: "reload" });
if (await this.disposeIfShutdownRequested()) return "disposed";
- this.teardownBridge?.();
- this.teardownBridge = undefined;
try {
await this.buildRunner();
if (await this.disposeIfShutdownRequested()) return "disposed";
for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
-
+ // A new prompt can start while reload is rebuilding; finish any such
+ // turn before touching the live tool list.
+ await this.harness.waitForIdle();
+ if (this.disposed) return "disposed";
await this.reapplyTools();
if (await this.disposeIfShutdownRequested()) return "disposed";
+ // Keep event forwarding live until the replacement generation is
+ // fully ready, then swap bridge listeners without a bridge-down gap.
+ await this.harness.waitForIdle();
+ if (this.disposed) return "disposed";
+ previousBridgeTeardown?.();
+ if (this.teardownBridge === previousBridgeTeardown) {
+ this.teardownBridge = undefined;
+ }
this.installBridge();
await this.runner?.emit({ type: "session_start", reason: "reload" });
} catch (error) {
- // A failed rebuild left the bridge torn down; reinstall it over the
- // current runner so extension events keep flowing rather than going
- // silently dark for the rest of the session.
+ // If reload fails before bridge swap, keep the prior generation as
+ // authoritative so host bookkeeping still matches the harness.
+ if (this.teardownBridge === previousBridgeTeardown) {
+ this.runner = previousRunner;
+ }
+ // A failed rebuild must not leave the host bridge-less.
if (this.runner && !this.teardownBridge) this.installBridge();
throw error;
}
@@ -388,6 +402,7 @@
// dropped rather than allowed to crash `setTools`, which throws on
// duplicate names.
const hostNames = new Set(this.hostTools.map((tool) => tool.name));
+ const seenExtensionNames = new Set<string>();
const nextExtensionTools = wrapRegisteredTools(
this.runner.getAllRegisteredTools(),
this.runner,
@@ -401,6 +416,16 @@
this.loadErrors.push({ path: tool.name, error });
}
return false;
+ }).filter((tool) => {
+ if (!seenExtensionNames.has(tool.name)) {
+ seenExtensionNames.add(tool.name);
+ return true;
+ }
+ const error = `extension tool "${tool.name}" is duplicated across extensions and was dropped`;
+ if (!this.loadErrors.some((e) => e.path === tool.name && e.error === error)) {
+ this.loadErrors.push({ path: tool.name, error });
+ }
+ return false;
});
const extensionNames = new Set(nextExtensionTools.map((tool) => tool.name));
const base = this.harness
@@ -423,10 +448,10 @@
for (const name of extensionNames) {
if (!this.inactiveExtensionTools.has(name)) activeNames.add(name);
}
- this.extensionTools = nextExtensionTools;
this.applyingTools = true;
try {
await this.harness.setTools(final, [...activeNames]);
+ this.extensionTools = nextExtensionTools;
} finally {
this.applyingTools = false;
}
diff --git a/packages/cli/test/bridge.test.ts b/packages/cli/test/bridge.test.ts
new file mode 100644
--- /dev/null
+++ b/packages/cli/test/bridge.test.ts
@@ -1,0 +1,52 @@
+import type { AgentHarness } from "@onkernel/cua-agent";
+import type { ExtensionRunner } from "@earendil-works/pi-coding-agent";
+import { describe, expect, it, vi } from "vitest";
+import { installBridge, type BridgeState } from "../src/extensions/bridge";
+
+type HarnessListener = (event: { type: string; [key: string]: unknown }) => Promise<void> | void;
+
+class FakeHarness {
+ private listener: HarnessListener | undefined;
+
+ subscribe(next: HarnessListener): () => void {
+ this.listener = next;
+ return () => {
+ if (this.listener === next) this.listener = undefined;
+ };
+ }
+
+ on(_type: string, _next: (...args: unknown[]) => unknown): () => void {
+ return () => {};
+ }
+
+ async emit(event: { type: string; [key: string]: unknown }): Promise<void> {
+ if (!this.listener) return;
+ await this.listener(event);
+ }
+}
+
+describe("installBridge", () => {
+ it("still schedules queued reload drain when agent_end emit throws", async () => {
+ const harness = new FakeHarness();
+ const runner = {
+ emit: vi.fn(async (event: { type: string }) => {
+ if (event.type === "agent_end") throw new Error("agent_end failed");
+ }),
+ } as unknown as ExtensionRunner;
+ const state: BridgeState = { turnIndex: 0, isIdle: false };
+ const drainPendingReload = vi.fn();
+ const reapplyTools = vi.fn(async () => {});
+
+ installBridge(
+ harness as unknown as AgentHarness,
+ runner,
+ state,
+ reapplyTools,
+ drainPendingReload,
+ );
+
+ await expect(harness.emit({ type: "agent_end", messages: [] })).rejects.toThrow(/agent_end failed/);
+ await Promise.resolve();
+ expect(drainPendingReload).toHaveBeenCalledTimes(1);
+ });
+});
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -133,6 +133,64 @@
expect(toolNames).not.toContain("alpha_tool");
});
+ it("keeps tool bookkeeping coherent after a failed reload setTools pass", async () => {
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ const extFile = join(extDir, "learned.ts");
+ writeFileSync(extFile, makeToolExtension("alpha_tool"));
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const created = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ });
+ host = created;
+ await created.load();
+ expect(fx.harness.getTools().map((tool) => tool.name)).toContain("alpha_tool");
+
+ writeFileSync(extFile, makeToolExtension("beta_tool"));
+ const realSetTools = fx.harness.setTools.bind(fx.harness);
+ let failOnce = true;
+ fx.harness.setTools = (async (tools, activeToolNames) => {
+ if (failOnce) {
+ failOnce = false;
+ throw new Error("setTools boom");
+ }
+ return realSetTools(tools, activeToolNames);
+ }) as typeof fx.harness.setTools;
+
+ await expect(created.reload()).rejects.toThrow(/setTools boom/);
+ expect(fx.harness.getTools().map((tool) => tool.name)).toContain("alpha_tool");
+ expect(fx.harness.getTools().map((tool) => tool.name)).not.toContain("beta_tool");
+
+ await created.reload();
+ const names = fx.harness.getTools().map((tool) => tool.name);
+ expect(names).toContain("beta_tool");
+ expect(names).not.toContain("alpha_tool");
+ });
+
+ it("drops duplicate tool names registered by multiple extensions", async () => {
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ writeFileSync(join(extDir, "one.ts"), makeToolExtension("dup_tool"));
+ writeFileSync(join(extDir, "two.ts"), makeToolExtension("dup_tool"));
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const created = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ });
+ host = created;
+ await created.load();
+
+ const names = fx.harness.getTools().map((tool) => tool.name);
+ expect(names.filter((name) => name === "dup_tool")).toHaveLength(1);
+ // No duplicate reaches the harness tool list, so load/reload cannot crash.
+ expect(created.loadErrors.every((e) => e.path !== "<reload>")).toBe(true);
+ });
+
it("throws when load() is called after dispose", async () => {
const created = await loadHost();
await created.dispose();
diff --git a/packages/cli/test/self-extend.test.ts b/packages/cli/test/self-extend.test.ts
--- a/packages/cli/test/self-extend.test.ts
+++ b/packages/cli/test/self-extend.test.ts
@@ -40,6 +40,24 @@
/** A module that fails to parse — stands in for a broken authoring attempt. */
const BROKEN_EXTENSION = "export default function ( { // syntactically broken\n";
+/** Tracks whether bridge-forwarded agent_start keeps flowing during reloads. */
+const START_COUNTER_EXTENSION = [
+ "let starts = 0;",
+ "export default function (pi) {",
+ ' pi.on("agent_start", () => {',
+ " starts += 1;",
+ " });",
+ " pi.registerTool({",
+ ' name: "start_counter",',
+ ' label: "start counter",',
+ ' description: "report the bridged agent_start count",',
+ ' parameters: { type: "object", properties: {}, additionalProperties: false },',
+ ' async execute() { return { content: [{ type: "text", text: `starts=${starts}` }], details: {} }; },',
+ " });",
+ "}",
+ "",
+].join("\n");
+
interface WriteExtensionResult {
content: Array<{ type: string; text?: string }>;
details: {
@@ -319,6 +337,73 @@
expect(sawAuthoredCall).toBe(true);
});
+ it("keeps bridge forwarding live while queued reload waits behind the next prompt", async () => {
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ writeFileSync(join(extDir, "counter.ts"), START_COUNTER_EXTENSION);
+ fx = await buildTestHarness({
+ turns: [
+ {
+ steps: [
+ {
+ type: "tool_call",
+ toolName: "write_extension",
+ args: { filename: "authored_tool.ts", code: makeToolExtension("authored_tool") },
+ },
+ ],
+ },
+ { steps: [{ type: "text", text: "authored" }] },
+ { steps: [{ type: "tool_call", toolName: "start_counter", args: {} }] },
+ { steps: [{ type: "text", text: "counted" }] },
+ ],
+ });
+
+ host = new HarnessExtensionHost({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ configuredPaths: [extDir],
+ agentDir: mkdtempSync(join(tmpdir(), "cua-agentdir-")),
+ selfExtend: true,
+ });
+ await host.load();
+
+ const hostWithPrivate = host as unknown as { buildRunner: () => Promise<void> };
+ const realBuildRunner = hostWithPrivate.buildRunner.bind(hostWithPrivate);
+ let releaseReloadBuild!: () => void;
+ const reloadBuildGate = new Promise<void>((resolve) => {
+ releaseReloadBuild = resolve;
+ });
+ let sawReloadBuild!: () => void;
+ const reloadBuildSeen = new Promise<void>((resolve) => {
+ sawReloadBuild = resolve;
+ });
+ let holdNextBuild = true;
+ hostWithPrivate.buildRunner = async () => {
+ if (holdNextBuild) {
+ holdNextBuild = false;
+ sawReloadBuild();
+ await reloadBuildGate;
+ }
+ return realBuildRunner();
+ };
+
+ let startCounterText = "";
+ fx.harness.subscribe((event) => {
+ if (event.type === "tool_execution_end" && event.toolName === "start_counter") {
+ const content = (event.result as { content: Array<{ type: string; text?: string }> }).content;
+ startCounterText = content.map((part) => part.text ?? "").join("");
+ }
+ });
+
+ await fx.harness.prompt("author it");
+ await reloadBuildSeen;
+ await fx.harness.prompt("count starts");
+ releaseReloadBuild();
+ await host.drainPendingReload();
+
+ expect(startCounterText).toContain("starts=2");
+ }, 5000);
+
it("drains a reload latched while another reload is in flight", async () => {
const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
const extFile = join(extDir, "learned.ts");You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Dispose leaves extension tools registered
- disposeNow now removes host/extension-managed tools from the harness and clears extension tool state so shutdown leaves only base tools active.
Or push these changes by commenting:
@cursor push 0b26eec44d
Preview (0b26eec44d)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -341,6 +341,21 @@
this.teardownBridge?.();
this.teardownBridge = undefined;
await this.runner?.emit({ type: "session_shutdown", reason: "quit" });
+ const managedNames = new Set([
+ ...this.hostTools.map((tool) => tool.name),
+ ...this.extensionTools.map((tool) => tool.name),
+ ]);
+ if (managedNames.size > 0) {
+ const baseTools = this.harness.getTools().filter((tool) => !managedNames.has(tool.name));
+ const baseNames = new Set(baseTools.map((tool) => tool.name));
+ const activeBaseNames = this.harness
+ .getActiveTools()
+ .map((tool) => tool.name)
+ .filter((name) => baseNames.has(name));
+ await this.harness.setTools(baseTools, activeBaseNames);
+ }
+ this.extensionTools = [];
+ this.inactiveExtensionTools.clear();
this.runner = undefined;
}
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -139,6 +139,16 @@
await expect(created.load()).rejects.toThrow(/disposed/);
});
+ it("removes extension tools from the harness on dispose", async () => {
+ const created = await loadHost();
+ expect(fx!.harness.getTools().map((tool) => tool.name)).toContain("click_visual");
+ await created.dispose();
+ const tools = fx!.harness.getTools().map((tool) => tool.name);
+ const active = fx!.harness.getActiveTools().map((tool) => tool.name);
+ expect(tools).not.toContain("click_visual");
+ expect(active).not.toContain("click_visual");
+ });
+
it("ignores a second load() rather than stacking a duplicate registration", async () => {
const created = await loadHost();
const count = () =>You can send follow-ups to the cloud agent here.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
There are 5 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Failed extension load leaves bridge
load()now catches startup failures and tears down the partially wired host (bridge/tools/runner) before rethrowing so failed loads do not leak harness wiring.
- ✅ Fixed: Invalid write still queues reload
write_extensionnow setsreloadRequestedonly when the trial load is valid, preventing broken authored files from scheduling unnecessary reload cycles.
- ✅ Fixed: Dispose skips in-flight manual reload
- Manual reloads are now tracked as
activeReloadanddispose()awaits both queued and manual in-flight reload promises before teardown.
- Manual reloads are now tracked as
Or push these changes by commenting:
@cursor push 4d6f179469
Preview (4d6f179469)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -157,6 +157,8 @@
private reloadRequested = false;
/** The in-flight queued reload, so a drain can await one already running. */
private pendingReload: Promise<ReloadOutcome> | undefined;
+ /** Any in-flight reload (queued or manual), awaited during dispose. */
+ private activeReload: Promise<ReloadOutcome> | undefined;
/** Sticky shutdown request raised by `ctx.shutdown()` or owner disposal. */
private shutdownRequested = false;
/** Guards `dispose` so `ctx.shutdown()` and an owner call don't double-tear-down. */
@@ -218,21 +220,26 @@
// the supported way to re-discover extensions.
if (this.disposed) throw new Error("cannot load a disposed extension host");
if (this.loaded) return;
- await this.buildRunner();
- await this.reapplyTools();
- this.installBridge();
- await this.runner?.emit({ type: "session_start", reason: "startup" });
- // Mark loaded only after wiring succeeds: an earlier throw leaves `loaded`
- // false so a half-built host (no bridge or tool union) isn't mistaken for
- // ready by a later load() call.
- this.loaded = true;
- // Startup is over: from here an extension sendUserMessage may carry the
- // first-turn screenshot (it can no longer steal it from the user's first
- // prompt, which the CLI captured before extensions loaded).
- this.startedUp = true;
- // An extension that calls ctx.shutdown() during session_start disposes via
- // requestShutdown; honor it so load doesn't resolve a torn-down host as ready.
- if (this.shutdownRequested) await this.dispose();
+ try {
+ await this.buildRunner();
+ await this.reapplyTools();
+ this.installBridge();
+ await this.runner?.emit({ type: "session_start", reason: "startup" });
+ // Mark loaded only after wiring succeeds.
+ this.loaded = true;
+ // Startup is over: from here an extension sendUserMessage may carry the
+ // first-turn screenshot (it can no longer steal it from the user's first
+ // prompt, which the CLI captured before extensions loaded).
+ this.startedUp = true;
+ // An extension that calls ctx.shutdown() during session_start disposes via
+ // requestShutdown; honor it so load doesn't resolve a torn-down host as ready.
+ if (this.shutdownRequested) await this.dispose();
+ } catch (error) {
+ // load can fail after reapply/bridge install (e.g. during session_start);
+ // tear down the partially wired host before surfacing the startup error.
+ await this.disposeNow().catch(() => {});
+ throw error;
+ }
}
/**
@@ -254,53 +261,61 @@
return "coalesced";
}
this.reloading = true;
- try {
- // Loop so a reload requested mid-reload — via the reentrancy guard above,
- // or a write_extension during this reload — is applied before reload()
- // resolves, instead of waiting for the next idle boundary. The latch is
- // cleared at the top of each pass and re-checked at the bottom.
- do {
- this.reloadRequested = false;
- // Don't swap the runner/bridge mid-turn: wait for the agent loop to be
- // idle first. All callers reach here at or after an idle point (the
- // /reload command runs between prompts; the self-extend drain is
- // scheduled off-stack at agent_end), so this resolves promptly and
- // cannot deadlock on an awaited-in-loop reload.
- await this.harness.waitForIdle();
- if (this.disposed) return "disposed";
- const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
- await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
- if (await this.disposeIfShutdownRequested()) return "disposed";
- this.teardownBridge?.();
- this.teardownBridge = undefined;
- try {
- await this.buildRunner();
+ const run = (async (): Promise<ReloadOutcome> => {
+ try {
+ // Loop so a reload requested mid-reload — via the reentrancy guard above,
+ // or a write_extension during this reload — is applied before reload()
+ // resolves, instead of waiting for the next idle boundary. The latch is
+ // cleared at the top of each pass and re-checked at the bottom.
+ do {
+ this.reloadRequested = false;
+ // Don't swap the runner/bridge mid-turn: wait for the agent loop to be
+ // idle first. All callers reach here at or after an idle point (the
+ // /reload command runs between prompts; the self-extend drain is
+ // scheduled off-stack at agent_end), so this resolves promptly and
+ // cannot deadlock on an awaited-in-loop reload.
+ await this.harness.waitForIdle();
+ if (this.disposed) return "disposed";
+ const flags = this.runner?.getFlagValues() ?? new Map<string, boolean | string>();
+ await this.runner?.emit({ type: "session_shutdown", reason: "reload" });
if (await this.disposeIfShutdownRequested()) return "disposed";
- for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
+ this.teardownBridge?.();
+ this.teardownBridge = undefined;
+ try {
+ await this.buildRunner();
+ if (await this.disposeIfShutdownRequested()) return "disposed";
+ for (const [name, value] of flags) this.runner?.setFlagValue(name, value);
- await this.reapplyTools();
- if (await this.disposeIfShutdownRequested()) return "disposed";
- this.installBridge();
- await this.runner?.emit({ type: "session_start", reason: "reload" });
- } catch (error) {
- // A failed rebuild left the bridge torn down; reinstall it over the
- // current runner so extension events keep flowing rather than going
- // silently dark for the rest of the session.
- if (this.runner && !this.teardownBridge) this.installBridge();
- throw error;
- }
- } while (this.reloadRequested && !this.disposed);
+ await this.reapplyTools();
+ if (await this.disposeIfShutdownRequested()) return "disposed";
+ this.installBridge();
+ await this.runner?.emit({ type: "session_start", reason: "reload" });
+ } catch (error) {
+ // A failed rebuild left the bridge torn down; reinstall it over the
+ // current runner so extension events keep flowing rather than going
+ // silently dark for the rest of the session.
+ if (this.runner && !this.teardownBridge) this.installBridge();
+ throw error;
+ }
+ } while (this.reloadRequested && !this.disposed);
+ } finally {
+ this.reloading = false;
+ }
+ // Honor a shutdown requested during the final emit, after `reloading` cleared.
+ // Still inside reload() (pendingReload may point at us), so tear down via
+ // disposeNow rather than dispose to avoid awaiting our own reload.
+ if (this.shutdownRequested) {
+ await this.disposeNow();
+ return "disposed";
+ }
+ return "reloaded";
+ })();
+ this.activeReload = run;
+ try {
+ return await run;
} finally {
- this.reloading = false;
+ if (this.activeReload === run) this.activeReload = undefined;
}
- // Honor a shutdown requested during the final emit, after `reloading` cleared.
- // Still inside reload() (pendingReload may point at us), so tear down via
- // disposeNow rather than dispose to avoid awaiting our own reload.
- if (this.shutdownRequested) {
- await this.disposeNow();
- return "disposed";
- }
- return "reloaded";
}
/**
@@ -322,8 +337,10 @@
// doesn't race a live reload. Shutdowns raised from inside reload() use
// `disposeNow` directly, since awaiting the running reload from within its own
// call stack would deadlock.
- const inFlight = this.pendingReload;
- if (inFlight) await inFlight.catch(() => {});
+ const inFlight = new Set<Promise<ReloadOutcome>>();
+ if (this.pendingReload) inFlight.add(this.pendingReload);
+ if (this.activeReload) inFlight.add(this.activeReload);
+ for (const reload of inFlight) await reload.catch(() => {});
await this.disposeNow();
}
@@ -345,7 +362,11 @@
// runner binding is gone. (Moot at process exit, but `ctx.shutdown()` from
// an extension disposes the host while the CLI keeps running.)
await this.removeMergedTools();
- await this.runner?.emit({ type: "session_shutdown", reason: "quit" });
+ // A startup failure can dispose before `session_start` finished, so there is
+ // no live session to shut down yet.
+ if (this.startedUp) {
+ await this.runner?.emit({ type: "session_shutdown", reason: "quit" });
+ }
this.runner = undefined;
}
@@ -583,9 +604,8 @@
await writeFile(target, code, "utf8");
const trial = await this.trialLoadExtension(target);
- this.reloadRequested = true;
-
const valid = trial.errors.length === 0 && trial.registeredTools.length > 0;
+ if (valid) this.reloadRequested = true;
const summary = valid
? `wrote ${target}; registered tool(s): ${trial.registeredTools.join(", ")}. it will be available on your next prompt.`
: `wrote ${target} but it did not load: ${
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -185,6 +185,41 @@
expect(new Set(names).size).toBe(names.length);
});
+ it("waits for an in-flight manual reload before disposing", async () => {
+ const created = await loadHost();
+ const originalWaitForIdle = fx!.harness.waitForIdle.bind(fx!.harness);
+ let releaseGate: (() => void) | undefined;
+ const gate = new Promise<void>((resolve) => {
+ releaseGate = resolve;
+ });
+ let firstWaitResolve: (() => void) | undefined;
+ const firstWait = new Promise<void>((resolve) => {
+ firstWaitResolve = resolve;
+ });
+ let first = true;
+ fx!.harness.waitForIdle = async () => {
+ if (first) {
+ first = false;
+ firstWaitResolve?.();
+ await gate;
+ }
+ return originalWaitForIdle();
+ };
+
+ const reload = created.reload();
+ await firstWait;
+ const dispose = created.dispose();
+ const status = await Promise.race([
+ dispose.then(() => "disposed"),
+ new Promise<"pending">((resolve) => setTimeout(() => resolve("pending"), 20)),
+ ]);
+ expect(status).toBe("pending");
+ releaseGate?.();
+ expect(await reload).toBe("disposed");
+ await dispose;
+ expect(created.isDisposed()).toBe(true);
+ });
+
it("does not let a startup extension message consume the first-turn screenshot", async () => {
const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
writeFileSync(join(extDir, "startup-msg.ts"), SEND_ON_STARTUP_EXTENSION);
diff --git a/packages/cli/test/self-extend.test.ts b/packages/cli/test/self-extend.test.ts
--- a/packages/cli/test/self-extend.test.ts
+++ b/packages/cli/test/self-extend.test.ts
@@ -213,6 +213,11 @@
expect(text).toContain("did not load");
// The live toolset is unchanged.
expect(toolNames(fx.harness)).toEqual(before);
+ // Invalid authoring should not queue a full reload; the host's discover
+ // errors stay untouched unless a reload actually runs.
+ expect(host!.loadErrors).toHaveLength(0);
+ const reloadedBrokenFile = await waitFor(() => host!.loadErrors.length > 0, 200);
+ expect(reloadedBrokenFile).toBe(false);
});
it("makes the authored tool callable after the queued reload drains at idle", async () => {You can send follow-ups to the cloud agent here.
rgarcia
force-pushed
the
hypeship/harness-extension-host
branch
from
cd52ac3 to
9eb2e83
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 6 total unresolved issues (including 5 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Failed extension load skips teardown
loadHarnessExtensionsnow disposes theHarnessExtensionHostwhenhost.load()throws so partially applied tools/bridge state are cleaned up before rethrowing.
Or push these changes by commenting:
@cursor push 01d30f3a7a
Preview (01d30f3a7a)
diff --git a/packages/cli/src/extensions/setup.ts b/packages/cli/src/extensions/setup.ts
--- a/packages/cli/src/extensions/setup.ts
+++ b/packages/cli/src/extensions/setup.ts
@@ -38,6 +38,11 @@
initialScreenshot: args.initialScreenshot,
selfExtend: args.selfExtend,
});
- await host.load();
- return host;
+ try {
+ await host.load();
+ return host;
+ } catch (error) {
+ await host.dispose().catch(() => {});
+ throw error;
+ }
}
diff --git a/packages/cli/test/extension-loader.test.ts b/packages/cli/test/extension-loader.test.ts
--- a/packages/cli/test/extension-loader.test.ts
+++ b/packages/cli/test/extension-loader.test.ts
@@ -33,6 +33,24 @@
].join("\n");
}
+function makeFailOnStartupExtension(toolName: string): string {
+ return [
+ "export default function (pi) {",
+ ' pi.on("session_start", (event) => {',
+ ' if (event.reason === "startup") throw new Error("startup boom");',
+ " });",
+ " pi.registerTool({",
+ ` name: ${JSON.stringify(toolName)},`,
+ ` label: ${JSON.stringify(toolName)},`,
+ ` description: ${JSON.stringify(toolName)},`,
+ ' parameters: { type: "object", properties: {}, additionalProperties: false },',
+ ' async execute() { return { content: [{ type: "text", text: "ok" }], details: {} }; },',
+ " });",
+ "}",
+ "",
+ ].join("\n");
+}
+
function tempAgentDir(): string {
return mkdtempSync(join(tmpdir(), "cua-agentdir-"));
}
@@ -56,6 +74,26 @@
expect(fx.harness.getTools().map((t) => t.name)).toContain("loader_probe");
});
+ it("tears down host wiring when extension startup load fails", async () => {
+ fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
+ const baseTools = fx.harness.getTools().map((t) => t.name);
+ const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));
+ writeFileSync(join(extDir, "bad-startup.ts"), makeFailOnStartupExtension("startup_probe"));
+
+ await expect(
+ loadHarnessExtensions({
+ harness: fx.harness,
+ session: fx.session,
+ cwd: fx.cwd,
+ noExtensions: false,
+ agentDir: tempAgentDir(),
+ configuredPaths: [extDir],
+ }),
+ ).rejects.toThrow("startup boom");
+
+ expect(fx.harness.getTools().map((t) => t.name)).toEqual(baseTools);
+ });
+
it("returns undefined when extensions are disabled", async () => {
fx = await buildTestHarness({ turns: [{ steps: [{ type: "text", text: "ok" }] }] });
const extDir = mkdtempSync(join(tmpdir(), "cua-ext-"));You can send follow-ups to the cloud agent here.
rgarcia
force-pushed
the
hypeship/harness-extension-host
branch
2 times, most recently
from
79e4b07 to
89d5a86
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Extension screenshots ignore browser mode
- Added a browser-mode guard in HarnessExtensionHost.maybeInitialScreenshot so extension sendUserMessage no longer attaches an OS-display screenshot when mode is browser.
Or push these changes by commenting:
@cursor push 3aaedc4ea9
Preview (3aaedc4ea9)
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -696,6 +696,9 @@
// During startup the user's first prompt owns the first-turn screenshot; an
// extension message here must not consume it (see `startedUp`).
if (!this.startedUp) return undefined;
+ // Browser mode tools use viewport coordinates; skip OS-display capture so we
+ // do not mix coordinate frames on the first turn.
+ if (this.harness.getMode() === "browser") return undefined;
if (await sessionHasPriorTurn(this.session)) return undefined;
return this.initialScreenshot();
}You can send follow-ups to the cloud agent here.
Load pi extensions (arbitrary TS, default-exported factory) against cua's lower-level AgentHarness, which pi's AgentSession-based extension system does not bind to. Reuses pi's host-agnostic loader and runner: binds the runner's action seams to the harness, bridges harness events into the runner's extension-event emitters, registers extension tools, and mirrors AgentSession.reload for hot-reload. Tier A scope: tools, events, model/thinking/active-tool control, and session-entry writes, headless (no-op UI). Slash commands, flags, ctx.ui.*, and the session-replacement family are deferred (stubbed to throw). Re-applies the extension-tool union on model_update because CuaAgentHarness.setModel rebuilds tools from construction-time extraTools and would otherwise drop runtime-registered ones. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- reapplyTools: preserve base active state and re-activate extension tools unless explicitly deactivated through the host's setActiveTools seam (tracked in inactiveExtensionTools). Keeps extension tools active across a setModel — which rebuilds the harness tool list and drops them — while honoring an opt-out, instead of unconditionally re-enabling every extension tool. - reapplyTools: coalesce a reapply requested while setTools is in flight into a follow-up pass rather than dropping it. - reload: latch a ctx.shutdown() raised during the reload critical section and honor it at await boundaries, so an unawaited dispose can't tear down the freshly rebuilt runner and bridge. - sendUserMessage: route through the host and attach the first-turn screenshot via an optional initialScreenshot callback, matching the CLI's prompt sites. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Five end-to-end tests drive HarnessExtensionHost through the full learn-a-tool loop: an inefficient first run (base computer-use steps), a meta-agent-authored learned tool written to disk, host.reload() discovering it, and a second run that calls the learned tool in one step. Each models a real computer-use use case — DOM table extraction, template-match click, parameterized form-fill macro, navigation shortcut, and a pagination de-dup extractor that additionally proves an extension's agent_start handler is re-bound after reload (result reports runs=1: the handler re-fired, and the fresh-from-disk import reset the prior count). Learned tools are pure JS over inputs that stand in for screenshots/DOM (the fake harness has no browser), exercising load/reload, reapplyTools registration+activation, and the event bridge end to end. No host changes were needed. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
reapplyTools built the base tool list by filtering the live harness tools only against the newly loaded extension set, so a tool registered by a prior generation that a reload removed or renamed lingered on the harness, bound to the dead runner generation. Exclude prior-generation extension tool names from base as well, and cover it with a reload test that renames a tool. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Construct and load HarnessExtensionHost in setupHarnessRuntime via a new browser-free helper (extensions/setup.ts), carry it on HarnessRuntime, and dispose it before closing the browser handle on all three run paths (print/interactive/action). Add a /reload TUI command that hot-swaps edited extensions and surfaces load errors, wire it into autocomplete, and pass the first-turn screenshot from the browser handle. Gate project-local extensions behind trust: global <agentDir>/extensions load on every run, but the implicit <cwd>/.pi/extensions scan and the explicit <cwd>/.agents/extensions dir only load when the project is trusted (persisted pi trust or --trust-extensions); --no-extensions disables loading. Project extensions execute arbitrary TypeScript in-process, so they are never auto-run by default. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
cua already runs agent-authored code (bash, file edits, the browser), so gating project-local extensions behind trust was inconsistent and blocked the self-improve loop — an agent writes a learned tool into the project extension dir and it should load on the next run. Remove the projectExtensionsTrusted host option and the --trust-extensions flag: <cwd>/.agents/extensions, the implicit <cwd>/.pi/extensions scan, and global ~/.pi/agent/extensions all load on every run. --no-extensions still disables loading entirely. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add a flag-gated write_extension tool that lets the agent author its own pi extension at runtime. It writes a TypeScript extension module into the project extension dir, trial-loads it in isolation for immediate validation feedback (without touching the live runner), and queues a reload that fires at the next idle boundary (agent_end), so the authored tool joins the toolset for subsequent prompts with no manual /reload. The auto-reload is scheduled off-stack from the bridge's agent_end handler (queueMicrotask), never awaited inside the tool's execute or an event handler, so it can never swap the runner mid-turn. write_extension is a persistent host-provided tool that survives setModel's tool rebuild; an extension registering a colliding name is dropped and logged instead of crashing setTools. Authored filenames are constrained to a bare .ts basename inside the extension dir (no traversal). Off by default. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Robustness edges now that the host has live wiring and multiple reload entry points: - load(): throw if already disposed, no-op on a second call, and dispose if an extension calls ctx.shutdown() during session_start — never resolves a torn-down or double-bridged host. - reload(): reentrancy guard (a reload during reload re-arms the latch instead of double-tearing-down the bridge), await waitForIdle so it can't swap the runner/bridge mid-turn, and reinstall the bridge if the rebuild throws so extension events keep flowing. - /reload command: report a shutting-down notice instead of false success when an extension disposed the host mid-reload. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
reload() now loops while a reload was requested mid-reload (via the reentrancy guard, or a write_extension during the reload), so the request is applied before reload() resolves instead of waiting for the next idle boundary — and a /reload no longer reports success while changes are still pending. promptUserMessage catches its own errors: the seam voids the call, so a failed harness.prompt (e.g. prompting while the harness is mid-turn) is recorded for the agent to see rather than surfacing as an unhandled promise rejection. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Builds on the reentrant-reload coalescing already in place: - reload() returns an outcome (reloaded/coalesced/disposed). When a /reload coalesces onto an in-flight reload the reentrancy guard returns before the changes are applied, so the command now reports "a reload is already in progress" instead of a false success. - dispose() awaits an in-flight queued reload before tearing down, so the print/interactive/action cleanup `finally` blocks can't dispose the runner and close the browser mid-reload. Teardown is split into disposeNow() so a shutdown raised from inside reload() (an extension calling ctx.shutdown() during session_shutdown) tears down without awaiting its own reload — which would deadlock. - The first-turn screenshot is decided once at startup, before extensions load, and threaded via skipInitialScreenshot. An extension that sends a user message during session_start no longer flips the live prior-turn check and leaves the user's real first prompt without the browser frame; a startedUp guard keeps the host from consuming the screenshot during startup. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Follow-up edge cases on the freshly-wired host: - reapplyTools: drop an extension tool that shadows a built-in name (the shadow would vanish the built-in when the extension is later removed on reload), and record the applied extensionTools only after setTools succeeds — a setTools throw previously advanced the record while the harness kept the old set, desyncing the next reapply's prior-name filtering. - cli-harness: compute the first-turn screenshot decision inside the try that closes the browser handle on failure, so a session read error can't leak the already-provisioned browser. - load(): set `loaded` only after wiring succeeds, so a throw during buildRunner/reapply/bridge install doesn't leave a half-built host that a later load() treats as ready. - drainPendingReload(): re-arm the latch if reload() throws, so the next idle boundary retries instead of stranding a valid on-disk extension until a manual /reload. - bridge: schedule the queued agent_end reload in `finally`, so a throwing agent_end handler can't drop it. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Teardown detached the bridge and cleared the runner but left the host's host-provided and extension tools registered (and active) on the harness, so the model could still call a tool whose runner binding was gone. This is moot at process exit but matters when an extension calls ctx.shutdown() mid-session, which disposes the host while the CLI keeps running. disposeNow now restores the harness to its base tools before the runner goes away. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…xture pi 0.80's Models API replaced the global api-registry with isolated Models collections, so the shared scripted-provider test fixture no longer mutates global state and dropped its dispose() teardown. Remove the now-stale fx.dispose() calls from the extension-host test afterEach hooks; the host's own dispose() still runs, and the scripted collection needs no teardown. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
rgarcia
force-pushed
the
hypeship/harness-extension-host
branch
from
89d5a86 to
76e2973
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: TUI import outside cleanup finally
- Moved the dynamic
./tui/mainimport insiderunInteractiveCommand's existing try/finally so runtime resources are always disposed if import or execution throws.
- Moved the dynamic
Or push these changes by commenting:
@cursor push d8ba2bcef1
Preview (d8ba2bcef1)
diff --git a/packages/cli/src/cli-harness.ts b/packages/cli/src/cli-harness.ts
--- a/packages/cli/src/cli-harness.ts
+++ b/packages/cli/src/cli-harness.ts
@@ -650,8 +650,8 @@
flags: HarnessCliFlags,
): Promise<number> {
const runtime = await setupHarnessRuntime(flags);
- const { runInteractive } = await import("./tui/main");
try {
+ const { runInteractive } = await import("./tui/main");
return await runInteractive({
cwd: process.cwd(),
harness: runtime.harness,You can send follow-ups to the cloud agent here.
rgarcia
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 3 potential issues.
Bugbot Autofix prepared fixes for 2 of the 3 issues found in the latest run.
- ✅ Fixed: setMode skips tool mutation lock
- Wrapped
CuaAgentHarness.setModeinmutateToolsand routed extensionsetActiveToolswrites through the same harness tool-mutation path to serialize concurrent tool updates.
- Wrapped
- ✅ Fixed: Coalesced reload hides failures
- Coalesced reload calls now await the in-flight reload and
/reloadnow still surfacesloadErrorsafter a coalesced outcome instead of returning early.
- Coalesced reload calls now await the in-flight reload and
Or push these changes by commenting:
@cursor push a6d22f0c63
Preview (a6d22f0c63)
diff --git a/packages/agent/src/agent.ts b/packages/agent/src/agent.ts
--- a/packages/agent/src/agent.ts
+++ b/packages/agent/src/agent.ts
@@ -778,28 +778,30 @@
* plane conflicts with the requested mode.
*/
async setMode(mode: CuaMode): Promise<void> {
- if (mode === this.runtime.mode) return;
- const previousNames = new Set(this.getTools().map((tool) => tool.name));
- this.runtime.setMode(mode);
- const tools = this.runtime.tools();
- // Tools that survive the mode switch (extraTools, shared names) keep
- // their requested activation state; names new in this mode activate.
- const requested = this.requestedActiveToolNames;
- const active = requested
- ? tools.map((tool) => tool.name).filter((name) => !previousNames.has(name) || requested.includes(name))
- : tools.map((tool) => tool.name);
- try {
- await super.setTools(tools, active);
- } catch (err) {
- // The pre-switch tools stay exposed, so restore the runtime they are
- // bound to — including its still-live translator.
- this.runtime.rollbackSwitch();
- throw err;
- }
- this.runtime.commitSwitch();
- // The requested subset now reflects this mode's toolset; without this a
- // later setModel would restore the pre-switch names.
- if (requested) this.requestedActiveToolNames = active;
+ await this.mutateTools(async () => {
+ if (mode === this.runtime.mode) return;
+ const previousNames = new Set(this.getTools().map((tool) => tool.name));
+ this.runtime.setMode(mode);
+ const tools = this.runtime.tools();
+ // Tools that survive the mode switch (extraTools, shared names) keep
+ // their requested activation state; names new in this mode activate.
+ const requested = this.requestedActiveToolNames;
+ const active = requested
+ ? tools.map((tool) => tool.name).filter((name) => !previousNames.has(name) || requested.includes(name))
+ : tools.map((tool) => tool.name);
+ try {
+ await super.setTools(tools, active);
+ } catch (err) {
+ // The pre-switch tools stay exposed, so restore the runtime they are
+ // bound to — including its still-live translator.
+ this.runtime.rollbackSwitch();
+ throw err;
+ }
+ this.runtime.commitSwitch();
+ // The requested subset now reflects this mode's toolset; without this a
+ // later setModel would restore the pre-switch names.
+ if (requested) this.requestedActiveToolNames = active;
+ });
}
/** The action plane(s) currently exposed to the model. */
diff --git a/packages/agent/test/agent.test.ts b/packages/agent/test/agent.test.ts
--- a/packages/agent/test/agent.test.ts
+++ b/packages/agent/test/agent.test.ts
@@ -1976,6 +1976,46 @@
expect(harness.getModel().id).toBe("gpt-5.5");
});
+ it("serializes setMode while a model switch tool mutation is in flight", async () => {
+ const { env, session } = await createHarnessServices();
+ let gate: Promise<void> | undefined;
+ const gatedSession = new Proxy(session, {
+ get(target, prop, receiver) {
+ const value = Reflect.get(target, prop, receiver);
+ if (prop === "appendActiveToolsChange" && gate) {
+ const pending = gate;
+ gate = undefined;
+ return async (...args: unknown[]) => {
+ await pending;
+ return (value as (...a: unknown[]) => Promise<void>).apply(target, args);
+ };
+ }
+ return typeof value === "function" ? (value as (...a: unknown[]) => unknown).bind(target) : value;
+ },
+ });
+ const harness = new CuaAgentHarness({
+ env,
+ session: gatedSession,
+ browser,
+ client,
+ model: "anthropic:claude-opus-4-5",
+ });
+ let release!: () => void;
+ gate = new Promise<void>((resolve) => {
+ release = resolve;
+ });
+ const modelSwitch = harness.setModel("anthropic:claude-opus-4-7");
+ await new Promise<void>((resolve) => setTimeout(resolve, 0));
+ const modeSwitch = harness.setMode("browser");
+ release();
+ await Promise.all([modelSwitch, modeSwitch]);
+
+ expect(harness.getMode()).toBe("browser");
+ const toolNames = harness.getTools().map((tool) => tool.name);
+ expect(toolNames).toContain("snapshot");
+ expect(toolNames).not.toContain("computer_click");
+ });
+
it("treats a repeated setMode as a no-op", async () => {
const harness = new CuaAgentHarness({
...(await createHarnessServices()),
diff --git a/packages/cli/src/extensions/host.ts b/packages/cli/src/extensions/host.ts
--- a/packages/cli/src/extensions/host.ts
+++ b/packages/cli/src/extensions/host.ts
@@ -208,7 +208,10 @@
reload(): Promise<ReloadOutcome> {
if (this.disposed) return Promise.resolve("disposed");
- if (this.reloadPromise) return Promise.resolve("coalesced");
+ if (this.reloadPromise)
+ return this.reloadPromise.then((outcome) =>
+ outcome === "disposed" ? "disposed" : "coalesced",
+ );
const operation = this.reloadNow();
this.reloadPromise = operation;
const clear = () => {
@@ -471,7 +474,9 @@
if (active.has(tool.name)) this.inactiveExtensionTools.delete(tool.name);
else this.inactiveExtensionTools.add(tool.name);
}
- await this.harness.setActiveTools(names);
+ await this.mutateHarnessTools(async () => {
+ await this.harness.setActiveTools(names);
+ });
}
private uninstallBridge(): void {
diff --git a/packages/cli/src/tui/main.ts b/packages/cli/src/tui/main.ts
--- a/packages/cli/src/tui/main.ts
+++ b/packages/cli/src/tui/main.ts
@@ -581,11 +581,15 @@
// An extension calling ctx.shutdown() during the reload tears the host
// down; don't claim a successful reload.
messages.addNotice("session is shutting down; extensions were not reloaded");
- } else if (outcome === "coalesced") {
+ return;
+ }
+ if (outcome === "coalesced") {
messages.addNotice("a reload is already in progress");
- } else if (opts.host.loadErrors.length > 0) {
+ }
+ if (opts.host.loadErrors.length > 0) {
for (const { path, error } of opts.host.loadErrors) messages.addError(`${path}: ${error}`);
} else {
+ if (outcome === "coalesced") return;
messages.addNotice("extensions reloaded");
}
} catch (err) {
diff --git a/packages/cli/test/extensions.test.ts b/packages/cli/test/extensions.test.ts
--- a/packages/cli/test/extensions.test.ts
+++ b/packages/cli/test/extensions.test.ts
@@ -159,10 +159,27 @@
it("coalesces a reload requested while another is in flight", async () => {
const created = await loadHost();
- // A second request must report that the first reload still owns the operation.
+ const originalWaitForIdle = fx!.harness.waitForIdle.bind(fx!.harness);
+ let release!: () => void;
+ const gate = new Promise<void>((resolve) => {
+ release = resolve;
+ });
+ fx!.harness.waitForIdle = async () => {
+ await gate;
+ await originalWaitForIdle();
+ };
+ // A second request must report that the first reload still owns the operation
+ // and should not resolve until the in-flight reload has finished.
const first = created.reload();
- const second = await created.reload();
- expect(second).toBe("coalesced");
+ let secondResolved = false;
+ const second = created.reload().then((outcome) => {
+ secondResolved = true;
+ return outcome;
+ });
+ await new Promise((resolve) => setTimeout(resolve, 20));
+ expect(secondResolved).toBe(false);
+ release();
+ expect(await second).toBe("coalesced");
expect(await first).toBe("reloaded");
});
diff --git a/packages/cli/test/reload-command.test.ts b/packages/cli/test/reload-command.test.ts
--- a/packages/cli/test/reload-command.test.ts
+++ b/packages/cli/test/reload-command.test.ts
@@ -67,6 +67,22 @@
expect(notices).not.toContain("extensions reloaded");
});
+ it("reports loadErrors when joining an in-progress reload", async () => {
+ const reload = vi.fn(async () => "coalesced" as const);
+ const host = {
+ reload,
+ loadErrors: [{ path: "/ext/broken.ts", error: "boom" }],
+ isDisposed: () => false,
+ } as unknown as HarnessExtensionHost;
+ const { opts, messages, notices, errors } = setup(host);
+
+ await applyReloadCommand(opts, messages);
+
+ expect(notices.some((n) => n.includes("already in progress"))).toBe(true);
+ expect(errors).toContain("/ext/broken.ts: boom");
+ expect(notices).not.toContain("extensions reloaded");
+ });
+
it("does not report success when the host disposed during reload", async () => {
const reload = vi.fn(async () => {});
const host = { reload, loadErrors: [], isDisposed: () => true } as unknown as HarnessExtensionHost;You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit eac28f9. Configure here.
| "@aws/lambda-invoke-store": "^0.3.0", | ||
| "@smithy/core": "^3.29.2", | ||
| "@smithy/signature-v4": "^5.6.3", | ||
| "@smithy/types": "^4.16.0", |
There was a problem hiding this comment.
Lockfile bumps unrelated packages
Medium Severity
The lockfile diff bumps many @aws-sdk, @smithy, google-auth-library, protobufjs, and related transitive versions alongside the pi vendor pin, not only workspace changes for @earendil-works/pi-* file:vendor/pi/... entries. Unrelated dependency drift can change runtime behavior outside the extensions feature.
Additional Locations (1)
Triggered by learned rule: Lockfile changes must be scoped to the PR's intent — no silent dependency drift
Reviewed by Cursor Bugbot for commit eac28f9. Configure here.
There was a problem hiding this comment.
Bugbot Autofix determined this is a false positive.
The reported transitive lockfile movements are tied to the intentional switch to vendored @earendil-works/pi-* 0.80.7 tarballs rather than an accidental unrelated lockfile regeneration in this branch.
You can send follow-ups to the cloud agent here.
Contributor
Author
End-to-end QAFresh QA against
No regressions or new defects found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/reload--self-extend, exposing a Pi-wrappedadd_toolthat accepts one constrained tool definition<cwd>/.agents/extensions, and activate the new tool beforeadd_toolreturnsAgentHarnessextension adapter behind one CLI-facing interface so it can be removed when Pi's planned harness hooks and extension facade become publicExtension architecture
packages/cli/src/extensions/setup.tsis the only runtime entrypoint. The temporary compatibility implementation lives underextensions/compat/and is split along Pi's planned architecture:context.tsmaps Pi extension context actions onto the harnesshooks.tsforwards harness hooks and lifecycle events toExtensionRunnertool-registry.tsreconciles extension tools with CUA model/tool changeshost.tsowns discovery, lifecycle, disposal, and manual reloadadd-tool.tsis kept outside that compatibility directory. It owns the durable artifact format and validation, so replacing the compatibility host with upstream Pi support does not require changing generated tools.add_toolbehavioradd_toolaccepts{ name, label?, description, parameters, execute }. It derives<name>.ts, renders one import-free extension with onepi.registerToolcall, rejects collisions and existing artifacts, trial-loads the staged file, publishes with no-overwrite semantics, and awaits live harness installation. The following provider turn in the sameharness.prompt()run can call the new tool; no idle-boundary reload or second prompt is required.Generated execute code is trusted local Node.js code, not a sandbox.
/reloadremains available for externally edited, renamed, or removed extensions.Pi dependency requirement
Message-anchored dynamic tools require upstream Pi commit
3d8f74357c169d24f996a1611ecc4be72b7744bd. The npm registry still reports0.80.6as the latest@earendil-works/pi-coding-agent, so this branch temporarily pins all four Pi packages to lockstep0.80.7-anchor.3d8f743tarballs through immutable raw GitHub URLs.vendor/pi/README.mdrecords provenance and checksums. These file dependencies should be replaced by the first normal npm release containing that commit.Tests
npm run typecheckenv -u GOOGLE_API_KEY -u GEMINI_API_KEY npm run test --workspace @onkernel/cua-ai— 146 passednpm run test --workspace @onkernel/cua-agent— 160 passed, 12 live tests skippednpm run test --workspace @onkernel/cua-cli— 130 passed, 5 fixture tests skippednpm ls @earendil-works/pi-ai @earendil-works/pi-agent-core @earendil-works/pi-coding-agent @earendil-works/pi-tui@onkernel/cua-ai,@onkernel/cua-agent, and@onkernel/cua-cligit diff --checkThe aggregate all-workspaces test was not rerun because the unrelated
@onkernel/ptywrightworkspace has an existing generated-ESM resolution failure; the three affected workspaces pass independently.Note
Medium Risk
Runtime extension loading and same-run
add_toolexecute trusted local code and change the live tool surface; Pi packages are pinned to raw GitHub tarballs instead of npm until upstream ships the required commit.Overview
Pi extensions and
--self-extendbring project/user Pi extensions into the CUA CLI via a temporary AgentHarness adapter (extensions/compat/*), with--no-extensions,/reload, and cleanup on exit.--self-extendaddsadd_tool, which validates a single tool definition, writes it under.agents/extensions, trial-loads it, and installs it on the harness before the call returns so the new tool is usable on the next model turn in the same run (execute code is trusted local Node, not sandboxed).Harness behavior changes
CuaAgentHarness.setModelto run tool-list updates through a newmutateToolsqueue so model switches stay serialized with extension-driven tool changes. Startup now fixes first-turn screenshot skipping before extensions load so an extension’s startup message cannot steal the user’s initial frame.Dependencies: all
@earendil-works/pi-*packages are pinned to lockstep0.80.7-anchor.3d8f743tarballs from the repo vendor URLs (needed for message-anchored dynamic tools).package-lock.jsonalso bumps AWS SDK / Smithy and related transitive versions.@onkernel/cua-aigains tests for OpenAI tool search and Anthropic deferred/eager dynamic tool payloads (including native browser routing).Reviewed by Cursor Bugbot for commit f4e3944. Bugbot is set up for automated code reviews on this repo. Configure here.