diff --git a/.github/workflows/deployment.yml b/.github/workflows/deployment.yml index 311690c33ba..4e56fb0bf24 100644 --- a/.github/workflows/deployment.yml +++ b/.github/workflows/deployment.yml @@ -340,7 +340,7 @@ jobs: rpmsign --addsign dist/*.rpm - name: Attest release artifacts if: inputs.environment == 'production' - uses: actions/attest-build-provenance@a2bbfa25375fe432b6a289bc6b6cd05ecd0c4c32 # v4.1.0 + uses: actions/attest@59d89421af93a897026c735860bf21b6eb4f7b26 # v4.1.0 with: subject-path: "dist/gh_*" create-storage-record: false # (default: true) diff --git a/acceptance/testdata/skills/skills-install-namespaced.txtar b/acceptance/testdata/skills/skills-install-namespaced.txtar index db39bead0f3..9aa83ef5650 100644 --- a/acceptance/testdata/skills/skills-install-namespaced.txtar +++ b/acceptance/testdata/skills/skills-install-namespaced.txtar @@ -1,20 +1,21 @@ -# Two namespaced skills with the same base name in the same repo should +# Two namespaced skills with different base names in the same repo should # be independently installable using path-based disambiguation. +# Skills are installed flat (by base name) so each must have a unique name. # Use gh as a credential helper exec gh auth setup-git -# Create a repo with two namespaced skills that share the name "deploy" +# Create a repo with two namespaced skills that have unique base names exec gh repo create $ORG/$SCRIPT_NAME-$RANDOM_STRING --public --add-readme defer gh repo delete --yes $ORG/$SCRIPT_NAME-$RANDOM_STRING exec gh repo clone $ORG/$SCRIPT_NAME-$RANDOM_STRING cd $SCRIPT_NAME-$RANDOM_STRING -mkdir -p skills/alice/deploy -mkdir -p skills/bob/deploy -cp $WORK/alice-skill.md skills/alice/deploy/SKILL.md -cp $WORK/bob-skill.md skills/bob/deploy/SKILL.md +mkdir -p skills/alice/alice-deploy +mkdir -p skills/bob/bob-deploy +cp $WORK/alice-skill.md skills/alice/alice-deploy/SKILL.md +cp $WORK/bob-skill.md skills/bob/bob-deploy/SKILL.md exec git add -A exec git commit -m 'Add namespaced skills' @@ -23,25 +24,25 @@ exec git push origin main # Publish so the skills are discoverable exec gh skill publish --tag v1.0.0 -# Install alice's deploy skill using the full path to disambiguate -exec gh skill install $ORG/$SCRIPT_NAME-$RANDOM_STRING skills/alice/deploy --scope user --force -stdout 'Installed alice/deploy' +# Install alice's skill using the full path to disambiguate +exec gh skill install $ORG/$SCRIPT_NAME-$RANDOM_STRING skills/alice/alice-deploy --scope user --force +stdout 'Installed alice/alice-deploy' -# Install bob's deploy skill using the full path -exec gh skill install $ORG/$SCRIPT_NAME-$RANDOM_STRING skills/bob/deploy --scope user --force -stdout 'Installed bob/deploy' +# Install bob's skill using the full path +exec gh skill install $ORG/$SCRIPT_NAME-$RANDOM_STRING skills/bob/bob-deploy --scope user --force +stdout 'Installed bob/bob-deploy' -# Verify both were installed to separate directories -exists $HOME/.copilot/skills/alice/deploy/SKILL.md -exists $HOME/.copilot/skills/bob/deploy/SKILL.md +# Verify both were installed to flat directories (by base name) +exists $HOME/.copilot/skills/alice-deploy/SKILL.md +exists $HOME/.copilot/skills/bob-deploy/SKILL.md # Verify each has the correct content -grep 'Alice' $HOME/.copilot/skills/alice/deploy/SKILL.md -grep 'Bob' $HOME/.copilot/skills/bob/deploy/SKILL.md +grep 'Alice' $HOME/.copilot/skills/alice-deploy/SKILL.md +grep 'Bob' $HOME/.copilot/skills/bob-deploy/SKILL.md -- alice-skill.md -- --- -name: deploy +name: alice-deploy description: Alice's deployment skill --- @@ -51,7 +52,7 @@ Deploys infrastructure using Alice's conventions. -- bob-skill.md -- --- -name: deploy +name: bob-deploy description: Bob's deployment skill --- diff --git a/acceptance/testdata/skills/skills-publish-dry-run.txtar b/acceptance/testdata/skills/skills-publish-dry-run.txtar index 786204951e1..fe4d160c314 100644 --- a/acceptance/testdata/skills/skills-publish-dry-run.txtar +++ b/acceptance/testdata/skills/skills-publish-dry-run.txtar @@ -1,5 +1,6 @@ # Publish dry-run from a directory with no skills/ should fail gracefully -! exec gh skill publish --dry-run $WORK +mkdir $WORK/empty-dir +! exec gh skill publish --dry-run $WORK/empty-dir stderr 'no skills found in' # Publish dry-run against a valid skill directory should succeed @@ -10,10 +11,6 @@ stdout 'hello-world' exec gh skill publish --dry-run --tag v1.0.0 $WORK/test-repo stdout 'hello-world' -# Publish dry-run with --fix -exec gh skill publish --dry-run --fix $WORK/test-repo -stdout 'hello-world' - -- test-repo/skills/hello-world/SKILL.md -- --- name: hello-world diff --git a/acceptance/testdata/telemetry/accessibility-dimensions-disabled.txtar b/acceptance/testdata/telemetry/accessibility-dimensions-disabled.txtar new file mode 100644 index 00000000000..2a10d23da71 --- /dev/null +++ b/acceptance/testdata/telemetry/accessibility-dimensions-disabled.txtar @@ -0,0 +1,9 @@ +# Telemetry log mode records accessibility features as disabled by default +env GH_TELEMETRY=log +env GH_TELEMETRY_SAMPLE_RATE=100 + +exec gh version +stderr '"accessible_colors": "false"' +stderr '"accessible_prompter": "false"' +stderr '"color_labels": "false"' +stderr '"spinner_disabled": "false"' diff --git a/acceptance/testdata/telemetry/accessibility-dimensions.txtar b/acceptance/testdata/telemetry/accessibility-dimensions.txtar new file mode 100644 index 00000000000..9df0b524019 --- /dev/null +++ b/acceptance/testdata/telemetry/accessibility-dimensions.txtar @@ -0,0 +1,13 @@ +# Telemetry log mode records accessibility feature state as dimensions +env GH_TELEMETRY=log +env GH_TELEMETRY_SAMPLE_RATE=100 +env GH_ACCESSIBLE_COLORS=true +env GH_ACCESSIBLE_PROMPTER=true +env GH_COLOR_LABELS=true +env GH_SPINNER_DISABLED=true + +exec gh version +stderr '"accessible_colors": "true"' +stderr '"accessible_prompter": "true"' +stderr '"color_labels": "true"' +stderr '"spinner_disabled": "true"' diff --git a/docs/release-process-deep-dive.md b/docs/release-process-deep-dive.md index 31f44f6efd2..4d060841a5a 100644 --- a/docs/release-process-deep-dive.md +++ b/docs/release-process-deep-dive.md @@ -495,7 +495,7 @@ release: rpmsign --addsign dist/*.rpm - name: Attest release artifacts if: inputs.environment == 'production' - uses: actions/attest-build-provenance@520d128f165991a6c774bcb264f323e3d70747f4 # v2.2.0 + uses: actions/attest@59d89421af93a897026c735860bf21b6eb4f7b26 # v4.1.0 with: subject-path: "dist/gh_*" - name: Run createrepo diff --git a/go.mod b/go.mod index 92681afac7f..158b1fe2b1d 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/cli/cli/v2 go 1.26.0 -toolchain go1.26.2 +toolchain go1.26.3 require ( charm.land/bubbles/v2 v2.1.0 @@ -38,7 +38,7 @@ require ( github.com/in-toto/attestation v1.2.0 github.com/joho/godotenv v1.5.1 github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 - github.com/klauspost/compress v1.18.5 + github.com/klauspost/compress v1.18.6 github.com/mattn/go-colorable v0.1.14 github.com/mattn/go-isatty v0.0.22 github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d diff --git a/go.sum b/go.sum index 9980facc36e..be25979b0d9 100644 --- a/go.sum +++ b/go.sum @@ -365,8 +365,8 @@ github.com/joho/godotenv v1.5.1 h1:7eLL/+HRGLY0ldzfGMeQkb7vMd0as4CfYvUVzLqw0N0= github.com/joho/godotenv v1.5.1/go.mod h1:f4LDr5Voq0i2e/R5DDNOoa2zzDfwtkZa6DnEwAbqwq4= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51 h1:Z9n2FFNUXsshfwJMBgNA0RU6/i7WVaAegv3PtuIHPMs= github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51/go.mod h1:CzGEWj7cYgsdH8dAjBGEr58BoE7ScuLd+fwFZ44+/x8= -github.com/klauspost/compress v1.18.5 h1:/h1gH5Ce+VWNLSWqPzOVn6XBO+vJbCNGvjoaGBFW2IE= -github.com/klauspost/compress v1.18.5/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= +github.com/klauspost/compress v1.18.6 h1:2jupLlAwFm95+YDR+NwD2MEfFO9d4z4Prjl1XXDjuao= +github.com/klauspost/compress v1.18.6/go.mod h1:cwPg85FWrGar70rWktvGQj8/hthj3wpl0PGDogxkrSQ= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= diff --git a/internal/ghcmd/cmd.go b/internal/ghcmd/cmd.go index 9d4908bd868..b7e15bd5f4e 100644 --- a/internal/ghcmd/cmd.go +++ b/internal/ghcmd/cmd.go @@ -71,11 +71,15 @@ func Main() exitCode { ghExecutablePath := executablePath("gh") additionalCommonDimensions := ghtelemetry.Dimensions{ - "version": strings.TrimPrefix(buildVersion, "v"), - "is_tty": strconv.FormatBool(ioStreams.IsStdoutTTY()), - "agent": string(agents.Detect()), - "ci": strconv.FormatBool(ci.IsCI()), - "github_actions": strconv.FormatBool(ci.IsGitHubActions()), + "version": strings.TrimPrefix(buildVersion, "v"), + "is_tty": strconv.FormatBool(ioStreams.IsStdoutTTY()), + "agent": string(agents.Detect()), + "ci": strconv.FormatBool(ci.IsCI()), + "github_actions": strconv.FormatBool(ci.IsGitHubActions()), + "accessible_colors": strconv.FormatBool(ioStreams.AccessibleColorsEnabled()), + "accessible_prompter": strconv.FormatBool(ioStreams.AccessiblePrompterEnabled()), + "color_labels": strconv.FormatBool(ioStreams.ColorLabels()), + "spinner_disabled": strconv.FormatBool(ioStreams.GetSpinnerDisabled()), } var telemetryService ghtelemetry.Service diff --git a/internal/prompter/accessible_prompter_test.go b/internal/prompter/accessible_prompter_test.go index b67156bdfc3..ee6eba3a93e 100644 --- a/internal/prompter/accessible_prompter_test.go +++ b/internal/prompter/accessible_prompter_test.go @@ -1,10 +1,11 @@ -//go:build !windows +//go:build linux || darwin package prompter_test import ( "fmt" "io" + "os" "slices" "strings" "testing" @@ -17,6 +18,7 @@ import ( "github.com/hinshun/vt10x" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "golang.org/x/sys/unix" ) // The following tests are broadly testing the accessible prompter, and NOT asserting @@ -34,8 +36,6 @@ import ( // but doesn't mandate that prompts always look exactly the same. func TestAccessiblePrompter(t *testing.T) { - beforePasswordSendTimeout := 100 * time.Millisecond - t.Run("Select", func(t *testing.T) { console := newTestVirtualTerminal(t) p := newTestAccessiblePrompter(t, console) @@ -505,8 +505,8 @@ func TestAccessiblePrompter(t *testing.T) { _, err := console.ExpectString("Enter password") require.NoError(t, err) - // Wait to ensure huh has time to set the echo mode - time.Sleep(beforePasswordSendTimeout) + // Wait until huh has disabled echo mode on the TTY + require.NoError(t, waitForEchoDisabled(console.Tty(), 5*time.Second)) // Enter a number _, err = console.SendLine(dummyPassword) @@ -596,8 +596,8 @@ func TestAccessiblePrompter(t *testing.T) { _, err := console.ExpectString("Paste your authentication token:") require.NoError(t, err) - // Wait to ensure huh has time to set the echo mode - time.Sleep(beforePasswordSendTimeout) + // Wait until huh has disabled echo mode on the TTY + require.NoError(t, waitForEchoDisabled(console.Tty(), 5*time.Second)) // Enter some dummy auth token _, err = console.SendLine(dummyAuthToken) @@ -641,8 +641,8 @@ func TestAccessiblePrompter(t *testing.T) { _, err = console.ExpectString("Paste your authentication token:") require.NoError(t, err) - // Wait to ensure huh has time to set the echo mode - time.Sleep(beforePasswordSendTimeout) + // Wait until huh has disabled echo mode on the TTY + require.NoError(t, waitForEchoDisabled(console.Tty(), 5*time.Second)) // Now enter some dummy auth token to return control back to the test _, err = console.SendLine(dummyAuthTokenForAfterFailure) @@ -956,3 +956,21 @@ func testCloser(t *testing.T, closer io.Closer) { t.Errorf("Close failed: %s", err) } } + +// waitForEchoDisabled polls the TTY until echo mode is disabled or the +// timeout is reached. This is used in password and auth token tests to +// ensure that huh has configured the terminal before we send input. +func waitForEchoDisabled(tty *os.File, timeout time.Duration) error { + deadline := time.Now().Add(timeout) + for time.Now().Before(deadline) { + termios, err := unix.IoctlGetTermios(int(tty.Fd()), ioctlGetTermios) + if err != nil { + return fmt.Errorf("getting terminal attributes: %w", err) + } + if termios.Lflag&unix.ECHO == 0 { + return nil + } + time.Sleep(time.Millisecond) + } + return fmt.Errorf("timed out waiting for echo mode to be disabled") +} diff --git a/internal/prompter/echo_darwin_test.go b/internal/prompter/echo_darwin_test.go new file mode 100644 index 00000000000..2cb3130d9db --- /dev/null +++ b/internal/prompter/echo_darwin_test.go @@ -0,0 +1,7 @@ +//go:build darwin + +package prompter_test + +import "golang.org/x/sys/unix" + +const ioctlGetTermios = unix.TIOCGETA diff --git a/internal/prompter/echo_linux_test.go b/internal/prompter/echo_linux_test.go new file mode 100644 index 00000000000..ad63bd1d526 --- /dev/null +++ b/internal/prompter/echo_linux_test.go @@ -0,0 +1,7 @@ +//go:build linux + +package prompter_test + +import "golang.org/x/sys/unix" + +const ioctlGetTermios = unix.TCGETS diff --git a/pkg/cmd/copilot/copilot.go b/pkg/cmd/copilot/copilot.go index 1f2b7779858..50b00e9fe45 100644 --- a/pkg/cmd/copilot/copilot.go +++ b/pkg/cmd/copilot/copilot.go @@ -20,6 +20,7 @@ import ( "github.com/MakeNowJust/heredoc" "github.com/cli/cli/v2/internal/ci" "github.com/cli/cli/v2/internal/config" + "github.com/cli/cli/v2/internal/gh/ghtelemetry" "github.com/cli/cli/v2/internal/prompter" "github.com/cli/cli/v2/internal/safepaths" ghzip "github.com/cli/cli/v2/internal/zip" @@ -37,7 +38,7 @@ type CopilotOptions struct { Remove bool } -func NewCmdCopilot(f *cmdutil.Factory, runF func(*CopilotOptions) error) *cobra.Command { +func NewCmdCopilot(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, runF func(*CopilotOptions) error) *cobra.Command { opts := &CopilotOptions{ IO: f.IOStreams, HttpClient: f.HttpClient, @@ -80,6 +81,8 @@ func NewCmdCopilot(f *cmdutil.Factory, runF func(*CopilotOptions) error) *cobra. `), DisableFlagParsing: true, RunE: func(cmd *cobra.Command, args []string) error { + telemetry.SetSampleRate(ghtelemetry.SAMPLE_ALL) + stopParsePos := -1 for i, arg := range args { if arg == "--" { diff --git a/pkg/cmd/copilot/copilot_test.go b/pkg/cmd/copilot/copilot_test.go index e7c8fb02755..07e0191e6ba 100644 --- a/pkg/cmd/copilot/copilot_test.go +++ b/pkg/cmd/copilot/copilot_test.go @@ -14,6 +14,8 @@ import ( "runtime" "testing" + "github.com/cli/cli/v2/internal/gh/ghtelemetry" + "github.com/cli/cli/v2/internal/telemetry" "github.com/cli/cli/v2/pkg/cmdutil" "github.com/cli/cli/v2/pkg/httpmock" "github.com/cli/cli/v2/pkg/iostreams" @@ -110,7 +112,7 @@ func TestNewCmdCopilot(t *testing.T) { assert.NoError(t, err) var gotOpts *CopilotOptions - cmd := NewCmdCopilot(f, func(opts *CopilotOptions) error { + cmd := NewCmdCopilot(f, &telemetry.CommandRecorderSpy{}, func(opts *CopilotOptions) error { gotOpts = opts return nil }) @@ -586,3 +588,19 @@ func TestDownloadCopilot(t *testing.T) { require.Equal(t, localPath, path, "downloadCopilot() path mismatch") }) } + +func TestCopilotCommandIsSampledAt100(t *testing.T) { + spy := &telemetry.CommandRecorderSpy{} + factory := &cmdutil.Factory{} + cmd := NewCmdCopilot(factory, spy, func(opts *CopilotOptions) error { + return nil + }) + cmd.SetArgs([]string{}) + cmd.SetIn(&bytes.Buffer{}) + cmd.SetOut(&bytes.Buffer{}) + cmd.SetErr(&bytes.Buffer{}) + + _, err := cmd.ExecuteC() + require.NoError(t, err) + require.Equal(t, ghtelemetry.SAMPLE_ALL, spy.LastSampleRate) +} diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 9f4fa6f5bdc..4a23fc59ea4 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -152,7 +152,7 @@ func NewCmdRoot(f *cmdutil.Factory, telemetry ghtelemetry.CommandRecorder, versi cmd.AddCommand(skillsCmd.NewCmdSkills(f, telemetry)) // Root commands with standalone functionality and no subcommands - cmd.AddCommand(copilotCmd.NewCmdCopilot(f, nil)) + cmd.AddCommand(copilotCmd.NewCmdCopilot(f, telemetry, nil)) cmd.AddCommand(statusCmd.NewCmdStatus(f, nil)) cmd.AddCommand(creditsCmd.NewCmdCredits(f, nil)) cmd.AddCommand(licensesCmd.NewCmdLicenses(f))