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.

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: gzip set 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. Include phase and action when 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 as old_state. Omit fields that are None (they are filtered by to_dict()).
  • trigger: what caused this event. Always include source (one of "api", "worker", "decider"). Add action for deliberate operations, reason for error/system transitions, error for failure details.
  • metadata: context orthogonal to the state transition — request info on auth events, type-specific data on submit. Not for duplicating state already in old_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, reset needs_mr = "true"
  • Failed → reset to active, update content, clear error ("retried"), set actionable = 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_30m and tick_5m synthetic event mechanism has been replaced by the pending_until column on work_item_statuses. The sweep now finds items with expired pending_until and 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 and tick_5m promotion mechanism have been replaced by pending_until on the dimension row. Non-zero delays now set status="true" with pending_until set to the earliest per-job not_before timestamp. The pipeline dimension has been replaced by head_pipeline_id on mr_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_jobs executor triggered when the pipeline dimension 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_until replaces ticks: executors set pending_until to schedule future re-evaluation. The sweep finds expired pending_until and notifies — no synthetic events, no hash bucketing, no "pending_tick_5m" status values.
  • Dropped dimensions: approval, merge_status, has_conflicts were write-only — the reconciler set them but no code read them. The pipeline dimension carried a single integer and is better modeled as a typed column on mr_details.
  • Deferred dimensions: the decider pre-filters dimensions where pending_until is 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: requires decider.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 both decider.decide() returns None and 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.