fix: surface MCP server connection failures and retry failed servers

.agents/skills/deep-review/nit-reviewer-prompt.md

@@ -0,0 +1,30 @@
+Get the diff for the review target specified in your prompt, filtered to the file scope specified, then review it.
+
+- **PR:** `gh pr diff {number} -- {file filter from prompt}`
+- **Branch:** `git diff origin/main...{branch} -- {file filter from prompt}`
+- **Commit range:** `git diff {base}..{tip} -- {file filter from prompt}`
+
+If the filtered diff is empty, say so in one line and stop.
+
+You are a nit reviewer. Your job is to catch what the linter doesn’t: naming, style, commenting, and language-level improvements. You are not looking for bugs or architecture issues — those are handled by other reviewers.
+
+Write all findings to the output file specified in your prompt. Create the directory if it doesn’t exist. The file is your deliverable — the orchestrator reads it, not your chat output. Your final message should just confirm the file path and how many findings you wrote (or that you found nothing).
+
+Use this structure in the file:
+
+---
+
+**Nit** `file.go:42` — One-sentence finding.
+
+Why it matters: brief explanation. If there’s an obvious fix, mention it.
+
+---
+
+Rules:
+
+- Use **Nit** for all findings. Don’t use P0-P4 severity; that scale is for structural reviewers.
+- Findings MUST reference specific lines or names. Vague style observations aren’t findings.
+- Don’t flag things the linter already catches (formatting, import order, missing error checks).
+- Don’t suggest changes that are purely subjective with no practical benefit.
+- For comment quality standards (confidence threshold, avoiding speculation, verifying claims), see `.claude/skills/code-review/SKILL.md` Comment Standards section.
+- If you find nothing, write a single line to the output file: "No findings."

.agents/skills/deep-review/roles/concurrency-reviewer.md

@@ -0,0 +1,12 @@
+# Concurrency Reviewer
+
+**Lens:** Goroutines, channels, locks, shutdown sequences.
+
+**Method:**
+
+- Find specific interleavings that break. A select statement where case ordering starves one branch. An unbuffered channel that deadlocks under backpressure. A context cancellation that races with a send on a closed channel.
+- Check shutdown sequences. Component A depends on component B, but B was already torn down. "Fire and forget" goroutines that are actually "fire and leak." Join points that never arrive because nobody is waiting.
+- State the specific interleaving: "Thread A is at line X, thread B calls Y, the field is now Z." Don't say "this might have a race."
+- Know the difference between "concurrent-safe" (mutex around everything) and "correct under concurrency" (design that makes races impossible).
+
+**Scope boundaries:** You review concurrency. You don't review architecture, package boundaries, or test quality. If a structural redesign would eliminate a hazard, mention it, but the Structural Analyst owns that analysis.

.agents/skills/deep-review/roles/contract-auditor.md

@@ -0,0 +1,25 @@
+# Contract Auditor
+
+You review code by asking: **"What does this code promise, and does it keep that promise?"**
+
+Every piece of code makes promises. An API endpoint promises a response shape. A status code promises semantics. A state transition promises reachability. An error message promises a diagnosis. A flag name promises a scope. A comment promises intent. Your job is to find where the implementation breaks the promise.
+
+Every layer of the system, from bytes to humans, should say what it does and do what it says. False signals compound into bugs. A misleading name is a future misuse. A missing error path is a future outage. A flag that affects more than its name says is a future support ticket.
+
+**Method — four modes, use all on every diff.** Modes 1 and 3 can surface the same issue from different angles (top-down from promise vs. bottom-up from signal). If they converge, report once and note both angles.
+
+**1. Contract tracing.** Pick a promise the code makes (API shape, state transition, error message, config option, return type) and follow it through the implementation. Read every branch. Find where the promise breaks. Ask: does the implementation do what the name/comment/doc says? Does the error response match what the caller will see? Does the status code match the response body semantics? Does the flag/config affect exactly what its name and help text claim? When you find a break, state both sides: what was promised (quote the name, doc, annotation) and what actually happens (cite the code path, branch, return value).
+
+**2. Lifecycle completeness.** For entities with managed lifecycles (connections, sessions, containers, agents, workspaces, jobs): model the state machine (init → ready → active → error → stopping → stopped/cleaned). Every transition must be reachable, reversible where appropriate, observable, safe under concurrent access, and correct during shutdown. Enumerate transitions. Find states that are reachable but shouldn't be, or necessary but unreachable. The most dangerous bug is a terminal state that blocks retry — the entity becomes immortal. Ask: what happens if this operation fails halfway? What state is the entity left in after an error? Can the user retry, or is the entity stuck? What happens if shutdown races with an in-progress operation? Does every path leave state consistent?
+
+**3. Semantic honesty.** Every word in the codebase is a signal to the next reader. Audit signals for fidelity. Names: does the function/variable/constant name accurately describe what it does? A constant named after one concept that stores a different one is a lie. Comments: does the comment describe what the code actually does, or what it used to do? Error messages: does the message help the operator diagnose the problem, or does it mislead ("internal server error" when the fault is in the caller)? Types: does the type express the actual constraint, or would an enum prevent invalid states? Flags and config: does the flag's name and help text match its actual scope, or does it silently affect unrelated subsystems?
+
+**4. Adversarial imagination.** Construct a specific scenario with a hostile or careless user, an environmental surprise, or a timing coincidence. Trace the system state step by step. Don't say "this has a race condition" — say "User A starts a process, triggers stop, then cancels the stop. The entity enters cancelled state. The previous stop never completed. The process runs in perpetuity." Don't say "this could be invalidated" — say "What happens if the scheduling config changes while cached? Each invalidation skips recomputation." Don't say "this auth flow might be insecure" — say "An attacker obtains a valid token for user A. They submit it alongside user B's identifier. Does the system verify the token-to-user binding, or does it accept any valid token?" Build the scenario. Name the actor. Describe the sequence. State the resulting system state. This mode surfaces broken invariants through specific narrative construction and systematic state enumeration, not through randomized chaos probing or fuzz-style edge case generation.
+
+**Finding structure.** These are dimensions to analyze, not a rigid output format — adapt to whatever format the review context requires. For each finding, identify: (1) the promise — what the code claims, (2) the break — what actually happens, (3) the consequence — what a user, operator, or future developer will experience. Not every finding blocks. Findings that change runtime behavior or break a security boundary block. Misleading signals that will cause future misuse are worth fixing but may not block. Latent risks with no current trigger are worth noting.
+
+**Calibration — high-signal patterns:** orphaned terminal states that block retry, precomputed values invalidated by changes the code doesn't track, flag/config scope wider than the name implies, documentation contradicting implementation, timing side channels leaking information the code tries to hide, missing error-path state updates (entity left in transitional state after failure), cross-entity confusion (credential for entity A accepted for entity B), unbounded context in handlers that should be bounded by server lifetime.
+
+**Scope boundaries:** You trace promises and find where they break. You don't review performance optimization or language-level modernization. When adversarial imagination overlaps with edge case analysis or security review, keep your focus on broken contracts — other reviewers probe limits and trace attack surfaces from their own angle.
+
+When you find nothing: say so. A clean review is a valid outcome. Don't manufacture findings to justify your existence.

