feat(config): add env overrides for loop control and background task limits - #1993
Conversation
…limits Add three operational environment overrides, resolved as env > config.toml > default in both engines (agent-core and agent-core-v2), matching the existing KIMI_IMAGE_MAX_EDGE_PX / KIMI_SUBAGENT_TIMEOUT_MS pattern: - KIMI_LOOP_MAX_STEPS_PER_TURN overrides loop_control.max_steps_per_turn - KIMI_LOOP_MAX_RETRIES_PER_STEP overrides loop_control.max_retries_per_step - KIMI_CODE_BACKGROUND_MAX_RUNNING_TASKS overrides background.max_running_tasks Invalid values are ignored and fall back to the config value. The v2 engine resolves them through config section env bindings (effective-only, never persisted); the v1 engine resolves them at the consumption point.
🦋 Changeset detectedLatest commit: dcbd760 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 057c6fbb5c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Environment overrides resolved into the effective config could be echoed back through IConfigService.set/replace (e.g. GET then POST /api/v1/config) and persisted into config.toml, outliving the env var. This affected the pre-existing image / subagent / keep-alive bindings as well as the new loop control and max-running-tasks bindings. Extend ConfigStripEnv with a getEnv parameter and add a shared stripEnvBoundFields helper: while a field's env var is set, writes restore the field's raw on-disk value (or drop it) instead of persisting an echoed env value; when unset, normal writes persist. Register it for the loopControl, task/background, image, and subagent sections. Also fix ConfigService.stripEnv looking up rawSnake by the camelCase domain key; on-disk sections are keyed snake_case.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37fdbca8e8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An invalid env value (e.g. KIMI_LOOP_MAX_STEPS_PER_TURN=abc) is ignored on the read path but still marked the field env-owned on the write path, so a config write for that field was silently dropped. stripEnvBoundFields now derives the guard from the section's envBindings and skips fields whose env value fails the binding's parse, so invalid env values are ignored on both paths — and the duplicated field/env descriptor list is gone. Also drop function-level comments added beside helpers; agent-core-v2 keeps comments solely in the top-of-file block, so the strip semantics now live in the config.ts / configService.ts headers.
Two follow-ups from review: - ConfigService.get()/getAll() re-applied section env bindings on the already-overlaid effective cache (and get() mutated it in place), so a valid override degraded to invalid or unset kept serving the stale value until the next reload — and an echoed stale value could then be persisted by a config write. Reads now recompute from a cached env-free validated base, so degraded or removed env values fall back to the file immediately. - stripEnvBoundFields restored env-owned fields from the raw snake sub-object, missing values persisted under legacy keys (e.g. max_steps_per_run). Section stripEnv now receives the env-free, fromToml-normalized raw base, so legacy aliases are honored.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f2106cf997
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
auth.test.ts removed its temp home with a plain recursive rm, which races late async writers in the server shutdown path and flakes with ENOTEMPTY (seen on main CI and locally on main). Harden the cleanup with the same maxRetries/retryDelay options sessions.test.ts already uses.
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…tries One feature entry (loop/background env overrides, both engines plus the CLI) and one fix entry (env values persisting on writes and sticking after degrade/unset, agent-core-v2 plus the CLI).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d790d1b88c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…s on get()
Two follow-ups from review:
- stripEnvBoundFields returned an empty object when every written field
was env-owned, so a section with a non-empty default (e.g. subagent)
stored raw = {} and the default stopped applying until the next
reload. A fully stripped result now clears the raw section instead.
- get(domain) only recomputed env overlays for sections with env
bindings, so domains written solely by a ConfigEffectiveOverlay
(models / defaultModel from KIMI_MODEL_NAME) kept serving stale cached
values after the env changed. get() now derives every non-memory
domain from the fresh env-free base, matching getAll().
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f46ac4d16
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…verlay The previous version imported the model env overlay to activate it, which the CI runners failed to resolve from this test file (both tsgo and vitest, while the same specifier resolves elsewhere — not reproducible locally). An inline ConfigEffectiveOverlay double exercises the same ConfigService contract with a tighter seam and no module dependency.
…nv targets Two follow-ups from review: - stripEnvBoundFields cleared the raw section whenever the stripped result was empty, so an env echo write could drop unknown forward-compatible fields from the TOML table. An emptied section now keeps its raw table while the env-free base still holds other fields, and is cleared only when nothing remains (defaults keep applying). - applyEnvBindings reused nested child objects in place, so a nested env binding on a persistable key would mutate the env-free validated base and serve stale values after the env var is removed. Children are now cloned before descending.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a275463008
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…rsist
Returning {} for a fully stripped write stored the empty object into the
raw layer while the TOML table survived via rawSnake, so the bases
diverged; a second echo write then saw an empty base, cleared the
section, and deleted the table — dropping unknown forward-compatible
fields. When nothing persistable remains, the write is now a no-op for
the section (the env-free base is kept as-is), and the section is
cleared only when the base is empty.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bfbead39ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… persist A strip may restore on-disk values from the unvalidated raw base (e.g. an env-masked invalid field), smuggling them past the merge-time validation into the stored config: the section would then fail the next buildValidated pass, dropping accompanying valid edits from the runtime while the invalid value stayed persisted. set() now revalidates the stripped result — discarding the parse output so unknown fields survive — and rejects the write instead, matching replace().
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
[loop_control](max_steps_per_turn,max_retries_per_step) and[background] max_running_taskscould only be set throughconfig.toml. Operators running CI jobs, containers, or wrapper scripts had no way to inject these operational limits per-run without editing a config file — unlike neighboring settings that already accept env overrides (KIMI_IMAGE_MAX_EDGE_PX,KIMI_SUBAGENT_TIMEOUT_MS,KIMI_CODE_BACKGROUND_KEEP_ALIVE_ON_EXIT).What changed
Adds three environment overrides, resolved as env > config.toml > default in both engines; invalid values are ignored and fall back to the config value:
KIMI_LOOP_MAX_STEPS_PER_TURN→loop_control.max_steps_per_turn(non-negative integer;0means unlimited, same as the config field)KIMI_LOOP_MAX_RETRIES_PER_STEP→loop_control.max_retries_per_step(non-negative integer)KIMI_CODE_BACKGROUND_MAX_RUNNING_TASKS→background.max_running_tasks(positive integer)The implementation follows the repo's two established patterns per engine:
agent-core-v2: declarativeenvBindingson the owning config sections (loopControl, andtask+ legacybackground), applied by the config env overlay on every read. The overlay lands only ineffective, so env values are never persisted toconfig.toml.agent-core: consumption-point resolvers (resolveMaxStepsPerTurn/resolveMaxRetriesPerStep/resolveMaxRunningTasks), mirroring the existingresolveSubagentTimeoutMs.Docs updated (
env-vars.mdandconfig-files.md, zh + en); changesets included.Verification: full unit suites pass for both engines; smoke-tested against a real kap-server boot (env beats config; invalid env falls back to config) and a real
kimi -prun (KIMI_LOOP_MAX_STEPS_PER_TURN=1ends the turn withloop.max_steps_exceededafter one step; the control run without the env succeeds).Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.