← Back to PRs

#4897: fix: config logic issues (#4689, #4654)

by lailoo open 2026-01-30 20:41 View on GitHub →
commands agents
## Summary This PR fixes 2 configuration-related issues: ### Fixes 1. **#4689 exec host defaults to 'sandbox' even when sandbox.mode is 'off'** - File: `src/agents/pi-tools.ts` - When `sandbox.mode` is `"off"`, exec commands now default to `host: "gateway"` instead of `"sandbox"` - This prevents Docker containers from spawning when sandbox is disabled 2. **#4654 doctor --fix resolves ${...} env var references to plaintext** - File: `src/commands/doctor-config-flow.ts` - Use `snapshot.parsed` instead of `snapshot.config` to preserve `${...}` env var references when writing config back - Secrets (API keys, tokens) are no longer written as plaintext ### Testing - All existing tests pass - Build succeeds Fixes #4689, #4654 <!-- greptile_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> This PR adjusts configuration handling in two places: - `src/agents/pi-tools.ts` now derives an `effectiveHost` for the `exec` tool so that when there is no active sandbox context, the default host falls back to `gateway` instead of `sandbox`, preventing unintended sandbox container creation when sandboxing is effectively disabled. - `src/commands/doctor-config-flow.ts` now seeds the doctor repair flow from `snapshot.parsed` rather than `snapshot.config`, so `doctor --fix` preserves `${...}` env var references instead of rewriting substituted secrets back to disk. - The doctor test `src/commands/doctor.warns-state-directory-is-missing.test.ts` was updated to include the `parsed` field in the config snapshot mock, matching the new behavior. Overall, the changes fit the existing config IO contract: `snapshot.parsed` represents the raw parsed file (pre-substitution), while `snapshot.config` represents the validated/substituted runtime config used by the app. Using `parsed` in doctor aligns with the goal of not persisting resolved secrets. <h3>Confidence Score: 4/5</h3> - This PR is likely safe to merge; changes are small and targeted with low behavioral risk. - The changes are localized and align with existing config snapshot semantics (`parsed` vs `config`). The only notable risk is around `exec` host defaulting behavior clarity; functionality appears consistent with the stated fix, but the comment could mislead future maintainers. - src/agents/pi-tools.ts (exec host defaulting logic/comment semantics) <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs