[jcogilvie]

Description of your changes

Summary

Adds project.NewXpkgClient and uses it in project build, project run, render op, render xr, dependency add, dependency update-cache and composition generate. These commands used to build the xpkg client and resolver inline, or in render xr's case through a private helper. BuildWithMaxConcurrency now treats 0 as 1. Before this change, render xr and render op hung with --max-concurrency=0 whenever the project had embedded functions.

Why

• The commands built the same client and resolver in the same way: the cache dir defaulting to dependency.DefaultCacheDir(), the remote fetcher and optional ImageConfigs. Putting that in one place keeps them consistent.
• project build and project run clamped max(1, MaxConcurrency) themselves, but render xr and render op passed the flag through unchanged. With 0 the builder's semaphore channel is unbuffered, so every embedded function build blocks forever, which hangs whenever the project had embedded functions. Clamping inside the option fixes this for every caller.

What changed

• New internal/project/xpkg.go with NewXpkgClient(cacheDir, opts...), which returns the client and a resolver backed by it.
• project build and project run call it with the project's ImageConfigs and drop their local max(1, ...).
• render op calls it with the project's ImageConfigs.
• render xr drops newClientAndResolver and calls the helper in its place: with the project's ImageConfigs for projects, and with none for Configurations, as before.
• dependency add, dependency update-cache and composition generate call it with the project's ImageConfigs, replacing their inline client and resolver construction.
• BuildWithMaxConcurrency(0) now means 1.

Everything else stays the same: registries, ImageConfigs, cache dirs and credentials match main for every command. Error wrapping changes only in project build, project run, dependency add, dependency update-cache and composition generate: a client construction failure is now wrapped as "cannot create xpkg client", as render op and render xr already did.

Testing

• TestBuilderZeroConcurrency builds a project with BuildWithMaxConcurrency(0) and fails if the build doesn't finish within 30s. Without the clamp it times out; with the clamp it passes in about 1s.
• ./nix.sh run .#test and ./nix.sh run .#lint pass.

Related

• Split out of feat(project): add alpha project simulate on the ported crossplane-diff engine #436 at @adamwg's request. feat(project): add alpha project simulate on the ported crossplane-diff engine #436 will rebase onto this and only call the helper. Its project simulate is a further caller.
• Sibling split-outs from feat(project): add alpha project simulate on the ported crossplane-diff engine #436: fix(render): stale-backend errors, CROSSPLANE_DOCKER_NETWORK for every engine #477, refactor(simulate): move resource simulate's planning and diff server lifecycle into internal/simulate #479, feat(simulate)!: exit 3 on changes and 1 on errored plans in resource simulate #480.

I have:

• Read and followed Crossplane's contribution process.
• Run ./nix.sh flake check to ensure this PR is ready for review. (./nix.sh run .#test and ./nix.sh run .#lint pass.)
• Added or updated unit tests.
• Linked a PR or a docs tracking issue to document this change. (no docs change)
• Added backport release-x.y labels to auto-backport this PR.

Need help with this checklist? See the cheat sheet.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 57 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: crossplane/cli
• Branch: main
• File: .coderabbit.yaml
• Commit: bf5a02b
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 62f6e2f6-6748-4f53-a696-a0a74855b733

Commits

Reviewing files that changed from the base of the PR and between bf5a02b and d94b09b.

Files selected for processing: 10

• cmd/crossplane/composition/generate.go
• cmd/crossplane/dependency/add.go
• cmd/crossplane/dependency/cache.go
• cmd/crossplane/project/build.go
• cmd/crossplane/project/run.go
• cmd/crossplane/render/op/cmd.go
• cmd/crossplane/render/xr/cmd.go
• internal/project/build.go
• internal/project/build_test.go
• internal/project/xpkg.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[adamwg]

The fact that we need both a Client and a Resolver in so many places makes me wonder whether we should have an abstraction that does both. E.g., we could have a Client in internal/xpkg that extends the upstream xpkg.Client with Resolve (since the Resolver uses an xpkg.Client underneath anyway).

That said, this is an improvement as-is and we can consider further refactors later if you prefer.