DETACHED_PROCESS leaves the gh send-telemetry child with no console at
all. When the transitive dependency thlib/go-timezone-local invokes
`tzutil /g` to resolve the local IANA timezone, the console-subsystem
tzutil binary allocates a fresh conhost — producing a visible window
flash on every gh invocation, which accumulates as orphan terminals
under terminal configurations that keep windows open on exit.
CREATE_NO_WINDOW gives the child a non-visible console that descendants
inherit, suppressing the flash. CREATE_NEW_PROCESS_GROUP is preserved
so Ctrl+C still does not propagate to the detached telemetry child.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The sample_rate common dimension was set once at service creation
time and never updated when SetSampleRate was called later. This
caused commands like 'gh skill publish' that override the sample
rate via PersistentPreRunE to report the wrong sample_rate in
telemetry events (e.g. 1 instead of 100).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add skills specific telemetry
* Remove VisibilityFuture, inline goroutine at call sites
The VisibilityFuture/FetchRepoVisibilityAsync/Wait wrapper was an
unidiomatic async abstraction built for a single pattern used in
exactly two call sites. In Go the channel is already the future;
wrapping it in a struct with a Wait(timeout) method adds no value.
Delete the abstraction and inline a local visResult struct,
buffered channel, goroutine, and select at each call site. Behavior
is preserved exactly: err -> "unknown", timeout -> "unknown",
success+public -> include skill_names.
FetchRepoVisibility (synchronous) is kept as-is.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix nonsense copilot tests
* Update telemetry tests for public-only dims and search event removal
Production telemetry emission changed:
- preview: skill_owner/skill_repo/skill_name (renamed from skill_names)
are now emitted only when repo_visibility=public.
- install: skill_owner/skill_repo/skill_names are now emitted only
when repo_visibility=public.
- search: the initial skill_search event was removed entirely; the
skill_search_install event no longer carries query/owner dims.
Update tests to match: rename skill_names -> skill_name in preview,
make owner/repo assertions conditional on public visibility in both
preview and install, and reduce the search test to a single event
with explicit Empty assertions for the removed query/owner dims so a
privacy regression cannot pass silently.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Test CategorizeHost and switch telemetry to skill_host_type
Add TestCategorizeHost covering all four classification branches
(github.com, ghes, tenancy, uncategorized) with cases verified
against the real ghauth implementation rather than guessed.
Update install and preview unit tests to assert the new
skill_host_type dimension name, and fix a typo in the preview
acceptance txtar (skill_hos_type -> skill_host_type).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Shrink visibility wait and test unknown visibility
The 2s visibilityWaitTimeout was wildly overprovisioned: by the time
telemetry emission reaches the select, the command has already done
several serial GitHub REST calls (and for install, a git sparse-checkout
plus possibly interactive prompts), so the one-call visibility fetch
has almost always completed. Drop the timeout to 200ms — a short safety
net for a stalled REST call, not a wait budget for a healthy one.
Also adds a table-driven case to TestFetchRepoVisibility covering an
unknown/future visibility value from the API, addressing @babakks'
review nitpick.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Harden SpawnSendTelemetry against relative executable paths
- Use io.Copy for telemetry subprocess stdin write
- Clean up GH_TELEMETRY/DO_NOT_TRACK help text
- Fall back to built-in defaults (NoOp telemetry) on config load failure
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
