chore: bootstrap FUNDING.yml and trufflehog.yml#93
Merged
KooshaPari merged 7 commits intomainfrom May 2, 2026
Merged
Conversation
…ants Serde serialization uses uppercase strings (POC, LTS, DEP, EOL) so variants must match. Adding #[allow] is cleaner than renaming. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace inline complex tuple type with named type alias to fix clippy type_complexity warning. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Add comprehensive terminal session management with PTY support - Add protocol methods and topics handlers - Add IVDE renderer components and services - Add biome.json for code quality - Add .biomeignore for exclusions Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (43)
Note
|
|
codeant-ai
Bot
added
the
size:XXL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+3
to
+7
| const BASE = process.env.BASE_URL || "http://localhost:5173"; | ||
| test("homepage loads", async ({ page }) => { | ||
| await page.goto(BASE); | ||
| await expect(page.locator("body")).toBeVisible(); | ||
| }); |
There was a problem hiding this comment.
Suggestion: This test hardcodes its own default host (localhost) instead of using Playwright's configured baseURL (which is 127.0.0.1 in the repo config). In environments where localhost resolves differently (e.g., IPv6) than the bound web server address, navigation can fail and create flaky CI behavior. Use relative navigation or the same default host as the Playwright config. [possible bug]
Severity Level: Major ⚠️
- ❌ Docs e2e tests can fail despite server running correctly.
- ⚠️ `npm run docs:check` may intermittently fail in CI.Steps of Reproduction ✅
1. Open `/workspace/HeliosLab/docs/package.json` and note line 6 defines `docs:dev` as
`vitepress dev . --host 127.0.0.1 --port 5173`, and line 11 defines `docs:test:e2e` as
`playwright test --config tests/playwright.config.ts`, with `docs:test`/`docs:check`
(lines 12–13) chaining this script.
2. Open `/workspace/HeliosLab/docs/tests/playwright.config.ts`: lines 8–13 configure
Playwright with `use.baseURL: process.env.BASE_URL || "http://127.0.0.1:5173"` (line 11),
and lines 14–20 configure `webServer` to run `npm run docs:dev` with `url:
"http://127.0.0.1:5173/"` (line 17), so the docs server listens only on 127.0.0.1:5173.
3. Open `/workspace/HeliosLab/docs/tests/e2e/docsite.spec.ts`: line 3 defines `const BASE
= process.env.BASE_URL || "http://localhost:5173";` and lines 4–6 define the
`test("homepage loads", ...)` which calls `await page.goto(BASE);` (line 5), using an
absolute URL so Playwright does not apply its `baseURL` from the config.
4. On a machine where the hostname `localhost` resolves to `::1` (IPv6) but the docs dev
server is bound only to `127.0.0.1` (per `docs:dev` host and `webServer.url` above), run
`npm run docs:test:e2e` or `npm run docs:check` from `/workspace/HeliosLab/docs`; the
`homepage loads` test in `docsite.spec.ts` will attempt to open `http://localhost:5173`
(IPv6 ::1), fail to connect to the server listening on 127.0.0.1:5173, and intermittently
error out, making the e2e/docs check flaky.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/tests/e2e/docsite.spec.ts
**Line:** 3:7
**Comment:**
*Possible Bug: This test hardcodes its own default host (`localhost`) instead of using Playwright's configured `baseURL` (which is `127.0.0.1` in the repo config). In environments where `localhost` resolves differently (e.g., IPv6) than the bound web server address, navigation can fail and create flaky CI behavior. Use relative navigation or the same default host as the Playwright config.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is ON, but it could not run because the branch was deleted or merged before autofix could start.
Reviewed by Cursor Bugbot for commit 3118f73. Configure here.
| Dep, // Deprecated | ||
| AR, // Archived | ||
| EOL, // End of Life | ||
| Eol, // End of Life |
There was a problem hiding this comment.
Renamed Stage variants break serde serialization consistency
High Severity
The Stage enum variants POC, LTS, DEP, and EOL were renamed to Poc, Lts, Dep, and Eol, but no #[serde(rename = "...")] attributes were added. Since Stage derives Serialize and Deserialize, serde will now use the new mixed-case names (e.g. "Poc") while Display and FromStr still use the uppercase forms (e.g. "POC"). This creates an inconsistency where serialization round-tripping differs depending on whether serde or Display/FromStr is used, and breaks deserialization of any previously serde-serialized Stage values.
Reviewed by Cursor Bugbot for commit 3118f73. Configure here.
Comment on lines
113
to
+123
| const matrix = readJson( | ||
| resolve( | ||
| root, | ||
| "kitty-specs/001-colab-agent-terminal-control-plane/contracts/protocol-parity-matrix.json" | ||
| ) | ||
| resolve( | ||
| root, | ||
| "kitty-specs/001-colab-agent-terminal-control-plane/contracts/protocol-parity-matrix.json", | ||
| ), | ||
| ); | ||
| const contract = readJson( | ||
| resolve( | ||
| root, | ||
| "kitty-specs/001-colab-agent-terminal-control-plane/contracts/orchestration-envelope.schema.json" | ||
| ) | ||
| resolve( | ||
| root, | ||
| "kitty-specs/001-colab-agent-terminal-control-plane/contracts/orchestration-envelope.schema.json", | ||
| ), |
There was a problem hiding this comment.
🟠 Architect Review — HIGH
The protocol parity gate still reads the matrix and schema from "kitty-specs/001-colab-agent-terminal-control-plane/...", but this repository now only contains those artifacts under "agileplus-specs/..." and ".archive/kitty-specs/...". As a result, running npm run gate:protocol-parity from the repo root fails with a file-not-found error before any parity validation is executed.
Suggestion: Update the gate to read from the canonical "agileplus-specs/colab-agent-terminal-control-plane/contracts/..." paths (or support both old and new locations with clear precedence) so that the parity check can run successfully in CI and local workflows.
Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is an **Architect / Logical Review** comment left during a code review. These reviews are first-class, important findings — not optional suggestions. Do NOT dismiss this as a 'big architectural change' just because the title says architect review; most of these can be resolved with a small, localized fix once the intent is understood.
**Path:** tools/gates/protocol-parity.mjs
**Line:** 113:123
**Comment:**
*HIGH: The protocol parity gate still reads the matrix and schema from "kitty-specs/001-colab-agent-terminal-control-plane/...", but this repository now only contains those artifacts under "agileplus-specs/..." and ".archive/kitty-specs/...". As a result, running `npm run gate:protocol-parity` from the repo root fails with a file-not-found error before any parity validation is executed.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
If a suggested approach is provided above, use it as the authoritative instruction. If no explicit code suggestion is given, you MUST still draft and apply your own minimal, localized fix — do not punt back with 'no suggestion provided, review manually'. Keep the change as small as possible: add a guard clause, gate on a loading state, reorder an await, wrap in a conditional, etc. Do not refactor surrounding code or expand scope beyond the finding.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Quality Gate Report✅ Unit Tests: PASSED |
Comment on lines
+83
to
+89
| cwd: string, | ||
| write: (text: string) => void, | ||
| ) => Promise<boolean>; | ||
|
|
||
| class TerminalManager { | ||
| private terminals: Map<string, TerminalSession> = new Map(); | ||
| private terminalToWindow: Map<string, string> = new Map(); // terminalId -> windowId | ||
| private windowHandlers: Map<string, TerminalMessageHandler> = new Map(); // windowId -> handler | ||
| private pluginCommandChecker?: PluginCommandChecker; | ||
| private pluginCommandExecutor?: PluginCommandExecutor; | ||
| private editCommandHandler?: BuiltinCommandHandler; | ||
|
|
||
| /** | ||
| * @deprecated Use setWindowMessageHandler instead for proper multi-window support | ||
| */ | ||
| setMessageHandler(handler: TerminalMessageHandler) { | ||
| // Legacy support - register as "default" window | ||
| this.windowHandlers.set("default", handler); | ||
| } | ||
|
|
||
| setWindowMessageHandler(windowId: string, handler: TerminalMessageHandler) { | ||
| this.windowHandlers.set(windowId, handler); | ||
| } | ||
|
|
||
| removeWindowMessageHandler(windowId: string) { | ||
| this.windowHandlers.delete(windowId); | ||
| // Kill all terminals owned by this window | ||
| for (const [terminalId, ownerWindowId] of this.terminalToWindow.entries()) { | ||
| if (ownerWindowId === windowId) { | ||
| this.killTerminal(terminalId); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private getMessageHandler(terminalId: string): TerminalMessageHandler | undefined { | ||
| const windowId = this.terminalToWindow.get(terminalId); | ||
| if (windowId) { | ||
| return this.windowHandlers.get(windowId); | ||
| } | ||
| // Fallback to default handler for legacy support | ||
| return this.windowHandlers.get("default"); | ||
| } | ||
|
|
||
| /** | ||
| * Set the plugin command handlers for intercepting terminal input | ||
| */ | ||
| setPluginCommandHandlers( | ||
| checker: PluginCommandChecker, | ||
| executor: PluginCommandExecutor | ||
| ) { | ||
| this.pluginCommandChecker = checker; | ||
| this.pluginCommandExecutor = executor; | ||
| } | ||
|
|
||
| /** | ||
| * Set the handler for the built-in 'edit' command | ||
| */ | ||
| setEditCommandHandler(handler: BuiltinCommandHandler) { | ||
| this.editCommandHandler = handler; | ||
| } | ||
|
|
||
| /** | ||
| * Check if a command line is the built-in 'edit' command | ||
| * Returns the file path argument if it matches, null otherwise | ||
| */ | ||
| private checkEditCommand(commandLine: string): string[] | null { | ||
| const trimmed = commandLine.trim(); | ||
| // Match 'edit <path>' or 'colab <path>' | ||
| const editMatch = trimmed.match(/^edit\s+(.+)$/); | ||
| const colabMatch = trimmed.match(/^colab\s+(.+)$/); | ||
|
|
||
| if (editMatch) { | ||
| // Parse arguments (handle quoted paths, multiple files, etc.) | ||
| return this.parseEditArgs(editMatch[1]); | ||
| } | ||
| if (colabMatch) { | ||
| return this.parseEditArgs(colabMatch[1]); | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Parse edit command arguments, handling quoted paths | ||
| */ | ||
| private parseEditArgs(argsString: string): string[] { | ||
| const args: string[] = []; | ||
| let current = ''; | ||
| let inQuote = false; | ||
| let quoteChar = ''; | ||
|
|
||
| for (const char of argsString) { | ||
| if (!inQuote && (char === '"' || char === "'")) { | ||
| inQuote = true; | ||
| quoteChar = char; | ||
| } else if (inQuote && char === quoteChar) { | ||
| inQuote = false; | ||
| quoteChar = ''; | ||
| } else if (!inQuote && char === ' ') { | ||
| if (current) { | ||
| args.push(current); | ||
| current = ''; | ||
| } | ||
| } else { | ||
| current += char; | ||
| } | ||
| } | ||
| if (current) { | ||
| args.push(current); | ||
| } | ||
| return args; | ||
| } | ||
|
|
||
| createTerminal(cwd: string = process.cwd(), shell?: string, cols: number = 80, rows: number = 24, windowId?: string): string { | ||
| const terminalId = randomUUID(); | ||
|
|
||
| // Determine shell | ||
| const defaultShell = process.platform === "win32" ? "cmd.exe" : | ||
| process.platform === "darwin" ? "/bin/zsh" : "/bin/bash"; | ||
| const terminalShell = shell || process.env["SHELL"] || defaultShell; | ||
|
|
||
| // console.log(`Creating PTY terminal ${terminalId} with shell: ${terminalShell}, cwd: ${cwd}, windowId: ${windowId}`); | ||
|
|
||
| // Path to the Zig PTY binary - in the MacOS directory alongside the main executable | ||
| const ptyBinaryPath = path.join(process.cwd(), "colab-pty"); | ||
|
|
||
| const spawnOptions = { | ||
| stdin: "pipe", | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| cwd: process.cwd(), | ||
| allowUnsafeCustomBinary: true, | ||
| } as Parameters<typeof spawn>[1]; | ||
| const proc = spawn([ptyBinaryPath], spawnOptions); | ||
|
|
||
| const terminal: TerminalSession = { | ||
| id: terminalId, | ||
| process: proc, | ||
| cwd, | ||
| shell: terminalShell, | ||
| ready: false, | ||
| currentCwd: cwd, // Initialize with the starting directory | ||
| inputBuffer: '', // Buffer for plugin command detection | ||
| }; | ||
|
|
||
| // Track which window owns this terminal | ||
| if (windowId) { | ||
| this.terminalToWindow.set(terminalId, windowId); | ||
| } | ||
|
|
||
| // Handle PTY output | ||
| this.readPtyOutput(proc, terminalId); | ||
|
|
||
| // Handle process exit | ||
| proc.exited.then((exitCode: number) => { | ||
| // console.log(`PTY process ${terminalId} exited with code ${exitCode}`); | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (terminal) { | ||
| this.cleanupReaders(terminal); | ||
| } | ||
| const handler = this.getMessageHandler(terminalId); | ||
| handler?.({ | ||
| type: "terminalExit", | ||
| terminalId, | ||
| exitCode, | ||
| signal: 0, | ||
| }); | ||
| this.terminals.delete(terminalId); | ||
| this.terminalToWindow.delete(terminalId); | ||
| }); | ||
|
|
||
| this.terminals.set(terminalId, terminal); | ||
|
|
||
| // Send spawn message to PTY binary | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'spawn', | ||
| spawn: { | ||
| shell: terminalShell, | ||
| cwd: cwd, | ||
| cols: cols, | ||
| rows: rows, | ||
| } | ||
| }); | ||
|
|
||
| return terminalId; | ||
| } | ||
|
|
||
| private sendPtyMessage(terminalId: string, message: PtyMessage) { | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (!terminal) return; | ||
|
|
||
| try { | ||
| const jsonMessage = JSON.stringify(message) + '\n'; | ||
| const stdin = terminal.process.stdin; | ||
| if (!hasWrite(stdin)) { | ||
| console.error(`Terminal ${terminalId} stdin is not writable`); | ||
| return; | ||
| } | ||
| stdin.write(jsonMessage); | ||
| } catch (error) { | ||
| console.error("Error sending PTY message:", error); | ||
| } | ||
| } | ||
|
|
||
| private async readPtyOutput(proc: ReturnType<typeof spawn>, terminalId: string) { | ||
| try { | ||
| if (!hasReader(proc.stdout) || !hasReader(proc.stderr)) { | ||
| throw new Error("PTY stdio streams are not readable"); | ||
| } | ||
| const stdoutReader = proc.stdout.getReader(); | ||
| const stderrReader = proc.stderr.getReader(); | ||
|
|
||
| // Store readers on terminal session for cleanup | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (terminal) { | ||
| terminal.stdoutReader = stdoutReader; | ||
| terminal.stderrReader = stderrReader; | ||
| } | ||
|
|
||
| // Read stdout (JSON messages from PTY binary) | ||
| (async () => { | ||
| try { | ||
| let buffer = ''; | ||
| while (true) { | ||
| const { done, value } = await stdoutReader.read(); | ||
| if (done) break; | ||
|
|
||
| const text = new TextDecoder().decode(value); | ||
| buffer += text; | ||
|
|
||
| // Process complete JSON lines | ||
| const lines = buffer.split('\n'); | ||
| buffer = lines.pop() || ''; // Keep incomplete line in buffer | ||
|
|
||
| for (const line of lines) { | ||
| if (line.trim()) { | ||
| try { | ||
| const response: PtyResponse = JSON.parse(line); | ||
| this.handlePtyResponse(terminalId, response); | ||
| } catch (error) { | ||
| console.error("Error parsing PTY response:", error, line); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } catch (error) { | ||
| // Ignore errors when reader is cancelled during cleanup | ||
| if (!(error instanceof Error && error.name === 'AbortError')) { | ||
| console.error("Error reading PTY stdout:", error); | ||
| } | ||
| } | ||
| })(); | ||
|
|
||
| // Read stderr (PTY binary errors) | ||
| (async () => { | ||
| try { | ||
| while (true) { | ||
| const { done, value } = await stderrReader.read(); | ||
| if (done) break; | ||
|
|
||
| const text = new TextDecoder().decode(value); | ||
| console.error(`PTY ${terminalId} stderr:`, text); | ||
| } | ||
| } catch (error) { | ||
| // Ignore errors when reader is cancelled during cleanup | ||
| if (!(error instanceof Error && error.name === 'AbortError')) { | ||
| console.error("Error reading PTY stderr:", error); | ||
| } | ||
| } | ||
| })(); | ||
|
|
||
| } catch (error) { | ||
| console.error("Error setting up PTY output readers:", error); | ||
| } | ||
| } | ||
|
|
||
| private cleanupReaders(terminal: TerminalSession) { | ||
| try { | ||
| terminal.stdoutReader?.cancel(); | ||
| } catch (e) { | ||
| // Already closed or cancelled | ||
| } | ||
| try { | ||
| terminal.stderrReader?.cancel(); | ||
| } catch (e) { | ||
| // Already closed or cancelled | ||
| } | ||
| } | ||
|
|
||
| private handlePtyResponse(terminalId: string, response: PtyResponse) { | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (!terminal) return; | ||
|
|
||
| const messageHandler = this.getMessageHandler(terminalId); | ||
| // console.log(`PTY ${terminalId} response:`, response); | ||
|
|
||
| switch (response.type) { | ||
| case 'ready': | ||
| terminal.ready = true; | ||
| // console.log(`PTY terminal ${terminalId} is ready`); | ||
| break; | ||
|
|
||
| case 'data': | ||
| if (response.data) { | ||
| messageHandler?.({ | ||
| type: "terminalOutput", | ||
| terminalId, | ||
| data: response.data, | ||
| }); | ||
| } | ||
| break; | ||
|
|
||
| case 'cwd_update': | ||
| if (response.data) { | ||
| // Update the stored current working directory | ||
| terminal.currentCwd = response.data; | ||
| } | ||
| break; | ||
|
|
||
| case 'error': | ||
| console.error(`PTY error for ${terminalId}:`, response.error_msg); | ||
| messageHandler?.({ | ||
| type: "terminalOutput", | ||
| terminalId, | ||
| data: `Error: ${response.error_msg}\r\n`, | ||
| }); | ||
| break; | ||
| } | ||
| } | ||
|
|
||
| writeToTerminal(terminalId: string, data: string): boolean { | ||
| // console.log(`Writing to PTY terminal ${terminalId}:`, JSON.stringify(data)); | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (!terminal) { | ||
| console.log("Terminal not found:", { terminal: !!terminal }); | ||
| return false; | ||
| } | ||
|
|
||
| if (!terminal.ready) { | ||
| console.log("Terminal not ready yet"); | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| // Filter dangerous control characters from pasted content (multi-character input) | ||
| // Single character input is preserved to allow intentional Ctrl+D for EOF to running programs | ||
| // But pasted content should never contain EOF (\x04) as it can accidentally close the shell | ||
| if (data.length > 1) { | ||
| // Remove \x04 (Ctrl+D/EOF) from pasted content to prevent accidental shell exit | ||
| data = data.replace(/\x04/g, ''); | ||
| // If filtering removed all content, nothing to send | ||
| if (data.length === 0) { | ||
| return true; | ||
| } | ||
| } | ||
| const messageHandler = this.getMessageHandler(terminalId); | ||
|
|
||
| // Check for built-in and plugin command interception | ||
| // Handle special characters | ||
| if (data === '\r' || data === '\n') { | ||
| // Enter pressed - check if buffer matches a built-in or plugin command | ||
| const commandLine = terminal.inputBuffer.trim(); | ||
|
|
||
| // Check for built-in 'edit' command first | ||
| const editArgs = this.checkEditCommand(commandLine); | ||
| if (editArgs && editArgs.length > 0 && this.editCommandHandler) { | ||
| // Clear the buffer | ||
| terminal.inputBuffer = ''; | ||
|
|
||
| // Echo newline to terminal | ||
| messageHandler?.({ | ||
| type: "terminalOutput", | ||
| terminalId, | ||
| data: '\r\n', | ||
| }); | ||
|
|
||
| // Execute edit command | ||
| const write = (text: string) => { | ||
| messageHandler?.({ | ||
| type: "terminalOutput", | ||
| terminalId, | ||
| data: text, | ||
| }); | ||
| }; | ||
|
|
||
| const cwd = terminal.currentCwd || terminal.cwd; | ||
| this.editCommandHandler(editArgs, terminalId, cwd, write).then(() => { | ||
| // Show a new prompt after command completes | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'input', | ||
| input: { data: '' } | ||
| }); | ||
| }); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| // Check for plugin command | ||
| if (this.pluginCommandChecker && this.pluginCommandExecutor) { | ||
| const pluginCommand = this.pluginCommandChecker(commandLine); | ||
|
|
||
| if (pluginCommand) { | ||
| // Clear the buffer | ||
| terminal.inputBuffer = ''; | ||
|
|
||
| // Echo newline to terminal | ||
| messageHandler?.({ | ||
| type: "terminalOutput", | ||
| terminalId, | ||
| data: '\r\n', | ||
| }); | ||
|
|
||
| // Execute plugin command and stream output | ||
| const write = (text: string) => { | ||
| messageHandler?.({ | ||
| type: "terminalOutput", | ||
| terminalId, | ||
| data: text, | ||
| }); | ||
| }; | ||
|
|
||
| const cwd = terminal.currentCwd || terminal.cwd; | ||
| this.pluginCommandExecutor(commandLine, terminalId, cwd, write).then(() => { | ||
| // Show a new prompt after command completes | ||
| // Send empty input to trigger shell prompt | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'input', | ||
| input: { data: '' } | ||
| }); | ||
| }); | ||
|
|
||
| return true; | ||
| } | ||
| } | ||
|
|
||
| // Not a built-in or plugin command, clear buffer and send to PTY | ||
| terminal.inputBuffer = ''; | ||
| } else if (data === '\x7f' || data === '\b') { | ||
| // Backspace - remove last char from buffer | ||
| terminal.inputBuffer = terminal.inputBuffer.slice(0, -1); | ||
| } else if (data === '\x03') { | ||
| // Ctrl+C - clear buffer | ||
| terminal.inputBuffer = ''; | ||
| } else if (data === '\x15') { | ||
| // Ctrl+U - clear line/buffer | ||
| terminal.inputBuffer = ''; | ||
| } else if (data.length === 1 && data.charCodeAt(0) >= 32) { | ||
| // Regular printable character - add to buffer | ||
| terminal.inputBuffer += data; | ||
| } | ||
|
|
||
| // Send input to PTY binary, chunking large inputs to avoid buffer overflow | ||
| // The Zig PTY binary has an 8192 byte buffer. We chunk at 2048 to be safe | ||
| // because JSON escaping can significantly increase size (e.g., \n -> \\n, \x1b -> \\u001b) | ||
| const MAX_CHUNK_SIZE = 2048; | ||
|
|
||
| if (data.length <= MAX_CHUNK_SIZE) { | ||
| // Small input, send directly | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'input', | ||
| input: { | ||
| data: data | ||
| } | ||
| }); | ||
| } else { | ||
| // Large input (paste), chunk it to avoid StreamTooLong error | ||
| for (let i = 0; i < data.length; i += MAX_CHUNK_SIZE) { | ||
| const chunk = data.slice(i, i + MAX_CHUNK_SIZE); | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'input', | ||
| input: { | ||
| data: chunk | ||
| } | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| console.error("Error writing to PTY terminal:", error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| resizeTerminal(terminalId: string, cols: number, rows: number): boolean { | ||
| // console.log(`Resizing PTY terminal ${terminalId}: ${cols}x${rows}`); | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (!terminal) return false; | ||
|
|
||
| try { | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'resize', | ||
| resize: { | ||
| cols: cols, | ||
| rows: rows | ||
| } | ||
| }); | ||
| return true; | ||
| } catch (error) { | ||
| console.error("Error resizing PTY terminal:", error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| killTerminal(terminalId: string): boolean { | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (!terminal) { | ||
| return false; | ||
| } | ||
|
|
||
| try { | ||
| // Cancel stream readers first to stop the read loops | ||
| this.cleanupReaders(terminal); | ||
|
|
||
| // Send shutdown message to PTY binary | ||
| this.sendPtyMessage(terminalId, { | ||
| type: 'shutdown' | ||
| }); | ||
|
|
||
| // Give PTY time to cleanup, then kill the process | ||
| setTimeout(() => { | ||
| try { | ||
| terminal.process.kill(); | ||
| } catch (error) { | ||
| console.error("Error killing PTY process:", error); | ||
| } | ||
| }, 100); | ||
|
|
||
| this.terminals.delete(terminalId); | ||
| this.terminalToWindow.delete(terminalId); | ||
| return true; | ||
| } catch (error) { | ||
| console.error("Error killing terminal:", error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| getTerminal(terminalId: string): TerminalSession | undefined { | ||
| return this.terminals.get(terminalId); | ||
| } | ||
|
|
||
| getAllTerminals(): TerminalSession[] { | ||
| return Array.from(this.terminals.values()); | ||
| } | ||
|
|
||
| cleanup() { | ||
| for (const terminal of this.terminals.values()) { | ||
| try { | ||
| // Cancel stream readers first to stop the read loops | ||
| this.cleanupReaders(terminal); | ||
|
|
||
| // Send shutdown message to each PTY | ||
| this.sendPtyMessage(terminal.id, { | ||
| type: 'shutdown' | ||
| }); | ||
|
|
||
| // Kill the process after a short delay | ||
| setTimeout(() => { | ||
| try { | ||
| terminal.process.kill(); | ||
| } catch (error) { | ||
| console.error("Error killing PTY process during cleanup:", error); | ||
| } | ||
| }, 100); | ||
| } catch (error) { | ||
| console.error("Error during cleanup:", error); | ||
| } | ||
| } | ||
| this.terminals.clear(); | ||
| } | ||
|
|
||
| async getTerminalCwd(terminalId: string): Promise<string | null> { | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (!terminal) { | ||
| return null; | ||
| } | ||
|
|
||
| // Send a get_cwd message to the PTY binary to get the current directory | ||
| try { | ||
| this.sendPtyMessage(terminalId, { type: 'get_cwd' }); | ||
|
|
||
| // Wait a bit for the response | ||
| await new Promise(resolve => setTimeout(resolve, 100)); | ||
|
|
||
| // Return the current tracked CWD | ||
| return terminal.currentCwd || terminal.cwd; | ||
| } catch (error) { | ||
| console.error(`Error getting CWD for terminal ${terminalId}:`, error); | ||
| return terminal.cwd; | ||
| } | ||
| } | ||
| private terminals: Map<string, TerminalSession> = new Map(); | ||
| private terminalToWindow: Map<string, string> = new Map(); // terminalId -> windowId |
There was a problem hiding this comment.
Suggestion: The async built-in command execution uses .then(...) without a .catch(...), so rejected promises become unhandled rejections and can destabilize the process. Add rejection handling around the command execution path. [possible bug]
Severity Level: Critical 🚨
- ❌ Unhandled edit command rejections may crash main process.
- ⚠️ Built-in edit command unstable on filesystem errors.Steps of Reproduction ✅
1. On startup, `src/main/index.ts:19-24` wires the built-in edit command by calling
`terminalManager.setEditCommandHandler(...)` with an `async` handler that performs
filesystem operations (`existsSync`, `statSync`, `writeFileSync`) and returns a
`Promise<boolean>` (`index.ts:26-72`).
2. A user opens the integrated terminal via the `createTerminal` RPC
(`src/main/index.ts:2050-2089`) and types an `edit` command (e.g., `edit /some/path`),
which is sent over RPC `writeToTerminal` to `terminalManager.writeToTerminal` in
`src/main/utils/terminalManager.ts:27-184`.
3. When the user presses Enter, `writeToTerminal` detects the built-in edit command (via
`checkEditCommand`) and, at `terminalManager.ts:82-89`, executes
`this.editCommandHandler(editArgs, terminalId, cwd, write).then(() => { ... })` without
attaching any `.catch(...)` or surrounding try/catch around the promise.
4. If the edit handler's promise rejects—for example, `statSync(filePath)` in
`index.ts:42-45` throws due to permission issues or other filesystem errors—the rejection
propagates out of `this.editCommandHandler(...).then(...)` at `terminalManager.ts:82-89`
as an unhandled promise rejection. In Bun/Node environments, such unhandled rejections are
logged and may terminate the process depending on configuration, destabilizing the main
process when users invoke `edit` on problematic paths.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/main/utils/terminalManager.ts
**Line:** 83:89
**Comment:**
*Possible Bug: The async built-in command execution uses `.then(...)` without a `.catch(...)`, so rejected promises become unhandled rejections and can destabilize the process. Add rejection handling around the command execution path.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+119
to
+131
| ): TerminalMessageHandler | undefined { | ||
| const windowId = this.terminalToWindow.get(terminalId); | ||
| if (windowId) { | ||
| return this.windowHandlers.get(windowId); | ||
| } | ||
| // Fallback to default handler for legacy support | ||
| return this.windowHandlers.get("default"); | ||
| } | ||
|
|
||
| /** | ||
| * Set the plugin command handlers for intercepting terminal input | ||
| */ | ||
| setPluginCommandHandlers( |
There was a problem hiding this comment.
Suggestion: Plugin command execution also chains .then(...) without handling rejections, which can leak unhandled promise rejections when plugin execution fails. Handle errors explicitly so terminal state and process stability are preserved. [possible bug]
Severity Level: Critical 🚨
- ❌ Plugin terminal commands can crash app on rejection.
- ⚠️ One misbehaving plugin destabilizes all terminal sessions.Steps of Reproduction ✅
1. During startup, `src/main/index.ts:19-24` wires plugin terminal commands by calling
`terminalManager.setPluginCommandHandlers(...)`, passing
`pluginManager.getTerminalCommand` as the checker and
`pluginManager.executeTerminalCommand(commandLine, terminalId, cwd, write)` as the
executor.
2. A user opens a terminal (via RPC `createTerminal` wired in `index.ts:2050-2089`) and
types a plugin-specific command that `pluginManager.getTerminalCommand` recognizes,
causing `writeToTerminal` in `src/main/utils/terminalManager.ts:27-184` to treat the line
as a plugin command.
3. On Enter, `writeToTerminal` detects a plugin command and at
`terminalManager.ts:118-124` calls `this.pluginCommandExecutor(commandLine, terminalId,
cwd, write).then(() => { ... })` to execute the plugin, echoing a newline and later
sending empty input to trigger a new shell prompt (`terminalManager.ts:124-131`).
4. If the plugin execution promise rejects—e.g., due to an exception thrown inside
`pluginManager.executeTerminalCommand(...)` in `src/main/index.ts:182` or in plugin code
it delegates to—the `.then(...)` chain at `terminalManager.ts:118-131` has no
`.catch(...)` and no surrounding try/catch, so the rejection becomes an unhandled promise
rejection. This can surface as logged unhandled rejections or terminate the process,
especially for third-party plugins that are more likely to throw errors.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/main/utils/terminalManager.ts
**Line:** 119:131
**Comment:**
*Possible Bug: Plugin command execution also chains `.then(...)` without handling rejections, which can leak unhandled promise rejections when plugin execution fails. Handle errors explicitly so terminal state and process stability are preserved.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+244
to
+265
| // Handle PTY output | ||
| this.readPtyOutput(proc, terminalId); | ||
|
|
||
| // Handle process exit | ||
| proc.exited.then((exitCode: number) => { | ||
| // console.log(`PTY process ${terminalId} exited with code ${exitCode}`); | ||
| const terminal = this.terminals.get(terminalId); | ||
| if (terminal) { | ||
| this.cleanupReaders(terminal); | ||
| } | ||
| const handler = this.getMessageHandler(terminalId); | ||
| handler?.({ | ||
| type: "terminalExit", | ||
| terminalId, | ||
| exitCode, | ||
| signal: 0, | ||
| }); | ||
| this.terminals.delete(terminalId); | ||
| this.terminalToWindow.delete(terminalId); | ||
| }); | ||
|
|
||
| this.terminals.set(terminalId, terminal); |
There was a problem hiding this comment.
Suggestion: readPtyOutput is started before the terminal is inserted into this.terminals, but readPtyOutput immediately looks up the terminal to attach stream readers. This ordering race prevents readers from being stored, so later cleanup cannot reliably cancel them. Insert the terminal into the map before calling readPtyOutput. [race condition]
Severity Level: Major ⚠️
- ⚠️ TerminalManager.killTerminal can't cancel stdout/stderr readers.
- ⚠️ cleanup() leaves readers running until PTY exit.Steps of Reproduction ✅
1. Start the main process, which wires the terminal RPCs in `src/main/index.ts:2050-2089`
(see `createTerminal` RPC returning `terminalManager.createTerminal(cwd, shell, 80, 24,
windowId)`).
2. From a renderer window, invoke the RPC `createTerminal` (e.g., user opens a new
integrated terminal), which calls `TerminalManager.createTerminal` in
`src/main/utils/terminalManager.ts:197-278`.
3. Inside `createTerminal`, note that at `terminalManager.ts:244-245` the code calls
`this.readPtyOutput(proc, terminalId);` (commented as `// Handle PTY output`) before
inserting the session into `this.terminals` at `terminalManager.ts:265`
(`this.terminals.set(terminalId, terminal);`).
4. In `readPtyOutput` (`src/main/utils/terminalManager.ts:298-315`), the function obtains
`stdoutReader`/`stderrReader` and immediately looks up the session via `const terminal =
this.terminals.get(terminalId);` but, because `createTerminal` has not yet called
`this.terminals.set`, `terminal` is `undefined` and the readers are never stored on the
`TerminalSession`. Later, when the renderer invokes `killTerminal` via RPC
(`index.ts:2070-2072`), `TerminalManager.killTerminal` at `terminalManager.ts:206-237`
calls `cleanupReaders(terminal)`, but `terminal.stdoutReader`/`stderrReader` are still
`undefined`, so cleanup cannot cancel the active readers and relies solely on process
exit.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/main/utils/terminalManager.ts
**Line:** 244:265
**Comment:**
*Race Condition: `readPtyOutput` is started before the terminal is inserted into `this.terminals`, but `readPtyOutput` immediately looks up the terminal to attach stream readers. This ordering race prevents readers from being stored, so later cleanup cannot reliably cancel them. Insert the terminal into the map before calling `readPtyOutput`.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| // Get API tokens from goldfishdb via RPC | ||
| let tokens: SyncedSettings["tokens"] = []; | ||
| try { | ||
| const tokensResult = await request?.getTokens?.(); |
There was a problem hiding this comment.
Suggestion: The sync code calls request?.getTokens?.(), but the main RPC surface does not expose a getTokens method (it has token mutation methods like addToken/deleteToken instead). Because of optional chaining this silently returns undefined, so token export will always be empty and cloud sync loses all saved tokens. Use an RPC method that actually exists (or add getTokens on the main side) and fail loudly if it is missing. [logic error]
Severity Level: Major ⚠️
- ❌ Cloud settings exports omit all GoldfishDB tokens.
- ⚠️ Remote machines never receive API tokens via sync.Steps of Reproduction ✅
1. Run the desktop app with renderer `src/renderers/ivde/index.tsx` and main process
`src/main/index.ts` so `electrobun.rpc` is available.
2. Persist some tokens into GoldfishDB so they appear in the `tokens` collection that
`fetchProjects()` reads in `src/main/index.ts:18-36` (`const { data: tokens } =
db.collection("tokens").query();` at line 30).
3. From the renderer, invoke `uploadSettings(passphrase)` exported from
`src/renderers/ivde/services/settingsSyncService.ts:281-320` (e.g., via a UI action that
triggers Colab Cloud sync). This calls `gatherSyncableSettings()` at line 291.
4. Inside `gatherSyncableSettings` (`settingsSyncService.ts:141-152`), line 144 executes
`const tokensResult = await request?.getTokens?.();`, but the main RPC request handlers in
`src/main/index.ts` (see plugin handlers at 2548-2587 and token handlers at 2860-2919)
define `pluginGetInstalled`, `pluginInstall`, `addToken`, and `deleteToken` but no
`getTokens` method (repository-wide search for `getTokens` in `src/main` finds no
definitions). Because `request.getTokens` is `undefined`, the optional call is skipped,
`tokensResult` is `undefined`, and the local `tokens` array remains empty, so the
encrypted payload uploaded to `/api/sync/settings` never includes any of the tokens stored
in GoldfishDB.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/renderers/ivde/services/settingsSyncService.ts
**Line:** 144:144
**Comment:**
*Logic Error: The sync code calls `request?.getTokens?.()`, but the main RPC surface does not expose a `getTokens` method (it has token mutation methods like `addToken`/`deleteToken` instead). Because of optional chaining this silently returns `undefined`, so token export will always be empty and cloud sync loses all saved tokens. Use an RPC method that actually exists (or add `getTokens` on the main side) and fail loudly if it is missing.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix| if (settings.tokens && settings.tokens.length > 0) { | ||
| try { | ||
| for (const token of settings.tokens) { | ||
| await request?.setToken?.(token); |
There was a problem hiding this comment.
Suggestion: Applying synced tokens uses request?.setToken?.(token), but there is no corresponding setToken RPC handler in the main process contract. This means token restore is effectively a no-op, so downloaded settings do not actually rehydrate tokens. Call an existing token RPC method or add setToken server-side and handle missing methods as an error. [logic error]
Severity Level: Critical 🚨
- ❌ Downloaded Colab Cloud settings never recreate any tokens.
- ⚠️ Token-dependent integrations break after restoring from cloud.Steps of Reproduction ✅
1. On a fresh machine (or profile) start the app so main `src/main/index.ts` and renderer
`src/renderers/ivde/index.tsx` are running, and log into Colab Cloud so
`state.appSettings.colabCloud?.accessToken` is set (checked in `downloadSettings` at
`settingsSyncService.ts:328-330`).
2. Trigger `downloadSettings(passphrase)` from the renderer (function exported at
`src/renderers/ivde/services/settingsSyncService.ts:325-374`). It performs a GET to
`${getApiBaseUrl()}/api/sync/settings` and decodes the JSON response at lines 334-343.
3. Assume the server returns a previously uploaded settings blob whose decrypted
`SyncedSettings` contains a non-empty `tokens` array. `downloadSettings` decrypts the
payload at lines 352-359 and then calls `await applySyncedSettings(settings);` at line
365.
4. Inside `applySyncedSettings` (`settingsSyncService.ts:201-276`), the token rehydration
loop at lines 223-232 executes `await request?.setToken?.(token);` for each token.
However, the main-process RPC handlers in `src/main/index.ts` around 2860-2919 only expose
token mutations on the `send` channel as `addToken` and `deleteToken` (see `addToken: ({
name, url, endpoint, token }) => { ... db.collection("tokens").insert(...) }` at 2875-2880
and `deleteToken` at 2893-2898), and there is no `setToken` handler defined on the
`request` channel (repository-wide search for `setToken` in `src/main` finds no
definition). As a result `request.setToken` is `undefined`, the optional call is skipped,
no `db.collection("tokens").insert(...)` occurs, and after sync the `fetchProjects()`
function at `src/main/index.ts:18-37` still returns an empty `tokens` array. The
renderer's `state.tokens` (displayed in the Global Settings "Tokens" section at
`src/renderers/ivde/index.tsx:72-79`) remains empty, so downloaded tokens are never
restored.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/renderers/ivde/services/settingsSyncService.ts
**Line:** 227:227
**Comment:**
*Logic Error: Applying synced tokens uses `request?.setToken?.(token)`, but there is no corresponding `setToken` RPC handler in the main process contract. This means token restore is effectively a no-op, so downloaded settings do not actually rehydrate tokens. Call an existing token RPC method or add `setToken` server-side and handle missing methods as an error.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished reviewing your PR. |
|
CodeAnt AI is running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
codeant-ai
Bot
added
size:XXL
Sequence DiagramThis PR adds a TruffleHog configuration so that CI runs a secrets scan on each change, traversing the repository with specific depth and ignore rules and then reporting any findings back to developers. sequenceDiagram
participant Developer
participant CI
participant TruffleHog
participant Repo
Developer->>CI: Push code or open merge request
CI->>TruffleHog: Start secrets scan using trufflehog.yml
TruffleHog->>Repo: Traverse files with configured depth and excludes
Repo-->>TruffleHog: File contents to scan
TruffleHog-->>CI: Scan results and potential secret findings
CI-->>Developer: Surface scan status and any secrets detected
Generated by CodeAnt AI |
Comment on lines
71
to
+85
| @@ -76,19 +77,19 @@ pub enum Stage { | |||
| CN, // Canary | |||
| RC, // Release Candidate | |||
| GA, // General Availability | |||
| LTS, // Long-Term Support | |||
| Lts, // Long-Term Support | |||
| HF, // Hotfix | |||
| SS, // Sunset / Stability-only | |||
| DEP, // Deprecated | |||
| Dep, // Deprecated | |||
| AR, // Archived | |||
| EOL, // End of Life | |||
| Eol, // End of Life | |||
There was a problem hiding this comment.
Suggestion: Renaming enum variants to Poc/Lts/Dep/Eol while still deriving Serialize/Deserialize changes the wire format from uppercase stage codes (e.g. "POC") to Rust-case names (e.g. "Poc"). Any existing serialized data or downstream clients expecting uppercase stage values will fail to deserialize or see contract-breaking payloads. Keep serialized names stable by adding explicit serde renames per variant (or custom Serialize/Deserialize using the existing Display/FromStr mapping). [logic error]
Severity Level: Critical 🚨
- ❌ Downstream services serializing `Stage` via serde receive changed strings.
- ⚠️ Future APIs using `Stage` expose inconsistent stage code casing.
- ⚠️ Stored JSON data with old codes fails deserialization.Steps of Reproduction ✅
1. Open `pheno-core/src/lib.rs` and locate the `Stage` enum definition at lines 66–86,
where it is declared `#[derive(..., Serialize, Deserialize)] pub enum Stage { SP, Poc, IP,
A, FP, B, EP, CN, RC, GA, Lts, HF, SS, Dep, AR, Eol }`.
2. In the same file's `tests` module (`pheno-core/src/lib.rs`, lines 282+), add a new test
that uses serde to serialize a stage, for example:
```rust
#[test]
fn test_stage_serde_wire_format() {
assert_eq!(serde_json::to_string(&Stage::Poc).unwrap(), "\"Poc\"");
}
then run cargo test -p pheno-core.
-
Observe that the assertion passes: serde, using the
Serializederive onStage
(lines 68–69), emits the Rust-case variant name ("Poc") as the wire value for
Stage::Poc, and similarly would emit"Lts","Dep","Eol"for those variants. -
Compare this to the textual contract encoded in the same file: the
Displayimpl at
lines 118–127 mapsStage::Poc/Lts/Dep/Eolto uppercase strings
"POC"/"LTS"/"DEP"/"EOL", and theFromStrimpl at lines 131–140 only accepts
those uppercase codes. Any downstream code or stored JSON that previously relied on serde
forStageand expected these uppercase codes will now see mixed-case
"Poc"/"Lts"/"Dep"/"Eol", causing deserialization failures or contract-breaking
payloads unless explicit#[serde(rename = "...")](or custom Serialize/Deserialize tied
toDisplay/FromStr) is added.
</details>
[Fix in Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20pheno-core%2Fsrc%2Flib.rs%0A%2A%2ALine%3A%2A%2A%2071%3A85%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Renaming%20enum%20variants%20to%20%60Poc%60%2F%60Lts%60%2F%60Dep%60%2F%60Eol%60%20while%20still%20deriving%20%60Serialize%60%2F%60Deserialize%60%20changes%20the%20wire%20format%20from%20uppercase%20stage%20codes%20%28e.g.%20%60%22POC%22%60%29%20to%20Rust-case%20names%20%28e.g.%20%60%22Poc%22%60%29.%20Any%20existing%20serialized%20data%20or%20downstream%20clients%20expecting%20uppercase%20stage%20values%20will%20fail%20to%20deserialize%20or%20see%20contract-breaking%20payloads.%20Keep%20serialized%20names%20stable%20by%20adding%20explicit%20serde%20renames%20per%20variant%20%28or%20custom%20Serialize%2FDeserialize%20using%20the%20existing%20Display%2FFromStr%20mapping%29.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A) | [Fix in VSCode Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt=This%20is%20a%20comment%20left%20during%20a%20code%20review.%0A%0A%2A%2APath%3A%2A%2A%20pheno-core%2Fsrc%2Flib.rs%0A%2A%2ALine%3A%2A%2A%2071%3A85%0A%2A%2AComment%3A%2A%2A%0A%09%2ALogic%20Error%3A%20Renaming%20enum%20variants%20to%20%60Poc%60%2F%60Lts%60%2F%60Dep%60%2F%60Eol%60%20while%20still%20deriving%20%60Serialize%60%2F%60Deserialize%60%20changes%20the%20wire%20format%20from%20uppercase%20stage%20codes%20%28e.g.%20%60%22POC%22%60%29%20to%20Rust-case%20names%20%28e.g.%20%60%22Poc%22%60%29.%20Any%20existing%20serialized%20data%20or%20downstream%20clients%20expecting%20uppercase%20stage%20values%20will%20fail%20to%20deserialize%20or%20see%20contract-breaking%20payloads.%20Keep%20serialized%20names%20stable%20by%20adding%20explicit%20serde%20renames%20per%20variant%20%28or%20custom%20Serialize%2FDeserialize%20using%20the%20existing%20Display%2FFromStr%20mapping%29.%0A%0AValidate%20the%20correctness%20of%20the%20flagged%20issue.%20If%20correct%2C%20How%20can%20I%20resolve%20this%3F%20If%20you%20propose%20a%20fix%2C%20implement%20it%20and%20please%20make%20it%20concise.%0AOnce%20fix%20is%20implemented%2C%20also%20check%20other%20comments%20on%20the%20same%20PR%2C%20and%20ask%20user%20if%20the%20user%20wants%20to%20fix%20the%20rest%20of%20the%20comments%20as%20well.%20if%20said%20yes%2C%20then%20fetch%20all%20the%20comments%20validate%20the%20correctness%20and%20implement%20a%20minimal%20fix%0A)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** pheno-core/src/lib.rs
**Line:** 71:85
**Comment:**
*Logic Error: Renaming enum variants to `Poc`/`Lts`/`Dep`/`Eol` while still deriving `Serialize`/`Deserialize` changes the wire format from uppercase stage codes (e.g. `"POC"`) to Rust-case names (e.g. `"Poc"`). Any existing serialized data or downstream clients expecting uppercase stage values will fail to deserialize or see contract-breaking payloads. Keep serialized names stable by adding explicit serde renames per variant (or custom Serialize/Deserialize using the existing Display/FromStr mapping).
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+11
to
+12
| baseURL: process.env.BASE_URL || "http://127.0.0.1:5173", | ||
| trace: "on-first-retry", |
There was a problem hiding this comment.
Suggestion: use.baseURL is configurable via BASE_URL, but webServer.url is hardcoded to 127.0.0.1:5173. When BASE_URL points elsewhere (different host/port), Playwright will target one URL while waiting for a different server URL, causing startup/wait failures. Use the same resolved base URL value for both settings. [logic error]
Severity Level: Major ⚠️
- ❌ Docs Playwright tests can hang on server startup.
- ⚠️ BASE_URL-based test runs misaligned with dev server.Steps of Reproduction ✅
1. Note Playwright config at `docs/tests/playwright.config.ts:8-20` defines `use.baseURL`
from `process.env.BASE_URL` (line 11) but hardcodes `webServer.url` to
`http://127.0.0.1:5173/` (line 17).
2. Also note test `docs/tests/e2e/docsite.spec.ts:3-5` derives its own `BASE` from
`process.env.BASE_URL || "http://localhost:5173"` and calls `page.goto(BASE)`.
3. Configure docs dev server to listen on a non-default port (e.g., 4173) and run tests
with `BASE_URL=http://127.0.0.1:4173 npx playwright test docs/tests` so `use.baseURL` and
test `BASE` both point at `:4173` while `webServer.url` still waits on `:5173`.
4. Observe Playwright's webServer integration (same config file, line 14-20) keeps polling
`http://127.0.0.1:5173/` and times out, while tests—if they start—target
`http://127.0.0.1:4173`, demonstrating the divergence between base URL and server wait
URL.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/tests/playwright.config.ts
**Line:** 11:12
**Comment:**
*Logic Error: `use.baseURL` is configurable via `BASE_URL`, but `webServer.url` is hardcoded to `127.0.0.1:5173`. When `BASE_URL` points elsewhere (different host/port), Playwright will target one URL while waiting for a different server URL, causing startup/wait failures. Use the same resolved base URL value for both settings.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+518
to
+530
| this.pluginCommandExecutor( | ||
| commandLine, | ||
| terminalId, | ||
| cwd, | ||
| write, | ||
| ).then(() => { | ||
| // Show a new prompt after command completes | ||
| // Send empty input to trigger shell prompt | ||
| this.sendPtyMessage(terminalId, { | ||
| type: "input", | ||
| input: { data: "" }, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
Suggestion: The plugin command executor is called with .then(...) but without rejection handling. A rejected plugin execution will become an unhandled promise rejection and can leave terminal UX in a broken state. Handle failures explicitly and emit an error/output message. [possible bug]
Severity Level: Critical 🚨
- ❌ Faulty plugin terminal commands cause unhandled promise rejections.
- ⚠️ Integrated terminal may not restore prompt after plugin error.Steps of Reproduction ✅
1. The main process wires plugin terminal commands at `src/main/index.ts:179-182` by
calling `terminalManager.setPluginCommandHandlers`, passing
`pluginManager.executeTerminalCommand(...)` as the `PluginCommandExecutor`.
2. Renderer-side, terminal input from windows is routed via RPC `writeToTerminal` at
`src/main/index.ts:2064-2065`, which calls `terminalManager.writeToTerminal(terminalId,
data)`.
3. In `writeToTerminal` (`src/main/utils/terminalManager.ts:493-533`), when the buffered
command line matches a plugin command (`this.pluginCommandChecker`), it invokes
`this.pluginCommandExecutor(commandLine, terminalId, cwd, write).then(() => {
this.sendPtyMessage(...); })` (lines 518-529) without any `.catch` or try/catch around the
Promise.
4. If a plugin's `executeTerminalCommand` implementation rejects (e.g., a plugin throws in
its handler during development), that rejection bubbles out as an unhandled Promise
rejection and the completion callback sending an empty input to re-display the shell
prompt (lines 523-529) never runs, leaving the integrated terminal without a proper prompt
and emitting unhandled async error logs.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/main/utils/terminalManager.ts
**Line:** 518:530
**Comment:**
*Possible Bug: The plugin command executor is called with `.then(...)` but without rejection handling. A rejected plugin execution will become an unhandled promise rejection and can leave terminal UX in a broken state. Handle failures explicitly and emit an error/output message.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
Comment on lines
+237
to
+242
| try { | ||
| // Check if plugin is already installed | ||
| const installedPlugins = await request?.pluginGetInstalled?.(); | ||
| const isInstalled = installedPlugins?.some( | ||
| (p) => p.name === plugin.name, | ||
| ); |
There was a problem hiding this comment.
Suggestion: pluginGetInstalled is called inside the loop for every plugin, causing repeated RPC round-trips (N+1 pattern) as plugin count grows. Fetch installed plugins once before the loop and reuse the result to avoid unnecessary overhead. [performance]
Severity Level: Major ⚠️
- ⚠️ Settings download performs redundant pluginGetInstalled RPC calls.
- ⚠️ Large plugin sets slow down plugin sync application.Steps of Reproduction ✅
1. The plugin restoration logic in `applySyncedSettings`
(`src/renderers/ivde/services/settingsSyncService.ts:234-272`) loops over
`settings.plugins` starting at line 237.
2. For each plugin in this array, it calls `const installedPlugins = await
request?.pluginGetInstalled?.();` (line 240) and then checks `const isInstalled =
installedPlugins?.some((p) => p.name === plugin.name);` (line 241).
3. The main process RPC implementation for `pluginGetInstalled` is defined in
`src/main/index.ts:2622-2635`, where it maps `pluginManager.getInstalledPlugins()` to an
RPC handler, meaning each call from the renderer incurs a full RPC round-trip and a
traversal of all installed plugins.
4. When `applySyncedSettings` is invoked with many plugins (e.g., on settings download),
this pattern issues `N` identical `pluginGetInstalled` RPCs for `N` plugins instead of
fetching once and reusing the result, causing measurable overhead on the settings sync
path as plugin count grows.Fix in Cursor | Fix in VSCode Claude
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** src/renderers/ivde/services/settingsSyncService.ts
**Line:** 237:242
**Comment:**
*Performance: `pluginGetInstalled` is called inside the loop for every plugin, causing repeated RPC round-trips (N+1 pattern) as plugin count grows. Fetch installed plugins once before the loop and reuse the result to avoid unnecessary overhead.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
|
CodeAnt AI finished running the review. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
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.
User description
Summary
FUNDING.ymlfor GitHub Sponsorstrufflehog.ymlsecrets scanning workflowTest plan
Note
Medium Risk
Mostly formatting/tooling changes, but it also renames
Stageenum variants (e.g.,POC→Poc,LTS→Lts) and updates Rust/FFI entrypoints, which can break downstream code and bindings if not coordinated.Overview
Repo-wide formatting/tooling pass: adds
biome.json/.biomeignore, updatespackage.jsonlint to fail on any diagnostics, and reformats many JSON/TS/JS/Vue/docs assets to Biome-preferred style (including protocol/spec contract JSON and runtimeMETHODS/TOPICSlists).Behavioral/API-adjacent updates: Rust
pheno-ffi-goexports are now explicitlyunsafe extern "C"with documented safety invariants, Python FFI introduces anAuditRecordtype alias, andpheno-corerenames severalStagevariants (POC/LTS/DEP/EOL) to Rust-idiomatic casing with corresponding test updates;pheno-clitweaks table headers/output labels.Reviewed by Cursor Bugbot for commit 3118f73. Bugbot is set up for automated code reviews on this repo. Configure here.
CodeAnt-AI Description
Add repo-wide formatting, secrets scanning, and protocol surface alignment
What Changed
Impact
✅ Fewer secrets committed by mistake✅ More consistent code and docs formatting✅ Clearer command-line tables🔄 Retrigger CodeAnt AI Review
Details
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.