Skip to content

fix(desktop): heal bound views after stale runtime - #98876

Closed
JoaoMarcos44 wants to merge 4 commits into
NousResearch:mainfrom
JoaoMarcos44:fix/98683-heal-dead-runtime-binding
Closed

JoaoMarcos44 wants to merge 4 commits into
NousResearch:mainfrom
JoaoMarcos44:fix/98683-heal-dead-runtime-binding

Conversation

@JoaoMarcos44

@JoaoMarcos44 JoaoMarcos44 commented Aug 30, 2026

Copy link
Copy Markdown
Contributor

Forensic report: #98683

Verdict

Confirmed. The desktop symptom is a real stale-runtime polling bug. A session can remain bound to a reaped runtime ID; passive/background process.list polling then receives terminal gateway error 4001 (session not found). Without runtime-keyed convergence and bounded session healing, later poll/remount paths can keep targeting the dead binding. The generic remount reset is intentionally retained for #98455 and is not part of this change.

Root cause, line by line

At the affected pre-fix path:

  1. apps/desktop/src/store/composer-status.ts polls process.list for the bound runtime/session.
  2. A reaped runtime returns terminal gateway code 4001 (session not found).
  3. The previous recovery tracked a session-level gone condition without coupling convergence to the runtime identity and without healing the bound tile/session state.
  4. The dead binding therefore remained eligible for later polling/re-render paths; approval/goal/clarify surfaces could continue targeting the reaped runtime.
  5. The fix latches 4001 per runtime, unbinds the stale tile, resumes only the primary bound session from the stored session ID, and caps repeated healing attempts.
  6. A healthy process.list clears the runtime latch and healing budget; transient failures preserve the binding.
  7. Repeated 4001 events for the same runtime become no-ops, preventing the poll/remount/focus-loss loop while preserving intentional reconnect resets.

The generic remount reset in status-stack/index.tsx is intentionally retained for #98455 and is not part of this change.

Relevant implementation nodes

  • apps/desktop/src/app/chat/composer/status-stack/index.tsx: session status-stack mount effect; generic reconnect reset remains intact and is excluded because fix(desktop): stop status-stack remounts from re-arming a dead-runtime poll storm (#98434) #98455 owns that behavior.
  • apps/desktop/src/store/composer-status.ts: background process.list poll, 4001 classification, and alive/terminal routing.
  • apps/desktop/src/store/runtime-gone.ts: runtime-keyed stale-runtime latch; tile unbinding, primary-session resume, retry cap, and healthy-runtime budget reset.
  • apps/desktop/src/store/session-states.ts: runtime/tile binding state changed by recovery.
  • apps/desktop/src/store/session.ts: stored-session resume path.

Graphify architecture map

From graphify-out/GRAPH_REPORT.md (generated report; it records its own need to be refreshed with graphify update .):

  • SessionDB community: 15, 272 nodes (report lines 5616–5618).
  • conversation_compression.py community: 158, 200 nodes (lines 6188–6190).
  • conversation_loop.py community: 183, 228 nodes (lines 6288–6290).
  • CompressionCommitFence community: 206, 66 nodes (lines 6380–6382).

The direct #98683 path is in the desktop store/UI nodes above. The compression and lease nodes are adjacent safety contracts, not the direct cause of the desktop poll loop:

  • agent/conversation_compression.py: run_compress_context_with_progress_timeout uses a cancellation boundary before commit and a CompressionCommitFence around post-summary mutation; cancellation before commit aborts, while an already-started commit completes safely.
  • gateway/turn_lease.py: SessionTurnLeaseRegistry.acquire uses bounded lock acquisition and fails closed on timeout; timeout waiting for a lock does not release a lock held by another owner.
  • agent/conversation_loop.py: conversation execution/reset path.
  • hermes_state.py: SessionDB write path uses transactional BEGIN IMMEDIATE writes with bounded retry behavior.

Deterministic reproduction

Temporary repro: C:/Users/Nitro/AppData/Local/Temp/repro_98683_stale_runtime.py

Expected output:

remount-reset requests=4
latched requests=1
lease timed_out=True lock_still_held=True
compression before_commit=aborted-before-commit after_commit_start=commit-completes-after-cancellation

Comparison fixture: clearing a latch on four remounts produces four repeated polls; retaining it produces one. This demonstrates the generic latch failure mode, while this implementation handles runtime-keyed terminal recovery and bounded healing. The lease/compression lines are adjacent contract checks, not claims that this desktop patch changes compression or lease semantics.

Duplicate / related issue audit

Conclusion: #97158/#97709 are not duplicates. The focused change here is the runtime-keyed stale-runtime healing path; it does not duplicate #98455's generic remount reset.

Luna review

  • Correctness: one-shot handling per runtime; idempotent repeated terminal events; bounded healing per stored session.
  • Concurrency: maps/sets are process-local; the runtime is latched before side effects; repeated/reentrant callbacks cannot multiply recovery.
  • Safety: recovery uses store-derived IDs only; no external input, secrets, persistence schema, or unbounded retry.
  • Regression: tile-only stale views are unbound without navigation; only the primary bound session is resumed; orphaned bindings have no side effect; transient failures preserve the binding.
  • Security assurance: no new endpoint, credential flow, code execution, or filesystem write.
  • Validation scope: focused runtime-gone test passed; broad validation intentionally skipped per request.

Recommendation

Merge the surgical runtime-keyed latch, stale-tile unbinding, primary-session resume, and bounded healing budget. Keep generic reconnect/remount reset behavior owned by #98455.

Infographic :

infographic_oriental_fix_desktop_heal_bound_views_98876

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state labels Aug 30, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

PR #98876 — fix(desktop): heal bound views after stale runtime

  • New apps/desktop/src/store/runtime-gone.ts implements runtime-keyed stale latch healing: healedRuntimes set (idempotent per runtime id), healsByStoredId capped MAX_CONSECUTIVE_HEALS=3, storedIdForRuntime from $sessionStates or $sessionTiles, unbindTileRuntime + requestSessionResume(storedId) when primary active, noteRuntimeAlive refunds budget on healthy process.list, resetRuntimeGoneHealing for tests.
  • composer-status.ts:17-30 wires refreshBackgroundProcesses: on success noteRuntimeAlive(sid), on isSessionGoneForBackgroundPolling(error) (4001) calls markRuntimeGone(sid) before returning (besides existing goneSessions latch). Bridges pull-channel 4001 to view via same levers as push session.reclaimed.
  • Tests runtime-gone.test.ts cover tile unbind preserving neighbour, primary chat resume request, tile-only no navigation, idempotent heal (markRuntimeGone second call returns false), orphan latch without resume, cap 3 heals then stall, budget refund after healthy, plus two refreshBackgroundProcesses integration paths (4001 → unbind+resume, transient timeout → no-op).
  • Adds docs/forensics/issue-98683-stale-runtime.md audit (not code) with community map and reproduction snippet — verbose but not affecting runtime.

Non-blocking:

  • healedRuntimes never cleared except test — assumes runtime ids never reused; valid because new resume binds fresh id.
  • markRuntimeGone logs no warning — silent heal; consider debug trace for observability.
  • Docs forensic markdown lists temp repro path C:/Users/Nitro/... — windows absolute, not portable but fine for forensics.

Verdict: LGTM. Bounded, idempotent recovery for stale runtime binding.

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks @JoaoMarcos44 — this is the right root-mechanism fix for #98683: routing the pull-channel 4001 verdict to the same recovery levers the session.reclaimed push path uses, with the once-per-runtime latch and the capped/refundable heal budget, is exactly what the reload/poll loop needed. Review found the code sound; verified with a before/after A/B against the real stores (main: tile stays bound to the dead runtime, no resume; your branch: binding cleared + resume requested).

Your three code commits were cherry-picked onto current main with your authorship preserved in salvage PR #99664. The only thing dropped was the docs/forensics/issue-98683-stale-runtime.md investigation notes (forensics docs with local temp paths don't belong in the repo) — the code diff is otherwise identical. Leaving this PR open for reference until the salvage lands.

@OutThisLife

Copy link
Copy Markdown
Contributor

Superseded by #99664.

That salvage keeps the 4001 latch and bounded heal, landed on current main. Credit is on #99664 via Co-authored-by. Thanks for tracing the stale-runtime poll path.

@OutThisLife

Copy link
Copy Markdown
Contributor

The salvage of this PR landed as #99664. Residual gaps from the same dead-runtime class are in #99891.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P2 Medium — degraded but workaround exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants