# Hummingbird Agent 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 the Hummingbird Agent — every major architectural choice, the alternatives considered, why the chosen approach won, and what would break if reversed. For the full design context, see [Hummingbird Agent Design](https://hummingbird-project.io/docs/background/tools/hummingbird-agent-design/). Provider adapter decisions are in [Hummingbird AI Design](https://hummingbird-project.io/docs/background/tools/hummingbird-ai/). ## Design Decisions ### kubectl exec over kubernetes Python exec API - **Chosen:** `kubectl exec` as subprocess for sandbox command execution. - **Alternative:** kubernetes Python client `stream()` API. - **Why:** Three showstopper bugs in the Python client: (1) WebSocket v1-v4 has no stdin EOF signal, so `cat > /file` hangs forever; (2) `BrokenPipeError` on stdin larger than ~1MB; (3) `stream()` accumulates all stdout in memory with no control. `kubectl` avoids all three and provides the same subprocess interface as Podman. - **If reversed:** `write_file()` would hang or fail on large data. Pod log retrieval with large outputs would OOM. Sandbox reliability would drop significantly. ### YAML config over env vars for operational settings - **Chosen:** Single YAML file for workflows, projects, limits, data source declarations. - **Alternative:** Everything in env vars (original design). - **Why:** Env vars cannot express structured data (workflow-project mappings, per-project token overrides, data source config with multiple fields). YAML provides structure, validation, and audit trails. - **If reversed:** Token scoping would be lost (no per-workflow-project token overrides). Project allowlists would be impossible. The config would be unauditable. ### Token env var names in YAML, not values - **Chosen:** YAML contains `token_env: GITLAB_TOKEN_RO` (the env var name), not the actual token value. - **Alternative:** Inline secrets in YAML, or env vars for everything. - **Why:** The YAML file can be committed, reviewed, and audited. Actual secrets stay in env vars (injected via K8s Secrets). Inline secrets would make the config file a secret itself. - **If reversed:** The config file would become a secret, breaking audit trails and code review workflows. ### Orchestrator token prefix convention (ORCHESTRATOR_*) - **Chosen:** Orchestrator tokens use `ORCHESTRATOR_GITLAB_TOKEN_*` env var names by convention. - **Alternative:** Same env var namespace as model tokens, distinguished by context. - **Why:** Structural separation makes it impossible to accidentally pass an orchestrator token to a model tool (or vice versa). The `ORCHESTRATOR_` prefix is never used in YAML `token_env` fields. - **If reversed:** A misconfiguration could leak write-capable tokens to the model, which could then expose them via tool calls. ### Auto-spill over conversation compaction - **Chosen:** Large outputs are saved to sandbox files with previews returned to the model. - **Alternative:** Replace old tool results in `contents` with compact summaries (conversation compaction). - **Why:** Compaction requires modifying the `contents` list, which risks violating provider conversation constraints. Auto-spill achieves ~90% of the token reduction without modifying model history. See [Hummingbird AI](https://hummingbird-project.io/docs/background/tools/hummingbird-ai/) for adapter format requirements. - **If reversed:** Token usage would increase ~15-30%. Per-call context would grow unbounded. Sessions would hit the context limit much sooner. Conversation compaction could be added _on top of_ auto-spill in the future, but is not needed with current workloads. ### Iteration count over cumulative token budget - **Chosen:** `max_iterations` as the primary cost control lever. - **Alternative:** Cumulative token budget (stop when total billed tokens exceed a threshold). - **Why:** With full history replay, each API call re-sends everything. Cumulative billed tokens double-count: call 1 = 5K, call 2 = 10K (including 5K again), total = 15K billed but only 10K new content. With auto-spill keeping per-call size bounded, iteration count is a much simpler and more predictable proxy for actual cost. - **If reversed:** The budget model would be confusing and inaccurate. Cost estimates would be wrong. The cumulative metric is still logged for observability, but it does not drive termination. ### ToolDef notes in system prompt, not tool schema - **Chosen:** Domain knowledge (Konflux dual-fetch, TF XML structure) goes in `ToolDef.notes`, injected into the system prompt. - **Alternative:** Put everything in the tool schema `description`. - **Why:** Tool schemas have character limits and are sent in the `tools` field of every API call. Long descriptions waste tokens on tool defs. System prompt notes are sent once and can be arbitrarily detailed. - **If reversed:** Tool descriptions would be bloated. Domain knowledge would need to be duplicated in every workflow `.md` file. ### Full history replay (no compaction) - **Chosen:** The `contents` list grows monotonically. Items are never removed or modified (except empty response retries). - **Alternative:** Compact old turns to reduce context size. - **Why:** Provider APIs impose strict conversation structure. Modifying or removing items risks invalid history. Auto-spill handles the growth problem at the source (preventing large items from entering history). - **If reversed:** Risk of provider API errors from malformed conversation structure and of confusing the model with summarized results. ### fetch_to_sandbox kept alongside auto-spill - **Chosen:** Both `fetch_to_sandbox` and auto-spill coexist. - **Alternative:** Remove `fetch_to_sandbox` since auto-spill handles large outputs automatically. - **Why:** `fetch_to_sandbox` provides explicit path control (model can choose meaningful filenames), batch fetching (one tool call for multiple fetches), and no-preview responses (useful when the model will process with jq anyway). - **If reversed:** The model would lose path control and batch fetching would require multiple auto-spilled calls. The workflow would need more iterations to achieve the same result. ### K8s library for lifecycle + kubectl for exec (hybrid) - **Chosen:** Use the `kubernetes` Python library for pod create/read/delete, `kubectl` subprocess for exec. - **Alternative:** All-kubectl (subprocess for everything) or all-library. - **Why:** The library provides typed pod status polling and clean error handling for lifecycle. `kubectl` provides reliable exec (see "kubectl exec over kubernetes Python exec API" above). Using the library for exec would require solving the three WebSocket bugs. - **If reversed:** Either lifecycle management would be fragile (parsing kubectl JSON output for pod status) or exec would be unreliable. ### Single-file YAML config over multiple config files - **Chosen:** Everything in one `config.yaml` with `settings` and `workflows` sections. - **Alternative:** Separate files per workflow, or settings.yaml + workflows.yaml. - **Why:** Single source of truth. All project-workflow mappings visible in one place. Settings-level defaults flow down to all projects. No file discovery logic needed. - **If reversed:** Token resolution would need cross-file lookups. Project index would need multi-file aggregation. Config validation would be more complex. ### Workflow .md as pure prompt, metadata in YAML - **Chosen:** The workflow `.md` file is pure system prompt text. Metadata (`action`, `model`, `max_iterations`, `data_sources`, `projects`) lives in the YAML config. - **Alternative:** YAML frontmatter in the `.md` file (original design). - **Why:** Separation of concerns. The `.md` file is the model's instructions -- it should be readable and editable by anyone writing prompts. The YAML config is the orchestrator's instructions -- it controls routing, limits, and credentials. Mixing them in one file conflates two audiences. - **If reversed:** Prompt authors would need to understand YAML config structure. Token/project config would be scattered across `.md` files instead of centralized. ### Nudge via system prompt suffix, not user message - **Chosen:** Empty response nudge and budget warnings are appended to the system prompt as suffixes. - **Alternative:** Inject synthetic user messages into `contents`. - **Why:** User messages in `contents` must come from the actual user (the initial event, or a reply). Injecting synthetic messages pollutes the conversation history that is saved in sessions. System prompt suffixes are transient -- they affect one API call without permanently modifying the conversation state. - **If reversed:** Saved sessions would contain synthetic user messages. Resumed conversations would be confusing. The model might respond to the synthetic messages instead of the user's actual input. ### Session ID reuse over new-ID-per-resume - **Chosen:** Resumed sessions keep the original session ID. S3 state is overwritten in place. - **Alternative:** Each resume generates a new UUID, saving alongside the original (append-only tree of sessions). - **Why:** GitLab discussions are linear, not branching. There is no scenario where two different sessions from the same thread are both valid. A new UUID per resume creates orphaned S3 snapshots that are never referenced again, since `find_session_for_reply()` always picks the latest marker. Reusing the ID is simpler, uses less storage, and matches the linear conversation model. - **If reversed:** S3 would accumulate orphaned session snapshots. Each resume would need a new note marker, but the old markers would still be in the thread, creating confusion about which session is current. ### Reply to unauthorized agent-directed notes instead of silent skip - **Chosen:** When an unauthorized user sends a slash command or replies to an agent thread, the agent replies in the same discussion with a short access-denied message. Non-agent-directed notes are silently ignored (debug log, no access check). - **Alternative:** Log a warning and skip silently for all unauthorized notes (the original behavior). - **Why:** Silent skip gives no feedback to someone who intentionally tried to engage the agent, which is confusing. On the other hand, checking access and logging a warning for every random comment on a public project is noisy and pointless. Moving the access check after intent detection cleanly separates the two cases. The reply uses the discussions API so it automatically inherits confidentiality from the parent note/thread. - **If reversed:** Users without sufficient access who try `/hummingbird` would get no feedback. The agent log would be noisy with warnings for every comment on public MRs. ### Deployment-backed pod pool over Python-thread pool - **Chosen:** A Kubernetes Deployment maintains pre-warmed standby pods. The agent claims a pod by relabeling it; the Deployment replaces it. - **Alternative:** A Python-side thread pool that pre-creates pods and queues them for use. - **Why:** The Deployment provides self-healing (restart crashed pods), native scaling (`kubectl scale`), rolling updates (image changes), and monitoring via standard K8s tooling. A Python pool would need to reimplement all of these. - **If reversed:** Pod replenishment, crash recovery, and image updates would all need custom code. Scaling would require agent redeployment. ### Pod isolation: never reuse sandbox pods - **Chosen:** Each workflow run gets its own pod. After cleanup, the pod is deleted. Claimed pods are detached from the ReplicaSet. - **Alternative:** Return used pods to the pool and reset them. - **Why:** Residual state from a previous run (files, environment, running processes) could leak between MR investigations, creating security and correctness risks. Deletion is simple and foolproof. - **If reversed:** Would need a reliable pod-reset mechanism and auditing that no state survives between runs. ### Absolute reap-by deadline over relative claimed-at age - **Chosen:** Active pods are annotated with `hummingbird/reap-by` (an absolute UTC timestamp). The reaper deletes pods where `now > reap-by`. - **Alternative:** Annotate with `claimed-at` and compute age relative to `max_active_seconds`; or use `activeDeadlineSeconds` on the pod spec. - **Why:** An absolute deadline simplifies the reaper to a single comparison. It also supports varying deadlines: claim sets `reap-by = now + max_active_seconds`, while linger sets `reap-by = now + linger_seconds`. With a relative timestamp, the reaper would need to know which mode the pod is in. `activeDeadlineSeconds` applies from pod creation, not from claim -- standby pods would expire before being used. - **If reversed:** The reaper would need mode-aware age calculations. Lingering pods would require a separate annotation or reaper path. ### Indefinite claim wait over timeout - **Chosen:** `SandboxPool.claim()` polls indefinitely (governed by shutdown event), logging at DEBUG then WARNING. - **Alternative:** Timeout after N seconds and raise an error. - **Why:** The pool is typically smaller than `max_concurrent_agents` for cost reasons. Waiting for a replacement pod is normal operational behavior, not an error. A timeout would cause spurious failures during burst traffic. The SQS semaphore already bounds concurrency. - **If reversed:** Burst traffic would cause avoidable failures instead of brief delays. ### Pool config in Deployment only, not in agent configmap - **Chosen:** In pool mode (`k8spool`), the pod image, resources, labels, and security context are defined solely in the Deployment template. The agent config only needs `namespace` and `active_deadline_seconds`. - **Alternative:** Keep image/resources/metadata in the agent configmap too (as in `k8s` mode). - **Why:** Eliminates config duplication. The Deployment template is the single source of truth. Changes to pod resources or image only require updating one place and re-rolling the Deployment. - **If reversed:** Config drift between the Deployment and the agent configmap would be a constant risk. ### Pod lingering over immediate cleanup for reply latency - **Chosen:** After a successful workflow, pool sandbox pods linger for `linger_seconds` (default 300) with a `session-id` annotation. Replies within the window reclaim the pod via `try_reclaim()`, skipping pod creation and S3 archive restoration. - **Alternative:** Always delete the pod immediately and restore from S3 on every reply. - **Why:** Reply latency drops from seconds (pod claim + S3 restore) to near-zero. The S3 archive is still saved as a fallback if the pod is gone. The `reap-by` annotation ensures lingering pods are cleaned up if no reply arrives. The `session-id` annotation doubles as a concurrency guard: it is only present while lingering, preventing active pods from being reclaimed. - **If reversed:** Every reply would pay full pod + restore latency, even for immediate follow-ups. User experience for conversational interactions would degrade noticeably. ### linger() on Sandbox protocol over isinstance checks in runner - **Chosen:** All sandbox backends implement `linger(session_id)`. Non-pool backends fall back to `cleanup()`. The runner calls `sb.linger()` without type checks. - **Alternative:** `isinstance(sb, K8sPoolSandbox)` in `run_workflow()`. - **Why:** Keeps the runner backend-agnostic. Adding a new backend requires only implementing the protocol, not touching the runner. The fallback behavior is co-located with each backend. - **If reversed:** The runner would need to know about every backend type and their linger capabilities. ### GCP SA key over Workload Identity Federation - **Chosen:** GCP service account key stored in Vault, rotated via cki-tools credential manager (prepare/switch/clean cycle). - **Alternative:** Workload Identity Federation (WIF) with projected SA token. Two sub-options: (a) automatic OIDC discovery if the cluster issuer is public, (b) manual JWKS upload for internal issuers. - **Why:** mpp-prod's OIDC issuer is `https://kubernetes.default.svc` (internal, not publicly reachable), so GCP STS cannot discover it automatically. Manual JWKS upload works but requires re-upload after SRE-triggered signing key rotations. SA key integrates with the existing credential manager rotation infrastructure (same pattern as AWS keys and GitLab tokens), needs no OIDC reachability, and enables automated validate/update via `google-auth` in CI. The application code (`hummingbird_ai.http.VertexAuth` / `google.auth.default()`) works identically with both approaches -- switching to WIF later requires only infrastructure changes. - **If reversed:** Replace SA key with WIF projected token + credential config ConfigMap. The `gcp_service_account_key` token type in cki-tools would no longer be needed for this use case. ### Developer+ trust filtering over unfiltered or sanitized discussion comments - **Chosen:** `gitlab_get_mr_discussions` checks each note author's project access level (Developer+ / >= 30). Untrusted notes are replaced with a fixed placeholder in mixed-trust discussions, or the entire discussion is dropped if all notes are untrusted. Agent notes are always trusted (detected by session marker) but have transcripts and metadata stripped. - **Alternatives considered:** - (a) Include all comments unfiltered. Simplest, but any external contributor can craft comments that manipulate the model's review output (prompt injection). - (b) Sanitize/escape untrusted content (strip markdown, quote as code blocks, prefix with "[external]"). There is no reliable escaping mechanism for LLM prompts -- the model interprets natural language regardless of formatting. Escaping gives a false sense of security. - (c) Only include agent-authored notes (skip all human comments). Safe, but defeats the purpose: the model would never see developer responses to its own findings. - (d) Include only discussions where the agent participated. Better, but still misses developer-initiated review threads that provide relevant context. - **Why:** Developer+ is the same threshold used for pipeline trigger authorization (invariant #8) and slash command access. It matches the trust boundary already established: people who can push code and approve MRs are trusted to provide review context. The placeholder approach preserves discussion structure (the model sees that _someone_ replied) without exposing the content. Full-drop for all-untrusted discussions avoids noise from discussions that contain zero useful context. - **If reversed:** (a) opens a prompt injection vector on any project that accepts external MRs. (b) provides no actual protection. (c) makes follow-up reviews unable to see developer explanations, causing repeated false positives. (d) misses developer-initiated context. ### Bot-author filtering for session marker parsing over unfiltered note scanning - **Chosen:** `find_session_for_reply()`, `scan_agent_threads()`, and the `handle_note()` self-filter all resolve the bot user ID via `get_bot_user_id()` (which calls `gl.auth()` on the orchestrator token) and only consider notes where `author.id` matches. In `find_session_for_reply()`, the first matching marker wins. - **Alternative:** Parse markers from any note and keep the last match; substring self-filter in `handle_note` (previous behavior). - **Why:** The previous unfiltered approach allowed session hijacking: a user reply or agent review prose containing an example marker (``) was parsed as a real session, causing "session expired" errors in production. For `handle_note`, the substring check caused the reverse problem: a user embedding the marker prefix in a reply silently suppressed the event (denial of service). The `gl.auth()` call is one `GET /user` per invocation -- negligible cost. `_get_client_with_bot_id()` returns both the authenticated client and the bot user ID, avoiding redundant client construction. - **If reversed:** Any MR participant can inject a marker to hijack the session ID or inflate rate-limit counts. The agent's own review prose containing example markers causes spurious "session expired" messages. ### ijson streaming over buffered JSON for K8s list responses - **Chosen:** `session.get(stream=True)` + `ijson.parse(resp.raw)` for `iter_paginated()`. Items are yielded one at a time via `ObjectBuilder`; the `metadata.continue` pagination token is captured in the same parse pass. `resp.raw.decode_content = True` is required because Kubearchive returns gzip Content-Encoding; without it `ijson` sees compressed bytes. - **Alternative:** `session.get().json()` loads the full page into memory (the original implementation). - **Why:** A single K8s list page can contain 500 PipelineRuns (80+ MB JSON). Parsing this with `.json()` creates a +247 MB RSS spike (raw bytes + decoded string + parsed dict coexist). With 4 concurrent workflows, this exceeds any reasonable pod memory limit. `ijson` stream-parsing yields items one at a time, reducing the peak to +12 MB for the same data -- a 95% reduction. The continue token appears in the `metadata` object (before or after `items` depending on the server); the event-driven parser captures it regardless of order. - **If reversed:** Large MRs (100+ components) would OOM the agent pod. Concurrent workflows would multiply the problem. The pod memory limit would need to scale with the largest possible page size, which is unbounded. ### Action token tier for per-workflow identity - **Chosen:** A dedicated Tier 2 of workflow action tokens, separate from both orchestrator tokens (Tier 1) and model tool tokens (Tier 3). Each workflow gets a dedicated GitLab bot user per project for all write operations (notes, thread resolution). - **Alternative:** Extend orchestrator tokens with a per-workflow fallback chain (`ORCHESTRATOR__GITLAB_TOKEN_` → `ORCHESTRATOR_GITLAB_TOKEN_` → `ORCHESTRATOR_GITLAB_TOKEN`). - **Why:** Orchestrator tokens serve a fundamentally different purpose (operational reads + infrastructure notes) than workflow output (analysis results, review comments). Mixing them in one fallback chain risks a single bot identity posting notes on behalf of multiple workflows, eliminating the audit trail. A separate tier with explicit `token: str` parameters makes the code path unambiguous: callers must choose which tier they use. The naming convention (`HUMMINGBIRD_AGENT_ACTION__GITLAB_TOKEN_`) groups tokens alphabetically by workflow, making env var auditing straightforward. - **If reversed:** All workflow notes would appear from the same bot user, making it impossible to distinguish which agent (code-review vs analyze-failures vs renovate-babysit) produced a given note in the MR timeline. GitLab's audit log would attribute all actions to one identity. ### Per-project-only action token config (no workflow-level default) - **Chosen:** `action_tokens` is defined at the project level only in YAML config. No workflow-level `action_tokens` default is supported. - **Alternative:** Allow workflow-level `action_tokens` that projects inherit by default, with per-project overrides. - **Why:** GitLab project access tokens are project-scoped. A workflow-level default would be misleading -- a single token cannot write to multiple projects. Forcing per-project declaration makes the mapping explicit and prevents configuration errors where a cross-project default silently fails. - **If reversed:** Operators could set a workflow-level token that only works for one project, causing silent 403 errors for other projects in the same workflow. ### SSH over kubectl exec for KubeVirt VMs - **Chosen:** SSH (`ssh -T sandbox@ip command`) for VM command execution. - **Alternative:** `virtctl console` (serial console), KubeVirt websocket API, or `qemu-guest-agent exec`. - **Why:** VMs have no `kubectl exec` equivalent. `virtctl console` provides a serial terminal, not a programmable exec channel. The websocket API requires a VNC or serial connection. `qemu-guest-agent` is not installed in the VM image. SSH provides clean stdin/stdout/stderr separation, timeout control, and is already well-tested in subprocess pipelines. - **If reversed:** Would need `qemu-guest-agent` in the image and the `exec` endpoint, which has poorer error reporting and no streaming stdin. ### Secret volume for SSH key injection - **Chosen:** Kubernetes Secret mounted as a virtio disk (serial `ssh-pubkeys`) read by the VM's `inject-ssh-keys.service` at boot. - **Alternative:** `cloudInitNoCloud` or `accessCredentials` with `qemu-guest-agent`. - **Why:** The VM image already has `inject-ssh-keys.service` which reads from the virtio disk. `cloudInitNoCloud` requires cloud-init in the image. `accessCredentials` requires `qemu-guest-agent`. The Secret volume approach requires no additional software in the VM image. - **If reversed:** Would need cloud-init or guest agent in the bootc image, adding complexity and attack surface. ### VMIRS for VM pool replenishment - **Chosen:** `VirtualMachineInstanceReplicaSet` (VMIRS) as the replenishment controller for the VM pool. - **Alternative:** Custom controller, or VirtualMachine with RunStrategy. - **Why:** VMIRS is KubeVirt's native equivalent of a Deployment's ReplicaSet -- it maintains a desired count of identical VMIs. When a VMI is claimed (ownerReferences cleared), the VMIRS creates a replacement automatically. Same pattern as the pod pool with Deployment. - **If reversed:** Would need a custom controller to maintain the VMI pool, adding operational complexity. ### Per-workflow sandbox backend - **Chosen:** `sandbox:` field in workflow config, resolved per-workflow with fallback to CLI `--sandbox` flag. - **Alternative:** Global-only sandbox selection via CLI. - **Why:** Different workflows have different isolation requirements. Code review needs only a lightweight container, while failure analysis may need a full VM with root access. Per-workflow selection avoids running all workflows in VMs (wasteful) or all in containers (insufficient isolation). - **If reversed:** All workflows would share the same sandbox backend, requiring either over-provisioning (VMs for everything) or accepting weaker isolation for some workflows.