# fix(cli): close keyring-interrupt race in Ctrl-C signal-abort

`b1c063c`→[main](/content/gh/entireio/cli/commits/main/index.html)·

Soph·2w ago·6 files·+272 added/-26 removed

PR #1604 made Ctrl-C break enclosing shell loops by re-raising the caught
signal, gating the silent-abort path on a caughtSignal atomic set by the
top-level signal handler goroutine. A Ctrl-C during a blocked keyring read
could still slip through: the keyring's own signal listener returns a wrapped
context.Canceled independently of the root-context cancellation, so the main
flow could reach the abort gate and read caughtSignal before the handler
goroutine stored it. When that happened the raw "...: context canceled" error
printed as a failure and the process exited 1 (not signal-killed), so the
loop kept respawning — the exact symptom the PR fixed.

Unify "were we signalled?" behind a single shared source of truth:

\- Add internal/procsignal: a tiny package holding the caught terminating
signal, importable by both cmd/entire and the tokenstore keyring path
(no import cycle).
\- cmd/entire/main.go: replace the local caughtSignal atomic with procsignal;
the handler and the abort gate both go through it.
\- tokenstore keyring: on the interrupt branch, record the signal via
procsignal on the *same goroutine* that unwinds to main's gate
(recordInterruptSignal), turning the cross-goroutine race into a
same-goroutine happens-before. Timeouts (DeadlineExceeded) are untouched.

Tests:
\- procsignal store/load/reset unit tests.
\- TestRecordInterruptSignal: a Ctrl-C abort records SIGINT; timeout/success
do not.
\- TestDieFromSignal_TerminatesBySignal: deterministic regression guard that
re-execs the test binary and asserts it dies *by* the signal (WIFSIGNALED,
SIGINT/SIGTERM) rather than exiting normally — the WIFSIGNALED property a
"simplify back to os.Exit(130)" would silently regress.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

## Sessions

72e0907000baView transcript

