Skip to content

Conversation

@nioasoft
Copy link

@nioasoft nioasoft commented Jan 24, 2026

Summary

This PR fixes critical bugs that cause 500 errors when creating features, and adds proper MCP server support to the Expand session.

Bug Fixes

SQLAlchemy PendingRollbackError (gkpj)

  • Add explicit session.rollback() in context managers before re-raising exceptions
  • Fixes 500 errors when database operations fail (constraint violations, schema mismatches)
  • Applied to: features.py, schedules.py, database.py

Database Migration for Legacy Columns

  • Add migration to make testing_in_progress column nullable
  • Fixes INSERT failures on databases created before the column was removed from the model
  • The column was removed in commit 486979c but existing DBs retained the NOT NULL constraint

Features

MCP Server Support for Expand Session

  • Add MCP server configuration to ExpandChatSession
  • Enables feature_create_bulk tool for creating features directly from the Expand chat
  • Previously, the expand-project.md skill instructed Claude to use an MCP tool that wasn't configured

Improved MCP Config for Assistant Session

  • Use JSON file path instead of dict for mcp_servers parameter
  • More reliable MCP server connection with Claude CLI

Test plan

  • Start server and open UI
  • Try to create a feature via POST /api/projects/{name}/features - should succeed
  • Open Expand chat and ask to create features - should use MCP tool successfully
  • Verify no 500 errors in server logs

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Configurable Playwright browser selection (firefox, chrome, webkit, msedge) and headless mode settings
    • Database health monitoring diagnostic endpoint
    • MCP tools integration for streamlined feature management
  • Bug Fixes

    • Enhanced database session error handling to prevent transaction state issues
  • Style

    • New Twitter-inspired UI theme with light and dark mode support
  • Documentation

    • Expanded environment configuration guide for Playwright options
    • Comprehensive Custom Updates documentation with setup examples

✏️ Tip: You can customize this high-level summary in your review settings.

nioasoft and others added 5 commits January 23, 2026 23:09
…tion

- Add WAL mode, busy timeout (30s), and retry logic for all SQLite connections
- Create get_robust_connection() and robust_db_connection() context manager
- Add execute_with_retry() with exponential backoff for transient errors
- Add check_database_health() function for integrity verification
- Update progress.py to use robust connections instead of raw sqlite3
- Add /api/projects/{name}/db-health endpoint for corruption diagnosis
- Add DatabaseHealth schema for health check responses

Fixes database corruption issues caused by concurrent access from multiple
processes (MCP server, FastAPI server, progress tracking).

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Merged upstream improvements:
- Optimized count_passing_tests to use single aggregate query
- Removed testing_in_progress columns (simplified architecture)
- Added composite index for status queries
- Per-project bash command allowlist system
- Orchestrator observability in Mission Control
- Various fixes and improvements

Preserved local improvements:
- Robust SQLite connection handling (WAL mode, busy timeout)
- Database health check endpoint
- Retry logic for transient errors
- Corruption detection and reporting

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create custom-theme.css for theme overrides that won't conflict
with upstream updates. The file loads after globals.css, so its
CSS variables take precedence.

This approach ensures:
- Zero merge conflicts on git pull (new file, not in upstream)
- Theme persists across upstream updates
- Easy to modify without touching upstream code

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
UI Changes:
- Replace neobrutalism with clean Twitter/Supabase-style design
- Remove all shadows, use thin borders (1px)
- Single accent color (Twitter blue) for all status indicators
- Rounded corners (1.3rem base)
- Fix dark mode contrast and visibility
- Make KanbanColumn themeable via CSS classes

Backend Changes:
- Default Playwright browser changed to Firefox (lower CPU)
- Default Playwright mode changed to headless (saves resources)
- Add PLAYWRIGHT_BROWSER env var support

Documentation:
- Add CUSTOM_UPDATES.md with all customizations documented
- Update .env.example with new Playwright options

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
## Bug Fixes

### SQLAlchemy PendingRollbackError (gkpj)
- Add explicit `session.rollback()` in context managers before re-raising exceptions
- Fixes 500 errors when database operations fail (constraint violations, etc.)
- Applied to: features.py, schedules.py, database.py

### Database Migration for Legacy Columns
- Add migration to make `testing_in_progress` column nullable
- Fixes INSERT failures on databases created before column removal
- The column was removed from the model but existing DBs had NOT NULL constraint

## Features

### MCP Server Support for Expand Session
- Add MCP server configuration to ExpandChatSession
- Enables `feature_create_bulk` tool for creating features directly
- Previously, Expand skill instructed Claude to use MCP tool that wasn't available

### Improved MCP Config for Assistant Session
- Use JSON file path instead of dict for mcp_servers parameter
- More reliable MCP server connection with Claude CLI

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Jan 24, 2026

📝 Walkthrough

Walkthrough

This PR introduces robust SQLite database utilities with automatic retry logic and health checks, makes Playwright configuration environment-driven with headless mode as default, integrates MCP-based feature creation tooling into chat sessions, adds a database health diagnostic endpoint, improves exception handling with rollback safeguards across routers, and implements a new Twitter-style UI theme.

Changes

Cohort / File(s) Summary
Database Robustness & Health Diagnostics
api/database.py, progress.py, server/routers/projects.py, server/schemas.py
Added robust SQLite connection utilities with configurable WAL, timeouts, and retry logic; health check endpoint and schema; migrated progress.py to use robust access layer with error differentiation.
Exception Handling & Rollback Safeguards
server/routers/features.py, server/routers/schedules.py
Explicit rollback on exceptions within DB session context managers to prevent PendingRollbackError and maintain proper transaction state.
Playwright Configuration
.env.example, client.py
Introduced configurable PLAYWRIGHT_BROWSER and PLAYWRIGHT_HEADLESS environment variables with documented defaults (firefox, headless=true); added helper function to read browser setting and updated client creation flow with browser/headless options and per-agent isolation.
MCP Feature Tooling Integration
server/services/assistant_chat_session.py, server/services/expand_chat_session.py
Switched MCP configuration from inline dict to file-based approach (.claude_mcp_config.json); registered feature-creation MCP tools; added system prompt guidance for tool-based feature creation; includes cleanup of temporary config files.
UI Theme & Styling
ui/src/components/KanbanColumn.tsx, ui/src/main.tsx, ui/src/styles/custom-theme.css
Migrated KanbanColumn from CSS variables to class-based color tokens; added comprehensive Twitter-style theme stylesheet with light/dark mode support, component styling, and accessibility-conscious focus/hover states.
Documentation
CUSTOM_UPDATES.md
New comprehensive update guide covering UI theme customization, Playwright configuration, SQLite connection handling, database health endpoint, and affected files with code examples and revert instructions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 The database now bounces back, resilient and spry,
With health checks and retries that never say die.
Playwright picks firefox with headless so bright,
MCP tools craft features with Config-file might,
While Twitter-style themes paint the UI just right! ✨

🚥 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 accurately summarizes the two main changes: fixing SQLAlchemy PendingRollbackError issues and adding MCP support for Expand/Assistant chat sessions.
Docstring Coverage ✅ Passed Docstring coverage is 96.15% 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 docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 Fix all issues with AI agents
In `@api/database.py`:
- Around line 429-492: The migration in _migrate_add_testing_columns should
defensively remove any leftover temporary table to avoid inconsistent state from
interrupted runs; before creating features_new, execute a DROP TABLE IF EXISTS
features_new to ensure the CREATE TABLE step always starts fresh and doesn't
rely solely on CREATE TABLE IF NOT EXISTS, then proceed with the existing
CREATE/INSERT/DROP/RENAME steps (keep references to features_new, features, and
the surrounding try/except/conn.commit/conn.rollback logic).

In `@CUSTOM_UPDATES.md`:
- Around line 255-262: The documented DatabaseHealth schema in CUSTOM_UPDATES.md
is out of sync with the actual DatabaseHealth model in server/schemas.py; open
the DatabaseHealth class in server/schemas.py, copy its exact field names,
types, and defaults (including Optional/nullability and default values) and
update the markdown snippet so it matches the implementation exactly (use the
same typing/annotations and default for details if present) and ensure the doc
reflects any renamed fields or added/removed attributes.
- Around line 209-216: The docs show an incorrect signature for
execute_with_retry; update the snippet so the function signature exactly matches
the implementation in api/database.py (use the same parameter names, types and
default values and ensure the docstring matches), and also confirm the
check_database_health docstring reflects the actual return shape used in
api/database.py; reference the execute_with_retry and check_database_health
symbols when making the corrections so the docs and code signatures are
consistent.

In `@ui/src/styles/custom-theme.css`:
- Around line 107-173: Global rules are removing all focus indicators, so add
explicit keyboard-only focus styles using :focus-visible for interactive
controls (e.g., .neo-btn, [class*="neo-btn"], button, .neo-input, .neo-textarea,
input, textarea, select) to restore a clear high-contrast ring (prefer
box-shadow or outline with accessible color and sufficient contrast) while
keeping mouse interactions unchanged. Implement :focus-visible selectors for the
button and input classes (e.g., .neo-btn:focus-visible,
.neo-input:focus-visible, etc.), ensure the styles are not overly subtle (use a
visible width/blur and accessible color), and avoid reintroducing them via broad
!important overrides that conflict with :hover; leave existing box-shadow:none
for non-keyboard focus if desired but make :focus-visible take precedence.
🧹 Nitpick comments (3)
progress.py (1)

75-107: Consider using execute_with_retry for read queries, too.
This keeps lock/busy behavior consistent with has_features.

♻️ Suggested refactor
-        with robust_db_connection(db_file) as conn:
-            cursor = conn.cursor()
-            # Single aggregate query instead of 3 separate COUNT queries
-            # Handle case where in_progress column doesn't exist yet (legacy DBs)
-            try:
-                cursor.execute("""
-                    SELECT
-                        COUNT(*) as total,
-                        SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing,
-                        SUM(CASE WHEN in_progress = 1 THEN 1 ELSE 0 END) as in_progress
-                    FROM features
-                """)
-                row = cursor.fetchone()
-                total = row[0] or 0
-                passing = row[1] or 0
-                in_progress = row[2] or 0
-            except sqlite3.OperationalError:
-                # Fallback for databases without in_progress column
-                cursor.execute("""
-                    SELECT
-                        COUNT(*) as total,
-                        SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing
-                    FROM features
-                """)
-                row = cursor.fetchone()
-                total = row[0] or 0
-                passing = row[1] or 0
-                in_progress = 0
+        # Single aggregate query instead of 3 separate COUNT queries
+        # Handle case where in_progress column doesn't exist yet (legacy DBs)
+        try:
+            row = execute_with_retry(
+                db_file,
+                """
+                SELECT
+                    COUNT(*) as total,
+                    SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing,
+                    SUM(CASE WHEN in_progress = 1 THEN 1 ELSE 0 END) as in_progress
+                FROM features
+                """,
+                fetch="one"
+            )
+            total = (row[0] or 0) if row else 0
+            passing = (row[1] or 0) if row else 0
+            in_progress = (row[2] or 0) if row else 0
+        except sqlite3.OperationalError:
+            # Fallback for databases without in_progress column
+            row = execute_with_retry(
+                db_file,
+                """
+                SELECT
+                    COUNT(*) as total,
+                    SUM(CASE WHEN passes = 1 THEN 1 ELSE 0 END) as passing
+                FROM features
+                """,
+                fetch="one"
+            )
+            total = (row[0] or 0) if row else 0
+            passing = (row[1] or 0) if row else 0
+            in_progress = 0
-        with robust_db_connection(db_file) as conn:
-            cursor = conn.cursor()
-            cursor.execute(
-                "SELECT id, category, name FROM features WHERE passes = 1 ORDER BY priority ASC"
-            )
-            features = [
-                {"id": row[0], "category": row[1], "name": row[2]}
-                for row in cursor.fetchall()
-            ]
-            return features
+        rows = execute_with_retry(
+            db_file,
+            "SELECT id, category, name FROM features WHERE passes = 1 ORDER BY priority ASC",
+            fetch="all"
+        )
+        return [{"id": row[0], "category": row[1], "name": row[2]} for row in rows]

