refactor(kit): Expose core Kit CLI functionality as a reusable Go library #905#1097
Conversation
Signed-off-by: Arnab Chatterjee <arnabchat2001@gmail.com>
arnab2001
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6da19a0973
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| desc, err := localRepo.PullModel(ctx, repo, *opts.modelRef, &opts.NetworkOptions) | ||
| desc, err := localRepo.PullModel(ctx, repo, *opts.ModelRef, &opts.NetworkOptions) |
There was a problem hiding this comment.
The new kit.Pull API forwards opts.NetworkOptions directly into localRepo.PullModel without running NetworkOptions.Complete or otherwise enforcing a minimum concurrency, so zero-value PullOptions (a common Go usage) pass Concurrency=0. In pkg/lib/repo/local/pull.go:65-85, that becomes semaphore.NewWeighted(0) followed by Acquire(..., 1), which blocks until context cancellation and makes pulls hang/fail even for valid references; the old CLI path avoided this because flag parsing set a default and Complete validated >=1.
Useful? React with 👍 / 👎.
pkg/kit/auth.go
Outdated
| if err != nil { | ||
| return err | ||
| } | ||
| reg, err := remote.NewRegistry(opts.Registry, &opts.NetworkOptions) |
There was a problem hiding this comment.
This new library entrypoint passes opts.NetworkOptions straight to remote.NewRegistry, so callers who use zero-value LoginOptions get TLSVerify=false; pkg/lib/network/auth.go:71 then sets InsecureSkipVerify = !opts.TLSVerify, silently disabling certificate verification. The CLI previously defaulted --tls-verify to true via flag wiring, but the extracted pkg/kit API now exposes an insecure default for normal programmatic use and can allow MITM interception of registry credentials unless every caller manually overrides it.
Useful? React with 👍 / 👎.
Signed-off-by: Arnab Chatterjee <arnabchat2001@gmail.com>
Signed-off-by: Arnab Chatterjee <arnabchat2001@gmail.com>
|
Addressed both Codex P1 suggestions by adding safe default normalization in pkg/kit API entrypoints (pull/login), and re-ran targeted tests for kit/network/repo packages. |
arnab2001
changed the title
| @@ -66,17 +91,15 @@ func removeAllModels(ctx context.Context, opts *removeOptions) error { | |||
| output.Errorf("Failed to remove %s@%s: %s", repository, manifestDesc.Digest, err) | |||
| continue | |||
| } | |||
| // Skip future manifest descriptors with this digest, since we just removed it. | |||
| skipManifests[manifestDesc.Digest] = true | |||
| output.Infof("Removed %s@%s", repository, manifestDesc.Digest) | |||
| } | |||
There was a problem hiding this comment.
Why have all the comments on this file been removed?
| kitOpts := &kit.RemoveOptions{ | ||
| NetworkOptions: opts.NetworkOptions, | ||
| ConfigHome: opts.configHome, | ||
| ForceDelete: opts.forceDelete, | ||
| RemoveAll: opts.removeAll, | ||
| Remote: opts.remote, | ||
| ModelRef: opts.modelRef, | ||
| ExtraTags: opts.extraTags, |
There was a problem hiding this comment.
We're now duplicating the options for every command twice -- once in the cmd package and once in the kit package. Why not import the e.g. kit.RemoveOptions struct here and bind flags into it instead of defining it twice?
2 participants
Description
Refactors CLI command implementations into a reusable Go API under
pkg/kit.pkg/kitoperations forInfo,Inspect,List,Login,Logout,Pull,Push,Remove,Tag, andUnpackwrappers.pkg/cmd/*/cmd.go) to callpkg/kitinstead of local command-specific logic.pkg/cmd/*.pkg/kit/list_test.go.pkg/kit/doc.go,pkg/kit/README.md)..gitignoreto ignore root-only binaries (/kit,/kit.exe).go test ./...passes.Closes #905
AI-Assisted Code