Build local-first Visual Intelligence OS#7
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request transitions the "Visual Intelligence Audit" tool into a comprehensive, local-first "Visual Intelligence OS" (v0.2.0). Key changes include the introduction of a SQLite-based asset graph, a revamped CLI, an updated Model Context Protocol (MCP) server, a static HTML dashboard generator, and extensive documentation and JSON schemas. The code review feedback is highly constructive and identifies several critical improvements: securing path traversal checks in the MCP server allowlist, updating the Node.js engine requirement in package.json to reflect the experimental node:sqlite dependency, preventing Out-Of-Memory crashes by hashing large files in chunks, correcting the MCP protocol version to the stable 2024-11-05 release, adding error handling for malformed configuration files, resolving relative paths in allowedRoots against the project root, and improving SVG viewBox parsing robustness.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function isAllowedPath(value) { | ||
| if (!value || typeof value !== 'string') return true | ||
| if (!/^[a-zA-Z]:\\|^\//.test(value)) return true | ||
| const normalized = path.resolve(value).toLowerCase() | ||
| return ALLOWED_ROOTS.some(root => normalized === root || normalized.startsWith(root + path.sep)) | ||
| } |
There was a problem hiding this comment.
The custom regex /^[a-zA-Z]:\\|^\// used to detect absolute paths fails to match Windows absolute paths using forward slashes (e.g., C:/Windows/...), allowing them to bypass the allowlist check completely. Additionally, relative paths with directory traversal (e.g., ../../etc/passwd) bypass the check because they are not absolute. Using path.resolve with ROOT first and checking the resolved absolute path against ALLOWED_ROOTS secures both absolute and relative paths robustly.
function isAllowedPath(value) {
if (!value || typeof value !== 'string') return true
const absolute = path.resolve(ROOT, value).toLowerCase()
return ALLOWED_ROOTS.some(root => absolute === root || absolute.startsWith(root + path.sep))
}| import path from 'path' | ||
| import crypto from 'crypto' | ||
| import { pathToFileURL } from 'url' | ||
| import { DatabaseSync } from 'node:sqlite' |
There was a problem hiding this comment.
The node:sqlite module (DatabaseSync) is an experimental feature introduced in Node.js v22.5.0. However, package.json specifies a Node.js engine requirement of "node": ">=18.0.0". Running this code on Node.js versions prior to v22.5.0 (such as Node 18 or 20) will result in a Cannot find module 'node:sqlite' runtime error. Please update the engines.node field in package.json to require >=22.5.0 to reflect this dependency.
| const stats = fs.statSync(filePath) | ||
| const bytes = fs.readFileSync(filePath) | ||
| const sha256 = crypto.createHash('sha256').update(bytes).digest('hex') |
There was a problem hiding this comment.
Reading the entire file into memory using fs.readFileSync can cause Out-Of-Memory (OOM) crashes on large media files (such as video or audio assets, which are explicitly supported in videoExtensions and audioExtensions). To prevent this, hash the file in chunks using a stream-like approach, and only read the first 64KB header chunk for dimension detection.
const stats = fs.statSync(filePath)
const hashFileSync = (p) => {
const hash = crypto.createHash('sha256')
const fd = fs.openSync(p, 'r')
const buffer = Buffer.alloc(64 * 1024)
let bytesRead = 0
try {
do {
bytesRead = fs.readSync(fd, buffer, 0, buffer.length, null)
hash.update(buffer.subarray(0, bytesRead))
} while (bytesRead > 0)
} finally {
fs.closeSync(fd)
}
return hash.digest('hex')
}
const readHeaderSync = (p, len) => {
const fd = fs.openSync(p, 'r')
const buffer = Buffer.alloc(len)
try {
fs.readSync(fd, buffer, 0, len, 0)
} finally {
fs.closeSync(fd)
}
return buffer
}
const sha256 = hashFileSync(filePath)
const bytes = stats.size > 64 * 1024
? readHeaderSync(filePath, 64 * 1024)
: fs.readFileSync(filePath)| try { | ||
| if (method === 'initialize') { | ||
| send(id, { | ||
| protocolVersion: '2025-06-18', |
There was a problem hiding this comment.
The Model Context Protocol (MCP) specification defines "2024-11-05" as the stable protocol version. Using an unsupported version like "2025-06-18" will cause standard MCP clients (such as Claude Desktop or Claude Code) to reject the connection or fail to initialize.
| protocolVersion: '2025-06-18', | |
| protocolVersion: '2024-11-05', |
| export function loadConfig(root = findProjectRoot()) { | ||
| const configPath = path.join(root, 'vis.config.json') | ||
| const local = fs.existsSync(configPath) | ||
| ? JSON.parse(fs.readFileSync(configPath, 'utf-8')) | ||
| : {} | ||
| return normalizeConfig({ ...DEFAULT_CONFIG, ...local }) | ||
| } |
There was a problem hiding this comment.
If vis.config.json is malformed or contains syntax errors (such as trailing commas), JSON.parse will throw an unhandled exception and crash the CLI. Wrapping the parsing logic in a try-catch block ensures the tool falls back gracefully to default configuration with a warning.
export function loadConfig(root = findProjectRoot()) {
const configPath = path.join(root, 'vis.config.json')
let local = {}
if (fs.existsSync(configPath)) {
try {
local = JSON.parse(fs.readFileSync(configPath, 'utf-8'))
} catch (error) {
console.warn(`Warning: Failed to parse ${configPath}. Using default configuration.`, error.message)
}
}
return normalizeConfig({ ...DEFAULT_CONFIG, ...local })
}| const viewBox = text.match(/\bviewBox=["']([^"']+)["']/)?.[1]?.trim().split(/\s+/).map(Number) | ||
| if (viewBox?.length === 4) return { width: viewBox[2], height: viewBox[3], durationSeconds: null } |
There was a problem hiding this comment.
The current viewBox parsing logic splits strictly by spaces (\s+) and uses Number, which fails (returns NaN) if the values contain units (e.g., px, em) or are comma-separated. Splitting by /\s*,\s*|\s+/ and using parseFloat makes the parsing robust against standard SVG variations.
const viewBox = text.match(/\bviewBox=["']([^"']+)["']/)?.[1]?.trim().split(/[\s,]+/).map(parseFloat)
if (viewBox?.length === 4 && !viewBox.some(isNaN)) {
return { width: Math.round(viewBox[2]), height: Math.round(viewBox[3]), durationSeconds: null }
}| const ALLOWED_ROOTS = [ | ||
| ROOT, | ||
| ...(process.env.VIS_ALLOWED_ROOTS || '').split(path.delimiter).filter(Boolean), | ||
| ...(CONFIG.allowedRoots || []), | ||
| ].map(p => path.resolve(p).toLowerCase()) |
There was a problem hiding this comment.
Relative paths in CONFIG.allowedRoots are currently resolved against process.cwd(). If the CLI or server is started from a different directory, these paths will resolve incorrectly. Resolving them relative to ROOT ensures consistent behavior.
| const ALLOWED_ROOTS = [ | |
| ROOT, | |
| ...(process.env.VIS_ALLOWED_ROOTS || '').split(path.delimiter).filter(Boolean), | |
| ...(CONFIG.allowedRoots || []), | |
| ].map(p => path.resolve(p).toLowerCase()) | |
| const ALLOWED_ROOTS = [ | |
| ROOT, | |
| ...(process.env.VIS_ALLOWED_ROOTS || '').split(path.delimiter).filter(Boolean), | |
| ...(CONFIG.allowedRoots || []).map(p => path.resolve(ROOT, p)), | |
| ].map(p => path.resolve(p).toLowerCase()) |
1 participant
Summary
vis doctor/npm run doctorfor cross-machine install health, plus explicit Claude/Codex MCP setup stepsdocs/OPEN_SOURCE_TECH_RADAR.mdso future GitHub/project absorption has a license and adapter/core decision recordPR readiness hardening
vis.config.jsonwith a useful path/errorVIS_MCP_PROTOCOL_VERSIONviewBoxparsing edge casesnode:sqliteusage (>=22.13.0, Node 24+ recommended)Verification
npm run lintnpm testnode bin\vis.mjs doctor --jsonMain merge recommendation
Keep this PR as draft until Frank's other PC / Claude cross-check runs. Do not direct-push to
main.After the other PC confirms
npm run lint,npm test,node bin\vis.mjs doctor, dashboard rendering, MCP install, and no generateddata/files staged, this branch should be marked ready and merged intomain.Cross-check
See
README.mdanddocs/PROGRESS_REPORT_2026-06-26.mdfor other-PC Claude install and validation steps. Useorigin/claude/asset-os-coordinationas coordination/reference only; it should not be merged over this implementation branch without file-by-file review.