Code Review Report

The alias map defines both '@' and '@/' with '@' listed first. Rollup/Vite alias resolution treats string aliases as prefix matches, so '@' will also match imports that begin with '@/'. This means the more specific '@/' alias never applies, and '@/...' imports resolve to './src' (server) instead of './web/src' or can even generate incorrect paths. If frontend code or tests use '@/...' expecting the web root, they will either fail to resolve or silently pull server modules, producing incorrect behavior.

Use an alias array with the longer pattern first (e.g., find `^@/` then `^@`) or rename the web alias to a non-overlapping token like `@web`. If keeping '@/', ensure the replacement includes a trailing slash and that it is applied before '@'.

Coverage configuration explicitly includes `tests/unit/**/*.ts`, which instruments and reports on test files themselves. This inflates coverage percentages and can mask untested production code. It also adds overhead during coverage runs. If coverage thresholds are used in CI, this can lead to false confidence.

Remove `tests/unit/**/*.ts` from `coverage.include` and, if desired, add `tests/**` to `coverage.exclude` so only production code is measured.

The production backend is launched with `spawn('node', [backendPath])`, which requires a system `node` binary to be available on PATH. Packaged Electron apps typically run on machines without Node installed, and Electron does not ship a separate `node` executable in PATH. This can cause the backend process to fail to start, leaving the UI unable to reach the API. To confirm severity, verify whether the packaged app bundles a Node binary or explicitly sets `ELECTRON_RUN_AS_NODE` with `process.execPath`.

Use the Electron runtime to run the backend: set `ELECTRON_RUN_AS_NODE=1` and spawn `process.execPath` with the backend script, or bundle a Node binary alongside the app and spawn that explicit path. Add error handling to surface a user-facing message when the backend cannot start.

The main process opens any URL received from the renderer (IPC) or window.open without validating scheme or origin. If renderer content is compromised (XSS, user-provided HTML, or untrusted content), it can trigger dangerous protocol handlers (e.g., `file://`, `ms-...`) or open local files. Electron security guidance recommends whitelisting protocols. Confirm whether renderer ever displays untrusted content or user-supplied HTML.

Validate URLs in the main process: parse with `new URL(url)`, allow only `http:`/`https:` (and optionally `mailto:`), and reject or log others. Apply the same validation in windowOpenHandler and IPC handler.

Both main and settings windows set `sandbox: false`. This disables Chromium's renderer sandbox, increasing the impact of any renderer compromise even with contextIsolation and nodeIntegration disabled. Electron’s security guidance recommends sandboxing renderers when possible. Confirm whether any preload or renderer dependencies truly require sandbox to be off.

Enable `sandbox: true` and verify preload IPC still works. If a sandbox is not feasible, document the constraint and add compensating controls (strict CSP, navigation lock-down, disable remote modules).

The restart handler calls `stopBackend()` (which sends kill) and waits a fixed 1 second before starting a new backend process. If the backend takes longer to exit or ignores SIGTERM, the new process can start while the old one is still running, causing port conflicts or orphaned processes. To confirm, inspect backend shutdown behavior under load.

Wait for the child process `close` event (with a timeout) before starting a new backend. If it doesn’t exit in time, send SIGKILL and surface a warning. Handle spawn errors and report restart failures to the renderer.

The preload script constructs `assetBasePath` as `file://${distPath}` using a raw filesystem path and assumes the build output is always `../web/dist`. This produces invalid file URLs on Windows or paths with spaces (missing proper URL encoding and `file:///` form), and it can point to the wrong folder when the app loads from `../dist` (which is a valid production path in `loadProductionBuild`). `resolveAssetPath` then prepends this base, causing assets to fail to load in packaged builds. Confirm the actual production build output location and whether `resolveAssetPath` is used for UI assets.

Generate the base using `pathToFileURL(distPath + path.sep).href` and align the path with the actual HTML location chosen in `loadProductionBuild`. Consider sending the resolved base from the main process after selecting the HTML path, or compute it from `app.getAppPath()`/`process.resourcesPath` reliably.

The CLI icon uses a hard-coded root-relative path (`/assets/cli-icons/claude-code.jpg`). With Vite configured to use `base: './'` and Electron loading via `file://`, root-relative URLs resolve to the filesystem root instead of the app's dist directory. This makes the icon missing in production builds. Confirm if this pattern appears in other components.

Import the asset through Vite (e.g., `import claudeIcon from '@/assets/cli-icons/claude-code.jpg'`) or use `resolveAssetPath('assets/cli-icons/claude-code.jpg')` without a leading `/` to respect the base path.

The health check probes `http://{serverHost}:{serverPort}/socket.io` using the configured bind host. When the host is `0.0.0.0` or `::`, the URL is not routable from the local machine, so the probe fails and the health check reports a degraded/unhealthy websocket despite a functioning server. This is a common production configuration for binding to all interfaces.

Use a loopback address (e.g., `127.0.0.1`) for self-probes when the configured host is `0.0.0.0`/`::`, or avoid HTTP probing altogether and rely on `io` state when available.

Both `setupApp` and `index.ts` read `package.json` from `process.cwd`, which often points outside the app root in packaged desktop apps or when started from a different working directory. This causes the app version to fall back to `0.0.0`, while `/api/app/info` uses a more robust search and may return the correct version. The result is inconsistent and misleading version reporting across endpoints and server metadata.

Centralize version lookup using the same robust path search as `src/server/api/app.ts`, or embed version at build time and avoid `process.cwd` dependency.

Server startup reads `config/default.json` unconditionally to determine port/host, while `setupApp` uses app-data config when running as a desktop app. In desktop mode, user settings are stored under the app data directory, so changes to port/host are not reflected at startup. This creates a mismatch between UI settings and actual server binding.

When `isDesktopApp()` is true, load the config from `getAppDataPath()` (the same path used by `SettingsService`) or reuse `SettingsService` to provide port/host before creating `StandardServer`.

The analyze route pulls `configManager` from `app.locals` to determine `scanner.maxLargestFiles`, but `setupApp` only populates `settingsService` (and not `configManager`). If no other middleware sets `app.locals.configManager`, the code silently falls back to the hardcoded default (20), making configuration changes ineffective and difficult to diagnose. This is likely a real miswire because the rest of the app uses `SettingsService` for config.

Replace `configManager` usage with `settingsService` (e.g., `req.app.locals.settingsService.get('scanner.maxLargestFiles')`) or ensure `configManager` is initialized and attached in `setupApp`. Add a small integration test verifying the setting is honored.

Relative paths are derived by string replacement using `project.path + '/'`, which assumes POSIX separators and exact prefix matching. On Windows, paths use backslashes, so the replacement fails and `relativePath` becomes the full absolute path. This will surface in API responses and UI displays, and can also mis-handle paths where the project path appears elsewhere in the string.

Use `path.relative(project.path, file.path)` and normalize separators if the frontend expects POSIX-style paths. This will work across platforms and avoid incorrect replacements.

PATCH /api/sessions/:id writes the provided config object directly onto the session without applying the same validation/transform rules used at creation. This allows invalid models, inconsistent CLI settings, or unsupported options to be persisted, which can break later session execution or cause opaque runtime errors when the session is started.

Validate config updates with a schema (e.g., a partial form of createSessionSchema) and reapply default/transform logic. Reject invalid models/CLI combinations with a 400 and clear error message.

The session creation schema allows verificationEnabled to be true while verificationCli is undefined. The refinement only validates the verification model when verificationCli is provided, so sessions can be created with verification enabled but no CLI/model to execute it. This is likely to cause failures or silent skips during the verification phase.

Add a schema rule requiring verificationCli when verificationEnabled is true, and set a default verificationCli (e.g., same as cli) if that is intended. Ensure verificationModel is derived when verification is enabled.

The file-stats endpoint reads entire files into memory to count line breaks. With large files or large file lists this can be slow and memory intensive, even though only line counts are needed.

Use streaming line counting (e.g., read in chunks and count '\n'), and enforce per-request file count/size limits.

The codebase copy endpoint accepts selectedFiles from the request and passes them directly to codebaseScanner.getFileContent without validating that the paths are relative or within the project root. If getFileContent does not enforce a base path, this could allow path traversal or reading arbitrary files.

Validate each selected file: reject absolute paths and traversal segments, resolve against project.path, and ensure the resolved path stays within the project directory before reading. Prefer a shared helper used by other endpoints.

Both the codebase copy and project export endpoints concatenate all file contents into a single string and return it in JSON. On large repositories this can create very large in-memory strings and huge HTTP responses, risking OOM, long GC pauses, or request timeouts.

Add size caps and return 413 when exceeded, allow chunked/streamed responses, or require explicit file selection with a maximum total size. Consider gzip compression for large exports.

The /logs/entries endpoint loads the entire current log file into memory and returns every line. As logs grow, this becomes expensive and can slow or destabilize the server.

Add pagination or tailing (e.g., last N lines), limit max bytes per request, and optionally support range queries.

aiGroupingSchema validates CLI and model types but does not validate that the model is valid for the chosen CLI. Unlike the auto-create schema, there is no `isValidModel` refine here. This allows invalid model values into analyzeGroupingsWithAI and prompt-preview, causing runtime errors instead of a clean 400 validation response.

Add an isValidModel refine to aiGroupingSchema similar to autoCreateSessionsSchema. Return a 400 validation error for invalid models.

getAllProjects updates branch info with `Promise.all` but does not catch errors for individual projects. If a single repository path is missing or `gitService.getCurrentBranch` throws, the entire call rejects and all projects fail to load.

Wrap each branch lookup in a try/catch so one failing project doesn’t abort the entire list. Keep the previous branch value and log an error per project.

The log stream sets up a recurring interval to check for date changes, but `stop()` only closes the file watcher. The interval continues to run indefinitely, which can keep the process alive and waste resources even after the service is stopped.

Clear `dateCheckInterval` in stop(), and set it to null. Also consider clearing it when tearing down the service in other lifecycle paths.

The log tailing logic only processes when `newSize > filePosition`. If the log file is truncated or rotated within the same day, `newSize` becomes smaller than `filePosition`, and the code never resets the cursor. New log lines written after truncation are ignored until the file grows beyond the old position, causing missing logs.

If `newSize < filePosition`, reset `filePosition` (typically to 0) and optionally read from the start. Also handle `rename`/`change` events to re-open the file when it is replaced.

The Codex authentication check infers auth status by running `codex --help` and then uses an OR between two substring checks. This logic marks the CLI as authenticated whenever either substring is missing, which is common across different help outputs or localized messages. As a result, the UI can report Codex as authenticated when it is not, leading to failed runs later and confusing status messaging.

Replace the OR with a more robust check. Prefer checking exit codes and explicit auth commands if available (e.g., `codex auth status`). If relying on help output, require both substrings (AND) or match a single canonical phrase. Add explicit unit tests for multiple outputs.

TagService loads tags asynchronously in the constructor, but read methods (getAll/getById/search/validateIds) do not await initialization. If these reads happen early during startup, they can return empty or incomplete tag data even though the tags file exists.

Make read methods async and call `await ensureInitialized()`, or ensure initialization completes before the service is used (e.g., during app bootstrap).

The verification output schema allows `description` and `suggestedFix` to be empty strings (optional with default `''`) for new issues, but `FixPromptBuilder` throws if either field is missing or blank. If a verification agent emits a new issue without these fields and that issue is later fed into the fix pipeline, prompt construction will fail at runtime.

Either enforce non-empty `description` and `suggestedFix` in `verificationNewIssueSchema`, or add a normalization layer that maps alternative fields into these required ones before invoking `FixPromptBuilder`.

The update service can repeatedly reschedule itself and ignore user settings. `checkForUpdates()` calls `loadUpdateSettings()`, and `loadUpdateSettings()` starts auto-check when updates are enabled. `startAutoCheck()` immediately calls `checkForUpdates()` again and resets the interval. This creates a self-triggering loop of update checks and interval resets. Separately, `initialize()` always calls `startAutoCheck()` regardless of `updates.enabled`, so a user who disables updates still gets background checks. This combination risks rapid repeated GitHub API calls, overlapping checks, and failur

1) Remove `startAutoCheck()` from `loadUpdateSettings()` or add a flag to skip scheduling when called from `checkForUpdates()`. 2) In `initialize()`, only call `startAutoCheck()` if `updates.enabled` is true. 3) Add a re-entrancy guard (e.g., if `status.checking` is true, skip scheduling) to prevent overlapping checks.

When the GitHub check determines there is no newer release or returns 404, the service only sets `available=false` and leaves `latestVersion` and `releaseNotes` unchanged. If a previous check found an update, stale metadata remains and can be shown to the user even though updates are no longer available.

Explicitly set `latestVersion` and `releaseNotes` to `null` when no update is available or on 404 responses. Consider clearing them when an error occurs as well.

The scope stats scanner uses `fs.stat()` and recursive directory traversal without symlink handling. `fs.stat()` follows symlinks, so a symlinked directory inside the project can cause the scan to traverse outside the project boundary or loop back to a parent path. This can lead to scanning unintended files or infinite recursion in pathological cases.

Use `fs.lstat()` to detect symlinks. Either skip symlinked entries or resolve them and verify the realpath is within the project root. Optionally track visited realpaths/inodes to prevent cycles.

The scanner defines `SYSTEM_EXCLUDES` (e.g., `node_modules`, `dist`, `build`) but `scanDirectory()` never applies them. It only ignores `.git` and `.gitignore` unless the caller passes explicit additional rules or the project’s `.gitignore` covers these directories. This can cause large dependency and build folders to be scanned in full.

Add `SYSTEM_EXCLUDES` to the ignore rules inside `scanDirectory()` (or use `buildIgnoreRules()` consistently). Ensure the default excludes are always applied unless explicitly overridden.

Both `requestStartReviewSession()` and `requestStartFixSession()` register a session as running before invoking `startCallback()`. If `startCallback()` throws, the session remains in `runningSessions` and is never cleaned up. This inflates running counts and can block the queue indefinitely, because the system believes a session is active when it never started.

Wrap `startCallback()` in a try/catch. On failure, remove the session from `runningSessions`, emit a failure event, and allow queued sessions to proceed.

When the discovery agent returns success without structured output, reviewFile/reviewFileBatch record a failure and return null, but runDiscoveryPhase still marks all files in the batch as reviewed. This means the session can complete a group even though the file’s review failed and is marked retryable, and resume will skip that file entirely.

Treat missing structured output as a batch error: either throw in reviewFile/reviewFileBatch or have runDiscoveryPhase skip adding files to filesReviewed when results contain nulls. Then mark the group as failed/paused so resume reprocesses those files.

When a group completes, its issuesDetected is set to the session-wide issuesDetected counter. This inflates per‑group counts for later groups and misrepresents which group actually found which issues.

Track per‑group issue counts (increment when issues are detected in that group) and set group.issuesDetected to that value on completion.

checkOrphanedIssues builds file paths directly from issue.primaryFile without validation. primaryFile can originate from LLM output (evidence.path) and may contain traversal segments. This allows pathExists checks outside the project directory, potentially probing the filesystem and incorrectly resolving issues.

Normalize and validate issue file paths at ingestion (upsertFromDetection) or before filesystem access in checkOrphanedIssues. Reject or sanitize any path that escapes the project root.

When verification is configured to use Codex but Codex isn’t available, the fallback to Claude Code keeps the verificationModel unchanged. If verificationModel is a Codex-only model, the Claude runner will fail, effectively disabling verification despite fallback.

When falling back to Claude Code, switch to a Claude-compatible model (e.g., getDefaultModel('claude-code')) or surface a clear error requiring a compatible verificationModel for the fallback path.

In batch mode, processDiscoveryOutput is called once per file with the same output. Resolved issues are processed each time, incrementing session.issuesResolved (and run stats) for each file in the batch, inflating counts and repeating updates.

In batch mode, process resolved issues only once (e.g., only on the primary file) or add a guard that tracks which resolved shortIds have been applied for this output.

Multiple git/gh commands are built by string interpolation and executed via execSync. Branch names, prefixes, custom branch names, and PR titles/bodies can originate from user configuration or issue titles. Because execSync uses a shell when given a string, quotes or shell metacharacters can break the command or be interpreted, leading to command failures or command injection in worst cases. Even benign quotes/newlines in titles can cause PR creation to fail.

Replace execSync(string) with execFile/spawn and pass arguments as an array. Validate/sanitize branch names (e.g., allow only [A-Za-z0-9._/-]) and reject invalid values. For PR bodies, write to a temp file and use `gh pr create --body-file` or pass as a single arg without shell interpretation.

Session cancellation only kills ProcessRunner processes. When a fix session uses the Codex CLI, the CodexRunner process continues running after cancel, potentially continuing to modify files and commit changes even though the session is marked cancelled. This is inconsistent with the cancellation contract.

Track Codex runIds per session/issue and call codexRunner.cancel on cancelSession; or query codexRunner.listActive and terminate any runIds containing the sessionId. Ensure queue state is updated accordingly.

pauseSession sets status to PAUSED, but startSessionInternal treats any non-RUNNING status as a break condition and then proceeds to complete the session, overwriting the status to COMPLETED. As a result, paused sessions cannot be resumed and pending issues can be silently skipped.

When a session transitions to PAUSED, exit startSessionInternal early without finalizing or updating to COMPLETED. Preserve PAUSED status and allow resume to continue at currentIssueIndex.

resumeSession resets failed issue entries to PENDING but does not adjust session.issuesFailed. This causes aggregate counts and logs to remain inflated even after a successful retry, which can mislead users and automation.

When resuming, recompute issuesFixed/Failed/Skipped/NeedingIntervention from the current issue statuses, or decrement counters for each reset.

Issues are set to FIXING at the start of fixSingleIssue. On success, issueService.markResolved is only called when a commitHash exists; in no-commit flows (or autoCommit false) the issue status remains FIXING. Similarly, for the 'pending' (needs intervention) path, issueService is not updated at all. This leaves issue lifecycle state inconsistent with the fix session outcome.

Always update the issue service on completion: mark resolved even without a commit hash (allow empty commit metadata) and set an explicit needs-intervention status for the pending case.

For each issue, fixSingleIssue rebuilds ignore rules and re-scans the directory structure. In sessions with many issues or large repositories, this results in repeated full-tree scans and unnecessary I/O, slowing fix runs substantially.

Compute ignore rules and directory structure once per session (or cache by projectPath) and reuse across all issues, invalidating only when necessary.

CodexRunner writes a prompt file to the temp directory before spawning the CLI. If spawn emits an error (e.g., executable not found or permission denied), the error handler resolves without removing the prompt file. This leaves sensitive prompt content on disk and can accumulate temporary files.

Delete the prompt file in the error handler (and in any early return paths) using a finally block to ensure cleanup regardless of spawn outcome.

CLI subscription detection uses shell redirection and fallback constructs like `2>/dev/null` and `|| echo` inside exec. These are Unix shell features and will fail on Windows' default shell, causing detection errors or misleading subscription results on Windows platforms.

Use execFile with argument arrays to avoid shell features, or conditionally build commands per platform (e.g., `2>nul` on Windows). Capture stderr directly instead of redirecting it in the shell.

Issue creation derives `highlightLines`, `lineStart/lineEnd`, and `codeSnippet` from detection evidence, but re-detection updates do not refresh these fields when the evidence changes. The update path only updates `lineStart/lineEnd` if `detection.lineStart` is set and ignores `evidence.line` / `evidence.highlightLines`, and it does not update `codeSnippet` from `evidence.snippet`. This can leave users seeing outdated line references or code snippets after a file changes, which undermines verification and fix workflows.

When `detection.evidence` is present, normalize and update `highlightLines`, `lineStart/lineEnd`, and `codeSnippet` from the evidence. Consider a helper to keep legacy and new fields in sync on both create and update.

getByFile assumes `affectedFiles` is always an array and calls `.includes()` directly. If issues were created in earlier versions or imported without that field, this method will throw at runtime. This can break any API or UI path that queries issues by file for legacy data sets.

Guard with optional chaining or default to an empty array (e.g., `i.affectedFiles?.includes(file)`), and/or normalize loaded issues in `loadIssues` to always include `affectedFiles`.

Stash selection is based on a substring match against the stash list, which is ambiguous when multiple stash messages contain the same session id fragment. The first matching line is used, so if there are multiple similarly named stashes (e.g., nested/overlapping session IDs or past runs), finalize/abort can pop the wrong stash. That can apply unrelated changes or discard the intended ones, which is especially risky when users have multiple active sessions or reruns.

Match stash messages exactly using a structured format (e.g., `git stash list --format="%gd|%s"`) and compare the message for equality. Alternatively, parse the stash ref directly from the `git stash push` output and store it immediately. Avoid substring matching to prevent collisions.

Several git commands pass user-selected file paths as arguments without a `--` delimiter. If a file name begins with '-' (which is valid on many filesystems), git treats it as an option rather than a pathspec, which can change command behavior or cause failures. This is a known git CLI footgun and can be triggered by normal files with unusual names or by untrusted inputs from AI/user selections.

Use `--` before pathspecs for git commands (e.g., `git add -- <file>`, `git diff --cached -- <file>`, `git checkout HEAD -- <file>`), and consider rejecting or normalizing dash-leading filenames at validation time. Prefer `execFile` with argument arrays for all git calls to avoid quoting pitfalls.

recordFileReview claims it creates both file-started and file-completed events, but only a file-completed event is actually appended. If any timeline UI, metrics, or debugging tooling expects a start event, the absence can lead to misleading timelines or missing progress signals. This mismatch between documentation and behavior is a functional risk in observability features.

Either emit a file-started event when the review begins (using `startTime`), or update the documentation and any consumers to reflect that only file-completed is recorded.

ProcessRunner builds a shell command string by concatenating arguments that include user- or config-provided values (model, allowed/disallowed tools) and executes it via `sh -c`. If any of those values can be influenced externally (e.g., via API settings or config), shell metacharacters will be interpreted, enabling arbitrary command execution. This is a realistic risk because the API already has model-validation gaps elsewhere, so the assumption that model/tool names are safe may not hold. Confirm by tracing where `options.model` and tool lists come from and whether they are strictly validate

Avoid shells entirely: spawn the CLI directly with an арг array and pipe the prompt file to stdin (e.g., `spawn(claudePath, args, { stdio: [...] })` + `fs.createReadStream(promptFile).pipe(proc.stdin)`). If shell use is unavoidable, strictly validate `model`, `allowedTools`, and `disallowedTools` against a whitelist and reject any unexpected characters.

ProcessRunner hard-codes `/tmp/viberails-prompts` and relies on `sh -c` with `cat` to feed the prompt. This is Unix-specific and will fail on Windows where `/tmp` and `sh`/`cat` are not available by default. Any feature using ProcessRunner (session grouping, Claude Code runs) will be unavailable for Windows users.

Use `os.tmpdir()` to construct the prompt directory and remove shell usage by spawning the CLI directly and piping prompt contents to stdin. This makes the process fully cross-platform.

The prompt file path is created by joining a temp directory with `${runId}.md` without validating or sanitizing `runId`. If `runId` can be influenced by external input, a value containing path separators (e.g., `../`) could escape the temp dir and overwrite arbitrary files. This is a common path traversal/clobber risk; confirmation requires verifying that all run IDs are always generated server-side and never user-controlled.

Validate `runId` against a strict UUID/short-id regex, or ignore caller-provided values and generate a safe filename internally (e.g., using `fs.mkdtemp` + `path.join`). Also consider `path.basename` to strip traversal components.

SessionGroupingService constructs a full directory tree including every file and size, and embeds it into the LLM prompt along with the full file list. There is no cap or summarization strategy. For large repos (monorepos, vendor folders, or incomplete excludes), this can produce prompts that exceed model context limits or cause long runtimes/timeouts. This is likely in real-world usage where users forget to exclude large directories.

Add prompt size guards: limit the number of files/dirs listed, summarize large directories with counts only, and fall back to tool-based exploration when the prompt exceeds a threshold. Consider a max prompt token budget per model.

The tests wait for 10ms using `setTimeout` to let the constructor’s async load complete. This is timing-dependent and can be flaky on slow CI or loaded machines, causing nondeterministic failures. Tests should await a deterministic signal instead of sleeping.

Expose an initialization promise on ProjectService (e.g., `this.ready`) or await the internal load method in tests. Replace timeouts with deterministic waiting on async completion.

The server-side grouping logic uses `Math.ceil(size * 0.25)` for token estimation, while the web helper uses `Math.round`. This yields off-by-one (and potentially larger) discrepancies between UI display and backend calculations (e.g., 1 byte → UI=0, server=1). This inconsistency can mislead users when they are trying to keep groups within model limits.

Centralize token estimation into a shared helper (e.g., in `src/shared`) and have both server and web import the same function with a consistent rounding strategy.

The ISSUE_STATUS enum includes a 'duplicate' status, but isIssueTerminal() does not consider duplicates terminal. Any logic that relies on isIssueTerminal to filter or finalize issues will keep duplicate issues in the active pipeline, which can inflate counts, allow reprocessing, or keep them eligible for fixes when they should be considered resolved.

Update isIssueTerminal to include ISSUE_STATUS.DUPLICATE (and any other terminal statuses), then verify downstream filtering logic uses this helper consistently.

Constants are labeled as the single source of truth, but the same status values are redefined as string unions in codeReview.ts. This creates a drift risk: adding/removing a status in constants won't update the types, leading to compile-time mismatches or runtime values that the UI or API types don't accept.

Derive status types directly from the constants (e.g., `export type IssueStatus = typeof ISSUE_STATUS[keyof typeof ISSUE_STATUS]`) and remove duplicated unions. Update any exports to re-use these derived types.

Several socket event payload fields are typed as plain `string` even though the project already defines strict enums for severity/category/status. This removes compile-time guarantees and allows invalid values to propagate into UI code that likely expects known enums (e.g., color/status mapping).

Import and use IssueSeverity, IssueCategory, and IssueStatus for all relevant socket event fields; consider adding runtime validation (e.g., zod) for event payloads if they cross trust boundaries.

ThemeToggle finds a target container once and retries only once after 100ms. If the header or target container mounts later (slow render, conditional layout, or route transition), the toggle never appears because the component returns null permanently.

Render the toggle directly within the header layout (prop or slot), or keep observing the DOM (MutationObserver / repeated retry) until the target container exists. Also re-run lookup when route/layout changes.

Several smoke/UI tests call `page.goto` and `request.get` with literal `http://localhost:7000/7001` URLs. This bypasses Playwright’s `baseURL` and any environment configuration used elsewhere (e.g., projects-flow uses API_URL/FRONTEND_URL). If the app runs on different ports in CI, behind a proxy, or in parallel test environments, these tests hit the wrong host and fail even when the app is healthy. This is a portability and maintainability problem for test execution.

Centralize base URLs (e.g., in Playwright config or shared test helper). Use `page.goto('/')` with `baseURL`, and build API calls from `process.env.API_URL` or `test.use({ baseURL })`. Align smoke/ui-health tests with the configuration used in other e2e tests.

In `smoke.spec.ts`, `test.beforeEach` attaches console listeners using throwaway arrays, and each test attaches another set of listeners with the actual arrays used for assertions. This leads to duplicated console event handling and duplicated log output, which can confuse debugging and, if counting logic evolves, can cause false positives.

Attach console guards only once per page. Either remove the `beforeEach` call and keep per-test attachment, or keep `beforeEach` and pass the arrays to it via a fixture so tests don’t attach additional listeners.

Tests named “frontend loads without errors” and “UI should load without console errors” only fail when a message matches `ERROR_DENYLIST`. Any other `console.error`/`console.warn` is logged but does not fail the test, which undermines the intent of the smoke checks and allows regressions to slip through unnoticed.

Fail on any console error/warning by default, and add a small allowlist for known benign messages if needed. If partial filtering is intentional, rename the tests and document the scope.

The UI tests wait for `networkidle`, which requires no ongoing network activity. This app uses WebSocket/Socket.IO connections (and potentially long-polling), which can keep the network busy and prevent the idle condition from ever being met. This makes tests flaky or slow in real-time apps.

Replace `networkidle` with a deterministic readiness check (e.g., `#root` visible, a `data-testid` ready flag) or use `domcontentloaded` plus targeted waits for key UI elements.

The WebSocket test checks connectivity by running `fetch` in the browser against the Socket.IO polling endpoint and treats HTTP 400 as success. Browser fetches are subject to CORS; if the backend doesn’t allow `localhost:7001`, the test fails even if the socket is reachable. Conversely, a 400 response doesn’t prove the real-time handshake works, so the test can pass while the WebSocket is actually broken.

Use Playwright `request` (Node context) or open a real WebSocket (e.g., `new WebSocket`) and assert `onopen`/`onerror`. Validate expected status codes and paths rather than accepting 400 as success.

