feat(tegg): [3/3] add @AgentController decorator with plugin integration#5827
Merged
jerryliang64 merged 13 commits intonextfrom Mar 11, 2026
Merged
feat(tegg): [3/3] add @AgentController decorator with plugin integration#5827jerryliang64 merged 13 commits intonextfrom
jerryliang64 merged 13 commits intonextfrom
Conversation
- Add @AgentController() decorator that auto-registers 7 OpenAI-compatible HTTP routes under /api/v1 - Add AgentHandler interface, AgentInfoUtil metadata utilities, and AgentControllerProto/AgentControllerObject for full IoC lifecycle integration - Add NodeSSEWriter implementation with lazy header writing and lowercase HTTP headers - Enforce createStore() method in @AgentController (no default store) - Use AgentRuntime.create() with AgentExecutor interface - Add @eggjs/tegg/agent public API entry point for re-exports - Add AGENT_CONTROLLER_PROTO_IMPL_TYPE and metadata symbols in @eggjs/tegg-types - Register agent controller factory in controller plugin boot hook (app.ts) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds agent controller and agent-runtime features: Node SSE writer, AgentController decorator and AgentHandler interface, metadata utilities, AgentControllerProto/Object with AgentRuntime wiring and streamRun → NodeSSEWriter integration, package exports, tests, and fixtures. Changes
Sequence Diagram(s)sequenceDiagram
participant Lifecycle as EggObject Lifecycle
participant ACO as AgentControllerObject
participant User as Controller (user object)
participant Store as AgentStore
participant Runtime as AgentRuntime
participant Client as HTTP Client/Response
Lifecycle->>ACO: init(ctx)
ACO->>User: call createStore()? (if implemented)
User-->>Store: returns AgentStore
ACO->>Store: store.init()? (if present)
ACO->>Runtime: AgentRuntime.create(executor=User, store, logger)
ACO->>ACO: bind runtime methods (asyncRun/syncRun/streamRun...)
ACO->>User: call user.init()
User-->>ACO: init complete
Client->>ACO: streamRun HTTP request
ACO->>ACO: disable auto-response, create NodeSSEWriter(res)
ACO->>Runtime: runtime.streamRun(input)
Runtime-->>ACO: yield AgentStreamMessage
ACO->>Client: NodeSSEWriter.writeEvent(event, data)
Client-->>ACO: may close connection -> NodeSSEWriter.onClose triggers
ACO->>Runtime: stop/cleanup on client disconnect
Lifecycle->>ACO: destroy(ctx)
ACO->>Runtime: runtime.destroy()
Runtime-->>ACO: cleanup complete
ACO->>User: run preDestroy/destroy hooks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request, the third in a series, introduces the Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Deploying egg with
|
| Latest commit: |
2af6c6f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://918411d4.egg-cci.pages.dev |
| Branch Preview URL: | https://feat-pr3-agent-controller.egg-cci.pages.dev |
Deploying egg-v3 with
|
| Latest commit: |
2af6c6f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://dce1b716.egg-v3.pages.dev |
| Branch Preview URL: | https://feat-pr3-agent-controller.egg-v3.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces the @AgentController decorator, a significant feature for creating OpenAI-compatible API endpoints, integrating new decorators, interfaces (AgentHandler), and a NodeSSEWriter with the tegg IoC container. A medium-severity SSE injection vulnerability has been identified in NodeSSEWriter.ts due to unsanitized event parameters. Additionally, there's a potential maintainability issue in AgentControllerObject.ts due to code duplication. Addressing the SSE injection is critical for security, and resolving the code duplication will improve maintainability.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
tegg/core/agent-runtime/test/NodeSSEWriter.test.ts (1)
38-40: Drop the repeatedas anycasts in these tests.The fixture only needs a small
ServerResponsesurface, but the current casts and lint suppressions bypass the repo’s TS rules and make API drift easy to miss. A typed helper or anunknownbridge would keep the mock checked without eight separate suppressions.As per coding guidelines, "Avoid
anytype; useunknownwhen type is truly unknown in TypeScript".Also applies to: 54-56, 65-67, 74-76, 89-90, 102-103, 118-119, 135-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-runtime/test/NodeSSEWriter.test.ts` around lines 38 - 40, The tests use repeated "as any" casts for the fixture (res) which bypass type checks; replace those casts by either (a) creating a small typed mock helper like createResMock(): Partial<ServerResponse> that implements only the methods used by NodeSSEWriter and use it as res as ServerResponse, or (b) use an unknown bridge (res as unknown as ServerResponse) instead of "as any", and remove the eslint-disable comments; update each instantiation of NodeSSEWriter (and other places flagged) to use the typed mock or unknown cast so TypeScript validates the ServerResponse surface while keeping the test behavior the same.tegg/core/controller-decorator/test/fixtures/AgentFooController.ts (1)
3-4: Consider using.jsextension for ESM imports.The imports use
.tsextensions, but the coding guidelines specify using.jsextensions for ESM files. There's also inconsistency within this PR -AgentController.test.tsline 16 imports this fixture with.jsextension ('./fixtures/AgentFooController.js').🔧 Suggested change for consistency
-import { AgentController } from '../../src/decorator/agent/AgentController.ts'; -import type { AgentHandler } from '../../src/decorator/agent/AgentHandler.ts'; +import { AgentController } from '../../src/decorator/agent/AgentController.js'; +import type { AgentHandler } from '../../src/decorator/agent/AgentHandler.js';As per coding guidelines: "All imports should use
.jsextensions for ESM files".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/controller-decorator/test/fixtures/AgentFooController.ts` around lines 3 - 4, Update the ESM imports in the AgentFooController fixture to use .js extensions to match project conventions and the test import; specifically change the import specifiers that reference '../../src/decorator/agent/AgentController.ts' and '../../src/decorator/agent/AgentHandler.ts' to use '../../src/decorator/agent/AgentController.js' and '../../src/decorator/agent/AgentHandler.js' so the fixture (AgentFooController.ts) and AgentController.test.ts use consistent .js imports.tegg/plugin/controller/src/lib/AgentControllerObject.ts (1)
220-227: Consider validating logger availability before AgentRuntime creation.If
AgentControllerObject.setLogger()is not called beforecreateObject(), the logger passed toAgentRuntime.create()will beundefined. While the wiring inapp.tsensuressetLoggeris called in the constructor, this implicit dependency could cause issues if the order changes or in test scenarios.🛡️ Suggested defensive check
// Create runtime with AgentRuntime.create factory + if (!AgentControllerObject.logger) { + throw new Error('AgentControllerObject.setLogger() must be called before creating agent controllers'); + } const runtime = AgentRuntime.create({ executor: this._obj as AgentExecutor, store, logger: AgentControllerObject.logger, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/plugin/controller/src/lib/AgentControllerObject.ts` around lines 220 - 227, The code passes AgentControllerObject.logger into AgentRuntime.create without ensuring it is set; update createObject (the block that calls AgentRuntime.create) to validate that AgentControllerObject.logger is available (e.g., check this.logger or AgentControllerObject.logger) before creating the runtime and either throw a clear error or provide a sensible default logger; ensure the check occurs right before calling AgentRuntime.create so instance[AGENT_RUNTIME], this.runtime, and the executor wiring only proceed when a valid logger is present.tegg/plugin/controller/src/lib/AgentControllerProto.ts (1)
92-94: Avoidanytype; useunknownfor type-safe delegation.The
...args: anyparameter violates the coding guideline to avoidanytype. Additionally, public methods should have explicit return type annotations perisolatedDeclarationsrequirements.🔧 Proposed fix
- constructEggObject(...args: any): object { + constructEggObject(...args: unknown[]): object { return this.delegate.constructEggObject(...args); }As per coding guidelines: "Avoid
anytype; useunknownwhen type is truly unknown in TypeScript".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/plugin/controller/src/lib/AgentControllerProto.ts` around lines 92 - 94, Change the constructEggObject signature to avoid `any` by declaring the rest parameter as `...args: unknown[]` and add an explicit return type annotation (e.g., `: object`) to satisfy isolatedDeclarations; forward the arguments to the delegate call via `this.delegate.constructEggObject(...args)` (if the delegate API requires a narrower type, narrow or assert the args at the call site inside the method) so the public method stays type-safe and declaration-compliant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/core/agent-runtime/src/NodeSSEWriter.ts`:
- Around line 30-33: writeEvent currently ignores res.write()'s return value and
can overflow memory; update NodeSSEWriter.writeEvent (and ensureHeaders if
needed) to handle backpressure by either: (a) implementing the drain
pattern—check the boolean result of this.res.write(...), and when it returns
false stop emitting/writing further events until this.res.once('drain', ...)
resumes writing; or (b) refactor the producer to a Readable stream and pipe it
to this.res so Node handles backpressure automatically; ensure _closed is still
respected and that pending events are queued/paused during the drain period to
avoid dropping data.
In `@tegg/core/controller-decorator/src/decorator/agent/AgentHandler.ts`:
- Line 14: The AgentHandler.createStore signature uses Promise<unknown> and is
optional; update it to return the concrete store contract and make it required
so incompatible implementations can't compile. Replace the declaration
createStore?(): Promise<unknown>; with createStore(): Promise<AgentStore>; (or
the actual store interface name used in your codebase), import that store
interface into the file, and update all concrete classes implementing
AgentHandler to return the correct AgentStore type from createStore.
---
Nitpick comments:
In `@tegg/core/agent-runtime/test/NodeSSEWriter.test.ts`:
- Around line 38-40: The tests use repeated "as any" casts for the fixture (res)
which bypass type checks; replace those casts by either (a) creating a small
typed mock helper like createResMock(): Partial<ServerResponse> that implements
only the methods used by NodeSSEWriter and use it as res as ServerResponse, or
(b) use an unknown bridge (res as unknown as ServerResponse) instead of "as
any", and remove the eslint-disable comments; update each instantiation of
NodeSSEWriter (and other places flagged) to use the typed mock or unknown cast
so TypeScript validates the ServerResponse surface while keeping the test
behavior the same.
In `@tegg/core/controller-decorator/test/fixtures/AgentFooController.ts`:
- Around line 3-4: Update the ESM imports in the AgentFooController fixture to
use .js extensions to match project conventions and the test import;
specifically change the import specifiers that reference
'../../src/decorator/agent/AgentController.ts' and
'../../src/decorator/agent/AgentHandler.ts' to use
'../../src/decorator/agent/AgentController.js' and
'../../src/decorator/agent/AgentHandler.js' so the fixture
(AgentFooController.ts) and AgentController.test.ts use consistent .js imports.
In `@tegg/plugin/controller/src/lib/AgentControllerObject.ts`:
- Around line 220-227: The code passes AgentControllerObject.logger into
AgentRuntime.create without ensuring it is set; update createObject (the block
that calls AgentRuntime.create) to validate that AgentControllerObject.logger is
available (e.g., check this.logger or AgentControllerObject.logger) before
creating the runtime and either throw a clear error or provide a sensible
default logger; ensure the check occurs right before calling AgentRuntime.create
so instance[AGENT_RUNTIME], this.runtime, and the executor wiring only proceed
when a valid logger is present.
In `@tegg/plugin/controller/src/lib/AgentControllerProto.ts`:
- Around line 92-94: Change the constructEggObject signature to avoid `any` by
declaring the rest parameter as `...args: unknown[]` and add an explicit return
type annotation (e.g., `: object`) to satisfy isolatedDeclarations; forward the
arguments to the delegate call via `this.delegate.constructEggObject(...args)`
(if the delegate API requires a narrower type, narrow or assert the args at the
call site inside the method) so the public method stays type-safe and
declaration-compliant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 050a68fd-27d8-4089-baae-a041e2b72d19
⛔ Files ignored due to path filters (2)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamltegg/core/controller-decorator/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (17)
tegg/core/agent-runtime/src/NodeSSEWriter.tstegg/core/agent-runtime/src/index.tstegg/core/agent-runtime/test/NodeSSEWriter.test.tstegg/core/controller-decorator/src/decorator/agent/AgentController.tstegg/core/controller-decorator/src/decorator/agent/AgentHandler.tstegg/core/controller-decorator/src/decorator/index.tstegg/core/controller-decorator/src/util/AgentInfoUtil.tstegg/core/controller-decorator/src/util/index.tstegg/core/controller-decorator/test/AgentController.test.tstegg/core/controller-decorator/test/fixtures/AgentFooController.tstegg/core/tegg/package.jsontegg/core/tegg/src/agent.tstegg/core/types/src/controller-decorator/MetadataKey.tstegg/plugin/controller/package.jsontegg/plugin/controller/src/app.tstegg/plugin/controller/src/lib/AgentControllerObject.tstegg/plugin/controller/src/lib/AgentControllerProto.ts
tegg/core/controller-decorator/src/decorator/agent/AgentHandler.ts
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5827 +/- ##
==========================================
+ Coverage 85.39% 85.74% +0.34%
==========================================
Files 659 665 +6
Lines 12813 12963 +150
Branches 1474 1486 +12
==========================================
+ Hits 10942 11115 +173
+ Misses 1751 1728 -23
Partials 120 120 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Save close handler reference and remove it in end() to prevent memory leaks. Clear closeCallbacks after execution to release refs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
tegg/core/agent-runtime/src/NodeSSEWriter.ts (1)
33-36:⚠️ Potential issue | 🟠 Major
writeEvent()still ignores write backpressure.Line 36 drops the boolean from
res.write(). On long-lived SSE streams, a slow reader will make that returnfalse; continuing to emit after that lets Node buffer unsent chunks in memory. Please stop producing or queue events until'drain'fires.In current Node.js, what does http.ServerResponse.write() returning false mean, and what is the recommended backpressure handling pattern for manual SSE responses?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-runtime/src/NodeSSEWriter.ts` around lines 33 - 36, The writeEvent method in NodeSSEWriter currently ignores write backpressure by discarding the boolean return of this.res.write; update NodeSSEWriter.writeEvent to observe the boolean result from this.res.write(...) and, when it returns false, stop writing further events and queue the event payloads (or set a _paused flag) and attach a one-time 'drain' listener on this.res to flush the queue and clear the paused state; reuse ensureHeaders() as before, respect this._closed, and make sure the drain handler is cleaned up to avoid leaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/core/agent-runtime/src/NodeSSEWriter.ts`:
- Around line 43-48: The end() implementation clears the close listener and
closeCallbacks before calling this.res.end(), which prevents onClose() callbacks
from running on normal completion; change end() to set this._closed = true
early, then call this.res.end(), and only after the stream has finished (or
immediately after res.end() while guarding against duplicate invocation) remove
the 'close' listener (onResClose) and clear/execute closeCallbacks so registered
onClose() cleanup always runs; reference functions/fields: end(), onResClose,
onClose(), closeCallbacks, res.end(), and _closed to locate and adjust the
logic.
- Line 3: Update the ES module import specifier in NodeSSEWriter by replacing
the TypeScript file extension with .js: change the import of SSEWriter from
'./SSEWriter.ts' to './SSEWriter.js' (the import statement that declares "import
type { SSEWriter } from './SSEWriter.ts';"); ensure the rest of the file still
compiles with the type-only import style.
---
Duplicate comments:
In `@tegg/core/agent-runtime/src/NodeSSEWriter.ts`:
- Around line 33-36: The writeEvent method in NodeSSEWriter currently ignores
write backpressure by discarding the boolean return of this.res.write; update
NodeSSEWriter.writeEvent to observe the boolean result from this.res.write(...)
and, when it returns false, stop writing further events and queue the event
payloads (or set a _paused flag) and attach a one-time 'drain' listener on
this.res to flush the queue and clear the paused state; reuse ensureHeaders() as
before, respect this._closed, and make sure the drain handler is cleaned up to
avoid leaks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ec09778b-34b9-424d-b3b1-707c3af0bb83
📒 Files selected for processing (1)
tegg/core/agent-runtime/src/NodeSSEWriter.ts
…tions - Add agent/index.ts barrel export to match http/index.ts and mcp/index.ts - Rename setIsAgentController to setAgentController for set/get consistency Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tegg/core/controller-decorator/src/util/AgentInfoUtil.ts (1)
18-24: Consider unifying metadata access through MetadataUtil for consistency.These methods use raw
Reflect.defineMetadata/Reflect.getMetadatawhile other methods in this class useMetadataUtil. The divergence is understandable sinceMetadataUtilis typed forEggProtoImplClass, notFunction.For long-term maintainability, consider either:
- Extending
MetadataUtilwith function-level metadata helpers (e.g.,defineFunctionMetaData,getFunctionBooleanMetaData)- Or add a brief comment explaining why raw
Reflectis used hereThis is a minor consistency nit and can be deferred.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/controller-decorator/src/util/AgentInfoUtil.ts` around lines 18 - 24, The setNotImplemented and isNotImplemented functions use raw Reflect metadata while the rest of the class uses MetadataUtil; to unify, add function-level helpers to MetadataUtil (e.g., defineFunctionMetaData(key: string, target: Function, value: any) and getFunctionBooleanMetaData(key: string, target: Function): boolean) and then replace calls in AgentInfoUtil.setNotImplemented and AgentInfoUtil.isNotImplemented to use MetadataUtil.defineFunctionMetaData(CONTROLLER_AGENT_NOT_IMPLEMENTED, fn, true) and MetadataUtil.getFunctionBooleanMetaData(CONTROLLER_AGENT_NOT_IMPLEMENTED, fn) respectively; alternatively, if you prefer not to change MetadataUtil now, add a concise comment above these two methods explaining why raw Reflect is used and referencing the typing limitation of MetadataUtil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tegg/core/controller-decorator/src/util/AgentInfoUtil.ts`:
- Around line 18-24: The setNotImplemented and isNotImplemented functions use
raw Reflect metadata while the rest of the class uses MetadataUtil; to unify,
add function-level helpers to MetadataUtil (e.g., defineFunctionMetaData(key:
string, target: Function, value: any) and getFunctionBooleanMetaData(key:
string, target: Function): boolean) and then replace calls in
AgentInfoUtil.setNotImplemented and AgentInfoUtil.isNotImplemented to use
MetadataUtil.defineFunctionMetaData(CONTROLLER_AGENT_NOT_IMPLEMENTED, fn, true)
and MetadataUtil.getFunctionBooleanMetaData(CONTROLLER_AGENT_NOT_IMPLEMENTED,
fn) respectively; alternatively, if you prefer not to change MetadataUtil now,
add a concise comment above these two methods explaining why raw Reflect is used
and referencing the typing limitation of MetadataUtil.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b569f124-ab75-4c17-85e5-a825df264bca
📒 Files selected for processing (4)
tegg/core/controller-decorator/src/decorator/agent/AgentController.tstegg/core/controller-decorator/src/decorator/agent/index.tstegg/core/controller-decorator/src/decorator/index.tstegg/core/controller-decorator/src/util/AgentInfoUtil.ts
✅ Files skipped from review due to trivial changes (1)
- tegg/core/controller-decorator/src/decorator/agent/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tegg/core/controller-decorator/src/decorator/agent/AgentController.ts
…Obj mutation - Unify CreateRunInput.input.messages to use InputMessage[] from AgentStore, eliminating duplicate type definitions - Add abortController.signal.aborted check in streamRun catch block to mark aborted runs as cancelled instead of failed, consistent with syncRun/asyncRun - Replace msgObj mutation with new object spread to avoid aliasing issues - Replace inline import() types with top-level imports in AgentRuntime types - Add streamRun thread messages persistence test for parity with syncRun/asyncRun Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Change EggLogger import from 'egg-logger' to 'egg' in AgentControllerObject to fix oxlint TS2307 module resolution error - Update @eggjs/tegg exports snapshot to include new AgentController exports from controller-decorator Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/core/types/src/agent-runtime/AgentRuntime.ts`:
- Line 1: Update the ESM import specifier in AgentRuntime.ts to use a .js
extension: change the type import line that imports AgentRunConfig,
InputMessage, MessageObject, RunStatus from './AgentStore.ts' to import from
'./AgentStore.js' (keep it as an `import type` and only modify the module
specifier).
In `@tegg/plugin/controller/src/lib/AgentControllerObject.ts`:
- Line 19: Update the local ESM import specifier to use a .js extension: replace
the import of AgentControllerProto in AgentControllerObject (the line importing
AgentControllerProto) so it references './AgentControllerProto.js' instead of
'./AgentControllerProto.ts' to follow the repo's ESM import convention.
- Around line 205-225: The store created by createStoreFn and initialized via
store.init may leak if AgentRuntime.create throws before this.runtime is
assigned; wrap the store initialization and AgentRuntime.create call in a
try/catch/finally such that on any error prior to assigning this.runtime you
call store.destroy() (or await store.destroy() if async) to clean up resources,
rethrow the error, and only set this.runtime = runtime after successful
creation; reference createStoreFn, store.init, AgentRuntime.create, this.runtime
and store.destroy in your changes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7ccc6c33-3efd-43ea-915a-6587144c0eb4
⛔ Files ignored due to path filters (1)
tegg/core/tegg/test/__snapshots__/exports.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
tegg/core/agent-runtime/src/AgentRuntime.tstegg/core/agent-runtime/test/AgentRuntime.test.tstegg/core/types/src/agent-runtime/AgentRuntime.tstegg/plugin/controller/src/lib/AgentControllerObject.ts
…ymbols Add missing AGENT_CONTROLLER_PROTO_IMPL_TYPE and CONTROLLER_AGENT_* symbols to the exports stability snapshot, fixing CI shard 3/3 failures. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mutation Replace loose index signature with explicit fields (role, status, content, runId, threadId) for type safety. Add MessageConverter.completeMessage() factory method to avoid mutating message objects in AgentRuntime streaming, aligning with the immutable pattern already used by RunBuilder. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add unit tests to cover previously untested code paths: - AgentControllerProto: constructor delegation, symbol property copying, all getter/method delegations, and createProto static factory - AgentInfoUtil: setEnhanced/isEnhanced methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tegg/core/agent-runtime/test/AgentRuntime.test.ts (1)
322-358: Assert thatthread.message.createdstaysin_progress.This test only checks event names. The new
completeMessage()path exists to prevent theThreadMessageCreatedpayload from being rewritten tocompletedby reference, andMockSSEWriteris exactly the harness that can catch that regression. Please assert that the created event still hasstatus === in_progressandcontent === []after the completed event is emitted.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tegg/core/agent-runtime/test/AgentRuntime.test.ts` around lines 322 - 358, The test is missing assertions that the ThreadMessageCreated event's payload remains status === ThreadMessageStatus.InProgress and content === [] after the ThreadMessageCompleted event; using MockSSEWriter, find the ThreadMessageCreated event in writer.events (event === AgentSSEEvent.ThreadMessageCreated), capture its data before/after the completion sequence and assert its status is InProgress and its content is an empty array (use the same event object from writer.events to catch reference mutations introduced by completeMessage()). Ensure you reference AgentSSEEvent.ThreadMessageCreated, AgentSSEEvent.ThreadMessageCompleted, MockSSEWriter, and the completeMessage() path when adding these assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tegg/core/agent-runtime/src/AgentRuntime.ts`:
- Around line 303-324: The code currently mutates the terminal builder rb to
completed before an awaited update and then reuses rb in the error path
(rb.cancelling() / rb.fail()), which can cause InvalidRunStateTransitionError
and emit a completed snapshot; instead, after any failed await to
this.store.updateRun(run.id, rb.complete()/...), rehydrate the authoritative run
state from the store (e.g., fetch the persisted run record for run.id) and
construct a new RunBuilder from that persisted record before calling
cancelling() or fail(); then use that fresh builder to call
this.store.updateRun(...) and
writer.writeEvent(AgentSSEEvent.ThreadRunCancelled/ThreadRunFailed, ...) so you
never call cancelling()/fail() on a builder already transitioned to completed.
---
Nitpick comments:
In `@tegg/core/agent-runtime/test/AgentRuntime.test.ts`:
- Around line 322-358: The test is missing assertions that the
ThreadMessageCreated event's payload remains status ===
ThreadMessageStatus.InProgress and content === [] after the
ThreadMessageCompleted event; using MockSSEWriter, find the ThreadMessageCreated
event in writer.events (event === AgentSSEEvent.ThreadMessageCreated), capture
its data before/after the completion sequence and assert its status is
InProgress and its content is an empty array (use the same event object from
writer.events to catch reference mutations introduced by completeMessage()).
Ensure you reference AgentSSEEvent.ThreadMessageCreated,
AgentSSEEvent.ThreadMessageCompleted, MockSSEWriter, and the completeMessage()
path when adding these assertions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e136a122-362c-4338-bd21-b2b265a981cb
⛔ Files ignored due to path filters (1)
tegg/core/types/test/__snapshots__/index.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (6)
tegg/core/agent-runtime/src/AgentRuntime.tstegg/core/agent-runtime/src/MessageConverter.tstegg/core/agent-runtime/test/AgentRuntime.test.tstegg/core/agent-runtime/test/MessageConverter.test.tstegg/core/agent-runtime/test/RunBuilder.test.tstegg/core/types/src/agent-runtime/AgentStore.ts
"Node" prefix is redundant in a Node.js framework — the class actually adapts the HTTP ServerResponse for SSE, so "Http" is more precise. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move ContentBlockType, InputContentPart, TextContentBlock, MessageContentBlock, InputMessage, and MessageObject into a dedicated AgentMessage.ts file to eliminate the circular dependency between AgentStore.ts and AgentRuntime.ts (previously worked around with an inline import() type). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The runtime already throws if createStore is missing. Align the interface with the actual behavior so implementors get a compile-time error instead of a runtime surprise. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The previous commit made createStore required in AgentHandler interface but missed updating this test fixture, causing typecheck CI failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@AgentController()decorator that auto-registers 7 OpenAI-compatible HTTP routes under/api/v1AgentHandlerinterface,AgentInfoUtilmetadata utilities, andAgentControllerProto/AgentControllerObjectfor full IoC lifecycle integrationNodeSSEWriterimplementation with lazy header writing and lowercase HTTP headerscreateStore()method in@AgentController(no default store — users must provide their ownAgentStore)AgentRuntime.create()withAgentExecutorinterface (replaces removedcreateAgentRuntime+AgentControllerHost)AGENT_CONTROLLER_PROTO_IMPL_TYPEand metadata symbols in@eggjs/tegg-typesapp.ts)@eggjs/tegg/agentpublic API entry point for re-exportsStacked PRs
This is part of a 3-PR series (review in order):
Changes from previous PR (#5820)
AgentHandler.tsandAgentFooController.tsnow import types from@eggjs/tegg-types/agent-runtimeinstead of non-existentmodel/AgentControllerTypes.tsNodeSSEWriter: New implementation in@eggjs/agent-runtimewith lazy header writing, lowercase HTTP headers (content-type), and singleres.write()per eventcreateStore(): RemovedFileAgentStorefallback —@AgentControllernow requires users to implementcreateStore()AgentRuntime.create(): ReplacedcreateAgentRuntime({ host })withAgentRuntime.create({ executor })to match PR2's APIagent.tsexports: RemovedFileAgentStore, addedNodeSSEWriter, types now re-exported from@eggjs/agent-runtime@eggjs/controller-decoratorexports (they live in@eggjs/tegg-types/agent-runtime)Test plan
tsc --noEmittype checking passes for all affected packages🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores