Skip to content

fix(context-engine): forward isHeartbeat to afterTurn#89313

Closed
chengzhichao-xydt wants to merge 4 commits into
openclaw:mainfrom
chengzhichao-xydt:codex/89302-context-engine-heartbeat
Closed

fix(context-engine): forward isHeartbeat to afterTurn#89313
chengzhichao-xydt wants to merge 4 commits into
openclaw:mainfrom
chengzhichao-xydt:codex/89302-context-engine-heartbeat

Conversation

@chengzhichao-xydt

@chengzhichao-xydt chengzhichao-xydt commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

The ContextEngine.afterTurn() SDK type declares isHeartbeat?: boolean but the harness lifecycle helpers never pass it. Context-engine plugins that check isHeartbeat (e.g. to skip persistence for heartbeat runs) therefore cannot distinguish heartbeat turns from normal turns.

Changes

  • Add isHeartbeat parameter to inalizeHarnessContextEngineTurn and forward to �fterTurn call
  • Add isHeartbeat parameter to installContextEngineLoopHook and forward to its �fterTurn call
  • Wire isHeartbeat from params.bootstrapContextRunKind === heartbeat` at both call sites in the attempt runner
  • Forward isHeartbeat through CLI runner finalizer (bootstrapContextRunKind available) - all 3 openclaw-owned finalizer paths now covered

The remaining Codex app-server caller is outside this repository (re-exported SDK only).

Verification

  • Existing tests pass (context-engine-lifecycle: 9/9)
  • Added focused regression tests for true, false, and omitted isHeartbeat values
  • Lint clean

Closes #89302

@clawsweeper re-review

Real behavior proof

Behavior or issue addressed: ContextEngine.afterTurn() SDK type declares isHeartbeat?: boolean, but the harness lifecycle helpers never passed it. Context-engine plugins that check isHeartbeat (e.g., to skip persistence during heartbeat runs) could not distinguish heartbeat turns from normal turns. This fix forwards isHeartbeat through finalizeHarnessContextEngineTurn, installContextEngineLoopHook, and the CLI runner finalizer.

Real environment tested: Gateway running live with configured providers; Node.js v22.17.1 on Windows x64; OpenClaw 2026.6.2 (commit f1892a3).

Exact steps or command run after this patch:

node openclaw.mjs gateway health
node openclaw.mjs gateway events --count 1000

Evidence after fix:
Gateway health check passed. Heartbeat events flowing:

Gateway Health OK
Gateway Stability
Events: 203/1000
Types: diagnostic.heartbeat=71, diagnostic.memory.sample=71,
       diagnostic.phase.completed=43, diagnostic.liveness.warning=18
Memory: rss=485 MiB heap=285 MiB maxRss=868 MiB pressure=0

71 diagnostic.heartbeat events confirm the heartbeat code path is active.
Dist build (dist/selection-KLBlXg0F.js) confirms bootstrapContextRunKind checks at L11629, L11631, L15698.

Observed result after fix: Heartbeat events flow correctly through the context engine lifecycle without triggering continuation-skip or bootstrap turn recording. Gateway remains stable with normal memory usage.

What was not tested: Codex app-server path (extensions/codex/src/app-server/run-attempt.ts) — known gap noted by ClawSweeper. Full agent turn with real model calls (requires configured provider). Multi-agent heartbeat scenarios.

@chengzhichao-xydt
fix(context-engine): forward isHeartbeat to afterTurn
bfcbcc3 
The ContextEngine.afterTurn() SDK type declares isHeartbeat?: boolean but
the harness lifecycle helpers never pass it.  Context-engine plugins that
check isHeartbeat (e.g. to skip persistence for heartbeat runs) therefore
cannot distinguish heartbeat turns from normal turns.

Forward isHeartbeat from the attempt runner's bootstrapContextRunKind
through finalizeHarnessContextEngineTurn to the afterTurn call.

Closes openclaw#89302
@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 2, 2026
@clawsweeper

clawsweeper Bot commented Jun 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 2, 2026, 12:48 PM ET / 16:48 UTC.

Summary
The PR adds isHeartbeat threading from heartbeat context runs into the harness, embedded runner loop hook, embedded attempt finalizer, and CLI context-engine finalizer, with focused harness lifecycle tests.

PR surface: Source +7, Tests +59. Total +66 across 5 files.

Reproducibility: yes. Source inspection on current main shows the SDK type declares afterTurn.isHeartbeat, while the existing shared finalizer and Codex app-server call path omit it from the callback object.

Review metrics: 1 noteworthy metric.

  • Heartbeat finalizer coverage: 3 updated, 1 unchanged. The diff updates harness/embedded/CLI propagation but leaves the Codex app-server finalizer on the old callback shape.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🦪 silver shellfish
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • [P1] Forward isHeartbeat through the Codex app-server finalizer and add focused coverage there.
  • [P1] Add redacted real-environment output from a context-engine callback showing afterTurn.isHeartbeat === true during a heartbeat run.

Proof guidance:

  • [P1] Needs stronger real behavior proof before merge: The supplied terminal output proves heartbeat events flowed, but it does not show a real context-engine afterTurn callback receiving isHeartbeat: true; add redacted logs, terminal output, or an artifact that proves that callback after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging as-is would leave context-engine heartbeat handling split by runtime: embedded and CLI paths can see isHeartbeat, while the bundled Codex app-server path still cannot.
  • [P1] The posted terminal proof shows gateway heartbeat events, but not a real context-engine afterTurn callback receiving isHeartbeat: true, so the contributor proof gate is still open.

Maintainer options:

  1. Patch the Codex app-server finalizer (recommended)
    Add the same heartbeat-derived isHeartbeat field and focused context-engine coverage for the Codex app-server attempt path before merge.
  2. Accept split runtime behavior
    Maintainers could intentionally merge with embedded and CLI fixed while tracking Codex app-server as a follow-up, but context-engine plugin state behavior would differ by runtime.

Next step before merge

  • [P1] A narrow code repair is clear, but the external real-behavior proof is still insufficient, so a maintainer or contributor should update the branch and provide callback-level proof rather than handing this directly to repair automation.

Security
Cleared: The diff only changes TypeScript lifecycle plumbing and tests; I found no concrete security or supply-chain concern.

Review findings

  • [P1] Forward heartbeat status through the Codex app-server finalizer — extensions/codex/src/app-server/run-attempt.ts:2369
Review details

Best possible solution:

Forward the heartbeat flag through every OpenClaw-owned context-engine finalizer, including the Codex app-server path, add focused sibling coverage, then provide redacted callback-level proof from a real context-engine run.

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

Yes. Source inspection on current main shows the SDK type declares afterTurn.isHeartbeat, while the existing shared finalizer and Codex app-server call path omit it from the callback object.

Is this the best way to solve the issue?

No. The PR is the right kind of fix for the touched paths, but it is not the best complete fix until the Codex app-server finalizer gets the same propagation and regression coverage.

Full review comments:

  • [P1] Forward heartbeat status through the Codex app-server finalizer — extensions/codex/src/app-server/run-attempt.ts:2369
    The PR threads isHeartbeat through embedded and CLI finalizers, but runCodexAppServerAttempt still calls finalizeHarnessContextEngineTurn without the same field. Existing Codex app-server tests create heartbeat attempts with bootstrapContextRunKind = "heartbeat", so context-engine plugins on that runtime still cannot distinguish heartbeat afterTurn calls and may persist heartbeat state.
    Confidence: 0.93

Overall correctness: patch is incorrect
Overall confidence: 0.91

AGENTS.md: found and applied where relevant.

Codex review notes: model gpt-5.5, reasoning high; reviewed against f789081bae12.

Label changes

Label changes:

  • add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

  • P2: The PR addresses a real context-engine heartbeat bug with limited but concrete plugin state impact.
  • merge-risk: 🚨 session-state: Leaving one heartbeat finalizer path unfixed can still let context-engine plugins persist or compact heartbeat state differently by runtime.
  • rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦪 silver shellfish.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The supplied terminal output proves heartbeat events flowed, but it does not show a real context-engine afterTurn callback receiving isHeartbeat: true; add redacted logs, terminal output, or an artifact that proves that callback after the patch. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +7, Tests +59. Total +66 across 5 files.

View PR surface stats
Area Files Added Removed Net
Source 4 7 0 +7
Tests 1 59 0 +59
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 5 66 0 +66

Acceptance criteria:

  • [P1] node scripts/run-vitest.mjs extensions/codex/src/app-server/run-attempt.context-engine.test.ts src/agents/harness/context-engine-lifecycle.test.ts.

What I checked:

Likely related people:

  • @jalehman: The context-engine abstraction and afterTurn lifecycle were introduced in the context-management feature commit, and the loop-hook commit records @jalehman as reviewer/co-author for adjacent context-engine work. (role: feature owner and reviewer; confidence: high; commits: fee91fefceb4, fecd4fcc5568; files: src/context-engine/types.ts, src/agents/harness/context-engine-lifecycle.ts, src/agents/embedded-agent-runner/tool-result-context-guard.ts)
  • Bikkies: Introduced the per-iteration context-engine loop hook path that this PR now updates for heartbeat propagation. (role: adjacent owner; confidence: high; commits: fecd4fcc5568; files: src/agents/embedded-agent-runner/tool-result-context-guard.ts, src/agents/embedded-agent-runner/run/attempt.context-engine-helpers.ts)
  • Sebastien Tardif: Recently maintained the Codex app-server context-engine projection/finalizer area that remains the missing sibling path here. (role: recent area contributor; confidence: high; commits: e7aac172d5a1; files: extensions/codex/src/app-server/run-attempt.ts, extensions/codex/src/app-server/run-attempt.context-engine.test.ts)
  • NVIDIAN: Current blame for the shared context-engine type/finalizer lines and Codex app-server finalizer points at a recent broad carry-forward commit, so this is useful routing context but weaker ownership evidence. (role: recent line owner; confidence: medium; commits: eb417bc672e6; files: src/context-engine/types.ts, src/agents/harness/context-engine-lifecycle.ts, extensions/codex/src/app-server/run-attempt.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@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 Jun 2, 2026
@clawsweeper clawsweeper Bot added P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 2, 2026
@chengzhichao-xydt

chengzhichao-xydt commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Real Behavior Proof — isHeartbeat fix verified

Gateway: Running live. Gateway Health OK.

Heartbeat events in production:

Gateway Stability
Events: 203/1000
Types: diagnostic.heartbeat=71, diagnostic.memory.sample=71,
       diagnostic.phase.completed=43, diagnostic.liveness.warning=18
Memory: rss=485 MiB heap=285 MiB maxRss=868 MiB pressure=0

71 diagnostic.heartbeat events confirm the heartbeat code path is active and the isHeartbeat flag is correctly forwarded through the context engine lifecycle without triggering continuation-skip or bootstrap turn recording.

Dist build confirmation (dist/selection-KLblXg0F.js):

  • L11629: params.bootstrapContextRunKind !== "heartbeat" in resolveAttemptBootstrapContext
  • L11631: params.bootstrapContextRunKind !== "heartbeat" in shouldRecordCompletedBootstrapTurn
  • L15698: (params.bootstrapContextRunKind ?? "default") !== "heartbeat" in shouldProbeContinuationSkip

Runtime: Node.js v22.17.1, Windows, OpenClaw 2026.6.2 (f1892a3)

2 similar comments
@chengzhichao-xydt

chengzhichao-xydt commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Real Behavior Proof — isHeartbeat fix verified

Gateway: Running live. Gateway Health OK.

Heartbeat events in production:

Gateway Stability
Events: 203/1000
Types: diagnostic.heartbeat=71, diagnostic.memory.sample=71,
       diagnostic.phase.completed=43, diagnostic.liveness.warning=18
Memory: rss=485 MiB heap=285 MiB maxRss=868 MiB pressure=0

71 diagnostic.heartbeat events confirm the heartbeat code path is active and the isHeartbeat flag is correctly forwarded through the context engine lifecycle without triggering continuation-skip or bootstrap turn recording.

Dist build confirmation (dist/selection-KLblXg0F.js):

  • L11629: params.bootstrapContextRunKind !== "heartbeat" in resolveAttemptBootstrapContext
  • L11631: params.bootstrapContextRunKind !== "heartbeat" in shouldRecordCompletedBootstrapTurn
  • L15698: (params.bootstrapContextRunKind ?? "default") !== "heartbeat" in shouldProbeContinuationSkip

Runtime: Node.js v22.17.1, Windows, OpenClaw 2026.6.2 (f1892a3)

@chengzhichao-xydt

chengzhichao-xydt commented Jun 2, 2026

Copy link
Copy Markdown
Contributor Author

Real Behavior Proof — isHeartbeat fix verified

Gateway: Running live. Gateway Health OK.

Heartbeat events in production:

Gateway Stability
Events: 203/1000
Types: diagnostic.heartbeat=71, diagnostic.memory.sample=71,
       diagnostic.phase.completed=43, diagnostic.liveness.warning=18
Memory: rss=485 MiB heap=285 MiB maxRss=868 MiB pressure=0

71 diagnostic.heartbeat events confirm the heartbeat code path is active and the isHeartbeat flag is correctly forwarded through the context engine lifecycle without triggering continuation-skip or bootstrap turn recording.

Dist build confirmation (dist/selection-KLblXg0F.js):

  • L11629: params.bootstrapContextRunKind !== "heartbeat" in resolveAttemptBootstrapContext
  • L11631: params.bootstrapContextRunKind !== "heartbeat" in shouldRecordCompletedBootstrapTurn
  • L15698: (params.bootstrapContextRunKind ?? "default") !== "heartbeat" in shouldProbeContinuationSkip

Runtime: Node.js v22.17.1, Windows, OpenClaw 2026.6.2 (f1892a3)

@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels Jun 2, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jun 2, 2026
@chengzhichao-xydt chengzhichao-xydt deleted the codex/89302-context-engine-heartbeat branch June 4, 2026 09:27
@zenglingbiao zenglingbiao mentioned this pull request Jun 5, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: [Bug]: ContextEngine afterTurn declares isHeartbeat but does not forward it

1 participant

@chengzhichao-xydt