Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .env.staging.example
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@
# Anthropic API Key (required for Lead Agent functionality)
ANTHROPIC_API_KEY=your-anthropic-api-key-here

# Authentication secret (REQUIRED)
# The backend hard-fails on startup if this is unset while auth is enabled
# (auth is enabled by default) — see issue #643. Generate with:
# openssl rand -hex 32
AUTH_SECRET=your-secure-random-secret-here

# Database Path (staging database location)
DATABASE_PATH=/home/frankbria/projects/codeframe/staging/.codeframe/state.db

Expand Down
32 changes: 32 additions & 0 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ jobs:
REMOTE_PATH: ${{ secrets.PROJECT_PATH }}
ENV_ANTHROPIC_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
ENV_OPENAI_KEY: ${{ secrets.OPENAI_API_KEY }}
ENV_AUTH_SECRET: ${{ secrets.AUTH_SECRET }}
ENV_DATABASE_PATH: ${{ secrets.DATABASE_PATH }}
ENV_API_HOST: ${{ secrets.API_HOST }}
ENV_API_PORT: ${{ secrets.API_PORT }}
Expand All @@ -79,6 +80,16 @@ jobs:
run: |
echo "📝 Creating .env.staging file..."

# Fail fast if the AUTH_SECRET secret is missing/empty. The backend
# hard-fails on the default secret when auth is enabled (issue #643),
# which otherwise only surfaces as a health-check timeout much later.
if [ -z "${ENV_AUTH_SECRET}" ] || ! printf '%s' "${ENV_AUTH_SECRET}" | grep -q '[^[:space:]]'; then
echo "❌ AUTH_SECRET GitHub Actions secret is not set (or blank/whitespace-only)."
echo " Set it to a secure random value, e.g.:"
echo " gh secret set AUTH_SECRET --body \"\$(openssl rand -hex 32)\""
exit 1
fi

