Skip to content

Commit a5352a8

Browse files
committed
fix(execution): address observer review feedback
1 parent a74821d commit a5352a8

3 files changed

Lines changed: 63 additions & 7 deletions

File tree

packages/core/api/src/server/execution-stack.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,10 +110,6 @@ export const makeExecutionStack = <
110110
const codeExecutor = yield* CodeExecutorProvider;
111111
const { decorate } = yield* EngineDecorator;
112112

113-
// Compose every registered plugin's runtime.executionObserver, bound to the
114-
// executor's own extensions. Resolves to a no-op when no plugin observes,
115-
// so a stack with no history/metrics plugin is unaffected. `plugins()` is
116-
// the erased AnyPlugin[]; the caller's TPlugins phantom recovers the tuple.
117113
const { plugins } = yield* PluginsProvider;
118114
// PluginsProvider erases the tuple to AnyPlugin[]; recover the caller's
119115
// TPlugins phantom so the extensions arg (the executor) lines up.

packages/core/execution/src/engine-observer.test.ts

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { describe, expect, it } from "@effect/vitest";
2-
import { Effect, Predicate } from "effect";
2+
import { Effect, Predicate, Schema } from "effect";
33

4-
import { createExecutor, definePlugin } from "@executor-js/sdk";
4+
import { createExecutor, definePlugin, ElicitationResponse, tool } from "@executor-js/sdk";
55
import type { ExecutionEvent, ExecutionObserver } from "@executor-js/sdk";
66
import { makeTestConfig } from "@executor-js/sdk/testing";
77
import type { CodeExecutor, ExecuteResult } from "@executor-js/codemode-core";
@@ -14,8 +14,32 @@ const emptyPlugin = definePlugin(() => ({
1414
staticSources: () => [],
1515
}));
1616

17+
const approvalPlugin = definePlugin(() => ({
18+
id: "observer-approval-test" as const,
19+
storage: () => ({}),
20+
staticSources: () => [
21+
{
22+
id: "approval.ctl",
23+
kind: "control" as const,
24+
name: "Approval Ctl",
25+
tools: [
26+
tool({
27+
name: "run",
28+
description: "Requires approval",
29+
annotations: { requiresApproval: true } as const,
30+
inputSchema: Schema.toStandardSchemaV1(Schema.toStandardJSONSchemaV1(Schema.Struct({}))),
31+
execute: () => Effect.succeed("ran"),
32+
}),
33+
],
34+
},
35+
],
36+
}));
37+
1738
const makeExecutor = () => createExecutor(makeTestConfig({ plugins: [emptyPlugin()] as const }));
1839

40+
const makeApprovalExecutor = () =>
41+
createExecutor(makeTestConfig({ plugins: [approvalPlugin()] as const }));
42+
1943
// A code executor that issues one builtin tool call (tools.search) and then
2044
// completes, enough to exercise the full event sequence.
2145
const toolCallingExecutor: CodeExecutor = {
@@ -25,6 +49,13 @@ const toolCallingExecutor: CodeExecutor = {
2549
.pipe(Effect.as({ result: "ok", logs: [] } satisfies ExecuteResult), Effect.orDie),
2650
};
2751

52+
const approvalCallingExecutor: CodeExecutor = {
53+
execute: (_code, invoker) =>
54+
invoker
55+
.invoke({ path: "approval.ctl.run", args: {} })
56+
.pipe(Effect.as({ result: "ok", logs: [] } satisfies ExecuteResult), Effect.orDie),
57+
};
58+
2859
const collectingObserver = () => {
2960
const events: ExecutionEvent[] = [];
3061
const observer: ExecutionObserver = {
@@ -70,6 +101,35 @@ describe("execution engine observer emission", () => {
70101
}),
71102
);
72103

104+
it.effect("emits inline interaction events when execute handles elicitation", () =>
105+
Effect.gen(function* () {
106+
const executor = yield* makeApprovalExecutor();
107+
const { events, observer } = collectingObserver();
108+
const engine = createExecutionEngine({
109+
executor,
110+
codeExecutor: approvalCallingExecutor,
111+
observer,
112+
});
113+
114+
const result = yield* engine.execute("noop", {
115+
trigger: { kind: "test" },
116+
onElicitation: () => Effect.succeed(ElicitationResponse.make({ action: "accept" })),
117+
});
118+
expect(result.result).toBe("ok");
119+
120+
const started = events.find((e) => Predicate.isTagged(e, "ExecutionStarted"));
121+
const interactionStarted = events.find((e) => Predicate.isTagged(e, "InteractionStarted"));
122+
const interactionResolved = events.find((e) => Predicate.isTagged(e, "InteractionResolved"));
123+
124+
expect(interactionStarted?.executionId).toBe(started?.executionId);
125+
expect(interactionResolved?.executionId).toBe(started?.executionId);
126+
expect(interactionResolved?.interactionId).toBe(interactionStarted?.interactionId);
127+
expect(interactionStarted?.context.request.message).toContain("approval");
128+
expect(interactionResolved?.status).toBe("accepted");
129+
expect(interactionResolved?.response?.action).toBe("accept");
130+
}),
131+
);
132+
73133
it.effect("does nothing observable when no observer is configured", () =>
74134
Effect.gen(function* () {
75135
const executor = yield* makeExecutor();

packages/core/sdk/src/execution-observer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ const logExecutionObserverFailure = (
107107
): Effect.Effect<void> =>
108108
Effect.logWarning("execution observer failed", {
109109
cause: Cause.pretty(cause),
110-
event: event.constructor.name,
110+
event: event._tag,
111111
...(pluginId ? { pluginId } : {}),
112112
});
113113

0 commit comments

Comments
 (0)