Work Queue Service Design
Architectural design document for workqueue-service. Covers the
reasoning behind every major design choice so that future changes can be
made safely. For operational usage, see
Work Queue Service.
1. Design Philosophy
Four principles shaped the service’s architecture:
Security boundary. The service sits between untrusted callers (AI agents, CI jobs, automation scripts) and trusted write operations (GitLab pushes, MR creation). All changes are inspectable structured data, not opaque blobs. Path blocklists and action validation run before any GitLab write.
Type-agnostic queue, type-specific behaviour. The processing loop, claiming, heartbeat, and lease management are generic. Type-specific logic (MR creation, CVE remediation) lives in pluggable subpackages that register deciders and executors at import time. Adding a new work item type requires no changes to the generic layer.
Single pending action. Only one action per work item at a time. After each action, the decider re-evaluates with current state. This avoids queuing actions that assume a future state which may not hold.
Phase + dimensions, not a linear state machine. The coarse lifecycle (active/completed/failed/cancelled) is separate from independent readiness signals (CI, approval, Konflux build). Dimensions can regress (force-push resets CI), which a linear state machine cannot express.
2. Architecture
Callers submit work items via REST API. The ProcessingManager drains
pending actions in per-item threads, dispatching to type-specific
executors. Executors create and update GitLab MRs via the Commits API.
flowchart LR
callers["Callers\n(CI, CLI, agents)"] --> API["REST API\n+ Auth"]
API --> DB[("PostgreSQL")]
Processing["ProcessingManager"] --> DB
Processing --> GL["GitLab\nCommits API"]
API -.-> Processing
3. Design Decisions Record
DDR-1: Phase + dimensions vs linear state machine
Decision: Replace the 12-state linear state machine with a phase + dimensions model.
Context: The initial design used a single state column:
pending → in_progress → created → mr_ci → approved → merged → post_merge_ci → building → releasing → completed. This broke because
post-creation concerns (CI, approval, Konflux build) are independent —
CI can pass before approval, approval can be revoked, force-push resets
CI but not approval.
Alternatives: (a) Composite states (ci_passed_approval_pending) —
exponential explosion. (b) Multiple boolean columns — rigid, schema
migration for each new signal.
Rationale: Phase tracks coarse lifecycle; independent dimensions in a separate table track readiness signals with variant support for multi-instance cases (per-component Konflux builds). Dimensions can regress naturally. New dimension types need no schema migration.
DDR-2: Four generic phases (active/completed/failed/cancelled)
Decision: Narrow from 6 MR-specific phases to 4 generic phases.
Context: The intermediate design had pending, active, merged,
completed, failed, closed — mixing generic concepts with
MR-specific milestones.
Alternatives: Keep MR-specific phases and add more for each type.
Rationale: Phase answers “is this item done, and how?” Type-specific
milestones (MR merged, CI passed) are tracked via dimensions and
detail-table columns. Items start as active — there is no pending
phase because the decider immediately assigns an action at creation time.
DDR-3: Dimensions in a separate table with variant support
Decision: Store dimensions in work_item_statuses table, not as
columns on work_items.
Context: Initially considered a column per dimension
(ci_status, approval_status, etc.).
Alternatives: (a) Column per dimension — schema migration for each
new one. (b) JSONB column — flexible but loses CHECK constraints and
complicates queries. (c) Separate table with composite PK
(work_item_id, dimension, variant).
Rationale: Option (c). New dimensions need no migration. Variants handle multi-instance signals (multiple Konflux components). Each work item only has rows for dimensions that apply to it. The “are all dimensions satisfied?” query is a natural GROUP BY/HAVING.
DDR-4: Decider as pure function, not state machine transitions
Decision: The decider is a pure function
decide(item) → action | None registered per type. It only determines
the next action to execute.
Context: The original design had a TRANSITIONS dict mapping
state → frozenset[state] with validate_transition().
Alternatives: (a) Transition table — rigid, can’t express “depends on which dimensions passed”. (b) Decider function — flexible, testable.
Rationale: The decider is trivially testable (given state, assert action). State transitions come from executors (they apply phase changes directly) and events (they update dimensions). The decider just picks the next action based on current state.
DDR-5: Single pending action, not a queue
Decision: One pending action per work item at a time.
Context: Considered queuing multiple actions (“merge, then trigger Konflux build”).
Alternatives: Action queue table per work item.
Rationale: Queued actions assume future state that may not hold. If
“merge” fails, the queued “trigger Konflux build” is nonsense. After
each action, the decider re-evaluates with current state. The attempts
counter tracks consecutive failures, not total actions.
DDR-6: Claiming via leased_until (reusable)
Decision: Time-based lease (leased_until column) for work item
claiming, reusable across all phases.
Context: The original design used state = 'in_progress' as a
one-time claiming mechanism.
Alternatives: (a) claimed_by + claimed_at columns — identifies
the worker but adds complexity. (b) state = 'in_progress' — ties
claiming to phase, not reusable. (c) leased_until timestamp — simple,
reusable, self-recovering.
Rationale: Option (c). Items can be reclaimed repeatedly throughout their lifecycle. Expired leases are automatically reclaimable by other workers. The heartbeat context manager extends the lease during processing.
DDR-7: Generic + detail tables (joined table inheritance)
Decision: Split work_items into generic queue table + per-type
detail tables (mr_details, future cve_details).
Context: The initial design had all MR-specific columns on the
work_items table.
Alternatives: (a) Single table with nullable type-specific columns — sparse, doesn’t enforce type constraints. (b) JSONB payload column — flexible but loses type safety. (c) Joined table inheritance.
Rationale: Option (c). SQLAlchemy’s polymorphic_load="selectin"
automatically loads the correct subclass. New types add their own detail
table — no changes to the generic layer. Type-specific columns have
proper types and constraints.
DDR-8: Type-dispatched decider/executor registries
Decision: Registry pattern for deciders and executors. Each type
registers at import time via decider.register("mr", _mr_decide).
Context: Needed a way to dispatch to type-specific logic without the generic layer knowing about types.
Alternatives: (a) if/elif chains in the processing loop. (b) Class-based dispatch (strategy pattern). (c) Registry functions.
Rationale: Option (c). Simple, explicit, no class hierarchy needed.
The mr/__init__.py import triggers registration. New types just add
their own register() calls.
DDR-9: Structured file actions vs git patch_data
Decision: Replace opaque patch_data (BYTEA, git format-patch
output) with structured file_actions (JSONB) and commit_message.
Context: The MR service sits on the security boundary — it writes to GitLab repos on behalf of untrusted callers. HUM-851 and the AI Investigation design rules require pre-commit inspection (path blocklists, content validation).
Alternatives: (a) Keep patch_data, clone repo, apply patch, then
inspect diff — inspection happens after creating a working tree with
credentials. (b) Parse patches with unidiff library — gives diffs not
full content, still needs original files. (c) Structured file actions.
Rationale: Option (c). Inspection is a pure function over data the
service already has — no clone, no git binary, no temp dirs. Path
blocklists and action validation run before any API call. The
file_actions JSONB column is also a queryable audit record.
DDR-10: GitLab Commits API vs git subprocess
Decision: Use the GitLab Commits API for branch creation and file
commits. Delete git_ops.py.
Context: The original executor shelled out to git (clone, am,
push), requiring: git binary in the container, subprocess timeout
handling, GIT_ASKPASS credential management, temp directory lifecycle.
Alternatives: (a) Keep git subprocess with GIT_ASKPASS and timeout. (b) GitLab Commits API.
Rationale: Option (b). gitlab_sync.py in the same repo already
proves the pattern. The Commits API’s start_branch parameter creates
the branch automatically. No subprocess, no credentials in URLs, no temp
dirs. Callers have full file contents (not patches), so git am
semantics aren’t needed.
DDR-11: OIDC issuer from discovery, not hardcoded
Decision: Read the OIDC issuer from the /.well-known/openid-configuration
discovery response. Rewrite the JWKS URI to use kubernetes.default.svc.
Context: The initial implementation hardcoded
issuer = "https://kubernetes.default.svc". This failed on EaaS clusters
where --service-account-issuer is
https://oidc.op1.openshiftapps.com/.... The JWKS URI from discovery
pointed to an IP:6443 that was unreachable from pods.
Alternatives: (a) Environment variable override
(KUBE_OIDC_ISSUER). (b) Read from discovery.
Rationale: Option (b). No configuration needed — works on any cluster
automatically. The JWKS fetch uses the path from discovery but rewrites
the host to kubernetes.default.svc (always reachable on port 443). SA
bearer token authenticates both requests (OpenShift blocks anonymous
JWKS access).
DDR-12: item_type not type
Decision: Name the discriminator column item_type everywhere (DB,
ORM, API, schemas).
Context: type is a Python builtin. Using it as a function
parameter triggers ruff A002.
Alternatives: (a) Use type with alias in different contexts.
(b) Rename to item_type everywhere.
Rationale: Option (b). Clean rename, no aliases, no special casing. Consistent at every layer.
DDR-13: source field removed
Decision: Remove the source column from mr_details. The
caller_identity field (set automatically from authentication) serves
the same purpose.
Context: source was a user-declared label (“rpms-ci”,
“dependency-updater”) while caller_identity is the authenticated
identity. Having both was redundant.
Alternatives: Keep both for cases where a shared SA submits on behalf of different logical sources.
Rationale: Removed until needed. caller_identity captures who
submitted. If per-source filtering is needed later, it can be
re-introduced as a field on MRWorkItemCreate.
DDR-14: attempts counts consecutive failures, not total actions
Decision: attempts increments only on executor failure and resets
to 0 on success.
Context: The initial implementation incremented attempts before
every action execution. A work item chaining 4 actions (ensure_mr →
merge → trigger_build → complete) would exhaust MAX_ATTEMPTS on the
third action.
Alternatives: (a) Separate total_actions counter. (b) Only count
failures.
Rationale: Option (b). attempts answers “how many consecutive
times has this action failed?” — the signal for escalation. Total
actions executed is an audit concern, not a processing concern.
DDR-15: Executor applies state directly; decider only picks next action
Decision: Executors directly modify item.phase, item.mr_iid,
etc. The decider only returns the next action to execute.
Context: Initially the processing loop set the phase based on
ExecutorResult.success. This mixed two concerns.
Alternatives: (a) Processing loop sets phase. (b) Decider sets phase. (c) Executor sets phase.
Rationale: Option (c). The executor knows what happened (“I created an MR, so phase stays active”). Events that update dimensions also directly set state. The pattern is the same: something happens → state is updated → decider evaluates for the next action. Symmetry with event-driven dimension updates.
DDR-16: Items stay active after MR creation
Decision: After the ensure_mr executor succeeds, the item remains
in active phase with no further actions pending.
Context: After MR creation, the decider returns None (no action).
The item sits in active indefinitely.
Alternatives: (a) Set phase to completed after MR creation.
(b) Keep active for future event-driven lifecycle tracking.
Rationale: Option (b). The active phase covers the entire lifecycle
— from submission through CI, approval, merge, and post-merge tracking.
The decider returns no actions until event-driven dimension updates
(future SQS consumer) trigger the next step.
DDR-17: Audit log to S3, not database
Decision: Service-level audit (state transitions, decider decisions, actions taken) goes to S3 as immutable objects.
Context: Raw webhook events are already archived by
sns-s3-archiver. The service needs its own audit trail for state
transitions and decider evaluations.
Alternatives: (a) Database table (work_item_events). (b) S3
immutable objects. (c) Both.
Rationale: Option (b). S3 is immutable and cheap. The operational
DB stays lean. Raw events are already in S3 via sns-s3-archiver;
service-level audit captures the service’s reaction to those events.
DDR-18: Agent direction — dumber, not smarter
Decision: The hummingbird-agent should become a pure compute engine. The work queue service is the MR lifecycle orchestrator. The agent never calls the work queue service; the work queue service calls the agent.
Context: HUM-857 proposed adding create_merge_request as a new
action in the agent’s actions.py. This was the wrong direction.
Alternatives: (a) Agent gains more actions (push, merge, MR management). (b) Agent becomes stateless compute; work queue service orchestrates.
Rationale: Option (b). The agent keeps LLM loop, tool registry, and sandbox — nothing else. Event routing, rate limiting, placeholder notes, result posting, session management, and all GitLab writes move to the work queue service. HUM-857 closed as Obsolete.
DDR-19: Shared database
Decision: The work queue service uses the same events PostgreSQL
database as hummingbird-status and hummingbird-dashboard.
Context: Cross-service joins (e.g. work_items JOIN
gitlab_merge_requests JOIN pipelineruns) are useful for the dashboard.
Alternatives: Separate database per service.
Rationale: Shared DB with service-specific Alembic migration chains
(alembic_version_mr_service). Each service manages its own tables
independently. The migration chain is isolated so one service can evolve
its schema without affecting others.
DDR-20: Pydantic config over cki-lib/jsonschema
Decision: Use Pydantic for config validation. Load config from a YAML file at startup.
Context: Needed structured config for authorization rules and per-type operational settings. Alternatives: cki-lib config helpers, jsonschema, or plain dicts.
Alternatives: (a) cki-lib — adds a dependency, the service stays self-contained without it. (b) jsonschema — separate schema file, no type-safe access. (c) Pydantic — already a dependency (API schemas).
Rationale: Option (c). Pydantic provides type-safe attribute access, defaults, cross-field validators (e.g. mutual exclusivity of groups vs issuer+claims), and optional JSON Schema export. No new dependencies.
DDR-21: Claim-based rules (Vault bound_claims pattern)
Decision: Authorization rules match on arbitrary JWT claims, not
just sub. All claim conditions AND; list values OR.
Context: Studied JWT trust/authz config from AWS API Gateway, HashiCorp Vault, Kubernetes Structured Auth, Istio, Traefik Hub, and oauth2-proxy. All support matching on arbitrary claims.
Alternatives: (a) Subject allowlist — rigid, requires exact sub
values. (b) Claim-based matching — flexible, supports project_path,
ref, groups, etc.
Rationale: Option (b). Vault’s bound_claims pattern is proven and
simple. Values are coerced to lowercase strings before comparison to
handle int/string mismatches (GitLab’s numeric project_id) and
bool/string mismatches (ref_protected: true).
DDR-22: Issuer per rule, audience as claim
Decision: issuer is the only special field on a token rule — it
determines which JWKS keys to use. All other JWT conditions (including
aud) are standard claim matches in the claims dict.
Context: Considered a separate jwt.issuers section and/or treating
aud as a first-class field alongside issuer.
Alternatives: (a) Global jwt section with trusted issuers +
separate rules section. (b) aud as a dedicated field on each rule.
(c) Issuer per rule, audience as a claim.
Rationale: Option (c). Keeps each rule self-contained and
self-documenting. The Pydantic validator enforces that every issuer rule
includes aud in claims to prevent cross-service token reuse.
DDR-23: Unified rule model for groups and tokens
Decision: A single rules list where every rule maps a remote
identity (groups or issuer+claims) to local permissions.
Context: The original design had admin_groups/retry_groups
(local permission → remote groups) alongside rules (remote identity →
local permissions) — opposite mapping directions.
Alternatives: Keep separate group and token rule formats.
Rationale: Unified model. Every rule follows the same structure: remote identity → local permissions. Pydantic cross-field validators enforce mutual exclusivity.
DDR-24: Bearer-only auth with users/~ self-authentication
Decision: All API auth uses Authorization: Bearer tokens. Opaque
OCP tokens (sha256~...) are validated by calling the OCP users/~
API with the user’s own token (self-authenticating).
Context: The oauth-proxy cannot simultaneously forward cookie-session
tokens AND pass through non-OCP Bearer tokens on the same path. The
TokenReview API would be cleaner but requires system:auth-delegator
ClusterRoleBinding — a cluster-scoped permission not available on
managed OCP clusters.
Alternatives: (a) Trust X-Forwarded-User from the proxy on all
paths — injection risk on bypassed paths. (b) TokenReview API —
requires cluster RBAC. (c) users/~ self-authentication.
Rationale: Option (c). No special RBAC needed. The users/~
endpoint returns username and groups. Results are cached with a
thread-safe TTL cache (5 minutes). The /me endpoint bridges OAuth
cookie sessions to Bearer tokens.
DDR-25: handled_types as explicit opt-in
Decision: handled_types declares which work item types this
instance serves. Empty means handle nothing.
Context: Different deployments may serve different types from the
same container image. Types carry per-type config (e.g. gitlab_url
for MR).
Alternatives: (a) Handle all types by default. (b) Explicit opt-in.
Rationale: Option (b). Prevents accidental catch-all deployments.
The sweep returns early when handled_types is empty — no items stolen
from other instances in a shared-database deployment.
DDR-26: k8s shorthand skipped when not in-cluster
Decision: Rules with issuer: k8s are silently skipped when the
in-cluster CA bundle is absent.
Context: The K8s OIDC issuer URL varies by cluster and is auto-detected at runtime. Local development doesn’t have a cluster.
Alternatives: (a) Error on startup if k8s issuer can’t be
resolved. (b) Skip silently.
Rationale: Option (b). Allows using a production config file for local development and testing without startup errors.
DDR-27: OAuth proxy as login-only bridge with /me endpoint and cookie refresh
Decision: The oauth-proxy sidecar handles only the OAuth login flow.
API endpoints bypass the proxy. A proxy-protected /me endpoint
bridges cookie sessions to Bearer tokens. The proxy must be configured
with --cookie-refresh=1h to prevent stale-token failures.
Context: The OCP oauth-proxy cannot simultaneously forward
cookie-session tokens AND pass through non-OCP Bearer tokens on the
same path. Protected paths validate Bearer tokens via TokenReview
(requires cluster RBAC we cannot obtain) or reject non-OCP tokens.
Bypassed paths pass all Bearer tokens through but ignore cookies.
Additionally, the OCP access token embedded in the proxy’s session
cookie has its own expiry (default 24h), independent of the cookie
lifetime. Without --cookie-refresh, the cookie outlives the token,
causing the proxy to forward expired credentials while still reporting
the user as authenticated — the “logged in but not authorized” failure
observed in the hummingbird-dashboard deployment.
Alternatives: (a) All paths protected — blocks non-OCP Bearer
tokens. (b) --openshift-delegate-urls — requires
system:auth-delegator ClusterRoleBinding. (c) App reads proxy cookies
directly — tight coupling to proxy internals. (d) Login-only proxy
with /me bridge.
Rationale: Option (d). The /me endpoint is the only
proxy-protected API path. It reads X-Forwarded-Access-Token (set by
the proxy from the cookie session) and returns the OCP access token to
the caller. Subsequent API calls use Authorization: Bearer sha256~...
on bypassed paths, where the app validates via the OCP users/~ API.
--cookie-refresh=1h keeps the embedded token fresh. TTL-bounded
caching (5 minutes) on users/~ results ensures expired-token failures
are never permanent (unlike @functools.cache which caches failures
indefinitely).
DDR-28: Audit log with logging + optional S3 archival
Decision: Always log audit entries via Python logging at INFO
level, and optionally archive to S3 as gzipped JSON objects when
WORKQUEUE_SERVICE_AUDIT_S3_BUCKET is set.
Context: Needed an immutable record of state transitions, action outcomes, and auth decisions for compliance and debugging.
Alternatives: (a) Database table — bloats the operational DB with a different concern. (b) S3-only — no visibility without querying Athena; loses events if S3 is unreachable.
Why logging + S3: Logging to stdout is zero-config and integrates with existing container log collection for real-time visibility. S3 provides the durable, queryable archive (Athena) for compliance.
Key design rules:
- Audit failures never propagate —
write_audit_entry()catches all exceptions and logs them, but never raises. - S3 keys use
YYYY/MM/DD/HH/MM/prefix for Athena partitioning. ContentEncoding: gzipset so S3 API consumers get transparent decompression.
Field conventions (follow these when adding new audit calls):
actor: caller identity for API/auth events, thread name for worker events,"system"for decider/system-triggered transitions.old_state: state before the change. Includephaseandactionwhen both are relevant. Omit on creation (no prior state) and on auth events (no work item state).new_state: state after the change. Same keys asold_state. Omit fields that are None (they are filtered byto_dict()).trigger: what caused this event. Always includesource(one of"api","worker","decider"). Addactionfor deliberate operations,reasonfor error/system transitions,errorfor failure details.metadata: context orthogonal to the state transition — request info on auth events, type-specific data on submit. Not for duplicating state already inold_state/new_state.
DDR-29: Events processed in work-item threads, not consumer threads
Decision: The SQS consumer thread stores events in the
pending_events table. Existing per-work-item threads drain and
process them.
Context: The ProcessingManager already runs one thread per work item with lease protection. Processing events in the same thread means events and actions are naturally serialized per work item.
Alternatives: (a) Consumer threads handle events directly — concurrency conflicts with action threads. (b) FIFO queue + in-memory dispatch — receipt handle management, visibility timeout extensions, message loss on crash.
Rationale: Option (c). Events join the existing thread structure. The decider runs after each event, determining follow-up actions. The single-thread-per-item invariant is preserved.
DDR-30: pending_events table as durable event store
Decision: Use a PostgreSQL table (pending_events) instead of
FIFO queues or in-memory queues for event delivery to work-item threads.
Context: Since work-item threads do the processing, the SQS consumer just needs to deliver events to them durably.
Alternatives: (a) FIFO queue + in-memory dispatch — complex EventBroker, receipt handle management, message loss on crash. (b) FIFO queue + direct handling — concurrency conflicts.
Rationale: The DB provides durability (crash recovery), serialization
(single thread per work item), and ordering (last_event_at guard). No
FIFO queue, router thread, MessageGroupId computation, in-memory queues,
or visibility timeout management needed.
DDR-31: SQS delete after INSERT, not after full processing
Decision: Delete the SQS message after INSERT+COMMIT into
pending_events, not after full event processing.
Context: The hummingbird-agent deletes after handling (no DB intermediary). The workqueue-service can delete after INSERT+COMMIT because the event is crash-recoverable in PostgreSQL.
Alternatives: Delete after full processing — requires visibility timeout extensions, receipt handle passing, heartbeat management.
Rationale: After INSERT+COMMIT the event is durable in PostgreSQL.
Duplicate SQS delivery may INSERT the same event twice; the
last_event_at guard makes processing idempotent.
DDR-32: Resolver filters by phase, consumer stays read-only
Decision: The resolver filters by phase = 'active' on the
MRWorkItem query. SQLAlchemy’s joined table inheritance handles the
work_items table access implicitly. The consumer does SELECT +
INSERT only, no UPDATE, no row lock contention.
Context: Only active items accept event-driven transitions (see
DDR-37). Filtering at resolve time avoids unnecessary INSERTs into
pending_events, thread spawns, and no-op processing for terminal
items.
Alternatives: (a) Resolve without phase filter, guard only at processing time — wastes work for terminal items. (b) Consumer checks phase and applies transitions directly — contention with work-item threads.
Rationale: The phase filter adds negligible cost (PK lookup on
work_items, typically one row). The consumer remains read-only.
Business logic (phase transitions, decider, audit) is applied by the
generic handle_event layer in the work-item thread.
DDR-33: Raw events stored, interpretation separated from application
Decision: The pending_events table stores the raw event JSON
body and event type. Type-specific handlers interpret events (read-only
on the work item), the generic layer applies the result.
Context: Event processing has three concerns: interpretation (what does this event mean?), application (phase transition, decider, audit), and routing (which work items?). Mixing them makes handlers hard to test and inconsistent across types.
Alternatives: (a) Handlers apply all changes directly — mixes interpretation with mutation, each handler reimplements decider/audit boilerplate. (b) Pre-process events in the consumer — consumer needs type-specific knowledge.
Rationale: Handlers return an EventResult dataclass (phase,
event_ts, trigger metadata). The generic handle_event applies the
phase transition via update_work_item_from_event, runs the decider,
and emits audit entries. Handlers have read-only access to the work
item and session (e.g. to check existing dimensions or detail fields)
but EventResult is their only output — they never mutate the work
item directly. This ensures decider/audit logic is consistent across
all item types. See DDR-38 for the dimension delivery extension.
DDR-34: Per-work-item advisory lock for insert/drain coordination
Decision: Use pg_advisory_xact_lock(key) (transaction-scoped)
keyed by work_item_id.int & 0x7FFFFFFFFFFFFFFF to coordinate the
consumer’s INSERT with the work-item thread’s exit decision.
Context: Without coordination, the consumer can INSERT an event between the thread’s final drain check and its exit, orphaning the event until the next sweep (~5 minutes).
Alternatives: (a) Accept the race — events wait up to 5 minutes. (b) Global lock — unnecessary contention.
Rationale: Per-work-item advisory lock closes the gap. Either the
INSERT commits before the thread’s re-drain (thread sees it and
continues), or the INSERT commits after the thread exits (notify()
spawns a new thread). The hash space is 2^63, making collisions
negligible. The sweep is the safety net for the astronomically unlikely
collision case.
DDR-35: Sweep for startup recovery
Decision: Extend the periodic sweep to also check for work items
with unprocessed pending_events rows.
Context: Events that arrive while the service is stopped, or that survive a mid-processing crash, must be processed on restart.
Alternatives: Separate startup-only scan.
Rationale: Unifying startup recovery with the periodic sweep keeps
a single code path. The sweep runs immediately on manager.start(),
catching events from downtime. It also serves as a safety net behind
the advisory lock.
DDR-36: Consumer lifecycle and shutdown ordering
Decision: The SQS consumer is a single daemon thread started from the FastAPI lifespan. Shutdown stops the consumer before work-item threads.
Context: If work-item threads stop while the consumer is still INSERTing, events could be orphaned. The consumer must stop first.
Alternatives: Non-daemon thread with explicit join.
Rationale: Single thread is sufficient for low event volume (MR
state changes). Daemon thread ensures process exit even if the thread
hangs. Shutdown ordering: (1) set shutdown_event, (2) join consumer
thread (timeout=30s), (3) call manager.stop(). No consumer when
WORKQUEUE_SERVICE_EVENTS_QUEUE_URL is unset.
DDR-37: Terminal phase semantics — only active items accept events
Decision: Only items in phase = 'active' accept event-driven
phase transitions. completed, failed, and cancelled are terminal
phases — recovery requires explicit action via the retry API.
Context: Initially only failed was guarded (phase != 'failed').
completed and cancelled items could be modified by events: a
closed event could transition a completed item to cancelled, and an
opened event could reactivate a cancelled item.
Alternatives: (a) Guard only failed — inconsistent, completed
items are accidentally mutable. (b) Growing exclusion list
(NOT IN ('failed', 'completed')) — fragile if new phases are added.
(c) Positive match phase = 'active'.
Rationale: Option (c). A single positive match is clearer and
future-proof. If a work item was cancelled (MR closed) or completed
(MR merged), only deliberate human action (retry API) should
reactivate it — not an automated event. The retry API accepts both
failed and cancelled items. The resolver also filters by
phase = 'active' at query time (DDR-32) to avoid unnecessary
INSERT/processing for terminal items.
DDR-38: Dimension updates via EventResult
Decision: Event handlers return dimension upserts as part of
EventResult. The generic handle_event layer applies them.
Context: DDR-33 established that handlers return structured results and the generic layer applies them. Dimensions (DDR-1, DDR-3) need the same pattern — handlers identify which dimensions changed, the generic layer upserts them and emits audit entries.
Alternatives: (a) Handlers call crud.upsert_dimension() directly —
breaks the read-only handler contract from DDR-33. (b) Separate
dimension-specific consumer — duplicates the resolver/handler
infrastructure.
Rationale: EventResult.dimensions is a list of DimensionUpdate
dataclasses. After phase transition logic, handle_event iterates over
dimensions and calls crud.upsert_dimension() for each, emitting an
AuditEntry with event_type="dimension_change" using the existing
dimensions field. Dimension upserts run even when the phase transition
is stale (the early return is refactored to skip only the phase/decider
block). This ensures dimension-only events (e.g. note events with no
phase change) are still applied.
DDR-39: Server-derived idempotency keys with phase-aware uniqueness
Decision: Replace the client-supplied idempotency_key field with a
server-derived key mr:{project}:{source_branch}:{generation}. Enforce
uniqueness only among non-terminal phases via a partial unique index.
Context: The original API required callers to supply a unique
idempotency_key per submission. Recurring callers (dist-git sync,
upstream sync) had to invent unique keys for each invocation and track
them across runs. The global unique constraint also blocked new
submissions for a branch that had a previous completed or cancelled item
with the same key.
Alternatives: (a) Client-supplied key with a prefix convention —
callers still need to generate and track keys. (b) Server-derived key
from the natural deduplication identity (project, source_branch) with
a generation counter for lifecycle restarts.
Rationale: Option (b). The idempotency key is a DB-level safety net,
not a caller concern. The natural identity is (project, source_branch);
the generation counter increments when a new item is created after the
previous one reached a terminal phase. The partial unique index
(WHERE phase NOT IN ('completed', 'cancelled')) ensures at most one
active/failed item per idempotency key while allowing completed items to
coexist with their successors.
DDR-40: Upsert semantics for recurring MR management
Decision: Replace one-shot insert_mr_work_item with
upsert_mr_work_item that looks up an existing active/failed item by
(project, source_branch) and determines the correct action from a
content hash comparison.
Context: Recurring callers submit MRs for the same branch repeatedly. Under the old insert model, each submission created a new work item — even when the previous one was still active, leading to orphaned branches and duplicate MRs.
Alternatives: (a) Client manages deduplication — check before submit, handle conflicts. (b) Server-side upsert — callers always submit current desired state, the service deduplicates transparently.
Rationale: Option (b). The upsert does a
(project, source_branch) lookup among non-terminal items and branches
on the result:
- No match → create a new item (
actionable = true) - Active, same content → no-op (return existing,
"unchanged") - Active, different content → update fields, set
actionable = true, resetneeds_mr = "true" - Failed → reset to active, update content, clear error (
"retried"), setactionable = true - Terminal (completed/cancelled) → create a new generation (DDR-39)
A SHA-256 content hash over normalized file_actions (sorted by
file_path + action, compact JSON) enables cheap same-content
detection without comparing the full payload.
The ensure_mr executor creates a new branch and MR when no MR exists
(item.mr_iid is None), or force-pushes updated content to the existing
branch via the Commits API (force: True) when one does. It syncs MR
metadata (title, description) on force-push. Merging is handled by
reconcile_mr when all merge requirements are satisfied. This avoids
creating a new branch and MR for every content update.
On concurrent insert races, the loser catches the IntegrityError
(from DDR-39’s partial unique index), rolls back, and retries the full
upsert — the winner is now visible and the correct action is taken.
DDR-41: ThreadPoolExecutor with bounded workers
Decision: Replace unbounded per-item threading.Thread spawning
with a fixed-size ThreadPoolExecutor (10 workers).
Context: The periodic tick mechanism (DDR-42) can wake many idle items simultaneously. Unbounded thread creation would cause resource spikes (~109 threads in the worst case).
Alternatives: (a) Keep per-item threads with a semaphore. (b) Bounded thread pool.
Rationale: Option (b). ThreadPoolExecutor provides built-in
queuing when all workers are busy, deduplication via _submitted set,
and clean shutdown via pool.shutdown(wait=True). The 10-worker limit
caps concurrent GitLab API calls.
DDR-42: Hash-bucketed tick events for periodic re-evaluation
Superseded by DDR-48. The
tick_30mandtick_5msynthetic event mechanism has been replaced by thepending_untilcolumn onwork_item_statuses. The sweep now finds items with expiredpending_untiland notifies them directly — no event insertion needed.
Decision: Insert synthetic tick_30m PendingEvent rows for idle
active items using hash bucketing (6 buckets × 5-minute sweep = 30-minute
full cycle).
Context: MRs can become stale when the target branch moves (another MR merges the same content). No reliable GitLab webhook exists for this case. Periodic re-evaluation via tick events enables detection.
Alternatives: (a) Reconciliation sweep as a separate job. (b) Tick events via the existing sweep mechanism.
Rationale: Option (b). Reuses the existing sweep and event processing
infrastructure. Hash bucketing spreads the load deterministically. The
NOT EXISTS guard prevents tick accumulation when the pool is backlogged.
Named tick_30m to allow future ticks at different intervals.
DDR-43: Empty diff detection via repository_compare
Decision: Detect and handle empty MR diffs at two call sites:
ensure_mr (prevention on create and force-push) and check_empty_diff
(periodic via sweep).
Context: The detection-execution race causes empty MRs when the target branch moves between change detection and MR creation. Also, existing MRs can become empty after creation.
Alternatives: (a) Compare file content hashes before commit. (b)
GitLab repository_compare API after commit/push.
Rationale: Option (b). The repository_compare endpoint returns
the diff between two branches. Empty diffs list means the branches
are identical. This is authoritative and handles edge cases that hash
comparison might miss (e.g. whitespace normalization). Cost: one API
call per check (~4 calls/minute across all items for periodic checks).
DDR-44: Per-item processing deadline
Decision: Exit the work item loop after 60 seconds of wall-clock time.
Context: The tick mechanism (DDR-42) and check_stale_mr action
could cause extended processing loops for items that repeatedly generate
new actions. A deadline prevents any single item from holding a pool
worker indefinitely.
Alternatives: (a) Limit loop iterations. (b) Wall-clock deadline.
Rationale: Option (b). Wall-clock time is predictable regardless of action duration. The deadline is between loop iterations, not mid-executor, so executors have their own HTTP timeouts. The item is re-picked by the next sweep if work remains.
DDR-45: Dimension-driven decider with needs_* flags
Decision: The decider scans needs_* dimensions in priority order
instead of encoding per-concern conditionals.
Context: The original decider checked item.mr_iid is None for
create, dashboard_link dimension absence for dashboard notes, and
would need more checks for auto-merge, stale detection, metadata sync.
Each new concern added another conditional.
Alternatives: (a) Per-concern conditionals in decider. (b)
Dimension-driven: events/executors set needs_* flags, decider scans
them.
Rationale: Option (b). The decider becomes a simple priority scan.
Executors own their trigger-to-next-step logic by setting the
appropriate needs_* dimension. Events set needs_reconcile to
request re-evaluation. Adding a new concern = new dimension + handler
in reconciler, no decider changes needed.
Update (DDR-48): The original three-category model (has_*,
needs_*, observed state) was unified into a single needs_* trigger
model with "true"/"false" status. See DDR-48.
DDR-46: reconcile_mr pattern
Decision: Single reconcile_mr executor handles all MR state
convergence, with cheap (always) and expensive (flagged) tiers.
Uses direct mr.merge() instead of GitLab auto-merge.
Context: Separate executors per concern (check_stale_mr,
enable_auto_merge, sync_metadata) each needed their own
registration, decider rule, and dimension tracking. GitLab auto-merge
(auto_merge=True) introduced state-management complexity:
merge_when_pipeline_succeeds tracking, stale SHA detection after
force-push, and recurring 422 noise for non-mergeable MRs.
Rationale: One mergerequests.get() call serves all cheap checks.
The reconciler checks detailed_merge_status == "mergeable" and calls
mr.merge() directly — no GitLab auto-merge state to track, no
skip-lists to maintain, no stale SHA detection. The mr.state == "opened"
guard prevents merge calls on already-merged MRs. Pipeline completion
and approval webhooks trigger reconcile via the needs_reconcile
dimension, keeping merge latency to seconds. Expensive operations
(empty diff via repository_compare) only run when needs_empty_check
is actionable (periodic via pending_until, DDR-48). The check_empty_diff executor handles the
expensive operation separately; the decider’s priority order runs it
before reconcile_mr so it doesn’t sit behind repeated reconcile runs
from webhooks. The reconciler is idempotent — all corrections are
conditional on actual vs desired state.
DDR-47: CI job retry herder
Updated by DDR-48. The
needs_ci_retry="pending_tick_5m"status andtick_5mpromotion mechanism have been replaced bypending_untilon the dimension row. Non-zero delays now setstatus="true"withpending_untilset to the earliest per-jobnot_beforetimestamp. Thepipelinedimension has been replaced byhead_pipeline_idonmr_details.
Problem: Hummingbird CI pipelines frequently fail due to transient infrastructure errors (runner system failures, git clone timeouts, OOM kills). Manual retries waste developer time and delay merges.
Options considered:
- A. Executor action in workqueue-service (selected): Add a
retry_ci_jobsexecutor triggered when thepipelinedimension goes to"failed". Shares existing GitLab tokens, project config, DB-persisted state, and audit trail. - B. Standalone herder service: Maximum CKI fidelity but requires a new deployment, duplicates token management, and has no MR lifecycle integration.
- C. Reconciliation polling: No new webhooks, but 30-minute detection latency and wastes API calls polling healthy pipelines.
Decision: Option A. The matcher engine is ported from the CKI
pipeline-herder (cki_tools/pipeline_herder/utils.py) with a reduced
field set (dropped web_url, variables, builtin, file_name,
action, exemplars, slack_webhook). The recursive matcher tree,
/regex/ and literal matching, tail_lines windowing, and
short-circuit filter chain are preserved verbatim.
The executor uses CachedPipeline/CachedJob wrappers for efficient
API usage: the job list is fetched once and reused for every job’s retry
safety check, and job traces are only downloaded if earlier checks
(failure_reason, job_name) pass. Non-zero delays are tracked per-job via
a not_before timestamp computed from build_finished_at + retry_delays[retries_count] (both from the build webhook payload), so
jobs with different delay durations are retried as soon as their
individual delay elapses. The dimension’s pending_until is set to the
earliest per-job not_before; the sweep notifies the item when any
job’s delay has passed.
Build (job) events are received via the existing SNS→SQS pipeline (one
line added to the SQS filter policy, job_events: true in the GitLab
webhook config). Each failed job triggers its own event, matching the
CKI pipeline-herder’s per-job model. The build event handler accumulates
failed jobs in the needs_ci_retry dimension’s jobs list; the executor
processes all jobs in the list when triggered. Build events are resolved
to MR work items by matching pipeline_id against
head_pipeline_id on mr_details (set by MR webhooks and
reconciliation).
DDR-48: Uniform needs_* dimension model with pending_until
Decision: Unify all dimensions under a single needs_* trigger
model with "true"/"false" status and an optional pending_until
column for deferred re-evaluation. Drop synthetic tick events.
Context: The dimension system had grown three distinct categories
(DDR-45): has_* positive completion markers, needs_* action
triggers, and observed state dimensions (approval, pipeline,
merge_status, has_conflicts). The has_* and needs_* split created
asymmetric decider logic — some dimensions used != "true" checks,
others used == "true". Observed state dimensions (approval,
merge_status, has_conflicts) were written by the reconciler but never
read by the decider; they added rows without driving actions. The
tick_5m/tick_30m synthetic event mechanism (DDR-42) required hash
bucketing, NOT EXISTS guards, and special "pending_tick_5m" status
values to handle deferred re-evaluation.
Changes:
| Old | New | Notes |
|---|---|---|
has_mr |
needs_mr |
"false" = MR exists |
has_dashboard_link + dashboard_link |
needs_dashboard |
"false" = note posted, detail has note/url |
needs_reconcile |
needs_reconcile |
Gains pending_until (30 min cycle) |
needs_empty_check |
needs_empty_check |
Gains pending_until (30 min cycle) |
needs_ci_retry with "pending_tick_5m" |
needs_ci_retry |
pending_until replaces tick promotion |
approval |
Dropped | Nothing reads it |
pipeline |
head_pipeline_id on mr_details |
Typed integer column |
merge_status |
Dropped | Nothing reads it |
has_conflicts |
Dropped | Nothing reads it |
Alternatives: (a) Keep the three-category model and add
pending_until alongside tick events — increases complexity with two
deferral mechanisms. (b) Unify to needs_* with pending_until.
Rationale: Option (b).
- Uniform semantics: every dimension is a
needs_*trigger. Status is"true"(action needed) or"false"(handled). The decider uses a single check pattern:!= "false"means actionable. - Absent = actionable: missing dimension rows are treated as needing action, so new items don’t require explicit initialization of every dimension.
pending_untilreplaces ticks: executors setpending_untilto schedule future re-evaluation. The sweep finds expiredpending_untiland notifies — no synthetic events, no hash bucketing, no"pending_tick_5m"status values.- Dropped dimensions:
approval,merge_status,has_conflictswere write-only — the reconciler set them but no code read them. Thepipelinedimension carried a single integer and is better modeled as a typed column onmr_details. - Deferred dimensions: the decider pre-filters dimensions where
pending_untilis in the future, treating them as"false". This is equivalent to the old tick promotion but without intermediate status values.
DDR-49: Replace pending_action TEXT with actionable BOOLEAN; centralize decider
Decision: Drop the pending_action TEXT column. Replace it with
actionable BOOLEAN DEFAULT FALSE. The decider is called in exactly one
place: the processing loop’s derive-and-dispatch step.
Context: pending_action served three roles: (1) sweep
discoverability (WHERE pending_action IS NOT NULL), (2) processing loop
continue/exit signal, (3) executor dispatch key. Roles 2 and 3 made the
column authoritative for which action to run, creating a sync burden
with the dimension-driven decider. The column was also written by the
CRUD layer (upsert, retry API) and the event handler — all of which called
decider.decide() to compute a value that the processing loop would
re-derive anyway.
Alternatives:
- Remove entirely — rejected: the decider treats absent dimensions as
actionable (
dims.get("needs_X") != "false"is True when the row does not exist), so a SQL query over dimension rows cannot find items whose next action derives from a missing row. A wake-up signal persisted to the DB is required. next_action TEXT(renamed cache) — rejected: requiresdecider.decide()calls in the CRUD layer to populate the value. Keeps CRUD dependent on the decider. The authoritative-looking name invites future dispatch reads.actionable BOOLEAN— selected.
Rationale: A boolean is sufficient to bridge the absent-dimension
discoverability gap. Stale True is harmless (false-positive sweep →
loop starts, decider returns None, exits immediately).
With the dispatch key removed from the column, the processing loop must
call decider.decide() for dispatch — which it already did via the exit
check. The post-executor call and the event-handler call are now redundant
and removed. The retry API no longer calls the decider either; it just
sets actionable = True.
actionable write invariant: actionable is written in exactly two
places:
True: whenever work is enqueued for the processing loop — on item creation, content update, retry, and event insertion (insert_pending_event). Never written inside the processing loop itself.False: the loop’s step 2 exit branch only — committed atomically with the decision to return, when bothdecider.decide()returnsNoneand no pending events exist.
This invariant closes a crash-safety window: if actionable were cleared
mid-loop (e.g. when the decider transiently returns None while pending
events still exist), a crash between that commit and the next loop
iteration would leave the item with actionable = False and no pending
event row — invisible to the sweep and permanently stuck. By writing
False only on exit, actionable remains True throughout the loop and
the item stays sweep-visible for the entire duration of processing.
The same applies to the event-driven entry path: insert_pending_event
sets actionable = True in the same transaction as the event row, so if
the service crashes after the loop deletes the event but before it exits
cleanly, the item is still sweep-visible.
Failure paths (_fail_max_attempts, _fail_unknown_action, executor
failure) commit the phase/error change and allow the loop to continue to
step 2, which derives actionable = False naturally (decider returns
None because phase != "active") and exits.
Upsert/retry needs_mr seeding: the updated and retried paths must
explicitly reset needs_mr: "true" because the previous ensure_mr
executor set it to "false". With pending_action removed, the decider
is the only dispatcher — it must see needs_mr as not-false to return
ensure_mr. New items do not need seeding (absent dimension is treated
as actionable).
Commit split: this change and the create_mr/update_mr →
ensure_mr consolidation (DDR-5 update, DDR-40) are in separate commits.
The schema migration (actionable column, index rebuild) ships in a
follow-up commit as three Alembic revisions: add column, backfill data,
drop old column.
4. Future Work
Event-driven dimension updates
The dimension mechanism is implemented with all MR dimensions following
the uniform needs_* model (DDR-48). MR state events drive phase
transitions (merge -> completed, close -> cancelled). Future
dimensions: Konflux PipelineRun events updating
needs_konflux_build/needs_konflux_release. Each follows the same
EventResult.dimensions pattern.
Non-managed MR tracking
Design how the service handles MRs it did not create (MRs without the
managed-by::workqueue label). Requires subscribing to note events
so the service can discover dashboard notes posted by the Lambda and
adopt them as needs_dashboard dimensions. Prerequisite for retiring
the Lambda.
Status rendering
An executor that edits the dashboard note (using the note_id from
the needs_dashboard dimension detail) with aggregated dimension
status (CI, Konflux).
CVE work item type
Add cve/ subpackage with CveWorkItem, CVE-specific decider
(analyze → label → create advisory MR → check VEX → close), and
CVE-specific executors. No changes to the generic layer.
Content validation and size limits
Add content-level inspection to the pre-commit checks: maximum file size, binary detection, and content pattern matching. Currently only path blocklists and action validation are enforced.
Reconciliation sweep
Note: The reconcile_mr executor (DDR-46) now provides per-item
reconciliation. A full sweep (querying GitLab for all active items in
bulk) remains a future optimization for catching items that fall
through individual reconciliation.
Service rename
The service was renamed from hummingbird-mr-service to
workqueue-service to reflect its generalized purpose. A further rename
to hummingbird-work-queue or similar is deferred until a second work
item type is implemented.