Skip to content

Compaction Runner: emit transcript updates post-compact#25558

Open
rodrigouroz wants to merge 4 commits intoopenclaw:mainfrom
rodrigouroz:codex/pr20038-06
Open

Compaction Runner: emit transcript updates post-compact#25558
rodrigouroz wants to merge 4 commits intoopenclaw:mainfrom
rodrigouroz:codex/pr20038-06

Conversation

@rodrigouroz
Copy link
Contributor

@rodrigouroz rodrigouroz commented Feb 24, 2026 •
edited
Loading

Summary

  • Problem: the compact runner path updated the session transcript in-place but did not emit a transcript-update event afterward.
  • Why it matters: downstream transcript consumers can miss the post-compaction state until some later write happens to the session file.
  • What changed: emit the session transcript update immediately after a successful session.compact(...) call.
  • What did NOT change (scope boundary): no post-compaction memory sync implementation in this PR.

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

  • Gateway / orchestration
  • Memory / storage
  • Skills / tool execution
  • Auth / tokens
  • Integrations
  • API / contracts
  • UI / DX
  • CI/CD / infra

Linked Issue/PR

User-visible / Behavior Changes

  • Successful compaction now emits the same transcript update signal that other transcript-mutating paths already emit.

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): default

Steps

  1. pnpm test -- src/agents/pi-embedded-runner/compact.hooks.test.ts
  2. pnpm tsgo
  3. pnpm exec oxfmt --check src/agents/pi-embedded-runner/compact.ts src/agents/pi-embedded-runner/compact.hooks.test.ts

Expected

  • The compact runner emits a transcript update after successful compaction.
  • The new regression test passes.

Actual

  • Local verification 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 the compact runner now calls emitSessionTranscriptUpdate(...) immediately after a successful session.compact(...).
  • Added and ran a regression test proving a transcript update is emitted once with the normalized session file path.
  • Verified no behavior change to the failure/cancel paths in this PR.

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.
  • Files/config to restore: src/agents/pi-embedded-runner/compact.ts
  • Known bad symptoms reviewers should watch for: duplicate or missing post-compaction transcript update side effects in downstream listeners.

Risks and Mitigations

  • Risk: downstream listeners could react differently now that the compact runner emits promptly after success.
    • Mitigation: this aligns the compact runner with the existing transcript-update signaling model used by other transcript-mutating paths.

This was referenced Feb 24, 2026
Draft
Draft
Draft
@rodrigouroz rodrigouroz force-pushed the codex/pr20038-06 branch 2 times, most recently from 8666603 to 46e9211 Compare February 24, 2026 15:38
@rodrigouroz rodrigouroz mentioned this pull request Feb 24, 2026
Closed
18 tasks
@openclaw-barnacle openclaw-barnacle bot added the app: web-ui App: web-ui label Feb 24, 2026
@rodrigouroz rodrigouroz force-pushed the codex/pr20038-06 branch 5 times, most recently from 4e9f814 to 905ec22 Compare February 24, 2026 16:39
@rodrigouroz rodrigouroz marked this pull request as draft February 24, 2026 16:59
@openclaw-barnacle openclaw-barnacle bot removed the app: web-ui App: web-ui label Feb 26, 2026
@openclaw-barnacle
Copy link

openclaw-barnacle bot commented Mar 4, 2026

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 Mar 4, 2026
@rodrigouroz rodrigouroz marked this pull request as ready for review March 7, 2026 17:57
@rodrigouroz
Copy link
Contributor Author

rodrigouroz commented Mar 7, 2026

@codex review
@greptile-apps[bot] review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 7, 2026

Greptile Summary

This PR fixes a missing emitSessionTranscriptUpdate signal in the compaction runner path, aligning it with the transcript-update signaling model already used by other transcript-mutating paths (appendMessage in session-tool-result-guard.ts and mirrorTranscript in transcript.ts).

Changes:

  • src/agents/pi-embedded-runner/compact.ts: Adds emitSessionTranscriptUpdate(params.sessionFile) immediately after a successful session.compact() call (line 741). Placement is correct — compactWithSafetyTimeout rethrows on timeout/failure so the emit is naturally skipped on error, and all early-return paths return before reaching this line.
  • src/agents/pi-embedded-runner/compact.hooks.test.ts: Adds a regression test that registers a live onSessionTranscriptUpdate listener, runs compaction with a whitespace-padded sessionFile, and asserts the listener was called exactly once with the trimmed path. The test correctly uses the real (non-mocked) transcript-events.ts module and cleans up the listener in a finally block to prevent leaks.

The fix is minimal, well-scoped, and consistent with existing patterns in the codebase. No issues found.

Confidence Score: 5/5

  • This PR is safe to merge — it is a minimal, targeted addition of one function call in the success path with a clear regression test.
  • The change is a single-line addition in an already-guarded success path that follows the exact pattern used by every other transcript-mutating call site in the codebase. The new regression test correctly verifies the emit contract (call count, trimmed path) and prevents listener leaks. No error handling, early-return, or lock-release paths are affected. The implementation correctly trims the sessionFile path before emitting (verified in transcript-events.ts), and placement immediately after compaction succeeds is consistent with other transcript-update patterns throughout the codebase.
  • No files require special attention.

Last reviewed commit: 4e48de0

@chatgpt-codex-connector
Copy link

chatgpt-codex-connector bot commented Mar 7, 2026

Codex Review: Didn't find any major issues. Keep it up!

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

@rodrigouroz
Copy link
Contributor Author

rodrigouroz commented Mar 7, 2026

@jalehman Hi Josh, this is the natural continuation to #25557
Thank you!

@jalehman jalehman self-assigned this Mar 10, 2026
jalehman added a commit to rodrigouroz/openclaw that referenced this pull request Mar 11, 2026
@jalehman
fix: note compaction transcript update (openclaw#25558 thanks @rodrig…
f5606c7 
…ouroz)
jalehman added a commit to rodrigouroz/openclaw that referenced this pull request Mar 11, 2026
@jalehman
fix: repair cron nested lane guard (openclaw#25558 thanks @rodrigouroz)
2c399b0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@jalehman jalehman

Labels

agents Agent runtime and tooling size: XS stale Marked as stale due to inactivity

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rodrigouroz @jalehman @thewilloftheshadow