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
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ If `docker ps` fails, ask the user to start OrbStack.
- **`riptide.json` is config, not data.** `openshift/collector/riptide.json` (in-repo sample) declares teams (name + `group_email`) and org-wide automation rules; edits go through PRs and the pod hot-reloads via mtime in `RiptideConfigStore.maybe_reload()`. Never propose moving it into Postgres.
- **Per-team bearer keys live in a separate file**, mounted in production from the `riptide-collector-team-keys` Secret (never committed); `openshift/collector/team-keys.json` is a dev sample with deterministic test hashes (raw dev bearers in `compose.yaml`). Stored as sha256, hot-reloaded by `TeamKeysStore` like the config. The bearer **is** the team identity — every webhook is tagged `team = caller_team`.
- **No `service` column, no `service_id` on the wire.** Per-source aggregations group by `repo_full_name` / `pipeline_name` / `app_name` / `repo`; org-wide rollups by `team`. Identifiers are lowercased at ingest (`commit_sha`, `revision`, `repo_full_name`, `branch_name`, `repo`) so joins are case-stable. Never propose a unified `service` column or `service_id` — it served only single-pane labelling and was dropped.
- **`automation` is org-wide.** Bot definitions live at the config root, not per team.
- **`automation` is org-wide.** Bot definitions live at the config root, not per team. Config is the last resort, not the first: prefer what the upstream payload already states (the acting user's `type == "SERVICE"` — `actor` on push and reviewer events, `pullRequest.author.user` on PR lifecycle ones → `automation_source = "service-account"`, ranked above the `*-bot` name guess) and what senders declare about themselves (`reviewer_handle` / `actor_handle` + kind). Only accounts nobody reports get a config entry.
- **Metrics are computed on read, not at ingest.** No aggregation tables or scheduled rollup jobs in v1. Schema additions preserve raw events; new metrics are SQL against existing rows or future materialized views.
- **Commit SHA joins Bitbucket↔Pipeline; Argo CD joins on the image reference.** `bitbucket_events.commit_sha = pipeline_events.commit_sha` is deterministic (App-repo SHA both sides). `argocd_events.revision` is the **GitOps-repo SHA** — proven empirically, four Apps for one service share one revision — so it does NOT match the other two. Image **tags are not commit SHAs** either (measured in production: 0 of 4 936 image refs, all semver) — never build correlation on parsing a SHA out of a tag. The contract is the **full image reference**: senders report `pipeline_events.image_ref` (`registry/path:tag`), Argo CD stores the same strings in `payload->'images'` from `.app.status.summary.images`, and the join is exact. Historical rows predating `image_ref` fall back to the release-note correlator in `docs/correlating-deploys-to-commits.md`. Never propose `service_id` or hand-coded name mappings to fix correlation.
- **`change_type` lives on Bitbucket events only.** Don't denormalise it onto pipeline / Argo rows; join Pipeline rows via `commit_sha` and Argo rows via `payload->'images'` ↔ `pipeline_events.image_ref` at read time.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ because there's no clock-start to subtract from in the first place.
| **Tickets per deploy** | `COUNT(DISTINCT unnest(jira_keys))` per deploy — small-batch indicator. Jira keys are extracted at write time from PR title, description, branch name, and commit messages via regex `[A-Z][A-Z0-9]+-\d+`, deduplicated, GIN-indexed. |
| **Untracked-work rate** | `COUNT(*) WHERE jira_keys = '{}'` over merged PRs — process-compliance signal. |
| **Per-ticket flow** | `WHERE 'ABC-1234' = ANY(jira_keys)` returns every event for a ticket across Bitbucket / pipeline / Argo (joined via commit_sha). |
| **Human vs automated split** | `WHERE NOT is_automated` (Renovate / Dependabot / Snyk / Mend / generic-bot detection runs at write time and tags `automation_source`), plus the `non_human_identities` filter for accounts a sender declared. Keep `bot` and `service` apart when reading: a **bot** authors work of its own and its velocity is worth its own view, while a **service** account (a CI user pushing merges) authors nothing and should simply not appear in human activity. Default dashboards exclude both. |
| **Human vs automated split** | `WHERE NOT is_automated` (Renovate / Dependabot / Snyk / Mend / generic-bot detection runs at write time and tags `automation_source`; an account the git host itself marks as a service account — Bitbucket DC's built-in system user, which files the default PR tasks — is tagged `service-account` with no config entry), plus the `non_human_identities` filter for accounts a sender declared. Keep `bot` and `service` apart when reading: a **bot** authors work of its own and its velocity is worth its own view, while a **service** account (a CI user pushing merges) authors nothing and should simply not appear in human activity. Default dashboards exclude both. |
| **AI reviewer precision** *(noergler)* | `1 - count(noergler_events WHERE event_type='feedback' AND verdict='disagreed') / sum(findings_count) FILTER (WHERE event_type='pr_completed')` per repo × week — findings on both sides, since one PR can collect several disagreements. Higher = the AI review is more useful. Filter on `outcome='merged'` to score precision only on PRs that shipped. |

### FinOps signals
Expand Down
13 changes: 13 additions & 0 deletions src/riptide_collector/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ def detect_automation_source(
author: str | None,
branch_name: str | None,
author_display_name: str | None = None,
author_is_service_account: bool = False,
) -> str | None:
"""Match an event's author against the configured automation sources.

Expand All @@ -220,6 +221,12 @@ def detect_automation_source(
login, bot name only in `displayName`) is otherwise indistinguishable
from a human, and its instant review comments drive the DX Core 4
pickup-time metric toward zero.

`author_is_service_account` is the git host's own verdict about the
account (Bitbucket DC marks its built-in system user `type: SERVICE`).
It ranks below the configured sources — those name the tool, which is
more specific — but above the `*-bot` name heuristic, since a stated
fact beats a guess from the handle.
"""
config = self._config
handles = [name for name in (author, author_display_name) if name]
Expand All @@ -236,6 +243,12 @@ def detect_automation_source(
for prefix in source.branch_prefixes:
if branch_name.startswith(prefix):
return source.name
# The host's verdict beats guessing from the name: an account called
# `ci-bot` that Bitbucket reports as a service account is a service
# account, and mislabelling it `other-bot` would put a technical user
# into bot-velocity views, which are meant to show work bots author.
if author_is_service_account:
return "service-account"
if any(looks_bot_shaped(handle) for handle in handles):
return "other-bot"
return None
19 changes: 19 additions & 0 deletions src/riptide_collector/parsers_bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,10 @@ class BitbucketEventDraft:
# detection can match on either. Some review bots post under an
# ordinary-looking login and are only recognisable by display name.
author_display_name: str | None
# Bitbucket's own verdict on the account: DC marks its built-in system
# user (default PR tasks, stale-PR notices) `type: SERVICE`. Trusting the
# host beats asking every installation to name a server-side account.
author_is_service_account: bool
branch_name: str | None
change_type: str | None
jira_keys: list[str]
Expand Down Expand Up @@ -151,6 +155,17 @@ def _user_display_name(user: dict[str, Any]) -> str | None:
return value if isinstance(value, str) and value else None


def _is_service_account(user: dict[str, Any]) -> bool:
"""Whether Bitbucket itself classifies this user as a service account.

BBS DC sets `type` to NORMAL for people and SERVICE for accounts the
server acts as — the built-in system user that files default PR tasks and
posts stale-PR notices. Those comments land within a second of a PR being
opened, so counted as human they make review pickup time look instant.
"""
return str(user.get("type", "")).upper() == "SERVICE"


def _synth_delivery_id(event_key: str | None, body: dict[str, Any]) -> str:
pr = _as_dict(body.get("pullRequest"))
pr_id = pr.get("id")
Expand Down Expand Up @@ -209,6 +224,7 @@ def extract_event(
commit_sha: str | None = None
author: str | None = None
author_display_name: str | None = None
author_is_service_account = False
is_revert = False

# Reviewer-activity events carry the actor (the reviewer / commenter)
Expand All @@ -235,6 +251,7 @@ def extract_event(
author_user = _as_dict(_as_dict(pr.get("author")).get("user"))
author = _user_handle(author_user)
author_display_name = _user_display_name(author_user)
author_is_service_account = _is_service_account(author_user)
# PR-side revert detection: the title is the only signal we have
# without a REST round-trip. Push-side detection would need the
# commit messages between fromHash..toHash.
Expand Down Expand Up @@ -279,6 +296,7 @@ def extract_event(
actor = _as_dict(body.get("actor"))
author = _user_handle(actor)
author_display_name = _user_display_name(actor)
author_is_service_account = _is_service_account(actor)

repo_full_name = lower(raw_repo_full_name)
branch_name = lower(branch_name)
Expand All @@ -292,6 +310,7 @@ def extract_event(
commit_sha=commit_sha,
author=author,
author_display_name=author_display_name,
author_is_service_account=author_is_service_account,
branch_name=branch_name,
change_type=parse_change_type(branch_name),
jira_keys=extract_jira_keys(title, description, branch_name),
Expand Down
1 change: 1 addition & 0 deletions src/riptide_collector/routers/bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ async def bitbucket_webhook( # pyright: ignore[reportUnusedFunction]
draft.author,
draft.branch_name,
draft.author_display_name,
draft.author_is_service_account,
)

try:
Expand Down
36 changes: 36 additions & 0 deletions tests/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,42 @@ def test_rejects_production_stage_in_ignored_list(self, tmp_path: Path) -> None:
load_config_from_path(path)


class TestServiceAccountDetection:
def test_host_declared_service_account_is_automation(self, tmp_path: Path) -> None:
# Bitbucket marks its own system user as a service account, so no
# installation has to list it by name.
path = _write(tmp_path / "c.json", VALID)
store = RiptideConfigStore(path)

assert (
store.detect_automation_source("bitbucket.system-user", None, "Bitbucket", True)
== "service-account"
)

def test_configured_source_keeps_its_own_name(self, tmp_path: Path) -> None:
# The host's verdict is a fallback: a configured bot stays attributed
# to the tool it is, so per-source views keep working.
path = _write(tmp_path / "c.json", VALID)
store = RiptideConfigStore(path)

assert store.detect_automation_source("renovate-bot", None, None, True) == "renovate"

def test_human_unaffected(self, tmp_path: Path) -> None:
path = _write(tmp_path / "c.json", VALID)
store = RiptideConfigStore(path)

assert store.detect_automation_source("alice", "feature/x", "Alice", False) is None

def test_bot_shaped_name_does_not_mask_a_service_account(self, tmp_path: Path) -> None:
# A stated fact beats a guess from the handle: labelling a technical
# account `other-bot` would drop it into bot-velocity views, which
# exist to show work that bots actually author.
path = _write(tmp_path / "c.json", VALID)
store = RiptideConfigStore(path)

assert store.detect_automation_source("ci-bot", None, None, True) == "service-account"


class TestAutomationByDisplayName:
def test_display_name_match_when_login_is_ordinary(self, tmp_path: Path) -> None:
# A review bot provisioned as a normal user account: the login is a
Expand Down
59 changes: 59 additions & 0 deletions tests/test_parsers_bitbucket.py
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,65 @@ def test_occurred_at_parsed_from_iso_string(self) -> None:
assert result.occurred_at.tzinfo is not None


class TestServiceAccounts:
def test_service_type_actor_flagged(self) -> None:
# Bitbucket's built-in system user (default PR tasks, stale-PR
# notices) comments within a second of a PR opening.
body = _load("bitbucket_pr_comment_added.json")
body["actor"]["type"] = "SERVICE"

result = extract_event(
body,
x_event_key="pr:comment:added",
x_request_uuid="r",
x_hook_uuid=None,
)

assert isinstance(result, BitbucketEventDraft)
assert result.author_is_service_account is True

def test_normal_type_actor_not_flagged(self) -> None:
body = _load("bitbucket_pr_comment_added.json")
body["actor"]["type"] = "NORMAL"

result = extract_event(
body,
x_event_key="pr:comment:added",
x_request_uuid="r",
x_hook_uuid=None,
)

assert isinstance(result, BitbucketEventDraft)
assert result.author_is_service_account is False

def test_missing_type_defaults_to_not_a_service_account(self) -> None:
# Older payloads and other hosts may not send `type` at all; absence
# must not promote a person to a service account.
body = _load("bitbucket_pr_comment_added.json")
body["actor"].pop("type", None)

result = extract_event(
body,
x_event_key="pr:comment:added",
x_request_uuid="r",
x_hook_uuid=None,
)

assert isinstance(result, BitbucketEventDraft)
assert result.author_is_service_account is False

def test_pr_author_service_type_flagged(self) -> None:
# PR lifecycle events attribute to the PR opener, so the check has to
# follow the same user the author came from.
body = _load("bitbucket_pr_merged.json")
body["pullRequest"]["author"]["user"]["type"] = "SERVICE"

result = extract_event(body, x_event_key="pr:merged", x_request_uuid="r", x_hook_uuid=None)

assert isinstance(result, BitbucketEventDraft)
assert result.author_is_service_account is True


class TestAuthorDisplayName:
def test_actor_display_name_travels_with_reviewer_events(self) -> None:
# Given a reviewer-activity event, where the actor is the author
Expand Down
29 changes: 29 additions & 0 deletions tests/test_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,35 @@ async def test_renovate_pr_tagged_as_automated(
assert row.automation_source == "renovate"
assert row.is_automated is True

async def test_host_service_account_tagged_as_automated(
self,
client: AsyncClient,
session_factory: async_sessionmaker[AsyncSession],
) -> None:
# End to end: Bitbucket's own system user posts the default PR tasks
# seconds after a PR opens. Tagged from the host's `type: SERVICE`,
# with nothing in the automation config naming the account.
del session_factory
payload = _load("bitbucket_pr_comment_added.json")
payload["actor"] = {
"name": "bitbucket.system-user",
"slug": "bitbucket.system-user",
"displayName": "Bitbucket",
"type": "SERVICE",
}
response = await post_bitbucket(
client,
payload,
extra_headers={"X-Request-UUID": "uuid-s", "X-Event-Key": "pr:comment:added"},
)
assert response.status_code == 202

async with self._fresh_session_factory(client)() as session:
row = (await session.execute(select(BitbucketEvent))).scalar_one()
assert row.author == "bitbucket.system-user"
assert row.automation_source == "service-account"
assert row.is_automated is True

async def test_idempotency_same_uuid_inserts_once(
self,
client: AsyncClient,
Expand Down