Merge branch 'main' into feat/codex-skill-discovery · Entire

Merge branch 'main' into feat/codex-skill-discovery

7e6b018→main·

peyton-alt·4d ago·6 files·+130 added/-8 removed

Changes

6

322 unmodified lines

323
324
325
326
326
327
328
329
2 unmodified lines

332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394

322 unmodified lines

stdin := bufio.NewReader(strings.NewReader("\n"))

var stdout bytes.Buffer
    err := handlePush(context.Background(), ft, firstLine, &Options{}, stdin, &stdout)
    err := handlePush(context.Background(), ft, &refAdvCache{}, firstLine, &Options{}, stdin, &stdout)
    if err == nil {
        t.Fatal("expected error from send-pack exit 1")
    }
2 unmodified lines

stdout.String(), helperStatusLine)
    }
}

// TestInvariant_PushReusesListForPushAdvertisement pins the fix for ENCLI-267.
// Within one helper session the "push" command MUST reuse the ref
// advertisement fetched during "list for-push" rather than re-fetching
// info/refs. Git snapshots the remote refs from "list for-push" into its
// remote_refs list *before* running the pre-push hook, and the hook pushes
// per-checkpoint refs to the same remote. A fresh info/refs at push time then
// hands send-pack a ref (the freshly-pushed checkpoint) Git never asked to
// push; send-pack emits `error <ref> no match`, and Git — not finding it in
// remote_refs — warns `helper reported unexpected status of <ref>`. Reusing
// the list-for-push snapshot mirrors remote-curl.c's discovery cache and keeps
// that phantom ref out of send-pack's view.
func TestInvariant_PushReusesListForPushAdvertisement(t *testing.T) {
    // No t.Parallel(): t.Setenv("PATH", ...) mutates process-global state.
    if runtime.GOOS == "windows" {
        t.Skip("shell-script PATH stub is POSIX-only")
    }

ref := testRefMain
    oldSHA := strings.Repeat("a", 40)

// Stub git send-pack: emit the empty-request terminator, drain stdin,
    // then the trailing flush + a plain "ok" helper-status, exit 0.
    stubDir := t.TempDir()
    stub := "#!/bin/sh\nprintf '0000'\ncat > /dev/null\nprintf '0000ok " + ref + "\n'\nexit 0\n"
    if err := os.WriteFile(filepath.Join(stubDir, "git"), []byte(stub), 0o755); err != nil {
        t.Fatalf("writing stub git: %v", err)
    }
    t.Setenv("PATH", stubDir+string(os.PathListSeparator)+os.Getenv("PATH"))

// The checkpoint ref the pre-push hook would push between list-for-push
    // and push: absent from the first advertisement, present in the second, so
    // a re-fetch (the bug) would expose it to send-pack.
    checkpointRef := "refs/entire/checkpoints/9H/01KX2ATMJ3FAZZFZ8CP1CA279H"
    receivePackCalls := 0
    ft := &fakeTransport{
        infoRefsResp: func() (io.ReadCloser, error) {
            receivePackCalls++
            refLine := oldSHA + " " + ref + "\x00report-status object-format=sha1\n"
            if receivePackCalls == 1 {
                return stringRC(serviceAnnouncement(serviceReceivePack, refLine)), nil
            }
            return stringRC(serviceAnnouncement(serviceReceivePack, refLine,
                oldSHA+" "+checkpointRef+"\n")), nil
            },
        serviceRPCResp: func(string, []byte) (io.ReadCloser, error) {
            return stringRC(""), nil
        },
    }

stdin := strings.NewReader("list for-push\npush " + oldSHA + ":" + ref + "\n\n")
    var stdout bytes.Buffer
    if err := Run(context.Background(), ft, 2, stdin, &stdout); err != nil {
        t.Fatalf("Run: %v", err)
    }

if receivePackCalls != 1 {
        t.Fatalf("receive-pack info/refs fetched %d times; want 1 (push must reuse the list-for-push advertisement)", receivePackCalls)
    }
}

Minternal/remotehelper/githelper/invariants_test.go+61/-1

17 unmodified lines

18
19
20
21
21
22
23
24
25
26
26
27
28
29

17 unmodified lines

// writes one "<value> <name>" line per ref followed by a blank-line
// terminator. HEAD is emitted as "@<target> HEAD" when the symref
// capability resolves; detached HEAD falls back to "<sha> HEAD".
func handleList(ctx context.Context, t Transport, forPush bool, stdout io.Writer) error {
func handleList(ctx context.Context, t Transport, adv *refAdvCache, forPush bool, stdout io.Writer) error {
    service := serviceUploadPack
    if forPush {
        service = serviceReceivePack
    }
    refs, err := t.InfoRefs(ctx, service)
    refs, err := adv.infoRefs(ctx, t, service)
    if err != nil {
        return fmt.Errorf("list %s info/refs: %w", service, err)
    }
}

Minternal/remotehelper/githelper/list.go+2/-2

106 unmodified lines

107
108
109
110
110
111
112
113

106 unmodified lines

defer server.Close()

var out bytes.Buffer
        if err := handleList(context.Background(), testTransport(server), tt.forPush, &out); err != nil {
        if err := handleList(context.Background(), testTransport(server), &refAdvCache{}, tt.forPush, &out); err != nil {
            t.Fatalf("handleList: %v", err)
        }
        if out.String() != tt.want {

Minternal/remotehelper/githelper/list_test.go+1/-1

41 unmodified lines

42
43
44
45
45
46
47
48
49
50
51
51
52
53
54
55
56
57
58

41 unmodified lines

//  6. Send-pack writes a trailing flush + helper-status lines to
//     stdout; we discard the flush and relay helper-status to git,
//     then append the blank line that terminates the status batch.
func handlePush(ctx context.Context, t Transport, firstLine string, opts *Options, stdin *bufio.Reader, stdout io.Writer) error {
func handlePush(ctx context.Context, t Transport, adv *refAdvCache, firstLine string, opts *Options, stdin *bufio.Reader, stdout io.Writer) error {
    refspecs, err := readPushBatch(firstLine, stdin)
    if err != nil {
        return err
    }

refsResp, err := t.InfoRefs(ctx, serviceReceivePack)
    // Reuse the advertisement "list for-push" already fetched. Re-fetching
    // here would observe refs the pre-push hook pushed after Git's ref
    // snapshot, which send-pack reports and Git flags as "unexpected status"
    // (see refAdvCache / ENCLI-267).
    refsResp, err := adv.infoRefs(ctx, t, serviceReceivePack)
    if err != nil {
        return fmt.Errorf("fetching receive-pack info/refs: %w", err)
    }
}

Minternal/remotehelper/githelper/push.go+6/-2

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
51
52
53
54
55

package githelper

import (
    "bytes"
    "context"
    "fmt"
    "io"
)

// refAdvCache memoizes the receive-pack ref advertisement across a single
// helper session so the "push" command reuses the exact ref snapshot that
// "list for-push" fetched. This mirrors remote-curl.c's discovery cache
// (get_refs/last_refs): Git builds its remote_refs list from the
// "list for-push" advertisement, then runs the pre-push hook, then issues
// "push". The Entire pre-push hook pushes per-checkpoint refs to the same
// remote in that window, so a fresh info/refs at push time would hand
// send-pack a ref Git never asked to push. send-pack then reports
// `error <ref> no match` and Git — not finding it in remote_refs — warns
// `helper reported unexpected status of <ref>` (ENCLI-267). Reusing the
// snapshot keeps that phantom ref out of send-pack's view.
//
// Only the receive-pack (for-push) advertisement is cached; upload-pack and
// v2 fetches pass straight through, matching remote-curl's per-for_push cache.
type refAdvCache struct {
    receivePack []byte
    cached      bool
}

// infoRefs returns the ref advertisement for service. The receive-pack
// advertisement is fetched from the Transport once and replayed from an
// in-memory buffer on subsequent calls; every other service is fetched fresh.
func (c *refAdvCache) infoRefs(ctx context.Context, t Transport, service string) (io.ReadCloser, error) {
    if service != serviceReceivePack {
        rc, err := t.InfoRefs(ctx, service)
        if err != nil {
            return nil, fmt.Errorf("fetch %s advertisement: %w", service, err)
        }
        return rc, nil
    }
    if c.cached {
        return io.NopCloser(bytes.NewReader(c.receivePack)), nil
    }
    rc, err := t.InfoRefs(ctx, service)
    if err != nil {
        return nil, fmt.Errorf("fetch %s advertisement: %w", service, err)
    }
    defer rc.Close()
    buf, err := io.ReadAll(rc)
    if err != nil {
        return nil, fmt.Errorf("buffer %s advertisement: %w", service, err)
    }
    c.receivePack = buf
    c.cached = true
    return io.NopCloser(bytes.NewReader(buf)), nil
}

Ainternal/remotehelper/githelper/refadv_cache.go+55

30 unmodified lines

31
32
33
34
35
36
37
38
39
21 unmodified lines

61
62
63
61
64
65
66
67
20 unmodified lines

88
89
90
88
91
92
93
94

30 unmodified lines

func Run(ctx context.Context, t Transport, protocolVersion int, stdin io.Reader, stdout io.Writer) error {
    commandReader := bufio.NewReader(stdin)
    opts := &Options{}
    // One advertisement snapshot per session: "push" reuses what
    // "list for-push" fetched. See refAdvCache / ENCLI-267.
    adv := &refAdvCache{}

for {
        line, err := commandReader.ReadString('\n')
21 unmodified lines

fmt.Fprintln(stdout)

case line == "list" || line == "list for-push":
            if err := handleList(ctx, t, line == "list for-push", stdout); err != nil {
            if err := handleList(ctx, t, adv, line == "list for-push", stdout); err != nil {
                return err
            }

20 unmodified lines

return nil

case strings.HasPrefix(line, "push "):
            if err := handlePush(ctx, t, line, opts, commandReader, stdout); err != nil {
            if err := handlePush(ctx, t, adv, line, opts, commandReader, stdout); err != nil {
                return err
            }