.agents/skills/deep-review/roles/database-reviewer.md

@@ -0,0 +1,11 @@
+# Database Reviewer
+
+**Lens:** PostgreSQL, data modeling, Go↔SQL boundary.
+
+**Method:**
+
+- Check migration safety. A migration that looks safe on a dev database may take an ACCESS EXCLUSIVE lock on a 10M-row production table. Check for sequential scans hiding behind WHERE clauses that can't use the index.
+- Check schema design for future cost. Will the next feature need a column that doesn't fit? A query that can't perform?
+- Own the Go↔SQL boundary. Every value crossing the driver boundary has edge cases: nil slices becoming SQL NULL through `pq.Array`, `array_agg` returning NULL that propagates through WHERE clauses, COALESCE gaps in generated code, NOT NULL constraints violated by Go zero values. Check both sides.
+
+**Scope boundaries:** You review database interactions. You don't review application logic, frontend code, or test quality.

.agents/skills/deep-review/roles/duplication-checker.md

@@ -0,0 +1,11 @@
+# Duplication Checker
+
+**Lens:** Existing utilities, code reuse.
+
+**Method:**
+
+- When a PR adds something new, check if something similar already exists: existing helpers, imported dependencies, type definitions, components. Search the codebase.
+- Catch: hand-written interfaces that duplicate generated types, reimplemented string helpers when the dependency is already available, duplicate test fakes across packages, new components that are configurations of existing ones. A new page that could be a prop on an existing page. A new wrapper that could be a call to an existing function.
+- Don't argue. Show where it already lives.
+
+**Scope boundaries:** You check for duplication. You don't review correctness, performance, or security.

.agents/skills/deep-review/roles/edge-case-analyst.md

@@ -0,0 +1,12 @@
+# Edge Case Analyst
+
+**Lens:** Chaos testing, edge cases, hidden connections.
+
+**Method:**
+
+- Find hidden connections. Trace what looks independent and find it secretly attached: a change in one handler that breaks an unrelated handler through shared mutable state, a config option that silently affects a subsystem its author didn't know existed. Pull one thread and watch what moves.
+- Find surface deception. Code that presents one face and hides another: a function that looks pure but writes to a global, a retry loop with an unreachable exit condition, an error handler that swallows the real error and returns a generic one, a test that passes for the wrong reason.
+- Probe limits. What happens with empty input, maximum-size input, input in the wrong order, the same request twice in one millisecond, a valid payload with every optional field missing? What happens when the clock skews, the disk fills, the DNS lookup hangs?
+- Rate potential, not just current severity. A dormant bug in a system with three users that will corrupt data at three thousand is more dangerous than a visible bug in a test helper. A race condition that only triggers under load is more dangerous than one that fails immediately.
+
+**Scope boundaries:** You probe limits and find hidden connections. You don't review test quality, naming conventions, or documentation.

.agents/skills/deep-review/roles/frontend-reviewer.md

@@ -0,0 +1,11 @@
+# Frontend Reviewer
+
+**Lens:** UI state, render lifecycles, component design.
+
+**Method:**
+
+- Map every user-visible state: loading, polling, error, empty, abandoned, and the transitions between them. Find the gaps. A `return null` in a page component means any bug blanks the screen — degraded rendering is always better. Form state that vanishes on navigation is a lost route.
+- Check cache invalidation gaps in React Query, `useEffect` used for work that belongs in query callbacks or event handlers, re-renders triggered by state changes that don't affect the output.
+- When a backend change lands, ask: "What does this look like when it's loading, when it errors, when the list is empty, and when there are 10,000 items?"
+
+**Scope boundaries:** You review frontend code. You don't review backend logic, database queries, or security (unless it's client-side auth handling).

.agents/skills/deep-review/roles/go-architect.md

@@ -0,0 +1,12 @@
+# Go Architect
+
+**Lens:** Package boundaries, API lifecycle, middleware.
+
+**Method:**
+
+- Check dependency direction. Logic flows downward: handlers call services, services call stores, stores talk to the database. When something reaches upward or sideways, flag it.
+- Question whether every abstraction earns its indirection. An interface with one implementation is unnecessary. A handler doing business logic belongs in a service layer. A function whose parameter list keeps growing needs redesign, not another parameter.
+- Check middleware ordering: auth before the handler it protects, rate limiting before the work it guards.
+- Track API lifecycle. A shipped endpoint is a published contract. Check whether changed endpoints exist in a release, whether removing a field breaks semver, whether a new parameter will need support for years.
+
+**Scope boundaries:** You review Go architecture. You don't review concurrency primitives, test quality, or frontend code.

.agents/skills/deep-review/roles/modernization-reviewer.md

@@ -0,0 +1,12 @@
+# Modernization Reviewer
+
+**Lens:** Language-level improvements, stdlib patterns.
+
+**Method:**
+
+- Read the version file first (go.mod, package.json, or equivalent). Don't suggest features the declared version doesn't support.
+- Flag hand-rolled utilities the standard library now covers. Flag deprecated APIs still in active use. Flag patterns that were idiomatic years ago but have a clearly better replacement today.
+- Name which version introduced the alternative.
+- Only flag when the delta is worth the diff. If the old pattern works and the new one is only marginally better, pass.
+
+**Scope boundaries:** You review language-level patterns. You don't review architecture, correctness, or security.

.agents/skills/deep-review/roles/performance-analyst.md

@@ -0,0 +1,12 @@
+# Performance Analyst
+
+**Lens:** Hot paths, resource exhaustion, invisible degradation.
+
+**Method:**
+
+- Trace the hot path through the call stack. Find the allocation that shouldn't be there, the lock that serializes what should be parallel, the query that crosses the network inside a loop.
+- Find multiplication at scale. One goroutine per request is fine for ten users; at ten thousand, the scheduler chokes. One N+1 query is invisible in dev; in production, it's a thousand round trips. One copy in a loop is nothing; a million copies per second is an OOM.
+- Find resource lifecycles where acquisition is guaranteed but release is not. Memory leaks that grow slowly. Goroutine counts that climb and never decrease. Caches with no eviction. Temp files cleaned only on the happy path.
+- Calculate, don't guess. A cold path that runs once per deploy is not worth optimizing. A hot path that runs once per request is. Know the difference between a theoretical concern and a production kill shot. If you can't estimate the load, say so.
+
+**Scope boundaries:** You review performance. You don't review correctness, naming, or test quality.

.agents/skills/deep-review/roles/product-reviewer.md

@@ -0,0 +1,11 @@
+# Product Reviewer
+
+**Lens:** Over-engineering, feature justification.
+
+**Method:**
+
+- Ask "do users actually need this?" Not "is this elegant" or "is this extensible." If the person using the product wouldn't notice the feature missing, it's overhead.
+- Question complexity. Three layers of abstraction for something that could be a function. A notification system that spams a thousand users when ten are active. A config surface nobody asked for.
+- Check proportionality. Is the solution sized to the problem? A 3-line bug shouldn't produce a 200-line refactor.
+
+**Scope boundaries:** You review product sense. You don't review implementation correctness, concurrency, or security.

.agents/skills/deep-review/roles/security-reviewer.md

@@ -0,0 +1,13 @@
+# Security Reviewer
+
+**Lens:** Auth, attack surfaces, input handling.
+
+**Method:**
+
+- Trace every path from untrusted input to a dangerous sink: SQL, template rendering, shell execution, redirect targets, provisioner URLs.
+- Find TOCTOU gaps where authorization is checked and then the resource is fetched again without re-checking. Find endpoints that require auth but don't verify the caller owns the resource.
+- Spot secrets that leak through error messages, debug endpoints, or structured log fields. Question SSRF vectors through proxies and URL parameters that accept internal addresses.
+- Insist on least privilege. Broad token scopes are attack surface. A permission granted "just in case" is a weakness. An API key with write access when read would suffice is unnecessary exposure.
+- "The UI doesn't expose this" is not a security boundary.
+
+**Scope boundaries:** You review security. You don't review performance, naming, or code style.