Skip to content

fix(engine,web): resolve max-iterations gate from dashboard in --web-bg - #202

Merged
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/198-resume-web-bg-max-iterations
May 18, 2026
Merged

fix(engine,web): resolve max-iterations gate from dashboard in --web-bg#202
Jason Robert (jrob5756) merged 2 commits into
mainfrom
fix/198-resume-web-bg-max-iterations

Conversation

@jrob5756

Copy link
Copy Markdown
Collaborator

Fixes#198.

Problem

conductor resume --web-bg (and --web) exited silently when a workflow exceeded max_iterations. The bg child process was forked with --no-interactive and stdin=subprocess.DEVNULL, so when the engine hit the limit, IntPrompt.ask raised EOFError, got coerced to 0 (stop), and the workflow ended with no way to recover.

Issue #134 made the gate visible to the dashboard but explicitly left resolution from the dashboard out of scope. This PR closes that gap.

Solution

Resolution policy in the engine

The new _resolve_max_iterations_gate(gate_id) helper picks the path based on environment:

ConditionPath
skip_gatesAuto-stop, no UI (unchanged)
No web dashboardLegacy CLI prompt (unchanged)
Web dashboard + bg_mode or non-TTY stdinWeb-only wait (no CLI prompt)
Web dashboard + TTY foregroundRace CLI vs web (same pattern as _handle_human_gate_with_web_race)

The web-only branch deliberately skips the CLI prompt because IntPrompt.ask on stdin=DEVNULL would synchronously raise EOFError, get coerced to stop, and race-win every dashboard click. It also races against dashboard.wait_for_stop() so POST /api/stop (or /api/kill) can terminate the wait if no dashboard tab is open.

Gate correlation

Each iteration_limit_reached payload now carries a uuid4 gate_id. The dashboard must echo it back in iteration_limit_response. The server matches on gate_id and discards stale responses, so a delayed double-click from a previously-resolved gate cannot be misapplied to a later gate for the same agent or parallel group. iteration_limit_resolved also includes the gate_id so subscribers can correlate the pair.

Changes

Backend

  • web/server.py: add _iteration_limit_response_queue, WS routing for iteration_limit_response messages, and wait_for_iteration_limit_response(gate_id) with stale-discard.
  • engine/workflow.py: new _resolve_max_iterations_gate / _wait_for_web_iteration_limit helpers; both _check_iteration_with_prompt and _check_parallel_group_iteration_with_prompt now generate and emit a gate_id.

