# Work Queue Service Design Decisions

LLMS index: [llms.txt](https://hummingbird-project.io/llms.txt) | Full content: [llms-full.txt](https://hummingbird-project.io/llms-full.txt)

---

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](https://hummingbird-project.io/docs/background/tools/workqueue-service-design/). For operational
usage, see [Work Queue Service](https://hummingbird-project.io/docs/background/tools/workqueue-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.

## DDR-27: OAuth proxy as login-only bridge with /me endpoint and cookie refresh

**Decision**: The oauth-proxy sidecar handles only the OAuth login flow.
API endpoints bypass the proxy. A proxy-protected `/me` endpoint
bridges cookie sessions to Bearer tokens. The proxy must be configured
with `--cookie-refresh=1h` to prevent stale-token failures.

**Context**: The OCP oauth-proxy cannot simultaneously forward
cookie-session tokens AND pass through non-OCP Bearer tokens on the
same path. Protected paths validate Bearer tokens via TokenReview
(requires cluster RBAC we cannot obtain) or reject non-OCP tokens.
Bypassed paths pass all Bearer tokens through but ignore cookies.
Additionally, the OCP access token embedded in the proxy's session
cookie has its own expiry (default 24h), independent of the cookie
lifetime. Without `--cookie-refresh`, the cookie outlives the token,
causing the proxy to forward expired credentials while still reporting
the user as authenticated — the "logged in but not authorized" failure
observed in the hummingbird-dashboard deployment.

**Alternatives**: (a) All paths protected — blocks non-OCP Bearer
tokens. (b) `--openshift-delegate-urls` — requires
`system:auth-delegator` ClusterRoleBinding. (c) App reads proxy cookies
directly — tight coupling to proxy internals. (d) Login-only proxy
with `/me` bridge.

**Rationale**: Option (d). The `/me` endpoint is the only
proxy-protected API path. It reads `X-Forwarded-Access-Token` (set by
the proxy from the cookie session) and returns the OCP access token to
the caller. Subsequent API calls use `Authorization: Bearer sha256~...`
on bypassed paths, where the app validates via the OCP `users/~` API.
`--cookie-refresh=1h` keeps the embedded token fresh. TTL-bounded
caching (5 minutes) on `users/~` results ensures expired-token failures
are never permanent (unlike `@functools.cache` which caches failures
indefinitely).

## DDR-28: Audit log with logging + optional S3 archival

**Decision**: Always log audit entries via Python `logging` at INFO
level, and optionally archive to S3 as gzipped JSON objects when
`WORKQUEUE_SERVICE_AUDIT_S3_BUCKET` is set.

**Context**: Needed an immutable record of state transitions, action
outcomes, and auth decisions for compliance and debugging.

**Alternatives**: (a) Database table — bloats the operational DB with
a different concern. (b) S3-only — no visibility without querying
Athena; loses events if S3 is unreachable.

**Why logging + S3**: Logging to stdout is zero-config and integrates
with existing container log collection for real-time visibility. S3
provides the durable, queryable archive (Athena) for compliance.

**Key design rules**:

- Audit failures never propagate — `write_audit_entry()` catches all
  exceptions and logs them, but never raises.
- S3 keys use `YYYY/MM/DD/HH/MM/` prefix for Athena partitioning.
- `ContentEncoding: gzip` set so S3 API consumers get transparent
  decompression.

**Field conventions**: See [Audit Log Field Conventions](https://hummingbird-project.io/docs/background/tools/workqueue-service/#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.