fix(addons): #727 follow-up fixes — pack_dir expansion, declared deps, min_lean_ctx preflight - #780
Merged
Merged
Conversation
…ersions (yvgude#727) Finding A: `addon add` / `addon update` now resolve and install `[[dependencies]]` from `AddonManifest::dependencies` on every install source (local `lean-ctx-addon.toml` included), not only the hosted `PackageManifest`. A local install of a `{pack_dir:}` addon no longer aborts with "not a declared dependency". The resolve/install helpers now take `&[PackageDependency]` + root name; the hosted `PackageManifest` no longer rides along. Finding B: `install_declared_dependencies` returns the `ResolvedDep`s it actually landed — already-satisfied deps at their locked version, freshly resolved ones at the picked version — and `provision_and_wire` expands `{pack_dir:}` against that slice instead of a separate highest-match resolve. The version burned into the wiring is the one on disk, closing the drift that could point `{pack_dir:}` at a non-existent directory. `already_satisfied` now returns the full `ResolvedDep` at the locked version. Finding C: `min_lean_ctx` contract doc gains a caveat — enforcement exists only from the release that introduced `[[dependencies]]` (3.9.x); older binaries silently ignore both the gate and the declaration. Tests: deps::already_satisfied_returns_the_locked_version_as_a_resolved_dep (Finding B, red->green via return-type change); manifest::local_manifest_dependencies_drive_pack_dir_expansion (Finding A invariant guard).
|
All contributors have signed the CLA. ✅ |
Contributor
Author
|
I have read the CLA Document and I hereby sign the CLA |
…ude#727) The four depth-1 dependency helpers this PR added to addon_cmd (addon_self_ref, resolve_declared_deps, install_declared_deps, refresh_pack_dependencies, plus their test) pushed the file to 1623 lines; the LOC gate (scripts/loc-gate.sh, limit 1500) forbids further growth of that file, so they move to a new cli/addon_deps module with no behavior change.
5 tasks
…#727) The tests compared a Path::join result against hardcoded /-separator strings and therefore only failed on Windows.
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Summary
Six follow-up fixes on top of #743 (
kind=skillscontent packs + depth-1 dependencyresolution), which is already merged. Each one closes a gap that surfaced while wiring a
real pack through the install path:
{pack_dir:}expander for[mcp.env]— pure, testable, no I/O[[dependencies]]authoring surface inlean-ctx-addon.toml, forwarded into the published packmin_lean_ctxis enforced in preflight instead of being parsed and ignored{pack_dir:}always expands to a path that exists on disk@ns/name, so it never fired)Touches only
core/addons/*,core/context_package/*,cli/addon_cmd.rs,cli/pack_remote.rsand two docs — file-disjoint from #721, so the two can land ineither order.
Refs #727.
Test plan
cd rust && cargo test— run ascargo nextest run(this repo's test runner): 8759 passedcd rust && cargo clippy --all-targets --all-features -- -D warnings— cleancd rust && cargo fmt --check— cleannpm test/ build steps — n/a, no cookbook or package changesNotes for reviewers
differ from the lockfile-pinned version that actually gets installed. The wiring always uses
the install step's output, never a second resolve.
resolve_declared_deps.root_nameinresolve_one/resolve_dependencies/install_declared_dependenciesmust be thescoped reference (
Some("@ns/name")) orNone— a bare slug silently disables theself-dependency guard. That is the regression the last commit fixes; two tests pin it.
[[dependencies]]and themin_lean_ctxgate are additive. The gate is only enforced from therelease that ships them onward; older binaries parse neither and are unaffected.
deps::already_satisfiednow returnsOption<ResolvedDep>instead ofOption<String>, anddeps::resolve_dependenciestakes(&[PackageDependency], root_name)instead of&PackageManifest. Both are internal.docs/contracts/addon-manifest-v1.md—[[dependencies]]+min_lean_ctxsemanticsdocs/guides/addons.md— authoring a pack with dependenciesContributor License Agreement
First-time contributors: a bot will ask you to sign our one-time
CLA (it keeps lean-ctx
Apache-2.0 and free for individual developers — see §8). You sign once by
replying to this PR with:
I have read the CLA Document and I hereby sign the CLA