feat(agents): multi-agent network system with orchestration and message bus#955
feat(agents): multi-agent network system with orchestration and message bus#955murdore wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
✅ Single Commit Policy - COMPLIANTStatus: Policy requirements met • 1 commit • Valid format • Ready for merge 📊 View validation details📝 Commit Details
✅ Validation Results
🤖 Automated validation by NeuroLink Single Commit Enforcement |
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdds a multi-agent orchestration subsystem: agent and network runtime classes, coordination and distribution components, orchestration and topology management, an in-memory MessageBus, CLI command support, type definitions, documentation, and test fixtures/suites. All changes are additive; no behavioral removals noted. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Agent
participant NeuroLink
participant Provider
Client->>Agent: execute(input)
Agent->>Agent: validate input & build prompt
Agent->>NeuroLink: generate(prompt, options)
NeuroLink->>Provider: send request
Provider-->>NeuroLink: response
NeuroLink-->>Agent: return result
Agent->>Agent: parse/validate output
Agent->>Client: AgentResult
sequenceDiagram
participant Client
participant AgentNetwork
participant NeuroLink
participant Router
participant AgentTool_A
participant AgentTool_B
Client->>AgentNetwork: execute(input)
AgentNetwork->>AgentNetwork: ensure tools initialized
AgentNetwork->>NeuroLink: generate(router prompt + agent tools)
NeuroLink->>Router: router decision (tool call)
Router-->>NeuroLink: selectedPrimitive (agent tool)
NeuroLink->>AgentTool_A: invoke agent_<id> tool
AgentTool_A-->>NeuroLink: tool result
NeuroLink->>AgentTool_B: invoke agent_<id> tool (if routed)
AgentTool_B-->>NeuroLink: tool result
NeuroLink-->>AgentNetwork: aggregated result
AgentNetwork->>Client: NetworkExecutionResult
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
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 |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a first-class multi-agent orchestration layer to NeuroLink, including agents, agent networks, advanced orchestration primitives, fixtures, and a live end-to-end test.
Changes:
- Introduces core multi-agent primitives (
Agent,AgentNetwork,RouterAgent) plus advanced components (orchestrator, supervisor, evaluator/optimizer, coordinator/task distributor, message bus + protocols). - Extends SDK exports and type barrels to expose the new orchestration surface area.
- Adds CLI wiring for
agent/networkcommand groups, test fixtures, and a live continuous test script.
Reviewed changes
Copilot reviewed 40 out of 42 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/fixtures/agents/routing-rules.json | Adds routing rule + decision fixtures for routing tests. |
| test/fixtures/agents/network-topologies.json | Adds topology and router config fixtures for network tests. |
| test/fixtures/agents/messages.json | Adds MessageBus fixtures and protocol scenarios. |
| test/fixtures/agents/agent-definitions.json | Adds agent definition fixtures for unit/integration coverage. |
| test/continuous-test-agents-live.ts | Adds live SDK→CLI→Agent→AgentNetwork smoke tests against real providers. |
| src/lib/types/index.ts | Re-exports multi-agent types via the canonical types barrel. |
| src/lib/types/cli.ts | Adds CLI argument types for agent/network command groups. |
| src/lib/neurolink.ts | Adds SDK entrypoints for creating/executing/streaming agents/networks + advanced factories. |
| src/lib/index.ts | Exposes multi-agent primitives and advanced modules in the package public API. |
| src/lib/agent/supervisor/supervisor-agent.ts | Implements supervision/HITL hooks, approval workflow, escalation, and auditing. |
| src/lib/agent/supervisor/index.ts | Exports supervisor module surface. |
| src/lib/agent/routerAgent.ts | Implements LLM-based task router over registered primitives. |
| src/lib/agent/prompts/routingPrompts.ts | Adds routing prompt builders and response parsing helpers. |
| src/lib/agent/orchestration/topology.ts | Adds topology graph builder + stats/pathing for agent networks. |
| src/lib/agent/orchestration/orchestrator.ts | Adds orchestrator for multi-network lifecycle, execution, and coordination. |
| src/lib/agent/orchestration/index.ts | Exports orchestration module surface. |
| src/lib/agent/index.ts | Barrels agent modules + advanced orchestration exports. |
| src/lib/agent/evaluation/index.ts | Exports evaluation module surface. |
| src/lib/agent/evaluation/evaluator.ts | Implements evaluation scoring + iterative optimization utilities. |
| src/lib/agent/coordination/task-distributor.ts | Adds task distribution, queues, retries, and capability matching. |
| src/lib/agent/coordination/index.ts | Exports coordination module surface. |
| src/lib/agent/coordination/coordinator.ts | Adds coordinator strategies (sequential/parallel/pipeline/etc.). |
| src/lib/agent/communication/protocols.ts | Adds protocol manager + default protocol handlers (handshake/delegation/consensus/heartbeat). |
| src/lib/agent/communication/message-bus.ts | Adds pub/sub + request/response + history + DLQ message bus. |
| src/lib/agent/communication/index.ts | Exports communication module surface. |
| src/cli/parser.ts | Registers new CLI command groups. |
| src/cli/factories/commandFactory.ts | Wires agent/network command factories into the CLI factory. |
| docs/agents/VERIFICATION.md | Adds manual verification checklist for multi-agent features. |
| docs/agents/TESTING.md | Adds testing guide for unit/integration/live tests. |
| docs/agents/CONFIGURATION.md | Adds configuration documentation for agents/networks/topologies/routing/message bus. |
| docs/agents/CLI-COVERAGE.md | Documents CLI coverage/gaps for multi-agent features. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private async queueExecution( | ||
| request: ExecutionRequest, | ||
| ): Promise<NetworkExecutionResult> { | ||
| return new Promise((resolve, reject) => { | ||
| const _queuedRequest = { | ||
| ...request, | ||
| resolve, | ||
| reject, | ||
| }; | ||
| this.executionQueue.push(request); | ||
| this.executionQueue.sort((a, b) => { | ||
| const priorityOrder = { high: 0, normal: 1, low: 2 }; | ||
| return ( | ||
| (priorityOrder[a.priority ?? "normal"] ?? 1) - | ||
| (priorityOrder[b.priority ?? "normal"] ?? 1) | ||
| ); | ||
| }); | ||
|
|
||
| this.emitter.emit("execution:queued", { networkId: request.networkId }); | ||
| }); | ||
| } |
| // Subscribe to reply topic | ||
| this.subscribe(replyTo, senderId, (msg) => { | ||
| const pending = this.pendingRequests.get(correlationId); | ||
| if (pending) { | ||
| clearTimeout(pending.timeout); | ||
| this.pendingRequests.delete(correlationId); | ||
| pending.resolve(msg); | ||
| } | ||
| }); |
| createOrchestrator( | ||
| config?: import("./types/index.js").OrchestratorConfig, | ||
| ): import("./agent/orchestration/index.js").NetworkOrchestrator { | ||
| const { NetworkOrchestrator } = require("./agent/orchestration/index.js"); | ||
| logger.debug("[NeuroLink] Creating network orchestrator", { | ||
| maxConcurrentExecutions: config?.maxConcurrentExecutions, | ||
| defaultMode: config?.defaultMode, | ||
| }); | ||
| return new NetworkOrchestrator(this, config); | ||
| } |
| case "escalate": | ||
| this.escalate({ | ||
| type: "timeout", | ||
| description: "Approval timeout", | ||
| severity: 2, | ||
| context: { action }, | ||
| }); | ||
| return { | ||
| approved: false, | ||
| reason: "Timeout - escalated", | ||
| timestamp: Date.now(), | ||
| }; |
| constructor( | ||
| onAgentStatusChange?: ( | ||
| agentId: string, | ||
| status: "healthy" | "degraded" | "unhealthy" | "offline", | ||
| ) => void, | ||
| ) { | ||
| this.onAgentStatusChange = onAgentStatusChange; | ||
|
|
||
| // Start cleanup interval | ||
| setInterval(() => this.checkForOfflineAgents(), 30000); | ||
| } |
| **Status: GAP IDENTIFIED** | ||
|
|
||
| The Multi-Agent Networks feature currently has **NO CLI commands** implemented. This document identifies the missing CLI functionality and provides recommendations for implementation. |
Documentation Validation Results
🚧 Please fix the failing checks before merging. Commit: |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (36)
test/fixtures/agents/network-topologies.json-258-263 (1)
258-263:⚠️ Potential issue | 🟠 MajorFailover scenario references an unavailable fallback agent.
primaryAgent: "error-prone"andfallbackAgent: "data-processor"are not co-located in any declared network, so this scenario is not executable as written.Suggested fixture correction
"failoverScenario": { "description": "Task with failover when primary agent fails", "input": "Process this data (primary unavailable)", "primaryAgent": "error-prone", - "fallbackAgent": "data-processor" + "fallbackAgent": "validator" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/agents/network-topologies.json` around lines 258 - 263, The failoverScenario fixture references primaryAgent "error-prone" and fallbackAgent "data-processor" but those two agents are not placed together in any declared network; update the fixture so the scenario is executable by either changing fallbackAgent to an agent that is co-located with "error-prone" or by modifying/adding a network declaration to include both "error-prone" and "data-processor"; ensure you edit the "failoverScenario" object (primaryAgent and/or fallbackAgent) or the network topology definitions so both agent names exist in the same network.src/cli/factories/commandFactory.ts-1592-1601 (1)
1592-1601:⚠️ Potential issue | 🟠 MajorMove agent and network command options from
src/cli/commands/agent.tstosrc/cli/factories/commandFactory.ts.Option and flag definitions (
.option()calls) are currently scattered throughoutsrc/cli/commands/agent.tsacross multiple builder methods (buildCreateOptions,buildListOptions,buildNetworkCreateOptions, etc.). Per CLI architecture guidelines, all command options must be centralized incommandFactory.ts, not delegated to command modules. The delegators at lines 1592–1601 should have corresponding builder methods incommandFactory.tsthat define all options/flags before returning the command module.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/factories/commandFactory.ts` around lines 1592 - 1601, The agent and network command option definitions must be moved from src/cli/commands/agent.ts into commandFactory.ts: add builder methods on CommandFactory (e.g., buildAgentCreateOptions, buildAgentListOptions, buildNetworkCreateOptions matching the existing builders in agent.ts) that register all .option() calls for each command, call those builders before returning AgentCommandFactory.createAgentCommands() and AgentCommandFactory.createNetworkCommands() in createAgentCommands() and createNetworkCommands(), and remove the .option() registrations from the command module builder methods (e.g., buildCreateOptions, buildListOptions, buildNetworkCreateOptions) so command modules only define handlers and names while commandFactory owns all flags/options.docs/agents/CLI-COVERAGE.md-5-8 (1)
5-8:⚠️ Potential issue | 🟠 MajorCoverage report contradicts the current CLI implementation.
The document states CLI coverage is none and recommends implementing commands that already exist in
src/cli/commands/agent.ts(agent/network create/list/execute/run). This makes the report unreliable for users and maintainers.Also applies to: 29-37, 60-270, 300-347, 385-386
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/CLI-COVERAGE.md` around lines 5 - 8, The coverage doc incorrectly claims there are no CLI commands; update docs/agents/CLI-COVERAGE.md to reflect the actual implemented commands found in src/cli/commands/agent.ts (functions/commands: agent network create, list, execute, run) by removing or correcting the "NO CLI commands" claim and reconciling the coverage sections (including ranges noted around 29-37, 60-270, 300-347, 385-386) so the report accurately references the existing CLI capabilities and any real gaps only.src/lib/types/cli.ts-1279-1342 (1)
1279-1342:⚠️ Potential issue | 🟠 MajorNew CLI types violate the required type naming convention.
AgentCommandArgsandNetworkCommandArgsshould use the CLI prefix to maintain globally unique exported names acrosssrc/lib/types/**.Suggested rename
-export type AgentCommandArgs = BaseCommandArgs & { +export type CliAgentCommandArgs = BaseCommandArgs & { // ... }; -export type NetworkCommandArgs = BaseCommandArgs & { +export type CliNetworkCommandArgs = BaseCommandArgs & { // ... };As per coding guidelines, "Every exported type name must be globally unique across all files in
src/lib/types/. Use domain prefixes to disambiguate:Cli*for CLI types."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/cli.ts` around lines 1279 - 1342, Exported type names AgentCommandArgs and NetworkCommandArgs violate the project naming rule; rename them to use the CLI prefix (e.g., CliAgentCommandArgs and CliNetworkCommandArgs) and update all references and exports accordingly so the new names are used wherever AgentCommandArgs and NetworkCommandArgs appear (types, imports, and exports) to restore global uniqueness across src/lib/types/*.docs/agents/CONFIGURATION.md-123-147 (1)
123-147:⚠️ Potential issue | 🟠 MajorDocumented config contracts diverge from actual exported types.
AgentNetworkConfigand especiallyRouterConfighere do not matchsrc/lib/types/agentNetwork.ts(e.g., documentedtype/fallbackAgent/maxRetriesvs actualprovider/model/instructions/maxAttempts/confidenceThreshold). This can cause invalid user configurations.Also applies to: 155-173
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/CONFIGURATION.md` around lines 123 - 147, The docs' AgentNetworkConfig and RouterConfig definitions are out of sync with the actual exported types in src/lib/types/agentNetwork.ts; update the documentation to mirror the real type shapes (or import/generate them) so examples and field names match exactly — e.g., replace the documented type/fallbackAgent/maxRetries with the actual RouterConfig structure and fields such as provider, model, instructions, maxAttempts, confidenceThreshold and ensure AgentNetworkConfig's agents, workflows, tools, router and defaults entries match the exported types; also apply the same corrections to the other documented blocks noted (the later section with the same divergence).docs/agents/VERIFICATION.md-289-302 (1)
289-302:⚠️ Potential issue | 🟠 MajorCLI gap status is outdated and currently incorrect.
Line 289 onward says agent/network CLI commands are unavailable, but
src/cli/commands/agent.tsalready implementsagent create/list/execute(run)andnetwork create/list/execute(run). This will produce false verification outcomes.Suggested doc correction
-**STATUS: GAP IDENTIFIED** +**STATUS: PARTIAL COVERAGE** -The following CLI commands are NOT implemented: +The following commands are implemented: +- [x] `neurolink agent create` +- [x] `neurolink agent list` +- [x] `neurolink agent execute` (`run` alias) +- [x] `neurolink network create` +- [x] `neurolink network list` +- [x] `neurolink network execute` (`run` alias) -**Recommendation**: Implement CLI commands for Multi-Agent Networks feature. +Remaining gaps should be tracked only for commands not present (e.g., `show/status/delete` if still missing).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/VERIFICATION.md` around lines 289 - 302, The VERIFICATION.md incorrectly marks agent/network CLI commands as unavailable; update the doc to reflect the implemented commands by removing or checking off the outdated checklist and replacing it with the actual supported commands (e.g., the agent create/list/execute(run) and network create/list/execute(run) entries implemented in src/cli/commands/agent.ts), and ensure any mention of network status reflects whether a `network status` command exists in the CLI codebase so the verification output matches the real implementations.src/lib/agent/evaluation/evaluator.ts-437-440 (1)
437-440:⚠️ Potential issue | 🟠 MajorHandle
maxIterations <= 0before returning here.If a caller passes
maxIterations: 0, the loop never runs andcurrentResult!/currentScore!throw on return. Validate the config up front or return a typed “no iterations executed” result instead of relying on non-null assertions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/evaluation/evaluator.ts` around lines 437 - 440, The return path in evaluate (the code that returns finalResult: currentResult!, iterations, finalScore: currentScore!) assumes at least one iteration ran; add an upfront validation in the evaluator configuration (or inside the evaluate function) to handle maxIterations <= 0 by either throwing a clear error or returning a typed "no iterations executed" result (e.g., finalResult: null/undefined and finalScore: null/undefined with iterations: 0); update the logic around currentResult/currentScore (remove non-null assertions) so evaluate/Evaluator returns a safe, documented value when maxIterations is zero.src/lib/agent/routerAgent.ts-104-140 (1)
104-140:⚠️ Potential issue | 🟠 MajorValidate the routed primitive against the registered registry before returning.
parseRoutingResponse()only proves the model emitted JSON, not that the chosenselectedPrimitive.idactually exists inthis.primitives. If the model hallucinates an ID,route()returns it as success andAgentNetwork.execute()later dies withPrimitive not found. This should fall back here, the same way parse/generate failures do.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/routerAgent.ts` around lines 104 - 140, The parsed routing result (from parseRoutingResponse in route()) may contain a hallucinated selectedPrimitive.id that isn't registered in this.primitives; before returning the AgentRoutingDecision, verify that parsed.selectedPrimitive.id exists in the router's registry (this.primitives or the underlying map/array), and if it does not, log a warning and return the fallback via createFallbackDecision(task, primitivesArray[0]) exactly like the catch path; place this validation right after building the decision (using AgentRoutingDecision, selectedPrimitive.id, this.primitives and createFallbackDecision) so confidence checks still run only for validated decisions.test/continuous-test-suite-agents.ts-294-1171 (1)
294-1171:⚠️ Potential issue | 🟠 MajorThis “continuous integration” suite mostly validates fixtures, not the implementations it claims to cover.
The agent/network/routing/message-bus sections overwhelmingly assert JSON fixtures and mock behavior. A broken
RouterAgent,AgentNetwork, orMessageBusimplementation could still pass this file unchanged. At minimum, the suite needs one real end-to-end path per component instead of treating fixture shape checks as integration coverage.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-agents.ts` around lines 294 - 1171, The integration suite only validates fixtures and mocks; add at least one real end-to-end test per component by instantiating and exercising the real classes when imports succeed: inside testIntegration() where Agent, AgentNetwork, and MessageBus are conditionally imported, replace or augment the fixture-only/skipped tests with real behavioral checks that create an Agent via new AgentClass(...), send a message through a real MessageBus instance and assert delivery, and create an AgentNetwork instance to route a task between agents and assert the routed result; use existing helpers runTest, skipTest, and createMockSdk to wrap these checks and only skip when the concrete module import fails.src/lib/agent/coordination/task-distributor.ts-230-243 (1)
230-243:⚠️ Potential issue | 🟠 MajorUnresolved dependencies can spin this queue forever.
A task only proceeds when every dependency is
"completed". If one dependency fails or references a missing task, this branch keeps moving the item to the back of the queue forever, andsubmitTask()can hang behind it. Treat failed/missing dependencies as terminal or give this path its own retry/timeout budget.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/task-distributor.ts` around lines 230 - 243, The loop that checks item.task.dependencies can spin forever when a dependency is missing or failed; update the dependency check in task-distributor.ts (around item.task.dependencies / this.activeResults / this.taskQueue) to detect terminal dependency states: if depResult is undefined or depResult.status === "failed" (or another non-retriable status) mark the waiting task as failed/terminal (update its result/status and remove from the queue) so submitTask() is not blocked, or alternatively add a per-item retry budget (e.g., task.retryCount vs MAX_DEP_RETRIES on the queued item) and increment it each time the deps are incomplete — when budget is exceeded mark the task failed and log the reason; ensure you update activeResults and do not continually requeue items with unrecoverable dependencies.test/continuous-test-suite-agents.ts-1192-1248 (1)
1192-1248:⚠️ Potential issue | 🟠 MajorThe CLI coverage report is stale and now hides regressions.
This PR adds agent/network CLI commands in
src/cli/commands/agent.ts, but this suite still prints “NO CLI commands” and skips every related case. That turns newly added functionality into a permanent false negative instead of validating it.src/cli/commands/agent.ts-175-230 (1)
175-230:⚠️ Potential issue | 🟠 Major
agent create --file ...cannot work with the current option schema.The handler supports loading a definition from
--file, butid,name,description, andinstructionsare all markeddemandOption: true. Yargs will reject the command before Line 375 ever runs, so the example at Lines 227-229 is impossible. These fields need to be conditionally required only when--fileis absent.Also applies to: 375-395
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/agent.ts` around lines 175 - 230, The options id, name, description, and instructions are currently marked demandOption in buildCreateOptions which prevents using --file; remove demandOption: true for those four options in buildCreateOptions and instead add a runtime check in the create command handler (the function that processes the "agent create" command) to enforce that when argv.file is not provided, id, name, description, and instructions must be present (returning a helpful error or exiting otherwise); this keeps the --file flow working while maintaining the original validation when no file is used.src/lib/agent/agent.ts-60-62 (1)
60-62:⚠️ Potential issue | 🟠 MajorThe public event API never emits anything.
Agentexposeson()/off()and allocates anEventEmitter, but neitherexecute()norstream()callsthis.emitter.emit(...). Consumers can subscribe successfully and still never receive start/completion/error events.Also applies to: 120-343, 503-516
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agent.ts` around lines 60 - 62, Agent's public event API never emits because the EventEmitter stored on the Agent instance is never used; update the Agent class (referencing the emitter property and the execute() and stream() methods) to emit lifecycle events: call this.emitter.emit('start', {...}) at the beginning of execute() and stream(), emit this.emitter.emit('complete', {result,...}) on successful completion, and emit this.emitter.emit('error', {error,...}) inside catch blocks (or a finally with error detection) so subscribers receive start/completion/error notifications; for stream() also emit progress or chunk events (e.g., this.emitter.emit('data', chunk) or 'progress') as each chunk is produced so streaming consumers get updates.src/lib/agent/agentNetwork.ts-317-377 (1)
317-377:⚠️ Potential issue | 🟠 MajorPrimitive failures are swallowed, so the network can “complete” with
nulloutput.
executePrimitive()converts every primitive exception into{ output: null, error }, and the main loop never treatsstepResult.erroras terminal. That means repeated agent/workflow/tool failures can still end instatus: "completed"with"null"content aftermaxStepsis exhausted. Either rethrow here or stop the loop when a step fails.Also applies to: 707-713
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agentNetwork.ts` around lines 317 - 377, The loop currently swallows primitive failures because executePrimitive returns {output: null, error} and the caller never treats stepResult.error as terminal; update the loop after receiving stepResult (the block handling executePrimitive, stepResult, finalResult and the isTaskComplete check) to immediately handle failures: if stepResult.error is truthy, either rethrow the error (propagate it) or set the overall status to failed and stop the loop (e.g., break/return) so we don't return status "completed" with null output; apply the same change to the equivalent handling at the other location that mirrors this logic (the block around isTaskComplete at lines referenced in the review), ensuring executePrimitive, stepResult, finalResult and isTaskComplete are used to detect and abort on failures rather than continue.src/lib/agent/agentNetwork.ts-461-624 (1)
461-624:⚠️ Potential issue | 🟠 MajorStreaming mode never populates
trace.stepsor usage totals.
traceandtotalUsageare initialized, but this path never pushes aNetworkExecutionStepand never accumulates usage. As a result,network-complete.result.trace.steps.lengthis always0and usage remains empty in streaming mode, even after real work was done.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agentNetwork.ts` around lines 461 - 624, The streaming branch never appends a NetworkExecutionStep to trace.steps nor adds tokens to totalUsage, so update the loop to always create and push a step after primitive execution: for non-streaming use the usage from executePrimitive's result (result.usage) to increment totalUsage and include that usage in the pushed NetworkExecutionStep; for streaming create a local usage accumulator (e.g., usageDuringStream) while iterating agentPrimitive.agent.stream (collect usage if chunks or the agent exposes final usage) and after the stream finishes increment totalUsage with usageDuringStream and include it in the pushed step; ensure the pushed step contains primitive id/type/name, input, output (finalResult), timestamp and usage so network-complete.trace.steps is populated for both streaming and non-streaming runs.src/lib/agent/evaluation/evaluator.ts-219-241 (1)
219-241:⚠️ Potential issue | 🟠 MajorReplace the greedy regex JSON extraction with a bounded parser.
Both parsers run
/\{[\s\S]*\}/against uncontrolled model output. Besides being brittle when the response contains multiple brace blocks, this is the exact pattern CodeQL flags for pathological backtracking. A small balanced-brace scanner or fenced-JSON extractor is safer here.Also applies to: 248-270
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/evaluation/evaluator.ts` around lines 219 - 241, The current parseEvaluationResponse method uses a greedy regex /\{[\s\S]*\}/ which can catastrophically backtrack and mis-extract when model output contains multiple brace blocks; replace that regex in parseEvaluationResponse with a bounded/fenced JSON extractor: locate the first '{', iterate characters while tracking a brace depth counter (handle string literals and escape sequences so braces inside strings are ignored), stop when depth returns to zero to slice the balanced JSON substring, then JSON.parse that slice and preserve the current return shape (overall, breakdown, feedback) or return null on failure; apply the same balanced-brace extraction fix to the other JSON extraction in this file that mirrors parseEvaluationResponse.src/lib/agent/agent.ts-406-470 (1)
406-470:⚠️ Potential issue | 🟠 MajorUse the shared tool transformer here instead of hand-rolling provider payloads.
This code manually builds the
toolsobject and passes it straight intogenerate()/stream(). That bypasses the provider-specific formatting path used elsewhere in NeuroLink and is likely to break tools on providers with stricter schemas. As per coding guidelines "UsetransformAvailableTools()to format tools for AI model calls."Also applies to: 477-500
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agent.ts` around lines 406 - 470, The tools object is being passed raw into NeuroLink.generate/stream, bypassing provider-specific formatting; update buildGenerateOptions (and the equivalent payload construction used for streaming) to call the shared transformAvailableTools(...) helper to produce provider-formatted tools before including them in the returned options. Concretely: obtain filtered tools via getFilteredTools(), if non-empty call transformAvailableTools(filteredTools, this.provider) (or the Neurolink instance method exposing that transformer) and include the transformed result as the tools payload instead of the raw filteredTools; apply the same change to the streaming payload construction used elsewhere (the code around buildGenerateOptions and the stream call that currently injects tools directly).src/cli/commands/agent.ts-36-37 (1)
36-37: 🛠️ Refactor suggestion | 🟠 MajorRemove the type re-export from this CLI module.
Files outside
src/lib/types/should not re-export types. Keep these imports local and expose the types only from the canonical barrel. As per coding guidelines "Files outsidesrc/lib/types/must not re-export types (export type { X } from). Consumers should import types fromsrc/lib/types/directly."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/agent.ts` around lines 36 - 37, Remove the type re-export "export type { AgentCommandArgs, NetworkCommandArgs }" from this CLI module; instead keep those types imported locally with an import type (if the file needs them) and do not re-export them, and ensure external consumers import AgentCommandArgs and NetworkCommandArgs from the canonical barrel in src/lib/types/ rather than from this module.src/lib/agent/coordination/task-distributor.ts-467-480 (1)
467-480:⚠️ Potential issue | 🟠 MajorUse
withTimeouthere instead of a rawPromise.race().The timeout promise is never cancelled when
agent.execute()wins, so every successful task leaves a live timer behind until it expires. The shared utility already solves this pattern cleanly and avoids timer leakage. As per coding guidelines "Error handling should useErrorFactoryfor typed errors andwithTimeoututility for async calls."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/task-distributor.ts` around lines 467 - 480, Replace the Promise.race timer leakage by wrapping the agent.execute call with the shared withTimeout utility: compute timeoutMs as task.deadline ? task.deadline - Date.now() : this.config.taskTimeout, then call withTimeout(agent.execute(task.input, { context: task.metadata, timeout: timeoutMs }), timeoutMs, /* create a typed timeout error via ErrorFactory */) so the timeout is cancelled when execute resolves and the thrown timeout uses ErrorFactory for a typed error; update the code around agent.execute, withTimeout, ErrorFactory, and this.config.taskTimeout accordingly.src/lib/agent/orchestration/topology.ts-514-547 (1)
514-547:⚠️ Potential issue | 🟠 Major
fromJSON()should restore the serialized topology id.
toJSON()exportsid, butfromJSON()ignores it and keeps the constructor-generatedtopologyId. A serialize/deserialize round trip changes the public identifier exposed bygetId().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/orchestration/topology.ts` around lines 514 - 547, fromJSON currently ignores the serialized topology id so a toJSON→fromJSON round-trip loses the original id; update the fromJSON(data) method to restore the exported id by assigning data.id (or data.topologyId if that's the serialized name) back to this.topologyId (or the internal field used by getId()), then proceed to restore config.type, nodes and edges as before; ensure the identifier name matches the key emitted by toJSON (id) and that getId() will return the restored value.src/lib/agent/coordination/coordinator.ts-499-512 (1)
499-512:⚠️ Potential issue | 🟠 MajorTimeouts do not cancel the underlying agent execution.
Promise.race()returns a timeout error, butAgent.execute()has no abort/cancel hook, so the timed-out run keeps going after Line 511 removes it fromactiveExecutions. That breaks concurrency accounting and allows late side effects from work the coordinator believes is finished.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/coordinator.ts` around lines 499 - 512, The timeout path currently wins the Promise.race but does not cancel the underlying executionPromise, so Agent.execute() keeps running after this.activeExecutions.delete(agent.id); update the coordinator to create an AbortController per execution, pass its signal into Agent.execute (or add an abort hook to Agent.execute if missing), and on the timeout branch call controller.abort() before returning the timeout error; ensure the finally block still deletes from activeExecutions and that Agent.execute respects the abort signal to stop work and release resources.src/lib/agent/supervisor/supervisor-agent.ts-348-354 (1)
348-354:⚠️ Potential issue | 🟠 MajorDon't fire-and-forget timeout escalations.
handleApprovalTimeout()callsthis.escalate(...)without awaiting or catching it. If the escalation handler or HITL notify path rejects, that becomes an unhandled rejection and the caller never sees the escalation outcome.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/supervisor/supervisor-agent.ts` around lines 348 - 354, The escalate call in handleApprovalTimeout is being fire-and-forgotten; change it to await and handle errors so rejections don't become unhandled and the caller can observe the outcome. Specifically, in the "escalate" branch where this.escalate({ type: "timeout", ... }) is invoked, await the Promise (e.g., await this.escalate(...)) and wrap it in a try/catch to log or rethrow the error as appropriate so failures in escalate or downstream HITL notify paths are surfaced to callers of handleApprovalTimeout.src/lib/agent/orchestration/topology.ts-247-295 (1)
247-295:⚠️ Potential issue | 🟠 MajorReject non-positive
maxChildrenbefore building a hierarchy.If
maxChildren <= 0, the inner assignment loop never runs and every non-root node remains orphaned. Validate this in both the topology build path and the fluent builder setter.Also applies to: 610-612
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/orchestration/topology.ts` around lines 247 - 295, The buildHierarchicalTopology function should explicitly reject non-positive maxChildren values and the fluent builder setter that sets config.maxChildren must validate inputs; update buildHierarchicalTopology to check this.config.maxChildren (or maxChildren local) and throw or fallback (e.g., throw new Error or set to 1) when <= 0 before the queue loop so child assignment cannot silently fail, and update the fluent builder setter (the method that assigns maxChildren in the topology config) to validate the argument and throw an informative error when a non-positive number is passed.src/lib/agent/coordination/coordinator.ts-90-138 (1)
90-138:⚠️ Potential issue | 🟠 MajorPer-call coordinator overrides are ignored after the switch.
Line 90 merges
optionsintoconfig, butexecuteSequential(),executeParallel(), andexecuteAgentWithTimeout()keep readingthis.config. Overrides likecontinueOnFailure,maxConcurrency, andagentTimeouttherefore never take effect for a single run.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/coordinator.ts` around lines 90 - 138, The per-call overrides in options are not used because downstream methods still read this.config; update the coordination flow so the merged local config is honored by either (a) passing the local config object into all executor methods (executeSequential, executeParallel, executePipeline, executeRoundRobin, executeLeastBusy, and any customCoordinator invocation) and updating executeAgentWithTimeout and other helper functions to accept and use that config parameter (e.g., agentTimeout, maxConcurrency, continueOnFailure), or (b) ensure those methods read from a config argument instead of this.config; replace usages of this.config inside executeSequential/executeParallel/executeAgentWithTimeout with the passed-in config so per-call overrides take effect.src/lib/agent/communication/protocols.ts-46-58 (1)
46-58:⚠️ Potential issue | 🟠 MajorDiscovery and heartbeat messages never reach
handleProtocolMessage().The manager only subscribes to
"protocol"anddirect:${agentId}, butdiscover()publishes to"protocol:discovery"andsendHeartbeat()publishes to"protocol:heartbeat". With no subscribers on those topics, discovery requests time out and heartbeats are never observed.Also applies to: 146-151, 283-301
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/communication/protocols.ts` around lines 46 - 58, The manager subscribes only to "protocol" and `direct:${agentId}` so messages published by `discover()` ("protocol:discovery") and `sendHeartbeat()` ("protocol:heartbeat") never reach `handleProtocolMessage`; update the subscriptions in the `messageBus.subscribe` calls (the ones that currently call this.handleProtocolMessage.bind(this)) to also subscribe to the specific protocol subtopics (e.g., "protocol:discovery" and "protocol:heartbeat") or use the message bus's wildcard/namespace subscription (e.g., "protocol:*") so `handleProtocolMessage` receives discovery and heartbeat messages; apply the same change to the other similar subscription sites noted around the file (lines referenced near the existing calls).src/lib/agent/orchestration/orchestrator.ts-81-97 (1)
81-97:⚠️ Potential issue | 🟠 MajorGlobal agent cap is declared but never enforced.
maxTotalAgentsis part ofresourceLimits, butcreateNetwork()only checksmaxNetworksandmaxAgentsPerNetwork. Repeated network creation can still exceed the configured total-agent ceiling.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/orchestration/orchestrator.ts` around lines 81 - 97, The createNetwork() path in Orchestrator currently enforces maxNetworks and maxAgentsPerNetwork but ignores resourceLimits.maxTotalAgents; before allowing a new network (or adding default agents during creation) compute the current total agents by summing agent counts across this.networks (e.g., iterate this.networks.values() and sum their .agents.length or whatever agent collection is used) and compare to this.config.resourceLimits.maxTotalAgents, and if the new network would cause the total to exceed that ceiling throw a clear Error mentioning maxTotalAgents; ensure you reference the same checks around createNetwork() and use this.config.resourceLimits.maxTotalAgents as the guard.src/lib/agent/orchestration/orchestrator.ts-141-169 (1)
141-169:⚠️ Potential issue | 🟠 Major
enableHierarchyis currently a no-op.
createHierarchicalNetwork()never checks the flag before creating parent/child networks, so callers can build hierarchies even when the orchestrator is configured to disable them.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/orchestration/orchestrator.ts` around lines 141 - 169, The createHierarchicalNetwork function ignores the orchestrator flag for hierarchies; add a guard that checks this.config.enableHierarchy at the start of createHierarchicalNetwork and prevent hierarchical builds when false: if enableHierarchy is false and a parentNetworkId is provided (or the caller intends hierarchical supervision), reject by throwing a clear Error (or fallback to creating a non-hierarchical network) instead of proceeding; update logic around parentNetworkId handling and the call to createNetwork (symbols: createHierarchicalNetwork, this.config.enableHierarchy, parentNetworkId, createNetwork) so hierarchies cannot be created when the flag is disabled.src/lib/agent/coordination/coordinator.ts-565-592 (1)
565-592:⚠️ Potential issue | 🟠 MajorConcurrent dependency batches can stamp duplicate step indices.
Each async assignment captures
const stepIndex = steps.lengthbefore any promise in the batch pushes a step. Multiple tasks in the same batch can therefore write the sameindex, corrupting the execution trace.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/coordinator.ts` around lines 565 - 592, The bug is that each async task captures const stepIndex = steps.length before concurrent promises push into steps, causing duplicate indices; fix by assigning the index at push time using a single shared, incrementing counter (e.g., a coordinator-level numeric field like this.nextStepIndex) or by computing stepIndex immediately before pushing (stepIndex = steps.length) so the assignment happens synchronously; update the block around batchPromises/executeAgentWithTimeout and steps.push to use that unique index (and increment the counter) instead of the pre-captured stepIndex to ensure unique, monotonic step indices in the execution trace.src/lib/agent/orchestration/orchestrator.ts-217-228 (1)
217-228:⚠️ Potential issue | 🟠 MajorExecution state checks are inconsistent between normal and streaming runs.
executeNetwork()only guards the"executing"state, so"paused"and"shutdown"networks still run.streamNetwork()skips the queue andactiveExecutionsbookkeeping entirely, so it can also bypassmaxConcurrentExecutions.Also applies to: 335-355
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/orchestration/orchestrator.ts` around lines 217 - 228, executeNetwork currently only treats networks with state "executing" as busy and skips checks for "paused" and "shutdown", and streamNetwork bypasses queuing and activeExecutions counting entirely, allowing runs to bypass maxConcurrentExecutions; update both executeNetwork and streamNetwork to consistently check networkInfo.get(networkId) state (treat "paused" and "shutdown" as non-runnable and enqueue or reject as appropriate), reuse the same concurrency guard logic (check this.activeExecutions.size against this.config.maxConcurrentExecutions and call this.queueExecution when limit reached), and ensure both paths update this.activeExecutions and call this.executeNetworkInternal (or the streaming equivalent) in the same places so bookkeeping is uniform; reference functions/fields: executeNetwork, streamNetwork, this.networkInfo, this.activeExecutions, this.config.maxConcurrentExecutions, this.queueExecution, executeNetworkInternal.src/lib/agent/coordination/coordinator.ts-369-374 (1)
369-374:⚠️ Potential issue | 🟠 Major
roundRobinnever rotates past agent 0.
context.currentStepstarts at0for everycoordinate()call and this method never increments or persists a cursor, soselectedIndexis always0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/coordinator.ts` around lines 369 - 374, The round-robin selector always picks agent 0 because context.currentStep is initialized to 0 per coordinate() call and never updated; fix by maintaining a persistent cursor instead of relying on context.currentStep local value — add and use a Coordinator-scoped rotating index (e.g., this.roundRobinIndex) or persist/increment context.currentStep across coordinate() invocations, compute selectedIndex = this.roundRobinIndex % agents.length (or the persisted context value), call executeAgentWithTimeout(agent, ...) and then increment and modulo the cursor so future coordinate() calls rotate through agents; update any references in coordinate() and related methods accordingly.src/lib/agent/communication/protocols.ts-191-200 (1)
191-200:⚠️ Potential issue | 🟠 MajorDelegated agent failures are reported as successful completions.
Agent.execute()returns anAgentResultwithstatus: "error"instead of throwing on execution failures. This handler therefore replies withaccepted: true/state: "completed"even for failed delegated runs, anddelegate()then stamps the local session as completed unconditionally. Checkresult.statusand derivesession.statefrom the response payload.Also applies to: 406-421
src/lib/agent/supervisor/supervisor-agent.ts-516-534 (1)
516-534:⚠️ Potential issue | 🟠 MajorSanitize audit payloads before logging them at info level.
action.detailscarries raw inputs/options, and the result snapshot can include tool outputs or error text. Logging that object directly turns audit mode into a prompt/secret leakage path.As per coding guidelines,
Use transformParamsForLogging() to safely strip secrets before logging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/supervisor/supervisor-agent.ts` around lines 516 - 534, In auditLog (SupervisorAgent.auditLog) you're currently logging raw action.details and result fields; call transformParamsForLogging() to sanitize action.details before including it in logEntry and replace result.content/result.error/result.toolsUsed entries with their sanitized equivalents (or contentLength-only) so secrets aren't leaked; specifically, transform action.details via transformParamsForLogging(action.details), sanitize each toolsUsed/output/error via transformParamsForLogging or omit full text and keep only metadata (e.g., contentLength, tool names), then log the sanitized logEntry and emit it.src/lib/types/agentNetwork.ts-81-88 (1)
81-88:⚠️ Potential issue | 🟠 Major
AgentResult.toolExecutionsdoes not match the normal runtime shape.The regular
generate()path normalizes tool executions to{ name, input, output, duration }, but this type advertises{ toolName, args, result, success, duration }. That makes the public contract lie about what callers actually receive outside the MCP flow.Based on learnings, the normal
generate()flow should returntoolExecutionsnormalized bytransformToolExecutions()with{ name, input, output, duration }, while the MCP path is the only one that uses a different shape.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/agentNetwork.ts` around lines 81 - 88, The AgentResult.toolExecutions type is incorrect for the normal runtime: update the toolExecutions declaration to match the normalized shape returned by generate()/transformToolExecutions()—i.e., use an array of objects with { name: string; input: unknown; output: unknown; duration: number }—and keep the MCP-specific shape separate (e.g., add a new field or union only if MCP consumers need the alternate { toolName, args, result, success, duration } shape); ensure references to transformToolExecutions(), generate(), and AgentResult.toolExecutions are consistent so callers see the same runtime shape.src/lib/agent/supervisor/supervisor-agent.ts-115-122 (1)
115-122:⚠️ Potential issue | 🟠 MajorApply approved mutations to the actual agent invocation.
Lines 116-117 update
action.details, but Line 122 still executes the originalinput/options. A reviewer can therefore approve a rewritten payload while the wrapped agent still runs the old one.Possible fix
if (preReview.modifications) { Object.assign(action.details, preReview.modifications); } } // Execute the agent - const result = await this.agent.execute(input, options); + const executionInput = (action.details.input ?? input) as AgentInput; + const executionOptions = ( + action.details.options ?? options + ) as AgentExecutionOptions | undefined; + const result = await this.agent.execute( + executionInput, + executionOptions, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/supervisor/supervisor-agent.ts` around lines 115 - 122, The code updates approved mutations into action.details (preReview.modifications) but still calls this.agent.execute with the original input/options; change the invocation so the agent runs the mutated payload: after applying Object.assign(action.details, preReview.modifications), build or replace the execution arguments (e.g., update input and/or options from action or action.details) and call this.agent.execute with those mutated values instead of the original input/options; ensure you reference the same objects mutated (preReview, action, action.details, and this.agent.execute) so the approved modifications are actually used at runtime.src/lib/agent/communication/message-bus.ts-235-243 (1)
235-243:⚠️ Potential issue | 🟠 MajorSuccessful requests leak their temporary reply subscriptions.
The timeout path unsubscribes
reply:*, but the success path only clears the timer and pending map. Every completedrequest()leaves one dead subscription/topic behind.Possible fix
- this.subscribe(replyTo, senderId, (msg) => { + const subscriptionId = this.subscribe(replyTo, senderId, (msg) => { const pending = this.pendingRequests.get(correlationId); if (pending) { clearTimeout(pending.timeout); this.pendingRequests.delete(correlationId); + this.unsubscribe(subscriptionId); pending.resolve(msg); } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/communication/message-bus.ts` around lines 235 - 243, The success handler inside the request flow currently clears the timeout and pendingRequests but fails to remove the temporary reply subscription (created via subscribe(replyTo, senderId, ...)), leaking topics; update the success path in request() so after clearTimeout(pending.timeout) and this.pendingRequests.delete(correlationId) you also call the corresponding unsubscribe for the replyTo subscription (using the same senderId/correlationId context or stored subscription handle) to mirror the timeout path and ensure the temporary subscription is removed when pending.resolve(msg) is invoked.src/lib/types/agentNetwork.ts-807-817 (1)
807-817:⚠️ Potential issue | 🟠 Major
NetworkStreamChunkbreaks discriminated union narrowing by includingNetworkStreamChunkBase.Because
NetworkStreamChunkBaseaccepts anyNetworkStreamChunkType(a union of all literal types), checkingchunk.type === "network-start"cannot prove thatnetworkIdandinputexist—bothNetworkStartChunkand the base member match. Consumers must use type assertions (e.g.,as NetworkStartChunk) instead of relying on automatic narrowing, losing type safety on the public streaming API. RemoveNetworkStreamChunkBasefrom the union and list only concrete members, or ensure the base member'stypefield excludes literals that appear in concrete members.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/agentNetwork.ts` around lines 807 - 817, NetworkStreamChunk currently includes NetworkStreamChunkBase which has a permissive type field and defeats discriminated-union narrowing; remove NetworkStreamChunkBase from the NetworkStreamChunk union (or alternately change NetworkStreamChunkBase so its `type` excludes the concrete literal types) so that checks like `chunk.type === "network-start"` reliably narrow to NetworkStartChunk and expose `networkId` and `input`; update the NetworkStreamChunk declaration to list only the concrete members (e.g., NetworkStartChunk, RoutingDecisionChunk, PrimitiveStartChunk, PrimitiveEndChunk, AgentTextChunk, AgentToolCallChunk, AgentToolResultChunk, NetworkCompleteChunk, NetworkErrorChunk) or adjust NetworkStreamChunkBase.type to avoid overlapping literals.
🟡 Minor comments (4)
docs/agents/CLI-COVERAGE.md-311-320 (1)
311-320:⚠️ Potential issue | 🟡 MinorOrdered-list numbering violates markdownlint (MD029).
These list items should use sequential numbering style (
1,2,3...) to satisfy repo linting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/CLI-COVERAGE.md` around lines 311 - 320, The ordered list under the "Priority 3: Status Commands" section is using non-sequential numbers (5–11) which triggers MD029; update the list so it uses sequential numbering starting at 1 for that block (e.g., 1. `neurolink agent create`, 2. `neurolink network create`, 3. `neurolink agent show`, 4. `neurolink network show`, 5. `neurolink network status`, 6. `neurolink agent delete`, 7. `neurolink network delete`) to satisfy markdownlint.docs/agents/TESTING.md-177-180 (1)
177-180:⚠️ Potential issue | 🟡 MinorImport path in example is incorrect.
The import path
../../src/lib/agents/Agent.jsuses plural "agents" and capitalized "Agent", but the actual module path issrc/lib/agent/agent.js(singular, lowercase).📝 Suggested fix
-import { Agent } from "../../src/lib/agents/Agent.js"; +import { Agent } from "../../../src/lib/agent/agent.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/TESTING.md` around lines 177 - 180, Update the example import to use the actual module path: replace the incorrect import of Agent from "../../src/lib/agents/Agent.js" with the correct singular/lowercase path (src/lib/agent/agent.js) so the example imports the Agent module that exists; ensure the import statement in the TESTING.md snippet references the module name "agent" (lowercase) and the correct relative path to that file.src/lib/agent/evaluation/evaluator.ts-505-513 (1)
505-513:⚠️ Potential issue | 🟡 MinorThe streamed completion payload can still be wrong even when optimization succeeds.
This branch hardcodes
totalDuration: 0, so consumers ofoptimizeStream()get a bogus final metric. It also usescurrentResult!/currentScore!, which will throw ifmaxIterationsis0.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/evaluation/evaluator.ts` around lines 505 - 513, The final "optimization-complete" payload in optimizeStream (e.g., the yield in evaluator.ts) currently hardcodes totalDuration: 0 and uses non-null asserted currentResult!/currentScore!, which will produce bogus metrics or throw when no iterations ran; fix by tracking a start timestamp at the beginning of optimizeStream (e.g., startTime = Date.now()), compute totalDuration = Date.now() - startTime for the final payload, and replace currentResult! and currentScore! with safe fallbacks (e.g., currentResult ?? initialResult or null, and currentScore ?? null or a sentinel) so the finalResult/finalScore are valid even if iterations.length === 0. Ensure iteration count remains iterations.length and that any newly added startTime is declared in the same scope as the existing loop and final yield.src/lib/agent/agent.ts-186-193 (1)
186-193:⚠️ Potential issue | 🟡 MinorPreserve tool execution durations instead of hardcoding
0.
generate()already returns normal-flowtoolExecutionsin the normalized shape, including timing. Overwriting every entry withduration: 0throws away that observability data for no benefit. Based on learningsgenerate()returnstoolExecutionsnormalized bytransformToolExecutions()with{ name, input, output, duration }.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agent.ts` around lines 186 - 193, The mapping that builds toolExecutions overwrites real timing info by setting duration: 0; update the transform in agent.ts (the toolExecutions mapping used after generate()/transformToolExecutions()) to preserve the original duration (use te.duration or fallback to 0) instead of hardcoding 0 so existing timing/observability from result.toolExecutions is kept.
🧹 Nitpick comments (6)
test/unit/agent/agent.test.ts (1)
11-11: Import types from the barrel file, not specific type files.As per coding guidelines, code outside
src/lib/types/should import internal types from the barrel (../types/index.js), not from specific type files.📝 Suggested fix
-import type { AgentDefinition } from "../../../src/lib/types/agentNetwork.js"; +import type { AgentDefinition } from "../../../src/lib/types/index.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/agent/agent.test.ts` at line 11, The test is importing AgentDefinition directly from "../../../src/lib/types/agentNetwork.js"; update the import to come from the types barrel instead (e.g., "../../../src/lib/types/index.js" or the barrel export path used in the repo) so AgentDefinition is imported from the central barrel export; locate the import of AgentDefinition in test/unit/agent/agent.test.ts and replace its source module to the types barrel while keeping the type name unchanged.test/integration/agent/coordinator.integration.test.ts (1)
11-17: Import types from the barrel file, not specific type files.As per coding guidelines, code outside
src/lib/types/should import internal types from the barrel.📝 Suggested fix
-import type { - AgentPrimitive, - Primitive, - RouterConfig, - ToolPrimitive, - WorkflowPrimitive, -} from "../../../src/lib/types/agentNetwork.js"; +import type { + AgentPrimitive, + Primitive, + RouterConfig, + ToolPrimitive, + WorkflowPrimitive, +} from "../../../src/lib/types/index.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/agent/coordinator.integration.test.ts` around lines 11 - 17, The import statement currently pulling types from "../../../src/lib/types/agentNetwork.js" should use the barrel export instead; update the import to import AgentPrimitive, Primitive, RouterConfig, ToolPrimitive, WorkflowPrimitive from the package barrel (i.e., "../../../src/lib/types") so the test imports types via the centralized barrel file rather than a specific internal type file, leaving the exact type names unchanged.test/integration/agent/agent.integration.test.ts (1)
12-12: Import types from the barrel file, not specific type files.As per coding guidelines, code outside
src/lib/types/should import internal types from the barrel.📝 Suggested fix
-import type { AgentDefinition } from "../../../src/lib/types/agentNetwork.js"; +import type { AgentDefinition } from "../../../src/lib/types/index.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/agent/agent.integration.test.ts` at line 12, The test imports the AgentDefinition type directly from the specific type file; update the import to use the types barrel module that re-exports internal types instead. Locate the import statement referencing AgentDefinition and replace the module path so it imports AgentDefinition from the central types barrel (the module that re-exports types from src/lib/types) rather than from the specific agentNetwork type file. Ensure it remains a type-only import (import type { AgentDefinition }) and run tests to verify no type resolution errors.test/integration/agent/network.integration.test.ts (1)
12-17: Import types from the barrel file, not specific type files.As per coding guidelines, code outside
src/lib/types/should import internal types from the barrel.📝 Suggested fix
-import type { - AgentDefinition, - AgentNetworkConfig, - NetworkStreamChunk, - NetworkWorkflowDefinition, -} from "../../../src/lib/types/agentNetwork.js"; +import type { + AgentDefinition, + AgentNetworkConfig, + NetworkStreamChunk, + NetworkWorkflowDefinition, +} from "../../../src/lib/types/index.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/integration/agent/network.integration.test.ts` around lines 12 - 17, The test imports the types AgentDefinition, AgentNetworkConfig, NetworkStreamChunk, and NetworkWorkflowDefinition directly from a specific type file; change the import so these types are imported from the lib/types barrel (the package's index re-export) instead of the specific type file — update the import statement that currently references those four identifiers to import them from the types barrel module so the test follows the project guideline of using barrel exports.src/lib/neurolink.ts (1)
27-28: DeferAgentandAgentNetworkloading from the main SDK entry point.These eager imports pull the multi-agent runtime into every import of
src/lib/neurolink.ts, even when callers never use the new orchestration APIs. Moving them behind a lazy loader would keep the feature truly opt-in.Based on learnings,
src/lib/neurolink.tsis the main SDK entry point that orchestrates all components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 27 - 28, The file currently eagerly imports Agent and AgentNetwork which forces the multi-agent runtime into every import; change those top-level imports to lazy/dynamic imports so the runtime is only loaded when used: remove the static imports of Agent and AgentNetwork and instead load them with dynamic import(...) inside the functions/exports that actually use them (e.g., where Agent or AgentNetwork constructors or types are referenced), or expose getters that call await import('./agent/agent.js') and import('./agent/agentNetwork.js') before returning the classes; ensure exported API surface (function names or exported getters) still returns the same Agent/AgentNetwork references so callers that never invoke those paths don’t pay the import cost.test/continuous-test-suite-agents.ts (1)
42-99: Usetypealiases here instead ofinterface.This file introduces several
interfacedeclarations, which conflicts with the repository-wide TypeScript rule. As per coding guidelines "Never useinterface. Always usetype X = { ... }."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-agents.ts` around lines 42 - 99, The declared interfaces (TestResult, TestSuiteResult, AgentDefinitionFixture, NetworkTopologyFixture, RoutingRuleFixture, MessageFixture) violate the repo rule to use type aliases; convert each interface to a type alias (e.g., type TestResult = { ... }) preserving the exact property names, optional markers, and types (including Record<string, unknown>), so semantics remain identical while replacing the interface keyword with type for all six declarations.
| // In-memory storage for agents and networks (session-based) | ||
| const registeredAgents: Map<string, AgentDefinition> = new Map(); | ||
| const registeredNetworks: Map<string, AgentNetworkConfig> = new Map(); |
There was a problem hiding this comment.
Process-local registries make the new CLI unusable across normal invocations.
registeredAgents and registeredNetworks are reset on every CLI process start. In practice, neurolink agent create ... and a later neurolink agent execute ... run in different processes, so the second command never sees the first command’s registration. This breaks the primary create/list/execute workflow unless everything happens inside one process, which the CLI does not support.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/cli/commands/agent.ts` around lines 39 - 41, The process-local registries
registeredAgents and registeredNetworks are ephemeral and get reset between CLI
invocations (so commands like neurolink agent create then neurolink agent
execute in separate processes don't see prior registrations); persist agent and
network registrations to a stable store instead of in-memory Maps: replace or
augment registeredAgents and registeredNetworks usage with a read/write-backed
provider (e.g., JSON file, key-value DB, or existing config persistence module)
and update functions that interact with these symbols (agent creation, listing,
execution code paths that reference registeredAgents/registeredNetworks) to load
from the persistent store at command start and save updates after mutations,
ensuring that create/list/execute operate across separate CLI processes.
| createOrchestrator( | ||
| config?: import("./types/index.js").OrchestratorConfig, | ||
| ): import("./agent/orchestration/index.js").NetworkOrchestrator { | ||
| const { NetworkOrchestrator } = require("./agent/orchestration/index.js"); | ||
| logger.debug("[NeuroLink] Creating network orchestrator", { | ||
| maxConcurrentExecutions: config?.maxConcurrentExecutions, | ||
| defaultMode: config?.defaultMode, | ||
| }); | ||
| return new NetworkOrchestrator(this, config); | ||
| } | ||
|
|
||
| /** | ||
| * Create a SupervisorAgent for oversight and control of agent execution. | ||
| * | ||
| * @param config - Supervisor configuration options | ||
| * @returns A new SupervisorAgent instance | ||
| * @since 8.38.0 | ||
| */ | ||
| createSupervisor( | ||
| config?: import("./types/index.js").SupervisorConfig, | ||
| ): import("./agent/supervisor/index.js").SupervisorAgent { | ||
| const { SupervisorAgent } = require("./agent/supervisor/index.js"); | ||
| logger.debug("[NeuroLink] Creating supervisor agent", { | ||
| supervisionLevel: config?.level, | ||
| }); | ||
| return new SupervisorAgent(this, config); | ||
| } | ||
|
|
||
| /** | ||
| * Create an AgentEvaluator for assessing agent output quality. | ||
| * | ||
| * @param config - Evaluator configuration options | ||
| * @returns A new AgentEvaluator instance | ||
| * @since 8.38.0 | ||
| */ | ||
| createEvaluator( | ||
| config?: import("./types/index.js").EvaluatorConfig, | ||
| ): import("./agent/evaluation/index.js").AgentEvaluator { | ||
| const { AgentEvaluator } = require("./agent/evaluation/index.js"); | ||
| logger.debug("[NeuroLink] Creating agent evaluator", { | ||
| model: config?.model, | ||
| provider: config?.provider, | ||
| }); | ||
| return new AgentEvaluator(this, config); | ||
| } | ||
|
|
||
| /** | ||
| * Create a ResultOptimizer for iteratively improving agent outputs. | ||
| * | ||
| * @param evaluator - Optional evaluator instance (creates default if not provided) | ||
| * @returns A new ResultOptimizer instance | ||
| * @since 8.38.0 | ||
| */ | ||
| createOptimizer( | ||
| evaluator?: import("./agent/evaluation/index.js").AgentEvaluator, | ||
| ): import("./agent/evaluation/index.js").ResultOptimizer { | ||
| const { ResultOptimizer } = require("./agent/evaluation/index.js"); | ||
| logger.debug("[NeuroLink] Creating result optimizer"); | ||
| return new ResultOptimizer(this, evaluator); | ||
| } | ||
|
|
||
| /** | ||
| * Create an AgentCoordinator for managing agent coordination strategies. | ||
| * | ||
| * @param config - Coordinator configuration options | ||
| * @returns A new AgentCoordinator instance | ||
| * @since 8.38.0 | ||
| */ | ||
| createCoordinator( | ||
| config?: import("./types/index.js").CoordinatorConfig, | ||
| ): import("./agent/coordination/index.js").AgentCoordinator { | ||
| const { AgentCoordinator } = require("./agent/coordination/index.js"); | ||
| logger.debug("[NeuroLink] Creating agent coordinator", { | ||
| strategy: config?.strategy, | ||
| maxConcurrency: config?.maxConcurrency, | ||
| }); | ||
| return new AgentCoordinator(this, config); | ||
| } | ||
|
|
||
| /** | ||
| * Create a MessageBus for inter-agent communication. | ||
| * | ||
| * @param config - Message bus configuration options | ||
| * @returns A new MessageBus instance | ||
| * @since 8.38.0 | ||
| */ | ||
| createMessageBus( | ||
| config?: import("./types/index.js").MessageBusConfig, | ||
| ): import("./agent/communication/index.js").MessageBus { | ||
| const { MessageBus } = require("./agent/communication/index.js"); | ||
| logger.debug("[NeuroLink] Creating message bus", { | ||
| maxHistorySize: config?.maxHistorySize, | ||
| }); | ||
| return new MessageBus(config); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json module type =="
jq -r '.type // "missing"' package.json 2>/dev/null || true
echo
echo "== tsconfig module settings =="
fd -i 'tsconfig*.json' -x sh -c 'echo "--- $1"; jq -r ".compilerOptions | {module, moduleResolution}" "$1" 2>/dev/null || sed -n "1,120p" "$1"' sh {}
echo
echo "== factory methods using require() =="
sed -n '12290,12390p' src/lib/neurolink.ts
echo
echo "== createRequire / require usage in src/lib/neurolink.ts =="
rg -n 'createRequire|from "module"|require\(' src/lib/neurolink.ts
echo
echo "== runtime imports from agent modules back into neurolink =="
rg -nP --type=ts -C2 '^(?!\s*import\s+type\b)\s*import\s+.*from\s+"(\.\./)*neurolink(\.js)?"' src/lib/agentRepository: juspay/neurolink
Length of output: 8878
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== First 30 lines of src/lib/neurolink.ts =="
head -n 30 src/lib/neurolink.ts
echo
echo "== Check for top-level await =="
rg -n 'top-?level.*await|^await ' src/lib/neurolink.ts | head -20
echo
echo "== Check line 13 specifically =="
sed -n '10,20p' src/lib/neurolink.ts
echo
echo "== Check if neurolink.ts is used in agent modules =="
rg -r '(import|from).*neurolink' src/lib/agent --type=ts | grep -v 'test\|spec' || echo "No matches found"
echo
echo "== Check for createRequire pattern =="
rg 'createRequire|require\s*=' src/lib/neurolink.ts || echo "No createRequire found"
echo
echo "== Check typescript target and module in main tsconfig =="
cat tsconfig.json | grep -A 30 'compilerOptions'Repository: juspay/neurolink
Length of output: 1326
Replace require() calls with static ESM imports in factory methods.
The file uses ESM (package.json type is "module" and line 13 has top-level await import("dotenv")), but the 6 factory methods at lines 12296, 12314, 12331, 12349, 12364, and 12382 use require(), which will fail at runtime with require is not defined / ERR_REQUIRE_ESM when called.
No circular dependency risk was found (agent modules do not import from src/lib/neurolink.ts), so replacing the require() calls with static ESM imports is safe:
Example fix pattern
+ import { NetworkOrchestrator } from "./agent/orchestration/index.js";
+ import { SupervisorAgent } from "./agent/supervisor/index.js";
+ import { AgentEvaluator } from "./agent/evaluation/index.js";
+ import { ResultOptimizer } from "./agent/evaluation/index.js";
+ import { AgentCoordinator } from "./agent/coordination/index.js";
+ import { MessageBus } from "./agent/communication/index.js";
createOrchestrator(
config?: import("./types/index.js").OrchestratorConfig,
): import("./agent/orchestration/index.js").NetworkOrchestrator {
- const { NetworkOrchestrator } = require("./agent/orchestration/index.js");
logger.debug("[NeuroLink] Creating network orchestrator", {
maxConcurrentExecutions: config?.maxConcurrentExecutions,
defaultMode: config?.defaultMode,
});
return new NetworkOrchestrator(this, config);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/neurolink.ts` around lines 12293 - 12386, The factory methods
createOrchestrator, createSupervisor, createEvaluator, createOptimizer,
createCoordinator, and createMessageBus currently use require() which will fail
in ESM; replace those dynamic requires with static ESM imports at the top of the
module (import { NetworkOrchestrator } from "./agent/orchestration/index.js",
import { SupervisorAgent } from "./agent/supervisor/index.js", import {
AgentEvaluator, ResultOptimizer } from "./agent/evaluation/index.js", import {
AgentCoordinator } from "./agent/coordination/index.js", and import { MessageBus
} from "./agent/communication/index.js") and then instantiate using the imported
classes in each factory function (e.g., return new NetworkOrchestrator(this,
config), new SupervisorAgent(this, config), new AgentEvaluator(this, config),
new ResultOptimizer(this, evaluator), new AgentCoordinator(this, config), new
MessageBus(config)).
murdore
force-pushed
the
feat/multi-agent-networks
branch
from
4190e64 to
438f5f0
Compare
Proof of Work — Live Test Results (Post-Rebase)Rebased onto Build & Validation
Live Tests —
|
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
murdore
force-pushed
the
feat/multi-agent-networks
branch
from
438f5f0 to
a505e75
Compare
v2 — Architecture Reset + PR Review FixesRebased onto What Changed (v1 → v2)Architecture reset — The entire
Deleted modules (duplicated existing systems):
Fixed all 46 PR review issues:
Leveraged existing systems:
Build & Validation
Live Tests — 16/16 PASSVertex AI — 8/8 PASSGoogle AI Studio — 8/8 PASSPerformance Improvement (agents-as-tools vs dual-LLM routing)
Commit Graph1 commit ahead, 0 behind. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
murdore
force-pushed
the
feat/multi-agent-networks
branch
from
a505e75 to
e2dfab7
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
|
@coderabbitai review Please do a full review of the current diff. The PR was significantly rewritten since your last review on April 14:
Your previous review found 3 critical, 36 major issues. Please re-evaluate the current code. |
|
User feedback: PR was significantly rewritten since the April 14 review. Key changes: deleted 9,300 lines (RouterAgent, SupervisorAgent, AgentEvaluator, ProtocolManager, vitest tests), rewrote AgentNetwork to use agents-as-tools via ai SDK generate loop, fixed all 46 issues from the previous review, and rebased onto release 9.55.1. Previous review found 3 critical and 36 major issues — please re-evaluate the current code fresh. 🧠 Learnings used✅ Actions performedFull review triggered. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
murdore
force-pushed
the
feat/multi-agent-networks
branch
from
e2dfab7 to
a5fc552
Compare
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
|
@coderabbitai summary |
✅ Actions performedSummary regeneration triggered. |
murdore
force-pushed
the
feat/multi-agent-networks
branch
from
a5fc552 to
b6ad1c3
Compare
|
@coderabbitai full review This is a fresh force-push (commit b6ad1c3) rebased onto release 9.56.0. The previous review from April 14 is entirely stale — the code has been architecturally rewritten. Please do a complete review of the full diff. |
|
User feedback: Fresh force-push (commit ✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
♻️ Duplicate comments (1)
src/cli/commands/agent.ts (1)
36-38:⚠️ Potential issue | 🔴 CriticalPersist CLI registrations across invocations.
These registries are process-local, so a normal
neurolink agent create ...followed byneurolink agent execute ...in a second CLI process cannot find the created agent/network. Back these with a config/store, or make execution accept a definition/config file directly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/agent.ts` around lines 36 - 38, The registries registeredAgents and registeredNetworks are in-memory only so CLI commands run in separate processes lose created agents/networks; persist registrations to a shared store (e.g., a config file or simple JSON DB) or allow commands to accept definition/config files directly. Modify the code paths that write to registeredAgents/registeredNetworks (e.g., the agent create handler) to serialize and save the entry to a persistent store and update the read path used by the agent execute handler to load from that store instead of the in-process Map; alternatively add CLI flags to agent execute (or a helper like loadAgentDefinition/saveAgentDefinition) to accept a file path and load the AgentDefinition/AgentNetworkConfig from disk so cross-process invocations can find the definitions.
🟠 Major comments (21)
docs/agents/CONFIGURATION.md-192-207 (1)
192-207:⚠️ Potential issue | 🟠 MajorUpdate documentation: NetworkDefaults type definition is incorrect.
The
NetworkDefaultstype documented at lines 192–207 does not match the actual type insrc/lib/types/agentNetwork.ts. The documentation showsmaxConcurrentTasks,taskTimeout, andretryPolicyas required fields, but the actual type definesmaxSteps,timeout, andtemperature—all optional. Replace the documented type snippet with the correct definition from the source file.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/CONFIGURATION.md` around lines 192 - 207, The documented NetworkDefaults type is out of sync: update the snippet for NetworkDefaults to match the source definition by replacing the required fields (maxConcurrentTasks, taskTimeout, retryPolicy) with the actual optional properties used in the code—maxSteps, timeout, and temperature—marking them optional as in the source; specifically, locate the NetworkDefaults snippet in the docs and replace it with the exact type signature from the source for NetworkDefaults (preserving JSDoc comments if present) so the docs reflect the real type shape.test/continuous-test-suite-agents.ts-1416-1484 (1)
1416-1484:⚠️ Potential issue | 🟠 MajorUpdate CLI coverage to test the new agent/network commands instead of skipping them.
The PR objective includes
neurolink agentandneurolink networksubcommands, but this suite documents them as missing and records all checks asSKIP. That can mask a missing or broken CLI surface while CI still reports success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-agents.ts` around lines 1416 - 1484, The testCLICoverage suite currently marks neurolink agent/network commands as skipped; replace those skipTest calls with real runTest checks that invoke the built CLI (use the cliPath already computed in the "CLI entry point exists (dist/cli/index.js)" test) and assert successful execution and expected output for the new subcommands. For each formerly skipped case (the skipTest calls for "CLI: neurolink agent create", "CLI: neurolink agent execute", "CLI: neurolink network create", "CLI: neurolink network execute") implement runTest that spawns/executes `node cliPath` with the appropriate args (e.g., `agent create <name>`, `agent execute <id> <input>`, `network create <name>`, `network execute <id>`), await completion, check exit code === 0 and that stdout or stderr contains a short expected string (e.g., success or usage text), and surface any error as a test failure; reuse existing helper runTest function and the cliPath resolution in testCLICoverage to locate the binary.test/continuous-test-agents-live.ts-57-76 (1)
57-76:⚠️ Potential issue | 🟠 MajorApply
TEST_CONFIG.timeoutto live provider-backed calls.The live SDK/Agent/Network calls can hang indefinitely even though the suite defines a timeout. Wrap each provider-backed await/stream setup with the project
withTimeoututility so CI fails deterministically on stalled providers.As per coding guidelines, Error Handling — Use
ErrorFactoryfor typed errors, wrap async calls withwithTimeoututility,formatProviderErrormust return errors never throw.Also applies to: 150-159, 166-177, 200-220, 232-253
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-agents-live.ts` around lines 57 - 76, Wrap every live provider-backed async call in this file (e.g., calls inside testSdkGenerate, testSdkStream and the other ranges noted) with the project withTimeout utility using TEST_CONFIG.timeout so stalled providers fail deterministically; specifically, replace direct awaits/stream setups (calls to sdk.generate, sdk.stream, agent.run, network.send, etc.) with withTimeout(<originalPromise>, TEST_CONFIG.timeout) and ensure any error handling uses ErrorFactory to create typed errors and that formatProviderError is used to transform provider errors into non-throwing Error objects before passing them to ErrorFactory or logging. Make sure to update all occurrences referenced (testSdkGenerate, testSdkStream and the other listed blocks) and keep existing behavior/return types intact.test/continuous-test-suite-agents.ts-1388-1400 (1)
1388-1400:⚠️ Potential issue | 🟠 MajorAssert that MessageBus actually delivers the published message.
received.length >= 0is always true, so this test passes even ifpublish()never invokes subscribers. Assert one delivered message and validate its payload.Strengthen the pub/sub assertion
bus.publish("test-topic", { data: "hello" }); // Allow microtask queue to flush await new Promise<void>((resolve) => setTimeout(resolve, 0)); - assert( - received.length >= 0, - "MessageBus should handle pub/sub without throwing", - ); + assertEqual(received.length, 1, "MessageBus should deliver one message"); + assertEqual( + (received[0] as { data: string }).data, + "hello", + "MessageBus should deliver the published payload", + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-agents.ts` around lines 1388 - 1400, The test currently uses a tautological assertion (received.length >= 0) so it never verifies delivery; update the test that constructs MessageBusCtor and uses subscribe/publish to assert that received.length >= 1 (or === 1) after the microtask flush and also validate the delivered message payload (e.g., deep-equal the first element of the received array to the published object { data: "hello" }) to ensure subscribe and publish actually deliver the message; locate the logic in the test block that creates bus via MessageBusCtor, calls bus.subscribe and bus.publish and replaces the weak assertion on received with the stronger length and payload checks.test/continuous-test-suite-agents.ts-1153-1190 (1)
1153-1190:⚠️ Potential issue | 🟠 MajorDo not convert implementation import failures into skipped tests.
These imports target required PR functionality. Catching every import error as
SKIPwill hide syntax errors, missing transitive imports, or module initialization failures and still allow the suite to pass. Only skip truly optional modules, or fail the test on unexpected import errors.Fail unexpected import errors
- try { - const mod = await import("../src/lib/agent/agent.js"); - AgentCtor = mod.Agent as unknown as AgentClass; - logDebug("Imported Agent class"); - } catch { - results.push( - skipTest( - "Import Agent class", - "Agent module not available - run `pnpm build` first", - ), - ); - } + results.push( + await runTest("Import Agent class", async () => { + const mod = await import("../src/lib/agent/agent.js"); + AgentCtor = mod.Agent as unknown as AgentClass; + assertDefined(AgentCtor, "Agent export should exist"); + logDebug("Imported Agent class"); + }), + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-agents.ts` around lines 1153 - 1190, The import blocks for Agent, AgentNetwork and MessageBus currently catch all errors and convert them into skipped tests (via results.push(skipTest(...))), which masks real import failures; update the try/catch logic in the modules that set AgentCtor, AgentNetworkCtor and MessageBusCtor so that unexpected import errors are rethrown (or cause a failing test result) instead of calling skipTest — only wrap in a skip branch for a documented optional module by checking a clear optional flag or matching a known benign error; replace the broad catch blocks around the dynamic imports of "../src/lib/agent/agent.js", "../src/lib/agent/agentNetwork.js", and "../src/lib/agent/communication/message-bus.js" to either let the exception bubble or convert it to a failure result, leaving skipTest only for truly optional imports.test/continuous-test-agents-live.ts-10-10 (1)
10-10:⚠️ Potential issue | 🟠 MajorReplace
execSyncwithexecFileSyncand passTEST_CONFIG.modelto CLI tests.The CLI tests currently use shell interpolation which ignores
TEST_CONFIG.model, while SDK tests use it. This causes SDK and CLI code paths to test different model configurations. UseexecFileSyncwith an argument array and include--modelwhen configured.Safer CLI invocation
-import { execSync } from "child_process"; +import { execFileSync } from "child_process"; @@ +function runCli(args: string[]): string { + return execFileSync("node", ["dist/cli/index.js", ...args], { + encoding: "utf-8", + timeout: TEST_CONFIG.timeout, + stdio: ["ignore", "pipe", "pipe"], + }); +} + async function testCliGen(): Promise<boolean | null> { try { - const out = execSync( - `node dist/cli/index.js gen "Say PONG" --provider ${TEST_CONFIG.provider} --max-tokens ${TEST_CONFIG.maxTokens}`, - { - encoding: "utf-8", - timeout: TEST_CONFIG.timeout, - stdio: ["ignore", "pipe", "pipe"], - }, - ); + const out = runCli([ + "gen", + "Say PONG", + "--provider", + TEST_CONFIG.provider, + "--max-tokens", + String(TEST_CONFIG.maxTokens), + ...(TEST_CONFIG.model ? ["--model", TEST_CONFIG.model] : []), + ]); @@ async function testCliStream(): Promise<boolean | null> { try { - const out = execSync( - `node dist/cli/index.js stream "Count to 3" --provider ${TEST_CONFIG.provider} --max-tokens ${TEST_CONFIG.maxTokens}`, - { - encoding: "utf-8", - timeout: TEST_CONFIG.timeout, - stdio: ["ignore", "pipe", "pipe"], - }, - ); + const out = runCli([ + "stream", + "Count to 3", + "--provider", + TEST_CONFIG.provider, + "--max-tokens", + String(TEST_CONFIG.maxTokens), + ...(TEST_CONFIG.model ? ["--model", TEST_CONFIG.model] : []), + ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-agents-live.ts` at line 10, Replace the unsafe shell invocation using execSync with execFileSync and pass TEST_CONFIG.model into the CLI test argument list: locate the test that calls execSync in test/continuous-test-agents-live.ts, change the import to use execFileSync, invoke the CLI binary with an argv array instead of a shell string, and include ['--model', TEST_CONFIG.model] (only when TEST_CONFIG.model is set) so the CLI tests run with the same model configuration as the SDK tests.src/lib/agent/coordination/coordinator.ts-581-604 (1)
581-604:⚠️ Potential issue | 🟠 MajorDo not mark failed dependency producers as completed.
executeWithDependencies()adds the agent tocompletedbefore checkingresult.error, and the catch path does the same. Any dependent assignment will run even though its prerequisite failed. Track successful completions separately from failed agents and fail/requeue dependents accordingly.Also applies to: 607-612
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/coordinator.ts` around lines 581 - 604, In executeWithDependencies(), stop adding failing agents to the completed set: only add assignment.agent.id to the success tracker (rename or add e.g., successful or completedSuccess) and to context.previousResults when result.error is falsy; do not move the id out of pending or into completed on error (and same change in the catch path around the block that currently pushes to completed and pending.delete); update downstream dependency checks to consult the successful set (not completed) so dependents are failed or requeued when prerequisites errored; apply the same fix for the similar logic around the 607–612 area where failed agents are currently marked completed.src/lib/agent/coordination/coordinator.ts-178-184 (1)
178-184:⚠️ Potential issue | 🟠 MajorReturn an error when no agents are registered.
sequentialandparallelreturnsuccess: truewith empty output when the coordinator has no agents, unlikeroundRobinandleastBusy. This can make an orchestration appear successful without doing any work.Suggested fix
const agents = this.getAgents(); + if (agents.length === 0) { + return { + success: false, + agentResults: context.previousResults, + steps: [], + finalOutput: "", + errors: [{ agentId: "coordinator", error: "No agents registered" }], + duration: Date.now() - context.metadata.startTime, + metadata: { + executionId: context.metadata.executionId, + strategy: "sequential", + agentsExecuted: 0, + agentsFailed: 0, + }, + }; + }Also applies to: 257-263
src/lib/agent/coordination/task-distributor.ts-136-140 (1)
136-140:⚠️ Potential issue | 🟠 MajorDo not return pending results for concurrent submissions.
When
processQueue()is already running, anothersubmitTask()awaits a no-op and returns the currentpendingresult before the task executes. Queue processing should settle each submitted task’s promise only after completion/failure.Also applies to: 193-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/task-distributor.ts` around lines 136 - 140, submitTask currently returns the "pending" entry from activeResults immediately when processQueue() is already running, so concurrent submissions can get unresolved results; change submitTask and the related logic so each submission creates and stores a per-task Promise (or a pending result object with an associated resolver) in activeResults keyed by task.id, have processQueue resolve/reject that Promise when the task finishes/fails, and make submitTask await that task-specific Promise before returning; apply the same change to the other submission path referenced around the second occurrence (lines ~193-199) so no caller receives a naked pending value before the task has completed.src/cli/commands/agent.ts-530-550 (1)
530-550:⚠️ Potential issue | 🟠 MajorFail the CLI when streaming emits an error chunk.
Agent.stream()/AgentNetwork.stream()yieldagent-error/network-errorinstead of throwing, but these handlers only print the error and continue, leavingprocess.exitCodeas success. Track stream errors and setprocess.exitCode = 1.Suggested fix pattern
+ let streamFailed = false; for await (const chunk of agent.stream(input, { context, maxSteps: argv.maxSteps, })) { ... } else if (chunk.type === "agent-error") { + streamFailed = true; logger.always(chalk.red(`\nError: ${chunk.error}`)); } } + if (streamFailed) { + process.exitCode = 1; + }+ let streamFailed = false; for await (const chunk of neurolink.streamNetwork( ... case "network-error": { const errorChunk = chunk as NetworkErrorChunk; + streamFailed = true; logger.always(chalk.red(`\nNetwork error: ${errorChunk.error}`)); break; } } } + if (streamFailed) { + process.exitCode = 1; + }Also applies to: 787-880
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/agent.ts` around lines 530 - 550, In the stream consumption loop for Agent.stream (the for-await-of over agent.stream(input, { context, maxSteps })) detect error chunks (chunk.type === "agent-error" and also handle "network-error" where present), set process.exitCode = 1 when such a chunk is received, and ensure the loop stops (break/return) after handling the error so the CLI exits with failure; update the handlers in this block (and the analogous block around the other stream code at the later range) to both log the error (logger.always/chalk) and set process.exitCode = 1 before breaking out.src/lib/agent/coordination/task-distributor.ts-479-488 (1)
479-488:⚠️ Potential issue | 🟠 MajorUse the effective deadline for the outer timeout.
task.deadline - Date.now()is passed intoagent.execute(), but the actualwithTimeout()still usesthis.config.taskTimeout, so an imminent deadline can be exceeded. Compute one effective timeout and use it for both calls.Suggested fix
+ const timeoutMs = task.deadline + ? Math.max(0, task.deadline - Date.now()) + : (this.config.taskTimeout ?? 60000); const agentResult = await withTimeout( agent.execute(task.input, { context: task.metadata, - timeout: task.deadline - ? task.deadline - Date.now() - : this.config.taskTimeout, + timeout: timeoutMs, }), - this.config.taskTimeout ?? 60000, + timeoutMs, "Task execution timeout", );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/task-distributor.ts` around lines 479 - 488, Compute a single effective timeout value and use it for both the call to agent.execute(...) and the outer withTimeout(...) so an imminent task.deadline can't be bypassed; for example, compute const effectiveTimeout = task.deadline ? Math.max(0, task.deadline - Date.now()) : this.config.taskTimeout ?? 60000, then pass effectiveTimeout into agent.execute(..., { timeout: effectiveTimeout, context: task.metadata }) and as the timeout argument to withTimeout(..., effectiveTimeout, "Task execution timeout") so both use the same deadline-aware duration.src/lib/agent/agentNetwork.ts-157-199 (1)
157-199:⚠️ Potential issue | 🟠 MajorExpose configured network tools to the router.
initializeTools()createsToolPrimitives forconfig.tools, butexecute()andstream()only passbuildAgentTools()into NeuroLink. As a result, network-level tools are discovered but never callable. Merge tool primitives into thetoolsobject passed togenerate()/stream().Also applies to: 352-362, 454-464
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agentNetwork.ts` around lines 157 - 199, initializeTools() currently registers ToolPrimitive entries in this.primitives but those primitives are never included in the tools object passed into neurolink.generate(...) and neurolink.stream(...), so network-level tools are not callable; update the code paths that call buildAgentTools() (used in generate and stream) to merge primitives whose type === "tool" (created by initializeTools()) into the resulting tools map/object before passing it into neurolink.generate / neurolink.stream, ensuring each ToolPrimitive (id like `tool-${toolName}`) contributes its tool metadata and execute/stream handlers to the tools argument; apply the same merge logic in the other two similar spots referenced (around lines 352-362 and 454-464) so all calls supply the combined set of agent-local and network-level tools.src/lib/agent/agent.ts-153-161 (1)
153-161:⚠️ Potential issue | 🟠 MajorValidate all inputs against
inputSchema, including strings.A string input bypasses validation entirely, so schemas like
z.string().min(1)or constrained string formats are never enforced. RunsafeParse(input)wheneverinputSchemais present.Suggested fix
- if (this.inputSchema && typeof input !== "string") { + if (this.inputSchema) { const validation = this.inputSchema.safeParse(input); if (!validation.success) { throw new Error( `Input validation failed: ${validation.error.message}`,Also applies to: 287-295
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agent.ts` around lines 153 - 161, The code currently skips validation when input is a string; remove the typeof input !== "string" guard so that whenever this.inputSchema exists you always call this.inputSchema.safeParse(input) and throw on validation.failure (same behavior as current error path). Update both occurrences that check inputSchema (the block using this.inputSchema.safeParse(input) around the shown diff and the duplicate at the later occurrence referenced) to always run safeParse against the input (including strings) and keep the existing error message behavior.src/lib/agent/coordination/coordinator.ts-493-516 (1)
493-516:⚠️ Potential issue | 🟠 MajorHonor per-call
agentTimeoutoverrides.
coordinate()mergesoptionsinto a localconfig, butexecuteAgentWithTimeout()readsthis.config.agentTimeout, socoordinate(task, { agentTimeout })is ignored. Pass the resolved timeout into this helper.Suggested fix
private async executeAgentWithTimeout( agent: AgentInstance, input: string, context: CoordinationContext, + timeoutMs?: number, ): Promise<AgentResult> { - const timeout = this.config.agentTimeout ?? 60000; + const timeout = timeoutMs ?? this.config.agentTimeout ?? 60000;Then pass
config.agentTimeoutfrom strategy call sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/coordination/coordinator.ts` around lines 493 - 516, The helper executeAgentWithTimeout currently reads this.config.agentTimeout so per-call overrides passed into coordinate() are ignored; change executeAgentWithTimeout to accept a timeout parameter (e.g., agentTimeout) and use that instead of this.config.agentTimeout, then update all callers (including coordinate and any strategy call sites that invoke executeAgentWithTimeout) to pass the resolved config.agentTimeout or fallback value so per-call overrides are honored; refer to executeAgentWithTimeout, coordinate, config.agentTimeout/agentTimeout, and this.activeExecutions when making the change.src/lib/types/agentNetwork.ts-683-821 (1)
683-821:⚠️ Potential issue | 🟠 MajorKeep
NetworkStreamChunkTypeandNetworkStreamChunkin sync.The type alias includes
routing-start,routing-end,primitive-progress,agent-thinking,workflow-step, andnetwork-progress, but the union has no corresponding chunk shapes. Consumers cannot exhaustively handle those advertised events. Add chunk types for them or remove the unused literals.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/agentNetwork.ts` around lines 683 - 821, NetworkStreamChunkType lists literals (routing-start, routing-end, primitive-progress, agent-thinking, workflow-step, network-progress) that have no corresponding chunk shapes in the NetworkStreamChunk union, so update the types so the type alias and union stay in sync: either add explicit chunk types (e.g., RoutingStartChunk, RoutingEndChunk, PrimitiveProgressChunk, AgentThinkingChunk, WorkflowStepChunk, NetworkProgressChunk) defined as NetworkStreamChunkBase & { type: "<literal>"; ...relevant fields like stepIndex, primitive{id,name,type} or progress payload } and include each in the NetworkStreamChunk union, or remove the unused literals from NetworkStreamChunkType if those events aren’t emitted; make changes around the NetworkStreamChunkType, the new chunk type names, and the NetworkStreamChunk union to keep identifiers consistent.src/lib/agent/agentNetwork.ts-341-362 (1)
341-362:⚠️ Potential issue | 🟠 MajorPropagate network context into execution.
NetworkExecutionInput.contextandNetworkExecutionOptions.contextare ignored, so CLI--contextand SDK context do not influence routing or delegated agent calls. Include the merged context in the router prompt/options and pass it when invoking agent tools.Suggested direction
- const result = await this.neurolink.generate({ + const executionContext = { + ...input.context, + ...options?.context, + threadId: input.threadId, + resourceId: input.resourceId, + traceId, + }; + + const result = await this.neurolink.generate({ input: { text: message }, systemPrompt, provider: this.config.router?.provider as string | undefined, model: this.config.router?.model, maxSteps, tools: agentTools, + context: executionContext, } as Parameters<NeuroLink["generate"]>[0]);Also applies to: 436-464
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/agentNetwork.ts` around lines 341 - 362, The router currently ignores NetworkExecutionInput.context and NetworkExecutionOptions.context so CLI/SDK context never reaches the router or delegated agents; merge the contexts (input.context with options.context and any defaults) early after extractMessageContent, inject the merged context into buildRouterSystemPrompt and into the options passed to neurolink.generate (the call in generate(...) using systemPrompt, provider, model, maxSteps, tools), and ensure buildAgentTools or the agent tool invocation includes that merged context so delegated agent calls receive it; update references around extractMessageContent, buildAgentTools, buildRouterSystemPrompt, neurolink.generate, and any agentTools invocation to pass the merged context through.src/lib/agent/agent.ts-173-175 (1)
173-175:⚠️ Potential issue | 🟠 MajorEnforce
AgentExecutionOptions.timeout.The public option is accepted but never applied, and the provider calls are awaited directly. Wrap
neurolink.generate()/neurolink.stream()withwithTimeoutwhenoptions.timeoutis set. As per coding guidelines,Error Handling — Use ErrorFactory for typed errors, wrap async calls with withTimeout utility, formatProviderError must return errors never throw.Suggested fix
+import { withTimeout } from "../utils/async/withTimeout.js"; ... - const result = await this.neurolink.generate(generateOptions); + const generation = this.neurolink.generate(generateOptions); + const result = options?.timeout + ? await withTimeout( + generation, + options.timeout, + `Agent execution timeout after ${options.timeout}ms`, + ) + : await generation; ... - const streamResult = await this.neurolink.stream(streamOptions); + const stream = this.neurolink.stream(streamOptions); + const streamResult = options?.timeout + ? await withTimeout( + stream, + options.timeout, + `Agent stream timeout after ${options.timeout}ms`, + ) + : await stream;Also applies to: 300-302, 455-505
src/cli/commands/agent.ts-172-219 (1)
172-219:⚠️ Potential issue | 🟠 MajorRemove unconditional
demandOptionconstraints and use.check()for conditional validation.The agent command requires
--id,--name,--description, and--instructionseven when--fileis provided, which prevents the documented file-based flow (agent create --file agent-config.json) from working. Similarly, the network command requires--nameeven though it only strictly needs--file(the handler allows--nameto be overridden from the file content). Yargs will reject the command at the argument-parsing level before the handler logic can run.Move these to optional options and add a
.check()validator:
- For agent:
--id,--name,--description,--instructionsrequired only if--fileis absent- For network:
--filealways required;--nameoptional (can come from file)Suggested shape
.option("id", { type: "string", description: "Unique agent identifier", - demandOption: true, }).option("name", { type: "string", description: "Human-readable agent name", - demandOption: true, }).option("description", { type: "string", description: "Description of agent capabilities (for routing)", - demandOption: true, }).option("instructions", { type: "string", description: "System instructions for the agent", - demandOption: true, }).option("file", { type: "string", description: "Load agent definition from JSON file", }) + .check((argv) => { + if (argv.file) return true; + const missing = ["id", "name", "description", "instructions"].filter( + (key) => !argv[key], + ); + if (missing.length > 0) { + throw new Error(`Missing required option(s): ${missing.join(", ")}`); + } + return true; + })For network command (lines 280–295), remove
demandOption: truefromname:.option("name", { type: "string", description: "Network name", - demandOption: true, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/agent.ts` around lines 172 - 219, The current buildCreateOptions(yargs: Argv) marks --id, --name, --description and --instructions as demandOption=true which blocks the file-based flow; make those options optional (remove demandOption) and add a .check() on the returned yargs that enforces: if --file is absent then require --id, --name, --description and --instructions; similarly for the network command remove demandOption from --name (so file can supply it) and add a .check() on that command (or its builder, e.g., buildNetworkOptions) that validates --file is present and only requires --name when --file is absent or when you want to allow override. Use the function names buildCreateOptions and the network builder (e.g., buildNetworkOptions) to locate where to change options and add the conditional .check() validators.src/lib/agent/communication/message-bus.ts-214-214 (1)
214-214: 🛠️ Refactor suggestion | 🟠 MajorUse
ErrorFactoryfor typed errors.Raw
new Error(...)is used for the request timeout (line 214), reply validation (line 254), and shutdown rejection (line 528). Per coding guidelines, errors in this codebase should be constructed viaErrorFactoryso callers get typed errors and consistent categorization (e.g. timeout vs. validation vs. lifecycle).As per coding guidelines: "Use
ErrorFactoryfor typed errors".Also applies to: 254-254, 528-528
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/communication/message-bus.ts` at line 214, Replace raw new Error(...) uses with the codebase ErrorFactory so callers receive typed, categorized errors: import ErrorFactory (and any ErrorType/Category enum) into src/lib/agent/communication/message-bus.ts and change the timeout rejection (the reject(...) that currently does new Error(`Request timed out after ${timeoutMs}ms`)) to create a typed timeout error via ErrorFactory (use the timeout/TimeoutError category), change the reply validation rejection (the validation failure at the reply handling) to use ErrorFactory with a validation/ValidationError category, and change the shutdown rejection (the rejection at shutdown) to use ErrorFactory with a lifecycle/shutdown error category; ensure each replacement preserves the original message text and uses the appropriate ErrorFactory method/enum so callers can distinguish timeout vs validation vs lifecycle errors.src/lib/agent/communication/message-bus.ts-298-352 (1)
298-352:⚠️ Potential issue | 🟠 MajorDead-letter queue is unbounded and duplicates messages per failed handler.
Two issues in
deliverMessage:
deadLetterQueuehas no max size — under sustained handler failures it grows unbounded (unlikemessageHistory, which is capped bymaxHistorySize). This is a memory leak in a long-running orchestrator.- The same
messageis pushed to the DLQ once per failing handler. If a topic has N subscribers and K of them throw, the DLQ ends up with K copies of an identical message, which also distortsgetStats().deadLetterQueueSizeand any downstream retry/inspection logic.Consider bounding the DLQ (e.g. a
maxDeadLetterQueueSizeinMessageBusConfig, dropping oldest on overflow) and deduplicating per-message (push at most once, optionally annotated with the failing subscription IDs).🛠️ Sketch
- for (const sub of subs) { + let dlqPushed = false; + for (const sub of subs) { ... deliveryPromises.push( Promise.resolve(sub.handler(message)).catch((error) => { logger.error(`[MessageBus] Message delivery failed`, { ... }); - if (this.config.enableDeadLetterQueue) { - this.deadLetterQueue.push(message); - } + if (this.config.enableDeadLetterQueue && !dlqPushed) { + dlqPushed = true; + this.deadLetterQueue.push(message); + if (this.deadLetterQueue.length > this.config.maxDeadLetterQueueSize) { + this.deadLetterQueue.shift(); + } + } }), ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/communication/message-bus.ts` around lines 298 - 352, The deliverMessage method currently pushes the same AgentMessage into this.deadLetterQueue for every failing subscription and the queue is unbounded; update the MessageBusConfig to include maxDeadLetterQueueSize, enforce that bound when adding to this.deadLetterQueue (drop oldest entries when full), and change deliverMessage so it only enqueues a given message once per delivery attempt: collect failing subscription IDs while iterating subscribers in deliverMessage (e.g., failingSubs array), and after all handlers are attempted push a single entry to this.deadLetterQueue containing the message plus metadata of failing subscription IDs (ensuring you check and trim the queue to maxDeadLetterQueueSize before push); also update any getStats() deadLetterQueueSize logic to report the bounded length.src/lib/agent/communication/message-bus.ts-199-243 (1)
199-243:⚠️ Potential issue | 🟠 Major
request()leaks the reply subscription and pending entry ifpublish()rejects.The reply subscription and
pendingRequestsentry (plus itssetTimeout) are created beforeawait this.publish(...). Ifpublishthrows, the caller receives the error, but:
- the subscription on
reply:<correlationId>remains until the request timeout fires,- the
pendingRequestsentry and its timer stay alive forrequestTimeoutms,- the original
responsePromiseis orphaned and will eventually reject as "timed out" with no listener (potential unhandled rejection depending on Node config).Wrap the publish in try/catch and clean up (
clearTimeout,pendingRequests.delete,unsubscribeByTopic) before rethrowing.- // Send the request - await this.publish(topic, senderId, payload, { - type: "request", - correlationId, - replyTo, - }); - - return responsePromise; + try { + await this.publish(topic, senderId, payload, { + type: "request", + correlationId, + replyTo, + }); + } catch (err) { + const pending = this.pendingRequests.get(correlationId); + if (pending) { + clearTimeout(pending.timeout); + this.pendingRequests.delete(correlationId); + } + this.unsubscribeByTopic(replyTo, senderId); + throw err; + } + + return responsePromise;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/communication/message-bus.ts` around lines 199 - 243, The request() flow can leak the reply subscription and pendingRequests entry if publish() rejects; after creating correlationId/replyTo, the code must wrap the await this.publish(...) call in try/catch and on any error perform cleanup before rethrowing: look up the pending entry in this.pendingRequests for correlationId (the object containing timeout), clearTimeout(pending.timeout), this.pendingRequests.delete(correlationId), and call this.unsubscribeByTopic(replyTo, senderId) to remove the subscription created via subscribe(...), then rethrow the publish error so the caller still sees the original failure; ensure you reference the existing symbols request, publish, subscribe, pendingRequests, unsubscribeByTopic, correlationId, replyTo and the timeout stored on the pending entry (config.requestTimeout remains as the fallback).
🧹 Nitpick comments (4)
docs/agents/VERIFICATION.md (1)
155-165: Minor: replace?placeholders with an empty checkbox orTBD.The bare
?in the Status column renders as literal question marks. Consider[ ]orTBDso the checklist reads as actionable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/agents/VERIFICATION.md` around lines 155 - 165, Replace the literal '?' placeholders in the Status column of the verification table with actionable placeholders (e.g., unchecked markdown checkbox "[ ]" or "TBD") so the checklist is readable and actionable; update each Status cell for the rows containing Agent Class, AgentNetwork, MessageBus, HubSpokeTopology, MeshTopology, HierarchicalTopology, CLI Commands, and Integration to use the chosen replacement consistently (I recommend "[ ]" for a checkbox).test/continuous-test-suite-agents.ts (1)
32-101: Move reusable test helper types out of the runner.The fixture/result and constructor-shape types are declared inline in the executable test. Put these test-only types under
test/types/*.tsand import them here to match the project type-location convention.Based on learnings, Project standard: Place reusable/shared types under
src/lib/types/*.ts; test-only helper types undertest/types/*.ts; avoid declaring local types inside source implementation files.Also applies to: 1100-1147
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-agents.ts` around lines 32 - 101, Extract the local test-only type declarations (ColorName, TestStatus, TestResult, TestSuiteResult, AgentDefinitionFixture, NetworkTopologyFixture, RoutingRuleFixture, MessageFixture) into a new test/types/*.ts file and replace the inline definitions in this test with imports; create named exports for each type in the new file and update this file to import those types (e.g., import { TestResult, AgentDefinitionFixture, ... } from "test/types/your-file"), ensuring no runtime code changes—only move type declarations and adjust the top-of-file imports.test/continuous-test-agents-live.ts (1)
13-18: Move the test result shape into the shared test type location.
TestResultis a test helper type; keeping it local makes it harder to reuse across the two continuous agent runners. Move it totest/types/*.tsand import it here.Based on learnings, Project standard: Place reusable/shared types under
src/lib/types/*.ts; test-only helper types undertest/types/*.ts; avoid declaring local types inside source implementation files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-agents-live.ts` around lines 13 - 18, Extract the local TestResult type declaration and place it into the shared test types module under test/types (create a test-only types file and export TestResult), then replace the local declaration in continuous-test-agents-live.ts with an import of TestResult and update any references; ensure the exported shape matches the original (name: string; result: boolean | null; error: string | null; duration: number) and run tests to verify no type errors.src/lib/agent/communication/message-bus.ts (1)
389-394: Direct-message recipient check is effectively dead code — worth confirming intent.Direct messages are published to topic
direct:${recipientId}(line 179), and only the intended recipient would subscribe to that topic. Themessage.recipientId !== sub.subscriberIdguard here therefore only matters if a subscriber intentionally subscribes to someone else’sdirect:topic (e.g. debugging/tap). If that’s the intended semantics, a brief comment would help; otherwise this branch can go.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/agent/communication/message-bus.ts` around lines 389 - 394, The guard that returns false when message.type === "direct" and message.recipientId !== sub.subscriberId is effectively dead given direct messages are published to topic `direct:${recipientId}`; either remove this branch to avoid redundant checks or keep it but add a clear comment explaining it's an intentional safeguard for subscribers that may manually subscribe to another user's `direct:` topic (e.g., debugging/tap). Locate the check around the direct message handling (references: message.type, message.recipientId, sub.subscriberId, and the publishing of topic "direct:${recipientId}") and implement one of the two options: delete the if-block and its comment, or retain it and add a concise comment explaining the defensive intent and when it matters.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0a488aa8-f921-439a-a964-1f078d46801f
📒 Files selected for processing (30)
docs/agents/CLI-COVERAGE.mddocs/agents/CONFIGURATION.mddocs/agents/TESTING.mddocs/agents/VERIFICATION.mdsrc/cli/commands/agent.tssrc/cli/factories/commandFactory.tssrc/cli/parser.tssrc/lib/agent/agent.tssrc/lib/agent/agentNetwork.tssrc/lib/agent/communication/index.tssrc/lib/agent/communication/message-bus.tssrc/lib/agent/coordination/coordinator.tssrc/lib/agent/coordination/index.tssrc/lib/agent/coordination/task-distributor.tssrc/lib/agent/index.tssrc/lib/agent/orchestration/index.tssrc/lib/agent/orchestration/orchestrator.tssrc/lib/agent/orchestration/topology.tssrc/lib/agent/prompts/routingPrompts.tssrc/lib/index.tssrc/lib/neurolink.tssrc/lib/types/agentNetwork.tssrc/lib/types/cli.tssrc/lib/types/index.tstest/continuous-test-agents-live.tstest/continuous-test-suite-agents.tstest/fixtures/agents/agent-definitions.jsontest/fixtures/agents/messages.jsontest/fixtures/agents/network-topologies.jsontest/fixtures/agents/routing-rules.json
| private async processExecutionQueue(): Promise<void> { | ||
| while ( | ||
| this.executionQueue.length > 0 && | ||
| this.activeExecutions.size < this.config.maxConcurrentExecutions | ||
| ) { | ||
| const request = this.executionQueue.shift()! as ExecutionRequest & { | ||
| resolve?: (result: NetworkExecutionResult) => void; | ||
| reject?: (err: unknown) => void; | ||
| }; | ||
| const network = this.networks.get(request.networkId); | ||
| const info = this.networkInfo.get(request.networkId); | ||
|
|
||
| if (network && info && info.state !== "executing") { | ||
| // Execute without blocking the queue processing, wiring up the queued promise | ||
| this.executeNetworkInternal( | ||
| network, | ||
| info, | ||
| request.input, | ||
| request.options, | ||
| ) | ||
| .then((result) => request.resolve?.(result)) | ||
| .catch((err) => request.reject?.(err)); | ||
| } else if (request.reject) { | ||
| request.reject( | ||
| new Error(`Network unavailable for execution: ${request.networkId}`), | ||
| ); | ||
| } |
There was a problem hiding this comment.
Do not reject same-network queued executions during queue drain.
After starting one queued request for a network, the loop can continue while global capacity remains; the next queued request for the same network sees info.state === "executing" and is rejected. Leave it queued for the next drain instead.
Suggested direction
- const request = this.executionQueue.shift()! as ExecutionRequest & {
+ const request = this.executionQueue.shift()! as ExecutionRequest & {
resolve?: (result: NetworkExecutionResult) => void;
reject?: (err: unknown) => void;
};
...
- if (network && info && info.state !== "executing") {
+ if (network && info && info.state === "executing") {
+ this.executionQueue.push(request);
+ break;
+ }
+
+ if (network && info) {
// Execute without blocking the queue processing, wiring up the queued promise
this.executeNetworkInternal(📝 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.
| private async processExecutionQueue(): Promise<void> { | |
| while ( | |
| this.executionQueue.length > 0 && | |
| this.activeExecutions.size < this.config.maxConcurrentExecutions | |
| ) { | |
| const request = this.executionQueue.shift()! as ExecutionRequest & { | |
| resolve?: (result: NetworkExecutionResult) => void; | |
| reject?: (err: unknown) => void; | |
| }; | |
| const network = this.networks.get(request.networkId); | |
| const info = this.networkInfo.get(request.networkId); | |
| if (network && info && info.state !== "executing") { | |
| // Execute without blocking the queue processing, wiring up the queued promise | |
| this.executeNetworkInternal( | |
| network, | |
| info, | |
| request.input, | |
| request.options, | |
| ) | |
| .then((result) => request.resolve?.(result)) | |
| .catch((err) => request.reject?.(err)); | |
| } else if (request.reject) { | |
| request.reject( | |
| new Error(`Network unavailable for execution: ${request.networkId}`), | |
| ); | |
| } | |
| private async processExecutionQueue(): Promise<void> { | |
| while ( | |
| this.executionQueue.length > 0 && | |
| this.activeExecutions.size < this.config.maxConcurrentExecutions | |
| ) { | |
| const request = this.executionQueue.shift()! as ExecutionRequest & { | |
| resolve?: (result: NetworkExecutionResult) => void; | |
| reject?: (err: unknown) => void; | |
| }; | |
| const network = this.networks.get(request.networkId); | |
| const info = this.networkInfo.get(request.networkId); | |
| if (network && info && info.state === "executing") { | |
| this.executionQueue.push(request); | |
| break; | |
| } | |
| if (network && info) { | |
| // Execute without blocking the queue processing, wiring up the queued promise | |
| this.executeNetworkInternal( | |
| network, | |
| info, | |
| request.input, | |
| request.options, | |
| ) | |
| .then((result) => request.resolve?.(result)) | |
| .catch((err) => request.reject?.(err)); | |
| } else if (request.reject) { | |
| request.reject( | |
| new Error(`Network unavailable for execution: ${request.networkId}`), | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/agent/orchestration/orchestrator.ts` around lines 334 - 360, The loop
currently rejects queued requests when networkInfo shows info.state ===
"executing", causing same-network requests to fail during a drain; instead, in
processExecutionQueue replace the rejection branch for already-executing
networks so that the request (from executionQueue) is re-queued (e.g., push back
onto executionQueue) for later processing rather than calling request.reject;
update the branch that checks network/info/state within processExecutionQueue
(where executeNetworkInternal is invoked) to re-enqueue the request and
continue, leaving rejection only for truly missing networks.
| try { | ||
| // Run all test suites | ||
| suiteResults.push(await testAgentClass()); | ||
| suiteResults.push(await testNetworkTopologies()); | ||
| suiteResults.push(await testRoutingRules()); | ||
| suiteResults.push(await testMessageBus()); | ||
| suiteResults.push(await testIntegration()); | ||
| suiteResults.push(await testCLICoverage()); | ||
| } catch (error) { | ||
| log(`\n❌ Fatal error running tests: ${error}`, "red"); | ||
| } |
There was a problem hiding this comment.
Fail the run when a suite throws outside runTest.
The fatal catch only logs the error; if fixture loading or a suite setup throws before adding failed results, totalFailed can remain 0 and the process exits successfully. Track a fatal error flag or rethrow after logging.
Ensure fatal suite errors produce a non-zero exit
const startTime = Date.now();
const suiteResults: TestSuiteResult[] = [];
+ let fatalError: unknown;
@@
} catch (error) {
+ fatalError = error;
log(`\n❌ Fatal error running tests: ${error}`, "red");
}
@@
- if (totalFailed > 0) {
+ if (fatalError || totalFailed > 0) {
log("\n❌ Some tests failed!", "red");
process.exit(1);Also applies to: 1567-1574
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/continuous-test-suite-agents.ts` around lines 1524 - 1534, The try/catch
that runs suites (calling testAgentClass, testNetworkTopologies,
testRoutingRules, testMessageBus, testIntegration, testCLICoverage and pushing
into suiteResults) only logs a fatal error and does not mark the run as failed;
change it to set a fatalError flag (e.g., fatalSuiteError = true) in the catch
or rethrow the error after logging so that later logic that computes totalFailed
or the process exit code treats the run as failed; ensure the same change is
applied to the similar block around lines 1567-1574 so any exception before
runTest or before adding failed results produces a non-zero exit.
b6ad1c3 to
df44ecc
Compare
|
@coderabbitai full review v3 push (commit df44ecc) — fixes all 4 critical + all 21 major findings from your April 20 review: Critical fixes:
Major fixes:
Rebased onto release 9.56.1. All checks green, 16/16 live tests pass. |
|
User feedback: v3 push (commit ✅ Actions performedFull review triggered. |
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
…essage bus Adds a multi-agent orchestration layer to the NeuroLink SDK: - Core agents: Agent, AgentNetwork, RouterAgent with LLM-powered task routing - Orchestration: NetworkOrchestrator for managing multiple agent networks - Coordination: AgentCoordinator with sequential/parallel/pipeline strategies - Supervision: SupervisorAgent with HITL integration and escalation handling - Evaluation: AgentEvaluator and ResultOptimizer for quality assessment - Communication: MessageBus and ProtocolManager for inter-agent messaging - Topology: NetworkTopology and TopologyBuilder for network graph modeling SDK integration: - NeuroLink.createAgent(), createNetwork(), executeNetwork(), streamNetwork() - NeuroLink.createOrchestrator(), createSupervisor(), createEvaluator() - NeuroLink.createOptimizer(), createCoordinator(), createMessageBus() CLI: New agent and network commands via AgentCommandFactory Types: AgentNetworkTypes with full type exports Tests: Unit + integration test suites Docs: Agent configuration, testing, and CLI coverage documentation
murdore
force-pushed
the
feat/multi-agent-networks
branch
from
df44ecc to
29e5681
Compare
🤖 AI Review & Build Compliance ✅Status: AI analysis complete • Build rules validated • Ready for review 📊 View detailed analysis results🛡️ Analysis Complete
📋 Ready for Merge When
🤖 AI analysis complete - check individual code comments for specific feedback |
Summary
Adds a first-class multi-agent orchestration layer on top of NeuroLink. Agents, agent networks, and advanced orchestration primitives (coordinator, message bus, supervisor, evaluator, topology, router) can now be composed directly from the SDK and the CLI.
What's new
Agent— wraps a NeuroLink instance with specialized instructions, tools, temperature, and execution metrics.AgentNetwork— multi-agent orchestration with LLM-powered task routing (RouterAgent), iterative execution withmaxSteps, streaming, and full execution traces.neurolink.createOrchestrator() / createSupervisor() / createEvaluator() / createOptimizer() / createCoordinator() / createMessageBus()):NetworkOrchestrator+NetworkTopologyfor multi-network coordinationSupervisorAgent+ HITL hooks for oversightAgentEvaluator+ResultOptimizerfor quality scoring / iterative refinementAgentCoordinator+TaskDistributorfor coordination strategies and task distributionMessageBus+ protocol handlers (handshake, delegation, consensus, heartbeat) for inter-agent communicationneurolink agent <subcommand>andneurolink network <subcommand>command groups.src/lib/types/agentNetwork.tsand is re-exported through the canonicalsrc/lib/types/index.tsbarrel, per CLAUDE.md rules 2, 8, 9, 10, 11, 12, 13.Agent,AgentNetwork,RouterAgent; integration tests for agent / coordinator / network flows; live test suite (test/continuous-test-agents-live.ts) that exercises the full SDK → CLI → Agent → AgentNetwork stack against a real provider.Single-commit diff
42 files, +21,174 lines, zero deletions. All files are pure additions on top of
origin/release(9.54.4).Validation
pnpm run check— 0 errors, 0 warnings (3,655 files)pnpm run build— SDK + React hooks + CLI + browser bundle + publint all greenpnpm run lint— 0 errors (25 pre-existing non-null-assertion warnings unrelated to new rules)TEST_PROVIDER=vertex npx tsx test/continuous-test-agents-live.ts— 8/8 PASS:Test plan
TEST_PROVIDER=openai npx tsx test/continuous-test-agents-live.tsnode dist/cli/index.js agent create --id echo --name Echo --description "..." --instructions "..."thenagent executenetwork create --file test/fixtures/agents/network-topologies.jsonvitest run test/unit/agent test/integration/agentSummary by CodeRabbit
New Features
Documentation