Skip to content

fix(compression): harden pre-compress provider checkpoints - #2

Closed
GottZ wants to merge 2 commits into
Tranquil-Flow:fix/on-pre-compress-return-discardedfrom
GottZ:gottz/pre-compress-provider-context-hardening
Closed

fix(compression): harden pre-compress provider checkpoints#2
GottZ wants to merge 2 commits into
Tranquil-Flow:fix/on-pre-compress-return-discardedfrom
GottZ:gottz/pre-compress-provider-context-hardening

Conversation

@GottZ

@GottZ GottZ commented Jul 13, 2026

Copy link
Copy Markdown

Summary

  • preserve MemoryProvider.on_pre_compress() output deterministically in the durable compression handoff instead of asking the summarizer to reproduce it
  • collect and sanitize provider context before compression side effects; optionally require a non-empty checkpoint via compression.require_memory_checkpoint
  • fail closed for oversized checkpoint data or context engines that cannot explicitly preserve it
  • replace broad TypeError retry dispatch with signature inspection and exactly one engine invocation
  • harden resume/recompression with durable summary markers, merged-tail restoration, and role-alternation preservation
  • keep gateway agent caching aware of the new compression setting and document the operator-facing behavior

This is a focused hardening layer on top of 9d2a83c1b / the branch used for NousResearch#7195. It is based directly on fix/on-pre-compress-return-discarded, so it does not pull the unrelated upstream history into this repository.

Compatibility

  • Existing context engines with strict signatures remain compatible while providers return no checkpoint context.
  • If non-empty checkpoint context exists, an engine must explicitly accept pre_compress_context; otherwise compression aborts without dropping messages.
  • Historical merged handoffs (summary → END marker → surviving tail) are restored before recompression. Ambiguous unmarked prefix text in the protected head is preserved rather than deleted.

Verification

  • scripts/run_tests.sh across 10 focused compression, persistence, plugin, gateway-cache, and redaction files: 256 passed, 0 failed
  • Ruff on all changed Python files: passed
  • python -m compileall on all changed Python files: passed
  • git diff --check: passed
  • both commits verified as SSH-signed for git@gottz.de

The local historical checkout reused a newer shared virtualenv that lacked the branch's pytest-timeout plugin, so the focused wrapper run overrode only pytest addopts; CI should run with the repository's normal dependency set. No full-suite or release-readiness claim is made.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Hermes’ context compression flow so memory-provider “pre-compress” checkpoint/context can be preserved deterministically across compression/resume cycles, with stricter sanitization and an optional “fail closed” policy when a durable checkpoint is required.

Changes:

  • Add compression.require_memory_checkpoint config + gateway cache-signature coverage, and enforce checkpoint collection/sanitization before any compression side effects.
  • Replace retry-by-TypeError compression-engine dispatch with signature inspection and a single invocation that only passes supported kwargs (including pre_compress_context when explicitly supported).
  • Improve compression durability/continuity (durable summary end markers, merged-tail restoration, provider-context blocks) and add focused regression tests, plus stricter URL credential redaction for durable boundaries.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
website/docs/user-guide/cli.md Documents the new compression.require_memory_checkpoint operator setting and its failure behavior.
hermes_cli/config.py Adds compression.require_memory_checkpoint to default config.
gateway/run.py Includes the new compression subkey in the gateway agent-cache signature inputs.
agent/agent_init.py Plumbs require_memory_checkpoint into agent.compression_memory_checkpoint_required.
agent/conversation_compression.py Adds pre-compress checkpoint gating, signature-based engine invocation, and early-abort behavior.
agent/context_engine.py Extends the context engine interface with force and pre_compress_context.
agent/context_compressor.py Makes summaries durably marked, appends provider context deterministically, and restores legacy merged-tail handoffs.
agent/memory_provider.py Updates the hook contract docs to describe durable handoff preservation and checkpoint-required behavior.
agent/memory_manager.py Updates hook doc wording and tightens failure logging.
agent/redact.py Adds redact_url_credentials() and strict durable-boundary secret redaction patterns.
tests/test_on_pre_compress_memory.py Replaces prior string-search tests with behavioral contracts for checkpoints, redaction, engine compatibility, and resume paths.
tests/agent/test_context_compressor_summary_continuity.py Updates summary continuity fixtures to use the durable end marker.
tests/gateway/test_agent_cache.py Verifies the new compression config key participates in gateway cache signatures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread agent/conversation_compression.py
@GottZ
GottZ force-pushed the gottz/pre-compress-provider-context-hardening branch from 0d523f6 to 1eeff98 Compare July 13, 2026 14:58
@GottZ
GottZ requested a review from Copilot July 13, 2026 15:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

Comment on lines +425 to +432
if pre_compress_context and not accepts_pre_compress_context:
return _abort_for_checkpoint(
"configured context engine cannot preserve memory checkpoint context"
)

