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-05 19:58:55 +07:00
parent 660b4d3cec
commit 75247efbd5
5 changed files with 47 additions and 55 deletions
@@ -36,6 +36,10 @@ import {
type AgentProviderDefinition, type AgentProviderDefinition,
} from "./provider-manifest.js"; } from "./provider-manifest.js";
function isNonEmptyStringArray(value: string[]): value is [string, ...string[]] {
return value.length > 0;
}
export type { AgentProviderDefinition }; export type { AgentProviderDefinition };
export { AGENT_PROVIDER_DEFINITIONS, getAgentProviderDefinition }; export { AGENT_PROVIDER_DEFINITIONS, getAgentProviderDefinition };
@@ -461,9 +465,11 @@ function addDerivedProviders(
} }
if (override.extends === "acp") { if (override.extends === "acp") {
if (!override.command) { if (!override.command || !isNonEmptyStringArray(override.command)) {
throw new Error(`ACP provider '${providerId}' requires a 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, { resolvedProviders.set(providerId, {
definition: createDerivedDefinition( definition: createDerivedDefinition(
@@ -484,7 +490,7 @@ function addDerivedProviders(
createBaseClient: (logger) => createBaseClient: (logger) =>
new GenericACPAgentClient({ new GenericACPAgentClient({
logger, logger,
command: override.command!, command,
env: override.env, env: override.env,
}), }),
}); });
@@ -43,8 +43,9 @@ function truncateForDiagnostic(value: string): string {
return `${trimmed.slice(0, DIAGNOSTIC_OUTPUT_CAP)}…(truncated)`; return `${trimmed.slice(0, DIAGNOSTIC_OUTPUT_CAP)}…(truncated)`;
} }
function readStringProperty(error: object, key: string): string | undefined { function readStringProperty(error: Error, key: string): string | undefined {
const value = (error as Record<string, unknown>)[key]; if (!(key in error)) return undefined;
const value = (error as Error & Record<string, unknown>)[key];
if (typeof value === "string") { if (typeof value === "string") {
return value; return value;
} }
@@ -54,8 +55,9 @@ function readStringProperty(error: object, key: string): string | undefined {
return undefined; return undefined;
} }
function readUnknownProperty(error: object, key: string): unknown { function readUnknownProperty(error: Error, key: string): unknown {
return (error as Record<string, unknown>)[key]; if (!(key in error)) return undefined;
return (error as Error & Record<string, unknown>)[key];
} }
function pushIfNonEmpty(sections: string[], label: string, value: string | undefined): void { function pushIfNonEmpty(sections: string[], label: string, value: string | undefined): void {
@@ -5,7 +5,7 @@ import { ACPAgentClient } from "./acp-agent.js";
interface GenericACPAgentClientOptions { interface GenericACPAgentClientOptions {
logger: Logger; logger: Logger;
command: string[]; command: [string, ...string[]];
env?: Record<string, string>; env?: Record<string, string>;
} }
@@ -13,20 +13,16 @@ export class GenericACPAgentClient extends ACPAgentClient {
private readonly command: [string, ...string[]]; private readonly command: [string, ...string[]];
constructor(options: GenericACPAgentClientOptions) { constructor(options: GenericACPAgentClientOptions) {
if (options.command.length === 0) {
throw new Error("Generic ACP provider requires a non-empty command");
}
super({ super({
provider: "acp", provider: "acp",
logger: options.logger, logger: options.logger,
runtimeSettings: { runtimeSettings: {
env: options.env, 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[] }> { protected override async resolveLaunchCommand(): Promise<{ command: string; args: string[] }> {
@@ -166,9 +166,13 @@ function parseLargeAgentStreamPayloadPrompt(
if (!Number.isFinite(bytes) || bytes <= 0) { if (!Number.isFinite(bytes) || bytes <= 0) {
return null; return null;
} }
const kindValue = match[2]?.toLowerCase();
if (kindValue !== "diff" && kindValue !== "file" && kindValue !== "image") {
return null;
}
return { return {
bytes: Math.min(bytes, 1_000_000), bytes: Math.min(bytes, 1_000_000),
kind: match[2]?.toLowerCase() as LargeAgentStreamPayloadRequest["kind"], kind: kindValue,
}; };
} }
@@ -859,19 +859,14 @@ export function toolDetailBranchByName<
output: z.infer<OutputSchema> | null, output: z.infer<OutputSchema> | null,
) => ToolCallDetail | undefined, ) => ToolCallDetail | undefined,
) { ) {
return z const schema = z.object({
.object({ name: z.literal(name),
name: z.literal(name), input: inputSchema.nullable(),
input: inputSchema.nullable(), output: outputSchema.nullable(),
output: outputSchema.nullable(), });
}) return schema.transform((value: z.infer<typeof schema>) => {
.transform((value) => { return mapper(value.input, value.output);
const parsed = value as unknown as { });
input: z.infer<InputSchema> | null;
output: z.infer<OutputSchema> | null;
};
return mapper(parsed.input, parsed.output);
});
} }
export function toolDetailBranchByToolName< export function toolDetailBranchByToolName<
@@ -887,19 +882,14 @@ export function toolDetailBranchByToolName<
output: z.infer<OutputSchema> | null, output: z.infer<OutputSchema> | null,
) => ToolCallDetail | undefined, ) => ToolCallDetail | undefined,
) { ) {
return z const schema = z.object({
.object({ toolName: z.literal(toolName),
toolName: z.literal(toolName), input: inputSchema.nullable(),
input: inputSchema.nullable(), output: outputSchema.nullable(),
output: outputSchema.nullable(), });
}) return schema.transform((value: z.infer<typeof schema>) => {
.transform((value) => { return mapper(value.input, value.output);
const parsed = value as unknown as { });
input: z.infer<InputSchema> | null;
output: z.infer<OutputSchema> | null;
};
return mapper(parsed.input, parsed.output);
});
} }
export function toolDetailBranchByNameWithCwd< export function toolDetailBranchByNameWithCwd<
@@ -916,19 +906,13 @@ export function toolDetailBranchByNameWithCwd<
cwd: string | null, cwd: string | null,
) => ToolCallDetail | undefined, ) => ToolCallDetail | undefined,
) { ) {
return z const schema = z.object({
.object({ name: z.literal(name),
name: z.literal(name), input: inputSchema.nullable(),
input: inputSchema.nullable(), output: outputSchema.nullable(),
output: outputSchema.nullable(), cwd: z.string().optional().nullable(),
cwd: z.string().optional().nullable(), });
}) return schema.transform((value: z.infer<typeof schema>) => {
.transform((value) => { return mapper(value.input, value.output, value.cwd ?? null);
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);
});
} }