Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
2 changes: 1 addition & 1 deletion plugin/langchain/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export default class ModuleLangChainHook implements IBoot {
configWillLoad() {
this.#app.eggObjectLifecycleUtil.registerLifecycle(this.#graphObjectHook);
this.#app.eggObjectLifecycleUtil.registerLifecycle(this.#boundModelObjectHook);
this.#app.eggObjectFactory.registerEggObjectCreateMethod(CompiledStateGraphProto, CompiledStateGraphObject.createObject);
this.#app.eggObjectFactory.registerEggObjectCreateMethod(CompiledStateGraphProto, CompiledStateGraphObject.createObject(this.#app));
this.#app.eggPrototypeLifecycleUtil.registerLifecycle(this.#graphPrototypeHook);
}

Expand Down
145 changes: 138 additions & 7 deletions plugin/langchain/lib/graph/CompiledStateGraphObject.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,18 @@ import {
EggObject,
EggObjectStatus,
} from '@eggjs/tegg-runtime';
import { EggObjectName, EggPrototypeName, Id, IdenticalUtil } from '@eggjs/tegg';
import { EggObjectName, EggPrototypeName, Id, IdenticalUtil, ModuleConfig } from '@eggjs/tegg';
import { CompiledStateGraphProto } from './CompiledStateGraphProto';
import { EggPrototype } from '@eggjs/tegg-metadata';
import { ChatCheckpointSaverInjectName, ChatCheckpointSaverQualifierAttribute, GRAPH_EDGE_METADATA, GRAPH_NODE_METADATA, GraphEdgeMetadata, GraphMetadata, GraphNodeMetadata, IGraph, IGraphEdge, IGraphNode, TeggToolNode } from '@eggjs/tegg-langchain-decorator';
import { LangGraphTracer } from '../tracing/LangGraphTracer';
import { BaseCheckpointSaver, CompiledStateGraph } from '@langchain/langgraph';
import { Application, Context } from 'egg';
import { ModuleConfigUtil, TimerUtil } from '@eggjs/tegg-common-util';
import { LangChainConfigSchemaType } from 'typings';
import pathToRegexp from 'path-to-regexp';
import { AIMessage, HumanMessage, SystemMessage, ToolMessage } from '@langchain/core/messages';
import { Readable, Transform } from 'node:stream';

export class CompiledStateGraphObject implements EggObject {
private status: EggObjectStatus = EggObjectStatus.PENDING;
Expand All @@ -19,17 +25,19 @@ export class CompiledStateGraphObject implements EggObject {
readonly proto: CompiledStateGraphProto;
readonly ctx: EggContext;
readonly daoName: string;
private _obj: object;
_obj: object;

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

The _obj property was changed from public (default access modifier) to private. This is a good practice for encapsulation. However, in this PR, it has been changed back to public. Since it's only accessed within the class's methods (via a self closure in createHttpHandler), it can and should remain private to maintain proper encapsulation.

Suggested change
_obj: object;
private _obj: object;

readonly graphMetadata: GraphMetadata;
readonly graphName: string;
readonly app: Application;

constructor(name: EggObjectName, proto: CompiledStateGraphProto) {
constructor(name: EggObjectName, proto: CompiledStateGraphProto, app: Application) {
this.name = name;
this.proto = proto;
this.ctx = ContextHandler.getContext()!;
this.id = IdenticalUtil.createObjectId(this.proto.id, this.ctx?.id);
this.graphMetadata = proto.graphMetadata;
this.graphName = proto.graphName;
this.app = app;
}

async init() {
Expand All @@ -47,6 +55,8 @@ export class CompiledStateGraphObject implements EggObject {
return await originalInvoke.call(graph, input, config);
};

await this.boundByConfig();

this.status = EggObjectStatus.READY;
}

Expand Down Expand Up @@ -76,6 +86,125 @@ export class CompiledStateGraphObject implements EggObject {
return compileGraph;
}

async boundByConfig() {
const config = ModuleConfigUtil.loadModuleConfigSync(this.proto.unitPath) as ModuleConfig | undefined;
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? [];

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.

high

The default value for agents is [], but its type LangChainConfigSchemaType['agents'] is Record<string, ...>, which is an object. The correct default value should be an empty object {} to match the type definition. This will prevent potential issues and align the code with its declared type.

Suggested change
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? [];
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? {};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Incorrect default value type for agents.

agents is typed as a Record<string, ...> but defaults to [] (an array). This should default to {}.

-    const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? [];
+    const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? {};
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? [];
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? {};
🤖 Prompt for AI Agents
In plugin/langchain/lib/graph/CompiledStateGraphObject.ts around line 91, the
variable `agents` is declared with type LangChainConfigSchemaType['agents'] (a
Record<string,...>) but is defaulting to an empty array ([]); change the default
to an empty object ({}) so its runtime value matches its declared Record type
(e.g., const agents = config?.langchain?.agents ?? {}), ensuring
type-compatibility and avoiding runtime type mismatches.

const configName = `${this.graphName.slice(0, 1).toUpperCase()}${this.graphName.slice(1)}`;
if (Object.keys(agents).includes(configName)) {

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.

  • 能不能用 http controller 的方式去包一层? 不要手写一遍 http 的逻辑
  • http 要和 graph 解耦

const { path: methodRealPath, type, stream, timeout } = agents[configName];

if ((type ?? '').toLocaleLowerCase() === 'http') {
const router = this.app.router;
const regExp = pathToRegexp(methodRealPath!, {
sensitive: true,
});
const handler = this.createHttpHandler(stream, timeout);
Reflect.apply(router.post, router,
[ `${this.graphName}.Invoke`, methodRealPath, ...[], handler ]);
this.app.rootProtoManager.registerRootProto('AgentControllerInvoke', (ctx: Context) => {
if (regExp.test(ctx.path)) {
return this.proto;
}
}, '');
}

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.

high

The path property in the agent configuration is optional, so methodRealPath can be undefined. Using the non-null assertion operator (!) on methodRealPath will cause a runtime error if the path is not provided in the configuration. It's safer to check for the existence of methodRealPath in the conditional statement.

Suggested change
if ((type ?? '').toLocaleLowerCase() === 'http') {
const router = this.app.router;
const regExp = pathToRegexp(methodRealPath!, {
sensitive: true,
});
const handler = this.createHttpHandler(stream, timeout);
Reflect.apply(router.post, router,
[ `${this.graphName}.Invoke`, methodRealPath, ...[], handler ]);
this.app.rootProtoManager.registerRootProto('AgentControllerInvoke', (ctx: Context) => {
if (regExp.test(ctx.path)) {
return this.proto;
}
}, '');
}
if ((type ?? '').toLocaleLowerCase() === 'http' && methodRealPath) {
const router = this.app.router;
const regExp = pathToRegexp(methodRealPath, {
sensitive: true,
});
const handler = this.createHttpHandler(stream, timeout);
Reflect.apply(router.post, router,
[ `${this.graphName}.Invoke`, methodRealPath, ...[], handler ]);
this.app.rootProtoManager.registerRootProto('AgentControllerInvoke', (ctx: Context) => {
if (regExp.test(ctx.path)) {
return this.proto;
}
}, '');
}

}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Missing validation for required path field.

If path is undefined in the config, pathToRegexp(methodRealPath!) will throw a runtime error. Add validation before using the path.

       if ((type ?? '').toLocaleLowerCase() === 'http') {
+        if (!methodRealPath) {
+          throw new Error(`Agent ${configName} is configured as HTTP but missing 'path' field`);
+        }
         const router = this.app.router;
         const regExp = pathToRegexp(methodRealPath!, {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async boundByConfig() {
const config = ModuleConfigUtil.loadModuleConfigSync(this.proto.unitPath) as ModuleConfig | undefined;
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? [];
const configName = `${this.graphName.slice(0, 1).toUpperCase()}${this.graphName.slice(1)}`;
if (Object.keys(agents).includes(configName)) {
const { path: methodRealPath, type, stream, timeout } = agents[configName];
if ((type ?? '').toLocaleLowerCase() === 'http') {
const router = this.app.router;
const regExp = pathToRegexp(methodRealPath!, {
sensitive: true,
});
const handler = this.createHttpHandler(stream, timeout);
Reflect.apply(router.post, router,
[ `${this.graphName}.Invoke`, methodRealPath, ...[], handler ]);
this.app.rootProtoManager.registerRootProto('AgentControllerInvoke', (ctx: Context) => {
if (regExp.test(ctx.path)) {
return this.proto;
}
}, '');
}
}
}
async boundByConfig() {
const config = ModuleConfigUtil.loadModuleConfigSync(this.proto.unitPath) as ModuleConfig | undefined;
const agents: LangChainConfigSchemaType['agents'] = config?.langchain?.agents ?? [];
const configName = `${this.graphName.slice(0, 1).toUpperCase()}${this.graphName.slice(1)}`;
if (Object.keys(agents).includes(configName)) {
const { path: methodRealPath, type, stream, timeout } = agents[configName];
if ((type ?? '').toLocaleLowerCase() === 'http') {
if (!methodRealPath) {
throw new Error(`Agent ${configName} is configured as HTTP but missing 'path' field`);
}
const router = this.app.router;
const regExp = pathToRegexp(methodRealPath!, {
sensitive: true,
});
const handler = this.createHttpHandler(stream, timeout);
Reflect.apply(router.post, router,
[ `${this.graphName}.Invoke`, methodRealPath, ...[], handler ]);
this.app.rootProtoManager.registerRootProto('AgentControllerInvoke', (ctx: Context) => {
if (regExp.test(ctx.path)) {
return this.proto;
}
}, '');
}
}
}
🤖 Prompt for AI Agents
In plugin/langchain/lib/graph/CompiledStateGraphObject.ts around lines 89 to
111, the code assumes agents[configName].path (methodRealPath) is defined and
passes it to pathToRegexp which will throw if undefined; add an explicit check
that methodRealPath is a non-empty string before using it, and if it's
missing/invalid either log a clear warning/error and skip registering the
route/handler (i.e., return or continue) so runtime crashes are avoided; remove
the non-null assertion usage and guard all subsequent uses (regExp creation,
router.post call, and proto registration) behind this validation.


createHttpHandler(stream?: boolean, timeout?: number) {
const self = this;
return async function(ctx: Context) {
const invokeFunc = (self._obj as CompiledStateGraph<any, any>).invoke;
const streamFunc = (self._obj as CompiledStateGraph<any, any>).stream;
const args = ctx.request.body;
const genArgs = Object.entries(args).reduce((acc, [ key, value ]) => {
if (Array.isArray(value) && typeof value[0] === 'object') {
acc[key] = value.map(obj => {
switch (obj.role) {
case 'human':
return new HumanMessage(obj);
case 'ai':
return new AIMessage(obj);
case 'system':
return new SystemMessage(obj);
case 'tool':
return new ToolMessage(obj);
default:
throw new Error('unknown message type');
}
});
} else {
acc[key] = value;
}
return acc;
}, {});

const defaultConfig = {
configurable: {
thread_id: process.pid.toString(),

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.

critical

Using process.pid for thread_id is incorrect in a web server context. All concurrent requests handled by the same process will share the same thread_id, leading to mixed-up conversation states and data corruption. A unique ID must be generated for each request. Using crypto.randomUUID() is a robust solution. Note: you'll need to add import crypto from 'node:crypto'; at the top of the file.

Suggested change
thread_id: process.pid.toString(),
thread_id: crypto.randomUUID(),

},
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Thread ID using process.pid is not unique per request.

Using process.pid as thread_id means all concurrent requests share the same conversation thread, which can cause data corruption or unexpected behavior when multiple requests run simultaneously. Consider using a request-specific identifier (e.g., from request headers or a generated UUID).

       const defaultConfig = {
         configurable: {
-          thread_id: process.pid.toString(),
+          thread_id: ctx.request.get('x-thread-id') || ctx.traceId || crypto.randomUUID(),
         },
       };

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In plugin/langchain/lib/graph/CompiledStateGraphObject.ts around lines 141 to
145, the default thread_id is set to process.pid which is global to the process
and not unique per request; change the code to accept a request-scoped
identifier (e.g., pass thread_id into the constructor or method from request
headers/context) and fall back to a generated UUID (use a UUID v4 generator)
only when no request id is provided, ensuring each request/conversation gets a
unique thread_id.


let body: unknown;
try {
body = await TimerUtil.timeout<unknown>(() => Reflect.apply(stream ? streamFunc : invokeFunc, (self._obj as CompiledStateGraph<any, any>), [ genArgs, defaultConfig ]), timeout);
} catch (e: any) {
if (e instanceof TimerUtil.TimeoutError) {
ctx.logger.error(`timeout after ${timeout}ms`);
ctx.throw(500, 'timeout');
}
throw e;
}

// https://github.com/koajs/koa/blob/master/lib/response.js#L88
// ctx.status is set
const explicitStatus = (ctx.response as any)._explicitStatus;

if (
// has body
body != null ||
// status is not set and has no body
// code should by 204
// https://github.com/koajs/koa/blob/master/lib/response.js#L140
!explicitStatus
) {

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

Relying on the private property _explicitStatus from Koa's response object is fragile and may break with future updates to the framework. This logic can be simplified by just setting ctx.body = body and letting Koa handle the response status correctly (e.g., setting status 204 for an empty body). This would make the code more robust and easier to maintain.

ctx.body = body;
if (stream) {
ctx.set({
'content-type': 'text/event-stream',
'cache-control': 'no-cache',
'transfer-encoding': 'chunked',
'X-Accel-Buffering': 'no',
});
const transformStream = new Transform({
objectMode: true,
transform(chunk: any, _encoding: string, callback) {
try {
// 如果 chunk 是对象,转换为 JSON
let data: string;
if (typeof chunk === 'string') {
data = chunk;
} else if (typeof chunk === 'object') {
data = JSON.stringify(chunk);
} else {
data = String(chunk);
}

// 格式化为 SSE 格式
const sseFormatted = `data: ${data}\n\n`;
callback(null, sseFormatted);
} catch (error) {
callback(error);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

TypeScript type error in error callback.

The error variable is typed as unknown in the catch block, but callback expects Error | null | undefined. Add a type assertion.

             } catch (error) {
-              callback(error);
+              callback(error instanceof Error ? error : new Error(String(error)));
             }
🤖 Prompt for AI Agents
In plugin/langchain/lib/graph/CompiledStateGraphObject.ts around lines 195-196,
the catch block passes the caught variable typed as unknown into callback which
expects Error | null | undefined; change the call to assert or convert the
unknown to an Error before passing it (for example use a type assertion like
casting to Error or wrap non-Error values in new Error(String(error)) so
callback receives Error | null | undefined).

}
},
});
ctx.body = Readable.fromWeb(body as any, { objectMode: true }).pipe(transformStream);
} else {
ctx.body = body;
}
}
};
}

async boundNodes(stateGraph: EggObject) {
const graphObj = stateGraph.obj as IGraph<any, any>;
const nodes = this.graphMetadata.nodes ?? [];
Expand Down Expand Up @@ -122,9 +251,11 @@ export class CompiledStateGraphObject implements EggObject {
return this._obj;
}

static async createObject(name: EggObjectName, proto: EggPrototype): Promise<CompiledStateGraphObject> {
const compiledStateGraphObject = new CompiledStateGraphObject(name, proto as CompiledStateGraphProto);
await compiledStateGraphObject.init();
return compiledStateGraphObject;
static createObject(app: Application) {
return async function(name: EggObjectName, proto: EggPrototype): Promise<CompiledStateGraphObject> {
const compiledStateGraphObject = new CompiledStateGraphObject(name, proto as CompiledStateGraphProto, app);
await compiledStateGraphObject.init();
return compiledStateGraphObject;
};
}
}
2 changes: 2 additions & 0 deletions plugin/langchain/lib/graph/CompiledStateGraphProto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ export class CompiledStateGraphProto implements EggPrototype {
readonly name: EggPrototypeName;
readonly graphMetadata: GraphMetadata;
readonly graphName: string;
readonly unitPath: string;

constructor(loadUnit: LoadUnit, protoName: string, graphName: string, graphMetadata: GraphMetadata) {
this.loadUnitId = loadUnit.id;
this.qualifiers = [];
this.name = protoName;
this.graphMetadata = graphMetadata;
this.graphName = graphName;
this.unitPath = loadUnit.unitPath;
this.id = IdenticalUtil.createProtoId(loadUnit.id, protoName);
}

Expand Down
2 changes: 1 addition & 1 deletion plugin/langchain/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
"typescript": true
},
"engines": {
"node": ">=18.0.0"
"node": ">=20.0.0"
},
"dependencies": {
"@eggjs/egg-module-common": "^3.64.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,11 @@ mcp:
clientName: barSse
version: 1.0.0
transportType: SSE
type: http
type: http

langchain:
agents:
FooGraph:
path: /graph/stream
type: http
stream: true
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,6 @@ module.exports = function() {
enable: false,
},
},
bodyParser: {
enable: false,
},
};
return config;
};
55 changes: 55 additions & 0 deletions plugin/langchain/test/llm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,5 +76,60 @@ describe('plugin/langchain/test/llm.test.ts', () => {
.get('/llm/graph')
.expect(200, { value: 'hello graph toolhello world' });
});

it('should agent controller work', async () => {
const url = await app.httpRequest()
.post('/graph/stream').url;
const response = await fetch(url, {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({ messages: [{ role: 'human', content: 'hello world' }] }),
});


if (!response.ok) {
throw new Error(`HTTP ${response.status}`);
}

if (!response.body) {
throw new Error('Response body is null');
}

const reader = response.body.getReader();
const decoder = new TextDecoder();
let buffer = '';

const messages: object[] = [];

try {
// eslint-disable-next-line no-constant-condition
while (true) {
const { done, value } = await reader.read();

if (done) break;

buffer += decoder.decode(value, { stream: true });
const lines = buffer.split('\n');
buffer = lines.pop() || '';

lines.forEach(line => {
if (line.startsWith('data: ')) {
const data = line.slice(6);
try {
const parsed = JSON.parse(data);
messages.push(parsed);
} catch (e) {
throw e;
}
}
});
}
} finally {
reader.releaseLock();
}
assert(messages.length === 3);
});
}
});
23 changes: 23 additions & 0 deletions plugin/langchain/typings/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,34 @@ export const ChatModelConfigModuleConfigSchema = Type.Object({
name: 'ChatModel',
});


export const LangChainConfigSchema = Type.Object({
agents: Type.Record(Type.String(), Type.Object({
path: Type.Optional(Type.String({
description: 'http path',
})),
stream: Type.Optional(Type.Boolean({
description: '是否流式返回',
})),
type: Type.Optional(Type.String({
description: 'Http',
})),
timeout: Type.Optional(Type.Number({
description: '接口超时时间',
})),
})),
}, {
title: 'langchain 设置',
name: 'langchain',
});

export type ChatModelConfigModuleConfigType = Static<typeof ChatModelConfigModuleConfigSchema>;
export type LangChainConfigSchemaType = Static<typeof LangChainConfigSchema>;

declare module '@eggjs/tegg' {
export type LangChainModuleConfig = {
ChatModel?: ChatModelConfigModuleConfigType;
langchain?: LangChainConfigSchema;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Type error: Use the type alias instead of the schema constant.

LangChainConfigSchema is a runtime schema object, not a type. The property should use LangChainConfigSchemaType to represent the inferred TypeScript type.

   export type LangChainModuleConfig = {
     ChatModel?: ChatModelConfigModuleConfigType;
-    langchain?: LangChainConfigSchema;
+    langchain?: LangChainConfigSchemaType;
   };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
langchain?: LangChainConfigSchema;
langchain?: LangChainConfigSchemaType;
🤖 Prompt for AI Agents
In plugin/langchain/typings/index.d.ts around line 138, the declaration uses the
runtime schema constant LangChainConfigSchema where a TypeScript type is
required; replace LangChainConfigSchema with the inferred type alias
LangChainConfigSchemaType so the langchain? property is typed correctly,
updating the type annotation to use LangChainConfigSchemaType instead of the
schema constant.

};

export interface ModuleConfig extends LangChainModuleConfig {
Expand Down
Loading