diff --git a/backend/AGENTS.md b/backend/AGENTS.md index 0ad43cd76..a472bfc36 100644 --- a/backend/AGENTS.md +++ b/backend/AGENTS.md @@ -127,6 +127,22 @@ The offline test suite must not require network access, provider credentials, or the LongMemEval dataset. Small LongMemEval-shaped fixtures must be synthetic and generated by tests. +`scripts/benchmark/concurrency/` measures multi-process contention on the +`users` table (N separate OS processes, not asyncio tasks) for SQLite vs +Postgres -- the scenario `CONFIGURATION.md` requires Postgres for. `worker.py` +connects directly via SQLAlchemy (skipping the ~8.5s Alembic bootstrap the +orchestrator already ran once) and mirrors the app's per-connection SQLite +PRAGMAs; `run_concurrency_bench.py` seeds a disposable per-run Postgres schema, +synchronises workers on a READY/GO barrier before timing, and exits non-zero on +any crash, short op count, or `errors > 0`. Postgres runs need a throwaway +database via `--pg-url`; nothing here touches `public`. Run from `backend/`: + +```bash +uv run python scripts/benchmark/concurrency/run_concurrency_bench.py \ + --backend sqlite --workers 2,4,8,16 --ops-per-worker 50 --read-ratio 0.7 +uv run pytest tests/test_bench_concurrency.py tests/test_bench_worker.py -q +``` + ## Commands **Root directory** (for full application): diff --git a/backend/app/gateway/auth/repositories/sqlite.py b/backend/app/gateway/auth/repositories/sqlite.py index c9adeac8e..b722771c6 100644 --- a/backend/app/gateway/auth/repositories/sqlite.py +++ b/backend/app/gateway/auth/repositories/sqlite.py @@ -21,7 +21,96 @@ from sqlalchemy.ext.asyncio import AsyncSession, async_sessionmaker from app.gateway.auth.models import User from app.gateway.auth.repositories.base import UserNotFoundError, UserRepository -from deerflow.persistence.user.model import UserRow +from deerflow.persistence.user.model import OAUTH_IDENTITY_INDEX_NAME, UserRow + +# ``email`` is ``mapped_column(unique=True, index=True)``, which SQLAlchemy +# (and 0001_baseline) realise as a single UNIQUE INDEX -- not a named UNIQUE +# constraint -- so a Postgres duplicate reports the index name here. +_EMAIL_UNIQUE_INDEX_NAME = "ix_users_email" + + +def _driver_constraint_name(exc: IntegrityError) -> str | None: + """The violated constraint's name from the driver exception, or ``None`` + when the driver does not expose one. + + ``exc.orig`` is NOT the raw driver error. SQLAlchemy's asyncpg dialect + re-raises a plain ``AsyncAdapt_asyncpg_dbapi.IntegrityError`` built from a + rendered string and carrying only ``pgcode``/``sqlstate`` + (``sqlalchemy/dialects/postgresql/asyncpg.py::_handle_exception``); the + real ``asyncpg.UniqueViolationError`` — the one with ``constraint_name`` — + survives as ``exc.orig.__cause__`` (``raise translated_error from error``). + aiosqlite exposes no constraint name at all. Check the wrapper, then its + cause. + """ + for obj in (exc.orig, getattr(exc.orig, "__cause__", None)): + name = getattr(obj, "constraint_name", None) + if name: + return str(name) + return None + + +def _is_oauth_identity_violation(exc: IntegrityError) -> bool: + """Distinguish the ``idx_users_oauth_identity`` (oauth_provider, oauth_id) + unique-index violation from any OTHER ``IntegrityError`` reaching this + commit (a duplicate primary key, or a duplicate-email race that slipped + past the pre-check above). + + Never uses the SQLAlchemy wrapper's ``str(exc)``: it embeds the full + failed INSERT statement, whose column list names + ``oauth_provider``/``oauth_id`` on every call regardless of which + constraint fired, so a substring check on it misclassifies every + commit-time ``IntegrityError`` on this table as an OAuth conflict + (reproduced on SQLite: a duplicate ``id`` raised "OAuth account already + linked: None/None"). + + Postgres: match :data:`OAUTH_IDENTITY_INDEX_NAME` against the driver + constraint name (see :func:`_driver_constraint_name`). SQLite: no + structured name, only a message naming the columns (``"UNIQUE constraint + failed: users.oauth_provider, users.oauth_id"``) — require BOTH oauth + column names, not a bare "oauth" substring. + """ + name = _driver_constraint_name(exc) + if name is not None: + return name == OAUTH_IDENTITY_INDEX_NAME + message = str(exc.orig).lower() + return "oauth_provider" in message and "oauth_id" in message + + +def _is_email_violation(exc: IntegrityError) -> bool: + """True when ``exc`` is the ``ix_users_email`` uniqueness violation, using + the same driver-exception inspection as + :func:`_is_oauth_identity_violation` (never ``str(exc)``, whose INSERT text + names every column).""" + name = _driver_constraint_name(exc) + if name is not None: + return name == _EMAIL_UNIQUE_INDEX_NAME + return "users.email" in str(exc.orig).lower() + + +def _is_uniqueness_violation(exc: IntegrityError) -> bool: + """True for a unique-index / primary-key violation specifically, as + opposed to a NOT NULL / CHECK / foreign-key ``IntegrityError`` on the same + INSERT -- only the former means "a user like this already exists".""" + sqlstate = getattr(exc.orig, "sqlstate", None) or getattr(exc.orig, "pgcode", None) + if sqlstate is not None: + return sqlstate == "23505" # unique_violation + message = str(exc.orig).lower() + return "unique constraint failed" in message or "primary key constraint failed" in message + + +def _violated_constraint(exc: IntegrityError) -> str | None: + """Best-effort name of the constraint behind ``exc``, for a diagnostic that + does not pin an unattributed violation on a specific column. Uses the + driver constraint name where available, else the column(s) SQLite names in + its message (``"UNIQUE constraint failed: users.id"``).""" + name = _driver_constraint_name(exc) + if name: + return name + marker = "constraint failed: " + message = str(exc.orig) + if marker in message: + return message.split(marker, 1)[1].splitlines()[0].strip() or None + return None def _normalize_email(email: str) -> str: @@ -84,7 +173,11 @@ class SQLiteUserRepository(UserRepository): # ── CRUD ────────────────────────────────────────────────────────── async def create_user(self, user: User) -> User: - """Insert a new user. Raises ``ValueError`` on duplicate email. + """Insert a new user. Raises ``ValueError`` on any uniqueness + violation -- duplicate email, a duplicate (provider, oauth_id) pair + for an OAuth-linked account, or a duplicate id -- with a message + naming the specific conflict. Other ``IntegrityError``\\ s (NOT NULL, + CHECK, foreign key) propagate unchanged. The email is canonicalised to lowercase before insert so the existing unique constraint enforces case-insensitive uniqueness for new rows and @@ -103,7 +196,30 @@ class SQLiteUserRepository(UserRepository): await session.commit() except IntegrityError as exc: await session.rollback() - raise ValueError(f"Email already registered: {user.email}") from exc + # The email pre-check above already ruled out an email + # collision under normal (non-racing) conditions, so + # IntegrityErrors reaching here are usually + # idx_users_oauth_identity -- but not always (a duplicate + # primary key, or an email collision that raced past the + # pre-check). Attribute the failure to the constraint that + # actually fired instead of assuming any one of them. + if _is_oauth_identity_violation(exc): + raise ValueError(f"OAuth account already linked: {user.oauth_provider}/{user.oauth_id}") from exc + if _is_email_violation(exc): + # A duplicate address that got past the pre-check: a + # concurrent insert of the same email. + raise ValueError(f"Email already registered: {user.email}") from exc + if _is_uniqueness_violation(exc): + # Some other unique index / primary key (in practice a + # duplicate id). "Already exists" fits, but don't dress it + # up as an email conflict for an address that isn't + # registered. + constraint = _violated_constraint(exc) + raise ValueError(f"User already exists (constraint: {constraint})" if constraint else "User already exists") from exc + # A NOT NULL / CHECK / foreign-key IntegrityError is not a + # "user already exists" condition and not part of this + # method's ValueError contract -- let it propagate. + raise return user async def get_user_by_id(self, user_id: str) -> User | None: diff --git a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md index a5c1b13c0..9da25304d 100644 --- a/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md +++ b/backend/packages/harness/deerflow/agents/middlewares/AGENTS.md @@ -64,7 +64,7 @@ it to that middleware's declaration in the same change. `__authorization_outcome` key (so `build_run_config` strips caller-supplied forgeries). Consumers pop it; the publisher and the consumer share only that contract module. -10. **SandboxAuditMiddleware** - Audits sandboxed shell/file operations for security logging before tool execution. Command classification is **defense-in-depth and audit, not a security boundary** — the sandbox itself is the isolation boundary. Command substitution is judged by *position*, not by the presence of `$(`: a substitution in **command position** (`$(curl url)`, `` `curl url` ``, the word after a `|`/`&&`/`;`, or any `eval`/`source` argument) executes fetched or interpreted content and is blocked, while **value position** (`x=$(curl url)`, `echo $(curl url)`, an argument, a `for` word list) only captures output and passes (#4611). `_HIGH_RISK_COMMAND_POSITION_PATTERNS` is therefore matched anchored against each split sub-command, never against the whole compound string, and `_split_compound_command(split_pipes=True)` supplies those sub-commands; rules that span a pipe (`| sh`, `base64 -d | ...`) still rely on `_classify_command`'s whole-command Pass 1. `_COMMAND_POSITION_PREFIX` extends the anchor over leading variable assignments and exec wrappers (`FOO=1 $(curl url)`, `env`/`command`/`builtin`/`exec`/`nohup`/`time`/`sudo`/`doas`), which are still command position; its assignment branch requires whitespace before the substitution, which is exactly what keeps `x=$(curl url)` in value position. Two execution contexts are deliberately **position-blind** and matched against the whole command in Pass 1, because they execute what they receive wherever they appear (including as an argument to something else, e.g. `xargs sh -c "$(curl url)"`): an `eval`/`source` argument, and an interpreter's **code-string flag** — `-c` (shells, `python`), `-e` (`perl`/`ruby`/`node`), `-p` (`perl`/`node`), `-r` (`php`) — plus the here-string (`<<<`) that reaches the same place through stdin. All three substitution spellings (`$(cmd`, `<(cmd`, `` `cmd ``) share one `_RISKY_SUBSTITUTION` opener so a rule cannot cover one spelling and miss another. An unquoted newline splits like `;`, because it separates statements the same way: leaving it joined let `echo hi\n$(curl url)` evade the anchored rules that its `;` spelling triggers. A heredoc body is data rather than statements, so `_split_compound_command` records headers (`<