Make a library's declared library dependencies reach its worker by cjkindel · Pull Request #5468 · griptape-ai/griptape-nodes-engine

cjkindel · 2026-09-03T16:13:32Z

The short version

A library can declare that it depends on another library. The engine loaded those dependencies; the worker did not.

The symptom, from CorridorKey: picking an OCIO colour space worked on the canvas and failed on run. Its OCIO path reaches into the OpenEXR library, which the orchestrator had loaded and the worker had never heard of. That divergence between editing and running is the exact thing the worker boundary exists to eliminate.

Two reasons it was missed, and the second is the interesting one:

A worker's list of libraries to load came only from its spawn request, so it never expanded through what those libraries themselves declare.
Once it did expand, the matching still failed: a declaration names a git repo, the registry is keyed by library name. griptape-nodes-library-openexr publishes itself as OpenEXR Library, so every library not named after its repo missed — silently, because a miss only warns and skips.

Where to look: _library_info_for_repo_name and its two callers. Both have to agree on every spelling of a declaration URL, since a disagreement only logs.

Detail

Targets expand transitively

The target list now expands transitively through each target's declared library dependencies, read from the discovered manifests rather than from LibraryRegistry, so this is usable before anything has loaded.

A dependency that cannot be resolved is left out and logged rather than fatal: declarations are required: false in practice, and a worker refusing to start because an optional companion library was absent is a worse failure than the feature that companion powers being unavailable.

The declaration names a REPO; the registry is keyed by NAME

Provisioning installs each download under a repo-name directory, so the path is where the repo name actually appears.

One normalization for both callers. A declaration URL may carry an @ref suffix and a .git extension, and both change the final path segment. The transitive resolver and the target expansion normalized differently, so a pinned declaration resolved for one and missed for the other.

Two adjacent silences went with it: a manifest that could not be read dropped every dependency it declared with no log at all (indistinguishable from declaring none), and the install-path fallback answered with whichever entry matched first — so a FAILURE record sitting beside the copy that loaded reported "not installed here" for a library that is.

Retiring the worker-reach-into-orchestrator warning

Included here because it was about this same boundary, and was measuring it wrong.

It fired on every request a worker forwarded during node execution, to warn that fetching orchestrator-owned state per call costs a round trip and returns a view stale on arrival. That concern is real. The rule was not measuring it.

Almost everything it fired on is engine machinery the author never wrote and cannot avoid — get_parameter_value issuing ListConnectionsForNodeRequest per parameter is the loudest, and it must keep forwarding, because connections are authoritative orchestrator state. So the rule overwhelmingly indicted the engine while presenting itself as author guidance.

Its remediation then told you to ignore it, offering a case ("an intentional write, e.g. publishing a parameter value") that goes through put_event and never reaches this code at all. A rule that fires constantly on unavoidable machinery and hands you an excuse teaches people to ignore the whole category, which costs more than the round trips it pointed at.

Requests remain the sanctioned boundary — the engine forbids manager access during worker execution and directs authors here — so the guidance moves to the docs, stated once, where it is not competing with a warning that cries wolf. Scoping the rule to library call sites was the alternative; it needs a stack walk on every forwarded request during execution, paying a hot-path cost to warn about hot-path costs.

Forwarding behavior is unchanged.

codecov · 2026-09-03T21:16:28Z

Codecov Report

❌ Patch coverage is 91.13924% with 7 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...pe_nodes/retained_mode/managers/library_manager.py	91.13%	7 Missing ⚠️

📢 Thoughts on this report? Let us know!

CorridorKey's OCIO path reaches into the OpenEXR library. The orchestrator loaded it, the worker did not, so selecting an OCIO colour space succeeded on the canvas and failed on run -- the exact class of divergence the worker boundary exists to eliminate. A worker loads only the libraries it serves, and that target list was built from the spawn request alone. It now expands transitively through each target's declared library dependencies. Read from the discovered manifests rather than from LibraryRegistry, so this is usable before anything has loaded. The declaration names a REPO while the registry is keyed by library NAME: `griptape-nodes-library-openexr` publishes itself as `OpenEXR Library`, so matching the repo name against library names missed every library not named after its repo -- silently, since a miss only warns and skips. Provisioning installs each download under a repo-name directory, so the path is where the repo name actually appears. One normalization for both callers. A declaration URL may carry an `@ref` suffix and a `.git` extension, and both change the final path segment; the transitive resolver and the target expansion normalized differently, so a pinned declaration resolved for one and missed for the other. Two adjacent silences went with it: a manifest that could not be read dropped every dependency it declared with no log at all, and the install-path fallback answered with whichever entry matched first, so a FAILURE record beside the copy that loaded reported "not installed here" for a library that is. The worker-reach-into-orchestrator warning is retired here, because it was about this same boundary and was measuring it wrong. It fired on every request a worker forwarded during node execution, to warn that fetching orchestrator-owned state per call costs a round trip and returns a view stale on arrival. That concern is real; the rule was not measuring it. Almost everything it fired on is engine machinery the author never wrote and cannot avoid -- `get_parameter_value` issuing `ListConnectionsForNodeRequest` per parameter is the loudest, and it must keep forwarding, because connections are authoritative orchestrator state. Its remediation then told you to ignore it, offering a case ("an intentional write, e.g. publishing a parameter value") that goes through `put_event` and never reaches this code at all. A rule that fires constantly on unavoidable machinery and hands you an excuse teaches people to ignore the category, which costs more than the round trips it pointed at. Requests remain the sanctioned boundary -- the engine forbids manager access during worker execution and directs authors here -- so the guidance moves to the docs, stated once, where it is not competing with a warning that cries wolf. Scoping the rule to library call sites was the alternative; it needs a stack walk on every forwarded request during execution, paying a hot-path cost to warn about hot-path costs. Forwarding behavior is unchanged. Author-facing docs stop teaching the pattern the guard refuses. Six places told node authors to read secrets or connections through a manager accessor, which raises once a node executes in a worker: three in getting_started, one each in authoring_libraries and error_handling, and the connection utilities in examples. They use the corresponding request instead. The param_components README keeps its before/after example, annotated, since the component it advertises already uses the request.

Three sites read a declaration URL, and the registration path still derived its pieces inline. That is the duplication the shared helper was added for: a `@ref` suffix and a `.git` extension both change the final path segment, so a site that normalizes differently resolves a declaration the others miss, and a miss only logs. The helper returned the repo name alone, which the registration path could not use -- it also needs the normalized URL and the ref to download a missing dependency. Widened to return all three, so every caller shares one interpretation instead of two of them sharing part of it. Covers the ref and normalized URL as well as the repo name: the download reads those two, and losing the ref installs the default branch instead of the declared one while still reporting success.

A worker loads its library's declared dependencies, so their execution pins have to be importable here too. They were not: the expanded targets reached the load loop through a local, while the gates that install and splice execution environments read the instance field, so a dependency loaded without the pins its own nodes need. Rather than build each dependency its own .venv-exec and splice them all, the dependency's execution set joins the depending library's single resolution. Splicing several independently resolved environments would reproduce between libraries the disagreement the combined edit/exec resolution already avoids within one -- two resolutions can pick different versions of anything they share, and whichever landed first would win -- and would have one worker writing a venv another library owns. It also leaves the target predicate, and the isolation it enforces, untouched. The combined set drives every decision, not just the install: a library declaring no execution dependencies of its own would otherwise retire the environment its dependency's pins were about to go into, since retirement deliberately runs ahead of every gate. The two walks over dependency declarations are now one generator, so the target expansion and the execution-set collection cannot disagree about the same manifest.

The existing tests patch the collection helper, so a break BETWEEN collection and install passed all of them: reading the wrong library's declarations, failing to resolve a repo name, or dropping the combined set before the install would each have gone unnoticed. Drives the real path instead, with two discovered manifests and nothing patched between the request and the installer. Verified by reverting the install list to the library's own set, which fails it. Roots now arrive at the declaration walk with their manifest already read. `install_library_dependencies_request` holds the schema it just loaded, so collecting from a name meant reading that file a second time for every library, in both processes, on the boot path. Also drops the worker note from the README's "Before" example, which is the pattern the page tells you not to copy, and states it once after the "After" example as the reason the component works under worker execution.

