Add global feature flag infrastructure by williammartin · Pull Request #14781 · cli/cli · GitHub
Skip to content

Add global feature flag infrastructure - #14781

Draft
williammartin wants to merge 6 commits into
trunkfrom
williammartin-global-feature-flags
Draft

williammartin wants to merge 6 commits into
trunkfrom
williammartin-global-feature-flags

Conversation

@williammartin

@williammartin williammartin commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Description

Global feature flags need to be available before command construction, but a network lookup cannot delay every invocation. This PR adds an immutable global snapshot backed by a stale-while-revalidate cache.

The provisional unauthenticated Twirp contract is enabled only through GH_GLOBAL_FEATURE_FLAG_SERVER_URL. Startup reads the cache synchronously, then uses a detached internal worker to refresh values for a later invocation.

The worker runs before configuration, telemetry, update checks, and command construction. Failed refreshes have a one-minute cooldown, while cache writes remain atomic and endpoint-isolated.

How did you test this change?

Given I built bin/gh and configured a local server with a 1.5 second response delay

When I ran bin/gh --help with GH_GLOBAL_FEATURE_FLAG_SERVER_URL set

Then the command returned in 0.253 seconds and the detached worker populated the cache afterwards

Given the local server returned HTTP 503

When I ran bin/gh --help three times

Then I saw one detached request because the remaining invocations observed the cooldown

I exercised both scenarios with:

$ python3 /tmp/gh-global-flags-smoke.py
success: enabled=0.253s normalized_cache=true
failure: three invocations produced one detached request

Key points

  • A goroutine cannot reliably warm the cache after a short-lived gh process exits, so refresh uses a detached subprocess.
  • Flags may take effect on the next invocation. They are rollout hints, not an authorisation mechanism.
  • No production CAFE endpoint is assumed. Without GH_GLOBAL_FEATURE_FLAG_SERVER_URL, startup stays offline with every flag disabled.
  • This does not implement user-scoped flags or use GitHub accounts, tokens, repository context, or host routing.

Notes for reviewers

I would start with internal/globalflags, which owns the provisional contract, cache, validation, and refresh state. internal/ghcmd/global_flags.go then shows the startup policy and isolated worker lifecycle.

Authorship and follow-up

Who wrote this:

  • A human wrote it.
  • An agent wrote it under close human direction.
  • An agent wrote it independently, and no human has guided the implementation beyond the initial prompt.

Who answers review comments:

  • @username will read and reply directly. Name the account.
  • An agent will draft replies and @williammartin will read them before they are posted.
  • Nobody has explicitly committed to replying.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f81de340-43a3-4313-9198-e79a7ab8f6a3
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f81de340-43a3-4313-9198-e79a7ab8f6a3
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f81de340-43a3-4313-9198-e79a7ab8f6a3
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f81de340-43a3-4313-9198-e79a7ab8f6a3
Copilot AI balanced review requested due to automatic review settings October 21, 2026 10:05

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Worker output handling can interrupt detached refreshes, and client construction inherits unrelated GitHub socket configuration.

2 open issues
What changed in this PR

Adds opt-in global feature flags to gh, making immutable cached values available before command construction without waiting for network requests.

Changes:

  • Adds validated snapshots, atomic caching, and refresh cooldowns.
  • Integrates detached refresh workers and shared process-detachment helpers.
  • Adds lifecycle tests and documents the provisional API.
File Description
pkg/​cmdutil/​factory.go Exposes the global snapshot.
internal/​telemetry/​telemetry.go Uses shared detachment helpers.
internal/​telemetry/​detach_windows.go Removes telemetry-specific Windows helper.
internal/​telemetry/​detach_unix.go Removes telemetry-specific Unix helper.
internal/​globalflags/​globalflags.go Implements snapshots and fetching.
internal/​globalflags/​globalflags_test.go Tests snapshot and API behavior.
internal/​globalflags/​cache.go Implements caching and retry state.
internal/​globalflags/​cache_test.go Tests cache validation and concurrency.
internal/​ghcmd/​global_flags.go Handles startup loading and workers.
internal/​ghcmd/​global_flags_unix_test.go Tests detached worker survival.
internal/​ghcmd/​global_flags_test.go Tests startup refresh policy.
internal/​ghcmd/​global_flags_integration_test.go Tests command-construction integration.
internal/​ghcmd/​cmd.go Wires snapshots and early worker dispatch.
internal/​flock/​unsupported_windows.go Classifies unsupported Windows locking.
internal/​flock/​unsupported_windows_test.go Tests Windows lock classification.
internal/​flock/​unsupported_unix.go Classifies unsupported Unix locking.
internal/​flock/​unsupported_unix_test.go Tests Unix lock classification.
internal/​detach/​detach_windows.go Provides shared Windows detachment.
internal/​detach/​detach_unix.go Provides shared Unix detachment.
docs/​command-development.md Documents constructor access to flags.
docs/​api-and-hosts.md Documents API, cache, and worker behavior.

Review effort: Balanced

Comment on lines +24 to +25
cmd.Stdout = io.Discard
cmd.Stderr = io.Discard
Comment on lines +47 to +50
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f81de340-43a3-4313-9198-e79a7ab8f6a3
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f81de340-43a3-4313-9198-e79a7ab8f6a3

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants