Build Full Web Alternative to Desktop

[OBenner]

Build a complete web-based alternative to the Electron desktop application. The desktop version is fully functional with all features, while the web version currently only implements login and usage statistics dashboard. This spec enables the web application to achieve feature parity with the desktop version by connecting existing backend routes, implementing WebSocket-based agent output streaming, and porting missing UI components.

Summary by CodeRabbit

• New Features

  • Web terminal (interactive shell) via WebSocket, Kanban board with drag‑and‑drop, File Explorer, Task Create wizard, and new pages: Changelog, Insights, Roadmap, Git Operations, Terminal.

• Improvements

  • Real‑time agent event broadcasts, WebSocket authentication, standardized/sanitized logging, additional health endpoints, refined error messages, expanded settings and usage/analytics views, safer type annotations.

• Tests

  • Large suite of unit/integration/E2E tests and Playwright configuration added.

• Documentation

  • Added security scan report and scan configuration.

[pantoaibot]

Auto review disabled due to large PR. If you still want me to review this PR? Please comment /review

[github-advanced-security]

CodeQL found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

[OBenner]

/review

[pantoaibot]

Limit exceeded. Please contact at support@getpanto.ai to increase the limit.

[OBenner]

/review

[pantoaibot]

Limit exceeded. Please contact at support@getpanto.ai to increase the limit.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds terminal WebSocket support and terminal manager, WebSocket broadcasting for agent events, many new frontend pages/components (Kanban, Terminal, Insights, Changelog, File Explorer, Settings), broad typing modernizations (PEP 604), shared route utilities, DB helpers, usage-tracking middleware, Bandit config/report, and extensive tests/tooling updates.

Changes

Cohort / File(s)	Summary
Terminal & WebSocket (backend) apps/web-backend/api/websocket.py, apps/web-backend/services/terminal_manager.py	Adds /ws/terminal endpoint, WebSocket auth, per-connection subscriptions and claims, terminal session lifecycle, PTY/subprocess cross-platform support, output reader, and session management APIs.
Agent Runner & Broadcasting (backend) apps/web-backend/services/agent_runner.py, apps/web-backend/api/routes/agents.py	Introduces lazy WebSocket broadcasting hooks, canonical spec_id usage, emits execution/progress/log/error events during agent lifecycles, and updates related function signatures/return types.
Shared route utilities (backend) apps/web-backend/api/routes/shared.py	New centralized helpers: sanitize_log, project/specs path resolution, spec discovery, subtask counting and progress utilities used by multiple routes.
Middleware, Config, Security & DB (backend) apps/web-backend/core/middleware.py, apps/web-backend/core/config.py, apps/web-backend/core/security.py, apps/web-backend/core/database.py, apps/web-backend/.bandit, apps/web-backend/SECURITY_SCAN_REPORT.md	Adds usage-tracking guards, optional dotenv loading, global cached settings, websocket token verifier, DB dependency/init/close helpers, Bandit config and scan report documentation.
Routes & Models (backend) apps/web-backend/api/routes/..., apps/web-backend/api/models/...	Multiple endpoints refactored to use shared helpers, added health endpoints (specs/tasks), removed DB deps from OAuth callbacks, standardized error messages, and migrated many type hints to modern union syntax and explicit nullability.
Migrations & DB scripts apps/web-backend/migrations/...	Formatting and typing updates (PEP 604 unions), import reorder, and normalized quoting in Alembic migration files.
Services & Utilities (backend) apps/web-backend/services/*	Type modernizations and formatting across services (git_service, usage_tracker), plus new terminal_manager and broadcast/no-op wiring in agent_runner.
Frontend Terminal & Tests apps/web-frontend/src/components/Terminal.tsx, apps/web-frontend/src/components/__tests__/Terminal.test.tsx, apps/web-frontend/src/pages/TerminalPage.tsx	Replaces placeholder with xterm.js-backed TerminalComponent, WebSocket protocol (input/output/resize/ping), reconnection UI, session/token params, and tests covering lifecycle and messaging.
New Frontend Pages & Routing apps/web-frontend/src/pages/*, apps/web-frontend/src/App.tsx	Adds many feature pages (Insights, Kanban, Roadmap, Changelog, Files, GitOperations, TaskCreate, TaskDetail, Settings, UsageDashboard, TerminalPage) and updates routing with wrapper components.
Kanban & DnD UI (frontend) apps/web-frontend/src/components/KanbanBoard.tsx, apps/web-frontend/src/components/SortableTaskCard.tsx, apps/web-frontend/src/components/__tests__/KanbanBoard.test.tsx	New drag-and-drop Kanban board, sortable task card wrapper, drag handlers, visual feedback and comprehensive mocked DnD tests.
File Explorer & UI primitives (frontend) apps/web-frontend/src/components/FileExplorer.tsx, apps/web-frontend/src/components/ui/*	Adds FileExplorer and multiple UI primitives (Input, Label, Switch, Textarea, CardHeader/CardTitle, Checkbox export, Tooltip exports, etc.) and adjusts exports.
Frontend API & WebSocket client apps/web-frontend/src/api/client.ts, apps/web-frontend/src/api/websocket.ts	Adds ApiClient.createTask, minor createWebSocketClient signature tweak, and sanitize helper usage in client logs.
Tests, Tooling & Manifests apps/web-frontend/package.json, apps/web-frontend/playwright.config.ts, apps/frontend/src/__tests__/setup.ts, apps/frontend/src/main/__tests__/ipc-handlers.test.ts	Expands frontend package.json with testing/tooling, adds Playwright config, test setup sanitizeForLog, CI timing tweaks, and many new unit/integration/E2E tests across backend and frontend.
PTY dependency & requirements apps/web-backend/requirements.txt	Adds optional pyprocess (ptyprocess) dependency for Unix PTY support with platform condition.
Miscellaneous numerous files across backend & frontend	Wide-ranging formatting, quote normalization, import reordering, PEP 604 typing migrations, small helper additions (run_test_2), and many non-functional refactors.

Sequence Diagram(s)

sequenceDiagram
    participant Browser as Client
    participant Frontend as Frontend App
    participant WS as WebSocket Server
    participant Auth as Auth (verify_websocket_token)
    participant TermMgr as TerminalManager
    participant PTY as PTY/Subprocess

    Browser->>Frontend: Open terminal page (session_id)
    Frontend->>WS: WS connect /ws/terminal?session_id=...&token=...
    WS->>Auth: verify_websocket_token(token)
    Auth-->>WS: user_claims
    WS->>TermMgr: create_session(session_id, working_dir, shell, user_claims)
    TermMgr->>PTY: start PTY/subprocess
    PTY-->>TermMgr: output ready
    TermMgr-->>WS: session created / output messages
    WS-->>Frontend: connected status / forward output

    Browser->>WS: send input (type: input, data)
    WS->>TermMgr: write_input(session_id, data)
    TermMgr->>PTY: write stdin
    PTY-->>TermMgr: output
    TermMgr-->>WS: forward output
    WS-->>Browser: display output

Loading

sequenceDiagram
    participant User as Client
    participant API as HTTP API
    participant Runner as AgentRunner
    participant WS as WebSocket Broadcaster
    participant Store as Task Store

    User->>API: POST /agents/run (spec_id, agent_type)
    API->>Runner: start_agent_task(..., project_dir)
    Runner->>WS: _init_websocket_broadcast()
    Runner->>WS: broadcast_execution_event(START)
    Runner->>Runner: run_agent_async phases (planner,coder,qa...)
    Runner->>WS: broadcast_execution_event(PROGRESS/LOG/ERROR/COMPLETE)
    Runner->>Store: update task status
    WS-->>User: events delivered to subscribers

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Suggested labels

feature

Poem

🐰 I hopped through sockets, PTYs, and code,

Cards now move where busy rabbits goad,
Tests nibble carrots, security peeks,
Pages bloom features in tidy streaks,
A rabbit cheers: "Merge time — onward we road!"

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title clearly summarizes the main objective of building a complete web application as an alternative to the desktop version.
Docstring Coverage	✅ Passed	Docstring coverage is 90.56% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch auto-claude/141-build-full-web-alternative-to-desktop

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

General approach: ensure that any log entry that might contain user-controlled data goes through a clear sanitization step that (a) strips control characters (including newlines) and (b) truncates length before being sent to a logging sink. For the specific CodeQL alert in the Vitest setup, we should avoid using the original args array directly when logging and instead construct a fully sanitized array/string first, then log that sanitized data.

Best targeted fix in apps/frontend/src/__tests__/setup.ts:

• Keep sanitizeForLog as the single sanitization routine.
• In the console.error override:
  • Compute a sanitized copy of the arguments up front, e.g. const sanitizedArgs = args.map(sanitizeForLog);
  • Derive message from sanitizedArgs[0] rather than from the raw args[0].
  • When actually logging, always pass only sanitized data to originalConsoleError, instead of reusing args.
• To satisfy CodeQL more explicitly, we can log a single sanitized string (which is already done) but ensure no tainted value flows into it unprocessed.

This change is fully contained within setup.ts and does not alter functionality: logs still appear only when [TEST] is present in the first argument, and the content remains the same after sanitization.

No changes are necessary in apps/frontend/scripts/download-python.cjs, apps/frontend/src/main/app-updater.ts, or apps/frontend/src/main/terminal/pty-daemon.ts for this particular log-injection issue, because they already sanitize control characters/newlines and truncate messages before logging.

[coderabbitai]

Actionable comments posted: 76

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (9)

apps/web-backend/core/oauth.py (1)

52-53: ⚠️ Potential issue | 🟡 Minor

Docstring is now inconsistent with module behavior.

The function's docstring states "Call this during app startup," but configure_oauth() is now auto-invoked on import. If a developer follows the docstring and explicitly calls it during startup, OAuth providers will be registered twice (Authlib overwrites silently, so no crash, but wasteful).

Either update the docstring to document the auto-configuration behavior, or remove the module-level call and keep explicit startup invocation.

📝 Proposed fix: update docstring to reflect auto-configuration

 def configure_oauth():
     """
     Configure OAuth clients for GitHub and GitLab
 
-    Registers OAuth providers with Authlib. Call this during app startup.
+    Registers OAuth providers with Authlib. Called automatically on module import.
     """

apps/web-backend/services/git_service.py (2)

80-88: ⚠️ Potential issue | 🟠 Major

Add timeout to httpx.AsyncClient to prevent indefinite hangs.

All methods create AsyncClient without a timeout. If GitHub/GitLab APIs are slow or unresponsive, requests will hang indefinitely, potentially exhausting async task capacity and degrading service reliability.

🛡️ Proposed fix: Add timeout to AsyncClient

Apply this pattern to all AsyncClient instantiations in this file (lines 81, 122, 173, 210, 253, 301):

-            async with httpx.AsyncClient() as client:
+            async with httpx.AsyncClient(timeout=30.0) as client:

Alternatively, define a class-level constant for consistent timeout configuration:

class GitService:
    # ...
    DEFAULT_TIMEOUT = 30.0  # seconds
    
    # Then use:
    async with httpx.AsyncClient(timeout=self.DEFAULT_TIMEOUT) as client:

---

257-260: ⚠️ Potential issue | 🟡 Minor

Hardcoded "main" ref may cause failures for GitLab repos with different default branches.

When ref is not provided, the code defaults to "main" for GitLab. Repositories using "master" or custom default branches will receive a 404 error.

Consider either:

1. Fetching the project's default_branch from get_repository() first
2. Documenting this limitation in the docstring

📖 Option: Document the limitation

         Args:
             owner: Repository owner username
             repo: Repository name
             path: File path in repository
-            ref: Git reference (branch, tag, commit SHA). Defaults to default branch.
+            ref: Git reference (branch, tag, commit SHA). Defaults to default branch
+                for GitHub; defaults to "main" for GitLab (may require explicit ref
+                for repos with different default branches).

apps/web-backend/test_websocket_imports.py (1)

9-45: 🧹 Nitpick | 🔵 Trivial

Make path resolution independent of working directory.
files_to_check uses relative paths, which fail when the script is run from repo root or CI. Resolve paths relative to __file__ for robustness.

🛠️ Proposed refactor

-import os
-import sys
+import os
+import sys
+from pathlib import Path
@@
-    files_to_check = ["api/models/agent_event.py", "api/websocket.py", "main.py"]
+    base_dir = Path(__file__).resolve().parent
+    files_to_check = [
+        base_dir / "api/models/agent_event.py",
+        base_dir / "api/websocket.py",
+        base_dir / "main.py",
+    ]
@@
-    for filepath in files_to_check:
-        if not os.path.exists(filepath):
-            print(f"✗ File not found: {filepath}")
+    for filepath in files_to_check:
+        if not filepath.exists():
+            print(f"✗ File not found: {filepath}")
             all_passed = False
             continue
@@
-        valid, error = check_syntax(filepath)
+        valid, error = check_syntax(filepath)

apps/web-backend/test_websocket_realtime.py (1)

66-99: ⚠️ Potential issue | 🟠 Major

WebSocket tests will fail because the /ws/agent-events endpoint enforces token authentication.

The endpoint requires a valid JWT token via the token query parameter (see websocket.py line 216 and verify_websocket_token validation). The test file connects to WEBSOCKET_URL = "ws://localhost:8000/ws/agent-events" without any token, so the server will immediately reject the connection with a WS_1008_POLICY_VIOLATION closure code.

All test connections across this file must include a valid token in the query parameter. Update WEBSOCKET_URL to include auth, or modify the connection calls to append the token: websockets.connect(f"{WEBSOCKET_URL}?token={token}").

apps/web-backend/core/config.py (1)

98-105: 🧹 Nitpick | 🔵 Trivial

Consider avoiding a fixed module-level settings instance.

settings = get_settings() freezes env-driven config at import time, which makes test overrides (or late env changes) hard to apply. Consider keeping get_settings() as the canonical access point or providing a reload_settings() helper that clears the cache and refreshes settings.

🔧 Example helper to refresh settings

 `@lru_cache`
 def get_settings() -> Settings:
     """Get cached settings instance"""
     return Settings()

+# Optional helper for tests / runtime overrides
+def reload_settings() -> Settings:
+    get_settings.cache_clear()
+    global settings
+    settings = get_settings()
+    return settings
+
 # Global settings instance
 settings = get_settings()

apps/web-backend/services/usage_tracker.py (1)

226-266: ⚠️ Potential issue | 🟠 Major

Align get_endpoint_stats return shape (requests vs total_requests).

When no data (or on error), the method returns "requests", but normal paths return "total_requests". This inconsistency can break callers that expect a stable key.

✅ Suggested fix

-                return {
-                    "endpoint": endpoint,
-                    "period": period,
-                    "requests": 0,
-                    "methods": {},
-                }
+                return {
+                    "endpoint": endpoint,
+                    "period": period,
+                    "total_requests": 0,
+                    "methods": {},
+                }
@@
-            return {
-                "endpoint": endpoint,
-                "period": period,
-                "requests": 0,
-                "methods": {},
-                "error": str(e),
-            }
+            return {
+                "endpoint": endpoint,
+                "period": period,
+                "total_requests": 0,
+                "methods": {},
+                "error": str(e),
+            }

apps/web-backend/main.py (1)

82-90: 🧹 Nitpick | 🔵 Trivial

Move middleware import to top of file.

The UsageTrackingMiddleware import at line 83 is placed mid-file after other code. For consistency and readability, move it to the imports section at the top.

apps/web-backend/api/websocket.py (1)

592-598: 🧹 Nitpick | 🔵 Trivial

terminal_websocket endpoint not exported in __all__.

The new terminal_websocket endpoint is defined but not included in the __all__ list. While this doesn't affect functionality (the router registration handles routing), it creates inconsistency with the module's public API documentation.

♻️ Suggested fix

 __all__ = [
     "router",
     "manager",
     "broadcast_execution_event",
     "broadcast_log_event",
     "broadcast_error_event",
+    "terminal_websocket",
 ]

🤖 Fix all issues with AI agents

In `@apps/backend/project/config_parser.py`:
- Around line 9-11: The import block in config_parser.py is unsorted causing
Ruff I001; reorder the standard-library imports alphabetically so that "from
pathlib import Path" comes before "import json" and "import tomllib"
(standard-library imports first, alphabetized), then leave any third-party
imports after; update the top-of-file import section accordingly so imports are
grouped and sorted per Ruff rules.

In `@apps/frontend/src/main/__tests__/ipc-handlers.test.ts`:
- Around line 303-309: Replace the fixed setTimeout waits with a
vi.waitFor-based poll that waits until the expected post-save state is
observable: after calling ipcMain.invokeHandler("project:add", {},
TEST_PROJECT_PATH) use vi.waitFor to poll for the side-effect the
fire-and-forget saveAsync produces (for example, a persisted file, an emitted
event, or a change observable via ipcMain or the project list API) instead of
waiting a fixed 500ms/1000ms; update the assertions to run once vi.waitFor
completes so the test reliably waits for the saveAsync flush.

In `@apps/web-backend/api/routes/git.py`:
- Line 70: The github_callback function declares an unused dependency db:
Session (via get_db)—either persist the GitHub token as the TODO suggests or
remove the dead parameter; to fix, locate github_callback and either (A)
implement token storage using the injected db: create or update the appropriate
token model (e.g., GitHubToken or UserToken) with the returned access token and
user identifier, add db.add(...) and db.commit() (and any necessary model
import), or (B) if you won't store the token now, remove the db: Session =
Depends(get_db) parameter from github_callback and delete any related unused
imports to eliminate dead code.

In `@apps/web-backend/api/routes/specs.py`:
- Around line 279-297: The specs_health endpoint currently returns internal
filesystem information via _get_project_dir(); remove project_dir from the
public response (or only include it when settings.DEBUG is True) to avoid
exposing server paths to unauthenticated callers; update the return payload in
specs_health to omit project_dir unless wrapping its inclusion in an explicit
debug gate that checks settings.DEBUG, leaving specs_dir_exists and other
non-sensitive keys unchanged.

In `@apps/web-backend/api/routes/tasks.py`:
- Around line 280-298: The tasks_health endpoint currently returns internal
filesystem paths by including project_dir from _get_project_dir in the response;
change tasks_health to stop returning project_dir in production by removing
project_dir (and not calling _get_project_dir) or only include it when
settings.DEBUG (or equivalent debug flag) is true—keep specs_dir_exists (using
_get_specs_dir) but ensure any path string is only returned behind the debug
check so internal paths are not exposed to unauthenticated callers.

In `@apps/web-backend/api/routes/usage.py`:
- Around line 208-212: The API handlers currently include raw exception text in
the HTTPException detail (using detail=f"... {str(e)}"), which can leak
internals; keep the existing logger.error(..., exc_info=True) as-is but replace
the HTTPException detail with a generic message like "Internal server error
while fetching usage statistics" (or similarly generic text) and remove any
str(e) interpolation; apply the same change to the other handler around the
similar HTTPException at 286-291 so both logger.error and HTTPException calls
are separated (log full error, return generic detail).

In `@apps/web-backend/api/websocket.py`:
- Around line 436-446: The session_id taken from websocket.query_params is used
as a dict key and in logs but is not validated; add validation immediately after
retrieving session_id (the variable named session_id in websocket handling code)
to trim whitespace, enforce an allowed pattern (e.g., alphanumeric plus - and
_), and cap length (e.g., 256 chars); if the session_id fails validation, accept
then close the websocket with status.WS_1008_POLICY_VIOLATION and log a
sanitized message (use existing _sanitize_log or similar) rather than using the
raw value, and only proceed to use session_id (for dict keys or logs) when it
passes these checks.
- Around line 454-465: user_claims["working_dir"] is used directly when calling
terminal_manager.create_session (via session_id and working_dir) which permits
path traversal; before creating the session resolve and sanitize the path (e.g.,
os.path.realpath or pathlib.Path.resolve) and ensure it is inside an allowed
base directory (for example the user's home or a configured PROJECTS_ROOT) by
comparing commonpath or prefix, and if the check fails fall back to a safe
default (os.getcwd() or the user's home); apply this validation where
working_dir is read prior to calling terminal_manager.create_session so
terminal_manager.get_session / terminal_manager.create_session only ever receive
validated paths.

In `@apps/web-backend/core/database.py`:
- Around line 15-20: The current create_engine call unconditionally passes
pool_size and max_overflow which only apply to QueuePool; detect the DB dialect
from settings.DATABASE_URL (e.g., using SQLAlchemy's URL/make_url) and branch:
for SQLite use poolclass=StaticPool (and any SQLite-specific connect_args) and
omit pool_size/max_overflow, otherwise pass pool_size and max_overflow for
Postgres/QueuePool-compatible dialects; update the create_engine invocation
(symbol: create_engine and variable: engine) and import StaticPool accordingly
so production behaves like the test configuration.

In `@apps/web-backend/core/security.py`:
- Around line 163-190: verify_websocket_token currently raises HTTP 403 for
missing tokens but delegates to verify_token which raises HTTP 401 for invalid
tokens, producing inconsistent status codes; update verify_websocket_token to
normalize responses for WebSocket usage by catching HTTPException from
verify_token and re-raising it with status_code=status.HTTP_403_FORBIDDEN (or
alternatively modify verify_token to accept a websocket_context boolean and
return 403 when true); target the verify_websocket_token function and the
verify_token call to ensure both missing and invalid-token cases return the same
403 status for WebSocket authentication.

In `@apps/web-backend/main.py`:
- Around line 78-80: The SessionMiddleware is being initialized with a hardcoded
fallback secret_key ("dev-secret-key-change-in-production"); remove this weak
default and require a real SECRET_KEY: change the logic around
app.add_middleware / SessionMiddleware to not use the literal fallback, and
instead validate SECRET_KEY at startup (e.g., if SECRET_KEY is falsy and the app
is not running in a development/debug mode) raise an explicit RuntimeError or
exit with a clear message so the process fails fast in production; keep a safe
dev-only path that allows a locally provided debug secret when an explicit
debug/dev flag is set.

In `@apps/web-backend/migrations/versions/001_create_users.py`:
- Line 56: Remove the redundant index creation
op.create_index(op.f("ix_users_id"), "users", ["id"], unique=False) from the
migration and update the corresponding downgrade function to remove any
op.drop_index("ix_users_id", table_name="users") so the downgrade no longer
references a non-existent index; locate these calls in the migration module
(search for op.create_index with "ix_users_id" and any op.drop_index for
"ix_users_id") and delete the create_index line and its matching drop_index to
avoid creating/removing an index on the primary key.

In `@apps/web-backend/migrations/versions/002_create_repositories.py`:
- Line 61: Remove the redundant explicit index creation call
op.create_index(op.f("ix_repositories_id"), "repositories", ["id"],
unique=False) since the primary key is already indexed; delete that line from
the upgrade() implementation and also remove the corresponding
op.drop_index(...) call from the downgrade() implementation (or update
downgrade() to no longer reference ix_repositories_id) so the migration stays
consistent.

In `@apps/web-backend/SECURITY_SCAN_REPORT.md`:
- Around line 53-55: The Bandit report likely ignored the .bandit config; update
the documented scan command in SECURITY_SCAN_REPORT.md to explicitly load the
config and run from the repo root (e.g., use the Bandit CLI option to point to
the config file such as --configfile or -c with ".bandit"), and note that the
scan should be executed from the repository root so severity=MEDIUM in .bandit
is honored; update the example command and a one-line note clarifying that
.bandit must be present in the working directory or passed via --configfile to
ensure LOW findings are excluded.

In `@apps/web-backend/services/agent_runner.py`:
- Around line 189-193: The ternary-style conditional statement using
_broadcast_log_event (e.g., the line calling
_broadcast_log_event(spec_id=canonical_spec_id, log_line=f"Running planner agent
with model {model}", level="info") if _broadcast_log_event else None) should be
replaced with an explicit if-statement for readability and consistency with the
other broadcasts; change it to: if _broadcast_log_event: await
_broadcast_log_event(...). Apply the same refactor to the other similar
occurrences that call _broadcast_log_event (the calls constructing log_line
messages around planner/agent events) so all conditional broadcasts use explicit
if _broadcast_log_event: await _broadcast_log_event(...) instead of ternary
expressions.
- Line 102: Replace the incorrect lowercase type hint "any" with the typing
module's "Any": add "from typing import Any" if not already imported, and update
the return type annotations in the affected functions (the function with
signature ending "-> dict[str, any]:" and the get_task_status function) to "->
dict[str, Any]:" so both use the proper capitalized Any type.

In `@apps/web-backend/services/terminal_manager.py`:
- Around line 367-417: create_session currently returns a TerminalSession that
has not been started, forcing callers to remember to call await session.start();
change create_session to be async and await session.start() before
storing/returning it so sessions are ready-to-use; update the implementation in
TerminalManager.create_session to await TerminalSession.start() (referencing
TerminalSession.start and TerminalManager.create_session) and ensure callers
such as websocket.py that call get_session/create_session expect an
already-started session (adjust to await create_session where needed).
- Around line 282-289: The subprocess stdin write block in TerminalManager (the
branch checking self.process is not None and using self.process.stdin.write)
omits awaiting stdin.drain(), which can let the write buffer grow and delay
flushing; modify the write logic in that method to call await
self.process.stdin.drain() after write (or schedule an asynchronous periodic
flush if blocking is a concern), handle/catch asyncio.CancelledError and OSError
around the await, and ensure the method is async-compatible so awaiting drain
won't break callers.
- Around line 117-125: The call in _create_pty uses
ptyprocess.PtyProcess.spawn(self.shell, ...) but spawn expects argv as a
list/tuple; change the first argument to a list (e.g. [self.shell]) so
PtyProcess.spawn receives a sequence instead of a string; update the call in the
_create_pty function that invokes PtyProcess.spawn to pass [self.shell] (or an
appropriate list of argv) while leaving cwd/env/rows/cols unchanged.
- Around line 115-127: Replace deprecated asyncio.get_event_loop() with
asyncio.get_running_loop() in the async methods where the pty is created and at
the other occurrence (both inside async coroutines) to avoid DeprecationWarning;
specifically change the assignments that set loop = asyncio.get_event_loop()
(used before calling loop.run_in_executor in the block that defines _create_pty
and spawns ptyprocess.PtyProcess) to loop = asyncio.get_running_loop() so the
running loop is retrieved correctly from the coroutine context.

In `@apps/web-backend/tests/test_agents.py`:
- Around line 442-506: Remove or simplify the noisy always-passing
test_agent_test_summary: either delete the function entirely or convert its data
(the coverage dict and printed summary) into a module/class docstring or README
entry; if you want to keep it in tests for reference, replace the prints with a
non-executing docstring or mark the function as skipped (e.g., pytest.mark.skip)
so it does not produce test output. Ensure you update or remove the coverage
variable and any print statements inside test_agent_test_summary to avoid noisy
test runs.

In `@apps/web-backend/tests/test_integration.py`:
- Around line 343-371: Replace the inner async-run pattern that uses
asyncio.run() with pytest-asyncio style async tests: change
test_task_lifecycle_simulation to be async (async def
test_task_lifecycle_simulation(...)) and add `@pytest.mark.asyncio`, remove the
inner run_test() wrapper and call mock_agent_task directly via await; keep the
same logic using _running_tasks, mock_agent_task, and get_task_status but
execute it in the test coroutine instead of using asyncio.run(); apply the same
refactor to the other similar tests mentioned (the blocks referencing
_running_tasks and calls to asyncio.run()).
- Around line 770-855: The tests mutate the global manager
(manager.active_connections and manager.spec_subscriptions) which can leak
state; create a pytest fixture that provides an isolated ConnectionManager and
injects or swaps it for the module-level manager during each test, e.g.
instantiate a fresh ConnectionManager, assign it to the module-global manager
used by broadcast_execution_event / broadcast_log_event / broadcast_error_event
before the test, yield it to the test, and restore the original manager after
the test (or clear its dicts in teardown) so each test runs with a clean manager
instance and uses manager.disconnect for cleanup.

In `@apps/web-backend/tests/test_websocket.py`:
- Around line 522-572: The function test_websocket_test_summary is a
non-functional documentation test that always passes; remove this as a test and
relocate its content either into the module-level docstring at the top of the
file or into a separate documentation/markdown file. Specifically, delete the
test_websocket_test_summary function (or rename it to a non-test helper) and
copy the coverage dict and printed summary into the module docstring or docs
file so it no longer contributes to test count while preserving the
documentation.
- Around line 397-432: The test modifies the global manager
(api.websocket.manager) which can break isolation; instead create an isolated
ConnectionManager via a pytest fixture (e.g., isolated_manager) and monkeypatch
the module-level manager to that instance before calling
test_broadcast_execution_event; update the test to use isolated_manager to set
active_connections and spec_subscriptions and remove manual disconnect/cleanup
so the global manager is not mutated and tests remain isolated.
- Around line 252-271: The tests test_websocket_requires_token and
test_websocket_invalid_token only rely on "no exception" and should instead
assert the connection was closed by the server; update each test to attempt a
receive and assert closure—e.g., import and use WebSocketDisconnect in a
pytest.raises(WebSocketDisconnect) around
websocket.receive_text()/receive_bytes, or assert websocket.close_code is not
None (and equals the expected policy violation code) after the with-block to
ensure the server actually closed the socket; modify the two tests to include
one of these explicit assertions.

In `@apps/web-frontend/src/api/types.ts`:
- Around line 36-44: The TaskDetail interface's spec_content currently uses
optional string which doesn't allow backend-sent null; update the TaskDetail
type (interface TaskDetail) so spec_content is typed to accept null as well
(e.g., spec_content?: string | null or spec_content: string | null depending on
intended presence) and update any callers/assignments that assume non-null to
handle null values accordingly.

In `@apps/web-frontend/src/App.tsx`:
- Around line 103-150: The header currently uses plain <a href="..."> anchors in
App.tsx which cause full page reloads; import Link from react-router-dom and
replace each internal anchor (e.g., the anchors pointing to "/tasks", "/kanban",
"/roadmap", "/changelog", "/insights", "/tasks/create", "/login", "/signup")
with <Link to="..."> preserving the existing className and children so
navigation becomes SPA client-side routing; keep using <a> only for true
external URLs if any.
- Around line 159-177: TaskListWrapper (and similarly TaskDetailWrapper and
KanbanWrapper) uses a custom navigate that sets window.location.href causing
full page reloads; replace that with React Router's useNavigate hook: call
useNavigate() to obtain the navigate function and use it inside the existing
React.useCallback handlers (handleTaskClick, handleCreateTask, etc.) instead of
window.location.href so navigation is client-side. Locate the navigate
declaration in TaskListWrapper, TaskDetailWrapper, and KanbanWrapper and swap it
to const navigate = useNavigate(), keeping the existing callback shapes and
dependency arrays.

In `@apps/web-frontend/src/components/__tests__/KanbanBoard.test.tsx`:
- Around line 58-68: The id generator in createMockTask uses the deprecated
String.prototype.substr; update the id creation to use substring (or slice)
instead (e.g., replace Math.random().toString(36).substr(2, 9) with
Math.random().toString(36).substring(2, 11)) so createMockTask continues to
produce the same-length random suffix without using deprecated APIs.

In `@apps/web-frontend/src/components/__tests__/Terminal.test.tsx`:
- Around line 148-171: The tests currently guard clicks with conditionals (if
(closeButton) / if (terminalContainer)) which masks missing elements; update the
"calls onClose when X is clicked" and "calls onActivate when terminal is
clicked" tests in Terminal.test.tsx to assert the elements exist before
interacting: locate the close button (use screen.getAllByRole("button") and find
the element with svg.lucide-x or prefer a more specific query) and call
expect(foundCloseButton).toBeInTheDocument() then
fireEvent.click(foundCloseButton) and assert onClose; similarly locate the
terminal container (use container.querySelector(".flex.flex-col.h-full") or a
test id), assert expect(terminalContainer).toBeInTheDocument(), then
fireEvent.click(terminalContainer) and assert onActivate 호출. Ensure you remove
the conditional branches so the test fails when elements are missing.

In `@apps/web-frontend/src/components/FileExplorer.tsx`:
- Around line 326-355: fetchDirectory currently swallows errors and always adds
an Authorization header even when token is empty; update the fetchDirectory
implementation so it only sets Authorization when a non-empty token is present
(build headers conditionally), and on fetch failure call the component's
setError with a descriptive message (e.g., err.message or HTTP status text)
instead of silently returning null — then either rethrow the error or return a
sentinel that the caller expects so the error UI can render; reference the
fetchDirectory function, the Authorization header usage, and the setError state
updater to locate where to make these changes.
- Around line 380-428: The handleToggle function can overwrite concurrent
toggles because it awaits fetchDirectory before committing expandedPaths; move
the expanded state update to occur immediately (before any await) and use the
functional updater form of setExpandedPaths to derive the new Set from the
previous state instead of using the captured newExpanded variable, and likewise
update setLoadingPaths with a functional update when marking a path as loading
and when clearing it after the fetch; keep the rest of the children-fetch logic
(fetchDirectory, setFiles update) the same but ensure all state writes use
functional updates to avoid stale closures.
- Around line 310-375: The component currently defaults files to SAMPLE_FILES so
real data never loads automatically; change the files initial state to use
initialFiles || (apiUrl ? [] : SAMPLE_FILES) and add a useEffect that calls
handleRefresh on mount whenever apiUrl is provided (or when initialFiles is
undefined) so fetchDirectory is invoked automatically; update references to
FileExplorer, initialFiles, SAMPLE_FILES, fetchDirectory, handleRefresh and
rootPath to implement this initial-load behavior.

In `@apps/web-frontend/src/components/GitHubConnect.tsx`:
- Around line 59-63: The disconnect handler currently only updates UI state;
update handleDisconnect to call the backend disconnect endpoint (e.g.,
POST/DELETE to an auth/github/disconnect API) and only setStatus({ connected:
false }) and setSuccess(...) after receiving a successful HTTP response; handle
and surface non-2xx errors with setError or processLogger, show a
loading/disabled state while the request is in flight, and abort state changes
on failure. Locate the function handleDisconnect in GitHubConnect.tsx and wire
the network call, error handling, and loading flag so the UI reflects the actual
backend result.
- Around line 153-158: The "Refresh Connection" Button in the GitHubConnect
component currently only calls setSuccess and doesn't actually re-verify or
update user info; replace the onClick handler on that Button with an async
function that calls the backend endpoint that returns the current GitHub
connection status (e.g., fetch or apiClient.get('/auth/github/status') or
whatever existing API helper you have), update the component state (e.g., call
setUser, setConnectionStatus or the existing state-updating functions used in
GitHubConnect), and set success/error messages based on the response; if no
backend API exists yet, disable or remove the Button instead and leave a TODO
comment referencing the intended API call so it can't be clicked without
functionality.

In `@apps/web-frontend/src/components/KanbanBoard.tsx`:
- Around line 57-63: getVisualColumn currently casts any unhandled TaskStatus to
a column, which can silently misplace new statuses; update getVisualColumn to
validate that the resolved column exists in TASK_STATUS_COLUMNS (use
TASK_STATUS_COLUMNS.includes(...) or a Set of (typeof
TASK_STATUS_COLUMNS)[number]) and handle invalid cases explicitly — e.g.,
log/throw an error or return a safe default column like "todo"/"backlog" —
ensuring you reference getVisualColumn, TaskStatus and TASK_STATUS_COLUMNS when
adding the runtime check so new/unknown statuses are never implicitly cast into
an incorrect column.
- Line 273: In KanbanBoard, the call to useTranslation is unused; either remove
the unused useTranslation() call/import or destructure the t function (const { t
} = useTranslation(["common"]) or const [t] = useTranslation(["common"])) and
replace hardcoded strings such as "Refreshing...", "Refresh", and "Drop here"
with t('key') calls (add appropriate keys to the "common" namespace), ensuring
the KanbanBoard component uses the t symbol consistently and removing the
hook/import if you choose not to translate.
- Around line 376-386: When dropping onto another task we currently set
newStatus = overTask.status which can assign mapped statuses like "pr_created"
or "error"; instead derive the canonical column status for the target visual
column. Use the existing getVisualColumn(task.status) and
getVisualColumn(overTask.status) results to compute the canonical status (e.g.
call or add a helper like getCanonicalStatusForColumn(overTaskVisualColumn) or
map via a COLUMN_CANONICAL_STATUS lookup) and set newStatus to that canonical
value rather than overTask.status so dragged tasks always get the column's
canonical status.

In `@apps/web-frontend/src/components/Sidebar.tsx`:
- Around line 121-218: Navigation and footer buttons render icon-only when
isCollapsed and lack accessible labels; update the Button elements (the ones
inside the items.map render using Icon and in the footer Buttons for
onNewTaskClick and onSettingsClick) to include an aria-label when isCollapsed is
true (use item.label for nav items and the translated strings used for the New
Task and Settings spans for footer buttons) so screen readers get meaningful
text; ensure the aria-label is applied to the actual interactive element (the
Button passed into TooltipTrigger asChild) and conditionally omit or keep the
existing visible span when !isCollapsed.

In `@apps/web-frontend/src/components/SortableTaskCard.tsx`:
- Around line 45-56: The TaskCard's onClick can fire after a short drag; update
SortableTaskCard to guard clicks by the drag state: obtain the dragging flag
from your dnd-kit hook (e.g. isDragging from useSortable/useDraggable) and wrap
or replace the onClick prop passed to TaskCard with a handler that returns early
when isDragging is true; keep ref wiring (setNodeRef) and listeners/attributes
intact so drag still works, and ensure the wrapper uses the same onClick
signature (preserve onStatusChange, isSelectable, isSelected, onToggleSelect).

In `@apps/web-frontend/src/components/TaskCard.tsx`:
- Around line 93-106: The memo comparator taskCardPropsAreEqual is missing
checks for the task's priority and impact, causing priority/impact badge updates
to be missed; update the return expression in taskCardPropsAreEqual to also
compare prevTask.priority === nextTask.priority and prevTask.impact ===
nextTask.impact (or prevTask.metadata?.priority/impact if those fields live
under metadata) alongside the existing checks so changes to priority or impact
force a re-render.

In `@apps/web-frontend/src/components/Terminal.tsx`:
- Around line 330-336: The reconnect handler currently only closes wsRef.current
and never starts a new connection; update handleReconnect to (after closing and
nulling wsRef.current) invoke the module's WebSocket initialization routine
(e.g., call the existing connectWebSocket or initWebSocket function, or toggle
the state that triggers a connection) so a new socket is created immediately,
and include that connect function in handleReconnect's useCallback dependency
array so it captures the latest version.
- Line 17: The TerminalProps interface declares projectPath but it is unused in
the Terminal component; remove the unused prop by deleting projectPath from the
TerminalProps definition and any related type references, and ensure no
references to projectPath remain in the Terminal function/component (Terminal)
or its callers—if callers rely on passing projectPath and it is meaningful,
alternatively use it inside Terminal where appropriate (e.g., to set a working
directory) rather than leaving it declared but unused.
- Around line 229-245: The onclose handler currently only resets
reconnectTimeoutRef without re-establishing the WebSocket; update it so the
timeout actually triggers a reconnection by calling the connection routine (or
toggling a state that the useEffect which creates the socket depends on).
Specifically, modify ws.onclose to set reconnectTimeoutRef = setTimeout(() => {
reconnectTimeoutRef = null; /* call the connect function or
setRecreateSocket(prev => !prev) to re-run the useEffect that creates the WS */
}, 3000), and ensure you reference the same connect logic used when mounting
(the function inside the useEffect or a named connectWebSocket helper) and
properly clear pingIntervalRef and any previous timeouts; keep
setConnectionStatus and xtermRef writes intact.
- Around line 86-141: The initialization useEffect that creates the Terminal
(references: terminalRef, xtermRef, fitAddonRef, new Terminal(), FitAddon)
should not re-run on isActive changes; remove isActive from that effect's
dependency array so the terminal is created once on mount and only disposed on
unmount, since focus is handled by the separate effect. Keep the teardown
(fitAddon.dispose, terminal.dispose, nulling refs) as-is and ensure only stable
dependencies (or an empty array) remain for the initialization useEffect.
- Around line 282-299: The xterm listeners are being added via xterm.onData and
xterm.onResize but not removed—calling xterm.onData(() => {})/onResize(() => {})
just adds listeners; instead capture the IDisposable returned by
xterm.onData(handleData) and xterm.onResize(...) (e.g., store them in local
variables like dataDisposable and resizeDisposable or an array), and in the
cleanup return dispose those disposables (call dataDisposable.dispose() and
resizeDisposable.dispose()); also ensure you still remove the window resize
listener (handleResize) and keep using wsRef and handleData/handleResize as-is.

In `@apps/web-frontend/src/components/ui/card.tsx`:
- Around line 39-51: The ref generic for CardTitle is incorrect: it's declared
as HTMLParagraphElement while the component renders an <h3>; update the
forwardRef generic so the first type parameter is HTMLHeadingElement (keep the
props type as React.HTMLAttributes<HTMLHeadingElement>) for the CardTitle
declaration that uses React.forwardRef to ensure the ref type matches the
rendered element.

In `@apps/web-frontend/src/components/ui/switch.tsx`:
- Around line 26-27: The consumer onClick prop currently overwrites the internal
handler because {...props} is spread after onClick={handleClick}; update the
component to destructure onClick from props (e.g. const { onClick, ...rest } =
props) and pass a composed handler to the element (e.g. onClick={(e) => {
handleClick(e); onClick?.(e); }}) while spreading {...rest} afterwards;
reference the existing handleClick function and the props spread in this
component so the internal toggle always runs before (or after, as desired) the
consumer callback and the event is forwarded.

In `@apps/web-frontend/src/index.css`:
- Around line 14-16: In the font-family declaration (the font-family: rule),
remove the unnecessary quotes around single-word font family names "Roboto",
"Oxygen", "Ubuntu", and "Cantarell" so they become unquoted tokens (keep quotes
for multi-word families like "Segoe UI", "Fira Sans", "Droid Sans", "Helvetica
Neue"); update the font-family rule accordingly to satisfy Stylelint's
font-family-name-quotes rule.

In `@apps/web-frontend/src/pages/Changelog.tsx`:
- Around line 134-138: The date formatting in releaseDate uses a hard-coded
"en-US" locale which breaks i18n; update the code that builds releaseDate (the
new Date(release.date).toLocaleDateString(...) call) to use the current locale
from i18n (via useTranslation and i18n.language) or omit the locale to let the
browser choose, and keep the same options ({ year: "numeric", month: "long",
day: "numeric" }) so formatting respects the user's language preference.
- Around line 61-92: getChangeTypeInfo currently returns a variant property that
is never used by ChangeTypeBadge; either remove variant from the return shape
and from each returned object in getChangeTypeInfo, updating any types
referencing the return (e.g., the function signature and ChangeTypeBadge props)
to only include label and color, or make ChangeTypeBadge accept and consume the
variant (e.g., prop name "variant") and apply it to the badge rendering logic;
locate getChangeTypeInfo and ChangeTypeBadge and choose one of these two fixes
so the variant is not unused.

In `@apps/web-frontend/src/pages/FilesPage.tsx`:
- Around line 17-21: Replace the blocking alert in handleFileSelect with a
non-blocking in-app UI action: either navigate to a file viewer route (use the
app router / history push from wherever navigation is handled) or open a
modal/toast component that shows file.name and file.path; update
handleFileSelect (the useCallback) to call the router navigation function or the
modal/toast open method instead of alert so selection is accessible and
non-disruptive.

In `@apps/web-frontend/src/pages/GitOperations.tsx`:
- Around line 129-136: The page mixes localized calls with hardcoded English;
make all user-facing strings consistent by replacing hardcoded labels with t()
calls and keys (or remove all t() if you prefer non-i18n). Update the main
heading "Git Operations", subheadings "No Worktrees" and "About Git Worktrees",
button labels like "Try Again" and any static paragraphs currently hardcoded in
the GitOperations component to use t("...") keys, keeping existing
t("common:loading") and t("common:error") usage; ensure the JSX elements (e.g.,
the <h1>/<h2> headings, paragraphs, and <Button> labels) call t(...) and use
consistent key names so translations render correctly, and adjust any related
handlers like handleRefresh only if their UX text is changed.
- Around line 107-109: The clipboard write in handleCopyPath uses
navigator.clipboard.writeText without awaiting or handling failures and provides
no user feedback; update handleCopyPath to await the Promise returned by
navigator.clipboard.writeText(path), wrap it in try/catch to handle errors
(e.g., permission denied or insecure context), and surface success/failure
feedback to the user—use the existing UI feedback mechanism (toast/snackbar or a
local state flag) to show a confirmation on success and an error message on
failure; reference the handleCopyPath function and ensure any UI hook (e.g.,
showToast or setCopyStatus) is used to notify the user.
- Around line 190-201: Replace the direct window.location.href assignments in
the Button onClick handlers inside the GitOperations component with React Router
navigation: import and call useNavigate from react-router-dom (e.g., const
navigate = useNavigate()) and change the handlers to call navigate('/tasks') and
navigate('/tasks/create') respectively (or use navigate(path, { replace: true })
if you need replacement behavior); update the Button onClick references (the
ones currently setting window.location.href) to use the navigate function and
keep existing props like variant/className unchanged.

In `@apps/web-frontend/src/pages/Insights.tsx`:
- Around line 81-86: The current URL check in isValidUrl allows insecure
"http://" links; update the logic to disallow external "http://" by only
treating href as a valid external link when it starts with "https://" (keep "/"
and "#" as before), and add a separate boolean (e.g., isInsecureExternalLink)
that detects href starting with "http://" so the UI can either block or show a
warning icon/tooltip next to the rendered link; adjust the rendering code that
consumes isValidUrl/href to either prevent navigation for insecure links or
render the visual indicator when isInsecureExternalLink is true.
- Around line 534-540: handleDeleteSession currently reads the stale sessions
array from closure which can pick the wrong session; change it to compute the
new sessions first and base the currentSession update on that fresh array:
inside handleDeleteSession, create newSessions by filtering the previous
sessions (use the functional form setSessions(prev => { const newSessions =
prev.filter(...); return newSessions; }) or compute newSessions const by reading
the latest state via a functional updater), then call setCurrentSession using
newSessions (e.g., if deleted session was currentSession.id then
setCurrentSession(newSessions.length ? newSessions[0] : null), otherwise leave
currentSession untouched). Ensure you reference handleDeleteSession,
setSessions, setCurrentSession and currentSession?.id when making the change.

In `@apps/web-frontend/src/pages/Kanban.tsx`:
- Around line 109-122: The handleStatusChange currently only updates local state
optimistically; implement persistence by making an async API call to the backend
to update a task's status (e.g., PATCH /tasks/:id or whatever endpoint your API
exposes) inside handleStatusChange (or a helper like updateTaskStatus), send
taskId and newStatus, and await the response; on success keep the optimistic
change, on failure rollback the local state (restore prevTasks) and surface an
error (e.g., toast/processLogger), and ensure handleStatusChange is declared
async or uses .then/.catch and includes its dependencies in the useCallback deps
so state stays consistent; reference handleStatusChange and setTasks when adding
the API call, rollback logic, and error handling.
- Around line 36-47: The current fuzzy substring checks on apiStatus (in
Kanban.tsx) are fragile and can misclassify values like "no_progress" or
"ai_review"; replace the includes-based logic in the status assignment block
with an explicit mapping or exact-match switch: create a statusMap (or switch
statement) keyed by the exact apiStatus values and return "done", "in_progress",
"human_review", "queue", etc., for known keys and a sensible default otherwise;
ensure the update occurs where status is set (the block referencing
apiStatus.includes("done")/... ) so only exact matches from the map/switch
determine the final status.
- Around line 49-59: The Task conversion is mapping summary.status into
description and fabricating timestamps with new Date(); update the mapping in
the function that returns the Task object (the block creating
id/specId/title/description/status/... createdAt/updatedAt) so description uses
a proper field (e.g., derive from summary.progress or another available field)
instead of summary.status and set status from summary.status; for timestamps
either add created_at/updated_at to the TaskSummary interface and populate
createdAt/updatedAt from summary.created_at/summary.updated_at, or remove/omit
createdAt/updatedAt from the Task object until the API provides them—adjust
TaskSummary and the conversion logic accordingly.

In `@apps/web-frontend/src/pages/Login.tsx`:
- Around line 131-136: Replace the plain anchor tags used for internal routes
with React Router's Link to avoid full page reloads: import { Link } from
'react-router-dom' (if not already) and change the <a href="/forgot-password"
className="text-sm text-blue-600 hover:text-blue-700">Forgot password?</a> and
the similar <a href="/signup"...>...</a> into <Link to="/forgot-password"
className="...">Forgot password?</Link> and <Link to="/signup"
className="...">...</Link>, preserving className and any other props/children so
SPA state/navigation is retained.
- Around line 36-38: The login handler currently persists the JWT via
localStorage.setItem("token", ...) and user via localStorage.setItem("user",
...), which exposes tokens to XSS; change the flow so the server issues an
HttpOnly, Secure (and SameSite=strict/lax as appropriate) cookie for the
access/refresh token and remove client-side storage of the raw token in the
Login component (or keep token only in in-memory state if immediate use is
required), while optionally keeping non-sensitive user profile JSON in
localStorage; if you cannot migrate immediately, add a clear comment next to
localStorage.setItem("token", ...) documenting the XSS risk and ensure a strong
CSP and XSS mitigations are in place.
- Around line 116-130: The "Remember me" checkbox (input id="remember") is
currently uncontrolled and its value is never used; either remove it or make it
functional by adding component state (e.g., a boolean remember in the Login
component), bind the checkbox to that state (checked and onChange), and include
the remember flag in the login submission flow (pass it into the existing submit
handler such as handleSubmit or onSubmitLogin and forward to the backend/auth
function so token/session TTL can be adjusted); alternatively, if you choose to
remove it, delete the input and label to avoid misleading UI.

In `@apps/web-frontend/src/pages/Roadmap.tsx`:
- Around line 27-33: Replace the local RoadmapFeature interface with the shared
type: remove the interface declaration named RoadmapFeature and import the
corresponding exported type from the shared types module used by the frontend
(the shared RoadmapFeature type that includes rationale, complexity, impact,
dependencies, etc.), then update any local usages/placeholder data to satisfy
the shared type shape (or use a partial/mapper until the API provides full
fields) so the component consistently uses the centralized type.
- Around line 53-55: Remove the unused onGoToTask prop from the Roadmap public
API: delete onGoToTask from the RoadmapProps interface and remove any mention of
it in the Roadmap component signature (and its default/props destructuring) so
the component no longer declares an unused prop; alternatively, if navigation to
a task is intended, implement using the onGoToTask callback inside the Roadmap
component where task items are rendered (e.g., in the task click handler) and
ensure the prop is passed through from parent — reference RoadmapProps, Roadmap,
and onGoToTask to locate the relevant declarations and handlers.

In `@apps/web-frontend/src/pages/Settings.tsx`:
- Around line 71-74: handleSaveProfile currently logs the full API key (apiKey)
to the console; remove the apiKey from any console.log output and instead either
omit it entirely or log a masked version (e.g., first/last few chars only) and
log only non-sensitive fields like profileName and profileEmail; update the
console message in handleSaveProfile to avoid printing apiKey and ensure future
save-to-backend logic (saveProfile or similar) sends the key securely rather
than logging it.

In `@apps/web-frontend/src/pages/Signup.tsx`:
- Around line 8-193: The Signup component currently contains hardcoded
user-facing strings; import useTranslation from react-i18next at the top of the
file, call const { t } = useTranslation(/* appropriate namespaces e.g.
"common","errors","onboarding" */) inside the Signup function, and replace all
visible strings and error messages used in the JSX and in handleSubmit (e.g.,
"Create Account", form labels, placeholders, button text, "Please fill in all
required fields", "Passwords do not match", "Password must be at least 8
characters", "Signup failed", etc.) with t('your.namespace.key') keys; ensure
you create or reference appropriate i18n keys for labels, placeholders, button
copy and validation errors and keep keys grouped by namespace (common, errors,
onboarding) as suggested.

In `@apps/web-frontend/src/pages/TaskCreate.tsx`:
- Line 40: The useTranslation hook call is unused; either remove the call or
destructure and use its translator function to localize strings in this
component: change the call to destructure the t function
(useTranslation(["common","tasks"]) -> const { t } = useTranslation([...])) and
replace hardcoded UI strings in TaskCreate (component render, labels, buttons,
placeholders) with t("key") calls using appropriate keys from the "common" and
"tasks" namespaces; if you choose not to translate here, simply remove the
useTranslation invocation to eliminate the unused hook.
- Around line 51-81: The agentTypes constant is being recreated on every render
inside the TaskCreate component; move the agentTypes array declaration out of
the TaskCreate component (make it a top-level module constant) or wrap it in
useMemo inside TaskCreate to avoid unnecessary re-creation of the static list
and JSX icon nodes; update any references to agentTypes in the component to use
the relocated constant.

In `@apps/web-frontend/src/pages/TaskDetail.tsx`:
- Around line 66-96: agentTypes is a static configuration currently declared
inside the TaskDetail component causing it to be reallocated on every render;
move the const agentTypes declaration out of the component (e.g., top-level in
the module) so it is created once, keep its exact shape and typings (Array<{
type: AgentType; label: string; description: string; icon: React.ReactNode }>),
and ensure any referenced symbols (AgentType, Brain, Code, Search, Wrench)
remain imported/available at module scope; if you need it elsewhere consider
exporting it as a named export.
- Around line 170-186: pollAgentStatus schedules recursive setTimeouts and
doesn't clear them on unmount, causing memory leaks and state updates after
unmount; fix by adding a ref (e.g., timeoutRef via useRef) and a mounted flag
(e.g., isMountedRef) or an abort signal, clear any pending timeout before
scheduling a new one, guard setAgentStatus with the mounted check, and ensure
you clearTimeout(timeoutRef.current) and set isMountedRef.current = false in a
useEffect cleanup or returned cancel function so all pending polls are cancelled
when the component unmounts.

In `@apps/web-frontend/src/pages/UsageDashboard.tsx`:
- Around line 90-101: The current useEffect always calls both fetchDashboardData
and fetchStatsData when period or daysBack change, causing unnecessary fetches;
separate into two effects and stabilize the functions with useCallback: keep
fetchDashboardData wrapped in a useCallback with an empty dependency array (or
move it outside the effect) so it only runs once (or when its true deps change),
and wrap fetchStatsData in a useCallback that depends on [period, daysBack],
then have one useEffect that calls the dashboard loader once and another that
calls the stats loader when period/daysBack change (remove the combined loadData
that calls both).
- Around line 50-88: fetchDashboardData and fetchStatsData currently hardcode
user_id=1 and call fetch directly, bypassing the app's ApiClient (which provides
timeout, error handling, and logging); replace both raw fetch usages by calling
the centralized ApiClient (or a new UsageService that wraps ApiClient) to call
the endpoints (usage/dashboard and usage/stats), pass the authenticated user's
ID from the user context instead of the hardcoded 1, and rely on ApiClient's
timeout/AbortController and error mapping rather than manual fetch/error
handling in fetchDashboardData and fetchStatsData.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider using vi.waitFor instead of fixed delays for consistency.

The test already uses vi.waitFor elsewhere (lines 731-736, 743-753) for more reliable async waiting. Fixed timeouts can still be flaky on heavily loaded CI runners. Consider replacing these arbitrary delays with a polling approach that checks for the expected state.

♻️ Suggested improvement using vi.waitFor

-      // Use a longer delay to avoid race conditions on slower CI runners.
-      await new Promise((resolve) => setTimeout(resolve, 1000));
+      // Wait for ProjectStore's async initialization to complete
+      const { projectStore } = await import("../project-store");
+      await vi.waitFor(
+        () => expect(projectStore).toBeDefined(),
+        { timeout: 2000, interval: 50 }
+      );

       // Add project twice
       const result1 = await ipcMain.invokeHandler("project:add", {}, TEST_PROJECT_PATH);
-      // Wait for the fire-and-forget saveAsync to flush
-      await new Promise((resolve) => setTimeout(resolve, 500));
+      // Wait for the project to be persisted before adding again
+      await vi.waitFor(
+        async () => {
+          const listResult = await ipcMain.invokeHandler("project:list", {});
+          expect((listResult as { data: unknown[] }).data.length).toBeGreaterThan(0);
+        },
+        { timeout: 1000, interval: 50 }
+      );
       const result2 = await ipcMain.invokeHandler("project:add", {}, TEST_PROJECT_PATH);

🤖 Prompt for AI Agents

In `@apps/frontend/src/main/__tests__/ipc-handlers.test.ts` around lines 303 -
309, Replace the fixed setTimeout waits with a vi.waitFor-based poll that waits
until the expected post-save state is observable: after calling
ipcMain.invokeHandler("project:add", {}, TEST_PROJECT_PATH) use vi.waitFor to
poll for the side-effect the fire-and-forget saveAsync produces (for example, a
persisted file, an emitted event, or a change observable via ipcMain or the
project list API) instead of waiting a fixed 500ms/1000ms; update the assertions
to run once vi.waitFor completes so the test reliably waits for the saveAsync
flush.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unused db dependency parameter.

The db: Session dependency is injected but never used in github_callback. The TODO comment at line 126 suggests storing the token in the database, but this is not implemented.

Either implement the token storage or remove the unused dependency to avoid dead code.

🤖 Prompt for AI Agents

In `@apps/web-backend/api/routes/git.py` at line 70, The github_callback function
declares an unused dependency db: Session (via get_db)—either persist the GitHub
token as the TODO suggests or remove the dead parameter; to fix, locate
github_callback and either (A) implement token storage using the injected db:
create or update the appropriate token model (e.g., GitHubToken or UserToken)
with the returned access token and user identifier, add db.add(...) and
db.commit() (and any necessary model import), or (B) if you won't store the
token now, remove the db: Session = Depends(get_db) parameter from
github_callback and delete any related unused imports to eliminate dead code.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider aligning with shared types.

The local RoadmapFeature interface is a simplified version compared to the shared type in apps/frontend/src/shared/types/roadmap.ts, which includes additional fields like rationale, complexity, impact, dependencies, etc.

When the API integration is implemented (replacing the placeholder data), consider importing and reusing the shared types to maintain consistency across the codebase.

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/Roadmap.tsx` around lines 27 - 33, Replace the
local RoadmapFeature interface with the shared type: remove the interface
declaration named RoadmapFeature and import the corresponding exported type from
the shared types module used by the frontend (the shared RoadmapFeature type
that includes rationale, complexity, impact, dependencies, etc.), then update
any local usages/placeholder data to satisfy the shared type shape (or use a
partial/mapper until the API provides full fields) so the component consistently
uses the centralized type.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unused onGoToTask prop.

The onGoToTask prop is declared in RoadmapProps but never used within the Roadmap component. This creates dead code and misleading API surface.

Either remove the unused prop or implement the intended functionality (e.g., linking features to tasks).

🧹 Remove unused prop

-interface RoadmapProps {
-	onGoToTask?: (specId: string) => void;
-}
-
-export function Roadmap({ onGoToTask }: RoadmapProps) {
+export function Roadmap() {

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/Roadmap.tsx` around lines 53 - 55, Remove the
unused onGoToTask prop from the Roadmap public API: delete onGoToTask from the
RoadmapProps interface and remove any mention of it in the Roadmap component
signature (and its default/props destructuring) so the component no longer
declares an unused prop; alternatively, if navigation to a task is intended,
implement using the onGoToTask callback inside the Roadmap component where task
items are rendered (e.g., in the task click handler) and ensure the prop is
passed through from parent — reference RoadmapProps, Roadmap, and onGoToTask to
locate the relevant declarations and handlers.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Replace hardcoded UI/error strings with i18n keys and useTranslation().

This file uses many hardcoded user-facing strings (labels, placeholders, button text, errors, helper copy). Per project learnings, these must be translated via useTranslation() with appropriate namespaces. Based on learnings: Use i18n translation keys for all user-facing text in the frontend instead of hardcoded strings; import translation helpers from react-i18next and use useTranslation() hook with appropriate namespaces (common, navigation, settings, dialogs, tasks, errors, onboarding, welcome).

🌍 Suggested fix (pattern)

