fix(config): migrate legacy acp.stream keys on load#60824
fix(config): migrate legacy acp.stream keys on load#60824BlueBirdBack wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Fix issue openclaw#35957 by migrating old v2026.3.2 acp.stream keys to the current schema during normal legacy-config migration. - maxTurnChars -> maxOutputChars - maxToolSummaryChars -> maxSessionUpdateChars - remove maxStatusChars, maxMetaEventsPerTurn, metaMode, showUsage Add regression coverage for raw validation, migration output, normal config validation, and non-overwrite behavior when new keys already exist. Verified with Bug Lab fixture-backed check: - current main => bug_present / code-suspect - patched worktree => fixed / cannot_reproduce
Greptile SummaryThis PR fixes issue #35957 by wiring a new Key observations:
Confidence Score: 4/5Safe to merge; the migration is additive, correctly clones before mutating, and the renamed-key logic has solid test coverage. The core logic is sound and follows established codebase patterns. No files require special attention; the single style concern is in Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/config/legacy.migrations.runtime.ts
Line: 448-478
Comment:
**Type guards on removed keys may silently skip cleanup**
For the four keys being *removed with no replacement* (`maxStatusChars`, `maxMetaEventsPerTurn`, `metaMode`, `showUsage`), the migration only deletes the key when its runtime value matches the expected type. If a config has a wrongly-typed value (e.g. `maxStatusChars: "400"` as a string), the guard fails silently: the key is left in place, the legacy rule still flags it (it has no `match` predicate), and `migrateLegacyConfig` ends up returning `{ config: null, changes: ["Migration applied, but config still invalid…"] }` — indistinguishable to the user from a real schema error.
For the *renamed* keys (`maxTurnChars`, `maxToolSummaryChars`) the type guard makes sense because you want to migrate a valid number to the new name. But for the removed keys there is no value to preserve — removing them unconditionally would match the "auto-removed on load" intent in the rule messages and avoid the confusing stuck-migration case.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix(config): migrate legacy acp.stream k..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| if ( | ||
| Object.prototype.hasOwnProperty.call(stream, "maxStatusChars") && | ||
| typeof stream.maxStatusChars === "number" | ||
| ) { | ||
| delete stream.maxStatusChars; | ||
| changes.push("Removed acp.stream.maxStatusChars (no replacement)."); | ||
| } | ||
|
|
||
| if ( | ||
| Object.prototype.hasOwnProperty.call(stream, "maxMetaEventsPerTurn") && | ||
| typeof stream.maxMetaEventsPerTurn === "number" | ||
| ) { | ||
| delete stream.maxMetaEventsPerTurn; | ||
| changes.push("Removed acp.stream.maxMetaEventsPerTurn (no replacement)."); | ||
| } | ||
|
|
||
| if ( | ||
| Object.prototype.hasOwnProperty.call(stream, "metaMode") && | ||
| typeof stream.metaMode === "string" | ||
| ) { | ||
| delete stream.metaMode; | ||
| changes.push("Removed acp.stream.metaMode (no replacement)."); | ||
| } | ||
|
|
||
| if ( | ||
| Object.prototype.hasOwnProperty.call(stream, "showUsage") && | ||
| typeof stream.showUsage === "boolean" | ||
| ) { | ||
| delete stream.showUsage; | ||
| changes.push("Removed acp.stream.showUsage (no replacement)."); | ||
| } |
There was a problem hiding this comment.
For the four keys being removed with no replacement (maxStatusChars, maxMetaEventsPerTurn, metaMode, showUsage), the migration only deletes the key when its runtime value matches the expected type. If a config has a wrongly-typed value (e.g. maxStatusChars: "400" as a string), the guard fails silently: the key is left in place, the legacy rule still flags it (it has no match predicate), and migrateLegacyConfig ends up returning { config: null, changes: ["Migration applied, but config still invalid…"] } — indistinguishable to the user from a real schema error.
For the renamed keys (maxTurnChars, maxToolSummaryChars) the type guard makes sense because you want to migrate a valid number to the new name. But for the removed keys there is no value to preserve — removing them unconditionally would match the "auto-removed on load" intent in the rule messages and avoid the confusing stuck-migration case.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/config/legacy.migrations.runtime.ts
Line: 448-478
Comment:
**Type guards on removed keys may silently skip cleanup**
For the four keys being *removed with no replacement* (`maxStatusChars`, `maxMetaEventsPerTurn`, `metaMode`, `showUsage`), the migration only deletes the key when its runtime value matches the expected type. If a config has a wrongly-typed value (e.g. `maxStatusChars: "400"` as a string), the guard fails silently: the key is left in place, the legacy rule still flags it (it has no `match` predicate), and `migrateLegacyConfig` ends up returning `{ config: null, changes: ["Migration applied, but config still invalid…"] }` — indistinguishable to the user from a real schema error.
For the *renamed* keys (`maxTurnChars`, `maxToolSummaryChars`) the type guard makes sense because you want to migrate a valid number to the new name. But for the removed keys there is no value to preserve — removing them unconditionally would match the "auto-removed on load" intent in the rule messages and avoid the confusing stuck-migration case.
How can I resolve this? If you propose a fix, please make it concise.
|
Good catch. I pushed
Focused check: |
1 participant
Summary
Fixes #35957 by migrating legacy
acp.streamkeys from older configs during normal legacy-config migration on load.Migrated:
acp.stream.maxTurnChars→acp.stream.maxOutputCharsacp.stream.maxToolSummaryChars→acp.stream.maxSessionUpdateCharsRemoved as legacy keys with no replacement:
acp.stream.maxStatusCharsacp.stream.maxMetaEventsPerTurnacp.stream.metaModeacp.stream.showUsageTests
Added regression coverage for:
Verification
Verified with a Bug Lab fixture-backed check using an old
v2026.3.2config fixture:main=>FIXTURE_CHECK: bug_present/VERDICT: code-suspectFIXTURE_CHECK: fixed/VERDICT: cannot_reproduceNote
Local commit used
--no-verifybecause the repo currently has unrelated baseline type/dependency failures in other areas; the actual config fix was still verified separately with the fixture-backed repro check.