feat: transitive dependency resolution and parallel fetching#1
Conversation
- Add tools/lib/resolver.py: dependency graph resolution with topological sort (Kahn's algorithm), cycle detection, and version constraint conflict checking - Packages can declare 'require' in their cleo.json for transitive deps - cleo install/require resolves full dependency graph before installing - Install order is topological (deps before dependents) - cleo remove garbage-collects orphaned transitive deps - Add --jobs/-j flag to install, require, and update for parallel git fetches via ThreadPoolExecutor - Add required_by field to lock file for dependency tracking - Add 23 tests covering toposort, constraint compatibility, transitive deps, diamond dependencies, cycle detection, parallel install, and orphan removal - Update specs (package-format.md, cleo-lock.md) and README Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
🟡 cmd_check emits false warnings for legitimate transitive deps in lock
cmd_check at tools/cleo.py:1683-1685 warns about every package in the lock file that isn't in the manifest. Since transitive dependencies are intentionally in the lock (with required_by tracking) but NOT in the manifest, cleo check will emit a spurious warning for each transitive dep. This is confusing for users who just ran cleo require and see "in lock but not in cleo.json" warnings for valid transitive deps.
(Refers to lines 1683-1685)
Was this helpful? React with 👍 or 👎 to provide feedback.
| while queue: | ||
| if jobs > 1: | ||
| batch, queue = _take_batch(queue, visited_names) | ||
| results = _fetch_batch_parallel( | ||
| batch, resolve_version_fn, resolve_commit_fn, | ||
| pkg_cache_dir_fn, clone_fn, offline, jobs, | ||
| ) | ||
| for pending, depth, result in results: | ||
| _process_resolved( | ||
| pending, depth, result, resolved, graph, constraints, | ||
| queue, pkg_cache_dir_fn, max_depth, | ||
| ) | ||
| visited_names.add(pending.name) |
There was a problem hiding this comment.
🔴 Parallel resolve path crashes with ValueError or loops infinitely when transitive deps are shared
When jobs > 1 and two packages in the same batch depend on the same transitive dependency, _process_resolved adds that dependency to the queue twice (since it checks if dep_name in resolved but the dep was only queued, not yet resolved). On a later iteration, _take_batch puts the already-visited duplicate in remainder, and the batch becomes empty while the queue remains non-empty. This causes ThreadPoolExecutor(max_workers=min(jobs, 0)) → ValueError: max_workers must be greater than 0. Even if that were guarded, the while queue loop would spin forever since no progress is made on visited-but-remaining items.
Trace with diamond dependency (A→C, B→C) and jobs=2
- Iteration 1: batch=[A, B], fetch both in parallel.
_process_resolved(A)queues C._process_resolved(B)also queues C (not yet inresolved). queue=[(C,1),(C,1)]. - Iteration 2:
_take_batchputs first C in batch, second C in remainder. C is fetched. visited={A,B,C}, queue=[(C,1)]. - Iteration 3:
_take_batch→ C is visited → batch=[], remainder=[(C,1)]._fetch_batch_parallel(batch=[])→ThreadPoolExecutor(max_workers=0)→ crash.
Prompt for agents
In the parallel branch of the while loop in resolve_all (tools/lib/resolver.py, lines 173-185), two issues must be fixed:
1. When _take_batch returns an empty batch (all queue items are already visited), the code must handle the visited duplicates (recording their required_by and constraint info, like the sequential branch does at lines 188-192) and then break or drain them from the queue. Currently it calls _fetch_batch_parallel with an empty batch, which crashes with ThreadPoolExecutor(max_workers=0).
2. The fundamental cause is that _process_resolved at line 357 checks if dep_name in resolved but a dependency may be queued-but-not-yet-resolved when two packages in the same parallel batch share a dep. Both add it to the queue. The fix should either deduplicate the queue after processing a batch, or handle the empty-batch case by draining visited entries from the queue (updating required_by/constraints) before continuing the while loop.
A minimal fix would be: after _take_batch, if batch is empty, iterate remainder items to record their required_by and constraints (like the sequential branch does), then break out of the while loop.
Was this helpful? React with 👍 or 👎 to provide feedback.
| restored += 1 | ||
| continue | ||
|
|
||
| # No lock or package not yet in lock → resolve from constraint. | ||
| try: | ||
| url = _resolve_url(manifest, pkg_name, None) | ||
| except SystemExit as exc: | ||
| err(str(exc).replace(f"{TAG} ", "")) | ||
| continue | ||
|
|
||
| needs_resolve.append((pkg_name, constraint, url, bucket)) | ||
|
|
||
| # Resolve transitive dependencies for unlocked packages. | ||
| if needs_resolve and not args.dry_run and not args.offline: | ||
| try: | ||
| resolved_pkgs = resolve_all( | ||
| needs_resolve, | ||
| pkg_cache_dir_fn=_pkg_cache_dir, | ||
| clone_fn=_clone_or_fetch, | ||
| resolve_version_fn=resolve_version, | ||
| resolve_commit_fn=resolve_commit, | ||
| offline=args.offline, | ||
| jobs=jobs, | ||
| ) | ||
| except DependencyCycle as exc: | ||
| err(str(exc)) | ||
| return 1 | ||
| except VersionConflict as exc: | ||
| err(str(exc)) | ||
| return 1 | ||
|
|
||
| # Install in topological order (deps first). | ||
| for rpkg in resolved_pkgs: | ||
| if rpkg.name in new_lock: | ||
| continue # already handled (locked/restored) | ||
| result = install_package( | ||
| project, rpkg.name, rpkg.url, rpkg.constraint, rpkg.bucket, | ||
| locked_version=rpkg.version, | ||
| locked_commit=rpkg.commit, | ||
| install_mode=cli_install_mode or "copy", | ||
| dry_run=args.dry_run, | ||
| offline=args.offline, | ||
| quiet=args.quiet, | ||
| ) | ||
| if result is None: | ||
| continue | ||
| result.required_by = rpkg.required_by | ||
| new_lock[rpkg.name] = result | ||
| installed += 1 | ||
| else: | ||
| # Dry-run / offline: install top-level only (no transitive resolution). | ||
| for pkg_name, constraint, url, bucket in needs_resolve: | ||
| result = install_package( | ||
| project, pkg_name, url, constraint, bucket, | ||
| install_mode=cli_install_mode or "copy", |
There was a problem hiding this comment.
🔴 cmd_install silently drops transitive deps from lock file on re-install
cmd_install only iterates packages present in the manifest's require/require-local/require-user sections (line 1153-1154). Transitive dependencies that were recorded in cleo.lock (with required_by set) are never added to new_lock because they aren't in the manifest. When save_lock(project, new_lock) runs at line 1246, the lock file is overwritten without those transitive deps. This means a simple cleo install after a cleo require that pulled in transitive deps will silently erase all transitive dependency records from the lock, leaving their files on disk but untracked.
Reproduction scenario
cleo require Awhere A depends on B → lock has both A and B (B with required_by=["A"])cleo install→ iterates manifest (only A) → A is locked and present → skipped into new_lock → B is never processed → save_lock writes only A → B is gone from lock
(Refers to lines 1148-1246)
Prompt for agents
In cmd_install (tools/cleo.py), the new_lock dict is populated only from packages listed in the manifest. Transitive dependencies that are in the existing lock (with required_by set) but not in the manifest are silently dropped.
The fix should carry over locked transitive deps into new_lock. After the main loop that iterates manifest entries (around line 1191, after the for loop over buckets_to_install), add a pass that copies locked transitive deps into new_lock:
For each package in the existing lock that has a non-empty required_by list and is not already in new_lock, check whether at least one of its requirers is in new_lock. If so, carry it over to new_lock. This should be done iteratively (like the orphan removal loop in cmd_remove) since transitive deps can themselves have transitive deps.
This preserves the lock-strict semantics: if a transitive dep is already locked and its parent is still present, it should be preserved.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if not still_needed and not in_manifest: | ||
| # Remove orphan's files. | ||
| for item in dep_pkg.items: | ||
| if item.path: | ||
| p = Path(item.path) | ||
| if p.exists(): | ||
| if p.is_dir() and not p.is_symlink(): | ||
| shutil.rmtree(p) | ||
| else: | ||
| p.unlink() | ||
| del new_lock[dep_name] | ||
| removed += 1 | ||
| orphan_pass = True | ||
| if not args.quiet: | ||
| ok(f"removed orphan {dep_name}") |
There was a problem hiding this comment.
🟡 Orphan removal in cmd_remove skips hooks directory, settings.json hook registrations, and MCP server cleanup
The orphan removal loop at tools/cleo.py:1770-1799 only removes item files/dirs from disk. It does not clean up hook directories (tools/cleo.py:1728-1731), MCP server entries from settings.json (tools/cleo.py:1734-1743), or hook registrations from settings.json (tools/cleo.py:1745-1754) — all of which the main removal loop handles for directly-removed packages. If an orphaned transitive dep had hooks or MCP servers, those entries persist after removal.
Prompt for agents
In cmd_remove (tools/cleo.py), the orphan removal block at lines 1785-1799 only removes artifact files. It should also:
1. Remove the hook directory: project / '.claude' / 'hooks' / f'cleo-{dep_name.replace("/", "-")}'
2. Remove MCP server entries from settings.json if dep_pkg.mcp_server_key is set
3. Remove hook registrations from settings.json (keys starting with f'cleo-{safe_pkg}-')
This cleanup mirrors the main removal loop at lines 1726-1754. Consider extracting the cleanup logic into a helper function to avoid duplication.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Two new features:
1. Transitive dependency resolution — Packages can now declare
requirein their owncleo.json. When youcleo require acme/full-stackand that package depends onacme/rulesandacme/agents, cleo resolves the full dependency graph and installs everything in topological order (deps before dependents).tools/lib/resolver.pywith Kahn's algorithm for topological sortDependencyCycle)dep ^1.0, B needsdep ^2.0→ error before any files are written)cleo removegarbage-collects orphaned transitive depsrequired_byfield in lock file tracks why each package was installed2. Parallel fetching —
--jobs/-jflag oninstall,require, andupdateruns git clones/fetches in parallel viaThreadPoolExecutor. File materialization stays sequential to avoid race conditions.Review & Testing Checklist for Human
cleo require A— verify both appear incleo.lockwith correctrequired_bycleo require Afails with "cycle" errorcleo remove Ashould also remove Bcleo install -j 4on a project with 3+ packages — verify all install correctlyrequire)Test plan: Run
pytest tests/test_resolver.py -vto see all 23 new tests pass. Then try the transitive dep flow end-to-end with real packages.Notes
repositoriesin package-levelcleo.jsonfor deps not on GitHub (same format as consumer manifests)spec/package-format.md(newrequire/repositoriesfields),spec/cleo-lock.md(newrequired_byfield)Link to Devin session: https://app.devin.ai/sessions/e54a08fd22d14a329f30e5f93f74009a
Requested by: @Surt