normalizeTimestamp stringifies all inputs before parsing. If the API returns numeric epoch milliseconds (a common format), converting to a string and passing it to Date(string) is not reliably parsed by the JS Date parser, which can yield Invalid Date. The function then falls back to the current time, causing log entries to display with incorrect timestamps and potentially appear out of order.

Handle numeric values explicitly before stringification. For example: if (typeof value === 'number') return new Date(value).toISOString(); then handle Date instances directly; only string-parse when the input is a string. Consider parsing numeric strings with Number(value) if they match /^\d+$/.

The download handler uses a root-relative URL (`/api/logs/archives/...`). In Electron (file:// origin) or when the API base URL is configured to another host/port, this relative URL points to the wrong origin and the download fails or opens a blank page.

Build the download URL using the same base as apiRequest/getApiBaseUrl. If getApiBaseUrl is empty for same-origin, fall back to window.location.origin. Ensure the result is an absolute URL before calling window.open.

The toast dismiss button contains only an icon and no accessible name. Screen readers will announce it as an unlabeled button, making it difficult for keyboard and assistive tech users to dismiss notifications.

Add an accessible name via aria-label (e.g., aria-label="Dismiss notification") or include visually-hidden text inside the button.

The refresh callback in SessionDetailDialog calls itself instead of invoking the detail refetch function. This creates immediate infinite recursion and a stack overflow when users click Refresh or trigger actions that call handleRefresh (start/pause/cancel). As a result, the dialog cannot refresh, and the UI can crash/freeze on a common user action path.

Rename the callback to avoid shadowing and call the real refetch method. For example: `const handleRefresh = useCallback(() => { refetch(); refreshSession(); }, [refetch, refreshSession]);` Then verify all callers use the updated handler.

The session detail and issues fetch hooks do not guard against out-of-order responses. If a user closes the dialog or switches sessions quickly, earlier requests can resolve later and overwrite the state with the previous session’s data. This can show incorrect issues, metrics, or timeline for the currently selected session.

Add request sequencing or AbortController per fetch and only set state if the response matches the latest sessionId/open state. Example: capture `const currentId = sessionId` before the request and compare in `then`/`finally`, or use an `abort` signal and ignore aborted requests.

When dependency status fetch fails, the component only logs to console and leaves `status` null. The UI then shows a perpetual loading spinner with no error state or retry hint, which is misleading and prevents users from understanding why the dependencies never appear.

Add an error state and render an error message with a retry button when fetch fails. Optionally preserve and display the last-known status if available.

When the session fetch fails (network error/404), the dialog sets `session` to null and `loading` to false, but the render condition `loading || (sessionId && !session)` still evaluates true. This leaves the dialog body stuck on the loading spinner indefinitely with no error or empty-state fallback, so users can’t tell the request failed or why.

Introduce an explicit error state (e.g., `error: string | null`). Render loading only when `loading === true`. If `sessionId` exists and `session === null` and `error` is set, show an error/empty state with a retry button.

The JSON viewer uses clickable `div` containers for expand/collapse, which are not focusable and don’t respond to keyboard activation. This prevents keyboard-only users or screen readers from interacting with the viewer. A similar pattern appears in TagManager where tags are clickable `div`s for editing. These should be buttons or have proper ARIA roles, tabindex, and key handlers.

Replace interactive `div`s with `<button>` elements, add `aria-expanded`, and handle Enter/Space activation. Apply the same pattern to TagManager’s clickable tags so they can be edited via keyboard.

If the CLI status request fails, the component logs the error but leaves `status` null and `loading` false. The UI then falls back to the “Not found. Install with …” messaging, which is misleading when the real problem is a network/API failure. There’s no visible error state for the user.

Add an explicit `error` state and render an error banner or placeholder when fetch fails. Avoid showing install instructions unless the API confirms `available === false`.

The typeahead creates an `AbortController` and aborts the previous request, but never passes the controller’s `signal` to `apiRequest`. This means earlier in-flight requests continue and can resolve after newer ones, overwriting suggestions and `expandedPath` with stale data. The subsequent abort check also uses the latest controller, so it can’t detect outdated responses reliably.

Pass `signal: fetchControllerRef.current.signal` to `apiRequest` (and ensure `apiRequest` forwards it to `fetch`). Alternatively, track a requestId and ignore responses that aren’t the latest. Also clear `isLoading` only for the latest request.

The copy control renders as an icon-only button without an accessible name. Screen readers will announce an unlabeled button, and keyboard users won't know what it does. This is a WCAG failure for interactive controls and makes the prompt/response copy feature effectively invisible to assistive tech.

Add an explicit accessible name to the button (e.g., aria-label="Copy prompt" / "Copy response") or include visually-hidden text. Consider passing a label prop to CopyButton so each usage can specify context.

UnifiedSessionTimeline accepts a fileReviews map but never uses it; the prop is destructured into a throwaway variable. If events do not embed review data (prompt/response/grade), the timeline has no way to show it even when fileReviews is provided, causing missing details in the UI.

Merge fileReviews into timeline nodes during processEventsForTimeline (e.g., look up by file path or event id) or remove the prop and update callers to avoid passing unused data.

ExpandablePromptView computes tryPrettifyJson via useMemo on every render, even when the response section is collapsed. For large responses or many file reviews, this eagerly parses all responses and can cause noticeable render stalls even if the user never expands them.

Defer tryPrettifyJson until the response section is expanded (e.g., compute inside the expanded branch or gate the useMemo on showResponse). Consider a lazy state initialized on first expand.

Settings and defaults include a timeout value, but handleConfirm always sends timeout: 300000 regardless of settings or defaultConfig. There is no state variable for timeout, so any configured timeout is silently ignored.

Track timeout in component state initialized from defaultConfig (and optionally expose a UI input). Pass the state value through to onConfirm, converting units consistently with server expectations.

When switching CLI in ManualSessionDialog, the model is set to hard-coded strings ('opus' or 'gpt-5.2-codex') instead of using the configured defaults or available model list. This can select a model that doesn't exist in the current environment, causing session creation to fail or use an unsupported model.

Use getDefaultModel(newCli) or the first available model from getModelIdsForCli(newCli). Preserve the current model if it remains valid after CLI changes; otherwise fall back to defaults.

File preview state retains the previous totalCount while a new preview is loading. The Create button and handleCreate logic only check totalCount, not loading state, so users can create a session using outdated counts while a new preview is in-flight. This allows sessions to be created with patterns that now match zero files.

Clear counts when starting a new preview or set totalCount to 0 while loading. Disable the Create button and block handleCreate while filePreview.loading is true, and ensure the preview result used is the latest.

totalFiles and totalTokens are set once from the AI response and never recomputed when groups are removed. The preview header can therefore display stale totals that no longer match the remaining groups.

Derive totals from the current groups array (useMemo or inline reduce) instead of storing them separately, or update totals when groups change.

The Play/Start control is disabled unless there are created or paused sessions, but the Start All flow explicitly includes queued sessions in its count. This creates a mismatch where queued sessions cannot be started from the footer even though the dialog implies they will be included. If the system can leave sessions in a queued state (e.g., due to max parallel limits), users can end up unable to start them via the UI.

Align canPlay with the Start All logic by including queued sessions (or remove queued from Start All if they are not meant to be manually started). Update the tooltip text to match the actual behavior.

Several icon-only buttons do not provide an accessible name (aria-label or visually hidden text). Tooltips are not a reliable substitute for screen reader users. This affects session controls, copy buttons, and action menu triggers.

Add aria-labels or visually hidden text to all icon-only buttons (e.g., 'Start all sessions', 'Pause all sessions', 'Copy CLI command').

The footer returns null when there are no sessions or issues, but the CLI process polling effect still runs every 2 seconds. This causes unnecessary API traffic and CPU usage while the UI is not visible.

Gate polling behind a state condition (e.g., only when footer is visible or when sessions exist) or move polling to a higher-level component that stays visible.

Several selection controls are implemented as plain buttons without exposing selected state (aria-pressed, role=radio, or a RadioGroup). This prevents assistive technologies from understanding which option is currently active for selection-method, access-mode, and provider/reasoning choices.

Use a RadioGroup/ToggleGroup component or apply role="radio" with aria-checked (and a group label), or add aria-pressed to button toggles.

Both preview steps fire asynchronous requests without aborting or guarding against out-of-order responses. When the user rapidly changes CLI/model/options, earlier requests can resolve after later ones and overwrite state with a preview that no longer matches the current configuration.

Add an AbortController or request-id guard so only the latest request can set state. Clear preview when a new request starts to avoid showing mismatched data.

The FixPreviewStep exits early when issues are empty, but it does not clear the existing preview state. If a user deselects all issues after a preview was generated, the UI can still display the old CLI/prompt data even though there are no issues to fix.

When issues.length === 0 (or projectId is missing), reset preview and copied state to null and render an empty-state message.

The triage action handlers (approve/won’t-fix) wrap their async operations in try/finally but do not catch errors. If the backend call fails, the error bubbles up (potentially as an unhandled rejection) and the user receives no feedback. This leaves the UI in a confusing state: the action appears to have done nothing without explanation, and the error may disrupt the app or error boundary.

Add catch blocks to handle failures and show a user-visible error (toast or inline banner). Ensure actionLoading is reset and the issue is not auto-advanced when the API call fails. Optionally log details for diagnostics.

The global keydown handler only ignores events from input/textarea elements. When the notes modal is open, focus can be on buttons or the overlay, so pressing Enter/Space/Backspace can trigger triage actions (approve/won’t-fix/delete) while the user is trying to save/cancel notes. This can accidentally change issue status during note editing.

Disable global shortcuts when showNotesModal is true, or check if the event target is within the modal container (use a ref and `contains`). Also consider ignoring events when a dialog/overlay is open.

The API helper attaches an abort listener to the caller-provided AbortSignal but never removes it on success, and it doesn’t check if the external signal is already aborted before starting the request. This means a request can still be sent even when the caller already aborted, and each call adds a listener that persists for the life of the signal. Over time this can lead to unexpected requests (cancelled operations still executing) and memory/event listener leaks if signals are reused.

Before starting the fetch, check externalSignal?.aborted and throw an AbortError immediately. Use a try/finally block to always remove the external abort listener and clear the timeout on both success and error. Consider adding the listener with `{ once: true }` and explicitly calling `externalSignal?.removeEventListener('abort', handleExternalAbort)` in the success path as well.

Both the edit and new-group preview effects abort in-flight requests and return early when include patterns become empty, but they never reset `previewLoading` in that branch. Because the previous request’s `finally` only clears loading when not aborted, the loading state can remain true indefinitely. This leaves the UI showing a perpetual spinner and disables the Save/Add buttons even after the user re-enters valid patterns.

In both effects, explicitly set `previewLoading(false)` when include patterns are empty. Also consider tracking a request id and always clearing loading when aborting due to input changes, or clearing loading in the cleanup path.

The selected group IDs are stored independently from the `groups` prop and are never pruned when groups are removed outside of batch deletion. If a user selects a group and then removes it via the per-item delete button, the stale ID remains selected. This causes the header to show a non-zero selection count and can prompt the user to delete groups that no longer exist.

Add a `useEffect` that prunes `selectedGroups` to only IDs that still exist in `groups`, or clear selection when `groups` change as a result of single-item removal.

Settings values are flattened in `SettingsPage` before being passed to the Settings framework, producing keys like `updates.enabled`. However, the `showIf` predicates for the Updates settings read `values.updates?.enabled`, which will be undefined if values are flat. This likely prevents GitHub token/interval/auto-download/repository fields from appearing even when Updates is enabled.

Align `showIf` with the flattened key shape (e.g., `values['updates.enabled'] === true`) or update SettingsPage/SettingsFramework to pass nested values into `showIf` consistently.

When a step uses `waitFor`, clicking Next attaches a socket listener and returns without disabling the Next button. If the user clicks Next multiple times while waiting, multiple listeners are registered. When the event fires, each handler calls `moveNext`, which can skip steps and leave orphan listeners if only the last handler is cleaned up.

Disable the Next button while waiting, or guard with a flag that ensures only one listener is attached per waitFor step. Ensure cleanup removes any existing listener before adding a new one.

The helper intended to strip invalid filename characters uses a character class with `-_`, which forms a range that unintentionally allows many punctuation characters (including `/`, `\`, and `.`). That means path separators and `../` sequences are preserved. If this helper is used to build zip entry names or download filenames (as its name suggests), it can produce nested paths or traversal-style names rather than a safe basename.

Fix the regex by escaping the hyphen or placing it at the end (e.g. `/[^a-z0-9_\s-]/g`), and explicitly strip `/` and `\` plus leading dots and `..` segments. Add unit tests to assert that path separators and dot segments are removed.

The module-level documentation says ZIP exports are comprehensive and include both Markdown and JSON files per issue, but the implementation defaults to `contentFormat: 'json'` and writes only one format per zip. There is no code path that produces a mixed-format ZIP, so the stated behavior is not achievable.

Either implement a dual-format ZIP path (write both JSON and Markdown indexes plus per-issue files) when no explicit content format is chosen, or update the docs/UI copy to reflect the single-format behavior.

The IssueStatus type includes `done`, and other summaries treat `done` as resolved, but the JSON export’s `summary.byStatus` does not include it. Any `done` issues will be missing from the status breakdown, making the summary inconsistent with totalIssues and other export formats.

Add a `done` counter to `summary.byStatus` or compute the summary dynamically from the issues list so all statuses are included.

Both FixesPage and CodeReviewPage access `socketActions.socket` directly in the useEffect dependency array, but the effect body uses optional chaining (`socketActions?.socket`), implying socketActions may be undefined during initialization. If the hook returns undefined/null before the socket is ready, the dependency access will throw during render and crash the page.

Change dependencies to use a safely derived value (e.g., `const socket = socketActions?.socket;` then `[socketActions, socket, fetchData]`) or ensure the hook always returns a non-null actions object.

The session-config fetch is asynchronous and unguarded. If the user switches issues quickly, a slower request for the previous issue can resolve after a newer selection and overwrite `sessionData` with the wrong session config. This leads to incorrect metadata in the details panel and can mislead triage decisions.

Add a request token/sequence or AbortController per fetch. Before calling `setSessionData`, verify the response still matches `selectedIssue.detectedInSession` at the time of resolution. Alternatively store the requested sessionId in a ref and ignore out-of-date responses.

File content is fetched asynchronously with no guard that the response corresponds to the currently selected file. Rapidly switching files or issues can cause older responses to overwrite `codeFileContent`, leading to mismatched content in the code viewer.

Track the requested file path in a ref or use an AbortController. When the promise resolves, compare against the current `selectedCodeFile` (and projectId) before updating state. Ignore out-of-date responses.

The filter options are memoized but the dependency array omits `filterConfidence`. The confidence filter’s `Select` is controlled by a value captured in the memo closure; without recomputation, the UI can display a stale value or fail to reflect changes from user interaction or persisted state.

Add `filterConfidence` (and, for consistency, `setFilterConfidence`) to the `useMemo` dependency list, or avoid memoizing the filter options altogether.

Batch delete and batch tag operations use `Promise.all`, which rejects on the first failure. If some requests succeed and one fails, the catch path shows an error but does not reconcile successes (no refetch for deletes; no refetch for tagging). This can leave the UI out-of-sync with the server until a manual refresh.

Use `Promise.allSettled` and apply successful results to local state while surfacing partial failures. For tagging, always refetch on partial success. For deletion, remove only successfully deleted IDs and show a warning that some failed.

Whenever `selectedIssue` changes, the code automatically fetches the primary file contents even if the code panel is closed. This causes unnecessary network and server load for users who are just browsing issues and never open the code viewer.

Only fetch file content when `showCodePanel` is true, or lazily fetch when the panel is opened. Optionally prefetch with an idle callback or debounce if you still want fast open times.

Several async handlers await network calls and then call setProjects using the captured `projects` array from the render that started the request. If any other update happens while the request is in flight (socket scan-complete, another edit/delete, or another add), the later setProjects call can overwrite those newer changes. This can cause lost updates or stale stats in the UI. Confirm by checking whether socket scan-complete events and user edits can overlap with these requests.

Convert these updates to functional setState: `setProjects(prev => ...)` and merge updates by ID. Alternatively, refetch projects after each mutation and avoid using captured `projects` after an await.

The `refreshing` state stores only one project ID, but the UI allows multiple refreshes to be triggered. When project A and B refresh concurrently, the last trigger overwrites the refreshing id and the first completion clears it entirely, even if the second is still running. This results in incorrect spinner/disabled states and can mislead users about refresh status.

Track refreshing as a Set/Map of project IDs and add/remove ids per request; or prevent concurrent refreshes globally and keep a single refresh lock.

The refresh-and-copy flow waits a fixed 500ms after calling handleRefreshStats before copying. If refresh is asynchronous or takes longer (as implied by socket scan-complete events), the copy can run before the refresh finishes, resulting in stale content or token counts. Confirm by checking whether /refresh returns before the scan completes.

Wait for the scan-complete event for that project or have /refresh return only after completion. Then trigger copy from that completion handler.

The Lines column checks `project.stats?.totalLines` as a truthy value, which treats 0 as falsy and renders a dash. For empty projects or projects with zero lines, the UI will incorrectly show no data instead of 0.

Check for undefined/null explicitly: `project.stats?.totalLines != null` and render 0 when present.