refactor(agent-providers): fix type-aware lint errors in diagnostic-utils and generic-acp-agent (#705)

* refactor(agent-providers): add type guards and fix type-aware lint errors in diagnostic-utils, generic-acp-agent, and tool-call-detail-primitives

* fix(server): defer PTY onExit to allow pending data events to fire

On Linux, node-pty's onExit callback can arrive before the last buffered
PTY data chunk is delivered via onData. Wrapping finish() in setImmediate
gives libuv's I/O poll phase a chance to flush remaining PTY reads before
the promise resolves, preventing tail bytes from being silently dropped.

Fixes a flaky worktree-bootstrap test that reliably reproduced on Linux CI.

* unslop: remove dead guard, fix import order, trim comment

- generic-acp-agent: remove isNonEmptyStringArray and its guard; the
  constructor parameter is already typed [string, ...string[]] so the
  check can never fire
- provider-registry: move isNonEmptyStringArray below the import block
  instead of between two import groups
- worktree: condense 3-line comment to one line per project convention
This commit is contained in:
Mohamed Boudra
2026-05-04 21:50:23 +08:00
committed by GitHub
parent 165fc11955
commit a6ad4ff430
6 changed files with 49 additions and 56 deletions

View File

@@ -36,6 +36,10 @@ import {
type AgentProviderDefinition,
} from "./provider-manifest.js";
function isNonEmptyStringArray(value: string[]): value is [string, ...string[]] {
return value.length > 0;
}
export type { AgentProviderDefinition };
export { AGENT_PROVIDER_DEFINITIONS, getAgentProviderDefinition };
@@ -461,9 +465,11 @@ function addDerivedProviders(
}
if (override.extends === "acp") {
if (!override.command) {
if (!override.command || !isNonEmptyStringArray(override.command)) {
throw new Error(`ACP provider '${providerId}' requires a command`);
}
// Capture command in const for closure - TypeScript can't track type refinement inside closures
const command = override.command;
resolvedProviders.set(providerId, {
definition: createDerivedDefinition(
@@ -484,7 +490,7 @@ function addDerivedProviders(
createBaseClient: (logger) =>
new GenericACPAgentClient({
logger,
command: override.command!,
command,
env: override.env,
}),
});

View File

@@ -43,8 +43,9 @@ function truncateForDiagnostic(value: string): string {
return `${trimmed.slice(0, DIAGNOSTIC_OUTPUT_CAP)}…(truncated)`;
}
function readStringProperty(error: object, key: string): string | undefined {
const value = (error as Record<string, unknown>)[key];
function readStringProperty(error: Error, key: string): string | undefined {
if (!(key in error)) return undefined;
const value = (error as Error & Record<string, unknown>)[key];
if (typeof value === "string") {
return value;
}
@@ -54,8 +55,9 @@ function readStringProperty(error: object, key: string): string | undefined {
return undefined;
}
function readUnknownProperty(error: object, key: string): unknown {
return (error as Record<string, unknown>)[key];
function readUnknownProperty(error: Error, key: string): unknown {
if (!(key in error)) return undefined;
return (error as Error & Record<string, unknown>)[key];
}
function pushIfNonEmpty(sections: string[], label: string, value: string | undefined): void {

View File

@@ -5,7 +5,7 @@ import { ACPAgentClient } from "./acp-agent.js";
interface GenericACPAgentClientOptions {
logger: Logger;
command: string[];
command: [string, ...string[]];
env?: Record<string, string>;
}
@@ -13,20 +13,16 @@ export class GenericACPAgentClient extends ACPAgentClient {
private readonly command: [string, ...string[]];
constructor(options: GenericACPAgentClientOptions) {
if (options.command.length === 0) {
throw new Error("Generic ACP provider requires a non-empty command");
}
super({
provider: "acp",
logger: options.logger,
runtimeSettings: {
env: options.env,
},
defaultCommand: options.command as [string, ...string[]],
defaultCommand: options.command,
});
this.command = options.command as [string, ...string[]];
this.command = options.command;
}
protected override async resolveLaunchCommand(): Promise<{ command: string; args: string[] }> {

View File

@@ -166,9 +166,13 @@ function parseLargeAgentStreamPayloadPrompt(
if (!Number.isFinite(bytes) || bytes <= 0) {
return null;
}
const kindValue = match[2]?.toLowerCase();
if (kindValue !== "diff" && kindValue !== "file" && kindValue !== "image") {
return null;
}
return {
bytes: Math.min(bytes, 1_000_000),
kind: match[2]?.toLowerCase() as LargeAgentStreamPayloadRequest["kind"],
kind: kindValue,
};
}

View File

@@ -859,19 +859,14 @@ export function toolDetailBranchByName<
output: z.infer<OutputSchema> | null,
) => ToolCallDetail | undefined,
) {
return z
.object({
name: z.literal(name),
input: inputSchema.nullable(),
output: outputSchema.nullable(),
})
.transform((value) => {
const parsed = value as unknown as {
input: z.infer<InputSchema> | null;
output: z.infer<OutputSchema> | null;
};
return mapper(parsed.input, parsed.output);
});
const schema = z.object({
name: z.literal(name),
input: inputSchema.nullable(),
output: outputSchema.nullable(),
});
return schema.transform((value: z.infer<typeof schema>) => {
return mapper(value.input, value.output);
});
}
export function toolDetailBranchByToolName<
@@ -887,19 +882,14 @@ export function toolDetailBranchByToolName<
output: z.infer<OutputSchema> | null,
) => ToolCallDetail | undefined,
) {
return z
.object({
toolName: z.literal(toolName),
input: inputSchema.nullable(),
output: outputSchema.nullable(),
})
.transform((value) => {
const parsed = value as unknown as {
input: z.infer<InputSchema> | null;
output: z.infer<OutputSchema> | null;
};
return mapper(parsed.input, parsed.output);
});
const schema = z.object({
toolName: z.literal(toolName),
input: inputSchema.nullable(),
output: outputSchema.nullable(),
});
return schema.transform((value: z.infer<typeof schema>) => {
return mapper(value.input, value.output);
});
}
export function toolDetailBranchByNameWithCwd<
@@ -916,19 +906,13 @@ export function toolDetailBranchByNameWithCwd<
cwd: string | null,
) => ToolCallDetail | undefined,
) {
return z
.object({
name: z.literal(name),
input: inputSchema.nullable(),
output: outputSchema.nullable(),
cwd: z.string().optional().nullable(),
})
.transform((value) => {
const parsed = value as unknown as {
input: z.infer<InputSchema> | null;
output: z.infer<OutputSchema> | null;
cwd?: string | null;
};
return mapper(parsed.input, parsed.output, parsed.cwd ?? null);
});
const schema = z.object({
name: z.literal(name),
input: inputSchema.nullable(),
output: outputSchema.nullable(),
cwd: z.string().optional().nullable(),
});
return schema.transform((value: z.infer<typeof schema>) => {
return mapper(value.input, value.output, value.cwd ?? null);
});
}

View File

@@ -507,7 +507,8 @@ async function execSetupCommandStreamed(options: {
});
terminal.onExit(({ exitCode }) => {
finish(typeof exitCode === "number" ? exitCode : null);
// Defer so pending onData events fire first — node-pty onExit can arrive before the last PTY chunk on Linux.
setImmediate(() => finish(typeof exitCode === "number" ? exitCode : null));
});
} catch (error) {
emitOutput("stderr", error instanceof Error ? error.message : String(error));