{{ message }}
Repository navigation
Add global feature flag infrastructure - #14781
Draft
williammartin wants to merge 6 commits into
Draft
williammartin wants to merge 6 commits into
williammartin wants to merge 6 commits into
Conversation
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
Contributor
There was a problem hiding this comment.
🟡 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.
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


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/ghand configured a local server with a 1.5 second response delayWhen I ran
bin/gh --helpwithGH_GLOBAL_FEATURE_FLAG_SERVER_URLsetThen the command returned in 0.253 seconds and the detached worker populated the cache afterwards
Given the local server returned
HTTP 503When I ran
bin/gh --helpthree timesThen I saw one detached request because the remaining invocations observed the cooldown
I exercised both scenarios with:
Key points
ghprocess exits, so refresh uses a detached subprocess.GH_GLOBAL_FEATURE_FLAG_SERVER_URL, startup stays offline with every flag disabled.Notes for reviewers
I would start with
internal/globalflags, which owns the provisional contract, cache, validation, and refresh state.internal/ghcmd/global_flags.gothen shows the startup policy and isolated worker lifecycle.Authorship and follow-up
Who wrote this:
Who answers review comments: