Skip to content

fix(gateway): guard maxLines compact against active session runs#72765

Closed
chinadbo wants to merge 4 commits into
openclaw:mainfrom
chinadbo:fix/sessions-compact-maxlines-active-run
Closed

fix(gateway): guard maxLines compact against active session runs#72765
chinadbo wants to merge 4 commits into
openclaw:mainfrom
chinadbo:fix/sessions-compact-maxlines-active-run

Conversation

@chinadbo

Copy link
Copy Markdown

Summary

  • sessions.compact with maxLines skips the interruptSessionRunIfActive guard that the AI compaction path uses
  • When an agent run is active, archiveFileOnDisk renames the live transcript to .bak; the active runner continues writing to the renamed file while writeFileSync creates a new truncated file at the original path — all post-compact agent output is silently lost
  • The fix inserts the same interruptSessionRunIfActive guard used by the AI compaction path, mirroring the pattern also used by sessions.compaction.restore

Fix

Add interruptSessionRunIfActive call to the maxLines branch of sessions.compact before the transcript file is read and overwritten.

Test plan

  • sessions.compact with maxLines during an active run now aborts the run first (same behavior as AI compaction)
  • sessions.compact with maxLines when no run is active works unchanged
  • pnpm check:changed passes

Thanks @ioodu

@openclaw-barnacle openclaw-barnacle Bot added gateway Gateway runtime size: XS labels Apr 27, 2026
@greptile-apps

greptile-apps Bot commented Apr 27, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a missing interruptSessionRunIfActive guard to the maxLines branch of sessions.compact, preventing data loss when compaction runs against an active session. The fix mirrors the identical guard already present in the AI compaction path and the restore path.

Confidence Score: 5/5

Safe to merge — a minimal, targeted fix with no observed issues.

Single-file change that adds one guard matching an established pattern already used twice in the same handler. Parameters and error handling are identical to the AI compaction path. No new logic, no side-effect changes.

No files require special attention.

Reviews (1): Last reviewed commit: "fix(gateway): guard maxLines compact aga..." | Re-trigger Greptile

@chinadbo

Copy link
Copy Markdown
Author

The failing CI checks (checks-fast-contracts-plugins, checks-node-agentic-agents, checks-node-core) are pre-existing failures on main unrelated to this PR:

  • checks-fast-contracts-plugins: extensions/qwen/vllm missing @mariozechner/pi-ai runtime dep declaration
  • checks-node-agentic-agents: model-auth.test.ts assertion failure (custom-local vs ollama-local) — pre-existing test flake
  • checks-node-core: pre-existing

@chinadbo
chinadbo force-pushed the fix/sessions-compact-maxlines-active-run branch from e63091e to 4561004 Compare April 27, 2026 12:47
@openclaw-barnacle openclaw-barnacle Bot added size: S channel: discord Channel integration: discord extensions: memory-core Extension: memory-core commands Command implementations size: L and removed size: XS size: S labels Apr 27, 2026
@chinadbo
chinadbo force-pushed the fix/sessions-compact-maxlines-active-run branch from c41d0a0 to 0607295 Compare April 27, 2026 13:34
@openclaw-barnacle openclaw-barnacle Bot added size: XS and removed channel: discord Channel integration: discord extensions: memory-core Extension: memory-core commands Command implementations size: L labels Apr 27, 2026
@chinadbo
chinadbo force-pushed the fix/sessions-compact-maxlines-active-run branch 2 times, most recently from 5cec420 to 237adb0 Compare April 27, 2026 14:19
@steipete

Copy link
Copy Markdown
Contributor

Re-reviewed current head.

The earlier chatAbortControllers.values() TypeError looks fixed by the null-safe fallback, and the production code path is the right one: current main still interrupts active runs on the AI compaction path, but not before the maxLines archive/rewrite path.

Before merge, please add regression coverage. This PR currently touches only src/gateway/server-methods/sessions.ts; the risky behavior is transcript mutation during an active run. Suggested test: call sessions.compact with maxLines while the target session has an active run, assert the interrupt path is used before archive/write, and assert an interrupt error / still-active run prevents archiveFileOnDisk() and the transcript rewrite. Existing maxLines tests cover truncation, not this active-run guard.

Live check note: current PR checks are green except the parity gate.

@chinadbo

Copy link
Copy Markdown
Author

