feat(devtools): Kitchen dev dashboard with guard & activity monitoring#171
Merged
feat(devtools): Kitchen dev dashboard with guard & activity monitoring#171
Conversation
openTerminal was gated behind outputFormat === "pretty", but agent environments (Claude Code, Gemini CLI) force "json" format via env vars, making the terminal never open. Force pretty format when openTerminal is enabled. Also adds DevTool Kitchen planning docs (Phase 1-4).
Add /__kitchen dev-only dashboard with three panels: - Activity Stream: SSE-based real-time MCP activity feed via .mandu/activity.jsonl - Route Explorer: manifest-based route listing with summary stats - Guard Dashboard: on-demand architecture violation scanning New modules: file-tailer, activity-sse, routes-api, guard-api, kitchen-handler, kitchen-ui Server integration: auto-init in dev mode, /__kitchen prefix intercept in handleRequestInternal()
Kitchen Guard panel now shows actual preset and scan results.
- Add expand button (🔲) to PanelContainer header → opens /__kitchen full page - Add scan button to GuardPanel → calls /__kitchen/api/guard/scan API - Improve kitchen-ui.ts: XHR fallback, debug bar, Routes as default tab - Guard panel shows scan results from Kitchen API alongside HMR events
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughAdds a dev-only Kitchen dashboard at /__kitchen: a single-file UI, SSE activity stream, file tailing, file/diff/routes/contracts/guard APIs, persistent guard decisions, devtools preview tab and HMR events, server/CLI wiring, and comprehensive tests. Changes
Sequence DiagramssequenceDiagram
participant Browser
participant Server
participant Kitchen
participant Broadcaster
participant Tailer
Browser->>Server: GET /__kitchen/sse/activity
Server->>Kitchen: handle(req, pathname)
Kitchen->>Broadcaster: createResponse()
Broadcaster->>Tailer: start()
Tailer->>Tailer: watch .mandu/activity.jsonl
Broadcaster-->>Browser: SSE Response (headers + stream)
Broadcaster-->>Browser: { clientId, timestamp } (welcome)
Note over Tailer,Broadcaster: Tailer emits lines -> Broadcaster buffers (500ms)
Broadcaster-->>Browser: data: { activity events }
Broadcaster-->>Browser: heartbeat (30s)
sequenceDiagram
participant User
participant GuardUI
participant Server
participant Kitchen
participant GuardAPI
participant GuardEngine
User->>GuardUI: click "scan"
GuardUI->>Server: POST /__kitchen/api/guard/scan
Server->>Kitchen: handle(req, pathname)
Kitchen->>GuardAPI: handleScan()
GuardAPI->>GuardAPI: set scanning = true
GuardAPI->>GuardEngine: checkDirectory(config, rootDir)
GuardEngine-->>GuardAPI: ViolationReport
GuardAPI->>GuardAPI: cache report
GuardAPI-->>Kitchen: report JSON
Kitchen-->>Server: response
Server-->>GuardUI: JSON response
GuardUI->>User: render violations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @konamgil, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 Pull Request는 Mandu 프레임워크에 Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR introduces the Kitchen dashboard, a significant feature aimed at enhancing developer experience, including real-time activity monitoring. While the overall code structure is well-organized, a high-severity command injection vulnerability was identified in the ActivityMonitor class. This vulnerability arises from unsafely interpolating project-derived paths into shell commands, which could lead to remote code execution if the tool is run in a maliciously named directory. Additionally, several improvements are suggested: refactor the synchronous I/O in file-tailer to an asynchronous approach to prevent event loop blocking, modernize kitchen-ui's vanilla JS code with fetch API and let/const for better readability and maintainability, fix a minor bug that might display incorrect UI data, and resolve inconsistencies between documentation and implementation.
| var summaryHtml = '<div class="guard-summary">' + | ||
| '<div class="guard-stat"><div class="guard-stat-value">' + r.totalViolations + '</div><div class="guard-stat-label">Total</div></div>' + | ||
| '<div class="guard-stat"><div class="guard-stat-value sev-error">' + (r.bySeverity.error || 0) + '</div><div class="guard-stat-label">Errors</div></div>' + | ||
| '<div class="guard-stat"><div class="guard-stat-value sev-warning">' + (r.bySeverity.warning || 0) + '</div><div class="guard-stat-label">Warnings</div></div>' + |
There was a problem hiding this comment.
r.bySeverity.warning를 사용하여 경고 수를 가져오고 있습니다. 하지만 ViolationReport 타입(packages/core/src/guard/types.ts)의 bySeverity 속성은 warn을 키로 사용합니다. 이로 인해 경고 수가 항상 0으로 표시될 수 있습니다. r.bySeverity.warn으로 수정해야 합니다.
Suggested change
| '<div class="guard-stat"><div class="guard-stat-value sev-warning">' + (r.bySeverity.warning || 0) + '</div><div class="guard-stat-label">Warnings</div></div>' + | |
| '<div class="guard-stat"><div class="guard-stat-value sev-warning">' + (r.bySeverity.warn || 0) + '</div><div class="guard-stat-label">Warnings</div></div>' + |
Comment on lines
+65
to
+91
| private readNewContent(newSize: number): void { | ||
| let fd: number | undefined; | ||
| try { | ||
| fd = fs.openSync(this.filePath, "r"); | ||
| const length = newSize - this.position; | ||
| const buf = Buffer.alloc(length); | ||
| fs.readSync(fd, buf, 0, length, this.position); | ||
| this.position = newSize; | ||
|
|
||
| this.buffer += buf.toString("utf-8"); | ||
| const lines = this.buffer.split("\n"); | ||
| this.buffer = lines.pop() ?? ""; | ||
|
|
||
| for (const line of lines) { | ||
| const trimmed = line.trim(); | ||
| if (trimmed) { | ||
| this.emit("line", trimmed); | ||
| } | ||
| } | ||
| } catch { | ||
| // File read error — will retry on next poll | ||
| } finally { | ||
| if (fd !== undefined) { | ||
| try { fs.closeSync(fd); } catch { /* ignore */ } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
readNewContent 함수 내에서 fs.openSync, fs.readSync, fs.closeSync와 같은 동기 I/O 함수를 사용하고 있습니다. 이는 이벤트 루프를 블로킹하여 서버 성능에 영향을 줄 수 있습니다. fs/promises API를 사용하여 비동기적으로 파일을 읽도록 수정하는 것이 좋습니다. start 메서드에서 이 함수를 호출할 때 readNewContent(...).catch(console.error)와 같이 처리해야 합니다.
private async readNewContent(newSize: number): Promise<void> {
let fd: fs.promises.FileHandle | undefined;
try {
fd = await fs.promises.open(this.filePath, "r");
const length = newSize - this.position;
const buf = Buffer.alloc(length);
await fd.read(buf, 0, length, this.position);
this.position = newSize;
this.buffer += buf.toString("utf-8");
const lines = this.buffer.split("\n");
this.buffer = lines.pop() ?? "";
for (const line of lines) {
const trimmed = line.trim();
if (trimmed) {
this.emit("line", trimmed);
}
}
} catch {
// File read error — will retry on next poll
} finally {
await fd?.close();
}
}
Comment on lines
+437
to
+438
| GET /__kitchen/api/guard/check (POST도 가능) | ||
| → Response: KitchenGuardCheckResponse (on-demand 실행) |
There was a problem hiding this comment.
기획서에 명시된 Guard API 엔드포인트(GET /__kitchen/api/guard/check)가 실제 구현과 다릅니다. packages/core/src/kitchen/kitchen-handler.ts와 packages/core/src/devtools/client/components/panel/guard-panel.tsx에서는 POST /__kitchen/api/guard/scan을 사용하고 있습니다. 문서와 코드의 일관성을 위해 이 부분을 업데이트하는 것이 좋겠습니다.
Comment on lines
+506
to
+528
| var xhr = new XMLHttpRequest(); | ||
| xhr.open('GET', '/__kitchen/api/routes', true); | ||
| xhr.onload = function() { | ||
| if (xhr.status === 200) { | ||
| try { | ||
| var data = JSON.parse(xhr.responseText); | ||
| log('Routes loaded: ' + data.summary.total + ' routes', 'ok'); | ||
| renderRoutes(data); | ||
| } catch(e) { | ||
| log('Routes parse error: ' + e.message, 'err'); | ||
| } | ||
| } else { | ||
| log('Routes HTTP ' + xhr.status, 'err'); | ||
| document.getElementById('routes-list').innerHTML = | ||
| '<div class="empty-state">Failed to load routes (HTTP ' + xhr.status + ')</div>'; | ||
| } | ||
| }; | ||
| xhr.onerror = function() { | ||
| log('Routes network error', 'err'); | ||
| document.getElementById('routes-list').innerHTML = | ||
| '<div class="empty-state">Network error loading routes.</div>'; | ||
| }; | ||
| xhr.send(); |
There was a problem hiding this comment.
인라인 스크립트에서 XMLHttpRequest를 사용하고 있는데, fetch API로 교체하는 것을 고려해 보세요. fetch는 Promise 기반으로 동작하여 코드를 더 간결하고 현대적으로 만들 수 있습니다. 예를 들어 loadRoutes 함수를 다음과 같이 개선할 수 있습니다.
async function loadRoutes() {
log('Fetching routes...');
try {
const response = await fetch('/__kitchen/api/routes');
if (!response.ok) {
throw new Error(`HTTP ${response.status}`);
}
const data = await response.json();
log('Routes loaded: ' + data.summary.total + ' routes', 'ok');
renderRoutes(data);
} catch (e) {
log('Routes error: ' + e.message, 'err');
document.getElementById('routes-list').innerHTML =
'<div class="empty-state">Failed to load routes (' + e.message + ')</div>';
}
}
Comment on lines
+379
to
+670
| (function() { | ||
| var dbg = document.getElementById('debug-bar'); | ||
| function log(msg, cls) { | ||
| if (!dbg) return; | ||
| var s = document.createElement('span'); | ||
| s.className = cls || ''; | ||
| s.textContent = '[' + new Date().toLocaleTimeString() + '] ' + msg + ' '; | ||
| dbg.appendChild(s); | ||
| dbg.scrollTop = dbg.scrollHeight; | ||
| console.log('[Kitchen]', msg); | ||
| } | ||
|
|
||
| function escapeHtml(str) { | ||
| if (!str) return ''; | ||
| var d = document.createElement('div'); | ||
| d.appendChild(document.createTextNode(String(str))); | ||
| return d.innerHTML; | ||
| } | ||
|
|
||
| try { log('JS loaded', 'ok'); } catch(e) {} | ||
|
|
||
| // Tab switching | ||
| var tabs = document.querySelectorAll('.tab'); | ||
| for (var i = 0; i < tabs.length; i++) { | ||
| tabs[i].addEventListener('click', function() { | ||
| var all = document.querySelectorAll('.tab'); | ||
| var panels = document.querySelectorAll('.panel'); | ||
| for (var j = 0; j < all.length; j++) all[j].classList.remove('active'); | ||
| for (var j = 0; j < panels.length; j++) panels[j].classList.remove('active'); | ||
| this.classList.add('active'); | ||
| var p = document.getElementById('panel-' + this.getAttribute('data-panel')); | ||
| if (p) p.classList.add('active'); | ||
| }); | ||
| } | ||
|
|
||
| // ─── SSE Activity Stream ───────────────────── | ||
| var statusDot = document.getElementById('sse-status'); | ||
| var statusLabel = document.getElementById('sse-label'); | ||
| var activityList = document.getElementById('activity-list'); | ||
| var activityCount = 0; | ||
| var MAX_ITEMS = 200; | ||
| var sseRetryCount = 0; | ||
|
|
||
| function connectSSE() { | ||
| statusDot.className = 'status-dot connecting'; | ||
| statusLabel.textContent = 'Connecting...'; | ||
| log('SSE connecting...'); | ||
|
|
||
| var es; | ||
| try { | ||
| es = new EventSource('/__kitchen/sse/activity'); | ||
| } catch(e) { | ||
| log('SSE EventSource failed: ' + e.message, 'err'); | ||
| statusDot.className = 'status-dot disconnected'; | ||
| statusLabel.textContent = 'Failed'; | ||
| return; | ||
| } | ||
|
|
||
| es.onopen = function() { | ||
| statusDot.className = 'status-dot connected'; | ||
| statusLabel.textContent = 'Connected'; | ||
| sseRetryCount = 0; | ||
| log('SSE connected', 'ok'); | ||
| }; | ||
|
|
||
| es.onmessage = function(e) { | ||
| try { | ||
| var data = JSON.parse(e.data); | ||
| if (data.type === 'connected') { | ||
| log('SSE welcome: ' + data.clientId, 'ok'); | ||
| return; | ||
| } | ||
| if (data.type === 'heartbeat') return; | ||
| appendActivity(data); | ||
| } catch(err) { | ||
| log('SSE parse error: ' + err.message, 'err'); | ||
| } | ||
| }; | ||
|
|
||
| es.onerror = function(evt) { | ||
| log('SSE error (readyState=' + es.readyState + ')', 'err'); | ||
| statusDot.className = 'status-dot disconnected'; | ||
| statusLabel.textContent = 'Disconnected'; | ||
| es.close(); | ||
| sseRetryCount++; | ||
| var delay = Math.min(3000 * sseRetryCount, 15000); | ||
| log('SSE retry in ' + (delay/1000) + 's'); | ||
| setTimeout(connectSSE, delay); | ||
| }; | ||
| } | ||
|
|
||
| function appendActivity(data) { | ||
| if (activityCount === 0) { | ||
| activityList.innerHTML = ''; | ||
| } | ||
| activityCount++; | ||
|
|
||
| var item = document.createElement('div'); | ||
| item.className = 'activity-item'; | ||
|
|
||
| var ts = data.ts || data.timestamp || new Date().toISOString(); | ||
| var time = new Date(ts).toLocaleTimeString(); | ||
| var tool = data.tool || data.type || 'event'; | ||
| var detail = data.description || data.message || data.resource || JSON.stringify(data).substring(0, 120); | ||
|
|
||
| item.innerHTML = | ||
| '<span class="activity-time">' + escapeHtml(time) + '</span>' + | ||
| '<span class="activity-tool">' + escapeHtml(tool) + '</span>' + | ||
| '<span class="activity-detail">' + escapeHtml(detail) + '</span>'; | ||
|
|
||
| activityList.insertBefore(item, activityList.firstChild); | ||
|
|
||
| while (activityList.children.length > MAX_ITEMS) { | ||
| activityList.removeChild(activityList.lastChild); | ||
| } | ||
| } | ||
|
|
||
| document.getElementById('clear-activity').addEventListener('click', function() { | ||
| activityList.innerHTML = '<div class="empty-state">Waiting for MCP activity...</div>'; | ||
| activityCount = 0; | ||
| }); | ||
|
|
||
| connectSSE(); | ||
|
|
||
| // ─── Routes ────────────────────────────────── | ||
| function loadRoutes() { | ||
| log('Fetching routes...'); | ||
| var xhr = new XMLHttpRequest(); | ||
| xhr.open('GET', '/__kitchen/api/routes', true); | ||
| xhr.onload = function() { | ||
| if (xhr.status === 200) { | ||
| try { | ||
| var data = JSON.parse(xhr.responseText); | ||
| log('Routes loaded: ' + data.summary.total + ' routes', 'ok'); | ||
| renderRoutes(data); | ||
| } catch(e) { | ||
| log('Routes parse error: ' + e.message, 'err'); | ||
| } | ||
| } else { | ||
| log('Routes HTTP ' + xhr.status, 'err'); | ||
| document.getElementById('routes-list').innerHTML = | ||
| '<div class="empty-state">Failed to load routes (HTTP ' + xhr.status + ')</div>'; | ||
| } | ||
| }; | ||
| xhr.onerror = function() { | ||
| log('Routes network error', 'err'); | ||
| document.getElementById('routes-list').innerHTML = | ||
| '<div class="empty-state">Network error loading routes.</div>'; | ||
| }; | ||
| xhr.send(); | ||
| } | ||
|
|
||
| function renderRoutes(data) { | ||
| var summaryEl = document.getElementById('routes-summary'); | ||
| var listEl = document.getElementById('routes-list'); | ||
| var s = data.summary; | ||
|
|
||
| summaryEl.innerHTML = | ||
| '<span class="summary-item"><span class="summary-count">' + s.total + '</span> total</span>' + | ||
| '<span class="summary-item"><span class="summary-count">' + s.pages + '</span> pages</span>' + | ||
| '<span class="summary-item"><span class="summary-count">' + s.apis + '</span> APIs</span>' + | ||
| '<span class="summary-item"><span class="summary-count">' + s.withIslands + '</span> islands</span>'; | ||
|
|
||
| if (!data.routes.length) { | ||
| listEl.innerHTML = '<div class="empty-state">No routes found.</div>'; | ||
| return; | ||
| } | ||
|
|
||
| var html = ''; | ||
| for (var i = 0; i < data.routes.length; i++) { | ||
| var r = data.routes[i]; | ||
| var badges = ''; | ||
| if (r.hasSlot) badges += '<span class="badge">slot</span>'; | ||
| if (r.hasContract) badges += '<span class="badge">contract</span>'; | ||
| if (r.hasClient) badges += '<span class="badge">island</span>'; | ||
| if (r.hasLayout) badges += '<span class="badge">layout</span>'; | ||
| if (r.hydration && r.hydration !== 'none') badges += '<span class="badge">' + escapeHtml(r.hydration) + '</span>'; | ||
|
|
||
| html += '<div class="route-item">' + | ||
| '<span class="route-kind ' + r.kind + '">' + r.kind + '</span>' + | ||
| '<span class="route-pattern">' + escapeHtml(r.pattern) + '</span>' + | ||
| '<span class="route-badges">' + badges + '</span>' + | ||
| '</div>'; | ||
| } | ||
| listEl.innerHTML = html; | ||
| } | ||
|
|
||
| loadRoutes(); | ||
|
|
||
| // ─── Guard ─────────────────────────────────── | ||
| var scanBtn = document.getElementById('scan-guard'); | ||
| var guardStatusEl = document.getElementById('guard-status'); | ||
| var guardListEl = document.getElementById('guard-list'); | ||
|
|
||
| function loadGuardStatus() { | ||
| log('Fetching guard status...'); | ||
| var xhr = new XMLHttpRequest(); | ||
| xhr.open('GET', '/__kitchen/api/guard', true); | ||
| xhr.onload = function() { | ||
| if (xhr.status === 200) { | ||
| try { | ||
| var data = JSON.parse(xhr.responseText); | ||
| log('Guard: ' + (data.enabled ? 'enabled (' + data.preset + ')' : 'disabled'), 'ok'); | ||
| renderGuardData(data); | ||
| } catch(e) { | ||
| log('Guard parse error: ' + e.message, 'err'); | ||
| } | ||
| } | ||
| }; | ||
| xhr.onerror = function() { log('Guard network error', 'err'); }; | ||
| xhr.send(); | ||
| } | ||
|
|
||
| scanBtn.addEventListener('click', function() { | ||
| scanBtn.disabled = true; | ||
| scanBtn.textContent = 'Scanning...'; | ||
| log('Guard scan started...'); | ||
|
|
||
| var xhr = new XMLHttpRequest(); | ||
| xhr.open('POST', '/__kitchen/api/guard/scan', true); | ||
| xhr.onload = function() { | ||
| scanBtn.disabled = false; | ||
| scanBtn.textContent = 'Scan'; | ||
| if (xhr.status === 200) { | ||
| try { | ||
| var data = JSON.parse(xhr.responseText); | ||
| log('Guard scan done: ' + (data.report ? data.report.totalViolations + ' violations' : 'no report'), 'ok'); | ||
| renderGuardData(data); | ||
| } catch(e) { | ||
| log('Guard scan parse error: ' + e.message, 'err'); | ||
| } | ||
| } else { | ||
| log('Guard scan HTTP ' + xhr.status, 'err'); | ||
| guardStatusEl.textContent = 'Scan failed.'; | ||
| } | ||
| }; | ||
| xhr.onerror = function() { | ||
| scanBtn.disabled = false; | ||
| scanBtn.textContent = 'Scan'; | ||
| log('Guard scan network error', 'err'); | ||
| guardStatusEl.textContent = 'Scan failed.'; | ||
| }; | ||
| xhr.send(); | ||
| }); | ||
|
|
||
| function renderGuardData(data) { | ||
| if (!data.enabled) { | ||
| guardStatusEl.textContent = 'Guard is not configured for this project.'; | ||
| guardListEl.innerHTML = ''; | ||
| return; | ||
| } | ||
|
|
||
| guardStatusEl.innerHTML = 'Preset: <strong>' + escapeHtml(data.preset) + '</strong>'; | ||
|
|
||
| if (!data.report) { | ||
| guardListEl.innerHTML = '<div class="empty-state">No scan results yet. Click "Scan" to check.</div>'; | ||
| return; | ||
| } | ||
|
|
||
| var r = data.report; | ||
| var summaryHtml = '<div class="guard-summary">' + | ||
| '<div class="guard-stat"><div class="guard-stat-value">' + r.totalViolations + '</div><div class="guard-stat-label">Total</div></div>' + | ||
| '<div class="guard-stat"><div class="guard-stat-value sev-error">' + (r.bySeverity.error || 0) + '</div><div class="guard-stat-label">Errors</div></div>' + | ||
| '<div class="guard-stat"><div class="guard-stat-value sev-warning">' + (r.bySeverity.warning || 0) + '</div><div class="guard-stat-label">Warnings</div></div>' + | ||
| '<div class="guard-stat"><div class="guard-stat-value sev-info">' + (r.bySeverity.info || 0) + '</div><div class="guard-stat-label">Info</div></div>' + | ||
| '</div>'; | ||
|
|
||
| if (!r.violations.length) { | ||
| guardListEl.innerHTML = summaryHtml + '<div class="empty-state">No violations found!</div>'; | ||
| return; | ||
| } | ||
|
|
||
| var violHtml = ''; | ||
| var list = r.violations.length > 100 ? r.violations.slice(0, 100) : r.violations; | ||
| for (var i = 0; i < list.length; i++) { | ||
| var v = list[i]; | ||
| violHtml += '<div class="violation-item">' + | ||
| '<div class="violation-file">' + | ||
| '<span class="violation-sev ' + v.severity + '">' + v.severity + '</span>' + | ||
| escapeHtml(v.filePath) + ':' + v.line + | ||
| '</div>' + | ||
| '<div class="violation-msg">' + | ||
| escapeHtml(v.fromLayer) + ' → ' + escapeHtml(v.toLayer) + ': ' + escapeHtml(v.ruleDescription) + | ||
| '</div>' + | ||
| '</div>'; | ||
| } | ||
| guardListEl.innerHTML = summaryHtml + violHtml; | ||
| } | ||
|
|
||
| loadGuardStatus(); | ||
|
|
||
| })(); |
There was a problem hiding this comment.
스크립트 전체에서 var 키워드가 사용되고 있습니다. var는 함수 스코프를 가지므로 예상치 못한 동작을 유발할 수 있습니다. 블록 스코프를 가지는 let과 const를 사용하여 코드를 더 명확하고 안전하게 만드는 것이 좋습니다.
Comment on lines
+47
to
+62
| fs.watchFile( | ||
| this.filePath, | ||
| { interval: this.options.pollIntervalMs }, | ||
| (curr) => { | ||
| if (curr.size > this.position) { | ||
| this.readNewContent(curr.size); | ||
| } else if (curr.size < this.position) { | ||
| // File was truncated/recreated (MCP server restart) | ||
| this.position = 0; | ||
| this.buffer = ""; | ||
| if (curr.size > 0) { | ||
| this.readNewContent(curr.size); | ||
| } | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
fs.watchFile은 파일 시스템을 폴링 방식으로 감시하므로 비효율적일 수 있습니다. 특히 Windows에서 성능 문제가 발생할 수 있습니다. 프로젝트가 Bun 환경에서 실행되므로, 더 효율적인 Bun 내장 API인 Bun.file(path).watch() 사용을 고려해 보세요. 이는 OS 네이티브 파일 감시 메커니즘(예: inotify, kqueue)을 사용하여 더 나은 성능을 제공할 수 있습니다.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/commands/dev.ts (1)
309-315:⚠️ Potential issue | 🟠 MajorKitchen route data can go stale after live route updates
Line 311 and Line 373 replace
manifest, but Kitchen keeps the initial snapshot unlessupdateManifest()is called. This can make/__kitchen/api/routesshow outdated routes during dev.💡 Suggested fix
@@ const resolved = await resolveManifest(rootDir, { fsRoutes: config.fsRoutes }); manifest = resolved.manifest; + server.registry.kitchen?.updateManifest(manifest); @@ // Update server with new manifest manifest = result.manifest; + server.registry.kitchen?.updateManifest(manifest); console.log(` 📋 Routes: ${manifest.routes.length}`);Also applies to: 364-377
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/dev.ts` around lines 309 - 315, The manifest variable is reassigned after resolveManifest(...) and registerHandlers(...) (in the block around resolveManifest and the later 364-377 block) but Kitchen keeps its initial snapshot; call the Kitchen update helper to refresh the in-memory route snapshot after updating manifest. Specifically, after assigning manifest = resolved.manifest (both places) invoke the existing updateManifest(manifest) (or the Kitchen API method that updates routes) so that /__kitchen/api/routes reflects the new manifest; ensure the call occurs immediately after resolveManifest(...) and before registerHandlers(...) completes so Kitchen sees the latest routes.
🧹 Nitpick comments (8)
packages/core/src/kitchen/kitchen-ui.ts (1)
651-658: Normalize severity before injecting into HTML/class names.Line 657 directly interpolates
v.severity. Whitelist allowed values to avoid malformed DOM/class output from unexpected payloads.🧩 Proposed hardening
+ function normalizeSeverity(sev) { + return sev === 'error' || sev === 'warning' || sev === 'info' ? sev : 'info'; + } @@ - violHtml += '<div class="violation-item">' + + var sev = normalizeSeverity(v.severity); + violHtml += '<div class="violation-item">' + '<div class="violation-file">' + - '<span class="violation-sev ' + v.severity + '">' + v.severity + '</span>' + + '<span class="violation-sev ' + sev + '">' + sev + '</span>' + escapeHtml(v.filePath) + ':' + v.line + '</div>' +🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/kitchen-ui.ts` around lines 651 - 658, v.severity is interpolated directly into the HTML/class names (violHtml and the "violation-sev" span), which can allow unexpected values; sanitize and normalize severity by mapping/whitelisting allowed values (e.g., "low","medium","high","critical") in the code that builds violHtml (near the loop using list[i] and escapeHtml), fall back to a safe token like "unknown" for anything else, and use that normalized token both for the CSS class and the displayed text (ensure you also strip or replace non-word characters so the class name is always valid).packages/core/src/kitchen/api/guard-api.ts (1)
21-23: Reset cached report when guard config meaningfully changes.
updateConfigcurrently swaps config but keeps the previous cached report, which can show stale results under a new preset/config.♻️ Proposed fix
updateConfig(config: GuardConfig | null): void { + const prevPreset = this.config?.preset; this.config = config; + if (!config || config.preset !== prevPreset) { + this.cachedReport = null; + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/api/guard-api.ts` around lines 21 - 23, The updateConfig method currently replaces this.config but leaves any previously computed report cached, causing stale results when GuardConfig changes; modify updateConfig(config: GuardConfig | null) in guard-api.ts to compare the incoming config with the existing this.config (or treat any non-equal change as meaningful) and clear the cached report state (e.g., reset this.cachedReport / this.reportCache / lastReport or whatever report cache field the class uses) whenever the config meaningfully changes, then assign this.config = config.packages/core/tests/kitchen/file-tailer.test.ts (1)
34-42: Fixed sleeps make polling tests flakyThe hardcoded waits (
100/200ms) can intermittently fail under slower CI timing. Prefer condition-based waiting with timeout.💡 Suggested test utility pattern
+async function waitFor(predicate: () => boolean, timeoutMs = 2000, intervalMs = 25): Promise<void> { + const start = Date.now(); + while (!predicate()) { + if (Date.now() - start > timeoutMs) { + throw new Error("Timed out waiting for condition"); + } + await Bun.sleep(intervalMs); + } +} @@ - await Bun.sleep(200); + await waitFor(() => lines.length === 2);Also applies to: 62-73, 90-96, 115-120
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/kitchen/file-tailer.test.ts` around lines 34 - 42, Replace the fixed Bun.sleep calls in the test (around fs.appendFileSync and later appends) with a condition-based wait that polls until the file at logPath contains the expected new lines or until a timeout elapses; implement a small helper like waitFor or waitForLogLines used by the tests to repeatedly read fs.readFileSync(logPath) and assert presence/count of appended entries, and use that helper in place of Bun.sleep for the blocks around the first append (lines ~34-42) and the other mentioned ranges (62-73, 90-96, 115-120), ensuring the helper throws on timeout so tests fail deterministically rather than flakily.packages/core/src/runtime/server.ts (1)
1346-1349: Optionally clear Kitchen reference after shutdownSetting
registry.kitchen = nullafter stopping avoids stale object retention when the same registry is reused.💡 Suggested tweak
stop: () => { registry.kitchen?.stop(); + registry.kitchen = null; server.stop(); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/runtime/server.ts` around lines 1346 - 1349, The stop() implementation currently calls registry.kitchen?.stop() and server.stop() but leaves registry.kitchen pointing to the stopped Kitchen; update the stop handler (the stop function where registry.kitchen is referenced) to clear the reference after shutdown by setting registry.kitchen = null (after calling registry.kitchen?.stop()) so the registry does not retain a stale Kitchen instance when reused.packages/core/tests/kitchen/kitchen-handler.test.ts (1)
99-108: Add direct coverage forPOST /__kitchen/api/guard/scanThis suite validates guard GET, but not the scan endpoint used by the panel. Adding one scan-route test would lock in the core interaction path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/kitchen/kitchen-handler.test.ts` around lines 99 - 108, Add a test that sends a POST Request to "/__kitchen/api/guard/scan" using handler.handle (mirroring the existing GET test): construct a Request with method "POST", Content-Type "application/json" and an empty JSON body (or minimal payload), call await handler.handle(req, "/__kitchen/api/guard/scan"), assert the response is not null and response.status === 200 (or response.ok), then await result.json() and assert the JSON includes the guard state and scan output (e.g., data.enabled is false to match the GET guardConfig null case and that data.results or similar scan output field is defined). Ensure you reference handler.handle and the route "/__kitchen/api/guard/scan" when adding the test.packages/core/src/kitchen/kitchen-handler.ts (2)
71-78: Minor: Redundant empty string check.Line 71 already handles the empty string case with
|| "/", sosub === ""in line 74 can never be true.🧹 Simplify the condition
const sub = pathname.slice(KITCHEN_PREFIX.length) || "/"; // Kitchen dashboard UI - if (sub === "/" || sub === "") { + if (sub === "/") { return new Response(renderKitchenHTML(), {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/kitchen-handler.ts` around lines 71 - 78, The condition checking both sub === "/" and sub === "" is redundant because sub is already normalized with const sub = pathname.slice(KITCHEN_PREFIX.length) || "/";; update the if in kitchen-handler.ts to only check sub === "/" (remove sub === "") — locate the block that returns the Response(renderKitchenHTML(), ...) and simplify the conditional to a single comparison against "/" to eliminate the unreachable branch.
85-88: Consider adding HTTP method check for consistency.The routes API endpoint accepts any HTTP method, while the guard endpoints properly restrict to GET/POST. For consistency and to prevent unexpected behavior from POST/DELETE requests, consider restricting to GET.
🛡️ Restrict to GET method
// Routes API - if (sub === "/api/routes") { + if (sub === "/api/routes" && req.method === "GET") { return handleRoutesRequest(this.manifest); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/kitchen-handler.ts` around lines 85 - 88, The /api/routes branch currently accepts any HTTP method; update the conditional so it only calls handleRoutesRequest(this.manifest) when the incoming request method is GET (e.g., check req.method === 'GET' or this.request.method === 'GET' as appropriate in the surrounding context) and return a 405 Method Not Allowed for other methods; this makes behavior consistent with the other guarded endpoints that restrict methods.packages/core/src/kitchen/stream/file-tailer.ts (1)
65-91: Consider validatingreadSyncreturn value.
fs.readSyncreturns the number of bytes actually read, which can be less than requested in edge cases (e.g., file truncated between size check and read). The current code assumes alllengthbytes are read and updatesthis.position = newSizeunconditionally.For a dev tool this is low-risk, but for correctness, you could use the actual bytes read:
🔧 Use actual bytes read
try { fd = fs.openSync(this.filePath, "r"); const length = newSize - this.position; const buf = Buffer.alloc(length); - fs.readSync(fd, buf, 0, length, this.position); - this.position = newSize; - - this.buffer += buf.toString("utf-8"); + const bytesRead = fs.readSync(fd, buf, 0, length, this.position); + this.position += bytesRead; + + this.buffer += buf.toString("utf-8", 0, bytesRead); const lines = this.buffer.split("\n");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/stream/file-tailer.ts` around lines 65 - 91, The readNewContent method assumes fs.readSync always reads the requested length and sets this.position = newSize unconditionally; instead, read the actual bytes returned by fs.readSync (the bytesRead value), append only buf.slice(0, bytesRead).toString(...), and update this.position by bytesRead (or stop if bytesRead === 0). If bytesRead < length, either loop to continue reading until total bytes read equals length or treat it as a transient truncate/EOF and avoid advancing this.position to newSize; adjust this.buffer/lines processing to use the actual bytes read. Reference: readNewContent, fs.readSync, this.position, this.buffer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/plans/13_devtool_kitchen_dev_spec.md`:
- Around line 161-167: Update the spec's API path strings to match the actual
routes implemented by the Kitchen server: replace occurrences of "GET
/__kitchen/api/stream" and any "api/stream" or "api/guard/check" references with
the real endpoint shapes used in the codebase (including the guard scan route
variant), and ensure any SSE Content-Type and example event payloads remain
accurate; search for the string literals "api/stream", "api/guard/check", and
"guard scan" in the doc (also at the other noted occurrences) and modify them to
the shipped route names so the spec aligns with the implemented endpoints.
In `@docs/plans/13_devtool_kitchen_plan.md`:
- Around line 112-118: Update the SSE example to use the actual activity SSE
route used by the Kitchen UI/handler flow instead of the outdated GET
/__kitchen/api/stream; locate the route/constant or handler that registers the
Kitchen activity SSE (the Kitchen activity SSE route or handler in the Kitchen
UI/handler flow) and replace the example path with that exact path so the docs
match the current implementation.
- Around line 239-243: Update the plan text so the server path references point
to the runtime server module that actually registers the `/__kitchen/*` route:
replace the stale "server.ts" mentions with the runtime server module name where
the `/__kitchen/*` route is implemented (also update the other occurrence
referenced in the comment); ensure the lines describing route registration, SSE
broadcast (activity-monitor), Kitchen service directory, and Kitchen UI islands
consistently reference the runtime server module that performs the integration.
In `@packages/core/src/devtools/client/components/panel/guard-panel.tsx`:
- Around line 149-163: The current fetch to '/__kitchen/api/guard/scan' swallows
non-ok responses and exceptions so users see no feedback; update the handler in
guard-panel.tsx (the async scan caller that uses fetch, setScanResult and
setScanning) to surface a compact error message: if res.ok is false, try to
parse await res.json() for a message field or fall back to `${res.status}
${res.statusText}` and call setScanResult with an error property (or a small
error-state via a new setScanError state) instead of silently returning;
similarly, in the catch block set the same compact error message (e.g., error:
err.message) so the UI can display it, and keep setScanning(false) in finally;
update any types for setScanResult/state to include an optional error string.
In `@packages/core/src/devtools/client/components/panel/panel-container.tsx`:
- Line 258: The onClick handler in panel-container.tsx currently calls
window.open('/__kitchen', '_blank') without opener isolation; update the onClick
arrow function (the inline handler that opens '/__kitchen') to harden the new
tab by passing opener isolation flags — e.g., call window.open('/__kitchen',
'_blank', 'noopener,noreferrer') or replace with an anchor element using
target="_blank" and rel="noopener noreferrer" — so the opened page cannot access
window.opener.
In `@packages/core/src/kitchen/api/guard-api.ts`:
- Around line 52-73: handleScan reads this.config, awaits checkDirectory, then
reads this.config.preset which can change during the await; snapshot the config
to a local variable before the await (e.g. const config = this.config) and use
that local for the call to checkDirectory and when constructing the Response
(use config.preset instead of this.config.preset); keep the existing null/config
checks and ensure you still assign this.cachedReport = await
checkDirectory(config, this.rootDir) so the response is consistent even if
updateConfig mutates this.config concurrently.
In `@packages/core/src/kitchen/stream/activity-sse.ts`:
- Around line 123-136: The stop() method doesn't clear buffered events so
pendingEvents can be replayed after restart; update stop() (in activity-sse.ts)
to clear the pendingEvents buffer (and reset any related counters like
pendingBytes) when shutting down—after stopping this.tailer and clearing timers
set this.pendingEvents = [] and this.pendingBytes = 0 (or equivalent) to prevent
stale lines from being emitted on restart.
In `@packages/mcp/src/activity-monitor.ts`:
- Around line 284-287: The code currently forces this.outputFormat = "pretty"
when this.config.openTerminal is true, coupling persisted storage format to
terminal rendering; change it so this.outputFormat is always set from
resolveOutputFormat(this.config.output) and introduce a separate
terminal-specific setting (e.g., terminalOutputFormat or a boolean like
usePrettyForTerminal) that the terminal-rendering path uses to force "pretty"
without altering this.outputFormat; update any terminal-rendering logic that
referenced this.outputFormat (see the block around this.outputFormat and the
similar block at lines referenced 333-335) to consult the new terminal-specific
symbol instead.
---
Outside diff comments:
In `@packages/cli/src/commands/dev.ts`:
- Around line 309-315: The manifest variable is reassigned after
resolveManifest(...) and registerHandlers(...) (in the block around
resolveManifest and the later 364-377 block) but Kitchen keeps its initial
snapshot; call the Kitchen update helper to refresh the in-memory route snapshot
after updating manifest. Specifically, after assigning manifest =
resolved.manifest (both places) invoke the existing updateManifest(manifest) (or
the Kitchen API method that updates routes) so that /__kitchen/api/routes
reflects the new manifest; ensure the call occurs immediately after
resolveManifest(...) and before registerHandlers(...) completes so Kitchen sees
the latest routes.
---
Nitpick comments:
In `@packages/core/src/kitchen/api/guard-api.ts`:
- Around line 21-23: The updateConfig method currently replaces this.config but
leaves any previously computed report cached, causing stale results when
GuardConfig changes; modify updateConfig(config: GuardConfig | null) in
guard-api.ts to compare the incoming config with the existing this.config (or
treat any non-equal change as meaningful) and clear the cached report state
(e.g., reset this.cachedReport / this.reportCache / lastReport or whatever
report cache field the class uses) whenever the config meaningfully changes,
then assign this.config = config.
In `@packages/core/src/kitchen/kitchen-handler.ts`:
- Around line 71-78: The condition checking both sub === "/" and sub === "" is
redundant because sub is already normalized with const sub =
pathname.slice(KITCHEN_PREFIX.length) || "/";; update the if in
kitchen-handler.ts to only check sub === "/" (remove sub === "") — locate the
block that returns the Response(renderKitchenHTML(), ...) and simplify the
conditional to a single comparison against "/" to eliminate the unreachable
branch.
- Around line 85-88: The /api/routes branch currently accepts any HTTP method;
update the conditional so it only calls handleRoutesRequest(this.manifest) when
the incoming request method is GET (e.g., check req.method === 'GET' or
this.request.method === 'GET' as appropriate in the surrounding context) and
return a 405 Method Not Allowed for other methods; this makes behavior
consistent with the other guarded endpoints that restrict methods.
In `@packages/core/src/kitchen/kitchen-ui.ts`:
- Around line 651-658: v.severity is interpolated directly into the HTML/class
names (violHtml and the "violation-sev" span), which can allow unexpected
values; sanitize and normalize severity by mapping/whitelisting allowed values
(e.g., "low","medium","high","critical") in the code that builds violHtml (near
the loop using list[i] and escapeHtml), fall back to a safe token like "unknown"
for anything else, and use that normalized token both for the CSS class and the
displayed text (ensure you also strip or replace non-word characters so the
class name is always valid).
In `@packages/core/src/kitchen/stream/file-tailer.ts`:
- Around line 65-91: The readNewContent method assumes fs.readSync always reads
the requested length and sets this.position = newSize unconditionally; instead,
read the actual bytes returned by fs.readSync (the bytesRead value), append only
buf.slice(0, bytesRead).toString(...), and update this.position by bytesRead (or
stop if bytesRead === 0). If bytesRead < length, either loop to continue reading
until total bytes read equals length or treat it as a transient truncate/EOF and
avoid advancing this.position to newSize; adjust this.buffer/lines processing to
use the actual bytes read. Reference: readNewContent, fs.readSync,
this.position, this.buffer.
In `@packages/core/src/runtime/server.ts`:
- Around line 1346-1349: The stop() implementation currently calls
registry.kitchen?.stop() and server.stop() but leaves registry.kitchen pointing
to the stopped Kitchen; update the stop handler (the stop function where
registry.kitchen is referenced) to clear the reference after shutdown by setting
registry.kitchen = null (after calling registry.kitchen?.stop()) so the registry
does not retain a stale Kitchen instance when reused.
In `@packages/core/tests/kitchen/file-tailer.test.ts`:
- Around line 34-42: Replace the fixed Bun.sleep calls in the test (around
fs.appendFileSync and later appends) with a condition-based wait that polls
until the file at logPath contains the expected new lines or until a timeout
elapses; implement a small helper like waitFor or waitForLogLines used by the
tests to repeatedly read fs.readFileSync(logPath) and assert presence/count of
appended entries, and use that helper in place of Bun.sleep for the blocks
around the first append (lines ~34-42) and the other mentioned ranges (62-73,
90-96, 115-120), ensuring the helper throws on timeout so tests fail
deterministically rather than flakily.
In `@packages/core/tests/kitchen/kitchen-handler.test.ts`:
- Around line 99-108: Add a test that sends a POST Request to
"/__kitchen/api/guard/scan" using handler.handle (mirroring the existing GET
test): construct a Request with method "POST", Content-Type "application/json"
and an empty JSON body (or minimal payload), call await handler.handle(req,
"/__kitchen/api/guard/scan"), assert the response is not null and
response.status === 200 (or response.ok), then await result.json() and assert
the JSON includes the guard state and scan output (e.g., data.enabled is false
to match the GET guardConfig null case and that data.results or similar scan
output field is defined). Ensure you reference handler.handle and the route
"/__kitchen/api/guard/scan" when adding the test.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
docs/plans/13_devtool_kitchen_dev_spec.mddocs/plans/13_devtool_kitchen_plan.mdpackages/cli/src/commands/dev.tspackages/core/src/devtools/client/components/panel/guard-panel.tsxpackages/core/src/devtools/client/components/panel/panel-container.tsxpackages/core/src/index.tspackages/core/src/kitchen/api/guard-api.tspackages/core/src/kitchen/api/routes-api.tspackages/core/src/kitchen/index.tspackages/core/src/kitchen/kitchen-handler.tspackages/core/src/kitchen/kitchen-ui.tspackages/core/src/kitchen/stream/activity-sse.tspackages/core/src/kitchen/stream/file-tailer.tspackages/core/src/runtime/server.tspackages/core/tests/kitchen/activity-sse.test.tspackages/core/tests/kitchen/file-tailer.test.tspackages/core/tests/kitchen/kitchen-handler.test.tspackages/mcp/src/activity-monitor.ts
Comment on lines
+161
to
+167
| │ │ GET /__kitchen/api/stream │ | ||
| │ │◄──────────────────────────────┤ | ||
| │ │ Content-Type: text/event- │ | ||
| │ │ stream │ | ||
| │ │──────────────────────────────►│ | ||
| │ │ data: {"type":"tool.call"} │ | ||
| ``` |
There was a problem hiding this comment.
API paths in the spec need alignment with implemented Kitchen endpoints.
This spec still documents api/stream and api/guard/check, but the code introduced in this PR uses different route shapes (including the guard scan path). Please sync the spec to the shipped endpoints.
Also applies to: 434-439, 586-608
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/13_devtool_kitchen_dev_spec.md` around lines 161 - 167, Update the
spec's API path strings to match the actual routes implemented by the Kitchen
server: replace occurrences of "GET /__kitchen/api/stream" and any "api/stream"
or "api/guard/check" references with the real endpoint shapes used in the
codebase (including the guard scan route variant), and ensure any SSE
Content-Type and example event payloads remain accurate; search for the string
literals "api/stream", "api/guard/check", and "guard scan" in the doc (also at
the other noted occurrences) and modify them to the shipped route names so the
spec aligns with the implemented endpoints.
Comment on lines
+112
to
+118
| ```typescript | ||
| // SSE 엔드포인트 예시 | ||
| GET /__kitchen/api/stream | ||
| → Content-Type: text/event-stream | ||
| → data: {"type":"tool.call","tool":"mandu_guard_check","ts":"..."} | ||
| → data: {"type":"guard.violation","ruleId":"no-cross-import","file":"..."} | ||
| ``` |
There was a problem hiding this comment.
SSE endpoint in the plan is out of sync with current implementation.
The example here references GET /__kitchen/api/stream, but current Kitchen UI/handler flow in this PR uses a different activity SSE path. Please align this section to avoid integration/test confusion.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/plans/13_devtool_kitchen_plan.md` around lines 112 - 118, Update the SSE
example to use the actual activity SSE route used by the Kitchen UI/handler flow
instead of the outdated GET /__kitchen/api/stream; locate the route/constant or
handler that registers the Kitchen activity SSE (the Kitchen activity SSE route
or handler in the Kitchen UI/handler flow) and replace the example path with
that exact path so the docs match the current implementation.
Comment on lines
+149
to
+163
| const res = await fetch('/__kitchen/api/guard/scan', { method: 'POST' }); | ||
| if (res.ok) { | ||
| const data = await res.json(); | ||
| if (data.report) { | ||
| setScanResult({ | ||
| total: data.report.totalViolations, | ||
| errors: data.report.bySeverity?.error ?? 0, | ||
| warnings: data.report.bySeverity?.warn ?? data.report.bySeverity?.warning ?? 0, | ||
| }); | ||
| } | ||
| } | ||
| } catch { | ||
| // Kitchen API not available (non-dev or older version) | ||
| } finally { | ||
| setScanning(false); |
There was a problem hiding this comment.
Scan failures are swallowed without user feedback
When res.ok is false (e.g., 409 scan-in-progress) or request fails, users get no reason. Surface a compact error message so scan state is understandable.
💡 Suggested fix
@@
- const [scanResult, setScanResult] = useState<{ total: number; errors: number; warnings: number } | null>(null);
+ const [scanResult, setScanResult] = useState<{ total: number; errors: number; warnings: number } | null>(null);
+ const [scanError, setScanError] = useState<string | null>(null);
@@
setScanning(true);
setScanResult(null);
+ setScanError(null);
try {
const res = await fetch('/__kitchen/api/guard/scan', { method: 'POST' });
if (res.ok) {
const data = await res.json();
@@
}
+ } else {
+ const data = await res.json().catch(() => ({}));
+ setScanError(data?.message ?? '스캔에 실패했습니다.');
}
} catch {
- // Kitchen API not available (non-dev or older version)
+ setScanError('Kitchen API에 연결할 수 없습니다.');
} finally {
setScanning(false);
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/devtools/client/components/panel/guard-panel.tsx` around
lines 149 - 163, The current fetch to '/__kitchen/api/guard/scan' swallows
non-ok responses and exceptions so users see no feedback; update the handler in
guard-panel.tsx (the async scan caller that uses fetch, setScanResult and
setScanning) to surface a compact error message: if res.ok is false, try to
parse await res.json() for a message field or fall back to `${res.status}
${res.statusText}` and call setScanResult with an error property (or a small
error-state via a new setScanError state) instead of silently returning;
similarly, in the catch block set the same compact error message (e.g., error:
err.message) so the UI can display it, and keep setScanning(false) in finally;
update any types for setScanResult/state to include an optional error string.
| backgroundColor: `${colors.brand.accent}18`, | ||
| } : {}), | ||
| }} | ||
| onClick={() => window.open('/__kitchen', '_blank')} |
There was a problem hiding this comment.
Harden new-tab opening with noopener,noreferrer.
Line 258 opens a new tab without opener isolation. Add noopener,noreferrer to prevent window.opener access.
🔒 Proposed fix
- onClick={() => window.open('/__kitchen', '_blank')}
+ onClick={() => window.open('/__kitchen', '_blank', 'noopener,noreferrer')}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| onClick={() => window.open('/__kitchen', '_blank')} | |
| onClick={() => window.open('/__kitchen', '_blank', 'noopener,noreferrer')} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/devtools/client/components/panel/panel-container.tsx` at
line 258, The onClick handler in panel-container.tsx currently calls
window.open('/__kitchen', '_blank') without opener isolation; update the onClick
arrow function (the inline handler that opens '/__kitchen') to harden the new
tab by passing opener isolation flags — e.g., call window.open('/__kitchen',
'_blank', 'noopener,noreferrer') or replace with an anchor element using
target="_blank" and rel="noopener noreferrer" — so the opened page cannot access
window.opener.
Comment on lines
+52
to
+73
| if (!this.config) { | ||
| return Response.json( | ||
| { enabled: false, message: "Guard is not configured" }, | ||
| { status: 200 }, | ||
| ); | ||
| } | ||
|
|
||
| if (this.scanning) { | ||
| return Response.json( | ||
| { message: "Scan already in progress" }, | ||
| { status: 409 }, | ||
| ); | ||
| } | ||
|
|
||
| this.scanning = true; | ||
| try { | ||
| this.cachedReport = await checkDirectory(this.config, this.rootDir); | ||
| return Response.json({ | ||
| enabled: true, | ||
| preset: this.config.preset, | ||
| report: this.cachedReport, | ||
| }); |
There was a problem hiding this comment.
Snapshot config before await to avoid runtime race/null dereference.
this.config is mutable (via updateConfig), but handleScan reads it again after await. If config changes mid-scan, this.config.preset can throw or return mismatched metadata.
🛠️ Proposed fix
async handleScan(): Promise<Response> {
- if (!this.config) {
+ const config = this.config;
+ if (!config) {
return Response.json(
{ enabled: false, message: "Guard is not configured" },
{ status: 200 },
);
}
@@
this.scanning = true;
try {
- this.cachedReport = await checkDirectory(this.config, this.rootDir);
+ this.cachedReport = await checkDirectory(config, this.rootDir);
return Response.json({
enabled: true,
- preset: this.config.preset,
+ preset: config.preset,
report: this.cachedReport,
});
} finally {
this.scanning = false;
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (!this.config) { | |
| return Response.json( | |
| { enabled: false, message: "Guard is not configured" }, | |
| { status: 200 }, | |
| ); | |
| } | |
| if (this.scanning) { | |
| return Response.json( | |
| { message: "Scan already in progress" }, | |
| { status: 409 }, | |
| ); | |
| } | |
| this.scanning = true; | |
| try { | |
| this.cachedReport = await checkDirectory(this.config, this.rootDir); | |
| return Response.json({ | |
| enabled: true, | |
| preset: this.config.preset, | |
| report: this.cachedReport, | |
| }); | |
| const config = this.config; | |
| if (!config) { | |
| return Response.json( | |
| { enabled: false, message: "Guard is not configured" }, | |
| { status: 200 }, | |
| ); | |
| } | |
| if (this.scanning) { | |
| return Response.json( | |
| { message: "Scan already in progress" }, | |
| { status: 409 }, | |
| ); | |
| } | |
| this.scanning = true; | |
| try { | |
| this.cachedReport = await checkDirectory(config, this.rootDir); | |
| return Response.json({ | |
| enabled: true, | |
| preset: config.preset, | |
| report: this.cachedReport, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/kitchen/api/guard-api.ts` around lines 52 - 73, handleScan
reads this.config, awaits checkDirectory, then reads this.config.preset which
can change during the await; snapshot the config to a local variable before the
await (e.g. const config = this.config) and use that local for the call to
checkDirectory and when constructing the Response (use config.preset instead of
this.config.preset); keep the existing null/config checks and ensure you still
assign this.cachedReport = await checkDirectory(config, this.rootDir) so the
response is consistent even if updateConfig mutates this.config concurrently.
Comment on lines
+123
to
+136
| stop(): void { | ||
| if (!this.started) return; | ||
| this.started = false; | ||
|
|
||
| this.tailer.stop(); | ||
|
|
||
| if (this.throttleTimer) { | ||
| clearTimeout(this.throttleTimer); | ||
| this.throttleTimer = null; | ||
| } | ||
| if (this.heartbeatTimer) { | ||
| clearInterval(this.heartbeatTimer); | ||
| this.heartbeatTimer = null; | ||
| } |
There was a problem hiding this comment.
Clear buffered events on shutdown to prevent stale replay
pendingEvents remains populated in stop(). After restart, old buffered lines can be emitted with new traffic.
💡 Suggested fix
stop(): void {
@@
if (this.heartbeatTimer) {
clearInterval(this.heartbeatTimer);
this.heartbeatTimer = null;
}
+ this.pendingEvents = [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| stop(): void { | |
| if (!this.started) return; | |
| this.started = false; | |
| this.tailer.stop(); | |
| if (this.throttleTimer) { | |
| clearTimeout(this.throttleTimer); | |
| this.throttleTimer = null; | |
| } | |
| if (this.heartbeatTimer) { | |
| clearInterval(this.heartbeatTimer); | |
| this.heartbeatTimer = null; | |
| } | |
| stop(): void { | |
| if (!this.started) return; | |
| this.started = false; | |
| this.tailer.stop(); | |
| if (this.throttleTimer) { | |
| clearTimeout(this.throttleTimer); | |
| this.throttleTimer = null; | |
| } | |
| if (this.heartbeatTimer) { | |
| clearInterval(this.heartbeatTimer); | |
| this.heartbeatTimer = null; | |
| } | |
| this.pendingEvents = []; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/kitchen/stream/activity-sse.ts` around lines 123 - 136, The
stop() method doesn't clear buffered events so pendingEvents can be replayed
after restart; update stop() (in activity-sse.ts) to clear the pendingEvents
buffer (and reset any related counters like pendingBytes) when shutting
down—after stopping this.tailer and clearing timers set this.pendingEvents = []
and this.pendingBytes = 0 (or equivalent) to prevent stale lines from being
emitted on restart.
Comment on lines
+284
to
+287
| // When openTerminal is enabled, force pretty format for human-readable terminal output | ||
| this.outputFormat = this.config.openTerminal | ||
| ? "pretty" | ||
| : resolveOutputFormat(this.config.output); |
There was a problem hiding this comment.
Don’t couple terminal opening to persisted log format.
Line 284 forces pretty output whenever terminal opening is enabled, which changes persisted activity data format and can break JSONL-based consumers (e.g., activity stream ingestion). Keep storage format resolution independent from terminal behavior.
💡 Proposed fix
- // When openTerminal is enabled, force pretty format for human-readable terminal output
- this.outputFormat = this.config.openTerminal
- ? "pretty"
- : resolveOutputFormat(this.config.output);
+ // Keep persisted log format stable for downstream consumers
+ this.outputFormat = resolveOutputFormat(this.config.output);
@@
- if (this.config.openTerminal) {
+ if (this.config.openTerminal && this.outputFormat === "pretty") {
this.openTerminal();
}Also applies to: 333-335
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/mcp/src/activity-monitor.ts` around lines 284 - 287, The code
currently forces this.outputFormat = "pretty" when this.config.openTerminal is
true, coupling persisted storage format to terminal rendering; change it so
this.outputFormat is always set from resolveOutputFormat(this.config.output) and
introduce a separate terminal-specific setting (e.g., terminalOutputFormat or a
boolean like usePrettyForTerminal) that the terminal-rendering path uses to
force "pretty" without altering this.outputFormat; update any terminal-rendering
logic that referenced this.outputFormat (see the block around this.outputFormat
and the similar block at lines referenced 333-335) to consult the new
terminal-specific symbol instead.
…panel, guard decisions
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (1)
packages/core/src/devtools/client/components/panel/guard-panel.tsx (1)
167-170:⚠️ Potential issue | 🟡 MinorSilent failure on decision API calls.
Failed
approve/rejectrequests are silently swallowed. Users have no indication when their decision wasn't saved.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/devtools/client/components/panel/guard-panel.tsx` around lines 167 - 170, The catch block that swallows errors for the decision API calls (around the approve/reject handlers in guard-panel.tsx) must surface failures: update the catch to log the error (include the thrown error) and notify the user (e.g., via the existing toast/notification or set an error state) so they know the approve/reject failed; also revert any optimistic UI change if you optimistically updated state before the request. Locate the try/catch surrounding the approve/reject calls in the component (handlers named approve/reject or the useEffect wrapping them) and replace the empty catch with error logging plus user-facing notification and optional rollback logic.
🧹 Nitpick comments (10)
packages/core/src/bundler/dev.ts (1)
383-393: Missing"pong"in type union.The
"pong"message type is sent by the server (line 492) and handled in the client switch statement (line 674), but it's not included in theHMRMessage.typeunion. While this is a pre-existing issue, since you're modifying this type, consider adding it for completeness.🔧 Suggested fix
type: | "connected" | "reload" | "island-update" | "layout-update" | "css-update" | "error" | "ping" + | "pong" | "guard-violation" | "kitchen:file-change" | "kitchen:guard-decision";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/bundler/dev.ts` around lines 383 - 393, The HMRMessage.type union is missing the "pong" variant; update the type union declaration for HMRMessage (the union that lists "connected" | "reload" | ... | "kitchen:guard-decision") to include "pong" so the client/server message handling (server send at the pong site and client switch handling) type-checks correctly; locate the union in packages/core/src/bundler/dev.ts (the HMRMessage type) and add "pong" to the list of string literal types.packages/core/src/kitchen/api/diff-parser.ts (1)
8-26: Duplicate type definitions with diff-viewer.tsx.The
DiffLine,DiffHunk, andFileDiffinterfaces are also defined inpackages/core/src/devtools/client/components/panel/diff-viewer.tsx(lines 12-30) with identical structures. Consider defining these types in a single shared location and importing them where needed to avoid maintenance drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/api/diff-parser.ts` around lines 8 - 26, The DiffLine, DiffHunk, and FileDiff interfaces are duplicated; extract these interfaces into a single shared module (e.g., a new types file or existing shared utils) and update both packages/core/src/kitchen/api/diff-parser.ts and packages/core/src/devtools/client/components/panel/diff-viewer.tsx to import DiffLine, DiffHunk, and FileDiff from that shared location, removing the local definitions in each file so only the shared types are maintained and imported where used.packages/core/src/devtools/types.ts (1)
127-135: Duplicate type definition detected.The
GuardDecisioninterface is also defined inpackages/core/src/kitchen/api/guard-decisions.ts(lines 10-18) with an identical structure. Having the same type defined in two places can lead to maintenance drift if one is updated without the other.Consider consolidating by defining the type in one location and re-exporting it from the other.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/devtools/types.ts` around lines 127 - 135, The GuardDecision interface is duplicated; consolidate it by keeping a single source of truth and re-exporting it where needed: choose one module to own the definition (e.g., the existing GuardDecision type in the kitchen API), remove the duplicate interface from packages/core/src/devtools/types.ts, and update that file to import and re-export GuardDecision (or import it directly where used). Ensure references to GuardDecision (interface name) remain unchanged and update any barrels/exports so consuming code still imports the type the same way.packages/core/tests/kitchen/file-api.test.ts (1)
50-62: Consider adding a success-path test for handleFileDiff.The current tests only cover error cases (400, 403). A success-path test would improve confidence, though it requires initializing a git repository in the temp directory.
Example test with git setup
it("should return diff for a modified file", async () => { // Initialize git repo Bun.spawnSync(["git", "init"], { cwd: tmpDir }); Bun.spawnSync(["git", "config", "user.email", "test@test.com"], { cwd: tmpDir }); Bun.spawnSync(["git", "config", "user.name", "Test"], { cwd: tmpDir }); // Create and commit initial file fs.writeFileSync(path.join(tmpDir, "test.ts"), "const x = 1;"); Bun.spawnSync(["git", "add", "test.ts"], { cwd: tmpDir }); Bun.spawnSync(["git", "commit", "-m", "init"], { cwd: tmpDir }); // Modify file fs.writeFileSync(path.join(tmpDir, "test.ts"), "const x = 2;"); const url = new URL("http://localhost/__kitchen/api/file/diff?path=test.ts"); const res = await api.handleFileDiff(url); const data = await res.json(); expect(data.additions).toBeGreaterThan(0); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/tests/kitchen/file-api.test.ts` around lines 50 - 62, Add a success-path test for handleFileDiff in the describe("handleFileDiff") block that initializes a git repo in the test temp directory, commits an initial file, modifies it, then calls api.handleFileDiff(new URL(".../file/diff?path=test.ts")) and asserts the JSON response contains a positive additions count; specifically use Bun.spawnSync to run git init/config/add/commit in tmpDir, write the initial file via fs.writeFileSync, modify it, then await api.handleFileDiff(url) and expect((await res.json()).additions).toBeGreaterThan(0). Ensure the test uses the same tmpDir/test file location used by other kitchen tests and cleans up as appropriate.packages/core/src/kitchen/api/file-api.ts (1)
119-143: Consider adding a timeout for git operations.The
Bun.spawncalls for git commands don't have a timeout. If git becomes unresponsive (e.g., due to large repositories or network issues with remote operations), these operations could hang indefinitely, causing the API endpoint to never respond.Example using AbortSignal for timeout
private async getGitDiff(filePath: string): Promise<FileDiff> { + const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); + const proc = Bun.spawn(["git", "diff", "--", filePath], { cwd: this.rootDir, stdout: "pipe", stderr: "pipe", + signal: controller.signal, }); const stdout = await new Response(proc.stdout).text(); await proc.exited; + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/api/file-api.ts` around lines 119 - 143, The git spawn calls that run ["git", "diff", "--", filePath] and ["git", "diff", "--no-index", "/dev/null", filePath] can hang; add a timeout using an AbortController/AbortSignal passed into Bun.spawn's options (signal) and start a timer to call controller.abort() after a short configurable timeout (e.g., 5s); when aborting, ensure you kill/cleanup the spawned process and await proc.exited, then throw or return a clear timeout error so the caller doesn't hang. Apply this pattern to both proc and procNew invocations (the Bun.spawn calls in this diff) and ensure stderr/stdout are handled/closed after abort to avoid resource leaks.packages/core/src/devtools/client/state-manager.ts (1)
456-466:additionsanddeletionsare always 0 for file change events.The event data doesn't include diff statistics, so
RecentChangerecords will always show0additions/deletions. If this data is needed for UI display, consider enriching the event payload from the source or fetching diff stats on demand.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/devtools/client/state-manager.ts` around lines 456 - 466, The handler for case 'kitchen:file-change' currently sets additions and deletions to 0 causing misleading RecentChange records; update the logic in that switch so you do not hardcode zeros in the call to this.addRecentChange (referencing changeData and addRecentChange) — instead pass undefined/null for additions and deletions or remove those fields so the UI can treat them as unknown, or augment the flow by invoking a diff stats fetch (e.g., call a helper like fetchDiffStats(changeData.file) and merge its result into the object before calling addRecentChange) and fall back to undefined if fetching fails.packages/core/src/devtools/client/components/panel/diff-viewer.tsx (1)
13-31: Consider consolidating duplicate types withdiff-parser.ts.These types (
DiffLine,DiffHunk,FileDiff) duplicate the definitions inpackages/core/src/kitchen/api/diff-parser.ts. Re-exporting from the canonical source would reduce maintenance burden and ensure consistency.// Alternative: re-export from diff-parser export type { DiffLine, DiffHunk, FileDiff } from '../../../../kitchen/api/diff-parser';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/devtools/client/components/panel/diff-viewer.tsx` around lines 13 - 31, The DiffLine, DiffHunk, and FileDiff interfaces in diff-viewer.tsx duplicate the canonical types defined in diff-parser; remove the local type declarations and instead re-export or import the types DiffLine, DiffHunk, and FileDiff from the canonical diff-parser module (use the existing diff-parser export) and update any local references/imports to use those re-exported types so the component uses the single source of truth.packages/core/src/devtools/client/components/panel/preview-panel.tsx (1)
113-126: Consider providing user feedback on diff fetch failure.When
res.okis false or an exception occurs, the user receives no indication that the diff couldn't be loaded. For a dev tool, showing a brief error toast or inline message would improve the debugging experience.💡 Optional: Add error feedback
const handleFileClick = useCallback(async (filePath: string) => { setLoadingDiff(filePath); try { const res = await fetch(`/__kitchen/api/file/diff?path=${encodeURIComponent(filePath)}`); if (res.ok) { const diff = await res.json(); setSelectedDiff(diff); + } else { + console.warn(`[Kitchen] Failed to load diff for ${filePath}: ${res.status}`); } } catch { - // API not available + console.warn(`[Kitchen] Diff API not available`); } finally { setLoadingDiff(null); } }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/devtools/client/components/panel/preview-panel.tsx` around lines 113 - 126, The diff fetch currently swallows failures in handleFileClick: when fetch throws or res.ok is false the user sees no feedback. Update handleFileClick to set an error state or call the existing UI notification/toast function (or add one) so failures display to the user; specifically handle the non-ok branch (after the fetch) and the catch block to call setError/setToast with a descriptive message, ensure setLoadingDiff(null) remains in finally, and avoid clearing selected diff on failure unless intended (use setSelectedDiff only on success).packages/core/src/kitchen/api/guard-decisions.ts (1)
89-95: Consider using async filesystem operations for consistency.The method mixes synchronous
fs.existsSync/fs.mkdirSyncwith asyncBun.write. While functional, using async operations throughout would be more consistent with the async method signature.💡 Optional: Use async mkdir
+import { mkdir } from "fs/promises"; + private async persist(): Promise<void> { const dir = path.dirname(this.filePath); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); - } + await mkdir(dir, { recursive: true }); await Bun.write(this.filePath, JSON.stringify(this.decisions, null, 2)); }Note:
mkdirwithrecursive: trueis a no-op if the directory already exists.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/api/guard-decisions.ts` around lines 89 - 95, In persist() in guard-decisions.ts the code mixes synchronous fs.existsSync/fs.mkdirSync with async Bun.write; change to fully async filesystem calls by removing existsSync and mkdirSync and instead await an async mkdir (e.g., await fs.promises.mkdir(dir, { recursive: true })) before calling Bun.write; keep the same filePath and JSON.stringify logic and ensure any errors propagate or are handled consistently within persist().packages/core/src/kitchen/kitchen-handler.ts (1)
76-198: Consider refactoring to a route map for maintainability.The routing logic is functional but verbose with many if-else branches. As more Kitchen API endpoints are added, this could become harder to maintain. A route map or pattern-matching approach would scale better.
This is acceptable for the current scope, but consider extracting routes to a declarative structure if the handler grows further.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/kitchen/kitchen-handler.ts` around lines 76 - 198, The handle method in kitchen-handler.ts (checking KITCHEN_PREFIX, computing sub, then many if-branches) is verbose and should be refactored into a declarative route map: create a Map or array of route entries keyed by path patterns and HTTP methods that map to handler functions (e.g., entries for "/" -> renderKitchenHTML, "/sse/activity" -> sse.createResponse, "/api/routes" -> handleRoutesRequest(manifest), "/api/guard" -> guardAPI.handleGetReport, "/api/guard/scan" -> guardAPI.handleScan, "/api/guard/decisions" -> guardDecisions.load, "/api/guard/approve" and "/api/guard/reject" -> guardDecisions.save, "/api/guard/decisions/:id" DELETE -> guardDecisions.remove, "/api/file" -> fileAPI.handleReadFile, "/api/file/diff" -> fileAPI.handleFileDiff, "/api/file/changes" -> fileAPI.handleRecentChanges, "/api/contracts" -> contractAPI.handleList, "/api/contracts/validate" -> contractAPI.handleValidate, "/api/contracts/openapi" -> contractAPI.handleOpenAPI, "/api/contracts/openapi.yaml" -> contractAPI.handleOpenAPIYAML, "/api/contracts/:id" -> contractAPI.handleDetail), implement simple pattern matching for paramized segments (like :id) and method checks, then replace the if-chain in handle with a lookup that invokes the matched handler and returns its Response; preserve existing error handling for JSON body parsing and 404 fallback.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demo/test-app/app/api/asd/route` copy.ts:
- Line 4: Update the route doc comment that currently reads "GET /api" to the
correct path "GET /api/asd" in this file (replace the "* GET /api" doc line),
ensuring the route documentation matches the actual route path declared in this
module.
- Around line 1-13: The file containing the GET handler is misnamed ("route
copy.ts") so Mandu's file-based routing will skip it; rename the file to
route.ts and ensure the exported function GET remains unchanged so the framework
discovers the endpoint; no code changes are needed inside the GET function—just
rename the file to route.ts to register the route.
In `@demo/test-app/app/page.tsx`:
- Line 32: Remove the stray character after the Button closing tag: locate the
JSX closing token </Button> in the page component and delete the extraneous "s"
that follows it so the JSX reads correctly without rendering the stray
character.
In `@packages/core/src/kitchen/api/contract-api.ts`:
- Around line 160-182: The validation currently skips if input.query/body/params
is undefined, so required fields on methodSchema
(methodSchema.query/body/params) are not validated; update the three blocks to
always call the schema's safeParse when the schema exists (i.e., keep the
methodSchema.query/body/params checks) but remove the "input?.... !== undefined"
guard so you call methodSchema.query.safeParse(input?.query) (and similarly for
body and params), and push to errors when result.success is false; this ensures
missing required fields are caught.
In `@packages/core/src/kitchen/api/guard-decisions.ts`:
- Around line 29-44: The load() method assigns JSON.parse(text) directly to
this.decisions which is unsafe; change load() to validate the parsed value
before assigning by checking Array.isArray(parsed) and that each item has the
required GuardDecision shape (e.g., presence and types of key fields used
elsewhere), only then assign to this.decisions; if validation fails, treat the
file as corrupt (log/ignore) and fall back to setting this.decisions = [] so no
unsafe type assertion is used.
In `@packages/core/src/kitchen/kitchen-handler.ts`:
- Around line 115-145: The /api/guard/approve and /api/guard/reject handlers
construct violationKey from body.ruleId and body.filePath without validation;
update the handlers in kitchen-handler.ts (the branches checking sub ===
"/api/guard/approve" and sub === "/api/guard/reject") to validate that
body.ruleId and body.filePath are present and non-empty before calling
this.guardDecisions.save, and if either is missing return Response.json({ error:
"Missing required fields: ruleId and filePath" }, { status: 400 }); ensure the
validation runs prior to building violationKey and calling guardDecisions.save
to avoid storing "undefined::undefined".
---
Duplicate comments:
In `@packages/core/src/devtools/client/components/panel/guard-panel.tsx`:
- Around line 167-170: The catch block that swallows errors for the decision API
calls (around the approve/reject handlers in guard-panel.tsx) must surface
failures: update the catch to log the error (include the thrown error) and
notify the user (e.g., via the existing toast/notification or set an error
state) so they know the approve/reject failed; also revert any optimistic UI
change if you optimistically updated state before the request. Locate the
try/catch surrounding the approve/reject calls in the component (handlers named
approve/reject or the useEffect wrapping them) and replace the empty catch with
error logging plus user-facing notification and optional rollback logic.
---
Nitpick comments:
In `@packages/core/src/bundler/dev.ts`:
- Around line 383-393: The HMRMessage.type union is missing the "pong" variant;
update the type union declaration for HMRMessage (the union that lists
"connected" | "reload" | ... | "kitchen:guard-decision") to include "pong" so
the client/server message handling (server send at the pong site and client
switch handling) type-checks correctly; locate the union in
packages/core/src/bundler/dev.ts (the HMRMessage type) and add "pong" to the
list of string literal types.
In `@packages/core/src/devtools/client/components/panel/diff-viewer.tsx`:
- Around line 13-31: The DiffLine, DiffHunk, and FileDiff interfaces in
diff-viewer.tsx duplicate the canonical types defined in diff-parser; remove the
local type declarations and instead re-export or import the types DiffLine,
DiffHunk, and FileDiff from the canonical diff-parser module (use the existing
diff-parser export) and update any local references/imports to use those
re-exported types so the component uses the single source of truth.
In `@packages/core/src/devtools/client/components/panel/preview-panel.tsx`:
- Around line 113-126: The diff fetch currently swallows failures in
handleFileClick: when fetch throws or res.ok is false the user sees no feedback.
Update handleFileClick to set an error state or call the existing UI
notification/toast function (or add one) so failures display to the user;
specifically handle the non-ok branch (after the fetch) and the catch block to
call setError/setToast with a descriptive message, ensure setLoadingDiff(null)
remains in finally, and avoid clearing selected diff on failure unless intended
(use setSelectedDiff only on success).
In `@packages/core/src/devtools/client/state-manager.ts`:
- Around line 456-466: The handler for case 'kitchen:file-change' currently sets
additions and deletions to 0 causing misleading RecentChange records; update the
logic in that switch so you do not hardcode zeros in the call to
this.addRecentChange (referencing changeData and addRecentChange) — instead pass
undefined/null for additions and deletions or remove those fields so the UI can
treat them as unknown, or augment the flow by invoking a diff stats fetch (e.g.,
call a helper like fetchDiffStats(changeData.file) and merge its result into the
object before calling addRecentChange) and fall back to undefined if fetching
fails.
In `@packages/core/src/devtools/types.ts`:
- Around line 127-135: The GuardDecision interface is duplicated; consolidate it
by keeping a single source of truth and re-exporting it where needed: choose one
module to own the definition (e.g., the existing GuardDecision type in the
kitchen API), remove the duplicate interface from
packages/core/src/devtools/types.ts, and update that file to import and
re-export GuardDecision (or import it directly where used). Ensure references to
GuardDecision (interface name) remain unchanged and update any barrels/exports
so consuming code still imports the type the same way.
In `@packages/core/src/kitchen/api/diff-parser.ts`:
- Around line 8-26: The DiffLine, DiffHunk, and FileDiff interfaces are
duplicated; extract these interfaces into a single shared module (e.g., a new
types file or existing shared utils) and update both
packages/core/src/kitchen/api/diff-parser.ts and
packages/core/src/devtools/client/components/panel/diff-viewer.tsx to import
DiffLine, DiffHunk, and FileDiff from that shared location, removing the local
definitions in each file so only the shared types are maintained and imported
where used.
In `@packages/core/src/kitchen/api/file-api.ts`:
- Around line 119-143: The git spawn calls that run ["git", "diff", "--",
filePath] and ["git", "diff", "--no-index", "/dev/null", filePath] can hang; add
a timeout using an AbortController/AbortSignal passed into Bun.spawn's options
(signal) and start a timer to call controller.abort() after a short configurable
timeout (e.g., 5s); when aborting, ensure you kill/cleanup the spawned process
and await proc.exited, then throw or return a clear timeout error so the caller
doesn't hang. Apply this pattern to both proc and procNew invocations (the
Bun.spawn calls in this diff) and ensure stderr/stdout are handled/closed after
abort to avoid resource leaks.
In `@packages/core/src/kitchen/api/guard-decisions.ts`:
- Around line 89-95: In persist() in guard-decisions.ts the code mixes
synchronous fs.existsSync/fs.mkdirSync with async Bun.write; change to fully
async filesystem calls by removing existsSync and mkdirSync and instead await an
async mkdir (e.g., await fs.promises.mkdir(dir, { recursive: true })) before
calling Bun.write; keep the same filePath and JSON.stringify logic and ensure
any errors propagate or are handled consistently within persist().
In `@packages/core/src/kitchen/kitchen-handler.ts`:
- Around line 76-198: The handle method in kitchen-handler.ts (checking
KITCHEN_PREFIX, computing sub, then many if-branches) is verbose and should be
refactored into a declarative route map: create a Map or array of route entries
keyed by path patterns and HTTP methods that map to handler functions (e.g.,
entries for "/" -> renderKitchenHTML, "/sse/activity" -> sse.createResponse,
"/api/routes" -> handleRoutesRequest(manifest), "/api/guard" ->
guardAPI.handleGetReport, "/api/guard/scan" -> guardAPI.handleScan,
"/api/guard/decisions" -> guardDecisions.load, "/api/guard/approve" and
"/api/guard/reject" -> guardDecisions.save, "/api/guard/decisions/:id" DELETE ->
guardDecisions.remove, "/api/file" -> fileAPI.handleReadFile, "/api/file/diff"
-> fileAPI.handleFileDiff, "/api/file/changes" -> fileAPI.handleRecentChanges,
"/api/contracts" -> contractAPI.handleList, "/api/contracts/validate" ->
contractAPI.handleValidate, "/api/contracts/openapi" ->
contractAPI.handleOpenAPI, "/api/contracts/openapi.yaml" ->
contractAPI.handleOpenAPIYAML, "/api/contracts/:id" ->
contractAPI.handleDetail), implement simple pattern matching for paramized
segments (like :id) and method checks, then replace the if-chain in handle with
a lookup that invokes the matched handler and returns its Response; preserve
existing error handling for JSON body parsing and 404 fallback.
In `@packages/core/tests/kitchen/file-api.test.ts`:
- Around line 50-62: Add a success-path test for handleFileDiff in the
describe("handleFileDiff") block that initializes a git repo in the test temp
directory, commits an initial file, modifies it, then calls
api.handleFileDiff(new URL(".../file/diff?path=test.ts")) and asserts the JSON
response contains a positive additions count; specifically use Bun.spawnSync to
run git init/config/add/commit in tmpDir, write the initial file via
fs.writeFileSync, modify it, then await api.handleFileDiff(url) and
expect((await res.json()).additions).toBeGreaterThan(0). Ensure the test uses
the same tmpDir/test file location used by other kitchen tests and cleans up as
appropriate.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
demo/test-app/app/api/asd/route copy.tsdemo/test-app/app/page.tsxdemo/test-app/apps/server/main.tsdemo/test-app/apps/web/entry.tsxdemo/test-app/tests/example.test.tspackages/cli/src/commands/dev.tspackages/core/src/bundler/dev.tspackages/core/src/devtools/client/components/kitchen-root.tsxpackages/core/src/devtools/client/components/panel/diff-viewer.tsxpackages/core/src/devtools/client/components/panel/guard-panel.tsxpackages/core/src/devtools/client/components/panel/index.tspackages/core/src/devtools/client/components/panel/panel-container.tsxpackages/core/src/devtools/client/components/panel/preview-panel.tsxpackages/core/src/devtools/client/state-manager.tspackages/core/src/devtools/design-tokens.tspackages/core/src/devtools/types.tspackages/core/src/kitchen/api/contract-api.tspackages/core/src/kitchen/api/diff-parser.tspackages/core/src/kitchen/api/file-api.tspackages/core/src/kitchen/api/guard-decisions.tspackages/core/src/kitchen/index.tspackages/core/src/kitchen/kitchen-handler.tspackages/core/src/kitchen/kitchen-ui.tspackages/core/tests/kitchen/diff-parser.test.tspackages/core/tests/kitchen/file-api.test.tspackages/core/tests/kitchen/guard-decisions.test.tspackages/core/tests/kitchen/kitchen-handler-phase2.test.ts
✅ Files skipped from review due to trivial changes (1)
- demo/test-app/apps/web/entry.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/core/src/devtools/client/components/panel/panel-container.tsx
- packages/core/src/kitchen/index.ts
Comment on lines
+1
to
+13
| /** | ||
| * Health Check API | ||
| * | ||
| * GET /api | ||
| */ | ||
|
|
||
| export function GET() { | ||
| return Response.json({ | ||
| status: "ok", | ||
| timestamp: new Date().toISOString(), | ||
| framework: "Mandu", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Rename route copy.ts to route.ts to ensure route discovery.
With file-based API routing, app/api/asd/route copy.ts is likely skipped, so this endpoint may never register.
Based on learnings: "Define API routes in app/api/ folder as route.ts files using Mandu framework's filling() method."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demo/test-app/app/api/asd/route` copy.ts around lines 1 - 13, The file
containing the GET handler is misnamed ("route copy.ts") so Mandu's file-based
routing will skip it; rename the file to route.ts and ensure the exported
function GET remains unchanged so the framework discovers the endpoint; no code
changes are needed inside the GET function—just rename the file to route.ts to
register the route.
| /** | ||
| * Health Check API | ||
| * | ||
| * GET /api |
There was a problem hiding this comment.
Fix the route doc comment path.
Line 4 says GET /api, but this file path corresponds to /api/asd.
🛠️ Proposed fix
- * GET /api
+ * GET /api/asd📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| * GET /api | |
| * GET /api/asd |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demo/test-app/app/api/asd/route` copy.ts at line 4, Update the route doc
comment that currently reads "GET /api" to the correct path "GET /api/asd" in
this file (replace the "* GET /api" doc line), ensuring the route documentation
matches the actual route path declared in this module.
| Documentation | ||
| </a> | ||
| </Button> | ||
| </Button>s |
There was a problem hiding this comment.
Remove stray s after </Button>.
Line 32 adds an extra character that will render unexpectedly in the UI.
🛠️ Proposed fix
- </Button>s
+ </Button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| </Button>s | |
| </Button> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@demo/test-app/app/page.tsx` at line 32, Remove the stray character after the
Button closing tag: locate the JSX closing token </Button> in the page component
and delete the extraneous "s" that follows it so the JSX reads correctly without
rendering the stray character.
Comment on lines
+160
to
+182
| // Validate query | ||
| if (methodSchema.query && input?.query !== undefined) { | ||
| const result = methodSchema.query.safeParse(input.query); | ||
| if (!result.success) { | ||
| errors.push({ field: "query", issues: result.error.issues }); | ||
| } | ||
| } | ||
|
|
||
| // Validate body | ||
| if (methodSchema.body && input?.body !== undefined) { | ||
| const result = methodSchema.body.safeParse(input.body); | ||
| if (!result.success) { | ||
| errors.push({ field: "body", issues: result.error.issues }); | ||
| } | ||
| } | ||
|
|
||
| // Validate params | ||
| if (methodSchema.params && input?.params !== undefined) { | ||
| const result = methodSchema.params.safeParse(input.params); | ||
| if (!result.success) { | ||
| errors.push({ field: "params", issues: result.error.issues }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Validation skips missing required fields.
The current validation only runs if the input field is not undefined. If a schema requires a field (e.g., query is required by methodSchema.query) but input.query is undefined, no validation error is returned.
Consider validating against the schema even when input is undefined to catch missing required fields:
Proposed fix
// Validate query
- if (methodSchema.query && input?.query !== undefined) {
+ if (methodSchema.query) {
const result = methodSchema.query.safeParse(input?.query);
if (!result.success) {
errors.push({ field: "query", issues: result.error.issues });
}
}
// Validate body
- if (methodSchema.body && input?.body !== undefined) {
+ if (methodSchema.body) {
const result = methodSchema.body.safeParse(input?.body);
if (!result.success) {
errors.push({ field: "body", issues: result.error.issues });
}
}
// Validate params
- if (methodSchema.params && input?.params !== undefined) {
+ if (methodSchema.params) {
const result = methodSchema.params.safeParse(input?.params);
if (!result.success) {
errors.push({ field: "params", issues: result.error.issues });
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Validate query | |
| if (methodSchema.query && input?.query !== undefined) { | |
| const result = methodSchema.query.safeParse(input.query); | |
| if (!result.success) { | |
| errors.push({ field: "query", issues: result.error.issues }); | |
| } | |
| } | |
| // Validate body | |
| if (methodSchema.body && input?.body !== undefined) { | |
| const result = methodSchema.body.safeParse(input.body); | |
| if (!result.success) { | |
| errors.push({ field: "body", issues: result.error.issues }); | |
| } | |
| } | |
| // Validate params | |
| if (methodSchema.params && input?.params !== undefined) { | |
| const result = methodSchema.params.safeParse(input.params); | |
| if (!result.success) { | |
| errors.push({ field: "params", issues: result.error.issues }); | |
| } | |
| } | |
| // Validate query | |
| if (methodSchema.query) { | |
| const result = methodSchema.query.safeParse(input?.query); | |
| if (!result.success) { | |
| errors.push({ field: "query", issues: result.error.issues }); | |
| } | |
| } | |
| // Validate body | |
| if (methodSchema.body) { | |
| const result = methodSchema.body.safeParse(input?.body); | |
| if (!result.success) { | |
| errors.push({ field: "body", issues: result.error.issues }); | |
| } | |
| } | |
| // Validate params | |
| if (methodSchema.params) { | |
| const result = methodSchema.params.safeParse(input?.params); | |
| if (!result.success) { | |
| errors.push({ field: "params", issues: result.error.issues }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/kitchen/api/contract-api.ts` around lines 160 - 182, The
validation currently skips if input.query/body/params is undefined, so required
fields on methodSchema (methodSchema.query/body/params) are not validated;
update the three blocks to always call the schema's safeParse when the schema
exists (i.e., keep the methodSchema.query/body/params checks) but remove the
"input?.... !== undefined" guard so you call
methodSchema.query.safeParse(input?.query) (and similarly for body and params),
and push to errors when result.success is false; this ensures missing required
fields are caught.
Comment on lines
+29
to
+44
| async load(): Promise<GuardDecision[]> { | ||
| if (this.decisions !== null) return this.decisions; | ||
|
|
||
| try { | ||
| const file = Bun.file(this.filePath); | ||
| if (await file.exists()) { | ||
| const text = await file.text(); | ||
| this.decisions = JSON.parse(text); | ||
| return this.decisions!; | ||
| } | ||
| } catch { | ||
| // File doesn't exist or is corrupt | ||
| } | ||
| this.decisions = []; | ||
| return this.decisions; | ||
| } |
There was a problem hiding this comment.
Type assertion after JSON.parse is unsafe.
The JSON.parse(text) result is assigned directly to this.decisions without validation. If the file contains malformed data (e.g., not an array), this could cause runtime errors later.
🛡️ Optional: Add basic validation
if (await file.exists()) {
const text = await file.text();
- this.decisions = JSON.parse(text);
- return this.decisions!;
+ const parsed = JSON.parse(text);
+ this.decisions = Array.isArray(parsed) ? parsed : [];
+ return this.decisions;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/kitchen/api/guard-decisions.ts` around lines 29 - 44, The
load() method assigns JSON.parse(text) directly to this.decisions which is
unsafe; change load() to validate the parsed value before assigning by checking
Array.isArray(parsed) and that each item has the required GuardDecision shape
(e.g., presence and types of key fields used elsewhere), only then assign to
this.decisions; if validation fails, treat the file as corrupt (log/ignore) and
fall back to setting this.decisions = [] so no unsafe type assertion is used.
Comment on lines
+115
to
+145
| if (sub === "/api/guard/approve" && req.method === "POST") { | ||
| try { | ||
| const body = await req.json(); | ||
| const decision = await this.guardDecisions.save({ | ||
| violationKey: `${body.ruleId}::${body.filePath}`, | ||
| action: "approve", | ||
| ruleId: body.ruleId, | ||
| filePath: body.filePath, | ||
| reason: body.reason, | ||
| }); | ||
| return Response.json({ decision }); | ||
| } catch { | ||
| return Response.json({ error: "Invalid request body" }, { status: 400 }); | ||
| } | ||
| } | ||
|
|
||
| if (sub === "/api/guard/reject" && req.method === "POST") { | ||
| try { | ||
| const body = await req.json(); | ||
| const decision = await this.guardDecisions.save({ | ||
| violationKey: `${body.ruleId}::${body.filePath}`, | ||
| action: "reject", | ||
| ruleId: body.ruleId, | ||
| filePath: body.filePath, | ||
| reason: body.reason, | ||
| }); | ||
| return Response.json({ decision }); | ||
| } catch { | ||
| return Response.json({ error: "Invalid request body" }, { status: 400 }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing input validation for required fields.
Both /approve and /reject endpoints don't validate that ruleId and filePath are present in the request body. If either is undefined, the violationKey becomes "undefined::undefined", which could corrupt the decisions store.
🐛 Proposed fix: Add validation
if (sub === "/api/guard/approve" && req.method === "POST") {
try {
const body = await req.json();
+ if (!body.ruleId || !body.filePath) {
+ return Response.json(
+ { error: "Missing required fields: ruleId and filePath" },
+ { status: 400 },
+ );
+ }
const decision = await this.guardDecisions.save({
violationKey: `${body.ruleId}::${body.filePath}`,
action: "approve",
ruleId: body.ruleId,
filePath: body.filePath,
reason: body.reason,
});
return Response.json({ decision });
} catch {
return Response.json({ error: "Invalid request body" }, { status: 400 });
}
}
if (sub === "/api/guard/reject" && req.method === "POST") {
try {
const body = await req.json();
+ if (!body.ruleId || !body.filePath) {
+ return Response.json(
+ { error: "Missing required fields: ruleId and filePath" },
+ { status: 400 },
+ );
+ }
const decision = await this.guardDecisions.save({📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (sub === "/api/guard/approve" && req.method === "POST") { | |
| try { | |
| const body = await req.json(); | |
| const decision = await this.guardDecisions.save({ | |
| violationKey: `${body.ruleId}::${body.filePath}`, | |
| action: "approve", | |
| ruleId: body.ruleId, | |
| filePath: body.filePath, | |
| reason: body.reason, | |
| }); | |
| return Response.json({ decision }); | |
| } catch { | |
| return Response.json({ error: "Invalid request body" }, { status: 400 }); | |
| } | |
| } | |
| if (sub === "/api/guard/reject" && req.method === "POST") { | |
| try { | |
| const body = await req.json(); | |
| const decision = await this.guardDecisions.save({ | |
| violationKey: `${body.ruleId}::${body.filePath}`, | |
| action: "reject", | |
| ruleId: body.ruleId, | |
| filePath: body.filePath, | |
| reason: body.reason, | |
| }); | |
| return Response.json({ decision }); | |
| } catch { | |
| return Response.json({ error: "Invalid request body" }, { status: 400 }); | |
| } | |
| } | |
| if (sub === "/api/guard/approve" && req.method === "POST") { | |
| try { | |
| const body = await req.json(); | |
| if (!body.ruleId || !body.filePath) { | |
| return Response.json( | |
| { error: "Missing required fields: ruleId and filePath" }, | |
| { status: 400 }, | |
| ); | |
| } | |
| const decision = await this.guardDecisions.save({ | |
| violationKey: `${body.ruleId}::${body.filePath}`, | |
| action: "approve", | |
| ruleId: body.ruleId, | |
| filePath: body.filePath, | |
| reason: body.reason, | |
| }); | |
| return Response.json({ decision }); | |
| } catch { | |
| return Response.json({ error: "Invalid request body" }, { status: 400 }); | |
| } | |
| } | |
| if (sub === "/api/guard/reject" && req.method === "POST") { | |
| try { | |
| const body = await req.json(); | |
| if (!body.ruleId || !body.filePath) { | |
| return Response.json( | |
| { error: "Missing required fields: ruleId and filePath" }, | |
| { status: 400 }, | |
| ); | |
| } | |
| const decision = await this.guardDecisions.save({ | |
| violationKey: `${body.ruleId}::${body.filePath}`, | |
| action: "reject", | |
| ruleId: body.ruleId, | |
| filePath: body.filePath, | |
| reason: body.reason, | |
| }); | |
| return Response.json({ decision }); | |
| } catch { | |
| return Response.json({ error: "Invalid request body" }, { status: 400 }); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/core/src/kitchen/kitchen-handler.ts` around lines 115 - 145, The
/api/guard/approve and /api/guard/reject handlers construct violationKey from
body.ruleId and body.filePath without validation; update the handlers in
kitchen-handler.ts (the branches checking sub === "/api/guard/approve" and sub
=== "/api/guard/reject") to validate that body.ruleId and body.filePath are
present and non-empty before calling this.guardDecisions.save, and if either is
missing return Response.json({ error: "Missing required fields: ruleId and
filePath" }, { status: 400 }); ensure the validation runs prior to building
violationKey and calling guardDecisions.save to avoid storing
"undefined::undefined".
konamgil
force-pushed
the
feat/devtool-kitchen
branch
from
0ba15bd to
889d05f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
/__kitchen경로에서 개발 중 아키텍처 가드 위반, 라우트, 실시간 활동 로그를 확인할 수 있는 대시보드 추가/__kitchen/api/guard,/__kitchen/api/routes엔드포인트로 가드 위반 리포트 조회 및 스캔 트리거 지원Changes
packages/core/src/kitchen/— Kitchen handler, UI, guard-api, routes-api, activity-sse, file-tailerpackages/core/src/devtools/— guard-panel, panel-container 연동packages/cli/src/commands/dev.ts— guardConfig 전달packages/mcp/src/activity-monitor.ts— 에이전트 환경 터미널 오픈 수정docs/plans/— Kitchen dev 대시보드 설계 문서Test plan
bun test packages/core/tests/kitchen/전체 통과 확인bun run mandu dev실행 후/__kitchen접속하여 대시보드 UI 확인🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Tests
Other