# Probe auxiliary feasibility only after the checkpoint gate. A failed
# required checkpoint must not trigger provider calls or mutate the
# one-time feasibility state.
Comment thread tui_gateway/server.py
Comment on lines 2692 to 2696
try:
removed, usage = _compress_session_history(
removed, usage, changed = _compress_session_history(
session,
focus_topic,
approx_tokens=before_tokens,
GottZ added 2 commits July 13, 2026 15:25
Checkpoint only content compaction will discard while preserving original tool payloads, merged-tail data, and full-fidelity gateway transcripts. Isolate untrusted provider and context-engine mutations, fail closed before state changes when checkpoints are required, and normalize equal-copy compression results to no-ops. Persist sanitized provider context across compression and resume without treating ambiguous legacy prefix literals as handoffs.
Cover drop-window scope, nested mutation isolation, required-checkpoint no-ops, equal-copy engines, merged-summary tails, full-fidelity gateway transcripts, conservative legacy recognition, redaction, persistence, cache, and manual gateway/TUI session-state invariants.
@GottZ
GottZ force-pushed the gottz/pre-compress-provider-context-hardening branch from 1eeff98 to 9a2901e Compare July 13, 2026 15:31
Tranquil-Flow added a commit that referenced this pull request Jul 13, 2026
migrate_full_providers() in openclaw_to_hermes.py guarded the
"already exists" path with `not self.overwrite`, but on the overwrite
branch it fell through to `custom_providers.append(entry)` instead
of replacing the existing entry. Re-running `hermes claw migrate
--overwrite` produced two entries with the same name (NousResearch#18097, #2).

Track the existing entry's index, replace it in place when overwrite
is set, append only when no entry exists. Behavior without
--overwrite is unchanged: still records a conflict and skips.

Fixes the second of three sub-bugs in NousResearch#18097. The WhatsApp config
gap (sub-bug 1) is addressed by another open PR (NousResearch#18337). The stale
gateway PID report (sub-bug 3) needs more diagnostic info from the
reporter to confirm a reproducible code path.

Refs NousResearch#18097
Tranquil-Flow pushed a commit that referenced this pull request Jul 16, 2026
Completes the review's ask for "adapter-to-session-key integration coverage
for Discord and a non-Discord platform" on NousResearch#20096.

Drives a concrete adapter's real BasePlatformAdapter.build_source with an
injected gateway_runner, asserts the matched route's profile is stamped on
the source, and that build_session_key scopes the key under agent:<profile>:
(versus the shared agent:main: namespace). Covers Discord and Telegram — the
Telegram case is the bug-#2 path that previously fell through to default.
Adds a regression anchor: without gateway_runner, profile stays None and the
key lands in agent:main (the silent fallback the fix removes for non-Discord).

Co-Authored-By: Claude <noreply@anthropic.com>
Tranquil-Flow pushed a commit that referenced this pull request Jul 16, 2026
…st (NousResearch#65214)

Moves the fireworks entry in CANONICAL_PROVIDERS from its old slot
(after GMI Cloud) to directly below Nous Portal, ahead of OpenRouter.
Order propagates automatically to hermes model, the setup wizard,
Telegram /model, and the desktop provider catalog.
Tranquil-Flow pushed a commit that referenced this pull request Jul 20, 2026
…arch#65919)

* fix(desktop): preserve interim assistant text wiped at message.complete

When the agent emits interim text (commentary alongside tool calls, or the
attempted final answer before a verify-on-stop nudge), all UI surfaces
streamed it live but then wiped it at message.complete — keeping only the
final response. The user saw text appear during inference, then disappear.

This is the complete fix across all three layers: agent core, gateway
transport, and all UI surfaces (desktop + Ink TUI).

The verify-on-stop and pre_verify paths flagged the assistant's attempted
final answer as _verification_stop_synthetic, suppressing it from both
state.db and the UI. The user only saw the terse post-verification reply.

Now the assistant response is real content: it's persisted to state.db and
emitted as an interim message via _emit_interim_assistant_message(force_display=True)
before the verification loop runs. Only the synthetic nudge messages keep
the synthetic flags. The turn finalizer drops nudges from live history and
compares content (not just role) to avoid duplicating a published candidate.
Message sequence repair collapses verification candidates in the
consecutive-assistant merge.

Wire agent.interim_assistant_callback both at construction (_agent_cbs())
and per-turn (defense-in-depth), emitting a new message.interim event with
{text, already_streamed}. Gated on display.interim_assistant_messages
(default true). Cleared in the finally block so a stale closure can't
fire on a later turn.

Add message.interim to the GatewayEventName union (apps/shared) and a
typed payload to the TUI's GatewayEvent discriminated union.

The TUI already had the segment-anchoring machinery (flushStreamingSegment +
finalTail) but had no handler for message.interim. Added recordInterimMessage
+ interimBoundaryIndex to seal segments mid-turn, and updated
recordMessageComplete to only dedupe segments after the interim boundary.

Replaced the fragile sealed-set approach with a proper interimBoundaryPending
state flag on ClientSessionState. finalizeInterimAssistantMessage finalizes
the streaming bubble in place (or creates a standalone one), rotates the
stream ID so next deltas create a new bubble, and sets the flag. When the
final text equals an already-sealed interim, they stay as distinct messages.

Extracted mergeFinalAssistantText() as a pure function in chat-messages.ts,
used by both completeAssistantMessage and finalizeInterimAssistantMessage.
Split the bidirectional dedup predicate: reasoning is a restatement only when
the final FULLY covers it. A short final ("Done.") no longer swallows a
longer reasoning block that merely starts with it.

Honor display.interim_assistant_messages (default true) across all layers:
the tui_gateway gates the callback, the desktop wires it to a nanostores
atom via use-hermes-config. Updated hermes_cli/config.py and
cli-config.yaml.example comments to document the Desktop behavior.

_split_segment_tokens now accepts posix=False and _find_ad_hoc_match tries
both posix modes so ad-hoc verification scripts with Windows backslash
paths are matched correctly. (response_previewed forwarding from NousResearch#53553
is not included — our emit-interim + persist approach makes it unnecessary
since the attempted answer is now surfaced before the verification loop.)

- tsc: clean (desktop + TUI + shared)
- vitest desktop: 73/73 pass (7 interim-sealing + 5 mergeFinalAssistantText + 4 config atom)
- vitest TUI: 83/83 pass (4 new message.interim tests)
- python: 390 tests pass (340 tui_gateway + 33 verification/finalizer + 6 config gating + 3 evidence + 8 continuation budget)

Co-authored-by: Liam Zhang <yingliang-zhang@users.noreply.github.com>
Co-authored-by: Lucas D'Alessandro <lucasfdale@users.noreply.github.com>
Co-authored-by: Eric Manganaro <superposition@users.noreply.github.com>
Co-authored-by: sweetcornna <sweetcornna@users.noreply.github.com>
Co-authored-by: DECK6 <DECK6@users.noreply.github.com>
Co-authored-by: matantsevs <matantsevs@users.noreply.github.com>
Co-authored-by: gitcommit90 <gitcommit90@users.noreply.github.com>

* fix: prefix-match interim streamed content to avoid benign duplicate bubbles

_interim_content_was_streamed used exact equality (streamed == visible_content),
so a final response that was the streamed text plus a trailing delta — or a
partial stream before the verify nudge fired — failed the match and left
_response_was_previewed false. The turn then showed two bubbles (interim +
identical final) instead of settling the interim in place.

Relax to a prefix check (visible_content.startswith(streamed)) in both the
core match and the desktop's settle-in-place gate. The TUI already used
prefix matching via finalTail. The reverse direction (streamed longer than
final) is intentionally not matched — that could suppress a needed resend
in the gateway path where already_streamed=True calls on_segment_break().

* test(desktop): add partial-stream-then-nudge dedup edge case

Third edge case for the interim-sealing dedup: model streams part of its
answer via message.delta, verify nudge fires, interim seals the streamed
prefix, then the final response is the same text plus a trailing delta.
Asserts one bubble (not two) containing the full final text.

Acceptance protocol #2 — covers all three dedup edges:
  1. interim == final (existing)
  2. interim = strict prefix of final (existing)
  3. partial-stream-then-nudge (this commit)

---------

Co-authored-by: Liam Zhang <yingliang-zhang@users.noreply.github.com>
Co-authored-by: Lucas D'Alessandro <lucasfdale@users.noreply.github.com>
Co-authored-by: Eric Manganaro <superposition@users.noreply.github.com>
Co-authored-by: sweetcornna <sweetcornna@users.noreply.github.com>
Co-authored-by: DECK6 <DECK6@users.noreply.github.com>
Co-authored-by: matantsevs <matantsevs@users.noreply.github.com>
Co-authored-by: gitcommit90 <gitcommit90@users.noreply.github.com>
Tranquil-Flow pushed a commit that referenced this pull request Jul 20, 2026
Blocking #1 — gateway-connecting-overlay.tsx reduced-motion regression:
the top `if (reduce) setPhase('gone')` fired unconditionally on mount
whenever reduce-motion was on, so every OS reduced-motion user lost the
CONNECTING overlay during cold boot entirely (jumped to 'gone' before the
gateway was even open). The intent was to skip the exit *choreography*,
not to skip showing the overlay. Removed the unconditional top block and
the redundant nested preview block; kept only the third branch
(`gatewayState === 'open' && shownRef.current` → `reduce ? 'gone' :
'text-out'`) which correctly gates the short-circuit on connect. Also
fixed `if(reduce)` missing-space, 6-space misindent, and the same 3-line
comment pasted three times.

Nit #1 — tsconfig excludes e2e, so specs were never typechecked in CI.
Added tsconfig.e2e.json (extends base, includes e2e/ + playwright.config.ts,
adds @playwright/test types) and wired it into the typecheck script. This
surfaced three latent type errors that are fixed in the same commit:
  - fix-electron-tracing.ts: `app._context` and `electron._playwright` are
    private APIs — added `as any` on the access before the existing cast.
  - playwright.config.ts: `reducedMotion: 'reduce'` directly under `use:`
    is not a valid UseOptions property in playwright 1.58; it's a
    BrowserContextOption accessed via `contextOptions: { reducedMotion:
    'reduce' }`. The old form was silently ignored at runtime, so
    reduced-motion emulation wasn't actually active — screenshots could
    catch overlays mid-fade (exactly what the comment warned about).

Nit #2 — fix-electron-tracing.ts reaches into Playwright internals
(_playwright, _allContexts, _context) with no public contract. Added a
header comment calling out the `@playwright/test` exact pin (=1.58.2) so a
future bump knows to re-verify the private symbols still exist.

Nit NousResearch#3 — main.ts TEST_WORKER_INDEX block had stray 6-space indentation.

Verified: tsc -p . && tsconfig.electron && tsconfig.e2e → 0 errors;
vitest boot-failure-overlay (3/3) + boot-failure-reauth (21/21) pass;
npm run build clean; playwright e2e/boot-failure.spec.ts 2/2 pass.
@GottZ

GottZ commented Jul 20, 2026

Copy link
Copy Markdown
Author

Closing this follow-up because the upstream salvage, NousResearch#67938, has been merged.

@GottZ GottZ closed this Jul 20, 2026
Tranquil-Flow pushed a commit that referenced this pull request Aug 11, 2026
…on delegation callbacks (NousResearch#82592)

* fix(gateway): stop frozen-preview finals and dropped idle-session delegation callbacks

Two relay-plane delivery losses from the 2026-08-09 staging incident:

1. stream_consumer: the skip-redundant-finalize branch recorded _accumulated
   as the delivered turn-final payload even when the last ACKED edit was an
   earlier throttled preview snapshot, so delivered_final_matches reconciled
   True and the gateway suppressed the corrective final send — the user was
   left with a cut-off message ending in the streaming cursor. Extracted
   _mark_skip_redundant_finalize(): records the last acked wire payload
   (cursor-stripped), so a preview/final mismatch now returns False and the
   normal final send fires.

2. run.py: _classify_completion_target classified every ended parent session
   terminal unless it ended by compression. Idle/timeout session ends are the
   norm on scale-to-zero relay deployments and the chat route remains valid;
   completed async delegation results were terminally dropped. Ended parents
   now classify deliver unless the end was an explicit user boundary
   (session_reset / user_exit / session_switch).

* fix(relay): drain in-flight outbound frames before transport teardown

disconnect() failed every pending outbound future immediately with
'relay transport closed', so a trailing finalize edit racing turn
teardown was lost even though the connector socket could still serve
it. Bounded drain grace (5s) lets in-flight requests resolve; silent
connectors still tear down promptly. asyncio.wait (not gather+wait_for)
so a timeout doesn't cancel futures owned by the fail-remaining loop.

* fix(gateway): route completion injection through the alias-aware transport resolver

Third relay-plane delivery loss from the 2026-08-09 staging incidents: a
delegation batch completed while the gateway was up, the watcher drained
the event, and delivery vanished with no log line. _inject_watch_notification
resolved its adapter with a literal p.value == platform_name scan of
self.adapters — a relay-fronted gateway registers ONE adapter under
Platform.RELAY fronting N logical platforms, so 'slack' never matched and
the injection returned None ('no gateway route'), silently dropping the
completion. The handoff path already documents this exact trap and uses
resolve_delivery_transport; the injection path now does the same (native
wins; relay eligible only when it fronts the logical platform), with the
literal scan kept as fallback for stub runners and exotic platforms.

* fix(relay): clamp disconnect drain grace to the runner's adapter-disconnect budget

Review finding (JoaoMarcos44, NousResearch#82592): a fixed 5.0s drain in front of the
three 1.0s sequential teardown awaits gives an 8.0s worst case inside the
runner's 5.0s asyncio.wait_for(adapter.disconnect()) — tripping it cancels
teardown mid-drain, skips the fail-pending loop, and leaves outbound
callers blocked until _OUTBOUND_TIMEOUT_S (30s). The effective grace is
now budget - 3*TEARDOWN - margin (env-aware via the same
HERMES_GATEWAY_ADAPTER_DISCONNECT_TIMEOUT the runner reads), so the drain
can never push teardown past its caller's budget; a budget too small for
any drain disables it cleanly.

* test(gateway): pin the final-send suppression contract across a behaviour matrix

The gateway skips its own final send when the stream consumer claims the turn
final already reached the user. Every incident in that family — NousResearch#71643 (stale
finalize snapshot), NousResearch#78541 (payload-less multi-message split), NousResearch#82656 (frozen
preview left with a visible cursor) — is the same failure: the consumer claimed
delivery for text the platform never rendered, so the corrective send was
suppressed and the answer was lost with no retry.

Each was fixed with a scenario test pinned to one branch of
GatewayStreamConsumer.run(). The got_done handler now has five sibling branches
that each set the suppression flags and record a turn-final payload, and nothing
checks them as a group: a new branch, or a new early `return True` in
_send_or_edit, can reintroduce the class without failing a test.

Pin the invariant instead of the branch — if the consumer offers the gateway any
signal it would trust, the complete final text must have reached the wire — and
assert it across {edit always / dies / never / lies} x {send always / never} x
{fresh-final on / off} x {clean / interrupted stream}.

The adapter records only frames that actually rendered, so an ACK the platform
drops does not count as delivery. 24 honest-transport scenarios hold the
invariant as a hard assertion. The 16 lying-transport scenarios are checked too;
the single combination that still violates it is reported as an expected
failure documenting the open exposure rather than asserting it away.

Refs NousResearch#82656

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* fix(gateway,relay): prime relay egress routing for synthetic injections + cap stale completion replay

Defect NousResearch#4 from the 2026-08-09 staging incidents (upgrade-robustness):
after every gateway restart the durable async-delegation replay injected
completions correctly (post-741663cf1) but their replies bounced at the
connector — 'slack egress declined: target not routed to an onboarded
tenant'. The relay adapter re-attaches tenant discriminators
(metadata.scope_id / metadata.user_id) from per-chat caches warmed ONLY by
inbound traffic; synthetic turns race those cold caches on every deploy,
scale-to-zero wake, and crash recovery.

- relay adapter: prime_routing_cache() — feeds a synthetic event's
  session-store origin through the same _capture_scope used for real
  inbound (never raises).
- run.py injection path: prime the resolved adapter before handle_message
  (duck-typed; native adapters unaffected).
- async_delegation: 48h staleness cap in restore_undelivered_completions —
  a pending completion older than the cap is terminally dropped (payload
  stays queryable) instead of re-run as a fresh full-context turn; the
  post-restart replay of a July session burned a 102K-token context.

Also carried: JoaoMarcos44's suppression behaviour-matrix harness
(cherry-picked from NousResearch#82676, authorship preserved) — 39 passed + 1 xfail
(the documented ACK-then-drop transport-honesty residue).

* test: use recent timestamps in restored-ownership fixtures

test_restore_stamps_restored_flag persisted its completion with epoch-era
toy timestamps (dispatched_at=1.0), which the new 48h replay staleness cap
correctly classifies as stale — the fixture then exercised the cap instead
of the restored-flag contract (CI slice 4 failure). Timestamps are now
now-relative; the staleness behavior itself is pinned separately in
test_relay_injection_egress_priming.py.

* fix(gateway,relay): close four review findings on the relay delivery fixes

Review follow-ups on this branch (NousResearch#82592):

1. HIGH — classifier/resolver mismatch (falsely-acknowledged loss).
   _classify_completion_target now returns "deliver" for idle-ended
   parents, but _resolve_async_delegation_session still dropped every
   non-compression-ended pin: the durable row was acked at adapter
   acceptance, then the injection died inside the pipeline with no
   retry — strictly worse than the honest terminal drop on main, and
   the delivery leg defect #2's fix depends on did not exist. The
   resolver now retargets non-user-boundary ends (idle/timeout/
   lifecycle) to the chat's current session — session_entry already IS
   the routing key's current session for the same chat — while user
   boundaries (session_reset / new_session / user_exit /
   session_switch) stay fail-closed. Both sides share one module-level
   _USER_BOUNDARY_END_REASONS so the verdict and the routing decision
   cannot drift again; a coherence test asserts deliver-verdicts
   resolve non-None across representative end reasons.

2. HIGH — drain clamp missed adapter-level spend. The effective drain
   grace budgeted drain + 3x teardown, but RelayAdapter.disconnect
   spends revocation-monitor teardown + go_idle time BEFORE the
   transport drain inside the same runner wait_for; worst case still
   blew the budget and cancelled teardown mid-drain (skipping the
   fail-pending loop). The adapter now measures its own elapsed time
   and threads the REMAINING budget into
   transport.disconnect(budget_s=...); legacy/stub transports without
   the keyword fall back to the no-arg signature.

3. P1 — _request_response racing disconnect() could register a future
   after the fail-pending loop already ran, stranding the caller for
   the full _OUTBOUND_TIMEOUT_S (30s). Fail fast with the same
   "relay transport closed" error once _closing is set.

4. P1 — _build_process_event_source's last-resort reconstruction
   dropped scope_id, so a scoped relay completion whose session-store
   origin was unavailable primed no tenant discriminator and could
   still bounce off the connector's fail-closed egress guard.
   scope_id now threads through the reconstructed SessionSource, with
   a warning when a scoped chat reconstructs without one.

All four: RED reproduced with the fix reverted, GREEN after; relay/
delegation delivery families pass (43 + 71 + 179 across the touched
suites); full tests/gateway run shows only failures already failing
identically on merge base 2446c8b (env/dep issues).

* fix(gateway,relay): make pending-frame failure cancellation-safe; persist completion routing origin

Two remaining review findings on this branch (NousResearch#82592):

1. Cancellation could strand outbound waiters past the fail-pending
   loop. transport.disconnect() failed pending futures only at the END
   of the drain + three teardown awaits; a cancellation landing
   mid-drain (the runner's wait_for budget, an outer cleanup deadline)
   skipped the loop entirely and left registered futures unresolved —
   their callers blocked until _OUTBOUND_TIMEOUT_S (30s). The budget
   threading added earlier shrinks the window but is not a hard
   guarantee. The fail-pending loop (and the going_idle ack failure)
   now run in a `finally`, so no exit path — normal, error, or
   cancelled — can leave a registered future unresolved. Idempotent:
   done futures are skipped, a second disconnect() pass is a no-op.

2. Durable completions did not persist their routing origin, so the
   scope_id threading in the fallback SessionSource reconstruction had
   nothing to carry on the exact path it exists for (restart replay
   with session store + source cache gone): the async-delegation event
   producers never populated scope_id and the durable rows never
   stored it. Dispatch now snapshots the originating turn's
   scope_id/user_id/user_name from the session context
   (_capture_routing_origin — a new HERMES_SESSION_SCOPE_ID contextvar
   bound by the gateway at session-bind time alongside the existing
   vars), stores them in the existing task_json payload (no schema
   migration), and re-attaches them to all three completion-event
   shapes (live single, live batch, crash-recovery rebuild). The
   gateway's fallback reconstruction then primes both discriminators
   after a restart.

Tests: cancellation mid-drain -> every pending future resolves with
"relay transport closed" (mutation: moving the loop out of the finally
goes RED); second-pass disconnect idempotence; end-to-end
dispatch -> owner-death recovery -> event carries scope_id -> fallback
SessionSource primes it (mutations: dropping the dispatch capture or
the task_json persistence both go RED); live completion event carries
the origin. 94 passed + 1 xfailed across the delivery/delegation
suites; tests/tools delegation family 73 passed (2 collection errors
pre-existing on merge base 2446c8b).

---------

Co-authored-by: joaomarcos <joaomarcosdias444@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Ben Barclay <ben@nousresearch.com>
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