Added the regression test as requested. The test (sessions.compact with maxLines rejects when an embedded run is active and does not stop) uses embeddedRunMock to simulate an active run that refuses to stop, then asserts:

  1. sessions.compact with maxLines returns ok: false with UNAVAILABLE error code
  2. The transcript file is unchanged (still 10 lines)
  3. No .bak. archive file was created (i.e. archiveFileOnDisk was never called)

All 53 tests in the file pass.

@chinadbo
chinadbo force-pushed the fix/sessions-compact-maxlines-active-run branch from 9ede439 to 2cf2a8d Compare April 28, 2026 03:11
@clawsweeper

clawsweeper Bot commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: the active-run data-loss bug is real, but this branch is now conflicting, lacks real behavior proof, and the clean mergeable replacement PR already carries the same guard in the current sessions code with stronger tests and proof.

Canonical path: Use #91378 as the canonical current-main fix path and close this conflicting branch as superseded.

So I’m closing this here and keeping the remaining discussion on #91378.

Review details

Best possible solution:

Use #91378 as the canonical current-main fix path and close this conflicting branch as superseded.

Do we have a high-confidence way to reproduce the issue?

Yes from source inspection. Current main’s maxLines branch reaches transcript tail read, archive, and rewrite without the active-run interrupt used by the non-maxLines compaction branch.

Is this the best way to solve the issue?

No for this branch. The guard shape is right, but the clean replacement PR is the better solution because it applies to current main, preserves agent scoping, adds current-suite coverage, and supplies real behavior proof.

Security review:

Security review cleared: Security review cleared: this stale branch changes gateway session runtime logic and tests only, with no dependency, workflow, package, credential, or supply-chain surface changes.

AGENTS.md: found and applied where relevant.

What I checked:

  • Current main still has the maxLines gap: Current main calls interruptSessionRunIfActive in the non-maxLines compaction branch, but the maxLines branch proceeds to readRecentSessionTranscriptLines, archiveFileOnDisk, and writeFileSync without that guard. (src/gateway/server-methods/sessions.ts:2517, 66880a5d736c)
  • Current active-run helper is agent-scoped: The current helper accepts agentId and defaultAgentId, so a rebased fix needs to preserve current agent scoping rather than replay the old branch shape. (src/gateway/server-methods/session-active-runs.ts:63, 66880a5d736c)
  • This PR is stale relative to current main: Live PR metadata reports this branch as CONFLICTING/DIRTY, with triage: needs-real-behavior-proof and status: needs proof labels still present. (9b95bc649ab0)
  • Canonical replacement is open and viable: feat(cli): add openclaw sessions compact and fail loudly on CLI /compact (fixes #90640) #91378 is open, clean, mergeable, maintainer-editable, and labeled proof: sufficient / status: ready for maintainer look. (5951a4562037)
  • Replacement includes the same guard in the current code path: The replacement PR adds interruptSessionRunIfActive to the current maxLines branch with agentId: requestedAgentId and places regression coverage in the current server.sessions.compaction.test.ts suite. (src/gateway/server-methods/sessions.ts:2643, 99878847dfbc)
  • Replacement proof covers the destructive-write ordering: The replacement PR body and ClawSweeper review include real in-process Gateway proof showing maxLines truncation aborts before archive/truncate when an active run cannot stop. (5951a4562037)

Likely related people:

  • scoootscooob: Introduced the gateway compaction checkpoint/session compaction surface that this guard modifies. (role: feature introducer; confidence: high; commits: f4fcaa09a358; files: src/gateway/server-methods/sessions.ts, src/gateway/server.sessions.gateway-server-sessions-a.test.ts, src/gateway/session-compaction-checkpoints.ts)
  • Alix-007: Authored the open clean replacement PR that ports this guard to current main with current-suite tests and real behavior proof. (role: canonical replacement contributor; confidence: high; commits: 99878847dfbc, 5951a4562037; files: src/gateway/server-methods/sessions.ts, src/gateway/server.sessions.compaction.test.ts)
  • Peter Steinberger: Reviewed this PR’s active-run compaction path and requested regression coverage; also recently touched adjacent gateway sessions tests. (role: recent area reviewer; confidence: medium; commits: f6193687691d; files: src/gateway/server.sessions.gateway-server-sessions-a.test.ts)
  • Vincent Koc: Recently touched the same gateway sessions test area used by the stale branch. (role: recent adjacent contributor; confidence: medium; commits: 56625a189bf3; files: src/gateway/server.sessions.gateway-server-sessions-a.test.ts)

Codex review notes: model internal, reasoning high; reviewed against 66880a5d736c.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels May 20, 2026
@clawsweeper

clawsweeper Bot commented May 20, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper PR egg

🎁 Pass real behavior proof to wake the egg and unlock a hatchable treat.

Where did the egg go?
  • The egg game starts only after the PR passes the real-behavior proof check.
  • Before that, no creature or rarity is rolled. The treat waits for real proof.
  • This is still just collectible flavor: proof affects review readiness, not creature quality.

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 20, 2026
@openclaw-barnacle

Copy link
Copy Markdown

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

@openclaw-barnacle openclaw-barnacle Bot added the stale Marked as stale due to inactivity label Jun 4, 2026
@clawsweeper clawsweeper Bot added the P1 High-priority user-facing bug, regression, or broken workflow. label Jun 4, 2026
Alix-007 added a commit to Alix-007/openclaw that referenced this pull request Jun 8, 2026
…ve runs

The non-maxLines (LLM) compact branch interrupts an active session run before
compacting, but the maxLines truncate branch read the tail, archived, and
overwrote the transcript in place without that guard. Exposing `--max-lines`
as a documented CLI command (this PR) would make the active-run data-loss mode
tracked by openclaw#72765 easy to trigger from ordinary CLI usage.

Run the same interruptSessionRunIfActive guard in the maxLines branch before
reading the tail and truncating, matching the LLM compact path. Add gateway
regression coverage over a real in-process Gateway: with no active run, the
maxLines branch truncates the on-disk transcript 500 -> 50 and preserves the
original 500 lines in the .bak archive; with an active embedded run, the
maxLines branch fires the same interrupt (abort + wait-for-end) before
archiving and truncating.
Alix-007 added a commit to Alix-007/openclaw that referenced this pull request Jun 8, 2026
…ve runs

The non-maxLines (LLM) compact branch interrupts an active session run before
compacting, but the maxLines truncate branch read the tail, archived, and
overwrote the transcript in place without that guard. Exposing `--max-lines`
as a documented CLI command (this PR) would make the active-run data-loss mode
tracked by openclaw#72765 easy to trigger from ordinary CLI usage.

Run the same interruptSessionRunIfActive guard in the maxLines branch before
reading the tail and truncating, matching the LLM compact path. Add gateway
regression coverage over a real in-process Gateway: with no active run, the
maxLines branch truncates the on-disk transcript 500 -> 50 and preserves the
original 500 lines in the .bak archive; with an active embedded run, the
maxLines branch fires the same interrupt (abort + wait-for-end) before
archiving and truncating.
@openclaw-barnacle openclaw-barnacle Bot removed the stale Marked as stale due to inactivity label Jun 14, 2026
@clawsweeper clawsweeper Bot added the merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. label Jun 14, 2026
@clawsweeper

clawsweeper Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

@clawsweeper clawsweeper Bot closed this Jun 14, 2026
Alix-007 added a commit to Alix-007/openclaw that referenced this pull request Jun 19, 2026
…ve runs

The non-maxLines (LLM) compact branch interrupts an active session run before
compacting, but the maxLines truncate branch read the tail, archived, and
overwrote the transcript in place without that guard. Exposing `--max-lines`
as a documented CLI command (this PR) would make the active-run data-loss mode
tracked by openclaw#72765 easy to trigger from ordinary CLI usage.

Run the same interruptSessionRunIfActive guard in the maxLines branch before
reading the tail and truncating, matching the LLM compact path. Add gateway
regression coverage over a real in-process Gateway: with no active run, the
maxLines branch truncates the on-disk transcript 500 -> 50 and preserves the
original 500 lines in the .bak archive; with an active embedded run, the
maxLines branch fires the same interrupt (abort + wait-for-end) before
archiving and truncating.
steipete pushed a commit to Alix-007/openclaw that referenced this pull request Jun 19, 2026
…ve runs

The non-maxLines (LLM) compact branch interrupts an active session run before
compacting, but the maxLines truncate branch read the tail, archived, and
overwrote the transcript in place without that guard. Exposing `--max-lines`
as a documented CLI command (this PR) would make the active-run data-loss mode
tracked by openclaw#72765 easy to trigger from ordinary CLI usage.

Run the same interruptSessionRunIfActive guard in the maxLines branch before
reading the tail and truncating, matching the LLM compact path. Add gateway
regression coverage over a real in-process Gateway: with no active run, the
maxLines branch truncates the on-disk transcript 500 -> 50 and preserves the
original 500 lines in the .bak archive; with an active embedded run, the
maxLines branch fires the same interrupt (abort + wait-for-end) before
archiving and truncating.
sallyom added a commit that referenced this pull request Jun 19, 2026
…ompact` (fixes #90640) (#91378)

* feat(cli): add `sessions compact` command and fail loudly on CLI `/compact`

`sessions.compact` was reachable only as an internal Gateway RPC — no CLI
command, no docs — and `openclaw agent --message '/compact'` silently no-opped
with exit 0 because the slash-command handler rejects CLI-originated senders,
so the message fell through to an ordinary agent turn that compacted nothing.

- Add `openclaw sessions compact <key>` wrapping the existing `sessions.compact`
  RPC; exit non-zero on a transport error or an `ok:false` payload so automation
  never mistakes a silent no-op for success.
- Reject `openclaw agent --message '/compact'` with a redirect to the new
  command and exit 1 instead of a silent exit 0. The shared chat-side `/compact`
  handler is left untouched (no compatibility / message-delivery blast radius).
- Strictly validate `--max-lines` and `--timeout` (positive integers only).
- Document the command and the `sessions.compact` RPC in docs/cli/sessions.md.

Fixes #90640.

* fix(cli): inherit parent `sessions` options for `compact`

`openclaw sessions compact <key>` did not merge the parent `sessions`
command options the way its sibling subcommands (list/cleanup/info/…) do,
so a parent-level `--agent`/`--json` was silently dropped. In particular
`openclaw sessions --agent work compact <key>` compacted the default
agent's session instead of the work agent's — a wrong-target session-state
mutation.

Merge the parent options in the compact action (parent `--agent`/`--json`,
with the compact-level option taking precedence) and add regression
coverage for parent `--agent`, parent `--json`, and the compact-level
override.

Refs #90640.

* fix(cli): report pending Codex compaction and reject unsupported parent options

Address two ClawSweeper review findings on the `sessions compact` command:

- `sessions-compact.ts`: the Codex app-server `thread/compact/start` path
  returns `ok:true / compacted:false` with a pending marker, meaning the
  compaction was *started* asynchronously. The formatter collapsed every
  non-compacted success into "No compaction needed", so Codex users were told
  nothing happened. Report it as a started/pending compaction instead.
- `register.status-health-sessions.ts`: the parent `sessions` command defines
  list-only options (`--store`/`--all-agents`/`--active`/`--limit`) that the
  compact action previously ignored. Silently dropping a parent `--store` is
  dangerous — the gateway resolves the target store itself, so a user could
  believe they targeted one store while another is mutated. Reject any
  unsupported inherited parent option with a clear error and a non-zero exit.

Add regression tests for the pending-compaction message and the rejected
parent options.

Refs #90640.

* fix(gateway): guard sessions.compact maxLines truncation against active runs

The non-maxLines (LLM) compact branch interrupts an active session run before
compacting, but the maxLines truncate branch read the tail, archived, and
overwrote the transcript in place without that guard. Exposing `--max-lines`
as a documented CLI command (this PR) would make the active-run data-loss mode
tracked by #72765 easy to trigger from ordinary CLI usage.

Run the same interruptSessionRunIfActive guard in the maxLines branch before
reading the tail and truncating, matching the LLM compact path. Add gateway
regression coverage over a real in-process Gateway: with no active run, the
maxLines branch truncates the on-disk transcript 500 -> 50 and preserves the
original 500 lines in the .bak archive; with an active embedded run, the
maxLines branch fires the same interrupt (abort + wait-for-end) before
archiving and truncating.

* docs(cli): move sessions compact section above related links

The new "Compact a session" section was inserted between the cleanup
section's inline "Related:" list and the page's final "## Related"
block, splitting related-link content around the command docs. Move the
compact section above the related-links area and merge the orphaned
"Session config" link into the single final "## Related" block.

* fix(gateway): avoid no-op compact aborts

Signed-off-by: sallyom <[email protected]>

* fix(gateway): satisfy compact preflight lint

Signed-off-by: sallyom <[email protected]>

* fix(sessions): preserve compacted transcript structure

---------

Signed-off-by: sallyom <[email protected]>
Co-authored-by: sallyom <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jun 20, 2026
…ompact` (fixes openclaw#90640) (openclaw#91378)

* feat(cli): add `sessions compact` command and fail loudly on CLI `/compact`

`sessions.compact` was reachable only as an internal Gateway RPC — no CLI
command, no docs — and `openclaw agent --message '/compact'` silently no-opped
with exit 0 because the slash-command handler rejects CLI-originated senders,
so the message fell through to an ordinary agent turn that compacted nothing.

- Add `openclaw sessions compact <key>` wrapping the existing `sessions.compact`
  RPC; exit non-zero on a transport error or an `ok:false` payload so automation
  never mistakes a silent no-op for success.
- Reject `openclaw agent --message '/compact'` with a redirect to the new
  command and exit 1 instead of a silent exit 0. The shared chat-side `/compact`
  handler is left untouched (no compatibility / message-delivery blast radius).
- Strictly validate `--max-lines` and `--timeout` (positive integers only).
- Document the command and the `sessions.compact` RPC in docs/cli/sessions.md.

Fixes openclaw#90640.

* fix(cli): inherit parent `sessions` options for `compact`

`openclaw sessions compact <key>` did not merge the parent `sessions`
command options the way its sibling subcommands (list/cleanup/info/…) do,
so a parent-level `--agent`/`--json` was silently dropped. In particular
`openclaw sessions --agent work compact <key>` compacted the default
agent's session instead of the work agent's — a wrong-target session-state
mutation.

Merge the parent options in the compact action (parent `--agent`/`--json`,
with the compact-level option taking precedence) and add regression
coverage for parent `--agent`, parent `--json`, and the compact-level
override.

Refs openclaw#90640.

* fix(cli): report pending Codex compaction and reject unsupported parent options

Address two ClawSweeper review findings on the `sessions compact` command:

- `sessions-compact.ts`: the Codex app-server `thread/compact/start` path
  returns `ok:true / compacted:false` with a pending marker, meaning the
  compaction was *started* asynchronously. The formatter collapsed every
  non-compacted success into "No compaction needed", so Codex users were told
  nothing happened. Report it as a started/pending compaction instead.
- `register.status-health-sessions.ts`: the parent `sessions` command defines
  list-only options (`--store`/`--all-agents`/`--active`/`--limit`) that the
  compact action previously ignored. Silently dropping a parent `--store` is
  dangerous — the gateway resolves the target store itself, so a user could
  believe they targeted one store while another is mutated. Reject any
  unsupported inherited parent option with a clear error and a non-zero exit.

Add regression tests for the pending-compaction message and the rejected
parent options.

Refs openclaw#90640.

* fix(gateway): guard sessions.compact maxLines truncation against active runs

The non-maxLines (LLM) compact branch interrupts an active session run before
compacting, but the maxLines truncate branch read the tail, archived, and
overwrote the transcript in place without that guard. Exposing `--max-lines`
as a documented CLI command (this PR) would make the active-run data-loss mode
tracked by openclaw#72765 easy to trigger from ordinary CLI usage.

Run the same interruptSessionRunIfActive guard in the maxLines branch before
reading the tail and truncating, matching the LLM compact path. Add gateway
regression coverage over a real in-process Gateway: with no active run, the
maxLines branch truncates the on-disk transcript 500 -> 50 and preserves the
original 500 lines in the .bak archive; with an active embedded run, the
maxLines branch fires the same interrupt (abort + wait-for-end) before
archiving and truncating.

* docs(cli): move sessions compact section above related links

The new "Compact a session" section was inserted between the cleanup
section's inline "Related:" list and the page's final "## Related"
block, splitting related-link content around the command docs. Move the
compact section above the related-links area and merge the orphaned
"Session config" link into the single final "## Related" block.

* fix(gateway): avoid no-op compact aborts

Signed-off-by: sallyom <[email protected]>

* fix(gateway): satisfy compact preflight lint

Signed-off-by: sallyom <[email protected]>

* fix(sessions): preserve compacted transcript structure

---------

Signed-off-by: sallyom <[email protected]>
Co-authored-by: sallyom <[email protected]>
Co-authored-by: Peter Steinberger <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gateway Gateway runtime merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P1 High-priority user-facing bug, regression, or broken workflow. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants