Config/Compaction: expose safeguard preserve and quality settings by rodrigouroz · Pull Request #25557 · openclaw/openclaw

rodrigouroz · 2026-02-24T15:27:36Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: new safeguard behaviors lacked public config/schema surface and labels/help.
Why it matters: operators need explicit knobs and validation for rollout and tuning.
What changed: added compaction config keys/types/schema/help/labels and runtime wiring for preserve count and quality guard.
What did NOT change (scope boundary): no memory manager sync implementation in this PR.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

Closes n/a (final close tracked in Compaction Runner: wire post-compaction memory sync #25561)
Related [Bug]: Compaction summarization does not preserve exact identifiers #19207, (fix): Compaction: preserve recent context and sync session memory post-compact #20038

User-visible / Behavior Changes

New keys:
- agents.defaults.compaction.recentTurnsPreserve
- agents.defaults.compaction.qualityGuard.enabled
- agents.defaults.compaction.qualityGuard.maxRetries

Security Impact (required)

New permissions/capabilities? (No)
Secrets/tokens handling changed? (No)
New/changed network calls? (No)
Command/tool execution surface changed? (No)
Data access scope changed? (No)
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS
Runtime/container: Node 22 + pnpm
Model/provider: n/a (unit tests)
Integration/channel (if any): n/a
Relevant config (redacted): safeguard config overrides

Steps

pnpm vitest run src/agents/pi-extensions/compaction-safeguard.test.ts
pnpm tsgo
Verify schema accepts new compaction keys and runtime wiring test passes.

Expected

Config schema and runtime wiring test pass with new fields.

Actual

Local tests passed; typecheck passed.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios: new compaction keys are typed, validated, labeled/helped, and reach safeguard runtime.
Edge cases checked: runtime input clamp behavior for preserve count and retry limits.
What you did not verify: end-to-end user config migration in external deployments.

Compatibility / Migration

Backward compatible? (Yes)
Config/env changes? (No)
Migration needed? (No)
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly: revert this PR or remove newly added keys from config.
Files/config to restore: src/config/types.agent-defaults.ts, src/config/zod-schema.agent-defaults.ts, src/config/schema.help.ts, src/config/schema.labels.ts
Known bad symptoms reviewers should watch for: schema validation failures on malformed compaction values, while oversized retry counts remain accepted and clamp at runtime.

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk: misconfigured values (large preserve/retry) can increase compaction latency.
- Mitigation: runtime clamping plus non-breaking schema acceptance for oversized retry counts.

Stack: rebased after #25556 merge; no remaining stack dependency.

greptile-apps

_{11 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-24T15:30:07Z

src/agents/pi-embedded-runner/extensions.ts

+      recentTurnsPreserve: compactionCfg?.recentTurnsPreserve,
+      qualityGuardEnabled: compactionCfg?.qualityGuard?.enabled,
+      qualityGuardMaxRetries: compactionCfg?.qualityGuard?.maxRetries,


postIndexSync config is exposed but never consumed

The config field agents.defaults.compaction.postIndexSync is added to types, schema, help, and labels, but it's not actually read or passed to the runtime anywhere. Either wire it up or remove it from this PR.

Check where memory index sync should happen after compaction and pass this value through.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/agents/pi-embedded-runner/extensions.ts Line: 85-87 Comment: `postIndexSync` config is exposed but never consumed The config field `agents.defaults.compaction.postIndexSync` is added to types, schema, help, and labels, but it's not actually read or passed to the runtime anywhere. Either wire it up or remove it from this PR. Check where memory index sync should happen after compaction and pass this value through. How can I resolve this? If you propose a fix, please make it concise.

openclaw-barnacle · 2026-03-04T04:12:47Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

rodrigouroz · 2026-03-05T21:58:37Z

Rebased #25557 onto latest main after #25556 merged.

Scope cleanup in latest commits (c99260a0a / a61669e0e / 4e66c2bca):

Kept config/runtime exposure that is now consumed:
- agents.defaults.compaction.recentTurnsPreserve
- agents.defaults.compaction.qualityGuard.{enabled,maxRetries}
Dropped agents.defaults.compaction.postIndexSync from this PR because it is not consumed yet (to avoid exposing a dead knob in this split PR).
Updated schema/help/labels/types + safeguard runtime wiring/tests accordingly.

Local verification:

pnpm test -- src/agents/pi-extensions/compaction-safeguard.test.ts src/config/config.compaction-settings.test.ts src/config/schema.help.quality.test.ts
pnpm tsgo

@greptile-apps please review
@codex review

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c99260a0aa

ℹ️ 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".

chatgpt-codex-connector · 2026-03-05T22:02:55Z

src/config/zod-schema.agent-defaults.ts

          .object({
            enabled: z.boolean().optional(),
-            maxRetries: z.number().int().nonnegative().optional(),
+            maxRetries: z.number().int().min(0).max(3).optional(),


Avoid rejecting existing qualityGuard maxRetries values

Capping agents.defaults.compaction.qualityGuard.maxRetries at 3 in schema validation introduces a config-compatibility regression: configs that previously set any non-negative value (accepted before and effectively clamped at runtime) will now fail validation during config load/startup instead of running with the same clamped behavior. This only affects installations that already have maxRetries > 3, but for those users it is a hard boot/config error rather than a no-op tuning value.

Useful? React with 👍 / 👎.

greptile-apps · 2026-03-05T22:03:07Z

Greptile Summary

This PR exposes two new compaction safeguard config knobs — recentTurnsPreserve and qualityGuard.{enabled,maxRetries} — through the full config surface (types, Zod schema, help text, labels) and wires them to the existing safeguard runtime, making them operator-tuneable without touching core compaction logic.

Key changes:

recentTurnsPreserve added to AgentCompactionConfig, schema-validated with min(0).max(12), and passed from config to setCompactionSafeguardRuntime.
qualityGuard.maxRetries deliberately accepts oversized values at the schema level (no upper bound) and relies on runtime clamping — consistent with the "non-breaking acceptance" design called out in the PR description.
Help text for recentTurnsPreserve correctly documents "default: 3", matching DEFAULT_RECENT_TURNS_PRESERVE = 3 in the runtime.
The new integration test in compaction-safeguard.test.ts uses recentTurnsPreserve: 99, which bypasses Zod validation. Since the schema enforces max(12), this value is unreachable from a real config file and the test gives a misleading picture of the runtime-clamping contract for recentTurnsPreserve (see inline comment).

Confidence Score: 4/5

Safe to merge; changes are additive config surface with correct schema/runtime wiring and backward-compatible defaults.
All new keys are optional with sensible defaults, the schema and runtime cap for recentTurnsPreserve are in sync at 12, and the qualityGuard.maxRetries oversized-value acceptance is intentional and consistent with the existing design. The only concern is a misleading test value (recentTurnsPreserve: 99) that exercises a code path unreachable through real config loading, which could confuse future contributors but doesn't affect runtime correctness.
src/agents/pi-extensions/compaction-safeguard.test.ts — test uses a recentTurnsPreserve value that the schema would reject, creating a false impression of the clamping contract.

Comments Outside Diff (1)

src/agents/pi-extensions/compaction-safeguard.test.ts, line 409-440 (link)

Unreachable recentTurnsPreserve: 99 test value misleads about clamping behaviour

The Zod schema for recentTurnsPreserve enforces max(12) (see zod-schema.agent-defaults.ts line 98), so a value of 99 can never arrive from a real config file — the schema validation would reject it before it ever reached the runtime. Constructing the config directly as OpenClawConfig bypasses the schema, meaning this test exercises a code path that is unreachable in production for recentTurnsPreserve.

This is intentionally different from qualityGuard.maxRetries, where the schema imposes no upper bound and the PR explicitly relies on runtime clamping. Using 99 for recentTurnsPreserve in the same test implies the two fields share the same "accept-all-then-clamp" contract, which is inaccurate and could mislead future contributors.

Consider using a value that the schema actually accepts (e.g. 10) to test the wiring, and assert resolveRecentTurnsPreserve(10) returns 10 (no clamping needed). If you also want to document the runtime cap, add a separate focused unit test that calls resolveRecentTurnsPreserve directly with an oversized value — that way it's clear this is a defensive internal guard rather than a config-level behaviour.

_{Last reviewed commit: 3e2af97}

rodrigouroz · 2026-03-06T17:17:50Z

Follow-up in 3e2af97:

Removed the schema compatibility regression on agents.defaults.compaction.qualityGuard.maxRetries.
Validation now accepts any non-negative integer again, matching prior behavior, while safeguard runtime continues to clamp to the safe max internally.
Added a config regression test covering oversized retry values (99) so existing deployments keep loading cleanly.

Local verification:

pnpm test -- src/config/config.compaction-settings.test.ts src/config/schema.help.quality.test.ts src/agents/pi-extensions/compaction-safeguard.test.ts
pnpm tsgo

@greptile-apps please review
@codex review

rodrigouroz · 2026-03-06T18:49:27Z

@jalehman Hi Josh, it's me again :-)
I'm getting closer, I have 4 follow ups to go :-)

@rodrigouroz

@rodrigouroz

jalehman · 2026-03-07T15:13:16Z

Merged via squash.

Prepared head SHA: ea99040
Merge commit: 4c0b873

Thanks @rodrigouroz!

@jalehman