mirror of
https://github.com/RightNow-AI/openfang.git
synced 2026-07-30 14:49:10 +00:00
Follow-up to 79aa34c. The previous commit added the public surface
(DriverConfig field + OPENFANG_SUBPROCESS_TIMEOUT_SECS env var) but
left every DriverConfig construction site hardcoded to None — so the
struct field was wired but had no on-disk source feeding it. The env
var was the only operator-facing knob.
This commit plumbs the missing layer: the timeout is now deserializable
from config.toml on both the primary and global-fallback providers.
Public surface
- DefaultModelConfig.subprocess_timeout_secs: Option<u64>
- FallbackProviderConfig.subprocess_timeout_secs: Option<u64>
- Both fields are #[serde(default)] — existing config.toml files
without the field deserialize cleanly to None (no breaking change).
Placement rationale
- Per-provider on each config struct, not a top-level field or a new
[driver] section. This matches the existing per-provider config shape
and lets operators set different timeouts for primary vs. fallback
(e.g. tighter timeout on a fast fallback to fail over sooner). If a
second driver-level setting ever lands, refactoring two struct fields
into a [driver] section is cheap; we don't pre-pay for it now.
Wiring (kernel.rs)
- L663 primary driver ........ pulls config.default_model.subprocess_timeout_secs
- L687 auto-detect path ...... inherits default_model intent (the swap
is replacing the *provider*, not the
timeout policy)
- L736 global fallback loop .. pulls fb.subprocess_timeout_secs
- L5031 agent primary ......... inherits effective_default's value when
agent_provider == default_provider;
None for cross-provider overrides
- L5108 agent manifest fallback inherits dm's value when the manifest
fallback resolves to "default" (matching
the existing fb.provider sentinel logic);
None for explicit cross-provider entries
- L5139 global fallback (per-agent loop) — pulls fb.subprocess_timeout_secs
Sites kept as None (intentional)
- agent_loop.rs:1146, 1330: ModelNotFound recovery iterates over the
agent manifest's fallback_models (FallbackModel, not the config-toml
type) — no per-provider config in scope.
- routes.rs:7701: provider connectivity test endpoint; no config source.
- routes.rs:7529: dashboard hot-update path constructs a fresh DM with
defaults (None) — operator sets timeout via config.toml, not via the
set-key flow.
Tests
- test_subprocess_timeout_secs_in_toml: round-trips a TOML doc with
default_model.subprocess_timeout_secs = 600 and one fallback at 180,
one fallback omitted; asserts each value (or None) reaches the parsed
config struct.
- test_subprocess_timeout_secs_omitted_defaults_to_none: asserts a
legacy-shaped config.toml (no timeout fields) parses cleanly with
both fields = None — backward-compat guard.
- 4 existing claude_code driver timeout tests still pass.
Mechanical pass-throughs
- 8 test fixtures across openfang-kernel/tests and openfang-api/tests
gain subprocess_timeout_secs: None on their DefaultModelConfig
literals.
- 1 production literal in routes.rs gains the same field.
- The existing FallbackProviderConfig serde-roundtrip test gains
subprocess_timeout_secs: None plus an assertion.
Precedence comment in drivers/mod.rs::create_driver updated to reflect
that the config-field path is now real, with explicit pointers to the
kernel.rs wiring sites for future contributors.
Validated: cargo check --workspace --tests is clean; openfang-types
(362), openfang-runtime (933), and openfang-kernel (260) lib tests
all pass.