# Build env file content safely using printf (no shell interpretation)
ENV_CONTENT=$(printf '%s\n' \
"# CodeFRAME Environment Configuration" \
Expand All @@ -88,6 +99,11 @@ jobs:
"ANTHROPIC_API_KEY=${ENV_ANTHROPIC_KEY}" \
"OPENAI_API_KEY=${ENV_OPENAI_KEY}" \
"" \
"# Authentication" \
"# Required: the backend hard-fails on the default secret when auth" \
"# is enabled (the default) — see issue #643." \
"AUTH_SECRET=${ENV_AUTH_SECRET}" \
"" \
"# Database Configuration" \
"DATABASE_PATH=${ENV_DATABASE_PATH}" \
"" \
Expand Down Expand Up @@ -277,6 +293,7 @@ jobs:
REMOTE_PATH: ${{ secrets.PROJECT_PATH }}
ENV_ANTHROPIC_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
ENV_OPENAI_KEY: ${{ secrets.OPENAI_API_KEY }}
ENV_AUTH_SECRET: ${{ secrets.AUTH_SECRET }}
ENV_DATABASE_PATH: ${{ secrets.DATABASE_PATH }}
ENV_API_HOST: ${{ secrets.API_HOST }}
ENV_API_PORT: ${{ secrets.API_PORT }}
Expand All @@ -291,6 +308,16 @@ jobs:
run: |
echo "📝 Creating .env.production file..."

# Fail fast if the AUTH_SECRET secret is missing/empty. The backend
# hard-fails on the default secret when auth is enabled (issue #643),
# which otherwise only surfaces as a health-check timeout much later.
if [ -z "${ENV_AUTH_SECRET}" ] || ! printf '%s' "${ENV_AUTH_SECRET}" | grep -q '[^[:space:]]'; then
echo "❌ AUTH_SECRET GitHub Actions secret is not set (or blank/whitespace-only)."
echo " Set it to a secure random value, e.g.:"
echo " gh secret set AUTH_SECRET --body \"\$(openssl rand -hex 32)\""
exit 1
fi

# Build env file content safely using printf (no shell interpretation)
ENV_CONTENT=$(printf '%s\n' \
"# CodeFRAME Environment Configuration" \
Expand All @@ -300,6 +327,11 @@ jobs:
"ANTHROPIC_API_KEY=${ENV_ANTHROPIC_KEY}" \
"OPENAI_API_KEY=${ENV_OPENAI_KEY}" \
"" \
"# Authentication" \
"# Required: the backend hard-fails on the default secret when auth" \
"# is enabled (the default) — see issue #643." \
"AUTH_SECRET=${ENV_AUTH_SECRET}" \
"" \
"# Database Configuration" \
"DATABASE_PATH=${ENV_DATABASE_PATH}" \
"" \
Expand Down
6 changes: 4 additions & 2 deletions codeframe/adapters/e2b/budget.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
from datetime import datetime, timezone
from typing import Any

from codeframe.core.workspace import get_db_connection


def record_cloud_run(
workspace: Any,
Expand All @@ -32,7 +34,7 @@ def record_cloud_run(
scan_blocked: Number of files blocked by credential scanner.
"""
created_at = datetime.now(timezone.utc).isoformat()
conn = sqlite3.connect(workspace.db_path)
conn = get_db_connection(workspace)
try:
conn.execute(
"""
Expand Down Expand Up @@ -60,7 +62,7 @@ def get_cloud_run(workspace: Any, run_id: str) -> dict | None:
Returns:
Dict with cloud run fields, or None if not found.
"""
conn = sqlite3.connect(workspace.db_path)
conn = get_db_connection(workspace)
conn.row_factory = sqlite3.Row
try:
row = conn.execute(
Expand Down
47 changes: 41 additions & 6 deletions codeframe/core/workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,31 @@ def _get_state_dir(repo_path: Path) -> Path:
return repo_path / CODEFRAME_DIR


def _open_db(db_path: str | Path) -> sqlite3.Connection:
"""Open a workspace SQLite connection with concurrency safeguards.

Mirrors ``codeframe/platform_store/database.py``. The substantive change is
enabling **WAL journaling**: readers no longer block writers, which removes
the rollback-journal case where a writer hits ``database is locked``
immediately (the busy handler is skipped for that reader/writer deadlock).
WAL is a persistent, database-level setting, so applying it on every
connection is idempotent. ``busy_timeout`` is set to 5000ms to match
platform_store and make the value explicit (Python's ``sqlite3.connect``
already defaults to a 5s timeout). Matters under parallel batch execution
where multiple processes and background agent threads write the same DB.

The caller is responsible for closing the connection.
"""
# NOTE: pass ``db_path`` through unchanged (sqlite3.connect accepts both str
# and PathLike). Do NOT wrap in ``str()`` — that would coerce a non-path
# (e.g. a test's MagicMock) into a literal filename and silently create a
# junk DB file instead of raising, diverging from the prior connect call.
conn = sqlite3.connect(db_path)
conn.execute("PRAGMA journal_mode = WAL")
conn.execute("PRAGMA busy_timeout = 5000")
return conn


def _init_database(db_path: Path) -> None:
"""Initialize the workspace SQLite database with v2 schema.

Expand All @@ -70,7 +95,7 @@ def _init_database(db_path: Path) -> None:
- blockers: Human-in-the-loop blockers
- checkpoints: State snapshots
"""
conn = sqlite3.connect(db_path)
conn = _open_db(db_path)
cursor = conn.cursor()

# Workspace metadata
Expand Down Expand Up @@ -408,7 +433,7 @@ def _ensure_schema_upgrades(db_path: Path) -> None:
This function is idempotent and adds any new tables/columns
that were added after the initial schema creation.
"""
conn = sqlite3.connect(db_path)
conn = _open_db(db_path)
cursor = conn.cursor()

# Check if batch_runs table exists, if not create it
Expand Down Expand Up @@ -767,7 +792,7 @@ def create_or_load_workspace(repo_path: Path, tech_stack: Optional[str] = None)
workspace_id = str(uuid.uuid4())
now = _utc_now().isoformat()

conn = sqlite3.connect(db_path)
conn = _open_db(db_path)
cursor = conn.cursor()
cursor.execute(
"INSERT INTO workspace (id, repo_path, tech_stack, created_at, updated_at) VALUES (?, ?, ?, ?, ?)",
Expand Down Expand Up @@ -807,7 +832,7 @@ def get_workspace(repo_path: Path) -> Workspace:
# Ensure schema is up to date for existing workspaces
_ensure_schema_upgrades(db_path)

conn = sqlite3.connect(db_path)
conn = _open_db(db_path)
cursor = conn.cursor()
cursor.execute("SELECT id, repo_path, tech_stack, created_at FROM workspace LIMIT 1")
row = cursor.fetchone()
Expand Down Expand Up @@ -836,7 +861,17 @@ def get_db_connection(workspace: Workspace) -> sqlite3.Connection:
Returns:
SQLite connection
"""
return sqlite3.connect(workspace.db_path)
return _open_db(workspace.db_path)


def get_db_connection_by_path(db_path: str | Path) -> sqlite3.Connection:
"""Open a workspace DB connection from a raw path (WAL + busy_timeout).

Same connection setup as :func:`get_db_connection`, for callers that hold a
path rather than a :class:`Workspace` (e.g. the costs router's helpers that
tolerate fresh/locked DBs). The caller is responsible for closing it.
"""
return _open_db(db_path)


def workspace_exists(repo_path: Path) -> bool:
Expand Down Expand Up @@ -875,7 +910,7 @@ def update_workspace_tech_stack(repo_path: Path, tech_stack: Optional[str]) -> W

now = _utc_now().isoformat()

conn = sqlite3.connect(db_path)
conn = _open_db(db_path)
cursor = conn.cursor()
cursor.execute(
"UPDATE workspace SET tech_stack = ?, updated_at = ?",
Expand Down
6 changes: 3 additions & 3 deletions codeframe/ui/routers/costs_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from pydantic import BaseModel

from codeframe.core import tasks as tasks_module
from codeframe.core.workspace import Workspace
from codeframe.core.workspace import Workspace, get_db_connection_by_path
from codeframe.lib.rate_limiter import rate_limit_standard
from codeframe.platform_store.repositories.token_repository import TokenRepository
from codeframe.ui.dependencies import get_v2_workspace
Expand Down Expand Up @@ -79,7 +79,7 @@ def _query_costs(db_path: str, days: int) -> Dict:
Remove this workaround once the two schemas converge.
"""
try:
conn = sqlite3.connect(db_path)
conn = get_db_connection_by_path(db_path)
conn.row_factory = sqlite3.Row
except sqlite3.Error as e:
logger.warning("costs: failed to open %s: %s", db_path, e)
Expand Down Expand Up @@ -184,7 +184,7 @@ def _open_workspace_conn(db_path: str) -> Optional[sqlite3.Connection]:
fall back to an empty response rather than 500'ing the dashboard.
"""
try:
conn = sqlite3.connect(db_path)
conn = get_db_connection_by_path(db_path)
conn.row_factory = sqlite3.Row
return conn
except sqlite3.Error as e:
Expand Down
86 changes: 86 additions & 0 deletions tests/core/test_workspace.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
STATE_DB_NAME,
)

# Per CLAUDE.md, new v2 tests carry the marker explicitly (tests/core/ is also
# auto-marked v2 by conftest, but the convention makes the intent self-evident).
pytestmark = pytest.mark.v2


@pytest.fixture
def temp_repo(tmp_path: Path) -> Path:
Expand Down Expand Up @@ -175,3 +179,85 @@ def test_returns_correct_path(self, initialized_workspace: Workspace, temp_repo:

def test_path_exists(self, initialized_workspace: Workspace):
assert initialized_workspace.db_path.exists()


class TestConcurrentWriters:
"""Concurrency guards on workspace connections (issue #648)."""

def test_connection_sets_wal_and_busy_timeout(self, initialized_workspace: Workspace):
"""get_db_connection should enable WAL journaling and a busy timeout."""
conn = get_db_connection(initialized_workspace)
try:
journal_mode = conn.execute("PRAGMA journal_mode").fetchone()[0]
busy_timeout = conn.execute("PRAGMA busy_timeout").fetchone()[0]
finally:
conn.close()

assert journal_mode.lower() == "wal"
assert busy_timeout >= 5000

def test_writer_waits_for_held_lock_instead_of_failing(
self, initialized_workspace: Workspace
):
"""A second writer should *wait* for a briefly-held write lock and then
succeed — not fail immediately with ``database is locked``.

This is deterministic by design: one thread holds the write lock
(``BEGIN IMMEDIATE``) for well under the busy timeout, and the second
writer must block until the holder commits. A high-concurrency stress
test was avoided on purpose — SQLite's busy handler is not fair, so
N aggressively-contending writers can starve one past any fixed timeout,
which would make the test flaky without telling us anything new.
"""
import threading
import time

ws = initialized_workspace

# Dedicated probe table so the test is independent of the v2 schema.
setup = get_db_connection(ws)
try:
setup.execute(
"CREATE TABLE lock_probe (id INTEGER PRIMARY KEY, label TEXT)"
)
setup.commit()
finally:
setup.close()

lock_acquired = threading.Event()
hold_seconds = 0.5 # comfortably under the 5s busy timeout

def hold_write_lock() -> None:
conn = get_db_connection(ws)
conn.isolation_level = None # take manual control of the transaction
try:
conn.execute("BEGIN IMMEDIATE")
conn.execute("INSERT INTO lock_probe (id, label) VALUES (1, 'holder')")
lock_acquired.set()
time.sleep(hold_seconds)
conn.execute("COMMIT")
finally:
conn.close()

holder = threading.Thread(target=hold_write_lock)
holder.start()
assert lock_acquired.wait(timeout=5), "holder never acquired the write lock"

# The lock is held now. Without the busy timeout this write would raise
# ``database is locked`` immediately; with it, the write blocks until the
# holder commits (~hold_seconds) and then succeeds.
conn2 = get_db_connection(ws)
try:
conn2.execute("INSERT INTO lock_probe (id, label) VALUES (2, 'waiter')")
conn2.commit()
finally:
conn2.close()
holder.join(timeout=5)

# Both writes landed — the waiter was not dropped.
verify = get_db_connection(ws)
try:
count = verify.execute("SELECT COUNT(*) FROM lock_probe").fetchone()[0]
finally:
verify.close()
assert count == 2
Loading