Consume positional source/target left-to-right across commands · Entire

Consume positional source/target left-to-right across commands

653abf2→main·

Soph·1mo ago·6 files·+85 added/-28 removed

sync/replicate/plan, bootstrap, and probe consumed positional args by fixed index (args[0]→source, args[1]→target), so --source-url URL <target> left the lone positional in args[0] and the target slot empty — sync/bootstrap/plan errored with "requires source and target"; probe failed silently, dropping the target and probing only the source.

Extract the left-to-right consumption that convert-sha256 already did correctly into a shared resolvePositionalEndpoints helper, and route all four commands through it. When a positional is left over after both slots are filled (e.g. a flag-supplied source plus two positionals), reject it rather than silently ignoring one — the fixed-index code dropped a positional here too. Add a table test covering the mixed forms and the over-specified case.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Sessions

b21841a5f646View transcript

Changes

6

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50

package main

import "testing"

// Positional endpoints must be consumed left-to-right, skipping whatever was
// already given via a flag — so `--source-url URL <target>` fills the target
// slot rather than being dropped (the reported bug across sync/bootstrap/probe).
func TestResolvePositionalEndpoints(t *testing.T) {
    const src = "https://example.invalid/source.git"
    const tgt = "https://example.invalid/target.git"

cases := []struct {
        name                   string
        source, target         string
        args                   []string
        wantSource, wantTarget string
        wantErr                bool
    }{
        {name: "both positional", args: []string{src, tgt}, wantSource: src, wantTarget: tgt},
        {name: "source flag plus positional target", source: src, args: []string{tgt}, wantSource: src, wantTarget: tgt},
        {name: "target flag plus positional source", target: tgt, args: []string{src}, wantSource: src, wantTarget: tgt},
        {name: "both flags, no positionals", source: src, target: tgt, wantSource: src, wantTarget: tgt},
        {name: "source flag only, no positionals", source: src, wantSource: src},
        // Over-specified: source set by flag, then two positionals — the old
        // fixed-index code silently ignored one; we reject instead.
        {name: "source flag plus two positionals", source: src, args: []string{"a", "b"}, wantErr: true},
        {name: "both flags plus a positional", source: src, target: tgt, args: []string{"extra"}, wantErr: true},
    }
    for _, tc := range cases {
        t.Run(tc.name, func(t *testing.T) {
            source, target := tc.source, tc.target
            err := resolvePositionalEndpoints(&source, &target, tc.args)
            if tc.wantErr {
                if err == nil {
                    t.Fatalf("expected error for over-specified args, got nil")
                }
                return
            }
            if err != nil {
                t.Fatalf("unexpected error: %v", err)
            }
            if source != tc.wantSource {
                t.Errorf("source = %q, want %q", source, tc.wantSource)
            }
            if target != tc.wantTarget {
                t.Errorf("target = %q, want %q", target, tc.wantTarget)
            }
        })
    }
}

Acmd/git-sync/args_test.go+50

29 unmodified lines

30
31
32
33
34
35
36
37
33
34
35
36
37

29 unmodified lines

RunE: func(cmd *cobra.Command, args []string) error {
        req.Protocol = gitsync.ProtocolMode(protocolVal)

if req.Source.URL == "" && len(args) > 0 {
            req.Source.URL = args[0]
        }
        if req.Target.URL == "" && len(args) > 1 {
            req.Target.URL = args[1]
        }
        if err := resolvePositionalEndpoints(&req.Source.URL, &req.Target.URL, args); err != nil {
            return err
        }

if branches != "" {

Mcmd/git-sync/bootstrap.go+2/-5

111 unmodified lines

112
113
114
115
116
117
118
119
120
115
116
117
122
123
124
125
126
127
128
118
119
120
121
122

111 unmodified lines

return cmd
}

// resolveConvertSHA256Args consumes positional args left-to-right,
// skipping fields the user already supplied via flags. Without that
// rule, `--source-url <url> <dir>` would look like one positional and
// land in SourceURL — leaving TargetDir empty even though the user
// gave both. The two-flags-no-positionals and zero-flags-two-positionals
// shapes also work, as do the symmetric --target-dir + positional URL.
// resolveConvertSHA256Args fills the source URL and target dir from flags
// and/or positional args (see resolvePositionalEndpoints) and requires both.
func resolveConvertSHA256Args(req *sha256convert.Request, args []string) error {
    positional := args
    if req.SourceURL == "" && len(positional) > 0 {
        req.SourceURL = positional[0]
        positional = positional[1:]
    }
    if req.TargetDir == "" && len(positional) > 0 {
        req.TargetDir = positional[0]
        positional = positional[1:]
    }
    if err := resolvePositionalEndpoints(&req.SourceURL, &req.TargetDir, args); err != nil {
        return err
    }
    if req.SourceURL == "" || req.TargetDir == "" {
        return errors.New("convert-sha256 requires a source URL and a target directory")

Mcmd/git-sync/convert_sha256.go+4/-13

104 unmodified lines

105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135

104 unmodified lines

return value == "1" || value == "true" || value == "yes" || value == "on"
}

// resolvePositionalEndpoints fills the source and target from positional args
// left-to-right, skipping whichever was already supplied via a flag. Consuming
// by fixed index instead (args[0]→source, args[1]→target) breaks the mixed
// form `--source-url URL <target>`: the lone positional lands in args[0] and
// the target slot stays empty.
//
// A positional left over after both slots are filled means the user
// over-specified an endpoint (e.g. `--source-url URL a b`); reject it rather
// than silently pick one. Callers still validate that both ended up set.
func resolvePositionalEndpoints(source, target *string, args []string) error {
    positional := args
    if *source == "" && len(positional) > 0 {
        *source = positional[0]
        positional = positional[1:]
    }
    if *target == "" && len(positional) > 0 {
        *target = positional[0]
        positional = positional[1:]
    }
    if len(positional) > 0 {
        return fmt.Errorf("unexpected argument %q: source and target are already set via flags or earlier positionals", positional[0])
    }
    return nil
}

func splitCSV(value string) []string {
    parts := strings.Split(value, ",")
    out := make([]string, 0, len(parts))

Mcmd/git-sync/flags.go+25

28 unmodified lines

29
30
31
32
33
34
35
36
32
33
34
35
36

28 unmodified lines

RunE: func(cmd *cobra.Command, args []string) error {
        req.Protocol = gitsync.ProtocolMode(protocolVal)

if req.Source.URL == "" && len(args) > 0 {
            req.Source.URL = args[0]
        }
        if targetURL == "" && len(args) > 1 {
            targetURL = args[1]
        }
        if err := resolvePositionalEndpoints(&req.Source.URL, &targetURL, args); err != nil {
            return err
        }
        if req.Source.URL == "" {
            return errors.New("probe requires a source repository URL")

Mcmd/git-sync/probe.go+2/-5

47 unmodified lines

48
49
50
51
52
53
54
55
51
52
53
54
55

47 unmodified lines

req.Policy.Mode = gitsync.OperationMode(modeValue)
        req.Policy.Protocol = gitsync.ProtocolMode(protocolVal)

if branches != "" {