Skip to content

fix(compaction): recover stalled pre-prompt sessions - #1735

Open
GunP4ng wants to merge 3 commits into
code-yeongyu:mainfrom
GunP4ng:fix/astra-preprompt-compaction-recovery
Open

GunP4ng wants to merge 3 commits into
code-yeongyu:mainfrom
GunP4ng:fix/astra-preprompt-compaction-recovery

Conversation

@GunP4ng

@GunP4ng GunP4ng commented Sep 16, 2026

Copy link
Copy Markdown

Summary

  • route mandatory pre_prompt compaction failures through deterministic recovery
  • advance past unsafe split-turn payloads to the earliest suffix that still passes retained-message safety, atomic tool-chain, reconstruction, and budget checks
  • preserve fail-closed behavior for optional branch and extension compaction

Reproduction

A resumed Astra session on omo 5.0.0-0.beta.63 / Senpi 2026.9.15-2 reached mandatory pre-prompt compaction. Summary generation emitted no provider events for 300,000 ms, then compaction was rejected with the context unchanged. The persisted branch also contained an unsafe tool result after the prepared boundary and no later user boundary, so the existing deterministic fallback could not select a suffix.

Read-only replay of that 32 MB branch after this patch selects later-safe-boundary at 185,265 estimated tokens under a 576,000-token budget while preserving atomic tool pairs.

Tests

  • failing-first focused regression: 2 expected failures before implementation
  • focused fallback suite: 35/35 passed
  • full compaction suite: 75 files, 561 tests passed
  • NODE_OPTIONS=--max-old-space-size=8192 bun run check
  • NODE_OPTIONS=--max-old-space-size=8192 bun run build
  • Senpi QA common self-check: 10/10
  • isolated CLI smoke: 8/8, real auth unchanged
  • isolated real mock-provider loop: passed and wrote request/stdout receipts
  • independent gate reviewer: PASS

The broad mock-loop self-test currently reports 30/48 because of pre-existing protocol-leak expectation failures unrelated to this compaction diff.

Related work


Summary by cubic

Fixes mandatory pre_prompt compaction stalling when provider summarization fails. At the effective hard cap, pre-prompt failures now recover deterministically instead of deadlocking; below-cap proactive attempts stay fail-closed and preserve the full context. When earlier boundaries retain unsafe split-turn content, recovery advances to the earliest suffix that passes replay-safety, atomic tool-chain, and budget checks. Optional branch and extension compaction remain fail-closed.

Written for commit 29f8d9e. Summary will update on new commits.

Review in cubic

Route mandatory pre-prompt failures through deterministic recovery and advance past unsafe split-turn payloads to the earliest replay-safe suffix.

Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
@code-yeongyu

Copy link
Copy Markdown
Owner

Thanks for this — I reproduced the same class of user-visible deadlock independently while investigating a field report (a summarization stream that ran past 690 s and then left every following prompt failing with the session's internal retry-suppression marker). Your diff is on current main, failing-first, and it does not weaken any of the safety checks that recovery path relies on: the new later-safe-boundary scan inherits projectCandidate's unsafe-suffix, atomic-tool-chain and budget checks rather than bypassing them. I read it as correct for the stall class it targets.

One review item before merge. Adding pre_prompt to isRequiredCompactionFallbackReason makes every pre-prompt compaction failure eligible for destructive context reduction. PR #646 deliberately kept a hard-cap gate for that route (apply the deterministic fallback only when usage.tokens >= contextWindow - reserveTokens; below the cap, a failed proactive pre-prompt compaction stayed fail-closed and preserved full context). A below-cap pre-prompt compaction is proactive, and its failure wedges nothing, so dropping detail there costs the user context for no liveness gain. Could you either adopt that gate or state why removing it is correct here? Everything else reads mergeable to me.

What this PR intentionally does not cover — tracked separately in #1741 so it does not grow your diff:

  • classifyRequiredCompactionFallbackFailure is still a four-class enumeration, so a summary stream terminated by a provider or credential-rotation error still classifies as unknown and the fallback never runs — on every route, including the ones this PR widens.
  • The internal retry-suppression marker still reaches user-visible text.
  • Final result() settlement still happens after the watchdog timer's cleanup, which is an unbounded stall vector with no timer at all (your PR does not introduce it; fix(compaction): recover prepared suffix after summary timeout #646 uniquely carried a fix for it).
  • Nothing bounds the total wall clock of one compaction across attempts: the per-attempt budget is size-scaled up to a 30-minute ceiling, so a large session legitimately licenses the 690 s the reporter saw.

#646: its base predates the deterministic-fallback.ts rewrite and the size-scaled budget, so its fit-estimator and watchdog hunks patch shapes that no longer exist; the two mechanisms it uniquely carries are being lifted onto current main in #1741 rather than rebasing that branch.

GunP4ng and others added 2 commits September 16, 2026 04:45
Synchronize the review branch with current Senpi main before applying the requested hard-cap gate.

Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
Preserve full context when proactive below-cap pre-prompt compaction fails while retaining deterministic recovery at the effective hard limit.

Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
@GunP4ng

GunP4ng commented Sep 16, 2026

Copy link
Copy Markdown
Author

Addressed in 29f8d9ea0 after merging current main (555312b31).

I adopted the hard-cap gate rather than making every pre_prompt failure eligible:

  • manual, threshold, and overflow remain unconditionally eligible.
  • pre_prompt now requires known usage at usage.contextWindow - resolveEffectiveReserveTokens(...) or above.
  • unknown usage, invalid windows, and proactive below-cap pre_prompt failures remain fail-closed and preserve the full branch.
  • the same predicate gates both inherited warm failures and direct compaction failures.

The regression pins the effective-reserve boundary on the 10k test window: 9600 recovers and 9599 cancels without compaction. I also merged the current-main #1741 work before applying this response, so the branch now uses the latest provider-failure classification and timeout changes.

Updated verification:

  • focused deterministic fallback: 36/36
  • full compaction suite: 77 files, 576 tests
  • bun run check and bun run build: pass with the existing 8 GB TypeScript heap setting
  • targeted Biome and git diff --check: clean
  • CLI smoke: 8/8; real auth unchanged
  • built-module boundary QA: hard cap true, one token below false
  • affected-session read-only replay: later-safe-boundary, 122,165 / 576,000 tokens
  • independent gate reviewer: PASS

Thanks for calling out the proactive-context-loss case.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants