Skip to content

fix(context): preserve forced recovery with transcript delta ingestion - #1233

Open
jlapenna wants to merge 6 commits into
Martian-Engineering:mainfrom
jlapenna:fix/1232-delivered-media-recovery
Open

jlapenna wants to merge 6 commits into
Martian-Engineering:mainfrom
jlapenna:fix/1232-delivered-media-recovery

Conversation

@jlapenna

@jlapenna jlapenna commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Problem and resulting behavior

Forced-budget recovery counts OpenClaw's display-only nested-tool rows as missing persisted coverage, and a delivered assistant reply whose terminal MEDIA directives were consumed cannot match its original stored response. Recovery then refuses a fully represented tool turn.

This revision integrates main's delta-cursor ingestion (#1238) and legacy-reconciliation removal (#1243). It counts persistable roles at the existing current-turn coverage gate and lets the delta resync owner restamp a delivered assistant response only when host delivery facts, provider response identity, consumed URL order, and remaining structured content all match. The candidate must belong to the offered suffix and follow prior claims. Original stored bytes and parts remain intact.

The removed projection reconciler, weak-identity adoption planner, and legacy fresh-tail expansion are not restored. The new host-owned ingestion contract handles ordinary committed turns; #1228 remains linked for its historical bare-batch report rather than being claimed as fully closed by this narrowed delta. Existing refusal, initiating-user, unresolved-tool, and raw-history checks remain. Patch changesets and configuration documentation are included; no new host capability is required.

Closes #1224. Closes #1232. Related #1228 and openclaw/openclaw#165459.

Verification

Current head: ac476e92dcabf8eacda6e4afc81d1bda0c5ea376, a normal merge retaining 1842246 and current-main snapshot 13a9b6a; no history replacement.

  • Type checks and complete plugin/CLI/migration/UI build pass.
  • Focused overflow-recovery and resync-planner suites: 54 tests pass (51.97 seconds under concurrent local load). Replacing only the engine with the main version makes the three new recovery integration cases fail (5.76 seconds); candidate restores their recovery behavior while retaining original stored bytes.
  • Independent read-only review found no concrete P0/P1/P2 issue in the port and its delivered-response/suffix boundaries.
  • Full suite: 2,086 passed and 2 failed out of 2,088 tests (294.93 seconds), both default five-second timeouts: the existing 2,100-row/9-MB paging case and the existing successive-batch tool-burst case. These failures are retained, not relabeled as green or proven unrelated. Both cases pass on unchanged main in an isolated comparison; the candidate's isolated tool-burst case passes but its paging case still times out. A longer-budget diagnostic completed the paging assertions, which does not establish success under the default gate. No timeout, assertion, baseline, or CI control was weakened in the committed tree.
  • Fresh upstream CI run 38193388233 passes on this head: all 2,088 tests pass, and plugin-inspector and smoke-latest-openclaw pass. No unresolved review threads were found in the fully paginated current-head inspection. Earlier local timeouts remain recorded above; the successful hosted result is not claimed as a code fix for those local timings.

Earlier runtime evidence and limits

The prior candidate recovered the original failing native session from 44,770 to 11,450 active tokens with two durable leaf summaries, retaining its session and history. That observation belongs to the earlier implementation and is not presented as a new live deployment or current-head runtime proof. This revision changes neither the installed plugin nor the live Gateway, model, sessions, credentials, or printer state.

Upstream maintainer review and merge are required; this contributor has READ access and cannot arm auto-merge. The current scope is the surviving recovery repair under the new ingestion architecture.

@jlapenna

jlapenna commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

Hi @jalehman — could you give this recovery repair an initial maintainer review? The current head passes upstream test, plugin-inspector and smoke-latest-openclaw checks. The PR records a 2,138-test local suite and recovery of the original previously failing session (44,770 → 11,450 active tokens), without deleting history or relaxing coverage. Patch changesets are included. The most useful review focus is the strict delivered-media matching and current-turn provenance/legacy-fencing boundaries. Please let me know if another regression case or a narrower split would help.

@jlapenna jlapenna changed the title fix(context): reconcile native tool turns and delivered media during forced recovery fix(context): preserve forced recovery with transcript delta ingestion Oct 12, 2026
@jlapenna

jlapenna commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor Author

@jalehman The conflict is resolved at ac476e9 through a normal merge of current main. I narrowed the repair to the surviving delta-ingestion owners after #1238 and #1243: the deleted projection reconciler and weak-adoption path stay removed. Delivered-response matches must belong to the offered in-order suffix, with exact host delivery/provider/structured-content proof, and nested display rows no longer count as required persisted coverage.

Type checks, all package builds, 54 focused tests, and independent scoped review pass. The updated body also records the full-suite result honestly: 2,086 pass, two existing five-second test timeouts remain under local verification, and the paging timeout also recurs in isolation. Fresh upstream CI now passes on this candidate: https://github.com/Martian-Engineering/lossless-claw/actions/runs/38193388233 (all 2,088 tests, plugin-inspector and smoke-latest-openclaw pass). No unresolved review threads were found. The local timeout observations remain recorded; no timeout/assertion/CI change is included.

The most useful review focus is now the delta resync handoff and canonical-content preservation, plus the existing current-turn coverage boundary. I removed the broad #1228 closing claim because its old bare-batch implementation was superseded by main's new ingestion contract; #1224 and #1232 remain the repair's closing scope. No installed plugin or live session was changed.

jalehman added a commit that referenced this pull request Oct 19, 2026
Forced overflow recovery compared stored transcript entry ids against every
visible entry in the current turn, including OpenClaw's display-only
nested-tool `custom` rows, which are never persisted. Coverage could never
match on Tool Search turns, so recovery always refused. Count only
persistable roles, and log count-only diagnostics when coverage is refused.

Fixes #1224. Split from #1233.

Co-authored-by: Joe LaPenna <jlapenna@gmail.com>
@jalehman

jalehman commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Thanks for this, @jlapenna. I split the nested-tool coverage fix out into #1254 (with you credited as co-author) and landed it, which closes #1224.

For the delivered-media half (#1232), there's one change needed before it can land. matchesHostDeliveredAssistant requires openclawDelivery.textPhaseRequiresTerminal === true, but:

  • In stable OpenClaw through 2026.10.4, only the OpenAI-completions transport writes that field, so replies from other transports never match.
  • On OpenClaw main, openclaw#169020 removed the writer, the transcript delivery reader no longer reads the field, and it's deprecated as an ignored field (assistant-text-phase-terminal-hint). So the match stops working entirely on the next host release.

mediaUrls, the stable provider response ID, the exact in-order footer and the identical remaining structure are still enough proof. Could you:

  1. Drop the textPhaseRequiresTerminal requirement, and add a regression case without it.
  2. In matchHostDeliveredAssistantId, check for openclawDelivery.mediaUrls before the three store lookups, so unmatched assistant rows during a resync don't pay for queries that can't succeed.
  3. Rebase onto main, now that the coverage half is in.

About the two local timeouts: I checked them against the merge base. Run alone, both tests take the same time on base and PR (about 1.0–1.2s). When three full suites run at once, unchanged base times out on the same paging test, and plugin-tool-session-scope times out on both. So they come from load, not from this change.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants