Skip to content

Commit 4ab0876

Browse files
ObliviateRickLinclaude
ObliviateRickLin
and
claude
committed
fix(lobster): don't route schema-default flow fields into TaskFlow (#102011)
Ordinary lobster run/resume calls were misrouted into managed TaskFlow mode when the tool schema injected default flow fields (e.g. flowExpectedRevision=0, an empty flowStateJson): resume failed with "flowId required when using managed TaskFlow resume mode" and run with "flowControllerId required..." / "run action does not accept flowId or flowExpectedRevision". Managed mode is identified only by its intentional flow identifiers — flowId for resume, flowControllerId/flowGoal for run. Secondary fields (flowExpectedRevision, flowStateJson, step fields) that the schema can inject as defaults no longer select managed mode. Real managed flows are unchanged (managed resume still requires flowExpectedRevision; managed run still needs flowControllerId/flowGoal). Co-Authored-By: Claude Fable 5 <[email protected]>
1 parent 5bdea31 commit 4ab0876

2 files changed

Lines changed: 113 additions & 20 deletions

File tree

‎extensions/lobster/src/lobster-tool.test.ts‎

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,67 @@ describe("lobster plugin tool", () => {
234234
expect(mutation.applied).toBe(true);
235235
});
236236

237+
it("ignores a schema-default flowExpectedRevision on a managed run", async () => {
238+
const runner = {
239+
run: vi.fn().mockResolvedValue({
240+
ok: true,
241+
status: "ok",
242+
output: [],
243+
requiresApproval: null,
244+
}),
245+
};
246+
const taskFlow = createFakeTaskFlow();
247+
const tool = createLobsterTool(fakeApi(), { runner, taskFlow });
248+
249+
// The tool schema can inject flowExpectedRevision=0; a managed run must not
250+
// be rejected with "run action does not accept flowExpectedRevision".
251+
const res = await tool.execute("call-managed-run-injected-revision", {
252+
action: "run",
253+
pipeline: "noop",
254+
flowControllerId: "tests/lobster",
255+
flowGoal: "Run Lobster workflow",
256+
flowExpectedRevision: 0,
257+
});
258+
259+
expect(taskFlow.createManaged).toHaveBeenCalled();
260+
const details = requireRecord(res.details, "managed run injected-revision details");
261+
expect(details.ok).toBe(true);
262+
});
263+
264+
// A schema-injected flowStateJson (blank "" or empty "{}") without
265+
// controllerId/goal must not fail JSON parsing or select managed run parsing
266+
// (would otherwise fail "flowControllerId required when using managed TaskFlow
267+
// run mode" or "flowStateJson must be valid JSON").
268+
it.each(["", "{}"])(
269+
"routes run carrying only a schema-default flowStateJson %j as ordinary",
270+
async (flowStateJson) => {
271+
const runner = {
272+
run: vi.fn().mockResolvedValue({
273+
ok: true,
274+
status: "ok",
275+
output: [],
276+
requiresApproval: null,
277+
}),
278+
};
279+
const taskFlow = createFakeTaskFlow();
280+
const tool = createLobsterTool(fakeApi(), { runner, taskFlow });
281+
282+
const res = await tool.execute("call-ordinary-run-injected-state", {
283+
action: "run",
284+
pipeline: "noop",
285+
flowStateJson,
286+
flowExpectedRevision: 0,
287+
});
288+
289+
expect(taskFlow.createManaged).not.toHaveBeenCalled();
290+
expect(runner.run).toHaveBeenCalledWith(
291+
expect.objectContaining({ action: "run", pipeline: "noop" }),
292+
);
293+
const details = requireRecord(res.details, "ordinary run injected-state details");
294+
expect(details.ok).toBe(true);
295+
},
296+
);
297+
237298
it("rejects managed TaskFlow params when no bound taskFlow runtime is available", async () => {
238299
const tool = createLobsterTool(fakeApi(), {
239300
runner: { run: vi.fn() },
@@ -308,6 +369,35 @@ describe("lobster plugin tool", () => {
308369
expect(mutation.applied).toBe(true);
309370
});
310371

372+
it("routes resume carrying only a schema-default flowExpectedRevision as ordinary", async () => {
373+
const runner = {
374+
run: vi.fn().mockResolvedValue({
375+
ok: true,
376+
status: "ok",
377+
output: [],
378+
requiresApproval: null,
379+
}),
380+
};
381+
const taskFlow = createFakeTaskFlow();
382+
const tool = createLobsterTool(fakeApi(), { runner, taskFlow });
383+
384+
// flowExpectedRevision=0 is a tool-schema default; without a flowId this is
385+
// an ordinary resume and must not be routed into managed TaskFlow mode.
386+
const res = await tool.execute("call-ordinary-resume-injected-revision", {
387+
action: "resume",
388+
token: "resume-token",
389+
approve: true,
390+
flowExpectedRevision: 0,
391+
});
392+
393+
expect(taskFlow.resume).not.toHaveBeenCalled();
394+
expect(runner.run).toHaveBeenCalledWith(
395+
expect.objectContaining({ action: "resume", token: "resume-token", approve: true }),
396+
);
397+
const envelope = requireRecord(res.details, "ordinary resume injected-revision details");
398+
expect(envelope.ok).toBe(true);
399+
});
400+
311401
it("normalizes numeric string flowExpectedRevision before managed resume", async () => {
312402
const runner = {
313403
run: vi.fn().mockResolvedValue({

‎extensions/lobster/src/lobster-tool.ts‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -97,8 +97,15 @@ function parseOptionalFlowStateJson(value: unknown): JsonLike | undefined {
9797
if (typeof value !== "string") {
9898
throw new Error("flowStateJson must be a JSON string");
9999
}
100+
// A blank/whitespace flowStateJson is a schema default, not a managed-run
101+
// request; treat it as absent so it neither fails JSON parsing nor selects
102+
// managed run mode on an ordinary call.
103+
const trimmed = value.trim();
104+
if (!trimmed) {
105+
return undefined;
106+
}
100107
try {
101-
return JSON.parse(value) as JsonLike;
108+
return JSON.parse(trimmed) as JsonLike;
102109
} catch {
103110
throw new Error("flowStateJson must be valid JSON");
104111
}
@@ -111,20 +118,21 @@ function parseRunFlowParams(params: Record<string, unknown>): ManagedFlowRunPara
111118
const waitingStep = readOptionalTrimmedString(params.flowWaitingStep, "flowWaitingStep");
112119
const stateJson = parseOptionalFlowStateJson(params.flowStateJson);
113120
const resumeFlowId = readOptionalTrimmedString(params.flowId, "flowId");
114-
const resumeRevision = readOptionalNumber(params.flowExpectedRevision, "flowExpectedRevision");
115121

116-
const hasRunFields =
117-
controllerId !== undefined ||
118-
goal !== undefined ||
119-
currentStep !== undefined ||
120-
waitingStep !== undefined ||
121-
stateJson !== undefined;
122+
// Managed run is identified by the intentional controllerId/goal fields. The
123+
// step and flowStateJson fields can be injected as tool-schema defaults (e.g.
124+
// an empty flowStateJson), so they must not route an ordinary run into
125+
// managed TaskFlow mode.
126+
const hasRunFields = controllerId !== undefined || goal !== undefined;
122127

123128
if (!hasRunFields) {
124129
return null;
125130
}
126-
if (resumeFlowId !== undefined || resumeRevision !== undefined) {
127-
throw new Error("run action does not accept flowId or flowExpectedRevision");
131+
// Only an explicit flowId means the caller confused resume with run.
132+
// flowExpectedRevision is ignored here because the tool schema can inject a
133+
// default (e.g. 0), which must not trip an ordinary managed run.
134+
if (resumeFlowId !== undefined) {
135+
throw new Error("run action does not accept flowId");
128136
}
129137
if (!controllerId) {
130138
throw new Error("flowControllerId required when using managed TaskFlow run mode");
@@ -153,21 +161,16 @@ function parseResumeFlowParams(params: Record<string, unknown>): ManagedFlowResu
153161
const runGoal = readOptionalTrimmedString(params.flowGoal, "flowGoal");
154162
const stateJson = params.flowStateJson;
155163

156-
const hasResumeFields =
157-
flowId !== undefined ||
158-
expectedRevision !== undefined ||
159-
currentStep !== undefined ||
160-
waitingStep !== undefined;
161-
162-
if (!hasResumeFields) {
164+
// Managed resume is identified by flowId. flowExpectedRevision and step
165+
// fields can be injected as tool-schema defaults (e.g. flowExpectedRevision
166+
// 0), so on their own they must not route an ordinary resume into managed
167+
// TaskFlow mode.
168+
if (flowId === undefined) {
163169
return null;
164170
}
165171
if (runControllerId !== undefined || runGoal !== undefined || stateJson !== undefined) {
166172
throw new Error("resume action does not accept flowControllerId, flowGoal, or flowStateJson");
167173
}
168-
if (!flowId) {
169-
throw new Error("flowId required when using managed TaskFlow resume mode");
170-
}
171174
if (expectedRevision === undefined) {
172175
throw new Error("flowExpectedRevision required when using managed TaskFlow resume mode");
173176
}

0 commit comments

Comments
 (0)