fix: SQLAlchemy PendingRollbackError + MCP support for Expand/Assistant #92
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…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>
📝 WalkthroughWalkthroughAdds a robust SQLite access layer (WAL, busy timeout, retries, health checks), a DatabaseHealth API endpoint, migrates progress reads to the robust layer, ensures SQLAlchemy sessions rollback on exceptions, moves assistant MCP config to a file-based approach, and adds a custom Twitter-style UI theme. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Server as API_Server
participant ProjectsRouter as Projects_Router
participant DBUtils as api.database
participant SQLite as SQLite_File
Client->>API_Server: GET /api/projects/{name}/db-health
API_Server->>ProjectsRouter: route -> get_database_health(name)
ProjectsRouter->>DBUtils: resolve DB path (get_database_path) and call check_database_health(db_path)
DBUtils->>SQLite: open via get_robust_connection(db_path) (WAL, timeout)
DBUtils->>SQLite: PRAGMA integrity_check / PRAGMA journal_mode / basic queries
SQLite-->>DBUtils: integrity / journal_mode / errors
DBUtils-->>ProjectsRouter: health dict
ProjectsRouter-->>Client: 200 DatabaseHealth response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 withhas_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
fetchis"one"or"all", the function returns without callingcommit(). While reads typically don't require commits, the connection was opened in autocommit=False mode (sqlite3 default). This could leave implicit transactions open. Consider addingconn.commit()for all paths or documenting the intentional behavior.Also, the
last_errorvariable could remainNoneif 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 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check for foreign key references to the features table
rg "FOREIGN KEY.*features|ForeignKey.*['\"]features" --type py --type sql -i -nRepository: 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 -nRepository: 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 -100Repository: leonvanzyl/autocoder
Length of output: 1875
🏁 Script executed:
# Check the overall structure of database.py to understand the schema
head -100 api/database.pyRepository: 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.pyRepository: 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 -30Repository: 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} | ||
| """ |
There was a problem hiding this comment.
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 = {} | ||
| ``` |
There was a problem hiding this comment.
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; | ||
| } |
There was a problem hiding this comment.
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.
rudiheydra
added a commit
to rudiheydra/AutoBuildr
that referenced
this pull request
Jan 27, 2026
…imit logging format - Add test_feature_92_iteration_limit_logging.py with 17 comprehensive tests - Add verify_feature_92.py verification script - Tests verify log message includes: - Algorithm name (algorithm=BFS, compute_scheduling_scores) - Iteration count when limit was hit (iterations=N) - Total feature count (feature_count=N) - ERROR log level for visibility - Tests also verify defense-in-depth with both visited set and iteration limit - Marked feature AutoForgeAI#92 as passing Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
rudiheydra
added a commit
to rudiheydra/AutoBuildr
that referenced
this pull request
Jan 27, 2026
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
nioasoft
pushed a commit
to nioasoft/autocoder
that referenced
this pull request
Jan 29, 2026
…utoForgeAI#92 - Fix cycle detection in dependency_resolver.py: keep full path when pushing node back onto stack (fixes incorrect path truncation) - Add defensive DROP TABLE for interrupted migrations in migrations.py - Track and clean up MCP config files in assistant_chat_session.py Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
|
Please could you resolve these conflicts? |
Resolved conflicts in: - .env.example: Added EXTRA_READ_PATHS and Vertex AI config from upstream - CUSTOM_UPDATES.md: Merged SQLite docs (HEAD) with ui/main.tsx entry (upstream) - client.py: Accepted upstream's improved validation + get_extra_read_paths() - server/routers/projects.py: Kept db-health endpoint + added reset/settings endpoints - server/services/expand_chat_session.py: Accepted upstream's mcp_servers dict format - ui/src/components/KanbanColumn.tsx: Accepted upstream's shadcn/ui components - ui/src/main.tsx: Kept custom theme import Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Author
|
yes master
on it
…On Fri, Jan 30, 2026 at 11:34 AM Leon van Zyl ***@***.***> wrote:
*leonvanzyl* left a comment (AutoForgeAI/autoforge#92)
<#92 (comment)>
Please could you resolve these conflicts?
—
Reply to this email directly, view it on GitHub
<#92 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BS63L6PEYCLEVGOFTEOXVCD4JMQSNAVCNFSM6AAAAACSY53F4SVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTQMRSG42TONZWGU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
--
בברכה,
אסף בן עטיה
|
- Remove unused imports (wraps, Callable) from api/database.py - Fix import order in progress.py (alphabetize local imports) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/services/expand_chat_session.py (1)
189-244:⚠️ Potential issue | 🟡 MinorPass the MCP config file path instead of duplicating config as dict.
The code creates and writes
mcp_config_fileto disk but then ignores it, passing a separatemcp_serversdict toClaudeAgentOptionsinstead. Theassistant_chat_session.pyfile shows the intended pattern—it writes the config, then passes the file path as a string with the comment "Use file path for mcp_servers - more reliable than dict". Remove the unused file creation or use it:- mcp_servers=mcp_servers, + mcp_servers=str(mcp_config_file.resolve()),Remove the redundant
mcp_serversdict definition (lines 31–41) to avoid maintaining duplicate configuration.
🤖 Fix all issues with AI agents
In `@api/database.py`:
- Around line 19-21: Remove the unused imports to satisfy linting: delete wraps
from the functools import and remove Callable from the typing import in the
top-of-file import statement so only actually used symbols (e.g., Path, Any,
Optional) remain; update the import line that currently reads "from functools
import wraps" / "from typing import Any, Callable, Optional" to only import the
needed names (remove references to wraps and Callable).
In `@CUSTOM_UPDATES.md`:
- Around line 9-13: Update the table-of-contents anchors so each fragment
matches its corresponding numbered heading; for the entries referencing "UI
Theme Customization", "Playwright Browser Configuration", "SQLite Robust
Connection Handling", and "Update Checklist" include the section number prefix
in the anchor (e.g., change "#ui-theme-customization" to
"#1-ui-theme-customization" etc.) to ensure the TOC fragments exactly match the
headings and avoid MD051 warnings.
🧹 Nitpick comments (1)
server/services/expand_chat_session.py (1)
41-69: Consolidate MCP tool allow-lists to avoid drift.
Two identical lists (EXPAND_FEATURE_TOOLSandFEATURE_MCP_TOOLS) risk divergence.♻️ Suggested consolidation
-# Feature MCP tools needed for expand session -EXPAND_FEATURE_TOOLS = [ - "mcp__features__feature_create", - "mcp__features__feature_create_bulk", - "mcp__features__feature_get_stats", -] - -# Feature MCP tools for creating features -FEATURE_MCP_TOOLS = [ +FEATURE_MCP_TOOLS = [ "mcp__features__feature_create", "mcp__features__feature_create_bulk", "mcp__features__feature_get_stats", ]- allowed_tools=[ - "Read", - "Glob", - *EXPAND_FEATURE_TOOLS, - ], + allowed_tools=["Read", "Glob", *FEATURE_MCP_TOOLS],
- Fix TOC anchor in CUSTOM_UPDATES.md (#update-checklist -> AutoForgeAI#4-update-checklist) - Consolidate duplicate MCP tool lists in expand_chat_session.py - Remove unused mcp_config_file creation (dict is passed directly) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Collaborator
|
Hi @nioasoft, thanks for putting this together — there are some genuinely useful fixes in here. After reviewing the PR, I'm going to close it and implement the most valuable parts directly. The main reason is that the PR scope is significantly broader than the title suggests — the title describes the rollback fix and MCP support, but the PR also includes a robust SQLite connection layer, a database health endpoint, a migration rewrite, and a 411-line custom CSS theme. That makes it difficult to review and accept as a single unit. Specifically, the parts I'll be implementing:
The other items I'm skipping for now:
If you'd like to contribute any of the skipped items separately, feel free to open focused PRs for them — that makes review much easier. Thanks again for the contribution! |
leonvanzyl
added a commit
that referenced
this pull request
Feb 1, 2026
- Add explicit session.rollback() in exception handlers for database context managers in features.py, schedules.py, and database.py get_db() to prevent SQLAlchemy PendingRollbackError on failed transactions - Add EXPAND_FEATURE_TOOLS to expand session security settings allow list so the expand skill can use the MCP tools it references - Update assistant session prompt to direct the LLM to call MCP tools directly for feature creation instead of suggesting CLI commands Cherry-picked fixes from PR #92 (closed) with cleaner implementation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CoreAspectStu
pushed a commit
to CoreAspectStu/autocoder-custom
that referenced
this pull request
Feb 9, 2026
- Add explicit session.rollback() in exception handlers for database context managers in features.py, schedules.py, and database.py get_db() to prevent SQLAlchemy PendingRollbackError on failed transactions - Add EXPAND_FEATURE_TOOLS to expand session security settings allow list so the expand skill can use the MCP tools it references - Update assistant session prompt to direct the LLM to call MCP tools directly for feature creation instead of suggesting CLI commands Cherry-picked fixes from PR AutoForgeAI#92 (closed) with cleaner implementation. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
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)
session.rollback()in context managers before re-raising exceptionsfeatures.py,schedules.py,database.pyDatabase Migration for Legacy Columns
testing_in_progresscolumn nullable486979cbut existing DBs retained the NOT NULL constraintFeatures
MCP Server Support for Expand Session
ExpandChatSessionfeature_create_bulktool for creating features directly from the Expand chatImproved MCP Config for Assistant Session
mcp_serversparameterTest plan
/api/projects/{name}/features- should succeed🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.