Back to Forgecode

Slim Environment & Add `get_config` to `EnvironmentInfra`

plans/2026-03-31-slim-environment-and-add-get-config-v1.md

2.12.1116.9 KB
Original Source

Slim Environment & Add get_config to EnvironmentInfra

Objective

Reduce Environment to only the six fields that cannot be sourced from ForgeConfigos, pid, cwd, home, shell, base_path — and expose ForgeConfig directly through a new get_config method on EnvironmentInfra. All service/app code that currently accesses configuration fields through Environment will instead call infra.get_config() and read the canonical ForgeConfig fields directly.


Implementation Plan

Phase 1 — Slim Environment in forge_domain

  • Task 1. Remove all ForgeConfig-sourced fields from the Environment struct (crates/forge_domain/src/env.rs:54-198).

    The 37 fields below the // --- Infrastructure-derived fields --- comment block — including retry_config, max_search_lines, fetch_truncation_limit, session, commit, suggest, is_restricted, tool_supported, temperature, top_p, top_k, max_tokens, max_tool_failure_per_turn, max_requests_per_turn, compact, updates, and all remaining mapped fields — must be deleted. Only the six runtime-derived fields remain: os, pid, cwd, home, shell, base_path.

  • Task 2. Adjust history_path() to accept a custom_path parameter (crates/forge_domain/src/env.rs:267-271).

    history_path() currently reads self.custom_history_path, which originates from ForgeConfig. Change its signature to pub fn history_path(&self, custom_path: Option<&PathBuf>) -> PathBuf so callers pass the value sourced from ForgeConfig themselves. Update all call sites accordingly.

  • Task 3. Remove apply_op() from Environment (crates/forge_domain/src/env.rs:222-261).

    This method mutates session, commit, and suggest — all fields being removed. The mutation logic will move to the infra layer (see Phase 3). Delete apply_op and its #[cfg(test)] unit tests that exercise it via fixture_env.

  • Task 4. Remove the SessionConfig re-export from Environment's imports if it becomes unused (crates/forge_domain/src/env.rs:10-13).

    After removing session, commit, suggest from Environment, check whether SessionConfig, CommitConfig, SuggestConfig, RetryConfig, HttpConfig, MaxTokens, Temperature, TopK, TopP, Update, and Compact are still needed in this file. Remove any that are now unreferenced.

  • Task 5. Remove AutoDumpFormat from env.rs if it is no longer referenced there.

    AutoDumpFormat was used by the auto_dump field being removed. If it has no other use in env.rs, move its definition or re-export to the appropriate module, or leave it in forge_domain as a standalone type — do not silently delete it if it is still needed elsewhere in the domain.

  • Task 6. Update the fake::Dummy derive on Environment.

    With only six fields, the #[derive(fake::Dummy)] annotation and any #[dummy(...)] attribute overrides on removed fields must be cleaned up. Verify the derive still compiles.


Phase 2 — Add type Config and get_config to EnvironmentInfra

  • Task 7. Add the Config associated type to EnvironmentInfra (crates/forge_app/src/infra.rs:21-39).

    Add type Config; as an associated type on the trait. Every service bound written as F: EnvironmentInfra will need the concrete type resolved through the type Config mechanism, so consider whether a where Self::Config: ... bound is necessary at the trait level (e.g. Clone and Send if callers cache the value).

  • Task 8. Add fn get_config(&self) -> Self::Config to EnvironmentInfra.

    This method returns the full ForgeConfig to any consumer that holds a reference to an EnvironmentInfra implementor. Add it alongside the existing get_environment, get_env_var, and get_env_vars methods.

  • Task 9. Add forge_config as a direct dependency of forge_app (crates/forge_app/Cargo.toml).

    forge_app now references ForgeConfig in the trait definition. Add the path dependency if it does not already exist.


Phase 3 — Update ForgeEnvironmentInfra in forge_infra

  • Task 10. Implement type Config = ForgeConfig and get_config on ForgeEnvironmentInfra (crates/forge_infra/src/env.rs:393-443).

    get_config reads from the existing Arc<Mutex<Option<ForgeConfig>>> cache, loading from disk on the first call — identical to how get_environment already works. Return the ForgeConfig value (cloned from cache).

  • Task 11. Simplify to_environment() to populate only the six runtime fields (crates/forge_infra/src/env.rs:120-179).

    Remove the entire // --- ForgeConfig-mapped fields --- section. to_environment should only construct os, pid, cwd, home, shell, and base_path.

  • Task 12. Delete to_forge_config() and all its from_* helper conversion functions (crates/forge_infra/src/env.rs:181-354).

    These were needed to round-trip Environment ↔ ForgeConfig. With Environment no longer carrying config data, the round-trip path is eliminated. Remove to_forge_config, from_retry_config, from_http_config, from_tls_version, from_tls_backend, from_auto_dump_format, from_update_frequency, from_update, and from_compact.

  • Task 13. Rewrite update_environment() to operate on ForgeConfig directly (crates/forge_infra/src/env.rs:417-442).

    The new flow:

    1. Load ForgeConfig from disk via ConfigReader (defaults + global only, as today).
    2. Apply each ConfigOperation directly to the ForgeConfig using a new free function apply_config_op(fc: &mut ForgeConfig, op: ConfigOperation) defined in this file.
    3. Call fc.write() to persist.
    4. Invalidate the cache.

    The apply_config_op function replicates the SetProvider, SetModel, SetCommitConfig, and SetSuggestConfig mutation logic that was previously in Environment::apply_op, but targeting ForgeConfig's session, commit, and suggest fields of type Option<ModelConfig>.

  • Task 14. Delete the round-trip identity test (crates/forge_infra/src/env.rs:483-509).

    The test test_forge_config_environment_identity verified that fc → env → fc' → env' preserved equality. Since to_forge_config is removed, this test is invalid. Remove it and add a simpler test that confirms get_config returns the same values as were written via update_environment.

  • Task 15. Implement type Config = ForgeConfig and get_config on ForgeInfra (crates/forge_infra/src/forge_infra.rs:94-113).

    Add the delegation: fn get_config(&self) -> ForgeConfig { self.config_infra.get_config() }. Also add type Config = ForgeConfig;.

  • Task 16. Update ForgeInfra::new() to read config through get_config() rather than get_environment() (crates/forge_infra/src/forge_infra.rs:58-91).

    ForgeInfra::new() currently calls config_infra.get_environment() to extract env.parallel_file_reads and env.service_url for constructing ForgeDirectoryReaderService and ForgeGrpcClient. Change these to read from config_infra.get_config() using the canonical ForgeConfig field names: max_parallel_file_reads and services_url (parsed to Url).


Phase 4 — Update MockInfra in tests

  • Task 17. Add a ForgeConfig field to MockInfra (crates/forge_services/src/app_config.rs:148-278).

    MockInfra currently holds Arc<Mutex<Environment>>. Add config: Arc<Mutex<ForgeConfig>> alongside it, initialised from ForgeConfig::default() or a faked value in MockInfra::new().

  • Task 18. Implement type Config = ForgeConfig and get_config on MockInfra.

    Return a clone of the locked ForgeConfig value.

  • Task 19. Rewrite update_environment on MockInfra to operate on ForgeConfig.

    Replace the inline Environment-mutation logic with the same apply_config_op semantics (on ForgeConfig), keeping get_environment returning a still-valid slim Environment built from the stored cwd/os/shell/etc.

  • Task 20. Update MockInfra::get_environment() to return a slim Environment.

    Since Environment now has only six fields, construct it from static test values (os, pid, cwd, home, shell, base_path) rather than from a full fake.

  • Task 21. Update test assertions in app_config.rs that inspect env.session and env.suggest.

    Tests currently call fixture.get_environment() and then inspect env.session. Change these assertions to call fixture.get_config() and inspect config.session on the returned ForgeConfig. Update the field access paths to match ForgeConfig's field names (session: Option<ModelConfig>, model_id: Option<String>, etc.).


Phase 5 — Update all service and app consumers

  • Task 22. Update ForgeAppConfigService to read session config via get_config() (crates/forge_services/src/app_config.rs:33-129).

    Every call to self.infra.get_environment() followed by env.session, env.commit, or env.suggest must be replaced with self.infra.get_config() accessing config.session, config.commit, config.suggest of type Option<ModelConfig>. Adapt the field access to ModelConfig's provider_id: Option<String> and model_id: Option<String> fields.

  • Task 23. Update forge_services/src/auth.rsenv.service_urlget_config().services_url.

    Parse config.services_url to Url at the call site as needed.

  • Task 24. Update forge_services/src/context_engine.rsenv.max_file_read_batch_size (two sites).

    Replace with infra.get_config().max_file_read_batch_size.

  • Task 25. Update forge_services/src/attachment.rsenv.max_search_result_bytes.

    Replace with infra.get_config().max_search_result_bytes.

  • Task 26. Update forge_app/src/tool_executor.rsenv.fetch_truncation_limit.

    Replace with infra.get_config().max_fetch_chars.

  • Task 27. Update forge_app/src/tool_registry.rsenv.tool_timeout and env.max_search_lines.

    Replace with infra.get_config().tool_timeout_secs and infra.get_config().max_search_lines.

  • Task 28. Update forge_app/src/changed_files.rsenv.parallel_file_reads.

    Replace with infra.get_config().max_parallel_file_reads.

  • Task 29. Update forge_app/src/app.rsenv.tool_supported.

    Replace with infra.get_config().tool_supported.

  • Task 30. Audit all remaining get_environment() call sites that access now-removed fields.

    Run a codebase search for get_environment() followed by field access. For any that reference a field no longer on Environment, redirect to get_config(). Document any additional files not enumerated in Tasks 22–29.

  • Task 31. Add forge_config as a direct dependency to any crate that now imports ForgeConfig for field access (e.g. forge_services, forge_app).

    For each crate modified in Tasks 22–29, verify forge_config is listed in its Cargo.toml and add it if absent.


