Skip to content

test(provider): skip the keychain round-trip when the login keychain is locked - #58

Merged
m-szymanska merged 1 commit into
mainfrom
fix/pensieve-keychain-test-headless-skip
Aug 16, 2026
Merged

test(provider): skip the keychain round-trip when the login keychain is locked#58
m-szymanska merged 1 commit into
mainfrom
fix/pensieve-keychain-test-headless-skip

Conversation

@m-szymanska

Copy link
Copy Markdown
Member

Why

make release fails in make gates on ProviderSettingsTests.testKeychainStoreRoundTripsWithoutEntitlements whenever the release is driven from a non-interactive session (SSH into the build host). This blocked the 0.4.4 release cut.

The failure is an environment quirk, not a product defect — the same SHA is green on CI:

Pensieve/Sources/Pensieve/App/ProviderSettings.swift:146: error:
  -[PensieveTests.ProviderSettingsTests testKeychainStoreRoundTripsWithoutEntitlements]
  : failed: caught error: "keychain(-25308)"

-25308 is errSecInteractionNotAllowed. The test deliberately performs a real round-trip through the file-based login keychain — that is precisely the contract it guards (a SecAccessControl variant would route the item to the data-protection keychain, which Developer ID builds cannot touch, failing with errSecMissingEntitlement / -34018). In an SSH session the login keychain is locked and Security cannot raise the unlock panel, so SecItemAdd cannot succeed by any means available to the test.

What changed

Test-only. No production code is touched.

  • throw XCTSkip(...) for exactly one status: errSecInteractionNotAllowed (-25308), with an actionable message — "login keychain locked in a non-interactive session — run from a GUI session or CI". This follows the existing XCTSkip idiom in the suite (PreviewThemeTests, WindowChromeRecipeTests, EditorToolbarOverflowTests).
  • Every other failure still fails, in particular -34018 — the regression this test exists to catch. The skip is a narrow environment gate, not a weakened assertion.
  • The keychain calls are hoisted out of the XCTAssert autoclosures. XCTest swallows errors thrown inside an assertion and records them as failures, so a status raised there would never reach the catch — without this, the skip would not fire reliably.

Proof

Both runs on the build host over SSH — i.e. in the exact session shape that was failing.

Gate — the skip fires and the suite is green:

$ swift test --filter ProviderSettingsTests
Test Case '...testKeychainStoreRoundTripsWithoutEntitlements' started.
ProviderSettingsTests.swift:247: ...: Test skipped -
  login keychain locked in a non-interactive session — run from a GUI session or CI
Test Case '...testKeychainStoreRoundTripsWithoutEntitlements' skipped (0.015 seconds).
Executed 17 tests, with 1 test skipped and 0 failures (0 unexpected)
EXIT=0

Mutation check — the skip is targeted, not a blanket catch. Temporarily swapping the caught status to errSecMissingEntitlement and re-running on the same host:

Executed 17 tests, with 1 failure (1 unexpected)
ProviderSettings.swift:146: error: ...: failed: caught error: "keychain(-25308)"
EXIT=1

The -25308 path fails loudly once it is no longer the caught status, which is the evidence that the skip is bound to that one status and nothing else. The mutation was reverted; it is not part of this branch.

Full suite runs on CI.

…when the login keychain is locked

- `testKeychainStoreRoundTripsWithoutEntitlements` performs a real round-trip
  through the file-based login keychain on purpose — that is the contract it
  guards. In a non-interactive session (SSH, `make release` over a remote
  shell) the login keychain is locked and Security cannot raise the unlock
  panel, so `SecItemAdd` returns errSecInteractionNotAllowed (-25308) and
  `make gates` fails on an environment quirk, not on a product defect.
- Catch that single status and `throw XCTSkip(...)` with an actionable
  message. Every other failure still fails the suite — in particular
  errSecMissingEntitlement (-34018), the regression this test exists to catch.
- Hoist the keychain calls out of the XCTAssert autoclosures: XCTest swallows
  errors thrown inside an assertion and records them as failures, so a status
  raised there would never reach the catch.
- Test-only change; no production code is touched.
- Gate, over SSH on sztudio: `swift test --filter ProviderSettingsTests` →
  17 tests, 1 skipped, 0 failures, exit 0; the skipped one is this test,
  reported as "login keychain locked in a non-interactive session".
- Mutation check, same host: swapping the caught status to
  errSecMissingEntitlement turns the run into 17 tests, 1 failure, exit 1 —
  `caught error: "keychain(-25308)"` at ProviderSettings.swift:146 — proving
  the skip is targeted at -25308 and not a blanket catch.

Authored-By: claude <agents@vetcoders.io>

session_id: 01a0057d-7dd8-7421-8363-844e9cf10946

time: 2026-08-16T03:17:26+02:00

runtime: interactive
Copilot AI lite review requested due to automatic review settings August 16, 2026 01:18

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Code review skipped — your organization has reached its monthly code review spending cap.

An organization admin can view or raise the cap at claude.ai/admin-settings/claude-code. The cap resets at the start of the next billing period.

Once the cap resets or is raised, reopen this pull request to trigger a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adjusts the ProviderSettings keychain round-trip unit test to handle a known non-interactive-session macOS keychain behavior so make release (run over SSH) can pass make gates without weakening the test’s intended entitlement regression coverage.

Changes:

  • Adds a targeted XCTSkip when Keychain returns errSecInteractionNotAllowed (-25308), with an actionable skip message.
  • Refactors the test to run Keychain calls outside XCTAssert* autoclosures so the skip path can be reliably triggered via do/catch.
  • Imports Security in the test file to reference the Keychain status constant.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@m-szymanska
m-szymanska merged commit 114a99a into main Aug 16, 2026
5 checks passed
@m-szymanska
m-szymanska deleted the fix/pensieve-keychain-test-headless-skip branch August 16, 2026 01:33
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.

2 participants