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
81 changes: 81 additions & 0 deletions apps/backend/alembic/versions/0047_scan_active_index_per_ref.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
# SPDX-License-Identifier: Apache-2.0
# Copyright 2026 TRUSCA contributors
"""allow one in-flight scan per branch instead of one per project

Revision ID: 0047
Revises: 0046
Create Date: 2026-08-04

Phase: branch as a first-class axis (follows 0046)
Kind: schema (replaces one partial unique index; no data migration)
Forward-only: yes

What:
``ix_scans_project_active`` is rebuilt on ``(project_id, ref)`` with
``NULLS NOT DISTINCT``, keeping its ``status IN ('queued','running')``
predicate.

Why:
The index enforced at most one in-flight scan per PROJECT. Once branches
became a first-class axis — the current-state anchor prefers the main line,
the reads filter by ref — that limit stopped matching the model: pushing to
``main`` and ``release/1.x`` at the same time made one CI job wait for the
other's scan, or fail with a 409 it could do nothing about. The branches
write to disjoint snapshots, so nothing about the data required them to be
serialized.

Why NULLS NOT DISTINCT:
``ref`` is NULL for ad-hoc scans, which is the majority of rows. Under the
default NULLS DISTINCT a plain ``(project_id, ref)`` unique index would stop
constraining them at all — every manual re-trigger would queue another scan,
turning a stability guard into nothing for the common case. NULLS NOT
DISTINCT (PostgreSQL 15+; we pin 17) makes all ref-less rows of a project
collide with each other, so ad-hoc scans keep exactly today's behaviour and
only *named branches* gain concurrency.

Blast radius:
A project can now hold as many in-flight scans as it has distinct refs. The
bound that matters is the per-team cap (``_enforce_team_concurrency_cap``)
and the per-user trigger rate limit, both unchanged; the disk guard still
refuses new work when the workspace is full.

Locking / build strategy:
DROP then CREATE inside the migration transaction. The guard is absent for
that window, which is safe because ``scripts/upgrade.sh`` stops the app
containers before migrating — nothing can trigger a scan meanwhile. Doing it
the other way (create first, drop after) is impossible: the two indexes would
both have to hold, and the old one is exactly what we are relaxing.

Migration policy (CLAUDE.md §6):
- No column changes, no backfill, no data migration.
- Forward-only: ``downgrade()`` raises ``NotImplementedError``.
"""

from __future__ import annotations

from collections.abc import Sequence

from alembic import op

revision: str = "0047"
down_revision: str | None = "0046"
branch_labels: str | Sequence[str] | None = None
depends_on: str | Sequence[str] | None = None


def upgrade() -> None:
op.execute("DROP INDEX IF EXISTS ix_scans_project_active")
# Mirrors the Index(...) declaration in models.scan.Scan.__table_args__ —
# keep the two in step or ``alembic check`` reports the schema as drifted.
op.execute(
"""
CREATE UNIQUE INDEX ix_scans_project_active
ON scans (project_id, ref)
NULLS NOT DISTINCT
WHERE status IN ('queued','running')
"""
)


def downgrade() -> None:
raise NotImplementedError("forward-only migration (CLAUDE.md §6)")
13 changes: 10 additions & 3 deletions apps/backend/models/scan.py
Original file line number Diff line number Diff line change
Expand Up @@ -381,13 +381,20 @@ class Scan(Base):
Index("ix_scans_celery_task_id", "celery_task_id"),
# JSONB GIN — supports `metadata @> '{...}'` (e.g. "find scans of branch X").
Index("ix_scans_metadata_gin", "metadata", postgresql_using="gin"),
# Concurrency gate: at most one scan per project may be queued or
# running at any time. Mirrored in the migration via op.execute()
# because partial unique indexes need explicit DDL.
# Concurrency gate: at most one in-flight scan per (project, branch).
# Two branches write disjoint snapshots, so serializing them only made
# one CI job wait on another's push. NULLS NOT DISTINCT is what keeps
# this a guard rather than a formality: ``ref`` is NULL for ad-hoc
# scans (most rows), and under the default NULLS DISTINCT those would
# stop colliding entirely and a project could queue unbounded manual
# re-triggers. Migration 0047 mirrors this via op.execute() — partial
# unique indexes need explicit DDL.
Index(
"ix_scans_project_active",
"project_id",
"ref",
unique=True,
postgresql_nulls_not_distinct=True,
postgresql_where=text("status IN ('queued','running')"),
),
# W6-#42 — rematch beat's "due succeeded scans" hot path. Partial on
Expand Down
10 changes: 10 additions & 0 deletions apps/backend/schemas/project_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,16 @@ class ScanSummary(BaseModel):
"renders). ``null`` when the scan was run without a release label."
),
)
ref: str | None = Field(
default=None,
description=(
"Normalized git ref this scan targeted (``main``, ``pr-12``), or "
"``null`` for an ad-hoc trigger that carried none. The concurrency "
"gate is per-(project, ref), so a client deciding whether its own "
"trigger would conflict has to compare against this, not merely "
"against 'some scan is active'."
),
)

@model_validator(mode="before")
@classmethod
Expand Down
40 changes: 28 additions & 12 deletions apps/backend/services/scan_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@
inside `trigger_scan` flags the exact insertion point.

Concurrency contract (CLAUDE.md core rule #3 + models/scan.py partial unique
index `ix_scans_project_active`): at most one scan per project may be in
state queued|running. The DB rejects a second INSERT with IntegrityError; we
translate that to `ScanInProgressConflict` (409) so callers get a stable RFC
7807 envelope instead of a Python traceback.
index `ix_scans_project_active`): at most one scan per (project, branch) may
be in state queued|running, where all ref-less ad-hoc scans of a project count
as one branch. Two branches write disjoint snapshots, so they run in parallel;
re-triggering the same branch still conflicts. The DB rejects the second
INSERT with IntegrityError; we translate that to `ScanInProgressConflict` (409)
so callers get a stable RFC 7807 envelope instead of a Python traceback.
"""

from __future__ import annotations
Expand Down Expand Up @@ -108,6 +110,19 @@ class ScanInProgressConflict(ScanError):
title = "Scan Already In Progress"


def _in_progress_detail(project_id: object, ref: str | None) -> str:
"""Name the branch that is busy, not just the project.

Since the concurrency gate became per-(project, ref), "a scan is already
running for this project" would send the caller looking for a conflict that
may be on a branch they are not touching. The ref-less case keeps the old
wording because there is no branch to name.
"""
if ref:
return f"a scan is already queued or running for {ref} in project {project_id}"
return f"a scan is already queued or running for project {project_id}"


class ScanArchivedConflict(ScanError):
"""409 — the project is archived; archiving disables new scans (H-7).

Expand Down Expand Up @@ -336,8 +351,8 @@ async def _enforce_team_concurrency_cap(
"""Raise :class:`ConcurrentScanLimitExceeded` if the team is at the cap.

A cap of 0 (or negative) disables the check entirely — the operator has
opted out and only the per-project unique index + per-user rate limit
apply.
opted out and only the per-(project, branch) unique index + per-user rate
limit apply.

Note (race window — soft cap): this SELECT-then-INSERT is not atomic
across concurrent triggers from the same team. N requests can each read
Expand All @@ -347,9 +362,10 @@ async def _enforce_team_concurrency_cap(
M2 (security review): worst-case bound. The overshoot is bounded, not
unbounded, by two independent controls:

* the per-project unique partial index (``ix_scans_project_active``)
guarantees at most ONE active scan per project, so a single project
can never contribute more than 1 to the overshoot; and
* the unique partial index (``ix_scans_project_active``) guarantees at
most ONE active scan per (project, branch), so a project contributes at
most one per branch it is being pushed to rather than an unbounded
number of re-triggers; and
* the per-user scan-trigger rate limit (``SCAN_TRIGGER_RATE_LIMIT``,
default 20/min) bounds how many triggers any one member can fire in
the race window.
Expand Down Expand Up @@ -708,15 +724,15 @@ async def trigger_scan(
try:
await session.flush()
except IntegrityError as exc:
# The partial unique index on (project_id) WHERE status IN
# The partial unique index on (project_id, ref) WHERE status IN
# ('queued','running') is the canonical signal. Postgres returns the
# constraint name in the orig message; we don't switch on it because
# the only realistic constraint that fires from this INSERT is the
# active-scan one — projects are validated above and the FK target
# exists.
await session.rollback()
raise ScanInProgressConflict(
f"a scan is already queued or running for project {project_id_value}",
_in_progress_detail(project_id_value, scan.ref),
) from exc

# I-2: keep the project.latest_scan_id pointer in sync so list pages
Expand All @@ -735,7 +751,7 @@ async def trigger_scan(
# possible if the txn was held briefly). Translate identically.
await session.rollback()
raise ScanInProgressConflict(
f"a scan is already queued or running for project {project_id_value}",
_in_progress_detail(project_id_value, scan.ref),
) from exc

await session.refresh(scan)
Expand Down
86 changes: 86 additions & 0 deletions apps/backend/tests/integration/test_scans_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,92 @@ async def test_concurrent_trigger_returns_409_problem(client) -> None:
assert body.get("scan_already_in_progress") is True


async def test_two_branches_scan_concurrently(client) -> None:
"""The gate is per-(project, branch): disjoint branches must not serialize."""
team, user, project = await _seed(client, role="developer")
headers = _bearer_for(user)

first = await client.post(
f"/v1/projects/{project.id}/scans",
headers=headers,
json={"kind": "source", "metadata": {"ref": "refs/heads/main"}},
)
assert first.status_code == 202, first.text

second = await client.post(
f"/v1/projects/{project.id}/scans",
headers=headers,
json={"kind": "source", "metadata": {"ref": "refs/heads/release/1.x"}},
)
assert second.status_code == 202, second.text
assert first.json()["ref"] == "main"
assert second.json()["ref"] == "release/1.x"


async def test_same_branch_retrigger_still_conflicts(client) -> None:
# Relaxing the gate must not make it a formality — the same branch twice is
# still duplicate work on the same snapshot.
team, user, project = await _seed(client, role="developer")
headers = _bearer_for(user)

first = await client.post(
f"/v1/projects/{project.id}/scans",
headers=headers,
json={"kind": "source", "metadata": {"ref": "refs/heads/main"}},
)
assert first.status_code == 202, first.text

# A bare branch name must collide with the fully-qualified form the first
# trigger used — both normalize to `main`.
second = await client.post(
f"/v1/projects/{project.id}/scans",
headers=headers,
json={"kind": "source", "metadata": {"ref": "main"}},
)
assert second.status_code == 409, second.text
body = second.json()
assert body.get("scan_already_in_progress") is True
# The detail names the busy branch so the caller does not go looking for a
# conflict on a branch they never touched.
assert "main" in body["detail"]


async def test_adhoc_scans_still_serialize_with_each_other(client) -> None:
# ref is NULL for ad-hoc triggers. Under the default NULLS DISTINCT the
# relaxed index would stop constraining them at all, so this pins the
# NULLS NOT DISTINCT half of the migration.
team, user, project = await _seed(client, role="developer")
headers = _bearer_for(user)

first = await client.post(
f"/v1/projects/{project.id}/scans", headers=headers, json={"kind": "source"}
)
assert first.status_code == 202, first.text

second = await client.post(
f"/v1/projects/{project.id}/scans", headers=headers, json={"kind": "source"}
)
assert second.status_code == 409, second.text
assert second.json().get("scan_already_in_progress") is True


async def test_adhoc_scan_does_not_block_a_branch_scan(client) -> None:
team, user, project = await _seed(client, role="developer")
headers = _bearer_for(user)

adhoc = await client.post(
f"/v1/projects/{project.id}/scans", headers=headers, json={"kind": "source"}
)
assert adhoc.status_code == 202, adhoc.text

branch = await client.post(
f"/v1/projects/{project.id}/scans",
headers=headers,
json={"kind": "source", "metadata": {"ref": "refs/heads/main"}},
)
assert branch.status_code == 202, branch.text


async def test_trigger_on_archived_project_returns_409(client) -> None:
"""H-7: an archived project must reject new scans, not silently accept them."""
from datetime import UTC, datetime
Expand Down
19 changes: 16 additions & 3 deletions apps/frontend/src/features/projects/ProjectDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,15 @@ export function ProjectDetailPage() {
const activeScan = (overview.data?.recent_scans ?? []).find(
(scan) => scan.status === "queued" || scan.status === "running",
);
// The concurrency gate is per-(project, branch), and the Scan button always
// triggers an ad-hoc (ref-less) run — so only another ad-hoc scan can block
// it. A CI scan on `main` leaves the button usable; disabling on "any active
// scan" would hide the concurrency the gate now allows.
const blockingScan = (overview.data?.recent_scans ?? []).find(
(scan) =>
(scan.status === "queued" || scan.status === "running") &&
scan.ref == null,
);

if (!projectId) {
return (
Expand Down Expand Up @@ -493,6 +502,7 @@ export function ProjectDetailPage() {
demoReadOnly={writesDisabled}
onScan={() => setSourceDialogOpen(true)}
activeScan={activeScan ?? null}
blockingScan={blockingScan ?? null}
onReopenActiveScan={
activeScan ? () => handleReopenScan(activeScan) : undefined
}
Expand Down Expand Up @@ -768,6 +778,8 @@ interface ProjectDetailHeaderProps {
* progress drawer never strands the user.
*/
activeScan: ScanSummary | null;
/** The active ad-hoc scan that would actually conflict with the button. */
blockingScan: ScanSummary | null;
/** Re-open the live progress drawer for {@link activeScan}. */
onReopenActiveScan?: () => void;
/** Currently pinned scan id (`?scan=`), or undefined for the live view. */
Expand All @@ -792,6 +804,7 @@ function ProjectDetailHeader({
demoReadOnly,
onScan,
activeScan,
blockingScan,
onReopenActiveScan,
pinnedScanId,
latestScanId,
Expand Down Expand Up @@ -905,19 +918,19 @@ function ProjectDetailHeader({
<Button
size="sm"
onClick={onScan}
disabled={!canScan || activeScan !== null}
disabled={!canScan || blockingScan !== null}
title={
demoReadOnly
? t("page.scan_demo_disabled")
: activeScan !== null
: blockingScan !== null
? t("page.scan_already_active", {
defaultValue:
"A scan is already running for this project — open the in-progress drawer to view it.",
})
: undefined
}
data-testid="project-detail-scan"
data-scan-blocked={activeScan !== null ? "active" : undefined}
data-scan-blocked={blockingScan !== null ? "active" : undefined}
>
{t("page.scan")}
</Button>
Expand Down
6 changes: 6 additions & 0 deletions apps/frontend/src/features/projects/api/projectDetailApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ export interface ScanSummary {
* without a release label.
*/
release: string | null;
/**
* Normalized git ref the scan targeted, or `null` for an ad-hoc trigger.
* The concurrency gate is per-(project, ref), so "is my trigger blocked?"
* has to compare refs rather than just checking for any active scan.
*/
ref: string | null;
}

export interface ProjectOverviewResponse {
Expand Down
Loading
Loading