From 022592b806f9f2fd3ef6de11fdf0e11b91df5c26 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:18:58 +0530 Subject: [PATCH 01/11] fix(git): pass pathspecs verbatim after -- so special-char paths keep their own diff [AVCR-001] Co-Authored-By: Claude Sonnet 5.5 --- services/GitService.js | 17 ++++++++++++----- test/git-service-paths.test.js | 35 ++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 5 deletions(-) create mode 100644 test/git-service-paths.test.js diff --git a/services/GitService.js b/services/GitService.js index c3a6ee0..36169f9 100644 --- a/services/GitService.js +++ b/services/GitService.js @@ -30,15 +30,22 @@ class GitService { return reject(new Error(`Invalid git command type: ${commandType}`)); } - // Sanitize and validate arguments - const sanitizedArgs = args.filter(arg => { - if (typeof arg !== 'string') return false; - // Prevent command injection patterns + // execFile passes argv directly (no shell), so shell metacharacters are inert. + // Everything after a literal "--" is a pathspec and must be passed verbatim; + // only options before "--" are restricted. + const sepIndex = args.indexOf('--'); + const optionArgs = sepIndex === -1 ? args : args.slice(0, sepIndex); + const pathArgs = sepIndex === -1 ? [] : args.slice(sepIndex + 1); + + if (![...optionArgs, ...pathArgs].every(arg => typeof arg === 'string' && !arg.includes('\0'))) { + return reject(new Error('Invalid git argument')); + } + const sanitizedOptions = optionArgs.filter(arg => { if (/[;&|`$(){}[\]\\]/.test(arg)) return false; - // Prevent path traversal (allow relative paths within repo) if (arg.includes('..') && !arg.startsWith('./')) return false; return true; }); + const sanitizedArgs = sepIndex === -1 ? sanitizedOptions : [...sanitizedOptions, '--', ...pathArgs]; const gitCommand = baseCommand[0]; const gitArgs = [...baseCommand.slice(1), ...sanitizedArgs]; diff --git a/test/git-service-paths.test.js b/test/git-service-paths.test.js new file mode 100644 index 0000000..9f56374 --- /dev/null +++ b/test/git-service-paths.test.js @@ -0,0 +1,35 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); +const GitService = require('../services/GitService'); + +describe('GitService special-character paths [AVCR-001]', () => { + const origCwd = process.cwd(); + let dir; + const names = ['app/[id]/page.tsx', 'a(b)/c.txt', 'x$y{z}.txt', 'plain.txt']; + + beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'avcr-gs-')); + const git = (...a) => execFileSync('git', a, { cwd: dir, stdio: 'pipe' }); + git('init', '-q'); + for (const n of names) { + fs.mkdirSync(path.dirname(path.join(dir, n)), { recursive: true }); + fs.writeFileSync(path.join(dir, n), `content of ${n}\n`); + } + git('add', '-A'); + process.chdir(dir); + }); + afterAll(() => { + process.chdir(origCwd); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test.each(names)('getDiffForFile returns only the diff for %s', async (n) => { + const diff = await GitService.getDiffForFile(n); + expect(diff).toContain(`content of ${n}`); + for (const other of names.filter(x => x !== n)) { + expect(diff).not.toContain(`content of ${other}`); + } + }); +}); From 4529da881815db75a62cfba92862edae4d543f75 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:19:47 +0530 Subject: [PATCH 02/11] fix(export): include reviewer comments in unified review [AVCR-002] Co-Authored-By: Claude Sonnet 5.5 --- server.js | 2 ++ services/ReviewGenerator.js | 17 +++++++++++++- test/unified-review-comments.test.js | 33 ++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 1 deletion(-) create mode 100644 test/unified-review-comments.test.js diff --git a/server.js b/server.js index 48cd20e..f7d78fe 100644 --- a/server.js +++ b/server.js @@ -619,6 +619,8 @@ app.post('/api/export-for-ai', exportRateLimit, handleAsyncRoute(async (req, res const result = await ReviewGenerator.generateUnifiedReview({ includedFiles, excludedFiles, + comments, + lineComments, // Pass other options if needed by generateUnifiedReview, // though currently it handles content generation primarily. }); diff --git a/services/ReviewGenerator.js b/services/ReviewGenerator.js index 9272b56..ab6ca9e 100644 --- a/services/ReviewGenerator.js +++ b/services/ReviewGenerator.js @@ -95,9 +95,11 @@ class ReviewGenerator { * @param {Array} options.includedFiles - List of files to process * @param {Array} options.excludedFiles - List of excluded files * @param {Array} options.largeFiles - List of skipped large files + * @param {Object} options.comments - File-level comments keyed by file + * @param {Object} options.lineComments - Line-level comments * @returns {Promise} Result stats and content */ - static async generateUnifiedReview({ includedFiles, excludedFiles = [], largeFiles = [] }) { + static async generateUnifiedReview({ includedFiles, excludedFiles = [], largeFiles = [], comments = {}, lineComments = {} }) { const timestamp = new Date().toLocaleString(); let content = `# πŸ” Code Review - ${timestamp}\n\n`; content += '**Project:** AI Visual Code Review\n'; @@ -171,6 +173,19 @@ class ReviewGenerator { fileContent += fileTypeMap[ext] || '**Type:** Source File πŸ“„\n\n'; } + if (comments && comments[file]) { + fileContent += `## πŸ’­ Review Comment\n\n${comments[file]}\n\n`; + } + const fileLineComments = Object.entries(lineComments || {}) + .filter(([lineId]) => lineId.includes(file.replace(/[^a-zA-Z0-9]/g, '_'))); + if (fileLineComments.length > 0) { + fileContent += '## πŸ” Line Comments\n\n'; + fileLineComments.forEach(([lineId, comment]) => { + fileContent += `- **${lineId}:** ${comment}\n`; + }); + fileContent += '\n'; + } + // Get diff try { const diff = await GitService.getDiffForFile(file); diff --git a/test/unified-review-comments.test.js b/test/unified-review-comments.test.js new file mode 100644 index 0000000..4039bb9 --- /dev/null +++ b/test/unified-review-comments.test.js @@ -0,0 +1,33 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const { execFileSync } = require('child_process'); +const ReviewGenerator = require('../services/ReviewGenerator'); + +describe('unified review includes reviewer comments [AVCR-002]', () => { + const origCwd = process.cwd(); + let dir; + beforeAll(() => { + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'avcr-ur-')); + fs.writeFileSync(path.join(dir, 'a.js'), 'const a = 1;\n'); + execFileSync('git', ['init', '-q'], { cwd: dir }); + execFileSync('git', ['add', '-A'], { cwd: dir }); + process.chdir(dir); + }); + afterAll(() => { + process.chdir(origCwd); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test('file and line comments appear in AI_REVIEW.md', async () => { + const lineId = `${'a.js'.replace(/[^a-zA-Z0-9]/g, '_')}_line_1`; + await ReviewGenerator.generateUnifiedReview({ + includedFiles: ['a.js'], + comments: { 'a.js': 'FILE-COMMENT-MARKER' }, + lineComments: { [lineId]: 'LINE-COMMENT-MARKER' } + }); + const out = fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8'); + expect(out).toContain('FILE-COMMENT-MARKER'); + expect(out).toContain('LINE-COMMENT-MARKER'); + }); +}); From bd3ec1da55ae2e4e87260f039315a82ea0d54769 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 15:20:48 +0530 Subject: [PATCH 03/11] fix(server): bind 127.0.0.1 by default, HOST env for Docker, publish loopback only [AVCR-005] Co-Authored-By: Claude Sonnet 5.5 --- Dockerfile | 1 + docker-compose.yml | 6 ++++-- server.js | 5 ++++- test/bind-host.test.js | 23 +++++++++++++++++++++++ 4 files changed, 32 insertions(+), 3 deletions(-) create mode 100644 test/bind-host.test.js diff --git a/Dockerfile b/Dockerfile index 6df58b7..d753794 100644 --- a/Dockerfile +++ b/Dockerfile @@ -54,6 +54,7 @@ HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ # Set environment ENV NODE_ENV=production ENV PORT=3002 +ENV HOST=0.0.0.0 # Start application CMD ["node", "server.js"] diff --git a/docker-compose.yml b/docker-compose.yml index 30b0bbc..2da3057 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,10 +7,11 @@ services: dockerfile: Dockerfile container_name: ai-visual-code-review ports: - - "3002:3002" + - "127.0.0.1:3002:3002" environment: - NODE_ENV=production - PORT=3002 + - HOST=0.0.0.0 volumes: # Mount current directory for reviewing code - .:/workspace:ro @@ -31,10 +32,11 @@ services: target: builder container_name: ai-visual-code-review-dev ports: - - "3002:3002" + - "127.0.0.1:3002:3002" environment: - NODE_ENV=development - PORT=3002 + - HOST=0.0.0.0 volumes: - .:/app - /app/node_modules diff --git a/server.js b/server.js index f7d78fe..03df41b 100644 --- a/server.js +++ b/server.js @@ -12,6 +12,8 @@ const GitService = require('./services/GitService'); const app = express(); const PORT = process.env.PORT || 3002; +// Bind to loopback by default; the server is unauthenticated. Set HOST=0.0.0.0 (e.g. in Docker) to expose it. +const HOST = process.env.HOST || '127.0.0.1'; // Configuration const CONFIG = { @@ -775,7 +777,7 @@ app.use('*', (req, res) => { // Start server only if not in test mode if (process.env.NODE_ENV !== 'test') { - app.listen(PORT, () => { + app.listen(PORT, HOST, () => { console.log('πŸ” AI Visual Code Review Server'); console.log('==============================='); console.log(`🌐 Server running at: http://localhost:${PORT}`); @@ -802,4 +804,5 @@ app.resetRateLimit = () => { rateLimitStore.clear(); }; +app.HOST = HOST; module.exports = app; diff --git a/test/bind-host.test.js b/test/bind-host.test.js new file mode 100644 index 0000000..4f1a948 --- /dev/null +++ b/test/bind-host.test.js @@ -0,0 +1,23 @@ +const fs = require('fs'); +const path = require('path'); +const { execFileSync } = require('child_process'); + +describe('server bind address [AVCR-005]', () => { + const root = path.join(__dirname, '..'); + const probe = "process.env.NODE_ENV='test';console.log(require('./server').HOST)"; + + test('defaults to loopback', () => { + const env = { ...process.env }; delete env.HOST; + expect(execFileSync('node', ['-e', probe], { cwd: root, env }).toString().trim()).toBe('127.0.0.1'); + }); + test('HOST env overrides', () => { + const env = { ...process.env, HOST: '0.0.0.0' }; + expect(execFileSync('node', ['-e', probe], { cwd: root, env }).toString().trim()).toBe('0.0.0.0'); + }); + test('server passes HOST to listen and compose publishes loopback only', () => { + expect(fs.readFileSync(path.join(root, 'server.js'), 'utf8')).toMatch(/app\.listen\(PORT, HOST/); + const c = fs.readFileSync(path.join(root, 'docker-compose.yml'), 'utf8'); + expect(c).not.toMatch(/- "3002:3002"/); + expect(c).toMatch(/127\.0\.0\.1:3002:3002/); + }); +}); From 91b58b6808874b241262855ed63aadf976f60a87 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:43:26 +0530 Subject: [PATCH 04/11] fix(git): match paths literally and reject magic pathspecs [review: MAJOR-1] Paths after "--" were still read as glob/magic pathspecs, so the diff for app/[id]/page.tsx also contained app/i/page.tsx and app/d/page.tsx, a*b.txt pulled in aXb.txt, and ":/" or ":(exclude)x" reached git. GitService now passes the global --literal-pathspecs option on every command and rejects path args with a leading ":" (git's magic prefix); the web-view path check rejects a leading ":" as well. The path test fixture now has glob-matching siblings (MINOR-4) and magic-pathspec cases. Co-Authored-By: Claude Opus 5.5 --- services/GitService.js | 18 +++++++++++++++++- services/diffService.js | 5 +++++ test/diffService.test.js | 7 +++++++ test/git-service-paths.test.js | 19 ++++++++++++++++++- test/server.test.js | 5 ++++- 5 files changed, 51 insertions(+), 3 deletions(-) diff --git a/services/GitService.js b/services/GitService.js index 36169f9..ba9a0a7 100644 --- a/services/GitService.js +++ b/services/GitService.js @@ -40,6 +40,11 @@ class GitService { if (![...optionArgs, ...pathArgs].every(arg => typeof arg === 'string' && !arg.includes('\0'))) { return reject(new Error('Invalid git argument')); } + // A leading ":" introduces pathspec magic (":/", ":(exclude)x", ":!x", ...). + // Literal mode below already disables it; reject explicitly as well. + if (pathArgs.some(GitService.isMagicPathspec)) { + return reject(new Error('Magic pathspecs are not allowed')); + } const sanitizedOptions = optionArgs.filter(arg => { if (/[;&|`$(){}[\]\\]/.test(arg)) return false; if (arg.includes('..') && !arg.startsWith('./')) return false; @@ -48,7 +53,9 @@ class GitService { const sanitizedArgs = sepIndex === -1 ? sanitizedOptions : [...sanitizedOptions, '--', ...pathArgs]; const gitCommand = baseCommand[0]; - const gitArgs = [...baseCommand.slice(1), ...sanitizedArgs]; + // --literal-pathspecs (a global option, so it precedes the subcommand) makes git + // match each path exactly: no glob ("[id]", "*", "?") and no ":" magic. + const gitArgs = ['--literal-pathspecs', ...baseCommand.slice(1), ...sanitizedArgs]; const commandDisplay = [gitCommand, ...gitArgs].join(' '); execFile(gitCommand, gitArgs, { @@ -67,6 +74,15 @@ class GitService { }); } + /** + * True if a path would be parsed by git as magic pathspec syntax (leading ":"). + * @param {string} p + * @returns {boolean} + */ + static isMagicPathspec(p) { + return typeof p === 'string' && p.startsWith(':'); + } + /** * Get list of staged files * @returns {Promise} diff --git a/services/diffService.js b/services/diffService.js index 718254c..fac90af 100644 --- a/services/diffService.js +++ b/services/diffService.js @@ -132,6 +132,11 @@ class DiffService { return false; } + // A leading ":" is git pathspec magic (":/" = whole repo, ":(exclude)x", ":!x") + if (filePath.startsWith(':')) { + return false; + } + // Prevent dangerous characters that could be used for command injection const dangerousChars = /[;&|`$(){}[\]<>'"]/; if (dangerousChars.test(filePath)) { diff --git a/test/diffService.test.js b/test/diffService.test.js index 4431428..d2172b1 100644 --- a/test/diffService.test.js +++ b/test/diffService.test.js @@ -163,5 +163,12 @@ describe('DiffService', () => { expect(DiffService.isValidFilePath(path)).toBe(false); }); }); + + test('should reject git magic pathspecs [AVCR-001]', () => { + [':/', ':!src/index.js', ':^src/index.js', ':(exclude)src/index.js', ':(glob)**', ':'].forEach(p => { + expect(DiffService.isValidFilePath(p)).toBe(false); + }); + expect(DiffService.isValidFilePath('src/a:b.js')).toBe(true); + }); }); }); diff --git a/test/git-service-paths.test.js b/test/git-service-paths.test.js index 9f56374..ba0e4ff 100644 --- a/test/git-service-paths.test.js +++ b/test/git-service-paths.test.js @@ -7,7 +7,13 @@ const GitService = require('../services/GitService'); describe('GitService special-character paths [AVCR-001]', () => { const origCwd = process.cwd(); let dir; - const names = ['app/[id]/page.tsx', 'a(b)/c.txt', 'x$y{z}.txt', 'plain.txt']; + // Siblings are chosen so that, read as globs, one name matches another: + // "app/[id]/page.tsx" globs app/i/page.tsx and app/d/page.tsx; "a*b.txt" and "a?b.txt" glob aXb.txt. + const names = [ + 'app/[id]/page.tsx', 'app/i/page.tsx', 'app/d/page.tsx', + 'a*b.txt', 'a?b.txt', 'aXb.txt', + 'a(b)/c.txt', 'x$y{z}.txt', 'plain.txt' + ]; beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'avcr-gs-')); @@ -32,4 +38,15 @@ describe('GitService special-character paths [AVCR-001]', () => { expect(diff).not.toContain(`content of ${other}`); } }); + + test.each([':/', ':(exclude)plain.txt', ':!plain.txt', ':^plain.txt', ':(glob)*'])( + 'magic pathspec %s is rejected', async (spec) => { + await expect(GitService.getDiffForFile(spec)).rejects.toThrow(/Magic pathspecs/); + await expect(GitService.execute('diff-cached', ['--', spec])).rejects.toThrow(/Magic pathspecs/); + }); + + test('a path that is not staged returns an empty diff, not other files', async () => { + expect(await GitService.getDiffForFile('app/[x]/page.tsx')).toBe(''); + expect(await GitService.getDiffForFile('a*')).toBe(''); + }); }); diff --git a/test/server.test.js b/test/server.test.js index 5e2441a..8bf013f 100644 --- a/test/server.test.js +++ b/test/server.test.js @@ -71,7 +71,10 @@ describe('AI Visual Code Review Server', () => { '/etc/passwd', 'file', 'file|rm -rf /', - 'file;cat /etc/passwd' + 'file;cat /etc/passwd', + ':/', + ':!README.md', + ':(exclude)README.md' ]; for (const file of maliciousFiles) { From a6ad3e965e632da171d91b42e633e54ebca62c4e Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:44:43 +0530 Subject: [PATCH 05/11] fix(export): match line comments to their exact file in both generators [review: MINOR-1] Line comments were selected with lineId.includes(sanitizedFile), so a.js also received data.js's comments and index.js received src/index.js's. Both the unified and the per-file generator now use one helper that matches the whole client lineId (`__`). The test uses the real client lineId format and asserts isolation between files (N2). Co-Authored-By: Claude Opus 5.5 --- services/ReviewGenerator.js | 20 +++++++++--- test/unified-review-comments.test.js | 46 ++++++++++++++++++++++++++-- 2 files changed, 59 insertions(+), 7 deletions(-) diff --git a/services/ReviewGenerator.js b/services/ReviewGenerator.js index ab6ca9e..fb627ab 100644 --- a/services/ReviewGenerator.js +++ b/services/ReviewGenerator.js @@ -176,8 +176,7 @@ class ReviewGenerator { if (comments && comments[file]) { fileContent += `## πŸ’­ Review Comment\n\n${comments[file]}\n\n`; } - const fileLineComments = Object.entries(lineComments || {}) - .filter(([lineId]) => lineId.includes(file.replace(/[^a-zA-Z0-9]/g, '_'))); + const fileLineComments = this.getLineCommentsForFile(file, lineComments); if (fileLineComments.length > 0) { fileContent += '## πŸ” Line Comments\n\n'; fileLineComments.forEach(([lineId, comment]) => { @@ -237,6 +236,20 @@ class ReviewGenerator { }; } + /** + * Line comments that belong to exactly this file. The client builds lineIds as + * `${file.replace(/[^a-zA-Z0-9]/g, '_')}_${chunkIndex}_${lineIndex}`, so match the whole + * id: a substring test would also give `data.js`'s comments to `a.js`. + * @param {string} file + * @param {Object} lineComments - lineId -> comment + * @returns {Array<[string, string]>} + */ + static getLineCommentsForFile(file, lineComments) { + const prefix = file.replace(/[^a-zA-Z0-9]/g, '_'); + const idPattern = new RegExp(`^${prefix}_\\d+_\\d+$`); + return Object.entries(lineComments || {}).filter(([lineId]) => idPattern.test(lineId)); + } + /** * Generate content for a single file review */ @@ -272,8 +285,7 @@ class ReviewGenerator { } // Line Comments - const fileLineComments = Object.entries(lineComments || {}) - .filter(([lineId]) => lineId.includes(file.replace(/[^a-zA-Z0-9]/g, '_'))); + const fileLineComments = this.getLineCommentsForFile(file, lineComments); if (fileLineComments.length > 0) { content += '## πŸ” Line Comments\n\n'; diff --git a/test/unified-review-comments.test.js b/test/unified-review-comments.test.js index 4039bb9..1f67291 100644 --- a/test/unified-review-comments.test.js +++ b/test/unified-review-comments.test.js @@ -4,12 +4,26 @@ const path = require('path'); const { execFileSync } = require('child_process'); const ReviewGenerator = require('../services/ReviewGenerator'); +// Same format the web client uses (public/index.html renderDiff). +const lineIdFor = (file, chunk, line) => `${file.replace(/[^a-zA-Z0-9]/g, '_')}_${chunk}_${line}`; + +// Export text from the first mention of `file` up to the first mention of `nextFile`. +function sectionFor(out, file, nextFile) { + const start = out.indexOf(`\`${file}\``); + const end = nextFile ? out.indexOf(`\`${nextFile}\``, start + 1) : out.length; + return out.slice(start, end); +} + describe('unified review includes reviewer comments [AVCR-002]', () => { const origCwd = process.cwd(); let dir; + const files = ['a.js', 'data.js', 'index.js', 'src/index.js']; beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'avcr-ur-')); - fs.writeFileSync(path.join(dir, 'a.js'), 'const a = 1;\n'); + for (const f of files) { + fs.mkdirSync(path.dirname(path.join(dir, f)), { recursive: true }); + fs.writeFileSync(path.join(dir, f), `// ${f}\n`); + } execFileSync('git', ['init', '-q'], { cwd: dir }); execFileSync('git', ['add', '-A'], { cwd: dir }); process.chdir(dir); @@ -19,15 +33,41 @@ describe('unified review includes reviewer comments [AVCR-002]', () => { fs.rmSync(dir, { recursive: true, force: true }); }); + const lineComments = Object.fromEntries(files.map(f => [lineIdFor(f, 0, 1), `LINE-${f}`])); + test('file and line comments appear in AI_REVIEW.md', async () => { - const lineId = `${'a.js'.replace(/[^a-zA-Z0-9]/g, '_')}_line_1`; await ReviewGenerator.generateUnifiedReview({ includedFiles: ['a.js'], comments: { 'a.js': 'FILE-COMMENT-MARKER' }, - lineComments: { [lineId]: 'LINE-COMMENT-MARKER' } + lineComments: { [lineIdFor('a.js', 0, 1)]: 'LINE-COMMENT-MARKER' } }); const out = fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8'); expect(out).toContain('FILE-COMMENT-MARKER'); expect(out).toContain('LINE-COMMENT-MARKER'); }); + + test('unified export puts each line comment under its own file only [review: MINOR-1]', async () => { + await ReviewGenerator.generateUnifiedReview({ includedFiles: files, lineComments }); + const out = fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8'); + for (const f of files) { + expect(out.split(`LINE-${f}\n`).length - 1).toBe(1); + } + files.forEach((f, i) => { + const section = sectionFor(out, f, files[i + 1]); + expect(section).toContain(`LINE-${f}\n`); + for (const other of files.filter(x => x !== f)) { + expect(section).not.toContain(`LINE-${other}\n`); + } + }); + }); + + test('per-file export puts each line comment under its own file only [review: MINOR-1]', async () => { + for (const f of files) { + const content = await ReviewGenerator.generateFileContent(f, undefined, lineComments); + expect(content).toContain(`LINE-${f}\n`); + for (const other of files.filter(x => x !== f)) { + expect(content).not.toContain(`LINE-${other}\n`); + } + } + }); }); From ec4d1da876e6d191c065795b78838fca5e3e1281 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:46:55 +0530 Subject: [PATCH 06/11] fix(web): allow [ ] ( ) { } $ in reviewed file paths [review: MINOR-2] The web view rejected app/[id]/page.tsx with 400 "potential security risk", and file comments on such paths failed export validation. Git runs via execFile with --literal-pathspecs, so these characters are inert; drop them from isValidFilePath's blocklist (; & | ` < > ' " and the other checks stay). New supertest suite serves each special path from a temp repo and checks that only its own diff comes back and that a comment on app/[id]/page.tsx reaches AI_REVIEW.md. Co-Authored-By: Claude Opus 5.5 --- services/diffService.js | 6 +++-- test/diffService.test.js | 7 +++++- test/special-path-web.test.js | 46 +++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 3 deletions(-) create mode 100644 test/special-path-web.test.js diff --git a/services/diffService.js b/services/diffService.js index fac90af..9a70b95 100644 --- a/services/diffService.js +++ b/services/diffService.js @@ -137,8 +137,10 @@ class DiffService { return false; } - // Prevent dangerous characters that could be used for command injection - const dangerousChars = /[;&|`$(){}[\]<>'"]/; + // Reject characters with no legitimate use in a reviewed path. Git is run via execFile + // (no shell) with --literal-pathspecs, so [ ] ( ) { } $ are inert and are allowed: + // Next.js routes such as app/[id]/page.tsx or app/(group)/page.tsx must stay reviewable. + const dangerousChars = /[;&|`<>'"]/; if (dangerousChars.test(filePath)) { return false; } diff --git a/test/diffService.test.js b/test/diffService.test.js index d2172b1..8b55ca9 100644 --- a/test/diffService.test.js +++ b/test/diffService.test.js @@ -153,7 +153,6 @@ describe('DiffService', () => { 'file;rm -rf /', 'file|cat /etc/passwd', 'file`whoami`', - 'file$(whoami)', 'file&&rm -rf /', 'file||echo hack', 'file' @@ -164,6 +163,12 @@ describe('DiffService', () => { }); }); + test('should accept bracket, paren, brace and dollar paths [review: MINOR-2]', () => { + ['app/[id]/page.tsx', 'app/(marketing)/page.tsx', 'x$y{z}.txt', 'file$(whoami)'].forEach(p => { + expect(DiffService.isValidFilePath(p)).toBe(true); + }); + }); + test('should reject git magic pathspecs [AVCR-001]', () => { [':/', ':!src/index.js', ':^src/index.js', ':(exclude)src/index.js', ':(glob)**', ':'].forEach(p => { expect(DiffService.isValidFilePath(p)).toBe(false); diff --git a/test/special-path-web.test.js b/test/special-path-web.test.js new file mode 100644 index 0000000..0a34d7c --- /dev/null +++ b/test/special-path-web.test.js @@ -0,0 +1,46 @@ +const fs = require('fs'); +const os = require('os'); +const path = require('path'); +const request = require('supertest'); +const { execFileSync } = require('child_process'); +const app = require('../server'); + +// The web view must serve and accept comments on Next.js-style paths [review: MINOR-2]. +describe('web view special-character paths', () => { + const origCwd = process.cwd(); + let dir; + const names = ['app/[id]/page.tsx', 'app/i/page.tsx', 'app/(group)/x.tsx', 'x$y{z}.txt']; + + beforeAll(() => { + process.env.NODE_ENV = 'test'; + dir = fs.mkdtempSync(path.join(os.tmpdir(), 'avcr-web-')); + for (const n of names) { + fs.mkdirSync(path.dirname(path.join(dir, n)), { recursive: true }); + fs.writeFileSync(path.join(dir, n), `content of ${n}\n`); + } + execFileSync('git', ['init', '-q'], { cwd: dir }); + execFileSync('git', ['add', '-A'], { cwd: dir }); + process.chdir(dir); + }); + beforeEach(() => app.resetRateLimit && app.resetRateLimit()); + afterAll(() => { + process.chdir(origCwd); + fs.rmSync(dir, { recursive: true, force: true }); + }); + + test.each(names)('GET /api/file-diff serves only %s', async (n) => { + const res = await request(app).get('/api/file-diff').query({ file: n }).expect(200); + expect(res.body.diff).toContain(`content of ${n}`); + for (const other of names.filter(x => x !== n)) { + expect(res.body.diff).not.toContain(`content of ${other}`); + } + }); + + test('POST /api/export-for-ai accepts a file comment on app/[id]/page.tsx', async () => { + await request(app) + .post('/api/export-for-ai') + .send({ comments: { 'app/[id]/page.tsx': 'BRACKET-COMMENT' }, lineComments: {}, excludedFiles: [] }) + .expect(200); + expect(fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8')).toContain('BRACKET-COMMENT'); + }); +}); From 0ecb0c6c5a0c2dff29d075296899be9d562085d3 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:47:52 +0530 Subject: [PATCH 07/11] fix(docker): healthchecks probe 127.0.0.1 instead of localhost [review: MINOR-3] The server now binds 0.0.0.0 (IPv4) in the container. On node:18-alpine, "localhost" can resolve to ::1 first and Node 18 does not fall back, so the healthcheck could get ECONNREFUSED and mark the container unhealthy. Use the IPv4 loopback address in both the Dockerfile and compose healthchecks. Co-Authored-By: Claude Opus 5.5 --- Dockerfile | 2 +- docker-compose.yml | 2 +- test/bind-host.test.js | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index d753794..55e3684 100644 --- a/Dockerfile +++ b/Dockerfile @@ -49,7 +49,7 @@ EXPOSE 3002 # Health check HEALTHCHECK --interval=30s --timeout=3s --start-period=5s --retries=3 \ - CMD node -e "require('http').get('http://localhost:3002/api/health', (r) => process.exit(r.statusCode === 200 ? 0 : 1))" + CMD node -e "require('http').get('http://127.0.0.1:3002/api/health', (r) => process.exit(r.statusCode === 200 ? 0 : 1))" # Set environment ENV NODE_ENV=production diff --git a/docker-compose.yml b/docker-compose.yml index 2da3057..9211431 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -18,7 +18,7 @@ services: working_dir: /workspace restart: unless-stopped healthcheck: - test: ["CMD", "node", "-e", "require('http').get('http://localhost:3002/api/health', (r) => process.exit(r.statusCode === 200 ? 0 : 1))"] + test: ["CMD", "node", "-e", "require('http').get('http://127.0.0.1:3002/api/health', (r) => process.exit(r.statusCode === 200 ? 0 : 1))"] interval: 30s timeout: 3s retries: 3 diff --git a/test/bind-host.test.js b/test/bind-host.test.js index 4f1a948..fb86de3 100644 --- a/test/bind-host.test.js +++ b/test/bind-host.test.js @@ -20,4 +20,15 @@ describe('server bind address [AVCR-005]', () => { expect(c).not.toMatch(/- "3002:3002"/); expect(c).toMatch(/127\.0\.0\.1:3002:3002/); }); + // The server binds IPv4 only; "localhost" can resolve to ::1 first in node:18-alpine. + test('Docker healthchecks probe 127.0.0.1, not localhost [review: MINOR-3]', () => { + for (const f of ['Dockerfile', 'docker-compose.yml']) { + const hc = fs.readFileSync(path.join(root, f), 'utf8').split('\n').filter(l => l.includes('/api/health')); + expect(hc.length).toBeGreaterThan(0); + hc.forEach(l => { + expect(l).toContain('http://127.0.0.1:3002/api/health'); + expect(l).not.toContain('localhost'); + }); + } + }); }); From 79edb977e6f2db0cc74cf48ce7ba787ba9425767 Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 18:56:48 +0530 Subject: [PATCH 08/11] fix(export): make line-comment ids unique per file [review: codex-security P2] The client lineId replaced every non-alphanumeric character with "_", so a-b.js and a_b.js (or src/x.js and src_x.js) shared a prefix. A comment on one went to both, including when the other file was excluded from the export. Client and server now encode each non-alphanumeric character as __, which is injective and still safe in DOM ids. The test runs the client's own lineIdPrefix() from public/index.html against the server's, and checks that an excluded file's comment cannot leak. Co-Authored-By: Claude Opus 5.5 --- public/index.html | 8 +++++++- services/ReviewGenerator.js | 18 ++++++++++++++---- test/unified-review-comments.test.js | 27 ++++++++++++++++++++++++--- 3 files changed, 45 insertions(+), 8 deletions(-) diff --git a/public/index.html b/public/index.html index 9d62d07..bfd3055 100644 --- a/public/index.html +++ b/public/index.html @@ -1435,6 +1435,12 @@