… it changes A worker only produces the same result as the orchestrator if it resolves everything the same way, and several separate things decide that: which packages it imports, which project it holds, which workflow it is inside, and whether it hears about a switch. Each was wrong in a different way, and each failed silently. **Execution dependencies reach it as PYTHONPATH.** Splicing `.venv-exec` onto a running interpreter cannot give a library its own versions: a module already in sys.modules is never reconsidered, and a package that probed for an optional dependency at import time has cached the answer -- so a library shipping `safetensors` still hit `NameError` from inside huggingface_hub. PYTHONPATH is on sys.path before the process imports anything. That requires the directory to exist before the worker starts, so the ORCHESTRATOR builds it: it never imports from there, and the worker receiving the directory cannot be the process that creates it. Prepended rather than assigned, because a launcher-set PYTHONPATH is part of the environment the engine itself booted with. **The project comes across at registration.** Registration is the one moment both processes are guaranteed to exist, so the reply carries the project the orchestrator has committed and the worker adopts it BEFORE loading a library. Libraries resolve against the workspace and the workspace follows the project, so a worker that loaded first would resolve them against a different tree. Awaiting a reply the protocol already defines makes that ordering structural rather than a race two messages usually win. **The workflow context is lent, not derived from.** `push_workflow` has no worker-side caller, so a worker answered "there isn't one" to every workflow question: `workflow_dir` raised, the default `{outputs}` situation marks the reference optional, the raise was swallowed, and the path degraded to workspace-relative. The worker wrote real files where the orchestrator does not read, with no error -- invisible in every earlier check because the workflow sat at the workspace root, where the two paths coincide. Lending the CONTEXT rather than any value derived from it is what stops this needing a new handoff per value. An execution carrying no context clears the mirror, or the last workflow keeps resolving paths for the next node. **A switch reaches every worker, in order, before it reports success.** The fan-out was fire-and-forget, so the switch returned while a worker was still on the old workspace. Waiting serially and unbounded was worse: adoption runs a full library reload, so one slow worker held the rest from even receiving the request, and `route_to_worker` argues liveness from the heartbeat, which says nothing about a worker busy adopting. The asks run concurrently, each bounded by the startup grace. And because a worker receives activations from two sources on different loops, they carry the orchestrator's committed generation and only strictly newer ones are adopted -- the newest committed switch wins regardless of arrival order. **Routing waits for the worker to be usable.** A worker is routable the moment it registers, but registration is deliberately early, so forwarding into that window fails node creation with "Library not found". Execution waits on a per-library event set on every terminal outcome: loaded, spawn refused, spawn died, worker evicted, no session available. Bounded anyway, because "every" is a claim about code that keeps changing. Project switching is also exposed headlessly, which is what makes any of the above testable without a human clicking. That moves `ProjectID` to a runtime import so cattrs can structure it, which also changes `ExportProjectRequest.destination_path` from `str` to `Path`, along with the coercion on `ImportProjectResultSuccess.project_file_path` and two `validation` fields. All four round-trip; the export path is the one to look at. Building the execution environment moves behind a gate on the heavy set, so retiring one the manifest no longer declares moves with it: the removal now sits beside the build rather than inside _install_dependency_set, which this gate no longer reaches for a library declaring none. The removal is wrapped: it runs on the registration path, so a directory that will not delete -- held open by a running worker, or read-only on Windows -- would otherwise cost the library its node types over a leftover. Leaving it costs execution correctness, which is the cheaper failure.

Comment pass over this PR's additions: what survives is the constraint or the reason an obvious alternative is wrong, and what goes is incident history, restatement of the line below, and forward references to code that already explains itself. Eight blocks, none shorter than six lines, now four or fewer. The ProjectID alias keeps its justification, rephrased: pydantic resolves those annotations at runtime, so the import cannot be TYPE_CHECKING-only, and events are the import leaf. SetCurrentProjectRequest, GetCurrentProjectRequest and LoadProjectTemplateRequest come off the MCP server's supported set. They were added for a headless client to drive a project switch, but nothing drives one: no unit coverage, and no e2e harness uses them. Their imports go with them.

A worker could be told which project to run against by two different senders: the registration reply and the switch fan-out. Ordering them meant a generation counter on the wire and a lock on the worker, and the generation had leaked onto CurrentProjectChanged -- protocol bookkeeping in a domain event that clients also consume. Registration now sends the activation instead of answering with the project, down the same channel a mid-session switch uses. One sender, one adoption path, and a switch landing mid-registration is just a second message rather than a second source. The reply carries no project, and the worker waits for the activation to settle before it loads any library. The fan-out reads the committed pair rather than taking the id off the event, so the id and the generation describing it come from one read. Two switches in quick succession therefore both fan out the newest committed state and a worker cannot be told to go backwards. The lock stays, and its comment was wrong twice over: the two adoptions are both on the transport loop, not different ones, and one sender does not remove the need for it -- activation awaits internally, so two that arrive close together interleave regardless of who sent them.

These arrived on the branch below, where the worker installed its own execution environment. Here the orchestrator builds it -- the worker receives that directory as PYTHONPATH and so cannot be the process that creates it -- so they were asserting against a path this branch removes. The behaviour under test is unchanged: a declared dependency's execution pins resolve alongside the depending library's own, in one resolution. Only which process does it, and which call to watch, moved.

The test arrives from the branch below, where routing waited on wait_for_worker_library_load. Here it waits on wait_for_worker_ready, so the mock has to match or the await fails before the case under test is reached.

…an event Backgrounding the `.venv-exec` build to keep a torch install off the startup path bought a coordination layer whose only job was to make an install that had not finished look like one that had: a cancel-and-replace protocol, a task registry keyed by venv directory, a readiness event per build, and a spawn that waited on it. The edit-time install next to it was awaited all along, which is the answer to why only the execution set needed any of this. Awaited now, like its neighbour. That deletes _replace_execution_env_build, _build_execution_env, _execution_env_builds, execution_env_ready and wait_for_execution_env, and the spawn path simply reads the recorded reason instead of waiting first. Holding startup is the honest behaviour; installing less often is how to make it cheap. The invented library name went with it -- a failed build no longer needs a name to key an event on, so nothing falls back to "unknown".

Dropping the worker-side sys.path splice left `test_serializable_outputs_are_unaffected` on a premise this branch removed. It set `_is_worker` BEFORE registering, because that was how the execution venv reached sys.path; with the splice gone, worker mode installs nothing at all, so the node module could not import even its edit-time dependency and the library loaded zero nodes. Registration failed, and the guardrail the test is named for never ran. The flag now flips after registering, as the sibling guardrail test above it already does: loading a library is the orchestrator's job, and the flag only selects the execution path under test. Both dependency sets are declared edit-time so this one process can import both, because a real worker receives the execution set as PYTHONPATH from the spawn and no single-process test has that. The two environments being separate on disk is pinned in test_library_execution_dependency_split.py and is not what this test is about; the execution set stays declared so the library is still worker-routed and the guardrail applies at all. Caught by CI on a PR stacked on this branch. This branch's own Tests runs were cancelled, so no completed e2e run had reached the assertion, and it fails on Linux, Windows and macOS alike.

…ds it Building the environment before returning rather than behind an event left this test asserting the opposite of what now happens: `.venv-exec` exists by the time registration answers, so `assert not exec_venv.exists()` fails, and the comment explaining the absence described a scheduled build that no longer exists. Inverting the assertion is not enough to make it worth having. Nothing anywhere proved the execution environment gets built -- every assertion about it was a negative one, and the test that needs an existing environment mkdir's a fake. So this now asserts the directory exists AND that the heavy dependency is installed inside it, which is the mechanism the whole split rests on: the worker receives that directory as PYTHONPATH and cannot build it for itself. What the test was really pinning is untouched and still asserted: the execution set stays absent from the orchestrator's sys.path, and importing it here still raises. The name said "does not install execution dependencies at all", which was never true of the directory -- only of this process's import path -- so it now says which of the two it means.

`worker_ready` said how a library came to be executable rather than what a caller wants to know, which is whether it may route execution there at all. Now `library_ready` / `wait_for_library_ready`, and the docstring stops assuming a separate process: a library that runs in this one is trivially ready. `project_activation_settled` becomes `worker_settled`. It gates one thing today -- the project the orchestrator sent -- but the name is the barrier rather than its current contents, so a second piece of startup state gates this same event instead of adding another for the caller to wait on. Also records #266 against the generation counters and the adoption lock, which is where a reader meets them.

LibraryInfo carried two fields that answered questions about a PROCESS -- is a worker coming, has it loaded the library, did it die -- and WorkerManager wrote them. A record with one owner had two writers: `_start_workers` cleared `execution_unavailable_reason` before a spawn while `_refuse_spawn` wrote it, and `library_ready` was created by one manager and set by the other. Both move to WorkerManager, keyed by library name, alongside the maps it already keeps. Callers ask rather than reading fields: `wait_until_executable` replaces `wait_for_library_ready`, and `worker_unavailable_reason` replaces the field read. Boot asks `wait_for_libraries` for a collective ceiling instead of gathering events it was handed. The other direction goes too. WorkerManager no longer reads LibraryManager to decide whether to refuse a spawn: a library whose execution environment failed to build is simply never asked for a worker, decided where the build happened and the result is already known. What stays on LibraryManager is what it knows on its own -- a declared resource the machine lacks, an environment that would not build -- and `get_worker_for_library` composes both owners' answers for the one message a caller sees.

The reason and the gate are keyed by a bare library name, so they outlived the record they describe: an evicted worker's reason survived a reload that dropped the library's execution dependencies, and get_worker_for_library read it for every library rather than only worker-backed ones. Both fixed -- forget_library on unload, and the read gated on executes_in_worker. is_execution_available answered "settled", which is not the same as available; renamed has_settled. _refuse_spawn had become a second name for note_worker_unavailable, and the worker_manager None guards documented an optionality the annotations deny -- the None came from a test mock describing a shape the engine cannot be in. Restores the eviction-reason assertion that was deleted rather than moved, covers forget_library, and drops mocks for a method that no longer exists.

A manifest can declare legacy worker mode AND execution dependencies, and nothing rejects the pair: `_resolve_executes_in_worker` returns True for either reason and no validator looks at the combination. For that library the orchestrator took the WORKER_DELEGATED branch, which skipped `install_library_dependencies_request` entirely, and inside the worker the execution install is gated on `not self._is_worker`. So neither process built `.venv-exec`. Nothing downstream caught it. No install ran, so no failure was recorded, so `execution_env_failure_reason` stayed None and the spawn was not refused; `execution_site_packages` returned None because the directory was absent, and the worker started with no PYTHONPATH -- the raw ModuleNotFoundError at node-module import that the spawn refusal was added to prevent, reached from the one direction the refusal cannot see. Skipping the LOAD is not the same as skipping the environment. The orchestrator has to build `.venv-exec` whoever loads the library, because the worker receives that directory as PYTHONPATH and so cannot be the process that creates it. The call now happens for both branches and only the resulting lifecycle state differs. What the orchestrator must NOT do for such a library is build the edit-time venv, and `_this_process_owns_the_edit_venv` is where that belongs: it returns True for every library on the orchestrator, which its own docstring already contradicts for the worker-mode case ("nobody else creates `.venv` and the worker must"). It reads the library's `requires_worker` now, so the edit-time gate declines and the execution gate proceeds. An exec-install failure is recorded rather than returned, so routing a delegated library through here cannot fail its registration. A mutation check rather than a reading: reverting the ownership line to `return True` turns two of the three new tests red.

Both TODOs carried a full URL into griptape-ai/internal, whose visibility is INTERNAL, so for anyone reading this repo from outside the org the link is a 404 dressed up as an explanation. `os_manager.py` and `path_utils.py` already cite the same tracker as `griptape-ai/internal#178`, which reads as a reference rather than promising a page. Matching that: the reasoning those TODOs need is in the prose above them either way.

The execution set used to be spliced after the edit-time one, so it landed at `sys.path[0]` and won for any package in both. It now arrives as PYTHONPATH instead, which Python places at `sys.path[1]`, and `_add_library_edit_venv_to_sys_path` then inserts the edit-time site-packages at 0 -- ahead of it. The precedence inverted. That matters because the two directories can hold different versions of the same package. The edit-time install resolves `pip_dependencies` alone; the execution install resolves `[*pip_dependencies, *execution_dependencies]`, and adding a heavy pin is exactly what makes the combined resolver choose differently for something shared. So `process()` ran against the version the execution resolver rejected -- the disagreement the single combined resolution exists to prevent. The splice is skipped when this library's execution site-packages is already on `sys.path`, which is true only in the worker the library was spawned for. That environment is resolved over both sets, so it provides everything the edit-time one does. A library the worker was NOT spawned for is unaffected: its execution directory is not on the path, so its edit-time venv is still spliced, and so is every library's in the orchestrator. Checked by construction rather than by reading: with the execution directory on the path a later `insert(0, edit_site_packages)` resolves a package in both to the edit-time copy. Nothing covered this function, which is why the inversion was invisible -- the e2e suite uses one edit-time and one execution wheel with no overlap, so no test had a package in both. Two tests now: the execution directory stays first when it is present, and the edit-time venv is still spliced when it is not. The caller's docstring carries the invariant again. It said "goes on FIRST, so it wins for anything present in both" before this stack, and losing that sentence is what let the inversion read as tidying.

`spawn_worker` checked `_managed_worker_processes` for a duplicate, then wrote its entry 68 lines later, with the static-URL await and the fork in between. Two spawns for one library both got past the check and both forked; only one could be recorded, leaving the other's process untracked and holding that library's dependencies in memory until its own heartbeat lapsed. Two `StartWorkerRequest`s for one library name overlap whenever `_library_file_path_to_info` carries duplicate entries for it (a FAILURE entry alongside the LOADED copy), or when `_start_workers` runs twice before the first worker registers. The key is now claimed in the same step as the check, and released in a `finally` so a failed fork does not leave the library permanently unspawnable.

`_log_spawn_error` asked `task.exception()` first, which raises `CancelledError` on a cancelled task. Raising from a done-callback surfaces as loop-level "Exception in callback" noise and skips the `note_worker_unavailable` below it, so nothing records that no worker is coming. Cancellation reaches this callback at loop teardown, where nothing is waiting on the worker, so the cost today is the noise rather than a stalled run. Note on A1 of the tracking issue, which covers the same function: the claim that the spawn task can be garbage collected while parked does not hold. `Event.wait()` appends a future to the event's `_waiters`, and the awaiting task installs `Task.__wakeup` as that future's done-callback, so the task stays reachable from the manager for as long as it is parked. No change made there.

`route_to_worker` registers a request then awaits it. Every other exit from that await removes the entry: a response pops it in `_try_match`, eviction in `cancel_requests_by_tag`. Flow cancellation re-raised without removing anything, so the entry stayed in `_pending_requests` for the life of the process -- one per cancelled node execution. Each leaked entry keeps its worker's tag, so a later `cancel_requests_by_tag` walks and re-settles long-dead requests, and `pending_count`, the only visibility into that map, reports a number that never comes down. Nothing fails loudly; it degrades over a long editing session. `_cancel_request` did exactly the removal needed but was async, and awaiting inside an except block while a cancellation is being delivered adds a suspension point in the one place it is least wanted. Its body never awaited, so it becomes the public synchronous `discard_request` and its five internal callers drop their `await`.

`pending_count` and `pending_request_ids` read `_pending_requests` while holding nothing, and it is mutated from more than one loop: `_try_match` on the transport loop, `track_request` and `cancel_requests_by_tag` from whichever loop issued the request. `len()` only risks a stale answer, which is fine for a diagnostic. Building the id list iterates, so a concurrent insert or pop raises "dictionary changed size during iteration" -- a diagnostic that can raise is worse than one that answers a moment out of date. Both are diagnostics with no production caller today, so nothing holds the lock across a call into them.

`_orchestrator_static_server_base_url` picks between a direct read and a thread hop, and between two warnings when no URL arrives. Nothing exercised either choice: inverting the settled check left the suite green, and so did swapping the two warnings. Both choices matter to whoever has to act on them. Taking the hop when the answer is already in parks an uncancellable default-executor thread that teardown joins, and blaming the settle timeout for a resolution that raised points an operator at slow startup instead of the bind failure that actually happened. Five tests: the direct read, the hop with its timeout, each warning, and a cloud backend staying quiet because there its URLs outlive the worker anyway. Each fails if the branch it covers is inverted.

The static-URL tests set `static_server_base_url_settled`, which is a property on the real manager, so pyright rejected the assignment even though the object is a MagicMock. Casting once per test says what the object actually is instead of suppressing each line.

A regression in the spawn claim: `reset_workers` cleared the process registry but not the new claim set, so a spawn suspended across a library reload kept its claim and the reload's own spawn for that library was refused. The refusal records nothing (a worker is normally on its way when a key is claimed) so the next run would wait out the whole startup grace and then blame the library load. Cleared with the registry, with a test that fails without it. `note_worker_unavailable` is deliberately still NOT called from the duplicate guard: it records a reason and settles the library, which would falsely fail a library that already has, or is about to have, a worker. Comments that were wrong or that answered a reviewer are gone rather than reworded again: a claim that `list(dict.keys())` can raise "dictionary changed size during iteration" (it cannot, the whole build is one C loop under the GIL) and a `discard_request` docstring citing a suspension point that never existed. `test_an_undecided_url_is_waited_for_off_the_loop` asserted its own name only loosely; it now records the executing thread and fails if the blocking wait runs on the event loop.

`reset_workers` drops the claims so a reload can spawn its libraries again, which leaves a spawn suspended between its claim and the registry write resuming to find the key claimed by the reload's spawn. Releasing by name alone freed that one, so the library was admitted again while a spawn was genuinely in flight -- and a second fork leaves one of the two processes untracked, which is the leak the claim exists to prevent. Claims are now per attempt: the map holds a token identifying the holder, and the release is conditional on the stored token still being the one this attempt took. The test drives the interleaving: park a spawn, reset, let the reload claim the key, then release the stale spawn with its fork failing so no registry entry shadows a wrongly-freed claim. It fails against an unconditional release. The comment on the claim also drops its comparison with the guard this replaced and states the constraint instead: the registry entry is written only once the subprocess exists, and the work in between suspends.

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch from 9db3dcd to b445c25 Compare September 3, 2026 21:12

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch from 0331732 to 0ce2e3e Compare September 3, 2026 21:12

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch from b445c25 to b1b1471 Compare September 3, 2026 21:36

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch 2 times, most recently from 659a5e9 to b206432 Compare September 4, 2026 15:46

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch from 100da3c to c7c9fc6 Compare September 4, 2026 17:31

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch from b206432 to 9618b4f Compare September 4, 2026 17:31

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch from c7c9fc6 to 7ab76d0 Compare September 4, 2026 20:41

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch 2 times, most recently from 3dee3c4 to f001584 Compare September 4, 2026 20:57

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch from 7ab76d0 to 5e002d3 Compare September 4, 2026 20:57

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch from f001584 to 9ff7102 Compare September 4, 2026 21:13

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch 2 times, most recently from 97624a7 to a12bc73 Compare September 4, 2026 21:39

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch 2 times, most recently from 7424255 to cb74fb3 Compare September 4, 2026 21:48

cjkindel force-pushed the fix/worker-death-surfaces-as-an-error branch from 21c3056 to ea639fd Compare September 4, 2026 22:02

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch 3 times, most recently from f15e8f0 to 9c9c11e Compare September 4, 2026 23:04

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch from 63db608 to 057756e Compare September 18, 2026 19:32

cjkindel added 18 commits September 21, 2026 12:20

Point the worker-gone test at this branch's readiness wait

04a7121

The test arrives from the branch below, where routing waited on wait_for_worker_library_load. Here it waits on wait_for_worker_ready, so the mock has to match or the await fails before the case under test is reached.

cjkindel force-pushed the fix/library-dependencies-reach-the-worker branch from 057756e to 3aefb35 Compare September 21, 2026 19:20

cjkindel added 8 commits September 21, 2026 13:02

cjkindel closed this Sep 21, 2026

cjkindel removed this pull request from stack #5509 September 21, 2026 22:51

Navigation Menu

Make a library's declared library dependencies reach its worker - #5468

Make a library's declared library dependencies reach its worker#5468
cjkindel wants to merge 26 commits into
fix/worker-death-surfaces-as-an-errorfrom
fix/library-dependencies-reach-the-worker

cjkindel commented Sep 3, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Sep 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

cjkindel commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

The short version

Detail

Targets expand transitively

The declaration names a REPO; the registry is keyed by NAME

Retiring the worker-reach-into-orchestrator warning

Uh oh!

codecov Bot commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cjkindel commented Sep 3, 2026 •

edited

Loading

codecov Bot commented Sep 3, 2026 •

edited

Loading