Add desktop perf trace automation - #59
Closed
juliusmarminge wants to merge 83 commits into
Closed
juliusmarminge wants to merge 83 commits into
juliusmarminge wants to merge 83 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds end-to-end desktop performance automation: CI workflow steps and npm scripts to run perf traces, an Electron automation module that seeds state and records traces, a Bun/Node trace-runner that parses traces and can post PR summaries, and lightweight web UI perf attributes. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions
participant Runner as scripts/desktop-perf-trace.mjs
participant Dev as bun dev:desktop
participant Electron as Electron Main
participant Renderer as Web Renderer
participant Tracer as ContentTracing
participant FS as File System
participant GitHub as GitHub API
CI->>Runner: invoke (conditional, optional --post-pr)
Runner->>FS: prepare synthetic state dir
Runner->>Dev: spawn dev:desktop with PERF envs
Dev->>Electron: start app
Electron->>Tracer: start tracing
Electron->>Renderer: inject seed state & reload
loop scripted interactions
Electron->>Renderer: simulate clicks, typing, toggles
Renderer->>Tracer: emit timing events
end
Electron->>Tracer: stop tracing, write trace.json
Electron->>FS: write done.json
Runner->>FS: read trace.json & done.json
Runner->>Runner: parse events, compute aggregates & checks
Runner->>FS: write summary.md
Runner->>GitHub: post summary to PR (optional)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Add CI-driven desktop performance trace automation and virtualized renderer message list with thresholds (50 ms EventDispatch spike check, follow-up passes capped at 5) in
|
juliusmarminge
force-pushed
the
codething/b1a7277c
branch
from
February 16, 2026 16:13
a677c12 to
0cd5642
Compare
Greptile SummaryThis PR adds an automated desktop renderer performance tracing pipeline using Electron's
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant CI as CI Runner
participant Script as desktop-perf-trace.mjs
participant Electron as Electron Main Process
participant Renderer as Renderer (Web)
CI->>Script: xvfb-run bun run test:desktop-perf
Script->>Script: prepareDesktopPerfState(stateDir)
Script->>Electron: spawn bun dev:desktop (with env vars)
Electron->>Renderer: createWindow() & loadURL
Electron->>Renderer: waitForDidFinishLoad()
Electron->>Renderer: seedRendererState() via executeJavaScript
Electron->>Renderer: reload()
Electron->>Renderer: waitForDidFinishLoad()
Electron->>Electron: contentTracing.startRecording()
Electron->>Renderer: runRendererPerfInteractions() via executeJavaScript
Renderer-->>Electron: { threadClicks, typedChars, selectedModel }
Electron->>Electron: contentTracing.stopRecording(tracePath)
Electron->>Script: Write done.json marker file
Script->>Script: Poll for done.json
Script->>Script: summarizeTrace(tracePath)
Script->>Script: createMarkdownSummary()
alt --post-pr flag
Script->>CI: gh pr comment (post summary)
end
Script->>Electron: terminateProcessTree()
Script->>Script: Check thresholds (pass/fail)
Last reviewed commit: 0cd5642 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+345
to
+347
| window.webContents.reload(); | ||
| console.log("[desktop-perf] waiting for reload"); | ||
| await waitForDidFinishLoad(window.webContents); |
There was a problem hiding this comment.
Race condition after reload()
reload() is asynchronous — navigation may not have started when waitForDidFinishLoad is called on the next line. If isLoading() returns false (because the reload hasn't kicked in yet), the promise resolves immediately, and the perf automation proceeds before the page has actually reloaded. This could lead to running scripted interactions against stale DOM.
The safe fix is to register the did-finish-load listener before triggering the reload:
Suggested change
| window.webContents.reload(); | |
| console.log("[desktop-perf] waiting for reload"); | |
| await waitForDidFinishLoad(window.webContents); | |
| const reloadedPromise = new Promise<void>((resolve) => { | |
| window.webContents.once("did-finish-load", resolve); | |
| }); | |
| window.webContents.reload(); | |
| console.log("[desktop-perf] waiting for reload"); | |
| await reloadedPromise; |
Additional Comments (1)
The original code guarded |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Comment on lines
+282
to
+285
| return execFileSync("gh", ["pr", "view", "--json", "number", "--jq", ".number"], { | ||
| cwd: repoRoot, | ||
| encoding: "utf8", | ||
| }).trim(); |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:282 Consider validating that the gh pr view result is non-empty before returning. If jq returns null or the command outputs nothing, an empty string will be passed to gh pr comment, causing it to fail.
- return execFileSync("gh", ["pr", "view", "--json", "number", "--jq", ".number"], {
+ const result = execFileSync("gh", ["pr", "view", "--json", "number", "--jq", ".number"], {
cwd: repoRoot,
encoding: "utf8",
- }).trim();
+ }).trim();
+ if (!result) {
+ throw new Error("Could not detect PR number from gh pr view");
+ }
+ return result;🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 282-285:
Consider validating that the `gh pr view` result is non-empty before returning. If `jq` returns null or the command outputs nothing, an empty string will be passed to `gh pr comment`, causing it to fail.
Comment on lines
+36
to
+150
|
|
||
| function summarizeTrace(tracePath) { | ||
| const payload = JSON.parse(fs.readFileSync(tracePath, "utf8")); | ||
| const events = payload.traceEvents; |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:36 Suggestion: Harden JSON artifact reads. Files can exist but be partial/invalid, and trace.json may lack traceEvents. Consider a small helper that waits for the file, retries JSON.parse on failure, and validates structure (e.g., ensure payload.traceEvents is an array or default to []). Use it for both done.json and the trace before iterating.
Suggested change
| const events = payload.traceEvents; | |
| const events = payload.traceEvents ?? []; |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around line 36:
Suggestion: Harden JSON artifact reads. Files can exist but be partial/invalid, and `trace.json` may lack `traceEvents`. Consider a small helper that waits for the file, retries `JSON.parse` on failure, and validates structure (e.g., ensure `payload.traceEvents` is an array or default to `[]`). Use it for both `done.json` and the trace before iterating.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/ci.yml:
- Around line 56-64: The PR-posting step "Desktop perf trace test and PR
summary" currently runs for all pull_request events and uses GH_TOKEN; guard it
to only run for same-repo PRs by adding a condition that checks the PR head repo
equals the workflow repo (e.g. require
github.event.pull_request.head.repo.full_name == github.repository) instead of
the broad github.event_name == 'pull_request', and keep the GH_TOKEN env and the
run command (xvfb-run -a bun run test:desktop-perf:post) only under that guarded
step; additionally add/retain a fallback pull_request step that runs the
non-post command (xvfb-run -a bun run test:desktop-perf) for forked PRs so forks
still run perf traces without attempting to post comments.
In `@scripts/desktop-perf-trace.mjs`:
- Around line 34-162: The summarizeTrace function currently assumes
payload.traceEvents exists; add a guard immediately after parsing (inside
summarizeTrace, after const payload = JSON.parse(...)) to check that
payload.traceEvents is an array (e.g., if (!Array.isArray(payload.traceEvents))
throw new Error(`Invalid trace: missing or malformed traceEvents in
${tracePath}`) or return a clear value), so upstream callers get a readable
error instead of a crash when the trace file is empty/corrupt; reference the
payload and traceEvents variables in the check and include tracePath in the
error message for easier debugging.
🧹 Nitpick comments (2)
apps/desktop/src/main.ts (1)
46-59: Add timeout and failure event handling to prevent hangs.The current implementation only listens to
did-finish-loadand will stall indefinitely if navigation fails, the WebContents is destroyed, or loading stalls. Since this function is used in performance automation where manual intervention isn't possible, add a timeout with error handlers fordid-fail-loadanddestroyedevents, along with proper cleanup.Suggested implementation
-function waitForDidFinishLoad(webContents: WebContents): Promise<void> { +function waitForDidFinishLoad(webContents: WebContents, timeoutMs = 30_000): Promise<void> { if (!webContents.isLoading()) { return Promise.resolve(); } - return new Promise((resolve) => { - const onLoad = () => resolve(); - webContents.once("did-finish-load", onLoad); + return new Promise((resolve, reject) => { + const onFail = () => { + cleanup(); + reject(new Error("Navigation failed")); + }; + const onLoad = () => { + cleanup(); + resolve(); + }; + const timer = setTimeout(() => { + cleanup(); + reject(new Error("Timed out waiting for did-finish-load")); + }, timeoutMs); + const cleanup = () => { + clearTimeout(timer); + webContents.removeListener("did-finish-load", onLoad); + webContents.removeListener("did-fail-load", onFail); + webContents.removeListener("destroyed", onFail); + }; + webContents.once("did-finish-load", onLoad); + webContents.once("did-fail-load", onFail); + webContents.once("destroyed", onFail); }); }apps/web/src/components/ChatView.tsx (1)
1116-1123: Avoiddisabledon interactive buttons; usearia-disabled+ click guards instead. This keeps tooltips and keyboard access intact while still preventing action when busy. Apply the same pattern to the runtime toggle, send button, and approval actions.♿ Example adjustment (apply similarly to the other buttons)
- {pendingApprovals.map((approval) => { - const isResponding = respondingRequestIds.includes(approval.requestId); + {pendingApprovals.map((approval) => { + const isResponding = respondingRequestIds.includes(approval.requestId); + const handleApproval = (decision: ProviderApprovalDecision) => { + if (isResponding) return; + void onRespondToApproval(approval.requestId, decision); + }; return ( <Alert variant="warning" key={approval.requestId}> ... <AlertAction className="col-start-2! -col-end-1! mt-1.5 sm:row-start-auto sm:row-end-auto"> <Button size="xs" variant="default" - disabled={isResponding} - onClick={() => void onRespondToApproval(approval.requestId, "accept")} + aria-disabled={isResponding} + className={cn(isResponding && "opacity-50 cursor-not-allowed")} + onClick={() => handleApproval("accept")} > Approve once </Button>Based on learnings: Avoid using disabled props on buttons as they harm accessibility and break tooltips. Instead, use styling (e.g., opacity, hover states) and handle the disabled state through click handlers.
Also applies to: 1160-1164, 1380-1408
Comment on lines
+34
to
+162
| byName.max = Math.max(byName.max, event.dur); | ||
| durationByName.set(event.name, byName); | ||
|
|
||
| if (event.name === "EventDispatch") { | ||
| const type = event.args?.data?.type ?? ""; | ||
| dispatchRows.push({ type, dur: event.dur }); | ||
| } | ||
|
|
||
| if (event.name === "FunctionCall") { | ||
| const functionName = event.args?.data?.functionName ?? "(unknown)"; | ||
| functionRows.push({ functionName, dur: event.dur }); | ||
| } | ||
| } | ||
|
|
||
| const aggregateDispatchType = (type) => { | ||
| const rows = dispatchRows.filter((row) => row.type === type); | ||
| if (rows.length === 0) { | ||
| return { | ||
| count: 0, | ||
| totalMs: 0, | ||
| avgMs: 0, | ||
| maxMs: 0, | ||
| }; | ||
| } | ||
| const totalDur = rows.reduce((sum, row) => sum + row.dur, 0); | ||
| const maxDur = rows.reduce((max, row) => Math.max(max, row.dur), 0); | ||
| return { | ||
| count: rows.length, | ||
| totalMs: round(toMs(totalDur)), | ||
| avgMs: round(toMs(totalDur / rows.length)), | ||
| maxMs: round(toMs(maxDur)), | ||
| }; | ||
| }; | ||
|
|
||
| const aggregateFunction = (name) => { | ||
| const rows = functionRows.filter((row) => row.functionName === name); | ||
| if (rows.length === 0) { | ||
| return { | ||
| count: 0, | ||
| totalMs: 0, | ||
| avgMs: 0, | ||
| maxMs: 0, | ||
| }; | ||
| } | ||
| const totalDur = rows.reduce((sum, row) => sum + row.dur, 0); | ||
| const maxDur = rows.reduce((max, row) => Math.max(max, row.dur), 0); | ||
| return { | ||
| count: rows.length, | ||
| totalMs: round(toMs(totalDur)), | ||
| avgMs: round(toMs(totalDur / rows.length)), | ||
| maxMs: round(toMs(maxDur)), | ||
| }; | ||
| }; | ||
|
|
||
| const topUserTiming = [...userTiming.entries()] | ||
| .sort((a, b) => b[1] - a[1]) | ||
| .slice(0, 10) | ||
| .map(([name, count]) => ({ name, count })); | ||
|
|
||
| const topDurationEvents = [...durationByName.entries()] | ||
| .map(([name, value]) => ({ | ||
| name, | ||
| count: value.count, | ||
| totalMs: round(toMs(value.dur), 2), | ||
| avgMs: round(toMs(value.dur / value.count)), | ||
| maxMs: round(toMs(value.max)), | ||
| })) | ||
| .sort((a, b) => b.totalMs - a.totalMs) | ||
| .slice(0, 10); | ||
|
|
||
| const heap = (() => { | ||
| if (updateCounters.length === 0) return null; | ||
| const first = updateCounters[0]; | ||
| const last = updateCounters[updateCounters.length - 1]; | ||
| const min = Math.min(...updateCounters); | ||
| const max = Math.max(...updateCounters); | ||
| return { | ||
| firstMb: round(mb(first), 1), | ||
| lastMb: round(mb(last), 1), | ||
| minMb: round(mb(min), 1), | ||
| maxMb: round(mb(max), 1), | ||
| deltaMb: round(mb(last - first), 1), | ||
| }; | ||
| })(); | ||
|
|
||
| const longDispatchCount = dispatchRows.filter((row) => row.dur >= 50_000).length; | ||
|
|
||
| return { | ||
| keypress: aggregateDispatchType("keypress"), | ||
| textInput: aggregateDispatchType("textInput"), | ||
| input: aggregateDispatchType("input"), | ||
| keydown: aggregateDispatchType("keydown"), | ||
| dispatchDiscreteEvent: aggregateFunction("dispatchDiscreteEvent"), | ||
| performWorkUntilDeadline: aggregateFunction("performWorkUntilDeadline"), | ||
| longDispatchCount, | ||
| heap, | ||
| topUserTiming, | ||
| topDurationEvents, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Validate traceEvents presence for clearer failures.
If the trace file is empty/corrupt, payload.traceEvents will be undefined and subsequent loops will crash with a less helpful error. A small guard makes CI failures easier to diagnose.
Suggested guard
- const events = payload.traceEvents;
+ const events = payload.traceEvents;
+ if (!Array.isArray(events)) {
+ throw new Error(`Invalid trace payload: traceEvents missing in ${tracePath}`);
+ }🤖 Prompt for AI Agents
In `@scripts/desktop-perf-trace.mjs` around lines 34 - 162, The summarizeTrace
function currently assumes payload.traceEvents exists; add a guard immediately
after parsing (inside summarizeTrace, after const payload = JSON.parse(...)) to
check that payload.traceEvents is an array (e.g., if
(!Array.isArray(payload.traceEvents)) throw new Error(`Invalid trace: missing or
malformed traceEvents in ${tracePath}`) or return a clear value), so upstream
callers get a readable error instead of a crash when the trace file is
empty/corrupt; reference the payload and traceEvents variables in the check and
include tracePath in the error message for easier debugging.
| ); | ||
| } | ||
|
|
||
| const summary = summarizeTrace(tracePath); |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:400 Suggestion: Harden reading of child-written JSON artifacts. Poll/parse done.json with try/catch to handle partial writes, and ensure trace.json exists, parses, and defaults payload.traceEvents to [] if missing (or centralize via a small helper).
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around line 400:
Suggestion: Harden reading of child-written JSON artifacts. Poll/parse `done.json` with try/catch to handle partial writes, and ensure `trace.json` exists, parses, and defaults `payload.traceEvents` to `[]` if missing (or centralize via a small helper).
| function prepareDesktopPerfState(stateDir) { | ||
| fs.mkdirSync(stateDir, { recursive: true }); | ||
|
|
||
| const perfProjects = [ |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:303 The cwd paths are hardcoded to /tmp/perf/... which will fail on Windows. Consider using os.tmpdir() consistently, e.g. path.join(os.tmpdir(), 'perf', 'codething-mvp').
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around line 303:
The `cwd` paths are hardcoded to `/tmp/perf/...` which will fail on Windows. Consider using `os.tmpdir()` consistently, e.g. `path.join(os.tmpdir(), 'perf', 'codething-mvp')`.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@apps/desktop/src/main.ts`:
- Around line 50-58: The waitForDidFinishLoad function can hang due to a race
between the isLoading() check and adding the listener and also never rejects on
load failure; update waitForDidFinishLoad(WebContents) to register event
listeners first (did-finish-load, did-fail-load and/or crashed), then re-check
isLoading() and resolve immediately if not loading, ensure handlers remove each
other to avoid leaks, and make did-fail-load/crashed handlers reject the promise
with the error details; include robust cleanup (removeListener/once) so the
promise always settles.
Comment on lines
+50
to
+58
| function waitForDidFinishLoad(webContents: WebContents): Promise<void> { | ||
| if (!webContents.isLoading()) { | ||
| return Promise.resolve(); | ||
| } | ||
|
|
||
| return new Promise((resolve) => { | ||
| const onLoad = () => resolve(); | ||
| webContents.once("did-finish-load", onLoad); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/main.ts | head -80Repository: pingdotgg/codething-mvp
Length of output: 2931
🏁 Script executed:
grep -n "waitForDidFinishLoad" apps/desktop/src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 254
🏁 Script executed:
git ls-files apps/desktop/src/Repository: pingdotgg/codething-mvp
Length of output: 220
🏁 Script executed:
sed -n '330,360p' apps/desktop/src/main.ts | cat -nRepository: pingdotgg/codething-mvp
Length of output: 1480
🏁 Script executed:
sed -n '1,100p' apps/desktop/src/main.ts | grep -A 5 -B 5 "await waitForDidFinishLoad"Repository: pingdotgg/codething-mvp
Length of output: 49
🏁 Script executed:
sed -n '300,400p' apps/desktop/src/main.ts | cat -nRepository: pingdotgg/codething-mvp
Length of output: 4182
🏁 Script executed:
rg -A 10 "waitForDidFinishLoad" apps/desktop/src/main.tsRepository: pingdotgg/codething-mvp
Length of output: 1126
Fix potential hang in waitForDidFinishLoad due to race condition and missing error handling.
There is a race condition where the page can finish loading between the isLoading() check and listener registration, causing the promise to never resolve. Additionally, if page loading fails (network error, crash), the function never rejects and hangs indefinitely, blocking the entire perf automation flow.
🛠️ Suggested fix
function waitForDidFinishLoad(webContents: WebContents): Promise<void> {
- if (!webContents.isLoading()) {
- return Promise.resolve();
- }
-
- return new Promise((resolve) => {
- const onLoad = () => resolve();
- webContents.once("did-finish-load", onLoad);
- });
+ return new Promise((resolve, reject) => {
+ let settled = false;
+ const onLoad = () => {
+ if (settled) return;
+ settled = true;
+ cleanup();
+ resolve();
+ };
+ const onFail = (_event: Electron.Event, errorCode: number, errorDescription: string) => {
+ if (settled) return;
+ settled = true;
+ cleanup();
+ reject(new Error(`did-fail-load: ${errorCode} ${errorDescription}`));
+ };
+ const cleanup = () => {
+ webContents.removeListener("did-finish-load", onLoad);
+ webContents.removeListener("did-fail-load", onFail);
+ };
+ webContents.once("did-finish-load", onLoad);
+ webContents.once("did-fail-load", onFail);
+ if (!webContents.isLoading()) {
+ onLoad();
+ }
+ });
}📝 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
| function waitForDidFinishLoad(webContents: WebContents): Promise<void> { | |
| if (!webContents.isLoading()) { | |
| return Promise.resolve(); | |
| } | |
| return new Promise((resolve) => { | |
| const onLoad = () => resolve(); | |
| webContents.once("did-finish-load", onLoad); | |
| }); | |
| function waitForDidFinishLoad(webContents: WebContents): Promise<void> { | |
| return new Promise((resolve, reject) => { | |
| let settled = false; | |
| const onLoad = () => { | |
| if (settled) return; | |
| settled = true; | |
| cleanup(); | |
| resolve(); | |
| }; | |
| const onFail = (_event: Electron.Event, errorCode: number, errorDescription: string) => { | |
| if (settled) return; | |
| settled = true; | |
| cleanup(); | |
| reject(new Error(`did-fail-load: ${errorCode} ${errorDescription}`)); | |
| }; | |
| const cleanup = () => { | |
| webContents.removeListener("did-finish-load", onLoad); | |
| webContents.removeListener("did-fail-load", onFail); | |
| }; | |
| webContents.once("did-finish-load", onLoad); | |
| webContents.once("did-fail-load", onFail); | |
| if (!webContents.isLoading()) { | |
| onLoad(); | |
| } | |
| }); | |
| } |
🤖 Prompt for AI Agents
In `@apps/desktop/src/main.ts` around lines 50 - 58, The waitForDidFinishLoad
function can hang due to a race between the isLoading() check and adding the
listener and also never rejects on load failure; update
waitForDidFinishLoad(WebContents) to register event listeners first
(did-finish-load, did-fail-load and/or crashed), then re-check isLoading() and
resolve immediately if not loading, ensure handlers remove each other to avoid
leaks, and make did-fail-load/crashed handlers reject the promise with the error
details; include robust cleanup (removeListener/once) so the promise always
settles.
| } catch (error) { | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| console.error("[desktop-perf] failed to load automation module:", message); | ||
| const doneOutPath = process.env.T3CODE_DESKTOP_PERF_DONE_OUT?.trim() ?? ""; |
Contributor
There was a problem hiding this comment.
🟢 Low
src/main.ts:50 The fs.mkdirSync/fs.writeFileSync calls inside the catch block could throw if the path is invalid or unwritable. Since the caller uses void, this would cause an unhandled promise rejection. Consider wrapping these fs operations in a nested try-catch.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 50:
The `fs.mkdirSync`/`fs.writeFileSync` calls inside the `catch` block could throw if the path is invalid or unwritable. Since the caller uses `void`, this would cause an unhandled promise rejection. Consider wrapping these fs operations in a nested try-catch.
| }); | ||
| console.log("[desktop-perf] seeding renderer state"); | ||
| await seedRendererState(window); | ||
| window.webContents.reload(); |
Contributor
There was a problem hiding this comment.
🟢 Low
src/desktopPerfAutomation.ts:360 Race condition: reload() is async, so isLoadingMainFrame() may return false before the reload starts. Consider adding a small delay after reload() or listening for did-start-loading before calling waitForDidFinishLoad.
Suggested change
| window.webContents.reload(); | |
| window.webContents.reload(); | |
| await delay(50); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/desktopPerfAutomation.ts around line 360:
Race condition: `reload()` is async, so `isLoadingMainFrame()` may return `false` before the reload starts. Consider adding a small delay after `reload()` or listening for `did-start-loading` before calling `waitForDidFinishLoad`.
| } | ||
| const message = error instanceof Error ? error.message : String(error); | ||
| console.error("[desktop-perf] automation failed:", message); | ||
| if (PERF_DONE_OUT_PATH.length > 0) { |
Contributor
There was a problem hiding this comment.
🟢 Low
src/desktopPerfAutomation.ts:405 If fs.mkdirSync or fs.writeFileSync throws in the catch block (lines 406-420), the original error is lost. Consider wrapping the done-file write in a try-catch to ensure the original failure is preserved.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/desktopPerfAutomation.ts around line 405:
If `fs.mkdirSync` or `fs.writeFileSync` throws in the catch block (lines 406-420), the original error is lost. Consider wrapping the done-file write in a try-catch to ensure the original failure is preserved.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/desktop/src/desktopPerfAutomation.ts (1)
424-458: Minor: Error payload may reference a non-existent trace file.In the error handler,
tracePathis included in the error payload (line 449). If tracing never started or the stop operation failed, this path may not correspond to an actual file on disk. Consider omittingtracePathfrom the error payload when the file doesn't exist, or adding anexistsflag.💡 Optional enhancement
if (PERF_DONE_OUT_PATH.length > 0) { fs.mkdirSync(path.dirname(PERF_DONE_OUT_PATH), { recursive: true }); + const traceExists = fs.existsSync(tracePath); fs.writeFileSync( PERF_DONE_OUT_PATH, JSON.stringify( { status: "error", error: message, - tracePath, + tracePath: traceExists ? tracePath : null, startedAt: new Date(startedAt).toISOString(), completedAt: new Date().toISOString(), }, null, 2, ), ); }
Comment on lines
+295
to
+298
| return execFileSync("gh", ["pr", "view", "--json", "number", "--jq", ".number"], { | ||
| cwd: repoRoot, | ||
| encoding: "utf8", | ||
| }).trim(); |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:295 execFileSync throws an unhelpful error if gh CLI fails (not installed, not authenticated, or not in PR context). Consider wrapping in try-catch to provide a clearer error message about PR detection failure.
- return execFileSync("gh", ["pr", "view", "--json", "number", "--jq", ".number"], {
- cwd: repoRoot,
- encoding: "utf8",
- }).trim();
+ try {
+ return execFileSync("gh", ["pr", "view", "--json", "number", "--jq", ".number"], {
+ cwd: repoRoot,
+ encoding: "utf8",
+ }).trim();
+ } catch (error) {
+ throw new Error("Failed to detect PR number. Ensure gh CLI is installed, authenticated, and run from a PR branch.");
+ }🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 295-298:
`execFileSync` throws an unhelpful error if `gh` CLI fails (not installed, not authenticated, or not in PR context). Consider wrapping in try-catch to provide a clearer error message about PR detection failure.
| } catch { | ||
| child.kill("SIGTERM"); | ||
| } | ||
| return; |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:269 On Windows, the function returns immediately after taskkill without waiting for termination, unlike the non-Windows path which waits 1.5s and escalates to SIGKILL. Consider adding a similar wait-and-verify loop after taskkill to ensure the process is fully terminated before returning.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around line 269:
On Windows, the function returns immediately after `taskkill` without waiting for termination, unlike the non-Windows path which waits 1.5s and escalates to `SIGKILL`. Consider adding a similar wait-and-verify loop after `taskkill` to ensure the process is fully terminated before returning.
Comment on lines
+435
to
+554
| } | ||
|
|
||
| const measureThreadRender = async (threadId) => { | ||
| const selector = '[data-perf-thread-id="' + threadId + '"]'; |
Contributor
There was a problem hiding this comment.
🟢 Low
src/desktopPerfAutomation.ts:435 Thread IDs containing double quotes will break the selector on line 435, causing querySelector to throw a SyntaxError. Consider escaping quotes in threadId with CSS.escape().
Suggested change
| const selector = '[data-perf-thread-id="' + threadId + '"]'; | |
| const selector = '[data-perf-thread-id="' + CSS.escape(threadId) + '"]'; |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/desktopPerfAutomation.ts around line 435:
Thread IDs containing double quotes will break the selector on line 435, causing `querySelector` to throw a `SyntaxError`. Consider escaping quotes in `threadId` with `CSS.escape()`.
Comment on lines
+391
to
+555
| } | ||
|
|
||
| function resolveSeedPath(seedPathArg) { | ||
| const candidateArg = seedPathArg ?? process.env.T3CODE_DESKTOP_PERF_SEED_PATH ?? ""; |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:391 candidateArg is trimmed for the empty check but the untrimmed value is used later. If the env var contains leading whitespace like " /path", path.isAbsolute() returns false and the path resolves incorrectly. Consider using the trimmed value throughout.
Suggested change
| const candidateArg = seedPathArg ?? process.env.T3CODE_DESKTOP_PERF_SEED_PATH ?? ""; | |
| const candidateArg = (seedPathArg ?? process.env.T3CODE_DESKTOP_PERF_SEED_PATH ?? "").trim(); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around line 391:
`candidateArg` is trimmed for the empty check but the untrimmed value is used later. If the env var contains leading whitespace like `" /path"`, `path.isAbsolute()` returns false and the path resolves incorrectly. Consider using the trimmed value throughout.
Comment on lines
+354
to
+356
| try { | ||
| process.kill(-child.pid, "SIGTERM"); | ||
| } catch { |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:354 If spawn fails, child.pid is undefined and -child.pid becomes NaN, causing process.kill to throw. The catch block happens to recover, but consider adding an explicit guard (if (child.pid)) before the process group kill for clearer intent.
Suggested change
| try { | |
| process.kill(-child.pid, "SIGTERM"); | |
| } catch { | |
| try { | |
| if (child.pid) { | |
| process.kill(-child.pid, "SIGTERM"); | |
| } else { | |
| child.kill("SIGTERM"); | |
| } |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 354-356:
If `spawn` fails, `child.pid` is `undefined` and `-child.pid` becomes `NaN`, causing `process.kill` to throw. The catch block happens to recover, but consider adding an explicit guard (`if (child.pid)`) before the process group kill for clearer intent.
Member
Author
Desktop Dev Perf Trace
Interaction Run
Benchmark Thread Render
Input Event Metrics
Scheduler/Event Hotspots
Threshold Check
Heap Counters
Top User Timing Marks
Top Duration Events
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment has been minimized.
This comment has been minimized.
Comment on lines
+176
to
+177
| const min = Math.min(...updateCounters); | ||
| const max = Math.max(...updateCounters); |
Contributor
There was a problem hiding this comment.
🟡 Medium
scripts/desktop-perf-trace.mjs:176 Consider using reduce instead of spreading updateCounters into Math.min/Math.max. Large traces may have tens of thousands of entries, exceeding the call stack limit.
Suggested change
| const min = Math.min(...updateCounters); | |
| const max = Math.max(...updateCounters); | |
| const min = updateCounters.reduce((a, b) => Math.min(a, b), updateCounters[0]); | |
| const max = updateCounters.reduce((a, b) => Math.max(a, b), updateCounters[0]); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 176-177:
Consider using `reduce` instead of spreading `updateCounters` into `Math.min`/`Math.max`. Large traces may have tens of thousands of entries, exceeding the call stack limit.
Evidence trail:
Viewed `scripts/desktop-perf-trace.mjs:173-177` at commit `ae4ca8b3` (https://github.com/pingdotgg/codething-mvp/tree/ae4ca8b3c4fec051c8f58ce3309648bf8f3ec149).
Member
Author
Comment on lines
371
to
+372
| terminal.dispose(); | ||
| }; | ||
| }, [api, cwd, terminalId, threadId]); | ||
| }, [api, cwd, terminalId, threadId, autoFocus]); |
Contributor
There was a problem hiding this comment.
🟡 Medium
components/ThreadTerminalDrawer.tsx:371 Adding autoFocus to the dependency array causes the terminal to be fully disposed and recreated when switching tabs, losing scroll position and buffer content. Consider moving the auto-focus logic to the existing separate effect (lines 373-383) that already handles focusRequestId, and removing autoFocus from this effect's dependencies.
Suggested change
| }, [api, cwd, terminalId, threadId, autoFocus]); | |
| }, [api, cwd, terminalId, threadId]); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/web/src/components/ThreadTerminalDrawer.tsx around line 371:
Adding `autoFocus` to the dependency array causes the terminal to be fully disposed and recreated when switching tabs, losing scroll position and buffer content. Consider moving the auto-focus logic to the existing separate effect (lines 373-383) that already handles `focusRequestId`, and removing `autoFocus` from this effect's dependencies.
Evidence trail:
Viewed `apps/web/src/components/ThreadTerminalDrawer.tsx:250-371` at commit `6a8c803` (useEffect creates terminal, cleanup disposes terminal; dependency array includes `autoFocus`). Viewed `apps/web/src/components/ThreadTerminalDrawer.tsx:730-777` at commit `6a8c803` (autoFocus derived from active terminalId, changes when switching tabs).
Comment on lines
+409
to
+435
| const script = ` | ||
| (() => { | ||
| const key = "t3code:renderer-state:v7"; | ||
| localStorage.setItem(key, JSON.stringify(${JSON.stringify(state)})); |
Contributor
There was a problem hiding this comment.
🟢 Low
src/desktopPerfAutomation.ts:409 If state contains U+2028 or U+2029 characters, JSON.stringify won't escape them, causing a syntax error in the injected script. Consider escaping these characters after stringifying (e.g., replace \u2028 and \u2029 with their escape sequences).
Suggested change
| localStorage.setItem(key, JSON.stringify(${JSON.stringify(state)})); | |
| localStorage.setItem(key, JSON.stringify(${JSON.stringify(state).replace(/\u2028/g, "\\u2028").replace(/\u2029/g, "\\u2029")})); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/desktopPerfAutomation.ts around line 409:
If `state` contains U+2028 or U+2029 characters, `JSON.stringify` won't escape them, causing a syntax error in the injected script. Consider escaping these characters after stringifying (e.g., replace `\u2028` and `\u2029` with their escape sequences).
Evidence trail:
Viewed `apps/desktop/src/desktopPerfAutomation.ts` around `seedRendererState` at commit `6a8c803d` (lines ~395-420) showing `localStorage.setItem(key, JSON.stringify(${JSON.stringify(state)}));` and no post-escape handling.
| await delay(120); | ||
|
|
||
| await focusActiveTerminalInput(window); | ||
| await sendTextInput(window.webContents, `touch ${markerFilePath}`); |
Contributor
There was a problem hiding this comment.
🟢 Low
src/desktopPerfAutomation.ts:793 The touch ${markerFilePath} command will fail on Windows (no touch utility) and with paths containing spaces (unquoted). Consider using a cross-platform approach and quoting the path.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/desktopPerfAutomation.ts around line 793:
The `touch ${markerFilePath}` command will fail on Windows (no `touch` utility) and with paths containing spaces (unquoted). Consider using a cross-platform approach and quoting the path.
Evidence trail:
Viewed `apps/desktop/src/desktopPerfAutomation.ts:783-806` at commit `1478184` in https://github.com/pingdotgg/codething-mvp/tree/1478184decc6c8c6eab45395c173f4d1a26041ff
This comment has been minimized.
This comment has been minimized.
Comment on lines
+232
to
+238
| const deltaMs = typeof stat.deltaMs === "number" ? stat.deltaMs : "n/a"; | ||
| const deltaPct = typeof stat.deltaPct === "number" ? stat.deltaPct : "n/a"; | ||
| const followUpRange = | ||
| typeof followUpMinMs === "number" && typeof followUpMaxMs === "number" | ||
| ? `${followUpMinMs}-${followUpMaxMs} (${followUpSampleCount}x)` | ||
| : "n/a"; | ||
| return `| ${pad(threadShort, THREAD_COL_WIDTH)} | ${pad(titleShort, TITLE_COL_WIDTH)} | ${stat.messageCount} | ${firstRenderMs} | ${followUpRenderMs} | ${followUpRange} | ${deltaMs} | ${deltaPct}% |`; |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:232 Suggestion: Make formatThreadRenderRow consistent about numeric validation. Add a fallback for stat.messageCount, and only build followUpRange when followUpMinMs, followUpMaxMs, and followUpSampleCount are numbers; otherwise show n/a.
- const deltaMs = typeof stat.deltaMs === "number" ? stat.deltaMs : "n/a";
+ const deltaMs = typeof stat.deltaMs === "number" ? stat.deltaMs : "n/a";
+ const messageCount = typeof stat.messageCount === "number" ? stat.messageCount : "n/a";
const deltaPct = typeof stat.deltaPct === "number" ? stat.deltaPct : "n/a";
const followUpRange =
typeof followUpMinMs === "number" && typeof followUpMaxMs === "number"
? `${followUpMinMs}-${followUpMaxMs} (${followUpSampleCount}x)`
: "n/a";
- return `| ${pad(threadShort, THREAD_COL_WIDTH)} | ${pad(titleShort, TITLE_COL_WIDTH)} | ${stat.messageCount} | ${firstRenderMs} | ${followUpRenderMs} | ${followUpRange} | ${deltaMs} | ${deltaPct}% |`;
+ return `| ${pad(threadShort, THREAD_COL_WIDTH)} | ${pad(titleShort, TITLE_COL_WIDTH)} | ${messageCount} | ${firstRenderMs} | ${followUpRenderMs} | ${followUpRange} | ${deltaMs} | ${deltaPct}% |`;🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 232-238:
Suggestion: Make `formatThreadRenderRow` consistent about numeric validation. Add a fallback for `stat.messageCount`, and only build `followUpRange` when `followUpMinMs`, `followUpMaxMs`, and `followUpSampleCount` are numbers; otherwise show `n/a`.
Evidence trail:
Viewed `scripts/desktop-perf-trace.mjs:210-255` at commit `ce27526` in https://github.com/pingdotgg/codething-mvp/tree/ce27526da9fb63da44671b8e5e4655577e171ea9.
| resolve(value); | ||
| }; | ||
|
|
||
| socket.setTimeout(timeoutMs); |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/dev-electron.mjs:21 socket.setTimeout() sets an inactivity timeout, not a connection timeout. Consider wrapping the entire operation in a setTimeout with finish(false) to guarantee the function resolves within timeoutMs.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/scripts/dev-electron.mjs around line 21:
`socket.setTimeout()` sets an inactivity timeout, not a connection timeout. Consider wrapping the entire operation in a `setTimeout` with `finish(false)` to guarantee the function resolves within `timeoutMs`.
Evidence trail:
Viewed `apps/desktop/scripts/dev-electron.mjs:11-26` at `ce27526` (shows `socket.setTimeout(timeoutMs);`).
- add automated Electron renderer perf tracing with seeded state and scripted UI interactions - add `test:desktop-perf` and `test:desktop-perf:post` scripts to generate, validate, and summarize traces - run desktop perf trace in CI and post/update PR perf summaries on pull requests
- Move seeding, scripted UI interactions, and trace recording into `desktopPerfAutomation.ts` - Keep `main.ts` lightweight with lazy module loading and explicit load-failure reporting
- Run desktop perf tests on forked pull requests without posting PR summaries - Restrict PR summary posting to same-repo pull requests - Add timeout-wrapped trace stop logic and guarded cleanup to avoid hangs
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
This reverts commit d5cba5a.
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
Co-authored-by: Julius Marminge <juliusmarminge@users.noreply.github.com>
juliusmarminge
force-pushed
the
codething/b1a7277c
branch
from
February 18, 2026 03:53
19f1569 to
273a84c
Compare
Comment on lines
+139
to
+145
| const normalized = Math.trunc(resolved.parsed); | ||
| if (normalized !== resolved.parsed) { | ||
| console.warn( | ||
| `[desktop-perf] non-integer ${name}="${resolved.raw}" truncated to ${normalized} for consistency`, | ||
| ); | ||
| } | ||
| return normalized; |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:139 resolvePositiveIntegerEnv checks parsed > 0 before truncating, so "0.5" passes validation but becomes 0. Consider validating the truncated value instead.
Suggested change
| const normalized = Math.trunc(resolved.parsed); | |
| if (normalized !== resolved.parsed) { | |
| console.warn( | |
| `[desktop-perf] non-integer ${name}="${resolved.raw}" truncated to ${normalized} for consistency`, | |
| ); | |
| } | |
| return normalized; | |
| const normalized = Math.trunc(resolved.parsed); | |
| if (normalized !== resolved.parsed) { | |
| console.warn( | |
| `[desktop-perf] non-integer ${name}="${resolved.raw}" truncated to ${normalized} for consistency`, | |
| ); | |
| } | |
| if (normalized <= 0) { | |
| console.warn( | |
| `[desktop-perf] invalid ${name}="${resolved.raw}" (expected positive integer); using ${fallback}`, | |
| ); | |
| return fallback; | |
| } | |
| return normalized; |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 139-145:
`resolvePositiveIntegerEnv` checks `parsed > 0` before truncating, so `"0.5"` passes validation but becomes `0`. Consider validating the truncated value instead.
Evidence trail:
Viewed `scripts/desktop-perf-trace.mjs` lines 60-130 at commit `19f1569` (resolvePositiveIntegerEnv uses predicate `parsed > 0` then `Math.trunc`).
| } | ||
|
|
||
| const elapsedMs = Date.now() - startedAt; | ||
| if (elapsedMs >= timeoutMs) { |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/dev-electron.mjs:91 If T3CODE_ELECTRON_STARTUP_TIMEOUT_MS is a non-numeric string, STARTUP_TIMEOUT_MS becomes NaN, causing the timeout check to never trigger (comparisons with NaN are always false). Consider validating the timeout value or using Number.isNaN() to guard against infinite polling.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/scripts/dev-electron.mjs around line 91:
If `T3CODE_ELECTRON_STARTUP_TIMEOUT_MS` is a non-numeric string, `STARTUP_TIMEOUT_MS` becomes `NaN`, causing the timeout check to never trigger (comparisons with `NaN` are always `false`). Consider validating the timeout value or using `Number.isNaN()` to guard against infinite polling.
Evidence trail:
Viewed `apps/desktop/scripts/dev-electron.mjs:1-8` and `apps/desktop/scripts/dev-electron.mjs:40-115` and `apps/desktop/scripts/dev-electron.mjs:124-190` at commit `19f1569` in https://github.com/pingdotgg/codething-mvp/tree/19f15693b75c02d9a2d0bdb1cc002629526804c2.
| } | ||
| } | ||
|
|
||
| function materializeRuntimeSeed(seedPath, artifactsDir) { |
Contributor
There was a problem hiding this comment.
🟢 Low
scripts/desktop-perf-trace.mjs:597 Consider checking that seedPath is a file before reading it, since resolveSeedPath uses fs.existsSync() which passes for directories too. A directory path like --seed=/tmp would fail here with a confusing EISDIR error.
-function materializeRuntimeSeed(seedPath, artifactsDir) {
+function materializeRuntimeSeed(seedPath, artifactsDir) {
+ if (!fs.statSync(seedPath).isFile()) {
+ throw new Error(`Seed path is not a file: ${seedPath}`);
+ }🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around line 597:
Consider checking that `seedPath` is a file before reading it, since `resolveSeedPath` uses `fs.existsSync()` which passes for directories too. A directory path like `--seed=/tmp` would fail here with a confusing `EISDIR` error.
Evidence trail:
Viewed `scripts/desktop-perf-trace.mjs` around `resolveSeedPath` and `materializeRuntimeSeed` at `19f1569` (lines ~545-610) in https://github.com/pingdotgg/codething-mvp/tree/19f15693b75c02d9a2d0bdb1cc002629526804c2.
Contributor
Desktop Dev Perf Trace
Detailed trace metrics and diagnosticsRun Metadata
Interaction Run
Threshold Config
Benchmark Thread Render
Input Event Metrics
Scheduler/Event Hotspots
Threshold Check
Heap Counters
Top User Timing Marks
Top Duration Events
|
Comment on lines
+73
to
+83
| function rejectUpgrade(socket: Duplex, statusCode: number, message: string): void { | ||
| socket.write( | ||
| `HTTP/1.1 ${statusCode} ${statusCode === 401 ? "Unauthorized" : "Bad Request"}\r\n` + | ||
| "Connection: close\r\n" + | ||
| "Content-Type: text/plain\r\n" + | ||
| `Content-Length: ${Buffer.byteLength(message)}\r\n` + | ||
| "\r\n" + | ||
| message, | ||
| ); | ||
| socket.destroy(); | ||
| } |
Contributor
There was a problem hiding this comment.
🟠 High
src/wsServer.ts:73 Consider adding an 'error' listener to the socket before writing, otherwise an abrupt client disconnect could crash the process with an uncaught exception.
Suggested change
| function rejectUpgrade(socket: Duplex, statusCode: number, message: string): void { | |
| socket.write( | |
| `HTTP/1.1 ${statusCode} ${statusCode === 401 ? "Unauthorized" : "Bad Request"}\r\n` + | |
| "Connection: close\r\n" + | |
| "Content-Type: text/plain\r\n" + | |
| `Content-Length: ${Buffer.byteLength(message)}\r\n` + | |
| "\r\n" + | |
| message, | |
| ); | |
| socket.destroy(); | |
| } | |
| function rejectUpgrade(socket: Duplex, statusCode: number, message: string): void { | |
| socket.on("error", () => {}); | |
| socket.write( | |
| `HTTP/1.1 ${statusCode} ${statusCode === 401 ? "Unauthorized" : "Bad Request"}\r\n` + | |
| "Connection: close\r\n" + | |
| "Content-Type: text/plain\r\n" + | |
| `Content-Length: ${Buffer.byteLength(message)}\r\n` + | |
| "\r\n" + | |
| message, | |
| ); | |
| socket.destroy(); | |
| } |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/server/src/wsServer.ts around lines 73-83:
Consider adding an `'error'` listener to the socket before writing, otherwise an abrupt client disconnect could crash the process with an uncaught exception.
Evidence trail:
Viewed `apps/server/src/wsServer.ts:55-86` and `apps/server/src/wsServer.ts:214-246` at commit `273a84c` (https://github.com/pingdotgg/codething-mvp/tree/273a84ce0a155acb5170e3e3f7da838c3eaac6b0). Searched for `socket.on('error')` in `apps/server/src/wsServer.ts` (git_grep at `273a84c`).
| if (isDevelopment) { | ||
| void window.loadURL(process.env.VITE_DEV_SERVER_URL as string); | ||
| window.webContents.openDevTools({ mode: "detach" }); | ||
| const devUrl = new URL(process.env.VITE_DEV_SERVER_URL as string); |
Contributor
There was a problem hiding this comment.
🟢 Low
src/main.ts:259 new URL(process.env.VITE_DEV_SERVER_URL) throws synchronously for malformed URLs (e.g., missing protocol). If this is intentional to fail fast in dev mode, consider adding a comment; otherwise, consider wrapping in try-catch.
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file apps/desktop/src/main.ts around line 259:
`new URL(process.env.VITE_DEV_SERVER_URL)` throws synchronously for malformed URLs (e.g., missing protocol). If this is intentional to fail fast in dev mode, consider adding a comment; otherwise, consider wrapping in try-catch.
Evidence trail:
Viewed `apps/desktop/src/main.ts` around line 259 (lines 252-268) at commit `273a84c` showing `const devUrl = new URL(process.env.VITE_DEV_SERVER_URL as string);`.
- Track seed project/thread counts and benchmark thread coverage in automation payloads - Add centralized threshold evaluation with PASS/FAIL status and actionable markdown summary - Require default desktop perf seed file and enable benchmark sweep/follow-up env defaults
Comment on lines
+477
to
+478
| if (child.exitCode !== null) { | ||
| throw new Error(`desktop dev process exited early (code ${child.exitCode})`); |
Contributor
There was a problem hiding this comment.
🟡 Medium
scripts/desktop-perf-trace.mjs:477 Race condition: if the child writes the done file and exits between fs.existsSync and the exitCode check, this throws incorrectly. Consider re-checking for the done file after detecting exit.
Suggested change
| if (child.exitCode !== null) { | |
| throw new Error(`desktop dev process exited early (code ${child.exitCode})`); | |
| if (child.exitCode !== null) { | |
| if (fs.existsSync(donePath)) { | |
| return JSON.parse(fs.readFileSync(donePath, "utf8")); | |
| } | |
| throw new Error(`desktop dev process exited early (code ${child.exitCode})`); |
🚀 Want me to fix this? Reply ex: "fix it for me".
🤖 Prompt for AI
In file scripts/desktop-perf-trace.mjs around lines 477-478:
Race condition: if the child writes the done file and exits between `fs.existsSync` and the `exitCode` check, this throws incorrectly. Consider re-checking for the done file after detecting exit.
Evidence trail:
Viewed `scripts/desktop-perf-trace.mjs:468-486` at `b0eef5c` (function waitForDoneFile with existsSync then exitCode check).
| import waitOn from "wait-on"; | ||
| import fs from "node:fs"; | ||
| import net from "node:net"; | ||
| import path from "node:path"; |
Contributor
There was a problem hiding this comment.
Unused wait-on dependency after import removal
Low Severity
The import waitOn from "wait-on" was the sole usage of the wait-on package. This diff replaces it with custom waiting logic but leaves wait-on as a devDependency in apps/desktop/package.json. The package (which pulls in axios, joi, lodash, rxjs) is now dead weight in the dependency tree.
Additional Locations (1)
Contributor
|
Bugbot Autofix prepared fixes for 1 of the 1 bugs found in the latest run.
Or push these changes by commenting: Preview (9214539288)diff --git a/apps/desktop/package.json b/apps/desktop/package.json
--- a/apps/desktop/package.json
+++ b/apps/desktop/package.json
@@ -22,7 +22,6 @@
"electronmon": "^2.0.2",
"tsdown": "^0.20.3",
"typescript": "^5.7.3",
- "vitest": "^4.0.0",
- "wait-on": "^8.0.2"
+ "vitest": "^4.0.0"
}
}
diff --git a/bun.lock b/bun.lock
--- a/bun.lock
+++ b/bun.lock
@@ -24,7 +24,6 @@
"tsdown": "^0.20.3",
"typescript": "^5.7.3",
"vitest": "^4.0.0",
- "wait-on": "^8.0.2",
},
},
"apps/server": {
@@ -205,18 +204,6 @@
"@floating-ui/utils": ["@floating-ui/utils@0.2.10", "", {}, "sha512-aGTxbpbg8/b5JfU1HXSrbH3wXZuLPJcNEcZQFMxLs3oSzgtVu6nFPkbbGGUvBcUjKV2YyB9Wxxabo+HEH9tcRQ=="],
- "@hapi/address": ["@hapi/address@5.1.1", "", { "dependencies": { "@hapi/hoek": "^11.0.2" } }, "sha512-A+po2d/dVoY7cYajycYI43ZbYMXukuopIsqCjh5QzsBCipDtdofHntljDlpccMjIfTy6UOkg+5KPriwYch2bXA=="],
-
- "@hapi/formula": ["@hapi/formula@3.0.2", "", {}, "sha512-hY5YPNXzw1He7s0iqkRQi+uMGh383CGdyyIGYtB+W5N3KHPXoqychklvHhKCC9M3Xtv0OCs/IHw+r4dcHtBYWw=="],
-
- "@hapi/hoek": ["@hapi/hoek@11.0.7", "", {}, "sha512-HV5undWkKzcB4RZUusqOpcgxOaq6VOAH7zhhIr2g3G8NF/MlFO75SjOr2NfuSx0Mh40+1FqCkagKLJRykUWoFQ=="],
-
- "@hapi/pinpoint": ["@hapi/pinpoint@2.0.1", "", {}, "sha512-EKQmr16tM8s16vTT3cA5L0kZZcTMU5DUOZTuvpnY738m+jyP3JIUj+Mm1xc1rsLkGBQ/gVnfKYPwOmPg1tUR4Q=="],
-
- "@hapi/tlds": ["@hapi/tlds@1.1.6", "", {}, "sha512-xdi7A/4NZokvV0ewovme3aUO5kQhW9pQ2YD1hRqZGhhSi5rBv4usHYidVocXSi9eihYsznZxLtAiEYYUL6VBGw=="],
-
- "@hapi/topo": ["@hapi/topo@6.0.2", "", { "dependencies": { "@hapi/hoek": "^11.0.2" } }, "sha512-KR3rD5inZbGMrHmgPxsJ9dbi6zEK+C3ZwUwTa+eMwWLz7oijWUTWD2pMSNNYJAU6Qq+65NkxXjqHr/7LM2Xkqg=="],
-
"@jridgewell/gen-mapping": ["@jridgewell/gen-mapping@0.3.13", "", { "dependencies": { "@jridgewell/sourcemap-codec": "^1.5.0", "@jridgewell/trace-mapping": "^0.3.24" } }, "sha512-2kkt/7niJ6MgEPxF0bYdQ6etZaA+fQvDcLKckhy1yIQOzaoKjBBjSj63/aLVjYE3qhRt5dvM+uUyfCg6UKCBbA=="],
"@jridgewell/remapping": ["@jridgewell/remapping@2.3.5", "", { "dependencies": { "@jridgewell/gen-mapping": "^0.3.5", "@jridgewell/trace-mapping": "^0.3.24" } }, "sha512-LI9u/+laYG4Ds1TDKSJW2YPrIlcVYOwi2fUC6xB43lueCjgxV4lffOCZCtYFiH6TNOX+tQKXx97T4IKHbhyHEQ=="],
@@ -505,10 +492,6 @@
"ast-kit": ["ast-kit@3.0.0-beta.1", "", { "dependencies": { "@babel/parser": "^8.0.0-beta.4", "estree-walker": "^3.0.3", "pathe": "^2.0.3" } }, "sha512-trmleAnZ2PxN/loHWVhhx1qeOHSRXq4TDsBBxq3GqeJitfk3+jTQ+v/C1km/KYq9M7wKqCewMh+/NAvVH7m+bw=="],
- "asynckit": ["asynckit@0.4.0", "", {}, "sha512-Oei9OH4tRh0YqU3GxhX79dM/mwVgvbZJaSNaRk+bshkj0S5cfHcgYakreBjrHwatXKbz+IoIdYLxrKim2MjW0Q=="],
-
- "axios": ["axios@1.13.5", "", { "dependencies": { "follow-redirects": "^1.15.11", "form-data": "^4.0.5", "proxy-from-env": "^1.1.0" } }, "sha512-cz4ur7Vb0xS4/KUN0tPWe44eqxrIu31me+fbang3ijiNscE129POzipJJA6zniq2C/Z6sJCjMimjS8Lc/GAs8Q=="],
-
"babel-plugin-react-compiler": ["babel-plugin-react-compiler@19.0.0-beta-ebf51a3-20250411", "", { "dependencies": { "@babel/types": "^7.26.0" } }, "sha512-q84bNR9JG1crykAlJUt5Ud0/5BUyMFuQww/mrwIQDFBaxsikqBDj3f/FNDsVd2iR26A1HvXKWPEIfgJDv8/V2g=="],
"bail": ["bail@2.0.2", "", {}, "sha512-0xO6mYd7JB2YesxDKplafRpsiOzPt9V02ddPCLbY1xYGPOX24NTyN50qnUxgCPcSoYMhKpAuBTjQoRZCAkUDRw=="],
@@ -533,8 +516,6 @@
"cacheable-request": ["cacheable-request@7.0.4", "", { "dependencies": { "clone-response": "^1.0.2", "get-stream": "^5.1.0", "http-cache-semantics": "^4.0.0", "keyv": "^4.0.0", "lowercase-keys": "^2.0.0", "normalize-url": "^6.0.1", "responselike": "^2.0.0" } }, "sha512-v+p6ongsrp0yTGbJXjgxPow2+DL93DASP4kXCDKb8/bwRtt9OEF3whggkkDkGNzgcWy2XaF4a8nZglC7uElscg=="],
- "call-bind-apply-helpers": ["call-bind-apply-helpers@1.0.2", "", { "dependencies": { "es-errors": "^1.3.0", "function-bind": "^1.1.2" } }, "sha512-Sp1ablJ0ivDkSzjcaJdxEunN5/XvksFJ2sMBFfq6x0ryhQV/2b/KwFe21cMpmHtPOSij8K99/wSfoEuTObmuMQ=="],
-
"caniuse-lite": ["caniuse-lite@1.0.30001770", "", {}, "sha512-x/2CLQ1jHENRbHg5PSId2sXq1CIO1CISvwWAj027ltMVG2UNgW+w9oH2+HzgEIRFembL8bUlXtfbBHR1fCg2xw=="],
"ccount": ["ccount@2.0.1", "", {}, "sha512-eyrF0jiFpY+3drT6383f1qhkbGsLSifNAjA61IUjZjmLCWjItY6LB9ft9YhoDgwfmclB2zhu51Lc7+95b8NRAg=="],
@@ -561,8 +542,6 @@
"color-name": ["color-name@1.1.4", "", {}, "sha512-dOy+3AuW3a2wNbZHIuMZpTcgjGuLU/uBL/ubcZF9OXbDo8ff4O8yVp5Bf0efS8uEoYo5q4Fx7dY9OgQGXgAsQA=="],
- "combined-stream": ["combined-stream@1.0.8", "", { "dependencies": { "delayed-stream": "~1.0.0" } }, "sha512-FQN4MRfuJeHf7cBbBMJFXhKSDq+2kAArBlmRBvcvFE5BB1HZKXtSFASDhdlz9zOYwxh8lDdnvmMOe/+5cdoEdg=="],
-
"comma-separated-tokens": ["comma-separated-tokens@2.0.3", "", {}, "sha512-Fu4hJdvzeylCfQPp9SGWidpzrMs7tTrlu6Vb8XGaRGck8QSNZJJp538Wrb60Lax4fPwR64ViY468OIUTbRlGZg=="],
"convert-source-map": ["convert-source-map@2.0.0", "", {}, "sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg=="],
@@ -589,8 +568,6 @@
"defu": ["defu@6.1.4", "", {}, "sha512-mEQCMmwJu317oSz8CwdIOdwf3xMif1ttiM8LTufzc3g6kR+9Pe236twL8j3IYT1F7GfRgGcW6MWxzZjLIkuHIg=="],
- "delayed-stream": ["delayed-stream@1.0.0", "", {}, "sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ=="],
-
"dequal": ["dequal@2.0.3", "", {}, "sha512-0je+qPKHEMohvfRTCEo3CrPG6cAzAYgmzKyxRiYSSDkS6eGJdyVJm7WaYA5ECaAD9wLB2T4EEeymA5aFVcYXCA=="],
"detect-libc": ["detect-libc@2.1.2", "", {}, "sha512-Btj2BOOO83o3WyH59e8MgXsxEQVcarkUOpEYrubB0urwnN10yQ364rsiByU11nZlqWYZm05i/of7io4mzihBtQ=="],
@@ -601,8 +578,6 @@
"dts-resolver": ["dts-resolver@2.1.3", "", { "peerDependencies": { "oxc-resolver": ">=11.0.0" }, "optionalPeers": ["oxc-resolver"] }, "sha512-bihc7jPC90VrosXNzK0LTE2cuLP6jr0Ro8jk+kMugHReJVLIpHz/xadeq3MhuwyO4TD4OA3L1Q8pBBFRc08Tsw=="],
- "dunder-proto": ["dunder-proto@1.0.1", "", { "dependencies": { "call-bind-apply-helpers": "^1.0.1", "es-errors": "^1.3.0", "gopd": "^1.2.0" } }, "sha512-KIN/nDJBQRcXw0MLVhZE9iQHmG68qAVIBg9CqmUYjmQIhgij9U5MFvrqkUL5FbtyyzZuOeOt0zdeRe4UY7ct+A=="],
-
"electron": ["electron@33.4.11", "", { "dependencies": { "@electron/get": "^2.0.0", "@types/node": "^20.9.0", "extract-zip": "^2.0.1" }, "bin": { "electron": "cli.js" } }, "sha512-xmdAs5QWRkInC7TpXGNvzo/7exojubk+72jn1oJL7keNeIlw7xNglf8TGtJtkR4rWC5FJq0oXiIXPS9BcK2Irg=="],
"electron-to-chromium": ["electron-to-chromium@1.5.286", "", {}, "sha512-9tfDXhJ4RKFNerfjdCcZfufu49vg620741MNs26a9+bhLThdB+plgMeou98CAaHu/WATj2iHOOHTp1hWtABj2A=="],
@@ -623,10 +598,6 @@
"es-module-lexer": ["es-module-lexer@1.7.0", "", {}, "sha512-jEQoCwk8hyb2AZziIOLhDqpm5+2ww5uIE6lkO/6jcOCusfk6LhMHpXXfBLXTZ7Ydyt0j4VoUQv6uGNYbdW+kBA=="],
- "es-object-atoms": ["es-object-atoms@1.1.1", "", { "dependencies": { "es-errors": "^1.3.0" } }, "sha512-FGgH2h8zKNim9ljj7dankFPcICIK9Cp5bm+c2gQSYePhpaG5+esrLODihIorn+Pe6FGJzWhXQotPv73jTaldXA=="],
-
- "es-set-tostringtag": ["es-set-tostringtag@2.1.0", "", { "dependencies": { "es-errors": "^1.3.0", "get-intrinsic": "^1.2.6", "has-tostringtag": "^1.0.2", "hasown": "^2.0.2" } }, "sha512-j6vWzfrGVfyXxge+O0x5sh6cvxAog0a/4Rdd2K36zCMV5eJ+/+tOAngRO8cODMNWbVRdVlmGZQL2YS3yR8bIUA=="],
-
"es6-error": ["es6-error@4.1.1", "", {}, "sha512-Um/+FxMr9CISWh0bi5Zv0iOD+4cFh5qLeks1qhAopKVAJw3drgKbKySikp7wGhDL0HPeaja0P5ULZrxLkniUVg=="],
"esbuild": ["esbuild@0.27.3", "", { "optionalDependencies": { "@esbuild/aix-ppc64": "0.27.3", "@esbuild/android-arm": "0.27.3", "@esbuild/android-arm64": "0.27.3", "@esbuild/android-x64": "0.27.3", "@esbuild/darwin-arm64": "0.27.3", "@esbuild/darwin-x64": "0.27.3", "@esbuild/freebsd-arm64": "0.27.3", "@esbuild/freebsd-x64": "0.27.3", "@esbuild/linux-arm": "0.27.3", "@esbuild/linux-arm64": "0.27.3", "@esbuild/linux-ia32": "0.27.3", "@esbuild/linux-loong64": "0.27.3", "@esbuild/linux-mips64el": "0.27.3", "@esbuild/linux-ppc64": "0.27.3", "@esbuild/linux-riscv64": "0.27.3", "@esbuild/linux-s390x": "0.27.3", "@esbuild/linux-x64": "0.27.3", "@esbuild/netbsd-arm64": "0.27.3", "@esbuild/netbsd-x64": "0.27.3", "@esbuild/openbsd-arm64": "0.27.3", "@esbuild/openbsd-x64": "0.27.3", "@esbuild/openharmony-arm64": "0.27.3", "@esbuild/sunos-x64": "0.27.3", "@esbuild/win32-arm64": "0.27.3", "@esbuild/win32-ia32": "0.27.3", "@esbuild/win32-x64": "0.27.3" }, "bin": { "esbuild": "bin/esbuild" } }, "sha512-8VwMnyGCONIs6cWue2IdpHxHnAjzxnw2Zr7MkVxB2vjmQ2ivqGFb4LEG3SMnv0Gb2F/G/2yA8zUaiL1gywDCCg=="],
@@ -651,22 +622,12 @@
"fill-range": ["fill-range@7.1.1", "", { "dependencies": { "to-regex-range": "^5.0.1" } }, "sha512-YsGpe3WHLK8ZYi4tWDg2Jy3ebRz2rXowDxnld4bkQB00cc/1Zw9AWnC0i9ztDJitivtQvaI9KaLyKrc+hBW0yg=="],
- "follow-redirects": ["follow-redirects@1.15.11", "", {}, "sha512-deG2P0JfjrTxl50XGCDyfI97ZGVCxIpfKYmfyrQ54n5FO/0gfIES8C/Psl6kWVDolizcaaxZJnTS0QSMxvnsBQ=="],
-
- "form-data": ["form-data@4.0.5", "", { "dependencies": { "asynckit": "^0.4.0", "combined-stream": "^1.0.8", "es-set-tostringtag": "^2.1.0", "hasown": "^2.0.2", "mime-types": "^2.1.12" } }, "sha512-8RipRLol37bNs2bhoV67fiTEvdTrbMUYcFTiy3+wuuOnUog2QBHCZWXDRijWQfAkhBj2Uf5UnVaiWwA5vdd82w=="],
-
"fs-extra": ["fs-extra@8.1.0", "", { "dependencies": { "graceful-fs": "^4.2.0", "jsonfile": "^4.0.0", "universalify": "^0.1.0" } }, "sha512-yhlQgA6mnOJUKOsRUFsgJdQCvkKhcz8tlZG5HBQfReYZy46OwLcY+Zia0mtdHsOo9y/hP+CxMN0TU9QxoOtG4g=="],
"fsevents": ["fsevents@2.3.3", "", { "os": "darwin" }, "sha512-5xoDfX+fL7faATnagmWPpbFtwh/R77WmMMqqHGS65C3vvB0YHrgF+B1YmZ3441tMj5n63k0212XNoJwzlhffQw=="],
- "function-bind": ["function-bind@1.1.2", "", {}, "sha512-7XHNxH7qX9xG5mIwxkhumTox/MIRNcOgDrxWsMt2pAr23WHp6MrRlN7FBSFpCpr+oVO0F744iUgR82nJMfG2SA=="],
-
"gensync": ["gensync@1.0.0-beta.2", "", {}, "sha512-3hN7NaskYvMDLQY55gnW3NQ+mesEAepTqlg+VEbj7zzqEMBVNhzcGYYeqFo/TlYz6eQiFcp1HcsCZO+nGgS8zg=="],
- "get-intrinsic": ["get-intrinsic@1.3.0", "", { "dependencies": { "call-bind-apply-helpers": "^1.0.2", "es-define-property": "^1.0.1", "es-errors": "^1.3.0", "es-object-atoms": "^1.1.1", "function-bind": "^1.1.2", "get-proto": "^1.0.1", "gopd": "^1.2.0", "has-symbols": "^1.1.0", "hasown": "^2.0.2", "math-intrinsics": "^1.1.0" } }, "sha512-9fSjSaos/fRIVIp+xSJlE6lfwhES7LNtKaCBIamHsjr2na1BiABJPo0mOjjz8GJDURarmCPGqaiVg5mfjb98CQ=="],
-
- "get-proto": ["get-proto@1.0.1", "", { "dependencies": { "dunder-proto": "^1.0.1", "es-object-atoms": "^1.0.0" } }, "sha512-sTSfBjoXBp89JvIKIefqw7U2CCebsc74kiY6awiGogKtoSGbgjYE/G/+l9sF3MWFPNc9IcoOC4ODfKHfxFmp0g=="],
-
"get-stream": ["get-stream@5.2.0", "", { "dependencies": { "pump": "^3.0.0" } }, "sha512-nBF+F1rAZVCu/p7rjzgA+Yb4lfYXrpl7a6VmJrU8wF9I1CKvP/QwPNZHnOlwbTkY6dvtFIzFMSyQXbLoTQPRpA=="],
"get-tsconfig": ["get-tsconfig@4.13.6", "", { "dependencies": { "resolve-pkg-maps": "^1.0.0" } }, "sha512-shZT/QMiSHc/YBLxxOkMtgSid5HFoauqCE3/exfsEcwg1WkeqjG+V40yBbBrsD+jW2HDXcs28xOfcbm2jI8Ddw=="],
@@ -685,12 +646,6 @@
"has-property-descriptors": ["has-property-descriptors@1.0.2", "", { "dependencies": { "es-define-property": "^1.0.0" } }, "sha512-55JNKuIW+vq4Ke1BjOTjM2YctQIvCT7GFzHwmfZPGo5wnrgkid0YQtnAleFSqumZm4az3n2BS+erby5ipJdgrg=="],
- "has-symbols": ["has-symbols@1.1.0", "", {}, "sha512-1cDNdwJ2Jaohmb3sg4OmKaMBwuC48sYni5HUw2DvsC8LjGTLK9h+eb1X6RyuOHe4hT0ULCW68iomhjUoKUqlPQ=="],
-
- "has-tostringtag": ["has-tostringtag@1.0.2", "", { "dependencies": { "has-symbols": "^1.0.3" } }, "sha512-NqADB8VjPFLM2V0VvHUewwwsw0ZWBaIdgo+ieHtK3hasLz4qeCRjYcqfB6AQrBggRKppKF8L52/VqdVsO47Dlw=="],
-
- "hasown": ["hasown@2.0.2", "", { "dependencies": { "function-bind": "^1.1.2" } }, "sha512-0hJU9SCPvmMzIBdZFqNPXWa6dqh7WdH0cII9y+CyS8rG3nL48Bclra9HmKhVVUHyPWNH5Y7xDwAB7bfgSjkUMQ=="],
-
"hast-util-is-element": ["hast-util-is-element@3.0.0", "", { "dependencies": { "@types/hast": "^3.0.0" } }, "sha512-Val9mnv2IWpLbNPqc/pUem+a7Ipj2aHacCwgNfTiK0vJKl0LF+4Ba4+v1oPHFpf3bLYmreq0/l3Gud9S5OH42g=="],
"hast-util-to-jsx-runtime": ["hast-util-to-jsx-runtime@2.3.6", "", { "dependencies": { "@types/estree": "^1.0.0", "@types/hast": "^3.0.0", "@types/unist": "^3.0.0", "comma-separated-tokens": "^2.0.0", "devlop": "^1.0.0", "estree-util-is-identifier-name": "^3.0.0", "hast-util-whitespace": "^3.0.0", "mdast-util-mdx-expression": "^2.0.0", "mdast-util-mdx-jsx": "^3.0.0", "mdast-util-mdxjs-esm": "^2.0.0", "property-information": "^7.0.0", "space-separated-tokens": "^2.0.0", "style-to-js": "^1.0.0", "unist-util-position": "^5.0.0", "vfile-message": "^4.0.0" } }, "sha512-zl6s8LwNyo1P9uw+XJGvZtdFF1GdAkOg8ujOw+4Pyb76874fLps4ueHXDhXWdk6YHQ6OgUtinliG7RsYvCbbBg=="],
@@ -735,8 +690,6 @@
"jiti": ["jiti@2.6.1", "", { "bin": { "jiti": "lib/jiti-cli.mjs" } }, "sha512-ekilCSN1jwRvIbgeg/57YFh8qQDNbwDb9xT/qu2DAHbFFZUicIl4ygVaAvzveMhMVr3LnpSKTNnwt8PoOfmKhQ=="],
- "joi": ["joi@18.0.2", "", { "dependencies": { "@hapi/address": "^5.1.1", "@hapi/formula": "^3.0.2", "@hapi/hoek": "^11.0.7", "@hapi/pinpoint": "^2.0.1", "@hapi/tlds": "^1.1.1", "@hapi/topo": "^6.0.2", "@standard-schema/spec": "^1.0.0" } }, "sha512-RuCOQMIt78LWnktPoeBL0GErkNaJPTBGcYuyaBvUOQSpcpcLfWrHPPihYdOGbV5pam9VTWbeoF7TsGiHugcjGA=="],
-
"js-tokens": ["js-tokens@4.0.0", "", {}, "sha512-RdJUflcE3cUzKiMqQgsCu06FPu9UdIJO0beYbPhHN4k6apgJtifcoCtT9bcxOpYBtpD2kCM6Sbzg4CausW/PKQ=="],
"jsesc": ["jsesc@3.1.0", "", { "bin": { "jsesc": "bin/jsesc" } }, "sha512-/sM3dO2FOzXjKQhJuo0Q173wf2KOo8t4I8vHy6lF9poUp7bKT0/NHE8fPX23PwfhnykfqnC2xRxOnVw5XuGIaA=="],
@@ -775,8 +728,6 @@
"lightningcss-win32-x64-msvc": ["lightningcss-win32-x64-msvc@1.31.1", "", { "os": "win32", "cpu": "x64" }, "sha512-I9aiFrbd7oYHwlnQDqr1Roz+fTz61oDDJX7n9tYF9FJymH1cIN1DtKw3iYt6b8WZgEjoNwVSncwF4wx/ZedMhw=="],
- "lodash": ["lodash@4.17.23", "", {}, "sha512-LgVTMpQtIopCi79SJeDiP0TfWi5CNEc/L/aRdTh3yIvmZXTnheWpKjSZhnvMl8iXbC1tFg9gdHHDMLoV7CnG+w=="],
-
"lodash.difference": ["lodash.difference@4.5.0", "", {}, "sha512-dS2j+W26TQ7taQBGN8Lbbq04ssV3emRw4NY58WErlTO29pIqS0HmoT5aJ9+TUQ1N3G+JOZSji4eugsWwGp9yPA=="],
"longest-streak": ["longest-streak@3.1.0", "", {}, "sha512-9Ri+o0JYgehTaVBBDoMqIl8GXtbWg711O3srftcHhZ0dqnETqLaoIK0x17fUw9rFSlK/0NlsKe0Ahhyl5pXE2g=="],
@@ -795,8 +746,6 @@
"matcher": ["matcher@3.0.0", "", { "dependencies": { "escape-string-regexp": "^4.0.0" } }, "sha512-OkeDaAZ/bQCxeFAozM55PKcKU0yJMPGifLwV4Qgjitu+5MoAfSQN4lsLJeXZ1b8w0x+/Emda6MZgXS1jvsapng=="],
- "math-intrinsics": ["math-intrinsics@1.1.0", "", {}, "sha512-/IXtbwEk5HTPyEwyKX6hGkYXxM9nbj64B+ilVJnC/R6B0pH5G4V3b0pVbL7DBj4tkhBAppbQUlf6F6Xl9LHu1g=="],
-
"mdast-util-find-and-replace": ["mdast-util-find-and-replace@3.0.2", "", { "dependencies": { "@types/mdast": "^4.0.0", "escape-string-regexp": "^5.0.0", "unist-util-is": "^6.0.0", "unist-util-visit-parents": "^6.0.0" } }, "sha512-Tmd1Vg/m3Xz43afeNxDIhWRtFZgM2VLyaf4vSTYwudTyeuTneoL3qtWMA5jeLyz/O1vDJmmV4QuScFCA2tBPwg=="],
"mdast-util-from-markdown": ["mdast-util-from-markdown@2.0.2", "", { "dependencies": { "@types/mdast": "^4.0.0", "@types/unist": "^3.0.0", "decode-named-character-reference": "^1.0.0", "devlop": "^1.0.0", "mdast-util-to-string": "^4.0.0", "micromark": "^4.0.0", "micromark-util-decode-numeric-character-reference": "^2.0.0", "micromark-util-decode-string": "^2.0.0", "micromark-util-normalize-identifier": "^2.0.0", "micromark-util-symbol": "^2.0.0", "micromark-util-types": "^2.0.0", "unist-util-stringify-position": "^4.0.0" } }, "sha512-uZhTV/8NBuw0WHkPTrCqDOl0zVe1BIng5ZtHoDk49ME1qqcjYmmLmOf0gELgcRMxN4w2iuIeVso5/6QymSrgmA=="],
@@ -885,14 +834,8 @@
"micromatch": ["micromatch@4.0.8", "", { "dependencies": { "braces": "^3.0.3", "picomatch": "^2.3.1" } }, "sha512-PXwfBhYu0hBCPw8Dn0E+WDYb7af3dSLVWKi3HGv84IdF4TyFoC0ysxFd0Goxw7nSv4T/PzEJQxsYsEiFCKo2BA=="],
- "mime-db": ["mime-db@1.52.0", "", {}, "sha512-sPU4uV7dYlvtWJxwwxHD0PuihVNiE7TyAbQ5SWxDCB9mUYvOgroQOwYQQOKPJ8CIbE+1ETVlOoK1UC2nU3gYvg=="],
-
- "mime-types": ["mime-types@2.1.35", "", { "dependencies": { "mime-db": "1.52.0" } }, "sha512-ZDY+bPm5zTTF+YpCrAU9nK0UgICYPT0QtT1NZWFv4s++TNkcgVaT0g6+4R2uI4MjQjzysHB1zxuWL50hzaeXiw=="],
-
"mimic-response": ["mimic-response@3.1.0", "", {}, "sha512-z0yWI+4FDrrweS8Zmt4Ej5HdJmky15+L2e6Wgn3+iK5fWzb6T3fhNFq2+MeTRb064c6Wr4N/wv0DzQTjNzHNGQ=="],
- "minimist": ["minimist@1.2.8", "", {}, "sha512-2yyAR8qBkN3YuheJanUpWC5U3bb5osDywNB8RzDVlDwDHbocAJveqqj1u8+SVD7jkWT4yvsHCpWqqWqAxb0zCA=="],
-
"ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="],
"nanoid": ["nanoid@3.3.11", "", { "bin": { "nanoid": "bin/nanoid.cjs" } }, "sha512-N8SpfPUnUp1bK+PMYW8qSWdl9U+wwNWI4QKxOYDy9JAro3WMX7p2OeVRF9v+347pnakNevPmiHhNmZ2HbFA76w=="],
@@ -939,8 +882,6 @@
"property-information": ["property-information@7.1.0", "", {}, "sha512-TwEZ+X+yCJmYfL7TPUOcvBZ4QfoT5YenQiJuX//0th53DE6w0xxLEtfK3iyryQFddXuvkIk51EEgrJQ0WJkOmQ=="],
- "proxy-from-env": ["proxy-from-env@1.1.0", "", {}, "sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg=="],
-
"pump": ["pump@3.0.3", "", { "dependencies": { "end-of-stream": "^1.1.0", "once": "^1.3.1" } }, "sha512-todwxLMY7/heScKmntwQG8CXVkWUOdYxIvY2s0VWAAMh/nd8SoYiRaKjlr7+iCs984f2P8zvrfWcDDYVb73NfA=="],
"quansync": ["quansync@1.0.0", "", {}, "sha512-5xZacEEufv3HSTPQuchrvV6soaiACMFnq1H8wkVioctoH3TRha9Sz66lOxRwPK/qZj7HPiSveih9yAyh98gvqA=="],
@@ -989,8 +930,6 @@
"runtime-required": ["runtime-required@1.1.0", "", {}, "sha512-yX97f5E0WfNpcQnfVjap6vzQcvErkYYCx6eTK4siqGEdC8lglwypUFgZVTX7ShvIlgfkC4XGFl9O1KTYcff0pw=="],
- "rxjs": ["rxjs@7.8.2", "", { "dependencies": { "tslib": "^2.1.0" } }, "sha512-dhKf903U/PQZY6boNNtAGdWbG85WAbjT/1xYoZIC7FAY0yWapOBQVsVrDl58W86//e1VpMNBtRV4MaXfdMySFA=="],
-
"scheduler": ["scheduler@0.27.0", "", {}, "sha512-eNv+WrVbKu1f3vbYJT/xtiF5syA5HPIMtf9IgY/nKg0sWqzAUEvqY/xm7OcZc/qafLx/iO9FgOmeSAp4v5ti/Q=="],
"semver": ["semver@7.7.4", "", { "bin": { "semver": "bin/semver.js" } }, "sha512-vFKC2IEtQnVhpT78h1Yp8wzwrf8CM+MzKMHGJZfBtzhZNycRFnXsHk6E5TxIkkMsgNS7mdX3AGB7x2QM2di4lA=="],
@@ -1107,8 +1046,6 @@
"vitest": ["vitest@4.0.18", "", { "dependencies": { "@vitest/expect": "4.0.18", "@vitest/mocker": "4.0.18", "@vitest/pretty-format": "4.0.18", "@vitest/runner": "4.0.18", "@vitest/snapshot": "4.0.18", "@vitest/spy": "4.0.18", "@vitest/utils": "4.0.18", "es-module-lexer": "^1.7.0", "expect-type": "^1.2.2", "magic-string": "^0.30.21", "obug": "^2.1.1", "pathe": "^2.0.3", "picomatch": "^4.0.3", "std-env": "^3.10.0", "tinybench": "^2.9.0", "tinyexec": "^1.0.2", "tinyglobby": "^0.2.15", "tinyrainbow": "^3.0.3", "vite": "^6.0.0 || ^7.0.0", "why-is-node-running": "^2.3.0" }, "peerDependencies": { "@edge-runtime/vm": "*", "@opentelemetry/api": "^1.9.0", "@types/node": "^20.0.0 || ^22.0.0 || >=24.0.0", "@vitest/browser-playwright": "4.0.18", "@vitest/browser-preview": "4.0.18", "@vitest/browser-webdriverio": "4.0.18", "@vitest/ui": "4.0.18", "happy-dom": "*", "jsdom": "*" }, "optionalPeers": ["@edge-runtime/vm", "@opentelemetry/api", "@types/node", "@vitest/browser-playwright", "@vitest/browser-preview", "@vitest/browser-webdriverio", "@vitest/ui", "happy-dom", "jsdom"], "bin": { "vitest": "vitest.mjs" } }, "sha512-hOQuK7h0FGKgBAas7v0mSAsnvrIgAvWmRFjmzpJ7SwFHH3g1k2u37JtYwOwmEKhK6ZO3v9ggDBBm0La1LCK4uQ=="],
- "wait-on": ["wait-on@8.0.5", "", { "dependencies": { "axios": "^1.12.1", "joi": "^18.0.1", "lodash": "^4.17.21", "minimist": "^1.2.8", "rxjs": "^7.8.2" }, "bin": { "wait-on": "bin/wait-on" } }, "sha512-J3WlS0txVHkhLRb2FsmRg3dkMTCV1+M6Xra3Ho7HzZDHpE7DCOnoSoCJsZotrmW3uRMhvIJGSKUKrh/MeF4iag=="],
-
"watchboy": ["watchboy@0.4.3", "", { "dependencies": { "lodash.difference": "^4.5.0", "micromatch": "^4.0.2", "pify": "^4.0.1", "unixify": "^1.0.0" } }, "sha512-GHs1HxwvxSMBsqd/WfTOZhj5gBdMqf5HQpfgtKxDfZRxrlYPDdVLRB61LCeRzJaWANmvSIMlfmRVDwVmJFgAKA=="],
"why-is-node-running": ["why-is-node-running@2.3.0", "", { "dependencies": { "siginfo": "^2.0.0", "stackback": "0.0.2" }, "bin": { "why-is-node-running": "cli.js" } }, "sha512-hUrmaWBdVDcxvYqnyh09zunKzROWjbZTiNy8dBEjkS7ehEDQibXJ7XvlmtbwuTclUiIyN+CyXQD4Vmko8fNm8w=="], |
github-actions
Bot
added
the
vouch:trusted
Member
Author
|
Closing my own stale experiment during PR triage. |
aorwall
added a commit
to aorwall/t3code
that referenced
this pull request
Aug 6, 2026
…ingdotgg#59) Rendering nginx.conf from /etc/nginx/templates at startup broke the deployment. This image runs non-root with readOnlyRootFilesystem and — unlike moatless-frontend — the moatless-vibe chart deliberately does not mount an emptyDir over /etc/nginx/conf.d for it (t3-deployment.yaml says so explicitly, because until now the config was baked in). The entrypoint therefore could not write the rendered config: 20-envsubst-on-templates.sh: ERROR: /etc/nginx/templates exists, but /etc/nginx/conf.d is not writable and only warns, so nginx came up with no server block at all — serving nothing while still passing its own liveness probe. Substitute ${CSP_FRAME_SRC} with envsubst during the build instead, staging the template in /tmp so the entrypoint never sees it. The config stays baked and `nginx -t`-validated at build time, which is the contract the chart already documents for this image, so no chart change or lockstep deploy is needed. The value is now a build arg (default `https:`). T3 is not enabled on any local http cluster today (t3.enabled defaults to false and values-local.yaml sets no t3 block), so nothing needs the runtime override the frontend has; a cluster serving previews over http can pass --build-arg instead. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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
2 participants
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
contentTracing) with seeded state, scripted UI interactions, and JSON done-marker output for CI consumptionChatViewinto memoized subcomponents (ChatHeader, approvals panel, timeline sections) to reduce unnecessary rerenders and isolate hot pathsdata-perf-*) in key UI controls and memoizeChatMarkdownto reduce markdown rerender churn250msto1000msto lower background render pressureTesting
bun run test:desktop-perfviaxvfb-runin CI to verify trace generation and automation flowbun run test:desktop-perf:postviaxvfb-runon PRs to verify trace + PR summary publishing pathSummary by CodeRabbit
New Features
Chores
Note
Medium Risk
CI now runs Electron in headless Linux and (for internal PRs) can write PR comments, so workflow permissions and flakiness/timeout tuning are the main risks; app runtime behavior changes are limited to startup/dev tooling and module entrypoints.
Overview
Adds automated desktop performance trace testing to CI, including Linux runtime dependencies,
xvfbexecution, artifact upload, and a PR-only path that can post a perf summary (viapull-requests: write).Updates the desktop packaging/dev flow to ESM output (
main.mjs,preload.cjs), adjusts bundle verification accordingly, and refactorsdev-electron.mjsto replacewait-onwith custom bundle/dev-server readiness probes plus safer Linux sandbox defaults and clearer failure logging.Written by Cursor Bugbot for commit b0eef5c. This will update automatically on new commits. Configure here.