-
Notifications
You must be signed in to change notification settings - Fork 10
Add frontend tests, cypress and more e2e tests and scenarios #24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: 1.x
Are you sure you want to change the base?
Conversation
WalkthroughThis update introduces advanced knowledge management features, including a new document and fragment schema, repositories, analytics, import/export, batch operations, and advanced search/filtering. It expands the API with new endpoints, enhances the service and actions, and adds comprehensive unit and end-to-end tests. UI components and configuration files are also updated for improved testing and integration. Changes
Sequence Diagram(s)Advanced Knowledge Search and Analytics FlowsequenceDiagram
participant User
participant Frontend
participant API (Routes)
participant KnowledgeService
participant Database
User->>Frontend: Submits advanced search/analytics request
Frontend->>API (Routes): Sends POST/GET to /knowledge/search/advanced or /analytics
API (Routes)->>KnowledgeService: Calls advancedSearch()/getAnalytics()
KnowledgeService->>Database: Query documents/fragments with filters/aggregation
Database-->>KnowledgeService: Returns results/analytics data
KnowledgeService-->>API (Routes): Returns structured results/analytics
API (Routes)-->>Frontend: Responds with data
Frontend-->>User: Displays search results/analytics
Batch Operation and Import/Export FlowsequenceDiagram
participant User
participant Frontend
participant API (Routes)
participant KnowledgeService
participant Database
User->>Frontend: Initiates batch add/update/delete or import/export
Frontend->>API (Routes): Sends POST to /knowledge/batch or /import/export
API (Routes)->>KnowledgeService: Calls batchOperation()/importKnowledge()/exportKnowledge()
KnowledgeService->>Database: Performs batch DB operations or queries
Database-->>KnowledgeService: Returns operation results/data
KnowledgeService-->>API (Routes): Returns summary or exported data
API (Routes)-->>Frontend: Responds with operation result/data
Frontend-->>User: Shows operation status or download
Poem
✨ Finishing Touches
🧪 Generate Unit Tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 15
🔭 Outside diff range comments (4)
package.json (1)
74-84:eslintis invoked but not declared indevDependencies.The new
lint:ciscript relies on theeslintbinary, yeteslintis not listed underdevDependencies. CI will fail on a clean install."devDependencies": { + "eslint": "^9.1.0", "tsup": "8.5.0", ... }README.md (1)
618-636:index.tswill not exist in Vite build outputVite emits transpiled JavaScript (e.g.
index.[hash].js), never raw*.tsfiles.
Switching the fallback asset detection from.jsto.tswill break the panel in production because the browser will 404.- if (key.endsWith('.ts') || (value as any).file?.endsWith('.ts')) { + if (key.endsWith('.js') || (value as any).file?.endsWith('.js')) {Likewise update the default
jsFilevar above.src/actions.ts (1)
996-1013: Wrap per-case declarations to silencenoSwitchDeclarationserrorsVariables
headers,rowsetc. are declared directly insidecase 'csv':without braces.
Many runtimes hoist those bindings across cases leading to Biome’s lint errors and potential shadowing bugs.- case 'csv': - const headers = [...] - ... + case 'csv': { + const headers = [...] + ... + return [headers, ...rows].map(r => r.join(',')).join('\n') + }Apply the same block‐scoping to every
casewith its ownconst/let.src/routes.ts (1)
716-721: Serving.tsasapplication/javascriptmay break CSP / loadersIf you really are shipping TypeScript sources, browsers won’t execute them.
But if the files are transpiled your check should match.js, not.ts(same mis-extension as in README).- if (assetPath.endsWith('.ts')) { + if (assetPath.endsWith('.js')) {
🧹 Nitpick comments (40)
.gitignore (1)
5-12: Suggestion: Enhance.gitignorefor complete Cypress artifacts
It currently ignorescypress/screenshotsbut omitscypress/videos, which Cypress generates by default. Also, double-check that ignoring the entiredocsfolder is intentional, as it may hide important project documentation.src/frontend/ui/badge.tsx (1)
20-21: Formatting consistency for JSX markup
This flattens the<span>element into a single line. For readability and consistency across UI components, align with your project's preferred JSX formatting (e.g., multi-line children).src/frontend/ui/button.tsx (1)
22-26: Consistent formatting in component definitions
You’ve added a trailing comma aftertypeand split thebaseClassesstring over two lines. To maintain uniform styling across UI components, standardize how you handle trailing commas and multi-line strings (e.g., match theBadgeformatting conventions).src/frontend/ui/card.tsx (1)
16-22: Optional: adddisplayNamefor easier React-DevTools inspectionTiny DX win – assigning a
displayNamehelps when debugging component trees.export function CardHeader({ children, className = '' }: CardProps) { return <div className={`flex flex-col space-y-1.5 p-6 ${className}`}>{children}</div>; +} + +CardHeader.displayName = 'CardHeader'; export function CardFooter({ children, className = '' }: CardProps) { return <div className={`flex items-center p-6 pt-0 ${className}`}>{children}</div>; +} + +CardFooter.displayName = 'CardFooter'; @@ export function CardDescription({ children, className = '' }: CardProps) { return <p className={`text-sm text-muted-foreground ${className}`}>{children}</p>; } export function CardContent({ children, className = '' }: CardProps) { return <div className={`p-6 pt-0 ${className}`}>{children}</div>; } + +CardDescription.displayName = 'CardDescription'; +CardContent.displayName = 'CardContent';Also applies to: 32-38
src/provider.ts (1)
28-35: Rename themapcallback variable to avoid shadowingThe arrow-function parameter
knowledgeshadows the outerknowledgestring, which is slightly confusing.- firstFiveKnowledgeItems.map((knowledge) => `- ${knowledge.content.text}`).join('\n') + firstFiveKnowledgeItems.map((item) => `- ${item.content.text}`).join('\n')cypress/e2e/simple-test.cy.ts (1)
3-5: Avoid network-flaky external dependencies in E2E testsThe test relies on
https://example.cypress.io, which introduces a network dependency and potential flakiness (DNS issues, offline work, rate-limiting). Prefer hosting a minimal static fixture locally or stubbing the response withcy.intercept()to keep the test 100 % deterministic.src/ctx-embeddings.ts (3)
58-63: Inconsistent quote style within the same literal blockMost prompt strings were migrated to single quotes, but
- PDF: "You are a precision document augmentation tool. Your task is to expand a given PDF text chunk ... + PDF: 'You are a precision document augmentation tool. Your task is to expand a given PDF text chunk ...
292-296: Avoid re-assigning function parameters
minTokensandmaxTokensare parameters and are mutated in-place. That pattern can be error-prone (TS-eslintno-param-reassign) and makes intent less clear.- if (chunkTokens > maxTokens * 0.7) { - maxTokens = Math.ceil(chunkTokens * 1.3); - minTokens = chunkTokens; - } + if (chunkTokens > maxTokens * 0.7) { + const adjustedMax = Math.ceil(chunkTokens * 1.3); + const adjustedMin = chunkTokens; + maxTokens = adjustedMax; + minTokens = adjustedMin; + }(or assign to local
letvariables and keep parameters immutable).
345-350:contentType.includes()checks are case-sensitiveMIME strings can arrive in various casings (
application/PDF,text/Markdown). Convert once to lower-case to avoid false negatives:- if (contentType) { - if ( - contentType.includes('javascript') || + if (contentType) { + const type = contentType.toLowerCase(); + if ( + type.includes('javascript') ||Repeat for the other branches.
cypress.config.ts (1)
5-16: Hard-codedbaseUrland disabledchromeWebSecurity
- Hard-coding
http://localhost:3000ties the config to a single port. Make it configurable viaCYPRESS_BASE_URLorprocess.env.PORTso parallel CI jobs can run without port collisions.chromeWebSecurity: falseis powerful but disables same-origin protections. Double-check that it’s strictly required; otherwise leave it on for better parity with production.package.json (1)
81-82:cleanscript is not cross-platform.Using
rm -rfbreaks on Windows runners. Preferrimraf, already a transitive dep of many toolchains.- "clean": "rm -rf dist .turbo node_modules .turbo-tsconfig.json tsconfig.tsbuildinfo", + "clean": "rimraf dist .turbo node_modules .turbo-tsconfig.json tsconfig.tsbuildinfo",src/__tests__/unit/fragment-repository.test.ts (1)
226-234: Large 1536-element arrays slow the test suite.Cosine-similarity logic does not require production-sized embeddings. Switching to a shorter vector (e.g. length = 4) speeds execution while preserving behaviour.
No functional change, purely test-performance.
cypress/support/e2e.ts (2)
51-52: Remove useless empty export.The file already imports
./commands; the extraexport {}is redundant and flagged by Biome.-// Prevent TypeScript errors -export {};
55-59: Blindly suppressing all uncaught exceptions may hide real bugs.Returning
falsefor everyuncaught:exceptionevent prevents Cypress from signalling legitimate application errors. Consider filtering only the known React-dev warnings instead of blanket suppression.Cypress.on('uncaught:exception', (err) => { if (err.message.includes('React')) return false; // ignore React warnings });src/__tests__/e2e/attachment-handling.test.ts (1)
32-34: Avoidanycast when calling service methods.Casting
servicetoanybypasses type-safety and may mask interface drift. Import the concreteKnowledgeServicetype or extend the service retrieval typing onIAgentRuntime.const knowledgeService = service as KnowledgeService; await knowledgeService.addKnowledge(testDoc);src/__tests__/unit/schema.test.ts (2)
3-4: Remove unused imports to keep tests lint-clean
uuidv4and theUUIDtype are not referenced anywhere in this file. Dead imports triggerno-unused-vars/no-unused-importsrules in most TS setups and slow incremental builds.-import { v4 as uuidv4 } from 'uuid'; -import type { UUID } from '@elizaos/core';
31-63: Consider parameterising repetitive column-existence assertionsThe next two blocks manually list 14+ columns each. If the schema evolves you’ll have to update three separate hard-coded lists, which is brittle and easy to forget.
A compact alternative:
const expectedDocCols = [ 'id','agentId','worldId','roomId','entityId','originalFilename', 'contentType','content','fileSize','title','sourceUrl', 'createdAt','updatedAt','metadata', ]; expectedDocCols.forEach((col) => expect((documentsTable as any)[col]).toBeDefined() );Same pattern for
knowledgeFragmentsTable. This keeps tests declarative and future-proof.src/__tests__/e2e/knowledge-e2e.test.ts (1)
60-64: Avoiddeleteonprocess.envfor perf & lint compliance
delete process.env.KNOWLEDGE_PATHtriggersnoDelete/performance warnings and de-opts the hidden class of theprocess.envobject.
Assigningundefinedachieves the same semantic effect without the overhead.- delete process.env.KNOWLEDGE_PATH; + process.env.KNOWLEDGE_PATH = undefined as any;cypress/e2e/ui-components.cy.ts (1)
45-57: Minor: reduce DOM queries inside loopsInside the
variants/sizesloops Cypress queues a new command on every iteration, which can slow the suite.
Example optimisation:cy.get(variants.map(v => `[data-testid="button-${v}"]`).join(',')) .should('have.length', variants.length);Optional but speeds up large UI matrices.
cypress/e2e/ui-components-simple.cy.ts (1)
27-33: Nit: use.then()instead of second query for click countQuerying
click-counttwice adds extra overhead. Capture the alias once:cy.get('[data-testid="button-clickable"]').as('btn').click().click(); cy.get('[data-testid="click-count"]').should('contain', '2');Small gain in speed & readability.
src/repositories/document-repository.ts (1)
91-98: Wrap destructive queries in try/catch to surface DB errors
delete()currently propagates raw driver errors upstream. A guarded wrapper lets the service layer distinguish “not found” from “DB unavailable”:-const result = await this.db - .delete(documentsTable) - .where(eq(documentsTable.id, id)) - .returning(); +let result: unknown[]; +try { + result = await this.db + .delete(documentsTable) + .where(eq(documentsTable.id, id)) + .returning(); +} catch (err) { + // log & re-throw a repository-specific error + logger.error('Failed to delete document', err); + throw new RepositoryError('DELETE_DOCUMENT_FAILED', err); +}Helps the service layer produce meaningful 5xx vs 4xx responses.
src/__tests__/unit/document-repository.test.ts (1)
110-126: Add a positive test forfindByRoomto close coverage gap
findByRoomis the only public method without a success-path assertion. A quick clone of the agent test withroomIdwould raise overall repository coverage to 100 %.src/frontend/test-components.html (2)
224-241: Duplicatedata-testid="table-row"values hinder reliable selectorsMultiple elements share the same
data-testidmaking Cypress queries ambiguous and brittle. Give each row a unique id or rely on semantic selectors:-<tr data-testid="table-row" …> +<tr data-testid="table-row-1" …> … -<tr data-testid="table-row" …> +<tr data-testid="table-row-2" …>Repeat for other duplicates (e.g.
table-cell).
302-313: Guard against missing React global in module scopeWhen the page is served with strict CSP blocking inline scripts, the UMD bundles might not load, leaving
ReactDOMundefined and throwing. A tiny guard prevents hard failure in CI environments:-if (container) { - const root = ReactDOM.createRoot(container); - root.render(React.createElement(KnowledgeTab, { agentId: 'test-agent-123' })); +if (container && window.ReactDOM?.createRoot) { + const root = window.ReactDOM.createRoot(container); + root.render(window.React.createElement(KnowledgeTab, { agentId: 'test-agent-123' })); }Keeps the test page usable even if CDN fetches fail.
src/__tests__/unit/action-chaining.test.ts (1)
421-440: SeedMath.random()to stabilise relevance-based ranking testsUsing raw
Math.random()can create rare flaky failures when two documents receive identical relevance scores. Either seed a pseudo-random generator or sort deterministically:-relevance: Math.random(), +relevance: i / 10, // deterministic ascending relevanceImproves test repeatability across CI runs.
src/__tests__/unit/advanced-features.test.ts (2)
30-32:generateMockUuidmay collide for suffixes ≥ 10 000The helper always pads to 4 digits, so
generateMockUuid(12345)yields the same value asgenerateMockUuid(2345).
Pad to 12 digits (like elsewhere) or stringify with a counter to avoid accidental ID collisions in larger suites.
346-433: Batch / import tests bypass the public APIThese tests call
mockKnowledgeService.batchOperationandimportKnowledgedirectly instead of exercising the exported actions or HTTP layer.
That doesn’t protect you against wiring mistakes in the real handlers/routes. Consider adding end-to-end tests (or at least invoking the corresponding actions) so regressions in the glue code are caught.cypress/support/commands.ts (2)
45-48: Dead code –blobandfileare never usedThe created
BlobandFileobjects are immediately discarded.
Remove them to reduce noise and keep the command lean.
21-23: Alias should include@
cy.wait()expects the alias string to start with@. Enforce it or prepend automatically to avoid silent time-outs.-cy.wait(alias, { timeout }); +cy.wait(alias.startsWith('@') ? alias : `@${alias}`, { timeout });src/schema.ts (2)
28-34: Storing large binaries intextcolumn is riskyBase-64 PDFs can exceed the Postgres
textpractical limits and bloat tables / WAL.
Preferbytea(binary) or move file blobs to object storage and keep only a reference URL in the row.
33-34:updatedAtnever updates
defaultNow()sets the timestamp only on insert. Add a trigger or update hook so modifications refreshupdatedAt.src/repositories/fragment-repository.ts (2)
112-122: Double-computing cosine similarity is wastefulYou already sort by similarity in SQL; recomputing it in JS for every row adds O(k·d) work.
Return the similarity from SQL (e.g.,1 - cosine_distance AS similarity) and reuse it.
90-93: Unboundedlimit& lowthresholdinvite heavy queriesConsider clamping
limit(e.g., ≤ 100) and validatingthreshold∈ [0,1] to protect the DB from expensive vector scans.src/__tests__/unit/action.test.ts (1)
670-689: Re-using the sameidin the loop reduces test isolationEvery iteration creates a
Memorywith the same UUID (73).
Use the loop index to generate distinct IDs to avoid accidental state leakage when future code relies on uniqueness.src/__tests__/e2e/advanced-features-e2e.test.ts (1)
214-218: Imported doc is not removed during cleanup
neural-networks.txtisn’t matched by the current filename filters, leaving data behind for later tests. Add that filename (or a deterministic tag) to the cleanup predicate.src/frontend/ui/memory-graph.tsx (1)
105-111: Heavy re-processing on every render
processGraphDataruns insideuseEffectand always triggerssetGraphData, even when the derived structure is identical. Wrap it inuseMemo(or diff check) to cut unnecessary state updates on large graphs.src/__tests__/e2e/startup-loading.test.ts (1)
22-24: Avoid using project-rootdocsfolder in testsWriting directly to
${cwd}/docsrisks clobbering real documentation. Usefs.mkdtempSync(path.join(os.tmpdir(), 'docs-'))for an isolated workspace.src/config.ts (1)
24-33: Boolean env parsing is too strictOnly the literal string
'true'is accepted. Consider treating'1','yes', or upper-case variants as truthy for better DX.src/tests.ts (1)
483-484: Preferprocess.env.KEY = undefinedoverdelete
delete process.env.*incurs a de-opt in V8 and triggersnoDeletelint errors. Assignundefinedinstead.-delete process.env.KNOWLEDGE_PATH; +process.env.KNOWLEDGE_PATH = undefined;Also applies to: 519-520, 1241-1242
src/service.ts (1)
904-915: Retrieving 10 000 fragments unbounded may exhaust memory
count: 10000is an arbitrary hard-coded limit.
For large agents this could load millions of KB into RAM and block the event loop.
Consider:
- Stream results or paginate.
- Make the limit configurable (
KNOWLEDGE_ANALYTICS_MAX_SAMPLES).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (47)
.gitignore(1 hunks)README.md(4 hunks)__tests__/action.test.ts(0 hunks)cypress.config.ts(1 hunks)cypress/e2e/simple-test.cy.ts(1 hunks)cypress/e2e/ui-components-simple.cy.ts(1 hunks)cypress/e2e/ui-components.cy.ts(1 hunks)cypress/support/commands.ts(1 hunks)cypress/support/e2e.ts(1 hunks)images/README.md(0 hunks)package.json(3 hunks)src/__tests__/e2e/advanced-features-e2e.test.ts(1 hunks)src/__tests__/e2e/attachment-handling.test.ts(1 hunks)src/__tests__/e2e/knowledge-e2e.test.ts(1 hunks)src/__tests__/e2e/startup-loading.test.ts(1 hunks)src/__tests__/unit/action-chaining.test.ts(1 hunks)src/__tests__/unit/action.test.ts(1 hunks)src/__tests__/unit/advanced-features.test.ts(1 hunks)src/__tests__/unit/document-repository.test.ts(1 hunks)src/__tests__/unit/fragment-repository.test.ts(1 hunks)src/__tests__/unit/schema.test.ts(1 hunks)src/__tests__/unit/update-knowledge.test.ts(1 hunks)src/__tests__/unit/utils.test.ts(1 hunks)src/actions.ts(6 hunks)src/config.ts(1 hunks)src/ctx-embeddings.ts(11 hunks)src/docs-loader.ts(2 hunks)src/document-processor.ts(4 hunks)src/frontend/index.tsx(1 hunks)src/frontend/test-components.html(1 hunks)src/frontend/ui/badge.tsx(1 hunks)src/frontend/ui/button.tsx(1 hunks)src/frontend/ui/card.tsx(2 hunks)src/frontend/ui/memory-graph.tsx(1 hunks)src/index.ts(1 hunks)src/provider.ts(2 hunks)src/repositories/document-repository.ts(1 hunks)src/repositories/fragment-repository.ts(1 hunks)src/repositories/index.ts(1 hunks)src/routes.ts(8 hunks)src/schema.ts(1 hunks)src/service.ts(8 hunks)src/tests.ts(33 hunks)src/types.ts(4 hunks)tsconfig.json(1 hunks)tsup.config.ts(1 hunks)vitest.config.ts(1 hunks)
💤 Files with no reviewable changes (2)
- images/README.md
- tests/action.test.ts
🧰 Additional context used
🧬 Code Graph Analysis (8)
src/__tests__/unit/fragment-repository.test.ts (2)
src/repositories/fragment-repository.ts (1)
FragmentRepository(20-212)src/types.ts (1)
KnowledgeFragment(165-177)
src/__tests__/unit/schema.test.ts (1)
src/schema.ts (3)
documentsTable(18-43)knowledgeFragmentsTable(48-81)knowledgeSchema(100-103)
src/__tests__/e2e/startup-loading.test.ts (2)
src/service.ts (1)
KnowledgeService(44-1115)src/docs-loader.ts (1)
loadDocsFromPath(42-135)
src/__tests__/e2e/advanced-features-e2e.test.ts (1)
src/service.ts (1)
KnowledgeService(44-1115)
src/__tests__/e2e/knowledge-e2e.test.ts (2)
src/service.ts (1)
KnowledgeService(44-1115)src/docs-loader.ts (1)
loadDocsFromPath(42-135)
src/__tests__/unit/action.test.ts (2)
src/service.ts (1)
KnowledgeService(44-1115)src/actions.ts (2)
processKnowledgeAction(21-315)searchKnowledgeAction(320-461)
src/__tests__/unit/advanced-features.test.ts (2)
src/service.ts (1)
KnowledgeService(44-1115)src/actions.ts (3)
advancedSearchAction(466-590)knowledgeAnalyticsAction(595-687)exportKnowledgeAction(692-787)
src/routes.ts (1)
src/service.ts (2)
KnowledgeService(44-1115)batchOperation(825-894)
🪛 Biome (1.9.4)
src/__tests__/unit/fragment-repository.test.ts
[error] 146-146: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 168-168: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 192-192: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 213-213: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 239-239: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 269-269: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 361-361: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 378-378: Do not add then to an object.
(lint/suspicious/noThenProperty)
[error] 393-393: Do not add then to an object.
(lint/suspicious/noThenProperty)
cypress/support/e2e.ts
[error] 51-52: This empty export is useless because there's another export or import.
This import makes useless the empty export.
Safe fix: Remove this useless empty export.
(lint/complexity/noUselessEmptyExport)
src/__tests__/e2e/knowledge-e2e.test.ts
[error] 63-64: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
src/service.ts
[error] 852-852: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1000-1000: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1001-1007: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1038-1038: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1053-1053: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1054-1054: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 1074-1074: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
src/tests.ts
[error] 483-483: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 519-519: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1241-1241: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 LanguageTool
README.md
[uncategorized] ~79-~79: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...attachments** - Share links to documents and they'll be downloaded and processed - *...
(COMMA_COMPOUND_SENTENCE)
🔇 Additional comments (17)
src/frontend/index.tsx (1)
5-5: Verify TypeScript & bundler resolution for.jsimport
Confirm that your TS config (allowJs,moduleResolution) and bundler handle explicit.jsextensions. Otherwise consider using an extension-agnostic import or pointing to the original.tsxsource.src/__tests__/unit/utils.test.ts (1)
2-2: Corrected import path
The import from'../../utils'now correctly resolves tosrc/utils.ts, fixing the previous module resolution error.tsup.config.ts (1)
18-19: Externalising the SQL plugin is the right moveMarking
@elizaos/plugin-sqlas external keeps the final bundle slim and avoids pulling heavy DB drivers into the build. No issues spotted.src/docs-loader.ts (1)
107-112: Verifymetadatais part ofAddKnowledgeOptions
metadatais newly added here. If the type intypes.tshasn’t been updated, TypeScript will complain. Please confirm the interface has been extended.src/repositories/index.ts (1)
1-6: Clean central re-exportMakes downstream imports shorter and clearer. Looks good.
tsconfig.json (1)
19-22:skipLibCheckmay hide real type errorsSetting
skipLibCheck: truespeeds up compilation but suppresses errors in dependency.d.tsfiles—those often surface legitimate issues. Consider enabling it only in CI cache misses or large monorepos, or add a comment explaining the trade-off.vitest.config.ts (1)
8-9: Globbing pattern may not match any tests
'src/**/*.{test,spec}.{js,ts,jsx,tsx}'is interpreted literally by picomatch: only files ending with. {test,spec}.{js…}will match. The usual pattern is'src/**/*.{test,spec}.{js,ts,jsx,tsx}'or two-brace form'src/**/*.@(test|spec).@(js|ts|jsx|tsx)'.Verify that
vitestactually discovers your unit tests; otherwise adjust the pattern.src/__tests__/unit/fragment-repository.test.ts (2)
82-88:createinput violates repository contract.
FragmentRepository.createexpectsOmit<KnowledgeFragment, 'id' | 'createdAt'>, yet the test passes a fullKnowledgeFragment(it includesidandcreatedAt). Type-checking is bypassed only because the value is stored in a variable first. Drop the forbidden fields to align with actual usage and avoid misleading tests.
48-76: Promise-like mock should return a realPromise, not an object with.then.Adding a
thenproperty to a plain object triggers Biome’snoThenPropertyrule and can mask async mistakes. Wrap the chain inPromise.resolveinstead:- mockChain.returning.mockImplementation(() => { - return Promise.resolve(mockChain._returnValue || []); - }); + mockChain.returning.mockImplementation(async () => mockChain._returnValue ?? []);Same pattern can be applied to the various
selectChainhelpers below.Likely an incorrect or invalid review comment.
cypress/support/e2e.ts (1)
24-49: Only types are declared for the custom commands; ensure implementations exist.
visitKnowledgePanel,uploadKnowledgeFile,searchKnowledge, anddeleteDocumentare typed here but never defined in this module. Verify thatcypress/support/commands.tsactually adds these commands, otherwise runtime failures will occur.src/__tests__/unit/update-knowledge.test.ts (1)
220-222: Test expectation allows silent failures.
deleteMemoryof a non-existent document currently counts as “success”. If the desired contract is to report an error when the document is missing, this assertion should be2(only existing docs), not3.Please confirm the intended behaviour.
src/document-processor.ts (1)
564-566: Defensive null-check before concatenating context
llmResponsemay already be a string; after the fallback you could still end up withundefinedif the provider returns{}.
Consider:const generatedContext = typeof llmResponse === 'string' ? llmResponse : (llmResponse as any)?.text ?? '';Prevents
getChunkWithContextfrom receivingundefined.src/__tests__/e2e/knowledge-e2e.test.ts (1)
138-155: Potential race: ensure fragments cascaded before assertionThe service might delete the document async and queue fragment cleanup.
A short poll / await would make this test less flaky:await retryAssert(async () => { const remaining = await service.getMemories({ tableName: 'knowledge', count: 100 }); expect(remaining.some(f => (f.metadata as any)?.documentId === testDocument.id)).toBe(false); }, 5, 200); // 5 tries, 200 ms apartConsider adding such helper to the test utils.
src/__tests__/e2e/advanced-features-e2e.test.ts (1)
190-198: Verify empty-query behaviour
advancedSearch({ query: '' … })may bypass embedding generation or throw, depending on implementation. Confirm it cannot return inconsistent similarity scores.src/tests.ts (1)
169-177:metadata.typecomparison may mismatch enum value
MemoryType.FRAGMENT/DOCUMENTare enums; storedmetadata.typemay be lower-case strings ('fragment','document'). The current filter could silently drop records in tests.src/actions.ts (1)
290-297:results.lengthis always0for path/text uploadsWhen no attachments are present the
resultsarray isn’t populated, yet it’s still returned to callers.
Return a more meaningful object (e.g.undefined) or populate it with the single processed file/text to keep the schema stable.src/types.ts (1)
336-355: Nice extensibility!The batch-operation and analytics types look solid and future-proof.
src/docs-loader.ts
Outdated
| clientDocumentId: createUniqueUuid((service as any).runtime, `docs-${fileName}-${Date.now()}`) as UUID, | ||
| contentType, | ||
| originalFilename: fileName, | ||
| worldId: worldId || agentId, | ||
| content, | ||
| roomId: agentId, | ||
| entityId: agentId, | ||
| metadata: { | ||
| source: 'docs', | ||
| path: filePath, | ||
| loadedAt: new Date().toISOString(), | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createUniqueUuid is given an object, producing corrupt IDs
createUniqueUuid expects a UUID seed string. Passing (service as any).runtime (an object) will stringify to [object Object], breaking UUID semantics and potentially violating FK constraints.
- clientDocumentId: createUniqueUuid((service as any).runtime, `docs-${fileName}-${Date.now()}`) as UUID,
+ const runtimeId = (service as any).runtime?.id ?? agentId;
+ clientDocumentId: createUniqueUuid(runtimeId, `docs-${fileName}-${Date.now()}`) as UUID,Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In src/docs-loader.ts around lines 100 to 112, the call to createUniqueUuid
incorrectly passes (service as any).runtime, which is an object, instead of a
UUID seed string. To fix this, extract or generate a proper UUID string from the
runtime object or use a valid string identifier as the seed argument for
createUniqueUuid to ensure it produces valid UUIDs and maintains FK integrity.
| export class DocumentRepository { | ||
| constructor(private db: PostgresJsDatabase<any>) {} | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Parameterise repository for better type-safety
PostgresJsDatabase<any> throws away the table schema information, forcing all downstream rows to any. Drizzle already provides strongly-typed row inference – keep it:
-export class DocumentRepository {
- constructor(private db: PostgresJsDatabase<any>) {}
+export class DocumentRepository<TSchema = typeof documentsTable> {
+ constructor(private readonly db: PostgresJsDatabase<TSchema>) {}This preserves column names & narrows row in mapToDocument, surfacing compile-time errors instead of silent any.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export class DocumentRepository { | |
| constructor(private db: PostgresJsDatabase<any>) {} | |
| export class DocumentRepository<TSchema = typeof documentsTable> { | |
| constructor(private readonly db: PostgresJsDatabase<TSchema>) {} |
🤖 Prompt for AI Agents
In src/repositories/document-repository.ts around lines 7 to 9, the constructor
parameter uses PostgresJsDatabase<any>, which loses table schema type
information and results in rows typed as any. To fix this, parameterize the
DocumentRepository class and the db parameter with the appropriate table schema
type from Drizzle to preserve strong typing. This will ensure that downstream
row objects, such as in mapToDocument, have accurate types reflecting column
names and types, improving type safety and compile-time error detection.
| await new Promise((resolve) => setTimeout(resolve, 2000)); | ||
| console.log('✓ Waited for startup document loading'); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fixed 2 s delay may still race on slow CI
Wait for service.getAnalytics().totalDocuments >= expected (with timeout) instead of setTimeout(2000).
🤖 Prompt for AI Agents
In src/__tests__/e2e/startup-loading.test.ts around lines 64 to 66, replace the
fixed 2-second delay using setTimeout with a loop or polling mechanism that
waits until service.getAnalytics().totalDocuments is greater than or equal to
the expected value, implementing a timeout to avoid infinite waiting. This
ensures the test waits for the actual condition rather than a fixed delay,
preventing race conditions on slow CI environments.
- KnowledgeService routes were failing because services were being registered with class name 'KnowledgeService' but retrieved with serviceType 'knowledge' - Runtime service registration uses serviceName || className as the key, but getService() uses serviceName - Added static serviceName property to match serviceType for: - KnowledgeService: serviceName = 'knowledge' - ShellService: serviceName = 'SHELL' - EnvManagerService: serviceName = 'ENV_MANAGER' - This ensures service registration and retrieval use the same key 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/config.ts (1)
42-44: Proper NaN guard implementation.The code correctly addresses the past review comment about NaN handling by checking
Number.isNaN()and providing safe default values. This prevents runtime issues when environment variables contain non-numeric values.src/service.ts (1)
846-873: Fix switch clause variable declaration issue.The
memoryvariable is declared directly in theupdatecase clause without block scoping, which can cause variable shadowing issues across switch cases.Apply this diff to wrap the case clause in block scope:
- case 'update': - if (!item.id || !item.metadata) throw new Error('Missing id or metadata for update'); - // Update document metadata - const memory = await this.runtime.getMemoryById(item.id as UUID); - if (memory) { - await this.runtime.updateMemory({ - ...memory, - id: memory.id!, // Ensure id is defined - metadata: { ...memory.metadata, ...item.metadata } as MemoryMetadata, - }); - result = { updated: true }; - } else { - throw new Error('Document not found'); - } - break; + case 'update': { + if (!item.id || !item.metadata) throw new Error('Missing id or metadata for update'); + // Update document metadata + const memory = await this.runtime.getMemoryById(item.id as UUID); + if (memory) { + await this.runtime.updateMemory({ + ...memory, + id: memory.id!, // Ensure id is defined + metadata: { ...memory.metadata, ...item.metadata } as MemoryMetadata, + }); + result = { updated: true }; + } else { + throw new Error('Document not found'); + } + break; + }
🧹 Nitpick comments (3)
cypress.config.ts (2)
25-27: Consider differentiating timeout values based on operation type.All timeouts are set to 10 seconds, which may not be optimal for all scenarios. Consider if different operations need different timeout values:
- Command timeout: For user interactions (current: 10s)
- Request timeout: For API calls (might need shorter: 5-8s)
- Response timeout: For server responses (might need longer: 15-20s)
- defaultCommandTimeout: 10000, - requestTimeout: 10000, - responseTimeout: 10000, + defaultCommandTimeout: 10000, + requestTimeout: 8000, + responseTimeout: 15000,
17-29: Consider adding retry configuration for improved test stability.The e2e configuration is comprehensive, but adding retry configuration could help with flaky tests common in e2e scenarios.
e2e: { baseUrl: process.env.CYPRESS_baseUrl || 'http://localhost:3000', supportFile: 'cypress/support/e2e.ts', specPattern: 'cypress/e2e/**/*.cy.{js,jsx,ts,tsx}', + retries: { + runMode: 2, + openMode: 0, + }, video: false,src/tests.ts (1)
483-483: Replacedeleteoperator with undefined assignment for better performance.The
deleteoperator can impact performance. Use undefined assignment instead as suggested by the static analysis tool.Apply this diff to improve performance:
- delete process.env.KNOWLEDGE_PATH; + process.env.KNOWLEDGE_PATH = undefined;Apply the same pattern to all three occurrences.
Also applies to: 519-519, 1243-1243
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (25)
cypress.config.ts(1 hunks)cypress/e2e/simple-test.cy.ts(1 hunks)cypress/support/component-index.html(1 hunks)cypress/support/component.ts(1 hunks)cypress/tsconfig.json(1 hunks)package.json(2 hunks)src/__tests__/e2e/advanced-features-e2e.test.ts(1 hunks)src/__tests__/e2e/attachment-handling.test.ts(1 hunks)src/__tests__/e2e/startup-loading.test.ts(1 hunks)src/__tests__/unit/action-chaining.test.ts(1 hunks)src/__tests__/unit/action.test.ts(1 hunks)src/__tests__/unit/advanced-features.test.ts(1 hunks)src/__tests__/unit/update-knowledge.test.ts(1 hunks)src/__tests__/unit/utils.test.ts(2 hunks)src/actions.ts(6 hunks)src/config.ts(1 hunks)src/docs-loader.ts(2 hunks)src/document-processor.ts(5 hunks)src/frontend/ui/memory-graph.tsx(1 hunks)src/index.ts(1 hunks)src/repositories/document-repository.ts(1 hunks)src/routes.ts(8 hunks)src/service.ts(7 hunks)src/tests.ts(33 hunks)src/utils.ts(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- cypress/tsconfig.json
- cypress/support/component-index.html
- cypress/support/component.ts
🚧 Files skipped from review as they are similar to previous changes (14)
- src/tests/unit/utils.test.ts
- src/docs-loader.ts
- src/tests/unit/update-knowledge.test.ts
- src/tests/e2e/attachment-handling.test.ts
- src/document-processor.ts
- src/index.ts
- cypress/e2e/simple-test.cy.ts
- src/tests/unit/advanced-features.test.ts
- package.json
- src/tests/unit/action.test.ts
- src/tests/e2e/advanced-features-e2e.test.ts
- src/repositories/document-repository.ts
- src/tests/e2e/startup-loading.test.ts
- src/frontend/ui/memory-graph.tsx
🧰 Additional context used
🪛 Biome (1.9.4)
src/service.ts
[error] 855-855: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Unsafe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
src/tests.ts
[error] 483-483: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 519-519: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 1243-1243: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (21)
cypress.config.ts (4)
1-3: LGTM! Clean configuration structure.The import and export structure follows Cypress best practices using the
defineConfigfunction.
4-16: Component testing configuration looks solid.The React + Vite setup is appropriate and the file paths follow Cypress conventions. The artifact management (screenshots enabled, videos disabled) strikes a good balance between debugging capability and storage efficiency.
18-18: Good practice using environment variable for base URL.This allows for flexible configuration across different environments while providing a sensible default.
28-28: Document the reason for disabling Chrome web security.Disabling
chromeWebSecuritycan introduce security risks and should be documented. Please ensure this is necessary for your cross-origin testing scenarios.Consider adding a comment explaining why this is needed:
+ // Disabled to allow cross-origin requests in testing environment chromeWebSecurity: false,If cross-origin testing isn't required, consider removing this setting for better security:
- chromeWebSecurity: false,src/tests.ts (3)
1122-1179: Well-structured test for new tables feature.The test comprehensively covers the new tables functionality including document creation, fragment verification, and cascade delete operations. The test setup and cleanup are properly handled.
1182-1246: Comprehensive startup document loading test with proper cleanup.The test properly creates a temporary directory, populates it with test files, and verifies document loading. The cleanup logic in the finally block ensures no test artifacts remain.
1248-1264: Clean integration of E2E tests.The integration of external E2E test modules using their
nameandfnproperties maintains clean separation of concerns while enabling comprehensive test coverage.src/utils.ts (1)
423-424: Good stylistic improvement for operator precedence clarity.Adding parentheses around the logical expression makes the operator precedence explicit and improves code readability without changing functionality.
src/__tests__/unit/action-chaining.test.ts (1)
1-594: Excellent comprehensive test suite for action chaining.This test file demonstrates strong testing practices with:
- Proper mocking of dependencies
- Comprehensive coverage of success and failure scenarios
- Multi-step action chain testing
- Clear test organization and readable assertions
- Good error propagation testing
The test coverage will help ensure reliable action chaining functionality.
src/config.ts (1)
22-105: Simplified configuration validation improves maintainability.The rewrite from Zod-based validation to explicit parameter extraction with clear defaults simplifies the codebase while maintaining necessary validation. The approach is more direct and easier to debug.
src/actions.ts (4)
128-187: Robust attachment processing with proper error handling.The attachment processing logic properly handles different attachment types (URL vs data), includes appropriate error handling for individual attachments, and maintains detailed results tracking. The metadata enrichment with source and message ID aids traceability.
474-603: Well-implemented advanced search action with natural language parsing.The advanced search action demonstrates good natural language processing to extract filters, date ranges, and sorting preferences. The implementation properly handles various search criteria and provides structured results.
608-706: Comprehensive analytics action with clear formatting.The analytics action provides valuable insights about the knowledge base with clear formatting. The response includes key metrics like document counts, storage size, and content type distribution in a user-friendly format.
157-157: Verify UUID generation function.The code uses
stringToUuid()to generate document IDs. Ensure this function produces valid UUIDs and handles collisions appropriately for production use.#!/bin/bash # Search for stringToUuid function implementation to verify UUID generation ast-grep --pattern 'function stringToUuid($_) { $$$ }' # Also search for its usage patterns rg -A 3 "stringToUuid"src/service.ts (3)
53-56: LGTM! Good implementation of feature flag pattern.The feature flag approach allows for safe rollout of new table implementations while maintaining backward compatibility.
724-821: LGTM! Well-implemented advanced search functionality.The advanced search method provides comprehensive filtering, sorting, and pagination capabilities with proper error handling and type safety.
904-957: LGTM! Robust analytics implementation.The analytics method provides useful insights into the knowledge base with proper error handling and fallbacks for missing data.
src/routes.ts (4)
314-408: LGTM! Comprehensive update handler implementation.The update handler properly supports both file replacement and metadata-only updates with appropriate cleanup and error handling.
920-967: LGTM! Efficient bulk delete implementation.The bulk delete handler processes operations in parallel with proper error aggregation and detailed result reporting.
1175-1198: LGTM! Route paths are correctly configured.The route paths are properly relative to the base mount path, avoiding the duplication issue mentioned in previous reviews.
617-617: Question: Serving TypeScript files instead of compiled JavaScript.The code references
.tsfile extensions for frontend assets, which is unusual in production. Typically, TypeScript files should be compiled to JavaScript before serving.Verify if this is intentional for development purposes or if these should be
.jsextensions for production builds:#!/bin/bash # Check if there are actual .ts files in the dist/assets directory find . -name "dist" -type d -exec find {} -name "*.ts" \; 2>/dev/null find . -name "dist" -type d -exec find {} -name "*.js" \; 2>/dev/null | head -10 # Check build configuration fd -t f "vite.config" -x cat {} fd -t f "tsconfig" -x cat {} | head -20Also applies to: 632-632, 716-716
| export const exportKnowledgeAction: Action = { | ||
| name: 'EXPORT_KNOWLEDGE', | ||
| description: 'Export knowledge base to various formats', | ||
|
|
||
| similes: ['export knowledge', 'download knowledge', 'backup knowledge', 'save knowledge to file'], | ||
|
|
||
| examples: [ | ||
| [ | ||
| { | ||
| name: 'user', | ||
| content: { | ||
| text: 'Export my knowledge base as JSON', | ||
| }, | ||
| }, | ||
| { | ||
| name: 'assistant', | ||
| content: { | ||
| text: "I'll export your knowledge base as JSON.", | ||
| actions: ['EXPORT_KNOWLEDGE'], | ||
| }, | ||
| }, | ||
| ], | ||
| ], | ||
|
|
||
| validate: async (runtime: IAgentRuntime, message: Memory) => { | ||
| const text = message.content.text?.toLowerCase() || ''; | ||
| const hasExportKeywords = ['export', 'download', 'backup', 'save'].some((k) => | ||
| text.includes(k) | ||
| ); | ||
| const hasKnowledgeWord = text.includes('knowledge'); | ||
|
|
||
| const service = runtime.getService(KnowledgeService.serviceType); | ||
| return !!(service && hasExportKeywords && hasKnowledgeWord); | ||
| }, | ||
|
|
||
| handler: async ( | ||
| runtime: IAgentRuntime, | ||
| message: Memory, | ||
| state?: State, | ||
| options?: { [key: string]: unknown }, | ||
| callback?: HandlerCallback | ||
| ): Promise<ActionResult> => { | ||
| try { | ||
| const service = runtime.getService<KnowledgeService>(KnowledgeService.serviceType); | ||
| if (!service) { | ||
| throw new Error('Knowledge service not available'); | ||
| } | ||
|
|
||
| const text = message.content.text || ''; | ||
|
|
||
| // Detect format | ||
| let format: 'json' | 'csv' | 'markdown' = 'json'; | ||
| if (text.includes('csv')) format = 'csv'; | ||
| else if (text.includes('markdown') || text.includes('md')) format = 'markdown'; | ||
|
|
||
| const exportData = await service.exportKnowledge({ | ||
| format, | ||
| includeMetadata: true, | ||
| includeFragments: false, | ||
| }); | ||
|
|
||
| // In a real implementation, this would save to a file or return a download link | ||
| // For now, we'll just return a preview | ||
| const preview = exportData.substring(0, 500) + (exportData.length > 500 ? '...' : ''); | ||
|
|
||
| const response: Content = { | ||
| text: `✅ Knowledge base exported as ${format.toUpperCase()}. Size: ${(exportData.length / 1024).toFixed(2)} KB\n\nPreview:\n\`\`\`${format}\n${preview}\n\`\`\``, | ||
| }; | ||
|
|
||
| if (callback) { | ||
| await callback(response); | ||
| } | ||
|
|
||
| return { | ||
| data: { | ||
| format, | ||
| size: exportData.length, | ||
| content: exportData, | ||
| }, | ||
| text: response.text, | ||
| }; | ||
| } catch (error) { | ||
| logger.error('Error in EXPORT_KNOWLEDGE:', error); | ||
| const errorResponse: Content = { | ||
| text: `Error exporting knowledge: ${error instanceof Error ? error.message : 'Unknown error'}`, | ||
| }; | ||
| if (callback) { | ||
| await callback(errorResponse); | ||
| } | ||
| return { data: { error: String(error) }, text: errorResponse.text }; | ||
| } | ||
| }, | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Export functionality with security considerations.
The export action supports multiple formats and includes size information. Consider adding access controls and rate limiting for export operations in production environments to prevent data exfiltration or abuse.
Consider implementing:
- Access control checks before allowing exports
- Rate limiting to prevent abuse
- File size limits for large knowledge bases
- Audit logging for export operations
🤖 Prompt for AI Agents
In src/actions.ts from lines 711 to 803, the exportKnowledgeAction lacks
security measures. Add access control checks to verify user permissions before
proceeding with export. Implement rate limiting to restrict the frequency of
export requests. Enforce file size limits to prevent exporting excessively large
knowledge bases. Include audit logging to record export operations for
monitoring and compliance.
| /** | ||
| * Documents table - stores the main document metadata and content | ||
| */ | ||
| export const documentsTable = pgTable( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how is this supposed to work with plugin-mysql?
This PR adds a bunch of testing coverage, action chaining, etc.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
Chores
.gitignoreto exclude additional environment, coverage, screenshots, and documentation files.