bug: background nodes are force-cancelled 30 seconds after foreground completion regardless of progress

[RohanAwhad]

Current Behavior

At workflow completion, WorkflowExecutor.execute() drains background tasks with a hard 30-second budget, then force-cancels whatever is still running:

if self.background_tasks:
    done, pending = await asyncio.wait(
        self.background_tasks,
        timeout=30.0,
    )
    for task in pending:
        task.cancel()

The cancelled tasks are never awaited or reaped (no asyncio.gather after the cancel loop), so a cancellation kills the task silently. _run_node_background only catches Exception, not CancelledError, so a cancelled background node emits neither node.completed nor node.failed — the work simply disappears.

So blocking=False does not mean "run asynchronously to completion". It means "you get at most 30 seconds after the foreground control flow finishes, then forced cancellation". LLM-backed background nodes routinely exceed this:

• Archivist nodes are AgentNode(role=ARCHIVIST, blocking=False) and have a configured agent timeout of 300 seconds.
• Spec generation (spec_generate) is a non-blocking FnNode that shells out to a gated spec-generate workflow.
• Since background failures don't halt the workflow (_run_node_background only logs a warning), work is silently lost.

Desired Behavior

• blocking=False nodes run to completion; the executor should await background tasks (or drain for a duration >= the node's own timeout) before returning.
• If a background node is cancelled or fails, that fact should be visible in the result (events, logs, or halt_reason), not silently swallowed.

Evidence

The 30s drain + cancel — factory/workflow/executor.py:125-131:

125:        if self.background_tasks:
126:            done, pending = await asyncio.wait(
127:                self.background_tasks,
128:                timeout=30.0,
129:            )
130:            for task in pending:
131:                task.cancel()

Note: no await asyncio.gather(...) follows — cancelled tasks are left un-reaped.

blocking=False → fire-and-forget task — factory/workflow/executor.py:227-233:

227:        if not node.blocking:
228:            task = asyncio.create_task(self._run_node_background(node))
229:            self.background_tasks.append(task)
230:            next_id = self._next_unconditional(node_id)
231:            if next_id:
232:                await self._execute_from(next_id)
233:            return

Background failure is swallowed; cancellation is unhandled — factory/workflow/executor.py:316-327:

316:        except Exception as exc:
317:            self._emit(
318:                "node.failed",
319:                NodeFailed(
...
327:            log.warning("background_node_failed", node=node_id, error=str(exc))

CancelledError is not an Exception in Python 3.11+, so it bypasses this handler entirely — cancelled nodes emit nothing and halt nothing.

Archivist is non-blocking with a 300s agent timeout — factory/workflow/definitions.py:330-338 (archivist_plan, blocking=False), factory/workflow/definitions.py:410-424 (archivist_build and spec_generate, both blocking=False), and factory/workflow/primitives.py:51 ("archivist": AgentConfig(role=AgentRole.ARCHIVIST, model="haiku", timeout=300)).

Background nodes invoke LLM subprocesses with long timeouts — factory/workflow/executor.py:781-846: _run_node → _run_agent → invoke_agent(..., timeout=float(timeout) if timeout is not None else 600.0). A legitimate archivist run may take up to 300s (or 600s default), while the drain gives it 30s.

Call Trace

1. Entry point: factory ceo headless run with engine="deterministic" → factory/cli/_ceo_helpers.py:841-843 constructs WorkflowExecutor and calls asyncio.run(executor.execute()). (Also reachable via factory workflow run → factory/workflow/cli.py:83-94.)
2. execute() at factory/workflow/executor.py:103 calls await self._execute_from(self.workflow.start_node) (line 117).
3. _execute_from (line 181) walks edges to _execute_action_node (line 220).
4. For blocking=False nodes, _execute_action_node creates a background task at line 228 (asyncio.create_task(self._run_node_background(node))) and immediately continues the foreground graph (lines 230-232) without awaiting it.
5. The background task runs _run_node_background (line 285) → _run_node (line 781) → _run_agent (line 814) → invoke_agent (line 835), a Claude Code subprocess with a 300-600s budget.
6. When the foreground chain completes (or halts), control returns to execute() and hits the drain block at lines 125-131: wait up to 30.0 seconds, then task.cancel() every still-pending task.
7. The cancelled task gets CancelledError at its next await point; _run_node_background's except Exception (line 316) does not catch it, execute() never awaits it, and the executor returns ExecutionResult with no trace — the archive/spec output is lost.

[RohanAwhad]

Adversarial review

Verdict: DEFENSIBLE — core mechanics verified byte-for-byte; severity claims partially overstated; overlaps #1247.

Every code claim re-verified against HEAD 4998c63

• executor.py:125-131 drain + cancel, no gather/reaping — EXACT; no comment explains the 30s
• executor.py:227-233 fire-and-forget task — EXACT
• executor.py:316-327 except Exception, no halted=True in bg path — EXACT (line 118: success = not halted set before drain → workflow.completed emitted with success=True regardless)
• definitions.py:330-338 archivist_plan blocking=False — correct (actual span 331-338, trivial off-by-one)
• definitions.py:410-424 archivist_build + spec_generate blocking=False — EXACT
• primitives.py:51 archivist 300s timeout — EXACT
• CancelledError bypasses except Exception — correct (real boundary is Python 3.8, not 3.11+)
• Call trace real and reachable. Graph ends at spec_generate (definitions.py:459) — it's the terminal node, so the drain fires ~immediately after archivist_build/spec_generate spawn.

Timing reality check (attacks the "routinely exceed" claim)

• archivist_plan: fired before builder→gate_build→deep-QA→gate_qa→gate_doc_freshness→gate_precheck — minutes of foreground. Virtually never bitten. Listed as if equally exposed.
• archivist_build + spec_generate: get only ~30s total. spec_generate shells to a nested workflow with 3 CEO LLM gates + researcher annotate (definitions.py:2270-2383) — essentially always >30s. The bite here is common, not rare. Core claim survives.

Hardest counterarguments and survival

1. "Work doesn't disappear — cancellation orphans the subprocess." Tested: cancelling a task awaiting proc.communicate() leaves the child running; a /tmp experiment's child completed and wrote its marker after the parent exited. For plain factory workflow run build, SPEC.md likely still appears (late, unobserved). Issue survives — in the exact deterministic CEO path it cites, _ceo_helpers.py:875-876 calls remove_worktree(wt_path) immediately after execute() returns, so orphaned spec-generate/archivist processes write into a deleted worktree → work genuinely lost. And what deterministically vanishes: events, node_outputs, node counts, halt semantics, completion-before-return.
2. "30s is a deliberate bounded-shutdown policy." No evidence: introduced in Workflow graph engine: composable primitives for factory orchestration #631 with zero rationale, no comment, no doc, no test pins it. Collapses.
3. "Lost work is recoverable / low-stakes." Partially true: archive/plan.md is write-only (corroborated by OPEN Study is blind to factory artifacts; archive is write-only; spec drifts silently #1146); build.md only read by mempalace (writer.py:80-91, tolerates absence). But spec_generate's SPEC.md feeds study/design/spec-update, and a skill-engine re-run hits the same 30s wall every cycle — not recoverable by re-running. Severity for archivist dented; for spec_generate it holds.

Factual errors / overstatements

• "the work simply disappears" — overstated as universal; true under the deterministic CEO path (worktree teardown race) and for all observability, false for the raw CLI path where the orphan completes.
• "LLM-backed background nodes routinely exceed this" — true for archivist_build/spec_generate, false for archivist_plan.
• "Python 3.11+" — real boundary is 3.8.
• Desired Behavior "drain ≥ node's own timeout" — underspecified: spec_generate is an FnNode with no timeout; the nested workflow is unbounded. Need a concrete contract (e.g., await all background tasks to completion, bounded only by each node's own budget, surface cancelled state in result).

#1247 overlap

Same handler, same root contract, different mechanism (exception vs force-cancel). A maintainer merging them into one "background lifecycle contract" issue is defensible but not forced.

Recommendation

Keep, with amendments. (1) Qualify "work disappears": add the orphan-subprocess nuance + _ceo_helpers.py:875-876 worktree-removal race (strongest proof of actual loss). (2) Re-weight severity to terminal nodes (archivist_build, spec_generate, and improve-mode's spec_update at definitions.py:898-914); demote archivist_plan. (3) Fix the Python version claim (3.8+) and the 330-338 off-by-one. (4) Cross-link #1247 and #1146; let maintainers decide merge vs separate.