Code Quality Audit: 26 findings across error handling, type safety, testing, architecture, security, performance, LLM-friendliness

[RobertLD]

Code Quality Audit — 26 Findings (7-Agent Deep Scan)

Systematic audit of the development branch using 7 parallel analysis agents covering: error handling, type safety, test coverage, architecture, security, LLM-friendliness, and performance.

Summary: 5 CRITICAL · 8 HIGH · 9 MEDIUM · 4 LOW

Relationship to #314: Issue #314 (closed) covered a prior audit against main. This is a fresh scan of development after dependabot merges (#320-#325) and feature work (#329). Some findings overlap; new findings marked with ✨.

---

🔴 CRITICAL (5)

1. 15+ silent error suppression catch blocks

Files: obsidian.ts, cli/index.ts, middleware.ts, slack.ts, dashboard.ts, webhooks.ts, versioning.ts, dedup.ts, workspace.ts

Empty catch blocks or catches that swallow errors without any logging. Some were fixed in PR #280 but 15+ remain.

Fix: Add log.warn() to all catch blocks. For intentionally silent catches, add a // intentionally silent comment.

• Audit all catch blocks and add logging or document silence

2. Zero provider tests (5 files completely untested)

Files: src/providers/openai.ts, ollama.ts, local.ts, provider.ts, index.ts

OpenAI, Ollama, and local embedding providers have zero unit tests, excluded from 75% coverage threshold.

Fix: Create tests/unit/providers/ with mocked HTTP response tests for each provider.

• Add provider tests with mocked HTTP responses

3. MCP server has zero test coverage (1170 lines, 50+ tool handlers)

Files: src/mcp/server.ts

The primary user-facing interface has zero tests — 1170 lines of untested code.

Fix: Add integration tests for tool registration, handler execution, error propagation.

• Add MCP server tests

4. Only 1 integration test file (8 tests total)

Files: tests/integration/workflow.test.ts

Nearly all 1000+ tests are unit tests. Real end-to-end flows (submit → index → search → retrieve) barely tested.

Fix: Add 10-15 integration tests covering: document lifecycle, pack install + search, connector sync, bulk ops, RAG pipeline.

• Expand integration test suite

5. CLI god file: 2616 lines ✨

Files: src/cli/index.ts

All CLI commands defined inline in a single massive file.

Fix: Extract to src/cli/commands/pack.ts, src/cli/commands/search.ts, src/cli/commands/connector.ts, etc.

• Decompose CLI into command modules

---

🟠 HIGH (8)

6. MCP server god file: 1170 lines

Files: src/mcp/server.ts

All MCP tool handlers defined inline in one file.

Fix: Extract to src/mcp/tools/search.ts, src/mcp/tools/documents.ts, etc.

• Decompose MCP server into tool modules

7. Routes handler: 865 lines with manual path matching

Files: src/api/routes.ts

Manual if/else chain for 35+ API routes.

Fix: Extract route handlers into src/api/handlers/ modules.

• Decompose route handlers

8. Connector tokens stored in plaintext

Files: ~/.libscope/connectors/*.json

Notion, Slack, Confluence API tokens stored as plain JSON files on disk.

Fix: Encrypt secrets at rest using a master key or OS keychain integration.

• Add connector token encryption

9. Unbounded SELECT * in export/dedup/graph

Files: src/core/export.ts, src/core/dedup.ts, src/core/graph.ts

Loads entire tables into memory. 10k+ documents could consume GBs of RAM.

Fix: Paginate with LIMIT/OFFSET or stream in batches.

• Add pagination to bulk data queries

10. N+1 user resolution in Slack sync

Files: src/connectors/slack.ts:388-392

resolveUser() makes sequential API calls per user ID.

Fix: Pre-collect unique user IDs, batch resolve with Promise.all().

• Batch Slack user resolution

11. Flaky hardcoded sleeps in webhook tests

Files: tests/unit/webhooks.test.ts:50,73

Uses setTimeout(r, 50) instead of fake timers.

Fix: Replace with vi.useFakeTimers().

• Fix flaky webhook tests

12. Dangerous as any cast in local provider

Files: src/providers/local.ts:42

Pipeline from @xenova/transformers cast to any.

Fix: Define a proper Pipeline interface.

• Type the transformers pipeline properly

13. Only 27% JSDoc coverage on exported functions ✨

Files: src/ (145 exported functions, ~40 documented)

Missing @param, @returns, @throws on most public API functions.

Fix: Add JSDoc to the top 50 most-used exported functions.

• JSDoc blitz on top 50 exports

---

🟡 MEDIUM (9)

14. Connector duplication: ~500 lines × 5 connectors

Files: src/connectors/confluence.ts, notion.ts, obsidian.ts, slack.ts, onenote.ts

Pagination, rate limiting, sync lifecycle duplicated across all 5 connectors.

Fix: Create abstract ConnectorBase class with shared lifecycle.

• Extract connector base class

15. CSP allows unsafe-inline in dashboard

Files: src/api/middleware.ts:37-39

script-src 'self' 'unsafe-inline' defeats CSP protection.

Fix: Use nonce-based CSP or move inline JS to external scripts.

• Remove unsafe-inline from CSP

16. Missing DB indexes on frequently-queried columns

Files: src/db/schema.ts

No index on webhooks.active or documents.content_hash.

Fix: Add indexes in the next migration.

• Add database indexes

17. O(n²) pairwise similarity in dedup and graph

Files: src/core/dedup.ts:188, src/core/graph.ts:239

5K docs = 12.5M comparisons.

Fix: Add batch size limits or approximate nearest neighbor.

• Optimize similarity algorithms

18. Sequential async indexing in connectors

Files: src/connectors/slack.ts:395-410

Documents indexed one at a time with await in a loop.

Fix: Use concurrency-limited Promise.all() (e.g., p-limit with concurrency 5).

• Add concurrent indexing in connectors

19. Coverage exclusions hide real gaps ✨

Files: vitest.config.ts

Providers, MCP server, CLI excluded from 75% coverage threshold.

Fix: Remove exclusions after adding corresponding tests.

• Address coverage exclusions

20. No module-level README files ✨

Files: src/core/, src/providers/, src/connectors/, src/api/, src/mcp/

LLMs and new developers must read agents.md to understand module boundaries.

Fix: Create README.md in each src/ subdirectory.

• Add module-level READMEs

21. Sparse inline "why" comments ✨

Files: src/core/search.ts, indexing.ts, dedup.ts

Complex algorithms lack explanation of design decisions.

Fix: Add 15-20 strategic inline comments explaining non-obvious decisions.

• Add strategic "why" comments

22. 1 raw throw new Error() remaining

Files: src/web/dashboard.ts

Uses throw new Error(res.statusText) instead of FetchError.

• Fix raw Error throw in dashboard

---

🟢 LOW (4)

23. 4+ catch blocks silently eat response parsing errors

Files: confluence.ts, http-utils.ts, notion.ts

.catch(() => "") silently discards response body parsing errors.

• Add debug logging to silent catches

24. 113 type assertions (88% safe, low overall risk)

Files: Various DB-layer files

Mostly safe as DbRowType casts. 1 dangerous cast tracked as HIGH (#12).

• Evaluate runtime validation for DB rows

25. No code templates for extending the system ✨

Files: N/A (new)

No scaffolds for adding new providers, connectors, or parsers.

• Add extension templates in templates/

26. No .editorconfig file ✨

Relies solely on Prettier/ESLint for formatting.

• Add .editorconfig

---

✅ What's Already Excellent

• TypeScript strict mode — full strict: true with noUncheckedIndexedAccess, exactOptionalPropertyTypes
• SQL injection safe — 100% parameterized queries
• SSRF protection — DNS validation + rebinding protection
• Error type hierarchy — LibScopeError → ValidationError, FetchError, etc.
• Zero @ts-ignore / @ts-expect-error
• Clean barrel exports and sensible directory structure
• agents.md (325 lines) — excellent AI agent documentation
• Rate limiting with constant-time API key comparison
• No eval() or Function() usage

---

Suggested Priority Order

1. Error handling cleanup (Bug: Coverage thresholds fail — 43% actual vs 80% required #1, Enhancement: URL fetcher has no timeout, redirect limit, or max body size #22, Enhancement: CI workflow does not run coverage — thresholds are untested #23) — low effort, high reliability gain
2. Test coverage expansion (Bug: FTS5 search never tested #2, Bug: Duplicate document indexing — no URL or content deduplication #3, Bug: url-fetcher.ts throws EmbeddingError instead of a fetch-specific error #4, Enhancement: MCP server error handler swallows non-LibScopeError details #11, Enhancement: import command has no progress indicator for large directories #19) — highest impact on confidence
3. God file decomposition (Bug: htmlToText regex is vulnerable to ReDoS and mishandles nested/malformed HTML #5, Bug: DB connection is a module-level singleton — prevents concurrent test isolation #6, Bug: createVectorTable SQL injection via string interpolation #7) — improves maintainability and testability
4. Performance fixes (Bug: saveUserConfig does shallow merge — nested keys like embedding.provider overwrite sibling keys #9, Enhancement: deleteDocument does not clean up chunk_embeddings or chunks_fts #10, Enhancement: No CLI list/show command for documents #17, Enhancement: No CLI delete command for documents #18) — prevents scaling issues
5. Security hardening (Bug: Chunking loses context — chunks have no heading breadcrumb #8, Enhancement: README.md outdated — missing URL ingestion, import command, FTS5 #15) — connector tokens and CSP
6. LLM-friendliness (Enhancement: CLI add/import commands create embedding provider unnecessarily for non-embedding tasks #13, Bug: MCP server has no list-documents tool #20, Bug: FTS5 trigger uses INSERT OR IGNORE — silently drops rows on conflict #21, chore(main): release 1.0.0 #25, fix: add id-token permission for npm provenance publishing #26) — improves AI-assisted development
7. Remaining cleanup (Enhancement: No graceful shutdown — DB connection not closed on SIGINT/SIGTERM #12, Bug: OllamaEmbeddingProvider.embedBatch returns array directly without validation #14, Bug: vector search SQL is missing AND keyword — filter conditions join incorrectly #16, build(deps): Bump esbuild from 0.21.5 to 0.27.3 in the npm_and_yarn group across 1 directory #24) — polish

Related issues: #330 (CLI logging UX), #314 (prior audit, closed)