feat(deepseek): full v1 integration — DeepSeek as 4th Triumvirate sibling (REQ-DS-001..030)#37
Merged
Merged
Conversation
…omplete) 3-round /goatrodeo ceremony with twin review (Codex + Gemini), live api.deepseek.com probe verification ($10 funded), Phase 3 quality gate CLEAN, Phase 4.4 canonical doc audit APPROVED. 30 REQ-IDs, 18 build tasks across 6 waves. Artifacts: - daemon/docs/specs/deepseek-integration-spec.md (canonical, 30 REQs) - daemon/docs/specs/deepseek-integration-goatrodeo-seed.md (seed brief) - daemon/docs/specs/findings/ (R1+R2+R3 deltas + research + verification) - daemon/docs/v1-deepseek/ (8 canonical build docs + policy-rules.yml) - daemon/docs/bugs/2026-05-25-daemon-session-ask-intermittent-failure.md - .goatrodeo.events.log (GR_EVT audit trail) Key decisions (D-01..D-07): - D-01 cloud deepseek-v4-pro via API key (documented exception to the subscriptions-only rule: no GPU, no subscription product, cheap metered) - D-02 hand-rolled reqwest + SSE (not async-openai) - D-03 true 4th participant - ask_agent on any topic, no sandbox v1 - D-04 model is a config knob (Pro <-> Flash one-line swap) - D-05 prepaid + 402 hard breaker-trip + exact metered ledger - D-07 thinking ON default; frugality = per-call thinking toggle; max_tokens 32K P5.2 gate: this commit is WAVE0_SHA. Every Wave 1+ worktree branches from here. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…017) 8 #[ignore]-gated integration probes against the real api.deepseek.com. Each probe is env-gated on TRIUMVIRATE_DEEPSEEK_API_KEY and prints a structured PROBE-NN OK line for capture into PROBE_RESULTS.md (the audit artifact). Coverage (REQ-DS-### in parens): PROBE-01 /user/balance shape (REQ-DS-018) PROBE-02 /models lists v4-pro AND v4-flash (REQ-DS-005) PROBE-03 streaming SSE: reasoning -> content -> usage -> DONE (REQ-DS-019/009/023) PROBE-04 completion_tokens INCLUDES reasoning_tokens (no double-add) (REQ-DS-009 A-04b) PROBE-05 max_tokens=64+thinking-on => finish_reason=length + empty content (C-1) PROBE-06 bad key => 401 + error.type=authentication_error (REQ-DS-006) PROBE-07 malformed request (json_object w/o 'json') => 400 + invalid_request_error PROBE-08 v4-flash thinking:disabled => content only, no reasoning_content (REQ-DS-027) Result on the spec branch (WAVE0 = 9ead5f8 + this commit): 8/8 PASSED; ~$0.01 spend; PROBE_RESULTS.md captured. Implementation notes for the production runner (T-006/T-010): - Usage may arrive in ANY chunk (not necessarily empty-choices) — be resilient (probe-03 was over-strict initially; fix: scan all chunks for top-level usage). - error.code is generic ("invalid_request_error" for 400/401/402/422) — classify by HTTP STATUS, not error.code (already enforced in REQ-DS-006 + policy-rules.yml). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…GAUNTLET F2) Lands the dispatch briefing + contract for T-001 plus all post-Wave-0 spec adjustments. Without this commit, any worker worktree off WAVE0_SHA would see the pre-audit briefing (with the wrong execute_ask_agent ref, the OR-ambiguous reality_test, and the wrong display_agent_name file location). Caught by GAUNTLET Filter + Tiebreaker — both verifiers flagged 'briefing not present in this worktree'. P5.3 audit fixes folded in: - R1: display_agent_name file ref split (mcp-tools/src/lib.rs:92 for the fn, mcp-tools/src/inter_agent.rs:275 for the supported-agents fallback). Added mcp-tools/src/lib.rs to allowed_files. Commit-format regex relaxed in policy-rules.yml to permit the repo's 'feat(deepseek): T-XXX' convention. IMPL_PLAN T-001 <files> aligned with the contract. Removed phantom integration_status.rs from allowed_files. - R2: replaced ambiguous 'OR' in reality_test with TWO INDEPENDENT assertions (3a session-spawn, 3b ask_agent path) so a worker can't satisfy only one error-path edit. - R3: micro-audit clean (Codex + Gemini both APPROVED). GAUNTLET P5.3.5 Finding 2 (Filter caught what the dispatch audit missed): - execute_ask_agent is pub(crate) and NOT callable from external integration tests. The correct test surface for the 'unknown agent' error from this code path is the public HTTP route POST /ask-agent. Updated briefing.md, contract.json reality_test, and IMPL_PLAN T-001 XML to reference the HTTP surface instead of the private fn. - Also clarified that triumvirate-crate integration tests need an in-process daemon spawn (use patterns from integration_http.rs) and should be #[ignore]- gated if they require longer setup than default tests tolerate. Result: this commit becomes the new WAVE0_SHA. Every Wave 1+ worktree should branch from here, not from the old 841f685 (which had the bugs). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ce (REQ-DS-001/013/016/022)
Surface-wiring only — no dispatch routing, no runner, no token-economics.
Every gate hardcoded to gemini/codex now also accepts "deepseek". After this
commit, ask_agent {agent:"deepseek"} reaches the routing layer (which will
error with 'no deepseek backend' until T-012 lands — expected).
Six edit points + tests:
- mcp-bridge::is_supported_agent_name extended to accept "deepseek"
- mcp-tools::display_agent_name has an explicit "deepseek" => "DeepSeek" arm
(catches the GAUNTLET Filter's capitaliser trap: the generic first-letter
fallback would produce "Deepseek", which is not the canonical brand)
- mcp-tools::inter_agent.rs spawn_session error text + supported_agents fallback
list both include "deepseek"
- triumvirate/main.rs /status JSON and session_spawn_route error text include "deepseek"
- triumvirate/agent_exec.rs execute_ask_agent unsupported-agent error text includes "deepseek"
Tests:
- mcp-bridge: 1 new unit test (supports_deepseek_name) — 9 assertions incl. case
variants + negatives + non-prefix-match guard
- mcp-tools: 3 new unit tests (display_agent_name_*) — covers the DeepSeek brand
canonicality, gemini/codex regression, and unknown-agent fallback
- triumvirate/tests/integration_http.rs: 3 new #[ignore]-gated integration tests
(i_ds_01..03) — /status array contains deepseek, session-spawn error text,
ask-agent error text — each with explicit gemini+codex regression guards
Pre-existing failing test unrelated to this work:
- tests::abe_red_team_enforcement_blocks_non_compliant_worker fails on main.rs:5935
(asserts a stub_script dispatch returns NOT Completed, but the daemon currently
DOES complete it — red-team enforcement of stub detection is broken). Reproduced
identically on the pre-T-001 tree. Worth filing as a separate bug.
Verification on this commit:
- cargo check --workspace: clean
- cargo test -p mcp-bridge --lib: 33/33 passed (+1 vs pre-T-001 count)
- cargo test -p mcp-tools --lib: 34/34 passed (+3 vs pre-T-001 count)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The abe_red_team_enforcement_blocks_non_compliant_worker test fails on main.rs:5935 — s3 (stub_script dispatch) returns Completed when the red-team enforcement is supposed to reject stub code. Reproduced on the pre-T-001 tree; NOT caused by the DeepSeek work, but discovered during the T-001 blast-radius guard run. See the bug file for code pointers + likely fix surface. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…d ApiKey (REQ-DS-002/003/015)
New module daemon/crates/mcp-bridge/src/deepseek_config.rs is the AUTHORITATIVE
config-owner — every downstream task that reads DeepSeek settings reads them
through DeepSeekConfig::from_env(), not via std::env::var directly. Keeps
defaults centralised and testable.
ApiKey newtype (REQ-DS-003 guardrail):
- Plaintext only accessible via .expose()
- Debug impl prints 'ApiKey(<redacted>)' — never the secret
- DeepSeekConfig#[derive(Debug)] flows through it, so {:?} on the whole struct
also cannot leak the key
15 env knobs loaded with REQ-DS-015 defaults:
- BASE_URL='https://api.deepseek.com/v1'
- API_KEY (no default; runner fails loud at request time if empty)
- MODEL='deepseek-v4-pro' (set to 'deepseek-v4-flash' to swap)
- MAX_TOKENS=32768 (shared with reasoning; tight cap empties answers per C-1)
- THINKING=Enabled (the seat's reasoning-diversity value)
- REASONING_EFFORT=High (xhigh→Max; low/medium→High per API)
- READ_TIMEOUT_SECS=60 (primary dead-stream detector)
- TIMEOUT_SECS=1800 (absolute outer ceiling)
- TCP_KEEPALIVE_SECS=30
- MAX_CONCURRENT=8, MAX_RPM=60
- REASONING_CAP_TOKENS=0 (disabled; validation: when set, MUST be < MAX_TOKENS)
- LOG_DIR=~/.triumvirate/deepseek-logs/
- LOG_REASONING_CAP_BYTES=262144 (256 KB)
- BULK_BYTES=16384 (anti-bulk intercept threshold)
Validation: REQ-DS-028 — REASONING_CAP_TOKENS >= MAX_TOKENS errors at load
(otherwise the cap is mathematically unreachable; round-3 paradox guard).
Tests: 8 unit tests in deepseek_config::tests, all serialised via a
process-static Mutex so cargo's parallel test runner doesn't see flakes from
shared env-var state. Coverage:
- All 15 defaults match REQ-DS-015 with API_KEY set + everything else unset
- ApiKey Debug doesn't leak the secret
- DeepSeekConfig (#[derive(Debug)]) doesn't leak the secret via the whole struct
- Overrides parse correctly (model, max_tokens, thinking, effort, base_url, timeouts, max_concurrent, bulk_bytes)
- reasoning_effort xhigh→Max, low/medium/high→High mapping
- thinking_mode truthy/falsy parsing
- reasoning_cap >= max_tokens fails validation; cap=0 means disabled
No new crate deps — ConfigError uses hand-rolled Display + std::error::Error
to avoid pulling in thiserror just for this module.
Verification: cargo check --workspace clean; cargo test -p mcp-bridge --lib
deepseek_config: 8/8 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e pub calculate_cost_usd (REQ-DS-009)
Three changes in two crates:
1. token-economics/src/attribution.rs
- NEW pub fn ensure_deepseek_prices(db: &TokenDb) -> Result<()>: idempotent
producer of both v4-pro (0.435/0.87/0.003625) and v4-flash (0.14/0.28/0.0028)
rows. Uses SELECT-then-INSERT to avoid a schema change (the existing
price_table has no UNIQUE constraint on model; INSERT OR IGNORE wouldn't
dedup). The pre-existing INSERT pattern in this file's #[cfg(test)] block
is test-only — this helper is the production seed (Codex P4.4 audit catch).
- calculate_cost_usd is now pub (was pub(crate)) so the DeepSeek runner can
compute per-consult cost synchronously from the usage chunk, instead of
waiting for the deferred attribution sweep (REQ-DS-021).
2. token-economics/src/lib.rs
- re-exports ensure_deepseek_prices and calculate_cost_usd alongside the
existing attribute_records.
3. triumvirate/src/main.rs (init_process_token_db)
- calls ensure_deepseek_prices(&db) on first DB-open, after the WAL setup
but before the OnceLock is set. Errors are logged via tracing::warn and
do NOT abort daemon startup (the cost calc just returns None until
resolved) — matches the existing 'cost_usd is None when no price row'
contract in attribute_records.
Tests (4 new + all 24 in token-economics pass):
- ensure_deepseek_prices_seeds_both_v4_pro_and_v4_flash (fresh DB → 1 row each)
- ensure_deepseek_prices_is_idempotent (5x calls → still 1 row each)
- calculate_cost_usd_matches_spec_pricing_v4_pro
(.305 for 1M-miss + 1M-out; /bin/zsh.873625 for 1M-cached + 1M-out)
- calculate_cost_usd_matches_spec_pricing_v4_flash
(/bin/zsh.42 and $0.2828 same shape)
- calculate_cost_usd_matches_wave0_probe_04_numbers
(anchors to the live api.deepseek.com probe: 18 in-miss + 174 out incl reasoning
→ ~$0.0001592; a stub that double-adds reasoning_tokens to output would fail)
No new crate deps. No schema change to price_table.
Verification: cargo check --workspace clean;
cargo test -p token-economics --lib: 24/24 passed.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-002/T-003)
Codex review of Wave 1 found 0 BLOCKERS, 3 SHOULD-FIX, 2 nits. All applied
+ regression tests added.
SHOULD-FIX 1 — Silent fallback on invalid numeric env vars
(mcp-bridge/src/deepseek_config.rs)
Previously `TRIUMVIRATE_DEEPSEEK_MAX_TOKENS=oops` quietly became the documented
default (32768). For a paid API surface that's an operator foot-gun. Now:
- new ConfigError::InvalidEnv { name, value, expected } variant
- parse_env<T> helper that fails loud on garbage
- read_env_u32_nonzero / u64_nonzero / usize_nonzero variants reject 0 for
knobs where it's nonsensical (timeouts, concurrency, max_tokens, bulk_bytes,
log_reasoning_cap_bytes). reasoning_cap_tokens explicitly KEEPS the plain
parser because 0 means 'disabled'.
SHOULD-FIX 2 — Seed temporal-active invariant mismatch
(token-economics/src/attribution.rs)
ensure_price_row used SELECT EXISTS(model = ?), which short-circuited if ANY
row existed for the model — including ended or future-dated rows that the
calculate_cost_usd temporal lookup would reject. The result: a 'successful'
seed could still leave cost_usd returning None. Fix: the existence check now
mirrors the lookup invariant (active for now AND end_date IS NULL), using
sqlite's strftime('%Y-%m-%dT%H:%M:%SZ', 'now') so no chrono dep is needed.
SHOULD-FIX 3 — with_clean_env not panic-safe
(mcp-bridge/src/deepseek_config.rs tests)
A panicking test left env vars cleared, corrupting later tests. Replaced the
inline save/restore with an RAII EnvGuard struct whose Drop restores on
unwind. The closure form is preserved so call sites don't change.
NIT 1 — ApiKey doc overclaim
Softened the module-level comment to 'Debug redacts; plaintext only via
.expose() and MUST NOT be passed to any logging/serde/Display sink'. The
guarantee is correct for current code but the old wording read like a
forever-true claim about future callers.
NIT 2 — Stale comment in integration_http.rs (i_ds_03)
Test asserts on body content only; comment said '/ask-agent returns 200' but
the executor-error path is currently 502. Updated to match reality.
Regression tests added (4 new, all green):
- invalid_numeric_env_fails_loud (MAX_TOKENS=oops → InvalidEnv)
- zero_rejected_on_knobs_where_it_makes_no_sense (8 knobs × =0 → InvalidEnv)
- with_clean_env_restores_on_panic (catch_unwind + sentinel assertion)
- ensure_deepseek_prices_seeds_even_when_an_ended_row_exists (pre-existing
stale row must NOT block the open-ended seed; cost calc returns spec price)
Verification:
cargo check --workspace --all-targets: clean
cargo test -p mcp-bridge --lib: 44/44 (11 deepseek_config, was 8)
cargo test -p mcp-tools --lib: 34/34
cargo test -p token-economics --lib: 25/25 (was 24)
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… breaker, classify) (REQ-DS-006/010)
NEW module daemon/crates/mcp-bridge/src/deepseek_resilience.rs.
Four independent pieces, every time-dependent path takes `now: Instant` so
tests drive transitions without sleeping:
1. classify(status: u16) → Classification (single source of truth)
- Hard: 400, 401, 402, 403, 422
- Transient: 429, 500..=599 (everything else defaults to Transient)
2. ConcurrencyCap — Arc<Semaphore> wrapper, cheap to clone, drives the
max_concurrent cap from DeepSeekConfig (default 8).
3. TokenBucket — leaky bucket for RPM cap (default 60). Pure: try_take(now)
is non-blocking; next_available(now) lets callers compose with sleep_until.
4. Breaker — three states explicit by name, NOT a generic copy of
agy_resilience.rs (its reset-window-cooldown doesn't map):
- Closed (default; requests pass)
- HardOpenInsufficientBalance (HTTP 402; no auto-recovery)
- OpenTransient { until, attempts } (429/5xx threshold crossed)
- HalfOpen { lease_expires_at, ... } (one probe allowed; lease-bounded)
Recovery: cooldown elapsed → HalfOpen lease → probe Success closes,
probe TransientError reopens with grown backoff (configurable multiplier,
default 2x, capped at 10min). 400/401/403/422 reset the streak but do NOT
open the breaker (caller bugs, not provider faults).
Reality tests (the IMPL_PLAN contract):
- breaker_402_opens_hard_no_recovery — {Ok,Ok,Err(402)} → Hard,
advancing time stays Hard
- breaker_three_consecutive_429_opens_transient — {Err(429)×3} → cooldown>0
- breaker_transitions_to_half_open_after_cooldown — advance past until → HalfOpen
- breaker_half_open_success_closes_failure_reopens — probe Ok → Closed;
probe Err → OpenTransient
with cd2 > cd1
- breaker_400_class_does_not_open_breaker — 400/401/422 keep Closed
- classify_matches_spec_table — all 10 status codes
- token_bucket_starts_full_then_refills_at_rpm_rate
- token_bucket_next_available_predicts_correct_wait
- concurrency_cap_holds_then_releases — async test, semaphore drop
Stub guards:
- A breaker that returns Closed regardless would fail the 402 case.
- A token bucket that ignores refill rate would fail the 1.5s/1-token check.
- A classify that lumps everything as Transient would fail the 400/401/402/403/422 cases.
Module exported from mcp-bridge lib.rs as `pub mod deepseek_resilience`.
NO new crate deps (tokio::sync::Semaphore is already pulled in).
NO HTTP calls — T-010 wires the resilience primitives into the runner.
Verification: cargo test -p mcp-bridge --lib deepseek_resilience: 9/9 passed.
cargo check --workspace: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…Q-DS-007)
NEW module daemon/crates/mcp-bridge/src/deepseek.rs.
Single function: `pub fn build_client(cfg: &DeepSeekConfig) -> Result<reqwest::Client, reqwest::Error>`.
Plumbs the three timing knobs from DeepSeekConfig (T-002) into reqwest:
- .read_timeout(cfg.read_timeout) — per-read rolling idle limit
- .timeout(cfg.timeout) — absolute outer ceiling
- .tcp_keepalive(cfg.tcp_keepalive)
Module is intentionally narrow — ONLY the client-builder concern. It does NOT
construct request bodies (T-009/T-010), parse SSE (T-006), or read the API
key (the runner will inject Authorization per request). This keeps the test
matrix small: we only need to prove the rolling-read-timeout PROPERTY of the
reqwest configuration, not any wire content.
Reality tests (REQ-DS-007 contract):
1. deepseek_client_timeouts_rolling_read_succeeds_even_when_total_exceeds_read_timeout
- In-process TCP server dribbles 15 bytes at 200ms intervals (~3s total).
- Client: read_timeout=500ms, absolute timeout=10s.
- Inter-byte gap (200ms) < read_timeout (500ms) → rolling timer resets.
- Total wall-clock (3s) > read_timeout (500ms) → an absolute-timeout stub
would have failed at 500ms. Real reqwest completes the request.
- Stub guard: a build_client implementation that uses only `timeout`
would fail at 500ms; a stub that ignores read_timeout entirely would
pass this test but fail #2 below.
2. deepseek_client_timeouts_rolling_read_times_out_when_gap_exceeds_read_timeout
- Server dribbles 5 bytes with 800ms gaps.
- Client: read_timeout=200ms, absolute=10s.
- Inter-byte gap (800ms) > read_timeout (200ms) → first byte read MUST trip
the rolling timer.
- Asserts err.is_timeout() || err.is_body() — never silent success.
3. build_client_succeeds_with_defaultish_config — sanity-only; confirms the
reqwest builder doesn't reject our default-shape config.
No hyper dep — raw `tokio::net::TcpListener` + hand-written HTTP/1.1 is
sufficient since we control both sides. Added `reqwest.workspace = true`
to mcp-bridge dependencies, plus `net` + `io-util` tokio features so the
in-process test server compiles. Cargo.lock updated for the test-only
hyper/tower transitive pulls.
Verification: cargo test -p mcp-bridge --lib: 56/56 passed in 2.84s.
cargo check --workspace --all-targets: clean.
Wave 2 done. Unblocks T-006 (SSE parser) and T-010 (runner).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NEW: StreamParser, RawUsage, EmbeddedError, ParseEvent, ParseError in
daemon/crates/mcp-bridge/src/deepseek.rs.
The parser is stateful: `feed(&mut self, chunk: &[u8])` repeatedly with
arbitrary byte slices. Bytes buffer until `\n\n` (or `\r\n\r\n` for
CRLF-rewriting proxies) terminates an event. A JSON object split across N
chunks is reassembled correctly.
Surfaces (per the IMPL_PLAN contract):
- `: …\n\n` keep-alive comment lines → KeepAlive event + keepalive_count++
- `data: <json>\n\n` → parsed RawChunk → accumulators updated:
* delta.reasoning_content → reasoning_acc + ReasoningDelta{added_chars}
* delta.content → content_acc + ContentDelta{added_chars}
* usage → usage + Usage
* finish_reason → finish_reason
* id → request_id (first-seen)
* system_fingerprint → system_fingerprint (latest)
* top-level error → embedded_error + EmbeddedError (T-007 hook)
- `data: [DONE]\n\n` → done=true + Done event
- Invalid JSON → typed ParseError::InvalidJson { snippet (≤200 chars), cause }
- Non-UTF8 → typed ParseError::Utf8
What the parser MUST NOT do (and doesn't):
- Make retry / breaker decisions (the runner owns that)
- Combine reasoning_tokens into completion_tokens (T-009 owns the mapping;
the parser preserves the raw numbers including the completion_tokens_details
nested field)
- Network anything
Reality test (IMPL_PLAN contract):
`deepseek_parser_chunked_handles_arbitrary_boundaries` — feeds a 3-chunk
synthetic stream containing keepalive + reasoning + content + usage + DONE,
with one split DELIBERATELY landing mid-JSON inside the usage object's
"prompt_tokens" key. Asserts all accumulators are exact, usage numbers
including completion_tokens_details.reasoning_tokens are preserved, done=true,
embedded_error=None. A stub doing one serde_json::from_str on the whole
buffer would error on the mid-JSON split.
Stub-guard tests:
- ignores_comment_keepalives — 3 `:\n\n` events → keepalive_count==3, no deltas
- handles_byte_at_a_time_feed — every byte its own `feed` call → same result
(pathological boundary case)
- invalid_json_returns_typed_error — `data: {not-json}` surfaces typed err,
not silent swallow
- tolerates_crlf_line_endings — CRLF terminator (`\r\n\r\n`) detected by
find_event_terminator alongside LF-LF; SSE spec permits both
Deps added to mcp-bridge:
serde.workspace = true
serde_json.workspace = true
(reqwest brings them transitively but we declare them explicitly because we
own derive structs and call serde_json::from_str directly.)
Verification:
cargo test -p mcp-bridge --lib deepseek::tests::deepseek_parser: 5/5 passed.
cargo check --workspace: clean.
Unblocks T-007 (guards), T-008 (runaway), T-009 (finalize/usage map/log).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…029/030)
NEW types in daemon/crates/mcp-bridge/src/deepseek.rs:
- DeepSeekFailureKind (typed runner failure surface; will grow in T-008/T-010)
- BadFinishReasonKind (Length, ContentFilter, Missing, Unknown(String))
- FinalizedStream (clean-success result)
- pub fn finalize_stream(parser) -> Result<FinalizedStream, DeepSeekFailureKind>
- fn classify_embedded_error(err) -> Classification
REQ-DS-029 ghost-success: an SSE chunk may contain a top-level `error`
object inside an HTTP-200 envelope. The parser stashes it as
`embedded_error`; finalize_stream detects it BEFORE checking finish_reason
(an embedded error makes the stream a lie regardless of how it 'ends') and
returns GhostSuccessEmbedded. The embedded error is mapped through the same
Classification axis as HTTP status codes (REQ-DS-029 no-new-breaker-policy):
- rate_limit / 429 / overloaded / server_error markers → Transient
- everything else (insufficient_balance, auth, billing, unknown) → Hard
REQ-DS-030 finish_reason guard:
- 'stop' → Ok(FinalizedStream)
- 'length' → BadFinishReason(Length)
- 'content_filter' → BadFinishReason(ContentFilter)
- None (never received) → BadFinishReason(Missing)
- anything else → BadFinishReason(Unknown(string))
NEVER returns Ok(parsed) with partial content. The runner (T-010) decides
whether to retry on length specifically.
Tests (6 new):
- deepseek_guards_ghost_success_insufficient_balance_is_hard
Stream: reasoning chunk + embedded error chunk + stop chunk + DONE.
Stub guard: a finalize that returns Ok because finish_reason='stop' fails
— embedded error takes precedence.
- deepseek_guards_embedded_rate_limit_is_transient
Confirms Classification mapping for the other axis.
- deepseek_guards_finish_reason_length_is_bad
Asserts the parser DID accumulate partial content AND that finalize
rejects it — proves the 'partial-content fail-loud' contract.
- deepseek_guards_finish_reason_content_filter_is_bad
- deepseek_guards_missing_finish_reason_is_bad
Stream ends with [DONE] but no finish_reason was ever emitted.
- deepseek_guards_finish_reason_stop_returns_ok
Happy path with usage chunk. Without this the guards-above could be
passing because finalize always returns Err.
Verification:
cargo test -p mcp-bridge --lib deepseek::tests::deepseek_guards: 6/6 passed.
cargo check --workspace: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…028)
NEW on StreamParser:
pub fn estimated_reasoning_tokens(&self) -> i64
Returns reasoning_acc.chars().count() / 4 — matches the /4 factor used
by T-009's estimated-usage fallback (consistent across the module).
pub fn is_runaway(&self, cap: u32) -> bool
Returns true when cap > 0 AND estimate >= cap. cap = 0 ⇒ disabled
(matches DeepSeekConfig.reasoning_cap_tokens default per REQ-DS-015).
NEW variant on DeepSeekFailureKind:
RunawayReasoning { observed_tokens: i64 }
Contract (important enough to encode as a test):
The runner (T-010) calls is_runaway() between feed() invocations. When it
returns true, the runner drops the reqwest stream-future cleanly and
returns DeepSeekFailureKind::RunawayReasoning. Crucially, the runner MUST
NOT call breaker.record() on this failure — runaway is a BUDGET decision,
not a provider fault. Today's commit captures that contract as a positive
test: a fresh breaker, a synthetic RunawayReasoning failure, and an
assertion that breaker.state() stays Closed.
Tests (4 new):
- deepseek_runaway_trips_when_cap_exceeded
1000 reasoning chars + cap=100 → estimated=250, is_runaway=true.
Stub guard: a parser that ignores the cap (always false) or always
fires (true regardless) both fail.
- deepseek_runaway_does_not_trip_when_cap_is_zero
1000 reasoning chars + cap=0 → is_runaway=false. Validates the
default-disabled behaviour.
- deepseek_runaway_boundary_inclusive
cap=250 with observed=250 trips (>=); cap=251 does NOT trip. A
stub using strict > would skip this edge.
- deepseek_runaway_does_not_call_breaker_record
Cross-task contract: breaker stays Closed when a RunawayReasoning
failure is produced. Encodes T-010's required behaviour today.
Verification:
cargo test -p mcp-bridge --lib deepseek::tests::deepseek_runaway: 4/4 passed.
cargo check --workspace: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…log (REQ-DS-009/018/021/023/026)
NEW types in daemon/crates/mcp-bridge/src/deepseek.rs:
TokenUsage { input_tokens, output_tokens, cached_tokens, usage_source }
UsageSource::{Exact, Estimated}
PerRequestLogRecord<'a>
pub fn map_usage(raw: &RawUsage) -> TokenUsage
pub fn estimate_usage(bytes_received: i64, prompt_estimate: i64) -> TokenUsage
pub fn cap_reasoning(s: &str, cap_bytes: usize) -> &str
pub fn write_per_request_log(log_dir, record) -> io::Result<PathBuf>
pub fn now_rfc3339_utc() -> String
fn sanitize_for_filename(s: &str) -> String
(a) USAGE MAPPING (REQ-DS-009 / A-04b)
input_tokens ← prompt_cache_miss_tokens
output_tokens ← completion_tokens (NOT + reasoning_tokens)
cached_tokens ← prompt_cache_hit_tokens
The completion_tokens field from DeepSeek ALREADY includes the reasoning
budget. Adding completion_tokens_details.reasoning_tokens overstates output
and breaks cost math. Anchored against the Wave-0 probe-04 numbers.
(b) ESTIMATED FALLBACK (REQ-DS-026)
When the usage chunk is missing (dirty disconnect, runaway abort, bad
finish_reason), bytes_received/4 for output + prompt_estimate for input,
usage_source = Estimated. The runner accumulates bytes_received itself
during streaming.
(c) PER-REQUEST LOG (REQ-DS-018 / REQ-DS-023)
JSON file at $LOG_DIR/<request_id>.json with { request_id, model,
system_fingerprint, reasoning_content (UTF-8-safe truncated to
log_reasoning_cap_bytes — default 256KB), content, usage, cost_usd,
finish_reason, timestamp }.
PRIVACY GUARDS (REQ-DS-023 scope_out): the record shape FORBIDS the API key
and the request messages payload. The test asserts the file body does not
contain either the API key string or any 'messages' field name.
sanitize_for_filename() strips path separators from request_id so a
malicious id can't escape log_dir.
Tests (7 new, all green):
- deepseek_usage_map_no_double_add — anchor: completion_tokens=174 incl
120 reasoning → output=174, NOT 294. Stub guard: a mapping that adds
reasoning_tokens fails here.
- deepseek_usage_map_probe04_cost_anchor — feeds map_usage into the REAL
token_economics::calculate_cost_usd against seeded prices, asserts
cost ≈ $0.00015921 (±$0.0000001). Computes the would-be-bad
double-add cost explicitly and asserts the actual cost is far from
it — defends against a double-add slipping through.
- deepseek_usage_estimate_fallback_uses_bytes_over_4 — 800 bytes → 200
output tokens, source=Estimated.
- deepseek_cap_reasoning_preserves_utf8_boundary — 🦀 (4 bytes) cap at 3
must truncate cleanly (no panic on non-boundary slice).
- deepseek_per_request_log_writes_expected_fields_and_excludes_secrets —
privacy regression guard: log file must contain reasoning_content,
system_fingerprint, cost_usd, AND must NOT contain a sample API key
string, 'Authorization' header name, request messages text, or a
'messages' field.
- deepseek_per_request_log_sanitizes_filename — request_id with '/'
separators gets sanitised; resulting path is INSIDE log_dir.
- deepseek_per_request_log_records_estimated_source — usage_source
serialises as 'estimated' (lowercase per #[serde(rename_all)]).
NEW deps on mcp-bridge:
[dependencies] chrono (default-features=false + clock + serde — matches
the daemon's existing chrono shape)
[dev-dependencies] token-economics (path) + tempfile — test-only, lets
the probe-04 cost anchor invoke the REAL calculate_cost_usd
against the REAL price seed, no stubs.
Verification:
cargo test -p mcp-bridge --lib deepseek::tests: 25/25 passed.
cargo check --workspace: clean.
Unblocks T-010 (runner orchestrates parse → guards → finalize → log).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…008/014/024)
NEW public surface in daemon/crates/mcp-bridge/src/deepseek.rs:
pub struct RunRequest { messages, session_id, prompt_chars_estimate }
pub struct RequestMessage { role, content }
pub struct ResilienceState { breaker, concurrency }
pub struct DeepSeekFailure { kind, usage, bytes_received, request_id }
pub fn build_request_body(cfg, req) -> serde_json::Value
pub async fn run(cfg, client, req, resilience) -> Result<ParsedAgentResult, DeepSeekFailure>
DeepSeekFailureKind grew six runner variants:
HardProvider(u16) — classify()==Hard from a 4xx
Transient(u16) — classify()==Transient from 429/5xx
NetworkPreFirstByte(String) — connect/DNS/build error (already retried 1x)
NetworkMidStream(String) — bytes_stream() Err after first byte
AbsoluteTimeoutExceeded — outer tokio::time::timeout fired
BreakerOpen(BreakerState) — refused at acquire time
ParserError(String) — SSE malformed
Orchestration sequence:
1. Breaker.try_acquire — hard refuse if open
2. ConcurrencyCap.acquire — Arc<Semaphore> permit held for the call
3. tokio::time::timeout(cfg.timeout, run_inner) ← REQ-DS-024
4. Breaker.record(outcome) on result
- Success → Outcome::Success
- HardProvider(c) / GhostSuccess(Hard) → HardError(c | 402)
- Transient(c) / GhostSuccess(Transient) → TransientError(c | 429)
- NetworkPreFirstByte / NetworkMidStream
/ ParserError / AbsoluteTimeoutExceeded → TransientError(0)
- RunawayReasoning / BadFinishReason / BreakerOpen → NO-OP (budget/policy
decisions, not provider
faults — T-008 contract)
run_inner retry budget (REQ-DS-008: no sibling substitution; in-runner
retries scoped):
- Pre-first-byte network failure: at most 1 retry
- 429 with Retry-After: at most 1 honored wait (clamped to 10s so a
malicious header can't park the worker forever)
These don't share a counter so a 429 after a network-retry still gets
its own honored wait. Anything else: SINGLE outer attempt, no retry.
consume_stream:
- Iterates resp.bytes_stream(), accumulates bytes_received
- feeds chunks to StreamParser (T-006)
- on each loop checks parser.is_runaway(cfg.reasoning_cap_tokens) (T-008)
- on bytes-error → NetworkMidStream + estimated usage (T-009 fallback)
- on parser.done → exits, calls finalize_stream (T-007 guards)
- maps usage via map_usage (T-009) or estimate_usage on missing usage chunk
- writes per-request log via write_per_request_log (best-effort,
warn-not-fail; tracing::warn! on IO error)
- returns agent_adapter::ParsedAgentResult
Reality tests (the IMPL_PLAN contract — 6 new, all green):
(a) deepseek_runner_happy_path_returns_ok_with_parsed_result
canned reasoning→content→usage→[DONE] → Ok with response_text=='final
answer text', session_id=='deepseek-test-session-001', usage{18,174,0}
(NOT 18,294,0 — no double-add), breaker Closed.
(b) deepseek_runner_402_returns_hard_provider_and_latches_breaker
→ Err(HardProvider(402)) + breaker == HardOpenInsufficientBalance.
(c) deepseek_runner_429_with_retry_after_then_succeeds
First connection 429 + Retry-After:1, second connection 200 happy
stream → Ok + elapsed >= 900ms (proves the wait actually happened).
(d) deepseek_runner_mid_stream_disconnect_returns_network_mid_stream
Server sends Content-Length:100000 but only writes a partial body
then closes → Err(NetworkMidStream) with usage.usage_source==Estimated
and bytes_received>0.
(e) deepseek_runner_log_excludes_secrets_and_messages
Privacy regression for the runner path specifically: the per-request
log file written during run() must NOT contain the API key string,
the request messages text, or any 'messages' field name.
(f) deepseek_runner_absolute_timeout_returns_typed_failure
Server accepts but never responds; absolute timeout 500ms → typed
failure (AbsoluteTimeoutExceeded or NetworkPreFirstByte — either
indicates the SLA fired).
Scripted TCP server (spawn_scripted_server) accepts a Vec<Vec<u8>> of
canned HTTP/1.1 response bytes — one script per incoming connection. No
hyper / wiremock dep added.
NEW deps on mcp-bridge:
[dependencies] futures-util = '0.3' (StreamExt for bytes_stream)
Verification:
cargo test -p mcp-bridge --lib deepseek::tests::deepseek_runner: 6/6.
cargo test -p mcp-bridge --lib: 84/84.
cargo check --workspace --all-targets: clean.
Wave 3 COMPLETE. Unblocks Wave 4 (dispatch + per-call surface): T-011
AskAgentRequest fields, T-012 dispatch route, T-013 retry/Err-path
persistence, T-014 stateless enforcement, T-015 anti-bulk byte cap.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e 3 hardening)
Codex review of Wave 3 (T-006..T-010) found 1 BLOCKER, 6 SHOULD-FIX,
4 NITs. All addressed. 7 new regression tests; mcp-bridge: 91/91 pass.
BLOCKER — max_rpm was a dead knob
TokenBucket existed in T-004 but ResilienceState only carried breaker+
concurrency, so cfg.max_rpm did nothing. Fix:
- Added `rpm: Arc<Mutex<TokenBucket>>` to ResilienceState (initialised
from cfg.max_rpm).
- run() Phase 2: lock → try_take/next_available → drop lock → sleep
→ retry. Lock is NEVER held across .await.
Regression test: deepseek_runner_rpm_gate_waits_for_token (drain bucket,
then run() must wait ~1s for refill before issuing the HTTP call).
SHOULD-FIX #1 — Two absolute timeouts racing
build_client set reqwest .timeout(cfg.timeout) AND run() wrapped
run_inner in tokio::time::timeout(cfg.timeout). Failure typing was
nondeterministic — a hung request could surface as
NetworkPreFirstByte(reqwest timeout) instead of AbsoluteTimeoutExceeded.
Fix: removed reqwest .timeout() (read_timeout kept for SSE idle).
tokio::time::timeout is now the SINGLE owner of the absolute SLA.
Test deepseek_runner_absolute_timeout_returns_typed_failure tightened
to require AbsoluteTimeoutExceeded exclusively.
SHOULD-FIX #2 — is_runner_transient disagreed with breaker recording
run() recorded NetworkPreFirstByte + ParserError as TransientError but
is_runner_transient() excluded them. Fix: aligned both surfaces.
SHOULD-FIX #3 — Multi-line SSE `data:` events
Per the SSE spec, multiple `data:` lines in one event must be joined by
'\n' and dispatched as ONE payload. Previous code parsed each line as
independent JSON. A pretty-printing proxy emitting
data: {"choices":\ndata: [{...}]}\n\n
would have errored on the first line. Fix: collect data: payloads,
join, then [DONE]-check or JSON-parse once.
Regression test: deepseek_parser_joins_multiline_data_payloads.
SHOULD-FIX #4 — UTF-8 snippet truncation could panic
&payload[..200] would panic if byte 200 landed inside a multi-byte
char. Fix: route through the existing cap_reasoning() UTF-8-boundary
truncator. Regression test: deepseek_parser_invalid_json_snippet_is_utf8_safe
(forces 🦀 to straddle byte 200).
SHOULD-FIX #5 — Half-open lease consumed before concurrency permit
Old phase order: breaker.try_acquire (could consume the HalfOpen lease)
→ semaphore.acquire().await. Under saturation, a caller could hold the
half-open lease while waiting for the semaphore, blocking other
callers from probing. Fix: reordered to (1) semaphore, (2) RPM,
(3) breaker — so the breaker lease is consumed ONLY when the caller
has resources to actually fire the probe.
SHOULD-FIX #6 — Stream could return Ok without [DONE]
consume_stream finalized on socket-EOF whenever finish_reason==stop,
even if parser.done was false. Per the OpenAI-compatible streaming
contract, [DONE] IS the application-layer terminator — a missing
[DONE] means the response was truncated. Fix: require parser.done.
Added DeepSeekFailureKind::StreamEndedWithoutDone (transient class).
Regression test: deepseek_runner_stream_without_done_sentinel_returns_typed_failure.
NIT #1 — Retry-After only supported delta-seconds
Per RFC 7231, Retry-After is delta-seconds OR HTTP-date (RFC 1123).
Fix: parse_retry_after() tries u64 first, then chrono RFC 2822
(which accepts the IMF-fixdate shape). Clamp to 10s unchanged.
Regression test: parse_retry_after_handles_seconds_and_http_date.
NIT #2 — Filename sanitiser allowed Windows-reserved basenames
CON, PRN, AUX, NUL, COM1-9, LPT1-9 (incl. CON.txt) — plus empty,
'.', '..' — got through. Fix: post-clean check rewrites those to a
`req-<safe-slug>` form.
Regression test: sanitize_for_filename_rejects_windows_reserved_and_dot_cases.
NIT #3 — Privacy doc note on reasoning_content potentially echoing prompt
Added a paragraph to PerRequestLogRecord explaining model OUTPUT
(reasoning + content) can indirectly contain user-prompt fragments
if the model parrots them. Not a daemon-side leak; a deployment
consideration. Documented at the type definition.
NIT #4 — Tests used Content-Length, not chunked transfer
Reqwest's bytes_stream() is exercised either way, but DeepSeek's real
/v1/chat/completions uses Transfer-Encoding: chunked. Added
deepseek_runner_handles_chunked_transfer_encoding — hand-built
chunked HTTP/1.1 response, one chunk per SSE event, terminator
'0\r\n\r\n'. Runner returns Ok with the expected content.
Codex clean checks (no fixes needed, captured for the record):
- No API key leak path. Key only enters Authorization header in
run_inner; reqwest errors are URL/body-based, not header-debug.
- No std::sync::Mutex held across .await in run().
- Semaphore permit lifetime correct (released after breaker recording).
- map_usage preserves no-double-add invariant.
- cap_reasoning UTF-8 safe.
Verification:
cargo test -p mcp-bridge --lib: 91/91 passed (was 84; +7 regression).
cargo check --workspace --all-targets: clean.
Wave 3 hardened. Ready for Wave 4.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…AgentRequest (REQ-DS-027)
shared-types/src/lib.rs:
pub struct AskAgentRequest now derives Default and carries four new
optional fields (skip-serialize-on-None so the wire shape is unchanged
for Gemini/Codex callers):
deepseek_thinking: Option<DeepSeekThinking> (Enabled | Disabled)
deepseek_reasoning_effort: Option<DeepSeekEffort> (Low | Medium | High | Max | Xhigh)
deepseek_include_reasoning: Option<bool>
deepseek_max_tokens: Option<u32>
Both new enums #[derive(Serialize, Deserialize, JsonSchema, ...)] with
rename_all=lowercase so the wire form matches the DeepSeek API (and so
JsonSchema-derived MCP tool schemas expose them in the same shape).
Xhigh is accepted as a per-call value but the runner (T-012) will
collapse Xhigh → Max because the API treats them identically.
Backward compatibility (the entire point of REQ-DS-027):
- Gemini/Codex requests have always had {agent, message, cwd?, repo?, branch?}.
With #[serde(default, skip_serializing_if = "Option::is_none")] on
each new field, both DESERIALIZE (missing keys → None) and SERIALIZE
(None → omitted) are unchanged for those callers.
- Tests assert: a bare {agent:'gemini',message:'x'} deserialises with
all 4 deepseek_* == None; serialising it back produces a body that
contains NO 'deepseek_' substring.
#[derive(Default)] addition let all 13 existing AskAgentRequest call
sites adopt `..Default::default()` instead of needing four explicit
None lines each. Touched files (all explicit-literal sites):
triumvirate/src/main.rs (10 sites — handlers + tests)
mcp-tools/src/inter_agent.rs (1 site)
mcp-tools/src/gemini_query.rs (2 sites)
mcp-bridge/src/lib.rs (1 site)
Reality tests (3 new in shared-types):
- ask_agent_request_optional_deepseek_backward_compatible
{agent:'gemini',message:'x'} → all 4 deepseek_* fields None.
- ask_agent_request_optional_deepseek_populates_and_round_trips
Full deepseek payload (incl. thinking='disabled', effort='xhigh',
include_reasoning:true, max_tokens:512) deserialises, then round-trips
losslessly. Stub guard: a serializer that drops 'disabled' on round-trip
would fail. Also asserts the None-omission wire-shape stability.
- deepseek_effort_accepts_all_five_levels
All five string forms parse to the matching enum variant.
shared-types: 30/30 (was 27, +3).
mcp-bridge: 91/91 unchanged.
mcp-tools: 34/34 unchanged.
token-economics: 25/25 unchanged.
cargo check --workspace: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wires the ask_agent({agent:'deepseek'}) HTTP path through to the DeepSeek
runner. Before this commit the runner existed in isolation; after, the
public MCP/HTTP surface actually reaches it.
agent_exec.rs:
run_named_agent_with_session_and_model gains an optional 7th param
`req_overrides: Option<&AskAgentRequest>` carrying the four T-011
per-call deepseek_* overrides. Caller at line 442 passes Some(req);
the degraded-route hop at line 834 passes None (no upstream request
context for those paths).
New 'deepseek' match arm calls run_deepseek_agent, which:
1. Reads cfg+client+resilience from a process-static OnceLock
(deepseek_runtime — initialised on first use; returns anyhow
wrapping ConfigError if env validation fails, so a typo in
DEEPSEEK_MAX_TOKENS surfaces at the SECOND consult onward
instead of silently bypassing).
2. Overlays per-call overrides via cfg_with_overrides:
deepseek_thinking → ThinkingMode
deepseek_reasoning_effort → ReasoningEffort
(Low/Medium/High → High; Max/Xhigh → Max — collapse
matches the DeepSeek API's own mapping)
deepseek_max_tokens → cfg.max_tokens
3. Builds a synthetic session_id 'deepseek-<uuid>' (T-014 prefix)
and a RunRequest. Per-call include_reasoning flag is threaded
through to the runner.
4. Calls mcp_bridge::deepseek::run.
5. On Err, wraps the typed DeepSeekFailure in
DeepSeekFailureWrapper (impl Error) and converts via
anyhow::Error::new so T-013 can downcast and inspect the
typed `usage` field for persist-before-Err.
Testable inner: run_deepseek_with_runtime(cfg, client, resilience,
message, req_overrides) bypasses the OnceLock so tests inject their
own mock runtime. Production callers go through run_deepseek_agent.
mcp-bridge/src/deepseek.rs (RunRequest extension):
+ #[derive(Default)] so non-runner-test sites can use ..Default::default()
+ include_reasoning: bool field
consume_stream now takes include_reasoning. When true AND reasoning is
non-empty, response_text is
'<reasoning>\n{r}\n</reasoning>\n\n{c}'
so the caller sees the trace IN-LINE without having to read the
per-request log file. Default false: response_text = content only,
reasoning persisted to the per-request log (REQ-DS-023 default).
Tests (4 new in agent_exec::deepseek_dispatch_tests):
- deepseek_dispatch_default_response_is_content_only
Spins up a scripted mock SSE server emitting reasoning+content+
usage+DONE. Dispatch returns 'clean final answer' as response_text,
NOT 'the model is thinking…' (reasoning excluded). session_id
starts with 'deepseek-'. The per-request log file is written and
DOES contain the reasoning. Stub guard: a dispatch always
including reasoning fails the response-text assertion.
- deepseek_dispatch_include_reasoning_true_returns_reasoning_in_response
Same server; include_reasoning=true. response_text contains the
reasoning AND the content AND the '<reasoning>' wrapper tags.
- cfg_with_overrides_collapses_effort_levels
All five Effort variants collapse correctly (Low/Med/High → High,
Max/Xhigh → Max). Thinking + max_tokens overrides work. None ⇒
base cfg preserved.
- deepseek_arm_exists_and_does_not_displace_gemini_or_codex
Cross-task regression — is_supported_agent_name still returns true
for gemini/codex/deepseek and false for unknown.
Wave 4 status: T-011 ✓, T-012 ✓. T-013/T-014/T-015 next.
Verification:
cargo test -p triumvirate --bin triumvirate deepseek_dispatch: 4/4.
cargo test -p mcp-bridge --lib: 91/91 unchanged.
cargo check --workspace --all-targets: clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eepSeek typed failure (REQ-DS-006/010/026)
Two surgical changes in execute_ask_agent, BOTH narrowly gated to
agent=='deepseek' so Gemini/Codex paths are untouched (REQ-DS-008
blast-radius safeguard).
1. attempt_schedule for deepseek = SINGLE attempt.
The runner already owns its scoped in-flight retries (pre-first-byte
network ×1; 429-with-Retry-After ×1). Adding the outer 3-attempt loop
would violate REQ-DS-008 (no sibling substitution) AND double-bill on
429 (each outer retry triggers a fresh in-runner Retry-After honor).
2. Persist-before-Err hook in the Err branch.
When the runner returns Err(anyhow wrapping DeepSeekFailureWrapper),
downcast to the typed failure and — IF agent=='deepseek' AND
wrapper.usage.is_some() — persist a token record before propagating the
Err. This rescues billable tokens on:
- HardProvider(402) — usage from finalize when streamed
- Transient(429) — usage from finalize when streamed
- NetworkMidStream(_) — usage from estimate_usage fallback
- StreamEndedWithoutDone — usage from finalize when present
- BadFinishReason(_) — usage from finalize when streamed
The usage_source field on the record mirrors the runner's classification
(Exact when the usage chunk arrived; Estimated when bytes/4 fallback fired).
NEW production fn:
pub(crate) fn persist_deepseek_err_tokens(request_id, fallback_session_id,
&mcp_bridge::deepseek::TokenUsage, Option<ds_request_id>, &resolved_cwd,
&resolved_repo)
Maps mcp_bridge::deepseek::UsageSource → token_economics::USAGE_SOURCE_*.
Prefers the DeepSeek-provided request_id as the session_id so the per-
request log file is cross-referencable; falls back to the daemon's
synthetic 'deepseek-err-<request_id>' otherwise.
No-ops cleanly when process_token_db() is uninitialised (safe under tests).
Tests (3 new in agent_exec::deepseek_dispatch_tests):
- deepseek_attempt_schedule_is_single_attempt
Canary for the convention: deepseek attempt count == 1, codex remains
on the generic 3-attempt schedule. A regression that removes the
deepseek branch (falls through to generic) fails this.
- persist_deepseek_err_tokens_safe_with_either_usage_source
Calls the helper with both UsageSource::Exact and ::Estimated shapes.
No panic = success under the no-DB safety property.
- persist_deepseek_err_path_is_gated_to_deepseek_agent_only
SOURCE-LEVEL regression guard. Scans agent_exec.rs (production scope,
excluding the test module) for every CALL to persist_deepseek_err_tokens
and asserts each is preceded — within 400 chars — by the gate
`if agent == "deepseek"`. A future change that hoists the call out
of the conditional (or that adds a Codex/Gemini call site) fails here.
This is the test the IMPL_PLAN's regression-guard contract demands.
Verification:
cargo test -p triumvirate --bin triumvirate deepseek_dispatch: 7/7.
cargo test -p mcp-bridge --lib: 91/91 unchanged.
cargo check --workspace --all-targets: clean.
Wave 4: T-011 ✓, T-012 ✓, T-013 ✓. T-014 + T-015 remain.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…e cap (REQ-DS-011/012/020/025)
T-014 (REQ-DS-020) stateless single-turn:
agent-worker/src/lib.rs:
require_reused_worker(agent='deepseek', cwd) — TWO new branches gated to
agent=='deepseek' (gemini/codex behaviour unchanged):
(1) worker entry absent → mint a stub WorkerState with session_id=None
and ask_count=1, insert it, return Ok. The dispatch layer never
sends a resume token, so an absent worker is the EXPECTED state for
the first DeepSeek consult.
(2) worker entry present but session_id is None → still return Ok
(incrementing ask_count). The session_id stays None forever — the
runner mints a synthetic 'deepseek-<uuid>' per call (T-012/T-014)
and the worker registry never sees it.
Net: callers that depend on require_reused_worker (e.g. an MCP tool
fronting a 'use existing session' guard) no longer treat DeepSeek's
honest statelessness as a missing-session error.
agent_exec.rs run_named_agent_with_session_and_model:
The deepseek arm already (T-012) generates 'deepseek-<uuid>' fresh per
call and never consults inbound session_id. T-014 adds an explicit
'_ = session_id' at the top of the function to silence unused-arg
warnings and document the intent — deepseek IGNORES resume tokens.
prewarm_daemon_workers (unchanged from before, asserted by test):
Only spawns 'gemini' + 'codex' prewarm workers. NO 'deepseek' slot.
Spawning would be wasted work (every consult is stateless) and would
burn the API key on an essentially-empty 'reply with: ready' probe.
T-015 (REQ-DS-011, REQ-DS-012, REQ-DS-025) anti-bulk + entry guards:
agent_exec.rs execute_ask_agent entry:
After is_supported_agent, BEFORE any worker acquisition, if
agent=='deepseek' AND req.message.len() > deepseek_bulk_bytes_cap() →
return Err with both 'payload too large' AND 'metered' in the message
so callers see the cost vector explicitly. Gemini/Codex are local CLIs
(bulk is free) — the check is agent-gated.
NEW helper deepseek_bulk_bytes_cap():
Reads TRIUMVIRATE_DEEPSEEK_BULK_BYTES; default 16384 (16KB). Invalid /
zero values fall back to the default (the DeepSeekConfig loader is the
authoritative validator — this helper exists so the cap is readable
without triggering full config load + OnceLock init for every consult).
Tests (5 new in agent_exec::deepseek_dispatch_tests):
- deepseek_stateless_distinct_session_ids_across_calls
Two sequential consults against the scripted mock server produce
DIFFERENT session_ids, both starting with 'deepseek-'. A stub that
treats the absence of a remote session as failure on the second call
would error out — instead Ok with a fresh uuid. (T-014)
- deepseek_prewarm_slot_is_a_safe_no_op
Source-grep on prewarm_daemon_workers's body asserts the function
contains prewarm_worker('gemini', …) AND prewarm_worker('codex', …)
AND DOES NOT contain prewarm_worker('deepseek', …). (T-014)
- deepseek_anti_bulk_rejects_oversized_payload
20KB payload + default 16KB cap → execute_ask_agent returns Err
containing 'payload too large' AND 'metered'. (T-015 reality (a))
- deepseek_anti_bulk_does_not_apply_to_gemini_or_codex
Source-grep regression: the production-side intercept
`req.message.len() > cap` is preceded within 300 chars by the
`if agent == "deepseek"` gate. (T-015 reality (b))
- deepseek_anti_bulk_cap_is_env_configurable
TRIUMVIRATE_DEEPSEEK_BULK_BYTES=32768 + 20KB payload → the error
surfaced does NOT contain 'payload too large' (the request proceeds
past the intercept). (T-015 reality (c))
- deepseek_path_has_no_auto_routing
Source-grep: every '"deepseek"' branch in production code has no
'auto_route' or 'router.route' in the surrounding ±500 chars.
(T-015 reality (d) / REQ-DS-011)
- deepseek_path_has_no_sandbox_init
Source-grep: neither mcp-bridge/src/deepseek.rs nor the
run_deepseek_agent body contains 'sandbox_init', 'init_sandbox',
or 'ensure_sandbox'. (T-015 reality (e) / REQ-DS-012)
Env-mutating tests share a static Mutex<()> (BULK_ENV_LOCK) so
parallel cargo-test execution doesn't race on
TRIUMVIRATE_DEEPSEEK_BULK_BYTES.
Verification:
cargo test -p triumvirate --bin triumvirate deepseek: 14/14 passed
cargo test -p mcp-bridge --lib: 91/91 unchanged
cargo test -p shared-types: 30/30 unchanged
cargo check --workspace --all-targets: clean
Wave 4 COMPLETE. T-011 ✓, T-012 ✓, T-013 ✓, T-014 ✓, T-015 ✓.
Pre-existing ABE bug (filed dc89676) remains the single workspace test
failure — not a Wave-4 regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…-DS-017/018) Wave 5 CLOSES the integration. T-016 (REQ-DS-017) — live API contract re-verification. Re-ran the 8-probe Wave-0 contract battery against api.deepseek.com on the funded account, with the FULL T-001..T-015 integration in place (HEAD at re-run = 313dd68, the Wave 4 closer). Result: 8/8 GREEN, no contract drift between Wave 0 and Wave 5. Total spend across both verification runs ≈ $0.02 against the funded account. PROBE_RESULTS.md gains a new section ('T-016 (Wave 5) re-run') with: - Captured stdout from the re-run (verbatim PROBE-01..PROBE-08 lines) - Cross-check table comparing Wave-0 vs Wave-5 numbers per probe - Integration ground-truth mapping (which REQ depends on which probe passing): REQ-DS-009/019/029/030/005/024 - Spending audit (~$0.02 for 16 calls across both runs) - Green-light line: 'cleared for merge to main' Invocation reproduced verbatim: TRIUMVIRATE_DEEPSEEK_API_KEY=<key> \ cargo test -p triumvirate --test deepseek_contract -- --ignored --nocapture T-017 (REQ-DS-018) — operator runbook. NEW file daemon/docs/deepseek-operator-runbook.md (~340 lines, 11 sections). Mirrors the agy-operator-runbook.md structure. The IMPL_PLAN demands a grep -cE check covering 9 required topics; this doc has 12 matches across the required pattern set. NO API key embedded (regression-grep confirmed clean). Sections: 1. One-time setup 1.1 Account must be funded (REQ-DS-017) 1.2 Balance monitoring via GET /user/balance 1.3 Configure the API key 1.4 Smoke test 2. Operating modes (disabled / enabled / per-call overrides REQ-DS-027) 3. Resilience behavior 3.1 Three-state breaker (REQ-DS-006/010) 3.2 Peak-hour 503 expectation (02:00-14:00 UTC) 3.3 Breaker tuning 3.4 Other in-runner safety mechanisms (read_timeout, abs timeout, concurrency cap, RPM cap, runaway cap, anti-bulk cap) 4. Observability 4.1 Per-request log file (REQ-DS-018/023) + system_fingerprint capture 4.2 Privacy guarantees baked into the log shape 4.3 Token economics (REQ-DS-009) 4.4 Price seed 5. Data-egress (the China-routed note + the do-NOT list) 6. CoT bifurcation (REQ-DS-023) 7. Environment variables (all 15 knobs documented) 8. Probe battery — the verification invocation (T-016 → CI link) 9. Key rotation — procedure + compromise response 10. Rollback (unset key + daemon restart) 11. Quick reference table Verification: grep -cE 'Account must be funded|GET /user/balance|env knob| peak-hour 503|system_fingerprint|data-egress|breaker tuning| probe battery|key rotation' deepseek-operator-runbook.md → 12 (IMPL_PLAN requires >= 9) ✅ grep 'sk-[0-9a-f]{20,}' deepseek-operator-runbook.md → no matches (API key NOT embedded — scope_out enforced) ✅ Wave 5 status: T-016 ✓, T-017 ✓. DeepSeek integration is COMPLETE — all 18 tasks across 6 waves done. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Closes the 8 cheap/local gaps surfaced by the post-Wave-5 research pass
(canonical DeepSeek docs + Codex sanity-check + web research). All changes
are additive — no Wave 1–5 behavior altered, no API spend, runner is
strictly more typed about edge cases that were previously silent.
NEW typed variants:
DeepSeekFailureKind::EmptyFinalAnswer { had_reasoning } (B.5)
finish_reason=stop with content=='' is now a typed failure, not
Ok(empty). Breaker-neutral (model declined to answer, not provider
fault).
BadFinishReasonKind::InsufficientSystemResource (B.6)
DeepSeek-documented finish_reason. Promoted from generic Unknown so
monitoring can distinguish capacity-driven failures. Classified as
transient via is_runner_transient + runner's breaker recording —
eligible for retry policy.
NEW production helpers:
record_fingerprint_for_model(model, fp) -> Option<String> (B.4)
Process-static per-model tracker. Emits tracing::warn! when the
fingerprint changes for a known model — DeepSeek doesn't announce
backend rollovers; this is the only signal. Wired into
consume_stream's post-finalize block.
NEW observability:
Cache-token invariant warn in consume_stream (B.3)
Per DeepSeek docs, prompt_tokens == prompt_cache_hit_tokens +
prompt_cache_miss_tokens. When the invariant breaks (their billing
model shifted, or a chunk is corrupt), log a structured warn with
the three numbers. Best-effort; consult succeeds.
Tests (8 new, all green):
B.1 b1_request_body_omits_sampling_params_that_thinking_mode_ignores
Locks in: build_request_body never emits temperature / top_p /
presence_penalty / frequency_penalty. Thinking mode silently
drops them per DeepSeek docs; we don't pretend we passed them.
B.2 b2_default_thinking_mode_is_enabled_even_for_flash
Encodes current behaviour. A future change that defaults flash
to thinking=disabled (test-plan recommendation) will fail this
and force a deliberate update.
B.3 b3_cache_invariant_violation_warns_but_does_not_fail_consult
Streams usage with broken hit+miss==prompt invariant. Consult
succeeds; usage mapped from miss/hit directly (coherent for
downstream cost math); warn surfaces via tracing.
B.4 b4_fingerprint_change_detector_tracks_per_model
Unit tests record_fingerprint_for_model directly: first
observation returns None; repeat returns Some(same); CHANGE
returns Some(old) and emits the warn internally.
B.5 b5_empty_content_with_finish_reason_stop_is_typed_failure
Reasoning-only stream with finish_reason=stop → EmptyFinalAnswer
with had_reasoning=true. Breaker stays Closed.
B.6 b6_insufficient_system_resource_finish_reason_is_transient
Stream with finish_reason='insufficient_system_resource' →
BadFinishReason(InsufficientSystemResource) AND
is_runner_transient() returns true.
B.7 b7_clean_stop_and_done_without_usage_chunk_estimates_usage
Clean stop+DONE, no usage object/chunk → Ok with usage_source
Estimated populated from bytes/4.
B.8 b8_anti_bulk_cap_measures_message_len_only
Documents the contract: a RunRequest with a 50KB session_id is
accepted (the 16KB cap lives in execute_ask_agent on
message.len() only — outside this crate).
DOCS:
daemon/docs/v1-deepseek/DEEPSEEK_TEST_PLAN.md (new, ~360 lines)
Sections A–E covering: canonical facts not exercised by probes,
20+ tiered test scenarios (free → expensive → destructive),
code-generation prompting playbook for V4-Pro (CO-STAR + XML +
cache-friendly prompt shape), operational watch list, known
unknowns Codex couldn't verify, suggested next actions.
Cited against DeepSeek's own docs + multiple third-party sources.
Verification:
cargo test -p mcp-bridge --lib: 99/99 passed (was 91; +8)
cargo test -p triumvirate --bin triumvirate deepseek_dispatch: 14/14
cargo check --workspace --all-targets: clean
No new crate deps. No env-knob changes. No behavior change for the
happy path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ns, not flat string (CRITICAL)
Live TIER-2 probe against api.deepseek.com on 2026-05-26 surfaced a
production-blocking bug missed by Wave 0–5 verification:
build_request_body sent {"thinking": "enabled"} (flat)
API rejects with HTTP 400 (invalid_request_error):
Failed to deserialize the JSON body into the target type:
thinking: invalid type: string "enabled", expected struct ThinkingOptions
Correct shape is {"thinking": {"type": "enabled"|"disabled"}}
Pre-fix impact: EVERY production consult through mcp_bridge::deepseek::run()
would have failed at the wire. The 8 Wave-0/5 contract probes passed because
they hand-craft their request JSON (already nested); they don't go through
build_request_body. The unit tests in deepseek.rs passed because the
scripted mock server returns canned responses regardless of request shape.
Fix:
build_request_body now emits:
"thinking": { "type": cfg.thinking.as_api_str() }
reasoning_effort stays flat — confirmed accepted by the live API.
NEW regression guards:
1. crates/mcp-bridge/src/deepseek.rs:
b9_thinking_wire_shape_is_nested_object_not_flat_string
- Asserts body["thinking"] is a JSON object with a "type" field
- Greps the serialized body for the FLAT form strings and rejects them
- Tests both Enabled and Disabled modes
2. crates/triumvirate/tests/deepseek_contract.rs:
probe_09_runner_end_to_end_against_live_api
#[ignore]-gated end-to-end live probe — calls mcp_bridge::deepseek::run()
against api.deepseek.com (NOT a mock server). Any future change that
reverts the wire shape fails here with HTTP 400.
3. b2 test updated: now asserts body["thinking"]["type"] == "enabled"
(nested object shape) instead of the broken flat-string assertion.
Verification:
cargo test -p mcp-bridge --lib: 100/100 (was 99; +1 b9 regression)
cargo test -p triumvirate --test deepseek_contract probe_09 -- --ignored
PROBE-09 OK: response="ok" usage=input:9/output:39/cached:0
Live probe cost: ~$0.002
Docs updated:
daemon/docs/v1-deepseek/PROBE_RESULTS.md gains a PROBE-09 section with
the side-by-side curl output, root cause, fix description, and the
permanent lesson: unit tests against mock servers do NOT verify wire
correctness against the real API. Every module that builds an HTTP
body should have at least one #[ignore]-gated live probe through that
exact builder.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…alibration Five validator probes against the live API on 2026-05-26. Total spend sub-cent. Net code changes from TIER 2: ZERO. The findings are operator-facing — the runner is correct, but two assumptions need calibrating in operational planning. B.10 — usage + cost math ✅ PASS Single v4-pro consult: prompt=8 miss=8 hit=0 completion=34 (incl reasoning=32). Computed cost $0.0000331. Confirms map_usage's no-double-add invariant works end-to-end on the live wire (output is reported completion_tokens, NOT + reasoning_tokens). Cache-token invariant hit+miss==prompt held — our B.3 warn would not fire on normal traffic. B.11 — cache hit on identical repeat⚠️ behavior NOT what docs implied Sent identical 76-token prompt twice (above the documented 64-token chunk threshold), 2 seconds apart. BOTH calls reported zero cache hits (hit=0 miss=76 each time). The cache IS best-effort per DeepSeek's own docs — this just empirically demonstrates that even identical prompts don't reliably hit. Cost-savings story is weaker than the implied 120× discount suggests. Operators must NOT plan budgets assuming cache hits. B.12 — single-byte prefix change⚠️ inconclusive Hit=0 miss=77 on the +1-char variant — but with no B.11 cache to invalidate, this can't confirm prefix-sensitivity. Code-review the prompt structure for non-deterministic prefix bytes instead. B.13 — large prompt (5KB, 1216 tokens) ✅ PASS No silent truncation. 960ms wall-clock. Model correctly addressed the embedded instruction. B.14 — max_tokens low + thinking → empty content⚠️ worst-case bill surprise max_tokens=30 + thinking=enabled → finish_reason=length, content='', completion_tokens=30 (ALL reasoning). Caller paid for 30 output tokens, got nothing usable. Our T-007 BadFinishReason::Length catches it as a typed failure. Mitigation is operator-side: set max_tokens high enough to cover both reasoning AND content. Suggested floors with thinking enabled added to the operator runbook (§3.5): trivial answer ≥256, non-trivial ≥1024, complex ≥4096. Files touched: daemon/docs/v1-deepseek/PROBE_RESULTS.md + 'TIER 2 sweep — B.10 → B.14' section with full results table, cost math computation, and the inconclusive flag on B.12. daemon/docs/deepseek-operator-runbook.md + new §3.5 'Operator findings from the 2026-05-26 live TIER-2 probes' with the two calibration notes (cache opportunism + max_tokens floor). Verification: All five probes ran live against api.deepseek.com. Total spend sub-cent (balance unchanged $9.99 → $9.99 at 2dp display precision). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…council Closes the pre-existing ABE bug filed at dc89676, which was the sole remaining workspace test failure across all of Wave 1-5 development. Root cause: post_exit_validator's stub_patterns list (["todo!()", "unimplemented!()", "TODO", "FIXME", "XXX", "HACK", "NotImplementedError", "placeholder", "not implemented", "implement me"]) did NOT match the red-team test's intentional stub marker `// pending: stub`. Pattern-set drift between what the test red-teamed and what the validator caught — neither side knew about the other. Fix: Add "// pending" and "// stub" (comment-prefixed) to stub_patterns. Decision process — 3-agent council eating our own dog food: We just shipped DeepSeek as the 4th sibling (PR #37). Before that PR merges, the daemon running locally still uses the pre-merge binary, so ask_agent({agent:'deepseek'}) via MCP isn't wired. Instead we hit api.deepseek.com directly via curl with the same prompt we sent to Codex and Gemini via the existing mcp__triumvirate__ask_agent tool. All three agents shown: - The test code (stub_script writes '// pending: stub') - The validator code (the 10-element stub_patterns list) - The constraint set (no regex dep, identifiers like pending_orders shouldn't false-positive, perf matters because runs on every commit) - Five options (A-E) ranked by them Convergent verdict — ALL THREE picked the SAME approach: add comment-prefixed patterns to stub_patterns. The only divergence was the colon: DeepSeek v4-pro: "// pending" + "// stub" (no colon) Codex: "// pending" + "// stub" (no colon, matches DeepSeek) Gemini: "// pending:" + "// stub:" (with colon, narrower) 2-of-3 (DeepSeek + Codex) won. The colon would have over-narrowed — realistic markers like `// pending implementation` should also trip the validator, and bare `// pending` is still narrow enough to avoid the legit-identifier false positives Gemini was concerned about. Total council cost: ~$0.005 across DeepSeek ($0.0016) + Gemini + Codex (subscription, no per-call $). The 3-agent council was the same shape the integration spec was hardened with during the Wave 3 Codex review — this is now a repeating pattern. Verification: - cargo test --bin triumvirate -- tests::abe_red_team_enforcement_blocks_non_compliant_worker --exact: PASSES - cargo test -p triumvirate --bin triumvirate: 172/172 (was 171/172) - All Wave 1-5 + B-fix-tests still green Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI ran cargo clippy --workspace --exclude pantheon -- -D warnings and
failed on five lints in code introduced by this PR. All fixes are
mechanical/idiomatic — no behavior change.
Lint 1: redundant_closure (deepseek_config.rs:234)
- .unwrap_or_else(|| default_log_dir())
+ .unwrap_or_else(default_log_dir)
Lint 2: while_let_loop (deepseek.rs StreamParser::feed)
Converted `loop { let Some(...) = find_event_terminator() else { break }; … }`
to `while let Some(...) = find_event_terminator() { … }`. Same semantics.
Lint 3: collapsible_if (deepseek.rs consume_data_chunk)
- if let Some(id) = chunk.id { if self.request_id.is_none() { … } }
+ if let Some(id) = chunk.id && self.request_id.is_none() { … }
Lint 4: io_other (deepseek.rs write_per_request_log)
- .map_err(|e| io::Error::new(io::ErrorKind::Other, e))
+ .map_err(io::Error::other)
Rust 1.74+ idiom.
Lint 5: collapsible_if (deepseek.rs record_fingerprint_for_model — B.4)
Same shape as Lint 3 — the fingerprint-change detector's nested
`if let Some(ref old) = prev { if old != fingerprint { … } }`
collapses to a let-chain.
Also removed: reset_fingerprint_tracker_for_tests (dead code; its body
already noted it couldn't reach the production OnceLock — vestigial helper
that never worked and would have triggered an unused-fn warning).
Also fixed in #[cfg(test)] code (caught by --all-targets, although CI
only runs default targets — fixing for completeness):
- useless_format: format!() with no interpolation → b"...".to_vec()
- manual_range_contains: x >= 8 && x <= 11 → (8..=11).contains(&x)
Verification:
cargo clippy --workspace --exclude pantheon -- -D warnings: CLEAN (matches CI)
cargo test -p mcp-bridge --lib: 100/100 pass
cargo check --workspace --exclude pantheon: clean
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
GitHub Actions cached an old SHA for dtolnay/rust-toolchain@stable that codeload.github.com can no longer serve. Fresh workflow trigger should re-resolve @stable to the current HEAD.
michaeljboscia
added a commit
that referenced
this pull request
May 26, 2026
…ey-loud-fail fix(deepseek): fail-loud on missing API key + on-disk key fallback (closes deployment gap from #37)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds DeepSeek as a first-class Triumvirate sibling agent alongside Gemini and Codex. 24 commits across 6 waves following the goatrodeo
daemon/docs/v1-deepseek/IMPLEMENTATION_PLAN.mdexecution contract.What ships
From
ask_agent({agent: "deepseek", message: "..."})end-to-end:is_supported_agent_name("deepseek")returns true (T-001)acquire_workermints a stub stateless worker (T-014, REQ-DS-020)run_deepseek_agent(T-012, REQ-DS-014)DeepSeekConfig::from_env()loads 15 env knobs, redacted-DebugApiKey(T-002, REQ-DS-002/003/015)ResilienceStategates: breaker, RPM token bucket, concurrency semaphore (T-004, REQ-DS-006/010)reqwest::Clientwith rolling read_timeout (T-005, REQ-DS-007)thinking: {type: ...}shape (B.9 critical fix — pre-fix the API would 400 every call)data:join (T-006, REQ-DS-019)InsufficientSystemResource+EmptyFinalAnswer(T-007 + B.5/B.6, REQ-DS-029/030)response_text= content-only by default,<reasoning>…</reasoning>wrap whendeepseek_include_reasoning: truedeepseek-<uuid>session ID per call, no resume tokens (T-014)DeepSeekFailurepreserved; token record persisted before bubbling (T-013, REQ-DS-026)system_fingerprintchange detection (B.4) — DeepSeek doesn't announce rollovers; fingerprint drift is the only signalVerification
Total unit/integration tests: 351+ green. The one workspace failure (
tests::abe_red_team_enforcement_blocks_non_compliant_worker) is a pre-existing bug filed atdc89676— confirmed pre-Wave-1 by reproducing onf529123. Not a regression from this branch.Codex peer review of Wave 3 (the SSE pipeline + runner) returned 1 BLOCKER + 6 SHOULD-FIX + 4 NITs. All addressed in commit
8061e0b. The blocker wasmax_rpmbeing a dead knob; the worst SHOULD-FIX was a double-timeout race.Live API verification (
api.deepseek.com, funded account):Total spend across all live verification runs: ~$0.02 against the funded account.
Critical bug caught + fixed mid-PR
B.9 (commit
aae4eb4): the runner'sbuild_request_bodywas emitting"thinking": "enabled"(flat string) — the API rejects this with HTTP 400invalid_request_error. The correct shape is"thinking": {"type": "enabled"|"disabled"}(nestedThinkingOptionsstruct). Pre-fix, every production consult throughrun()would have failed at the wire. The Wave 0–5 unit tests passed because they use scripted mock servers that return canned responses regardless of request shape. The 8 Wave-0/5 contract probes passed because each probe hand-crafts its own JSON (already nested) — they don't exercisebuild_request_body.Surfaced by running TIER-2 B.9 ("thinking-disabled wire-shape verification"). Permanent regression guards now in place:
b9_thinking_wire_shape_is_nested_object_not_flat_string(unit, mcp-bridge)probe_09_runner_end_to_end_against_live_api(integration, #[ignore]-gated)Generalized lesson captured in
PROBE_RESULTS.md: unit tests against mock servers do NOT verify wire correctness against the real API. Every module that builds an HTTP body should have at least one#[ignore]-gated live probe through that exact builder.Operator carry-overs
Documented in
daemon/docs/deepseek-operator-runbook.md§3.5:max_tokensmust cover reasoning + content. TIER-2 B.14 showedmax_tokens=30 + thinking=enabled = finish_reason=length, content=''— paid for 30 reasoning tokens, got nothing usable. Suggested floors: ≥256 trivial, ≥1024 non-trivial, ≥4096 complex.ensure_deepseek_pricesseed will overstate costs by ~4× until re-seeded.Files of interest for reviewers
daemon/crates/mcp-bridge/src/deepseek.rs— the runner (1600+ lines, the heart of the integration)daemon/crates/mcp-bridge/src/deepseek_resilience.rs— breaker, semaphore, token bucket, classify()daemon/crates/mcp-bridge/src/deepseek_config.rs— env loader + redactedApiKeydaemon/crates/triumvirate/src/agent_exec.rs— dispatch arm + persist-before-Err + anti-bulkdaemon/docs/v1-deepseek/IMPLEMENTATION_PLAN.md— the spec we executed againstdaemon/docs/v1-deepseek/PROBE_RESULTS.md— Wave 0 + Wave 5 + PROBE-09 + TIER-2 resultsdaemon/docs/v1-deepseek/DEEPSEEK_TEST_PLAN.md— Section B test scenarios + Section C V4-Pro prompting playbookdaemon/docs/deepseek-operator-runbook.md— full operator runbook (~340 lines)🤖 Generated with Claude Code