No Staged Changes

if (lineContainer) lineContainer.classList.remove('has-line-comment'); } + // Must match ReviewGenerator.lineIdPrefix on the server: each non-alphanumeric + // character becomes __, so distinct paths never share a lineId prefix. + function lineIdPrefix(filename) { + return filename.replace(/[^a-zA-Z0-9]/g, c => '_' + c.charCodeAt(0).toString(16) + '_'); + } + function renderDiff(container, diff, filename, parsedDiff) { if (!diff.trim() || !parsedDiff?.chunks?.length) { container.innerHTML = ` @@ -1453,7 +1459,7 @@

No Staged Changes

html += `
${escapeHtml(chunk.header)}
`; chunk.lines.forEach((line, lineIndex) => { - const lineId = `${filename.replace(/[^a-zA-Z0-9]/g, '_')}_${chunkIndex}_${lineIndex}`; + const lineId = `${lineIdPrefix(filename)}_${chunkIndex}_${lineIndex}`; const oldNum = line.oldLineNum; const newNum = line.newLineNum; diff --git a/services/ReviewGenerator.js b/services/ReviewGenerator.js index fb627ab..febeb32 100644 --- a/services/ReviewGenerator.js +++ b/services/ReviewGenerator.js @@ -236,17 +236,27 @@ class ReviewGenerator { }; } + /** + * File part of a client lineId. Must match lineIdPrefix() in public/index.html. + * Each non-alphanumeric character becomes `__`, so the mapping is injective: + * `a-b.js` and `a_b.js`, or `src/x.js` and `src_x.js`, get different prefixes. + * @param {string} file + * @returns {string} + */ + static lineIdPrefix(file) { + return file.replace(/[^a-zA-Z0-9]/g, c => `_${c.charCodeAt(0).toString(16)}_`); + } + /** * Line comments that belong to exactly this file. The client builds lineIds as - * `${file.replace(/[^a-zA-Z0-9]/g, '_')}_${chunkIndex}_${lineIndex}`, so match the whole - * id: a substring test would also give `data.js`'s comments to `a.js`. + * `${lineIdPrefix(file)}_${chunkIndex}_${lineIndex}`, so match the whole id: a substring + * test would also give `data.js`'s comments to `a.js`. * @param {string} file * @param {Object} lineComments - lineId -> comment * @returns {Array<[string, string]>} */ static getLineCommentsForFile(file, lineComments) { - const prefix = file.replace(/[^a-zA-Z0-9]/g, '_'); - const idPattern = new RegExp(`^${prefix}_\\d+_\\d+$`); + const idPattern = new RegExp(`^${this.lineIdPrefix(file)}_\\d+_\\d+$`); return Object.entries(lineComments || {}).filter(([lineId]) => idPattern.test(lineId)); } diff --git a/test/unified-review-comments.test.js b/test/unified-review-comments.test.js index 1f67291..0b4012a 100644 --- a/test/unified-review-comments.test.js +++ b/test/unified-review-comments.test.js @@ -4,8 +4,13 @@ const path = require('path'); const { execFileSync } = require('child_process'); const ReviewGenerator = require('../services/ReviewGenerator'); -// Same format the web client uses (public/index.html renderDiff). -const lineIdFor = (file, chunk, line) => `${file.replace(/[^a-zA-Z0-9]/g, '_')}_${chunk}_${line}`; +// Use the web client's own lineIdPrefix() from public/index.html, so the test breaks if +// client and server encodings drift apart. +const html = fs.readFileSync(path.join(__dirname, '..', 'public', 'index.html'), 'utf8'); +const clientFn = html.match(/function lineIdPrefix\(filename\) \{([\s\S]*?)\n\s*\}/); +// eslint-disable-next-line no-new-func +const clientLineIdPrefix = new Function('filename', clientFn[1]); +const lineIdFor = (file, chunk, line) => `${clientLineIdPrefix(file)}_${chunk}_${line}`; // Export text from the first mention of `file` up to the first mention of `nextFile`. function sectionFor(out, file, nextFile) { @@ -17,7 +22,8 @@ function sectionFor(out, file, nextFile) { describe('unified review includes reviewer comments [AVCR-002]', () => { const origCwd = process.cwd(); let dir; - const files = ['a.js', 'data.js', 'index.js', 'src/index.js']; + // Pairs that collided under the old lossy encoding: a-b.js/a_b.js, src/x.js/src_x.js. + const files = ['a.js', 'data.js', 'index.js', 'src/index.js', 'a-b.js', 'a_b.js', 'src/x.js', 'src_x.js']; beforeAll(() => { dir = fs.mkdtempSync(path.join(os.tmpdir(), 'avcr-ur-')); for (const f of files) { @@ -61,6 +67,21 @@ describe('unified review includes reviewer comments [AVCR-002]', () => { }); }); + test('a comment on an excluded file does not leak into a same-looking included file [codex P2]', async () => { + await ReviewGenerator.generateUnifiedReview({ + includedFiles: ['a_b.js'], excludedFiles: ['a-b.js'], + lineComments: { [lineIdFor('a-b.js', 0, 1)]: 'EXCLUDED-SECRET' } + }); + expect(fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8')).not.toContain('EXCLUDED-SECRET'); + }); + + test('client and server agree on the lineId prefix', () => { + for (const f of files.concat(['app/[id]/page.tsx', 'x$y{z}.txt', 'naΓ―ve.js'])) { + expect(ReviewGenerator.lineIdPrefix(f)).toBe(clientLineIdPrefix(f)); + expect(ReviewGenerator.lineIdPrefix(f)).toMatch(/^[a-zA-Z0-9_]+$/); + } + }); + test('per-file export puts each line comment under its own file only [review: MINOR-1]', async () => { for (const f of files) { const content = await ReviewGenerator.generateFileContent(f, undefined, lineComments); From 149edaf4aff37bd909060cd8c398776fb5a9bfdd Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:37:20 +0530 Subject: [PATCH 09/11] fix(export): read only own file-comment keys [codex: Check ownership before reading file comments] A staged file named constructor, toString or another Object.prototype key picked up the inherited member as its review comment in both the unified and the per-file export. ReviewGenerator.getFileComment now checks hasOwnProperty, and both generators use it. Co-Authored-By: Claude Opus 5.5 --- services/ReviewGenerator.js | 18 +++++++++++++++--- test/unified-review-comments.test.js | 22 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/services/ReviewGenerator.js b/services/ReviewGenerator.js index febeb32..ed8e807 100644 --- a/services/ReviewGenerator.js +++ b/services/ReviewGenerator.js @@ -52,7 +52,7 @@ class ReviewGenerator { const reviewFileName = `review-${safeName}.md`; const reviewFilePath = path.join(reviewPath, reviewFileName); - const content = await this.generateFileContent(file, comments[file], lineComments); + const content = await this.generateFileContent(file, this.getFileComment(comments, file), lineComments); await fs.promises.writeFile(reviewFilePath, content, 'utf8'); return { success: true, file, reviewFileName }; @@ -173,8 +173,9 @@ class ReviewGenerator { fileContent += fileTypeMap[ext] || '**Type:** Source File πŸ“„\n\n'; } - if (comments && comments[file]) { - fileContent += `## πŸ’­ Review Comment\n\n${comments[file]}\n\n`; + const fileComment = this.getFileComment(comments, file); + if (fileComment) { + fileContent += `## πŸ’­ Review Comment\n\n${fileComment}\n\n`; } const fileLineComments = this.getLineCommentsForFile(file, lineComments); if (fileLineComments.length > 0) { @@ -236,6 +237,17 @@ class ReviewGenerator { }; } + /** + * The reviewer's comment for `file`, or undefined. Only own keys count: a file named + * `constructor` or `toString` must not pick up an inherited Object.prototype member. + * @param {Object} comments - file -> comment + * @param {string} file + * @returns {string|undefined} + */ + static getFileComment(comments, file) { + return comments && Object.prototype.hasOwnProperty.call(comments, file) ? comments[file] : undefined; + } + /** * File part of a client lineId. Must match lineIdPrefix() in public/index.html. * Each non-alphanumeric character becomes `__`, so the mapping is injective: diff --git a/test/unified-review-comments.test.js b/test/unified-review-comments.test.js index 0b4012a..872d092 100644 --- a/test/unified-review-comments.test.js +++ b/test/unified-review-comments.test.js @@ -75,6 +75,28 @@ describe('unified review includes reviewer comments [AVCR-002]', () => { expect(fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8')).not.toContain('EXCLUDED-SECRET'); }); + test('files named like Object.prototype members get no inherited comment [codex P2]', async () => { + const protoFiles = ['constructor', 'toString', 'hasOwnProperty']; + for (const f of protoFiles) fs.writeFileSync(path.join(dir, f), `// ${f}\n`); + execFileSync('git', ['add', '--', ...protoFiles], { cwd: dir }); + try { + await ReviewGenerator.generateUnifiedReview({ includedFiles: protoFiles, comments: {} }); + const out = fs.readFileSync(path.join(dir, 'AI_REVIEW.md'), 'utf8'); + expect(out).not.toContain('Review Comment'); + expect(out).not.toContain('[native code]'); + const splitDir = path.join(dir, 'split-out'); + await ReviewGenerator.generateSplitReviews({ includedFiles: protoFiles, comments: {}, outputDir: splitDir }); + for (const name of fs.readdirSync(splitDir)) { + expect(fs.readFileSync(path.join(splitDir, name), 'utf8')).not.toContain('Review Comment'); + } + fs.rmSync(splitDir, { recursive: true, force: true }); + expect(ReviewGenerator.getFileComment({ constructor: 'OWN' }, 'constructor')).toBe('OWN'); + } finally { + execFileSync('git', ['rm', '-q', '--cached', '--', ...protoFiles], { cwd: dir }); + for (const f of protoFiles) fs.rmSync(path.join(dir, f)); + } + }); + test('client and server agree on the lineId prefix', () => { for (const f of files.concat(['app/[id]/page.tsx', 'x$y{z}.txt', 'naΓ―ve.js'])) { expect(ReviewGenerator.lineIdPrefix(f)).toBe(clientLineIdPrefix(f)); From 99e44be98dd19e76a900545f23f75db141e567ba Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Thu, 8 Oct 2026 19:38:37 +0530 Subject: [PATCH 10/11] fix(server): show and open the configured bind address [codex: Open the configured bind address] With HOST set to a concrete address such as 192.168.1.10, the banner and `ai-review start --open` still pointed at http://localhost:, which is not listening. services/serverUrl.js now derives the URL from HOST: a concrete address is used as is (IPv6 in brackets), 0.0.0.0 maps to 127.0.0.1 and :: to localhost. server.js and the CLI share the helper and its 127.0.0.1 default. Co-Authored-By: Claude Opus 5.5 --- bin/ai-review.js | 3 ++- server.js | 9 +++++---- services/serverUrl.js | 27 +++++++++++++++++++++++++++ test/bind-host.test.js | 39 ++++++++++++++++++++++++++++++++++++++- 4 files changed, 72 insertions(+), 6 deletions(-) create mode 100644 services/serverUrl.js diff --git a/bin/ai-review.js b/bin/ai-review.js index 8815485..4b28b19 100755 --- a/bin/ai-review.js +++ b/bin/ai-review.js @@ -3,6 +3,7 @@ const { spawn, execFileSync } = require('child_process'); const path = require('path'); const fs = require('fs'); +const { resolveHost, browseUrl } = require('../services/serverUrl'); const PORT = process.env.PORT || 3002; @@ -107,7 +108,7 @@ function startServer(port = PORT, openBrowser = false) { // Open browser if requested if (openBrowser) { setTimeout(() => { - const url = `http://localhost:${port}`; + const url = browseUrl(resolveHost(process.env), port); console.log(`\n🌐 Opening browser: ${url}`); const start = (process.platform === 'darwin' ? 'open' : process.platform === 'win32' ? 'start' : 'xdg-open'); diff --git a/server.js b/server.js index 03df41b..75f4d89 100644 --- a/server.js +++ b/server.js @@ -9,11 +9,11 @@ const DiffService = require('./services/diffService'); const GitStatusParser = require('./services/gitStatusParser'); const ReviewGenerator = require('./services/ReviewGenerator'); const GitService = require('./services/GitService'); +const { resolveHost, browseUrl } = require('./services/serverUrl'); const app = express(); const PORT = process.env.PORT || 3002; -// Bind to loopback by default; the server is unauthenticated. Set HOST=0.0.0.0 (e.g. in Docker) to expose it. -const HOST = process.env.HOST || '127.0.0.1'; +const HOST = resolveHost(process.env); // Configuration const CONFIG = { @@ -778,9 +778,10 @@ app.use('*', (req, res) => { // Start server only if not in test mode if (process.env.NODE_ENV !== 'test') { app.listen(PORT, HOST, () => { + const url = browseUrl(HOST, PORT); console.log('πŸ” AI Visual Code Review Server'); console.log('==============================='); - console.log(`🌐 Server running at: http://localhost:${PORT}`); + console.log(`🌐 Server running at: ${url}`); console.log(`πŸ“ Working directory: ${process.cwd()}`); console.log(`⏰ Started at: ${new Date().toLocaleString()}`); console.log(''); @@ -793,7 +794,7 @@ if (process.env.NODE_ENV !== 'test') { console.log(''); console.log('πŸ’‘ Usage:'); console.log(' 1. Stage changes: git add .'); - console.log(' 2. Open: http://localhost:3002'); + console.log(` 2. Open: ${url}`); console.log(' 3. Review and export for AI analysis'); console.log(''); }); diff --git a/services/serverUrl.js b/services/serverUrl.js new file mode 100644 index 0000000..b26e7ed --- /dev/null +++ b/services/serverUrl.js @@ -0,0 +1,27 @@ +// Bind to loopback by default; the server is unauthenticated. Set HOST=0.0.0.0 (e.g. in Docker) to expose it. +const DEFAULT_HOST = '127.0.0.1'; + +/** + * The address server.js binds to. + * @param {Object} env - process.env + * @returns {string} + */ +function resolveHost(env) { + return env.HOST || DEFAULT_HOST; +} + +/** + * URL a local browser should open for a server bound to `host`. Wildcard binds have no + * address of their own, so they map to loopback; a concrete address is used as is. + * @param {string} host + * @param {number|string} port + * @returns {string} + */ +function browseUrl(host, port) { + let h = String(host || DEFAULT_HOST).replace(/^\[(.*)\]$/, '$1'); + if (h === '0.0.0.0') h = '127.0.0.1'; + else if (h === '::') h = 'localhost'; + return `http://${h.includes(':') ? `[${h}]` : h}:${port}`; +} + +module.exports = { DEFAULT_HOST, resolveHost, browseUrl }; diff --git a/test/bind-host.test.js b/test/bind-host.test.js index fb86de3..175a5e0 100644 --- a/test/bind-host.test.js +++ b/test/bind-host.test.js @@ -1,6 +1,8 @@ const fs = require('fs'); const path = require('path'); -const { execFileSync } = require('child_process'); +const net = require('net'); +const { execFileSync, spawn } = require('child_process'); +const { browseUrl } = require('../services/serverUrl'); describe('server bind address [AVCR-005]', () => { const root = path.join(__dirname, '..'); @@ -31,4 +33,39 @@ describe('server bind address [AVCR-005]', () => { }); } }); + + test('browser URL follows the bind address [codex: Open the configured bind address]', () => { + expect(browseUrl('192.168.1.10', 3002)).toBe('http://192.168.1.10:3002'); + expect(browseUrl('127.0.0.1', 3002)).toBe('http://127.0.0.1:3002'); + expect(browseUrl('0.0.0.0', 3002)).toBe('http://127.0.0.1:3002'); + expect(browseUrl('::', 3002)).toBe('http://localhost:3002'); + expect(browseUrl('::1', 3002)).toBe('http://[::1]:3002'); + expect(browseUrl('[fe80::1]', 3002)).toBe('http://[fe80::1]:3002'); + expect(browseUrl(undefined, 3002)).toBe('http://127.0.0.1:3002'); + }); + + test('startup banner shows the configured bind address', async () => { + const port = await new Promise((resolve, reject) => { + const s = net.createServer().once('error', reject); + s.listen(0, '127.0.0.1', () => { const p = s.address().port; s.close(() => resolve(p)); }); + }); + const env = { ...process.env, NODE_ENV: 'development', HOST: '127.0.0.1', PORT: String(port) }; + const child = spawn('node', ['server.js'], { cwd: root, env }); + try { + const out = await new Promise((resolve, reject) => { + let buf = ''; + const timer = setTimeout(() => reject(new Error(`no banner: ${buf}`)), 10000); + child.stdout.on('data', d => { + buf += d; + if (buf.includes('2. Open:')) { clearTimeout(timer); resolve(buf); } + }); + child.once('exit', code => { clearTimeout(timer); reject(new Error(`exited ${code}: ${buf}`)); }); + }); + expect(out).toContain(`Server running at: http://127.0.0.1:${port}`); + expect(out).toContain(`2. Open: http://127.0.0.1:${port}`); + expect(out).not.toContain('localhost:'); + } finally { + child.kill('SIGTERM'); + } + }); }); From cdcdbaaeecb1972b48bb5314c6d6da8d9deebb7a Mon Sep 17 00:00:00 2001 From: Prax Lannister <73683289+praxstack@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:19:23 +0530 Subject: [PATCH 11/11] docs(audit): add 2026-10 audit reports Co-Authored-By: Claude Opus 5.5 --- docs/audit/FIXES.md | 12 + docs/audit/GRAPH_REPORT.md | 828 ++++++++++++++++++++++++++++++++++++ docs/audit/README.md | 3 + docs/audit/REVIEW.md | 85 ++++ docs/audit/SETUP.md | 67 +++ docs/audit/UNDERSTANDING.md | 296 +++++++++++++ 6 files changed, 1291 insertions(+) create mode 100644 docs/audit/FIXES.md create mode 100644 docs/audit/GRAPH_REPORT.md create mode 100644 docs/audit/README.md create mode 100644 docs/audit/REVIEW.md create mode 100644 docs/audit/SETUP.md create mode 100644 docs/audit/UNDERSTANDING.md diff --git a/docs/audit/FIXES.md b/docs/audit/FIXES.md new file mode 100644 index 0000000..5c34f47 --- /dev/null +++ b/docs/audit/FIXES.md @@ -0,0 +1,12 @@ +# FIXES - ai-visual-code-review (branch fix/audit-2026-10) + +P0/P1 kept findings: AVCR-001, AVCR-002, AVCR-005. (AVCR-017/018/019/029 are P1 but verdict duplicate, so skipped; 018 and 029 are merged into 005.) +Safe test command: 7 suites, 107/107 pass (not full npm test; worktree clean afterwards apart from the commits). + +| id | status | commit | evidence | remains | +|---|---|---|---|---| +| AVCR-001 | fixed | see git log (fix(git)) | test/git-service-paths.test.js: 3 of 4 cases fail before, 4/4 pass after | Args before `--` still use the old char filter (no caller passes any). | +| AVCR-002 | fixed | see git log (fix(export)) | test/unified-review-comments.test.js fails before, passes after | "Files processed: undefined" in the page text (SETUP 5.3) not addressed; it is client-side and outside this finding. | +| AVCR-005 | partial | see git log (fix(server)) | test/bind-host.test.js: 3 fail before, 3 pass after | Remote-access hardening beyond the loopback default is an owner decision (details kept privately by the owner). Docker still unfixed per AVCR-006 (no git in image, working_dir). Default now 127.0.0.1; HOST=0.0.0.0 set in Dockerfile/compose; compose publishes 127.0.0.1:3002. | + +Later review and Codex follow-ups on PR #43 (literal pathspecs, exact lineId matching, special-path web view, healthcheck host, own-key comment lookup, bind-address banner) are recorded in the PR itself. diff --git a/docs/audit/GRAPH_REPORT.md b/docs/audit/GRAPH_REPORT.md new file mode 100644 index 0000000..27650a8 --- /dev/null +++ b/docs/audit/GRAPH_REPORT.md @@ -0,0 +1,828 @@ +# Graph Report - ai-visual-code-review (2026-10-03) + +## Corpus Check +- 105 files Β· ~136,025 words +- Verdict: corpus is large enough that graph structure adds value. +- Unclassified: 7 file(s) not represented in the graph (top: (none) 6, .example 1) + +## Summary +- 1828 nodes Β· 2011 edges Β· 166 communities (149 shown, 17 thin omitted) +- Extraction: 96% EXTRACTED Β· 4% INFERRED Β· 0% AMBIGUOUS Β· INFERRED: 79 edges (avg confidence: 0.93) +- Token cost: 0 input Β· 0 output + +## Graph Freshness +- Built from commit: `c4d6472b` +- Run `git rev-parse HEAD` and compare to check if the graph is stale. +- Run `graphify update .` after code changes (no API cost). + +## Community Hubs (Navigation) +- GitStatusParser +- GitService +- AI Visual Code Review - Technical Context +- api-client.py +- Contributing to AI Visual Code Review +- 05-09-consolidated-audit-report.md +- 🎯 Unified Export System - Complete Guide +- Agent skill packs +- Final Test Results - Pre-Deployment Validation +- middleware/index.js +- πŸ—οΈ PHASE 3: Architectural Review - Analysis Report +- πŸ” QA & Sr SDE Review - AI Visual Code Review v2.3.1 +- Plan 008: Remove dead GET cache middleware +- [2.2.0] - 2025-10-19 +- 🟠 HIGH SEVERITY ISSUES (18) +- πŸ“š PHASE 2: Documentation Deep-Dive - Analysis Report +- Agent Skills +- contributes +- .generateUnifiedReview +- Progress - AI Visual Code Review +- AI Visual Code Review - System Patterns & Architecture +- Security Policy +- Test Results - Unified Export System +- πŸ”„ Upgrading AI Visual Code Review from v1.0.3 to v1.0.4+ +- server.js +- πŸ—‚οΈ Complete File Inventory +- βœ… Test Suite +- vscode-extension/package.json +- AI Visual Code Review - Architecture Documentation +- export-ai-review.js +- TWITTER_LAUNCH_THREADS.md +- 5.1 Functional Requirements +- AI Visual Code Review - Project Brief +- πŸ“‹ NPM Scripts Analysis & Fixes for ai-visual-code-review +- scripts +- Plan 004: Keep one lockfile and align `engines` with CI +- link-agent-skills.sh +- package.json +- Plan 001: Route `server.js` validation through `src/utils` +- Plan 006: Repair duplicated file-header markup in the review page +- [2.1.0] - 2025-10-14 +- Plan 003: Upgrade Express to the patched 4.x for the path-to-regexp advisory +- install-agent-skills.sh +- [2.3.0] - 2025-12-12 +- ai-review.js +- Active Context - AI Visual Code Review +- link-agent-skills.test.js +- agent-skill-packs.test.js +- Changelog +- 3.1 Bundle Generator Service +- [2.0.0] - 2024-10-10 +- ReviewGenerator.js +- run-tests.js +- gen-skills-index.js +- πŸš€ Complete Setup Guide +- compilerOptions +- Comprehensive Test Scenarios - Pre-Deployment +- bug_report.md +- scripts +- Test Categories +- Business Requirements Document (BRD) +- Business Requirements Document (BRD) +- feature_request.md +- πŸ” AI Visual Code Review v2.0 +- 🎯 Advanced Usage +- πŸ”§ Troubleshooting +- devDependencies +- Issue tracker: GitHub +- 4.1 Functional Requirements +- High-Level Design (HLD) +- PULL_REQUEST_TEMPLATE.md +- Plan 009: Drive CLI export listing through the Git working-tree module +- Detailed Workflow +- ✨ New Features +- A. CLI Argument Testing +- C. File Type Handling +- Domain Docs +- 4.2 Non-Functional Requirements +- devDependencies +- link-praxstack-skills.sh +- Common Issues +- Publishing to NPM +- B. Pattern Matching Edge Cases +- D. Error Scenarios +- F. Performance Testing +- Test Execution Plan +- 10. Success Metrics +- 17. Appendix +- 8. Implementation Phases +- πŸ› οΈ Development & Testing +- πŸ“ˆ Examples +- sample-component.tsx +- 🌐 Remote Repository Usage +- πŸš€ Migration Notes +- E. Integration Testing +- G. Cross-Platform Compatibility +- H. Output Validation +- I. Security Testing +- 10. Success Metrics +- 1. Executive Summary +- 3. Current State Analysis +- 5. User Stories +- 6. Business Rules +- 7. Assumptions and Constraints +- 9. Implementation Phases +- 11. Risks and Mitigation +- 12. Go-To-Market Strategy +- 13. Compliance and Legal +- 14. Dependencies and Prerequisites +- 1. Executive Summary +- 2. Market Analysis +- 3. Stakeholders +- 6. User Stories +- 7. Technical Architecture Overview +- 9. Business Rules +- 10. Deployment & Distribution +- 11. Testing Strategy +- 1. System Overview +- 5. Technology Stack +- 6. Security Architecture +- 7. Performance Considerations +- sentinel.md +- ✨ Core Features +- πŸ“‹ Usage +- πŸš€ Quick Start +- πŸš€ **What's New in v2.0** +- server.test.js +- install-agent-browser.sh +- install-cursor-native-stack.sh +- quick-ai-review.sh +- vendor-praxstack-extras.sh +- βš™οΈ CI/CD Integration +- 🐳 Docker Deployment +- test_cors_logic.js +- 11. Risks and Mitigation +- 14. Appendix +- 2. Stakeholders +- 12. Monitoring & Metrics +- 13. Future Enhancements +- 4. Data Model +- 8. Error Handling +- 9. Integration Points +- palette.md +- dependencies +- repository +- πŸ› Troubleshooting +- πŸ”§ Configuration +- 🀝 Contributing +- πŸ“Š Export Formats +- galleryBanner +- repository +- CLAUDE.md +- triage-labels.md +- praxstack-skill-collisions.md +- install.sh +- bin +- bugs +- engines +- setup-github.sh +- bugs +- dependencies +- engines + +## God Nodes (most connected - your core abstractions) +1. `Agent Skills` - 28 edges +2. `GitStatusParser` - 23 edges +3. `GitService` - 19 edges +4. `Business Requirements Document (BRD)` - 19 edges +5. `scripts` - 17 edges +6. `ReviewWebviewProvider` - 16 edges +7. `Comprehensive Test Scenarios - Pre-Deployment` - 16 edges +8. `πŸ” AI Visual Code Review v2.0` - 16 edges +9. `Business Requirements Document (BRD)` - 16 edges +10. `High-Level Design (HLD)` - 16 edges + +## Surprising Connections (you probably didn't know these) +- `Step 1: Characterization test that fails on current code` --references--> `main()` [INFERRED] + plans/005-pass-comments-into-unified-export.md β†’ test/api-client.py +- `Current state` --references--> `isTooLarge()` [INFERRED] + plans/009-cli-export-through-gitservice.md β†’ scripts/export-ai-review.js +- `2024-05-18 - O(K) Cache Eviction using Map Insertion Order` --references--> `cacheMiddleware()` [INFERRED] + .Jules/bolt.md β†’ server.js +- `Step 1: Delete the dead cache block` --references--> `cacheMiddleware()` [INFERRED] + plans/008-remove-dead-cache-middleware.md β†’ server.js +- `Scope` --references--> `ReviewGenerator` [INFERRED] + plans/009-cli-export-through-gitservice.md β†’ services/ReviewGenerator.js + +## Import Cycles +- None detected. + +## Communities (166 total, 17 thin omitted) + +### Community 0 - "GitStatusParser" +Cohesion: 0.06 +Nodes (35): Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes, Plan 002: Drop unused `server.js` imports and fail CI lint on error, Scope, Status (+27 more) + +### Community 1 - "GitService" +Cohesion: 0.07 +Nodes (16): { execFile }, path, activate(), exportForAICommand(), openReviewCommand(), quickReviewCommand(), setupAutoRefresh(), updateStatusBar() (+8 more) + +### Community 2 - "AI Visual Code Review - Technical Context" +Cohesion: 0.05 +Nodes (42): **Advantages of Current Stack**, AI Visual Code Review - Technical Context, πŸ—οΈ **Architecture Decisions**, **Async Operations**, **Backend Technologies**, **Binary Executables**, **Caching Strategy**, **CLI Implementation** (+34 more) + +### Community 3 - "api-client.py" +Cohesion: 0.05 +Nodes (17): 2026 high-signal layers, Agent skills, Domain docs, Issue tracker, Methodology XOR (one conductor per task), Prax's skills-and-personas, Project quick reference, Triage labels (+9 more) + +### Community 4 - "Contributing to AI Visual Code Review" +Cohesion: 0.05 +Nodes (39): 1. Correction, 2. Warning, 3. Temporary Ban, 4. Permanent Ban, Attribution, Contributor Covenant Code of Conduct, Enforcement, Enforcement Guidelines (+31 more) + +### Community 5 - "05-09-consolidated-audit-report.md" +Cohesion: 0.05 +Nodes (40): Accessibility Assessment (WCAG 2.1 AA), Accessibility Issues Found, All Issues Summary, βœ… AUDIT ITERATION 1 - COMPLETE, Backend Missing Tests, By Phase, Critical Security Issues, Current Test Coverage (+32 more) + +### Community 6 - "🎯 Unified Export System - Complete Guide" +Cohesion: 0.05 +Nodes (40): 1. Always Stage Changes First, 1. CLI Flow, 1. Review Specific Package in Monorepo, 2. Review Only Tests, 2. Use Specific Patterns for Large Repos, 2. Web Interface Flow, 3. DiffService Processing, 3. Exclude Generated Files by Default (+32 more) + +### Community 7 - "Agent skill packs" +Cohesion: 0.06 +Nodes (31): agent-browser CLI, Agent skill packs, Also credible, not vendored here, Awesome Copilot, Graphify, How to add another pack, MCP governance (Serena + Context7 only), Microsoft subset (+23 more) + +### Community 8 - "Final Test Results - Pre-Deployment Validation" +Cohesion: 0.05 +Nodes (37): 1. Server Startup, 1. βœ… Syntax Error (Backslash Escaping), 2. βœ… Pattern Matching, 2. Web Interface Loading, 3. Repository Detection, 3. βœ… Zero-Directory Glob, 4. βœ… Deleted File Handling, 4. File Selection (+29 more) + +### Community 9 - "middleware/index.js" +Cohesion: 0.08 +Nodes (28): Step 4: Optional delete of unused `src/utils/gitCommands.js`, CONFIG, { createRateLimit, resetRateLimit, generalRateLimit, exportRateLimit }, { securityHeaders, requestLogger, generateNonce }, CONFIG, createRateLimit(), exportRateLimit, generalRateLimit (+20 more) + +### Community 10 - "πŸ—οΈ PHASE 3: Architectural Review - Analysis Report" +Cohesion: 0.06 +Nodes (34): 1. Server Layer (`server.js` - 1,114 lines), 2. Services Layer, 3. CLI Layer (`bin/ai-review.js` - 189 lines), 4. Frontend Layer (`public/index.html` - 1,667 lines), 5. VSCode Extension (`vscode-extension/` - 552 lines TS), πŸ“Š Architecture Quality Scores, 🎯 Architecture Recommendations, πŸ“‹ Architecture Review Summary (+26 more) + +### Community 11 - "πŸ” QA & Sr SDE Review - AI Visual Code Review v2.3.1" +Cohesion: 0.06 +Nodes (33): Advanced Features, πŸ—οΈ Architecture Review, Best Practices Compliance, βœ… Certification, πŸ“ Code Quality Assessment, Code Smells Identified, πŸ“‹ Complete Feature Matrix, Core Features (+25 more) + +### Community 12 - "Plan 008: Remove dead GET cache middleware" +Cohesion: 0.07 +Nodes (27): 2024-05-18 - O(K) Cache Eviction using Map Insertion Order, 2026-03-04 - Parallelize independent Git CLI commands, 2026-03-05 - Parallelize independent Git CLI operations in API endpoints, Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes (+19 more) + +### Community 13 - "[2.2.0] - 2025-10-19" +Cohesion: 0.06 +Nodes (33): [2.2.0] - 2025-10-19, Algorithmic Efficiency, API Enhancements, Breaking Changes, Clean Architecture, Comprehensive Documentation, 🎯 Comprehensive Git Status Handling System, Comprehensive Status Support (+25 more) + +### Community 14 - "🟠 HIGH SEVERITY ISSUES (18)" +Cohesion: 0.06 +Nodes (31): By Category, By Severity, Code Duplication, πŸ“Š Code Quality Metrics, Complexity Analysis, CR-001: VSCode Extension Missing Required Files, CR-002: execSync with .catch() - Invalid JavaScript, CR-003: No HTTPS/TLS Support in Production (+23 more) + +### Community 15 - "πŸ“š PHASE 2: Documentation Deep-Dive - Analysis Report" +Cohesion: 0.06 +Nodes (30): 1. README.md Analysis, 2. SECURITY.md Analysis, 3. HLD-UI-Export-Bundle.md Analysis, 4. BRD-VSCode-Extension.md Analysis, 5. Memory Bank Analysis, Critical Issues (Require Immediate Attention), Critical Missing Documents, πŸ” Documentation Analysis by Category (+22 more) + +### Community 16 - "Agent Skills" +Cohesion: 0.07 +Nodes (29): Addy Osmani β€” Agent Skills, agent-browser, agent-deep-research, Agent Skills, Anthropic example skills (subset), Awesome Copilot (toolbox shelf), AWS agent toolkit, Cloudflare skills (+21 more) + +### Community 17 - "contributes" +Cohesion: 0.07 +Nodes (29): default, description, type, default, description, type, default, description (+21 more) + +### Community 18 - ".generateUnifiedReview" +Cohesion: 0.13 +Nodes (21): 2024-05-18 - Optimize asynchronous file operations and child processes in `services/ReviewGenerator.js`, Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes, Plan 005: Pass reviewer comments into unified AI review export, Scope (+13 more) + +### Community 19 - "Progress - AI Visual Code Review" +Cohesion: 0.08 +Nodes (25): Architecture, Audit & Planning, Current Version: 2.3.0, Deferred to Future Releases, Deployment Checklist, Docker, GitHub, Known Issues / Tech Debt (+17 more) + +### Community 20 - "AI Visual Code Review - System Patterns & Architecture" +Cohesion: 0.08 +Nodes (25): AI Visual Code Review - System Patterns & Architecture, **CLI Interface Pattern**, **Client-Side State Pattern**, πŸ”§ **Configuration Patterns**, πŸ—οΈ **Core Architecture Pattern**, πŸ“Š **Data Flow Patterns**, **Defense in Depth Pattern**, **Development vs Production Pattern** (+17 more) + +### Community 21 - "Security Policy" +Cohesion: 0.08 +Nodes (26): Automated Testing, Before Deployment, CI/CD Security Integration, Code Review Security Focus, Command Injection Prevention, Environment Variables, Input Validation & Sanitization, Manual Security Testing (+18 more) + +### Community 22 - "Test Results - Unified Export System" +Cohesion: 0.08 +Nodes (23): 1. βœ… Syntax Error (Backslash Escaping), 2. βœ… Pattern Matching Broken, 3. βœ… Zero-Directory Glob Pattern, 4. βœ… Deleted File Handling, Additional Testing (Optional), Bugs Fixed, Code Quality, Conclusion (+15 more) + +### Community 23 - "πŸ”„ Upgrading AI Visual Code Review from v1.0.3 to v1.0.4+" +Cohesion: 0.08 +Nodes (23): 🚨 **Breaking Changes**, **CI/CD Pipeline Updates:**, **πŸ”§ CLI Improvements**, **Docker Updates:**, πŸ“¦ **For Team/Organization Updates**, **Issue: Command not found after update**, **Issue: Old version still showing**, **Issue: Permission errors** (+15 more) + +### Community 24 - "server.js" +Cohesion: 0.09 +Nodes (16): ALLOWED_ORIGINS, app, CONFIG, cors, DiffService, { execFile }, exportRateLimit, express (+8 more) + +### Community 25 - "πŸ—‚οΈ Complete File Inventory" +Cohesion: 0.09 +Nodes (22): Areas for Improvement, πŸ—‚οΈ Complete File Inventory, πŸ“ Configuration Files, πŸ“¦ Dependencies Analysis, πŸ“ Design Documents (docs/), Development Dependencies, πŸ“ Documentation Files, πŸ“ˆ Executive Summary (+14 more) + +### Community 26 - "βœ… Test Suite" +Cohesion: 0.09 +Nodes (21): 🎯 Critical Tests, πŸ”¬ Detailed Output Format Test, πŸ“ Manual Verification Steps, πŸ“Š Output Verification Checklist, πŸš€ Pre-Deployment Checklist, πŸ“‹ Pre-Deployment Testing Checklist, πŸŽ‰ Success Criteria Summary, Test 10: Error Handling (+13 more) + +### Community 27 - "vscode-extension/package.json" +Cohesion: 0.10 +Nodes (20): simple-git, @types/node, @types/vscode, @typescript-eslint/eslint-plugin, @typescript-eslint/parser, vsce, activationEvents, categories (+12 more) + +### Community 28 - "AI Visual Code Review - Architecture Documentation" +Cohesion: 0.10 +Nodes (19): 1. Single HTML File (public/index.html), 2. execFile over exec, 3. Whitelist Git Commands, 4. Modular src/ Structure, AI Visual Code Review - Architecture Documentation, Allowed Git Commands (Whitelist), πŸ“‘ API Endpoints, πŸ”„ Data Flow (+11 more) + +### Community 29 - "export-ai-review.js" +Cohesion: 0.11 +Nodes (19): Scope, STOP conditions, args, config, DEFAULT_EXCLUDES, deletedFiles, DiffService, excludedFiles (+11 more) + +### Community 30 - "TWITTER_LAUNCH_THREADS.md" +Cohesion: 0.10 +Nodes (19): πŸš€ AI Visual Code Review - Twitter Launch Threads, CodeReview #AI #Developer #OpenSource, CodeReview #AI #NPM #OpenSource #Developer, CodeReview #Features #Developer, Developer #Setup #CodeReview, 🀝 Engagement Tactics, 🏷️ Hashtag Strategy, 🎯 Posting Strategy (+11 more) + +### Community 31 - "5.1 Functional Requirements" +Cohesion: 0.11 +Nodes (18): 5.1 Functional Requirements, 5.2 Non-Functional Requirements, 5. Business Requirements, FR10: Bundle Export (Future), FR1: Extension Installation & Activation, FR2: Review Panel Integration, FR3: Diff Viewing, FR4: Inline Commenting (+10 more) + +### Community 32 - "AI Visual Code Review - Project Brief" +Cohesion: 0.11 +Nodes (17): **1. GitHub-Like Experience**, **2. AI-Optimized Exports**, **3. Enterprise Security**, **4. Developer Experience**, AI Visual Code Review - Project Brief, πŸ† **Competitive Advantages**, 🎯 **Core Purpose**, πŸ“‹ **Current Status** (+9 more) + +### Community 33 - "πŸ“‹ NPM Scripts Analysis & Fixes for ai-visual-code-review" +Cohesion: 0.11 +Nodes (17): Broken Commands ❌ (Arguments Not Passed), βœ… Commands Status, βœ… Conclusion, Current Implementation (BROKEN), πŸ› Diff Formatting Issue, πŸ“ File to Fix in ai-visual-code-review Package, πŸ” Issue Identified, πŸ“‹ NPM Scripts Analysis & Fixes for ai-visual-code-review (+9 more) + +### Community 34 - "scripts" +Cohesion: 0.12 +Nodes (17): scripts, build, build:css, build:js, dev, generate:certs, lint, lint:fix (+9 more) + +### Community 35 - "Plan 004: Keep one lockfile and align `engines` with CI" +Cohesion: 0.12 +Nodes (16): Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes, Plan 004: Keep one lockfile and align `engines` with CI, Scope, Status (+8 more) + +### Community 36 - "link-agent-skills.sh" +Cohesion: 0.22 +Nodes (15): gstack_skill_name(), install_cursor_mcp(), install_cursor_rules(), install_pstack_models(), link_abs_or_skip(), link_cursor_skills(), link_gstack_cursor_skills(), link_packs() (+7 more) + +### Community 37 - "package.json" +Cohesion: 0.12 +Nodes (15): author, description, files, homepage, eslint, typescript, keywords, license (+7 more) + +### Community 38 - "Plan 001: Route `server.js` validation through `src/utils`" +Cohesion: 0.12 +Nodes (15): Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes, Plan 001: Route `server.js` validation through `src/utils`, Scope, Status (+7 more) + +### Community 39 - "Plan 006: Repair duplicated file-header markup in the review page" +Cohesion: 0.12 +Nodes (15): Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes, Plan 006: Repair duplicated file-header markup in the review page, Scope, Status (+7 more) + +### Community 40 - "[2.1.0] - 2025-10-14" +Cohesion: 0.13 +Nodes (15): [2.1.0] - 2025-10-14, πŸ› Bug Fixes, Critical Fixes, 🎯 Cross-Platform Export System, Deleted File Handling, πŸ“š Documentation, Enhanced Pattern Matching, ✨ Features (+7 more) + +### Community 41 - "Plan 003: Upgrade Express to the patched 4.x for the path-to-regexp advisory" +Cohesion: 0.13 +Nodes (14): Commands you will need, Current state, Done criteria, Git workflow, Maintenance notes, Plan 003: Upgrade Express to the patched 4.x for the path-to-regexp advisory, Scope, Status (+6 more) + +### Community 42 - "install-agent-skills.sh" +Cohesion: 0.29 +Nodes (14): assert_github_slug(), clone_repo(), copy_gstack_runtime(), copy_pack_license(), copy_root_skill_bundle(), copy_skill_files(), extract_pack(), extract_spec_kit() (+6 more) + +### Community 43 - "[2.3.0] - 2025-12-12" +Cohesion: 0.14 +Nodes (14): [2.3.0] - 2025-12-12, πŸ“Š Audit Results, Breaking Changes, Code Quality, πŸ› Critical Bug Fixes, Files Added (15+ new files), High Priority Fixes (HI-001 to HI-015), Migration Guide v2.2.x β†’ v2.3.0 (+6 more) + +### Community 44 - "ai-review.js" +Cohesion: 0.18 +Nodes (9): args, checkGitRepo(), fs, path, quickReview(), { spawn, execFileSync }, startServer(), VERSION (+1 more) + +### Community 45 - "Active Context - AI Visual Code Review" +Cohesion: 0.15 +Nodes (12): Active Context - AI Visual Code Review, Active Files Modified, Core Changes, Current State (December 2025), Key Decisions Made, New Modules, Next Steps, Open Questions (+4 more) + +### Community 46 - "link-agent-skills.test.js" +Cohesion: 0.17 +Nodes (8): { execFileSync }, fs, os, PACK, path, PROJECT_SKILLS, ROOT, SCRIPT + +### Community 47 - "agent-skill-packs.test.js" +Cohesion: 0.17 +Nodes (7): fs, MICROSOFT_SUBSET, PACKS, path, PSTACK_SLUGS, ROOT, SPECKIT_COMMANDS + +### Community 48 - "Changelog" +Cohesion: 0.18 +Nodes (11): [1.0.0] - 2024-09-15, API Compatibility, Changelog, Features, For Developers, For Users, Initial Release, Migration Guide v2.1.x β†’ v2.2.0 (+3 more) + +### Community 49 - "3.1 Bundle Generator Service" +Cohesion: 0.18 +Nodes (11): 3.1.1 Data Collection Module, 3.1.2 Template Engine, 3.1.3 Data Embedding Module, 3.1.4 Optimization Module, 3.1 Bundle Generator Service, 3.2.1 Application Bootstrap, 3.2.2 State Management, 3.2.3 UI Components (+3 more) + +### Community 50 - "[2.0.0] - 2024-10-10" +Cohesion: 0.20 +Nodes (10): [2.0.0] - 2024-10-10, API Changes, Breaking Changes, πŸ› οΈ Developer Experience, πŸ“š Documentation, πŸš€ Major Release - Production Ready, ⚑ Performance, πŸ”’ Security (+2 more) + +### Community 51 - "ReviewGenerator.js" +Cohesion: 0.20 +Nodes (6): DiffService, fs, GitService, GitStatusParser, path, DiffService + +### Community 52 - "run-tests.js" +Cohesion: 0.20 +Nodes (8): binContent, { execSync }, exportContent, exportScript, fs, path, result, results + +### Community 53 - "gen-skills-index.js" +Cohesion: 0.20 +Nodes (6): fs, OUT, PACK_META, path, ROOT, SKILLS_DIR + +### Community 54 - "πŸš€ Complete Setup Guide" +Cohesion: 0.20 +Nodes (10): πŸš€ Complete Setup Guide, 🏠 Local Installation, Option 1: Clone and Install (Recommended for Development), Option 2: Direct NPM Installation, Option 3: Local Development Setup, Prerequisites, Quick Check, πŸ“ž Support & Community (+2 more) + +### Community 55 - "compilerOptions" +Cohesion: 0.20 +Nodes (9): compilerOptions, lib, module, outDir, rootDir, sourceMap, strict, target (+1 more) + +### Community 56 - "Comprehensive Test Scenarios - Pre-Deployment" +Cohesion: 0.22 +Nodes (8): Automated Test Execution, Comprehensive Test Scenarios - Pre-Deployment, J1. Old Functionality Still Works, J2. Backward Compatibility, J. Regression Testing, Results, Test Data Setup, Test Results Summary + +### Community 57 - "bug_report.md" +Cohesion: 0.22 +Nodes (8): ❌ Actual Behavior, πŸ“ Additional Context, πŸ› Bug Description, πŸ–₯️ Environment, βœ… Expected Behavior, πŸ“„ Logs, πŸ“Έ Screenshots, πŸ“‹ Steps to Reproduce + +### Community 58 - "scripts" +Cohesion: 0.22 +Nodes (9): scripts, compile, lint, package, pretest, publish, test, vscode:prepublish (+1 more) + +### Community 59 - "Test Categories" +Cohesion: 0.25 +Nodes (8): A. CLI Argument Testing, B. Pattern Matching Edge Cases, C. File Type Handling, D. Error Scenarios, E. Integration Testing, F. Performance Testing, G. Cross-Platform Compatibility, Test Categories + +### Community 60 - "Business Requirements Document (BRD)" +Cohesion: 0.25 +Nodes (7): 12.1 Data Privacy, 12.2 Open Source License, 12. Compliance and Legal, 13. Approval, 8. Out of Scope, Business Requirements Document (BRD), Standalone HTML Bundle Export for Human Code Review + +### Community 61 - "Business Requirements Document (BRD)" +Cohesion: 0.25 +Nodes (7): 15. Out of Scope (Future Versions), 16. Approval and Sign-off, 4.1 Existing Solution Analysis, 4.2 VS Code Extension Ecosystem, 4. Current State Analysis, Business Requirements Document (BRD), VS Code Extension for AI Visual Code Review + +### Community 62 - "feature_request.md" +Cohesion: 0.25 +Nodes (7): βœ… Acceptance Criteria, πŸ“ Additional Context, πŸ”„ Alternatives Considered, πŸš€ Feature Description, πŸ“Έ Mockups/Examples, πŸ’‘ Problem Statement, πŸ“‹ Proposed Solution + +### Community 63 - "πŸ” AI Visual Code Review v2.0" +Cohesion: 0.25 +Nodes (8): πŸ™ Acknowledgments, πŸ€– AI Integration Guide, πŸ” AI Visual Code Review v2.0, πŸ“„ License, Multi-Layer Security Protection, Optimal Prompts, πŸ”’ Security Features, Supported Frameworks + +### Community 64 - "🎯 Advanced Usage" +Cohesion: 0.25 +Nodes (8): 🎯 Advanced Usage, Custom Port and Options, File Filtering, Git Hooks, Include/Exclude Patterns, Integration with Editors, Size Management, VS Code Tasks + +### Community 65 - "πŸ”§ Troubleshooting" +Cohesion: 0.25 +Nodes (8): Firewall/Proxy, Getting Help, Large Repositories, Memory Issues, Network Issues, Performance Issues, Reporting Issues, πŸ”§ Troubleshooting + +### Community 66 - "devDependencies" +Cohesion: 0.25 +Nodes (8): devDependencies, eslint, @types/node, @types/vscode, typescript, @typescript-eslint/eslint-plugin, @typescript-eslint/parser, vsce + +### Community 67 - "Issue tracker: GitHub" +Cohesion: 0.29 +Nodes (6): Conventions, Issue tracker: GitHub, Pull requests as a triage surface, Wayfinding operations, When a skill says "fetch the relevant ticket", When a skill says "publish to the issue tracker" + +### Community 68 - "4.1 Functional Requirements" +Cohesion: 0.29 +Nodes (7): 4.1 Functional Requirements, FR1: Bundle Generation, FR2: Interactive Review Interface, FR3: Comment Management, FR4: Comment Export, FR5: Review Metadata, FR6: Browser Compatibility + +### Community 69 - "High-Level Design (HLD)" +Cohesion: 0.29 +Nodes (6): 2.1 Architecture Diagram, 2.2 Component Interaction Flow, 2. System Architecture, Document Control, High-Level Design (HLD), Standalone HTML Bundle Export for Human Code Review + +### Community 70 - "PULL_REQUEST_TEMPLATE.md" +Cohesion: 0.29 +Nodes (6): πŸ“ Additional Notes, βœ… Checklist, πŸ“‹ Description, πŸ”— Related Issue, πŸ“Έ Screenshots (if applicable), πŸ”„ Type of Change + +### Community 71 - "Plan 009: Drive CLI export listing through the Git working-tree module" +Cohesion: 0.29 +Nodes (6): Commands you will need, Done criteria, Git workflow, Maintenance notes, Plan 009: Drive CLI export listing through the Git working-tree module, Status + +### Community 72 - "Detailed Workflow" +Cohesion: 0.29 +Nodes (7): Detailed Workflow, Quick Start (5 minutes), Step 1: Project Preparation, Step 2: Start Review Server, Step 3: Review Process, Step 4: Quick Export (Alternative), πŸ“ Using in Any Repository + +### Community 73 - "✨ New Features" +Cohesion: 0.33 +Nodes (6): Developer Experience, DevOps & CI/CD, Documentation, Enterprise Architecture, ✨ New Features, VSCode Extension + +### Community 74 - "A. CLI Argument Testing" +Cohesion: 0.33 +Nodes (6): A1. Basic Commands, A2. Include Patterns, A3. Exclude Patterns, A4. Size Limits, A5. Combined Arguments, A. CLI Argument Testing + +### Community 75 - "C. File Type Handling" +Cohesion: 0.33 +Nodes (6): C1. JavaScript Files, C2. TypeScript Files, C3. JSON Files, C4. Markdown Files, C5. Other File Types, C. File Type Handling + +### Community 76 - "Domain Docs" +Cohesion: 0.33 +Nodes (5): Before exploring, read these, Domain Docs, File structure, Flag ADR conflicts, Use the glossary's vocabulary + +### Community 77 - "4.2 Non-Functional Requirements" +Cohesion: 0.33 +Nodes (6): 4.2 Non-Functional Requirements, 4. Business Requirements, NFR1: Performance, NFR2: Usability, NFR3: Security, NFR4: Maintainability + +### Community 78 - "devDependencies" +Cohesion: 0.33 +Nodes (6): devDependencies, eslint, jest, nodemon, supertest, typescript + +### Community 79 - "link-praxstack-skills.sh" +Cohesion: 0.60 +Nodes (5): link_abs_or_skip(), relpath_from_cursor_skills(), link-praxstack-skills.sh script, skill_frontmatter_name(), valid_skill_slug() + +### Community 80 - "Common Issues" +Cohesion: 0.33 +Nodes (6): Common Issues, Issue: "Command not found: ai-review", Issue: "No staged changes found", Issue: "Not a git repository", Issue: "Permission denied", Issue: "Port 3002 already in use" + +### Community 81 - "Publishing to NPM" +Cohesion: 0.33 +Nodes (6): πŸ“¦ NPM Publishing & Distribution, Private Registry Publishing, Publishing to NPM, Step 1: Prepare for Publishing, Step 2: Publish to NPM, Step 3: Create Distribution Scripts + +### Community 82 - "B. Pattern Matching Edge Cases" +Cohesion: 0.40 +Nodes (5): B1. Glob Patterns, B2. Extension Patterns, B3. Directory Patterns, B4. Complex Patterns, B. Pattern Matching Edge Cases + +### Community 83 - "D. Error Scenarios" +Cohesion: 0.40 +Nodes (5): D1. Git Repository Errors, D2. File Access Errors, D3. Invalid Arguments, D4. Process Errors, D. Error Scenarios + +### Community 84 - "F. Performance Testing" +Cohesion: 0.40 +Nodes (5): F1. Small Projects, F2. Medium Projects, F3. Large Projects, F4. Large Files, F. Performance Testing + +### Community 85 - "Test Execution Plan" +Cohesion: 0.40 +Nodes (5): Phase 1: Critical Tests (Automated), Phase 2: Edge Cases (Automated), Phase 3: Manual Testing (User), Phase 4: Performance & Security (Automated), Test Execution Plan + +### Community 86 - "10. Success Metrics" +Cohesion: 0.40 +Nodes (5): 10.1 Adoption Metrics, 10.2 Engagement Metrics, 10.3 Quality Metrics, 10.4 Business Metrics, 10. Success Metrics + +### Community 87 - "17. Appendix" +Cohesion: 0.40 +Nodes (5): 17.1 VS Code Extension API References, 17.2 Competitive Analysis Links, 17.3 Market Research Data, 17.4 Glossary, 17. Appendix + +### Community 88 - "8. Implementation Phases" +Cohesion: 0.40 +Nodes (5): 8. Implementation Phases, Phase 1: MVP (Weeks 1-3) - Core Review Features, Phase 2: Enhanced Features (Weeks 4-5) - Commenting, Phase 3: Bundle Export (Week 6) - HTML Export, Phase 4: Polish & Release (Weeks 7-8) + +### Community 89 - "πŸ› οΈ Development & Testing" +Cohesion: 0.40 +Nodes (5): πŸ› οΈ Development & Testing, Enhanced API Endpoints, Local Development, Project Structure, Testing Suite + +### Community 90 - "πŸ“ˆ Examples" +Cohesion: 0.40 +Nodes (5): πŸ“ˆ Examples, Full-stack Project, Node.js API Project, Python Project, React/TypeScript Project + +### Community 92 - "🌐 Remote Repository Usage" +Cohesion: 0.40 +Nodes (5): 🌐 Remote Repository Usage, Scenario 1: Team Member Installation, Scenario 2: Project-Specific Installation, Scenario 3: Shared Configuration, Scenario 4: Git Hooks Integration + +### Community 95 - "πŸš€ Migration Notes" +Cohesion: 0.50 +Nodes (4): CLI Usage Changes (Optional), For Windows Users, From v2.0.0 to v2.1.0, πŸš€ Migration Notes + +### Community 96 - "E. Integration Testing" +Cohesion: 0.50 +Nodes (4): E1. CLI to Script Flow, E2. DiffService Integration, E3. File System Integration, E. Integration Testing + +### Community 97 - "G. Cross-Platform Compatibility" +Cohesion: 0.50 +Nodes (4): G1. macOS (Current Platform), G2. Linux (Test if possible), G3. Windows (Test if possible), G. Cross-Platform Compatibility + +### Community 98 - "H. Output Validation" +Cohesion: 0.50 +Nodes (4): H1. AI_REVIEW.md Content, H2. Console Output, H3. Exit Codes, H. Output Validation + +### Community 99 - "I. Security Testing" +Cohesion: 0.50 +Nodes (4): I1. Command Injection, I2. Path Traversal, I3. Input Validation, I. Security Testing + +### Community 100 - "10. Success Metrics" +Cohesion: 0.50 +Nodes (4): 10.1 Adoption Metrics, 10.2 Performance Metrics, 10.3 Quality Metrics, 10. Success Metrics + +### Community 101 - "1. Executive Summary" +Cohesion: 0.50 +Nodes (4): 1.1 Purpose, 1.2 Business Objectives, 1.3 Success Criteria, 1. Executive Summary + +### Community 102 - "3. Current State Analysis" +Cohesion: 0.50 +Nodes (4): 3.1 Current Capabilities, 3.2 Current Limitations, 3.3 Gap Analysis, 3. Current State Analysis + +### Community 103 - "5. User Stories" +Cohesion: 0.50 +Nodes (4): 5.1 High Priority, 5.2 Medium Priority, 5.3 Low Priority, 5. User Stories + +### Community 104 - "6. Business Rules" +Cohesion: 0.50 +Nodes (4): 6.1 Bundle Generation Rules, 6.2 Comment Rules, 6.3 Security Rules, 6. Business Rules + +### Community 105 - "7. Assumptions and Constraints" +Cohesion: 0.50 +Nodes (4): 7.1 Assumptions, 7.2 Constraints, 7.3 Dependencies, 7. Assumptions and Constraints + +### Community 106 - "9. Implementation Phases" +Cohesion: 0.50 +Nodes (4): 9. Implementation Phases, Phase 1: MVP (Weeks 1-2), Phase 2: Enhanced (Weeks 3-4), Phase 3: Polish (Week 5) + +### Community 107 - "11. Risks and Mitigation" +Cohesion: 0.50 +Nodes (4): 11.1 Technical Risks, 11.2 Business Risks, 11.3 Market Risks, 11. Risks and Mitigation + +### Community 108 - "12. Go-To-Market Strategy" +Cohesion: 0.50 +Nodes (4): 12.1 Launch Plan, 12.2 Marketing Channels, 12.3 Positioning, 12. Go-To-Market Strategy + +### Community 109 - "13. Compliance and Legal" +Cohesion: 0.50 +Nodes (4): 13.1 VS Code Marketplace Compliance, 13.2 Data Privacy, 13.3 Open Source License, 13. Compliance and Legal + +### Community 110 - "14. Dependencies and Prerequisites" +Cohesion: 0.50 +Nodes (4): 14.1 Technical Dependencies, 14.2 Development Dependencies, 14.3 Marketplace Requirements, 14. Dependencies and Prerequisites + +### Community 111 - "1. Executive Summary" +Cohesion: 0.50 +Nodes (4): 1.1 Purpose, 1.2 Business Objectives, 1.3 Success Criteria, 1. Executive Summary + +### Community 112 - "2. Market Analysis" +Cohesion: 0.50 +Nodes (4): 2.1 Market Size, 2.2 Competitive Landscape, 2.3 Market Opportunity, 2. Market Analysis + +### Community 113 - "3. Stakeholders" +Cohesion: 0.50 +Nodes (4): 3.1 Primary Stakeholders, 3.2 Secondary Stakeholders, 3.3 Internal Stakeholders, 3. Stakeholders + +### Community 114 - "6. User Stories" +Cohesion: 0.50 +Nodes (4): 6.1 High Priority, 6.2 Medium Priority, 6.3 Low Priority, 6. User Stories + +### Community 115 - "7. Technical Architecture Overview" +Cohesion: 0.50 +Nodes (4): 7.1 Extension Structure, 7.2 Core Components, 7.3 Data Flow, 7. Technical Architecture Overview + +### Community 116 - "9. Business Rules" +Cohesion: 0.50 +Nodes (4): 9.1 Extension Behavior, 9.2 Comment Management, 9.3 Export Behavior, 9. Business Rules + +### Community 117 - "10. Deployment & Distribution" +Cohesion: 0.50 +Nodes (4): 10.1 File Structure, 10.2 Distribution Methods, 10.3 Usage Instructions, 10. Deployment & Distribution + +### Community 118 - "11. Testing Strategy" +Cohesion: 0.50 +Nodes (4): 11.1 Bundle Generation Tests, 11.2 Browser Compatibility Tests, 11.3 Functional Tests, 11. Testing Strategy + +### Community 119 - "1. System Overview" +Cohesion: 0.50 +Nodes (4): 1.1 Purpose, 1.2 Design Goals, 1.3 Design Principles, 1. System Overview + +### Community 120 - "5. Technology Stack" +Cohesion: 0.50 +Nodes (4): 5.1 Bundle Generation (Node.js), 5.2 Frontend (Browser), 5.3 Performance Optimizations, 5. Technology Stack + +### Community 121 - "6. Security Architecture" +Cohesion: 0.50 +Nodes (4): 6.1 Content Security Policy, 6.2 XSS Prevention, 6.3 Data Validation, 6. Security Architecture + +### Community 122 - "7. Performance Considerations" +Cohesion: 0.50 +Nodes (4): 7.1 Bundle Size Optimization, 7.2 Runtime Performance, 7.3 Memory Management, 7. Performance Considerations + +### Community 123 - "sentinel.md" +Cohesion: 0.50 +Nodes (3): 2024-03-03 - [Fix Command Injection Vulnerabilities in Node.js Execution], 2024-03-04 - [Fix DOM XSS in Frontend Template Interpolation], 2024-10-18 - [Fix DOM-based XSS in Visual Review Interface] + +### Community 124 - "✨ Core Features" +Cohesion: 0.50 +Nodes (4): AI Integration, ✨ Core Features, Developer Experience, Visual Review Interface + +### Community 125 - "πŸ“‹ Usage" +Cohesion: 0.50 +Nodes (4): Basic Workflow, CLI Commands, Quick Script Usage, πŸ“‹ Usage + +### Community 126 - "πŸš€ Quick Start" +Cohesion: 0.50 +Nodes (4): Option 1: One-line Installation, Option 2: NPM Installation (when published), Option 3: Manual Setup, πŸš€ Quick Start + +### Community 127 - "πŸš€ **What's New in v2.0**" +Cohesion: 0.50 +Nodes (4): ⚑ **Performance Enhanced**, πŸ”’ **Security Hardened**, 🎨 **UX Improvements**, πŸš€ **What's New in v2.0** + +### Community 128 - "server.test.js" +Cohesion: 0.50 +Nodes (3): supertest, app, request + +### Community 129 - "install-agent-browser.sh" +Cohesion: 0.67 +Nodes (3): install_cli(), PATH, install-agent-browser.sh script + +### Community 130 - "install-cursor-native-stack.sh" +Cohesion: 0.67 +Nodes (3): PATH, install-cursor-native-stack.sh script, try() + +### Community 131 - "quick-ai-review.sh" +Cohesion: 0.83 +Nodes (3): is_too_large(), quick-ai-review.sh script, should_process_file() + +### Community 132 - "vendor-praxstack-extras.sh" +Cohesion: 0.83 +Nodes (3): copy_skill_files(), copy_teach_pro_max_slim(), vendor-praxstack-extras.sh script + +### Community 133 - "βš™οΈ CI/CD Integration" +Cohesion: 0.50 +Nodes (4): βš™οΈ CI/CD Integration, GitHub Actions, GitLab CI, Jenkins Pipeline + +### Community 134 - "🐳 Docker Deployment" +Cohesion: 0.50 +Nodes (4): Docker Compose, 🐳 Docker Deployment, Dockerfile, Kubernetes Deployment + +### Community 135 - "test_cors_logic.js" +Cohesion: 0.50 +Nodes (3): ALLOWED_ORIGINS, isOriginAllowed(), testCases + +### Community 136 - "11. Risks and Mitigation" +Cohesion: 0.67 +Nodes (3): 11.1 Technical Risks, 11.2 Business Risks, 11. Risks and Mitigation + +### Community 137 - "14. Appendix" +Cohesion: 0.67 +Nodes (3): 14.1 Glossary, 14.2 References, 14. Appendix + +### Community 138 - "2. Stakeholders" +Cohesion: 0.67 +Nodes (3): 2.1 Primary Stakeholders, 2.2 Secondary Stakeholders, 2. Stakeholders + +### Community 139 - "12. Monitoring & Metrics" +Cohesion: 0.67 +Nodes (3): 12.1 Generation Metrics, 12.2 Usage Metrics (Optional, Opt-In), 12. Monitoring & Metrics + +### Community 140 - "13. Future Enhancements" +Cohesion: 0.67 +Nodes (3): 13.1 Phase 2 Features, 13.2 Phase 3 Features, 13. Future Enhancements + +### Community 141 - "4. Data Model" +Cohesion: 0.67 +Nodes (3): 4.1 Embedded Review Data Structure, 4.2 Comment Data Structure, 4. Data Model + +### Community 142 - "8. Error Handling" +Cohesion: 0.67 +Nodes (3): 8.1 Generation Errors, 8.2 Runtime Errors, 8. Error Handling + +### Community 143 - "9. Integration Points" +Cohesion: 0.67 +Nodes (3): 9.1 CLI Integration, 9.2 Web Interface Integration, 9. Integration Points + +### Community 145 - "dependencies" +Cohesion: 0.67 +Nodes (3): dependencies, cors, express + +### Community 146 - "repository" +Cohesion: 0.67 +Nodes (3): repository, type, url + +### Community 147 - "πŸ› Troubleshooting" +Cohesion: 0.67 +Nodes (3): Common Issues, Performance Issues, πŸ› Troubleshooting + +### Community 148 - "πŸ”§ Configuration" +Cohesion: 0.67 +Nodes (3): πŸ”§ Configuration, Environment Variables, Project Configuration + +### Community 149 - "🀝 Contributing" +Cohesion: 0.67 +Nodes (3): 🀝 Contributing, Development Setup, Reporting Issues + +### Community 150 - "πŸ“Š Export Formats" +Cohesion: 0.67 +Nodes (3): πŸ“Š Export Formats, Individual File Reviews, Unified Review (`AI_REVIEW.md`) + +### Community 151 - "galleryBanner" +Cohesion: 0.67 +Nodes (3): galleryBanner, color, theme + +### Community 152 - "repository" +Cohesion: 0.67 +Nodes (3): repository, type, url + +## Knowledge Gaps +- **1171 isolated node(s):** `{ spawn, execFileSync }`, `path`, `fs`, `VERSION`, `args` (+1166 more) + These have ≀1 connection - possible missing edges or undocumented components. (Counts symbols only; 1265 node(s) total have ≀1 connection when file, concept and rationale nodes are included.) +- **17 thin communities (<3 nodes) omitted from report** β€” run `graphify query` to explore isolated nodes. + +## Suggested Questions +_Questions this graph is uniquely positioned to answer:_ + +- **Why does `main()` connect `api-client.py` to `.generateUnifiedReview`, `export-ai-review.js`?** + _High betweenness centrality (0.033) - this node is a cross-community bridge._ +- **Why does `STOP conditions` connect `export-ai-review.js` to `api-client.py`, `Plan 009: Drive CLI export listing through the Git working-tree module`?** + _High betweenness centrality (0.026) - this node is a cross-community bridge._ +- **Why does `isTooLarge()` connect `export-ai-review.js` to `ai-review.js`?** + _High betweenness centrality (0.022) - this node is a cross-community bridge._ +- **Are the 8 inferred relationships involving `GitStatusParser` (e.g. with `Current state` and `Step 1: Trim `server.js` requires`) actually correct?** + _`GitStatusParser` has 8 INFERRED edges - model-reasoned connections that need verification._ +- **What connects `{ spawn, execFileSync }`, `path`, `fs` to the rest of the system?** + _1171 weakly-connected nodes found - possible documentation gaps or missing edges._ +- **Should `GitStatusParser` be split into smaller, more focused modules?** + _Cohesion score 0.05727644652250146 - nodes in this community are weakly interconnected._ +- **Should `GitService` be split into smaller, more focused modules?** + _Cohesion score 0.07138047138047138 - nodes in this community are weakly interconnected._ \ No newline at end of file diff --git a/docs/audit/README.md b/docs/audit/README.md new file mode 100644 index 0000000..bcaeb27 --- /dev/null +++ b/docs/audit/README.md @@ -0,0 +1,3 @@ +# Audit reports (2026-10) + +These files come from a repository audit run from 2026-10-02 to 2026-10-11 and accompany PR #43 (`fix/audit-2026-10`). `UNDERSTANDING.md` describes the tool's purpose, architecture, flows and the problems found at audit time; `SETUP.md` gives the verified install, test and run steps; `GRAPH_REPORT.md` is the generated code/document knowledge-graph summary; `FIXES.md` records what the PR fixed and how it was tested; `REVIEW.md` is the independent review of the first fix commits (its findings were addressed later in the same PR). This repository is public, so security details (security reports, open security findings, raw findings and owner notes) are kept privately by the owner and are not included here. diff --git a/docs/audit/REVIEW.md b/docs/audit/REVIEW.md new file mode 100644 index 0000000..92a212d --- /dev/null +++ b/docs/audit/REVIEW.md @@ -0,0 +1,85 @@ +# REVIEW - ai-visual-code-review, branch fix/audit-2026-10 + +Reviewer: independent (Opus), 2026-10-08. Separate fix worktree, HEAD `bd3ec1d`, merge-base with origin/main `c4d6472`. Read-only: no edits, commits or checkouts in the worktree. "Before" behaviour was tested in a throwaway worktree (removed afterwards). Branch `git status` was clean before and after every run. + +> Note: this review is of the first three commits. MAJOR-1, MINOR-1 to MINOR-4 and N2 were addressed by later commits on PR #43 before merge. + +## Verdict: request-changes + +Two of the three commits do what they claim. The AVCR-001 fix is incomplete: paths are still passed to git as *glob* pathspecs, so the Next.js case named in the finding (`app/[id]/page.tsx`) still pulls other files' diffs into its section whenever a sibling path matches the character class. This is a one-line fix (`--literal-pathspecs` or a `:(literal)` prefix). The new unified-export comment code also puts line comments under the wrong files when one sanitized filename is a substring of another (`index.js` and `src/index.js`). + +Counts: blocker 0, major 1, minor 4, nit 6. + +## Per-commit verdicts + +| commit | finding | verdict | summary | +|---|---|---|---| +| 022592b fix(git): pass pathspecs verbatim after -- | AVCR-001 | request-changes | Paths are no longer dropped, and arguments that start with `-` are safe. Glob and magic pathspec semantics are still active, so `[id]`, `*` and `?` names still match other files. The test fixture misses that case. | +| 4529da8 fix(export): include reviewer comments in unified review | AVCR-002 | approve-with-nits (minor) | Comments now reach AI_REVIEW.md. Line comments are matched with `includes()` on a substring, which leaks them across files. The logic is copied from the per-file generator, which has the same bug. | +| bd3ec1d fix(server): bind 127.0.0.1 by default, HOST env | AVCR-005 (partial) | approve-with-nits | Verified live that the server listens on 127.0.0.1 only and the LAN IP is refused. Docker/compose set HOST=0.0.0.0 and compose publishes on loopback. One unverified risk on Node 18 healthchecks. | + +## Findings + +### MAJOR-1: AVCR-001 not fixed at the root. Pathspecs are still interpreted as globs and magic +- Where: `services/GitService.js:36-48` (pass-through), `:85` (`getDiffForFile`), `server.js:415`. +- Evidence (probe in a temp repo with staged `app/[id]/page.tsx`, `app/i/page.tsx`, `app/d/page.tsx`, `a*b.txt`, `aXb.txt`): + - Branch: `getDiffForFile('app/[id]/page.tsx')` returns the diffs of `app/[id]/page.tsx`, `app/i/page.tsx` and `app/d/page.tsx`. Base returned every staged file. Better, but the export section for `[id]` still contains the siblings' diffs. + - `getDiffForFile('a*b.txt')` returns `a*b.txt` and `aXb.txt` on both base and branch. + - Magic pathspecs are now reachable: `:(exclude)a.js` returns every file except a.js on the branch. On base, the old filter dropped it and every file was returned. A staged file whose name starts with `:` (for example `:!x`) gets magic treatment in the export. `/api/file-diff?file=:/` returns the whole staged diff (200; this is pre-existing, since `:` was never filtered). + - Git matches a literal path first and then also applies wildmatch. That is why the fixer's fixture passes: none of its four names glob-match each other (`test/git-service-paths.test.js:10`). +- Suggested fix: make git treat paths literally. Use `git --literal-pathspecs diff --cached -- ` (a global option, so it goes before `diff` in `ALLOWED_COMMANDS`), or set `GIT_LITERAL_PATHSPECS=1` in the execFile env, or prefix each path arg with `:(literal)`. Add a sibling such as `app/i/page.tsx` and a `*`-named file to the test fixture. This also disables `:/` and `:(exclude)`. + +### Option and argument injection (paths starting with `-`): no issue found +- Every path is passed after a literal `--` (`GitService.js:85`, `server.js:415`), and execFile uses argv with no shell. A probe of `getDiffForFile('--output=pwn.txt')` and `getDiffForFile('-p')` returned only those files' own diffs, and no output file was written. Live `/api/file-diff?file=--output%3Dpwn.txt` returned 400 (not staged) and created no file. NUL bytes and non-strings are now rejected outright instead of being silently dropped. All callers pass strings, so this is fine. +- The arguments before `--` keep the old character filter. No caller passes user data there (only `--name-status` at `server.js:347`). FIXES.md discloses this. + +### MINOR-1: Line comments land under the wrong files in the unified export +- Where: `services/ReviewGenerator.js:179-180` (new), the same pattern as the existing `:275-276`. +- Evidence: the client lineId is `${file.replace(/[^a-zA-Z0-9]/g,'_')}_${chunk}_${line}` (`public/index.html:1456`). The filter is `lineId.includes(sanitized)`. In a probe export with `a.js`, `data.js`, `src/x.js` and `src_x.js`, the comment `data_js_0_1` appeared twice (under data.js and also under a.js), and `src_x_js_0_2` appeared twice (under `src/x.js` and `src_x.js`). The common real case is `index.js` together with `src/index.js`. +- Suggested fix: match `new RegExp('^' + escape(sanitized) + '_\\d+_\\d+$')`. Better still, have the client send `{file, line}` instead of an opaque id. Apply the same fix to the per-file generator. + +### MINOR-2: The web view still rejects special-character paths, and FIXES.md does not say so +- Where: `services/diffService.js:136` (`dangerousChars` includes `( ) [ ] { } $`), used by `server.js:172` and `:226`. +- Evidence: live `GET /api/file-diff?file=app%2F%5Bid%5D%2Fpage.tsx` returns 400 "Invalid file path - potential security risk". File comments on such files are rejected by `validateExportRequest` (`server.js:226`). SETUP 5.1 / UNDERSTANDING 9.13 describe this as half of the user-visible bug. The "remains" column in FIXES.md for AVCR-001 does not mention it. +- Suggested fix: once pathspecs are literal (MAJOR-1), relax `isValidFilePath` to drop the shell-metacharacter class. Alternatively, record it in FIXES.md as remaining. + +### MINOR-3: Docker healthcheck uses `localhost` but the server now binds IPv4 only (unverified) +- Where: `Dockerfile:51-52`, `docker-compose.yml:21`, with `ENV HOST=0.0.0.0` (`Dockerfile:57`). +- Before this change the server bound `::` (dual-stack), so `localhost` resolving to `::1` still connected. Now the bind is 0.0.0.0 only. On `node:18-alpine`, `dns.lookup` uses verbatim order and Node 18 has no default autoSelectFamily, so `http.get('http://localhost:3002')` can fail with ECONNREFUSED on `::1` if the container's loopback has IPv6, and the container would then be marked unhealthy. Not verified: the node:18-alpine image is not cached locally and I did not pull it. CI's `curl localhost` runs on the host and falls back between families, so it is fine. +- Suggested fix: use `http://127.0.0.1:3002/api/health` in both healthchecks, or default HOST to `::` in Docker. + +### MINOR-4: The AVCR-001 test cannot detect the remaining bug +- `test/git-service-paths.test.js`: the fixture names never glob-match each other, so the test passes even though the bug in MAJOR-1 remains. Covered by the MAJOR-1 fix suggestion. + +### Nits +- N1 `test/bind-host.test.js:18-22`: the third test regex-greps the source of server.js and docker-compose.yml. That checks implementation trivia, not behaviour. A real listen on port 0 that checks `server.address().address` would test the actual behaviour. +- N2 `test/unified-review-comments.test.js:23`: it uses lineId `a_js_line_1`, which is not the client format `a_js__`, and it does not assert isolation between files (which would have caught MINOR-1). +- N3 `server.js:807`: `app.HOST = HOST` is attached to the Express app only so the test can read it. +- N4 `HOST` is not documented in README or `ai-review --help`, and the startup banner still prints `http://localhost:${PORT}` regardless of HOST. +- N5 Lint now reports 32 warnings, up from 31. The new one is `max-lines-per-function` on `generateUnifiedReview` (110 lines, limit 100). +- N6 FIXES.md says the safe run was "7 suites, 107/107". It was actually 8 suites (5 original + 3 new) with 107 tests. + +### Default bind and Docker/compose consistency (focus area): consistent +- `server.js:16` sets `HOST = process.env.HOST || '127.0.0.1'`, and `app.listen(PORT, HOST, ...)` is at `:780`. Live check: `lsof` showed `node 127.0.0.1:39517`. `localhost` and `127.0.0.1` returned 200, and `http://:39517` was refused. The server was stopped afterwards. +- `bin/ai-review.js start` spawns server.js with the inherited env, so HOST passes through, and it opens `http://localhost:port` (browsers fall back between families). The VS Code extension has no hard-coded host. +- Dockerfile `ENV HOST=0.0.0.0` is needed so container port publishing works. Compose sets HOST for both services. This is redundant for `production` but needed for the `dev` service (target `builder`, which has no ENV). Both compose services publish `127.0.0.1:3002:3002`. CI `docker run -p 3002:3002` still exposes all host interfaces, which is acceptable on a CI runner. The Docker image is still non-functional (no git, and compose `working_dir: /workspace` cannot find server.js). This is AVCR-004, and FIXES.md discloses it. +- FIXES.md correctly marks AVCR-005 as partial; the remaining remote-access item is an owner decision (details kept privately by the owner). + +## Check results (Node v24.21.0, git 2.56.0) + +| check | base c4d6472 | branch bd3ec1d | +|---|---|---| +| `npm run build` | exit 0 (placeholders) | exit 0 (placeholders) | +| `npm run lint` | exit 0, 0 errors, 31 warnings | exit 0, 0 errors, 32 warnings | +| safe jest (5 SETUP suites) | 5/5 suites, 99/99 | 5/5 suites, 99/99 | +| safe jest + 3 new suites | new suites: 7 of 8 tests fail (only `plain.txt` passes) | 8/8 suites, 107/107 | +| `npm test` (full) | not run (rewrites tracked files, per SETUP) | not run | +| `git status` after runs | n/a | clean | + +New tests run against base: `git-service-paths` failed 3 of 4 (plain.txt passes), `unified-review-comments` failed 1 of 1, and `bind-host` failed 3 of 3. This confirms that each test fails without its fix. + +## FIXES.md claims I could not confirm, or that are overstated +- AVCR-001 "fixed": overstated. Glob and magic pathspec semantics still mix in other files' diffs (MAJOR-1), and the web view still rejects these paths (MINOR-2). +- "Safe test command: 7 suites": it was 8 suites (N6). The 107/107 count is correct. +- AVCR-005 Docker behaviour: could not be verified end-to-end. The image is non-functional (AVCR-004), and the healthcheck risk in MINOR-3 is unverified. +- Everything else was confirmed: the tests fail before and pass after, compose publishes on loopback, and the default bind is 127.0.0.1. diff --git a/docs/audit/SETUP.md b/docs/audit/SETUP.md new file mode 100644 index 0000000..aff6c73 --- /dev/null +++ b/docs/audit/SETUP.md @@ -0,0 +1,67 @@ +# SETUP β€” ai-visual-code-review (macOS, verified 2026-10-02/04) + +Repo: `praxstack/ai-visual-code-review`, HEAD `c4d6472`, Node v24.21.0. Plain Node/Express app, no env vars, no API keys, no AI calls. No Xcode needed. + +## 1. Install +``` +cd ai-visual-code-review +npm ci # 443 pkgs +(cd vscode-extension && npm ci) # optional, only for the VS Code extension +``` + +## 2. Build / lint +``` +npm run build # two echo placeholders, exit 0, builds nothing +npm run lint # exit 0, 0 errors, 31 warnings (logs/lint.log) +(cd vscode-extension && npm run compile) # tsc exit 0 +``` + +## 3. Tests +SAFE (use this one): +``` +npx jest test/server.test.js test/diffService.test.js test/gitStatusParser.test.js \ + test/spaces-in-paths.test.js test/agent-skill-packs.test.js +# 5 suites, 99/99 pass, ~1.5 s (re-run 2026-10-04) +``` +FULL `npm test` (6 suites, 109/109 pass, but 239 s) β€” DO NOT run it inside the repo. `test/link-agent-skills.test.js` runs `scripts/link-agent-skills.sh` with cwd = repo root and rewrites 20 tracked `.cursor/skills/*` symlinks plus `docs/cursor-skill-collisions.md` (diff saved in `logs/test-side-effect.patch`). Run it in a throwaway clone: +``` +git clone /tmp/avcr-clone && cd /tmp/avcr-clone && npm ci && npm test +``` +If you did run it in place: `git checkout -- .cursor docs/cursor-skill-collisions.md`. +Results: 0 failures, 0 skipped. The 109 pass was observed in a clone (`logs/npm-test.log`, `logs/jest-results.json`). + +## 4. Run the app +Web UI (needs a git repo with STAGED changes as cwd): +``` +cd /path/to/some/git/repo && git add -A +PORT=3002 node /server.js # or: node .../bin/ai-review.js start +open http://localhost:3002 +``` +- Default port 3002 (env `PORT`). At audit time `PORT` was the only env var and there was no `HOST` setting; PR #43 makes the server bind `127.0.0.1` by default and adds a `HOST` env var. +- Verified with curl on 127.0.0.1 (logs/server-smoke.log): `/` 200 HTML, `/api/health`, `/api/summary`, `/api/staged-files`, `/api/file-diff?file=...` 200, unknown routes 404, `POST /api/log-comment`, `POST /api/export-for-ai` write a `code-reviews-/` dir. +- Startup banner prints "Open: http://localhost:3002" regardless of `PORT` (cosmetic). + +CLI: +``` +node bin/ai-review.js --version # ai-visual-code-review v2.4.1 +node bin/ai-review.js --help +cd && node /bin/ai-review.js quick # writes AI_REVIEW.md +``` +Outside a git repo it exits 1 with a clear message (verified). + +## 5. Known problems you will hit when testing +(Detail and repro in `UNDERSTANDING.md` section 9.) +1. Staged paths containing `( ) [ ] { } $` (Next.js `app/[id]/page.tsx`) make the export include the entire staged diff, and the web view rejects them with 400 (9.1, 9.13). +2. Web file rows render duplicated labels and diff inside the header; clicks misbehave (9.2). +3. Unified export drops all reviewer comments; page prints "Files processed: undefined" (9.3). +4. Diff lines beginning `--`/`++` vanish (9.8). CLI max-file-size limit is dead (9.10). +5. Rate limiter with `NODE_ENV=production` counts all requests against the export limit (9.9); dev mode hides it. +6. Docker: image has no `git`, compose `working_dir` is wrong, so it cannot work (9.6). Not built here. +7. `scripts/link-agent-skills.sh` / `install-agent-skills.sh` are written for throwaway cloud VMs and fail on macOS `/bin/bash` 3.2 (`declare -A`). Do not run them on a workstation. +8. VS Code extension compiles but is a stub (panel shows "Loading..." forever, 9.16). + +## 6. Environment notes +- Required env vars: none. Optional: `PORT`, `NODE_ENV`. +- Tracked file count is 11,679 because ~10.5k are vendored agent-skill packs under `.claude/skills`, `.agents/skills`, `.cursor/skills` (section 2 of UNDERSTANDING.md); the product is ~170 files. +- Cleanup state: no processes of this audit left running. Repo working tree clean. +- Xcode: not applicable. diff --git a/docs/audit/UNDERSTANDING.md b/docs/audit/UNDERSTANDING.md new file mode 100644 index 0000000..329e32f --- /dev/null +++ b/docs/audit/UNDERSTANDING.md @@ -0,0 +1,296 @@ +# UNDERSTANDING β€” ai-visual-code-review + +Audit date: 2026-10-02. Repo: GitHub `praxstack/ai-visual-code-review`, PUBLIC, default branch `main`, HEAD `c4d6472`, working tree clean before and after this pass). + +Everything below was re-derived from the repo in this pass. Leftovers from the interrupted earlier run were not trusted; where I re-ran something the command is given. Line numbers refer to HEAD `c4d6472`. + +--- + +## 1. What it is + +A local developer tool that turns **staged git changes** into (a) a GitHub-style dark web UI for reading diffs and attaching comments, and (b) a markdown file (`AI_REVIEW.md`, or one file per source file) meant to be pasted into ChatGPT/Claude for review. There is no AI call anywhere in the code: "AI integration" means "generates markdown plus a checklist for you to hand to an AI". + +Three front ends share one small service layer: + +| Front end | Entry | State | +|---|---|---| +| Web UI + JSON API | `server.js` (Express 4), `public/index.html` | Works, with serious UI and export defects (section 9) | +| CLI | `bin/ai-review.js` β†’ `scripts/export-ai-review.js` | Works, with defects on special paths and size limits | +| VS Code extension | `vscode-extension/` (TypeScript) | Compiles; functionally a stub | + +Published on npm as `ai-visual-code-review` (`latest` = 2.4.1, published 2025-12-15). `main` is 40 app-code commits ahead of that publish, including the security fixes. + +Maturity: small working prototype with heavy "enterprise-grade / production-ready" marketing in the docs. Real application code is about 3,000 lines of JS plus a 1,756-line single-file web page and about 1,150 lines of TypeScript. + +## 2. Why the repo has 11,679 tracked files + +`git ls-files | wc -l` = 11,679. Only about 170 belong to the product. + +| Path | Tracked files | Bytes | What it is | +|---|---|---|---| +| `.claude/skills/` | 5,267 | 50.3 MB | 26 vendored third-party agent-skill packs | +| `.agents/skills/` | 5,267 | 50.3 MB | Byte-identical second copy (`diff -rq` differs only in `README.md`) | +| `.cursor/skills/` | 976 | small | 970 relative symlinks into `.claude/skills` (mode 120000) | +| `.cursor/agents`, `.claude/agents`, `.codex/agents` | 21 | small | 7 persona files Γ— 3 harnesses | +| everything else | ~148 | ~2.4 MB | the product, docs, plans | + +Total tracked payload is about 103 MB; 99% is vendored agent tooling added in August 2026 (PRs #35–#42). It contains 710 `.py`, 298 `.mjs`, 144 `.sh` third-party scripts, twice. Only `spec-kit` is pinned to a tag (`scripts/install-agent-skills.sh` `SPECKIT_TAG="v1.0.1"`); every other pack is cloned from the upstream default branch with `git clone --depth 1` and no commit SHA is recorded anywhere (0 forty-hex strings in `docs/agent-skills.md`, `docs/agent-skill-packs.md`, `.claude/skills/README.md`). + +Other things committed that should not be: `AI_REVIEW_QUICK.md` (196 KB) and `AI_REVIEW_WEB.md` (241 KB) β€” generated output from 2025-10-11; `vscode-extension/out/` (compiled JS); `vscode-extension/ai-visual-code-review-1.0.0.vsix` (321 KB, stale); `test-output.txt`; `test_cors_logic.js`; both `package-lock.json` and `pnpm-lock.yaml` at root and in `vscode-extension/`. + +The npm package is not polluted by this: `package.json` `files` whitelists `bin/ public/ services/ scripts/ src/ server.js README.md LICENSE` (the published 2.4.1 tarball contains exactly those). + +## 3. Documents: authoritative vs historical + +| Document | Status | Notes | +|---|---|---| +| `plans/README.md` + `plans/001…009` | **Authoritative backlog** (2026-08-29) | Nine plans, all `TODO`. Accurate against HEAD; I re-verified 002, 003, 004, 005, 006, 008 | +| `AGENTS.md` | Authoritative for agent workflow | Mostly about skill packs. References `CONTEXT.md` and `docs/adr/` which do not exist | +| `CLAUDE.md`, `.claude/CLAUDE.md`, `.cursor/rules/*.mdc` | Current, tooling-only | graphify / context7 / pstack model routing | +| `openspec/config.yaml` | Current but empty | No specs or changes exist; `openspec/changes/` and `openspec/specs/` are gitignored | +| `docs/agent-skill-packs.md`, `docs/agent-skills.md`, `docs/mcp.md`, `docs/graphify.md`, `docs/openspec.md`, `docs/agents/*` | Current, tooling-only | Not about the product | +| `docs/cursor-skill-collisions.md`, `docs/praxstack-skill-collisions.md` | Generated | Rewritten every time the linker runs (and by `npm test`) | +| `ARCHITECTURE.md` | Stale (v2.3.0, Dec 2025) | Describes `src/` as live; it is dead code | +| `README.md` | Stale ("v2.0") | Wrong repo URL, missing files, undocumented/nonexistent features | +| `CHANGELOG.md` | Stale | Stops at 2.3.0; package is 2.4.1; nothing for 2.3.1, 2.4.0, 2.4.1 or the 2026 work | +| `SECURITY.md`, `SETUP.md`, `CONTRIBUTING.md`, `UNIFIED_EXPORT_GUIDE.md` | Stale | Document env vars and scripts that do not exist | +| `UPDATE_GUIDE.md` | Historical | "v1.0.3 β†’ v1.0.4" | +| `memory-bank/*` | Historical (Dec 2025) | Says "30/30 tests", "all critical issues resolved" | +| `docs/BRD-*.md`, `docs/HLD-UI-Export-Bundle.md` | Draft / Design, never built | HTML bundle export has zero code; extension BRD phases 2–4 not started | +| `docs/07-validation/QA_REVIEW_v2.3.1.md` | Historical self-certification | "88% production ready", "DevOps 95%" β€” not supported by the code | +| `star-team-audit/` | Historical (Dec 2025 audit, 138 issues) | Source of the CR-/HI- ids used in CHANGELOG | +| `TEST_PLAN.md`, `TEST_RESULTS.md`, `FINAL_TEST_RESULTS.md`, `COMPREHENSIVE_TEST_SCENARIOS.md`, `NPM_SCRIPTS_ANALYSIS.md`, `TWITTER_LAUNCH_THREADS.md`, `.clinerules`, `.jules/*`, `.Jules/palette.md` | Historical | Oct 2025 launch material and bot journals | + +## 4. Architecture + +``` + browser (public/index.html, vanilla JS, inline script) CLI (bin/ai-review.js) + β”‚ fetch /api/* β”‚ spawn / execFileSync + β–Ό β–Ό + server.js (Express 4.21.2, cors) ───────────────► scripts/export-ai-review.js + β”‚ β”‚ + β”œβ”€β”€ services/GitService.js allowlisted `git` via execFile, cwd = process.cwd() + β”œβ”€β”€ services/diffService.js unified-diff parser + markdown formatter + path validator + β”œβ”€β”€ services/gitStatusParser.js status code β†’ label/icon/markdown header + └── services/ReviewGenerator.js writes AI_REVIEW.md or code-reviews-/*.md into cwd + + src/config, src/middleware, src/utils ← never required by anything (dead copy of server.js internals) + + vscode-extension/ (separate package) ← own TS copies of gitService/diffService, no shared code +``` + +The server always operates on `process.cwd()`: you run `ai-review` inside the repo you want reviewed, and the tool's own install dir only supplies `public/` and code. + +### Module map + +| Path | Lines | Role | +|---|---|---| +| `server.js` | 803 | Whole HTTP app: CORS allow-list, JSON body, request logger, static, security headers, in-memory rate limiter, inline validators, 7 API routes, dead cache middleware, `app.listen` | +| `bin/ai-review.js` | 242 | CLI arg parsing; `start/serve/s` spawns `node server.js`; `quick/q` runs the export script; `--open` launches a browser | +| `scripts/export-ai-review.js` | 319 | CLI export: include/exclude glob matching, (dead) size limit, calls `ReviewGenerator` | +| `scripts/quick-ai-review.sh` | 383 | Legacy bash exporter (still shipped) | +| `services/GitService.js` | 111 | Singleton; 5 allowlisted git commands; argument filter; helpers `getStagedFiles/getDiffForFile/getDiffStats/getFileStatuses` | +| `services/diffService.js` | 156 | `parseDiff`, `formatEnhancedDiff`, `generateEnhancedDiffMarkdown`, `isValidFilePath` | +| `services/gitStatusParser.js` | 251 | Status parsing/labels | +| `services/ReviewGenerator.js` | 356 | `generateUnifiedReview`, `generateSplitReviews`, `generateFileContent`, checklist template | +| `src/**` | 458 | Dead: config, nonce-CSP middleware, rate limiter, validators, git wrapper β€” `grep -rn "require('./src\|require('../src"` finds nothing | +| `public/index.html` | 1,756 | CSS (lines 7–625), markup, one inline script (711–1754): `AppState`, loaders, accordion, comment modal, exports | +| `vscode-extension/src/*` | 1,157 | `extension.ts` (commands, status bar, export), `gitService.ts`, `diffService.ts`, `stagedFilesProvider.ts`, `reviewWebviewProvider.ts` | +| `scripts/install-agent-skills.sh`, `link-agent-skills.sh`, `link-praxstack-skills.sh`, `vendor-praxstack-extras.sh`, `gen-skills-index.js`, `install-cursor-native-stack.sh`, `install-agent-browser.sh` | 1,567 | Agent-tooling vendoring and Cursor Cloud boot, unrelated to the product | +| `test/*.test.js` | 6 suites | See section 7 | +| `run-tests.js`, `test/api-client.py`, `test/utils.js`, `test/sample-component.tsx`, `test/config.json` | β€” | Legacy runner and fixtures, not part of `npm test` | + +### Entry points + +- `npm start` / `node server.js` β†’ `app.listen(PORT)` (default 3002). +- `ai-review [start|serve|s] [--port N] [--open]`, `ai-review quick|q [--include … --exclude … --max-size N --no-size-limit --split --output-dir D]`, `ai-review -v|-h`. +- `node scripts/export-ai-review.js …`, `./scripts/quick-ai-review.sh …`. +- VS Code commands `aiCodeReview.openReview | exportForAI | quickReview | refreshFiles`. +- `.cursor/environment.json` install hook: `npm ci && bash scripts/install-cursor-native-stack.sh`. + +### HTTP API (all in `server.js`) + +| Route | Behaviour | +|---|---| +| `GET /` | `public/index.html`, served by `express.static` (line 137) | +| `GET /api/health` | staged/unstaged counts, `cwd`, hard-coded `version: '1.0.0'` (line 320) | +| `GET /api/summary` | `git diff --cached --stat` | +| `GET /api/staged-files` | names, `fileStatuses` (from `--name-status`), unstaged deletions | +| `GET /api/file-diff?file=` | validates path, returns raw diff + parsed chunks | +| `POST /api/log-comment` | logs a sanitized line to stdout; stores nothing | +| `POST /api/export-for-ai` | writes `AI_REVIEW.md` into cwd | +| `POST /api/export-individual-reviews` | writes `code-reviews-/review-*.md` + `00_SUMMARY.md` into cwd | + +### Main user flow (web) + +1. `git add …` in the target repo, run `ai-review`. +2. Page loads `/api/health`, `/api/summary`, `/api/staged-files`; re-polls health every 30 s. +3. Click a file row β†’ `/api/file-diff` β†’ rendered diff; per-line comment button and file comment button open one shared modal. +4. Comments live in page memory only (`AppState.fileComments`, `AppState.lineComments`). +5. "Export for AI Review" POSTs comments + unchecked files; server writes the markdown. + +## 5. Data, state, persistence, external services + +- **No database, no config file, no keychain, no cookies, no localStorage** (`grep localStorage public/index.html` β†’ none). Comments are lost on reload; README's "persistence" is not implemented. +- Server memory: `rateLimitStore` (Map ip β†’ timestamps, never pruned for idle IPs), `requestCache` (dead). +- Files written: `AI_REVIEW.md` and `code-reviews-*/` in the reviewed repo's cwd (both gitignored in this repo, not in the user's). +- Inputs read: only `git` output for the cwd. Environment variables actually read: `PORT`, `NODE_ENV` (plus `HTTPS_*` in dead `src/config`). No `dotenv`; `.env.example` is decorative. +- External services at runtime: none. No telemetry, no network calls. Build/tooling time: npm registry; GitHub (skill vendoring); `curl | bash` installers for bun and uv, `npm i -g @fission-ai/openspec@latest`, `agent-browser`; MCP servers `npx -y @upstash/context7-mcp` and `uvx --from git+https://github.com/oraios/serena` (all unpinned) in `.cursor/mcp.json`. CI uploads to Codecov. +- No secrets required. No API keys anywhere in the product. + +## 6. Build, run, test + +| Purpose | Command | Result in this pass | +|---|---|---| +| Install | `npm ci` | `node_modules` present and valid (express 4.21.2, jest 29.7.0) | +| Run | `npm start` or `node bin/ai-review.js` | Starts; binds all interfaces by default (section 9.7). I ran it only with a preload that forces 127.0.0.1, on ports 39417/39418, and stopped it | +| Tests | `npm test` (jest) | 5 non-mutating suites: **99/99 pass in 2.3 s** (`npx jest test/server.test.js test/diffService.test.js test/gitStatusParser.test.js test/spaces-in-paths.test.js test/agent-skill-packs.test.js`). The sixth, `test/link-agent-skills.test.js`, was not run in the repo because it rewrites tracked files (section 9.11) | +| Lint | `npm run lint` | exit 0, 31 warnings, 0 errors | +| Build | `npm run build` | Two `echo` placeholders; nothing is built | +| Extension | `cd vscode-extension && npm ci && npm run compile` | `tsc` exit 0; compiled JS identical to tracked `out/` (only source maps differ) | +| Prod audit | `npm audit --omit=dev` | 2 high, 2 moderate (express ← body-parser / path-to-regexp / qs) | +| Docker | `docker build .` / `docker compose up` | Image has no `git`; see 9.6 | + +## 7. Tests + +| Suite | Tests what | Value | +|---|---|---| +| `server.test.js` | Route shapes, validator rejections, headers on `/api/health`, 429 on 15 parallel exports | Shallow: never stages a file, never checks export content; "accept valid request" accepts 200 or 400 | +| `diffService.test.js` | Parser happy paths, `isValidFilePath` | No `---`/`+++` content lines, no "\ No newline", no binary, no rename | +| `gitStatusParser.test.js` | Status table | Fine | +| `spaces-in-paths.test.js` | One real `execFile`, one `expect(true).toBe(true)` | Near zero | +| `agent-skill-packs.test.js` | Asserts vendored packs/docs exist | Tests the vendoring, not the product | +| `link-agent-skills.test.js` | Runs the linker twice with a temp `HOME` | Mutates tracked files; ~70 s per linker run | + +No test covers `ReviewGenerator` output, `scripts/export-ai-review.js` pattern matching, `bin/ai-review.js`, the web page, or the extension. `jest.config.js` has coverage thresholds commented out. + +## 8. Git and GitHub + +- 125 commits. Authors: Prax Lannister 71, google-labs-jules[bot] 29, Cursor Agent 22, cursor[bot] 3. +- Cadence: Oct 2025 (15), Dec 2025 (12), **Mar 2026 (69 β€” bot PR storm: Sentinel/Bolt/Palette)**, Jun 2026 (3), Aug 2026 (26 β€” skill vendoring). No product change since March 2026 other than the `bee70fd` syntax repair in August. +- Tags: `v1.0.1`, `v1.0.2`, `v2.1.1` only. No GitHub releases. No tag for 2.2.0–2.4.1. +- Open issues: 0. Open PRs: 0. 42 PRs total (36 merged, 6 closed). +- CI: last 15 runs all green; latest main run `33323864552` (2026-08-30): test 18.x, test 20.x, build, docker all pass. Annotations show the lint warnings and a Node 20 action deprecation. +- Remote branches with commits not in `main` (all stale, none has an open PR): + +| Branch | Ahead | Content | +|---|---|---| +| `sentinel-fix-info-disclosure-…` | 1 | Removes `process.cwd()` from the fallback HTML in `server.js` β€” small real fix, unmerged | +| `sentinel-security-fixes-…` | 1 | `bin/ai-review.js` + `index.html` (superseded by main) | +| `jules-bolt-perf-rate-limit-…` | 1 | Tweaks dead `src/middleware/rateLimit.js` + a 771-line AGENTS.md | +| `palette-keyboard-hints-…`, `palette-ux-improvements-…`, `palette/accessible-file-headers-…` | 1 each | More edits to the file-header markup | +| `add-agent-instructions-…`, `jules-add-agent-instructions-…` | 1 each | Agent instruction docs | +| `prax/vendor-skill-packs-3d82` | 12 | Squash-merged as #38 | +| `prax/setup-pstack-cursor-models-3d82` | 2 | PR #37 closed | + +- Repo URL drift: `package.json`, README, CLI help, extension manifest, npm metadata all point at `PrakharMNNIT/ai-visual-code-review` (GitHub currently redirects it to `praxstack/…`). + +### CI workflow (`.github/workflows/ci.yml`) + +- `test` (Node 18.x, 20.x): `npm ci`; lint with `continue-on-error: true`; `npm test`; coverage with `continue-on-error`; `codecov/codecov-action@v3` non-blocking. +- `build`: `npm run build` (two echoes), then compiles the extension. +- `docker` (main only): builds the image, runs it, `curl -f /api/health`. This passes even though the container cannot run git, because `/api/health` uses `Promise.allSettled` and always returns 200. +- No publish/release job; `prepublishOnly: npm test` is the only gate. + +## 9. Concrete problems found while reading (all re-verified unless marked) + +Reproduction repo: a scratch git repo with staged `app/(marketing)/page.tsx` (A), `app/[id]/page.tsx` (M), `config/.env` (A), `docs/notes.md` (M, removes a line `-- dashes line`), `src/big.txt` (A, 12,000 lines), `src/it's.js` (A), a rename, a delete, `src/util.js` (M). + +### 9.1 Paths containing `( ) [ ] { } $` get the whole staged diff +`services/GitService.js:34-41` silently filters out any argument matching `/[;&|`$(){}[\]\\]/`. `getDiffForFile('app/[id]/page.tsx')` therefore runs `git diff --cached --` with no path and returns every staged file's diff. Reproduced: the `(marketing)` section of `AI_REVIEW.md` contained 8 hunks (the file has 1) and the export was 24k lines; `src/big.txt`, which I had excluded, appeared twice. Both the CLI and the web export are affected. `plans/README.md` lists this under "considered and rejected" because `/api/file-diff` validates first, but the export paths do not validate. + +### 9.2 Web page file rows are merge debris +`public/index.html:1201-1262` opens four nested `
` per file with duplicated attributes and both string-built (`toggleFile('${file}', …)`, unescaped) and data-attribute handlers. Observed in a real browser against the sample repo: each path label is rendered twice per row; the diff is rendered inside the header (290 px wide in a 984 px row); a real click on diff text collapses the diff; clicking the header of an already loaded file does not collapse it (even number of handlers fire). `diff-line` rows carry two comment buttons each (lines 1472-1473). This is plan 006, still TODO. ESLint ignores this file and no test loads it. + +### 9.3 Unified export drops all reviewer comments +`server.js:619-624` calls `generateUnifiedReview({ includedFiles, excludedFiles })`; `comments`/`lineComments` are validated, counted and reported back (`commentsIncluded`, line 632) but never written. Reproduced: response `commentsIncluded: 1`, `grep -c COMMENT-MARKER AI_REVIEW.md` β†’ 0. Plan 005, TODO. The page then prints "Files processed: undefined" because it reads `data.filesProcessed` (`index.html:1578`), which the server does not send. + +### 9.4 (security item) +Security-related; details kept privately by the owner. + +### 9.5 Committed `.vsix` is a stale pre-fix build +Security-related; details kept privately by the owner. + +### 9.6 Docker deployment cannot work +`docker run --rm node:18-alpine which git` β†’ not found (verified). `Dockerfile` never installs git, so every git call fails and the app shows "no staged files". `docker-compose.yml` additionally sets `working_dir: /workspace` with the default `CMD ["node","server.js"]`, so it runs `/workspace/server.js` (only exists when reviewing this repo itself), and mounts the workspace `:ro`, so exports cannot be written. CI's docker job is green only because the health route cannot fail. + +### 9.7 Network exposure and response headers +Security-related; details kept privately by the owner. + +### 9.8 Diff parser loses lines +`services/diffService.js:36` skips any line inside a hunk that starts with `---` or `+++`. A removed line whose content starts with `--` (SQL comment, markdown rule, YAML separator, CLI flag) or an added line starting with `++` (`++i;`) vanishes from both the web view and the export, and following line numbers shift. Reproduced with `-- dashes line`: export shows 3 context lines and a phantom 4th. `\ No newline at end of file` and the trailing empty split element are also counted as context lines. + +### 9.9 Export rate limiter shares one bucket with all traffic +`createRateLimit` instances (lines 156 and 543) share `rateLimitStore` keyed only by IP. With `NODE_ENV=production` (the Docker default) the export limit of 10 is compared against every request from that IP. Reproduced: 10 GETs then the first export β†’ `429 {"current":10,"limit":10}`. The page's own 30 s health poll alone keeps the bucket above 10 forever. In the default dev mode localhost is exempt, which hides it. + +### 9.10 CLI size limit is dead +`scripts/export-ai-review.js:20-27` never sets `config.maxFileSize`; line 236 compares `lineCount > undefined`. Reproduced: 12,000-line file exported, "Too large: 0", although help says "default: 10000". The legacy bash script does set `MAX_FILE_SIZE=10000`. + +### 9.11 `npm test` rewrites tracked files and takes minutes +`test/link-agent-skills.test.js:11-16` runs `scripts/link-agent-skills.sh` with `cwd: ROOT`; the script prunes and recreates `.cursor/skills/*` symlinks and rewrites `docs/cursor-skill-collisions.md`. `find` order differs by filesystem, so on macOS collision winners change. Reproduced in a scratch clone: 21 tracked files modified (20 symlinks + the collisions doc), 71 s per run (the test runs it three times). The earlier interrupted run hit exactly this. + +### 9.12 Agent-skill linker script and `$HOME` +Security-related; details kept privately by the owner. + +### 9.13 Selection/validation breaks on ordinary paths +`DiffService.isValidFilePath` (`diffService.js:131-144`) rejects `[ ] ( ) { } $ ' "` and any path containing the substrings `.env`, `.git/`, `node_modules/`. Reproduced: `/api/file-diff?file=app/[id]/page.tsx` β†’ 400 "potential security risk" (the UI shows nothing for Next.js route files); unchecking such a file makes both exports fail with 400 "Invalid excluded file entries". `mail.envelope.ts` would be rejected for containing `.env`. + +### 9.14 Sensitive files in exports +Security-related; details kept privately by the owner. + +### 9.15 Smaller verified defects +- Renames: `/api/staged-files` keys `fileStatuses` by the old name (`server.js:362` takes the 2nd tab field of `R100\told\tnew`), so the renamed file shows "Modified". +- `ReviewGenerator.js:103-104,246`: every export says `**Project:** AI Visual Code Review` and "v2.0" regardless of the repo being reviewed. +- `ReviewGenerator.js:261`: line comments are matched to files by substring of the sanitized name, so `a.js` picks up comments for `src/a.js`. +- `export-ai-review.js:191`: regex `/\/?\\*?$/` does not strip `/*`, so `--exclude "dist/*"` only excludes direct children. +- `public/index.html:6`: `` is "Visual Code Review - SDE2 PrepOps" (another project's name) until JS replaces it. +- `server.js:502-504`: cache middleware registered after the routes it should wrap β€” never runs (plan 008). +- `server.js:4-9`: five unused imports (plan 002). + +### 9.16 VS Code extension is a stub +`reviewWebviewProvider.ts:339-352`: the main panel posts `getStagedFiles` but has no `message` listener, so it shows "Loading..." forever. `extension.ts:27` registers a webview view id `aiCodeReviewWebview` that `package.json` never contributes; the activity-bar container has no views. No diff view, no comments; `exportForAI(comments, lineComments)` ignores its arguments. `simple-git` is a declared but unused dependency; settings `maxFileSize`, `excludePatterns`, `showLineNumbers` are never read; `npm test` points at a nonexistent `out/test/runTest.js`. + +## 10. Roadmap vs reality + +Spot-checks of "done" claims (CHANGELOG 2.0–2.3, `memory-bank/progress.md`, README): + +| # | Claim | Verdict | Evidence | +|---|---|---|---| +| 1 | HI-001 "Monolithic server.js split into modular structure" | False | `src/` is never required; `server.js` still 803 lines with its own copies | +| 2 | HI-010 "CSP nonce" | Not active | Live CSP is `'unsafe-inline'` (`server.js:145`); the HTML page gets no CSP | +| 3 | CR-003 "HTTPS configuration ready" | Not wired | Only in dead `src/config`; `server.js` has no `https`; `generate:certs` output is unused | +| 4 | HI-005 "CORS restricted to localhost" | True | `server.js:119-131` | +| 5 | HI-004 "Path length validation" | True | `server.js:165` | +| 6 | CR-002 "execSync vulnerability fixed" | True on `main`; see private notes | 9.4 | +| 7 | CR-001 "VSCode extension missing TypeScript files" | True (compiles) | but 9.16 | +| 8 | HI-003 "Duplicate AppState consolidated" | Partial | "Backward compatibility aliases" and separate `let current*` globals remain (`index.html:777-786`); `AppState.addDiff` unused | +| 9 | HI-015 "Duplicate saveComment consolidated" | True | one definition, line 1356 | +| 10 | "Request caching with TTL for GET endpoints" (2.0.0) | Dead | 9.15 | +| 11 | "Rate limiting 50/15 min, 10 exports/15 min" | Buggy | 9.9; skipped entirely for localhost in dev | +| 12 | "Security headers on all responses" | See private notes | 9.7 | +| 13 | "Line-by-line commenting with … persistence" | False | no storage of any kind | +| 14 | "Comments included in export" | False for unified, true for split | 9.3 | +| 15 | "`--max-size` default 10000" | False | 9.10 | +| 16 | "Blue Renamed badge" | False in web | 9.15 | +| 17 | "Docker support with health checks" | Non-functional | 9.6 | +| 18 | `npm run test:security` (README, SECURITY, SETUP) | Missing | not in `package.json` | +| 19 | `.ai-review.json`, `GIT_DIFF_CONTEXT`, `GIT_DIFF_ALGORITHM`, `RATE_LIMIT_*`, `CORS_ORIGINS`, `MAX_REQUEST_SIZE`, `GIT_TIMEOUT` | Not implemented | only `PORT` and `NODE_ENV` are read | +| 20 | "ADDED 20+ new npm scripts" | False | 16 scripts, 2 are echo placeholders | +| 21 | "Comprehensive tests for GitStatusParser" | True | `test/gitStatusParser.test.js` | +| 22 | README project tree (`public/review.html`) | Stale | file is `public/index.html` | + +Declared roadmap: CHANGELOG "v2.3.0 planned: HTML bundle export, VSCode extension, submodule/LFS support, performance dashboard" and "v3.0.0: real-time collaboration, WebSocket, AI analysis integration, enterprise auth"; memory-bank "v2.4.0: full CSP nonce injection, route splitting". None of it exists. Version 2.4.1 shipped with none of the 2.4.0 items. The only live, accurate plan of record is `plans/` (9 items, 0 done). + +Version strings disagree: `package.json` 2.4.1; `/api/health` 1.0.0; exports "v2.0"; README "v2.0"; Dockerfile label 2.3.0; ARCHITECTURE 2.3.0; extension 1.0.0. + +## 11. Risk areas for the bug hunt + +1. **Path handling end to end** β€” `GitService.execute` arg filter, `isValidFilePath`, the string-built handlers in `index.html`, quoting of non-ASCII names by `git diff --name-only` (no `-z`), renames, paths with spaces in `status --porcelain` parsing (`substring(3)`). +2. **`DiffService.parseDiff`** β€” `---`/`+++` content, "\ No newline", binary diffs, mode-only changes, renames with edits, line number 0 treated as falsy in `formatEnhancedDiff`. +3. **`public/index.html` accordion and comment modal** β€” nested headers, duplicate ids, `currentLineId` not reset when a file comment follows a cancelled line comment, comments keyed by sanitized ids that collide (`a/b.js` vs `a_b.js`). +4. **Export correctness** β€” comments dropped, excluded files leaking, markdown fence breakage when a diff contains ``` , hard-coded project name, concurrent `Promise.all` over hundreds of git processes. +5. **Network exposure** β€” details kept privately by the owner. +6. **CLI pattern matching** β€” `matchPattern` directory/extension/glob branches, `--include` plus default excludes interaction. +7. **Release/distribution** β€” published artifacts lag behind `main`; no changelog; no release automation. +8. **Agent-tooling scripts** β€” tests that mutate the tree, bash 4 requirement; other items details kept privately by the owner. +9. **VS Code extension** β€” single-repo assumption (`repositories[0]`), status bar computed once, stub webviews.