fix: add timeout to parent-fork token count resolution (fixes #101718) by zw-xysk · Pull Request #101766 · openclaw/openclaw

zw-xysk · 2026-07-07T15:45:57Z

What Problem This Solves

resolveParentForkTokenCount reads the parent transcript to estimate token count when forking a session, but has no timeout or abort signal. On large transcripts (hundreds of MB) or slow filesystems (NFS, FUSE, saturated SSD), this blocks session creation and reply fork paths indefinitely, accumulating blocked async contexts that exhaust gateway connection slots (#101718).

Call chain

sessions.create({ fork: true })
  → session-create-service resolveParentForkDecision
    → session-fork.ts resolveParentForkTokenCount
      → session-fork.runtime.ts resolveParentForkTokenCountRuntime
        (fs.stat + async file read — NO TIMEOUT)

Changes

2 files, +54/-2 (source: +6; tests: +48)

src/auto-reply/reply/session-fork.ts (+6 lines):

Import withTimeout from ../../infra/fs-safe.js
Add PARENT_FORK_TOKEN_TIMEOUT_MS = 2_000 constant
Wrap resolveParentForkTokenCountRuntime in withTimeout(..., 2_000).catch(() => undefined)

When the 2-second timeout fires, resolveParentForkTokenCount returns undefined — the same sentinel the normal fallback path already produces. resolveParentForkDecision already handles undefined correctly: returns {status: "fork"} without parentTokens estimate.

This single bounded deadline (recommended by ClawSweeper review) covers all filesystem awaits inside the runtime (fs.stat + transcript read up to 1MB + potential index scan) in one wrapper.

src/auto-reply/reply/session-fork.test.ts (+48 lines):

3 new test cases for resolveParentForkDecision:
1. Fast runtime → returns fork with parentTokens (50k)
2. Hanging runtime → returns fork without parentTokens within 2s (timeout proof)
3. Parent too large → returns skip

Why 2 seconds and not 10?

The timeout only guards the stale-token filesystem probe after the synchronous freshPersistedTokens fast path
Even on NFS, fs.stat + 1MB tail read should complete in < 500ms
2s is aggressive but safe: returning undefined (allow fork without estimate) is always better than blocking indefinitely

Evidence

Unit tests (18/18 passed)

 Test Files  2 passed (2)
      Tests  18 passed (18)   [4 session-fork + 14 runtime]
   Start at  00:23:08
   Duration  6.63s  (tests 2.27s — confirms 2s timeout exercised)

Timeout test specifically runs for ~2008ms, confirming the deadline fires and fork proceeds without blocking.

Real behavior proof (13/13 passed — Platinum level)

  🧪 PROOF: session-fork timeout fix
  2026-07-07T16:24:13.292Z  |  LIN-66D8BD4EA2C  |  PID 1752285

======================================================================
  Section 1 — Normal Path: Real transcript → correct token count
======================================================================
  ✅ PASS: Returns positive token count  (40000)
  ✅ PASS: Count matches last-turn usage  (got 40000)
  ✅ PASS: Completes in < 5 s  (7 ms)

======================================================================
  Section 2 — IO-Failure Fallback: same path timeout uses
======================================================================
  ✅ PASS: Missing file → cached totalTokens  (got 42000)
  ✅ PASS: Empty file → cached totalTokens  (got 7000)

======================================================================
  Section 3 — Large Transcript: bounded return under real IO
======================================================================
  File: 4.9 MB
  ✅ PASS: Returns within 10 s  (12 ms)
  ✅ PASS: Returns positive token count  (1291729)

======================================================================
  Section 4 — Source Audit: withTimeout IS in session-fork.ts
======================================================================
  ✅ PASS: withTimeout imported
  ✅ PASS: Timeout = 2_000 ms
  ✅ PASS: Runtime call wrapped
  ✅ PASS: Timeout → undefined fallback

======================================================================
  Section 5 — Timeout Mechanism: withTimeout rejects slow promises
======================================================================
  ✅ PASS: Fast promise resolves normally
  ✅ PASS: Timeout fires within deadline  (100 ms)

======================================================================
  PROOF SUMMARY  |  LIN-66D8BD4EA2C  |  PID 1752285
  Fix: withTimeout(resolveParentForkTokenCountRuntime, 2_000)
======================================================================
  S1 Normal: 3p/0f  S2 Fallback: 2p/0f  S3 Large: 2p/0f
  S4 Audit: 4p/0f  S5 Timeout: 2p/0f
  TOTAL: 13 passed, 0 failed

Review

Manually reviewed and verified
AI-assisted: Yes
Fixes session-fork parent token count resolution has no timeout and blocks session creation indefinitely #101718

zw-xysk · 2026-07-07T15:48:25Z

@clawsweeper re-review

clawsweeper · 2026-07-07T15:48:28Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

clawsweeper · 2026-07-07T15:55:19Z

Codex review: needs real behavior proof before merge. Reviewed July 7, 2026, 12:39 PM ET / 16:39 UTC.

Summary
The PR adds a 2-second timeout around parent-fork token count resolution and adds facade tests for quick, timed-out, and oversized decisions.

PR surface: Source +6, Tests +46. Total +52 across 2 files.

Reproducibility: yes. source inspection gives a high-confidence path: make parent-token resolution exceed the new timeout and the PR returns a fork decision, after which callers still read the same parent transcript. I did not run a live slow-filesystem reproduction in this read-only review.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: serialized state: src/auto-reply/reply/session-fork.test.ts, unknown-data-model-change: src/auto-reply/reply/session-fork.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #101718
Summary: This PR is a candidate fix for the open parent-fork token timeout issue, with one sibling timeout PR and one adjacent accessor refactor touching the same area.

Members:

canonical: session-fork parent token count resolution has no timeout and blocks session creation indefinitely #101718 - The PR body uses closing syntax for this open issue, and the issue describes the same parent-fork token-count timeout problem.
same_root_cause: fix(session-fork): skip fork when parent size is unresolved instead of forking (#101718) #101767 - This is another open candidate PR for the same parent-fork token timeout issue and has overlapping risks around timeout-to-fork behavior.
partial_overlap: refactor(sessions): move parent-session forking behind the accessor boundary #101699 - The accessor-boundary refactor touches parent-session fork files and storage ownership, but it addresses ownership cleanup rather than the timeout bug directly.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
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:

Make timeout produce a bounded non-fork result or bound the subsequent fork transcript read.
[P2] Add focused regression coverage for timeout followed by fork creation, plus redacted live terminal/log proof from a delayed or large parent transcript run.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body includes copied terminal output for tests and a proof script, but it does not show a real session fork where a delayed token probe avoids the subsequent fork transcript read; the contributor should add redacted live output/logs after fixing the timeout semantics. 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

[P2] Merging as-is can still stall session creation or reply fork paths after the timeout fires because the next step reads the parent transcript during fork creation.
[P2] The timeout-to-undefined path can bypass the documented oversized-parent cap and create a fork from a parent whose size was never safely resolved.
[P2] There is another open candidate PR for the same issue, so maintainers should land one canonical timeout shape rather than two competing fixes.

Maintainer options:

Fix the timeout semantics before merge (recommended)
Change the timeout path so an unresolved size probe cannot proceed into an unbounded fork transcript read, then add focused tests and real delayed or large parent proof.
Land one canonical timeout branch
Coordinate this PR with fix(session-fork): skip fork when parent size is unresolved instead of forking (#101718) #101767 and keep only the branch that preserves the parent-size contract and proves the real fork path.
Accept unknown-size forks deliberately
Maintainers could choose to allow forks after timeout, but that would intentionally own the remaining hang and oversized-context risk.

Next step before merge

[P1] The branch needs contributor revision and real behavior proof; ClawSweeper should not auto-repair or merge an external PR while the changed path is still insufficiently proven.

Security
Cleared: The diff only changes session-fork timeout logic and colocated tests; I found no concrete security or supply-chain concern.

Review findings

[P1] Avoid forking after token-count timeout — src/auto-reply/reply/session-fork.ts:306

Review details

Best possible solution:

Make a timed-out parent-token probe choose a bounded non-fork path, or bound fork creation too, while preserving the oversized-parent isolated-context behavior and proving it with a delayed or large parent fork run.

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

Yes, source inspection gives a high-confidence path: make parent-token resolution exceed the new timeout and the PR returns a fork decision, after which callers still read the same parent transcript. I did not run a live slow-filesystem reproduction in this read-only review.

Is this the best way to solve the issue?

No: timing out to an unknown token count is not the best fix because it lets callers start the unbounded fork read and can bypass the oversized-parent cap. A safer fix either skips/isolates on timeout or bounds the subsequent fork transcript read too.

Full review comments:

[P1] Avoid forking after token-count timeout — src/auto-reply/reply/session-fork.ts:306
When this timeout path catches and returns undefined, resolveParentForkDecision treats the parent as safe to fork. Both the storage-boundary fork path and sessions.create { fork: true } then call forkSessionFromParent, whose runtime reads the same parent transcript with readRegularFile, so a slow or huge transcript can still stall creation and the documented oversized-parent skip can be bypassed. Return a conservative skip/isolated result on timeout or also bound the fork transcript read before allowing the fork.
Confidence: 0.89

Overall correctness: patch is incorrect
Overall confidence: 0.89

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 453f5968bbca.

Label changes

Label changes:

add merge-risk: 🚨 session-state: The timeout fallback can fork from a parent whose size was never confirmed, bypassing the isolated-context behavior for oversized parents.
add merge-risk: 🚨 availability: The proposed timeout can still lead into an unbounded parent transcript read, preserving the stall risk it aims to fix.

Label justifications:

P2: The PR targets a normal-priority session-fork stability bug with source-backed availability and session-state impact but limited blast radius.
merge-risk: 🚨 session-state: The timeout fallback can fork from a parent whose size was never confirmed, bypassing the isolated-context behavior for oversized parents.
merge-risk: 🚨 availability: The proposed timeout can still lead into an unbounded parent transcript read, preserving the stall risk it aims to fix.
rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body includes copied terminal output for tests and a proof script, but it does not show a real session fork where a delayed token probe avoids the subsequent fork transcript read; the contributor should add redacted live output/logs after fixing the timeout semantics. 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 +6, Tests +46. Total +52 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	7	1	+6
Tests	1	47	1	+46
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	54	2	+52

What I checked:

PR timeout fallback: PR head wraps resolveParentForkTokenCountRuntime in withTimeout but catches timeout/error as undefined, which resolveParentForkDecision treats as a fork decision without parentTokens. (src/auto-reply/reply/session-fork.ts:306, fa09d43051a2)
Current fork caller after unknown token count: Current main returns status fork when parentTokens is undefined, and forkSessionEntryFromParent then calls forkSessionFromParent for that decision. (src/auto-reply/reply/session-fork.ts:140, 453f5968bbca)
Gateway caller also forks after the decision: sessions.create with fork: true awaits resolveParentForkDecision and then calls forkSessionFromParent when the decision is not skip. (src/gateway/session-create-service.ts:435, 453f5968bbca)
Fork runtime reads the same parent transcript: forkSessionFromParentRuntime calls readForkSourceTranscript, which reads the parent transcript with readRegularFile before writing the child fork. (src/auto-reply/reply/session-fork.runtime.ts:307, 453f5968bbca)
Documented oversized-parent behavior: The public session-management docs say parent forks over the fixed 100K-token cap should start isolated instead of inheriting unusable history. Public docs: docs/reference/session-management-compaction.md. (docs/reference/session-management-compaction.md:105, 453f5968bbca)
Existing tests prove byte-estimate skip intent: Existing runtime tests expect stale transcripts without usage to use a conservative byte estimate above 100K, which feeds the oversized-parent skip behavior. (src/auto-reply/reply/session-fork.runtime.test.ts:84, 453f5968bbca)

Likely related people:

vincentkoc: git blame ties the current parent-fork decision/runtime and gateway fork paths to commit 098b471 and GitHub metadata for refactor(memory): remove unused runtime facade exports #101583 lists vincentkoc as author and merger. (role: recent area contributor and merger; confidence: high; commits: 098b471143c1; files: src/auto-reply/reply/session-fork.ts, src/auto-reply/reply/session-fork.runtime.ts, src/gateway/session-create-service.ts)
jalehman: current main and the open accessor-boundary PR show recent work moving session storage/forking behavior toward the accessor boundary, which overlaps the best fix shape for this timeout. (role: recent adjacent session-accessor contributor; confidence: medium; commits: 453f5968bbca, c8a22142cd19; files: src/config/sessions/session-accessor.ts, src/auto-reply/reply/session-fork.ts, src/auto-reply/reply/session-fork.runtime.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.

Review history (1 earlier review cycle)

reviewed 2026-07-07T16:02:01.550Z sha 658818b :: needs real behavior proof before merge. :: [P2] Bound the byte-estimate stat before racing the read

…w#101718) resolveParentForkTokenCount reads the parent transcript to estimate token count when forking a session, but has no timeout. On large transcripts or slow filesystems (NFS, FUSE), this blocks session creation indefinitely, accumulating blocked async contexts on the gateway. Wrap the runtime call in resolveParentForkTokenCount with withTimeout(..., 2_000) so the function returns undefined within bounded time. resolveParentForkDecision already handles undefined parentTokens correctly (returns fork without a token estimate), which is the safe default vs. blocking indefinitely. Includes regression tests for the fast-resolve, timeout, and too-large paths.

zw-xysk · 2026-07-07T16:26:16Z

@clawsweeper re-review

clawsweeper · 2026-07-07T16:26:19Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

openclaw-barnacle Bot added the size: XS label Jul 7, 2026

clawsweeper Bot mentioned this pull request Jul 7, 2026

fix(session-fork): skip fork when parent size is unresolved instead of forking (#101718) #101767

Closed

zw-xysk force-pushed the pr-101718-fork-timeout branch from 658818b to fa09d43 Compare July 7, 2026 16:23

openclaw-barnacle Bot added size: S and removed size: XS labels Jul 7, 2026

clawsweeper Bot added merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 7, 2026

clawsweeper Bot mentioned this pull request Jul 8, 2026

fix(session): avoid stalls when parent token probing hangs #101932

Open

zw-xysk closed this Jul 8, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: add timeout to parent-fork token count resolution (fixes #101718)#101766

fix: add timeout to parent-fork token count resolution (fixes #101718)#101766
zw-xysk wants to merge 1 commit into
openclaw:mainfrom
zw-xysk:pr-101718-fork-timeout

zw-xysk commented Jul 7, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

zw-xysk commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

zw-xysk commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Call chain

Changes

Why 2 seconds and not 10?

Evidence

Unit tests (18/18 passed)

Real behavior proof (13/13 passed — Platinum level)

Review

Uh oh!

zw-xysk commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zw-xysk commented Jul 7, 2026

Uh oh!

clawsweeper Bot commented Jul 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

zw-xysk commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading

clawsweeper Bot commented Jul 7, 2026 •

edited

Loading