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
138 changes: 109 additions & 29 deletions apps/server/integration/providerService.integration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Path from "effect/Path";
import * as Queue from "effect/Queue";
import * as Ref from "effect/Ref";
import * as Stream from "effect/Stream";

import { ProviderAdapterRegistry } from "../src/provider/Services/ProviderAdapterRegistry.ts";
Expand Down Expand Up @@ -54,35 +55,58 @@ interface IntegrationFixture {
readonly layer: Layer.Layer<ProviderService, unknown, never>;
}

const makeIntegrationFixture = Effect.gen(function* () {
const cwd = yield* makeWorkspaceDirectory;
const harness = yield* makeTestProviderAdapterHarness();

const registry = makeAdapterRegistryMock({
[ProviderDriverKind.make("codex")]: harness.adapter,
});
interface RecordedAnalyticsEvent {
readonly event: string;
readonly properties: Readonly<Record<string, unknown>> | undefined;
}

const directoryLayer = ProviderSessionDirectoryLive.pipe(
Layer.provide(ProviderSessionRuntime.layer),
/**
* Analytics layer that keeps captured events in memory so tests can assert on
* telemetry payloads. `AnalyticsService.layerTest` discards them.
*/
const makeRecordingAnalytics = Effect.gen(function* () {
const recorded = yield* Ref.make<ReadonlyArray<RecordedAnalyticsEvent>>([]);
const layer = Layer.succeed(
AnalyticsService,
AnalyticsService.of({
record: (event, properties) =>
Ref.update(recorded, (current) => [...current, { event, properties }]),
flush: Effect.void,
}),
);

const shared = Layer.mergeAll(
directoryLayer,
Layer.succeed(ProviderAdapterRegistry, registry),
ServerSettingsService.layerTest(DEFAULT_SERVER_SETTINGS),
AnalyticsService.layerTest,
Layer.succeed(ProviderEventLoggers, NoOpProviderEventLoggers),
).pipe(Layer.provide(SqlitePersistenceMemory));

const layer = makeProviderServiceLive().pipe(Layer.provide(shared));

return {
cwd,
harness,
layer,
} satisfies IntegrationFixture;
return { layer, get: Ref.get(recorded) } as const;
});

const makeIntegrationFixture = (options?: { readonly analytics?: Layer.Layer<AnalyticsService> }) =>
Effect.gen(function* () {
const cwd = yield* makeWorkspaceDirectory;
const harness = yield* makeTestProviderAdapterHarness();

const registry = makeAdapterRegistryMock({
[ProviderDriverKind.make("codex")]: harness.adapter,
});

const directoryLayer = ProviderSessionDirectoryLive.pipe(
Layer.provide(ProviderSessionRuntime.layer),
);

const shared = Layer.mergeAll(
directoryLayer,
Layer.succeed(ProviderAdapterRegistry, registry),
ServerSettingsService.layerTest(DEFAULT_SERVER_SETTINGS),
options?.analytics ?? AnalyticsService.layerTest,
Layer.succeed(ProviderEventLoggers, NoOpProviderEventLoggers),
).pipe(Layer.provide(SqlitePersistenceMemory));

const layer = makeProviderServiceLive().pipe(Layer.provide(shared));

return {
cwd,
harness,
layer,
} satisfies IntegrationFixture;
});

const collectEventsDuring = <A, E, R>(
stream: Stream.Stream<ProviderRuntimeEvent>,
count: number,
Expand Down Expand Up @@ -126,7 +150,7 @@ const runTurn = (input: {

it.live("replays typed runtime fixture events", () =>
Effect.gen(function* () {
const fixture = yield* makeIntegrationFixture;
const fixture = yield* makeIntegrationFixture();

yield* Effect.gen(function* () {
const provider = yield* ProviderService;
Expand Down Expand Up @@ -161,7 +185,7 @@ it.live("replays typed runtime fixture events", () =>

it.live("replays file-changing fixture turn events", () =>
Effect.gen(function* () {
const fixture = yield* makeIntegrationFixture;
const fixture = yield* makeIntegrationFixture();
const { join } = yield* Path.Path;
const { writeFileString } = yield* FileSystem.FileSystem;

Expand Down Expand Up @@ -198,7 +222,7 @@ it.live("replays file-changing fixture turn events", () =>

it.live("runs multi-turn tool/approval flow", () =>
Effect.gen(function* () {
const fixture = yield* makeIntegrationFixture;
const fixture = yield* makeIntegrationFixture();
const { join } = yield* Path.Path;
const { writeFileString } = yield* FileSystem.FileSystem;

Expand Down Expand Up @@ -250,7 +274,7 @@ it.live("runs multi-turn tool/approval flow", () =>

it.live("rolls back provider conversation state only", () =>
Effect.gen(function* () {
const fixture = yield* makeIntegrationFixture;
const fixture = yield* makeIntegrationFixture();
const { join } = yield* Path.Path;
const { writeFileString, readFileString } = yield* FileSystem.FileSystem;

Expand Down Expand Up @@ -302,3 +326,59 @@ it.live("rolls back provider conversation state only", () =>
}).pipe(Effect.provide(fixture.layer));
}).pipe(Effect.provide(NodeServices.layer)),
);

it.live("reports runtime mode per turn and on mode transitions", () =>
Effect.gen(function* () {
const analytics = yield* makeRecordingAnalytics;
const fixture = yield* makeIntegrationFixture({ analytics: analytics.layer });
const threadId = ThreadId.make("thread-integration-runtime-mode");

yield* Effect.gen(function* () {
const provider = yield* ProviderService;
const startSession = (runtimeMode: "approval-required" | "full-access") =>
provider.startSession(threadId, {
threadId,
provider: ProviderDriverKind.make("codex"),
providerInstanceId: codexInstanceId,
cwd: fixture.cwd,
runtimeMode,
});

yield* startSession("approval-required");
yield* runTurn({
provider,
harness: fixture.harness,
threadId,
userText: "supervised turn",
response: { events: codexTurnTextFixture },
});

// Toggling the mode restarts the session, which is the only place the
// transition is observable.
yield* startSession("full-access");
yield* runTurn({
provider,
harness: fixture.harness,
threadId,
userText: "full access turn",
response: { events: codexTurnTextFixture },
});

const recorded = yield* analytics.get;

assert.deepEqual(
recorded
.filter((entry) => entry.event === "provider.turn.sent")
.map((entry) => entry.properties?.runtimeMode),
["approval-required", "full-access"],
);

assert.deepEqual(
recorded
.filter((entry) => entry.event === "provider.runtime_mode.changed")
.map((entry) => [entry.properties?.from, entry.properties?.to]),
[["approval-required", "full-access"]],
);
}).pipe(Effect.provide(fixture.layer));
}).pipe(Effect.provide(NodeServices.layer)),
);
20 changes: 20 additions & 0 deletions apps/server/src/provider/Layers/ProviderService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,7 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
adapter,
instanceId,
threadId: input.threadId,
runtimeMode: binding.runtimeMode,
isActive: true,
} as const;
}
Expand All @@ -468,6 +469,7 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
adapter,
instanceId,
threadId: input.threadId,
runtimeMode: binding.runtimeMode,
isActive: false,
} as const;
}
Expand All @@ -480,6 +482,7 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
adapter: recovered.adapter,
instanceId,
threadId: input.threadId,
runtimeMode: recovered.session.runtimeMode,
isActive: true,
} as const;
});
Expand Down Expand Up @@ -629,6 +632,19 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
input.modelSelection.model.trim().length > 0,
});

// Changing runtime mode restarts the session, so the transition is only
// observable here, by diffing against the mode the previous session for
// this thread was bound to. Recording it separately is what makes the
// "started supervised, switched to full access" funnel answerable.
const previousRuntimeMode = persistedBinding?.runtimeMode;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Medium Layers/ProviderService.ts:639

provider.runtime_mode.changed is emitted by comparing persistedBinding.runtimeMode to the new session's runtime mode without checking that persistedBinding belongs to resolvedInstanceId. When the same thread is started on a different provider instance, the event fires even though the mode difference is due to an instance/provider switch rather than an intentional mode toggle, corrupting the transition analytics. The resume cursor and cwd fallbacks already guard with persistedBinding?.providerInstanceId === resolvedInstanceId; this comparison needs the same guard.

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/server/src/provider/Layers/ProviderService.ts around line 639:

`provider.runtime_mode.changed` is emitted by comparing `persistedBinding.runtimeMode` to the new session's runtime mode without checking that `persistedBinding` belongs to `resolvedInstanceId`. When the same thread is started on a different provider instance, the event fires even though the mode difference is due to an instance/provider switch rather than an intentional mode toggle, corrupting the transition analytics. The resume cursor and cwd fallbacks already guard with `persistedBinding?.providerInstanceId === resolvedInstanceId`; this comparison needs the same guard.

if (previousRuntimeMode !== undefined && previousRuntimeMode !== input.runtimeMode) {
yield* analytics.record("provider.runtime_mode.changed", {
provider: sessionWithInstance.provider,
from: previousRuntimeMode,
to: input.runtimeMode,
});
}

return sessionWithInstance;
}).pipe(
withMetrics({
Expand Down Expand Up @@ -703,6 +719,10 @@ const makeProviderService = Effect.fn("makeProviderService")(function* (
provider: routed.adapter.provider,
model: input.modelSelection?.model,
interactionMode: input.interactionMode,
// Session-start events alone skew runtime mode toward users who toggle
// often, since every toggle restarts the session. Recording it per turn
// gives a usage-weighted view and lets it cross with interactionMode.
runtimeMode: routed.runtimeMode,
attachmentCount: input.attachments.length,
hasInput: typeof input.input === "string" && input.input.trim().length > 0,
});
Expand Down
Loading