Repository navigation
feat(config): unify config resolution with Settings fallback (Phase 2, #1119) - #1203
Conversation
…nearai#1119) Five config modules ignored their Settings structs, meaning values saved via `config set` or the setup wizard had no effect at runtime. This also closes the Phase 1 gap where auto-generated gateway tokens were lost on restart. - Wire WasmConfig::resolve() to read WasmSettings (7 fields) - Wire SandboxModeConfig::resolve() to read SandboxSettings (8 fields) - Wire BuilderModeConfig::resolve() to read BuilderSettings (5 fields) - Wire resolve_safety_config() to read SafetySettings (2 fields) - Wire ClaudeCodeConfig::resolve() to read sandbox.claude_code_enabled, fixing a bug where the setup wizard's choice was silently ignored - Preserve ClaudeCodeConfig::from_env() for worker containers (no DB) - Persist auto-generated GATEWAY_AUTH_TOKEN to "default" namespace so it survives restarts without requiring an env var - Add ENV_MUTEX to all new env-mutating tests for parallel safety
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly refactors the configuration resolution mechanism across several core modules, ensuring that settings persisted via the 'config set' command or the setup wizard are correctly applied at runtime. It addresses a critical gap where previously, certain configuration values and auto-generated gateway tokens were not consistently loaded after application restarts. The changes unify the configuration flow, prioritizing environment variables while falling back to stored settings, and include new tests to validate this behavior. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a solid improvement that unifies configuration resolution for several modules (builder, safety, sandbox, wasm, claude_code) to fall back to Settings values from the database. This ensures that settings configured via config set or the setup wizard are correctly applied at runtime. The addition of tests for this new fallback logic is excellent. Furthermore, the change to persist the auto-generated gateway auth token fixes an important bug where the token was lost on restart. I have one minor suggestion to simplify the code.
| .unwrap_or_else(|| { | ||
| if ss.extra_allowed_domains.is_empty() { | ||
| Vec::new() | ||
| } else { | ||
| ss.extra_allowed_domains.clone() | ||
| } | ||
| }); |
zmanian
left a comment
There was a problem hiding this comment.
Review: APPROVE
Well-structured, low-risk PR that methodically wires 5 config modules to read from persisted Settings as fallback when env vars aren't set. Previously, values set via config set or the setup wizard were silently ignored at runtime.
Modules wired: WasmConfig (7 fields), SandboxModeConfig (8 fields), BuilderModeConfig (5 fields), safety config (2 fields), ClaudeCodeConfig (1 field).
Architecture: Consistent pattern across all modules -- priority chain is env var > settings > hardcoded default. Matches how already-wired modules (AgentConfig, HeartbeatConfig, etc.) work.
Notable decisions:
ClaudeCodeConfigcleanly split intoresolve(settings)andresolve_env_only()for worker containers without DB access- Gateway auth token persisted via
db.set_setting()to survive restarts - Fully backward compatible -- env vars still take priority, default values match previous hardcoded defaults
Minor suggestions (non-blocking):
- Simplify
extra_allowed_domainsfallback -- theif is_empty()check is unnecessary:
// Before (unnecessary conditional)
.unwrap_or_else(|| {
if ss.extra_allowed_domains.is_empty() {
Vec::new()
} else {
ss.extra_allowed_domains.clone()
}
})
// After
.unwrap_or_else(|| ss.extra_allowed_domains.clone())- Add a test for the
extra_allowed_domainssettings fallback path. - Add a test for
ClaudeCodeConfig::from_env()container path to guard against regressions.
CI all green. Safe to merge.
…nearai#1119) (nearai#1203) Unify config resolution with Settings fallback (Phase 2)
…nearai#1119) (nearai#1203) Unify config resolution with Settings fallback (Phase 2)
Summary
Five config modules ignored their Settings structs, meaning values
saved via
config setor the setup wizard had no effect at runtime.This also closes the Phase 1 gap where auto-generated gateway tokens
were lost on restart.
Change Type
Linked Issue
Part of #1119 (Phase 2)
Validation
cargo fmtcargo clippy --all --benches --tests --examples --all-featuresSecurity Impact
None
Database Impact
None
Blast Radius
Rollback Plan
Review track: