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
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import * as Stream from "effect/Stream";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Option from "effect/Option";
import * as Path from "effect/Path";
import * as Schema from "effect/Schema";
import * as HttpClient from "effect/http/HttpClient";
Expand Down Expand Up @@ -471,13 +472,15 @@ describe("AcpRegistryAdapterV2", () => {
const openWithStoredPicks = Effect.fn("openWithStoredPicks")(function* (input: {
readonly setup: unknown;
readonly pickFrames: ReadonlyArray<Frame>;
readonly agentId?: string;
readonly customModels?: ReadonlyArray<string>;
readonly storedModel?: string;
readonly storedModePick?: string;
readonly turnModels?: ReadonlyArray<string>;
readonly rejectedModel?: string;
readonly storedOptions?: ReadonlyArray<{ readonly id: string; readonly value: string }>;
readonly failsConfiguration?: boolean;
readonly liveConfiguration?: AcpRegistryLiveConfiguration;
}) {
const spawner = yield* ChildProcessSpawner.ChildProcessSpawner;
// Scope release gives the replay agent at most a second to exit, so the status
Expand Down Expand Up @@ -524,7 +527,7 @@ describe("AcpRegistryAdapterV2", () => {
selfInvocation: yield* resolveSelfInvocation(),
instanceId,
settings: yield* decodeAcpRegistryAdapterSettings({
agentId: "fixture-agent",
agentId: input.agentId ?? "fixture-agent",
authMethodId: "test",
customModels: input.customModels ?? [],
}),
Expand All @@ -545,6 +548,8 @@ describe("AcpRegistryAdapterV2", () => {
layerInjectedRuntimeCatalog,
Layer.mock(AcpRegistryRuntimeCoordinator.AcpRegistryRuntimeCoordinator)({
withForegroundStartup: (_startupKey, effect) => effect,
getLiveConfiguration: () =>
Effect.sync(() => Option.fromUndefinedOr(published ?? input.liveConfiguration)),
publishLiveConfiguration: (_instanceId, configuration) =>
Effect.sync(() => {
published = configuration;
Expand Down Expand Up @@ -799,6 +804,85 @@ describe("AcpRegistryAdapterV2", () => {
}).pipe(Effect.provide(layerTest), Effect.scoped),
);

// Copilot's session/new reports its built-in default, not the model it will run
// (github/copilot-cli#5363), so the selected model must be written even when it matches,
// and an unset effort gets the value the composer shows.
it.effect(
"writes the selected Copilot model and the shown effort when session/new reports defaults",
() => {
const copilotModel = (currentValue: string) => ({
...modelOption("model", currentValue),
options: ["claude-sonnet-5.5", "claude-haiku-4.5"].map((value) => ({
value,
name: value,
})),
});
const effort = (currentValue: string) => ({
id: "reasoning_effort",
name: "Reasoning effort",
category: "thought_level",
type: "select",
currentValue,
options: ["medium", "xhigh"].map((value) => ({ value, name: value })),
});
return openWithStoredPicks({
agentId: "github-copilot-cli",
setup: { configOptions: [copilotModel("claude-sonnet-5.5"), effort("medium")] },
storedModel: "claude-sonnet-5.5",
liveConfiguration: {
models: [],
currentModelId: null,
configOptions: [
{
id: "reasoning_effort",
label: "Reasoning effort",
type: "select",
role: "reasoning",
currentValue: "xhigh",
options: [
{ id: "medium", label: "medium" },
{ id: "xhigh", label: "xhigh" },
],
},
],
},
pickFrames: [
// The new session's own update replaces the shared live configuration.
{
type: "emit_inbound",
frame: {
kind: "notification",
method: "session/update",
params: {
sessionId: "agent-session",
update: {
sessionUpdate: "config_option_update",
configOptions: [copilotModel("claude-sonnet-5.5"), effort("medium")],
},
},
},
},
outbound("session/set_config_option", {
sessionId: "agent-session",
configId: "model",
value: "claude-sonnet-5.5",
}),
answer("session/set_config_option", {
configOptions: [copilotModel("claude-sonnet-5.5"), effort("medium")],
}),
outbound("session/set_config_option", {
sessionId: "agent-session",
configId: "reasoning_effort",
value: "xhigh",
}),
answer("session/set_config_option", {
configOptions: [copilotModel("claude-sonnet-5.5"), effort("xhigh")],
}),
],
}).pipe(Effect.provide(layerTest), Effect.scoped);
},
);

it.effect.each([
"config",
"legacy",
Expand Down
24 changes: 24 additions & 0 deletions packages/provider-acp-registry/src/server/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,8 @@ export const makeAcpRegistryAdapterV2 = Effect.fn("makeAcpRegistryAdapterV2")(fu
normalizeSessionUpdate: copilotRouting.normalizeSessionUpdate,
extractSubagentUpdate: extractCopilotSubagentUpdate,
restartRuntimeAfterPromptError: isCopilotLostSessionError,
// session/new reports a built-in default, not the model Copilot runs (github/copilot-cli#5363).
alwaysSetModel: true,
}),
makeRuntime: options.makeRuntime ?? makeAcpRegistryRuntime(runtimeOptions, catalog),
...(runtimeCoordinator === undefined
Expand All @@ -268,6 +270,28 @@ export const makeAcpRegistryAdapterV2 = Effect.fn("makeAcpRegistryAdapterV2")(fu
options.instanceId,
normalizeAcpRegistryCommands(commands),
),
// A new Copilot session starts at its own effort, not the one the composer shows.
...(isCopilot
? {
unsetOptionDefaults: runtimeCoordinator
.getLiveConfiguration(options.instanceId)
.pipe(
Effect.map(
Option.match({
onNone: () => [],
onSome: ({ configOptions }) =>
configOptions.flatMap((descriptor) =>
descriptor.type === "select" &&
descriptor.role === "reasoning" &&
descriptor.currentValue !== undefined
? [{ id: descriptor.id, value: descriptor.currentValue }]
: [],
),
}),
),
),
}
: {}),
onSessionConfigurationUpdate: (configOptions, modeState, modelState) =>
runtimeCoordinator.publishLiveConfiguration(
options.instanceId,
Expand Down
17 changes: 14 additions & 3 deletions packages/provider-acp/src/server/AcpSessionRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1186,7 +1186,11 @@ export class AcpSessionRuntime extends Context.Service<
readonly setConfigOption: (
configId: string,
value: string | boolean,
options?: { readonly allowUnlistedValue?: boolean },
options?: {
readonly allowUnlistedValue?: boolean;
/** Sends the write even when the cached value already matches it. */
readonly resendCurrentValue?: boolean;
},
) => Effect.Effect<EffectAcpSchema.SetSessionConfigOptionResponse, EffectAcpErrors.AcpError>;
readonly setModel: (model: string) => Effect.Effect<void, EffectAcpErrors.AcpError>;
readonly request: (
Expand Down Expand Up @@ -2031,15 +2035,22 @@ export const make = Effect.fn("AcpSessionRuntime.make")(function* (
const setConfigOption = (
configId: string,
value: string | boolean,
options?: { readonly allowUnlistedValue?: boolean },
options?: {
readonly allowUnlistedValue?: boolean;
readonly resendCurrentValue?: boolean;
},
): Effect.Effect<EffectAcpSchema.SetSessionConfigOptionResponse, EffectAcpErrors.AcpError> =>
validateConfigOptionValue(configId, value, options?.allowUnlistedValue === true).pipe(
Effect.andThen(getStartedState),
Effect.flatMap((started) =>
SubscriptionRef.get(configOptionsRef).pipe(
Effect.flatMap((configOptions) => {
const existing = findSessionConfigOption(configOptions, configId);
if (existing && configOptionCurrentValueMatches(existing, value)) {
if (
existing &&
options?.resendCurrentValue !== true &&
configOptionCurrentValueMatches(existing, value)
) {
return Effect.succeed({
configOptions,
} satisfies EffectAcpSchema.SetSessionConfigOptionResponse);
Expand Down
40 changes: 31 additions & 9 deletions packages/provider-acp/src/server/adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,10 @@ export interface AcpAdapterV2Flavor {
ChildProcessSpawner.ChildProcessSpawner | SecureRandom.SecureRandom | Scope.Scope
>;
readonly customModels?: ReadonlyArray<string>;
/** The agent may run another model than session/new reports, so the model is always written. */
readonly alwaysSetModel?: boolean;
/** Values the client shows for options a new session's selection leaves unset. */
readonly unsetOptionDefaults?: Effect.Effect<NonNullable<ModelSelection["options"]>>;
readonly resolveModelId?: (selection: ModelSelection) => string | undefined;
/**
* The agent advertises its model option only after `session/new` returns
Expand Down Expand Up @@ -7155,6 +7159,8 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* (
},
);

// Read before the session exists: its own config updates replace what the client showed.
const unsetOptionDefaults = yield* flavor.unsetOptionDefaults ?? Effect.succeed([]);
const initialStart = yield* Effect.result(
startAcpRuntime(input.threadId, input.initialNativeThreadId),
);
Expand Down Expand Up @@ -7253,6 +7259,7 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* (
startResult: AcpSessionRuntime.AcpSessionRuntimeStartResult,
modelSelection: ModelSelection,
runtimePolicy: ProviderAdapter.ProviderAdapterV2RuntimePolicy,
unsetOptionDefaults: NonNullable<ModelSelection["options"]> = [],
) {
yield* Ref.set(activeSelection, null);
yield* validateModelSelection(modelSelection);
Expand Down Expand Up @@ -7288,7 +7295,9 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* (
)
) {
yield* runtime
.setConfigOption(modelOption.id, requestedModel)
.setConfigOption(modelOption.id, requestedModel, {
resendCurrentValue: flavor.alwaysSetModel === true,
})
.pipe(Effect.mapError(rejectedModel));
appliedModel = requestedModel;
}
Expand Down Expand Up @@ -7343,13 +7352,21 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* (
: undefined;
return granted !== undefined && !isRuntimeModeAbove(granted, ceiling);
};
const optionSelections = (modelSelection.options ?? []).flatMap((selection) => {
if (flavor.ownedModelOptionIds?.includes(selection.id) === true) return [];
if (!withinCeiling(selection)) return [];
return [
selection.value === ACP_EMPTY_OPTION_VALUE ? { ...selection, value: "" } : selection,
];
});
const explicitSelections = modelSelection.options ?? [];
const implicitSelections = unsetOptionDefaults.filter(
(implicit) => !explicitSelections.some((selection) => selection.id === implicit.id),
);
const optionSelections = [...explicitSelections, ...implicitSelections].flatMap(
(selection) => {
if (flavor.ownedModelOptionIds?.includes(selection.id) === true) return [];
if (!withinCeiling(selection)) return [];
return [
selection.value === ACP_EMPTY_OPTION_VALUE
? { ...selection, value: "" }
: selection,
];
},
);
const availableConfigIds = new Set(configOptions.map((option) => option.id));
const hasNativeConfigWithSyntheticModeId = availableConfigIds.has(
ACP_SESSION_MODE_OPTION_ID,
Expand Down Expand Up @@ -7501,7 +7518,12 @@ export const makeAcpAdapterV2 = Effect.fn("makeAcpAdapterV2")(function* (
);
});

yield* configureSession(started, input.modelSelection, input.runtimePolicy);
yield* configureSession(
started,
input.modelSelection,
input.runtimePolicy,
started.sessionId !== input.initialNativeThreadId ? unsetOptionDefaults : [],
);
yield* Ref.set(activeSelection, input.modelSelection);
yield* Ref.set(activeInteractionMode, input.runtimePolicy.interactionMode);
const createdAt = yield* DateTime.now;
Expand Down
Loading