[?\
Fix CLI Ctrl-C Signal-Handling RaceClaude Code·8 steps](/content/gh/entireio/cli/session/495fc7f0-4c2e-4b95-9802-574da39da542#timeline-72e0907000ba/index.html)

## Changes

6

- cmd/entire

- Mmain.go+23/-23

- Mmain_test.go+75

- internal

- entireclient/tokenstore

- Mkeyring_timeout.go+18/-1

- Mkeyring_timeout_test.go+69/-2

- procsignal

- Aprocsignal.go+51

- Aprocsignal_test.go+36

```
7 unmodified lines
8
9
10
11
11
12
13
14
15
16
17
18
19
20
24 unmodified lines

45
46
47
48
48
49
50
51
37 unmodified lines

89
90
91
92
93
94
95
96
97
98
99
92
93
94
95
96
97
98
99
100
101
101
102
102
103
104
105
106
107
108
109
110
111
112
113
21 unmodified lines

135
136
137
133
134
135
136
137
138
139
140
139
140
141
142
143
142
143
144
145
144
145
146
147
148

7 unmodified lines

"os/signal"
	"runtime"
	"strings"
	"sync/atomic"
	"syscall"
	"time"

"github.com/entireio/cli/cmd/entire/cli"
	"github.com/entireio/cli/cmd/entire/cli/api"
	"github.com/entireio/cli/cmd/entire/cli/versioninfo"
	"github.com/entireio/cli/internal/procsignal"
	"github.com/spf13/cobra"
```

24 unmodified lines

// re-raises that same signal — a SIGTERM (from a supervisor /
		// container stop) must exit 143, not masquerade as a SIGINT 130.
		sig := <-sigChan
		caughtSignal.Store(sig)
		procsignal.Store(sig)
		if sig == os.Interrupt {
			fmt.Fprintln(os.Stderr, "\nInterrupting… press Ctrl-C again to force quit.")
		} else {
	37 unmodified lines

var silent *cli.SilentError

switch {
		case errors.Is(err, context.Canceled) && caughtSignal.Load() != nil:
			// A signal cancelled the root context (our handler fired). Don't
			// dump the raw transport/keyring cancellation string ("...:
			// context canceled", "read access token: signal: interrupt") as
			// if it were a failure — die quietly by re-raising the signal
			// that triggered it (see dieFromSignal) so an enclosing
			// `while ...; do entire; done` loop actually breaks on a single
			// Ctrl-C, and a SIGTERM shutdown still exits 143.
		case errors.Is(err, context.Canceled) && procsignal.Load() != nil:
			// A signal cancelled the root context (our handler fired) or a
			// keyring read was aborted by Ctrl-C. Don't dump the raw
			// transport/keyring cancellation string ("...: context canceled",
			// "read access token: signal: interrupt") as if it were a failure
			// — die quietly by re-raising the signal that triggered it (see
			// dieFromSignal) so an enclosing `while ...; do entire; done` loop
			// actually breaks on a single Ctrl-C, and a SIGTERM shutdown still
			// exits 143.
			//
			// We gate on the handler having fired rather than on the error
			// type alone: a context.Canceled that arose without a signal
			// We gate on a signal having been recorded rather than on the
			// error type alone: a context.Canceled that arose without a signal
			// (e.g. an internally-cancelled sub-context) is a genuine error
			// and must fall through to normal reporting, not masquerade as a
			// user abort (which would also wrongly break an enclosing loop).
			// procsignal is the shared source of truth written both by the
			// handler above and by the keyring interrupt path; the latter
			// records the signal on this same goroutine before returning, so
			// this Load never races that write.
			cancel()
			dieFromSignal(terminatingSignal())
		case errors.As(err, &silent):
21 unmodified lines

cancel() // Cleanup on successful exit
	}
}

// caughtSignal records the terminating signal (SIGINT or SIGTERM) the handler
// observed, so a later cancellation-driven exit can re-raise the *same* signal
// rather than always SIGINT. Read via terminatingSignal.
var caughtSignal atomic.Value // stores os.Signal

// terminatingSignal returns the signal that cancelled the root context,
// defaulting to SIGINT when the cancellation came from something other than
// our signal handler (so a stray context.Canceled still exits 130).
// defaulting to SIGINT when the cancellation came from something other than a
// recorded terminating signal (so a stray context.Canceled still exits 130).
// The recorded signal lives in the shared procsignal package, written by the
// handler goroutine (SIGINT/SIGTERM) and by the keyring interrupt path (SIGINT).
func terminatingSignal() os.Signal {
	if v := caughtSignal.Load(); v != nil {
		if s, ok := v.(os.Signal); ok {
			return s
		}
	}
	if s := procsignal.Load(); s != nil {
		return s
	}
	return os.Interrupt
}
```

Mcmd/entire/main.go+23/-23

```
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
1 unmodified line

38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
```

package main

import (
	"errors"
	"os"
	"os/exec"
	"runtime"
	"syscall"
	"testing"
)

// dieFromSignalEnvVar, when set on a re-executed test binary, makes TestMain
// invoke dieFromSignal with the named signal instead of running the test suite.
// The parent process then inspects how the child died. This is how
// TestDieFromSignal_TerminatesBySignal exercises real process death without
// killing the test runner itself.
const dieFromSignalEnvVar = "ENTIRE_TEST_DIE_FROM_SIGNAL"

func TestMain(m *testing.M) {
	// Child mode: exercise dieFromSignal and let it terminate this process by
	// the signal. dieFromSignal never returns on success; the os.Exit below is
	// only reached if the re-raise couldn't be delivered.
	switch os.Getenv(dieFromSignalEnvVar) {
	case "INT":
		dieFromSignal(os.Interrupt)
		os.Exit(exitCodeForSignal(os.Interrupt))
	case "TERM":
		dieFromSignal(syscall.SIGTERM)
		os.Exit(exitCodeForSignal(syscall.SIGTERM))
	}
	os.Exit(m.Run())
}

// nonNumericSignal is an os.Signal that isn't a syscall.Signal, exercising
// exitCodeForSignal's fallback branch.
type nonNumericSignal struct{}
1 unmodified line

func (nonNumericSignal) String() string { return "non-numeric" }
func (nonNumericSignal) Signal() {}

// TestDieFromSignal_TerminatesBySignal is the regression guard for the headline
// behavior: an enclosing `while true; do entire …; done` loop only breaks when
// the process is *killed by* the signal (WIFSIGNALED), not when it exits
// normally with code 130. A "simplification" of dieFromSignal back to a
// plain os.Exit(130) would leave the exit code looking right while silently
// breaking loop-escape — this test catches exactly that by re-executing the test binary
// in child mode and asserting it died by the signal.
func TestDieFromSignal_TerminatesBySignal(t *testing.T) {
	t.Parallel()
	if runtime.GOOS == "windows" {
		t.Skip("signal-to-self / WIFSIGNALED semantics do not apply on Windows")
	}

tests := []struct {
		name string
		env  string
		want syscall.Signal
	}{
		{"SIGINT", "INT", syscall.SIGINT},
		{"SIGTERM", "TERM", syscall.SIGTERM},
	}
	for _, tc := range tests {
		t.Run(tc.name, func(t *testing.T) {
			t.Parallel()

// -test.run=^$ matches no test; TestMain's child branch runs before
			// m.Run() and terminates the process, so no test actually executes.
			cmd := exec.CommandContext(t.Context(), os.Args[0], "-test.run=^$")
			cmd.Env = append(os.Environ(), dieFromSignalEnvVar+"="+tc.env)

err := cmd.Run()

var exitErr *exec.ExitError
			if !errors.As(err, &exitErr) {
				t.Fatalf("child did not exit with an error status; err=%v (expected death by signal)", err)
			}
			ws, ok := exitErr.Sys().(syscall.WaitStatus)
			if !ok {
				t.Fatalf("no syscall.WaitStatus available: %T", exitErr.Sys())
			}
			if !ws.Signaled() {
				t.Fatalf("child exited normally with code %d; want death by signal %v — dieFromSignal must re-raise, not os.Exit", ws.ExitStatus(), tc.want)
			}
			if ws.Signal() != tc.want {
				t.Fatalf("child killed by %v, want %v", ws.Signal(), tc.want)
			}
		})
	}
}

// TestExitCodeForSignal locks the conventional 128+signum mapping the
// Ctrl-C/SIGTERM fix relies on, so a future "simplification" back to a
// hardcoded 130 can't silently regress SIGTERM's 143.
```

Mcmd/entire/main_test.go+75

```
1 unmodified line

2
3
4
5
6
7
8
9
10
11
12
13
14
15
44 unmodified lines

60
61
62
60
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80

1 unmodified line

import (
	"context"
	"errors"
	"fmt"
	"os"
	"os/signal"
	"runtime"
	"time"

"github.com/entireio/cli/internal/procsignal"
)

// defaultKeyringTimeout caps how long every OS keyring call may take.
44 unmodified lines

sigCh := make(chan os.Signal, 1)
	signal.Notify(sigCh, os.Interrupt)
	defer signal.Stop(sigCh)
	return callKeyringWithInterrupt(op, keyringTimeout(), fn, sigCh)
	return recordInterruptSignal(callKeyringWithInterrupt(op, keyringTimeout(), fn, sigCh))
}

// recordInterruptSignal records the shared "we were signalled" marker when the
// keyring call was aborted by a Ctrl-C (a wrapped context.Canceled from the
// interrupt branch below). It runs on the goroutine that unwinds to the CLI's
// top-level signal-abort gate, so the store is ordered before that gate reads
// procsignal — closing the race against the asynchronous signal handler that
// also received the SIGINT. A timeout wraps context.DeadlineExceeded, not
// Canceled, so it is left untouched.
func recordInterruptSignal(val string, err error) (string, error) {
	if errors.Is(err, context.Canceled) {
		procsignal.Store(os.Interrupt)
	}
	return val, err
}

// callKeyringWithInterrupt is the testable core of callKeyringWithTimeout:
```

Minternal/entireclient/tokenstore/keyring_timeout.go+18/-1

```
2 unmodified lines

3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
40 unmodified lines

62
63
64
58
65
66
67
68
32 unmodified lines

101
102
103
97
104
105
106
107
8 unmodified lines

116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181

2 unmodified lines

import (
	"context"
	"errors"
	"fmt"
	"os"
	"strings"
	"testing"
	"time"

"github.com/entireio/cli/internal/procsignal"
)

// notReturnedSentinel is the value fn returns when the test expects the
// interrupt/timeout branch to win the select, so this value must never surface.
const notReturnedSentinel = "should not be returned"

func TestCallKeyringWithTimeout_ReturnsValueWhenFast(t *testing.T) {
	t.Parallel()

40 unmodified lines

start := time.Now()
	_, err := callKeyringWithTimeout("get", func() (string, error) {
		time.Sleep(5 * time.Second)
		return "should not be returned", nil
		return notReturnedSentinel, nil
	})
	elapsed := time.Since(start)

32 unmodified lines

_, err := callKeyringWithInterrupt("get", 10*time.Second, func() (string, error) {
		close(started)
		time.Sleep(10 * time.Second) // never completes within the test
		return "should not be returned", nil
		return notReturnedSentinel, nil
	}, interrupt)
	elapsed := time.Since(start)

8 unmodified lines

}

// recordInterruptSignal must record a shared SIGINT marker for a Ctrl-C abort
// (wrapped context.Canceled) so the CLI's signal-abort gate recognizes it
// without racing the async top-level handler — but must leave the marker
// untouched for a timeout or any non-abort error. This test mutates the
// process-global procsignal state, so it can't run in parallel.
func TestRecordInterruptSignal(t *testing.T) {
	t.Run("records SIGINT on interrupt abort", func(t *testing.T) {
		procsignal.Reset()
		t.Cleanup(procsignal.Reset)

val, err := recordInterruptSignal(callKeyringWithInterruptResult())
		if val != "" || !errors.Is(err, context.Canceled) {
			t.Fatalf("passthrough changed value/err: val=%q err=%v", val, err)
		}
		if got := procsignal.Load(); got != os.Interrupt {
			t.Fatalf("procsignal.Load() = %v, want SIGINT", got)
		}
	})

t.Run("leaves marker unset on timeout", func(t *testing.T) {
		procsignal.Reset()
		t.Cleanup(procsignal.Reset)

timeoutErr := fmt.Errorf("get timed out: %w", context.DeadlineExceeded)
		if _, err := recordInterruptSignal("", timeoutErr); !errors.Is(err, context.DeadlineExceeded) {
			t.Fatalf("passthrough changed err: %v", err)
		}
		if got := procsignal.Load(); got != nil {
			t.Fatalf("procsignal.Load() = %v, want nil (timeout is not a signal abort)", got)
		}
	})

t.Run("leaves marker unset on success", func(t *testing.T) {
		procsignal.Reset()
		t.Cleanup(procsignal.Reset)

if _, err := recordInterruptSignal("token", nil); err != nil {
			t.Fatalf("passthrough changed err: %v", err)
		}
		if got := procsignal.Load(); got != nil {
			t.Fatalf("procsignal.Load() = %v, want nil", got)
		}
	})
}

// callKeyringWithInterruptResult produces the exact (val, err) shape the
// interrupt branch returns, so the test exercises recordInterruptSignal against
// the real wrapped error rather than a hand-rolled one.
func callKeyringWithInterruptResult() (string, error) {
	interrupt := make(chan os.Signal, 1)
	interrupt <- os.Interrupt
	return callKeyringWithInterrupt("get", time.Second, func() (string, error) {
		// The pre-loaded interrupt wins the select immediately; this brief
		// sleep just keeps fn from racing it, then the goroutine exits into
		// the buffered result channel (no leak).
		time.Sleep(50 * time.Millisecond)
		return notReturnedSentinel, nil
	}, interrupt)
}

func TestKeyringTimeout_DefaultWhenUnset(t *testing.T) {
	t.Setenv(keyringTimeoutEnvVar, "")
```

Minternal/entireclient/tokenstore/keyring_timeout_test.go+69/-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

// Package procsignal records the OS signal, if any, that initiated process
// shutdown. It gives the CLI a single source of truth for "were we signalled?"
// shared by the two places that can observe a terminating signal:
//
//   - the top-level signal handler in cmd/entire, which cancels the root
//     context on SIGINT/SIGTERM, and
//   - the keyring interrupt path in internal/entireclient/tokenstore, which
//     detects Ctrl-C via its own signal.Notify listener so a stuck keyring
//     read unblocks immediately.
//
// Before this package the two mechanisms were uncoordinated: the keyring path
// returned a context.Canceled error while the top-level handler stored the
// caught signal asynchronously on a different goroutine, so the CLI's
// signal-abort gate could read the store before it was set and misreport a
// user abort as a failure (and fail to break an enclosing shell loop).
// Recording the signal here — on the same goroutine that unwinds to the gate —
// removes that race.
package procsignal

import (
	"os"
	"sync/atomic"
)

// holder wraps the stored signal so atomic.Value always observes one concrete
// type. Storing differing concrete types (or nil) into an atomic.Value panics;
// wrapping avoids both.
type holder struct{ sig os.Signal }

var caught atomic.Value // stores holder

// Store records sig as the signal that initiated shutdown. Safe for concurrent
// use; last writer wins, which is fine because every caller stores a genuine
// terminating signal.
func Store(sig os.Signal) {
	caught.Store(holder{sig: sig})
}

// Load returns the recorded terminating signal, or nil if none was recorded.
func Load() os.Signal {
	if h, ok := caught.Load().(holder); ok {
		return h.sig
	}
	return nil
}

// Reset clears the recorded signal. It exists for tests that need a clean
// slate; production code never clears it (the process is on its way out).
func Reset() {
	caught.Store(holder{})
}
```

Ainternal/procsignal/procsignal.go+51

```
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

package procsignal

import (
	"os"
	"syscall"
	"testing"
)

// These tests mutate the package-global caught signal, so they must not run in
// parallel with each other.

func TestStoreLoad(t *testing.T) {
	Reset()
	if got := Load(); got != nil {
			 t.Fatalf("Load() after Reset = %v, want nil", got)
	}

Store(os.Interrupt)
	if got := Load(); got != os.Interrupt {
		t.Fatalf("Load() = %v, want %v", got, os.Interrupt)
	}

// Storing a different concrete-typed signal must not panic and must win.
	Store(syscall.SIGTERM)
	if got := Load(); got != syscall.SIGTERM {
		t.Fatalf("Load() = %v, want SIGTERM", got)
	}
}

func TestResetClears(t *testing.T) {
	Store(os.Interrupt)
	Reset()
	if got := Load(); got != nil {
		t.Fatalf("Load() after Reset = %v, want nil", got)
	}
}
```