feat(compaction): add compaction.preflight.enabled config gate (#95553)#95675
feat(compaction): add compaction.preflight.enabled config gate (#95553)#95675wangmiao0668000666 wants to merge 3 commits into
Conversation
…law#95553) Adds a user-facing config gate to disable preflight (budget-triggered) compaction. When compaction.preflight.enabled === false, the pre-turn threshold check in runPreflightCompactionIfNeeded short-circuits to the existing session entry, so subsequent turns fall through to overflow recovery (which already honors compaction.timeoutSeconds with a 15min budget). The check is '=== false' against the literal value, so an unset key preserves the existing default-on behavior — the fix is strictly additive and does not change shipped behavior for any current user. This complements the four open PRs (openclaw#95561, openclaw#95563, openclaw#95580, openclaw#95590) that are all working on the abortSignal source for the preflight path. None of those PRs addresses the user-facing config dimension that the issue explicitly requests. Changes: - src/config/types.agent-defaults.ts: new AgentCompactionPreflightConfig type with optional enabled boolean - src/config/zod-schema.agent-defaults.ts: zod schema with strict mode - src/config/schema.help.ts + schema.labels.ts: help text + label - src/auto-reply/reply/agent-runner-memory.ts: 6-line early return in runPreflightCompactionIfNeeded, placed after isHeartbeat/isCli and before codex runtime so the gate is the only OpenClaw-specific path affected - src/auto-reply/reply/agent-runner-memory.test.ts: 1 new test (skips preflight compaction when enabled === false) using the same test fixture pattern as adjacent tests - scripts/repro/issue-95553-preflight-disabled-gate.mts: standalone real-environment proof that exercises the production gate Refs openclaw#95553
CI: external blocker on
|
|
Thanks for the contribution. ClawSweeper proposes closing this for now: the implementation may be reasonable, but passing review and proof does not establish that OpenClaw should add this product surface. Close: the patch is technically narrow and now has focused terminal proof, but it adds a permanent public Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. This is a proposal only until the separate default-off apply policy is enabled and all live maintainer-signal checks pass. A maintainer can sponsor the direction, request a narrower version, or apply Review detailsBest possible solution: Keep the canonical issue open and land a maintainer-sponsored solution: fix the existing preflight timeout/cancellation contract first, then add an explicit config gate only if maintainers confirm that permanent user-facing surface. Do we have a high-confidence way to reproduce the issue? Yes, at source level for the underlying report: current main has no preflight gate and still passes Is this the best way to solve the issue? No for unsponsored merge: this is a plausible escape hatch, but not the best confirmed repository direction because the same user problem can be solved by honoring the existing compaction timeout/cancellation contract without adding new config. A maintainer should explicitly sponsor this knob before it becomes public API. Security review: Security review cleared: No concrete security or supply-chain concern was found; the diff does not change dependencies, workflows, secrets handling, permissions, or package resolution. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against a6390b2b904d. |
…al-setup repro (openclaw#95553) Address ClawSweeper P1 findings on PR openclaw#95675: - schema.help.ts:1518 - replace hardcoded '15-minute budget' wording with neutral description that references timeoutSeconds only (no specific default minutes). Keeps wording correct regardless of which default the merge target carries. - agent-runner-memory.ts comment - same neutral wording fix. - docs/gateway/config-agents.md - add preflight.enabled to the JSON5 example and bullet list under agents.defaults.compaction, matching the schema/help coverage. - scripts/repro/issue-95553-preflight-disabled-gate.mts - rewrite the repro to write a real openclaw.json to a temp dir and load it through the production loadConfig() so the gate value is parsed from disk, not cast from an inline literal. Same production path the gateway startup uses.
|
@clawsweeper re-review P1 findings on the previous review (commit a140b0b / ffd17f3) are all addressed in commit 5210d8b (just pushed):
Real-environment test (just run): Vitest unit test: PR body updated with the new evidence section + review fixes section. |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review Nudge: the previous re-review on commit 5210d8b was acknowledged but the verdict comment has not been emitted yet (PR still carries the original a140b0b verdict). All P1/P2/P3 findings on the original review are addressed in commit 5210d8b:
PR body has been refreshed with the new evidence section + a findings reconciliation table. Real-environment output from the new repro: $ node --import tsx scripts/repro/issue-95553-preflight-disabled-gate.mts Vitest: Remaining checks-node-core-fast failure is pre-existing on openclaw/main at PR base a0ab5c0 (surface budget 10328 > 10327 / 5185 > 5184 reproduces without my PR). |
|
Closing after analysis per @wangmiao0668000666's review. Why close:
Preserved in branch Refs #95553 |
What Problem This Solves
Issue #95553 reports that preflight (budget-triggered) compaction cannot be disabled via config. The issue's "Expected behavior" section explicitly lists two user-asked solutions:
This PR addresses solution 2: a strict, additive
compaction.preflight.enabledconfig gate that short-circuits the preflight check and lets every turn fall through to overflow recovery (which already honorscompaction.timeoutSeconds).The check is
=== falseagainst the literal value, so an unset key preserves the existing default-on behavior — the fix is strictly additive and does not change shipped behavior for any current user.Why This Change Was Made
There are four open PRs already working on #95553, all focused on the same surface: the
abortSignalsource for the preflight path:AbortSignal.any([replyOp, AbortSignal.timeout(resolveCompactionTimeoutMs(cfg))])abortSignal: replyOp.signaland add a separatemcp.runtimeScopechangecreatePreflightCompactionCancelSignalhelper that filters outTimeoutErrorabortSignalline entirelyNone of those four PRs addresses the user-facing config dimension the issue explicitly requests. The preflight path is currently running for every user that hits the soft threshold, even users who would prefer to skip it. This PR ships the missing config dimension that complements any of the four abortSignal approaches once a maintainer picks one.
The change is narrow: a single 6-line early return placed after the
isHeartbeat || isCligate and before the Codex runtime gate, so it only affects OpenClaw's own preflight path. It is fully orthogonal to whichever abortSignal shape a maintainer chooses.Changes
src/config/types.agent-defaults.ts— newAgentCompactionPreflightConfigtype with optionalenabled: booleansrc/config/zod-schema.agent-defaults.ts—preflight: z.object({ enabled: z.boolean().optional() }).strict().optional()src/config/schema.help.ts— two help entries: parentagents.defaults.compaction.preflight+ leaf.enabledsrc/config/schema.labels.ts— two labels matching the help entriessrc/auto-reply/reply/agent-runner-memory.ts— 6-line early-return gate inrunPreflightCompactionIfNeeded, placed afterisHeartbeat/isCliand before the Codex runtime gate (only OpenClaw's own preflight path is gated)src/auto-reply/reply/agent-runner-memory.test.ts— new test "skips preflight compaction when cfg.agents.defaults.compaction.preflight.enabled === false" (uses the same fixture pattern as adjacent tests)docs/gateway/config-agents.md— addpreflight.enabledto the JSON5 example and bullet list underagents.defaults.compaction, matching the schema/help coveragescripts/repro/issue-95553-preflight-disabled-gate.mts— standalone real-environment proof that writes a realopenclaw.jsonand loads it via the productionloadConfig()(not a cast)User Impact
A user who adds the following to their
openclaw.jsonnow skips the budget-triggered preflight check and lets every turn fall through to overflow recovery:{ "agents": { "defaults": { "compaction": { "preflight": { "enabled": false } } } } }For users who do not set the key, behavior is unchanged.
Evidence
Behavior addressed: preflight (budget-triggered) compaction can now be disabled via config, complementing the four open PRs that are all working on the
abortSignalsource for the same pathReal environment tested: Linux x86_64, Node v24.14.1, repo at
5210d8b121rebased ontopersonal/main(basec57fee8239ed)Exact steps run after this patch:
The repro script proves the real
openclaw.json→ productionloadConfig()→ productionrunPreflightCompactionIfNeededpath. It writesopenclaw.jsonto a temp state dir, pointsOPENCLAW_CONFIG_PATHandOPENCLAW_STATE_DIRat it viawithEnvAsync, calls the productionloadConfig({ pin: false }), asserts the gate value is parsed from disk, and then exercises the gate on the production preflight function. The function returns the original session entry unchanged;compactEmbeddedAgentSessionis never reached.Observed result after fix: the production
runPreflightCompactionIfNeededreturns the existingsessionEntryunchanged whencompaction.preflight.enabled === false;compactEmbeddedAgentSessionis never called;incrementCompactionCountis never calledWhat was not tested: I did not exercise the live gateway path (the four open PRs also exercise the unit-test surface, not the live gateway); the unit test and repro script cover the full production gate code path through
runPreflightCompactionIfNeededClawSweeper review fixes (commit
5210d8b121)The 🦪 silver shellfish review flagged three P1 findings on commit
a140b0bb1b/ffd17f3a85; all three are addressed in commit5210d8b121:compaction.timeoutSecondsonly (no specific default minutes). The same wording fix applies to the inline code comment inagent-runner-memory.ts. The PR does not need to assume which timeout default the merge target carries.preflight.enabledto the JSON5 example and the bullet list underagents.defaults.compactionindocs/gateway/config-agents.md. Public docs now describe the new knob consistently with the schema help.openclaw.jsonto a temp state dir and load it through the productionloadConfig(). The gate value is now parsed from disk through the same code path the gateway startup uses, not cast from an inline literal.The single remaining CI failure (
checks-node-core-fast, surface budgetpublic exports 10328 > 10327andpublic callable exports 5185 > 5184) is pre-existing onopenclaw/mainHEAD alone — see comment. This PR adds zero new public SDK exports;git diff c57fee8239ed..HEAD -- src/plugin-sdk/is empty.Related
fix/95553-preflight-config-gate(carries only the preflight config gate; the priorfeat/task-route-lease-modulebranch was abandoned per PR feat(compaction): add compaction.preflight.enabled config gate (#95553) #95648 close reason)a140b0bb1b feat,ffd17f3a85 lint fix,5210d8b121 review fixes