Frontend

  • types/events.ts: gate_id: string on IterationLimitReachedData; optional echo on IterationLimitResolvedData.
  • stores/workflow-store.ts: new sendIterationLimitResponse action; activity-log copy updated from "awaiting console input" → "awaiting decision".
  • components/dialogs/IterationLimitModal.tsx (new): top-level modal (gate can target a parallel group, so attaching to the per-agent detail panel didn't fit) with iteration count, recent agent history, number input, Continue/Stop. Hidden when skip_gates is true. Does not close on Escape — accidental dismissal would orphan the workflow.
  • App.tsx: mounts the modal (not in replay mode).
  • StatusBar.tsx: matching "awaiting decision" copy update.

Tests

  • tests/test_web/test_server.py (+3): gate_id-matched response routing; stale discard; end-to-end WS message routing into the dedicated queue.
  • tests/test_engine/test_iteration_limit_events.py (+9): bg-mode web-only path continues / stops / terminates on stop-event; skip-gates preserved with web dashboard attached; gate_id correlates reached/resolved for both single-agent and parallel group; no-dashboard fallback intact; TTY+web race with web winner; malformed (negative) additional_iterations clamps to stop.
  • One existing exact-dict assertion updated for the new gate_id field.

The EventCollector test helper now properly initializes its WorkflowEventEmitter base so tests can subscribe to events and react (used by the new web-only tests to enqueue dashboard responses on iteration_limit_reached).

Acceptance criteria (from #198)

  • conductor resume --web-bg (and --web) shows an interactive prompt in the dashboard when iteration_limit_reached fires.
  • User can continue the workflow with N additional iterations or stop, mirroring the terminal prompt.
  • Choice is delivered to the running workflow via a new web API; the existing iteration_limit_resolved event still fires (now with a matching gate_id).
  • If no client is connected, behavior degrades gracefully: POST /api/stop terminates the wait; without that, the legacy CLI-EOFError→stop path still triggers when stdin is somehow consumed. (A configurable timeout was explicitly "ideally" in the issue and can be a follow-up.)
  • Regression tests cover both the API endpoint and the engine path that waits on the dashboard reply.

Verification

  • make lint
  • make typecheck ✅ (the single pre-existing warning in dialog_evaluator.py is unrelated)
  • uv run pytest -m "not performance"2629 passed, 15 skipped
  • npm run build ✅ frontend builds cleanly

(No frontend test framework exists in the repo, so the originally-planned Vitest tests were skipped per AGENTS.md guidance not to add new tooling. TypeScript compilation verifies the new component.)

`conductor resume --web-bg` (and `--web`) exited silently when a workflow
exceeded `max_iterations`. The bg child process was forked with
`--no-interactive` and `stdin=subprocess.DEVNULL`, so when the engine
hit the limit, `IntPrompt.ask` raised `EOFError`, got coerced to `0`
(stop), and the workflow ended with no way to recover. Issue #134 had
made the gate visible to the dashboard but explicitly left resolution
from the dashboard out of scope. This PR closes that gap.
Resolution policy in the engine (issue #198):
- `skip_gates`: handler auto-stops, no UI (unchanged).
- No web dashboard: legacy CLI prompt path (unchanged).
- Web dashboard + bg mode or non-TTY stdin: web-only wait. The CLI
prompt is deliberately NOT invoked because it would synchronously
raise `EOFError`, get coerced to "stop", and race-win every dashboard
click. Also races against `dashboard.wait_for_stop()` so
`POST /api/stop` (or `/api/kill`) can terminate the wait if the user
has no dashboard tab open.
- Web dashboard + TTY foreground: race CLI vs web (same pattern as
`_handle_human_gate_with_web_race`).
Each `iteration_limit_reached` payload now carries a uuid4 `gate_id`,
and the dashboard must echo it back in `iteration_limit_response`. The
server matches on `gate_id` and discards stale responses, so a delayed
double-click from a previously-resolved gate cannot be misapplied to a
later gate for the same agent or parallel group. `iteration_limit_resolved`
also includes the `gate_id` so subscribers can correlate the pair.
Backend:
- `web/server.py`: add `_iteration_limit_response_queue`, WS routing
for `iteration_limit_response` messages, and
`wait_for_iteration_limit_response(gate_id)` with stale-discard.
- `engine/workflow.py`: new `_resolve_max_iterations_gate(gate_id)` and
`_wait_for_web_iteration_limit(gate_id)` helpers; both
`_check_iteration_with_prompt` (single agent) and
`_check_parallel_group_iteration_with_prompt` (parallel group) now
generate a `gate_id`, emit it on `iteration_limit_reached` /
`iteration_limit_resolved`, and route through the new helper.
Frontend:
- `types/events.ts`: add `gate_id: string` to `IterationLimitReachedData`
and optional `gate_id?: string` to `IterationLimitResolvedData`.
- `stores/workflow-store.ts`: add `sendIterationLimitResponse` action
that emits the WS message with `gate_id`; activity-log text updated
from "awaiting console input" to "awaiting decision" now that the
dashboard can resolve the gate itself.
- `components/dialogs/IterationLimitModal.tsx` (new): top-level modal
(gate can target a parallel group, so attaching to the per-agent
detail panel didn't fit) with iteration count, recent agent history,
number input, and Continue/Stop buttons. Hidden when `skip_gates` is
true. Does not close on Escape — accidental dismissal would orphan
the workflow.
- `App.tsx`: mount the modal at the app root (not in replay mode).
- `StatusBar.tsx`: matching "awaiting decision" copy update.
Tests:
- `tests/test_web/test_server.py` (+3): `wait_for_iteration_limit_response`
matches by `gate_id` / discards stale; end-to-end WS routing for
`iteration_limit_response` lands in the dedicated queue.
- `tests/test_engine/test_iteration_limit_events.py` (+9): bg-mode
web-only path continues / stops / terminates on stop-event;
skip_gates preserved with web dashboard attached; `gate_id`
correlates `reached`/`resolved` for both single-agent and parallel
group; no-dashboard fallback intact; TTY+web race with web winner;
malformed (negative) `additional_iterations` clamps to stop.
- One existing exact-dict assertion updated for the new `gate_id` field.
The `EventCollector` test helper now properly initializes its
`WorkflowEventEmitter` base so tests can subscribe to events and react
to them (used by the new web-only tests to enqueue dashboard responses
on `iteration_limit_reached`).
Fixes#198.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Polish and harden the dashboard-resolution path from the previous commit
based on multi-agent PR review findings.
Critical:
- Reword IterationLimitModal docstring: it claimed the user could
"dismiss the modal without a choice", but the modal is intentionally
non-dismissable (no Escape, no close button, no click-outside).
Accidental dismissal would orphan the workflow.
- Tighten sendIterationLimitResponse parameter type from a loose
{ agent_name?: string; group_name?: string } to a new
IterationLimitResponseTarget discriminated union, so a future bug
cannot send both fields (or neither). The modal's buildTarget
now uses an explicit IterationLimitResponseTarget return type.
- Replace contextlib.suppress(asyncio.CancelledError, Exception) in
two iteration-limit race-cleanup sites with a narrowed pattern that
only swallows CancelledError and logs any other exception with
traceback. The previous broad suppression would have hidden genuine
bugs in the loser task -- making race-related regressions nearly
impossible to debug. Extracted the now-shared 14-line pattern into
a _drain_iteration_limit_losers static helper.
Observability:
- Log when _wait_for_web_iteration_limit stops because the dashboard
stop signal fired (POST /api/stop / /api/kill). Previously
indistinguishable from a user explicitly clicking "Stop" in the modal.
- Log when a dashboard response carries a malformed
additional_iterations value (non-numeric, wrong type). The fallback
to "stop" is preserved, but the malformed value is now visible in logs
so the underlying frontend or transport bug isn't silent.
Docs:
- Clarify _resolve_max_iterations_gate docstring with an explicit
Raises: asyncio.CancelledError section explaining how the caller's
outer finally converts that into aborted=True.
Tests:
- New test_web_only_path_used_when_stdin_is_not_a_tty_without_bg_mode
exercises the third "web-only" branch: --web foreground but with
redirected stdin (< /dev/null, CI, container). The fix's
cli_usable = not self._bg_mode and sys.stdin.isatty() guard now has
direct coverage; a refactor that drops the isatty() check would
fail this test rather than silently regress to #198.
Verification:
- make lint passes
- npm run build passes
- All iteration-limit and server tests pass (84 focused; 1069 across
engine/web/gates/cli).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jrob5756
Jason Robert (jrob5756)force-pushed the fix/198-resume-web-bg-max-iterations branch from a43cdf8 to 7d4f829CompareMay 18, 2026 21:02
@jrob5756
Jason Robert (jrob5756) merged commit dc29c2c into mainMay 18, 2026
9 checks passed
@jrob5756
Jason Robert (jrob5756) deleted the fix/198-resume-web-bg-max-iterations branch May 18, 2026 21:18
@jrob5756Jason Robert (jrob5756) mentioned this pull request May 21, 2026
4 tasks
Jason Robert (jrob5756) added a commit that referenced this pull request May 21, 2026
- feat(script): script agents output schemas (#206, #118)
- feat(validate): warn on undeclared agent.output refs and field-level mismatches in explicit mode (#208)
- feat(copilot): attribute verbose logs to agents in parallel/for-each runs (#207)
- fix(resume): replay original event log into dashboard on --web (#167, #205)
- fix(windows): make --web-bg startup crashes diagnosable (#116, #204)
- fix(engine,web): resolve max-iterations gate from dashboard in --web-bg (#202)
- fix(bg): detach --web-bg child from Windows job to prevent kill-on-close (#200)
- fix(bg): stop passing redundant --silent to bg child (#199, #196)
- fix(cli): suppress web-bg dashboard output in silent mode (#203, #211)
- fix(config): auto-fetch sibling sub-workflow from registry cache during validation (#197)
- fix(registry): mirror repo layout in cache so cross-workflow refs resolve (#194)
- fix(copilot): tolerate SDK metadata parsing errors when listing models (#193)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Jason Robert (jrob5756) added a commit that referenced this pull request Jul 21, 2026
* fix(engine,cli): resolve human gates from dashboard in --web-bg (#286)
Extend the #198/#202 max-iterations gate policy to human gates.
_handle_gate_with_web previously raced the CLI prompt against the web
dashboard unconditionally; in a --web-bg child (stdin=DEVNULL),
Prompt.ask raised EOFError instantly, race-won, cancelled the web arm,
and crashed the workflow before a dashboard user could respond.
- engine/workflow.py: add a cli_usable = not self._bg_mode and
sys.stdin.isatty() tier to _handle_gate_with_web. When false, wait
web-only via _wait_for_web_gate (no CLI arm). Foreground TTY keeps
the existing CLI/web race. Kill/stop while parked at a gate is
handled by the existing outer _execute_with_stop_signal path
(checkpoints via handle_dashboard_stop, issue #245) -- no new inner
stop race needed.
- cli/app.py: replace the pre-fork _abort_web_bg_if_human_gate abort
with _workflow_has_human_gate detection + a post-launch
_print_web_bg_human_gate_notice pointing at the dashboard URL and
`conductor gate-respond`. Applies to both run --web-bg and resume
--web-bg (including resume --from <checkpoint> without a workflow
arg).
- docs/cli-reference.md: update the --web-bg + human_gate section to
describe dashboard/CLI gate resolution instead of the incompatibility.
- plugins/conductor/skills/conductor/references/execution.md: document
gate resolution via the dashboard/gate-respond for --web and --web-bg.
- tests: rewrite the "aborts before fork" CLI tests to assert the fork
proceeds with a gate notice (run, resume, resume-from-checkpoint-only,
human_gate nested in for_each, --skip-gates suppresses the notice,
gate-free workflows show no notice). Add engine tests asserting the
bg-mode gate waits web-only (CLI handler never invoked) and that
foreground TTY still races CLI vs web.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix(engine): raise clear error when bg-mode gate has no dashboard
Address code-review findings on #324:
- engine/workflow.py: _handle_gate_with_web now raises HumanGateError
when bg_mode is active and no web dashboard is attached (e.g. the
dashboard failed to start in a --web-bg child), instead of silently
falling through to the CLI prompt handler and crashing with an
uncaught, contextless EOFError -- the same failure class issue #286
was filed to fix, reachable via a narrower trigger. The no-dashboard
CLI-only path is preserved for non-bg-mode runs (foreground, piped
stdin, tests), which were already safe and must not be newly
restricted.
- tests/test_engine/test_workflow.py: add a regression test asserting
bg_mode + no dashboard raises HumanGateError without ever invoking
the CLI handler; strengthen the foreground-TTY race test to assert
the CLI handler was actually invoked (previously it only asserted
the web response won, which passed even if the race never occurred).
- cli/app.py: log (debug) when the best-effort human_gate probe fails
to load a workflow, instead of swallowing silently; document the
urlparse(url).port invariant relied on by the gate notice; fix a
grammatically broken comment at the resume() call site.
- plugins/conductor/skills/conductor/references/execution.md: soften
"fully compatible" overclaim to note the dashboard-failed-to-start
case now surfaces a clear error rather than hanging or crashing.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Jason Robert <jasonrobert@microsoft.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto 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.

bug(resume): --web-bg exits silently when max_iterations is reached instead of prompting for additional iterations

1 participant

@jrob5756