Also applies to: 137-147

client.py (1)

247-270: Minor: get_playwright_headless() is called twice.

The function is called once on line 254 and again on line 256 for logging. Consider caching the result to avoid redundant calls.

Suggested improvement
         browser = get_playwright_browser()
+        headless = get_playwright_headless()
         playwright_args = [
             "@playwright/mcp@latest",
             "--viewport-size", "1280x720",
             "--browser", browser,
         ]
-        if get_playwright_headless():
+        if headless:
             playwright_args.append("--headless")
-        print(f"   - Browser: {browser} (headless={get_playwright_headless()})")
+        print(f"   - Browser: {browser} (headless={headless})")
api/database.py (1)

269-335: Missing commit for read operations may cause unintended behavior.

When fetch is "one" or "all", the function returns without calling commit(). While reads typically don't require commits, the connection was opened in autocommit=False mode (sqlite3 default). This could leave implicit transactions open. Consider adding conn.commit() for all paths or documenting the intentional behavior.

Also, the last_error variable could remain None if no retries occurred but an unexpected path led to line 335, though this seems unlikely given the current logic.

Suggested fix to ensure consistent transaction handling
                 if fetch == "one":
                     result = cursor.fetchone()
                 elif fetch == "all":
                     result = cursor.fetchall()
                 else:
-                    conn.commit()
                     result = cursor.rowcount
+                
+                conn.commit()

                 return result

Comment on lines 429 to +492
def _migrate_add_testing_columns(engine) -> None:
"""Legacy migration - no longer adds testing columns.
"""Legacy migration - handles testing columns that were removed from the model.
The testing_in_progress and last_tested_at columns were removed from the
Feature model as part of simplifying the testing agent architecture.
Multiple testing agents can now test the same feature concurrently
without coordination.
This function is kept for backwards compatibility but does nothing.
Existing databases with these columns will continue to work - the columns
are simply ignored.
This migration ensures these columns are nullable so INSERTs don't fail
on databases that still have them with NOT NULL constraints.
"""
pass
with engine.connect() as conn:
# Check if testing_in_progress column exists with NOT NULL
result = conn.execute(text("PRAGMA table_info(features)"))
columns = {row[1]: {"notnull": row[3], "dflt_value": row[4]} for row in result.fetchall()}

if "testing_in_progress" in columns and columns["testing_in_progress"]["notnull"]:
# SQLite doesn't support ALTER COLUMN, need to recreate table
# Instead, we'll use a workaround: create a new table, copy data, swap
logger.info("Migrating testing_in_progress column to nullable...")

try:
# Step 1: Create new table without NOT NULL on testing columns
conn.execute(text("""
CREATE TABLE IF NOT EXISTS features_new (
id INTEGER NOT NULL PRIMARY KEY,
priority INTEGER NOT NULL,
category VARCHAR(100) NOT NULL,
name VARCHAR(255) NOT NULL,
description TEXT NOT NULL,
steps JSON NOT NULL,
passes BOOLEAN NOT NULL DEFAULT 0,
in_progress BOOLEAN NOT NULL DEFAULT 0,
dependencies JSON,
testing_in_progress BOOLEAN DEFAULT 0,
last_tested_at DATETIME
)
"""))

# Step 2: Copy data
conn.execute(text("""
INSERT INTO features_new
SELECT id, priority, category, name, description, steps, passes, in_progress,
dependencies, testing_in_progress, last_tested_at
FROM features
"""))

# Step 3: Drop old table and rename
conn.execute(text("DROP TABLE features"))
conn.execute(text("ALTER TABLE features_new RENAME TO features"))

# Step 4: Recreate indexes
conn.execute(text("CREATE INDEX IF NOT EXISTS ix_features_id ON features (id)"))
conn.execute(text("CREATE INDEX IF NOT EXISTS ix_features_priority ON features (priority)"))
conn.execute(text("CREATE INDEX IF NOT EXISTS ix_features_passes ON features (passes)"))
conn.execute(text("CREATE INDEX IF NOT EXISTS ix_features_in_progress ON features (in_progress)"))
conn.execute(text("CREATE INDEX IF NOT EXISTS ix_feature_status ON features (passes, in_progress)"))

conn.commit()
logger.info("Successfully migrated testing columns to nullable")
except Exception as e:
logger.error(f"Failed to migrate testing columns: {e}")
conn.rollback()
raise
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check for foreign key references to the features table
rg "FOREIGN KEY.*features|ForeignKey.*['\"]features" --type py --type sql -i -n

Repository: leonvanzyl/autocoder

Length of output: 46


🏁 Script executed:

# Also check for any references to features table in raw SQL migrations or schema definitions
rg "REFERENCES\s+features\s*\(" --type py --type sql -i -n

Repository: leonvanzyl/autocoder

Length of output: 46


🏁 Script executed:

# Look for other table definitions that might reference features
rg "class.*\(.*Base\)" api/ -A 10 --type py | head -100

Repository: leonvanzyl/autocoder

Length of output: 1875


🏁 Script executed:

# Check the overall structure of database.py to understand the schema
head -100 api/database.py

Repository: leonvanzyl/autocoder

Length of output: 3416


🏁 Script executed:

# Find all model classes that might have relationships
rg "class \w+\(Base\)" api/database.py -A 15 | grep -E "(class|ForeignKey|relationship)"

Repository: leonvanzyl/autocoder

Length of output: 216


🏁 Script executed:

# Read the full migration function with context
sed -n '420,495p' api/database.py

Repository: leonvanzyl/autocoder

Length of output: 3842


🏁 Script executed:

# Check if there are any other references to "features" table in schema/migrations
rg "features" api/database.py | head -30

Repository: leonvanzyl/autocoder

Length of output: 1726


The foreign key concern is not applicable to this codebase.

The verification found no foreign key references to the features table. The only model using foreign keys is ScheduleOverride, which references schedules.id. The migration can safely drop and recreate the features table without risk of breaking constraint violations.

The suggestion to defensively drop any pre-existing features_new table before creation remains valid, though the current CREATE TABLE IF NOT EXISTS pattern prevents SQL errors. Adding DROP TABLE IF EXISTS features_new before Step 1 would improve robustness against data inconsistency from interrupted migrations:

Suggested defensive improvement
             try:
                 # Step 1: Create new table without NOT NULL on testing columns
+                conn.execute(text("DROP TABLE IF EXISTS features_new"))
                 conn.execute(text("""
                     CREATE TABLE IF NOT EXISTS features_new (
🤖 Prompt for AI Agents
In `@api/database.py` around lines 429 - 492, The migration in
_migrate_add_testing_columns should defensively remove any leftover temporary
table to avoid inconsistent state from interrupted runs; before creating
features_new, execute a DROP TABLE IF EXISTS features_new to ensure the CREATE
TABLE step always starts fresh and doesn't rely solely on CREATE TABLE IF NOT
EXISTS, then proceed with the existing CREATE/INSERT/DROP/RENAME steps (keep
references to features_new, features, and the surrounding
try/except/conn.commit/conn.rollback logic).

Comment on lines +209 to +216
def execute_with_retry(conn, sql, params=None, max_retries=3):
"""Execute SQL with exponential backoff retry for transient errors."""

def check_database_health(db_path: str) -> dict:
"""
Check database integrity and return health status.
Returns: {healthy: bool, message: str, details: dict}
"""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Fix execute_with_retry signature in docs.
Current snippet documents a different signature than api/database.py.

📄 Suggested doc fix
-def execute_with_retry(conn, sql, params=None, max_retries=3):
-    """Execute SQL with exponential backoff retry for transient errors."""
+def execute_with_retry(db_path, query, params=(), fetch="none", max_retries=SQLITE_MAX_RETRIES):
+    """Execute SQL with exponential backoff retry for transient errors."""
🤖 Prompt for AI Agents
In `@CUSTOM_UPDATES.md` around lines 209 - 216, The docs show an incorrect
signature for execute_with_retry; update the snippet so the function signature
exactly matches the implementation in api/database.py (use the same parameter
names, types and default values and ensure the docstring matches), and also
confirm the check_database_health docstring reflects the actual return shape
used in api/database.py; reference the execute_with_retry and
check_database_health symbols when making the corrections so the docs and code
signatures are consistent.

Comment on lines +255 to +262
**New schema added:**

```python
class DatabaseHealth(BaseModel):
healthy: bool
message: str
details: dict = {}
```
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Update DatabaseHealth schema doc to match implementation.
The documented fields don’t align with server/schemas.py.

📄 Suggested doc fix
-class DatabaseHealth(BaseModel):
-    healthy: bool
-    message: str
-    details: dict = {}
+class DatabaseHealth(BaseModel):
+    healthy: bool
+    journal_mode: str | None = None
+    integrity: str | None = None
+    error: str | None = None
📝 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
**New schema added:**
```python
class DatabaseHealth(BaseModel):
healthy: bool
message: str
details: dict = {}
```
**New schema added:**
🤖 Prompt for AI Agents
In `@CUSTOM_UPDATES.md` around lines 255 - 262, The documented DatabaseHealth
schema in CUSTOM_UPDATES.md is out of sync with the actual DatabaseHealth model
in server/schemas.py; open the DatabaseHealth class in server/schemas.py, copy
its exact field names, types, and defaults (including Optional/nullability and
default values) and update the markdown snippet so it matches the implementation
exactly (use the same typing/annotations and default for details if present) and
ensure the doc reflects any renamed fields or added/removed attributes.

Comment on lines +107 to +173
/* ===== BUTTONS ===== */
.neo-btn,
[class*="neo-btn"],
button {
border-width: 1px !important;
box-shadow: none !important;
text-transform: none !important;
font-weight: 500 !important;
transform: none !important;
border-radius: var(--radius-neo-lg) !important;
font-family: var(--font-neo-sans) !important;
}

.neo-btn:hover,
[class*="neo-btn"]:hover,
button:hover {
transform: none !important;
box-shadow: none !important;
}

.neo-btn:active,
[class*="neo-btn"]:active {
transform: none !important;
}

/* Primary button */
.neo-btn-primary {
background-color: var(--color-neo-accent) !important;
border-color: var(--color-neo-accent) !important;
color: white !important;
}

/* Success button - use accent blue instead of green */
.neo-btn-success {
background-color: var(--color-neo-accent) !important;
border-color: var(--color-neo-accent) !important;
color: white !important;
}

/* Danger button - subtle red */
.neo-btn-danger {
background-color: var(--color-neo-danger) !important;
border-color: var(--color-neo-danger) !important;
color: white !important;
}

/* ===== INPUTS ===== */
.neo-input,
.neo-textarea,
input,
textarea,
select {
border: 1px solid var(--color-neo-border) !important;
box-shadow: none !important;
border-radius: var(--radius-neo-md) !important;
background-color: var(--color-neo-neutral-50) !important;
}

.neo-input:focus,
.neo-textarea:focus,
input:focus,
textarea:focus,
select:focus {
box-shadow: none !important;
border-color: var(--color-neo-accent) !important;
outline: none !important;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add explicit focus-visible styles (keyboard accessibility).
Global box-shadow: none likely removes existing focus rings; buttons/links currently have no replacement focus indicator.

✅ Suggested focus-visible styles
 .neo-btn,
 [class*="neo-btn"],
 button {
   border-width: 1px !important;
   box-shadow: none !important;
   text-transform: none !important;
   font-weight: 500 !important;
   transform: none !important;
   border-radius: var(--radius-neo-lg) !important;
   font-family: var(--font-neo-sans) !important;
 }
 
+.neo-btn:focus-visible,
+button:focus-visible,
+a:focus-visible,
+[role="button"]:focus-visible {
+  outline: 2px solid var(--color-neo-accent) !important;
+  outline-offset: 2px !important;
+}
📝 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
/* ===== BUTTONS ===== */
.neo-btn,
[class*="neo-btn"],
button {
border-width: 1px !important;
box-shadow: none !important;
text-transform: none !important;
font-weight: 500 !important;
transform: none !important;
border-radius: var(--radius-neo-lg) !important;
font-family: var(--font-neo-sans) !important;
}
.neo-btn:hover,
[class*="neo-btn"]:hover,
button:hover {
transform: none !important;
box-shadow: none !important;
}
.neo-btn:active,
[class*="neo-btn"]:active {
transform: none !important;
}
/* Primary button */
.neo-btn-primary {
background-color: var(--color-neo-accent) !important;
border-color: var(--color-neo-accent) !important;
color: white !important;
}
/* Success button - use accent blue instead of green */
.neo-btn-success {
background-color: var(--color-neo-accent) !important;
border-color: var(--color-neo-accent) !important;
color: white !important;
}
/* Danger button - subtle red */
.neo-btn-danger {
background-color: var(--color-neo-danger) !important;
border-color: var(--color-neo-danger) !important;
color: white !important;
}
/* ===== INPUTS ===== */
.neo-input,
.neo-textarea,
input,
textarea,
select {
border: 1px solid var(--color-neo-border) !important;
box-shadow: none !important;
border-radius: var(--radius-neo-md) !important;
background-color: var(--color-neo-neutral-50) !important;
}
.neo-input:focus,
.neo-textarea:focus,
input:focus,
textarea:focus,
select:focus {
box-shadow: none !important;
border-color: var(--color-neo-accent) !important;
outline: none !important;
}
/* ===== BUTTONS ===== */
.neo-btn,
[class*="neo-btn"],
button {
border-width: 1px !important;
box-shadow: none !important;
text-transform: none !important;
font-weight: 500 !important;
transform: none !important;
border-radius: var(--radius-neo-lg) !important;
font-family: var(--font-neo-sans) !important;
}
.neo-btn:focus-visible,
button:focus-visible,
a:focus-visible,
[role="button"]:focus-visible {
outline: 2px solid var(--color-neo-accent) !important;
outline-offset: 2px !important;
}
.neo-btn:hover,
[class*="neo-btn"]:hover,
button:hover {
transform: none !important;
box-shadow: none !important;
}
.neo-btn:active,
[class*="neo-btn"]:active {
transform: none !important;
}
/* Primary button */
.neo-btn-primary {
background-color: var(--color-neo-accent) !important;
border-color: var(--color-neo-accent) !important;
color: white !important;
}
/* Success button - use accent blue instead of green */
.neo-btn-success {
background-color: var(--color-neo-accent) !important;
border-color: var(--color-neo-accent) !important;
color: white !important;
}
/* Danger button - subtle red */
.neo-btn-danger {
background-color: var(--color-neo-danger) !important;
border-color: var(--color-neo-danger) !important;
color: white !important;
}
/* ===== INPUTS ===== */
.neo-input,
.neo-textarea,
input,
textarea,
select {
border: 1px solid var(--color-neo-border) !important;
box-shadow: none !important;
border-radius: var(--radius-neo-md) !important;
background-color: var(--color-neo-neutral-50) !important;
}
.neo-input:focus,
.neo-textarea:focus,
input:focus,
textarea:focus,
select:focus {
box-shadow: none !important;
border-color: var(--color-neo-accent) !important;
outline: none !important;
}
🤖 Prompt for AI Agents
In `@ui/src/styles/custom-theme.css` around lines 107 - 173, Global rules are
removing all focus indicators, so add explicit keyboard-only focus styles using
:focus-visible for interactive controls (e.g., .neo-btn, [class*="neo-btn"],
button, .neo-input, .neo-textarea, input, textarea, select) to restore a clear
high-contrast ring (prefer box-shadow or outline with accessible color and
sufficient contrast) while keeping mouse interactions unchanged. Implement
:focus-visible selectors for the button and input classes (e.g.,
.neo-btn:focus-visible, .neo-input:focus-visible, etc.), ensure the styles are
not overly subtle (use a visible width/blur and accessible color), and avoid
reintroducing them via broad !important overrides that conflict with :hover;
leave existing box-shadow:none for non-keyboard focus if desired but make
:focus-visible take precedence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant