# Armada Failure Cases Analysis

> Generated by deep code inspection of Armada's dispatch, scheduling, docking,
> running, merging, and cleanup subsystems. This document identifies every
> plausible failure mode and its root cause, then suggests mitigations.

---

## Table of Contents

1. [Merge Conflicts (The Big One)](#1-merge-conflicts)
2. [Dock / Worktree Failures](#2-dock--worktree-failures)
3. [Captain Lifecycle Failures](#3-captain-lifecycle-failures)
4. [Mission State Machine Failures](#4-mission-state-machine-failures)
5. [Dispatch & Scheduling Failures](#5-dispatch--scheduling-failures)
6. [Merge Queue Failures](#6-merge-queue-failures)
7. [Landing / Post-Completion Failures](#7-landing--post-completion-failures)
8. [Git Operation Failures](#8-git-operation-failures)
9. [Cleanup & Data Hygiene Failures](#9-cleanup--data-hygiene-failures)
10. [Concurrency & Race Conditions](#10-concurrency--race-conditions)
11. [Process Management Failures](#11-process-management-failures)
12. [Recovery & Escalation Failures](#12-recovery--escalation-failures)
13. [Configuration & Settings Failures](#13-configuration--settings-failures)
14. [Database & State Consistency Failures](#14-database--state-consistency-failures)

---

## 1. Merge Conflicts

### FC-1.1: Multiple concurrent missions on the same vessel touch overlapping files

**Root cause:** `MissionService.TryAssignAsync` (line 88-101) only *warns* when
a vessel already has active missions -- it does not *block* them unless one is
detected as "broad scope" by keyword heuristics. Two narrow-scope missions that
happen to edit the same files will both get assigned, both run on separate
worktrees branched from the same base, and both produce conflicting changes.

**Why it hurts:** When either mission lands (LocalMerge, PullRequest, or
MergeQueue), the second one will conflict with the first because it was branched
from a base that did not include the first mission's changes.

**Mitigation:**
- Add file-level locking or a "files touched" manifest per mission.
- Serialize all missions per vessel (only one InProgress at a time).
- Rebase the second mission's branch onto the updated target before landing.

### FC-1.2: Worktrees are all branched from the same base snapshot

**Root cause:** `DockService.ProvisionAsync` (line 179) creates worktrees with
`git worktree add -b <branch> <path> <baseBranch>`. All concurrent missions for
a vessel branch from the same `baseBranch` at fetch time. None of them see each
other's in-flight changes.

**Why it hurts:** Even if missions don't touch the same files, structural
changes (renames, moves, config file edits) in one mission can conflict with
another. The more concurrent missions per vessel, the higher the conflict rate.

**Mitigation:**
- After each successful landing, rebase all remaining in-progress branches
  onto the updated target before the agent continues working.
- Or serialize missions per vessel entirely.

### FC-1.3: Merge queue processes entries against a potentially stale target

**Root cause:** `MergeQueueService.ProcessEntryAsync` (line 387) does a
`git fetch` then creates a temporary worktree from the target branch. If the
remote has been updated by something *outside* Armada (human push, CI, another
tool), the fetch may succeed but the merge may conflict with those external
changes. Conversely, if a previous merge queue entry just landed but the
remote hasn't propagated yet, the next entry merges against a stale target.

**Mitigation:**
- Add a post-land verification that confirms the push succeeded before
  processing the next entry in the group.
- Add retry-with-rebase logic for merge conflicts in the queue.

### FC-1.4: LocalMerge into user's WorkingDirectory with dirty state

**Root cause:** `HandleMissionCompleteAsync` (line 2658) calls
`GitService.MergeBranchLocalAsync` which does `git merge FETCH_HEAD --no-edit`
into the user's working directory. If the working directory has uncommitted
changes or is on a different branch, the merge will fail.

**Mitigation:**
- Check that the working directory is clean and on the correct branch before
  attempting a local merge.
- Stash uncommitted changes, merge, then pop.
- Or refuse the merge and report why it failed.

### FC-1.5: The "broad scope" detection is keyword-based and unreliable

**Root cause:** `MissionService.IsBroadScope` (line 402-430) searches mission
title/description for strings like "refactor entire", "rewrite", etc. Missions
that are actually broad-scope but don't use these keywords will not be detected.
Missions that use these keywords in benign ways will be falsely blocked.

**Mitigation:**
- Allow explicit `broadScope: true/false` flag on mission creation.
- Use file-path analysis (if available) instead of keyword heuristics.

---

## 2. Dock / Worktree Failures

### FC-2.1: Stale worktree directories block provisioning

**Root cause:** `DockService.ProvisionAsync` (line 143-164) tries to clean up
stale worktree directories before creating new ones. If `git worktree remove`
fails (locked files on Windows, permission errors), the leftover directory
prevents `git worktree add` from succeeding on the same path.

**Why it hurts:** The mission fails to provision and stays Pending forever,
or the captain becomes permanently unable to accept work on that vessel.

**Mitigation:**
- The code already has `ForceRemoveDirectoryAsync` with retries (5 attempts
  with 0/500/1000/2000/3000ms delays). But this is insufficient on Windows
  when antivirus or indexing services hold file handles.
- Use longer retry delays or a background cleanup service.
- Detect and log which process holds the lock.

### FC-2.2: Worktree "refusing to fetch" errors cascade

**Root cause:** When a worktree is checked out, `git fetch --all --prune` in
`GitService.FetchAsync` (line 121) can fail with "refusing to fetch into branch
checked out at..." errors. The code catches this and falls back to
`git fetch origin` (line 130), but this fallback may also fail.

**Why it hurts:** If fetch fails, the new worktree branches from stale data.
All subsequent merges become conflicts because the branches are based on an
old version of the target.

**Mitigation:**
- Always prune worktrees *before* fetching (the code does this, but the prune
  itself can fail silently).
- Track worktree registrations in the database and ensure cleanup is
  deterministic.

### FC-2.3: Dock provisioned but captain claim fails -- orphaned worktree

**Root cause:** In `MissionService.TryAssignAsync` (line 162-181), after the
dock is provisioned, `TryClaimAsync` may fail if another concurrent assignment
raced to claim the same captain. The code calls `_Docks.ReclaimAsync` to clean
up the orphaned dock, but if reclaim fails, the worktree leaks.

**Mitigation:**
- Add a periodic background task that scans for dock records with no
  associated active mission and cleans them up.

### FC-2.4: ForceRemoveDirectoryAsync fails silently on final attempt

**Root cause:** `DockService.ForceRemoveDirectoryAsync` (line 326-373) catches
the final exception with just a warning log. The directory persists on disk, but
the dock database record may already be updated to `Active=false`.

**Why it hurts:** Database says the dock is reclaimed; filesystem says it still
exists. Next provisioning attempt for the same captain/vessel path will hit the
stale directory and may fail.

**Mitigation:**
- Track cleanup failures in the database (e.g., a `CleanupFailed` flag on Dock).
- Run a periodic filesystem reconciliation.

### FC-2.5: Read-only files on Windows blocking directory deletion

**Root cause:** Git objects inside `.git` directories are often marked read-only.
`ForceRemoveDirectoryAsync` (line 346-358) clears read-only attributes before
deletion, but this enumeration itself can fail if the directory is very large or
partially locked.

**Mitigation:**
- Use `robocopy /mir` to an empty folder (Windows-specific) or `cmd /c rd /s /q`
  as a nuclear option.
- Or defer deletion to a background worker with exponential backoff.

---

## 3. Captain Lifecycle Failures

### FC-3.1: Captain stuck in "Working" state after process death

**Root cause:** If the server restarts while a captain's agent process is running,
the process dies but the captain record still says `Working`. The
`CleanupStaleCaptainsAsync` (line 313-387) method runs on startup to fix this,
but it relies on `Process.GetProcessById` which may give false positives if the
OS reuses the PID.

**Why it hurts:** If the PID is reused by an unrelated process, the cleanup
thinks the captain is still alive and doesn't reset it. The captain is
permanently stuck.

**Mitigation:**
- Store the process start time in the captain record and compare with the
  actual process start time to detect PID reuse.

### FC-3.2: Captain stuck in "Stalled" state requires manual intervention

**Root cause:** When recovery attempts are exhausted (`RecoveryAttempts >=
MaxRecoveryAttempts`), the captain is set to `Stalled` (line 470 in
AdmiralService). There is no automatic mechanism to reset a stalled captain
back to `Idle` -- it requires a manual `armada_update_captain` or
`armada_stop_captain` call.

**Why it hurts:** Stalled captains reduce capacity. If all captains stall,
the entire system stops processing work.

**Mitigation:**
- Add auto-reset for stalled captains after a configurable cooldown period.
- Or auto-delete stalled captains and let the pool maintenance respawn new ones.

### FC-3.3: Race between process exit handler and health check

**Root cause:** Both `HandleAgentProcessExited` (event-driven, line 2370) and
`HealthCheckCaptainAsync` (periodic polling, line 494) can detect the same
process death. Both will attempt to handle the completion. While there are
some guards (checking terminal mission status), the two paths executing
concurrently can lead to double-handling.

**Why it hurts:** Double `HandleCompletionAsync` calls can cause duplicate
merge attempts, double dock reclamation, or state corruption.

**Mitigation:**
- Use a `ConcurrentDictionary<string, bool>` guard keyed by mission ID to
  ensure only one handler processes a given completion.
- The `_InFlightCompletions` dictionary partially addresses this but only
  tracks the background merge task, not the initial completion itself.

### FC-3.4: Captain released to Idle before background merge completes

**Root cause:** In `MissionService.HandleCompletionAsync` (line 382-383),
the captain is released to Idle *immediately* after the background merge
task is queued via `Task.Run`. The captain can be assigned a new mission
while the previous mission's merge is still in progress.

**Why it hurts:** If the new mission is on the same vessel, the merge from
the old mission and the worktree setup for the new mission can conflict with
each other's git operations.

**Mitigation:**
- The `_VesselMergeLocks` semaphore in ArmadaServer partially addresses this
  for the merge operation itself, but the dock provisioning for the new mission
  may also do git operations (fetch, worktree add) that conflict.
- Consider not releasing the captain until the background merge completes.

---

## 4. Mission State Machine Failures

### FC-4.1: Mission stuck in "WorkProduced" forever

**Root cause:** If `OnMissionComplete` handler throws and the exception is
swallowed (line 339 in MissionService), the mission stays as `WorkProduced`.
No landing is attempted, and there's no automatic retry.

**Why it hurts:** Work was done but never integrated. The branch exists in the
bare repo but nobody knows to look at it.

**Mitigation:**
- Add a background scanner for missions in `WorkProduced` state older than
  a threshold (e.g., 30 minutes) and retry the landing.

### FC-4.2: Mission stuck in "LandingFailed" with no retry path

**Root cause:** When landing fails (merge conflict, push failure, PR creation
failure), the mission transitions to `LandingFailed` (line 2793-2796).
There is no automatic retry mechanism for landing failures.

**Why it hurts:** The work is done and the branch exists, but it requires
manual intervention to merge. This is the most common "semi-failed state" that
users encounter.

**Mitigation:**
- Add `armada_retry_landing` tool/API that re-runs the landing logic.
- Add automatic rebase-and-retry for `LandingFailed` missions.
- At minimum, surface `LandingFailed` missions prominently in status output.

### FC-4.3: Mission stuck in "PullRequestOpen" if PR merge polling times out

**Root cause:** `PollAndPullAfterMergeAsync` (line 2883) polls for PR merge
status 30 times with 10-second intervals (5 minutes total). If the PR isn't
merged within 5 minutes, the polling stops and the mission stays in
`PullRequestOpen` forever.

**Why it hurts:** The mission never transitions to `Complete`. Voyages never
complete because they wait for all missions to reach a terminal state.

**Mitigation:**
- Add the PR polling to the periodic health check loop instead of a one-shot
  poll.
- Or use GitHub webhooks to receive merge notifications.

### FC-4.4: Mission transitions don't validate all state changes

**Root cause:** The `HandleAgentOutput` method (line 2346-2356) checks
`IsValidTransition` for agent-reported status changes, but several other
code paths do direct `mission.Status = ...` assignments without validation.
A buggy agent or race condition could push a mission into an invalid state.

**Mitigation:**
- Centralize all status transitions through a single validated method.
- Log invalid transition attempts as errors, not just warnings.

### FC-4.5: Voyage marked Complete while missions are in LandingFailed or PullRequestOpen

**Root cause:** `VoyageService.CheckCompletionsAsync` (line 52-55) considers
a voyage complete when all missions are `Complete`, `Failed`, or `Cancelled`.
But `LandingFailed`, `WorkProduced`, and `PullRequestOpen` are NOT in this
terminal set. So a voyage with missions in those states will never complete.

**Why it hurts:** The voyage stays "InProgress" indefinitely, confusing users.
Conversely, `WorkProduced` missions that nobody lands will block voyage
completion forever.

**Mitigation:**
- Treat `LandingFailed` and `WorkProduced` as terminal for voyage completion
  purposes, OR add automatic retry logic.
- Add `PullRequestOpen` handling in the health check to periodically re-poll.

---

## 5. Dispatch & Scheduling Failures

### FC-5.1: Pending missions starve if no idle captains exist

**Root cause:** `DispatchPendingMissionsAsync` (line 764-803) checks for idle
captains and skips dispatch if none are available. Pending missions sit in the
queue until a captain becomes idle, but if captains are stuck in `Working` or
`Stalled` states, no missions ever get dispatched.

**Mitigation:**
- Surface starvation warnings when pending missions exceed a threshold.
- Auto-recover stalled captains (see FC-3.2).
- Auto-scale the captain pool (the code supports this via `MinIdleCaptains`).

### FC-5.2: Mission without VesselId is silently ignored

**Root cause:** `DispatchPendingMissionsAsync` (line 789) skips missions with
no `VesselId`. These missions will sit in `Pending` forever with no indication
of why they're not being dispatched.

**Mitigation:**
- Validate `VesselId` at mission creation time.
- Or mark orphaned missions as `Failed` with an error message.

### FC-5.3: Vessel not found during dispatch -- mission stays Pending

**Root cause:** `DispatchPendingMissionsAsync` (line 791-792) silently
`continue`s if the vessel lookup returns null. The mission stays Pending
forever.

**Mitigation:**
- Fail the mission explicitly if the vessel no longer exists.
- Or log a warning and skip with a counter.

### FC-5.4: Dispatch processes missions in arbitrary order

**Root cause:** `DispatchPendingMissionsAsync` iterates all pending missions
but doesn't sort by priority. The separate `HandleCompletionAsync` path
(line 389) does sort by priority, but the health-check dispatch path does not.

**Mitigation:**
- Sort pending missions by `Priority` then `CreatedUtc` in all dispatch paths.

### FC-5.5: Auto-spawned captains have generic names that may collide

**Root cause:** `MaintainCaptainPoolAsync` (line 873) names captains as
`captain-<N>` based on total count. If captains are deleted and recreated,
the numbering can produce names that already exist (e.g., if captain-3 was
deleted and there are now 2 captains, the next auto-spawned one is captain-3
again -- but the worktree path uses the captain name, so it may conflict with
stale worktree directories from the old captain-3).

**Mitigation:**
- Use unique suffixes (UUID or timestamp) in auto-spawned captain names.
- Or use the captain ID prefix in the worktree path instead of the name.

---

## 6. Merge Queue Failures

### FC-6.1: _ProcessLock prevents concurrent queue processing but also blocks re-entry

**Root cause:** `MergeQueueService.ProcessQueueAsync` (line 72-76) uses a
boolean `_Processing` flag with a lock. If called while already processing,
the second call silently returns. This means that newly queued entries won't
be processed until the current batch finishes.

**Why it hurts:** If processing takes a long time (many entries, slow tests),
new entries have to wait until the entire batch is done.

**Mitigation:**
- Use a queue-based approach where new entries can be appended during processing.
- Or immediately schedule another processing pass after the current one finishes.

### FC-6.2: Merge queue worktree cleanup failure leaves orphaned directories

**Root cause:** `CleanupWorktreeAsync` (line 554-564) in MergeQueueService
catches cleanup errors and logs them but doesn't track the failure. The
temporary merge-queue worktree under `_merge-queue/` persists on disk.

**Why it hurts:** Over time, disk fills with orphaned merge-queue worktrees.
These can also block git operations if they have branches checked out.

**Mitigation:**
- Add periodic cleanup of the `_merge-queue/` directory.
- Track merge-queue worktrees in the database.

### FC-6.3: Test command runs with no timeout

**Root cause:** `MergeQueueService.RunTestsAsync` (line 489-519) starts the
test process and waits for it with only the CancellationToken. There's no
explicit timeout, so a hanging test command will block the entire merge queue
group forever.

**Mitigation:**
- Add a configurable test timeout (e.g., 30 minutes) with automatic process kill.

### FC-6.4: Merge uses `--no-ff` but doesn't do a rebase first

**Root cause:** `MergeBranchAsync` (line 473-487) does `git merge --no-ff`
directly. If the branch has diverged significantly from the target, the merge
is more likely to conflict. A rebase-then-fast-forward-merge strategy would
reduce conflicts.

**Mitigation:**
- Offer a rebase-first option for merge queue entries.
- Or at least fetch+rebase the feature branch before attempting the merge.

### FC-6.5: Failed merge queue entries are not automatically retried

**Root cause:** When a merge queue entry fails (conflict or test failure), it
is marked `Failed` and left. No retry is attempted. The entry must be
manually re-enqueued.

**Why it hurts:** Transient failures (network issues during push, flaky tests)
permanently fail the entry.

**Mitigation:**
- Add configurable retry count for merge queue entries.
- Distinguish between "merge conflict" (likely permanent) and "test failure"
  or "push failure" (potentially transient) for retry decisions.

### FC-6.6: Merge queue push uses raw branch name which may have been deleted

**Root cause:** `LandEntryAsync` (line 450-471) pushes `integrationBranch:targetBranch`.
If the remote has protection rules, force-push restrictions, or the target branch
was updated between the merge and the push, the push can fail.

**Mitigation:**
- Add a pre-push fetch-and-check to verify the target hasn't moved.
- Or use `--force-with-lease` for safer pushing.

---

## 7. Landing / Post-Completion Failures

### FC-7.1: Landing runs as fire-and-forget in a background Task

**Root cause:** `MissionService.HandleCompletionAsync` (line 332-360) queues
the landing as `Task.Run(async () => ...)`. If the server shuts down before
this task completes, the landing is silently lost. The mission stays as
`WorkProduced` forever.

**Why it hurts:** Server restarts during active landings cause missions to
get stuck in `WorkProduced` with no retry mechanism.

**Mitigation:**
- Track pending landings in the database and retry on server startup.
- Or use a persistent work queue instead of `Task.Run`.

### FC-7.2: Dock is reclaimed in the background task's `finally` block, racing with the server's HandleMissionCompleteAsync

**Root cause:** The background task in `MissionService.HandleCompletionAsync`
(line 346-358) reclaims the dock in its `finally` block. But
`HandleMissionCompleteAsync` in `ArmadaServer` (line 2860-2868) also attempts
to reclaim the same dock. Both code paths run the reclaim.

**Why it hurts:** The `DockService.ReclaimAsync` idempotency guard (checking
`dock.Active`) prevents double removal of the worktree itself, but the double
execution wastes resources and can produce confusing log entries.

**Mitigation:**
- Choose a single owner for dock reclamation. Either the MissionService's
  background task OR the ArmadaServer's handler, not both.

### FC-7.3: LocalMerge fails when WorkingDirectory is on a different branch

**Root cause:** `GitService.MergeBranchLocalAsync` (line 226) does
`git checkout targetBranch` before merging, but if the user's working directory
has uncommitted changes, the checkout will fail.

**Mitigation:**
- Detect dirty working directories and abort with a clear error message.
- Or stash changes before the checkout/merge and pop afterwards.

### FC-7.4: Push after local merge fails but landing is marked "succeeded"

**Root cause:** In `HandleMissionCompleteAsync` (line 2694-2707), if the local
merge succeeds but the subsequent push fails, `landingSucceeded` is still `true`.
The mission is marked `Complete` even though the remote doesn't have the changes.

**Mitigation:**
- Track push success separately and surface it in the mission status.
- Or mark as `LandingFailed` if push is required and fails.

### FC-7.5: PR creation fails after push succeeds -- branch is pushed but no PR exists

**Root cause:** In `HandleMissionCompleteAsync` (line 2551-2645), the push and
PR creation are sequential. If push succeeds but PR creation fails (gh CLI not
installed, auth issues, rate limiting), the branch is on the remote but the
mission is marked `LandingFailed` with no PR.

**Why it hurts:** The branch is orphaned on the remote. Nobody knows it exists.

**Mitigation:**
- If push succeeds but PR creation fails, retry PR creation.
- At minimum, record the push success so the branch can be manually PR'd.

---

## 8. Git Operation Failures

### FC-8.1: 120-second timeout on git operations kills long-running clones/pushes

**Root cause:** `GitService.RunProcessAsync` (line 446) has a 120-second
timeout on all git operations. Large repos or slow networks can exceed this,
causing clone, fetch, or push to be killed mid-operation.

**Why it hurts:** A killed clone leaves a corrupted bare repo. A killed push
may leave a partially-pushed branch. A killed fetch may leave stale refs.

**Mitigation:**
- Make the timeout configurable per operation type.
- Use longer timeouts for clone/push (e.g., 10-15 minutes).
- Verify repo integrity after timeout kills.

### FC-8.2: Bare repo fetch refspec may not be configured

**Root cause:** `GitService.FetchAsync` (line 91-104) checks and sets the
fetch refspec, but this is a runtime check on every fetch. If the config write
fails silently, every subsequent fetch may not update local branches.

**Mitigation:**
- Verify the refspec is set during bare clone and fail loudly if it isn't.

### FC-8.3: `git branch --list` doesn't check remote branches

**Root cause:** `GitService.BranchExistsAsync` (line 358-372) only checks local
branches. In a bare repo context this is fine, but the branch may exist on the
remote and not locally if fetch hasn't been run recently.

**Mitigation:**
- Add remote branch checking where needed.

### FC-8.4: Git merge abort may fail, leaving dirty merge state

**Root cause:** `MergeQueueService.MergeBranchAsync` (line 481-485) catches a
merge failure and tries `git merge --abort`. But the abort itself can fail if
the merge left the repository in a particularly bad state. This leaves a dirty
merge state in the temporary worktree.

**Why it hurts:** The worktree cleanup may fail because git operations in a
dirty-merge state can behave unpredictably.

**Mitigation:**
- Use `git reset --hard` as a fallback after merge abort failure.

---

## 9. Cleanup & Data Hygiene Failures

### FC-9.1: DataExpiryService only runs every 100 health check cycles

**Root cause:** Data expiry runs every ~50 minutes at default settings
(100 cycles x 30 seconds). In between, old records accumulate.

**Why it hurts:** If the system is heavily used, the database can grow
significantly between purge cycles.

**Mitigation:**
- Make the expiry interval configurable.
- Run expiry more frequently or based on database size.

### FC-9.2: DataExpiryService doesn't clean up missions in LandingFailed or WorkProduced

**Root cause:** `DataExpiryService.PurgeExpiredDataAsync` (line 92-99) only
deletes standalone missions with status `Complete`, `Failed`, or `Cancelled`.
Missions in `LandingFailed`, `WorkProduced`, or `PullRequestOpen` are never
automatically cleaned up.

**Why it hurts:** These semi-terminal states accumulate in the database
indefinitely.

**Mitigation:**
- Add `LandingFailed`, `WorkProduced`, and `PullRequestOpen` to the expiry
  list (with a longer retention period if desired).

### FC-9.3: Merge queue entry branches are only cleaned up on explicit delete

**Root cause:** `MergeQueueService.DeleteAsync` (line 157-206) cleans up git
branches, but this only runs when explicitly called. The data expiry service
deletes database records for terminal merge entries, but the git branches
associated with those entries are NOT cleaned up by data expiry.

**Why it hurts:** Remote branches from merge queue entries accumulate
indefinitely on the git host.

**Mitigation:**
- Have the data expiry service also clean up branches before deleting records.
- Or have `DeleteAsync` be called by the expiry service instead of raw SQL.

### FC-9.4: Stale captain log files are never deleted

**Root cause:** Log rotation in the health check loop (line 3097-3103) rotates
captain log files by size, but logs for deleted captains are never cleaned up.
Mission log files under `logs/missions/` are never rotated or cleaned up at all.

**Mitigation:**
- Add log cleanup for deleted captains and old mission logs.

### FC-9.5: Inactive dock records accumulate even after cleanup

**Root cause:** `DockService.ReclaimAsync` sets `Active=false` and clears
`CaptainId`, but the dock record stays in the database. The data expiry service
only deletes inactive docks older than the retention period.

**Why it hurts:** With many missions, the docks table grows large and slows
down dock-related queries.

**Mitigation:**
- Delete dock records immediately after successful reclamation if no debugging
  is needed.

---

## 10. Concurrency & Race Conditions

### FC-10.1: TryAssignAsync race between two health check dispatches

**Root cause:** `FindAvailableCaptainAsync` (line 510-518) reads the list of
idle captains, then `TryClaimAsync` atomically updates one. But between the
read and the claim, two concurrent dispatch calls may both find the same
captain idle and both attempt to provision docks for it. Only one claim
succeeds, but both may have created worktrees.

**Why it hurts:** The losing dispatch creates an orphaned worktree (though
the code does call `ReclaimAsync` on the orphaned dock).

**Mitigation:**
- The `TryClaimAsync` atomic UPDATE already handles this correctly at the
  database level. The orphaned worktree cleanup works but wastes time/resources.
- Consider selecting the captain *after* provisioning, or lock at the
  captain level before provisioning.

### FC-10.2: _ProcessToCaptain dictionary uses `lock` instead of ConcurrentDictionary

**Root cause:** `_ProcessToCaptain` and `_ProcessToMission` (line 71-76 in
ArmadaServer) are plain `Dictionary<int, string>` protected by `lock`. This
works but is error-prone -- any code path that forgets the lock will corrupt
the dictionary.

**Mitigation:**
- Use `ConcurrentDictionary<int, string>` for both maps.

### FC-10.3: Mission database updates are not atomic with captain updates

**Root cause:** Throughout the codebase, mission status updates and captain
state updates are done as separate database calls. If the process crashes
between the two updates, the state becomes inconsistent (e.g., mission says
`Assigned` but captain says `Idle`).

**Mitigation:**
- Use database transactions for coupled state changes.
- Add consistency checks in the health check loop (already partially done
  with `RecoverOrphanedMissionsAsync`).

### FC-10.4: Health check and process exit handler can double-complete a mission

**Root cause:** `HealthCheckCaptainAsync` polls process status. `HandleAgentProcessExited`
reacts to the `Process.Exited` event. Both can fire near-simultaneously for the
same process exit. The terminal-state check (line 411-420 in `HandleProcessExitAsync`)
guards against this, but there's a small TOCTOU window where both threads read
the mission status as `InProgress` before either updates it.

**Mitigation:**
- Use an atomic database operation (UPDATE WHERE status = InProgress) for the
  status transition, similar to `TryClaimAsync`.

---

## 11. Process Management Failures

### FC-11.1: PID reuse causes false "process alive" detection

**Root cause:** The health check uses `Process.GetProcessById(processId)` to
check if an agent is still running. On busy systems, PIDs are reused quickly.
The health check might find an unrelated process with the same PID and think
the agent is still alive.

**Mitigation:**
- Store the process start time and verify it matches.
- Or use process names/command lines for verification.

### FC-11.2: Process.Kill may not kill child processes on all platforms

**Root cause:** `BaseAgentRuntime.StopAsync` (line 191) uses
`process.Kill(entireProcessTree: true)`, but on some platforms (especially
when the agent spawns subprocesses via shell), the entire tree may not be
killed reliably.

**Mitigation:**
- Use platform-specific process group management (e.g., job objects on Windows,
  process groups on Linux).

### FC-11.3: Agent process stdin closed immediately, but some runtimes may need it

**Root cause:** `BaseAgentRuntime.StartAsync` (line 156) closes stdin
immediately after process start. Some AI agent runtimes might expect interactive
stdin (e.g., for confirmation prompts), causing them to hang or crash.

**Mitigation:**
- Make stdin closing behavior configurable per runtime.

### FC-11.4: Graceful stop waits only 10 seconds before kill

**Root cause:** `BaseAgentRuntime.StopAsync` (line 187) waits 10 seconds for
graceful exit, then kills. An agent that is in the middle of committing or
pushing may be killed mid-operation, leaving the git repo in a dirty state.

**Mitigation:**
- Increase the graceful shutdown timeout.
- Or signal the agent to finish current operation and exit.

---

## 12. Recovery & Escalation Failures

### FC-12.1: TryRecoverAsync doesn't regenerate CLAUDE.md

**Root cause:** `CaptainService.TryRecoverAsync` (line 129-198) repairs the
worktree and re-launches the agent, but it doesn't regenerate the mission's
CLAUDE.md file. If the original CLAUDE.md was corrupted or deleted by the
previous agent run, the recovered agent has no instructions.

**Mitigation:**
- Call `GenerateClaudeMdAsync` in the recovery path.

### FC-12.2: Recovery re-launches agent on potentially dirty worktree

**Root cause:** `TryRecoverAsync` calls `RepairWorktreeAsync` which does
`git checkout -- .` and `git clean -fd`. This discards any uncommitted work
the previous agent run had produced but not yet committed. The recovered
agent starts from scratch on a clean worktree.

**Why it hurts:** Work is lost. The agent may redo work that was already done,
wasting time and API credits.

**Mitigation:**
- Before repair, check if there are uncommitted changes and auto-commit them
  to a temporary branch.
- Or skip the clean and let the agent continue from where it left off.

### FC-12.3: RecoveryAttempts counter is per-captain, not per-mission

**Root cause:** `captain.RecoveryAttempts` (line 133 in CaptainService) is
incremented on each recovery attempt but only reset to 0 when the captain is
released (line 209). If a captain fails multiple missions in a row (each
failing on the first attempt), the recovery counter accumulates across missions.

**Why it hurts:** A captain that had legitimate transient failures on previous
missions may exhaust its recovery budget on a new mission that also has a
transient issue but would have recovered on the first retry.

**Mitigation:**
- Reset `RecoveryAttempts` when a new mission is assigned, not just when released.
- Actually, the `ReleaseAsync` method (line 209) does reset to 0 on release,
  which happens between missions. The issue is if a captain goes from working
  -> recovery -> working -> recovery without being released in between.

### FC-12.4: Orphaned mission recovery always calls HandleCompletionAsync

**Root cause:** `RecoverOrphanedMissionsAsync` (line 706-762) assumes that if
an orphaned mission's process is not alive, the work was completed. It calls
`HandleCompletionAsync` which triggers the full landing flow. But the agent
may have crashed without producing any useful work.

**Why it hurts:** A merge is attempted on a branch that may have no changes
(or broken partial changes), leading to empty merges or merge conflicts.

**Mitigation:**
- Check if the branch actually has commits beyond the base before triggering
  completion.
- Use `git diff --stat` to verify work was actually produced.

---

## 13. Configuration & Settings Failures

### FC-13.1: Landing mode resolution is complex and error-prone

**Root cause:** Landing mode resolves through a 4-level cascade:
`voyage.LandingMode > vessel.LandingMode > settings.LandingMode > derive from booleans`.
Plus legacy boolean flags (`AutoPush`, `AutoCreatePullRequests`,
`AutoMergePullRequests`) that can override or conflict with the landing mode.

**Why it hurts:** Users don't know which setting is taking effect. A vessel
configured for `LocalMerge` but a voyage configured for `PullRequest` will
use `PullRequest`, surprising the user.

**Mitigation:**
- Log the resolved landing mode clearly when a mission completes.
- Add a dry-run or preview mode that shows what would happen.
- Deprecate the legacy boolean flags.

### FC-13.2: Vessel WorkingDirectory not validated at creation time

**Root cause:** For `LocalMerge` landing mode, the vessel needs both
`LocalPath` (bare repo) and `WorkingDirectory` (user's checkout). These are
not validated at vessel creation time. If they're missing, the landing fails
silently and falls through to the "no landing configured" path.

**Mitigation:**
- Validate that the required paths exist and are valid git repositories when
  the vessel is created.
- Or validate at dispatch time and fail fast.

### FC-13.3: MergeQueueTestCommand has no validation

**Root cause:** The test command string is passed directly to the shell. If it
contains errors or references tools not installed on the server, every merge
queue entry will fail with a cryptic error.

**Mitigation:**
- Validate the test command at configuration time (e.g., dry-run).
- Show the full test command output in the error message (already partially
  done via `TruncateOutput`).

### FC-13.4: DefaultBranch may not match the actual repository default

**Root cause:** `Vessel.DefaultBranch` defaults to `"main"` (line 66 in
Vessel.cs), but many repositories use `"master"` or other default branch names.
If not explicitly set, all worktrees and merges target the wrong branch.

**Why it hurts:** Worktree creation fails (`fatal: invalid reference: main`),
or merges go to the wrong branch, or diffs compare against a non-existent branch.

**Mitigation:**
- Auto-detect the default branch from the remote during vessel creation.
- Validate the branch exists during bare clone.

---

## 14. Database & State Consistency Failures

### FC-14.1: SQLite concurrent write contention

**Root cause:** Multiple service components (health check, dispatch, merge
queue, API handlers) all write to the same SQLite database concurrently.
SQLite handles this with WAL mode (if configured) but can still throw
"database is locked" errors under heavy contention.

**Mitigation:**
- Use PostgreSQL or SQL Server for production deployments with high concurrency.
- Or add retry logic around database writes.
- Ensure WAL mode is enabled for SQLite.

### FC-14.2: Server crash leaves inconsistent state

**Root cause:** There are no database transactions wrapping multi-step state
changes. If the server crashes between updating the mission status and updating
the captain status, the database is left inconsistent.

**Example:** Mission updated to `Assigned` + captain still `Idle` = mission
thinks it's assigned but captain takes another mission. Or: captain updated to
`Working` + mission still `Pending` = captain thinks it's working but mission
gets re-assigned to another captain.

**Mitigation:**
- Use database transactions for all coupled state changes.
- The startup `CleanupStaleCaptainsAsync` partially handles this, but only
  for the captain side.

### FC-14.3: In-memory state (`_ProcessToCaptain`, etc.) lost on restart

**Root cause:** The `_ProcessToCaptain` and `_ProcessToMission` maps are
in-memory only. On server restart, all process-to-captain/mission mappings are
lost. The health check fallback (checking captain.ProcessId) compensates, but
the process exit event handler won't fire for processes started before the
restart.

**Mitigation:**
- Persist process mappings to the database.
- Or rely entirely on the health check loop for process monitoring.

### FC-14.4: DataExpiryService uses raw SQL that bypasses the ORM

**Root cause:** `DataExpiryService` uses direct `SqliteConnection` with raw
SQL (line 70 in DataExpiryService). This bypasses any ORM-level validations
or event hooks. If the schema changes, the raw SQL may break silently.

**Mitigation:**
- Use the database driver interfaces for deletion operations.
- Or add schema version checks before running expiry queries.

---

## Summary: Top Failure Patterns

| Pattern | Frequency | Severity | Key Cases |
|---------|-----------|----------|-----------|
| **Merge conflicts from concurrent missions on same vessel** | Very High | High | FC-1.1, FC-1.2 |
| **Missions stuck in semi-terminal states** | High | High | FC-4.1, FC-4.2, FC-4.3, FC-4.5 |
| **Stale worktrees blocking operations** | High | Medium | FC-2.1, FC-2.2, FC-2.5 |
| **Race conditions in completion handling** | Medium | High | FC-3.3, FC-3.4, FC-10.4 |
| **Landing failures with no retry** | High | High | FC-4.2, FC-6.5, FC-7.1 |
| **Orphaned git branches** | Medium | Low | FC-7.5, FC-9.3 |
| **Captains stuck in non-recoverable states** | Medium | Medium | FC-3.1, FC-3.2 |
| **State inconsistency after crash** | Low | High | FC-14.2, FC-14.3 |

## Top Recommendations (Highest Impact)

1. **Serialize missions per vessel** -- The single highest-impact change.
   Only allow one `InProgress` mission per vessel at a time. This eliminates
   FC-1.1, FC-1.2, and dramatically reduces merge conflicts.

2. **Add automatic landing retry** -- Missions in `LandingFailed` should be
   automatically retried with a rebase after a configurable delay. This
   eliminates FC-4.2, the most common manual recovery scenario.

3. **Move PR polling into the health check loop** -- Instead of a one-shot
   5-minute poll, periodically check all `PullRequestOpen` missions. This
   eliminates FC-4.3.

4. **Make landing synchronous, not fire-and-forget** -- Don't release the
   captain until landing completes (or fails). This eliminates FC-3.4 and
   FC-7.1, and simplifies the concurrency model.

5. **Add a "stuck mission" detector** -- A health check addition that flags
   missions in `WorkProduced`, `LandingFailed`, or `PullRequestOpen` for
   longer than a threshold as needing attention. This provides visibility
   into FC-4.1, FC-4.2, FC-4.3.

6. **Use database transactions for coupled state changes** -- Wrap
   mission + captain + dock state changes in transactions to eliminate
   FC-14.2.

7. **Auto-reset stalled captains** -- After a configurable cooldown (e.g.,
   15 minutes), automatically reset `Stalled` captains to `Idle` and reset
   their recovery counter. This eliminates FC-3.2.

8. **Validate vessel configuration at creation** -- Ensure `DefaultBranch`,
   `WorkingDirectory`, and `LocalPath` are valid when the vessel is created,
   not when a mission fails. This eliminates FC-13.2 and FC-13.4.
