test(desktop): enroll BYOK fingerprints so paywall/agent tests match #11454's contract - #12277
Merged
Merged
Conversation
…code 26 Two pre-existing blockers on `main` that fail the pre-push gate for any desktop PR when the local toolchain is newer than the pinned Xcode 16.4: - `AppState+Permissions.swift` has swift-format drift, and `desktop-swift-format-lint` runs `lint-scope` over every first-party Swift file. Formatter output only. - `AppState+Transcription.swift:848` captures `alertPresenter` implicitly in an escaping closure. Swift 6.2 (Xcode 26.x) rejects this; Xcode 16.4 accepts it. Writing `self.` is what the diagnostic asks for and is the same strong capture the implicit form already produced, so behavior is unchanged on both toolchains. Failure-Class: none
Every desktop CI lane (Static & Test Contracts, Release Compile, Build & Tests aggregate) has been red since main's d49f978 landed `var alertPresenter: any DesktopAlertPresenting = AppKitSheetAlertPresenter()`: the pinned Xcode 16.4 toolchain segfaults (signal 11) in silgen emitStoredPropertyInitialization while lowering that existential-erasure default initializer. Reproduced on main itself (d49f978, fb67ca9, 50cf064 all failed; d06e220 passed only because its desktop jobs were path-filter skipped) and on unrelated PRs (#12269, #12272), so this is not specific to this branch. Move the initializer from the stored-property default position into init(). Identical semantics on both toolchains - AppState is @mainactor with a single designated init, and the alert tests overwrite the presenter immediately after construction. Xcode 26.6 parses, swift-format lint passes, changelog gate passes. Failure-Class: none
…11454's contract #11454 replaced the old "all keys present in UserDefaults" check for isByokActive with a stricter one: the selected provider's *current* key must match a fingerprint already persisted via APIKeyService.persistEnrolledFingerprints (set by activateBYOK reconciliation after BYOKValidator confirms the key). Seven tests across BYOKPaywallTests and AgentRuntimeProcessTests still set up state the old way — raw UserDefaults keys, no enrollment — and started failing the moment CI could actually reach them (#12276): main's own contract job caught this on #11454 before merge, but a compiler crash landed 8 minutes earlier (tracked separately in #12275) blocked every real desktop Swift test run afterward, so it went unnoticed. This is a test-only fix that transcribes #11454's already-stated and already-tested enrollment contract into the tests that never learned about it; no Sources change. - BYOKPaywallTests: add `enroll(_:)`, calling `persistEnrolledFingerprints` with the SHA-256 fingerprint of the provider's current key, exactly as `activateBYOK` reconciliation would after successful validation. Five tests were asserting on raw key presence: testByokActiveRequiresSelectedLLMKey (needs a *second* enrollment after setAllBYOKKeys() rewrites openrouter's key and invalidates the first fingerprint), testBuildHeadersAttachSelectedLLMByokKey, testBuildHeadersSuppressesOnlyInvalidByokHeader, testPaywallFlagSuppressedWhenByokActive, and testRemovingDeepgramKeyLeavesSelectedLLMByokActive (both of the last two now select the provider explicitly rather than relying on legacy first-match inference, since every provider's key is set and the test must enroll the same provider it selects). - AgentRuntimeProcessTests: enroll the selected provider in testUsableByokEnvironmentIncludesAllKeysWhenAllProvidersAreUsable and testUsableByokEnvironmentSuppressesAllKeysWhenOneProviderIsKnownBad — usableBYOKEnvironment() gates on isByokActive before the CredentialHealthManager suppression these tests exercise. Both existing `defer` blocks now also save/restore the enrollment map. Not touched: testBuildHeadersCanExplicitlyExcludeByokKeys and testLowLevelTransportDefaultsToExcludingByokKeys currently pass vacuously — neither enrolls a provider, so isByokActive is false and headers come back nil regardless of includeBYOK. Flagging for whoever owns BYOK rather than fixing here, since giving them real coverage means deciding what "excluded despite being active" should assert, and that's a product call, not a mechanical transcription of #11454. No assertions were weakened or removed — every fix completes test setup to match the stated contract. testPaywallFlagSuppressedWhenByokActive is the only coverage that an enrolled BYOK user is never paywalled; it was made to pass by enrolling correctly, not by loosening what it checks. Fixes: #12276 Failure-Class: none
sujeito-operator
added a commit
to sujeito-operator/omi
that referenced
this pull request
Aug 28, 2026
…itly-escaping The production fix this class was born alongside -- writing the capture explicitly at AppState+Transcription.swift:848 -- reached main in BasedHardware#12277, so this branch rebases to the registry entry alone. evidence_prs now cites BasedHardware#12277 rather than being empty: the class has a merged PR evidencing it.
sujeito-operator
added a commit
to sujeito-operator/omi
that referenced
this pull request
Aug 28, 2026
… the caller Rebased onto main. The desktop Swift lanes that held this approved PR were red on main itself; BasedHardware#12277 fixed both causes, so this rebase is what turns them green. The checks-manifest edit is re-applied to main's text rather than overwriting it, so the legacy-memory-surface-ratchet entry main added since the merge base survives. One prose fix in the reason field: BasedHardware#12194 is still open, so it proposes the composite action rather than having moved it.
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
1 participant
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.
Summary
Test-only fix.
BYOKPaywallTestsandAgentRuntimeProcessTestshave been failing onmainsince #11454 merged (#12276) — 7 tests still set up BYOK state the old way (rawUserDefaultskeys, no enrollment) against a contract #11454 already changed and already tests correctly elsewhere:APIKeyService.isByokActivenow requires the selected provider's current key to match a fingerprint persisted viapersistEnrolledFingerprints(written byactivateBYOKreconciliation afterBYOKValidatorsucceeds), not just raw key presence.This PR transcribes that already-stated, already-merged contract into the tests that never learned about it. No
Sources/change — every fix completes test setup, none weakens or removes an assertion.BYOKPaywallTests.swift: adds a privateenroll(_:)helper that persists the fingerprint of a provider's current key, then calls it in the 5 failing tests (testByokActiveRequiresSelectedLLMKey,testBuildHeadersAttachSelectedLLMByokKey,testBuildHeadersSuppressesOnlyInvalidByokHeader,testPaywallFlagSuppressedWhenByokActive,testRemovingDeepgramKeyLeavesSelectedLLMByokActive).AgentRuntimeProcessTests.swift: enrolls the selected provider in the 2 failing tests (testUsableByokEnvironmentIncludesAllKeysWhenAllProvidersAreUsable,testUsableByokEnvironmentSuppressesAllKeysWhenOneProviderIsKnownBad); both existingdeferblocks now also save/restore the enrollment map.chore(desktop): unbreak the local Swift build and formatter gate on Xcode 26andchore(desktop): dodge the Xcode 16.4 SILGen segfault on alertPresentercommits (tracked in main's Desktop Swift CI has read green since 08-26 18:24Z while the alertPresenter SILGen crash from d49f978512 sits unfixed #12275) so this branch can actually compile and reach test execution on the CI-pinned Xcode 16.4 — otherwise this PR's own contract job would hit the same masked crash it's trying to fix visibility for.Two sharp edges (why this isn't a mechanical find-and-replace)
testByokActiveRequiresSelectedLLMKeyneeds two enrollments:setAllBYOKKeys()at line 43 (of the original file) rewrites openrouter's key to"sk-test-openrouter", which changes its fingerprint and invalidates the enrollment from line 40. Re-enroll after the rewrite.testPaywallFlagSuppressedWhenByokActiveandtestRemovingDeepgramKeyLeavesSelectedLLMByokActivenow set.byokLLMProviderexplicitly instead of relying on legacy first-match inference — with every provider's key present viasetAllBYOKKeys(), inference could silently resolve to a provider we didn't enroll.What I deliberately did not touch
testBuildHeadersCanExplicitlyExcludeByokKeysandtestLowLevelTransportDefaultsToExcludingByokKeyscurrently pass vacuously: neither enrolls a provider, soisByokActiveis already false and headers come backnilregardless ofincludeBYOK. They're not asserting what their names claim. Flagging for whoever owns BYOK rather than fixing here — giving them real coverage means deciding what "explicitly excluded while actually active" should assert, which is a product call, not a transcription of an existing contract.Context
This PR is on the critical path for #12267, #12269, #12271 (unrelated telemetry-fix PRs currently blocked by this same regression) and for
main's own CI health.Verification
git pushpre-push gate ran a full local Swift build (Build complete) plus the repo's manifest/format/lint checks — all passed.Desktop Swift Static & Test Contractsrun is the real verification for the two fixed suites; linking once green.Failure-Class: none