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 @@ -15,6 +15,7 @@ import { ChildProcessSpawner } from "effect/unstable/process";
import * as EffectAcpErrors from "effect-acp/errors";

import * as ServerConfig from "../../config.ts";
import * as ServerSettings from "../../serverSettings.ts";
import type {
AcpRegistryAvailableCommands,
AcpRegistryLiveConfiguration,
Expand Down Expand Up @@ -87,6 +88,7 @@ const testLayer = Layer.mergeAll(
IdAllocator.layer,
serverConfigLayer,
registryLayer,
ServerSettings.layerTest(),
);

describe("AcpRegistryAdapterV2", () => {
Expand Down Expand Up @@ -121,6 +123,8 @@ describe("AcpRegistryAdapterV2", () => {
assert.isTrue(BUILT_IN_PROVIDER_ADAPTER_DRIVER_KINDS_V2.has(ACP_REGISTRY_PROVIDER));
assert.equal(AcpRegistryAdapterV2Driver.driverKind, ACP_REGISTRY_PROVIDER);
assert.deepEqual(AcpRegistryAdapterV2Driver.defaultConfig(), {
source: "registry",
commandArgs: [],
enabled: true,
agentId: "",
commandPath: "",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,14 +183,17 @@ function makeAcpRegistryRuntime(options: AcpRegistryAdapterV2Options) {

export function makeAcpRegistryAdapterV2(options: AcpRegistryAdapterV2Options) {
const runtimeCoordinator = options.runtimeCoordinator;
const isDevin = options.settings.agentId === "devin";
const registryAgentId = options.settings.source === "local" ? "" : options.settings.agentId;
const startupKey =
options.settings.source === "local" ? `local:${options.instanceId}` : registryAgentId;
const isDevin = registryAgentId === "devin";
const flavor: AcpAdapterV2Flavor = {
driver: ACP_REGISTRY_PROVIDER,
capabilities: AcpProviderCapabilitiesV2,
promptFailure: (cause) => acpRegistryPromptFailure(options.settings.agentId, cause),
promptFailure: (cause) => acpRegistryPromptFailure(registryAgentId, cause),
// Per-agent exceptions (Mistral Vibe, Devin): see the note above
// registerMistralVibeAcpExtensions before adding any more.
...(options.settings.agentId === "mistral-vibe"
...(registryAgentId === "mistral-vibe"
? { registerExtensions: registerMistralVibeAcpExtensions }
: {}),
...(isDevin
Expand Down Expand Up @@ -230,7 +233,7 @@ export function makeAcpRegistryAdapterV2(options: AcpRegistryAdapterV2Options) {
});
},
withRuntimeStartup: <A, E, R>(effect: Effect.Effect<A, E, R>) =>
runtimeCoordinator.withForegroundStartup(options.settings.agentId, effect),
runtimeCoordinator.withForegroundStartup(startupKey, effect),
}),
...(options.assertComplete === undefined ? {} : { assertComplete: options.assertComplete }),
};
Expand Down
105 changes: 63 additions & 42 deletions apps/server/src/provider/Drivers/AcpRegistryDriver.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,19 @@ describe("acpRegistrySnapshotReadiness", () => {
}),
).toMatchObject({ installed: false, version: "1.2.3", status: "error" });

expect(
acpRegistrySnapshotReadiness({
status: "missing_runner",
version: null,
distribution: "local",
}),
).toEqual({
installed: false,
version: null,
status: "error",
message: "Local ACP executable is not available on this environment's PATH.",
});

expect(
acpRegistrySnapshotReadiness({
status: "unprepared",
Expand Down Expand Up @@ -312,49 +325,57 @@ describe("acpRegistrySnapshotReadiness", () => {
expect(snapshot.models[0]?.isDefault).toBe(true);
});

it("reports failed authentication without hiding successful local inspection", () => {
const snapshot = buildCheckedAcpRegistrySnapshot({
...identity,
settings: decodeSettings({ agentId: "test-agent", authMethodId: "grok-login" }),
checkedAt: "2026-08-13T10:00:00.000Z",
inspection: {
status: "ready",
agentId: "test-agent",
version: "1.0.0",
distribution: "binary",
},
probeError: new AcpRegistryOperationError({
reason: "authentication_failed",
message: "Login required.",
authMethods: [
{
id: "api-key",
name: "API key",
description: null,
type: "env_var",
},
{
id: "grok-login",
name: "Log in with Grok",
description: null,
type: "agent",
},
],
}),
});
it.each(["registry", "local"] as const)(
"reports failed authentication after successful %s inspection",
(source) => {
const snapshot = buildCheckedAcpRegistrySnapshot({
...identity,
settings: decodeSettings({
source,
...(source === "local" ? { commandPath: "test-agent" } : { agentId: "test-agent" }),
authMethodId: "grok-login",
}),
checkedAt: "2026-08-13T10:00:00.000Z",
inspection: {
status: "ready",
agentId: "test-agent",
version: source === "local" ? null : "1.0.0",
distribution: source === "local" ? "local" : "binary",
},
probeError: new AcpRegistryOperationError({
reason: "authentication_failed",
message: "Login required.",
authMethods: [
{
id: "api-key",
name: "API key",
description: null,
type: "env_var",
},
{
id: "grok-login",
name: "Log in with Grok",
description: null,
type: "agent",
},
],
}),
});

expect(snapshot).toMatchObject({
installed: true,
version: "1.0.0",
status: "warning",
auth: {
status: "unauthenticated",
type: "agent",
label: "Log in with Grok",
},
message: 'Sign in in provider settings using "Log in with Grok".',
});
});
expect(snapshot).toMatchObject({
installed: true,
version: source === "local" ? null : "1.0.0",
status: "warning",
setup: { canAuthenticate: true },
auth: {
status: "unauthenticated",
type: "agent",
label: "Log in with Grok",
},
message: 'Sign in in provider settings using "Log in with Grok".',
});
},
);

it.effect("runs the disposable probe only after local inspection is ready", () =>
Effect.gen(function* () {
Expand Down
23 changes: 17 additions & 6 deletions apps/server/src/provider/Drivers/AcpRegistryDriver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,8 @@ export function acpRegistrySnapshotReadiness(
installed: false,
version: null,
status: "warning",
message: "Select an ACP Registry agent before starting a thread.",
message:
"Select an ACP Registry agent or configure a local ACP executable before starting a thread.",
};
case "not_found":
return {
Expand All @@ -159,7 +160,10 @@ export function acpRegistrySnapshotReadiness(
installed: false,
version: inspection.version,
status: "error",
message: `ACP Registry agent '${inspection.agentId}' requires '${inspection.runner}' on this environment's PATH.`,
message:
inspection.distribution === "local"
? "Local ACP executable is not available on this environment's PATH."
: `ACP executable '${inspection.runner}' is not available on this environment's PATH.`,
};
case "unprepared":
return {
Expand Down Expand Up @@ -196,11 +200,14 @@ function baseSnapshot(
readonly documentationUrl?: string;
readonly message?: string;
readonly probe?: AcpRegistryConfigurationProbeResult;
readonly probeError?: AcpRegistryOperationError;
},
): ServerProvider {
const iconUrl =
resolveOfficialAcpRegistryIconUrl(input.probe?.probe.icon) ??
officialAcpRegistryIconUrlForAgentId(input.settings.agentId);
input.settings.source === "local"
? null
: (resolveOfficialAcpRegistryIconUrl(input.probe?.probe.icon) ??
officialAcpRegistryIconUrlForAgentId(input.settings.agentId));
return {
instanceId: input.instanceId,
driver: DRIVER_KIND,
Expand All @@ -225,7 +232,9 @@ function baseSnapshot(
input.installed &&
(input.probe
? input.probe.probe.authMethods.length > 0
: input.settings.agentId.length > 0),
: input.settings.source === "local"
? (input.probeError?.authMethods?.length ?? 0) > 0
: input.settings.agentId.length > 0),
},
...(input.message ? { message: input.message } : {}),
models: modelsFromDiscovery(input.probe?.probe, input.settings.customModels),
Expand Down Expand Up @@ -633,7 +642,9 @@ export const AcpRegistryDriver: ProviderDriver<AcpRegistrySettings, AcpRegistryD
const publishEnrichment = (
Option.isSome(runtimeCoordinator)
? runtimeCoordinator.value.runBackgroundProbe(
effectiveConfig.agentId,
effectiveConfig.source === "local"
? `local:${instanceId}`
: effectiveConfig.agentId,
enrichProviderCached(snapshot),
)
: enrichProviderCached(snapshot).pipe(Effect.map(Option.some))
Expand Down
31 changes: 28 additions & 3 deletions apps/server/src/provider/acp/AcpRegistryAuth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,12 @@ const catalog: AcpRegistrySupport.AcpRegistryCatalog["Service"] = {
uninstallManagedBinary: () => Effect.die("unused"),
};

const makeHarness = (method: AcpSchema.AuthMethod, failVerification = false) =>
const makeHarness = (
method: AcpSchema.AuthMethod,
failVerification = false,
settings = decodeSettings({ agentId: "test-agent" }),
providerInstanceId = instanceId,
) =>
Effect.gen(function* () {
const verify = yield* Deferred.make<void>();
const changed: boolean[] = [];
Expand All @@ -74,8 +79,8 @@ const makeHarness = (method: AcpSchema.AuthMethod, failVerification = false) =>
const written: string[] = [];
const sizes: number[][] = [];
const controller = yield* makeAcpRegistryAuth({
instanceId,
settings: decodeSettings({ agentId: "test-agent" }),
instanceId: providerInstanceId,
settings,
cwd: "/workspace",
environment: { PATH: "/tools", OVERRIDE: "base" },
onChanged: (value) =>
Expand Down Expand Up @@ -211,6 +216,26 @@ const makeHarness = (method: AcpSchema.AuthMethod, failVerification = false) =>
};
}).pipe(Effect.provideService(AcpRegistrySupport.AcpRegistryCatalog, catalog));

it.effect("keeps local credentials separate from a matching registry agent ID", () =>
Effect.gen(function* () {
const registry = yield* makeHarness(browserMethod);
const local = yield* makeHarness(
browserMethod,
false,
decodeSettings({ source: "local", commandPath: "/different/agent" }),
ProviderInstanceId.make("test-agent"),
);
assert.deepEqual(registry.controller.credentialBinding, {
owner: "provider",
key: "acp:test-agent",
});
assert.deepEqual(local.controller.credentialBinding, {
owner: "provider",
key: "acp:local:test-agent",
});
}).pipe(Effect.scoped, Effect.provide(NodeServices.layer)),
);

it.effect(
"discovers methods without signing in, then waits for browser consent and session verification",
() =>
Expand Down
12 changes: 6 additions & 6 deletions apps/server/src/provider/acp/AcpRegistryAuth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ export const makeAcpRegistryAuth = Effect.fn("makeAcpRegistryAuth")(function* (o
const coordinator = yield* Effect.serviceOption(
AcpRegistryRuntimeCoordinator.AcpRegistryRuntimeCoordinator,
);
const agentKey =
options.settings.source === "local" ? `local:${options.instanceId}` : options.settings.agentId;
const failure = (operation: string, detail: string, cause?: unknown) =>
new ProviderSetupError({ instanceId: options.instanceId, operation, detail, cause });
const resolve = catalog
Expand Down Expand Up @@ -139,7 +141,7 @@ export const makeAcpRegistryAuth = Effect.fn("makeAcpRegistryAuth")(function* (o

const methods = Option.isSome(coordinator)
? coordinator.value
.runBackgroundProbe(options.settings.agentId, discoverMethods)
.runBackgroundProbe(agentKey, discoverMethods)
.pipe(
Effect.flatMap((result) =>
Option.isSome(result)
Expand Down Expand Up @@ -327,7 +329,7 @@ export const makeAcpRegistryAuth = Effect.fn("makeAcpRegistryAuth")(function* (o
}),
);
yield* Option.isSome(coordinator)
? coordinator.value.withForegroundStartup(options.settings.agentId, login)
? coordinator.value.withForegroundStartup(agentKey, login)
: login;
yield* options.onChanged(true);
});
Expand All @@ -353,15 +355,13 @@ export const makeAcpRegistryAuth = Effect.fn("makeAcpRegistryAuth")(function* (o
}),
);
const signOut = (
Option.isSome(coordinator)
? coordinator.value.withForegroundStartup(options.settings.agentId, logout)
: logout
Option.isSome(coordinator) ? coordinator.value.withForegroundStartup(agentKey, logout) : logout
).pipe(Effect.andThen(options.onChanged(false)));
return yield* ProviderAuthFlow.make({
instanceId: options.instanceId,
// ACP doesn't advertise its credential scope. Conservatively treat all
// instances of the same agent on this environment as sharing credentials.
credentialBinding: { owner: "provider", key: `acp:${options.settings.agentId}` },
credentialBinding: { owner: "provider", key: `acp:${agentKey}` },
methods,
...(options.settings.authMethodId ? { defaultMethodId: options.settings.authMethodId } : {}),
authenticate,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ export const makeAcpRegistryAuthenticationState = Effect.fn("makeAcpRegistryAuth
// Cosmetic settings and model discovery can rebuild the driver without
// changing the account. Credential overrides and profile paths cannot.
const binding = hash({
...(input.settings.source === "local"
? { source: "local", commandArgs: input.settings.commandArgs }
: {}),
agentId: input.settings.agentId,
commandPath: input.settings.commandPath,
distribution: input.settings.distribution,
Expand Down
Loading
Loading