Phase 6 — Update other infra consumers

  • Task 32. Update ForgeHttpInfra::new() which currently accepts Environment (crates/forge_infra/src/http.rs).

    If ForgeHttpInfra uses any Environment field that originated from ForgeConfig (e.g. env.http, env.retry_config), change it to accept ForgeConfig instead and read config.http and config.retry directly.

  • Task 33. Update ForgeCommandExecutorService::new() which currently accepts Environment (crates/forge_infra/src/executor.rs).

    If it reads fields like env.tool_timeout, env.shell, or env.cwd, split the arguments: pass the slim Environment for shell/cwd, and pass the relevant ForgeConfig fields for config-sourced values.


Phase 7 — Remove dead code and verify

  • Task 34. Remove unused imports and dead code across all modified files.

    After removing fields from Environment, many imports in forge_domain/src/env.rs, forge_infra/src/env.rs, and consumer files will become unused. Clean them all up.

  • Task 35. Remove the AutoDumpFormat-related and other now-unused conversion helpers from forge_infra/src/env.rs.

    The to_auto_dump_format, to_compact, to_update, to_update_frequency, to_session_config, to_tls_version, to_tls_backend, to_http_config, and to_retry_config functions may all become dead code. Remove any that are no longer called.

  • Task 36. Run cargo insta test --accept to verify all tests pass and update any snapshot tests affected by the structural changes.


Verification Criteria

  • Environment contains exactly six fields: os, pid, cwd, home, shell, base_path. All path helper methods continue to compile and behave correctly.
  • EnvironmentInfra has type Config; and fn get_config(&self) -> Self::Config as part of its public interface.
  • Every impl EnvironmentInfra in the codebase (production: ForgeEnvironmentInfra, ForgeInfra; test: MockInfra) declares type Config = ForgeConfig.
  • No service or app code accesses a field on the return value of get_environment() that was formerly sourced from ForgeConfig; all such accesses use get_config() instead.
  • The to_forge_config() function and all from_* conversion helpers are deleted from forge_infra/src/env.rs.
  • update_environment() operates directly on ForgeConfig without involving Environment mutation.
  • apply_op() no longer exists on Environment; mutation logic lives in a free function in forge_infra/src/env.rs.
  • cargo insta test --accept completes without errors.
  • cargo check produces no warnings for unused imports or dead code in any modified file.

Potential Risks and Mitigations

  1. SessionConfig vs ModelConfig type mismatch
    Environment.session was of type Option<SessionConfig> (domain type), while ForgeConfig.session is Option<ModelConfig> (config type). Service code that destructures SessionConfig will need updating to work with ModelConfig's field names. Additionally, SessionConfig may become an orphaned domain type if nothing else uses it — audit its usages before deciding to retain or remove it.
    Mitigation: Before removing SessionConfig, run a workspace-wide search for all usages and migrate or remove them as part of Tasks 22 and 21.

  2. cwd used in ForgeCommandExecutorService and ForgeHttpInfra
    These types currently take a full Environment but may rely on a mix of infra fields (cwd, shell) and config fields (tool_timeout, http, retry). A naive split risks passing two objects where one worked before.
    Mitigation: Tasks 32–33 explicitly address this. The slim Environment is passed for runtime fields; individual ForgeConfig fields are passed separately, preserving the single-responsibility boundary at each constructor.

  3. history_path() signature change breaks all callers
    This is the only path helper that currently reads a ForgeConfig field. Changing its signature is a mechanical but wide-reaching refactor.
    Mitigation: Make the change in Task 2 early, then use compiler errors to find all call sites. Each call site must source custom_history_path from a locally available ForgeConfig (obtained via get_config()).

  4. Crate dependency graph expansion
    forge_services and forge_app currently have no direct dependency on forge_config. Adding one is architecturally significant — it means domain-level services now know about the infrastructure config format.
    Mitigation: This is an explicit requirement of the task. Task 31 ensures all Cargo.toml files are updated consistently. If future work wants to reintroduce a domain abstraction layer, a ForgeConfig-to-domain mapping can be re-introduced separately.

  5. fake::Dummy derive on slim Environment may fail
    The fake crate derives require all field types to implement Dummy. With six simple fields (String, u32, PathBuf, Option<PathBuf>), this should remain straightforward, but the url::Url #[dummy(expr = ...)] override that was on service_url is being removed.
    Mitigation: Task 6 ensures the derive is re-validated after the structural change.


Alternative Approaches

  1. Keep Environment as-is but add get_config() alongside get_environment(): This is the minimal additive change — no consumer code is disrupted, and new code can opt into get_config(). Trade-off: Environment remains a redundant mirror of ForgeConfig, the to_environment/to_forge_config round-trip complexity is preserved, and the synchronisation risk (duplicate fields drifting out of step) persists.

  2. Embed ForgeConfig as a field inside Environment rather than replacing the fields: Environment could hold pub config: ForgeConfig plus the six infra fields, allowing a gradual migration of callers. Trade-off: Transitional approach that still results in duplicated field access patterns and retains the to_environment mapping, just restructured. Cleaner than alternative 1 but more disruptive than the targeted plan above.