') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); fix(web): arm --web-bg grace timer on root workflow completion (#318) by jrob5756 · Pull Request #323 · microsoft/conductor · GitHub
Skip to content

fix(web): arm --web-bg grace timer on root workflow completion (#318) - #323

Merged
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/318-web-bg-grace-timer
Jul 21, 2026
Merged

fix(web): arm --web-bg grace timer on root workflow completion (#318)#323
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/318-web-bg-grace-timer

Conversation

@jrob5756

@jrob5756Jason Robert (jrob5756) commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes#318.

In --web-bg mode (or CONDUCTOR_WEB_BG=1), the detached process is supposed to auto-shutdown after workflow completes + all clients disconnect + 30s grace. But the grace timer was only ever armed from WebSocket-disconnect code paths:

  • the /ws endpoint's finally block on client disconnect, and
  • the broadcaster's failed-send cleanup.

_on_event only set self._workflow_completed = True on a terminal event — it never armed the timer. So if no dashboard client ever connected, no disconnect fired, the timer was never started, _bg_event was never set, and wait_for_clients_disconnect() blocked forever after the workflow had already finished. The detached child became a zombie holding its port and PID file — making --web-bg unusable for headless/programmatic supervision.

Fix

  • _on_event now arms the grace timer on the workflow's root terminal event, gated on the existing _is_root_event(event_dict) helper. _maybe_start_grace_timer() already no-ops while clients are connected, so watched runs keep their current behavior (timer only arms once the last client disconnects).
  • Gating on the root event also fixes a coupled latent bug: previously _workflow_completed was set on any terminal event, including nested sub-workflow ones (which carry data.subworkflow_path). Un-gated, arming from _on_event would let a sub-workflow finishing mid-run trigger a premature shutdown.
  • _maybe_start_grace_timer is now loop-safe: it no-ops when there's no running event loop (a synchronous emit() in a unit test with no server) instead of leaking an orphan _grace_countdown coroutine.

Tests

Added to tests/test_web/test_server.py (TestAutoShutdown):

  • test_unwatched_run_arms_grace_timer_on_completion — root completion arms the timer with zero clients ever connected, and the post-run wait resolves.
  • test_subworkflow_completion_does_not_arm_or_set_flag — a nested sub-workflow terminal event neither sets the flag nor arms the timer; the root event still does.

Both tests fail on the pre-fix code (verified by reverting), so they're non-tautological.

Verification

  • ✅ 133 passed (tests/test_web + tests/test_cli/test_web_flags.py)
  • ✅ make lint clean
  • ✅ make typecheck clean (the one remaining diagnostic is pre-existing in engine/dialog_evaluator.py, unrelated)

No CHANGELOG / docs / frontend changes needed.

@jrob5756
Jason Robert (jrob5756) marked this pull request as ready for review July 21, 2026 15:29
Jason Robertand others added 2 commits July 21, 2026 11:29
Jason Robert
fix(web): arm --web-bg grace timer on root workflow completion (#318)
7edf0f0
In --web-bg mode the auto-shutdown grace timer was only armed from the
WebSocket-disconnect code paths. If no dashboard client ever connected,
no disconnect fired, the timer was never started, and the detached
process blocked forever in wait_for_clients_disconnect() after the
workflow had already finished, leaking its port and PID file.
Arm the timer from _on_event on the *root* workflow's terminal event,
gated on the existing _is_root_event helper so a nested sub-workflow
completion (which carries subworkflow_path) can't trigger a premature
shutdown while the root run is still executing. Also make
_maybe_start_grace_timer no-op when there is no running event loop
(synchronous emit() in tests) rather than leaking an orphan coroutine.
Adds regression tests for the unwatched-run and sub-workflow-gating
cases; both fail on the pre-fix code.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert
fix(web): address code review findings on#318grace-timer fix
dc4b409
- Log at debug level when _maybe_start_grace_timer's no-running-loop
guard fires, so an unexpected occurrence outside tests (which would
silently reproduce the #318 hang with zero diagnostic trail) leaves
a trace.
- Trim the duplicated rationale comment in _on_event down to the
call-site-specific "why arm here" reasoning, pointing to
_is_root_event's and _maybe_start_grace_timer's own docstrings
instead of restating them.
- Name the terminal-event condition (is_terminal_event) to avoid an
awkward line-wrapped if-statement.
- Add regression tests: workflow_failed (not just workflow_completed)
arms the timer for an unwatched run; a connected client keeps the
timer unarmed on completion (the safety-critical case protecting
watched runs from the new _on_event call site); and the existing
flag-setting tests now also assert no exception was swallowed by
WorkflowEventEmitter.emit()'s subscriber catch-all when the
loop-safety guard fires, since a bare "_grace_task is None"
assertion alone can't distinguish a guarded no-op from an unguarded
RuntimeError getting silently logged there.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jrob5756
Jason Robert (jrob5756) merged commit bdc9648 into mainJul 21, 2026
10 checks passed
@jrob5756
Jason Robert (jrob5756) deleted the fix/318-web-bg-grace-timer branch July 21, 2026 15:36
@jrob5756Jason Robert (jrob5756) mentioned this pull request Jul 21, 2026
Merged
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[BUG] --web-bg run never exists if no dashboard client ever connects (auto-shutdown grace timer is only armed on WebSocket disconnect)

1 participant

@jrob5756