feat(safety): add native PII detection and response validation to generate/stream#952
feat(safety): add native PII detection and response validation to generate/stream#952murdore wants to merge 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughIntroduces request-level safety controls: input validation, PII detection/redaction, response validation, and tripwire evaluation; adds utility modules, types, CLI flags, integrates checks into generate/stream flows, and includes a comprehensive test suite. No public method signatures were removed. Changes
Sequence DiagramsequenceDiagram
actor Client
participant CLI as CLI Handler
participant InputVal as Input Validator
participant PII as PII Detector
participant Generator as Model/Generator
participant RespVal as Response Validator
participant Trip as Tripwire Evaluator
Client->>CLI: invoke generate/stream with options
CLI->>InputVal: apply inputValidation (trim/length/requireContent)
InputVal-->>CLI: valid / invalid
alt invalid
CLI-->>Client: abort request
else valid
CLI->>PII: detectAndRedactPII(text, config)
alt PII action == "abort"
PII-->>CLI: abort signal
CLI-->>Client: blocked
else PII action == "redact"
PII-->>CLI: redacted text
CLI->>Generator: proceed with redacted input
else
PII-->>CLI: continue
CLI->>Generator: proceed
end
Generator->>Generator: produce response (sync/stream)
Generator-->>CLI: response text/stream
CLI->>RespVal: validateResponse(text, config)
alt RespVal action == "abort"
RespVal-->>CLI: abort
CLI-->>Client: error
else RespVal action == "retry"
RespVal-->>CLI: retry request
CLI->>Generator: retry (with feedback if any)
else RespVal action == "continue"
RespVal-->>CLI: validated text
CLI->>Trip: evaluate tripwires(data)
alt Trip triggered with action == "abort"
Trip-->>CLI: abort
CLI-->>Client: blocked
else Trip warn/log or none
CLI-->>Client: return response
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
✅ 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 |
🤖 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 |
Documentation Validation Results
🚧 Please fix the failing checks before merging. Commit: |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new I/O processor middleware system for NeuroLink, including a registry + priority-ordered pipeline, built-in input/output processors, tripwire guards, presets, and SDK/CLI integration for configuring and running processor pipelines around generation/stream flows.
Changes:
- Add core processor infrastructure (registry, pipeline, presets, tripwires, utilities, typed error helpers).
- Add built-in input/output processors (validation, moderation/filtering, PII/toxicity, memory/context placeholders).
- Integrate processors into the SDK (
NeuroLinkconvenience methods + constructor/options types) and CLI command routing, with extensive unit/integration tests and fixtures.
Reviewed changes
Copilot reviewed 48 out of 48 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/processors/registry.test.ts | Unit coverage for processor registry behaviors (register/unregister/presets/build). |
| test/unit/processors/pipeline.test.ts | Unit coverage for pipeline execution order, conditions, abort/retry, tracing. |
| test/unit/processors/output/toxicityCheck.test.ts | Unit coverage for toxicity output processor config/actions. |
| test/unit/processors/input/piiDetection.test.ts | Unit coverage for PII input processor detection/redaction/config validation. |
| test/processors/sdk-integration.test.ts | SDK-level tests for NeuroLink processor convenience API (currently has type/import mismatches). |
| test/processors/integration.test.ts | End-to-end integration tests for registry/pipeline/presets (currently has type/import mismatches). |
| test/processors/IOProcessorError.test.ts | Tests for IOProcessorError factories/helpers and retryability checks. |
| test/fixtures/processors/pipeline-config.json | Fixture samples for valid/invalid pipeline configurations and tripwires. |
| test/fixtures/processors/input-samples.json | Fixture samples for input processor scenarios (PII/moderation/validation). |
| test/fixtures/processors/output-samples.json | Fixture samples for output processor scenarios (length/toxicity/filtering). |
| src/lib/types/stream.ts | Add per-request processors?: ProcessorPipelineConfig to StreamOptions. |
| src/lib/types/generate.ts | Add per-request processors?: ProcessorPipelineConfig to GenerateOptions/TextGenerationOptions. |
| src/lib/types/config.ts | Add processors config block to NeurolinkConstructorConfig. |
| src/lib/types/index.ts | Re-export canonical I/O processor types via ./ioProcessor.js. |
| src/lib/processors/utils/validationUtils.ts | Add JSON/config validation helper utilities (has a null-vs-object bug). |
| src/lib/processors/utils/processorFactory.ts | Add processor factory/composition/condition/retry/timeout helpers. |
| src/lib/processors/utils/metadataUtils.ts | Add metadata creation/merge/summary helpers for processor tracing/issues. |
| src/lib/processors/utils/index.ts | Barrel export for processor utility helpers. |
| src/lib/processors/tripwire.ts | Add tripwire evaluator + common built-in tripwires. |
| src/lib/processors/registry.ts | Add I/O ProcessorRegistry + defaultRegistry instance. |
| src/lib/processors/processorHelpers.ts | Add small runtime helper creators for metadata/result variants. |
| src/lib/processors/presets.ts | Add built-in presets (default/security/strict/quality/minimal). |
| src/lib/processors/input/messageValidationProcessor.ts | Add input validation processor (length/messages/system prompt/etc). |
| src/lib/processors/input/piiDetectionProcessor.ts | Add PII detection/redaction/warn/abort input processor. |
| src/lib/processors/input/contentModerationProcessor.ts | Add input moderation processor with heuristic + optional AI moderation. |
| src/lib/processors/input/memoryRetrievalProcessor.ts | Add memory retrieval input processor (currently placeholder retrieval). |
| src/lib/processors/input/semanticContextProcessor.ts | Add semantic context enrichment input processor (currently placeholder search). |
| src/lib/processors/input/index.ts | Barrel exports for input processors. |
| src/lib/processors/output/responseValidationProcessor.ts | Add response validation output processor (phrases/length/schema/custom validator). |
| src/lib/processors/output/lengthValidationProcessor.ts | Add response length output processor (warn/truncate/retry/abort). |
| src/lib/processors/output/contentFilteringProcessor.ts | Add content filtering output processor (word/pattern redact/retry/abort). |
| src/lib/processors/output/toxicityCheckProcessor.ts | Add toxicity output processor (heuristic categories/actions). |
| src/lib/processors/output/memoryPersistenceProcessor.ts | Add memory persistence output processor (currently no-op but reports success). |
| src/lib/processors/output/index.ts | Barrel exports for output processors. |
| src/lib/processors/errors/IOProcessorError.ts | Add typed IOProcessor error codes + helper constructors. |
| src/lib/processors/index.ts | Export I/O processor system and auto-register built-ins into defaultRegistry. |
| src/lib/neurolink.ts | Add processor registry/pipeline methods + constructor wiring (imports processor barrel via side-effect). |
| src/lib/index.ts | Re-export processor API from main package entry. |
| src/cli/parser.ts | Register new processors CLI command group. |
| src/cli/loop/optionsSchema.ts | Exclude processors from simple CLI options schema (SDK-only). |
| docs/tutorials.md | Tutorial updates (currently has missing frontmatter + encoding/snippet issues). |
| FEATURE-STATUS.md | Add feature status doc for I/O processors. |
Comments suppressed due to low confidence (1)
docs/tutorials.md:131
- The section header renders as
## �🌐 Web App Integration(replacement character), which indicates a file encoding/copy-paste issue. Also, the Express example readsreq.body.promptbut does not callapp.use(express.json()), soreq.bodywill be undefined for JSON requests. Fix the header text and add the JSON body middleware in the snippet.
## �🌐 Web App Integration
### Express.js API
```javascript
const express = require("express");
const { NeuroLink } = require("@juspay/neurolink");
const app = express();
const neurolink = new NeuroLink();
app.post("/api/generate", async (req, res) => {
const result = await neurolink.generate({
input: { text: req.body.prompt },
enableAnalytics: true,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 📚 Step-by-Step Integration Tutorials | ||
|
|
||
| ## 🚀 Quick Start (15 minutes) {#quick-start-15-minutes} |
There was a problem hiding this comment.
This docs page appears to have lost its YAML frontmatter (e.g., ---\ntitle: ...\nslug: ...\n---) that other docs pages in this repo use for navigation/metadata. Please restore the frontmatter at the top so the docs site can index and route this page correctly.
| // Type validation | ||
| if (schema.type) { | ||
| const expectedType = schema.type as string; | ||
| const actualType = Array.isArray(data) ? "array" : typeof data; | ||
|
|
||
| if (expectedType === "object" && actualType !== "object") { | ||
| errors.push(`Expected type 'object', got '${actualType}'`); | ||
| } else if (expectedType === "array" && !Array.isArray(data)) { | ||
| errors.push(`Expected type 'array', got '${actualType}'`); | ||
| } else if (expectedType === "string" && typeof data !== "string") { | ||
| errors.push(`Expected type 'string', got '${actualType}'`); | ||
| } else if (expectedType === "number" && typeof data !== "number") { | ||
| errors.push(`Expected type 'number', got '${actualType}'`); | ||
| } else if (expectedType === "boolean" && typeof data !== "boolean") { | ||
| errors.push(`Expected type 'boolean', got '${actualType}'`); | ||
| } else if (expectedType === "null" && data !== null) { | ||
| errors.push(`Expected type 'null', got '${actualType}'`); | ||
| } |
There was a problem hiding this comment.
validateJsonSchema treats null as type object (typeof null === "object"), so a schema with { type: "object" } will incorrectly accept null. Add an explicit data === null check when validating object type (and/or reuse isJsonObject) so null fails the object type check.
| try { | ||
| // Build the conversation turn to store | ||
| const turn: ConversationTurn = { | ||
| userMessage: data.input.text, | ||
| assistantMessage: data.responseText, | ||
| toolCalls: includeToolCalls ? data.toolCalls : undefined, | ||
| timestamp: Date.now(), | ||
| metadata: { | ||
| ...customMetadata, | ||
| provider: data.metadata.provider, | ||
| model: data.metadata.model, | ||
| requestId: data.metadata.requestId, | ||
| }, | ||
| }; | ||
|
|
||
| // Store in appropriate memory store | ||
| await storeConversationTurn(effectiveSessionId, turn, storeType, { | ||
| maxMessages, | ||
| ttlSeconds, | ||
| enableSummarization, | ||
| }); | ||
|
|
||
| return { | ||
| action: "continue", | ||
| data, | ||
| metadata: { | ||
| memoryPersisted: true, | ||
| storeType, | ||
| sessionId: effectiveSessionId, | ||
| includeToolCalls, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
storeConversationTurn() is currently a no-op placeholder, but on the success path the processor sets memoryPersisted: true. This is misleading for callers (it reports persistence even though nothing is stored). Either implement real persistence using the existing memory managers, or mark the operation as skipped/not-implemented (and set memoryPersisted: false with a reason) until it’s wired up.
| import { ProcessorPipeline } from "./processors/pipeline.js"; | ||
| import { | ||
| defaultRegistry as processorDefaultRegistry, | ||
| ProcessorRegistry, | ||
| } from "./processors/registry.js"; | ||
| // Side-effect import: registers built-in processors and presets into defaultRegistry | ||
| import "./processors/index.js"; |
There was a problem hiding this comment.
This side-effect import of ./processors/index.js will execute the processors barrel, which re-exports CLI/file-processor helpers and can pull Node-only/CLI dependencies into any environment that imports NeuroLink (including browser bundles). To avoid bundling/runtime issues, move the default-registry initialization into a dedicated, dependency-light module (e.g. processors/initializeDefaultRegistry) and import/call only that here.
| areProcessorsEnabled(): boolean { | ||
| return this.sdkProcessorsEnabled; | ||
| } | ||
|
|
||
| setProcessorsEnabled(enabled: boolean): void { | ||
| this.sdkProcessorsEnabled = enabled; | ||
| } | ||
|
|
||
| async processInput( | ||
| data: InputProcessorData, | ||
| metadataOverrides?: Partial<ProcessorMetadata>, | ||
| ): Promise< | ||
| ProcessorPipelineResult<InputProcessorData> & { processorsExecuted: number } | ||
| > { | ||
| if (!this.sdkProcessorPipeline) { | ||
| return { | ||
| action: "continue", | ||
| data, | ||
| feedback: [], | ||
| issues: [], | ||
| metadata: { | ||
| requestId: `neurolink-${Date.now()}`, | ||
| timestamp: Date.now(), | ||
| custom: {}, | ||
| issues: [], | ||
| processorTrace: [], | ||
| ...metadataOverrides, | ||
| }, | ||
| totalTime: 0, | ||
| processorsExecuted: 0, | ||
| }; | ||
| } | ||
| const result = await this.sdkProcessorPipeline.processInput( | ||
| data, | ||
| metadataOverrides, |
There was a problem hiding this comment.
setProcessorsEnabled(false) only toggles sdkProcessorsEnabled, but processInput/processOutput don’t check that flag—if a pipeline exists they will still execute processors. Add an early return when processors are disabled (similar to the !this.sdkProcessorPipeline branch) so enabling/disabling works as advertised.
| import type { | ||
| InputProcessor, | ||
| OutputProcessor, | ||
| InputProcessorData, | ||
| OutputProcessorData, | ||
| ProcessorMetadata, | ||
| } from "../../src/lib/processors/types.js"; |
There was a problem hiding this comment.
../../src/lib/processors/types.js is imported here, but that module does not exist in the codebase (there is no src/lib/processors/types.ts/js). This will break the test build; update the import to the canonical processor types (e.g. src/lib/types/ioProcessor.js / src/lib/types/index.js) or add the intended compatibility re-export module.
| import type { | ||
| InputProcessor, | ||
| OutputProcessor, | ||
| InputProcessorData, | ||
| OutputProcessorData, | ||
| ProcessorMetadata, | ||
| ProcessorResult, | ||
| } from "../../src/lib/processors/types.js"; |
There was a problem hiding this comment.
This test imports types from ../../src/lib/processors/types.js, which does not exist, and then constructs InputProcessorData/OutputProcessorData objects with only text/responseText. The canonical InputProcessorData/OutputProcessorData types require fields like options, messages, and metadata, so this file will not typecheck once the import is corrected. Update the imports and build valid processor data objects (or provide a helper that creates the required defaults).
| const customProcessor: InputProcessor = { | ||
| id: "custom-input", | ||
| name: "Custom Input", | ||
| description: "A custom input processor", | ||
| priority: 50, | ||
| async process(data, metadata) { | ||
| return { action: "continue", data, metadata }; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
These custom processor definitions use a (data, metadata, config) style signature and return { metadata } as if it were full pipeline metadata. The current InputProcessor/OutputProcessor API used by ProcessorPipeline calls process(data, config) and treats result.metadata as custom metadata fields to merge into metadata.custom (not a full metadata object). Adjust the test processors to match the actual signature/return shape so the tests reflect real behavior.
murdore
force-pushed
the
feat/io-processors
branch
from
a0b16c4 to
34eeaf6
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/processors/output/toxicityCheckProcessor.ts (1)
383-418:⚠️ Potential issue | 🟠 MajorHonor
categoriesin the heuristic fallback.
performHeuristicModeration()ignores the requested category list and always scores violence/harassment. When AI moderation is disabled or falls back, callers can still be warned/blocked for categories they explicitly opted out of.🛠️ Proposed fix
-async function performHeuristicModeration( - text: string, - _categories: ModerationCategory[], -): Promise<AIModerationResult> { +async function performHeuristicModeration( + text: string, + categories: ModerationCategory[], +): Promise<AIModerationResult> { const lowerText = text.toLowerCase(); let score = 0; const flaggedCategories: string[] = []; const categoryScores: Record<string, number> = {}; // Simple heuristics (these serve as a fallback when AI moderation fails) const violenceIndicators = ["kill", "murder", "attack", "destroy", "hurt"]; const harassmentIndicators = ["stupid", "idiot", "dumb", "hate you"]; - const violenceMatches = violenceIndicators.filter((w) => - lowerText.includes(w), - ); - if (violenceMatches.length > 0) { + if (categories.includes("violence")) { + const violenceMatches = violenceIndicators.filter((w) => + lowerText.includes(w), + ); + if (violenceMatches.length > 0) { const violenceScore = Math.min(0.3 * violenceMatches.length, 1.0); score = Math.max(score, violenceScore); categoryScores["violence"] = violenceScore; if (violenceScore > 0.5) { flaggedCategories.push("violence"); } } } - const harassmentMatches = harassmentIndicators.filter((w) => - lowerText.includes(w), - ); - if (harassmentMatches.length > 0) { + if (categories.includes("harassment")) { + const harassmentMatches = harassmentIndicators.filter((w) => + lowerText.includes(w), + ); + if (harassmentMatches.length > 0) { const harassmentScore = Math.min(0.2 * harassmentMatches.length, 1.0); score = Math.max(score, harassmentScore); categoryScores["harassment"] = harassmentScore; if (harassmentScore > 0.5) { flaggedCategories.push("harassment"); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/toxicityCheckProcessor.ts` around lines 383 - 418, performHeuristicModeration currently always evaluates violence/harassment heuristics regardless of what the caller requested; update performHeuristicModeration to honor its categories parameter by only computing and returning scores for categories that are present in the provided categories array (and preserve existing default behavior if categories is undefined/null). Specifically, inside performHeuristicModeration, gate each heuristic block (e.g., violence/harassment scoring logic and any other category-specific checks) behind checks like categories?.includes("<categoryName>") before running that category's logic, and ensure the returned result only contains keys for evaluated categories so callers who opted out won’t receive or be blocked by those scores. Ensure any callers expecting the old shape still handle missing category keys or keep a backward-compatible empty-score behavior when necessary.
🟠 Major comments (31)
docs/tutorials.md-122-127 (1)
122-127:⚠️ Potential issue | 🟠 MajorMissing
express.json()middleware breaks the example.The Express.js example accesses
req.body.prompt(line 130) but lacksapp.use(express.json())to parse incoming JSON. Users copying this snippet will getundefinedforreq.body.Proposed fix
const express = require("express"); const { NeuroLink } = require("@juspay/neurolink"); const app = express(); +app.use(express.json()); const neurolink = new NeuroLink();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/tutorials.md` around lines 122 - 127, The example is missing the Express JSON body-parsing middleware so req.body.prompt will be undefined; add the middleware by calling app.use(express.json()) after creating the app (after const app = express()) so incoming JSON is parsed before handlers that access req.body.prompt and any routes that use the neurolink instance.test/unit/processors/output/toxicityCheck.test.ts-9-14 (1)
9-14:⚠️ Potential issue | 🟠 MajorSwitch internal type imports to the canonical
src/lib/typesbarrel.Direct import from
ioProcessor.jsin a non-src/lib/typesfile violates the project’s type import rule.🔧 Proposed fix
import type { InputProcessorData, OutputProcessorData, ProcessorMetadata, ToxicityCheckConfig, -} from "../../../../src/lib/types/ioProcessor.js"; +} from "../../../../src/lib/types/index.js";As per coding guidelines: “Code outside
src/lib/types/must import internal types from the barrel (../types/index.jsor../types), never from specific type files.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/processors/output/toxicityCheck.test.ts` around lines 9 - 14, The test file imports internal types directly from ioProcessor.js; update the import to use the canonical barrel export instead. Replace the direct import of InputProcessorData, OutputProcessorData, ProcessorMetadata, ToxicityCheckConfig from "../../../../src/lib/types/ioProcessor.js" with an import from the types barrel (e.g., "../../../../src/lib/types" or "../../../../src/lib/types/index.js") so the test references the same exported types via the central barrel.test/unit/processors/input/piiDetection.test.ts-9-14 (1)
9-14:⚠️ Potential issue | 🟠 MajorUse the canonical type barrel import instead of direct
ioProcessortype-file import.This test imports internal types from a specific file path, which breaks the repository’s type-import boundary rule for non-
src/lib/typesfiles.🔧 Proposed fix
import type { InputProcessorData, PIIDetectionConfig, ProcessorMetadata, -} from "../../../../src/lib/types/ioProcessor.js"; +} from "../../../../src/lib/types/index.js";As per coding guidelines: “Code outside
src/lib/types/must import internal types from the barrel (../types/index.jsor../types), never from specific type files.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/processors/input/piiDetection.test.ts` around lines 9 - 14, The test imports internal types InputProcessorData, PIIDetectionConfig, and ProcessorMetadata directly from the ioProcessor type file; change the import to use the canonical types barrel (the src/lib/types index export) instead of the specific ioProcessor.js file so the test follows the repo's type-import boundary rule and still imports InputProcessorData, PIIDetectionConfig, and ProcessorMetadata from the barrel.test/processors/integration.test.ts-15-23 (1)
15-23:⚠️ Potential issue | 🟠 MajorType imports should come from
src/lib/typesbarrel, notsrc/lib/processors/types.js.This path bypasses the canonical type boundary and should be switched to the central types barrel import.
🔧 Proposed fix
import type { InputProcessor, OutputProcessor, InputProcessorData, OutputProcessorData, ProcessorMetadata, ProcessorResult, -} from "../../src/lib/processors/types.js"; +} from "../../src/lib/types/index.js";As per coding guidelines: “Code outside
src/lib/types/must import internal types from the barrel (../types/index.jsor../types), never from specific type files.”🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/processors/integration.test.ts` around lines 15 - 23, The test is importing internal types directly from "../../src/lib/processors/types.js" which bypasses the canonical types barrel; update the import to come from the central barrel (e.g., "../../src/lib/types" or "../../src/lib/types/index.js") and import the same symbols InputProcessor, OutputProcessor, InputProcessorData, OutputProcessorData, ProcessorMetadata, ProcessorResult so the test uses the approved canonical type boundary.src/lib/processors/input/piiDetectionProcessor.ts-146-154 (1)
146-154:⚠️ Potential issue | 🟠 MajorTrack matches per source field before redacting.
detectedPIIpositions are computed against the concatenatedtextToScan, then reused againstdata.textand every individual message. Those offsets do not map back to the original strings, so multi-message inputs will miss redactions and can also replace unrelated slices in earlier fields.Please either detect/redact per field, or store the source plus field-local offsets when building
detectedPII.Also applies to: 226-249
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/input/piiDetectionProcessor.ts` around lines 146 - 154, The current PII detection builds a single concatenated text (textToScan) and populates detectedPII offsets against that concatenation, then applies those offsets back to data.text and each message, which misaligns offsets across fields; update the logic in piiDetectionProcessor (references: detectedPII, textToScan, data.text, data.messages) to either (a) run detection separately for each source field—compute and redact PII per field using field-local offsets—or (b) when detecting over the concatenated text, record the source identifier and the field-local start/end for each DetectedPII (e.g., include a sourceField/messageIndex and local offsets) and use those to apply redactions back to the correct field; apply the same fix to the later block around the other occurrence (lines ~226-249) so offsets always map to their original source before replacing.src/lib/processors/presets.ts-261-275 (1)
261-275:⚠️ Potential issue | 🟠 MajorAvoid exposing live mutable preset references.
builtInPresetsandgetPreset()currently hand out the shared preset objects. Any consumer that tweaks the returned config mutates the global defaults for every later pipeline/test in the same process.Please return cloned presets and avoid exporting a directly mutable preset array.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/presets.ts` around lines 261 - 275, The builtInPresets array currently exposes live mutable objects (builtInPresets, defaultPreset, securityPreset, strictPreset, qualityPreset, minimalPreset) and getPreset(name) returns references that callers can mutate; stop exporting the raw array and return cloned presets instead: make the internal array non-exported (or freeze it) and change getPreset to locate the preset by name from the internal list but return a deep clone (use structuredClone or your existing deepClone utility) of the ProcessorPreset before returning; similarly, if you must export a list, expose a function like listPresets() that returns clones rather than exporting builtInPresets directly.src/lib/processors/output/memoryPersistenceProcessor.ts-205-248 (1)
205-248:⚠️ Potential issue | 🟠 MajorDo not report successful persistence while the store is still a no-op.
storeConversationTurn()currently does nothing, but the caller treats it as a successful write and returnsmemoryPersisted: true. That means this built-in processor silently drops conversation history while telling consumers it was saved.Until this is wired to the real memory managers, it should surface
skipped/not implementedsemantics instead of success.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/memoryPersistenceProcessor.ts` around lines 205 - 248, storeConversationTurn currently no-ops but returns success to callers; change it to surface "not implemented/skipped" semantics by rejecting the promise with a clear error instead of resolving. Modify the body of storeConversationTurn to return Promise.reject(new Error('Memory persistence not implemented: in-memory/redis managers not wired')) (or throw the same Error) so callers that await storeConversationTurn (e.g., code that sets memoryPersisted) will see a failure and can mark the operation as skipped; keep the function name storeConversationTurn and preserve the existing signature.test/unit/processors/registry.test.ts-9-13 (1)
9-13: 🛠️ Refactor suggestion | 🟠 MajorImport internal processor types from the barrel.
This test pulls internal types from
src/lib/types/ioProcessor.jsinstead of the canonical barrel. Please switch this tosrc/lib/types/index.jsso the test stays aligned with the repo’s type-import boundary.Proposed change
import type { InputProcessor, OutputProcessor, ProcessorPreset, -} from "../../../src/lib/types/ioProcessor.js"; +} from "../../../src/lib/types/index.js";As per coding guidelines,
!(src/lib/types)/**/*.ts: Code outsidesrc/lib/types/must import internal types from the barrel (../types/index.jsor../types), never from specific type files (../types/rag.js,../types/mcp.js).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/processors/registry.test.ts` around lines 9 - 13, The test imports internal types InputProcessor, OutputProcessor, and ProcessorPreset directly from ioProcessor.js; update the import to use the types barrel (the exported index for src/lib/types) instead of the specific file so the test follows the repo's type-import boundary — replace the module specifier for the existing import of InputProcessor, OutputProcessor, ProcessorPreset to import them from the types barrel (the index export of src/lib/types) while keeping the same named type imports.test/unit/processors/tripwire.test.ts-17-22 (1)
17-22: 🛠️ Refactor suggestion | 🟠 MajorImport internal tripwire types from the barrel.
This test reaches into
src/lib/types/ioProcessor.jsdirectly. Please switch it to the canonicalsrc/lib/types/index.jsbarrel to stay within the repo’s internal type-import contract.Proposed change
import type { InputProcessorData, OutputProcessorData, ProcessorMetadata, TripwireConfig, -} from "../../../src/lib/types/ioProcessor.js"; +} from "../../../src/lib/types/index.js";As per coding guidelines,
!(src/lib/types)/**/*.ts: Code outsidesrc/lib/types/must import internal types from the barrel (../types/index.jsor../types), never from specific type files (../types/rag.js,../types/mcp.js).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/processors/tripwire.test.ts` around lines 17 - 22, The test imports internal types (InputProcessorData, OutputProcessorData, ProcessorMetadata, TripwireConfig) directly from a specific type module; change the import to use the repository's types barrel (the types index/barrel) instead of the specific ioProcessor module so the test imports those symbols from the canonical barrel export; update the import statement in tripwire.test.ts to reference the central types barrel export and keep the same type names.src/lib/processors/input/piiDetectionProcessor.ts-181-186 (1)
181-186:⚠️ Potential issue | 🟠 MajorValidate custom regex patterns for type and safety before execution.
customPatternsare executed directly without runtime validation. Although the type definition declarespattern: RegExp, TypeScript doesn't enforce this at runtime. Non-RegExp values will causedetectPIIWithPatternto fail when accessing.sourceand.flags. Additionally, unchecked regex patterns can exhibit catastrophic backtracking on large prompts.Add explicit type validation and reject patterns with nested quantifiers. The codebase already implements similar checks in
customScorerUtils.tsandautoresearch/config.ts—use the same pattern:(/(\+|\*|\{)\S*(\+|\*|\{)/)to detect nested quantifiers.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/input/piiDetectionProcessor.ts` around lines 181 - 186, Validate each custom pattern before calling detectPIIWithPattern: in the loop over customPatterns (the block creating `detected = detectPIIWithPattern(textToScan, custom.name, custom.pattern)`), first confirm `custom.pattern` is an actual RegExp (e.g., instanceof RegExp) and if not, skip/log an error; then test the pattern's source for nested quantifiers using the same check used elsewhere (/(\+|\*|\{)\S*(\+|\*|\{)/) and skip/log/reject any pattern that matches to avoid catastrophic backtracking; only pass safe, validated RegExp objects into detectPIIWithPattern.src/lib/processors/output/lengthValidationProcessor.ts-82-90 (1)
82-90:⚠️ Potential issue | 🟠 MajorGuard
retryCountreads when metadata is absent.Both retry branches dereference
data.metadata.custom.retryCount, but this processor is publicly callable with bareOutputProcessorDataand the example in this file does exactly that. Withaction: "retry", this throws before returning a processor result.🛠️ Proposed fix
- const currentRetry = (data.metadata.custom.retryCount as number) || 0; + const currentRetry = Number(data.metadata?.custom?.retryCount ?? 0); ... - const currentRetry = (data.metadata.custom.retryCount as number) || 0; + const currentRetry = Number(data.metadata?.custom?.retryCount ?? 0);Also applies to: 146-154
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/lengthValidationProcessor.ts` around lines 82 - 90, The code dereferences data.metadata.custom.retryCount in the retry branches (variables: action, data, metadata, custom, retryCount, maxRetries) which will throw when metadata is absent; update the length validation processor (e.g., lengthValidationProcessor/handler) to safely read retryCount using optional chaining and a numeric fallback (e.g., const currentRetry = Number(data?.metadata?.custom?.retryCount) || 0) and do the same in the other retry branch (the one around lines 146-154) so both branches never throw when metadata or custom is undefined and still increment retryCount properly.src/lib/processors/output/lengthValidationProcessor.ts-122-125 (1)
122-125:⚠️ Potential issue | 🟠 MajorClamp truncation so the result never exceeds
maxLength.If
maxLengthis smaller thantruncationSuffix.length, the currentslice()call gets a negative end index and can return a string longer than the configured maximum.✂️ Proposed fix
- const truncatedText = - responseText.slice(0, maxLength - truncationSuffix.length) + - truncationSuffix; + const suffix = truncationSuffix.slice(0, maxLength); + const keepLength = Math.max(maxLength - suffix.length, 0); + const truncatedText = responseText.slice(0, keepLength) + suffix; ... - const truncatedText = - responseText.slice(0, maxLength - truncationSuffix.length) + - truncationSuffix; + const suffix = truncationSuffix.slice(0, maxLength); + const keepLength = Math.max(maxLength - suffix.length, 0); + const truncatedText = responseText.slice(0, keepLength) + suffix;Also applies to: 158-160
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/lengthValidationProcessor.ts` around lines 122 - 125, The truncate branch can produce output longer than maxLength when truncationSuffix.length > maxLength; in the action === "truncate" block compute a clamped slice length (e.g., clampLength = Math.max(0, maxLength - truncationSuffix.length)) and use responseText.slice(0, clampLength) + truncationSuffix so the final string never exceeds maxLength; apply the same clamp fix to the other truncate occurrence in this file (the second truncatedText computation).src/lib/processors/input/contentModerationProcessor.ts-132-138 (1)
132-138:⚠️ Potential issue | 🟠 MajorHandle simplified input without
messages.
data.messages.filter(...)assumes the array is always present. Other processors in this PR already treatmessagesas optional, so text-only calls can throw here before moderation starts.🛠️ Proposed fix
- const messageTexts = data.messages + const messageTexts = (data.messages ?? []) .filter((m) => m.role === "user") .map((m) => m.content) .join("\n");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/input/contentModerationProcessor.ts` around lines 132 - 138, The code assumes data.messages exists and calls data.messages.filter(...) which throws for text-only inputs; update the logic that builds textToScan so it safely handles missing or non-array messages (e.g., treat messages as an empty array when absent). Modify the section that defines messageTexts and textToScan (the symbols messageTexts and textToScan in contentModerationProcessor) to use a guarded/messages fallback (or Array.isArray check) before .filter/.map/.join so text-only calls proceed without error.src/lib/processors/output/toxicityCheckProcessor.ts-169-178 (1)
169-178:⚠️ Potential issue | 🟠 MajorGuard
retryCountaccess in the retry path.
data.metadata.custom.retryCountwill throw whenever upstream metadata does not initializecustom, soaction: "retry"is not actually safe for direct processor use.🛠️ Proposed fix
- const currentRetry = (data.metadata.custom.retryCount as number) || 0; + const currentRetry = Number(data.metadata?.custom?.retryCount ?? 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/toxicityCheckProcessor.ts` around lines 169 - 178, The retry path reads data.metadata.custom.retryCount directly which can throw if metadata.custom is undefined; update the logic in the retry branch (where action === "retry" and currentRetry is computed) to safely access retryCount using optional chaining or a guarded lookup (e.g., const currentRetry = ((data.metadata && data.metadata.custom && data.metadata.custom.retryCount) as number) || 0 or initialize data.metadata.custom before reading), and ensure retryCount, retryCount + 1, and maxRetries are returned consistently so processors like toxicityCheckProcessor's retry branch never access data.metadata.custom without first checking/creating it.src/lib/processors/input/contentModerationProcessor.ts-383-418 (1)
383-418:⚠️ Potential issue | 🟠 MajorHonor
categoriesin the heuristic fallback.
performHeuristicModeration()ignores the requested category list and always scores violence/harassment. If AI moderation is disabled or unavailable, this can block content for categories the caller did not enable.🛠️ Proposed fix
-async function performHeuristicModeration( - text: string, - _categories: ModerationCategory[], -): Promise<AIModerationResult> { +async function performHeuristicModeration( + text: string, + categories: ModerationCategory[], +): Promise<AIModerationResult> { const lowerText = text.toLowerCase(); let score = 0; const flaggedCategories: string[] = []; const categoryScores: Record<string, number> = {}; // Simple heuristics (these serve as a fallback when AI moderation fails) const violenceIndicators = ["kill", "murder", "attack", "destroy", "hurt"]; const harassmentIndicators = ["stupid", "idiot", "dumb", "hate you"]; - const violenceMatches = violenceIndicators.filter((w) => - lowerText.includes(w), - ); - if (violenceMatches.length > 0) { + if (categories.includes("violence")) { + const violenceMatches = violenceIndicators.filter((w) => + lowerText.includes(w), + ); + if (violenceMatches.length > 0) { const violenceScore = Math.min(0.3 * violenceMatches.length, 1.0); score = Math.max(score, violenceScore); categoryScores["violence"] = violenceScore; if (violenceScore > 0.5) { flaggedCategories.push("violence"); } } } - const harassmentMatches = harassmentIndicators.filter((w) => - lowerText.includes(w), - ); - if (harassmentMatches.length > 0) { + if (categories.includes("harassment")) { + const harassmentMatches = harassmentIndicators.filter((w) => + lowerText.includes(w), + ); + if (harassmentMatches.length > 0) { const harassmentScore = Math.min(0.2 * harassmentMatches.length, 1.0); score = Math.max(score, harassmentScore); categoryScores["harassment"] = harassmentScore; if (harassmentScore > 0.5) { flaggedCategories.push("harassment"); } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/input/contentModerationProcessor.ts` around lines 383 - 418, performHeuristicModeration currently ignores the _categories parameter and always evaluates violence/harassment; update the function to respect the requested categories by only running the violenceIndicators block when _categories contains the "violence" category and only running the harassmentIndicators block when _categories contains the "harassment" category (use a case-insensitive check or match against the ModerationCategory enum values), and only add entries to categoryScores and flaggedCategories for those enabled categories so the heuristic fallback only flags categories the caller requested.src/lib/processors/output/responseValidationProcessor.ts-125-137 (1)
125-137:⚠️ Potential issue | 🟠 MajorRetry mode assumes
metadata.customalways exists.
data.metadata.custom.retryCountwill throw when this processor is used directly with bareOutputProcessorDataor when upstream metadata omitscustom. That makesretryOnFailureunusable in those cases.🛠️ Proposed fix
- const currentRetry = (data.metadata.custom.retryCount as number) || 0; + const currentRetry = Number(data.metadata?.custom?.retryCount ?? 0);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/responseValidationProcessor.ts` around lines 125 - 137, The code assumes data.metadata.custom always exists and will throw when metadata or custom is undefined; change the access to use safe checks/optional chaining and defaults (e.g., const currentRetry = ((data.metadata?.custom as any)?.retryCount as number) || 0) and when returning the retry action ensure you do not rely on missing objects (either include retryCount/maxRetries in the returned object only or create/clone metadata.custom before mutating), and make the same defensive checks around any other metadata.custom accesses in the responseValidationProcessor (reference symbols: data.metadata.custom.retryCount, retryOnFailure, maxRetries, OutputProcessorData).src/lib/processors/input/messageValidationProcessor.ts-74-95 (1)
74-95:⚠️ Potential issue | 🟠 Major
requireContentrejects message-only input.This branch only inspects
workingData.text. Calls that supply chatmessageswithout a top-leveltextfield will abort as empty even when user messages contain content.🛠️ Proposed fix
if (requireContent) { - const trimmedText = - typeof workingData.text === "string" ? workingData.text.trim() : ""; - if (trimmedText.length === 0) { + const hasText = + typeof workingData.text === "string" && + workingData.text.trim().length > 0; + const hasMessageContent = messages.some((message) => + typeof message.content === "string" + ? message.content.trim().length > 0 + : Array.isArray(message.content) && message.content.length > 0, + ); + if (!hasText && !hasMessageContent) { return { action: "abort", feedback: "Message content is empty",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/input/messageValidationProcessor.ts` around lines 74 - 95, The validation branch using requireContent currently only checks workingData.text and wrongly aborts when top-level text is absent but chat-style workingData.messages contains user content; update the requireContent check (in messageValidationProcessor) to first trim and inspect workingData.text as before but if empty/absent also scan workingData.messages (an array) for any message objects with non-empty content (e.g., message.content, message.text, or nested content fields) and treat that as satisfying content; only return the same abort object when both the top-level text and all messages are empty or missing.src/lib/processors/utils/validationUtils.ts-58-155 (1)
58-155:⚠️ Potential issue | 🟠 Major
validateJsonSchema()accepts invalid structured payloads.This validator treats
nullas an"object"and never descends intopropertiesoritems, so schemas like{ type: "object", properties: { ... } }can pass responses that do not actually match the supplied schema.responseValidationProcessorwill then report false positives for structured-output checks.🛠️ Proposed direction
- const actualType = Array.isArray(data) ? "array" : typeof data; + const actualType = + data === null ? "null" : Array.isArray(data) ? "array" : typeof data; ... + if (schema.properties && isJsonObject(data)) { + for (const [key, propertySchema] of Object.entries(schema.properties)) { + errors.push( + ...validateJsonSchema( + data[key], + propertySchema as JsonObject, + ).map((error) => `${key}: ${error}`), + ); + } + } + + if (schema.items && Array.isArray(data)) { + data.forEach((item, index) => { + errors.push( + ...validateJsonSchema(item, schema.items as JsonObject).map( + (error) => `[${index}]: ${error}`, + ), + ); + }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/utils/validationUtils.ts` around lines 58 - 155, validateJsonSchema incorrectly treats null as an object and doesn't validate nested schemas (properties/items), allowing structured payloads to slip through; update validateJsonSchema to (1) treat null as its own type (so if schema.type === "object" require isJsonObject(data) && data !== null), (2) when schema.type is "object" or schema.properties is present, descend into schema.properties and validate each owned property recursively using validateJsonSchema for property values (use isJsonObject to guard), and (3) when schema.type is "array" or schema.items is present, validate each element against schema.items recursively; ensure enum/min/max/length checks still apply at the appropriate primitive levels and that you reference the existing function name validateJsonSchema, the schema keys properties and items, and helper isJsonObject when adding these checks.src/lib/neurolink.ts-12674-12744 (1)
12674-12744:⚠️ Potential issue | 🟠 Major
setProcessorsEnabled(false)currently doesn't disable processing.
processInput()andprocessOutput()only check whether a pipeline exists. Once a pipeline has been created, these methods keep executing processors even after the public toggle is set tofalse, so the enable/disable API is ineffective.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 12674 - 12744, processInput and processOutput only check for sdkProcessorPipeline and ignore the sdkProcessorsEnabled flag, so calling setProcessorsEnabled(false) doesn't stop processing; update both processInput and processOutput to short-circuit when this.sdkProcessorsEnabled is false (return the same default ProcessorPipelineResult shape with processorsExecuted: 0 and preserved metadataOverrides) before checking/using this.sdkProcessorPipeline, referencing the methods areProcessorsEnabled, setProcessorsEnabled, processInput, processOutput and the fields sdkProcessorsEnabled and sdkProcessorPipeline to locate the logic to change.src/lib/neurolink.ts-1112-1120 (1)
1112-1120:⚠️ Potential issue | 🟠 MajorFail fast on unknown presets in the constructor.
If
buildFromPreset()returnsundefined, this path still enables processors and creates an empty pipeline. A typo in a safety preset would silently disable the intended guards. Please validate the preset here the same waysetProcessorPreset()does.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 1112 - 1120, The constructor currently calls processorDefaultRegistry.buildFromPreset(config.processors.preset) and always assigns this.sdkProcessorPipeline = new ProcessorPipeline(...), which allows unknown presets to silently produce an empty pipeline; replicate the validation used in setProcessorPreset(): check the return value of processorDefaultRegistry.buildFromPreset(config.processors.preset) and if it is undefined or falsy, throw a descriptive error (or call the same validation helper used by setProcessorPreset()) instead of creating a ProcessorPipeline, otherwise proceed to construct this.sdkProcessorPipeline with built.inputProcessors and built.outputProcessors.src/lib/neurolink.ts-1108-1125 (1)
1108-1125:⚠️ Potential issue | 🟠 MajorProcessor config is not applied to normal
generate()/stream()calls.This class stores the pipeline and exposes manual
processInput()/processOutput()helpers, but the request lifecycle never invokes them. As written,config.processors.enabled/presethas no effect on normal SDK generation or streaming, which makes the new constructor config misleading. Based on learnings,src/lib/neurolink.tsis the main SDK entry point that orchestrates all components.Also applies to: 12682-12744
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 1108 - 1125, The configured processor pipeline isn't being applied because generate() and stream() never invoke the stored pipeline; update the request lifecycle in the Neurolink class so that when sdkProcessorsEnabled is true you run this.sdkProcessorPipeline.processInput(...) (or the existing processInput helper) to transform inputs before sending requests and run this.sdkProcessorPipeline.processOutput(...) (or processOutput helper) on responses/each streamed chunk before returning/streaming to callers; specifically, modify the generate and stream methods to branch on this.sdkProcessorsEnabled / this.sdkProcessorPipeline and call the pipeline before the network call and after receiving the result (including streaming chunks), preserving existing error handling and keeping behavior when no processors are configured.src/lib/neurolink.ts-12619-12621 (1)
12619-12621:⚠️ Potential issue | 🟠 MajorAvoid mutating the global registry from instance methods.
getProcessorRegistry()exposes the singletonprocessorDefaultRegistry, andaddInputProcessor()/addOutputProcessor()mutate it withreplace: true. Registering a processor on oneNeuroLinkinstance can therefore alter presets and processor resolution for every other instance in the same process.Also applies to: 12647-12670
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 12619 - 12621, The method getProcessorRegistry() currently returns the global processorDefaultRegistry, causing instance methods like addInputProcessor() and addOutputProcessor() to mutate this shared singleton, which affects all NeuroLink instances. To fix this, modify NeuroLink to maintain its own instance-specific ProcessorRegistry instead of referencing the global one. Change getProcessorRegistry() to return this instance-specific registry, and update addInputProcessor() and addOutputProcessor() to modify only the instance registry without altering the global singleton.src/lib/processors/input/semanticContextProcessor.ts-235-261 (1)
235-261:⚠️ Potential issue | 🟠 Major
semantic-contextis currently a no-op.
searchSemanticContext()always returns an empty array, so this built-in processor never enriches the request regardless ofvectorStore,embedFunction,topK, ornamespace. Because the processor is exported and registered as part of the feature, users will get silent pass-through behavior instead of semantic context retrieval. Please either wire this to a real/injected search implementation or fail fast when semantic search is configured but unavailable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/input/semanticContextProcessor.ts` around lines 235 - 261, searchSemanticContext currently returns [] regardless of configuration causing silent no-op; update it to detect when semantic search is requested (inspect SemanticContextConfig.vectorStore, embedFunction, topK, namespace) and either call an injected search handler (e.g. a provided config.searchFunction or DI client) to produce SearchResult[] using the embedFunction/default embedding and vector store query, or throw a clear error when a vector store is configured but no search client is available; ensure the function returns mapped SearchResult objects (text, score, metadata) and fails fast if required capabilities are missing.src/lib/processors/pipeline.ts-270-289 (1)
270-289:⚠️ Potential issue | 🟠 Major
stopOnAbortis ignored for normal processor aborts.Both input and output paths return immediately on
result.action === "abort", sostopOnAbort: falseonly changes exception handling. A processor-requested abort can never be bypassed, which makes the setting misleading and prevents the non-short-circuit mode from working as configured.Also applies to: 462-481
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/pipeline.ts` around lines 270 - 289, The pipeline currently short-circuits on any processor-requested abort (result.action === "abort") regardless of the stopOnAbort flag; update the abort handling in the input and output processor blocks (the branches that check result.action === "abort", processorsExecuted, and logger.debug) to honor stopOnAbort: if stopOnAbort is true, return immediately as now, but if stopOnAbort is false, do not return—accumulate result.feedback into feedback, update processorsExecuted and currentData.metadata.issues/metadata as needed, log the abort, and continue processing remaining processors (or set a non-short-circuit aborted marker in currentData.metadata) so the pipeline does not short-circuit while still recording the abort event.src/lib/processors/utils/processorFactory.ts-204-241 (1)
204-241:⚠️ Potential issue | 🟠 MajorExhausted retries can still return
action: "retry".If the wrapped processor keeps returning
retry, the loop exits and returnslastResult, which is still a retry result. That means the caller can keep retrying forever even though this wrapper has already exhaustedmaxRetries. The final exhausted retry needs to be converted into an abort.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/utils/processorFactory.ts` around lines 204 - 241, The loop can exit with lastResult.action === "retry" so ensure exhausted retries produce an abort: when calling processor.process (in the for loop that uses maxRetries, retryDelay and processor.process) if you receive a result with action "retry" and you're at the final attempt (attempt === maxRetries) convert/return an abort result (include processor.id in feedback and an issues array similar to the catch branch); also after the loop, if lastResult exists and lastResult.action === "retry", replace it with a constructed abort object (feedback mentioning processor.id and retries exhausted and an issues entry) before returning.src/lib/processors/pipeline.ts-592-615 (1)
592-615:⚠️ Potential issue | 🟠 MajorProvider/model-gated processors still run when metadata is missing.
checkConditions()only rejectsproviders/modelswhenmetadata.provider/metadata.modelis present. If the caller omits those fields entirely, the condition passes and the processor executes anyway. That defeats provider/model scoping for calls that do not populate metadata up front.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/pipeline.ts` around lines 592 - 615, checkConditions currently lets processors run when conditions.providers or conditions.models are specified but metadata.provider or metadata.model is missing; update checkConditions (in class using ProcessorConditions and ProcessorMetadata) so that if conditions.providers exists the function returns false unless metadata.provider is present and included in conditions.providers, and likewise if conditions.models exists require metadata.model to be present and included in conditions.models; keep the custom condition check (conditions.custom(metadata)) as-is. Ensure you reference the symbols conditions.providers, conditions.models, metadata.provider, metadata.model and the method checkConditions when making the change.test/processors/integration/io-processors.integration.test.ts-1736-1865 (1)
1736-1865:⚠️ Potential issue | 🟠 MajorAvoid hard wall-clock budgets in this integration suite.
These
<50ms/<100ms/<200msassertions will flap under CI noise, slower runners, and transient host contention even when the code is correct. Please make these deterministic (for example, assert algorithmic behavior only, gate them behind a benchmark flag, or relax them enough that normal CI variance does not fail the suite).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/processors/integration/io-processors.integration.test.ts` around lines 1736 - 1865, The test uses fragile hard wall-clock assertions (e.g., expect(...).toBeLessThan(50)) that will flake in CI; update the tests around ProcessorPipeline.processInput/processOutput, createDefaultTripwireEvaluator.evaluate, and ProcessorRegistry lookups to remove strict timing requirements and make them deterministic by either (a) gating these timing assertions behind an explicit benchmark flag (check process.env.RUN_BENCH === "1" and only run the expect(endTime - startTime).toBeLessThan(...) when set), or (b) replace wall-clock checks with algorithmic assertions (e.g., assert result.action/result.totalTime is defined, correct, or that repeated registry.getInputProcessor/getOutputProcessor returns expected processors) and relax or remove the tight thresholds so CI variance won't fail the suite.test/continuous-test-suite-processors.ts-36-42 (1)
36-42: 🛠️ Refactor suggestion | 🟠 MajorUse a type alias instead of an interface here.
This repo forbids
interfacein TypeScript sources/tests. Replaceinterface TestResultwithtype TestResult = { ... }.As per coding guidelines, "Never use
interface. 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-processors.ts` around lines 36 - 42, Replace the declared interface TestResult with a type alias: change "interface TestResult { ... }" to "type TestResult = { name: string; category: string; passed: boolean; duration: number; error?: string }" ensuring the same shape and optional property are preserved and updating any imports/usages if necessary to reference the new type name TestResult.src/lib/processors/utils/processorFactory.ts-75-90 (1)
75-90:⚠️ Potential issue | 🟠 MajorComposed processors drop issues and metadata from inner processors.
These helpers only carry forward
data. Any warnings, metadata, or enrichment emitted by earlier processors disappears before the composed processor returns, which breaks downstream tracing and safety aggregation in the pipeline. Please accumulate and mergeissues/metadataacross the chain, not just the transformed payload.Also applies to: 113-128
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/utils/processorFactory.ts` around lines 75 - 90, The composed processor's async process currently only carries forward result.data and loses issues/metadata from inner processors; update the loop in process to accumulate and merge any result.issues (arrays) and result.metadata (objects) from each processor into aggregate variables (e.g., aggregatedIssues, aggregatedMetadata) while still updating currentData from result.data, and return them on exit along with action and data; apply the same merging logic to the other composed block referenced around lines 113-128 so both composed processor implementations propagate and merge issues/metadata instead of dropping them.test/continuous-test-suite-processors.ts-105-111 (1)
105-111:⚠️ Potential issue | 🟠 MajorRemove pre-quoting of arguments to
spawn().With
shell: false,spawn()treats each array element as a separate argument. Wrapping space-containing args in quotes injects literal"characters intoargv, so inputs likeHello worldare passed incorrectly as"Hello world"instead ofHello world.Suggested fix
- const quotedArgs = args.map((arg) => - arg.includes(" ") ? `"${arg}"` : arg, - ); // Use node directly for faster startup instead of npx const cliPath = path.join(process.cwd(), "dist", "cli", "index.js"); - const proc = spawn("node", [cliPath, ...quotedArgs], { + const proc = spawn("node", [cliPath, ...args], { cwd: process.cwd(), timeout: TEST_CONFIG.timeout, shell: false,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-processors.ts` around lines 105 - 111, The code pre-quotes arguments via quotedArgs before calling spawn, which injects literal double quotes into argv when using spawn(..., { shell: false }); remove the pre-quoting step (delete/migrate usage of quotedArgs) and pass the original args array directly to spawn (i.e., use [cliPath, ...args]) so each space-containing argument is delivered as a single argv element; keep the cliPath and spawn(...) invocation but replace quotedArgs with args (or ensure shell: true if you purposely want shell parsing, but prefer using args as-is).src/cli/commands/processors.ts-873-876 (1)
873-876:⚠️ Potential issue | 🟠 MajorReject array roots in
validateProcessorConfig().
typeof [] === "object", soneurolink processor validatecurrently treats a JSON array as a valid top-level config. Since the result ofJSON.parse()at line 748 is cast toRecord<string, unknown>at line 777, an array root bypasses the structure check and incorrectly reports validation success.Suggested fix
- if (typeof config !== "object" || config === null) { + if (typeof config !== "object" || config === null || Array.isArray(config)) { errors.push("Configuration must be a JSON object"); return { valid: false, errors, warnings }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/processors.ts` around lines 873 - 876, validateProcessorConfig currently treats arrays as valid because typeof [] === "object"; update the initial type check in validateProcessorConfig to also reject arrays by adding an Array.isArray(config) condition (e.g., if (typeof config !== "object" || config === null || Array.isArray(config)) ), push the same "Configuration must be a JSON object" error when an array is provided, and return { valid: false, errors, warnings } as before so top-level array roots are correctly rejected.
🟡 Minor comments (6)
FEATURE-STATUS.md-4-4 (1)
4-4:⚠️ Potential issue | 🟡 MinorStale "Last Updated" date.
The document shows "January 31, 2026" but this PR is from April 2026. Consider updating to the actual date.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@FEATURE-STATUS.md` at line 4, Update the stale "Last Updated" metadata string by replacing the literal "**Last Updated:** January 31, 2026" with the correct current date (e.g., "**Last Updated:** April XX, 2026") in FEATURE-STATUS.md so the document reflects the PR timing; locate and edit the line containing the "**Last Updated:**" header and update the date value accordingly.docs/tutorials.md-118-118 (1)
118-118:⚠️ Potential issue | 🟡 MinorFix garbled character in heading.
The heading contains a mojibake/garbled character (
�) before the globe emoji. This appears to be an encoding issue that will render incorrectly in documentation.Proposed fix
-## �🌐 Web App Integration +## 🌐 Web App Integration🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/tutorials.md` at line 118, The heading line currently contains a garbled character "�" in the string "## �🌐 Web App Integration"; remove the invalid character and replace the heading with a well-encoded version such as "## 🌐 Web App Integration" (or plain "## Web App Integration" if you prefer to avoid emoji), ensuring the file is saved with UTF-8 encoding so the globe emoji renders correctly; update the heading text in the docs/tutorials.md content where "## �🌐 Web App Integration" appears.src/lib/processors/output/memoryPersistenceProcessor.ts-90-95 (1)
90-95:⚠️ Potential issue | 🟡 MinorRead
providerandmodelfrom the output result.This turn metadata is pulling
provider/modelfromdata.metadata, but the generated response data lives ondata.result. As written, persisted turns will recordundefinedfor both fields.Proposed change
metadata: { ...customMetadata, - provider: data.metadata.provider, - model: data.metadata.model, + provider: data.result.provider, + model: data.result.model, requestId: data.metadata.requestId, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/memoryPersistenceProcessor.ts` around lines 90 - 95, The metadata block in memoryPersistenceProcessor is pulling provider/model from data.metadata but the generated response has those fields on data.result; update the metadata assignment (where provider and model are set) to read provider and model from data.result (e.g., data.result?.metadata?.provider and data.result?.metadata?.model), falling back to data.metadata if absent, while leaving requestId as data.metadata.requestId so persisted turns record the actual provider/model from the result.test/fixtures/processors/pipeline-config.json-373-375 (1)
373-375:⚠️ Potential issue | 🟡 MinorUse the plural CLI command in the fixture example.
This sample says
neurolink processor run, but the new command family isneurolink processors .... Leaving the singular form here will bake the wrong invocation into fixtures/docs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/fixtures/processors/pipeline-config.json` around lines 373 - 375, The fixture's usage example uses the old singular CLI invocation; update the "cli" value under the "usage" object so it uses the new plural command family (change "neurolink processor run --preset security --input 'text'" to "neurolink processors run --preset security --input 'text'") so the example and fixtures reflect the correct command; locate the "usage" object and the "cli" string in the JSON and modify that string accordingly.src/lib/processors/output/contentFilteringProcessor.ts-138-138 (1)
138-138:⚠️ Potential issue | 🟡 MinorPotential TypeError if
data.metadata.customis undefined.Accessing
data.metadata.custom.retryCountwill throw ifcustomis undefined.🛡️ Proposed fix
if (action === "retry") { - const currentRetry = (data.metadata.custom.retryCount as number) || 0; + const currentRetry = (data.metadata.custom?.retryCount as number) || 0; if (currentRetry < maxRetries) {Also apply at line 170:
(action === "retry" && - ((data.metadata.custom.retryCount as number) || 0) >= maxRetries), + ((data.metadata.custom?.retryCount as number) || 0) >= maxRetries),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/output/contentFilteringProcessor.ts` at line 138, The code accesses data.metadata.custom.retryCount which can throw if custom is undefined; change the access to use optional chaining and a safe default (e.g., const currentRetry = (data.metadata.custom?.retryCount as number) || 0) or ensure data.metadata.custom is initialized before reading, and apply the same fix to the other occurrence that reads retryCount later in the file (the second access of data.metadata.custom.retryCount).src/lib/processors/utils/metadataUtils.ts-88-100 (1)
88-100:⚠️ Potential issue | 🟡 MinorComment says "deep copy" but implementation is shallow.
The JSDoc claims this returns a "Deep copy of metadata", but the implementation only creates a shallow copy. Nested objects within
customvalues, or nested properties inissuesitems, will still share references with the original.If callers expect true deep copy for safe mutation, this could lead to subtle bugs.
🔧 Proposed fix options
Option 1: Fix the comment to reflect actual behavior:
/** * Clone metadata for safe mutation * `@param` metadata - Metadata to clone - * `@returns` Deep copy of metadata + * `@returns` Shallow copy of metadata (top-level arrays/objects are cloned) */Option 2: Implement actual deep copy if needed:
export function cloneMetadata(metadata: ProcessorMetadata): ProcessorMetadata { - return { - ...metadata, - custom: { ...metadata.custom }, - issues: [...metadata.issues], - processorTrace: [...metadata.processorTrace], - }; + return JSON.parse(JSON.stringify(metadata)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/processors/utils/metadataUtils.ts` around lines 88 - 100, The JSDoc for cloneMetadata claims a "Deep copy" but the function only shallow-clones top-level fields (custom, issues, processorTrace); update cloneMetadata to produce a true deep clone of ProcessorMetadata (including nested objects inside custom values and nested properties inside issues and processorTrace) by replacing the shallow spreads with a deep-cloning approach (e.g., use structuredClone where available with a safe fallback like JSON.parse(JSON.stringify(...)) or a project deepClone utility) while preserving the ProcessorMetadata shape and types in the returned value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a85a0cd-d91a-4f67-a571-84b38fe5aa11
📒 Files selected for processing (48)
FEATURE-STATUS.mddocs/tutorials.mdsrc/cli/commands/processors.tssrc/cli/loop/optionsSchema.tssrc/cli/parser.tssrc/lib/index.tssrc/lib/neurolink.tssrc/lib/processors/errors/IOProcessorError.tssrc/lib/processors/index.tssrc/lib/processors/input/contentModerationProcessor.tssrc/lib/processors/input/index.tssrc/lib/processors/input/memoryRetrievalProcessor.tssrc/lib/processors/input/messageValidationProcessor.tssrc/lib/processors/input/piiDetectionProcessor.tssrc/lib/processors/input/semanticContextProcessor.tssrc/lib/processors/output/contentFilteringProcessor.tssrc/lib/processors/output/index.tssrc/lib/processors/output/lengthValidationProcessor.tssrc/lib/processors/output/memoryPersistenceProcessor.tssrc/lib/processors/output/responseValidationProcessor.tssrc/lib/processors/output/toxicityCheckProcessor.tssrc/lib/processors/pipeline.tssrc/lib/processors/presets.tssrc/lib/processors/processorHelpers.tssrc/lib/processors/registry.tssrc/lib/processors/tripwire.tssrc/lib/processors/utils/index.tssrc/lib/processors/utils/metadataUtils.tssrc/lib/processors/utils/processorFactory.tssrc/lib/processors/utils/validationUtils.tssrc/lib/types/config.tssrc/lib/types/generate.tssrc/lib/types/index.tssrc/lib/types/ioProcessor.tssrc/lib/types/stream.tstest/continuous-test-suite-processors.tstest/fixtures/processors/input-samples.jsontest/fixtures/processors/output-samples.jsontest/fixtures/processors/pipeline-config.jsontest/processors/IOProcessorError.test.tstest/processors/integration.test.tstest/processors/integration/io-processors.integration.test.tstest/processors/sdk-integration.test.tstest/unit/processors/input/piiDetection.test.tstest/unit/processors/output/toxicityCheck.test.tstest/unit/processors/pipeline.test.tstest/unit/processors/registry.test.tstest/unit/processors/tripwire.test.ts
| evaluate(data: InputProcessorData | OutputProcessorData): TripwireResult { | ||
| for (const tripwire of this.tripwires) { | ||
| try { | ||
| if (tripwire.condition(data)) { | ||
| logger.debug( | ||
| `[TripwireEvaluator] Tripwire triggered: ${tripwire.id}`, | ||
| ); | ||
|
|
||
| // Determine action to return | ||
| const action = tripwire.action === "abort" ? "abort" : "continue"; | ||
|
|
||
| return { | ||
| triggered: true, | ||
| tripwire, | ||
| action, | ||
| feedback: tripwire.message, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Do not stop evaluation on the first warning.
evaluate() returns on the first triggered tripwire even when that tripwire is only a warning, so a warning registered before an abort tripwire will mask the abort and let the caller continue. That breaks the tripwire safety layer. Please keep scanning after warn/continue results and only short-circuit once an aborting tripwire fires.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/processors/tripwire.ts` around lines 104 - 121, evaluate() currently
returns on the first triggered tripwire (from this.tripwires) even when its
action is "continue"/warn, which can mask a later "abort" tripwire; change
evaluate to scan all tripwires, short-circuit immediately only if a tripwire
with action "abort" is encountered, and otherwise accumulate any non-abort
triggers (e.g., collect triggered warnings/messages and the first non-abort
tripwire) and after the loop return a result reflecting either the abort
tripwire (if found), or if no aborts but one or more warnings were triggered
return a combined warning TripwireResult (triggered:true, tripwire = first
warning or aggregated info, action:"continue", feedback = combined messages),
otherwise return the non-triggered default; update the evaluate method and
related TripwireResult construction to support aggregated feedback while keeping
the existing logger.debug calls.
34eeaf6 to
08047cf
Compare
murdore
changed the title
🤖 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 |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
murdore
force-pushed
the
feat/io-processors
branch
from
08047cf to
6b65eb9
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 |
murdore
force-pushed
the
feat/io-processors
branch
from
6b65eb9 to
d2d6d2f
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 |
murdore
force-pushed
the
feat/io-processors
branch
from
d2d6d2f to
c665675
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 |
murdore
force-pushed
the
feat/io-processors
branch
from
c665675 to
c55c8ab
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 |
murdore
force-pushed
the
feat/io-processors
branch
from
c55c8ab to
12aa121
Compare
Rebased onto latest
|
|
@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 |
murdore
force-pushed
the
feat/io-processors
branch
from
12aa121 to
a9617bc
Compare
|
@coderabbitai full review |
|
Only repository collaborators, contributors, or members can run CodeRabbit commands. |
✅ Actions performedFull review triggered. |
|
Only repository collaborators, contributors, or members can run CodeRabbit commands. |
🤖 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: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/neurolink.ts (2)
3925-4013:⚠️ Potential issue | 🟠 MajorValidate the response before publishing success events, and handle retry before finalization.
Line 3925 emits
generation:endand Line 3940 emitsresponse:endbefore Line 3989 validates the response. If validation aborts or truncates, listeners already received a successful raw response. Also,retryOnFailure/maxRetriescannot actually regenerate fromfinalizeGenerateRequestResult()because the provider call has already completed.Move response validation before the end events, emit only validated/truncated content, and implement retry around the
generateTextInternal()call path before finalizing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 3925 - 4013, The current flow emits generation:end, response:end and message (see emitter.emit calls around generation:end/response:end/message) before running response validation and potential retries, which causes listeners to see unvalidated raw content and prevents retrying; fix by moving the response validation block (the validateResponse(...) logic that inspects options.responseValidation and validationResult) to occur immediately after assembling generateResult and before any emitter.emit("generation:end"/"response:end"/"message") or calling finalizeGenerateRequestResult(), and implement retry logic that wraps the provider call path (the generateTextInternal() invocation and/or the code path that produces textResult used to build generateResult) to honor responseValidation.retryOnFailure and maxRetries so that retries happen prior to finalization and only the validated/truncated content is emitted to listeners.
3583-3652:⚠️ Potential issue | 🟠 MajorPersist the redacted prompt, not the raw PII-bearing prompt.
originalPromptis captured before PII redaction and later stored in conversation memory. WithpiiDetection.action: "redact", the provider receives sanitized text, but memory still stores the original PII.Privacy-preserving direction
- const originalPrompt = this._extractOriginalPrompt(optionsOrPrompt); + let originalPrompt = this._extractOriginalPrompt(optionsOrPrompt); ... if (options.piiDetection?.enabled) { + const textBeforePiiProcessing = options.input.text; const piiResult = await detectAndRedactPII(options.input.text, { enabled: true, action: options.piiDetection.action ?? "warn", ... // Replace input text with redacted version options.input.text = piiResult.text; + if ( + (options.piiDetection.action ?? "warn") === "redact" && + piiResult.text !== textBeforePiiProcessing + ) { + originalPrompt = piiResult.text; + } }For stream, capture
originalPromptaftervalidateStreamRequestOptions()mutatesoptions.input.text:- const originalPrompt = options.input.text; options.fileRegistry = this.fileRegistry; await this.validateStreamRequestOptions(options, startTime); + const originalPrompt = options.input.text;Also applies to: 4107-4110, 6368-6451, 7347-7351
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 3583 - 3652, The code currently captures originalPrompt via this._extractOriginalPrompt(optionsOrPrompt) before input validation and PII redaction, so conversation memory ends up storing raw PII; move or reassign originalPrompt after validateStreamRequestOptions() and after the PII redaction block so it captures the sanitized options.input.text (or call this._extractOriginalPrompt(options.input?.text) after redaction). Update the same pattern in the corresponding stream handler (use validateStreamRequestOptions()) and the other noted locations (around the blocks containing PII detection/redaction) so memory/storage always persists the redacted prompt rather than the original raw input.
♻️ Duplicate comments (1)
src/lib/types/generate.ts (1)
515-572: 🛠️ Refactor suggestion | 🟠 MajorSame inline-duplication /
actionoptionality mismatch asStreamOptions.Mirror issue to the one on
src/lib/types/stream.ts(lines 552-600):
- Line 521:
action?: "redact" | "abort" | "warn"is optional here but required inPiiDetectionConfig(ioProcessor.tsline 28).- Lines 550-554: inlined
customValidatorreturn type dropsfield?: stringfromValidationIssue.detectTypes(522-533) re-enumeratesPiiTypeby hand.Prefer importing the canonical types and referencing them directly — then
TextGenerationOptions(lines 1133-1145) can keep itsGenerateOptions["piiDetection"]indexed access without the shape drifting behind it.♻️ Proposed change
-import type { ProcessorPipelineConfig } from "./ioProcessor.js"; +import type { + InputValidationConfig, + PiiDetectionConfig, + ProcessorPipelineConfig, + ResponseValidationConfig, +} from "./ioProcessor.js";- piiDetection?: { - enabled?: boolean; - action?: "redact" | "abort" | "warn"; - detectTypes?: Array< - | "email" - … - | "driversLicense" - >; - customPatterns?: RegExp[]; - allowList?: string[]; - redactionText?: string; - }; - - responseValidation?: { - … - }; - - inputValidation?: { - … - }; + piiDetection?: PiiDetectionConfig; + responseValidation?: ResponseValidationConfig; + inputValidation?: InputValidationConfig;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/generate.ts` around lines 515 - 572, The piiDetection block in TextGeneration-related types duplicates and drifts from canonical types: replace the inline shape with imports of PiiDetectionConfig and PiiType and use them directly (so detectTypes becomes PiiType[] and action matches the requiredness in PiiDetectionConfig), and update the customValidator return type to use the shared ValidationIssue type (preserving optional field?: string) instead of an ad-hoc inline type; also change any related references (e.g., TextGenerationOptions/GenerateOptions["piiDetection"]) to reference the imported canonical types to prevent future drift.
🧹 Nitpick comments (2)
test/continuous-test-suite-safety.ts (1)
21-27: Exercise the new public exports in this safety suite.These imports bypass
src/lib/index.ts, so the tests won’t catch a broken public export even though this PR adds one. Importing from../src/lib/index.jshere, or adding a small public-entrypoint smoke test, would cover the new API surface.Suggested import change
-import { detectAndRedactPII } from "../src/lib/utils/piiDetector.js"; -import { validateResponse } from "../src/lib/utils/responseValidator.js"; import { + detectAndRedactPII, + validateResponse, TripwireEvaluator, createDefaultTripwireEvaluator, commonTripwires, -} from "../src/lib/utils/tripwireEvaluator.js"; +} from "../src/lib/index.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/continuous-test-suite-safety.ts` around lines 21 - 27, The test is importing internal modules directly which skips the package's public entrypoint; update the imports to exercise the public API by importing detectAndRedactPII, validateResponse, TripwireEvaluator, createDefaultTripwireEvaluator, and commonTripwires from the public entrypoint (../src/lib/index.js) instead of their internal paths, or add a tiny smoke test that imports those symbols from the public export to ensure the new public export is exercised.src/lib/types/ioProcessor.ts (1)
14-150: CentralizeInputValidationConfigand consider domain prefixes for type names.Two improvements for this types module:
InputValidationConfigis duplicated inline: The shape{ trimWhitespace?, minLength?, maxLength?, requireContent? }appears identically in bothsrc/lib/types/stream.ts(lines 592–597) andsrc/lib/types/generate.ts(lines 562–567). Extracting this as an exported type here would eliminate duplication and mirrorPiiDetectionConfigandResponseValidationConfig, keeping all three safety-related configs in one place.Generic type names lack domain prefixes:
ValidationIssue,DetectedPII, andPiiTypeare generic names without domain prefixes. Per the coding guideline requiring globally unique exported type names insrc/lib/types/(using prefixes likeProcessor*,Stream*, etc. to disambiguate), consider renaming these to better reflect their domain—e.g.,ProcessorValidationIssue,ProcessorDetectedPII,ProcessorPiiType.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/ioProcessor.ts` around lines 14 - 150, Extract a shared exported type InputValidationConfig (shape: trimWhitespace?: boolean; minLength?: number; maxLength?: number; requireContent?: boolean) into this file and replace the duplicated inline shapes in src/lib/types/stream.ts and src/lib/types/generate.ts by importing and using InputValidationConfig; update any references to the inline anonymous shapes to the new type. Also apply domain-prefixed renames for the public types: rename PiiType → ProcessorPiiType, DetectedPII → ProcessorDetectedPII, and ValidationIssue → ProcessorValidationIssue (and update all references in this file and across the codebase), while keeping temporary backwards-compatible exports (e.g., export type PiiType = ProcessorPiiType) if needed to avoid breaking existing imports. Ensure you update PiiDetectionConfig, ResponseValidationConfig, and any other types that reference the renamed symbols to reference the new Processor* names.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/factories/commandFactory.ts`:
- Around line 487-536: commonOptions currently includes safety flags (piiRedact,
piiTypes, piiAction, inputMaxLength, trimWhitespace, requireContent,
outputMaxLength, outputMinLength) which makes commands like executeBatch()
accept them but not apply them; either move these flags out of commonOptions
into the generate/stream-specific option set or ensure every command that
exposes them forwards them to the execution layer. Specifically, remove or stop
exporting the safety flags from commonOptions (or
CLICommandFactory.commonOptions) and instead add CLICommandFactory.safetyOptions
only to createGenerateCommand() and createStreamCommand(); alternatively, if you
prefer global exposure, update executeBatch(), executeProviderStatus(),
executeSetup(), executeLoop(), etc. to accept and pass the corresponding
piiDetection/input validation/response validation fields through to the same
place executeGenerate() and executeRealStream() use so the flags are honored.
- Around line 2706-2723: The code currently casts user-provided argv.piiAction
and argv.piiTypes directly into expected enums (action/detectTypes) which allows
invalid tokens through; update commandFactory.ts to validate argv.piiAction
against the allowed set {"redact","abort","warn"} and validate each token from
argv.piiTypes against the supported PII type set before constructing
piiDetection (reject or error out on any invalid value), centralize this logic
into a shared parser/validator function (e.g., parsePiiAction and parsePiiTypes
or a single parsePiiDetection helper) and replace both places where casting is
used (the blocks that build action and detectTypes) to call the shared validator
so typos like "delete" or "foo" are rejected before reaching the SDK.
In `@src/lib/neurolink.ts`:
- Around line 6413-6452: The streaming path currently validates input but never
validates streamed output: update the stream() implementation to call
validateResponse() (or the existing responseValidation logic) on the
accumulatedContent before emitting the final "stream:complete" and before
persisting to memory; while streaming, enforce maxLength by truncating yielded
content when necessary and stop/abort if responseValidation returns an abort
rule (ensure validateResponse/responseValidation is invoked after each chunk or
at least on the final accumulated response), apply truncation to
accumulatedContent used for span finalization and memory storage, and ensure any
abort reason from validateResponse is thrown so spans are finalized and no
unvalidated content is stored (refer to function/method names validateResponse,
stream(), accumulatedContent, responseValidation, and the "stream:complete"
emission).
- Around line 3619-3649: Replace bare throws in the guardrail/PII/validation
paths with typed errors created via the project's ErrorFactory (e.g., use
ErrorFactory.createValidationError / createSafetyError) instead of `throw new
Error(...)` for the checks inside the input validation block and the PII block
that calls detectAndRedactPII; wrap the async detectAndRedactPII call with the
withTimeout utility and propagate its typed error rather than raw throws (use
the returned ErrorFactory error when piiResult.action === "abort"); ensure any
provider error formatting uses formatProviderError to produce an Error object
(never throw inside formatProviderError) so SDK callers can reliably detect
validation vs retryable provider failures.
In `@src/lib/types/stream.ts`:
- Around line 552-600: Replace the inline safety config declarations in
StreamOptions with the canonical types from ioProcessor.ts: import and use
PiiDetectionConfig for piiDetection, ResponseValidationConfig for
responseValidation, and (add or import) InputValidationConfig for
inputValidation so the public types stay consistent with runtime; ensure you
stop redeclaring the PII type union, required PiiDetectionConfig.action (not
optional), and the ValidationIssue shape (include optional field?: string) used
by customValidator, leaving processors?: ProcessorPipelineConfig as deprecated.
In `@src/lib/utils/piiDetector.ts`:
- Around line 196-210: The code treats allowList as a list of PII type names to
skip (e.g., "email"), which is misleading; rename the configuration property and
references to make intent explicit: change PiiDetectionConfig.allowList to
disabledTypes (or skipTypes) and update this file to check
disabledTypes.includes(piiType) instead of allowList; update the JSDoc in
PiiDetectionConfig (ioProcessor.ts) and any other usages of allowList
accordingly so the semantics are clear across scanField, PII_PATTERNS, and
detectTypes.
- Around line 99-128: scanField currently mutates shared RegExp.lastIndex and
loops with regex.exec, causing race conditions when PII_PATTERNS is reused
across concurrent detectAndRedactPII calls and risking infinite loops for
non-global custom patterns; fix by validating/forcing the global flag in
validateCustomPattern (reject or clone patterns without 'g') and rewrite
scanField to use String.prototype.matchAll on a cloned RegExp (new
RegExp(pattern.source, pattern.flags.includes('g') ? pattern.flags :
pattern.flags + 'g')) or otherwise ensure a fresh global RegExp per call so no
shared lastIndex is mutated and match iteration cannot hang.
In `@src/lib/utils/responseValidator.ts`:
- Around line 262-371: The truncation branch (when config.truncationAction ===
"truncate") mutates text before later checks so required/forbidden phrase checks
and validateJsonSchema run against the truncated text; change the implementation
to preserve the original responseText for downstream validations by running
phrase checks (config.requiredPhrases, config.forbiddenPhrases) and JSON schema
validation (config.jsonSchema via validateJsonSchema) against responseText (or a
copied original variable) while using text only for length handling and the
returned payload, and add a short comment by the truncation logic
(truncationAction / text / responseText / validateJsonSchema) clarifying that
truncation does not affect phrase/schema checks.
In `@src/lib/utils/tripwireEvaluator.ts`:
- Around line 85-164: In evaluate and evaluateAll, don’t silently swallow errors
from tripwire.condition: catch the error, and if logger.shouldLog("warn") then
logger.warn a short message including the tripwire.id (and the error) so broken
user-provided conditions are visible while preserving behavior; also ensure you
import logger from ./logger.js. Additionally, when computing message (in both
evaluate and evaluateAll), if tripwire.message is undefined fall back to
tripwire.name (or description if you prefer) so triggered results always include
a usable message.
---
Outside diff comments:
In `@src/lib/neurolink.ts`:
- Around line 3925-4013: The current flow emits generation:end, response:end and
message (see emitter.emit calls around generation:end/response:end/message)
before running response validation and potential retries, which causes listeners
to see unvalidated raw content and prevents retrying; fix by moving the response
validation block (the validateResponse(...) logic that inspects
options.responseValidation and validationResult) to occur immediately after
assembling generateResult and before any
emitter.emit("generation:end"/"response:end"/"message") or calling
finalizeGenerateRequestResult(), and implement retry logic that wraps the
provider call path (the generateTextInternal() invocation and/or the code path
that produces textResult used to build generateResult) to honor
responseValidation.retryOnFailure and maxRetries so that retries happen prior to
finalization and only the validated/truncated content is emitted to listeners.
- Around line 3583-3652: The code currently captures originalPrompt via
this._extractOriginalPrompt(optionsOrPrompt) before input validation and PII
redaction, so conversation memory ends up storing raw PII; move or reassign
originalPrompt after validateStreamRequestOptions() and after the PII redaction
block so it captures the sanitized options.input.text (or call
this._extractOriginalPrompt(options.input?.text) after redaction). Update the
same pattern in the corresponding stream handler (use
validateStreamRequestOptions()) and the other noted locations (around the blocks
containing PII detection/redaction) so memory/storage always persists the
redacted prompt rather than the original raw input.
---
Duplicate comments:
In `@src/lib/types/generate.ts`:
- Around line 515-572: The piiDetection block in TextGeneration-related types
duplicates and drifts from canonical types: replace the inline shape with
imports of PiiDetectionConfig and PiiType and use them directly (so detectTypes
becomes PiiType[] and action matches the requiredness in PiiDetectionConfig),
and update the customValidator return type to use the shared ValidationIssue
type (preserving optional field?: string) instead of an ad-hoc inline type; also
change any related references (e.g.,
TextGenerationOptions/GenerateOptions["piiDetection"]) to reference the imported
canonical types to prevent future drift.
---
Nitpick comments:
In `@src/lib/types/ioProcessor.ts`:
- Around line 14-150: Extract a shared exported type InputValidationConfig
(shape: trimWhitespace?: boolean; minLength?: number; maxLength?: number;
requireContent?: boolean) into this file and replace the duplicated inline
shapes in src/lib/types/stream.ts and src/lib/types/generate.ts by importing and
using InputValidationConfig; update any references to the inline anonymous
shapes to the new type. Also apply domain-prefixed renames for the public types:
rename PiiType → ProcessorPiiType, DetectedPII → ProcessorDetectedPII, and
ValidationIssue → ProcessorValidationIssue (and update all references in this
file and across the codebase), while keeping temporary backwards-compatible
exports (e.g., export type PiiType = ProcessorPiiType) if needed to avoid
breaking existing imports. Ensure you update PiiDetectionConfig,
ResponseValidationConfig, and any other types that reference the renamed symbols
to reference the new Processor* names.
In `@test/continuous-test-suite-safety.ts`:
- Around line 21-27: The test is importing internal modules directly which skips
the package's public entrypoint; update the imports to exercise the public API
by importing detectAndRedactPII, validateResponse, TripwireEvaluator,
createDefaultTripwireEvaluator, and commonTripwires from the public entrypoint
(../src/lib/index.js) instead of their internal paths, or add a tiny smoke test
that imports those symbols from the public export to ensure the new public
export is exercised.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 756eeaa7-7f31-4e9c-a4b8-930258de55e2
📒 Files selected for processing (13)
src/cli/factories/commandFactory.tssrc/cli/loop/optionsSchema.tssrc/lib/index.tssrc/lib/neurolink.tssrc/lib/processors/index.tssrc/lib/types/generate.tssrc/lib/types/index.tssrc/lib/types/ioProcessor.tssrc/lib/types/stream.tssrc/lib/utils/piiDetector.tssrc/lib/utils/responseValidator.tssrc/lib/utils/tripwireEvaluator.tstest/continuous-test-suite-safety.ts
| // Safety — PII detection | ||
| piiRedact: { | ||
| type: "boolean" as const, | ||
| description: | ||
| "Enable PII detection and redaction on input before sending to LLM", | ||
| alias: "pii-redact", | ||
| default: false, | ||
| }, | ||
| piiTypes: { | ||
| type: "string" as const, | ||
| description: | ||
| "Comma-separated PII types to detect (email,phone,ssn,creditCard,ipAddress,address,name,dateOfBirth,passport,driversLicense)", | ||
| alias: "pii-types", | ||
| }, | ||
| piiAction: { | ||
| type: "string" as const, | ||
| description: "Action when PII is found: redact, abort, or warn", | ||
| alias: "pii-action", | ||
| default: "redact", | ||
| }, | ||
| // Safety — Input validation | ||
| inputMaxLength: { | ||
| type: "number" as const, | ||
| description: "Maximum input text length (characters)", | ||
| alias: "input-max-length", | ||
| }, | ||
| trimWhitespace: { | ||
| type: "boolean" as const, | ||
| description: "Trim whitespace from input text", | ||
| alias: "trim-whitespace", | ||
| default: false, | ||
| }, | ||
| requireContent: { | ||
| type: "boolean" as const, | ||
| description: "Abort if input text is empty or whitespace", | ||
| alias: "require-content", | ||
| default: false, | ||
| }, | ||
| // Safety — Response validation | ||
| outputMaxLength: { | ||
| type: "number" as const, | ||
| description: | ||
| "Maximum response length (characters). Truncates if exceeded", | ||
| alias: "output-max-length", | ||
| }, | ||
| outputMinLength: { | ||
| type: "number" as const, | ||
| description: "Minimum response length (characters)", | ||
| alias: "output-min-length", | ||
| }, |
There was a problem hiding this comment.
Avoid exposing safety flags on commands that silently ignore them.
Line 487 adds these flags to commonOptions, so batch, provider status, setup, loop, and other commands accept them too. Only executeGenerate() and executeRealStream() forward them; for example, neurolink batch prompts.txt --pii-redact is accepted but executeBatch() does not pass piiDetection, so prompts can be sent unredacted. Move these flags into generate/stream-specific options, or wire them through every command that exposes them.
Suggested direction
+ private static readonly safetyOptions = {
+ // Safety — PII detection
+ piiRedact: { /* existing option */ },
+ piiTypes: { /* existing option */ },
+ piiAction: { /* existing option */ },
+ // Safety — Input validation
+ inputMaxLength: { /* existing option */ },
+ trimWhitespace: { /* existing option */ },
+ requireContent: { /* existing option */ },
+ // Safety — Response validation
+ outputMaxLength: { /* existing option */ },
+ outputMinLength: { /* existing option */ },
+ };
+
private static readonly commonOptions = {
// Core generation options
provider: {
...
},
- // Safety — PII detection
- piiRedact: { ... },
- piiTypes: { ... },
- piiAction: { ... },
- // Safety — Input validation
- inputMaxLength: { ... },
- trimWhitespace: { ... },
- requireContent: { ... },
- // Safety — Response validation
- outputMaxLength: { ... },
- outputMinLength: { ... },
};Then pass CLICommandFactory.safetyOptions as additionalOptions only from createGenerateCommand() and createStreamCommand() unless batch support is intentionally implemented too.
🤖 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 487 - 536, commonOptions
currently includes safety flags (piiRedact, piiTypes, piiAction, inputMaxLength,
trimWhitespace, requireContent, outputMaxLength, outputMinLength) which makes
commands like executeBatch() accept them but not apply them; either move these
flags out of commonOptions into the generate/stream-specific option set or
ensure every command that exposes them forwards them to the execution layer.
Specifically, remove or stop exporting the safety flags from commonOptions (or
CLICommandFactory.commonOptions) and instead add CLICommandFactory.safetyOptions
only to createGenerateCommand() and createStreamCommand(); alternatively, if you
prefer global exposure, update executeBatch(), executeProviderStatus(),
executeSetup(), executeLoop(), etc. to accept and pass the corresponding
piiDetection/input validation/response validation fields through to the same
place executeGenerate() and executeRealStream() use so the flags are honored.
| action: | ||
| (argv.piiAction as "redact" | "abort" | "warn") ?? "redact", | ||
| detectTypes: argv.piiTypes | ||
| ? ((argv.piiTypes as string) | ||
| .split(",") | ||
| .map((t) => t.trim()) as Array< | ||
| | "email" | ||
| | "phone" | ||
| | "ssn" | ||
| | "creditCard" | ||
| | "ipAddress" | ||
| | "address" | ||
| | "name" | ||
| | "dateOfBirth" | ||
| | "passport" | ||
| | "driversLicense" | ||
| >) | ||
| : undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify PII CLI parsing paths no longer rely on raw casts.
# Expected after the fix: no direct `argv.piiAction as ...` or split+cast of `argv.piiTypes`
# without validation against the supported action/type sets.
rg -n -C4 'piiAction|piiTypes|detectTypes' --type tsRepository: juspay/neurolink
Length of output: 12604
Validate PII action and type tokens before casting.
The current casts let typos like --pii-action delete or --pii-types email,foo reach the SDK as untrusted values. Reject invalid values against {"redact", "abort", "warn"} and the supported PII type set before building piiDetection. The same pattern appears twice in commandFactory.ts (lines 2706–2723 and 3008–3024); consider extracting a shared parser function.
Example validation approach
+const SUPPORTED_PII_ACTIONS = new Set(["redact", "abort", "warn"]);
+const SUPPORTED_PII_TYPES = new Set([
+ "email",
+ "phone",
+ "ssn",
+ "creditCard",
+ "ipAddress",
+ "address",
+ "name",
+ "dateOfBirth",
+ "passport",
+ "driversLicense",
+]);
+
+if (argv.piiAction && !SUPPORTED_PII_ACTIONS.has(String(argv.piiAction))) {
+ throw new Error(`Invalid --pii-action: ${argv.piiAction}`);
+}
+
+const parsedPiiTypes = argv.piiTypes
+ ? String(argv.piiTypes)
+ .split(",")
+ .map((t) => t.trim())
+ .filter(Boolean)
+ : undefined;
+
+const invalidPiiTypes = parsedPiiTypes?.filter(
+ (type) => !SUPPORTED_PII_TYPES.has(type),
+);
+if (invalidPiiTypes?.length) {
+ throw new Error(`Invalid --pii-types: ${invalidPiiTypes.join(", ")}`);
+}🤖 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 2706 - 2723, The code
currently casts user-provided argv.piiAction and argv.piiTypes directly into
expected enums (action/detectTypes) which allows invalid tokens through; update
commandFactory.ts to validate argv.piiAction against the allowed set
{"redact","abort","warn"} and validate each token from argv.piiTypes against the
supported PII type set before constructing piiDetection (reject or error out on
any invalid value), centralize this logic into a shared parser/validator
function (e.g., parsePiiAction and parsePiiTypes or a single parsePiiDetection
helper) and replace both places where casting is used (the blocks that build
action and detectTypes) to call the shared validator so typos like "delete" or
"foo" are rejected before reaching the SDK.
| if (iv.requireContent && !options.input.text.trim()) { | ||
| throw new Error( | ||
| "Input content is required but was empty or whitespace", | ||
| ); | ||
| } | ||
| if (iv.minLength && options.input.text.length < iv.minLength) { | ||
| throw new Error( | ||
| `Input text is too short (${options.input.text.length} < ${iv.minLength})`, | ||
| ); | ||
| } | ||
| if (iv.maxLength && options.input.text.length > iv.maxLength) { | ||
| throw new Error( | ||
| `Input text is too long (${options.input.text.length} > ${iv.maxLength})`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // PII detection and redaction | ||
| if (options.piiDetection?.enabled) { | ||
| const piiResult = await detectAndRedactPII(options.input.text, { | ||
| enabled: true, | ||
| action: options.piiDetection.action ?? "warn", | ||
| detectTypes: options.piiDetection.detectTypes, | ||
| customPatterns: options.piiDetection.customPatterns, | ||
| allowList: options.piiDetection.allowList, | ||
| redactionText: options.piiDetection.redactionText, | ||
| }); | ||
| if (piiResult.action === "abort") { | ||
| throw new Error( | ||
| piiResult.feedback ?? "Request blocked: PII detected in input", | ||
| ); |
There was a problem hiding this comment.
Return typed validation errors for the new guardrails.
These new validation/PII/response-validation failures currently throw bare Error, so SDK callers cannot reliably distinguish validation failures, PII blocks, and retryable provider failures. Prefer an ErrorFactory validation/safety helper, or add one if none exists. As per coding guidelines, **/*.ts: Error Handling — Use ErrorFactory for typed errors, wrap async calls with withTimeout utility, formatProviderError must return errors never throw.
Also applies to: 4002-4006, 6419-6449
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/neurolink.ts` around lines 3619 - 3649, Replace bare throws in the
guardrail/PII/validation paths with typed errors created via the project's
ErrorFactory (e.g., use ErrorFactory.createValidationError / createSafetyError)
instead of `throw new Error(...)` for the checks inside the input validation
block and the PII block that calls detectAndRedactPII; wrap the async
detectAndRedactPII call with the withTimeout utility and propagate its typed
error rather than raw throws (use the returned ErrorFactory error when
piiResult.action === "abort"); ensure any provider error formatting uses
formatProviderError to produce an Error object (never throw inside
formatProviderError) so SDK callers can reliably detect validation vs retryable
provider failures.
| // Input validation for stream | ||
| if (options.inputValidation && options.input?.text) { | ||
| const iv = options.inputValidation; | ||
| if (iv.trimWhitespace) { | ||
| options.input.text = options.input.text.trim(); | ||
| } | ||
| if (iv.requireContent && !options.input.text.trim()) { | ||
| throw new Error( | ||
| "Input content is required but was empty or whitespace", | ||
| ); | ||
| } | ||
| if (iv.minLength && options.input.text.length < iv.minLength) { | ||
| throw new Error( | ||
| `Input text is too short (${options.input.text.length} < ${iv.minLength})`, | ||
| ); | ||
| } | ||
| if (iv.maxLength && options.input.text.length > iv.maxLength) { | ||
| throw new Error( | ||
| `Input text is too long (${options.input.text.length} > ${iv.maxLength})`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| // PII detection and redaction for stream | ||
| if (options.piiDetection?.enabled && options.input?.text) { | ||
| const piiResult = await detectAndRedactPII(options.input.text, { | ||
| enabled: true, | ||
| action: options.piiDetection.action ?? "warn", | ||
| detectTypes: options.piiDetection.detectTypes, | ||
| customPatterns: options.piiDetection.customPatterns, | ||
| allowList: options.piiDetection.allowList, | ||
| redactionText: options.piiDetection.redactionText, | ||
| }); | ||
| if (piiResult.action === "abort") { | ||
| throw new Error( | ||
| piiResult.feedback ?? "Request blocked: PII detected in input", | ||
| ); | ||
| } | ||
| options.input.text = piiResult.text; | ||
| } |
There was a problem hiding this comment.
Apply output response validation in the streaming path too.
The stream path now has input-side safety, but streamed output never calls validateResponse(): raw chunks are yielded and stream:complete is emitted with unvalidated accumulatedContent. This makes responseValidation a no-op for stream().
For streaming, enforce maxLength/truncation while yielding, and apply abort rules before stream:complete, span finalization, and memory storage.
Also applies to: 6584-6661
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/neurolink.ts` around lines 6413 - 6452, The streaming path currently
validates input but never validates streamed output: update the stream()
implementation to call validateResponse() (or the existing responseValidation
logic) on the accumulatedContent before emitting the final "stream:complete" and
before persisting to memory; while streaming, enforce maxLength by truncating
yielded content when necessary and stop/abort if responseValidation returns an
abort rule (ensure validateResponse/responseValidation is invoked after each
chunk or at least on the final accumulated response), apply truncation to
accumulatedContent used for span finalization and memory storage, and ensure any
abort reason from validateResponse is thrown so spans are finalized and no
unvalidated content is stored (refer to function/method names validateResponse,
stream(), accumulatedContent, responseValidation, and the "stream:complete"
emission).
| /** PII detection — scans and optionally redacts PII from input before the LLM call. */ | ||
| piiDetection?: { | ||
| enabled?: boolean; | ||
| action?: "redact" | "abort" | "warn"; | ||
| detectTypes?: Array< | ||
| | "email" | ||
| | "phone" | ||
| | "ssn" | ||
| | "creditCard" | ||
| | "ipAddress" | ||
| | "address" | ||
| | "name" | ||
| | "dateOfBirth" | ||
| | "passport" | ||
| | "driversLicense" | ||
| >; | ||
| customPatterns?: RegExp[]; | ||
| allowList?: string[]; | ||
| redactionText?: string; | ||
| }; | ||
|
|
||
| /** Response validation — validates accumulated stream content after completion. */ | ||
| responseValidation?: { | ||
| minLength?: number; | ||
| maxLength?: number; | ||
| requiredPhrases?: string[]; | ||
| forbiddenPhrases?: string[]; | ||
| jsonSchema?: Record<string, unknown>; | ||
| customValidator?: (text: string) => { | ||
| category: string; | ||
| severity: "error" | "warning" | "info"; | ||
| message: string; | ||
| } | null; | ||
| truncationAction?: "abort" | "retry" | "truncate" | "warn"; | ||
| truncationSuffix?: string; | ||
| retryOnFailure?: boolean; | ||
| maxRetries?: number; | ||
| }; | ||
|
|
||
| /** Input validation — validates input text before any processing. */ | ||
| inputValidation?: { | ||
| trimWhitespace?: boolean; | ||
| minLength?: number; | ||
| maxLength?: number; | ||
| requireContent?: boolean; | ||
| }; | ||
|
|
||
| /** @deprecated Use `piiDetection`, `responseValidation`, and `inputValidation` instead. */ | ||
| processors?: ProcessorPipelineConfig; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Reference the shared configs in ioProcessor.ts instead of redeclaring them inline.
These three blocks re-declare the shape of the new safety configs rather than referencing PiiDetectionConfig / ResponseValidationConfig (and a future InputValidationConfig) from ./ioProcessor.js, which has already caused drift:
- Line 555:
action?: "redact" | "abort" | "warn"is optional here, butPiiDetectionConfig.actioninioProcessor.ts(line 28) is required. A caller passingpiiDetection: { enabled: true }type-checks againstStreamOptionsbut would be invalid againstPiiDetectionConfig; in the runtime (piiDetector.tsdestructuresactionwith no default) it silently falls through thewarnbranch. - Lines 580-584:
customValidatorreturn type is inlined and drops the optionalfield?: stringpresent inValidationIssue, so callers can't return an issue with a field path that the validator actually supports. PiiTypeis redeclared as an inline string-literal union (lines 556-567), so any future PII type added inioProcessor.tshas to be hand-synced here.
Pulling the canonical types in makes the public surface self-consistent and CLI-side option mapping stays aligned.
♻️ Proposed change
-import type { ProcessorPipelineConfig } from "./ioProcessor.js";
+import type {
+ InputValidationConfig,
+ PiiDetectionConfig,
+ ProcessorPipelineConfig,
+ ResponseValidationConfig,
+} from "./ioProcessor.js";- /** PII detection — scans and optionally redacts PII from input before the LLM call. */
- piiDetection?: {
- enabled?: boolean;
- action?: "redact" | "abort" | "warn";
- detectTypes?: Array<
- | "email"
- …
- | "driversLicense"
- >;
- customPatterns?: RegExp[];
- allowList?: string[];
- redactionText?: string;
- };
-
- /** Response validation — validates accumulated stream content after completion. */
- responseValidation?: {
- minLength?: number;
- …
- maxRetries?: number;
- };
-
- /** Input validation — validates input text before any processing. */
- inputValidation?: {
- trimWhitespace?: boolean;
- minLength?: number;
- maxLength?: number;
- requireContent?: boolean;
- };
+ /** PII detection — scans and optionally redacts PII from input before the LLM call. */
+ piiDetection?: PiiDetectionConfig;
+
+ /** Response validation — validates accumulated stream content after completion. */
+ responseValidation?: ResponseValidationConfig;
+
+ /** Input validation — validates input text before any processing. */
+ inputValidation?: InputValidationConfig;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/types/stream.ts` around lines 552 - 600, Replace the inline safety
config declarations in StreamOptions with the canonical types from
ioProcessor.ts: import and use PiiDetectionConfig for piiDetection,
ResponseValidationConfig for responseValidation, and (add or import)
InputValidationConfig for inputValidation so the public types stay consistent
with runtime; ensure you stop redeclaring the PII type union, required
PiiDetectionConfig.action (not optional), and the ValidationIssue shape (include
optional field?: string) used by customValidator, leaving processors?:
ProcessorPipelineConfig as deprecated.
| function scanField( | ||
| fieldText: string, | ||
| fieldName: string, | ||
| type: PiiType | "custom", | ||
| pattern: RegExp, | ||
| ): DetectedPII[] { | ||
| // Reset lastIndex to avoid cross-call state leaking between scans. | ||
| // We mutate the pattern's lastIndex directly rather than constructing a new | ||
| // RegExp from .source/.flags (which CodeQL flags as regex injection). | ||
| pattern.lastIndex = 0; | ||
| const regex = pattern; | ||
| const results: DetectedPII[] = []; | ||
|
|
||
| let match = regex.exec(fieldText); | ||
| while (match !== null) { | ||
| results.push({ | ||
| type, | ||
| value: match[0], | ||
| position: { start: match.index, end: match.index + match[0].length }, | ||
| field: fieldName, | ||
| }); | ||
| // Guard against zero-width matches causing infinite loops | ||
| if (match[0].length === 0) { | ||
| regex.lastIndex++; | ||
| } | ||
| match = regex.exec(fieldText); | ||
| } | ||
|
|
||
| return results; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm PII_PATTERNS are module-level singletons and scanField is the only
# consumer reaching into their lastIndex.
rg -nP --type=ts -C2 '\b(PII_PATTERNS|scanField)\b' src/lib/utils/piiDetector.ts
# Look for any callers that could issue detectAndRedactPII concurrently.
rg -nP --type=ts -C2 '\bdetectAndRedactPII\s*\(' srcRepository: juspay/neurolink
Length of output: 1973
🏁 Script executed:
# Get the full validateCustomPattern function and surrounding context
sed -n '81,89p' src/lib/utils/piiDetector.ts
# Get the full detectAndRedactPII function to check async/concurrency
sed -n '169,230p' src/lib/utils/piiDetector.tsRepository: juspay/neurolink
Length of output: 2067
🏁 Script executed:
# Get the full scanField function
sed -n '99,128p' src/lib/utils/piiDetector.ts
# Check for concurrent usage patterns - look for Promise.all, concurrent awaits
rg -nP --type=ts 'Promise\.all|Promise\.allSettled' src/lib/neurolink.ts | head -20
# Also check the exact signatures and usage in neurolink.ts around those detectAndRedactPII calls
sed -n '3630,3650p' src/lib/neurolink.ts
sed -n '6430,6450p' src/lib/neurolink.tsRepository: juspay/neurolink
Length of output: 2736
🏁 Script executed:
# Check if there are other places PII_PATTERNS is accessed or if scanField is the only consumer
rg -nP --type=ts '\bPII_PATTERNS\b' src/lib/utils/piiDetector.ts
# Verify that matchAll would work with the existing validated patterns
# Check TypeScript target version to ensure matchAll is supported
grep -i "target\|lib" tsconfig.json | head -20
# Check if there are any integration tests showing concurrent usage
fd -e test.ts -e spec.ts src/lib/utils | head -5Repository: juspay/neurolink
Length of output: 592
🏁 Script executed:
# Check the actual TypeScript target
cat tsconfig.json | grep -A5 -B5 '"target"'
# Verify the concurrency risk by checking how the SDK is meant to be used
# Look at the main entry points and if they can handle concurrent requests
grep -nP 'export.*function|export.*class' src/lib/neurolink.ts | head -20
# Check if detectAndRedactPII is exported or publicly used
rg -nP 'export.*detectAndRedactPII' srcRepository: juspay/neurolink
Length of output: 246
🏁 Script executed:
# Read the full tsconfig to check target
cat tsconfig.jsonRepository: juspay/neurolink
Length of output: 736
🏁 Script executed:
# Verify the infinite loop risk by checking what happens with non-global patterns
# Create a simple test to confirm the behavior
python3 << 'EOF'
import re
# Simulate the current scanField logic with a non-global pattern
pattern = re.compile(r'\bSECRET\b') # No 'g' flag equivalent
fieldText = "SECRET found here and ANOTHER SECRET"
# In JavaScript, regex.exec() with non-global patterns always starts at index 0
# Let's check what the equivalent would be
print("Non-global pattern test:")
print(f"Pattern: {pattern.pattern}")
print(f"Flags: {pattern.flags if hasattr(pattern, 'flags') else 'N/A (Python regex)'}")
print()
# In JS, this would infinite loop:
# let match = regex.exec(fieldText); // Returns match at index 0
# while (match !== null) { // Always true if pattern doesn't have /g
# ...
# match = regex.exec(fieldText); // Returns same match at index 0 again
# }
print("In JavaScript, exec() on /pattern/ (no /g/) always returns first match:")
print("- lastIndex is ignored for non-global patterns")
print("- exec() always returns match at position 0")
print("- while(match !== null) never exits → infinite loop ✓")
EOF
# Also verify matchAll requirement
python3 << 'EOF'
print("\nString.prototype.matchAll behavior:")
print("- Requires the global flag (/g)")
print("- Throws TypeError if pattern lacks /g/")
print("- Returns iterator (no state mutation across calls)")
print("✓ Solves both concurrency and non-global pattern issues")
EOFRepository: juspay/neurolink
Length of output: 549
Critical: Concurrency race on shared lastIndex and infinite-loop risk with non-global custom patterns.
The code has two real runtime bugs:
-
Concurrency race:
PII_PATTERNS(line 22) is a module-level singleton. When two concurrent SDK requests calldetectAndRedactPII(which is exported and expected to handle multi-client scenarios), both mutatepattern.lastIndexon the sameRegExpinstances. The per-call reset at line 108 cannot prevent races; the other request can advancelastIndexmid-iteration, causing missed or duplicated matches. -
Infinite loop with non-global patterns:
validateCustomPattern(lines 81-89) does not check for thegflag. If a consumer passescustomPatterns: [/\bSECRET\b/](nog), thenregex.exec(fieldText)always returns the first match withlastIndexstuck at0, so thewhile (match !== null)loop at lines 112–125 never terminates. Thetry/catchat line 215 cannot save it because nothing throws.
Both are fixed by using String.prototype.matchAll, which requires g and never mutates the shared regex:
Suggested fix
function validateCustomPattern(pattern: unknown): string | null {
if (!(pattern instanceof RegExp)) {
return "Custom pattern is not a RegExp instance";
}
+ if (!pattern.global) {
+ return `Custom pattern /${pattern.source}/${pattern.flags} must have the 'g' flag`;
+ }
if (hasCatastrophicBacktracking(pattern)) {
return `Custom pattern /${pattern.source}/ contains nested quantifiers that may cause catastrophic backtracking`;
}
return null;
}function scanField(
fieldText: string,
fieldName: string,
type: PiiType | "custom",
pattern: RegExp,
): DetectedPII[] {
- pattern.lastIndex = 0;
- const regex = pattern;
const results: DetectedPII[] = [];
- let match = regex.exec(fieldText);
- while (match !== null) {
+ for (const match of fieldText.matchAll(pattern)) {
results.push({
type,
value: match[0],
position: { start: match.index, end: match.index + match[0].length },
field: fieldName,
});
- if (match[0].length === 0) {
- regex.lastIndex++;
- }
- match = regex.exec(fieldText);
}
return results;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils/piiDetector.ts` around lines 99 - 128, scanField currently
mutates shared RegExp.lastIndex and loops with regex.exec, causing race
conditions when PII_PATTERNS is reused across concurrent detectAndRedactPII
calls and risking infinite loops for non-global custom patterns; fix by
validating/forcing the global flag in validateCustomPattern (reject or clone
patterns without 'g') and rewrite scanField to use String.prototype.matchAll on
a cloned RegExp (new RegExp(pattern.source, pattern.flags.includes('g') ?
pattern.flags : pattern.flags + 'g')) or otherwise ensure a fresh global RegExp
per call so no shared lastIndex is mutated and match iteration cannot hang.
| const allDetected: DetectedPII[] = []; | ||
|
|
||
| // Scan built-in PII types (Bug C3: scan the field in isolation) | ||
| for (const piiType of detectTypes) { | ||
| if (allowList.includes(piiType)) { | ||
| continue; | ||
| } | ||
| const pattern = PII_PATTERNS[piiType]; | ||
| if (!pattern) { | ||
| continue; | ||
| } | ||
|
|
||
| const hits = scanField(text, "text", piiType, pattern); | ||
| allDetected.push(...hits); | ||
| } |
There was a problem hiding this comment.
allowList semantics are surprising — document or rename.
At line 200, allowList.includes(piiType) treats each entry as a PII type name (e.g. "email", "phone") to skip entirely, not as literal values to leave unredacted. That's the opposite of what "allowList" usually means in a PII context (e.g., "don't redact support@company.com"), and nothing in the PiiDetectionConfig JSDoc in ioProcessor.ts disambiguates.
If the current behavior is intentional, rename the field (skipTypes / disabledTypes) or at least add a JSDoc on PiiDetectionConfig.allowList clarifying that values must be PiiType strings, not literal text. Otherwise, adding value-level allow-listing is the more common and useful behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils/piiDetector.ts` around lines 196 - 210, The code treats
allowList as a list of PII type names to skip (e.g., "email"), which is
misleading; rename the configuration property and references to make intent
explicit: change PiiDetectionConfig.allowList to disabledTypes (or skipTypes)
and update this file to check disabledTypes.includes(piiType) instead of
allowList; update the JSDoc in PiiDetectionConfig (ioProcessor.ts) and any other
usages of allowList accordingly so the semantics are clear across scanField,
PII_PATTERNS, and detectTypes.
| // ------------------------------------------------------------------------- | ||
| // 2. Maximum length — applies truncation or records the issue | ||
| // ------------------------------------------------------------------------- | ||
| if (config.maxLength !== undefined && text.length > config.maxLength) { | ||
| const action = config.truncationAction ?? "truncate"; | ||
|
|
||
| switch (action) { | ||
| case "truncate": { | ||
| // Bug C6 fix: ensure keepLength cannot go negative | ||
| const rawSuffix = config.truncationSuffix ?? "..."; | ||
| const suffix = rawSuffix.slice(0, config.maxLength); | ||
| const keepLength = Math.max(config.maxLength - suffix.length, 0); | ||
| text = text.slice(0, keepLength) + suffix; | ||
| issues.push({ | ||
| category: "length", | ||
| severity: "info", | ||
| message: `Response was truncated from ${responseText.length} to ${config.maxLength} characters`, | ||
| }); | ||
| if (logger.shouldLog("debug")) { | ||
| logger.debug( | ||
| `[responseValidator] truncated response to ${config.maxLength} chars`, | ||
| ); | ||
| } | ||
| break; | ||
| } | ||
|
|
||
| case "abort": { | ||
| issues.push({ | ||
| category: "length", | ||
| severity: "error", | ||
| message: `Response length ${text.length} exceeds maximum of ${config.maxLength}; aborting`, | ||
| }); | ||
| return { | ||
| text, | ||
| action: "abort", | ||
| issues, | ||
| feedback: `Response exceeded maximum length of ${config.maxLength} characters.`, | ||
| retryCount, | ||
| }; | ||
| } | ||
|
|
||
| case "retry": { | ||
| issues.push({ | ||
| category: "length", | ||
| severity: "error", | ||
| message: `Response length ${text.length} exceeds maximum of ${config.maxLength}; requesting retry`, | ||
| }); | ||
| return { | ||
| text, | ||
| action: "retry", | ||
| issues, | ||
| feedback: "Response exceeded max length, please shorten", | ||
| retryCount, | ||
| }; | ||
| } | ||
|
|
||
| case "warn": | ||
| default: { | ||
| issues.push({ | ||
| category: "length", | ||
| severity: "warning", | ||
| message: `Response length ${text.length} exceeds maximum of ${config.maxLength} (warn-only)`, | ||
| }); | ||
| logger.warn( | ||
| `[responseValidator] response length ${text.length} exceeds maxLength ${config.maxLength}`, | ||
| ); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // 3. Required phrases | ||
| // ------------------------------------------------------------------------- | ||
| if (config.requiredPhrases && config.requiredPhrases.length > 0) { | ||
| const lowerText = text.toLowerCase(); | ||
| for (const phrase of config.requiredPhrases) { | ||
| if (!lowerText.includes(phrase.toLowerCase())) { | ||
| issues.push({ | ||
| category: "phrase", | ||
| severity: "error", | ||
| message: `Required phrase "${phrase}" was not found in the response`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // 4. Forbidden phrases | ||
| // ------------------------------------------------------------------------- | ||
| if (config.forbiddenPhrases && config.forbiddenPhrases.length > 0) { | ||
| const lowerText = text.toLowerCase(); | ||
| for (const phrase of config.forbiddenPhrases) { | ||
| if (lowerText.includes(phrase.toLowerCase())) { | ||
| issues.push({ | ||
| category: "phrase", | ||
| severity: "error", | ||
| message: `Forbidden phrase "${phrase}" was found in the response`, | ||
| }); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------------- | ||
| // 5. JSON schema | ||
| // ------------------------------------------------------------------------- | ||
| if (config.jsonSchema) { | ||
| const schemaIssues = validateJsonSchema(text, config.jsonSchema); | ||
| issues.push(...schemaIssues); | ||
| } |
There was a problem hiding this comment.
Required/forbidden phrase and JSON-schema checks run against the post-truncation text.
When truncationAction === "truncate" (the default) at lines 269-286, text is replaced with the truncated string before the downstream checks. That leads to surprising outcomes:
- A required phrase that legitimately existed past
maxLengthgets reported as missing →"error"issue → request aborts or retries needlessly. - A forbidden phrase sitting past
maxLengthsilently disappears from the check, so the guardrail misses it. jsonSchemavalidation on truncated text almost always fails with"Response is not valid JSON", even if the untruncated response was valid JSON.
Consider running the phrase / JSON-schema checks against responseText (the original), and only use text for the length/return payload. At minimum, the behavior deserves a comment so callers know to avoid combining truncate with requiredPhrases / jsonSchema.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils/responseValidator.ts` around lines 262 - 371, The truncation
branch (when config.truncationAction === "truncate") mutates text before later
checks so required/forbidden phrase checks and validateJsonSchema run against
the truncated text; change the implementation to preserve the original
responseText for downstream validations by running phrase checks
(config.requiredPhrases, config.forbiddenPhrases) and JSON schema validation
(config.jsonSchema via validateJsonSchema) against responseText (or a copied
original variable) while using text only for length handling and the returned
payload, and add a short comment by the truncation logic (truncationAction /
text / responseText / validateJsonSchema) clarifying that truncation does not
affect phrase/schema checks.
| for (const tripwire of this.tripwires) { | ||
| let triggered: boolean; | ||
| try { | ||
| triggered = tripwire.condition(data); | ||
| } catch { | ||
| // Skip tripwires whose condition throws — don't let evaluation errors | ||
| // block normal operation. | ||
| continue; | ||
| } | ||
|
|
||
| if (!triggered) { | ||
| continue; | ||
| } | ||
|
|
||
| const message = | ||
| typeof tripwire.message === "function" | ||
| ? tripwire.message(data) | ||
| : tripwire.message; | ||
|
|
||
| const result: TripwireResult = { | ||
| triggered: true, | ||
| tripwire, | ||
| message, | ||
| action: tripwire.action, | ||
| }; | ||
|
|
||
| if (tripwire.action === "abort" && firstAbort === null) { | ||
| firstAbort = result; | ||
| } else if (tripwire.action === "warn" && firstWarn === null) { | ||
| firstWarn = result; | ||
| } else if (tripwire.action === "log" && firstLog === null) { | ||
| firstLog = result; | ||
| } | ||
| } | ||
|
|
||
| if (firstAbort !== null) { | ||
| return firstAbort; | ||
| } | ||
| if (firstWarn !== null) { | ||
| return firstWarn; | ||
| } | ||
| if (firstLog !== null) { | ||
| return firstLog; | ||
| } | ||
| return { triggered: false }; | ||
| } | ||
|
|
||
| /** | ||
| * Evaluate all tripwires and return every triggered result. | ||
| */ | ||
| evaluateAll(data: TripwireData): TripwireResult[] { | ||
| const results: TripwireResult[] = []; | ||
|
|
||
| for (const tripwire of this.tripwires) { | ||
| let triggered: boolean; | ||
| try { | ||
| triggered = tripwire.condition(data); | ||
| } catch { | ||
| continue; | ||
| } | ||
|
|
||
| if (!triggered) { | ||
| continue; | ||
| } | ||
|
|
||
| const message = | ||
| typeof tripwire.message === "function" | ||
| ? tripwire.message(data) | ||
| : tripwire.message; | ||
|
|
||
| results.push({ | ||
| triggered: true, | ||
| tripwire, | ||
| message, | ||
| action: tripwire.action, | ||
| }); | ||
| } | ||
|
|
||
| return results; | ||
| } |
There was a problem hiding this comment.
Log swallowed tripwire-condition errors, and fall back to tripwire.name for the message.
Two small gaps in both evaluate and evaluateAll:
- Lines 89-93 and 142-144 swallow any error thrown by
tripwire.condition()with no log. A buggy user-supplied condition is then invisible — the tripwire just never triggers, with no signal that anything is wrong. Alogger.warn(wrapped withlogger.shouldLog("warn")if you want to stay consistent with the rest of the codebase) including the tripwireidwould make this debuggable without altering behavior. - Lines 99-102 / 150-153 leave
messageasundefinedwhentripwire.messageisn't provided. SinceTripwireConfig.nameis required, falling back toname(ordescription) gives callers a usable string without forcing every custom tripwire to duplicate it.
♻️ Proposed change
- let triggered: boolean;
- try {
- triggered = tripwire.condition(data);
- } catch {
- // Skip tripwires whose condition throws — don't let evaluation errors
- // block normal operation.
- continue;
- }
+ let triggered: boolean;
+ try {
+ triggered = tripwire.condition(data);
+ } catch (err) {
+ if (logger.shouldLog("warn")) {
+ logger.warn(
+ `tripwire ${tripwire.id} threw during condition check`,
+ err,
+ );
+ }
+ continue;
+ }
@@
- const message =
- typeof tripwire.message === "function"
- ? tripwire.message(data)
- : tripwire.message;
+ const message =
+ typeof tripwire.message === "function"
+ ? tripwire.message(data)
+ : (tripwire.message ?? tripwire.name);(and import logger from ./logger.js.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/utils/tripwireEvaluator.ts` around lines 85 - 164, In evaluate and
evaluateAll, don’t silently swallow errors from tripwire.condition: catch the
error, and if logger.shouldLog("warn") then logger.warn a short message
including the tripwire.id (and the error) so broken user-provided conditions are
visible while preserving behavior; also ensure you import logger from
./logger.js. Additionally, when computing message (in both evaluate and
evaluateAll), if tripwire.message is undefined fall back to tripwire.name (or
description if you prefer) so triggered results always include a usable message.
…erate/stream
Integrate PII detection, response validation, input validation, and tripwire
evaluation directly into generate() and stream() as first-class options instead
of a separate I/O processor pipeline.
New SDK options:
- piiDetection: { enabled, action, detectTypes, customPatterns, allowList }
- responseValidation: { minLength, maxLength, truncationAction, jsonSchema, retryOnFailure }
- inputValidation: { trimWhitespace, minLength, maxLength, requireContent }
New CLI flags:
- --pii-redact, --pii-types, --pii-action
- --output-max-length, --output-min-length
- --input-max-length, --trim-whitespace, --require-content
New standalone utilities (src/lib/utils/):
- piiDetector.ts — 10 built-in PII regex patterns, per-field detection, safe custom regex handling
- responseValidator.ts — length, phrases, JSON schema, truncation, retry-with-feedback
- tripwireEvaluator.ts — 7 runtime health checks (empty response, max tokens, repetition, latency)
Removed the parallel I/O processor pipeline system (pipeline, registry, presets,
10 processors — 4 stubs, 3 duplicates of existing subsystems, 1 low-quality heuristic).
Kept only the 2 processors that added genuine new capability as extracted utilities.
murdore
force-pushed
the
feat/io-processors
branch
from
a9617bc to
b3dec87
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/neurolink.ts (2)
4215-4230:⚠️ Potential issue | 🟠 MajorValidate the final response before success-side effects, and handle
retry.At Line 4215 this method emits
generation:end/response:endwith the raw provider output before Line 4277 validates it. If validation aborts, observers still see a successful completion with unvalidated content. Also,validateResponse()can returnaction: "retry", but this branch currently treats that as success. Move validation ahead of the emits and mapretryto an actual retry/failure path.Also applies to: 4277-4303
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 4215 - 4230, Move the post-processing validation ahead of the success emits: call validateResponse(...) on textResult before emitting "generation:end" and "response:end" so observers only see validated outputs; if validateResponse(...) returns action: "retry" treat it as a retry/failure (e.g., throw/return a retry signal or trigger the existing retry path) instead of proceeding to success emits. Update the block around the emitter.emit("generation:end", ...) and emitter.emit("response:end", ...) to perform validation first using the same inputs (textResult, options, textOptions), map the "retry" branch to your retry mechanism, and only emit generation:end/response:end when validation indicates success. Ensure the pipelineAHandled flag is preserved when emitting after validation.
3867-3873:⚠️ Potential issue | 🔴 CriticalDo not persist the pre-redaction prompt.
originalPromptis captured before Line 3925 / Line 7099 sanitizeoptions.input.text, but Line 4403 and Line 8157 later store that pre-redaction value into memory. WithpiiDetection.action === "redact", raw PII still gets written to conversation history/background memory, which defeats the privacy control.Also applies to: 3925-3942, 4403-4405, 7031-7035, 7099-7115, 8154-8158
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 3867 - 3873, originalPrompt is captured before input sanitization so when piiDetection.action === "redact" the raw PII can be persisted; change the flow so the value persisted to memory/background is taken from the sanitized input (i.e., after options.input.text has been sanitized) rather than the pre-redaction originalPrompt returned by this._extractOriginalPrompt. Concretely, stop using the early originalPrompt variable for any memory/storage calls and instead derive the stored prompt from the sanitized options.input.text (or call _extractOriginalPrompt after sanitization), and update all places that write originalPrompt to memory/background (the spots referencing originalPrompt later in this file) to use the post-redaction value. Ensure the same change is applied consistently to the other affected blocks mentioned (around the 3925–3942, 4403–4405, 7031–7035, 7099–7115, 8154–8158 regions).
♻️ Duplicate comments (8)
src/lib/neurolink.ts (2)
3902-3942:⚠️ Potential issue | 🟡 MinorReturn typed errors from the new safety guardrails.
These branches still
throw new Error(...), so callers cannot reliably distinguish input-validation, PII, and response-validation failures from provider/runtime failures. Please route them through the existing typed error path (ErrorFactory/NeuroLinkError) as well.Based on learnings, "Use
ErrorFactoryfor typed errors in the codebase".Also applies to: 4277-4297, 7076-7115
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 3902 - 3942, The input-validation and PII branches currently throw generic Errors (inside the options.inputValidation block and after detectAndRedactPII) which prevents callers from distinguishing guardrail failures; replace those throw new Error(...) usages with the project’s typed error path by using ErrorFactory (or constructing a NeuroLinkError via ErrorFactory.create) and throw that typed error instead, ensuring messages and any metadata (e.g., type: "InputValidation" or "PIIDetected", relevant fields like minLength/maxLength or piiResult.feedback) are preserved; update the same pattern for the other listed regions (around detectAndRedactPII usage and response-validation areas referenced) so all guardrail failures use ErrorFactory/NeuroLinkError rather than plain Error.
7076-7115:⚠️ Potential issue | 🟠 Major
stream()still never appliesresponseValidation.This new block validates only the input. Downstream, streamed chunks are yielded,
stream:completeis emitted, and memory is stored without ever callingvalidateResponse(), somaxLength,truncationAction, andretryOnFailureremain no-ops forstream().Also applies to: 7259-7377, 7531-7538
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/neurolink.ts` around lines 7076 - 7115, The stream() path currently only runs input validation and PII redaction but never calls validateResponse(), so responseValidation options (maxLength, truncationAction, retryOnFailure) are ignored; update the stream() implementation to accumulate streamed chunks into a final response string, then before emitting the final "stream:complete" event and before storing memory call validateResponse(finalText, options.responseValidation) (or the existing validateResponse function signature used elsewhere), and implement the same behaviors used by non-streaming code: enforce maxLength (truncate or throw per truncationAction), honor retryOnFailure by triggering a retry if validation fails and retries remain, and propagate or surface validation errors consistently; reference the stream() function, validateResponse(), and the code locations that emit "stream:complete" and perform memory storage so you insert the validation step there.src/lib/utils/piiDetector.ts (2)
81-89:⚠️ Potential issue | 🟠 MajorMissing global-flag validation risks infinite loop.
If a consumer passes a custom pattern without the
gflag (e.g.,/\bSECRET\b/), theexec()loop inscanFieldwill return the same match infinitely becauselastIndexnever advances for non-global patterns.Add a check for
pattern.global:function validateCustomPattern(pattern: unknown): string | null { if (!(pattern instanceof RegExp)) { return "Custom pattern is not a RegExp instance"; } + if (!pattern.global) { + return `Custom pattern /${pattern.source}/${pattern.flags} must have the 'g' flag`; + } if (hasCatastrophicBacktracking(pattern)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/piiDetector.ts` around lines 81 - 89, validateCustomPattern currently only checks instance type and catastrophic backtracking; add a check that the RegExp has the global flag to prevent infinite loops in scanField's exec() loop. Update validateCustomPattern to return an error string when !pattern.global (e.g., "Custom pattern must include the 'g' flag") and reference validateCustomPattern and scanField so reviewers see that scanField relies on a global RegExp to advance lastIndex.
99-128:⚠️ Potential issue | 🔴 CriticalConcurrency race on shared
RegExp.lastIndex.
PII_PATTERNSis a module-level singleton. When two concurrentdetectAndRedactPIIcalls execute, both mutate the sameRegExp.lastIndexproperties viascanField. The reset at line 108 cannot prevent races — another call can advancelastIndexmid-iteration, causing missed or duplicated matches.The safest fix is to use
String.prototype.matchAll, which:
- Requires the
gflag (also fixing the infinite-loop risk)- Returns an iterator without mutating the regex
Suggested fix
function scanField( fieldText: string, fieldName: string, type: PiiType | "custom", pattern: RegExp, ): DetectedPII[] { - pattern.lastIndex = 0; - const regex = pattern; const results: DetectedPII[] = []; - - let match = regex.exec(fieldText); - while (match !== null) { + for (const match of fieldText.matchAll(pattern)) { results.push({ type, value: match[0], - position: { start: match.index, end: match.index + match[0].length }, + position: { start: match.index!, end: match.index! + match[0].length }, field: fieldName, }); - if (match[0].length === 0) { - regex.lastIndex++; - } - match = regex.exec(fieldText); } - return results; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/piiDetector.ts` around lines 99 - 128, scanField currently mutates and relies on RegExp.lastIndex allowing races; change it to use String.prototype.matchAll instead: ensure the incoming pattern has the global "g" flag (or create a new RegExp from pattern.source + "g" if needed), stop resetting or touching pattern.lastIndex, iterate over fieldText.matchAll(pattern) and build DetectedPII entries from each match (use match[0] and match.index for value and position), and remove the zero-width-match lastIndex++ guard. Update references in scanField and keep PiiType/"custom" and DetectedPII shape unchanged.src/lib/types/stream.ts (1)
574-622: 🛠️ Refactor suggestion | 🟠 MajorInline safety configs drift from canonical types in
ioProcessor.ts.These blocks redeclare the config shapes instead of referencing
PiiDetectionConfig,ResponseValidationConfig, and a futureInputValidationConfigfrom./ioProcessor.js. This causes several inconsistencies:
Line 577:
action?: "redact" | "abort" | "warn"is optional, butPiiDetectionConfig.action(ioProcessor.ts:28) is required. A caller passing{ enabled: true }type-checks here but fails at runtime sincepiiDetector.tsdestructuresactionwith no default.Lines 602-606:
customValidatorreturn type is missingfield?: stringpresent inValidationIssue, so callers can't return field-scoped issues.Lines 578-589:
PiiTypeis redeclared inline; future additions toPiiTypemust be synced manually.Reference the canonical types to keep the public surface consistent:
-import type { ProcessorPipelineConfig } from "./ioProcessor.js"; +import type { + InputValidationConfig, + PiiDetectionConfig, + ProcessorPipelineConfig, + ResponseValidationConfig, +} from "./ioProcessor.js"; ... - piiDetection?: { enabled?: boolean; action?: ... }; + piiDetection?: PiiDetectionConfig; - responseValidation?: { minLength?: ... }; + responseValidation?: ResponseValidationConfig; - inputValidation?: { trimWhitespace?: ... }; + inputValidation?: InputValidationConfig;Note: You'll need to add
InputValidationConfigtoioProcessor.tsfirst.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/stream.ts` around lines 574 - 622, The inline safety config types in stream.ts diverge from the canonical types in ioProcessor.ts—replace the inline declarations with imported types (PiiDetectionConfig, ResponseValidationConfig, and the new InputValidationConfig you must add to ioProcessor.ts) so the public API matches the canonical shapes; ensure piiDetection.action uses the same required/optional signature as PiiDetectionConfig (fix optional vs required mismatch), make the customValidator return type include the optional field?: string to match ValidationIssue, and reuse the shared PiiType (do not redeclare the union) so future changes remain consistent (update the processors?: ProcessorPipelineConfig deprecation comment to still reference the canonical types if needed).src/lib/types/generate.ts (1)
543-600: 🛠️ Refactor suggestion | 🟠 MajorSame type drift issue as
StreamOptions.The inline declarations for
piiDetection,responseValidation, andinputValidationduplicate the shapes fromioProcessor.tswith the same inconsistencies:
actionis optional (should be required perPiiDetectionConfig)customValidatorreturn type missingfield?: stringPiiTypeliteral union manually duplicatedReference the canonical types from
./ioProcessor.jsas suggested forstream.ts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/types/generate.ts` around lines 543 - 600, The inline option shapes for piiDetection, responseValidation, and inputValidation are drifting from the canonical definitions in ioProcessor; replace these inline types by importing and using the canonical types (e.g. PiiDetectionConfig, ResponseValidationConfig, InputValidationConfig or the exact exported names) from ./ioProcessor.ts, ensure PiiDetectionConfig makes action required (not optional), update the customValidator return type to include the optional field?: string, and reuse the exported PiiType literal union instead of duplicating it locally so the signatures for piiDetection, responseValidation, and inputValidation stay consistent with ioProcessor.src/cli/factories/commandFactory.ts (2)
487-536:⚠️ Potential issue | 🟠 MajorScope safety flags to commands that actually apply them.
Line 487 adds safety flags to
commonOptions, so commands likebatch,status,setup, andloopaccept them even when those handlers don’t forward safety config. This creates a false safety signal (e.g., user passes--pii-redactand expects sanitization). Move these options to generate/stream-specific option sets (or wire them through every command that exposes them).Suggested direction
+ private static readonly safetyOptions = { + piiRedact: { /* existing definition */ }, + piiTypes: { /* existing definition */ }, + piiAction: { /* existing definition */ }, + inputMaxLength: { /* existing definition */ }, + trimWhitespace: { /* existing definition */ }, + requireContent: { /* existing definition */ }, + outputMaxLength: { /* existing definition */ }, + outputMinLength: { /* existing definition */ }, + }; private static readonly commonOptions = { // ...core/shared options only - piiRedact: { ... }, - piiTypes: { ... }, - piiAction: { ... }, - inputMaxLength: { ... }, - trimWhitespace: { ... }, - requireContent: { ... }, - outputMaxLength: { ... }, - outputMinLength: { ... }, };- return CLICommandFactory.buildOptions(yargs ...); + return CLICommandFactory.buildOptions(yargs ..., CLICommandFactory.safetyOptions);🤖 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 487 - 536, The safety flags (piiRedact, piiTypes, piiAction, inputMaxLength, trimWhitespace, requireContent, outputMaxLength, outputMinLength) were added to commonOptions causing irrelevant commands (batch, status, setup, loop) to advertise them; move these option definitions out of commonOptions and into the option sets actually used by generation/streaming commands (e.g., generate and stream option objects) or explicitly wire the safety config through each command handler that supports it; update references in commandFactory.ts to remove the safety keys from commonOptions and add them to the specific option objects or handlers so only commands that forward/handle safety behavior accept those flags.
2716-2736:⚠️ Potential issue | 🟠 MajorValidate
--pii-actionand--pii-typesbefore type-casting.Line 2720 / Line 3042 cast raw CLI strings directly, and Line 2721-2735 / Line 3043-3057 cast comma-split tokens without allowlist checks. Invalid values can silently propagate to SDK requests. Add strict runtime parsing/validation once and reuse for both generate and stream paths.
Proposed shared parser pattern
+const ALLOWED_PII_ACTIONS = new Set(["redact", "abort", "warn"] as const); +const ALLOWED_PII_TYPES = new Set([ + "email", + "phone", + "ssn", + "creditCard", + "ipAddress", + "address", + "name", + "dateOfBirth", + "passport", + "driversLicense", +] as const); + +private static parsePiiDetection(argv: Record<string, unknown>) { + if (!argv.piiRedact) return undefined; + + const actionRaw = String(argv.piiAction ?? "redact"); + if (!ALLOWED_PII_ACTIONS.has(actionRaw as never)) { + throw new Error(`Invalid --pii-action: ${actionRaw}`); + } + + const detectTypes = argv.piiTypes + ? String(argv.piiTypes) + .split(",") + .map((t) => t.trim()) + .filter(Boolean) + : undefined; + + const invalid = detectTypes?.filter((t) => !ALLOWED_PII_TYPES.has(t as never)); + if (invalid?.length) { + throw new Error(`Invalid --pii-types: ${invalid.join(", ")}`); + } + + return { + enabled: true, + action: actionRaw, + detectTypes, + }; +}Then use
CLICommandFactory.parsePiiDetection(argv)in bothsdk.generate(...)andsdk.stream(...).Also applies to: 3038-3058
🤖 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 2716 - 2736, The CLI currently casts argv.piiAction and argv.piiTypes directly which can let invalid values pass into SDK calls; implement a single parser method (e.g., CLICommandFactory.parsePiiDetection(argv)) that: validates argv.piiAction is one of "redact" | "abort" | "warn" (default "redact" if missing), splits argv.piiTypes, trims tokens and filters/validates them against the allowlist ["email","phone","ssn","creditCard","ipAddress","address","name","dateOfBirth","passport","driversLicense"], and returns either undefined or a well-typed piiDetection object matching the expected shape; then replace the inline casts where piiDetection is built (references: piiDetection payload, sdk.generate(...), sdk.stream(...), and usages of argv.piiAction / argv.piiTypes) to call CLICommandFactory.parsePiiDetection(argv) so both generate and stream paths reuse the validated result.
🧹 Nitpick comments (1)
src/lib/utils/tripwireEvaluator.ts (1)
211-246: Repetition-loop detection is reasonable but may produce false positives on prose.The 20% duplicate-window threshold is a pragmatic heuristic. For highly structured content (lists, repeated headers), this may over-trigger. Consider exposing the threshold as a configurable parameter in a future iteration if users report false positives.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/utils/tripwireEvaluator.ts` around lines 211 - 246, The hard-coded 0.2 duplicate-window threshold in repetitionLoopTripwire's condition can cause false positives; make this threshold configurable by introducing a named parameter (e.g., duplicateWindowThreshold) and read it in the condition instead of the literal 0.2 so callers can adjust sensitivity; update the repetitionLoopTripwire object to accept the new config (or reference a shared config/env var) and document that condition uses duplicateWindowThreshold along with existing symbols like windowSize, totalWindows, duplicates, and the condition function so tests and callers can override it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/lib/neurolink.ts`:
- Around line 4215-4230: Move the post-processing validation ahead of the
success emits: call validateResponse(...) on textResult before emitting
"generation:end" and "response:end" so observers only see validated outputs; if
validateResponse(...) returns action: "retry" treat it as a retry/failure (e.g.,
throw/return a retry signal or trigger the existing retry path) instead of
proceeding to success emits. Update the block around the
emitter.emit("generation:end", ...) and emitter.emit("response:end", ...) to
perform validation first using the same inputs (textResult, options,
textOptions), map the "retry" branch to your retry mechanism, and only emit
generation:end/response:end when validation indicates success. Ensure the
pipelineAHandled flag is preserved when emitting after validation.
- Around line 3867-3873: originalPrompt is captured before input sanitization so
when piiDetection.action === "redact" the raw PII can be persisted; change the
flow so the value persisted to memory/background is taken from the sanitized
input (i.e., after options.input.text has been sanitized) rather than the
pre-redaction originalPrompt returned by this._extractOriginalPrompt.
Concretely, stop using the early originalPrompt variable for any memory/storage
calls and instead derive the stored prompt from the sanitized options.input.text
(or call _extractOriginalPrompt after sanitization), and update all places that
write originalPrompt to memory/background (the spots referencing originalPrompt
later in this file) to use the post-redaction value. Ensure the same change is
applied consistently to the other affected blocks mentioned (around the
3925–3942, 4403–4405, 7031–7035, 7099–7115, 8154–8158 regions).
---
Duplicate comments:
In `@src/cli/factories/commandFactory.ts`:
- Around line 487-536: The safety flags (piiRedact, piiTypes, piiAction,
inputMaxLength, trimWhitespace, requireContent, outputMaxLength,
outputMinLength) were added to commonOptions causing irrelevant commands (batch,
status, setup, loop) to advertise them; move these option definitions out of
commonOptions and into the option sets actually used by generation/streaming
commands (e.g., generate and stream option objects) or explicitly wire the
safety config through each command handler that supports it; update references
in commandFactory.ts to remove the safety keys from commonOptions and add them
to the specific option objects or handlers so only commands that forward/handle
safety behavior accept those flags.
- Around line 2716-2736: The CLI currently casts argv.piiAction and
argv.piiTypes directly which can let invalid values pass into SDK calls;
implement a single parser method (e.g.,
CLICommandFactory.parsePiiDetection(argv)) that: validates argv.piiAction is one
of "redact" | "abort" | "warn" (default "redact" if missing), splits
argv.piiTypes, trims tokens and filters/validates them against the allowlist
["email","phone","ssn","creditCard","ipAddress","address","name","dateOfBirth","passport","driversLicense"],
and returns either undefined or a well-typed piiDetection object matching the
expected shape; then replace the inline casts where piiDetection is built
(references: piiDetection payload, sdk.generate(...), sdk.stream(...), and
usages of argv.piiAction / argv.piiTypes) to call
CLICommandFactory.parsePiiDetection(argv) so both generate and stream paths
reuse the validated result.
In `@src/lib/neurolink.ts`:
- Around line 3902-3942: The input-validation and PII branches currently throw
generic Errors (inside the options.inputValidation block and after
detectAndRedactPII) which prevents callers from distinguishing guardrail
failures; replace those throw new Error(...) usages with the project’s typed
error path by using ErrorFactory (or constructing a NeuroLinkError via
ErrorFactory.create) and throw that typed error instead, ensuring messages and
any metadata (e.g., type: "InputValidation" or "PIIDetected", relevant fields
like minLength/maxLength or piiResult.feedback) are preserved; update the same
pattern for the other listed regions (around detectAndRedactPII usage and
response-validation areas referenced) so all guardrail failures use
ErrorFactory/NeuroLinkError rather than plain Error.
- Around line 7076-7115: The stream() path currently only runs input validation
and PII redaction but never calls validateResponse(), so responseValidation
options (maxLength, truncationAction, retryOnFailure) are ignored; update the
stream() implementation to accumulate streamed chunks into a final response
string, then before emitting the final "stream:complete" event and before
storing memory call validateResponse(finalText, options.responseValidation) (or
the existing validateResponse function signature used elsewhere), and implement
the same behaviors used by non-streaming code: enforce maxLength (truncate or
throw per truncationAction), honor retryOnFailure by triggering a retry if
validation fails and retries remain, and propagate or surface validation errors
consistently; reference the stream() function, validateResponse(), and the code
locations that emit "stream:complete" and perform memory storage so you insert
the validation step there.
In `@src/lib/types/generate.ts`:
- Around line 543-600: The inline option shapes for piiDetection,
responseValidation, and inputValidation are drifting from the canonical
definitions in ioProcessor; replace these inline types by importing and using
the canonical types (e.g. PiiDetectionConfig, ResponseValidationConfig,
InputValidationConfig or the exact exported names) from ./ioProcessor.ts, ensure
PiiDetectionConfig makes action required (not optional), update the
customValidator return type to include the optional field?: string, and reuse
the exported PiiType literal union instead of duplicating it locally so the
signatures for piiDetection, responseValidation, and inputValidation stay
consistent with ioProcessor.
In `@src/lib/types/stream.ts`:
- Around line 574-622: The inline safety config types in stream.ts diverge from
the canonical types in ioProcessor.ts—replace the inline declarations with
imported types (PiiDetectionConfig, ResponseValidationConfig, and the new
InputValidationConfig you must add to ioProcessor.ts) so the public API matches
the canonical shapes; ensure piiDetection.action uses the same required/optional
signature as PiiDetectionConfig (fix optional vs required mismatch), make the
customValidator return type include the optional field?: string to match
ValidationIssue, and reuse the shared PiiType (do not redeclare the union) so
future changes remain consistent (update the processors?:
ProcessorPipelineConfig deprecation comment to still reference the canonical
types if needed).
In `@src/lib/utils/piiDetector.ts`:
- Around line 81-89: validateCustomPattern currently only checks instance type
and catastrophic backtracking; add a check that the RegExp has the global flag
to prevent infinite loops in scanField's exec() loop. Update
validateCustomPattern to return an error string when !pattern.global (e.g.,
"Custom pattern must include the 'g' flag") and reference validateCustomPattern
and scanField so reviewers see that scanField relies on a global RegExp to
advance lastIndex.
- Around line 99-128: scanField currently mutates and relies on RegExp.lastIndex
allowing races; change it to use String.prototype.matchAll instead: ensure the
incoming pattern has the global "g" flag (or create a new RegExp from
pattern.source + "g" if needed), stop resetting or touching pattern.lastIndex,
iterate over fieldText.matchAll(pattern) and build DetectedPII entries from each
match (use match[0] and match.index for value and position), and remove the
zero-width-match lastIndex++ guard. Update references in scanField and keep
PiiType/"custom" and DetectedPII shape unchanged.
---
Nitpick comments:
In `@src/lib/utils/tripwireEvaluator.ts`:
- Around line 211-246: The hard-coded 0.2 duplicate-window threshold in
repetitionLoopTripwire's condition can cause false positives; make this
threshold configurable by introducing a named parameter (e.g.,
duplicateWindowThreshold) and read it in the condition instead of the literal
0.2 so callers can adjust sensitivity; update the repetitionLoopTripwire object
to accept the new config (or reference a shared config/env var) and document
that condition uses duplicateWindowThreshold along with existing symbols like
windowSize, totalWindows, duplicates, and the condition function so tests and
callers can override it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 18f10346-24d7-487c-957d-00a1859e07f6
📒 Files selected for processing (13)
src/cli/factories/commandFactory.tssrc/cli/loop/optionsSchema.tssrc/lib/index.tssrc/lib/neurolink.tssrc/lib/processors/index.tssrc/lib/types/generate.tssrc/lib/types/index.tssrc/lib/types/ioProcessor.tssrc/lib/types/stream.tssrc/lib/utils/piiDetector.tssrc/lib/utils/responseValidator.tssrc/lib/utils/tripwireEvaluator.tstest/continuous-test-suite-safety.ts
Summary
Restructures the I/O processor feature to integrate directly into generate() and stream() as native options.
What changed
piiDetection: { enabled, action, detectTypes }responseValidation: { maxLength, truncationAction, retryOnFailure }inputValidation: { trimWhitespace, minLength, maxLength, requireContent }--pii-redact,--pii-types,--output-max-length,--input-max-length,--trim-whitespaceWhat was removed
Before vs After
Bug fixes: C1 (tripwire short-circuit), C3 (PII offsets), C5 (null-as-object), C6 (truncation overflow), C10 (regex validation)
Test plan
npx vitest run— 18 tests passpnpm run check— 0 type errorspnpm run lint— 0 errorspnpm run build— publint cleanpnpm test— SDK/CLI generate/stream integrationSummary by CodeRabbit
New Features
Tests