Skip to content

tests(agents): mock timeout compaction side effects at runtime seam#65274

Merged
hxy91819 merged 1 commit into
openclaw:mainfrom
hxy91819:fix/timeout-compaction-hook-harness
Apr 12, 2026
Merged

tests(agents): mock timeout compaction side effects at runtime seam#65274
hxy91819 merged 1 commit into
openclaw:mainfrom
hxy91819:fix/timeout-compaction-hook-harness

Conversation

@hxy91819

@hxy91819 hxy91819 commented Apr 12, 2026

Copy link
Copy Markdown
Member

Summary

  • CI pipeline: checks-node-agentic
  • Failing problem: src/agents/pi-embedded-runner/run.timeout-triggered-compaction.test.ts -> fires compaction hooks during timeout recovery for ownsCompaction engines
  • Problem: the shared overflow-compaction harness still mocked ./compact.js after run.ts switched to importing runPostCompactionSideEffects from ./compaction-hooks.js
  • Why it matters: timeout-recovery coverage for owns-compaction engines started failing even though runtime behavior was still calling the side-effect seam
  • What changed: mock the current ./compaction-hooks.js seam in the shared harness while keeping the existing ./compact.js mock for compatibility
  • What did NOT change (scope boundary): runtime compaction behavior, hook execution order, and non-agent surfaces

Change Type (select all)

  • Bug fix
  • Feature
  • Refactor required for the fix
  • Docs
  • Security hardening
  • Chore/infra

Scope (select all touched areas)

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

Linked Issue/PR

Root Cause (if applicable)

  • Root cause: the test harness mocked the old barrel seam after src/agents/pi-embedded-runner/run.ts started importing runPostCompactionSideEffects directly from src/agents/pi-embedded-runner/compaction-hooks.ts
  • Missing detection / guardrail: the shared harness did not keep its mocked module path aligned with the runtime import boundary
  • Contributing context (if known): overflow and timeout recovery share the same harness, so the stale mock only showed up once a test asserted the post-compaction side-effect call directly

Regression Test Plan (if applicable)

  • Coverage level that should have caught this:
    • Unit test
    • Seam / integration test
    • End-to-end test
    • Existing coverage already sufficient
  • Target test or file: src/agents/pi-embedded-runner/run.timeout-triggered-compaction.test.ts
  • Scenario the test should lock in: owns-compaction timeout recovery fires post-compaction side effects through the same runtime seam that run.ts imports
  • Why this is the smallest reliable guardrail: the failure came from the shared mock boundary rather than the core timeout-recovery logic itself
  • Existing test that already covers this (if any): src/agents/pi-embedded-runner/run.overflow-compaction.test.ts exercises the same harness for overflow recovery
  • If no new test is added, why not: existing targeted coverage was sufficient once the harness mocked the right seam

User-visible / Behavior Changes

None.

Diagram (if applicable)

N/A

Before:
run.ts -> compaction-hooks.ts -> side effects
harness -> mocks compact.ts only
=> timeout recovery assertion misses the real call site

After:
run.ts -> compaction-hooks.ts -> mocked side effects seam
harness -> mocks compaction-hooks.ts (and keeps compact.ts compatibility mock)
=> timeout recovery assertion observes the runtime call path

Security Impact (required)

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

Repro + Verification

Environment

  • OS: Linux
  • Runtime/container: Node 22 / pnpm workspace
  • Model/provider: N/A
  • Integration/channel (if any): N/A
  • Relevant config (redacted): default test harness config

Steps

  1. Run pnpm test src/agents/pi-embedded-runner/run.timeout-triggered-compaction.test.ts src/agents/pi-embedded-runner/run.overflow-compaction.test.ts
  2. Observe timeout-recovery owns-compaction coverage
  3. Confirm both timeout and overflow compaction suites pass

Expected

  • The timeout-recovery test sees runPostCompactionSideEffects invoked through the current runtime seam.

Actual

  • Before this change, the timeout-recovery test failed because the harness mocked the wrong module path.

Evidence

Attach at least one:

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

Human Verification (required)

  • Verified scenarios: timeout-triggered compaction and overflow compaction both pass with the shared harness after mocking ./compaction-hooks.js
  • Edge cases checked: kept the legacy ./compact.js mock in place so older callers remain covered by the same harness
  • What you did not verify: full repo-wide agent/contract suites

Review Conversations

  • I replied to or resolved every bot review conversation I addressed in this PR.
  • I left unresolved only the conversations that still need reviewer or maintainer judgment.

Compatibility / Migration

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

Risks and Mitigations

  • Risk: another harness or test could drift from the runtime import seam in the future
    • Mitigation: this shared harness now mocks both the legacy barrel and the runtime seam used by run.ts

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS maintainer Maintainer-authored PR labels Apr 12, 2026
@greptile-apps

greptile-apps Bot commented Apr 12, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a stale mock boundary in the shared overflow-compaction test harness. After run.ts switched to importing runPostCompactionSideEffects directly from ./compaction-hooks.js, the harness was still only mocking ./compact.js, causing timeout-recovery assertions to miss the real call site. The fix adds a vi.doMock("./compaction-hooks.js", ...) entry pointing to the already-existing mockedRunPostCompactionSideEffects mock, while keeping the legacy ./compact.js mock for backward compatibility.

Confidence Score: 5/5

Safe to merge — test-only change with no runtime impact.

The fix is minimal, correct, and well-scoped: both mocked module paths point to the same pre-existing mock instance, the reset function already covers mockedRunPostCompactionSideEffects, and keeping the legacy ./compact.js mock avoids breaking overflow-compaction coverage. No production code is touched.

No files require special attention.

Reviews (1): Last reviewed commit: "tests: mock timeout compaction side effe..." | Re-trigger Greptile

@hxy91819
hxy91819 merged commit d2edb55 into openclaw:main Apr 12, 2026
44 of 47 checks passed
TOMUIV pushed a commit to TOMUIV/openclaw that referenced this pull request Apr 14, 2026
lovewanwan pushed a commit to lovewanwan/openclaw that referenced this pull request Apr 28, 2026
ogt-redknie pushed a commit to ogt-redknie/OPENX that referenced this pull request May 2, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 9, 2026
globalcaos pushed a commit to globalcaos/tinkerclaw that referenced this pull request May 13, 2026
github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request May 24, 2026
jameslcowan pushed a commit to jameslcowan/openclaw that referenced this pull request Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling maintainer Maintainer-authored PR size: XS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant