test(desktop): add a leak-proof seam for the effective-owner revocation

[aryanorastar]

What

Adds a leak-proof test seam over the process-global effective-owner revocation, and makes KernelTurnRecordedProjectionTests report an inherited revocation as itself.

This does not close a leg of #12039. It builds the instrument that leg needs, and records two hypotheses that are now empirically excluded. Flagging that up front so it is reviewed as tooling, not as a fix.

Why

RuntimeOwnerIdentity.effectiveOwnerTransitionInProgress reads a private singleton:

private final class EffectiveOwnerAuthorizationRevocation: @unchecked Sendable {
  static let shared = EffectiveOwnerAuthorizationRevocation()

Production brackets it correctly — LocalMutationAuthorization calls endAuthorizationRevocation() on both the success and catch paths. But an owner transition abandoned mid-flight (a suspended, never-resumed task) leaves it active, and then:

• currentOwnerId returns nil for every caller in the process, and
• no suite can clear it, because the singleton is file-private.

So one suite's abandoned transition silently changes behavior for everything that runs after it, and surfaces somewhere unrelated.

The seam

withEffectiveOwnerTransitionForTests is scoped rather than a begin()/end() pair, so a test cannot leave the revocation active even if its body throws — the leak is unrepresentable rather than merely discouraged. It takes the caller's isolation (isolation: isolated (any Actor)? = #isolation) so a @MainActor suite can pass an isolated closure without sending a non-Sendable value.

resetEffectiveOwnerTransitionForTests clears a revocation leaked from elsewhere.

Naming follows the existing resetForTests() convention (DesktopDiagnosticsManager, PushToTalkManager). I deliberately did not gate these behind #if DEBUG: a release-mode test lane exists, and DEBUG-gating symbols referenced from tests would break it.

Leak detection

KernelTurnRecordedProjectionTests.setUp now fails with the actual cause when it inherits an active revocation, then clears it so the suite still runs. Previously that inheritance could only appear as a confusing downstream assertion in whichever test ran first.

What this rules out for #12039

Both were tested, not assumed:

• An unresolvable owner does not cause the flake. With auth_userId and automationOwnerOverride both cleared, currentOwnerId returns nil — and the reset still completes: error=nil, clearCalls=[9]. performMainChatHarnessResetTransaction installs an owner itself via withAutomationOwnerIfMissing.
• An active global revocation does not cause it either. Driving the reset inside the new seam also completes successfully.

That leaves an open question worth a maintainer's eye, which I have not pinned either way: should a harness reset proceed while owner resolution is globally revoked? It currently does. Asserting the present behavior would enshrine something that may be a defect, so this PR only reports it.

Product invariants affected

• INV-AUTH-1 (product/invariants/auth-session.md) — RuntimeOwnerIdentity.swift sits under this invariant's globs.
• INV-CHAT-1 — same file, via the chat-surface owner glob.

Neither invariant's behavior changes. The diff to that file is purely additive: two …ForTests statics that wrap begin()/end() on the existing revocation singleton. No production call site is added or altered, currentOwnerId's guard is untouched, and no signed-in/owner state is written. The seam can only be reached from test code.

Verification

• 38/38 in KernelTurnRecordedProjectionTests.
• 91/91 with SuggestedTasksStoreTests, RuntimeOwnerIdentityTests and EffectiveOwnerDatabaseBoundaryTests in one process.
• testEffectiveOwnerTransitionSeamNeverLeaksTheRevocation covers both the return and throw paths of the seam.

One production file changes, and only by addition: two test-only statics. No existing behavior is modified.

Related: #12650 isolates the Suggested-tasks feedback owner (a different leg of #12039).

Failure-Class: none

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[cubic-dev-ai]

All reported issues were addressed across 3 files

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[kodjima33]

Desktop test-tooling seam (not a fix per author's own framing — 'reviewed as tooling, not as a fix'); scores 2/5 on the bug-fix heuristic and doesn't describe end-user behavior change, so it doesn't qualify for owner-override merge. Purely additive test seam, CI green/pending-only, no abuse signals. Approve-only, staying open for a maintainer look at the open question raised (owner-resolution-revoked harness reset) rather than merging speculatively.

[Git-on-my-level]

Thanks @aryanorastar — verified the full diff against the head; this is a clean, purely additive piece of test tooling and the right scope call (build the instrument, assert nothing you aren't sure of).

RuntimeOwnerIdentity.swift: the seam reads correctly. withEffectiveOwnerTransitionForTests captures wasRevokedOnEntry before begin(), and the defer ends only what the scope started — the correct shape for a shared boolean, and exactly what the nested/inherited-leak tests pin down. resetEffectiveOwnerTransitionForTests() delegating to end() is the minimal reachable reset. I also confirmed the production call sites on the same singleton — beginAuthorizationRevocation/endAuthorizationRevocation inside performEffectiveOwnerTransition — are untouched, so currentOwnerId's guard and production transition semantics are unchanged. Not gating the helpers behind #if DEBUG is right: run-swift-ci.sh --release-notification-regression runs swift test -c release, and the existing ungated resetForTests() convention (DesktopDiagnosticsManager, PushToTalkManager) matches.

On the earlier nested-scope finding: the c75efd3 fix (restore the entry state instead of ending unconditionally) addresses it correctly, and declining the refcount was the right call — a counter would have to live on the singleton that production's begin()/end() also drives, changing production transition semantics to fix a test seam. The residual limitation (a body that itself triggers a real transition can still be un-revoked mid-body) is inherent to the shared boolean and was the right thing to note rather than engineer around.

KernelTurnRecordedProjectionTests.swift: the setUp guard is the highest-value piece of the PR — an inherited leak now fails with its actual cause instead of surfacing as an unrelated "kernel owner surface clear failed" three suites later (#12039's exact failure shape), and it clears the flag so the suite still runs. The two new tests cover the return, throw, nested, and inherited-leak paths.

Changelog: {"kind": "none"} matches the existing convention for non-user-facing entries.

Both desktop checks are still queued on this head, so treat the above as statically verified; the lane is the confirmation. Leaving the merge call for human maintainer review once those land.

---

by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with need human response.

[aryanorastar]

Pushed fe68c92bb3, which answers the open question this PR raised rather than leaving it for a maintainer.

The question: should a harness reset proceed while owner resolution is globally revoked? It currently does.

The premise does not hold. The reset never runs under a revocation — it dissolves it first. withAutomationOwnerIfMissing asks currentOwnerId(), gets nil (the revocation working exactly as designed), concludes an owner is missing, and installs one through performEffectiveOwnerTransition. That transition's endAuthorizationRevocation calls EffectiveOwnerAuthorizationRevocation.end(), clearing the outstanding flag, because the revocation is a plain boolean rather than a counter. The body therefore executes with owner resolution restored.

testAnOwnerTransitionDissolvesAnOutstandingRevocation pins that. It asserts the mechanism, not the outcome: if the revocation ever became a refcount the body would run genuinely revoked, and the assertion catches that as the behavior change it would be.

Two supporting facts, verified statically and left as comments rather than tests:

• EffectiveOwnerTransitionFence.beginTransition() sets transitionActive only when no transition is active and queues the rest on transitionWaiters, so two transitions are mutually exclusive and the boolean never nests in production. The refcount you and I both declined is genuinely unnecessary there.
• performMainChatHarnessResetTransaction is guard AppBuild.isNonProduction, so this path never runs in production at all.

So: no defect, nothing to file, and no maintainer decision needed on this point.

One test I wrote and then deleted, because it was passing for the wrong reason. I had a second test asserting that captureAuthorizationSnapshot() returns nil through a revocation — the property that makes any of this safe, since the real mutation gate is RuntimeOwnerAuthorizationAuthority rather than the boolean. It failed its own precondition twice, and the cause is worth recording: captureAuthorizationSnapshot() calls currentOwnerId() with no arguments, so allowAutomationOverride resolves from AppBuild.isNonProduction, which is false under the XCTest host. It cannot see an automation-override owner at all and returns nil in tests regardless of any revocation. Every sibling API in that file takes an explicit allowAutomationOverride for exactly this reason; this one does not.

The only ways around it are writing a synthetic auth_userId — which RuntimeOwnerIdentity's own doc comment says leaves a ghost signed-in session — or depending on the host bundle id. Neither is worth it for an assertion that would have shipped vacuous. I would rather note the gap than dress it up: captureAuthorizationSnapshot is not reachable from a hermetic test, and giving it the same allowAutomationOverride parameter its siblings have is a small change worth doing on its own.

KernelTurnRecordedProjectionTests: 40 tests, 0 failures.