fix(daemon): normalize bare-vs-qualified subject-id in orphan-reconciliation (REQ-052 node fan-out) - #339
Open
Shooksie wants to merge 15 commits into
Open
fix(daemon): normalize bare-vs-qualified subject-id in orphan-reconciliation (REQ-052 node fan-out)#339Shooksie wants to merge 15 commits into
Shooksie wants to merge 15 commits into
Conversation
…down Environment plugins are stateful across the three-call contract: prepare registers an in-memory run (e.g. a live WS relay to a remote node) that exec/exec_stream/teardown reuse by handle. The previous per-call resident-host lease dropped between calls, letting the shared registry LRU-evict or ping-reap the process between prepare and exec — losing that state and surfacing as "no live relay connection for handle". EnvironmentClient now holds ONE ResidentHostLease for its lifetime (acquired lazily on the first RPC via pinned_host), so every RPC lands on the same pinned process. The client is now non-Clone (callers Arc it) so the single lease is not duplicated. Death handling splits by call kind: exec/exec_stream stay at-most-once (no retry — side effects + lost state), while the idempotent control ops prepare/teardown retry once. Either way a dead lease is reaped generation-scoped (drop from the client + invalidate the exact generation in the registry) so the next run spawns a live process instead of re-leasing the corpse, and a concurrent call's fresh lease is never evicted by a late failure of an older generation. Removes the now-unused with_resident_host / with_resident_host_once / ping_is_dead helpers and their ping tests.
…eout (REQUIREMENT-049) prepare shared the 60s ENVIRONMENT_RPC_TIMEOUT, but materializing a real execution context is slow + fail-prone: the Railway environment plugin creates a service, waits for deploy, and blocks up to its 300s dial timeout for the container to dial home, plus workspace clone time. A 60s host-side timeout fired mid-create and control_rpc's retry-once spun up a SECOND node while the first was still deploying (leak + failure on slow deploys). Dedicated ENVIRONMENT_PREPARE_TIMEOUT of 360s covers dial + clone + margin. teardown keeps the 60s control bound.
Move ephemeral node lifecycle from the per-phase runner into the daemon so all phases of ONE workflow run share ONE node, torn down once at run end. - New `dispatch::environment_broker::EnvironmentBroker`: a `run_id -> lease` map with single-flight `acquire` (per-run mutex; first call resolves a daemon-resident `EnvironmentClient` + `prepare`s the node once, later calls return the same handle), `exec_stream` proxy, and idempotent `teardown`. Resolving the client daemon-side keeps ONE plugin process (one relay = one node) pinned across prepare/exec/teardown for the whole run. - Private local-socket server (interprocess::local_socket tokio, newline-JSON) implementing the Acquire + Exec wire contract, per-daemon bearer auth, owner-only socket under the scoped state dir. - Durable JSON lease records (`workflow-environments/<run_id>.json`, temp+rename before prepare); startup reaper cold-tears-down records owned by a prior daemon instance (dead relay), then deletes them. - ProcessManager: when a dispatch routes to a NON-local environment (config routing rules/default) and a broker is wired, set the four ANIMUS_ENVIRONMENT_BROKER_* env vars on the runner and inject metadata.animus_run_id into the prepared spec. run_id is stable across a run's phases (resume dispatches reuse the persisted workflow id; a fresh first phase mints one the runner adopts). - Terminal-state teardown wired into completed-process reconciliation (Completed/Failed/Cancelled/Escalated => teardown; Running/Paused => keep). - Broker constructed + threaded into ProcessManager in the daemon composition path (bind failure is non-fatal: runners fall back to per-phase preparation).
…workflow id (REQUIREMENT-049) The broker run_id must be stable across every phase of a run so all phases acquire the SAME node. The original derivation minted wf-<uuid> for a fresh phase and used resume_workflow_id for later phases — but the runner mints its OWN workflow id (the daemon can't hand one down: --workflow-id means 'resume an EXISTING workflow'), so phase 1's minted id never matched phase 2's resume id => a different node per phase, no sharing. Key on dispatch.subject_id() instead (identical across every phase's dispatch, known before phase 1). Subjectless adhoc runs fall back to the resume-id / fresh mint (single-phase only).
…nvironment broker)
The daemon's EnvironmentBroker gate (configure_environment_broker) resolved the run environment from kind-level routing rules only, passing workflow_env=None. On deployments whose only environment config is workflow-level (workflows[].environment), resolve_environment returned None -> broker disengaged -> each per-phase runner prepared its OWN ephemeral node -> code-implement's edit never reached code-open-pr's workspace (no diff -> no PR). Thread a workflow-id -> environment map into ProcessManager (populated in daemon_run alongside environment_routing) and feed the dispatch's workflow-level environment as workflow_env, mirroring how the runner resolves each phase. A workflow with an environment: now engages the broker (keyed per subject) so every phase shares one node. Workflows without an environment stay local. Bump orchestrator-cli 0.7.0-rc.16. TASK-431 / REQUIREMENT-051.
… list_all
reconcile_stale_in_progress_tasks ran every heartbeat and, whenever any task
was in-progress, fetched EVERY workflow run with its full opaque blob
(hub.workflows().list() -> list_all -> journal/list with no filter) just to
cross-reference task_id + terminal status. On a board with ~1200 runs that is a
~6s all-runs scan that monopolized the shared journal host every ~8s — the
portal /workflows intermittent multi-second stalls (steady state was fine).
Add WorkflowServiceApi::list_summaries + WorkflowStateManager::list_all_summaries
backed by the journal's existing no-blob projection (journal/list {summary:true},
now carrying the indexed subject_id). The reconcile leg consumes summaries
(subject_id + status + terminal timestamp) — exact same cross-reference logic,
but ~40ms instead of ~6s and no blob fetch/deserialize. Pairs with
animus-postgres exposing subject_id in the summary projection.
Bumps orchestrator-cli to v0.7.0-rc.17 (on the rc.16 / REQ-048 broker lineage).
…ctive-only rc.17 fixed the stale-in-progress reconcile but the ~6s /workflows spikes persisted: OTHER per-tick legs still fetched every run with full blobs (hub.workflows().list() -> list_all -> unbounded journal/list). Convert them all: - DaemonTickMetrics::collect (runs EVERY tick — the ~8s spike cadence): count by status via list_summaries instead of the all-blobs list. - reconcile orphan-recovery / journal-resume / manual-timeout: these act only on non-terminal runs, so list_active() (pending/running/paused, few rows) not list. - load_workflow_ref_index (rebuilt every housekeeping tick by the budget scan): id + workflow_ref are indexed columns, so use the summary projection. Adds WorkflowServiceApi::list_active + workflow_ref to WorkflowRunSummary. The remaining list_all callers are non-tick (CLI history/cost, resume-on-startup, dev SQLite fallback). Works with the already-deployed animus-postgres bundle (summary carries subject_id + workflow_ref). Bumps orchestrator-cli to rc.18.
…(rc.19) Two portal-facing features: 1. `output read --workflow-id X --phase Y` resolves that phase's run_id from the phase session checkpoint (runs/<wf>/phases/*.session.json) and reads ITS transcript — so the UI can load logs for a specific/earlier phase, not just the workflow's latest run. 2. Workflow list gains a type filter + total, for numbered pagination. Repins control-protocol v0.7.0-rc.6 -> rc.7 (adds WorkflowListRequest.workflow_ref + WorkflowListResponse.total, both additive/optional). The workflow/list control handler now forwards workflow_ref into the query filter and returns page.total; workflow_ref becomes wire-expressible. Bumps orchestrator-cli to v0.7.0-rc.19.
The numbered-pager total came from query_ids -> ids.len(), and journal_client sent limit: None, which the reference backend defaults to 1000 — so a board with >1000 runs reported total 1000 and the pager capped at 20 pages. Request the backend's max (10_000) for the id/count query (ids are cheap, no blobs) so the total is accurate up to that ceiling. Bumps orchestrator-cli to rc.20.
Filtering the /workflows list by type was slow: workflow_query_can_use_db_page excluded workflow_ref, so a filtered query fell back to list_all() — fetching every run with full blobs and filtering in memory (~6s). The journal backend's list/query_ids already support a workflow_ref filter, so thread it through list_page + query_ids and let workflow_ref stay on the bounded DB-page path. Filtered lists are now server-side bounded + accurate-total, same as unfiltered. Folded into rc.20.
…liation A remote-animus (REQ-052) node runs the delegated workflow standalone and journals its run's subject UPSTREAM in the BARE form (TASK-1), while the delegating runner registers that subject QUALIFIED (task:TASK-1) in active_subject_ids. The orphan-reconciliation exclusion checks compared the two raw forms, never matched, and misclassified the node's own Running row as a resumable orphan mid-session -> re-dispatched it every tick -> spawned a duplicate ephemeral node until dispatch was paused (one enqueue -> N nodes). Add bare_subject_id() and normalize both sides before the exclusion checks in recover_orphaned_running_workflows and resumable_orphans_for_redispatch (including the live-orphan guard). Adds a unit test.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Root-caused this session: one
codingdispatch fanned out to N ephemeral nodes because the orphan-reconciliation exclusion checks compared a delegating runner's QUALIFIEDactive_subject_ids(task:TASK-632) against a remote node's BARE upstream-journaled subject (TASK-632) — never matched → the node's own Running row was re-dispatched every ~90s tick → duplicate node each time.Fix:
bare_subject_id()helper + normalize both sides before the exclusion checks inrecover_orphaned_running_workflowsandresumable_orphans_for_redispatch(incl. the live-orphan guard). Unit test added. Released as tag v0.7.0-rc.21 for the portal deploy.Verified: cargo check/fmt/clippy clean, reconciliation tests pass.