feat(templates): Electrobun shell template for phenotype web apps#81
feat(templates): Electrobun shell template for phenotype web apps#81KooshaPari wants to merge 3 commits into
Conversation
Implements line-level unified diff + patch-apply wrapping `similar` 2.7; exposes diff()/apply() with UnifiedDiff model + 12 TDD tests + 1 doctest. Preserves intent of archived KooshaPari/Diffuse `patch` skeleton. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds templates/electrobun-shell — a parameterized Electrobun desktop shell template. adopt.sh generates an app-specific shell (name, renderer URL, process-compose services, window config) in one command. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
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 Review limit reached
More reviews will be available in 57 minutes and 41 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (12)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
Bot
added
the
size:XL
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| @@ -0,0 +1,98 @@ | |||
| #!/usr/bin/env bash | |||
There was a problem hiding this comment.
Suggestion: Re-implement this adoption workflow in a Rust CLI (or route through an existing Rust binary) and keep shell usage to a thin invocation layer only, because project governance is Rust-first and disallows introducing new Bash implementations. [custom_rule]
Severity Level: Minor
Why it matters? 🤔
The repository governance explicitly states "Rust-first" and the README says "Rust default; no new Bash." This file is a brand-new Bash implementation, so the suggestion correctly identifies a real rule violation in the existing code.
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:** templates/electrobun-shell/adopt.sh
**Line:** 1:1
**Comment:**
*Custom Rule: Re-implement this adoption workflow in a Rust CLI (or route through an existing Rust binary) and keep shell usage to a thin invocation layer only, because project governance is Rust-first and disallows introducing new Bash implementations.
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![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Rust crate, phenotype-diff, for line-level unified diff and patch operations, alongside an Electrobun-based desktop shell template for wrapping web applications. The feedback suggests adding validation to prevent out-of-order or overlapping hunks in the diff application logic. For the desktop template, recommendations include removing an unused import, safely handling potential NaN values during window dimension parsing, generating a .gitignore file during adoption, correcting the TypeScript types reference for Bun, and replacing local absolute file paths in the documentation with relative ones.
| let hunk_old_start = hunk.header.old_start.saturating_sub(1); // convert to 0-based | ||
|
|
||
| // Validate the hunk start is reachable | ||
| if hunk_old_start > source_len { | ||
| return Err(DiffError::OutOfRange { | ||
| line: hunk.header.old_start, | ||
| source_len, | ||
| }); | ||
| } |
There was a problem hiding this comment.
To prevent issues with malformed, out-of-order, or overlapping hunks in a patch, we should validate that the hunk's starting line is not before the current source cursor position.
| let hunk_old_start = hunk.header.old_start.saturating_sub(1); // convert to 0-based | |
| // Validate the hunk start is reachable | |
| if hunk_old_start > source_len { | |
| return Err(DiffError::OutOfRange { | |
| line: hunk.header.old_start, | |
| source_len, | |
| }); | |
| } | |
| let hunk_old_start = hunk.header.old_start.saturating_sub(1); // convert to 0-based | |
| if hunk_old_start < src_cursor { | |
| return Err(DiffError::Malformed( | |
| "hunks are out of order or overlapping".to_string(), | |
| )); | |
| } | |
| // Validate the hunk start is reachable | |
| if hunk_old_start > source_len { | |
| return Err(DiffError::OutOfRange { | |
| line: hunk.header.old_start, | |
| source_len, | |
| }); | |
| } |
| import { BrowserWindow, ApplicationMenu } from "electrobun/bun"; | ||
| import { $ } from "bun"; | ||
| import { join } from "node:path"; |
There was a problem hiding this comment.
The join function imported from node:path is not used anywhere in this file and can be safely removed.
| import { BrowserWindow, ApplicationMenu } from "electrobun/bun"; | |
| import { $ } from "bun"; | |
| import { join } from "node:path"; | |
| import { BrowserWindow, ApplicationMenu } from "electrobun/bun"; | |
| import { $ } from "bun"; |
| width: parseInt(process.env.WINDOW_WIDTH ?? "1400"), | ||
| height: parseInt(process.env.WINDOW_HEIGHT ?? "900"), |
There was a problem hiding this comment.
If WINDOW_WIDTH or WINDOW_HEIGHT environment variables are set to invalid non-numeric values, parseInt will return NaN, which could cause issues when creating the BrowserWindow. Adding a fallback using the logical OR operator ensures we always have valid dimensions.
| width: parseInt(process.env.WINDOW_WIDTH ?? "1400"), | |
| height: parseInt(process.env.WINDOW_HEIGHT ?? "900"), | |
| width: parseInt(process.env.WINDOW_WIDTH ?? "1400") || 1400, | |
| height: parseInt(process.env.WINDOW_HEIGHT ?? "900") || 900, |
| # Write .env.example | ||
| cat > "$OUT_DIR/.env.example" <<EOF | ||
| # Override renderer URL (default: $RENDERER_URL) | ||
| RENDERER_URL=$RENDERER_URL | ||
|
|
||
| # Path to process-compose.yml for one-click service boot | ||
| # Leave unset to skip service boot | ||
| SERVICES_COMPOSE_FILE=${COMPOSE_FILE} | ||
|
|
||
| # Optional window size overrides | ||
| # WINDOW_WIDTH=1400 | ||
| # WINDOW_HEIGHT=900 | ||
| EOF |
There was a problem hiding this comment.
When adopting the desktop shell template, it is highly recommended to generate a .gitignore file in the output directory to prevent users from accidentally committing node_modules, build artifacts, or sensitive .env files.
# Write .env.example
cat > "$OUT_DIR/.env.example" <<EOF
# Override renderer URL (default: $RENDERER_URL)
RENDERER_URL=$RENDERER_URL
# Path to process-compose.yml for one-click service boot
# Leave unset to skip service boot
SERVICES_COMPOSE_FILE=${COMPOSE_FILE}
# Optional window size overrides
# WINDOW_WIDTH=1400
# WINDOW_HEIGHT=900
EOF
# Write .gitignore
cat > "$OUT_DIR/.gitignore" <<EOF
/node_modules
/dist
/.env
/.electrobun
EOF| "strict": true, | ||
| "noUncheckedIndexedAccess": true, | ||
| "lib": ["ESNext"], | ||
| "types": ["bun-types"], |
There was a problem hiding this comment.
Since @types/bun is installed as a devDependency in package.json, the corresponding TypeScript types should be referenced as "bun" instead of "bun-types". Referencing "bun-types" when it is not installed will cause tsc to fail with a missing type definition file error.
| "types": ["bun-types"], | |
| "types": ["bun"], |
|
|
||
| ## Reference: Tracera migration | ||
|
|
||
| `E:/Dev/Tracera/frontend/apps/desktop-electrobun` is the production reference implementation, |
There was a problem hiding this comment.
Avoid referencing local absolute development paths (such as E:/Dev/Tracera/...) in the documentation. Use a relative path or a generic reference instead.
| `E:/Dev/Tracera/frontend/apps/desktop-electrobun` is the production reference implementation, | |
| `frontend/apps/desktop-electrobun` is the production reference implementation, |
|
|
||
| while [[ $# -gt 0 ]]; do | ||
| case $1 in | ||
| --app-name) APP_NAME="$2"; shift 2 ;; |
There was a problem hiding this comment.
Suggestion: Each option branch reads $2 and does shift 2 without first verifying that a value is present. If a caller passes a flag at the end (for example --app-name with no value), set -u makes this crash with an unbound variable instead of a controlled error message. Add an explicit arity check before dereferencing $2 for value-taking flags. [incorrect condition logic]
Severity Level: Major ⚠️
- ❌ Adoption script crashes on missing option values.
- ⚠️ Users see cryptic unbound variable error message.Steps of Reproduction ✅
1. From the repo root, run `bash templates/electrobun-shell/adopt.sh --app-name` with no
value for `--app-name`.
2. Argument parsing loop at `templates/electrobun-shell/adopt.sh:22-33` matches `$1` to
`--app-name` and executes the branch at line 24.
3. With only one argument, `$2` is unset; due to `set -u` at line 12, dereferencing
`APP_NAME="$2"` at line 24 triggers an "unbound variable" error and immediate exit.
4. The script terminates before reaching required-argument checks at lines 35-37, and the
user sees a cryptic shell error instead of a controlled "missing value" message.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:** templates/electrobun-shell/adopt.sh
**Line:** 24:24
**Comment:**
*Incorrect Condition Logic: Each option branch reads `$2` and does `shift 2` without first verifying that a value is present. If a caller passes a flag at the end (for example `--app-name` with no value), `set -u` makes this crash with an unbound variable instead of a controlled error message. Add an explicit arity check before dereferencing `$2` for value-taking flags.
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| sed \ | ||
| -e "s|__APP_NAME__|$APP_NAME|g" \ | ||
| -e "s|__APP_ID__|$APP_ID|g" \ | ||
| -e "s|__APP_VERSION__|$APP_VERSION|g" \ | ||
| -e "s|__DEFAULT_DEV_URL__|$RENDERER_URL|g" \ | ||
| -e "s|__VIEWS_ENTRYPOINT__|$VIEWS_ENTRYPOINT|g" \ | ||
| "$TEMPLATE_DIR/$SRC" > "$DEST" |
There was a problem hiding this comment.
Suggestion: Raw user-provided strings are injected directly into sed replacement expressions. Values containing replacement metacharacters like &, \, or the chosen delimiter can corrupt substitutions and generate broken template files. Escape replacement strings (or use a safer templating approach) before passing them to sed. [logic error]
Severity Level: Major ⚠️
- ❌ Generated Electrobun config contains corrupted app name or URLs.
- ⚠️ Desktop shell may point at wrong renderer endpoint.Steps of Reproduction ✅
1. Run `bash templates/electrobun-shell/adopt.sh --app-name 'Foo & Bar' --app-id
com.example.app --renderer-url 'http://localhost:3000/?foo=1&bar=2' --views-entrypoint
'../web/dist/index.html' --out /tmp/desktop`.
2. The loop at `templates/electrobun-shell/adopt.sh:42-53` copies each template file,
invoking `sed` at lines 46-52 with unescaped replacement values like `$APP_NAME` and
`$RENDERER_URL`.
3. Because the values contain `&`, `sed` treats `&` as "the matched text" in the
replacement section (e.g., lines 47-51), altering the intended strings when generating
files under `/tmp/desktop`.
4. Inspect a generated file such as `/tmp/desktop/electrobun.config.ts` or
`/tmp/desktop/package.json` and observe that the app name or renderer URL no longer
matches the original input, indicating corrupted templating due to unescaped `sed`
replacements.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:** templates/electrobun-shell/adopt.sh
**Line:** 46:52
**Comment:**
*Logic Error: Raw user-provided strings are injected directly into `sed` replacement expressions. Values containing replacement metacharacters like `&`, `\`, or the chosen delimiter can corrupt substitutions and generate broken template files. Escape replacement strings (or use a safer templating approach) before passing them to `sed`.
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| RENDERER_URL=$RENDERER_URL | ||
|
|
||
| # Path to process-compose.yml for one-click service boot | ||
| # Leave unset to skip service boot | ||
| SERVICES_COMPOSE_FILE=${COMPOSE_FILE} |
There was a problem hiding this comment.
Suggestion: Environment values are written unquoted into .env.example, so paths/URLs containing spaces or # will be parsed incorrectly when copied into .env and loaded by just/dotenv. Quote or escape these values when writing the file to avoid truncation and invalid env parsing. [logic error]
Severity Level: Major ⚠️
- ❌ Dotenv misparses URL or path containing `#` character.
- ⚠️ Desktop shell misconfigured when env copied from example.Steps of Reproduction ✅
1. Run `bash templates/electrobun-shell/adopt.sh --app-name MyApp --app-id
com.example.myapp --renderer-url 'http://localhost:3000/#dashboard' --views-entrypoint
'../web/dist/index.html' --out /tmp/desktop`.
2. The `.env.example` block at `templates/electrobun-shell/adopt.sh:55-67` writes
`RENDERER_URL=$RENDERER_URL` at line 58 and `SERVICES_COMPOSE_FILE=${COMPOSE_FILE}` at
line 62 without any quoting or escaping.
3. Copy `/tmp/desktop/.env.example` to `/tmp/desktop/.env` and run `just dev` in
`/tmp/desktop`, which uses `set dotenv-load` at line 72 of the generated `justfile` to
load `.env`.
4. The dotenv loader parses `RENDERER_URL=http://localhost:3000/#dashboard` and treats the
`#dashboard` fragment as a comment, so the effective value is truncated to
`http://localhost:3000/`, causing the desktop shell to use an unintended renderer URL;
similarly, spaces in `SERVICES_COMPOSE_FILE` paths would be parsed incorrectly.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:** templates/electrobun-shell/adopt.sh
**Line:** 58:62
**Comment:**
*Logic Error: Environment values are written unquoted into `.env.example`, so paths/URLs containing spaces or `#` will be parsed incorrectly when copied into `.env` and loaded by `just`/dotenv. Quote or escape these values when writing the file to avoid truncation and invalid env parsing.
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
| let source_lines: Vec<&str> = source.split_inclusive('\n').collect(); | ||
| let source_len = source_lines.len(); |
There was a problem hiding this comment.
Suggestion: source.split_inclusive('\n') treats an empty source as one pseudo-line ([""]), so source_len becomes 1 instead of 0. This makes out-of-range checks too permissive for empty inputs and can allow invalid hunk start positions to pass validation. Handle the empty-source case explicitly so line indexing reflects real line count. [off-by-one]
Severity Level: Major ⚠️
- ❌ Empty-file patches can declare impossible line numbers undetected.
- ⚠️ OutOfRange error reports wrong line count for empty sources.Steps of Reproduction ✅
1. Add a new unit test in `crates/phenotype-diff/src/ops.rs` alongside
`diff_preserves_empty_string` (lines 27-33) that calls `apply` with an empty source string
(`let src = \"\";`).
2. Construct a `UnifiedDiff` manually using `HunkHeader`/`Hunk` from
`crates/phenotype-diff/src/model.rs` (lines 23-41), e.g. `UnifiedDiff { hunks: vec![Hunk {
header: HunkHeader { old_start: 2, old_lines: 0, new_start: 2, new_lines: 1 }, lines:
vec![DiffLine::Added(\"new\n\".into())] }] }`.
3. Call `apply(src, &patch)` which executes `source.split_inclusive('\n')` and
`source_lines.len()` at `crates/phenotype-diff/src/ops.rs:95-96`, producing `source_len ==
1` for the empty string.
4. Observe that `hunk.header.old_start == 2` yields `hunk_old_start == 1`, which passes
the `hunk_old_start > source_len` check at `ops.rs:103-108`, allowing a hunk that refers
to line 2 of a logically zero-line source to be treated as in-range instead of returning
`DiffError::OutOfRange { line: 2, source_len: 0 }` as the API contract suggests.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:** crates/phenotype-diff/src/ops.rs
**Line:** 95:96
**Comment:**
*Off By One: `source.split_inclusive('\n')` treats an empty source as one pseudo-line (`[""]`), so `source_len` becomes `1` instead of `0`. This makes out-of-range checks too permissive for empty inputs and can allow invalid hunk start positions to pass validation. Handle the empty-source case explicitly so line indexing reflects real line count.
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| let hunk_old_start = hunk.header.old_start.saturating_sub(1); // convert to 0-based | ||
|
|
||
| // Validate the hunk start is reachable | ||
| if hunk_old_start > source_len { | ||
| return Err(DiffError::OutOfRange { | ||
| line: hunk.header.old_start, | ||
| source_len, | ||
| }); | ||
| } | ||
|
|
||
| // Copy source lines that precede this hunk | ||
| while src_cursor < hunk_old_start { | ||
| output.push(source_lines[src_cursor].to_owned()); | ||
| src_cursor += 1; | ||
| } | ||
|
|
||
| // Apply each diff line in the hunk | ||
| for dl in &hunk.lines { |
There was a problem hiding this comment.
Suggestion: The apply logic never validates that hunk.header.old_lines/new_lines match the actual number of removed/context/added lines in the hunk body. As a result, malformed patches with inconsistent headers are silently accepted or misapplied instead of being rejected (despite having a Malformed error variant). Add a pre-check per hunk to enforce header/body consistency before applying. [incomplete implementation]
Severity Level: Major ⚠️
- ⚠️ Malformed patches may apply and silently change extra lines.
- ⚠️ DiffError::Malformed is unused, reducing validation guarantees.Steps of Reproduction ✅
1. In a new test in `crates/phenotype-diff/src/ops.rs` (near the existing apply tests at
lines 232-273), manually construct a `UnifiedDiff` using `HunkHeader`/`Hunk` from
`crates/phenotype-diff/src/model.rs:23-41` where `header.old_lines` and `header.new_lines`
do not match the number of `Removed`/`Context` or `Added` lines in `hunk.lines`.
2. For example, set `old_start: 1, old_lines: 1, new_start: 1, new_lines: 0` but put two
`DiffLine::Removed` entries in `lines`, targeting a two-line source string like
`"a\nb\n"`.
3. Call `apply("a\nb\n", &patch)` to execute the hunk logic starting at
`crates/phenotype-diff/src/ops.rs:100-118`; note that only `hunk.header.old_start` is used
for positioning and `old_lines`/`new_lines` are ignored.
4. Observe that `apply` removes both lines based solely on `hunk.lines` and returns
`Ok("")` without ever emitting `DiffError::Malformed` (defined but unused in
`crates/phenotype-diff/src/error.rs:4-12`), meaning malformed header/body combinations are
silently accepted and can misrepresent what the patch claims to do.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:** crates/phenotype-diff/src/ops.rs
**Line:** 101:118
**Comment:**
*Incomplete Implementation: The apply logic never validates that `hunk.header.old_lines`/`new_lines` match the actual number of removed/context/added lines in the hunk body. As a result, malformed patches with inconsistent headers are silently accepted or misapplied instead of being rejected (despite having a `Malformed` error variant). Add a pre-check per hunk to enforce header/body consistency before applying.
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| } catch (err) { | ||
| console.warn( | ||
| `[${APP_NAME}] process-compose boot skipped (not found or services already running):`, | ||
| (err as Error).message | ||
| ); | ||
| } |
There was a problem hiding this comment.
Suggestion: The catch block swallows every process-compose failure and always logs it as a benign skip. This hides real startup errors (invalid compose file, bad config, permission issues) and launches the app in a broken state. Only suppress expected "command not found/already running" cases and rethrow unexpected failures. [logic error]
Severity Level: Major ⚠️
- ❌ Desktop shell can start without required backend services running.
- ⚠️ Serious boot errors mislabeled as benign "skipped" conditions.Steps of Reproduction ✅
1. Set `SERVICES_COMPOSE_FILE` to a path with an invalid or unreadable
`process-compose.yml` in the environment used to run the Electrobun shell
(`templates/electrobun-shell/src/main.ts`).
2. Launch the app so `main()` at `main.ts:112-119` runs and calls `bootServices()` at
`main.ts:27-42` before creating the window.
3. Inside `bootServices`, the `process-compose up -d --config ${SERVICES_COMPOSE_FILE}`
command at `main.ts:34` fails (e.g., non-zero exit code or spawn error), causing Bun's `$`
helper to throw into the `catch (err)` block at `main.ts:36-41`.
4. Observe that the catch block logs a generic "boot skipped (not found or services
already running)" warning and then continues without rethrowing, so the main window is
created via `createMainWindow()` at `main.ts:45-55` even though the configured services
failed to start, leaving the desktop shell running in a broken state with only console
diagnostics.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:** templates/electrobun-shell/src/main.ts
**Line:** 36:41
**Comment:**
*Logic Error: The catch block swallows every `process-compose` failure and always logs it as a benign skip. This hides real startup errors (invalid compose file, bad config, permission issues) and launches the app in a broken state. Only suppress expected "command not found/already running" cases and rethrow unexpected failures.
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| width: parseInt(process.env.WINDOW_WIDTH ?? "1400"), | ||
| height: parseInt(process.env.WINDOW_HEIGHT ?? "900"), |
There was a problem hiding this comment.
Suggestion: parseInt output is used directly for window dimensions without validating the result. If WINDOW_WIDTH or WINDOW_HEIGHT contains a non-numeric value, NaN is passed into BrowserWindow frame sizing, which can cause window creation failures. Validate parsed values and fallback to defaults when parsing fails. [type error]
Severity Level: Major ⚠️
- ❌ Non-numeric env values can break BrowserWindow creation or layout.
- ⚠️ Misconfigured deployment environment destabilizes desktop shell startup.Steps of Reproduction ✅
1. Start the Electrobun shell from `templates/electrobun-shell/src/main.ts` with
`WINDOW_WIDTH` or `WINDOW_HEIGHT` set to a non-numeric value (for example,
`WINDOW_WIDTH=wide`).
2. When `main()` at `main.ts:112-119` executes, it calls `createMainWindow()` at
`main.ts:45-55` to construct a new `BrowserWindow`.
3. Inside `createMainWindow`, the `frame.width` and `frame.height` properties are computed
via `parseInt(process.env.WINDOW_WIDTH ?? "1400")` and `parseInt(process.env.WINDOW_HEIGHT
?? "900")` at `main.ts:50-51`, which yield `NaN` for non-numeric environment values.
4. Observe that these `NaN` values are passed directly into the `BrowserWindow`
constructor (imported from `"electrobun/bun"` at `main.ts:10`), leading to invalid window
sizing and potentially causing window creation to fail or behave unpredictably depending
on Electrobun's handling of `NaN`.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:** templates/electrobun-shell/src/main.ts
**Line:** 50:51
**Comment:**
*Type Error: `parseInt` output is used directly for window dimensions without validating the result. If `WINDOW_WIDTH` or `WINDOW_HEIGHT` contains a non-numeric value, `NaN` is passed into `BrowserWindow` frame sizing, which can cause window creation failures. Validate parsed values and fallback to defaults when parsing fails.
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. |
![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 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Sed substitution corrupts values containing special characters
- Added escape_sed_replace to escape backslashes, ampersands, and pipe delimiters before sed substitution of user-provided values.
- ✅ Fixed:
Hunktype missing from crate root re-exports- Added Hunk to the crate root pub use model re-exports alongside DiffLine, HunkHeader, and UnifiedDiff.
Or push these changes by commenting:
@cursor push 7b51a6749e
Preview (7b51a6749e)
diff --git a/crates/phenotype-diff/src/lib.rs b/crates/phenotype-diff/src/lib.rs
--- a/crates/phenotype-diff/src/lib.rs
+++ b/crates/phenotype-diff/src/lib.rs
@@ -28,5 +28,5 @@
pub mod ops;
pub use error::DiffError;
-pub use model::{DiffLine, HunkHeader, UnifiedDiff};
+pub use model::{DiffLine, Hunk, HunkHeader, UnifiedDiff};
pub use ops::{apply, diff};
diff --git a/templates/electrobun-shell/adopt.sh b/templates/electrobun-shell/adopt.sh
--- a/templates/electrobun-shell/adopt.sh
+++ b/templates/electrobun-shell/adopt.sh
@@ -36,6 +36,15 @@
[[ -z "$APP_ID" ]] && { echo "ERROR: --app-id required"; exit 1; }
[[ -z "$OUT_DIR" ]] && { echo "ERROR: --out required"; exit 1; }
+# Escape &, \, and | for sed replacement (delimiter is |).
+escape_sed_replace() {
+ local value="$1"
+ value="${value//\\/\\\\}"
+ value="${value//&/\\&}"
+ value="${value//|/\\|}"
+ printf '%s' "$value"
+}
+
TEMPLATE_DIR="$(cd "$(dirname "$0")" && pwd)"
mkdir -p "$OUT_DIR/src"
@@ -44,11 +53,11 @@
DEST="$OUT_DIR/$SRC"
mkdir -p "$(dirname "$DEST")"
sed \
- -e "s|__APP_NAME__|$APP_NAME|g" \
- -e "s|__APP_ID__|$APP_ID|g" \
- -e "s|__APP_VERSION__|$APP_VERSION|g" \
- -e "s|__DEFAULT_DEV_URL__|$RENDERER_URL|g" \
- -e "s|__VIEWS_ENTRYPOINT__|$VIEWS_ENTRYPOINT|g" \
+ -e "s|__APP_NAME__|$(escape_sed_replace "$APP_NAME")|g" \
+ -e "s|__APP_ID__|$(escape_sed_replace "$APP_ID")|g" \
+ -e "s|__APP_VERSION__|$(escape_sed_replace "$APP_VERSION")|g" \
+ -e "s|__DEFAULT_DEV_URL__|$(escape_sed_replace "$RENDERER_URL")|g" \
+ -e "s|__VIEWS_ENTRYPOINT__|$(escape_sed_replace "$VIEWS_ENTRYPOINT")|g" \
"$TEMPLATE_DIR/$SRC" > "$DEST"
doneYou can send follow-ups to the cloud agent here.
| -e "s|__APP_VERSION__|$APP_VERSION|g" \ | ||
| -e "s|__DEFAULT_DEV_URL__|$RENDERER_URL|g" \ | ||
| -e "s|__VIEWS_ENTRYPOINT__|$VIEWS_ENTRYPOINT|g" \ | ||
| "$TEMPLATE_DIR/$SRC" > "$DEST" |
There was a problem hiding this comment.
Sed substitution corrupts values containing special characters
Medium Severity
The sed replacements interpolate user-provided values ($APP_NAME, $RENDERER_URL, etc.) directly into the replacement string without escaping sed metacharacters. An & in any value (e.g., an app name like "Track & Field") is interpreted by sed as "the entire matched text," corrupting the output. Similarly, | (the chosen delimiter) or \ in any value would break the command entirely or produce garbled results.
Reviewed by Cursor Bugbot for commit 6a9a6f7. Configure here.
| pub mod ops; | ||
|
|
||
| pub use error::DiffError; | ||
| pub use model::{DiffLine, HunkHeader, UnifiedDiff}; |
There was a problem hiding this comment.
Hunk type missing from crate root re-exports
Low Severity
The crate root re-exports DiffLine, HunkHeader, and UnifiedDiff but omits Hunk, even though UnifiedDiff exposes pub hunks: Vec<Hunk>. External consumers who need Hunk as a type annotation (e.g., in function signatures or variable bindings) must reach into phenotype_diff::model::Hunk rather than importing it from the crate root like all other public types.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 6a9a6f7. Configure here.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| description = "Line-level unified diff and patch-apply library (migrated from KooshaPari/Diffuse); wraps `similar`" | ||
|
|
||
| [dependencies] | ||
| similar = "2.6" |
There was a problem hiding this comment.
CRITICAL: Version mismatch between Cargo.toml and Cargo.lock.
Cargo.toml specifies similar = "2.6" but Cargo.lock shows similar version 2.7.0. This inconsistency will cause build determinism issues.
| */ | ||
| import { BrowserWindow, ApplicationMenu } from "electrobun/bun"; | ||
| import { $ } from "bun"; | ||
| import { join } from "node:path"; |
There was a problem hiding this comment.
WARNING: Unused import - join is imported from node:path but never used in this file.
Code Review SummaryStatus: 2 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
Other Observations (not in diff)
Files Reviewed (5 files)
Reviewed by laguna-m.1-20260312:free · 490,488 tokens |
) Empirically verified on Windows 11 (RTX 3090 Ti host): `electrobun build` succeeds and produces a dev-win-x64/ bundle with launcher.exe, bun.exe, libNativeWrapper.dll, WebView2Loader.dll — no cl.exe or MSVC required. The CLI downloads pre-built platform-native binaries from GitHub Releases (electrobun-core-win-x64.tar.gz) at first-run; the compiler is only needed to build Electrobun itself, not to use it. Correct the misleading comment from "macOS only" to the accurate per-platform note so Windows and Linux consumers are not blocked by a wrong assumption. Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 4 total unresolved issues (including 2 from previous reviews).
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Unused import of
joinin template- Removed the unused
joinimport fromnode:pathintemplates/electrobun-shell/src/main.ts.
- Removed the unused
- ✅ Fixed: Local filesystem path committed in README
- Replaced the Windows-local
E:/Dev/Tracera/...path with the monorepo-relativeapps/desktop-electrobunreference in the README.
- Replaced the Windows-local
Or push these changes by commenting:
@cursor push e42bbad110
Preview (e42bbad110)
diff --git a/templates/electrobun-shell/README.md b/templates/electrobun-shell/README.md
--- a/templates/electrobun-shell/README.md
+++ b/templates/electrobun-shell/README.md
@@ -43,5 +43,5 @@
## Reference: Tracera migration
-`E:/Dev/Tracera/frontend/apps/desktop-electrobun` is the production reference implementation,
+`apps/desktop-electrobun` in the Tracera monorepo is the production reference implementation,
migrated from Electron + electron-vite to this template.
diff --git a/templates/electrobun-shell/src/main.ts b/templates/electrobun-shell/src/main.ts
--- a/templates/electrobun-shell/src/main.ts
+++ b/templates/electrobun-shell/src/main.ts
@@ -9,7 +9,6 @@
*/
import { BrowserWindow, ApplicationMenu } from "electrobun/bun";
import { $ } from "bun";
-import { join } from "node:path";
// ── Config ────────────────────────────────────────────────────────────────────
const APP_NAME = process.env.APP_NAME ?? "__APP_NAME__";You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 9cc5fca. Configure here.
| */ | ||
| import { BrowserWindow, ApplicationMenu } from "electrobun/bun"; | ||
| import { $ } from "bun"; | ||
| import { join } from "node:path"; |
There was a problem hiding this comment.
Unused import of join in template
Low Severity
join is imported from node:path but never used anywhere in main.ts. Since this is a template file, every project generated via adopt.sh inherits this dead import.
Reviewed by Cursor Bugbot for commit 9cc5fca. Configure here.
| ## Reference: Tracera migration | ||
|
|
||
| `E:/Dev/Tracera/frontend/apps/desktop-electrobun` is the production reference implementation, | ||
| migrated from Electron + electron-vite to this template. |
There was a problem hiding this comment.
Local filesystem path committed in README
Low Severity
The README references E:/Dev/Tracera/frontend/apps/desktop-electrobun, a developer-local Windows path. This is meaningless to other contributors and exposes internal development setup details in a reusable template.
Reviewed by Cursor Bugbot for commit 9cc5fca. Configure here.
User description
Summary
templates/electrobun-shell/— parameterized Electrobun desktop shell templateadopt.shgenerates a ready-to-run shell in one command (name, renderer URL, process-compose services, window size)process-compose up -dfrontend/apps/desktop-electrobunUsage
Test plan
bun install && bun devon macOS opens window🤖 Generated with Claude Code
Note
Low Risk
New template and standalone library with no changes to existing runtime crates; desktop launch only runs optional local process-compose when configured.
Overview
Adds
templates/electrobun-shell/so Phenotype web apps can be wrapped as desktop apps via Electrobun: parameterized config,adopt.shto copy the template and substitute app name/id, dev renderer URL, bundled views path, and optionalprocess-composepath, plus generated.env.exampleandjustfile. The main process optionally runsprocess-compose up -don launch, opens a configurable webview URL or bundledviews://app, and ships a default window and app menu.Also introduces workspace crate
phenotype-diff: line-leveldiff/applyon top ofsimilar, serde-friendlyUnifiedDiffmodels, andDiffErrorfor context mismatch, out-of-range hunks, and malformed patches, with unit tests for round-trips.Cargo.toml/Cargo.lockregister the crate andsimilardependency.Reviewed by Cursor Bugbot for commit 9cc5fca. Bugbot is set up for automated code reviews on this repo. Configure here.
CodeAnt-AI Description
Add a reusable desktop shell template and a unified diff library
What Changed
adopt.shscript that copies the template, fills in app details, and writes startup settings for the new desktop appphenotype-difflibrary that can create line-based unified diffs and apply them back to text, with clear errors for mismatched or invalid patchesImpact
✅ Faster desktop app setup✅ One-command local service startup✅ Safer patch application for text edits💡 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.