Skip to content

fix: activation was never sent on the launch that shows the disclosure - #418

Merged
JasonYeYuhe merged 2 commits into
mainfrom
fix-activation-latch
Aug 7, 2026
Merged

fix: activation was never sent on the launch that shows the disclosure#418
JasonYeYuhe merged 2 commits into
mainfrom
fix-activation-latch

Conversation

@JasonYeYuhe

Copy link
Copy Markdown
Collaborator

Why this is urgent

The v1.45 activation counter could not fire on a first launch. It is the counter the whole release exists to provide, and it would have made the first read of the funnel wrong — in the pessimistic direction, which is the reading that would have sent us rebuilding onboarding.

Found while doing the P0 from the v1.46 plan: prove the counter before reading any number.

What happened

AnonymousTelemetryCoordinator subscribes to ProviderState.$providers at launch. On the first non-empty emission it called recordFirstProviderDetectedIfNeeded() — correctly refused, because maySend requires hasSeenDisclosure and the disclosure card lives inside the lazily-built MenuBarExtra. Nothing was sent; the persisted activationReported correctly stayed false.

But the in-process latch activationSent was set on entry, before the call. It recorded that the coordinator had tried, not that it had succeeded:

guard !activationSent else { return }
activationSent = true                                    // <- set against a refusal
Task { await telemetry.recordFirstProviderDetectedIfNeeded() }

So when the user opened the menu and tapped "Got it", disclosureAcknowledged called back into reportActivation() and returned at the stale latch. The Combine sink could not rescue it either — removeDuplicates() stays latched on true while the list stays non-empty.

Activation therefore slipped to a later process launch, and was lost entirely for anyone who uninstalled before one. For an always-running menu bar app that can be days.

It hits the v1.44 → v1.45 upgrade wave hardest — their providers are already detected at launch, hours before they ever open the menu — which is exactly the population dominating the first days of v1.45 data. disclosureAcknowledged's documented purpose ("if a provider was already found while the card was up, that counts too") was dead code in the scenario it was written for.

The fix

recordInstallIfNeeded / recordFirstProviderDetectedIfNeeded now return whether the fact is durably recorded, and the coordinator latches on that result instead of on the attempt. A refusal stays retryable within the same launch.

Why it shipped — and why that is also fixed

The coordinator lived in the app target, which has no test bundle. Nothing could reach it, so ProviderPublisherSubscribeSemanticsTests re-implemented its Combine chain and tested the copy. The copy was faithful and passed — the defect was in the latch around the chain, which the copy did not have. Testing a replica cannot find that class of bug.

So the coordinator moves into CLIPulseCore (it already depended on nothing else) and AnonymousTelemetryCoordinatorTests drives the real object: real subscription, real latch, real actor. 4 pbxproj references removed; xcodebuild -configuration Release confirms the app still links it.

Verification

  • Negative control — the new guard was proven to fire. Reverting only the latch makes test_activationIsSentOnTheLaunchTheDisclosureIsAcknowledged fail 0 != 1, reproducing the shipped bug.
  • Runtime, on the notarized DEVID 1.45.0 DMG installed on a Mac with real CLI history, against production: disclosure gate holds (nothing sent pre-acknowledgement), then install and first_provider_detected both land — channel=devid, app_version=1.45.0. This is the P0 deliverable: the counter is alive.
  • swift test --package-path "CLI Pulse Bar/CLIPulseCore" — 2662 passed, 0 failures.
  • xcodebuild -scheme "CLI Pulse Bar" -configuration Release — BUILD SUCCEEDED.
  • ci_check_rpc_contract.py and check_migration_numbers.sh — pass. No schema change; no payload shape change.

Also in here

Corrects a comment in the sibling test claiming the second activation edge is "guarded downstream by the persisted activation flag". It is not — the in-process latch is hit first and the actor is never reached. That belief was load-bearing and wrong.

Not in here

Two smaller findings from the same audit, deliberately left for separate PRs so this one stays reviewable: the disclosure card's toggle reads "on" while privacy.localOnlyMode is silently forcing telemetry off, and the RPC's 20k/day ceiling returns 204 so the client marks the event reported forever.

🤖 Generated with Claude Code

JasonYeYuhe and others added 2 commits August 7, 2026 17:57
The v1.45 activation counter could not fire on a first launch. It is the
counter the whole release exists to provide, and it would have made the
first read of the funnel wrong in the pessimistic direction.

WHAT HAPPENED

`AnonymousTelemetryCoordinator` subscribes to `ProviderState.$providers` at
launch. On the first non-empty emission it called
`recordFirstProviderDetectedIfNeeded()` — which is correctly refused, because
`maySend` requires `hasSeenDisclosure` and the disclosure card lives inside
the lazily-built `MenuBarExtra`. Nothing was sent; the persisted
`activationReported` correctly stayed false.

