Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/coding-agent/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
### Fixed
- ACP session configuration now emits the spec-defined `category` field on the Mode, Model, and Thinking select options (`mode`, `model`, `thought_level`), so standards-compliant ACP clients such as Paseo discover models, modes, and thinking levels instead of an empty model picker (#3922).
- The ACP session model catalog is now filtered to active providers via `providers.list/active`, falling back to the full catalog on older session hosts, so ACP clients no longer list models for providers without usable credentials (#3922).
- Workflow-gate asks (ralplan approval, deep-interview questions) now surface through the ACP permission channel when the client does not advertise ACP form elicitation, so plain ACP clients such as Paseo can answer selector gates; free-text asks remain unanswered and the richer `ui` channel stays preferred when advertised (#3925).

- `todo_write` now rejects malformed raw arguments with bounded, authority-controlled correction codes instead of a generic rejection: unknown root keys, unknown operation-entry keys, done/drop entries without a task or phase target, and unknown init list-entry keys each surface a fixed message naming the accepted shape without echoing the offending input, while recoverable payloads keep the passthrough/coercion path and the existing ask-tool codes are untouched (#3916).
- The Alibaba Token Plan onboarding preset and `alibaba-token-plan-qwen-deepseek` profile now reference the provider-supported `qwen3.8-max` model id instead of `qwen-3.8-max`, preventing the built-in profile from selecting an HTTP 400 unsupported model (#3909).
Expand Down
10 changes: 8 additions & 2 deletions packages/coding-agent/src/modes/acp/acp-agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -737,7 +737,13 @@ export function acpRequestFailure(error: unknown): unknown {
}
}

/** Registers a permission provider only when the ACP client requires prompts. */
/**
* Registers the permission reverse channel whenever a form-less client needs
* to answer selector asks. The permission mode (prompt vs allow) only gates
* tool-authorization prompts via `permission_mode.set`; workflow questions
* still need a channel, so form-less clients always get the permission
* capability and the bus installs the permission-backed ask source on it.
*/
export function acpProviderRegistrations(
capabilities: ClientCapabilities | undefined,
env: NodeJS.ProcessEnv = process.env,
Expand All @@ -757,7 +763,7 @@ export function acpProviderRegistrations(
]
: []),
...(capabilities?.terminal ? [{ capability: "terminal", definitions: [] }] : []),
...(resolveAcpPermissionMode(capabilities, env) === "prompt"
...(resolveAcpPermissionMode(capabilities, env) === "prompt" || !capabilities?.elicitation?.form
? [{ capability: "permission", definitions: [] }]
: []),
...(capabilities?.elicitation?.form ? [{ capability: "ui", definitions: [] }] : []),
Expand Down
19 changes: 19 additions & 0 deletions packages/coding-agent/src/sdk/broker/ensure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,25 @@ async function ensureBrokerOnce(settings: EnsureBrokerSettings, initiator: Ensur
await sleep(50);
}
const exitedBeforeDiscovery = child.exitCode !== null || child.signalCode !== null;
if (exitedBeforeDiscovery && child.exitCode === 0) {
// A clean exit means another broker won the ownership lock (two ACP
// processes racing a cold broker state, e.g. a provider probe and an
// agent launch). The winner may publish its discovery right after our
// last poll; reuse it instead of failing the caller. Transient discovery
// read failures fall through to the common cleanup + failure path below.
try {
for (let retry = 0; retry < 20; retry++) {
const winner = await readBrokerDiscovery(settings.agentDir, settings.heartbeatTtlMs);
Comment on lines +289 to +290

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor the broker discovery deadline after a clean race loss

When two processes race to start a cold broker, the losing child can exit immediately while the winner is still initializing. This recovery loop waits for only 20 × 50 ms, even though the normal discovery budget is 10 seconds (30 seconds for fixtures), so a valid winner that publishes after the first second still causes ensureBroker to report “exited before discovery.” Continue polling until the existing discovery deadline rather than imposing this shorter fixed window.

Useful? React with 👍 / 👎.

if (winner) {
await owner.stop();
return { kind: "external-discovery", discovery: winner };
}
await sleep(50);
}
} catch {
// fall through to cleanup + failure
}
}
const failure = spawnError
? new Error(`Failed to spawn detached SDK broker: ${spawnError.message}`)
: exitedBeforeDiscovery
Expand Down
187 changes: 185 additions & 2 deletions packages/coding-agent/src/sdk/bus/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ import type {
AskSettlement,
AskSettlementResult,
} from "../../tools";
import { registerAskAnswerSource, registerWorkflowGateEmitterListener } from "../../tools/ask-answer-registry";
import { RECOMMENDED_SUFFIX } from "../../tools/ask";
import {
GJC_ASK_TIMEOUT_CODE,
registerAskAnswerSource,
registerWorkflowGateEmitterListener,
} from "../../tools/ask-answer-registry";
import { acpFinalTextFromMessage } from "../acp/final-text";
import { ensureBroker } from "../broker/ensure";
import { SessionIndex } from "../broker/session-index";
Expand Down Expand Up @@ -1783,6 +1788,158 @@ function createSdkUiAskAnswerSource(
awaitAnswerRequest,
};
}

/**
* Ask-answer source that bridges workflow-gate asks to the ACP permission
* channel (`session/request_permission`). Used when the client does not
* advertise ACP form elicitation (e.g. Paseo): the gate question is sent as
* a permission request whose options are the answer choices, and the
* selected optionId maps back to the answer. Only selector asks are bridged;
* free-text asks have no permission-option representation and stay
* unanswered (unchanged from today). Auto-approval follows the client's
* permission mode, so gates never self-approve under `prompt`.
*/
export function createSdkPermissionAskAnswerSource(
requestPermission: (params: Record<string, unknown>, signal?: AbortSignal) => Promise<unknown>,
): AskAnswerSource {
const awaitAnswerRequest = async (
request: AskAnswerRequest,
signal?: AbortSignal,
): Promise<AskAnswerSourceResult> => {
if (signal?.aborted) return undefined;
if (request.interaction !== "selector") return undefined;
// AskTool appends its synthetic "Other"/clarification transition entries
// at the end; a free-text editor this channel cannot complete. Remove
// exactly those trailing entries so a legitimate option that happens to
// share a transition label is preserved and recommendedIndex stays valid.
const transitionCount =
typeof request.transitionCount === "number" &&
Number.isInteger(request.transitionCount) &&
request.transitionCount > 0
? Math.min(request.transitionCount, request.options.length)
: 0;
const bridgedOptions =
transitionCount > 0 ? request.options.slice(0, request.options.length - transitionCount) : request.options;
// An ask with no model-supplied choices leaves only the synthetic
// transition entries; do not send an unanswerable permission request.
// An ask with no model-supplied choices leaves only the synthetic
// transition entries; do not send an unanswerable permission request
// unless an enabled control can still commit something.
if (bridgedOptions.length === 0 && !request.controls.some(control => control.enabled)) return undefined;
const recommendedLabel =
typeof request.recommendedIndex === "number" &&
Number.isInteger(request.recommendedIndex) &&
request.recommendedIndex >= 0 &&
request.recommendedIndex < bridgedOptions.length
? bridgedOptions[request.recommendedIndex]
: undefined;
const selectedOptions = request.selectedOptions;
const markSelection = selectedOptions !== undefined && selectedOptions.length > 0;
const choices = new Map<string, AskRemoteInteraction>();
const options: Array<Record<string, unknown>> = bridgedOptions.map((label, index) => {
const optionId = `option:${index}`;
choices.set(optionId, { kind: "value", value: label });
const selected = markSelection && selectedOptions?.includes(label) === true;
const name = markSelection ? `${selected ? "[x] " : "[ ] "}${label}` : label;
return {
optionId,
name: label === recommendedLabel ? `${name}${RECOMMENDED_SUFFIX}` : name,
kind: "allow_once",
};
});
for (const control of request.controls) {
if (!control.enabled) continue;
const optionId = `control:${control.id}`;
choices.set(optionId, { kind: "control", controlId: control.id });
options.push({ optionId, name: control.label, kind: "allow_once" });
}
const requestController = new AbortController();
const onRequestAbort = () => requestController.abort();
signal?.addEventListener("abort", onRequestAbort, { once: true });
const {
promise: requestPromise,
resolve: resolveRequest,
reject: rejectRequest,
} = Promise.withResolvers<unknown>();
const timeoutTimer =
request.timeoutMs === undefined
? undefined
: setTimeout(() => {
requestController.abort();
resolveRequest(undefined);
}, request.timeoutMs);
void requestPermission(
{
toolCall: {
toolCallId: crypto.randomUUID(),
toolName: "ask",
title: request.question,
rawInput: { question: request.question },
},
options,
},
requestController.signal,
).then(
value => {
resolveRequest(value);
},
error => {
rejectRequest(error);
},
);
const askTimeoutError = Object.assign(new Error("ask timed out"), { code: GJC_ASK_TIMEOUT_CODE });
let response: unknown;
try {
response = await requestPromise;
} catch (error) {
if (requestController.signal.aborted && !signal?.aborted) throw askTimeoutError;
throw error;
} finally {
if (timeoutTimer !== undefined) clearTimeout(timeoutTimer);
signal?.removeEventListener("abort", onRequestAbort);
}
if (signal?.aborted) return undefined;
// The configured ask timeout elapsed with no answer: throw the marked
// timeout error so the ask tool distinguishes it from a genuine
// cancellation (which must never auto-select) and its own
// auto-selection-on-timeout policy stays authoritative.
if (requestController.signal.aborted) throw askTimeoutError;
if (!isRecord(response)) return undefined;
// ACP clients (e.g. Paseo) return `{ outcome: { outcome, optionId } }`;
// accept the flat legacy shape as well.
const outcome = isRecord(response.outcome) ? response.outcome : response;
if (outcome.outcome === "cancelled") return undefined;
if (outcome.outcome !== "selected" || typeof outcome.optionId !== "string") return undefined;
const interaction = choices.get(outcome.optionId);
if (!interaction) return undefined;
if (interaction.kind === "value") return interaction.value;
let settled: Promise<AskSettlementResult> | undefined;
return {
source: "remote",
interaction,
settle(settlement) {
if (!settled) {
settled = Promise.resolve(
settlement.kind === "commit"
? { kind: "committed", ack: { status: "failed", reason: "unsupported" } }
: settlement.kind === "invalid"
? { kind: "invalid_closed" }
: { kind: "resolved_without_commit" },
);
}
return settled;
},
};
};
return {
async awaitAnswer(question, options, signal) {
const answer = await awaitAnswerRequest({ question, options, interaction: "selector", controls: [] }, signal);
if (!answer || typeof answer === "string") return answer;
return answer.interaction.kind === "value" ? answer.interaction.value : undefined;
},
awaitAnswerRequest,
};
}
/** Register the interactive `ask` answer source for a session (the ask tool
* races the local UI against a remote reply). Returns the deregister disposer. */
function registerInteractiveAnswerSource(
Expand Down Expand Up @@ -3631,6 +3788,17 @@ export function createNotificationsExtension(
const revisions = new RevisionStore(id, Date.now, { storageDir: stateRoot });
let host: SessionSdkHost | undefined;
let disposeUiAnswerSource: (() => void) | undefined;
let disposePermissionAnswerSource: (() => void) | undefined;
let permissionCapabilityActive = false;
const installPermissionAnswerSource = () => {
if (disposeUiAnswerSource || disposePermissionAnswerSource) return;
disposePermissionAnswerSource = registerAskAnswerSource(
id,
createSdkPermissionAskAnswerSource(
async (params, signal) => await host!.reverse.request("permission", "request", params, signal),
),
);
};
const installProviderDefinitions = (capability: string, definitions: unknown) => {
validateProviderDefinitions(capability, definitions);
if (capability === "permission") {
Expand Down Expand Up @@ -3658,6 +3826,11 @@ export function createNotificationsExtension(
};
throw new Error("permission provider returned an invalid response");
});
permissionCapabilityActive = true;
// Clients without ACP form elicitation (e.g. Paseo) still surface
// workflow-gate asks through the permission channel; a client that
// advertises `elicitation.form` keeps the richer `ui` source instead.
installPermissionAnswerSource();
return;
}
if (capability === "ui") {
Expand All @@ -3668,6 +3841,8 @@ export function createNotificationsExtension(
async (params, signal) => await host!.reverse.request("ui", "ui.elicit", params, signal),
),
);
disposePermissionAnswerSource?.();
disposePermissionAnswerSource = undefined;
return;
}
if (capability !== "fs") return;
Expand Down Expand Up @@ -3704,11 +3879,19 @@ export function createNotificationsExtension(
ctx.setSdkClientBridge?.(bridge);
};
const removeProviderDefinitions = (capability: string) => {
if (capability === "permission") ctx.setSdkPermissionProvider?.(undefined);
if (capability === "permission") {
ctx.setSdkPermissionProvider?.(undefined);
permissionCapabilityActive = false;
disposePermissionAnswerSource?.();
disposePermissionAnswerSource = undefined;
}
if (capability === "fs") ctx.setSdkClientBridge?.(undefined);
if (capability === "ui") {
disposeUiAnswerSource?.();
disposeUiAnswerSource = undefined;
// The permission lease may still be live; restore its ask source so
// later headless asks keep an ACP answer channel.
if (permissionCapabilityActive) installPermissionAnswerSource();
}
};

Expand Down
3 changes: 3 additions & 0 deletions packages/coding-agent/src/tools/ask-answer-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
* source; registering returns a disposer.
*/

/** Error code a remote ask source uses to signal that its own timeout fired. */
export const GJC_ASK_TIMEOUT_CODE = "gjc.ask.timeout";

import { logger } from "@gajae-code/utils";
import type { WorkflowGateEmitter } from "../modes/shared/agent-wire/workflow-gate-broker";
import type { AskAnswerSource } from "./index";
Expand Down
Loading
Loading