CI (integration shard 1) hit the uninstall-resurrect again: after
uninstall_repository, GET /api/tools/{guid} still returned the tool. The
log shows the interleaving - the reload_toolbox queued by the preceding
install's conf write executed while the uninstall was mid-flight, and its
rebuild (including a potentially long inline index populate) ran with no
synchronization against remove_tool_by_id.
Two halves:
1. _get_new_toolbox now builds and swaps the new toolbox under
app._toolbox_lock, the lock remove_tool_by_id and
invalidate_index_cache already hold. Whichever side wins, the other
sees a consistent world: removal-first means the rebuild reads the
already-cleaned index and conf; rebuild-first means the removal runs
against the swapped-in box. The lock is an RLock, so the populate
broadcast delivered inline on the same thread re-enters safely.
2. remove_tool_by_id also cleans the CURRENT app.toolbox when it is a
different object than self - a rebuild that won the race swapped in a
box built from the pre-removal index, and cleaning the superseded
instance alone would leave the live box serving the uninstalled tool.
Verified: new unit test (swapped-in box cleaned), 45 lazy unit tests,
fastp shed-install + repository-uninstall integration tests in lazy mode
(the lock's deadlock canary - install triggers watcher reload + inline
populate under the lock), tox lint/format/mypy.
_writable_store_index_needs_population compared the set of stored row
hashes against the set of index-referenced hashes symmetrically. The
store is append-only (only reconcile_index prunes), so rows orphaned by
a content change, an ad-hoc self-heal superseded by conf context, or an
uninstall accumulate legitimately - and one orphan made the comparison
unequal forever, turning every boot and every post-install reload into a
full inline repopulate and permanently defeating the freshness-token
scan skip (observed: 5 inline populates across one shed-install
integration test, 3 orphaned rows as the stable delta).
Check the direction that is actually an invariant: every source_hash the
index references must resolve to a stored row, or materialising that
tool would fail. Orphaned rows are logged at debug as prune candidates.
Measured on the same test: 5 inline populates -> 2 (cold boot + the
legitimate freshness-token change from the install itself).
The from-index fast panel init registered stubs into the live panel only,
keyed per collapsed entry id. Three fidelity gaps against the eager conf
walk broke the first lazy CI runs carrying it:
1. The integrated tool panel stayed empty on fresh boots. It is what
_load_tool_panel_views renders EDAM/static views from (and what
_save_integrated_tool_panel persists), so every panel view rendered
empty: the EDAM toolbox tests, all panel_views_dir integration tests,
and /api/users favorite EDAM operations 404d.
2. ToolIndex.entries collapses same-id tools to one winner, so it cannot
represent the panel: a top-level sectionless tool (upload1 in the
functional conf - test_index), a tool id referenced both top-level and
inside a section (multiple_versions), or expanded-content twins each
lost whichever placement their winning entry didn't carry.
3. Runtime shed installs lost their panel position and repository
metadata across the reload that follows: a full repopulate reset
placement order to conf order (eager inserts at the section head via
update_or_append), and the unchanged-file carry kept serving the
ad-hoc self-heal entry written during metadata generation, which has
no shed conf context - so the rendered panel entry lacked
tool_shed_repository.
Fixes, index side: ToolIndexEntry.in_panel marks conf tool items
(converter and data-manager discoveries are indexed for lookups but never
placed); ToolIndex.panel_items records one placement per conf reference
in discovery order, appended by add_entry (keyed on tool id + section,
composite-merged across stores); discovery yields every conf reference of
a path instead of dedup-ing (the store layer is content-addressed and
absorbs repeats); the unchanged-file carry reconciles conf-level context
(section, shed repository coordinates) from the current discovery; a full
rebuild merges the previous index's placement order (the placements list
is the panel-order memory, the role integrated_tool_panel.xml plays for
eager) and partial updates insert new placements at the head of their
section, matching the eager runtime insert.
Toolbox side: _init_tools_from_index registers all stubs unplaced and
replays placements through a shared _place_stub that mirrors every
placement into the integrated panel (hidden included, like the eager
__add_tool always did) and slots visible ones into the live panel.
Verified: lazy integration test_panel_views + test_edam_toolbox 18/18
(previously 16 of the lazy CI dispatch failures), lazy api test_index +
test_favorite_edam_operations, eager panel views 16/16, 153 unit, tox
lint/format/mypy. The index schema hash changes, so existing persisted
indexes rebuild once on upgrade.
Trivial type-annotation nits from the gx-review pass: Tool.parse_tests
-> None, the Tool.tests property -> list[ToolTestDescription] | None, and
the LazyToolBox._load_tool_panel override -> None.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
CondaDependencyResolver.__init__ calls ensure_installed(), which shells
out to 'conda info --json' — spawning the conda binary (~1-2s) — to set
self.disabled. Two independent reductions:
1. Gate the probe on auto_init. With auto_init set, the resolver is
expected to initialize conda at startup, so it still probes (and
installs, if needed) eagerly in __init__ — behaviour unchanged. With
auto_init off, nothing on the boot or job dependency-resolution path
reads disabled (only the dependency-resolvers admin view), so disabled
becomes a cached property probed lazily on first access.
2. Memoize CondaContext.conda_info(). It is queried several times over a
context's life (availability probe, conda version, conda-build
availability), each spawning conda. Cache the result and clear it in
_reset_conda_properties() (already called after installs) so a re-probe
reflects the new state. This also helps the auto_init/eager path.
An earlier revision deferred the probe unconditionally; because the boot
and job resolution paths never read disabled, conda was never
bootstrapped, breaking dependency resolution for conda-package tools.
The generic toolbox init still parsed every tool-conf item and routed
each through load_item even in lazy mode. When the source-store index
already carries panel section metadata, _init_tools_from_index builds
the in-memory registries directly from it: register a LazyTool stub per
indexed entry, place the latest version of each lineage into its panel
section, and skip the XML walk entirely. Falls back to the parent walk
when the index lacks section data (older stores), and runs the populator
first if any panel id is missing from the index.
Dynamic (writable shed) tool confs still need their config_elems tracked
for installs, so _init_dynamic_tool_confs_without_loading registers those
without loading their tools. _writable_store_index_needs_population adds a
source-hash vs index-hash reconciliation so a writable store that drifted
from its index repopulates even when the freshness token says fresh.
create_tool now falls back to an eager parse (instead of raising) when an
on-disk file has no index entry after the ad-hoc populate, and search
results collapse to the latest version per lineage via latest_search_hits.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
_load_tool_panel_views applies the EDAM ontology views over the whole
panel — one of the larger boot costs. When display_builtin_converters is
off (the lazy deployment default) those views aren't served, so the
AbstractToolBox gains a load_panel_views flag to skip the pass. Eager
deployments that display converters keep loading them.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
_index_versions_for prefix-scanned every index entry per lineage lookup,
and the EDAM panel views resolve a lineage per shed tool — ~11.5k scans
over ~9k entries. Profiled as the single largest toolbox boot cost:
18.0s cumulative, and _configure_toolbox drops from 25.8s to 11.7s
(under cProfile) with the scan replaced by one precomputed
versionless-guid -> (entry_id, version) map.
The map is cached against the ToolIndex object identity plus entry
count — reloads swap the object, registrations/removals mutate
membership in place — and in-place removals also reset it explicitly
(a pop-then-add could keep the count stable while changing membership).
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Walking a CVMFS shed conf, the eager _load_tool_tag_set paid two per-tool
costs the lazy toolbox doesn't need: an os.path.exists stat (network
round trip on CVMFS — minutes over ~10k tools) and, since conf-provided
repositories have no install-DB row by design, one 'Attempted to load
tool shed tool, but the repository ... was not found in database'
warning per tool per boot.
Two overridable hooks in AbstractToolBox, both eager-behavior-preserving:
_tool_file_on_disk (the existence gate) and _missing_repository_log_level
(the warning's severity). LazyToolBox answers both from an identity-cached
set of indexed source_paths: the populator established existence when it
stored the source and materialisation reads the raw source from the
store, so the stat proves nothing; the missing install-DB row is expected,
so it logs at debug. Unindexed paths keep exact eager behavior.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Discovery enumerated every conf and only afterwards dropped tools
routed to read-only (or untargeted) stores — paying the per-tool
existence checks for nothing. On a CVMFS shed conf that is thousands of
network stats; a full local populate spent minutes walking 8.5k tools
it was about to discard.
discover_tools gains only_confs; the populator passes the confs routed
to this run's writable stores (bundled follows the same gate — it
routes to the default store), and the boot coverage scan passes the
confs not routed to read-only members, replacing the post-walk skip.
Measured: full populate against a config with a read-only CVMFS store
drops to ~7s total, discovery 1.2s over the 344 default-routed files.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Comparing a stamped CVMFS revision at boot is unsound for the canonical
publishing workflow: the populator runs inside the publish transaction
at revision N, the publish commits as N+1, so every client reads
current != stamped forever. The revision can't record the bump it is
part of.
It's also unnecessary. A read-only store ships in the same transaction
as the tools it indexes, so a schema-valid index (load_index already
gates on INDEX_SCHEMA_HASH) is authoritative by construction — trust
it. index_is_fresh on a read-only store now returns whether the index
loads; False means no loadable index at all, which the composite warns
about and continues (only the publisher can fix it).
freshness: cvmfs is thereby watcher-only: the watcher compares current
revision against the last value seen on this client — transition
detection, immune to the off-by-one. The writable default store keeps
the tool_confs stamp-and-compare, where populate and boot share a host
and the comparison is meaningful.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
New opt-in watch_tool_source_stores (+ tool_source_store_watch_interval,
default 60s): a per-process thread polls the freshness probe of every
read-only named store — one extended-attribute read per store per tick
for CVMFS — and reacts to token transitions. inotify does not fire on
CVMFS, so polling is the only reliable signal, and catalog TTL bounds
propagation anyway.
On a transition the store's engine is disposed before the index reload:
CVMFS keeps serving the pre-publish file to descriptors that were open
before the catalog update, so pooled sqlite connections pin the old
snapshot until dropped. The same disposal now also runs for read-only
members in invalidate_index_cache, making the existing admin reload API
and reload_tool_source_cache broadcasts correct on CVMFS as the manual
trigger. The reload itself reuses invalidate_index_cache (stub
registration, removal reconcile) plus a per-changed-store whoosh rebuild
guarded by the corpus-signature skip.
Watchable stores are read-only members with a probe: writable stores
change through this process group's own populate paths, which already
broadcast their own reloads. The watcher compares against the last
*seen* token, not the persisted one, so a store whose publisher hasn't
repopulated yet logs once instead of re-firing every tick.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The populator now stamps a freshness token into each writable store's
persisted ToolIndex, captured before the tree walk. At boot,
_index_needs_population re-probes and a match skips the conf walk and
its per-file existence stats entirely — the store provably covers the
current tree. A mismatch (or no probe) falls back to the batched
coverage scan as before, so a wrong token can only cost work, never
correctness.
Two probe kinds (galaxy.tools.source_store.freshness):
- tool_confs: md5 over tool/data-manager conf contents plus recursive
directory mtimes of tool_dir entries. Detects the same
addition/removal drift the coverage scan does, at ~a-dozen-file cost.
Wired to the default store automatically.
- cvmfs: the CernVM-FS repository revision via the user.revision
extended attribute on the mount point — one syscall covers the whole
repository. Opt-in per named store (freshness: cvmfs, optional
freshness_path), since a store published on CVMFS in the same
transaction as its tools makes a matching revision a hard consistency
proof.
Composite aggregation: a stale writable member triggers the populator;
a stale read-only member only warns (the publisher owns repopulation);
a probe-less member downgrades to the coverage scan.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
_index_needs_population issued a get_by_source_path round trip for every
config-discovered tool; on a CVMFS-scale deployment that is thousands of
store queries before the panel builds. Fetch the stored paths once via
the new list_source_paths() and diff as a set.
Also stop the populate loop for read-only stores: a path routed to a
read-only member can never be healed by the inline populator (it skips
read-only targets), so a miss there re-triggered a full populate on
every boot. Warn and fall through to eager parsing instead.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The resolve brought over the pydantic ToolIndex and the incremental/twin
populate tests without adapting lazy-only surface: benchmarks.py still
called the deleted to_dict/from_dict, and the test config stubs lacked
the data-manager conf and biotools attributes the lazy populate path
reads. Convert benchmarks to model_dump/model_validate and consolidate
the two identical test stubs into one _populate_config helper carrying
the full attribute set.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Store discovery walked config.tool_configs, but in the standalone
populator CLI that list is never expanded to include shed_tool_config_file
(that expansion only happens during app boot). So a store declared with
store="..." on the shed tool_conf — the documented CVMFS pattern — was
invisible, and --target NAME failed with "not found; available:
[__default__]". Walk config.all_tool_config_files() in
_collect_per_conf_store_names, _build_stores, and _build_conf_to_store_map,
matching discover_tools. Regression test covers a store declared on the
shed conf.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Drop lib/galaxy/tools/source_store/models.py — 10 of its 11 pydantic
models were orphaned scaffolding for a tool-source admin API that no
longer exists (nothing referenced them). Keep the one that maps to a real
endpoint, SanitizeAllowlistResponse, moved next to its only consumer and
given a typed per-tool entry so it actually narrows the response. The
controller now returns SanitizeAllowlistResponse(**...).model_dump(),
which reproduces the exact JSON the client already consumes (verified
round-trip-identical).
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Three review fixes to the populator:
- Reload broadcast: send_reload_notification does a bare kombu exchange
publish that never declares the active-process queues, so on the kombu
sqlalchemy transport (a common default) the shed-install broadcast never
reaches peers — their cached index stays stale until restart. Add
_broadcast_reload, which routes through send_control_task (correct
declare_queues, derived from the worker_process table) when an app is in
hand; the app-less CLI keeps the best-effort raw publish. Thread app
through populate_store_inline / populate_for_paths / reconcile_index and
the shed-install, ad-hoc self-heal, and reset_shed_tools call sites.
- Macro watch: a changed macros file left its importing tools with stale
expanded content because the watcher filtered "macro" out by filename.
Accept every .xml and, on a <macros> root, re-expand the tool siblings
in its directory.
- Typing: annotate build_index_entry_from_source (ToolSource / stored /
discovered) and drop the redundant hasattr guards the ABC already
guarantees (parse_uuid stays guarded — it is XML-only).
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
ToolPanelViewSearch (eager) and ToolWhooshIndex (store) built the same
field set + boosts twice, inviting drift. Extract build_search_schema in
source_store.search as the single definition; the eager side passes
help_boost to add its help field (the populator has no rendered help text,
so the store schema omits it and help-text queries don't match in lazy
mode). Field boosts are unchanged, so ranking is identical.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Neither is on the production read path. Lazy search goes through the
populator-built whoosh index (LazyToolboxSearch -> ToolWhooshIndex), and
/api/sanitize_allow builds its response from config.sanitize_allowlist
against the toolbox. The two ToolIndex methods were called only from
benchmarks.py and unit tests, and ToolIndex.search's token ranking
diverged from whoosh's BM25F — a correctness trap. Remove both (whoosh
search stays covered by test_multi_store_search.py); keep the reusable
per-entry ToolIndexEntry.to_sanitize_entry projection.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The only local import left is lazy_toolbox's send_control_task, which
is a genuine cycle: galaxy.queue_worker imports LazyToolBox at module
level. Everything else - app boot, queue_worker, tool_panel_manager,
services/tools, search, populator, benchmarks, uses_shed and the test
modules - now imports at the top; the claimed app/tools circularity
never existed (galaxy.app already imports galaxy.tools).
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
walk_tool_directories() in tool_util.toolbox.base is now the single
implementation of the hidden/private-skipping directory walk;
AbstractToolBox.__watch_directory consumes it (keeping per-directory
watcher registration) and discover drops its parallel copy.
Two deliberate behaviour touch-ups on the eager side: entries are now
walked in sorted order (deterministic panel order for tool_dir tags),
and a directory's watcher now registers when any tool in it loaded
rather than only when the last candidate did.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
AbstractToolBox.__resolve_tool_path becomes the module-level
resolve_tool_path() in tool_util.toolbox.base and discover uses it,
dropping its own copy. This also fixes discover's fallback for confs
without a tool_path attribute: it guessed <root>/tools, while the
toolbox has always used config.tool_path; discover_tools_from_config
now takes that default (discover_tools passes config.tool_path).
Also hoists the MODEL_TOOLS_PATH import to module level - the parent
galaxy.tools package is imported with the subpackage anyway, so the
local import bought nothing.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Adopt #23067's store infrastructure wholesale where it is canonical, keeping
only genuine lazy-toolbox additions as deltas on top:
- Store package: adopt #23067's factory.py/interface.py split and facade
__init__.py, and its URL-only SqlAlchemyToolSourceStore. Re-apply lazy-only
deltas — ToolIndexEntry panel-contract fields (icon/xrefs/model_class/
form_style/is_workflow_compatible/source_path) and data_manager_id;
composite per-version index merge; scored multi-store whoosh search
(search_scored + tool_tags field); populator panel-contract derivation via
expand_ontology_data + biotools and data_manager_id stamping; data-manager /
converter discovery in discover.py; benchmarks.py.
- Config: replace tool_source_store + tool_source_disk_path with the single
SQLAlchemy URI tool_source_database_connection (defaulted in config/__init__.py,
validated via try_parsing, schema attr added), adopt #23067's tool_source_stores
wording, and keep the branch-only use_lazy_toolbox / lazy_toolbox_cache_size
options. Regenerated galaxy.yml.sample, galaxy_options.rst, and the schema-type
stub. galaxy_mock uses tool_source_database_connection.
- Docs: adopt #23067's tool_source_storage.rst (admin + dev) as the base and
re-add the lazy sections (LazyToolBox, batch-endpoint integration,
materialisation-count guard, LazyToolboxSearch multi-store search, benchmarks).
- Tests: adopt #23067's store + scripts unit tests; re-add the ours-only
composite entries_by_version merge test, data-manager discovery /
build-index tests, and multi-store search test, all on the URI config.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The store is a rebuildable, content-addressed cache — it does not need
Galaxy's database, and living there was the source of every subtle
lifecycle mechanism in the subsystem: store.commit() committed whatever
request or job transaction was in flight, reads needed a private-Session
isolation hack, the LazyToolBox constructor carried a post-init commit to
avoid queue-worker idle-in-transaction locks, shutdown needed a haltable
ordered before engine disposal, and a sa_session parameter threaded
through the factory, populator, watcher and CLI.
Standalone stores are now the only kind: the default is a SQLite file
under tool_source_disk_path (any SQLAlchemy URL via tool_source_stores
for shared multi-host deployments), committing per write on its own
engine. The tool_source_record/tool_index tables, their migration, the
model classes and the whole session plumbing are gone, and the store is
only initialized when use_lazy_toolbox is enabled — default deployments
no longer touch any of this at boot. The ToolSourceStore ABC loses
commit() (every backend persists per write). The dev/watch-mode watcher
now populates per file without broadcasting and keeps its single
debounced reload notification for the batch.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The populator writes one whoosh index per store, but search only opened
the default store's directory — tools served from a named store
(per-conf store="...") listed and ran fine yet never appeared in
/api/tools?q=. Search each configured store's index and merge hits by
BM25 score (ToolWhooshIndex.search_scored); scores from different
indexes aren't strictly comparable, but interleaving beats invisible
tools, and resolve_search_hit drops ids foreign to this toolbox.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The merge loop copied only entries and by_section, leaving the merged
index's per-version map empty — ToolIndex.get(tool_id, tool_version)
resolves exact versions through entries_by_version, so every non-newest
version of a member-store tool was unreachable (wrong default served,
get_all_versions empty). Merge per (id, version) with the same
earlier-member-wins rule.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Two gaps found by review: (1) invalidate_index_cache was add/update
only — it registered new index entries as stubs and refreshed existing
ones, but never dropped registrations whose ids vanished from the
reloaded index, so a peer-process uninstall left this process serving
the stale stub via the eager get_tool fall-through; (2) remove_tool_by_id
persisted the index removal locally but never broadcast, so peers had no
reason to reload until an unrelated populate ran.
Extract the in-memory removal bookkeeping into _remove_tool_in_memory,
diff the previous index against the reloaded one in
invalidate_index_cache and pop vanished ids (diff-based, so internal
and dynamic tools that never enter the index are untouched), and send
reload_tool_source_cache (noop_self) after persisting a removal.
_register_lazy_entry now registers the old-id bucket even when
old_id == tool_id, matching the eager __add_tool — super's removal
unconditionally pops that bucket.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Commit 4c16f4d819a (drop _init_lazy_toolbox) removed the
LazyLineageMap assignment together with the old init path, silently
downgrading the toolbox to the eager LineageMap. Boot-time lineages
still came out right (the eager walk's register() accumulates versions
by versionless id), but every post-boot lookup — peer installs surfaced
by invalidate_index_cache, reloads — fell back to building a
single-version lineage from one Tool object, hiding the other indexed
versions from get_safe_version and panel version dedup.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The LazyTool docstring claimed strict-by-default with a nonexistent
LAZY_TOOL_PERMISSIVE toggle (the code is permissive by default, toggled
by LAZY_TOOL_STRICT=1). The dev doc described a Redis backend, an API
layer (api/tool_sources.py, ToolAPICache), a _init_lazy_toolbox
mechanism, and an auto-enable behavior that don't exist, plus pre-move
module paths. Also drop change-narration comments referencing removed
intermediate code, an unused SqliteToolSourceStore alias, and hoist the
populator's local os/biotools imports now that the module lives inside
galaxy.tools.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The package is app-coupled (galaxy.model, galaxy.config, galaxy.queues,
galaxy.datatypes) and only consumed from app-side code, so it belongs
under galaxy.tools rather than a new top-level package. Riding the
existing packages/app tools symlink also removes the need for a
dedicated package symlink.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
The LAZY_TOOL_STRICT=1 lazy Integration run surfaced three execution-path
attributes that were only materialising via the permissive fallback:
- build_dependency_shell_commands (on __SET_METADATA__, which follows
most jobs) — the runner builds the dependency/env shell commands for
the tool it's about to run
- params_with_missing_data_table_entry / params_with_missing_index_file
(data-manager / index-file tools) — param validation that walks the
parsed parameter tree at job time
All three genuinely need a parsed Tool, so add them to _MATERIALIZE_OK.
Under permissive mode this was a silent WARN + materialise; under strict
it raised NotImplementedError and failed every job-running integration
test. This is exactly the audit strict is meant to force — the fix makes
the materialisation explicit rather than accidental.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
Add a materialise counter to LazyToolBox (_lazy_materialize_count, bumped
at the single _create_tool_from_stored_source chokepoint) and an
integration test that hits every batch reader the client uses — flat
listing, panel walk, panel views, tests-summary, all_requirements, and
search — asserting a zero materialise delta across them.
This catches accidental whole-toolbox materialisation that LAZY_TOOL_STRICT
can't: a tool-filter or _MATERIALIZE_OK attribute read inside a full-index
sweep parses tools without ever hitting the strict __getattr__ path. The
test was written against the search-materialisation regression fixed in
the previous commit and fails without it.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
ToolsService.search_tools resolved every whoosh hit through
_get_tool -> toolbox.get_tool, which in lazy mode loads the tool from the
store on demand. A single /api/tools?q= query therefore parsed one Tool
per hit. The populator-owned whoosh index also carries ids that were
never loaded into this toolbox (tool_conf.xml.sample alone lists ~150
legacy tools whose files aren't present), so lazy search both parsed and
returned tools the eager path skips: eager whoosh only holds loaded
tools, and eager _get_tool returns None for un-loaded ids.
Add LazyToolBox.resolve_search_hit, which looks the hit up in the
registered stubs (_tools_by_id, plus the shed short-id map) and returns
None for anything not part of this toolbox — no parse. search_tools uses
it in lazy mode and keeps the eager _get_tool path unchanged. Result:
search stays O(hits) index reads and matches eager scoping.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
AgentTools.get_tool_categories iterates toolbox.tools() and reads
tool.get_panel_section()[1] on every visible tool. get_panel_section was
not on the LazyTool stub surface, so under the permissive default that
sweep materialised the entire toolbox to build the category list (and
under LAZY_TOOL_STRICT=1 it would now raise).
The populator already stamps panel_section_id / panel_section_name onto
ToolIndexEntry, so forward the call off the entry — matching the
(section_id, section_name) / (None, None) contract of
Tool.get_panel_section — and keep the walk lazy.
Claude-Session: https://claude.ai/code/session_018L7ZmCv2ubKA3JNeSL8Pkr
test_repository_uninstall kept resurrecting the tool after all
index-level fixes. Local repro + probes showed removal completed on the
right toolbox but two in-memory paths still served the tool:
- The materialise LRU is keyed by whatever id the caller resolved with,
so the same tool sits under both its guid and its short id; the
guid-prefix purge left the short-id entry behind. Purge by cached
tool identity instead.
- _tools_by_old_id is keyed by old_id — the SHORT id for a shed guid —
and super().remove_tool_by_id deletes from it by object identity
only, which misses when the bucket holds an earlier registration
(stub or materialised instance) than _tools_by_id. The leftover
resurfaced through the eager get_tool fall-through. Scrub every
object belonging to the removed guid; sibling installs sharing the
short id survive.
Verified locally: the full TestRepositoryInstallIntegrationTestCase
passes 3/3 consecutive lazy runs (install, uninstall, update).
remove_tool_by_id runs under app._toolbox_lock, but the invalidation
path (queue-worker reload_tool_source_cache handler — and every
populate broadcasts one) reloaded the index and re-registered stubs
without it. Interleaved with an uninstall, that reloads the pre-removal
index and re-registers the just-removed tool. Take the same RLock.
When the target <section> already exists in the on-disk shed_tool_conf,
add_to_shed_tool_config merges the incoming children into it via lxml
append — which reparents them, leaving the section elements in
elem_list empty. The lazy branch persists the conf BEFORE loading (the
populate needs it on disk), so both the partial populate (no paths →
no enriched entries) and load_item (empty section → tool never enters
the in-memory panel; only the async conf-watcher reload rescued it) ran
against gutted elements. First install into a fresh section was immune,
which is why only the second fastp changeset lost tool_shed_repository
(test_only_latest_version_in_panel_fastp — now reproduced and verified
locally in both lazy and eager modes).
Deep-copy the elements before persisting and drive the populate +
load_item from the snapshot. The eager branch loads before persisting
and stays untouched.
The partial populate filters discovered tools on exact path strings.
_collect_new_tool_paths joined the shed conf dict's raw tool_path —
relative for typical shed_tool_conf files — while discovery resolves a
relative tool_path against the conf file's directory and returns an
absolute path. The strings never matched, so the enriched conf-driven
discovery was filtered out and the ad-hoc synthesis populated a
metadata-poor entry instead: the panel served fastp without
tool_shed_repository (test_only_latest_version_in_panel_fastp), and the
store grew a duplicate row under the unresolved path string.
Route both through discover.resolve_tool_path (now public) and thread
each tool's guid so even a genuinely-unreachable path yields a
guid-keyed entry.
The data_manager_id stamp landed in index.entries only —
update_index_entry never touched entries_by_version, and the two maps
serialize independently. Job-time materialise passes a tool_version, so
it resolves through the per-version map and got the unstamped entry
back after the next index reload (populate broadcasts one constantly):
exec_after_process still failed with 'Invalid data manager (<guid>)'.
Both backends now route updates through ToolIndex.add_entry, which
keeps the maps in step (and stops an older-version update from
clobbering the newer default entry). create_tool additionally stamps
the in-memory per-version twin, which is a distinct object after a
from_dict reload.
_register_new_index_entries_as_stubs skipped every tool already in
_tools_by_id, so a stub minted from an install-time ad-hoc entry kept
serving that metadata-poor entry after the conf-driven populate
enriched the index — the fastp panel payload was missing
tool_shed_repository (test_only_latest_version_in_panel_fastp).
Swap the entry behind the existing stub instead: the panel and
registries hold the stub object, and _overrides survive.
Shed-installed data managers load their tool at install time, before
shed_data_manager_conf.xml exists — the self-healed entry carries
data_manager_id=None, materialise never restores the conf id, and
DataManagerTool.exec_after_process fails with 'Invalid data manager
(<guid>)' at job finish (all six data-manager integration tests).
DataManager._load_tool already passes data_manager_id through
load_hidden_tool; create_tool now stamps it onto the resolved entry and
persists it, so job handlers materialising from the shared index
resolve the registry correctly.
mvdbeek