But the in-process latch `activationSent` was set on entry, before the call.
It recorded that the coordinator had TRIED, not that it had SUCCEEDED. So
when the user opened the menu and tapped "Got it", `disclosureAcknowledged`
called back into `reportActivation()` and returned at the stale latch. The
Combine sink could not rescue it either: `removeDuplicates()` stays latched
on `true` while the list stays non-empty.

Activation therefore slipped to a later process launch, and was lost entirely
for anyone who uninstalled before one. That hits the v1.44 -> v1.45 upgrade
wave hardest — their providers are already detected at launch, hours before
they ever open the menu — which is exactly the population dominating the
first days of data. `disclosureAcknowledged`'s documented purpose ("if a
provider was already found while the card was up, that counts too") was dead
code in the scenario it was written for.

THE FIX

`recordInstallIfNeeded` / `recordFirstProviderDetectedIfNeeded` now return
whether the fact is durably recorded, and the coordinator latches on that
result instead of on the attempt. A refusal stays retryable within the same
launch.

WHY IT SHIPPED, AND WHY THAT IS ALSO FIXED

The coordinator lived in the app target, which has no test bundle. Nothing
could reach it, so `ProviderPublisherSubscribeSemanticsTests` re-implemented
its Combine chain and tested the copy. The copy was faithful and passed —
the defect was in the latch around the chain, which the copy did not have.
Testing a replica cannot find that class of bug.

So the coordinator moves into CLIPulseCore (it already depended on nothing
else) and `AnonymousTelemetryCoordinatorTests` drives the real object: real
subscription, real latch, real actor. Verified as a guard that actually
fires — reverting the latch makes
`test_activationIsSentOnTheLaunchTheDisclosureIsAcknowledged` fail 0 != 1.

Also corrects a comment in the sibling test claiming the second activation
edge is "guarded downstream by the persisted activation flag". It is not:
the in-process latch is hit first and the actor is never reached. That belief
was load-bearing and wrong.

Verified end to end beforehand on the notarized DEVID 1.45.0 DMG against
production: install and activation both land, channel `devid`.

swift test: 2662 passed, 0 failures. xcodebuild Release: BUILD SUCCEEDED.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adversarial review mutation-tested this branch's own tests and found two of
them weaker than their names promised. All three fixes below are to the
tests and docs; the production fix is unchanged.

1. DELETED test_latchPolicyMustCacheTheOutcomeNotTheAttempt. It
   re-implemented BOTH latch policies inside the test body and asserted they
   disagreed, so it passed with the production fix fully reverted while its
   docstring claimed to pin production. A test that constrains only itself is
   worse than none, because it reads like coverage. Reverting the latch still
   fails 4 of the 4 AnonymousTelemetryCoordinatorTests, which is the real pin.

2. The headline regression test passed with the Combine subscription gutted
   to `_ = hasProviders`. "Nothing was sent" is satisfied both by the gate
   refusing a delivered emission and by the sink never delivering at all, and
   the test is about the first. The stub store now counts entries into the
   activation path, and the test asserts the sink actually delivered before
   concluding the gate refused it. Verified: with the sink gutted it now fails
   "the Combine sink never delivered; this test would then prove nothing".

3. Re-scoped test_activationIsSentOnlyOnce's docstring. It stays green with
   the in-process latch deleted outright, because the persisted flag alone is
   sufficient once a send has succeeded — so it pins the post-record
   invariant, not the latch, and not the in-flight window. Says so now.

Also corrects docs/ASC_APP_PRIVACY_v1.45.md. It argued iOS collects nothing
partly from "AnonymousTelemetryCoordinator exists in the Mac app" — which this
branch falsified by moving the type into CLIPulseCore, where it is compiled
into the iOS and Watch binaries. The conclusion is unchanged and still correct,
because nothing on those platforms constructs it, but the load-bearing fact is
now "nothing constructs it" rather than "it is absent from the binary". An App
Store privacy document may not rest on a premise that stopped being true.

Known and accepted: latching on the outcome means a re-entrant
reportActivation() during an in-flight send can emit a second activation POST.
Confirmed by review at artificial latency. It is server-idempotent —
install_id is the primary key and first_provider_detected_at is coalesced, so
the duplicate changes no stored value. The obvious in-flight guard was
explicitly rejected: setting a flag before the Task reintroduces a micro-window
of the v1.45 bug, where a "Got it" tap landing during a refused send is
suppressed and, with removeDuplicates latched, never retried.

swift test: 2661 passed, 0 failures.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@JasonYeYuhe
JasonYeYuhe merged commit db9c8e5 into main Aug 7, 2026
30 of 32 checks passed
@JasonYeYuhe
JasonYeYuhe deleted the fix-activation-latch branch August 7, 2026 10:29
JasonYeYuhe added a commit that referenced this pull request Aug 7, 2026
Two fixes to the v1.45 anonymous-install telemetry, both merged already:

  #418  activation was never sent on the launch that shows the disclosure
        card — the coordinator's latch recorded having TRIED rather than
        having SUCCEEDED, so the "Got it" tap hit a stale guard. It hit
        upgrading users hardest, which is the cohort dominating v1.45's
        first days, and biased the funnel pessimistically.

  #419  the disclosure card told local-only users, in the present tense,
        that statistics were being sent. They were not — localOnlyMode
        already forces telemetry off inside the store. Settings said so;
        the card did not.

DEVID + Homebrew only. No App Store submission is prepared here, for two
reasons: iOS 1.45.0 is still WAITING_FOR_REVIEW and a second version cannot
sit in review beside it, and release-surface changes are the owner's call
(feedback_appstore_update). macOS 1.45.0 is already READY_FOR_SALE, so MAS
users keep the v1.45 behaviour until a separate submission is made.

Android versionName/versionCode bumped in step with Apple to satisfy the
v1.21 G6 drift gate; no Android release is implied.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
JasonYeYuhe added a commit that referenced this pull request Aug 7, 2026
…ex (#423)

On 2026-08-07 one `git add -A` staged 176 untracked files and pushed them to
this PUBLIC repo (#421, closed). scripts/check_no_internal_docs.sh caught it,
but the branch had already been pushed and the commit is still reachable
through GitHub's PR refs. The gate worked; nothing upstream of it did.

Two holes let it happen, and neither was closed by the closing of that PR.

The 2026-07-31 ignore list carries the note "root-anchored so docs/ content is
unaffected". Of the 141 internal documents in that commit, 50 were not at the
root — 49 under `docs/`, plus `MachineRootHelper/SHIP_RUNBOOK.md` — so the
anchoring itself was the hole. Worse, `PROJECT_FIX_*` had no entry at all,
and it was 91 of the remaining 141: the largest class, and the one the working
habit still produces. The patterns are now un-anchored and mirror LAYER 1 of
check_no_internal_docs.sh, so the ignore file and the gate block the same set
instead of each covering what the other misses.

The other 21 files were iCloud Drive collision copies — " 2", " 3" appended
silently on conflict. Among them `scripts/check_no_internal_docs 2.sh` (a
stale copy of the gate itself) and two duplicates of
migrate_v0.72_provider_accounts. That last one is the dangerous shape:
migrations are matched by number and are the only record of what has run
against production, so a second v0.72 turns "did v0.72 run?" into "which
v0.72?".

Verified the new patterns mask nothing: zero tracked files match, checked with
`git ls-files | xargs git check-ignore`. A future real `Figure 2.png` fails
loudly at `git add` rather than being dropped silently, and `git add -f`
remains the override.

Also removed from the working tree, separately from this commit: 14 untracked
duplicates, each diffed against its canonical sibling first. Thirteen were
byte-identical; `doctor 2.sh` was a strict subset of the tracked doctor.sh
(the pre-#421 wording), and AnonymousTelemetryCoordinator.swift was the
pre-#418 copy left at the old app-target path, superseded by the CLIPulseCore
original and referenced nowhere in project.pbxproj.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
JasonYeYuhe added a commit that referenced this pull request Aug 7, 2026
Ships the helper-agent self-heal (#425), which missed the 1.46.0 cut by three
commits. That is the most user-visible fix of the day and 1.46.0 does not have
it: HelperAgentHealth.swift is on main but absent from the published 1.46.0 DMG.

WHY IT MATTERS MORE THAN THE TELEMETRY FIXES IT TRAVELS WITH

`yyh.CLI-Pulse.helper.agent` can be registered and still have never executed.
Measured on a real machine: 22,138 consecutive failed spawns over ~11 days,
while every signal the app used said healthy — SMAppService `.status` returned
`.enabled`, `register()` returned without throwing, and `launchctl kickstart`
exited 0 on a job that failed to exec milliseconds later.

A dead helper means no provider data, which means CLI Pulse shows nothing at
all. That is the first-value failure the v1.45 telemetry was built to measure,
happening silently, to users who then have no reason to come back.

ALSO IN THIS RELEASE (both already shipped in 1.46.0, restated for users who
skip it): the disclosure card no longer claims statistics are being sent while
local-only mode suppresses them (#419), and activation is no longer lost on the
launch that shows the disclosure card (#418).

DEVID + Homebrew only. iOS 1.45.0 is still WAITING_FOR_REVIEW and a second
version cannot sit in review beside it; App Store submission stays the owner's
call.

swift test: 2678 passed, 0 failures, 4 skipped — including 27 helper-agent
tests. Version drift gate passes (Apple 1.47.0 == Android 1.47.0, code 66).

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
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.

1 participant