Work Queue Service Design Decisions

Design Decision Registry (DDR) for workqueue-service — every major design choice, the alternatives considered, and why the chosen approach won. For the design overview, see Work Queue Service Design. For operational usage, see Work Queue Service.

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: See Audit Log Field Conventions in the operational documentation.

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)

Concurrent insert races are handled via DDR-39’s partial unique index — the loser catches the IntegrityError, rolls back, and retries.

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. 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 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.

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): 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 with a reduced field set. Build events are received via the existing SNS→SQS pipeline and resolved to MR work items by matching pipeline_id against head_pipeline_id on mr_details.

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. Observed state dimensions were write-only — the reconciler set them but no code read them. The tick_5m/tick_30m synthetic event mechanism (DDR-42) required hash bucketing, NOT EXISTS guards, and special "pending_tick_5m" status values.

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 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".

DDR-49: Replace pending_action TEXT with actionable BOOLEAN

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.

Alternatives:

  • Remove entirely — rejected: the decider treats absent dimensions as actionable, 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. 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). actionable is written in exactly two places: True when work is enqueued (creation, update, retry, event insertion), False only on the loop’s exit branch (committed atomically when both decider.decide() returns None and no pending events exist). This invariant ensures the item stays sweep-visible throughout processing — a crash at any point leaves actionable = True, preventing items from becoming permanently stuck.