feat(config): support per-agent compaction overrides#69987
Conversation
openclaw-barnacle
Bot
added
agents
|
Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit. |
Greptile SummaryThis PR adds optional per-agent
Confidence Score: 4/5Not safe to merge as-is due to a silent config regression on the no-list-entry path. Two callers that resolve compaction config when an agentId is provided both lack the ?? defaults fallback present in the analogous resolveAgentContextLimits function, causing the defaults compaction config to be silently dropped whenever the agentId has no matching agents.list entry. This is a present behavioral regression, not a theoretical risk. src/agents/pi-settings.ts (resolveCompactionConfig) and src/agents/pi-embedded-runner/extensions.ts (inline compactionCfg resolution) both need the ?? agents.defaults.compaction fallback. Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/agents/pi-settings.ts
Line: 44-48
Comment:
**Missing fallback to defaults when agent entry is absent**
When `agentId` is provided but `resolveAgentConfig` returns `undefined` (no matching entry in `agents.list`), this function returns `undefined` instead of `agents.defaults.compaction`. Before this PR those callers always received the defaults config; now a non-null `agentId` without a matching list entry silently drops the entire defaults compaction configuration — a regression for any deployment that has `agents.defaults.compaction` set but uses a session whose agentId isn't in `agents.list` (including the DEFAULT_AGENT_ID path when the list is empty).
Compare the existing `resolveAgentContextLimits`, which correctly falls back:
```ts
return resolveAgentConfig(cfg, agentId)?.contextLimits ?? defaults;
```
The fix:
```ts
function resolveCompactionConfig(params: { cfg?: OpenClawConfig; agentId?: string }) {
if (params.cfg && params.agentId) {
return (
resolveAgentConfig(params.cfg, params.agentId)?.compaction ??
params.cfg?.agents?.defaults?.compaction
);
}
return params.cfg?.agents?.defaults?.compaction;
}
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extensions.ts
Line: 92-95
Comment:
**Same missing-fallback bug as in `pi-settings.ts`**
When `agentId` is set but has no entry in `agents.list`, `resolveAgentConfig` returns `undefined` and `compactionCfg` becomes `undefined`, so no compaction extension is registered even if `agents.defaults.compaction.mode` is `"safeguard"`. The same fallback fix applies here:
```ts
const compactionCfg =
params.cfg && params.agentId
? (resolveAgentConfig(params.cfg, params.agentId)?.compaction ??
params.cfg?.agents?.defaults?.compaction)
: params.cfg?.agents?.defaults?.compaction;
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: .github/workflows/ci-check-testbox.yml
Line: 24-25
Comment:
**Large CI resource bump included in feature PR**
The runner was scaled from `blacksmith-8vcpu-ubuntu-2404` (5 min timeout) to `blacksmith-32vcpu-ubuntu-2404` (30 min timeout) — 4× CPU and 6× time. The PR description doesn't mention this change. If this is intentional (e.g., new tests are slower), it's worth a brief note; if it's leftover from local debugging, it should be reverted to avoid unnecessary compute cost on every CI run.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "feat(config): support per-agent compacti..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| function resolveCompactionConfig(params: { cfg?: OpenClawConfig; agentId?: string }) { | ||
| if (params.cfg && params.agentId) { | ||
| return resolveAgentConfig(params.cfg, params.agentId)?.compaction; | ||
| } | ||
| return params.cfg?.agents?.defaults?.compaction; |
There was a problem hiding this comment.
When agentId is provided but resolveAgentConfig returns undefined (no matching entry in agents.list), this function returns undefined instead of agents.defaults.compaction. Before this PR those callers always received the defaults config; now a non-null agentId without a matching list entry silently drops the entire defaults compaction configuration — a regression for any deployment that has agents.defaults.compaction set but uses a session whose agentId isn't in agents.list (including the DEFAULT_AGENT_ID path when the list is empty).
Compare the existing resolveAgentContextLimits, which correctly falls back:
return resolveAgentConfig(cfg, agentId)?.contextLimits ?? defaults;The fix:
function resolveCompactionConfig(params: { cfg?: OpenClawConfig; agentId?: string }) {
if (params.cfg && params.agentId) {
return (
resolveAgentConfig(params.cfg, params.agentId)?.compaction ??
params.cfg?.agents?.defaults?.compaction
);
}
return params.cfg?.agents?.defaults?.compaction;
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-settings.ts
Line: 44-48
Comment:
**Missing fallback to defaults when agent entry is absent**
When `agentId` is provided but `resolveAgentConfig` returns `undefined` (no matching entry in `agents.list`), this function returns `undefined` instead of `agents.defaults.compaction`. Before this PR those callers always received the defaults config; now a non-null `agentId` without a matching list entry silently drops the entire defaults compaction configuration — a regression for any deployment that has `agents.defaults.compaction` set but uses a session whose agentId isn't in `agents.list` (including the DEFAULT_AGENT_ID path when the list is empty).
Compare the existing `resolveAgentContextLimits`, which correctly falls back:
```ts
return resolveAgentConfig(cfg, agentId)?.contextLimits ?? defaults;
```
The fix:
```ts
function resolveCompactionConfig(params: { cfg?: OpenClawConfig; agentId?: string }) {
if (params.cfg && params.agentId) {
return (
resolveAgentConfig(params.cfg, params.agentId)?.compaction ??
params.cfg?.agents?.defaults?.compaction
);
}
return params.cfg?.agents?.defaults?.compaction;
}
```
How can I resolve this? If you propose a fix, please make it concise.| const compactionCfg = | ||
| params.cfg && params.agentId | ||
| ? resolveAgentConfig(params.cfg, params.agentId)?.compaction | ||
| : params.cfg?.agents?.defaults?.compaction; |
There was a problem hiding this comment.
pi-settings.ts
When agentId is set but has no entry in agents.list, resolveAgentConfig returns undefined and compactionCfg becomes undefined, so no compaction extension is registered even if agents.defaults.compaction.mode is "safeguard". The same fallback fix applies here:
const compactionCfg =
params.cfg && params.agentId
? (resolveAgentConfig(params.cfg, params.agentId)?.compaction ??
params.cfg?.agents?.defaults?.compaction)
: params.cfg?.agents?.defaults?.compaction;Prompt To Fix With AI
This is a comment left during a code review.
Path: src/agents/pi-embedded-runner/extensions.ts
Line: 92-95
Comment:
**Same missing-fallback bug as in `pi-settings.ts`**
When `agentId` is set but has no entry in `agents.list`, `resolveAgentConfig` returns `undefined` and `compactionCfg` becomes `undefined`, so no compaction extension is registered even if `agents.defaults.compaction.mode` is `"safeguard"`. The same fallback fix applies here:
```ts
const compactionCfg =
params.cfg && params.agentId
? (resolveAgentConfig(params.cfg, params.agentId)?.compaction ??
params.cfg?.agents?.defaults?.compaction)
: params.cfg?.agents?.defaults?.compaction;
```
How can I resolve this? If you propose a fix, please make it concise.| runs-on: blacksmith-32vcpu-ubuntu-2404 | ||
| timeout-minutes: 30 |
There was a problem hiding this comment.
The runner was scaled from blacksmith-8vcpu-ubuntu-2404 (5 min timeout) to blacksmith-32vcpu-ubuntu-2404 (30 min timeout) — 4× CPU and 6× time. The PR description doesn't mention this change. If this is intentional (e.g., new tests are slower), it's worth a brief note; if it's leftover from local debugging, it should be reverted to avoid unnecessary compute cost on every CI run.
Prompt To Fix With AI
This is a comment left during a code review.
Path: .github/workflows/ci-check-testbox.yml
Line: 24-25
Comment:
**Large CI resource bump included in feature PR**
The runner was scaled from `blacksmith-8vcpu-ubuntu-2404` (5 min timeout) to `blacksmith-32vcpu-ubuntu-2404` (30 min timeout) — 4× CPU and 6× time. The PR description doesn't mention this change. If this is intentional (e.g., new tests are slower), it's worth a brief note; if it's leftover from local debugging, it should be reverted to avoid unnecessary compute cost on every CI run.
How can I resolve this? If you propose a fix, please make it concise.
![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: 230a78ea14
ℹ️ 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".
| params.cfg && params.agentId | ||
| ? resolveAgentConfig(params.cfg, params.agentId)?.compaction | ||
| : params.cfg?.agents?.defaults?.compaction; |
There was a problem hiding this comment.
This lookup drops agents.defaults.compaction whenever an agentId is present but no matching agents.list[] entry exists. That now happens in normal defaults-only configs (no agents.list) because runtime paths pass sessionAgentId, so resolveAgentConfig(...) returns undefined and safeguard mode/custom instructions from defaults are silently ignored. The result is that sessions stop loading the safeguard extension even though defaults are configured.
Useful? React with 👍 / 👎.
| if (params.cfg && params.agentId) { | ||
| return resolveAgentConfig(params.cfg, params.agentId)?.compaction; | ||
| } |
There was a problem hiding this comment.
This helper has the same missing-fallback pattern: with a truthy agentId and no matching agents.list[] entry, it returns undefined instead of default compaction config. Since the run path now passes sessionAgentId, defaults-only setups lose configured reserveTokens, keepRecentTokens, and reserveTokensFloor, changing runtime compaction behavior from prior releases.
Useful? React with 👍 / 👎.
| typeof entry.compaction === "object" && entry.compaction | ||
| ? { ...agentDefaults?.compaction, ...entry.compaction } | ||
| : agentDefaults?.compaction, |
There was a problem hiding this comment.
This merge is only shallow, so nested objects like qualityGuard and memoryFlush are replaced wholesale instead of inheriting unspecified fields from defaults. For example, setting only agents.list[].compaction.qualityGuard.maxRetries drops default qualityGuard.enabled, which then evaluates to false in runtime and unexpectedly disables the guard.
Useful? React with 👍 / 👎.
1 participant
Summary
agents.defaults.compaction, so all agents had to share the same compaction instructions and runtime settings.agents.list[].compactionsupport, merged it over defaults, and threaded effective per-agent compaction config through embedded runtime compaction paths.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
agents.defaults.compaction.Regression Test Plan (if applicable)
src/agents/agent-scope.test.tssrc/agents/pi-settings.test.tssrc/agents/pi-embedded-runner/extensions.test.tscustomInstructionswins over defaultcustomInstructionsUser-visible / Behavior Changes
agents.list[].compactionentries.agents.defaults.compactionand override defaults when set.agents.list[].compaction.customInstructionsnow lets each agent provide its own compaction guidance.Diagram (if applicable)
Security Impact (required)
Yes, explain risk + mitigation:Repro + Verification
Environment
agents.defaults.compactionplusagents.list[].compactionSteps
agents.defaults.compaction.customInstructions.agents.list[].compaction.customInstructionsfor a specific agent.Expected
Actual
Evidence
Attach at least one:
Human Verification (required)
What you personally verified (not just CI), and how:
Review Conversations
Compatibility / Migration
Risks and Mitigations
agentIdthrough the affected embedded runtime setup paths and adds tests around the effective runtime behavior.