Skip to content

fix(monitor): drop stale completion wakes - #41

Merged
trvon merged 3 commits into
trvon:masterfrom
richardhristov:fix/stale-monitor-completion-wake
Aug 17, 2026
Merged

fix(monitor): drop stale completion wakes#41
trvon merged 3 commits into
trvon:masterfrom
richardhristov:fix/stale-monitor-completion-wake

Conversation

@richardhristov

Copy link
Copy Markdown
Contributor

Summary

  • guard the central loop:fire emission against stale extension contexts
  • drop an obsolete wake instead of crashing Pi when monitor completion races session replacement or reload
  • cover the direct MonitorCreate onDone completion path with a regression test

Root cause

#33 guarded events emitted by MonitorManager and awaited monitor shutdown during session teardown. However, an onDone completion callback calls onLoopFire() directly, and emitLoopFire() still used an unguarded pi.events.emit(). If that callback reaches the old extension runtime after it becomes stale, the synchronous emit throws as an uncaught exception.

Non-stale event-bus errors continue to propagate.

Verification

  • npm run typecheck
  • npm run lint (passes with 4 existing optional-chain warnings)
  • npm test (42 files, 658 tests)
  • npm run build
  • npm run test:package
  • npm audit --audit-level=moderate

@richardhristov

Copy link
Copy Markdown
Contributor Author

@trvon This is a regression in the stale-context path addressed by #33: direct onDone completion callbacks bypass MonitorManager.emit() and reach the raw loop:fire emit. CI is currently awaiting maintainer approval because this PR comes from a fork. Could you approve the run and review when convenient?

trvon

This comment was marked as outdated.

@trvon
trvon dismissed their stale review August 17, 2026 18:17

Dismissing review and continuing locally

@trvon trvon left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Resolved in two signed maintainer commits:

  • 6d859e4 adds a RED behavioral regression proving a stale runtime consumed and deleted an active loop's final fire without delivering a wake.
  • 7e83767 probes extension-context liveness before any fire accounting, retirement, auto-task creation, monitor-loop deletion, or workflow-wait mutation. The monitor-specific regression now also verifies that stale completion never reaches loop:fire and leaves its loop intact.

@trvon
trvon merged commit 84362cc into trvon:master Aug 17, 2026
2 checks passed
@trvon

trvon commented Aug 17, 2026

Copy link
Copy Markdown
Owner

@richardhristov Thanks for the PR!

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