Skip to content

Commit 1d8f7e9

Browse files
fentasclaude
andauthored
fix(provider): surface portable scripts, filter libs; persist interactive pick (#147)
## Summary Three connected asset-matching fixes driven by `b install github.com/arg-sh/argsh`: **1. Filter library/plugin bundles as default non-binaries.** `ignoreExtensions` now includes `.so`, `.dylib`, `.dll`, `.a`, `.lib`, `.jar`, `.aar`, `.vsix`. These are never CLI executables. When a user passes an explicit `--asset <glob>`, the filter still takes priority over this blocklist — they know what they want. **2. Portable-name fallback.** An asset whose filename equals the repo name (optionally with a script extension: `.sh`/`.py`/`.pl`/`.rb`/`.js`) is treated as a candidate even without an OS/arch marker. Scored on par with typical OS/arch matches so it ties with them and surfaces in the interactive picker. **3. Persist the interactive pick to `b.yaml`.** Previously, `b install --add github.com/arg-sh/argsh` ran the prompt once but didn't save the choice, so `b update` re-prompted (or silently auto-picked a possibly different asset). When the real interactive picker is used (TTY + not quiet + user picks), the chosen asset name is now written to `AssetFilter` → `entry.Asset` in `b.yaml`. Subsequent `b update` loads it back and `MatchAssets` narrows to that exact asset. Quiet / non-TTY auto-picks are **not** persisted because the user never saw or consented to the choice. ## Before / after Before: ``` Multiple assets match for argsh. Select one: [1] argsh-linux-amd64.so (522.7 KB) # shared library [2] argsh-linux-x64.vsix (2.4 MB) # VS Code extension [3] argsh-so-linux-amd64 (558.5 KB) # also a shared library ``` And `b install --add` did not persist the choice. After: ``` Multiple assets match for argsh. Select one: [1] argsh (34.6 KB) # the actual CLI bash script [2] argsh-so-linux-amd64 (558.5 KB) ``` With `--add`, the picked asset is written to `b.yaml` as `asset: <name>` and `b update` keeps using it. ## Test plan - [x] `TestMatchAssets_ArgshScriptIsVisible` reproduces the argsh release layout. - [x] `TestIsPortableName` covers the helper. - [x] `TestMatchAssetWithFilter` unchanged — explicit `--asset` glob still works for `.so`. - [x] `TestResolveAmbiguousAssets_Interactive_PersistsChoice` covers persistence with mocked TTY + stdin. - [x] `TestResolveAmbiguousAssets_QuietAutoPick_DoesNotPersist` ensures quiet auto-pick does not pin. - [x] `go test ./...` — all 39 packages pass. - [x] Manual end-to-end: `b install github.com/arg-sh/argsh` now installs the 35 KB bash script. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b4d8ce4 commit 1d8f7e9

4 files changed

Lines changed: 431 additions & 12 deletions

File tree

pkg/cli/resolve_test.go

Lines changed: 203 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package cli
22

33
import (
44
"fmt"
5+
"os"
6+
"path/filepath"
57
"runtime"
68
"strings"
79
"testing"
@@ -260,6 +262,207 @@ func TestResolveAmbiguousAssets_TiedScore_QuietPicksFirst(t *testing.T) {
260262
if !strings.HasPrefix(b.ResolvedAsset.Name, wantPrefix) {
261263
t.Errorf("ResolvedAsset.Name = %q, want prefix %q", b.ResolvedAsset.Name, wantPrefix)
262264
}
265+
// Quiet/non-TTY auto-picks must NOT persist — user never saw the choice.
266+
if b.AssetFilter != "" {
267+
t.Errorf("quiet auto-pick should not set AssetFilter, got %q", b.AssetFilter)
268+
}
269+
}
270+
271+
// fakeTrueTieProvider returns two assets that actually tie on score: both
272+
// have the same OS/arch, neither is an archive, neither contains the repo
273+
// name — both score 10.
274+
type fakeTrueTieProvider struct{}
275+
276+
func (f *fakeTrueTieProvider) Name() string { return "faketruetie" }
277+
func (f *fakeTrueTieProvider) Match(ref string) bool {
278+
return strings.HasPrefix(ref, "faketruetie://")
279+
}
280+
func (f *fakeTrueTieProvider) LatestVersion(ref string) (string, error) { return "v1.0.0", nil }
281+
func (f *fakeTrueTieProvider) FetchRelease(ref, version string) (*provider.Release, error) {
282+
name1 := fmt.Sprintf("pick-me-%s-%s", runtime.GOOS, runtime.GOARCH)
283+
name2 := fmt.Sprintf("or-me-%s-%s", runtime.GOOS, runtime.GOARCH)
284+
return &provider.Release{
285+
Version: version,
286+
Assets: []provider.Asset{
287+
{Name: name1, URL: "https://example.com/" + name1, Size: 1024},
288+
{Name: name2, URL: "https://example.com/" + name2, Size: 2048},
289+
},
290+
}, nil
291+
}
292+
293+
func init() {
294+
provider.Register(&fakeTrueTieProvider{})
295+
}
296+
297+
// TestResolveAmbiguousAssets_QuietAutoPick_DoesNotPersist ensures that a
298+
// quiet/non-TTY auto-pick of a genuinely tied ambiguity does NOT set
299+
// AssetFilter — the user never saw the choice so we won't pin it.
300+
func TestResolveAmbiguousAssets_QuietAutoPick_DoesNotPersist(t *testing.T) {
301+
b := &binary.Binary{
302+
Name: "tool",
303+
AutoDetect: true,
304+
ProviderRef: "faketruetie://org/unique",
305+
Version: "v1.0.0",
306+
}
307+
out := &streams.IO{Out: &discardWriter{}, ErrOut: &discardWriter{}}
308+
resolveAmbiguousAssets([]*binary.Binary{b}, true, out) // quiet=true
309+
310+
if b.ResolvedAsset == nil {
311+
t.Fatal("expected ResolvedAsset to be set (auto-pick)")
312+
}
313+
if b.AssetFilter != "" {
314+
t.Errorf("quiet auto-pick of tied candidates must not persist, got %q", b.AssetFilter)
315+
}
316+
}
317+
318+
// TestResolveAmbiguousAssets_Interactive_PersistsChoice verifies that when
319+
// the interactive picker is actually used (TTY + not quiet + user picks),
320+
// the chosen asset name is stored on AssetFilter so 'b install --add' can
321+
// persist it to b.yaml and 'b update' keeps using the same asset.
322+
func TestResolveAmbiguousAssets_Interactive_PersistsChoice(t *testing.T) {
323+
// Force the TTY check to true and stdin to a known choice.
324+
origTTY := isTTYFunc
325+
isTTYFunc = func() bool { return true }
326+
defer func() { isTTYFunc = origTTY }()
327+
328+
origStdin := os.Stdin
329+
r, w, err := os.Pipe()
330+
if err != nil {
331+
t.Fatalf("pipe: %v", err)
332+
}
333+
os.Stdin = r
334+
defer func() { os.Stdin = origStdin; r.Close() }()
335+
// Pick choice "1" (first listed).
336+
go func() {
337+
fmt.Fprintln(w, "1")
338+
w.Close()
339+
}()
340+
341+
b := &binary.Binary{
342+
// Name must not appear in assets so scoring doesn't award repo-name bonus
343+
// asymmetrically.
344+
Name: "tool",
345+
AutoDetect: true,
346+
ProviderRef: "faketruetie://org/unique",
347+
Version: "v1.0.0",
348+
}
349+
out := &streams.IO{Out: &discardWriter{}, ErrOut: &discardWriter{}}
350+
// quiet=false → interactive prompt is used
351+
resolveAmbiguousAssets([]*binary.Binary{b}, false, out)
352+
353+
if b.ResolvedAsset == nil {
354+
t.Fatal("expected ResolvedAsset to be set after interactive pick")
355+
}
356+
if b.AssetFilter == "" {
357+
t.Error("interactive pick should persist to AssetFilter for 'b install --add'")
358+
}
359+
// AssetFilter may have glob metachars escaped; the correct invariant is
360+
// that it matches the chosen asset's filename literally.
361+
if m, err := filepath.Match(b.AssetFilter, b.ResolvedAsset.Name); err != nil || !m {
362+
t.Errorf("AssetFilter=%q must match ResolvedAsset.Name=%q (matched=%v err=%v)",
363+
b.AssetFilter, b.ResolvedAsset.Name, m, err)
364+
}
365+
}
366+
367+
// TestResolveAmbiguousAssets_Interactive_EOFDoesNotPersist verifies that
368+
// when the user closes stdin without picking (EOF), the default pick is
369+
// used for this run but NOT persisted to AssetFilter.
370+
func TestResolveAmbiguousAssets_Interactive_EOFDoesNotPersist(t *testing.T) {
371+
origTTY := isTTYFunc
372+
isTTYFunc = func() bool { return true }
373+
defer func() { isTTYFunc = origTTY }()
374+
375+
origStdin := os.Stdin
376+
r, w, err := os.Pipe()
377+
if err != nil {
378+
t.Fatalf("pipe: %v", err)
379+
}
380+
os.Stdin = r
381+
defer func() { os.Stdin = origStdin; r.Close() }()
382+
w.Close() // EOF immediately — no input
383+
384+
b := &binary.Binary{
385+
Name: "tool",
386+
AutoDetect: true,
387+
ProviderRef: "faketruetie://org/unique",
388+
Version: "v1.0.0",
389+
}
390+
out := &streams.IO{Out: &discardWriter{}, ErrOut: &discardWriter{}}
391+
resolveAmbiguousAssets([]*binary.Binary{b}, false, out)
392+
393+
if b.ResolvedAsset == nil {
394+
t.Fatal("expected ResolvedAsset to be set to default on EOF")
395+
}
396+
if b.AssetFilter != "" {
397+
t.Errorf("EOF must not persist: AssetFilter = %q", b.AssetFilter)
398+
}
399+
}
400+
401+
// TestResolveAmbiguousAssets_Interactive_InvalidChoiceDoesNotPersist covers
402+
// the case where the user types a non-numeric or out-of-range value — the
403+
// default is used but not persisted.
404+
func TestResolveAmbiguousAssets_Interactive_InvalidChoiceDoesNotPersist(t *testing.T) {
405+
origTTY := isTTYFunc
406+
isTTYFunc = func() bool { return true }
407+
defer func() { isTTYFunc = origTTY }()
408+
409+
origStdin := os.Stdin
410+
r, w, err := os.Pipe()
411+
if err != nil {
412+
t.Fatalf("pipe: %v", err)
413+
}
414+
os.Stdin = r
415+
defer func() { os.Stdin = origStdin; r.Close() }()
416+
go func() {
417+
fmt.Fprintln(w, "nope")
418+
w.Close()
419+
}()
420+
421+
b := &binary.Binary{
422+
Name: "tool",
423+
AutoDetect: true,
424+
ProviderRef: "faketruetie://org/unique",
425+
Version: "v1.0.0",
426+
}
427+
out := &streams.IO{Out: &discardWriter{}, ErrOut: &discardWriter{}}
428+
resolveAmbiguousAssets([]*binary.Binary{b}, false, out)
429+
430+
if b.ResolvedAsset == nil {
431+
t.Fatal("expected ResolvedAsset to be set to default on invalid input")
432+
}
433+
if b.AssetFilter != "" {
434+
t.Errorf("invalid input must not persist: AssetFilter = %q", b.AssetFilter)
435+
}
436+
}
437+
438+
// TestEscapeAssetGlob verifies that glob metacharacters in asset filenames
439+
// are escaped so filepath.Match treats the persisted AssetFilter as a
440+
// literal name on subsequent runs.
441+
func TestEscapeAssetGlob(t *testing.T) {
442+
tests := []string{
443+
"argsh", // no metachars
444+
"argsh-so-linux", // dashes fine
445+
"foo*bar.tar.gz", // star
446+
"what?.zip", // question mark
447+
"a[tag]b", // brackets
448+
"mix*of?[all]", // mix
449+
}
450+
for _, in := range tests {
451+
pattern := escapeAssetGlob(in)
452+
matched, err := filepath.Match(pattern, in)
453+
if err != nil {
454+
t.Errorf("filepath.Match(%q, %q) error: %v", pattern, in, err)
455+
}
456+
if !matched {
457+
t.Errorf("escaped pattern %q must match its original %q", pattern, in)
458+
}
459+
// And it must not match an altered name (sanity check).
460+
if in != "x" {
461+
if m, _ := filepath.Match(pattern, "x"+in); m {
462+
t.Errorf("pattern %q unexpectedly matched %q", pattern, "x"+in)
463+
}
464+
}
465+
}
263466
}
264467

265468
// discardWriter implements io.Writer and discards all output.

pkg/cli/shared.go

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,21 @@ func resolveAmbiguousAssets(binaries []*binary.Binary, quiet bool, io *streams.I
379379
continue
380380
}
381381

382-
// Ambiguous — prompt user (or auto-pick in quiet/non-TTY mode)
382+
// Ambiguous — prompt user (or auto-pick in quiet/non-TTY mode).
383+
// We persist the chosen asset name as AssetFilter only when the
384+
// user explicitly picked it in the interactive prompt (so 'b update'
385+
// keeps the same asset). Quiet/non-TTY auto-picks, EOF, and invalid
386+
// input all fall through to a default without user consent and must
387+
// NOT be persisted.
388+
if !quiet && isTTYFunc() {
389+
asset, confirmed := promptAssetPick(b, candidates, io)
390+
b.ResolvedAsset = asset
391+
if confirmed && b.AssetFilter == "" {
392+
b.AssetFilter = escapeAssetGlob(asset.Name)
393+
}
394+
continue
395+
}
396+
// Non-interactive fallback — warn and pick the first tied candidate.
383397
sel := defaultAssetSelector(b, quiet, io)
384398
asset, err := sel(candidates)
385399
if err != nil {
@@ -389,6 +403,53 @@ func resolveAmbiguousAssets(binaries []*binary.Binary, quiet bool, io *streams.I
389403
}
390404
}
391405

406+
// promptAssetPick displays the interactive picker and returns the chosen
407+
// asset along with a flag indicating whether the user made an explicit,
408+
// valid choice. EOF or invalid input returns the default (first) candidate
409+
// with confirmed=false so callers don't persist an unconfirmed pick.
410+
func promptAssetPick(bin *binary.Binary, candidates []provider.Scored, io *streams.IO) (*provider.Asset, bool) {
411+
fmt.Fprintf(io.ErrOut, "\nMultiple assets match for %s. Select one:\n", color.New(color.Bold).Sprint(bin.Name))
412+
topScore := candidates[0].Score
413+
var choices []provider.Scored
414+
for _, c := range candidates {
415+
if c.Score == topScore {
416+
choices = append(choices, c)
417+
}
418+
}
419+
for i, c := range choices {
420+
fmt.Fprintf(io.ErrOut, " [%d] %s (%s)\n", i+1, c.Asset.Name, formatSize(c.Asset.Size))
421+
}
422+
fmt.Fprintf(io.ErrOut, "Choice [1-%d]: ", len(choices))
423+
424+
var input string
425+
if _, err := fmt.Fscanln(os.Stdin, &input); err != nil {
426+
return choices[0].Asset, false // EOF / read error — not confirmed
427+
}
428+
input = strings.TrimSpace(input)
429+
var idx int
430+
if _, err := fmt.Sscanf(input, "%d", &idx); err != nil || idx < 1 || idx > len(choices) {
431+
fmt.Fprintf(io.ErrOut, "Invalid choice, using %s\n", choices[0].Asset.Name)
432+
return choices[0].Asset, false // invalid — not confirmed
433+
}
434+
return choices[idx-1].Asset, true
435+
}
436+
437+
// escapeAssetGlob escapes glob metacharacters in an asset filename so that
438+
// later MatchAssets calls (which use filepath.Match) treat it as a literal
439+
// name. Without this, an asset whose filename contains '*', '?' or '['
440+
// could either fail to match or match unintended files on re-install.
441+
func escapeAssetGlob(name string) string {
442+
// filepath.Match recognises '*', '?' and '[...]'. A bare '[' with no
443+
// matching ']' makes the pattern invalid. Wrapping each metacharacter
444+
// in its own class turns it into a literal.
445+
r := strings.NewReplacer(
446+
"*", "[*]",
447+
"?", "[?]",
448+
"[", "[[]",
449+
)
450+
return r.Replace(name)
451+
}
452+
392453
// firstLine returns the first line of a string, trimming trailing CR/LF.
393454
// Used to display clean single-line error messages from multi-line git errors.
394455
func firstLine(s string) string {

0 commit comments

Comments
 (0)