-import { type FormEvent, useState } from "react";
+import { type FormEvent, useState } from "react";
+import { useTranslation } from "react-i18next";

 export function Signup() {
+	const { t } = useTranslation(["common", "errors", "onboarding"]);
 	const navigate = useNavigate();
@@
-			setError("Please fill in all required fields");
+			setError(t("errors:signup.required_fields"));
@@
-			setError("Passwords do not match");
+			setError(t("errors:signup.passwords_do_not_match"));
@@
-			setError("Password must be at least 8 characters");
+			setError(t("errors:signup.password_min_length"));
@@
-					<h1 className="text-3xl font-bold text-gray-900 mb-2">
-						Create Account
-					</h1>
+					<h1 className="text-3xl font-bold text-gray-900 mb-2">
+						{t("onboarding:create_account")}
+					</h1>
@@
-						Start building with Auto Code in the cloud
+						{t("onboarding:signup_subtitle")}
@@
-							Email Address *
+							{t("common:email_address")} *
@@
-							placeholder="you@example.com"
+							placeholder={t("common:email_placeholder")}
@@
-							Organization (Optional)
+							{t("common:organization_optional")}
@@
-							placeholder="Your Company"
+							placeholder={t("common:organization_placeholder")}
@@
-							Password *
+							{t("common:password")} *
@@
-							placeholder="Min. 8 characters"
+							placeholder={t("common:password_min_chars")}
@@
-							Confirm Password *
+							{t("common:confirm_password")} *
@@
-							placeholder="Re-enter password"
+							placeholder={t("common:password_reenter")}
@@
-						{isLoading ? "Creating Account..." : "Sign Up"}
+						{isLoading ? t("onboarding:creating_account") : t("onboarding:sign_up")}
@@
-						Already have an account?{" "}
+						{t("onboarding:already_have_account")}{" "}
@@
-							Sign In
+							{t("onboarding:sign_in")}
@@
-						By signing up, you agree to our Terms of Service and Privacy Policy
+						{t("onboarding:signup_terms")}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	import { type FormEvent, useState } from "react";
	import { useNavigate } from "react-router-dom";
	import { Button } from "../components/ui/button";
	import { signup } from "../lib/api";
	export function Signup() {
	const navigate = useNavigate();
	const [email, setEmail] = useState('');
	const [password, setPassword] = useState('');
	const [confirmPassword, setConfirmPassword] = useState('');
	const [organization, setOrganization] = useState('');
	const [isLoading, setIsLoading] = useState(false);
	const [error, setError] = useState<string | null>(null);
	const handleSubmit = async (e: FormEvent<HTMLFormElement>) => {
	e.preventDefault();
	setError(null);
	// Validation
	if (!email || !password || !confirmPassword) {
	setError('Please fill in all required fields');
	return;
	}
	if (password !== confirmPassword) {
	setError('Passwords do not match');
	return;
	}
	if (password.length < 8) {
	setError('Password must be at least 8 characters');
	return;
	}
	setIsLoading(true);
	try {
	// Call backend signup endpoint
	const result = await signup(email, password, organization || undefined);
	// Store JWT token in localStorage
	localStorage.setItem('token', result.access_token);
	localStorage.setItem('user', JSON.stringify(result.user));
	// Redirect to home page (or dashboard)
	navigate('/');
	} catch (err) {
	setError(err instanceof Error ? err.message : 'Signup failed');
	} finally {
	setIsLoading(false);
	}
	};
	return (
	<div className="min-h-screen bg-gray-50 flex items-center justify-center p-4">
	<div className="max-w-md w-full bg-white rounded-lg shadow-lg p-8">
	{/* Logo */}
	<div className="flex justify-center mb-6">
	<div className="w-16 h-16 bg-gradient-to-br from-blue-500 to-purple-600 rounded-xl flex items-center justify-center">
	<span className="text-3xl text-white font-bold">AC</span>
	</div>
	</div>
	{/* Title */}
	<div className="text-center mb-8">
	<h1 className="text-3xl font-bold text-gray-900 mb-2">
	Create Account
	</h1>
	<p className="text-gray-600">
	Start building with Auto Code in the cloud
	</p>
	</div>
	{/* Error Message */}
	{error && (
	<div className="mb-6 p-4 bg-red-50 border border-red-200 rounded-lg">
	<p className="text-sm text-red-600">{error}</p>
	</div>
	)}
	{/* Signup Form */}
	<form onSubmit={handleSubmit} className="space-y-4">
	{/* Email */}
	<div>
	<label htmlFor="email" className="block text-sm font-medium text-gray-700 mb-2">
	Email Address *
	</label>
	<input
	id="email"
	type="email"
	value={email}
	onChange={(e) => setEmail(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="you@example.com"
	required
	disabled={isLoading}
	/>
	</div>
	{/* Organization (Optional) */}
	<div>
	<label htmlFor="organization" className="block text-sm font-medium text-gray-700 mb-2">
	Organization (Optional)
	</label>
	<input
	id="organization"
	type="text"
	value={organization}
	onChange={(e) => setOrganization(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="Your Company"
	disabled={isLoading}
	/>
	</div>
	{/* Password */}
	<div>
	<label htmlFor="password" className="block text-sm font-medium text-gray-700 mb-2">
	Password *
	</label>
	<input
	id="password"
	type="password"
	value={password}
	onChange={(e) => setPassword(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="Min. 8 characters"
	required
	disabled={isLoading}
	/>
	</div>
	{/* Confirm Password */}
	<div>
	<label htmlFor="confirmPassword" className="block text-sm font-medium text-gray-700 mb-2">
	Confirm Password *
	</label>
	<input
	id="confirmPassword"
	type="password"
	value={confirmPassword}
	onChange={(e) => setConfirmPassword(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="Re-enter password"
	required
	disabled={isLoading}
	/>
	</div>
	{/* Submit Button */}
	<Button
	type="submit"
	className="w-full"
	disabled={isLoading}
	>
	{isLoading ? 'Creating Account...' : 'Sign Up'}
	</Button>
	</form>
	{/* Login Link */}
	<div className="mt-6 text-center">
	<p className="text-sm text-gray-600">
	Already have an account?{' '}
	<a href="/login" className="text-blue-600 hover:text-blue-700 font-medium">
	Sign In
	</a>
	</p>
	</div>
	{/* Terms */}
	<div className="mt-6 pt-6 border-t border-gray-200">
	<p className="text-xs text-gray-500 text-center">
	By signing up, you agree to our Terms of Service and Privacy Policy
	</p>
	</div>
	</div>
	</div>
	);
	const navigate = useNavigate();
	const [email, setEmail] = useState("");
	const [password, setPassword] = useState("");
	const [confirmPassword, setConfirmPassword] = useState("");
	const [organization, setOrganization] = useState("");
	const [isLoading, setIsLoading] = useState(false);
	const [error, setError] = useState<string | null>(null);
	const handleSubmit = async (e: FormEvent<HTMLFormElement>) => {
	e.preventDefault();
	setError(null);
	// Validation
	if (!email || !password || !confirmPassword) {
	setError("Please fill in all required fields");
	return;
	}
	if (password !== confirmPassword) {
	setError("Passwords do not match");
	return;
	}
	if (password.length < 8) {
	setError("Password must be at least 8 characters");
	return;
	}
	setIsLoading(true);
	try {
	// Call backend signup endpoint
	const result = await signup(email, password, organization || undefined);
	// Store JWT token in localStorage
	localStorage.setItem("token", result.access_token);
	localStorage.setItem("user", JSON.stringify(result.user));
	// Redirect to home page (or dashboard)
	navigate("/");
	} catch (err) {
	setError(err instanceof Error ? err.message : "Signup failed");
	} finally {
	setIsLoading(false);
	}
	};
	return (
	<div className="min-h-screen bg-gray-50 flex items-center justify-center p-4">
	<div className="max-w-md w-full bg-white rounded-lg shadow-lg p-8">
	{/* Logo */}
	<div className="flex justify-center mb-6">
	<div className="w-16 h-16 bg-gradient-to-br from-blue-500 to-purple-600 rounded-xl flex items-center justify-center">
	<span className="text-3xl text-white font-bold">AC</span>
	</div>
	</div>
	{/* Title */}
	<div className="text-center mb-8">
	<h1 className="text-3xl font-bold text-gray-900 mb-2">
	Create Account
	</h1>
	<p className="text-gray-600">
	Start building with Auto Code in the cloud
	</p>
	</div>
	{/* Error Message */}
	{error && (
	<div className="mb-6 p-4 bg-red-50 border border-red-200 rounded-lg">
	<p className="text-sm text-red-600">{error}</p>
	</div>
	)}
	{/* Signup Form */}
	<form onSubmit={handleSubmit} className="space-y-4">
	{/* Email */}
	<div>
	<label
	htmlFor="email"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	Email Address *
	</label>
	<input
	id="email"
	type="email"
	value={email}
	onChange={(e) => setEmail(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="you@example.com"
	required
	disabled={isLoading}
	/>
	</div>
	{/* Organization (Optional) */}
	<div>
	<label
	htmlFor="organization"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	Organization (Optional)
	</label>
	<input
	id="organization"
	type="text"
	value={organization}
	onChange={(e) => setOrganization(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="Your Company"
	disabled={isLoading}
	/>
	</div>
	{/* Password */}
	<div>
	<label
	htmlFor="password"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	Password *
	</label>
	<input
	id="password"
	type="password"
	value={password}
	onChange={(e) => setPassword(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="Min. 8 characters"
	required
	disabled={isLoading}
	/>
	</div>
	{/* Confirm Password */}
	<div>
	<label
	htmlFor="confirmPassword"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	Confirm Password *
	</label>
	<input
	id="confirmPassword"
	type="password"
	value={confirmPassword}
	onChange={(e) => setConfirmPassword(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder="Re-enter password"
	required
	disabled={isLoading}
	/>
	</div>
	{/* Submit Button */}
	<Button type="submit" className="w-full" disabled={isLoading}>
	{isLoading ? "Creating Account..." : "Sign Up"}
	</Button>
	</form>
	{/* Login Link */}
	<div className="mt-6 text-center">
	<p className="text-sm text-gray-600">
	Already have an account?{" "}
	<a
	href="/login"
	className="text-blue-600 hover:text-blue-700 font-medium"
	>
	Sign In
	</a>
	</p>
	</div>
	{/* Terms */}
	<div className="mt-6 pt-6 border-t border-gray-200">
	<p className="text-xs text-gray-500 text-center">
	By signing up, you agree to our Terms of Service and Privacy Policy
	</p>
	</div>
	import { type FormEvent, useState } from "react";
	import { useNavigate } from "react-router-dom";
	import { useTranslation } from "react-i18next";
	import { Button } from "../components/ui/button";
	import { signup } from "../lib/api";
	export function Signup() {
	const { t } = useTranslation(["common", "errors", "onboarding"]);
	const navigate = useNavigate();
	const [email, setEmail] = useState("");
	const [password, setPassword] = useState("");
	const [confirmPassword, setConfirmPassword] = useState("");
	const [organization, setOrganization] = useState("");
	const [isLoading, setIsLoading] = useState(false);
	const [error, setError] = useState<string | null>(null);
	const handleSubmit = async (e: FormEvent<HTMLFormElement>) => {
	e.preventDefault();
	setError(null);
	// Validation
	if (!email || !password || !confirmPassword) {
	setError(t("errors:signup.required_fields"));
	return;
	}
	if (password !== confirmPassword) {
	setError(t("errors:signup.passwords_do_not_match"));
	return;
	}
	if (password.length < 8) {
	setError(t("errors:signup.password_min_length"));
	return;
	}
	setIsLoading(true);
	try {
	// Call backend signup endpoint
	const result = await signup(email, password, organization || undefined);
	// Store JWT token in localStorage
	localStorage.setItem("token", result.access_token);
	localStorage.setItem("user", JSON.stringify(result.user));
	// Redirect to home page (or dashboard)
	navigate("/");
	} catch (err) {
	setError(err instanceof Error ? err.message : "Signup failed");
	} finally {
	setIsLoading(false);
	}
	};
	return (
	<div className="min-h-screen bg-gray-50 flex items-center justify-center p-4">
	<div className="max-w-md w-full bg-white rounded-lg shadow-lg p-8">
	{/* Logo */}
	<div className="flex justify-center mb-6">
	<div className="w-16 h-16 bg-gradient-to-br from-blue-500 to-purple-600 rounded-xl flex items-center justify-center">
	<span className="text-3xl text-white font-bold">AC</span>
	</div>
	</div>
	{/* Title */}
	<div className="text-center mb-8">
	<h1 className="text-3xl font-bold text-gray-900 mb-2">
	{t("onboarding:create_account")}
	</h1>
	<p className="text-gray-600">
	{t("onboarding:signup_subtitle")}
	</p>
	</div>
	{/* Error Message */}
	{error && (
	<div className="mb-6 p-4 bg-red-50 border border-red-200 rounded-lg">
	<p className="text-sm text-red-600">{error}</p>
	</div>
	)}
	{/* Signup Form */}
	<form onSubmit={handleSubmit} className="space-y-4">
	{/* Email */}
	<div>
	<label
	htmlFor="email"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	{t("common:email_address")} *
	</label>
	<input
	id="email"
	type="email"
	value={email}
	onChange={(e) => setEmail(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder={t("common:email_placeholder")}
	required
	disabled={isLoading}
	/>
	</div>
	{/* Organization (Optional) */}
	<div>
	<label
	htmlFor="organization"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	{t("common:organization_optional")}
	</label>
	<input
	id="organization"
	type="text"
	value={organization}
	onChange={(e) => setOrganization(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder={t("common:organization_placeholder")}
	disabled={isLoading}
	/>
	</div>
	{/* Password */}
	<div>
	<label
	htmlFor="password"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	{t("common:password")} *
	</label>
	<input
	id="password"
	type="password"
	value={password}
	onChange={(e) => setPassword(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder={t("common:password_min_chars")}
	required
	disabled={isLoading}
	/>
	</div>
	{/* Confirm Password */}
	<div>
	<label
	htmlFor="confirmPassword"
	className="block text-sm font-medium text-gray-700 mb-2"
	>
	{t("common:confirm_password")} *
	</label>
	<input
	id="confirmPassword"
	type="password"
	value={confirmPassword}
	onChange={(e) => setConfirmPassword(e.target.value)}
	className="w-full px-4 py-2 border border-gray-300 rounded-md focus:ring-2 focus:ring-blue-500 focus:border-transparent"
	placeholder={t("common:password_reenter")}
	required
	disabled={isLoading}
	/>
	</div>
	{/* Submit Button */}
	<Button type="submit" className="w-full" disabled={isLoading}>
	{isLoading ? t("onboarding:creating_account") : t("onboarding:sign_up")}
	</Button>
	</form>
	{/* Login Link */}
	<div className="mt-6 text-center">
	<p className="text-sm text-gray-600">
	{t("onboarding:already_have_account")}{" "}
	<a
	href="/login"
	className="text-blue-600 hover:text-blue-700 font-medium"
	>
	{t("onboarding:sign_in")}
	</a>
	</p>
	</div>
	{/* Terms */}
	<div className="mt-6 pt-6 border-t border-gray-200">
	<p className="text-xs text-gray-500 text-center">
	{t("onboarding:signup_terms")}
	</p>
	</div>
	</div>
	</div>
	);
	}

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/Signup.tsx` around lines 8 - 193, The Signup
component currently contains hardcoded user-facing strings; import
useTranslation from react-i18next at the top of the file, call const { t } =
useTranslation(/* appropriate namespaces e.g. "common","errors","onboarding" */)
inside the Signup function, and replace all visible strings and error messages
used in the JSX and in handleSubmit (e.g., "Create Account", form labels,
placeholders, button text, "Please fill in all required fields", "Passwords do
not match", "Password must be at least 8 characters", "Signup failed", etc.)
with t('your.namespace.key') keys; ensure you create or reference appropriate
i18n keys for labels, placeholders, button copy and validation errors and keep
keys grouped by namespace (common, errors, onboarding) as suggested.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash

# 1. Check if centralized API client exists
echo "=== Checking for centralized API client ==="
if [ -f "apps/web-frontend/src/api/client.ts" ]; then
  echo "✓ API client file exists"
  head -50 apps/web-frontend/src/api/client.ts
else
  echo "✗ API client file NOT found"
  find apps/web-frontend/src -name "*client*" -o -name "*api*" | head -20
fi

Repository: OBenner/Auto-Coding

Length of output: 1240

---

🏁 Script executed:

#!/bin/bash

# 2. Search for authentication context and user management
echo "=== Searching for authentication context ==="
rg -n "AuthContext|UserContext|useAuth|useUser|useCurrentUser" apps/web-frontend/src/ --type=tsx --type=ts

echo ""
echo "=== Searching for user_id patterns in other files ==="
rg -n "user_id|userId" apps/web-frontend/src/ --type=tsx --type=ts | head -30

Repository: OBenner/Auto-Coding

Length of output: 235

---

🏁 Script executed:

#!/bin/bash

# 3. Look at the complete structure of the API client
echo "=== Complete API client implementation ==="
wc -l apps/web-frontend/src/api/client.ts
echo ""
cat apps/web-frontend/src/api/client.ts

Repository: OBenner/Auto-Coding

Length of output: 6776

---

🏁 Script executed:

#!/bin/bash

# 4. Search for other API calls in the codebase to see usage patterns
echo "=== Checking other API calls and fetch patterns ==="
rg -n "apiClient\.|fetch(" apps/web-frontend/src/ --type=tsx --type=ts -A2 | head -50

Repository: OBenner/Auto-Coding

Length of output: 218

---

Replace hardcoded user_id and use centralized API client pattern.

Both fetchDashboardData and fetchStatsData have critical issues:

1. Hardcoded user_id=1 (lines 53, 73): This placeholder will display incorrect data for all users except user ID 1. The user ID needs to be obtained from the actual user context rather than hardcoded.

2. Bypassing centralized API client: The codebase has an established ApiClient at apps/web-frontend/src/api/client.ts that provides timeout handling (30s default), consistent error handling, and structured logging. Using raw fetch here bypasses these safeguards and creates inconsistent patterns across the application.

3. No timeout protection: Unlike the centralized client (which enforces a 30-second timeout with AbortController), these raw fetch calls could hang indefinitely if the backend becomes unresponsive.

To fix:

• Extend ApiClient with usage/dashboard and usage/stats endpoints, or create a dedicated usage service following the same pattern
• Replace the hardcoded user_id=1 with the actual authenticated user ID
• Remove raw fetch calls in favor of the client abstraction

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/UsageDashboard.tsx` around lines 50 - 88,
fetchDashboardData and fetchStatsData currently hardcode user_id=1 and call
fetch directly, bypassing the app's ApiClient (which provides timeout, error
handling, and logging); replace both raw fetch usages by calling the centralized
ApiClient (or a new UsageService that wraps ApiClient) to call the endpoints
(usage/dashboard and usage/stats), pass the authenticated user's ID from the
user context instead of the hardcoded 1, and rely on ApiClient's
timeout/AbortController and error mapping rather than manual fetch/error
handling in fetchDashboardData and fetchStatsData.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Optimize useEffect to avoid unnecessary API calls.

The useEffect calls both fetchDashboardData() and fetchStatsData() whenever period or daysBack changes. However, fetchDashboardData doesn't use these parameters and fetches the same data regardless. This causes unnecessary API calls.

♻️ Proposed optimization

Separate the effects and wrap functions in useCallback for stable references:

+import { useEffect, useState, useCallback } from "react";

-const fetchDashboardData = async () => {
+const fetchDashboardData = useCallback(async () => {
   try {
     const apiUrl = import.meta.env.VITE_API_URL || "http://localhost:8000";
     const response = await fetch(`${apiUrl}/api/usage/dashboard?user_id=1`);
     // ... rest of function
   }
-};
+}, []);

-const fetchStatsData = async () => {
+const fetchStatsData = useCallback(async () => {
   try {
     const apiUrl = import.meta.env.VITE_API_URL || "http://localhost:8000";
     const response = await fetch(
       `${apiUrl}/api/usage/stats?user_id=1&period=${period}&days_back=${daysBack}`,
     );
     // ... rest of function
   }
-};
+}, [period, daysBack]);

+// Fetch dashboard data only on mount
+useEffect(() => {
+  fetchDashboardData();
+}, [fetchDashboardData]);
+
+// Fetch stats data when period or daysBack changes
 useEffect(() => {
-  const loadData = async () => {
-    setIsLoading(true);
-    setError(null);
-
-    await Promise.all([fetchDashboardData(), fetchStatsData()]);
-
-    setIsLoading(false);
-  };
-
-  loadData();
+  setIsLoading(true);
+  setError(null);
+  
+  fetchStatsData().finally(() => setIsLoading(false));
 }, [period, daysBack]);

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/UsageDashboard.tsx` around lines 90 - 101, The
current useEffect always calls both fetchDashboardData and fetchStatsData when
period or daysBack change, causing unnecessary fetches; separate into two
effects and stabilize the functions with useCallback: keep fetchDashboardData
wrapped in a useCallback with an empty dependency array (or move it outside the
effect) so it only runs once (or when its true deps change), and wrap
fetchStatsData in a useCallback that depends on [period, daysBack], then have
one useEffect that calls the dashboard loader once and another that calls the
stats loader when period/daysBack change (remove the combined loadData that
calls both).

[coderabbitai]

Review continued from previous batch...

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing validation for session_id query parameter.

The session_id is taken directly from query parameters without validation and used as a dictionary key and in log messages. While _sanitize_log prevents log injection, a malicious or excessively long session_id could cause issues.

🛡️ Suggested validation

     token = websocket.query_params.get("token")
-    session_id = websocket.query_params.get("session_id", "default")
+    session_id = websocket.query_params.get("session_id", "default")
+    
+    # Validate session_id format
+    if not session_id or len(session_id) > 128 or not session_id.replace("-", "").replace("_", "").isalnum():
+        await websocket.accept()
+        await websocket.send_json({"type": "error", "message": "Invalid session_id format"})
+        await websocket.close(code=status.WS_1008_POLICY_VIOLATION)
+        return

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Extract and validate token from query parameters
	token = websocket.query_params.get("token")
	session_id = websocket.query_params.get("session_id", "default")
	try:
	user_claims = verify_websocket_token(token)
	except Exception as e:
	await websocket.accept()
	await websocket.close(code=status.WS_1008_POLICY_VIOLATION)
	logger.warning(f"Terminal WebSocket authentication failed: {e}")
	return
	# Extract and validate token from query parameters
	token = websocket.query_params.get("token")
	session_id = websocket.query_params.get("session_id", "default")
	# Validate session_id format
	if not session_id or len(session_id) > 128 or not session_id.replace("-", "").replace("_", "").isalnum():
	await websocket.accept()
	await websocket.send_json({"type": "error", "message": "Invalid session_id format"})
	await websocket.close(code=status.WS_1008_POLICY_VIOLATION)
	return
	try:
	user_claims = verify_websocket_token(token)
	except Exception as e:
	await websocket.accept()
	await websocket.close(code=status.WS_1008_POLICY_VIOLATION)
	logger.warning(f"Terminal WebSocket authentication failed: {e}")
	return

🤖 Prompt for AI Agents

In `@apps/web-backend/api/websocket.py` around lines 436 - 446, The session_id
taken from websocket.query_params is used as a dict key and in logs but is not
validated; add validation immediately after retrieving session_id (the variable
named session_id in websocket handling code) to trim whitespace, enforce an
allowed pattern (e.g., alphanumeric plus - and _), and cap length (e.g., 256
chars); if the session_id fails validation, accept then close the websocket with
status.WS_1008_POLICY_VIOLATION and log a sanitized message (use existing
_sanitize_log or similar) rather than using the raw value, and only proceed to
use session_id (for dict keys or logs) when it passes these checks.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Potential path traversal risk with working_dir from user claims.

The working_dir is extracted from user_claims and passed directly to terminal session creation. If the JWT contains a malicious path (e.g., ../../etc), it could allow directory traversal. The terminal_manager.create_session validates existence but not whether the path is within allowed bounds.

Consider validating that working_dir is within an allowed base directory:

🛡️ Suggested path validation

+    import os.path
+    
     # Get or create terminal session
     session = terminal_manager.get_session(session_id)
     if not session:
         # Create new session with user's home directory or default project directory
         working_dir = user_claims.get("working_dir", os.getcwd())
+        
+        # Validate working_dir is within allowed paths
+        allowed_base = os.path.abspath(os.getcwd())
+        abs_working_dir = os.path.abspath(working_dir)
+        if not abs_working_dir.startswith(allowed_base):
+            await websocket.send_json(
+                {"type": "error", "message": "Invalid working directory"}
+            )
+            await websocket.close(code=status.WS_1008_POLICY_VIOLATION)
+            return
+        working_dir = abs_working_dir

🤖 Prompt for AI Agents

In `@apps/web-backend/api/websocket.py` around lines 454 - 465,
user_claims["working_dir"] is used directly when calling
terminal_manager.create_session (via session_id and working_dir) which permits
path traversal; before creating the session resolve and sanitize the path (e.g.,
os.path.realpath or pathlib.Path.resolve) and ensure it is inside an allowed
base directory (for example the user's home or a configured PROJECTS_ROOT) by
comparing commonpath or prefix, and if the check fails fall back to a safe
default (os.getcwd() or the user's home); apply this validation where
working_dir is read prior to calling terminal_manager.create_session so
terminal_manager.get_session / terminal_manager.create_session only ever receive
validated paths.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Inconsistent HTTP status codes between missing and invalid tokens.

The function raises HTTP 403 for missing tokens (Line 186) but delegates to verify_token which raises HTTP 401 for invalid tokens (Line 63). This creates inconsistent error responses for WebSocket authentication failures.

Consider whether both cases should return the same status code for consistency, or document this intentional distinction.

🔧 Option A: Consistent 403 for WebSocket context

 def verify_websocket_token(token: str) -> dict:
     ...
     if not token:
         raise HTTPException(
             status_code=status.HTTP_403_FORBIDDEN,
             detail="Authentication token required",
         )
 
-    return verify_token(token)
+    try:
+        return verify_token(token)
+    except HTTPException:
+        raise HTTPException(
+            status_code=status.HTTP_403_FORBIDDEN,
+            detail="Invalid authentication token",
+        )

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	def verify_websocket_token(token: str) -> dict:
	"""
	Verify and decode JWT token for WebSocket connections.
	This function is specifically designed for WebSocket authentication,
	where tokens are typically passed via query parameters instead of headers.
	Args:
	token: JWT token string to verify
	Returns:
	Dictionary of decoded token claims
	Raises:
	HTTPException: If token is invalid or expired (403 status for WebSocket rejection)
	Example:
	# In WebSocket endpoint
	token = websocket.query_params.get("token")
	claims = verify_websocket_token(token)
	"""
	if not token:
	raise HTTPException(
	status_code=status.HTTP_403_FORBIDDEN,
	detail="Authentication token required",
	)
	return verify_token(token)
	def verify_websocket_token(token: str) -> dict:
	"""
	Verify and decode JWT token for WebSocket connections.
	This function is specifically designed for WebSocket authentication,
	where tokens are typically passed via query parameters instead of headers.
	Args:
	token: JWT token string to verify
	Returns:
	Dictionary of decoded token claims
	Raises:
	HTTPException: If token is invalid or expired (403 status for WebSocket rejection)
	Example:
	# In WebSocket endpoint
	token = websocket.query_params.get("token")
	claims = verify_websocket_token(token)
	"""
	if not token:
	raise HTTPException(
	status_code=status.HTTP_403_FORBIDDEN,
	detail="Authentication token required",
	)
	try:
	return verify_token(token)
	except HTTPException:
	raise HTTPException(
	status_code=status.HTTP_403_FORBIDDEN,
	detail="Invalid authentication token",
	)

🤖 Prompt for AI Agents

In `@apps/web-backend/core/security.py` around lines 163 - 190,
verify_websocket_token currently raises HTTP 403 for missing tokens but
delegates to verify_token which raises HTTP 401 for invalid tokens, producing
inconsistent status codes; update verify_websocket_token to normalize responses
for WebSocket usage by catching HTTPException from verify_token and re-raising
it with status_code=status.HTTP_403_FORBIDDEN (or alternatively modify
verify_token to accept a websocket_context boolean and return 403 when true);
target the verify_websocket_token function and the verify_token call to ensure
both missing and invalid-token cases return the same 403 status for WebSocket
authentication.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Hardcoded fallback secret key poses session security risk.

If SECRET_KEY is not configured, the fallback "dev-secret-key-change-in-production" allows session forgery. Consider failing fast in production instead of using a weak fallback.

🛡️ Suggested approach

+import sys
+
 # Configure session middleware for OAuth state management
+if not SECRET_KEY and os.getenv("ENVIRONMENT", "development") == "production":
+    logger.error("SECRET_KEY must be configured in production!")
+    sys.exit(1)
+
 app.add_middleware(
     SessionMiddleware, secret_key=SECRET_KEY or "dev-secret-key-change-in-production"
 )

🤖 Prompt for AI Agents

In `@apps/web-backend/main.py` around lines 78 - 80, The SessionMiddleware is
being initialized with a hardcoded fallback secret_key
("dev-secret-key-change-in-production"); remove this weak default and require a
real SECRET_KEY: change the logic around app.add_middleware / SessionMiddleware
to not use the literal fallback, and instead validate SECRET_KEY at startup
(e.g., if SECRET_KEY is falsy and the app is not running in a development/debug
mode) raise an explicit RuntimeError or exit with a clear message so the process
fails fast in production; keep a safe dev-only path that allows a locally
provided debug secret when an explicit debug/dev flag is set.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Python asyncio get_event_loop deprecated Python 3.10 get_running_loop

💡 Result:

• Inside async def / callbacks: use asyncio.get_running_loop(). It returns the currently running loop and raises RuntimeError if none is running. The docs explicitly say it’s preferred because get_event_loop() has “rather complex behavior.” [1]

  async def worker():
      loop = asyncio.get_running_loop()
      loop.create_task(do_stuff())

• Why get_event_loop() started “being deprecated” around 3.10: in Python 3.10 asyncio.get_event_loop() began emitting DeprecationWarning when there is no running event loop (the exact conditions changed across 3.10.x/3.11.x), and the docs note that in a future release it will become an error. [1] The CPython discussion also points toward making it effectively behave like get_running_loop() in the future (i.e., error if no loop is running). [2]

• In Python 3.12+ the docs mark this more formally: asyncio.get_event_loop() is deprecated when there is no current event loop, and again: will become an error in a future Python release. [3]

• From synchronous “main” code: prefer asyncio.run() (high-level entry point) instead of manually grabbing a loop. [1]

  async def main():
      ...
  
  if __name__ == "__main__":
      asyncio.run(main())

• If you truly need a loop object in sync code (advanced cases, threads, embedded runtimes): create and set it explicitly:

  loop = asyncio.new_event_loop()
  asyncio.set_event_loop(loop)
  try:
      loop.run_until_complete(main())
  finally:
      loop.close()

Sources: Python 3.10 asyncio event loop docs (warnings + “prefer get_running_loop”) [1], CPython issue on finishing get_event_loop deprecation [2], Python 3.12 docs deprecation note [3].

[1] (docs.python.org)
[2] (github.com)
[3] (docs.python.org)

Citations:

• 1: https://docs.python.org/3.10/library/asyncio-eventloop.html
• 2: Finish deprecation in asyncio.get_event_loop() python/cpython#93453
• 3: https://docs.python.org/3.12/library/asyncio-eventloop.html

---

🏁 Script executed:

# Find and examine the terminal_manager.py file
fd terminal_manager.py

Repository: OBenner/Auto-Coding

Length of output: 108

---

🏁 Script executed:

# Examine the actual code at lines 115-127 and surrounding context
cat -n apps/web-backend/services/terminal_manager.py | sed -n '100,135p'

Repository: OBenner/Auto-Coding

Length of output: 1505

---

🏁 Script executed:

# Also check line 224 and surrounding context
cat -n apps/web-backend/services/terminal_manager.py | sed -n '210,235p'

Repository: OBenner/Auto-Coding

Length of output: 1123

---

🏁 Script executed:

# Check the method signatures to verify they are async
cat -n apps/web-backend/services/terminal_manager.py | sed -n '110,130p'
cat -n apps/web-backend/services/terminal_manager.py | sed -n '220,230p'

Repository: OBenner/Auto-Coding

Length of output: 1332

---

Use asyncio.get_running_loop() instead of deprecated get_event_loop().

asyncio.get_event_loop() is deprecated in Python 3.10+ and emits a DeprecationWarning when called from within an async coroutine. Since both occurrences (lines 115 and 224) are inside async methods, use asyncio.get_running_loop() instead, which is the correct way to obtain the event loop from an async context.

♻️ Proposed fix

     async def _start_unix_pty(self) -> bool:
         """Start terminal with ptyprocess on Unix systems."""
         try:
-            loop = asyncio.get_event_loop()
+            loop = asyncio.get_running_loop()
 
             def _create_pty():

     async def _read_pty_output(self) -> str:
         """Read output from ptyprocess (non-blocking)."""
         if self.pty_process is None or not self.pty_process.isalive():
             return ""
 
         try:
-            loop = asyncio.get_event_loop()
+            loop = asyncio.get_running_loop()
 
             def _read():

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	loop = asyncio.get_event_loop()
	def _create_pty():
	proc = ptyprocess.PtyProcess.spawn(
	self.shell,
	cwd=self.working_dir,
	env=self.env,
	rows=self.rows,
	cols=self.cols,
	)
	return proc
	self.pty_process = await loop.run_in_executor(None, _create_pty)
	loop = asyncio.get_running_loop()
	def _create_pty():
	proc = ptyprocess.PtyProcess.spawn(
	self.shell,
	cwd=self.working_dir,
	env=self.env,
	rows=self.rows,
	cols=self.cols,
	)
	return proc
	self.pty_process = await loop.run_in_executor(None, _create_pty)

🤖 Prompt for AI Agents

In `@apps/web-backend/services/terminal_manager.py` around lines 115 - 127,
Replace deprecated asyncio.get_event_loop() with asyncio.get_running_loop() in
the async methods where the pty is created and at the other occurrence (both
inside async coroutines) to avoid DeprecationWarning; specifically change the
assignments that set loop = asyncio.get_event_loop() (used before calling
loop.run_in_executor in the block that defines _create_pty and spawns
ptyprocess.PtyProcess) to loop = asyncio.get_running_loop() so the running loop
is retrieved correctly from the coroutine context.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Usage stats are hardcoded placeholders.

The usage statistics (847 requests, 12.4K tokens, etc.) and progress bars are static. Ensure these are wired to real data before release, or add a visible "demo data" indicator to avoid user confusion.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Unused useTranslation hook result.

The hook is called but its return value (t function) is not destructured or used. Either remove this call or use it for translating the hardcoded strings in this component.

-	useTranslation(["common", "tasks"]);
+	const { t } = useTranslation(["common", "tasks"]);

Based on learnings: "Import translation helpers from react-i18next and use useTranslation() hook with appropriate namespaces"

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	useTranslation(["common", "tasks"]);
	const { t } = useTranslation(["common", "tasks"]);

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/TaskCreate.tsx` at line 40, The useTranslation
hook call is unused; either remove the call or destructure and use its
translator function to localize strings in this component: change the call to
destructure the t function (useTranslation(["common","tasks"]) -> const { t } =
useTranslation([...])) and replace hardcoded UI strings in TaskCreate (component
render, labels, buttons, placeholders) with t("key") calls using appropriate
keys from the "common" and "tasks" namespaces; if you choose not to translate
here, simply remove the useTranslation invocation to eliminate the unused hook.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Move agentTypes array outside component or memoize.

This array is recreated on every render. Since it contains static data (apart from the JSX icons), consider moving it outside the component or wrapping in useMemo.

♻️ Suggested refactor

+const AGENT_TYPE_CONFIG = [
+	{ type: "planner" as const, label: "Planner", description: "Creates implementation plan with subtasks" },
+	{ type: "coder" as const, label: "Coder", description: "Implements individual subtasks" },
+	{ type: "qa_reviewer" as const, label: "QA Reviewer", description: "Validates acceptance criteria" },
+	{ type: "qa_fixer" as const, label: "QA Fixer", description: "Fixes QA-reported issues" },
+];
+
+const AGENT_ICONS: Record<AgentType, React.ReactNode> = {
+	planner: <Brain className="h-5 w-5" />,
+	coder: <Code className="h-5 w-5" />,
+	qa_reviewer: <Search className="h-5 w-5" />,
+	qa_fixer: <Wrench className="h-5 w-5" />,
+};
+
 export function TaskCreate({ onCreateSuccess }: TaskCreateProps) {

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/TaskCreate.tsx` around lines 51 - 81, The
agentTypes constant is being recreated on every render inside the TaskCreate
component; move the agentTypes array declaration out of the TaskCreate component
(make it a top-level module constant) or wrap it in useMemo inside TaskCreate to
avoid unnecessary re-creation of the static list and JSX icon nodes; update any
references to agentTypes in the component to use the relocated constant.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Move agentTypes configuration outside the component.

The agentTypes array is recreated on every render. Since it's static configuration, define it outside the component to avoid unnecessary allocations:

♻️ Suggested refactor

+/**
+ * Agent type configuration
+ */
+const AGENT_TYPES: Array<{
+	type: AgentType;
+	label: string;
+	description: string;
+	icon: React.ReactNode;
+}> = [
+	{
+		type: "planner",
+		label: "Planner",
+		description: "Create implementation plan with subtasks",
+		icon: <Brain className="h-4 w-4" />,
+	},
+	// ... rest of configuration
+];
+
 export function TaskDetail({ taskId, onBack }: TaskDetailProps) {
-	/**
-	 * Agent type configuration
-	 */
-	const agentTypes: Array<{...}> = [...];

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/TaskDetail.tsx` around lines 66 - 96, agentTypes
is a static configuration currently declared inside the TaskDetail component
causing it to be reallocated on every render; move the const agentTypes
declaration out of the component (e.g., top-level in the module) so it is
created once, keep its exact shape and typings (Array<{ type: AgentType; label:
string; description: string; icon: React.ReactNode }>), and ensure any
referenced symbols (AgentType, Brain, Code, Search, Wrench) remain
imported/available at module scope; if you need it elsewhere consider exporting
it as a named export.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Polling lacks cleanup on component unmount — potential memory leak.

The recursive setTimeout in pollAgentStatus continues running after the component unmounts, causing:

1. Memory leaks from retained closures
2. React warnings about state updates on unmounted components

🐛 Suggested fix using useRef for cleanup

+	const pollingRef = useRef<NodeJS.Timeout | null>(null);
+
+	// Cleanup polling on unmount
+	useEffect(() => {
+		return () => {
+			if (pollingRef.current) {
+				clearTimeout(pollingRef.current);
+			}
+		};
+	}, []);
+
 	const pollAgentStatus = useCallback(async (agentTaskId: string) => {
+		let isCancelled = false;
+		
 		const poll = async () => {
+			if (isCancelled) return;
+			
 			try {
 				const status = await apiClient.getAgentStatus(agentTaskId);
 				setAgentStatus(status);
 
 				// Continue polling if still running
 				if (status.status === "running") {
-					setTimeout(() => poll(), 2000);
+					pollingRef.current = setTimeout(() => poll(), 2000);
 				}
 			} catch (err) {
 				console.error("Failed to poll agent status:", err);
 			}
 		};
 
 		poll();
+		
+		return () => {
+			isCancelled = true;
+		};
 	}, []);

🤖 Prompt for AI Agents

In `@apps/web-frontend/src/pages/TaskDetail.tsx` around lines 170 - 186,
pollAgentStatus schedules recursive setTimeouts and doesn't clear them on
unmount, causing memory leaks and state updates after unmount; fix by adding a
ref (e.g., timeoutRef via useRef) and a mounted flag (e.g., isMountedRef) or an
abort signal, clear any pending timeout before scheduling a new one, guard
setAgentStatus with the mounted check, and ensure you
clearTimeout(timeoutRef.current) and set isMountedRef.current = false in a
useEffect cleanup or returned cancel function so all pending polls are cancelled
when the component unmounts.

[coderabbitai]

Actionable comments posted: 18

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

apps/backend/run_test_2.py (1)

76-185: 🧹 Nitpick | 🔵 Trivial

Consider extracting shared test infrastructure to a common module.

The SimpleTester class and test helper patterns (_create_test_client, _run_agent_query, spec_dir resolution) are duplicated across run_test_2.py and run_tests_3_5.py. Extracting these to a shared module (e.g., test_helpers.py) would reduce duplication and improve maintainability.

Example shared module structure

# apps/backend/test_helpers.py
from pathlib import Path
import os

def resolve_spec_dir(script_dir: Path) -> Path:
    """Resolve spec directory with fallback to temp."""
    cwd = Path.cwd()
    spec_dir = (
        cwd / ".auto-claude" / "specs"
        / "175-run-manual-ui-testing-validation-33-scenarios-docu"
    )
    if not spec_dir.exists():
        spec_dir = script_dir / ".temp_spec"
        spec_dir.mkdir(exist_ok=True)
    return spec_dir

class BaseTestRunner:
    def __init__(self, project_dir: Path, spec_dir: Path):
        self.project_dir = project_dir
        self.spec_dir = spec_dir
        self.verbose = os.getenv("VERBOSE", "false").lower() == "true"
        self.model_id = os.getenv("LLM_TEST_MODEL_ID", "claude-sonnet-4-5-20250929")

    def _create_test_client(self, create_client_fn):
        return create_client_fn(
            project_dir=self.project_dir,
            spec_dir=self.spec_dir,
            agent_type="qa_reviewer",
            model=self.model_id,
        )

    async def _run_agent_query(self, client, task: str) -> str:
        response_text = ""
        async with client:
            await client.query(task)
            async for msg in client.receive_response():
                if hasattr(msg, "content"):
                    response_text += str(msg.content)
                elif hasattr(msg, "text"):
                    response_text += str(msg.text)
        return response_text

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/run_test_2.py` around lines 76 - 185, The SimpleTester class
duplicates test setup and helper logic across files; extract shared
functionality into a new test_helpers.py and update run_test_2.py to use it.
Move spec_dir resolution and env-derived defaults into a resolve_spec_dir
function and the BaseTestRunner class (or similar) in test_helpers.py, then have
SimpleTester inherit from BaseTestRunner and replace its _create_test_client and
_run_agent_query implementations with calls to the shared BaseTestRunner methods
(or pass create_client into a shared _create_test_client wrapper); ensure unique
symbols referenced are BaseTestRunner, resolve_spec_dir, SimpleTester,
_create_test_client, and _run_agent_query so callers are updated to import from
test_helpers.py.

apps/web-backend/api/routes/specs.py (1)

50-134: 🧹 Nitpick | 🔵 Trivial

Extract duplicate helper functions to a shared module.

The functions _count_subtasks, _count_subtasks_detailed, _get_progress_percentage, _list_specs, and _get_spec_dir are duplicated identically between specs.py and tasks.py. Since "Tasks and specs are synonymous" per the docstrings, consider extracting these utilities to a common module (e.g., api/utils/spec_helpers.py) to adhere to the DRY principle and simplify maintenance.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/routes/specs.py` around lines 50 - 134, Duplicate helper
functions (_count_subtasks, _count_subtasks_detailed, _get_progress_percentage,
_list_specs, _get_spec_dir) exist in both specs.py and tasks.py; extract them
into a shared module (for example api/utils/spec_helpers.py), move the
implementations there, update specs.py and tasks.py to import the functions from
the new module, ensure signatures and behavior remain identical (preserve Path
typing and exception handling around json loading), and run tests/linters to
confirm no import cycles or naming conflicts.

apps/web-backend/api/routes/agents.py (1)

273-279: ⚠️ Potential issue | 🟡 Minor

Consider removing project_dir from health endpoint response.

The health endpoint exposes the server's file system path, which could aid reconnaissance in a security context. The PR commit message mentions "Remove sensitive or unnecessary exposures (raw exception details, project_dir in health responses)" but this endpoint still returns project_dir.

🛡️ Suggested fix

     return {
         "status": "ok",
         "endpoint": "agents",
-        "project_dir": str(project_dir),
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/routes/agents.py` around lines 273 - 279, The health
endpoint currently calls _get_project_dir() and includes "project_dir" in its
returned dict, exposing a filesystem path; remove the call to _get_project_dir()
and stop including the "project_dir" key in the returned response (i.e., return
only "status" and "endpoint" or other non-sensitive fields), and delete or avoid
the unused project_dir variable so there are no side-effects from the removed
call in the handler that builds the response.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/backend/agents/templates/test_runner.py`:
- Around line 386-401: The docstring for the function whose signature ends with
") -> str:" and starts with "Create a test prompt for template testing." is
indented 4 spaces inside the triple quotes; fix it by aligning the docstring
text with the opening triple quotes (remove the extra 4-space indentation) so
the first line and subsequent lines are flush with the opening """ to follow
standard Python docstring conventions and avoid tooling issues.

In `@apps/backend/cli/template_commands.py`:
- Around line 261-263: The printed example uses a literal '{template.name}' so
the template.name value isn't interpolated; update the print string to use an
f-string that interpolates template.name (e.g., change the argument to print in
the same statement referencing create_client(template='{template.name}') to use
f-string interpolation) so the actual template.name is shown; locate the print
call that prints "2. Or use it programmatically via
create_client(template='{template.name}')" and replace the inner quoted
placeholder with an interpolated expression referencing template.name (keeping
the surrounding text intact).

In `@apps/backend/run_test_2.py`:
- Around line 177-179: The code currently does lazy imports of traceback inside
exception handlers (seen where traceback.print_exc() is called in run_test_2.py
around the exception blocks at the noted locations); move the import to the
module level by adding "import traceback" at the top of the file and remove the
local imports inside those except handlers so all traceback.print_exc() calls
use the module-level import for consistency with run_tests_3_5.py.
- Around line 100-104: Remove the unused local variable msg_type (assigned as
type(msg).__name__) from the block that builds response_text; either delete the
line "msg_type = type(msg).__name__" or replace it with a real use if intended,
keeping the existing checks on msg.content and msg.text (the code that appends
to response_text) intact so no other logic changes are needed.

In `@apps/backend/run_tests_3_5.py`:
- Around line 270-276: The test result dict is always setting "passed": True;
update the logic before appending to all_results so "passed" is computed from
perf_results (e.g., treat empty/None perf_results as failed, mark failed if any
benchmark entry contains an "error" field or its measured latency exceeds the
expected/target value). Modify the block that builds the dict (the one that
produces the "test_5_performance_benchmarks" entry) to compute a boolean like
all_ok = perf_results is not None and all(entry.get("error") is None and
entry.get("latency", float("inf")) <= entry.get("target_latency", float("inf"))
for entry in perf_results) and use that for "passed"; also ensure exceptions
during benchmarking set passed = False and capture an error message in the
result.
- Around line 63-66: Move the inline "import traceback" statements out of the
exception handlers and add a single "import traceback" at module level near the
other imports; then remove the in-handler imports and leave the existing
traceback.print_exc() calls as-is so the exception handlers still print the
tracebacks. This change touches the places calling traceback.print_exc()
(currently importing traceback inline before that call) — add the top-level
import and delete the redundant local imports.

In `@apps/web-backend/api/routes/git.py`:
- Around line 125-135: The response currently exposes the OAuth access token via
the "access_token" field in the returned dict (alongside "provider": "github"
and the "user" object); remove the "access_token" value from the JSON response,
persist the token server-side (e.g., DB or server session store keyed to a
session id or user record) inside the OAuth callback handler, and return only a
server-issued session identifier or JWT to the client instead of the raw token
so tokens are never returned in response bodies or subject to proxy/browser
leakage.

In `@apps/web-backend/services/agent_runner.py`:
- Around line 316-317: The call to _init_websocket_broadcast() inside
start_agent_task is redundant because run_agent_async already initializes the
broadcast (and there is a guard to prevent double-init); remove the
_init_websocket_broadcast() invocation from start_agent_task so that
start_agent_task simply delegates to run_agent_async without its own
initialization call, leaving the broadcast setup responsibility to
run_agent_async (keep the existing guard in _init_websocket_broadcast as-is).

In `@apps/web-backend/tests/test_integration.py`:
- Around line 862-924: Remove the always-passing noisy test function
test_integration_test_summary and relocate its human-readable coverage
dictionary and printed summary into documentation: either add that coverage
mapping and descriptive text as the module docstring at the top of the
tests/test_integration.py file or put it into a dedicated README or docs file
for integration test coverage. If you must keep it in tests for discoverability,
convert test_integration_test_summary into non-test documentation by renaming it
(remove the test_ prefix) or mark it with pytest.mark.skip and add a comment
pointing to the new documentation location so the test suite no longer prints
the summary.

In `@apps/web-frontend/src/components/FileExplorer.tsx`:
- Around line 383-387: The useEffect currently calls handleRefresh() which
returns a Promise and can trigger unhandled-promise lint warnings; update the
effect to explicitly ignore the returned Promise by prefixing the call with the
void operator (i.e. use void handleRefresh()) inside the useEffect that depends
on apiUrl and initialFiles so the promise is intentionally suppressed while
preserving the existing logic around apiUrl, initialFiles and the handleRefresh
function.
- Around line 513-523: The refresh Button in FileExplorer.tsx (the Button
element with onClick={handleRefresh} wrapping the RefreshCw icon) is missing an
aria-label; add an aria-label prop (e.g., aria-label={isLoading ? "Refreshing
files" : "Refresh files"}) to the Button to make its purpose clear to screen
readers while preserving the existing disabled and icon behavior.

In `@apps/web-frontend/src/components/GitHubConnect.tsx`:
- Around line 30-46: The OAuth callback handling in the GitHubConnect component
currently reads URL params but the backend returns JSON, so user data never
reaches the frontend; update the useEffect inside GitHubConnect to, on mount or
when a callback occurs, call a backend endpoint to fetch the OAuth result (e.g.,
implement /api/git/github/status to return { connected, user, ... } or adapt the
frontend to call the existing /api/git/status if extended to include user info),
then call setStatus(...) and setSuccess(...) / setError(...) with the returned
payload and keep the window.history.replaceState cleanup; if the backend
endpoint is missing, add a server route that reads the session/stored OAuth
result and returns the user connection data so status.user is populated for the
user-info render path.

In `@apps/web-frontend/src/components/Sidebar.tsx`:
- Around line 116-117: There are two separate TooltipProvider instances wrapping
the navigation items (the <TooltipProvider> around the div with className
"space-y-1") and the footer actions; consolidate them by moving a single
TooltipProvider higher in the Sidebar component so it wraps both the navigation
container and the footer actions container, remove the duplicate TooltipProvider
instance, and ensure TooltipProvider is still imported and used once around the
elements that render navigation items and footer actions.
- Around line 60-70: The nav items in baseNavItems use hardcoded English labels;
update to use translation keys (e.g., label: "nav.kanban") and resolve them at
render time using react-i18next: import useTranslation and call t() where
NavItem.label is rendered (keep ids like "kanban" as stable keys), or
alternatively store a separate labelKey property on each NavItem and call
t(labelKey) in the Sidebar render function; ensure you load the appropriate
namespace and update tests/fixtures that reference the plaintext labels.

In `@apps/web-frontend/src/components/Terminal.tsx`:
- Around line 224-253: Add a guard to avoid auto-reconnect after intentional
closes or when the closed socket is stale: introduce a ref flag (e.g.,
intentionalCloseRef) that handleReconnect and the cleanup/unmount set to true
before calling ws.close(); in ws.onclose, only schedule a reconnect if
intentionalCloseRef.current is false AND the closed ws equals wsRef.current (to
avoid acting on stale sockets); when clearing reconnectTimeoutRef via
clearTimeout, also set reconnectTimeoutRef.current = null; ensure
pingIntervalRef clearing also nulls the ref to avoid stale timers.
- Around line 350-423: The error overlay uses absolute positioning but the root
container div (the top-level element using cn(..., isActive && "ring-2
ring-primary")) is not positioned, so the overlay can cover the whole page;
update that root container's className to include "relative" (i.e., add
"relative" into the cn call around the flex container) so the overlay div (the
one with className "absolute inset-0 bg-black/80...") is positioned relative to
the Terminal component and confined to its bounds.
- Around line 73-82: The current useMemo for authToken won't update when
localStorage changes while the component is mounted; instead, read the token at
the time you open or (re)create the WebSocket connection so the socket uses the
latest token. Locate the effect or function that creates the WebSocket (the
"WebSocket effect" / connect/createSocket function) and replace the dependency
on the memoized authToken with a fresh read of
localStorage.getItem("auth_token") inside that effect (with try/catch), or add a
window.addEventListener('storage') that updates component state and triggers the
reconnect; remove reliance on the useMemo authToken for socket auth so
reconnects use the newest token.

---

Outside diff comments:
In `@apps/backend/run_test_2.py`:
- Around line 76-185: The SimpleTester class duplicates test setup and helper
logic across files; extract shared functionality into a new test_helpers.py and
update run_test_2.py to use it. Move spec_dir resolution and env-derived
defaults into a resolve_spec_dir function and the BaseTestRunner class (or
similar) in test_helpers.py, then have SimpleTester inherit from BaseTestRunner
and replace its _create_test_client and _run_agent_query implementations with
calls to the shared BaseTestRunner methods (or pass create_client into a shared
_create_test_client wrapper); ensure unique symbols referenced are
BaseTestRunner, resolve_spec_dir, SimpleTester, _create_test_client, and
_run_agent_query so callers are updated to import from test_helpers.py.

In `@apps/web-backend/api/routes/agents.py`:
- Around line 273-279: The health endpoint currently calls _get_project_dir()
and includes "project_dir" in its returned dict, exposing a filesystem path;
remove the call to _get_project_dir() and stop including the "project_dir" key
in the returned response (i.e., return only "status" and "endpoint" or other
non-sensitive fields), and delete or avoid the unused project_dir variable so
there are no side-effects from the removed call in the handler that builds the
response.

In `@apps/web-backend/api/routes/specs.py`:
- Around line 50-134: Duplicate helper functions (_count_subtasks,
_count_subtasks_detailed, _get_progress_percentage, _list_specs, _get_spec_dir)
exist in both specs.py and tasks.py; extract them into a shared module (for
example api/utils/spec_helpers.py), move the implementations there, update
specs.py and tasks.py to import the functions from the new module, ensure
signatures and behavior remain identical (preserve Path typing and exception
handling around json loading), and run tests/linters to confirm no import cycles
or naming conflicts.

---

Duplicate comments:
In `@apps/web-backend/migrations/versions/002_create_repositories.py`:
- Around line 34-63: The migration currently creates plaintext token columns
access_token and refresh_token on the repositories table; change storage to
encrypted form by replacing these Text columns with a binary or specially typed
encrypted column (e.g., use sa.LargeBinary or your project’s EncryptedType) and
add any needed iv/salt columns (e.g., access_token_iv) or a reference column for
a secrets manager; update the repositories model to encrypt on write/decrypt on
read using the project encryption util or a KMS/secrets service and add a
data-migration plan to encrypt existing tokens (or migrate them to the secrets
manager) so stored values are never plaintext in db.

In `@apps/web-backend/tests/test_integration.py`:
- Around line 769-854: The tests mutate global manager state
(manager.active_connections and manager.spec_subscriptions) which can leak
between tests; add an autouse pytest fixture in this test module that runs
around each test to snapshot and restore or clear those structures before and
after each test to guarantee isolation (e.g., backup manager.active_connections
and manager.spec_subscriptions at setup and restore them at teardown, or call a
manager.reset()/clear() if available) and ensure the fixture covers tests using
broadcast_execution_event, broadcast_log_event, and broadcast_error_event.
- Around line 342-445: Replace the current pattern of nested async functions
plus asyncio.run() in test_task_lifecycle_simulation,
test_task_cancellation_flow, and test_cleanup_removes_completed_tasks with
pytest-asyncio style tests: convert each top-level test_... function to async
def and add `@pytest.mark.asyncio`, move the inner async helper functions (e.g.,
mock_agent_task, long_running_task, quick_task, running_task) inside or keep
them as nested async defs, and remove asyncio.run() calls; ensure you still use
patch.dict(_running_tasks, {}, clear=True), call cancel_task, get_task_status,
and cleanup_completed_tasks the same way so behavior is unchanged while adopting
the pytest.mark.asyncio decorator pattern.

In `@apps/web-frontend/src/components/GitHubConnect.tsx`:
- Around line 59-63: The disconnect handler handleDisconnect currently only
updates local state via setStatus and setSuccess and contains a TODO; update
handleDisconnect to call the backend disconnect endpoint when available (e.g.,
invoke an API client or fetch to POST/DELETE the user's GitHub disconnect
route), handle errors and only clear local connected state (setStatus({
connected: false })) and show success (setSuccess) after a successful response,
and ensure failures set an appropriate error state; keep the existing local-only
behavior as a fallback if the backend is unreachable.
- Around line 153-158: The "Refresh Connection" button in the GitHubConnect
component currently just calls setSuccess with a placeholder string instead of
performing a real refresh; replace the placeholder handler on the Button (the
onClick that calls setSuccess) with the actual refresh logic: invoke the
existing refresh/reauth endpoint or function (e.g., call the same API that
initiates/revalidates the GitHub connection or token), handle success and error
by calling setSuccess and setError appropriately, and refresh any local state or
cached data (e.g., trigger the same state update or SWR/mutate logic used after
initial connect); ensure the handler references the same symbols used elsewhere
in this component (setSuccess, setError, any refreshGitHubStatus/reauth function
or API call) so the button actually revalidates the connection instead of
showing a placeholder message.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Docstring indentation is inconsistent.

The docstring content (lines 390-400) uses 4-space indentation from the function body, but the standard Python convention is to align docstring content with the opening triple quotes. This may cause issues with documentation generators and IDE tooling.

♻️ Proposed fix for docstring indentation

 def create_test_prompt(
     template: AgentTemplate,
     custom_instructions: str | None = None,
 ) -> str:
     """
-        Create a test prompt for template testing.
+    Create a test prompt for template testing.
 
-        Generates a test prompt that exercises the template's configuration:
-    - Tests if the agent understands its custom prompt
-    - Verifies tool awareness
-    - Checks if MCP servers are accessible
+    Generates a test prompt that exercises the template's configuration:
+    - Tests if the agent understands its custom prompt
+    - Verifies tool awareness
+    - Checks if MCP servers are accessible
 
-        Args:
-            template: AgentTemplate instance being tested
-            custom_instructions: Optional custom test instructions
+    Args:
+        template: AgentTemplate instance being tested
+        custom_instructions: Optional custom test instructions
 
-        Returns:
-            Test prompt string
+    Returns:
+        Test prompt string
     """

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	) -> str:
	"""
	Create a test prompt for template testing.
	Create a test prompt for template testing.
	Generates a test prompt that exercises the template's configuration:
	- Tests if the agent understands its custom prompt
	- Verifies tool awareness
	- Checks if MCP servers are accessible
	Generates a test prompt that exercises the template's configuration:
	- Tests if the agent understands its custom prompt
	- Verifies tool awareness
	- Checks if MCP servers are accessible
	Args:
	template: AgentTemplate instance being tested
	custom_instructions: Optional custom test instructions
	Args:
	template: AgentTemplate instance being tested
	custom_instructions: Optional custom test instructions
	Returns:
	Test prompt string
	Returns:
	Test prompt string
	"""
	) -> str:
	"""
	Create a test prompt for template testing.
	Generates a test prompt that exercises the template's configuration:
	- Tests if the agent understands its custom prompt
	- Verifies tool awareness
	- Checks if MCP servers are accessible
	Args:
	template: AgentTemplate instance being tested
	custom_instructions: Optional custom test instructions
	Returns:
	Test prompt string
	"""

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/agents/templates/test_runner.py` around lines 386 - 401, The
docstring for the function whose signature ends with ") -> str:" and starts with
"Create a test prompt for template testing." is indented 4 spaces inside the
triple quotes; fix it by aligning the docstring text with the opening triple
quotes (remove the extra 4-space indentation) so the first line and subsequent
lines are flush with the opening """ to follow standard Python docstring
conventions and avoid tooling issues.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Bug: F-string variable not interpolated due to nested quotes.

The {template.name} inside the single-quoted string literal '{template.name}' will not be interpolated—it will print literally as {template.name}. The outer f-string quotes are double, but the inner single quotes create a plain string.

🐛 Proposed fix

     print(
-        "  2. Or use it programmatically via create_client(template='{template.name}')"
+        f"  2. Or use it programmatically via create_client(template='{template.name}')"
     )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/cli/template_commands.py` around lines 261 - 263, The printed
example uses a literal '{template.name}' so the template.name value isn't
interpolated; update the print string to use an f-string that interpolates
template.name (e.g., change the argument to print in the same statement
referencing create_client(template='{template.name}') to use f-string
interpolation) so the actual template.name is shown; locate the print call that
prints "2. Or use it programmatically via
create_client(template='{template.name}')" and replace the inner quoted
placeholder with an interpolated expression referencing template.name (keeping
the surrounding text intact).

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove unused variable msg_type.

The variable msg_type is assigned but never used. This appears to be debug/logging code that was left behind.

Suggested fix

                 async for msg in client.receive_response():
-                    msg_type = type(msg).__name__
                     if hasattr(msg, "content"):
                         response_text += str(msg.content)
                     elif hasattr(msg, "text"):
                         response_text += str(msg.text)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/run_test_2.py` around lines 100 - 104, Remove the unused local
variable msg_type (assigned as type(msg).__name__) from the block that builds
response_text; either delete the line "msg_type = type(msg).__name__" or replace
it with a real use if intended, keeping the existing checks on msg.content and
msg.text (the code that appends to response_text) intact so no other logic
changes are needed.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consistent with run_tests_3_5.py: Consider module-level traceback import.

Same pattern as the other file - lazy traceback import inside exception handlers. Consider moving to module level for consistency.

Also applies to: 194-196

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/run_test_2.py` around lines 177 - 179, The code currently does
lazy imports of traceback inside exception handlers (seen where
traceback.print_exc() is called in run_test_2.py around the exception blocks at
the noted locations); move the import to the module level by adding "import
traceback" at the top of the file and remove the local imports inside those
except handlers so all traceback.print_exc() calls use the module-level import
for consistency with run_tests_3_5.py.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider moving traceback import to module level.

The traceback import is performed lazily inside exception handlers (here and at lines 134-136). While acceptable for test scripts, moving it to the module-level imports would be cleaner and avoid repeated imports.

Suggested refactor

 import asyncio
 import json
 import os
 import sys
 import time
+import traceback
 from pathlib import Path

Then remove the inline imports at lines 63-65 and 134-136.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/run_tests_3_5.py` around lines 63 - 66, Move the inline "import
traceback" statements out of the exception handlers and add a single "import
traceback" at module level near the other imports; then remove the in-handler
imports and leave the existing traceback.print_exc() calls as-is so the
exception handlers still print the tracebacks. This change touches the places
calling traceback.print_exc() (currently importing traceback inline before that
call) — add the top-level import and delete the redundant local imports.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider using translation keys for navigation labels.

The baseNavItems array uses hardcoded English labels. For full i18n support, consider using translation keys and resolving them at render time.

♻️ Optional refactor for i18n support

 const baseNavItems: NavItem[] = [
-	{ id: "kanban", label: "Kanban", icon: LayoutGrid, shortcut: "K" },
-	{ id: "terminals", label: "Terminals", icon: Terminal, shortcut: "A" },
-	{ id: "insights", label: "Insights", icon: Sparkles, shortcut: "N" },
-	{ id: "roadmap", label: "Roadmap", icon: Map, shortcut: "D" },
-	{ id: "ideation", label: "Ideation", icon: Lightbulb, shortcut: "I" },
-	{ id: "changelog", label: "Changelog", icon: FileText, shortcut: "L" },
-	{ id: "context", label: "Context", icon: BookOpen, shortcut: "C" },
-	{ id: "agent-tools", label: "Agent Tools", icon: Wrench, shortcut: "M" },
-	{ id: "worktrees", label: "Worktrees", icon: GitBranch, shortcut: "W" },
+	{ id: "kanban", labelKey: "navigation:kanban", icon: LayoutGrid, shortcut: "K" },
+	{ id: "terminals", labelKey: "navigation:terminals", icon: Terminal, shortcut: "A" },
+	// ... and resolve with t(item.labelKey) at render time
 ];

Based on learnings: "Import translation helpers from react-i18next and use useTranslation() hook with appropriate namespaces."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/components/Sidebar.tsx` around lines 60 - 70, The nav
items in baseNavItems use hardcoded English labels; update to use translation
keys (e.g., label: "nav.kanban") and resolve them at render time using
react-i18next: import useTranslation and call t() where NavItem.label is
rendered (keep ids like "kanban" as stable keys), or alternatively store a
separate labelKey property on each NavItem and call t(labelKey) in the Sidebar
render function; ensure you load the appropriate namespace and update
tests/fixtures that reference the plaintext labels.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider consolidating TooltipProvider instances.

There are two separate TooltipProvider instances—one wrapping navigation items (line 116) and another wrapping footer actions (line 164). These could be consolidated into a single provider at a higher level for slightly cleaner code, though the current structure works correctly.

♻️ Optional consolidation

 	return (
+		<TooltipProvider>
 		<div
 			className={cn(
 				"flex h-full flex-col border-r bg-background transition-all duration-200",
 				isCollapsed ? "w-16" : "w-64",
 			)}
 		>
 			{/* Header */}
 			...
 			{/* Navigation */}
 			<ScrollArea className="flex-1 px-2 py-4">
-				<TooltipProvider>
 					<div className="space-y-1">
 						...
 					</div>
-				</TooltipProvider>
 			</ScrollArea>
 
 			<Separator />
 
 			{/* Footer Actions */}
 			<div className="p-2 space-y-1">
-				<TooltipProvider>
 					...
-				</TooltipProvider>
 			</div>
 		</div>
+		</TooltipProvider>
 	);

Also applies to: 164-164

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/components/Sidebar.tsx` around lines 116 - 117, There
are two separate TooltipProvider instances wrapping the navigation items (the
<TooltipProvider> around the div with className "space-y-1") and the footer
actions; consolidate them by moving a single TooltipProvider higher in the
Sidebar component so it wraps both the navigation container and the footer
actions container, remove the duplicate TooltipProvider instance, and ensure
TooltipProvider is still imported and used once around the elements that render
navigation items and footer actions.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Auth token won’t refresh after login/rotation while mounted.

authToken only re-computes when token prop changes, so updates in localStorage (login or refresh) won’t be picked up unless the component remounts. Consider resolving the token inside the WebSocket effect (or subscribing to storage events) so reconnects use the newest token.

♻️ Suggested adjustment (read token on connect)

-	const authToken = useMemo(() => {
-		if (token) return token;
-		try {
-			return localStorage.getItem("auth_token") || "";
-		} catch {
-			return "";
-		}
-	}, [token]);
+	const resolveAuthToken = () => {
+		if (token) return token;
+		try {
+			return localStorage.getItem("auth_token") || "";
+		} catch {
+			return "";
+		}
+	};

-	useEffect(() => {
-		if (!authToken) {
+	useEffect(() => {
+		const authToken = resolveAuthToken();
+		if (!authToken) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/components/Terminal.tsx` around lines 73 - 82, The
current useMemo for authToken won't update when localStorage changes while the
component is mounted; instead, read the token at the time you open or (re)create
the WebSocket connection so the socket uses the latest token. Locate the effect
or function that creates the WebSocket (the "WebSocket effect" /
connect/createSocket function) and replace the dependency on the memoized
authToken with a fresh read of localStorage.getItem("auth_token") inside that
effect (with try/catch), or add a window.addEventListener('storage') that
updates component state and triggers the reconnect; remove reliance on the
useMemo authToken for socket auth so reconnects use the newest token.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Auto-reconnect fires on intentional close/unmount, risking duplicate sockets and setState-after-unmount.

ws.onclose always schedules a reconnect. That means:

• manual reconnect (handleReconnect) closes the socket, then onclose schedules another reconnect (double reconnect), and
• cleanup on unmount can still schedule a reconnect and trigger state updates after unmount.

Add a guard for intentional closes and stale sockets before scheduling reconnects, and null the timeout ref when cleared.

🐛 Guard against intentional closes and stale sockets

+	const shouldReconnectRef = useRef(true);
...
 	useEffect(() => {
+		shouldReconnectRef.current = true;
...
 		const ws = new WebSocket(url);
 		wsRef.current = ws;
...
 		ws.onclose = () => {
+			if (!shouldReconnectRef.current || wsRef.current !== ws) return;
 			setConnectionStatus("disconnected");
 			if (pingIntervalRef.current) {
 				clearInterval(pingIntervalRef.current);
 				pingIntervalRef.current = null;
 			}
...
 		return () => {
+			shouldReconnectRef.current = false;
 			// Cleanup on unmount
 			if (pingIntervalRef.current) {
 				clearInterval(pingIntervalRef.current);
+				pingIntervalRef.current = null;
 			}
 			if (reconnectTimeoutRef.current) {
 				clearTimeout(reconnectTimeoutRef.current);
+				reconnectTimeoutRef.current = null;
 			}
 			ws.close();
 			wsRef.current = null;
 		};
 	}, [authToken, wsUrl, sessionId, cwd, reconnectCount]);
...
 	const handleReconnect = useCallback(() => {
 		// Force reconnect by closing current connection
+		shouldReconnectRef.current = false;
 		if (wsRef.current) {
 			wsRef.current.close();
 			wsRef.current = null;
 		}
 		setReconnectCount((c) => c + 1);
 	}, []);

Also applies to: 326-333

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/components/Terminal.tsx` around lines 224 - 253, Add a
guard to avoid auto-reconnect after intentional closes or when the closed socket
is stale: introduce a ref flag (e.g., intentionalCloseRef) that handleReconnect
and the cleanup/unmount set to true before calling ws.close(); in ws.onclose,
only schedule a reconnect if intentionalCloseRef.current is false AND the closed
ws equals wsRef.current (to avoid acting on stale sockets); when clearing
reconnectTimeoutRef via clearTimeout, also set reconnectTimeoutRef.current =
null; ensure pingIntervalRef clearing also nulls the ref to avoid stale timers.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Error overlay may cover the entire page.

The overlay is absolute, but the container isn’t positioned, so it will be relative to the nearest ancestor (often the page). Add relative to the root container to confine the overlay to the terminal.

🎯 Suggested fix

-			className={cn(
-				"flex flex-col h-full bg-[`#0d1117`] border rounded-lg overflow-hidden",
+			className={cn(
+				"relative flex flex-col h-full bg-[`#0d1117`] border rounded-lg overflow-hidden",
 				isActive && "ring-2 ring-primary",
 			)}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	return (
	<div
	className={cn(
	"flex flex-col h-full bg-[#0d1117] border rounded-lg overflow-hidden",
	isActive && "ring-2 ring-primary",
	)}
	onClick={handleClick}
	>
	{/* Terminal Header */}
	<div className="flex items-center justify-between bg-[#161b22] px-3 py-2 border-b border-gray-700">
	<div className="flex items-center gap-2">
	<XtermIcon className="h-4 w-4 text-gray-400" />
	<span className="text-xs text-gray-300">{title}</span>
	{getStatusIndicator()}
	<span className="text-xs text-gray-500 ml-2">
	{connectionStatus === "connected" && "Connected"}
	{connectionStatus === "connecting" && "Connecting..."}
	{connectionStatus === "disconnected" && "Disconnected"}
	{connectionStatus === "error" && "Connection Error"}
	</span>
	</div>
	<div className="flex items-center gap-2">
	{connectionStatus === "error" && (
	<Button
	variant="ghost"
	size="sm"
	className="h-6 px-2 text-xs text-yellow-400 hover:text-yellow-300"
	onClick={handleReconnect}
	>
	Reconnect
	</Button>
	)}
	{onClose && (
	<Button
	variant="ghost"
	size="icon"
	className="h-6 w-6 text-gray-400 hover:text-white"
	onClick={(e) => {
	e.stopPropagation();
	onClose();
	}}
	>
	<X className="h-4 w-4" />
	</Button>
	)}
	</div>
	</div>
	{/* Terminal Content */}
	<div
	ref={terminalRef}
	className="flex-1 overflow-hidden"
	style={{ minHeight: 0 }}
	/>
	{/* Error Overlay */}
	{error && (
	<div className="absolute inset-0 bg-black/80 flex items-center justify-center z-10">
	<div className="bg-red-900/90 border border-red-500 rounded-lg p-6 max-w-md">
	<h3 className="text-lg font-semibold text-red-200 mb-2">
	Connection Error
	</h3>
	<p className="text-sm text-red-100 mb-4">{error}</p>
	<Button
	variant="outline"
	size="sm"
	className="text-red-200 border-red-500 hover:bg-red-800"
	onClick={handleReconnect}
	>
	Try Reconnecting
	</Button>
	</div>
	</div>
	)}
	return (
	<div
	className={cn(
	"relative flex flex-col h-full bg-[`#0d1117`] border rounded-lg overflow-hidden",
	isActive && "ring-2 ring-primary",
	)}
	onClick={handleClick}
	>
	{/* Terminal Header */}
	<div className="flex items-center justify-between bg-[`#161b22`] px-3 py-2 border-b border-gray-700">
	<div className="flex items-center gap-2">
	<XtermIcon className="h-4 w-4 text-gray-400" />
	<span className="text-xs text-gray-300">{title}</span>
	{getStatusIndicator()}
	<span className="text-xs text-gray-500 ml-2">
	{connectionStatus === "connected" && "Connected"}
	{connectionStatus === "connecting" && "Connecting..."}
	{connectionStatus === "disconnected" && "Disconnected"}
	{connectionStatus === "error" && "Connection Error"}
	</span>
	</div>
	<div className="flex items-center gap-2">
	{connectionStatus === "error" && (
	<Button
	variant="ghost"
	size="sm"
	className="h-6 px-2 text-xs text-yellow-400 hover:text-yellow-300"
	onClick={handleReconnect}
	>
	Reconnect
	</Button>
	)}
	{onClose && (
	<Button
	variant="ghost"
	size="icon"
	className="h-6 w-6 text-gray-400 hover:text-white"
	onClick={(e) => {
	e.stopPropagation();
	onClose();
	}}
	>
	<X className="h-4 w-4" />
	</Button>
	)}
	</div>
	</div>
	{/* Terminal Content */}
	<div
	ref={terminalRef}
	className="flex-1 overflow-hidden"
	style={{ minHeight: 0 }}
	/>
	{/* Error Overlay */}
	{error && (
	<div className="absolute inset-0 bg-black/80 flex items-center justify-center z-10">
	<div className="bg-red-900/90 border border-red-500 rounded-lg p-6 max-w-md">
	<h3 className="text-lg font-semibold text-red-200 mb-2">
	Connection Error
	</h3>
	<p className="text-sm text-red-100 mb-4">{error}</p>
	<Button
	variant="outline"
	size="sm"
	className="text-red-200 border-red-500 hover:bg-red-800"
	onClick={handleReconnect}
	>
	Try Reconnecting
	</Button>
	</div>
	</div>
	)}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/components/Terminal.tsx` around lines 350 - 423, The
error overlay uses absolute positioning but the root container div (the
top-level element using cn(..., isActive && "ring-2 ring-primary")) is not
positioned, so the overlay can cover the whole page; update that root
container's className to include "relative" (i.e., add "relative" into the cn
call around the flex container) so the overlay div (the one with className
"absolute inset-0 bg-black/80...") is positioned relative to the Terminal
component and confined to its bounds.

In general, to fix log injection you must ensure that any user-controlled data is sanitized before being written to logs. For plain-text logs that means removing or encoding newline and carriage-return characters (and typically other Unicode line/paragraph separators) so an attacker cannot break the intended log-line structure or inject spoofed entries. It’s also good practice to make user-controlled segments clearly delimited.

Here, the best fix is to strengthen WebSocketClient.sanitize so it explicitly strips all line-breaking characters (\r, \n, \u2028, \u2029) in addition to any other control characters, and continue truncating to a maximum length. Then keep using this method in log, but log the sanitized arguments in a way that is clearly separated from the fixed prefix and primary message. We only need to modify WebSocketClient.sanitize in apps/web-frontend/src/api/websocket.ts; the rest of the code can stay as-is. No new imports or helper methods are required.

Concretely:

• Update sanitize (around lines 79–83) to use a more explicit regex that removes line terminators and other control characters, e.g. /[\r\n\u2028\u2029]/g combined with the existing \p{Cc} or a simpler explicit class.
• Keep the truncation to 200 characters to avoid excessively long log entries.
• Leave the log method’s use of sanitize and sanitizedArgs.join(" ") in place; once newlines are explicitly removed and the content is bounded, the joined string will not be able to create additional log lines.

In general, the fix is to ensure that any user-controlled data being logged is sanitized so that it cannot break log structure or inject misleading content. For plain-text or console logs, this typically means removing or normalizing newline and carriage-return characters, and clearly delimiting or quoting logged values. Since this code already centralizes logging via WebSocketClient.sanitize(), the best fix is to enhance that sanitizer to strip \r and \n explicitly (and potentially other problematic whitespace) in addition to control characters and truncation.

Concretely, we should modify the sanitize method in apps/web-frontend/src/api/websocket.ts (around lines 79–83) so that it replaces \r and \n with a safe representation (for example, a single space) or removes them entirely before logging. This will ensure that safeType (and all other sanitized values) cannot contain line breaks that would produce additional log lines. We should not change the behavior of logging itself (this.log or the console.error call), nor alter method signatures or external behavior; only the internal sanitization logic should be updated. No new imports are needed; we can use standard String.prototype.replace with a simple regex.

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web-backend/tests/test_integration.py`:
- Around line 712-756: The test_agent_type_phase_mapping currently uses a
tautological phase_map and only checks membership; replace it to assert against
the real mapping source or emitted events: import or reference the actual
mapping (e.g., AGENT_PHASE_MAP or the function that maps agent_type -> phase
used in api.routes.agents) and assert AGENT_PHASE_MAP[agent_type] ==
expected_phase (or trigger the agent run and inspect the broadcasted
event/response to verify the emitted phase matches expected_phase); update the
test to use the real symbol (AGENT_PHASE_MAP or the mapper function) instead of
the local phase_map variable so the test fails if production mapping changes.

In `@apps/web-frontend/src/api/websocket.ts`:
- Around line 185-189: The disconnect() method currently mutates
this.config.reconnect which permanently disables auto-reconnect; instead add a
one-shot flag (e.g. this.manualDisconnect or this.suppressNextReconnect) and set
that true inside disconnect(), leave this.config.reconnect unchanged, call
this.clearReconnectTimer() and this.stopPing() as before, and update the
reconnect logic (used by connect()/the reconnect timer/reconnect handler) to
check the one-shot flag: if set, consume/reset it and skip the immediate
reconnect; otherwise proceed with normal reconnect behavior. Apply the same
change to the other location that currently mutates this.config.reconnect so all
manual disconnect paths use the one-shot flag.
- Around line 129-134: The code concatenates this.config.url with
"/ws/agent-events" and can produce a double slash if config.url ends with a
slash; normalize the base URL once in the connect flow by trimming trailing
slashes from this.config.url (e.g., compute a local normalizedBase =
this.config.url with trailing slashes removed) and use normalizedBase when
calling this.log and when constructing the WebSocket (this.ws = new
WebSocket(...)); update any other local uses in the same method (e.g., the log
message and the ws assignment) to reference the normalized URL instead of raw
this.config.url.

---

Duplicate comments:
In `@apps/web-backend/tests/test_integration.py`:
- Around line 763-855: Tests mutate the global manager causing state leakage;
replace use of the shared manager in TestBroadcastHelperIntegration by injecting
a fresh ConnectionManager per test (or using a pytest fixture) and ensure the
module-level manager is temporarily swapped and restored for each test;
specifically, create a fixture that constructs a new ConnectionManager, assigns
it to the module/global name used by
broadcast_execution_event/broadcast_log_event/broadcast_error_event (the current
`manager`), yields to the test, and restores the original manager in teardown so
tests like test_broadcast_execution_event_helper,
test_broadcast_log_event_helper, and test_broadcast_error_event_helper operate
on isolated manager state and avoid leaking subscriptions across tests.
- Around line 342-446: Replace use of asyncio.run() in
test_task_lifecycle_simulation, test_task_cancellation_flow, and
test_cleanup_removes_completed_tasks by converting their nested run_test
coroutine into the test body as an async def and decorating each test with
`@pytest.mark.asyncio`; remove the outer asyncio.run(...) calls and the extra
wrapper functions (e.g., run_test) so the async logic runs directly under the
test, ensure pytest is imported and pytest-asyncio is available in test
dependencies, and keep existing helper coroutines (mock_agent_task,
long_running_task, quick_task, running_task) unchanged but inline them or keep
them nested as needed.
- Around line 863-925: The function test_integration_test_summary always passes
and pollutes test output; remove it from the test suite by either moving its
printed coverage map into module docs/README (copy the coverage dict and
messages out of test_integration_test_summary) or make the function non-test so
pytest won't collect it (rename test_integration_test_summary to
integration_test_summary_doc or similar), or explicitly skip it with a pytest
marker (apply `@pytest.mark.skip`(reason="documentation summary") above
test_integration_test_summary and import pytest if needed); pick one approach
and update references accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

test_agent_type_phase_mapping is tautological and doesn’t validate real mapping logic.
It only checks membership in a hardcoded list, so it can’t fail if the production mapping is wrong. Consider asserting against the actual mapping source or emitted events.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/tests/test_integration.py` around lines 712 - 756, The
test_agent_type_phase_mapping currently uses a tautological phase_map and only
checks membership; replace it to assert against the real mapping source or
emitted events: import or reference the actual mapping (e.g., AGENT_PHASE_MAP or
the function that maps agent_type -> phase used in api.routes.agents) and assert
AGENT_PHASE_MAP[agent_type] == expected_phase (or trigger the agent run and
inspect the broadcasted event/response to verify the emitted phase matches
expected_phase); update the test to use the real symbol (AGENT_PHASE_MAP or the
mapper function) instead of the local phase_map variable so the test fails if
production mapping changes.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Normalize config.url before path concatenation.
If config.url ends with /, the current interpolation can produce //ws/agent-events, which can break routing on some servers. Consider trimming trailing slashes once per connect.

🛠️ Suggested fix

-			this.log(`Connecting to ${this.config.url}/ws/agent-events`);
-
-			this.ws = new WebSocket(`${this.config.url}/ws/agent-events`);
+			const baseUrl = this.config.url.replace(/\/+$/, "");
+			this.log(`Connecting to ${baseUrl}/ws/agent-events`);
+
+			this.ws = new WebSocket(`${baseUrl}/ws/agent-events`);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	try {
	this.setState("connecting");
	this.log(`Connecting to ${this.config.url}/ws/agent-events`);
	this.ws = new WebSocket(`${this.config.url}/ws/agent-events`);
	try {
	this.setState("connecting");
	const baseUrl = this.config.url.replace(/\/+$/, "");
	this.log(`Connecting to ${baseUrl}/ws/agent-events`);
	this.ws = new WebSocket(`${baseUrl}/ws/agent-events`);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/api/websocket.ts` around lines 129 - 134, The code
concatenates this.config.url with "/ws/agent-events" and can produce a double
slash if config.url ends with a slash; normalize the base URL once in the
connect flow by trimming trailing slashes from this.config.url (e.g., compute a
local normalizedBase = this.config.url with trailing slashes removed) and use
normalizedBase when calling this.log and when constructing the WebSocket
(this.ws = new WebSocket(...)); update any other local uses in the same method
(e.g., the log message and the ws assignment) to reference the normalized URL
instead of raw this.config.url.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

disconnect() permanently disables auto‑reconnect.
Line 187 mutates the config, so any later connect() won’t auto‑reconnect after drops. This is a reliability regression for long‑lived singleton clients. Prefer a one‑shot “manual disconnect” flag that only suppresses the next reconnect cycle.

🛠️ Suggested fix

 export class WebSocketClient {
 	private config: Required<WebSocketConfig>;
 	private ws: WebSocket | null = null;
 	private state: ConnectionState = "disconnected";
 	private reconnectAttempts = 0;
 	private reconnectTimer: ReturnType<typeof setTimeout> | null = null;
 	private pingTimer: ReturnType<typeof setInterval> | null = null;
 	private subscriptions = new Set<string>();
+	private manualDisconnect = false;

@@
 	disconnect(): void {
 		this.log("Disconnecting");
-		this.config.reconnect = false; // Disable auto-reconnect
+		this.manualDisconnect = true;
 		this.clearReconnectTimer();
 		this.stopPing();

@@
 			this.ws.onclose = () => {
 				this.log("Connection closed");
 				this.setState("disconnected");
 				this.stopPing();

 				// Attempt reconnect if enabled
-				if (
-					this.config.reconnect &&
-					this.reconnectAttempts < this.config.maxReconnectAttempts
-				) {
+				const shouldReconnect =
+					this.config.reconnect && !this.manualDisconnect;
+				this.manualDisconnect = false;
+				if (shouldReconnect && this.reconnectAttempts < this.config.maxReconnectAttempts) {
 					this.scheduleReconnect();
 				}
 			};

Also applies to: 163-174

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-frontend/src/api/websocket.ts` around lines 185 - 189, The
disconnect() method currently mutates this.config.reconnect which permanently
disables auto-reconnect; instead add a one-shot flag (e.g. this.manualDisconnect
or this.suppressNextReconnect) and set that true inside disconnect(), leave
this.config.reconnect unchanged, call this.clearReconnectTimer() and
this.stopPing() as before, and update the reconnect logic (used by connect()/the
reconnect timer/reconnect handler) to check the one-shot flag: if set,
consume/reset it and skip the immediate reconnect; otherwise proceed with normal
reconnect behavior. Apply the same change to the other location that currently
mutates this.config.reconnect so all manual disconnect paths use the one-shot
flag.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web-backend/api/websocket.py`:
- Around line 523-535: The handler currently echoes user-controlled msg_type and
exception text e back to clients; change the two websocket.send_json responses
so they do not include msg_type or str(e). Specifically, in the "unknown message
type" branch remove f"Unknown message type: {msg_type}" and send a generic
{"type":"error","message":"Unknown message type"}; in the generic exception
handler keep the full error logged via logger.error(..., exc_info=True) but send
a sanitized {"type":"error","message":"Internal server error"} (or similar) via
websocket.send_json; update references around msg_type, e, logger.error and
websocket.send_json in this file accordingly.

In `@apps/web-backend/main.py`:
- Around line 49-52: The warning about a default SECRET_KEY is now unreachable
because SECRET_KEY is always populated with secrets.token_urlsafe(32); update
the check so we detect if the value is the generated default rather than
comparing to the old literal: capture the environment value first (e.g.,
env_secret = os.environ.get("SECRET_KEY")), generate the fallback (e.g.,
generated_secret = secrets.token_urlsafe(32)) and set SECRET_KEY = env_secret or
generated_secret, then change the logger.warning condition to warn when
env_secret is falsy (meaning we fell back to generated_secret) so the warning
triggers only when a real env var was not provided (reference SECRET_KEY,
env_secret, generated_secret, and the logger.warning call).

---

Duplicate comments:
In `@apps/web-backend/api/websocket.py`:
- Around line 455-466: The working_dir taken from user_claims is used directly
when creating a terminal session (terminal_manager.create_session), introducing
a path traversal risk; before calling create_session for session_id, resolve and
validate the requested working_dir: compute an absolute, normalized path (no
symlink/.. surprises), ensure it is inside an allowed base directory or one of a
whitelist (e.g., the user's home or project roots), verify the path exists and
is a directory, and fall back to a safe default (os.getcwd() or the user's home)
if validation fails; then pass the validated/normalized path into
terminal_manager.create_session.
- Around line 437-447: session_id from websocket.query_params is used without
validation; add explicit format and length checks before using it (e.g., ensure
it matches an allowed pattern like alphanumeric/[-_], and enforce a max length
such as 64 characters), treat the default "default" as allowed, and reject/close
the socket with status.WS_1008_POLICY_VIOLATION and a warning log if validation
fails; perform this check in the same block where token is verified (near
websocket.query_params.get("session_id", "default")) and sanitize the value
before it is used as a dictionary key or in any operations.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

User input and exception details echoed in error responses.

Unlike agent_events_websocket which uses sanitized generic messages, this endpoint echoes msg_type (user-controlled) and str(e) (internal details) back to clients. This is inconsistent with the security improvements made elsewhere in this PR.

🛡️ Proposed fix to sanitize error responses

                 else:
                     await websocket.send_json(
                         {
                             "type": "error",
-                            "message": f"Unknown message type: {msg_type}",
+                            "message": "Unknown message type",
                         }
                     )

             except json.JSONDecodeError:
                 await websocket.send_json({"type": "error", "message": "Invalid JSON"})
             except Exception as e:
                 logger.error(f"Error processing terminal message: {e}")
-                await websocket.send_json({"type": "error", "message": str(e)})
+                await websocket.send_json({"type": "error", "message": "Internal error processing message"})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/websocket.py` around lines 523 - 535, The handler
currently echoes user-controlled msg_type and exception text e back to clients;
change the two websocket.send_json responses so they do not include msg_type or
str(e). Specifically, in the "unknown message type" branch remove f"Unknown
message type: {msg_type}" and send a generic {"type":"error","message":"Unknown
message type"}; in the generic exception handler keep the full error logged via
logger.error(..., exc_info=True) but send a sanitized
{"type":"error","message":"Internal server error"} (or similar) via
websocket.send_json; update references around msg_type, e, logger.error and
websocket.send_json in this file accordingly.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Stale warning condition - will never trigger with new fallback.

The condition SECRET_KEY == "your-secret-key-here-change-in-production" references the old hardcoded fallback that was removed. Since SECRET_KEY now defaults to a random token via secrets.token_urlsafe(32), this branch is unreachable.

🔧 Proposed fix to update the warning logic

     # Validate required configuration
-    if not SECRET_KEY or SECRET_KEY == "your-secret-key-here-change-in-production":
+    if not os.getenv("SECRET_KEY"):
         logger.warning(
-            "⚠️  SECRET_KEY not configured! Using default - DO NOT use in production!"
+            "⚠️  SECRET_KEY not configured! Using auto-generated key - sessions will not persist across restarts."
         )

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if not SECRET_KEY or SECRET_KEY == "your-secret-key-here-change-in-production":
	logger.warning("⚠️ SECRET_KEY not configured! Using default - DO NOT use in production!")
	logger.warning(
	"⚠️ SECRET_KEY not configured! Using default - DO NOT use in production!"
	)
	if not os.getenv("SECRET_KEY"):
	logger.warning(
	"⚠️ SECRET_KEY not configured! Using auto-generated key - sessions will not persist across restarts."
	)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/main.py` around lines 49 - 52, The warning about a default
SECRET_KEY is now unreachable because SECRET_KEY is always populated with
secrets.token_urlsafe(32); update the check so we detect if the value is the
generated default rather than comparing to the old literal: capture the
environment value first (e.g., env_secret = os.environ.get("SECRET_KEY")),
generate the fallback (e.g., generated_secret = secrets.token_urlsafe(32)) and
set SECRET_KEY = env_secret or generated_secret, then change the logger.warning
condition to warn when env_secret is falsy (meaning we fell back to
generated_secret) so the warning triggers only when a real env var was not
provided (reference SECRET_KEY, env_secret, generated_secret, and the
logger.warning call).

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)

apps/web-backend/core/middleware.py (1)

162-165: 🧹 Nitpick | 🔵 Trivial

Consider sanitizing user_id in log output.

While user_id is expected to be an integer from a trusted source (request.state), logging it directly could be risky if the source is ever compromised. Consider using the shared sanitize_log utility for consistency:

+from api.routes.shared import sanitize_log
+
                 logger.warning(
-                    f"Rate limit exceeded for user {user_id}: "
+                    f"Rate limit exceeded for user {sanitize_log(str(user_id))}: "
                     f"{current_count}/{self.rate_limit_requests} ({self.rate_limit_period})"
                 )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/core/middleware.py` around lines 162 - 165, The
logger.warning call currently interpolates user_id directly; update the
middleware to sanitize user_id before logging by calling the shared sanitize_log
utility and use that sanitized value in the logger.warning message (identify the
logger.warning invocation that formats f"Rate limit exceeded for user
{user_id}..." and replace user_id with sanitize_log(user_id)) so logs
consistently use the sanitization helper; ensure sanitize_log is imported or
available where the middleware function/class is defined.

apps/web-backend/api/routes/tasks.py (1)

34-76: 🧹 Nitpick | 🔵 Trivial

Consider extracting common logic with specs.py.

The list_tasks and get_task_detail implementations are nearly identical to their counterparts in specs.py. While the current duplication is acceptable given that tasks/specs are "synonymous" (as documented), consider whether a shared helper could reduce maintenance burden if these endpoints diverge in the future.

Also applies to: 98-193

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/routes/tasks.py` around lines 34 - 76, The list_tasks
and get_task_detail endpoints duplicate logic from specs.py; extract the shared
behavior by adding a helper (e.g., in specs.py or a new utils module) that
converts a spec dict into the API models (e.g., spec_to_task_summary(spec) and
spec_to_task_detail(spec)) and reuse list_specs/get_spec plus these converters
inside the list_tasks and get_task_detail functions (referencing list_tasks,
get_task_detail, list_specs, get_spec, TaskSummary, TaskDetail) so both routes
call the shared converter instead of duplicating the mapping logic.

apps/web-backend/api/routes/agents.py (1)

127-144: ⚠️ Potential issue | 🟡 Minor

Inconsistent error detail exposure.

Specific exceptions (FileNotFoundError, RuntimeError, ValueError) still expose str(e) in the response detail, while the generic Exception uses a static message. This inconsistency could leak implementation details.

Consider whether to standardize on generic messages for all errors or intentionally expose details only for expected/safe exceptions:

🔒 Option: Use generic messages consistently

     except FileNotFoundError as e:
         logger.warning(f"Spec not found: {e}")
         raise HTTPException(
             status_code=status.HTTP_404_NOT_FOUND,
-            detail=str(e),
+            detail="Spec not found",
         )
     except RuntimeError as e:
         logger.warning(f"Task already running: {e}")
         raise HTTPException(
             status_code=status.HTTP_409_CONFLICT,
-            detail=str(e),
+            detail="Task already running for this spec",
         )
     except ValueError as e:
         logger.warning(f"Invalid request: {e}")
         raise HTTPException(
             status_code=status.HTTP_400_BAD_REQUEST,
-            detail=str(e),
+            detail="Invalid request parameters",
         )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/routes/agents.py` around lines 127 - 144, The three
exception handlers in the route (FileNotFoundError, RuntimeError, ValueError)
currently return detail=str(e) which can leak implementation details; update the
handlers in the same try/except block (the handlers that call
logger.warning(...)) to return consistent controlled messages instead of raw
exception text—either use a single generic detail like "Requested resource
error" or map each exception to a safe message (e.g., FileNotFoundError -> "Spec
not found", RuntimeError -> "Task already running", ValueError -> "Invalid
request") while keeping the original logger.warning(e) for diagnostics; ensure
HTTPException is raised with the sanitized detail and keep logging of the
original exception for debugging.

apps/web-backend/core/config.py (1)

96-103: ⚠️ Potential issue | 🟡 Minor

Module-level instantiation may cause import-time side effects.

settings = get_settings() at line 103 instantiates Settings at import time, which calls _validate(). If DEBUG=false and SECRET_KEY isn't set, importing this module will raise ValueError. This could cause unexpected failures during testing or when importing for introspection.

Consider lazy initialization or catching the error gracefully during module load.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/core/config.py` around lines 96 - 103, Module-level
instantiation of Settings via settings = get_settings() triggers _validate() at
import time and can raise ValueError; remove the import-time call and make
Settings lazy by either (A) removing the module-level settings assignment and
always call the existing get_settings() function where needed, or (B) keep a
module-level variable but initialize it lazily (e.g., settings = None and set it
on first access via get_settings()) or (C) catch ValueError around the
instantiation and set settings to None or a safe fallback while logging the
error; reference the get_settings() function, the Settings class, the settings
variable, and the _validate() path when implementing the lazy/guarded
initialization so importing the module no longer raises on missing
SECRET_KEY/DEBUG combinations.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/web-backend/api/routes/shared.py`:
- Around line 17-19: The sanitize_log function only escapes newline and carriage
return; update sanitize_log to also neutralize other control and terminal-escape
vectors by (1) stripping or replacing ANSI escape sequences (escape introducer
\x1b[...] patterns) and (2) replacing other non-printable/control characters
(characters with codepoints < 0x20 and DEL 0x7F) with safe visible placeholders
or their escaped hex form; locate the sanitize_log definition and implement
these two steps (ANSI/ESC removal via a regex targeting \x1b sequences, then a
pass to replace remaining control bytes) so logs cannot be injected via control
or escape codes.
- Around line 174-177: The status string is being mutated by appending " (has
build)" to spec_status, producing non-enum values and risking downstream
breakage; remove the code that appends to spec_status (the if has_build:
spec_status = f"{spec_status} (has build)") so spec_status remains a clean
enum-like value, and if a combined display is needed, build that string at the
presentation layer or add a separate field (e.g., has_build or status_display)
rather than altering spec_status; update any local references to use the
existing has_build boolean instead of the modified spec_status.

In `@apps/web-backend/main.py`:
- Line 11: Replace the direct import "from dotenv import load_dotenv" with a
defensive import: wrap the import in try/except ImportError, and in the except
define a no-op fallback for load_dotenv (e.g., a lambda or def that does
nothing) and optionally log or warn; update any references to load_dotenv to
keep behavior unchanged when python-dotenv is absent. Ensure you modify the
symbol load_dotenv only so other code in this module still calls it safely.

---

Outside diff comments:
In `@apps/web-backend/api/routes/agents.py`:
- Around line 127-144: The three exception handlers in the route
(FileNotFoundError, RuntimeError, ValueError) currently return detail=str(e)
which can leak implementation details; update the handlers in the same
try/except block (the handlers that call logger.warning(...)) to return
consistent controlled messages instead of raw exception text—either use a single
generic detail like "Requested resource error" or map each exception to a safe
message (e.g., FileNotFoundError -> "Spec not found", RuntimeError -> "Task
already running", ValueError -> "Invalid request") while keeping the original
logger.warning(e) for diagnostics; ensure HTTPException is raised with the
sanitized detail and keep logging of the original exception for debugging.

In `@apps/web-backend/api/routes/tasks.py`:
- Around line 34-76: The list_tasks and get_task_detail endpoints duplicate
logic from specs.py; extract the shared behavior by adding a helper (e.g., in
specs.py or a new utils module) that converts a spec dict into the API models
(e.g., spec_to_task_summary(spec) and spec_to_task_detail(spec)) and reuse
list_specs/get_spec plus these converters inside the list_tasks and
get_task_detail functions (referencing list_tasks, get_task_detail, list_specs,
get_spec, TaskSummary, TaskDetail) so both routes call the shared converter
instead of duplicating the mapping logic.

In `@apps/web-backend/core/config.py`:
- Around line 96-103: Module-level instantiation of Settings via settings =
get_settings() triggers _validate() at import time and can raise ValueError;
remove the import-time call and make Settings lazy by either (A) removing the
module-level settings assignment and always call the existing get_settings()
function where needed, or (B) keep a module-level variable but initialize it
lazily (e.g., settings = None and set it on first access via get_settings()) or
(C) catch ValueError around the instantiation and set settings to None or a safe
fallback while logging the error; reference the get_settings() function, the
Settings class, the settings variable, and the _validate() path when
implementing the lazy/guarded initialization so importing the module no longer
raises on missing SECRET_KEY/DEBUG combinations.

In `@apps/web-backend/core/middleware.py`:
- Around line 162-165: The logger.warning call currently interpolates user_id
directly; update the middleware to sanitize user_id before logging by calling
the shared sanitize_log utility and use that sanitized value in the
logger.warning message (identify the logger.warning invocation that formats
f"Rate limit exceeded for user {user_id}..." and replace user_id with
sanitize_log(user_id)) so logs consistently use the sanitization helper; ensure
sanitize_log is imported or available where the middleware function/class is
defined.

---

Duplicate comments:
In `@apps/web-backend/main.py`:
- Around line 77-78: Ensure the app fails fast when an insecure or unset
SECRET_KEY would be used for SessionMiddleware: at startup check the SECRET_KEY
(and an environment mode flag like ENV or DEBUG) before calling
app.add_middleware(SessionMiddleware, secret_key=SECRET_KEY); if ENV indicates
production (or DEBUG is false) and SECRET_KEY is missing or equals the
auto-generated/placeholder value, raise a RuntimeError (or exit) with a clear
message so the service never starts with an insecure secret.
- Around line 49-52: The current warning branch is unreachable because
SECRET_KEY is always populated with secrets.token_urlsafe(32); update the check
to warn only when the secret was not provided via environment rather than
comparing to the old literal. Concretely, change the condition around SECRET_KEY
and the logger.warning call to detect whether os.environ.get("SECRET_KEY") is
missing/empty (or a separate flag indicating fallback was used) instead of
comparing SECRET_KEY to "your-secret-key-here-change-in-production", keeping
references to SECRET_KEY and logger.warning so the log still emits the same
message when an env-provided secret is absent.

In `@apps/web-frontend/src/api/websocket.ts`:
- Around line 185-188: The disconnect() method currently mutates
this.config.reconnect which permanently disables auto-reconnect; instead
introduce a one-shot/manual disconnect flag (e.g. this.manualDisconnect or
this.userInitiatedDisconnect) and set that true in disconnect() while leaving
this.config.reconnect untouched, update the onclose/reconnect logic (the handler
around connect()/onclose at lines ~169-174) to check the manual flag before
scheduling reconnects, and ensure the manual flag is reset appropriately when
connect() is called manually so future automatic reconnect behavior remains
controlled by this.config.reconnect.
- Around line 131-134: Normalize config.url before building the websocket URL:
in the method that logs/creates the socket (where this.log and this.ws are
used), trim any trailing slashes from this.config.url once per connect (e.g.,
compute a local baseUrl = this.config.url with trailing slashes removed) and use
`${baseUrl}/ws/agent-events` for both the log and WebSocket constructor so you
never produce a double-slash path; update the references to use that baseUrl
(leave this.config.url unchanged).

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider expanding log sanitization to cover more injection vectors.

The current implementation only escapes \n and \r. Consider also escaping other control characters that could be used for log injection or terminal escape sequences (e.g., ANSI escape codes starting with \x1b).

♻️ Suggested enhancement

 def sanitize_log(value: str) -> str:
     """Sanitize value for safe logging (prevent log injection)."""
-    return str(value).replace("\n", "\\n").replace("\r", "\\r")
+    sanitized = str(value).replace("\n", "\\n").replace("\r", "\\r")
+    # Remove ANSI escape sequences
+    import re
+    return re.sub(r'\x1b\[[0-9;]*m', '', sanitized)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/routes/shared.py` around lines 17 - 19, The sanitize_log
function only escapes newline and carriage return; update sanitize_log to also
neutralize other control and terminal-escape vectors by (1) stripping or
replacing ANSI escape sequences (escape introducer \x1b[...] patterns) and (2)
replacing other non-printable/control characters (characters with codepoints <
0x20 and DEL 0x7F) with safe visible placeholders or their escaped hex form;
locate the sanitize_log definition and implement these two steps (ANSI/ESC
removal via a regex targeting \x1b sequences, then a pass to replace remaining
control bytes) so logs cannot be injected via control or escape codes.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Status string modification creates inconsistent data.

Appending " (has build)" to spec_status (e.g., "complete (has build)") creates a compound status that may break downstream consumers expecting enum-like values. The has_build boolean is already included separately.

♻️ Suggested fix to keep status clean

-        # Add build indicator
-        if has_build:
-            spec_status = f"{spec_status} (has build)"
-
         specs.append(
             {
                 "number": number,

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Add build indicator
	if has_build:
	spec_status = f"{spec_status} (has build)"
	specs.append(
	{
	"number": number,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/api/routes/shared.py` around lines 174 - 177, The status
string is being mutated by appending " (has build)" to spec_status, producing
non-enum values and risking downstream breakage; remove the code that appends to
spec_status (the if has_build: spec_status = f"{spec_status} (has build)") so
spec_status remains a clean enum-like value, and if a combined display is
needed, build that string at the presentation layer or add a separate field
(e.g., has_build or status_display) rather than altering spec_status; update any
local references to use the existing has_build boolean instead of the modified
spec_status.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Missing defensive import for dotenv.

Unlike apps/web-backend/core/config.py which wraps the dotenv import in a try/except block, this file will raise ImportError if python-dotenv is not installed.

🛡️ Suggested fix for consistency

-from dotenv import load_dotenv
+try:
+    from dotenv import load_dotenv
+    load_dotenv()
+except ImportError:
+    pass  # dotenv not installed, will use environment variables directly

-# Load environment variables
-load_dotenv()

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/web-backend/main.py` at line 11, Replace the direct import "from dotenv
import load_dotenv" with a defensive import: wrap the import in try/except
ImportError, and in the except define a no-op fallback for load_dotenv (e.g., a
lambda or def that does nothing) and optionally log or warn; update any
references to load_dotenv to keep behavior unchanged when python-dotenv is
absent. Ensure you modify the symbol load_dotenv only so other code in this
module still calls it safely.

In general, to fix a “statement has no effect” warning on an await you can either remove it (if it truly is unnecessary) or make use of its result or side effects in an explicit way. Here we must not remove the await read_task, because we want to ensure the background task finalizes correctly after cancellation. Instead, we can bind its (ignored) result to a throwaway variable and add a short comment, which both documents the intention and typically satisfies static analyzers.

Concretely, in apps/web-backend/api/websocket.py, in the finally block of terminal_websocket, change line 548 from await read_task to binding the awaited value, e.g. _ = await read_task. This does not change runtime behavior, but it makes the statement clearly “do something” (assign to a variable), which should clear the CodeQL “no effect” report while preserving the intended task‑cancellation semantics. No new imports or helper methods are needed.

In general, to fix a “statement has no effect” warning on an await expression, either use the awaited result (e.g. assign it to a variable or assert on it) or, if the result truly doesn’t matter, assign it to a throwaway variable (like _ = await ...) to make the intent explicit. This makes it clear that the statement is present for its side effects (here, ensuring completion) rather than by mistake.

For this specific case in apps/web-backend/tests/test_integration.py, the best minimal fix is to bind the result of await task to a dummy variable inside the contextlib.suppress block. That preserves the exact behaviour (including suppressing asyncio.CancelledError) while removing the “no effect” warning: it is now an assignment statement, which has a clear effect (updating the dummy variable). No imports or new helpers are required.

Concretely, in the test_task_cancellation_flow test, inside the inner run_test coroutine, change the line await task (around line 399) to _ = await task. No other lines or files need to be modified.

In general, to fix a “statement has no effect” issue for an await whose result is unused, you either remove the statement if it is genuinely unnecessary, or make its side-effectful purpose explicit (e.g., via assignment or assertion). Here, we must keep the await to ensure the cancelled task is fully cleaned up, so the best fix is to assign the awaited value to a throwaway variable and add a short comment explaining that we are draining the task completion.

Concretely, in apps/web-backend/tests/test_integration.py around line 438–441, within test_cleanup_removes_completed_tasks, change the bare await running inside the contextlib.suppress(asyncio.CancelledError) block to assign its result to _, and optionally note that it is to ensure cancellation propagation. This does not change test semantics: running_task has no return, so the assigned value will be None, and the awaited side effect (allowing cancellation to complete) is preserved. No new imports or method definitions are needed.

[sonarqubecloud]

Quality Gate passed

Issues
100 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
2.7% Duplication on New Code

See analysis details on SonarQube Cloud