[codex] Honor config env fallbacks#14
Merged
Merged
Conversation
aab3a50 to
63598b7
Compare
63598b7 to
5025098
Compare
Contributor
Author
|
If the env isn't what's expected, updating the documentation would also work. |
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
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.
Background
The docs say values can be supplied through the
[env]table inconfig.toml, but the implementation only resolved real process environment variables throughos.Getenv. That madedoctor, GitHub/OpenAI token detection, and config-backed model overrides disagree with the documented behavior.The underlying design issue was that one config struct was being used both as the persisted file shape and as the runtime effective config. If runtime fallbacks are applied directly during normal
Load, a laterLoad -> edit -> Savepath can materialize process env or[env]fallback values into top-level persisted fields.Changes
Config.Envsupport for the TOML[env]table.Load/Saveon the persisted config layer: defaults and path normalization only, no runtime env overlay.LoadRuntime/ApplyRuntimeEnvfor commands that need the effective runtime config.config.toml [env]as a fallback.LoadRuntime.[env]into realghsubprocesses for both direct passthrough and cached fallback capture paths.[env]as a fallback rather than process-wide env injection.Root cause
config.Loaddecoded TOML into a default-seeded config, but token resolution only checkedos.Getenv, and model override fallback only ran when fields were empty. Fixing that directly exposed the larger layering problem: applying runtime fallback values insideLoadmutates the same struct later used bySave, so unrelated config edits could persist fallback values.The
ghshim also needed explicit subprocess env handling. Localsynccould use[env].GITHUB_TOKEN, but fallback calls to the realghbinary inherited only the process environment, sogitcrawl gh ...could run unauthenticated when the token existed only inconfig.toml.Validation
GOWORK=off go test ./internal/configGOWORK=off go test ./internal/cli -run 'TestGHShim(PassThroughUsesConfigEnvToken|CachedFallbackUsesConfigEnvToken|FallsBackForUnsupportedRead)'GOWORK=off go test ./...go run ./cmd/gitcrawl --config <tmp-config-with-[env]> --json doctorand confirmed token sources plus model overrides report fromconfig.toml [env].gitcrawl configurewith conflicting process-env and[env]model fallbacks and confirmed those fallback values are not written into[openai].