diff --git a/.claude/skills/check/SKILL.md b/.claude/skills/check/SKILL.md index adb03eeb..2b75ba73 100644 --- a/.claude/skills/check/SKILL.md +++ b/.claude/skills/check/SKILL.md @@ -95,7 +95,7 @@ hides every lint finding behind it — expect to fix a batch, not a queue. Steps 1–5 are the `--portable` subset. Everything below needs a macOS toolchain: 6. `swift test` with `-warnings-as-errors` -7. engine line-coverage gate (≥80%, `Tests/` excluded — see `MIN_COVERAGE`) +7. engine line-coverage gate (≥88%, `Tests/` excluded — see `MIN_COVERAGE`) 8. ThreadSanitizer + AddressSanitizer test passes 9. xcodegen drift check (regenerating must not change the committed `.pbxproj`) 10. codesign-skipped app build (warnings-as-errors) diff --git a/AGENTS.md b/AGENTS.md index 11db1794..768409f7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -169,7 +169,7 @@ the two checks that genuinely need the build products stay behind them (step 12) Everything above this line is the `--portable` subset. Everything below needs a macOS toolchain. 6. `swift test` with `-warnings-as-errors`, plus an engine **line-coverage gate** (≥ `MIN_COVERAGE`, - 80%, `Tests/` excluded — raise it as coverage grows). + 88%, `Tests/` excluded — raise it as coverage grows; `check.sh` records why ~91% is the ceiling). 7. **ThreadSanitizer** and **AddressSanitizer** test passes. 8. **xcodegen drift check** — regenerating must not change the committed `.pbxproj`. 9. **App build** with codesigning skipped (warnings-as-errors via `SWIFT_TREAT_WARNINGS_AS_ERRORS` in @@ -859,6 +859,16 @@ weak-reference assertions in `MemoryLeakTests.swift` (`expectNoLeak`) — LeakSa on Darwin, so AddressSanitizer catches memory _corruption_ but not leaks; `scripts/leaks.sh` covers the whole app under the Darwin leak detector. +**Mutation testing** — `scripts/mutate.sh`, opt-in and **not** part of `check.sh`. It flips one +operator in a source file, re-runs the suite, and reports whether anything noticed. Reach for it +instead of chasing the coverage number: the engine is near its line-coverage ceiling (the remainder is +Accessibility/TCC/CGEvent code no CI process can run), so the useful question is no longer "did this +line execute?" but "is it _asserted_?" — a surviving mutant is a behaviour change no test objected +to. Its default target list is the files already at or near 100% coverage, which is exactly where the +coverage number has nothing left to say. It stays out of `check.sh` deliberately: a run is minutes, +and survivors need judgement (an _equivalent_ mutant cannot change behaviour, so no test can catch +it), and a required gate that reports unactionable failures is one people learn to skip. + **Swift Testing traps that CI has caught more than once.** Nothing here can be typechecked without a macOS toolchain, so when you're writing tests from a Linux / web sandbox these are the ones that cost a red run. Check them by eye before pushing: diff --git a/Sources/BlurtEngine/Config/APIKeyValidator.swift b/Sources/BlurtEngine/Config/APIKeyValidator.swift index 4907c351..e798704c 100644 --- a/Sources/BlurtEngine/Config/APIKeyValidator.swift +++ b/Sources/BlurtEngine/Config/APIKeyValidator.swift @@ -38,7 +38,9 @@ public struct APIKeyValidator: Sendable { var components = URLComponents( url: baseURL.appendingPathComponent("v2/transcript"), - resolvingAgainstBaseURL: false + // `true` would behave identically here: the URL above is already absolute, so + // there is no base to resolve it against. Equivalent mutant, not a test gap. + resolvingAgainstBaseURL: false // mutate-ok: absolute URL, nothing to resolve ) components?.queryItems = [URLQueryItem(name: "limit", value: "1")] guard let url = components?.url else { return .unreachable } diff --git a/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift b/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift index 78a44c2f..e8749888 100644 --- a/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift +++ b/Sources/BlurtEngine/FocusCapture/FocusCapture+Editability.swift @@ -45,7 +45,16 @@ extension FocusCapture { /// genuinely no editable focus bundles no such framework and correctly falls /// back to copy. static func isElectronApp(_ app: NSRunningApplication?) -> Bool { - guard let bundleURL = app?.bundleURL else { return false } + isElectronBundle(app?.bundleURL) + } + + /// Pure decision behind `isElectronApp`: does the bundle at `bundleURL` ship the + /// Electron framework? Split from the `NSRunningApplication` wrapper for the same + /// reason as `isBrowserBundleID` — the detection is then unit-testable against a + /// fixture bundle, instead of requiring an Electron app to be installed *and* + /// running on the machine under test. + static func isElectronBundle(_ bundleURL: URL?) -> Bool { + guard let bundleURL else { return false } let electronFramework = bundleURL.appendingPathComponent( "Contents/Frameworks/Electron Framework.framework") return FileManager.default.fileExists(atPath: electronFramework.path) diff --git a/Sources/BlurtEngine/Injection/KeyInjector+SystemActions.swift b/Sources/BlurtEngine/Injection/KeyInjector+SystemActions.swift index 21354e0c..92ba8e65 100644 --- a/Sources/BlurtEngine/Injection/KeyInjector+SystemActions.swift +++ b/Sources/BlurtEngine/Injection/KeyInjector+SystemActions.swift @@ -41,17 +41,32 @@ extension KeyInjector { AXIsProcessTrusted() } - /// Posts Cmd-V. Returns `false` if the events couldn't be built. The real side - /// effect (a keystroke into the focused app) is why this is the injectable seam - /// tests replace. - static func postCmdV() -> Bool { + /// The Cmd-V key-down/key-up pair, or `nil` when CoreGraphics refuses to build + /// them. Split from `postCmdV` because only the *posting* is untestable: building + /// an event needs no Accessibility trust, while posting one sends a live + /// keystroke into whatever app has focus — not something `swift test` may do to + /// the machine it runs on. So the part carrying an actual invariant (the ⌘ flag + /// on both events and the `kVK_ANSI_V` keycode, which is what makes the paste a + /// paste) is asserted in `KeyInjectorSystemActionsTests`, and only the two + /// `.post` calls below stay covered by running the app. + static func cmdVEvents() -> (down: CGEvent, up: CGEvent)? { let vKey: CGKeyCode = 0x09 // kVK_ANSI_V guard let source = CGEventSource(stateID: .combinedSessionState), let down = CGEvent(keyboardEventSource: source, virtualKey: vKey, keyDown: true), let up = CGEvent(keyboardEventSource: source, virtualKey: vKey, keyDown: false) - else { return false } + else { return nil } + // Set on both: a key-up carrying no ⌘ reads as the modifier having been + // released mid-chord, which some apps treat as cancelling the shortcut. down.flags = .maskCommand up.flags = .maskCommand + return (down, up) + } + + /// Posts Cmd-V. Returns `false` if the events couldn't be built. The real side + /// effect (a keystroke into the focused app) is why this is the injectable seam + /// tests replace. + static func postCmdV() -> Bool { + guard let (down, up) = cmdVEvents() else { return false } // Post to the annotated session tap rather than the HID tap: the session tap // honors exactly the flags set above instead of OR-ing in the live hardware // modifier state, so a still-held hotkey modifier can't corrupt Cmd-V into a diff --git a/Sources/BlurtEngine/README.md b/Sources/BlurtEngine/README.md index 7deb40b4..fedcd755 100644 --- a/Sources/BlurtEngine/README.md +++ b/Sources/BlurtEngine/README.md @@ -245,7 +245,7 @@ await session.press() await session.release() ``` -Run `swift test` for the engine suites (`--filter DictationSessionTests` for one suite). `scripts/check.sh` is the full health gate CI runs — tests with warnings-as-errors, a ≥80% engine coverage gate, TSan/ASan passes, and the linters. On a machine without a macOS toolchain, `scripts/check.sh --portable` verifies docs/scripts/site changes only; the Swift side needs a Mac or CI. +Run `swift test` for the engine suites (`--filter DictationSessionTests` for one suite). `scripts/check.sh` is the full health gate CI runs — tests with warnings-as-errors, a ≥88% engine coverage gate, TSan/ASan passes, and the linters. On a machine without a macOS toolchain, `scripts/check.sh --portable` verifies docs/scripts/site changes only; the Swift side needs a Mac or CI. ## Embedding outside Blurt diff --git a/Tests/BlurtEngineTests/APIKeyDisplayTests.swift b/Tests/BlurtEngineTests/APIKeyDisplayTests.swift index 7a8c1a7a..a12d12ca 100644 --- a/Tests/BlurtEngineTests/APIKeyDisplayTests.swift +++ b/Tests/BlurtEngineTests/APIKeyDisplayTests.swift @@ -63,6 +63,16 @@ struct APIKeyDisplayTests { } } + @Test("the not-connected row reads as prose, not as an identifier") + func notConnectedIsProse() { + // Every other `!rendersIdentifier` assertion here is about `.connected(nil)` — a + // short key that gets masked to bare "Connected" — so the `.notConnected` arm was + // unpinned, and flipping it to `true` survived the whole suite + // (`scripts/mutate.sh`). Monospacing "Not connected" would style a sentence as a + // value. + #expect(!APIKeyDisplay.notConnected.rendersIdentifier) + } + @Test("the threshold keeps at least half of any masked key hidden") func thresholdIsTwiceTheTail() { #expect(APIKeyDisplay.minimumLengthToMask == APIKeyDisplay.revealedTailLength * 2) diff --git a/Tests/BlurtEngineTests/BlurtErrorTests.swift b/Tests/BlurtEngineTests/BlurtErrorTests.swift index 1550a8dc..ec06a344 100644 --- a/Tests/BlurtEngineTests/BlurtErrorTests.swift +++ b/Tests/BlurtEngineTests/BlurtErrorTests.swift @@ -63,6 +63,22 @@ struct BlurtErrorTests { #expect(BlurtError.sttFailed(underlying: a) == .sttFailed(underlying: sameIdentityOtherMessage)) } + @Test("wrapped equality requires both domain and code to match, not either") + func wrappedEqualityNeedsBothFields() { + // `wrappedEquality` above varies domain *and* code together, so it cannot tell + // `domain == … && code == …` from `||`: with `||` its unequal pair is still + // `false || false`. Varying one field at a time is what actually pins the + // conjunction. (Found by `scripts/mutate.sh` — the line was fully covered, and + // the mutation to `||` survived the whole suite.) + // + // Worth pinning rather than shrugging at: the engine tests assert error identity + // *through* this `==`, so a too-loose one wouldn't fail here — it would quietly + // weaken every `#expect(phase == .failed(.sttFailed(…)))` elsewhere. + let base = NSError(domain: "X", code: 1) + #expect(BlurtError.sttFailed(underlying: base) != .sttFailed(underlying: NSError(domain: "X", code: 2))) + #expect(BlurtError.sttFailed(underlying: base) != .sttFailed(underlying: NSError(domain: "Y", code: 1))) + } + @Test("wrapping cases of different kinds never compare equal") func crossKindInequality() { let e = NSError(domain: "X", code: 1, userInfo: [NSLocalizedDescriptionKey: "same"]) diff --git a/Tests/BlurtEngineTests/BrowserBundleIDTests.swift b/Tests/BlurtEngineTests/BrowserBundleIDTests.swift index 3073499b..967ad2fa 100644 --- a/Tests/BlurtEngineTests/BrowserBundleIDTests.swift +++ b/Tests/BlurtEngineTests/BrowserBundleIDTests.swift @@ -1,3 +1,5 @@ +import AppKit +import Foundation import Testing @testable import BlurtEngine @@ -60,3 +62,83 @@ struct BrowserBundleIDTests { #expect(!FocusCapture.isBrowserBundleID(nil)) } } + +/// The other half of the AX-opaque exemption: Electron detection, and the +/// `isAXOpaqueApp` disjunction the injector actually calls. +/// +/// Electron apps are classified by the framework they bundle rather than by +/// bundle ID, because the set is open-ended (every Electron app ever shipped), +/// so the fixtures here are directory trees rather than identifier strings — +/// `isElectronBundle` is split out of the `NSRunningApplication` wrapper for +/// exactly that reason. +@Suite("FocusCapture AX-opaque app classification") +struct AXOpaqueAppTests { + + /// An app bundle skeleton in a temp directory, with the Electron framework + /// present or absent. Only the *path* matters to the check — nothing is loaded — + /// so an empty directory at the framework's location is a faithful fixture. + private func makeBundle(withElectron: Bool) throws -> URL { + let bundle = URL.temporaryDirectory.appending(path: "Blurt-\(UUID().uuidString).app") + let contents = + withElectron + ? bundle.appending(path: "Contents/Frameworks/Electron Framework.framework") + : bundle.appending(path: "Contents/Frameworks") + try FileManager.default.createDirectory(at: contents, withIntermediateDirectories: true) + return bundle + } + + // MARK: isElectronBundle + + @Test("a bundle shipping the Electron framework is Electron") + func electronBundleDetected() throws { + let bundle = try makeBundle(withElectron: true) + defer { try? FileManager.default.removeItem(at: bundle) } + // The true arm is what keeps VS Code and Slack on the paste path: their focused + // text fields expose no editable AX signal, so without this they'd fall back to + // copy-only and the user's words would never land. + #expect(FocusCapture.isElectronBundle(bundle)) + } + + @Test("a native bundle with no Electron framework is not Electron") + func nativeBundleRejected() throws { + let bundle = try makeBundle(withElectron: false) + defer { try? FileManager.default.removeItem(at: bundle) } + // The false arm matters just as much: a native app with genuinely nothing + // editable focused must fall back to copy rather than beep a ⌘V. + #expect(!FocusCapture.isElectronBundle(bundle)) + } + + @Test("a bundle URL that doesn't exist is not Electron") + func missingBundleRejected() { + #expect(!FocusCapture.isElectronBundle(URL(filePath: "/nonexistent/Ghost.app"))) + } + + @Test("a nil bundle URL is not Electron") + func nilBundleURLRejected() { + #expect(!FocusCapture.isElectronBundle(nil)) + } + + // MARK: NSRunningApplication wrappers + + @Test("the test host is neither a browser nor Electron") + func testHostIsNotOpaque() { + // The one live `NSRunningApplication` a unit test can count on. Weak as an + // assertion about *this* process, but it pins the wrappers as pass-throughs to + // the two pure checks rather than, say, defaulting to opaque — which would make + // the injector paste into every non-editable target and beep. + let current = NSRunningApplication.current + #expect(!FocusCapture.isBrowserApp(current)) + #expect(!FocusCapture.isElectronApp(current)) + #expect(!FocusCapture.isAXOpaqueApp(current)) + } + + @Test("no app at all is not AX-opaque") + func nilAppIsNotOpaque() { + // `KeyInjector` passes its captured target, which is nil when nothing was + // captured — that must not be treated as opaque, or a paste with no known + // target would be attempted anyway. + #expect(!FocusCapture.isBrowserApp(nil)) + #expect(!FocusCapture.isElectronApp(nil)) + #expect(!FocusCapture.isAXOpaqueApp(nil)) + } +} diff --git a/Tests/BlurtEngineTests/DictationLogTests.swift b/Tests/BlurtEngineTests/DictationLogTests.swift index 2169ebbe..8f553ac3 100644 --- a/Tests/BlurtEngineTests/DictationLogTests.swift +++ b/Tests/BlurtEngineTests/DictationLogTests.swift @@ -6,6 +6,28 @@ import Testing private struct DecodedEntry: Decodable { let transcript: String let ts: String + /// `turns` and `keyterms` are the two fields `Entry.encode(to:)` writes only when + /// non-empty, so an omitted-key line has to decode rather than throw. Empty, not + /// optional — the repo bans optional collections, and it costs nothing here: + /// "omitted" vs "written as `[]`" is asserted on the raw line by + /// `nilFieldsAreOmitted`, which is the level that contract actually lives at. + let turns: [String] + let keyterms: [String] + + /// Spelled out because the custom `init(from:)` below suppresses the synthesis + /// that would otherwise derive these — mirroring `DictationLog.Entry`, which + /// states its own keys for the same reason. + enum CodingKeys: String, CodingKey { + case transcript, ts, turns, keyterms + } + + init(from decoder: any Decoder) throws { + let container = try decoder.container(keyedBy: CodingKeys.self) + transcript = try container.decode(String.self, forKey: .transcript) + ts = try container.decode(String.self, forKey: .ts) + turns = try container.decodeIfPresent([String].self, forKey: .turns) ?? [] + keyterms = try container.decodeIfPresent([String].self, forKey: .keyterms) ?? [] + } } /// Each test that genuinely needs a file gets a fresh empty one in a unique temp @@ -194,6 +216,22 @@ struct DictationLogTests { #expect(!line.contains("turns")) #expect(!line.contains("keyterms")) } + + @Test("a non-empty turns/keyterms list is written, with its values intact") + func conditionalFieldsAreWrittenWhenPresent() throws { + // The other direction of the same contract. `nilFieldsAreOmitted` pins the + // conditional arms' *skip*; without this, `encode(to:)` could stop writing + // either field entirely and only the negative test would still pass — leaving + // the corpus with no record of what steering a request carried. + let url = makeTempLogURL() + let context = TranscriptionContext( + appName: "Mail", priorText: "Hi Sam,", keyTerms: ["AssemblyAI", "LeMUR"]) + DictationLog.write(transcript: "p", context: context, to: url, now: Date()) + + let entry = try #require(firstEntry(in: url)) + #expect(entry.keyterms == ["AssemblyAI", "LeMUR"]) + #expect(entry.turns == ["Hi Sam,"]) + } } /// The shared encoder both logs write through. diff --git a/Tests/BlurtEngineTests/KeyInjectorSystemActionsTests.swift b/Tests/BlurtEngineTests/KeyInjectorSystemActionsTests.swift new file mode 100644 index 00000000..e683c745 --- /dev/null +++ b/Tests/BlurtEngineTests/KeyInjectorSystemActionsTests.swift @@ -0,0 +1,95 @@ +import AppKit +import CoreGraphics +import Testing + +@testable import BlurtEngine + +/// The system side of `KeyInjector`'s seams (`KeyInjector+SystemActions.swift`), +/// covering the parts that can be asserted without changing the state of the +/// machine running the suite. +/// +/// The line this suite draws: *reads* of process-global state are fair game +/// (`accessibilityTrusted`, the frontmost-app poll), and so is *building* a +/// CGEvent. What is deliberately left to running the real app is anything that +/// mutates the session — `activate` steals focus, and `postCmdV`'s two `.post` +/// calls would fire a live ⌘V into whatever the developer had open. That is the +/// same reason `check.sh` runs the XCUITest suite on CI only: it commandeers the +/// GUI session. +@Suite("KeyInjector system actions") +struct KeyInjectorSystemActionsTests { + + // MARK: - Cmd-V event construction + + @Test("cmdVEvents builds a V key-down/key-up pair") + func cmdVEventsBuildsPair() throws { + let events = try #require(KeyInjector.cmdVEvents()) + + #expect(events.down.type == .keyDown) + #expect(events.up.type == .keyUp) + // 0x09 is kVK_ANSI_V. Asserted numerically because the Carbon constant isn't + // importable here, which is why the source spells it as a literal too — this + // is the check that keeps that literal honest. + #expect(events.down.getIntegerValueField(.keyboardEventKeycode) == 0x09) + #expect(events.up.getIntegerValueField(.keyboardEventKeycode) == 0x09) + } + + @Test("both Cmd-V events carry the command flag") + func cmdVEventsCarryCommand() throws { + let events = try #require(KeyInjector.cmdVEvents()) + + // A ⌘-less key-down is a plain "v" — it types a character into the target + // instead of pasting, which is the visible failure this pins. + #expect(events.down.flags.contains(.maskCommand)) + // And a ⌘-less key-up reads as the modifier having been released mid-chord. + #expect(events.up.flags.contains(.maskCommand)) + } + + @Test("Cmd-V events carry no modifier beyond command") + func cmdVEventsCarryNoOtherModifier() throws { + let events = try #require(KeyInjector.cmdVEvents()) + + // ⌘⌥V and ⌘⇧V are "paste and match style" in most apps, and ⌃⌘V is bound + // elsewhere again — so a stray extra modifier doesn't fail loudly, it pastes + // the wrong way. `flags` is assigned (not OR-ed) in `cmdVEvents`, and this is + // what keeps it that way. + for flags in [events.down.flags, events.up.flags] { + #expect(!flags.contains(.maskAlternate)) + #expect(!flags.contains(.maskShift)) + #expect(!flags.contains(.maskControl)) + #expect(!flags.contains(.maskSecondaryFn)) + } + } + + // MARK: - Accessibility trust probe + + @Test("accessibilityTrusted reports the process-wide AX trust state") + func accessibilityTrustedMatchesSystem() { + // The test host's trust state isn't ours to set, so the assertable claim is + // that the seam is a pass-through and not, say, a hard-coded `true` that would + // make `KeyInjector` skip its permission check in production. + #expect(KeyInjector.accessibilityTrusted() == AXIsProcessTrusted()) + } + + // MARK: - Frontmost wait + + @Test("waitUntilFrontmost reports failure for an app that never comes frontmost") + func waitUntilFrontmostGivesUp() async { + // The test host is a command-line process with no windows, so the window + // server never reports it frontmost — the deterministic "activation didn't + // land" case. `KeyInjector.activateTargetApp` turns this `false` into + // `.targetAppLost` rather than pasting into the wrong app. + #expect(await KeyInjector.waitUntilFrontmost(.current) == false) + } + + @Test("waitUntilFrontmost gives up on a bounded deadline instead of hanging") + func waitUntilFrontmostIsBounded() async { + // Sits on the press→paste path, so an unbounded wait would freeze the paste, + // not just slow it. 350 ms budget; the ceiling leaves room for a loaded CI + // box's scheduling without being loose enough to pass an unbounded loop. + let clock = ContinuousClock() + let elapsed = await clock.measure { + _ = await KeyInjector.waitUntilFrontmost(.current) + } + #expect(elapsed < .seconds(3)) + } +} diff --git a/Tests/BlurtEngineTests/MemoizedKeyStoreTests.swift b/Tests/BlurtEngineTests/MemoizedKeyStoreTests.swift index 95dbfbe0..2b3979df 100644 --- a/Tests/BlurtEngineTests/MemoizedKeyStoreTests.swift +++ b/Tests/BlurtEngineTests/MemoizedKeyStoreTests.swift @@ -240,4 +240,27 @@ struct MemoizedKeyStoreTests { #expect(store.current == storage.stored) #expect(store.current != nil) } + + // MARK: - keychain wiring + + @Test("the keychain convenience init memoizes over the item it was given") + func keychainInitWiresBothClosures() { + // The init `APIKeyStore` actually constructs. Its whole body is two closure + // wirings, and a swapped or dropped one is invisible to every test above (they + // pass their own closures) — so this drives it against an isolated keychain + // item, never the real `AssemblyAIAPIKey` one. + let keychain = KeychainStore( + service: "dev.alex.blurt.tests", account: "memo-\(UUID().uuidString)") + defer { keychain.write(nil) } + let store = MemoizedKeyStore(keychain: keychain) + + // Read side: the item's value has to reach the memo. + #expect(keychain.write("sk-from-keychain")) + #expect(store.current == "sk-from-keychain") + + // Write side: a save has to land in that same item, not just in the memo. + #expect(store.save("sk-replaced")) + #expect(keychain.read() == .value("sk-replaced")) + #expect(store.current == "sk-replaced") + } } diff --git a/Tests/BlurtEngineTests/SystemClipboardTests.swift b/Tests/BlurtEngineTests/SystemClipboardTests.swift index 48ef9ca8..1564d87a 100644 --- a/Tests/BlurtEngineTests/SystemClipboardTests.swift +++ b/Tests/BlurtEngineTests/SystemClipboardTests.swift @@ -177,6 +177,23 @@ struct SystemClipboardTests { } } + @Test("write overwrites the clipboard with just the text") + func writeOverwrites() { + withClipboardRestored { + let pb = NSPasteboard.general + let clip = SystemClipboard() + + pb.clearContents() + pb.setString("previous", forType: .string) + // The `ClipboardAccess` half the degraded paste paths use: no restore is + // prepared, because the transcript is meant to *stay* on the clipboard for + // the user to paste by hand. So the previous contents must be gone. + clip.write("transcript") + + #expect(pb.string(forType: .string) == "transcript") + } + } + @Test("restore of an empty snapshot leaves the cleared pasteboard empty") func restoreEmptyIsNoOp() throws { try withClipboardRestored { diff --git a/scripts/check.sh b/scripts/check.sh index 1574fb72..9a459db9 100755 --- a/scripts/check.sh +++ b/scripts/check.sh @@ -42,10 +42,22 @@ if [ "$PORTABLE" -eq 0 ] && ! command -v swift >/dev/null 2>&1; then fi # Engine line-coverage floor (percent). Raise as coverage grows. -# Set to 80 to accommodate untestable syscall seams (e.g. the CGEvent paste -# poster and the Accessibility reads, which the CI test process can't exercise — -# it isn't Accessibility-trusted). -MIN_COVERAGE=80 +# +# The remaining uncovered engine code is almost entirely environment-bound rather +# than untested: the live-AX-tree reads in FocusCapture (~160 lines needing +# Accessibility trust *and* a focused text field), the TCC prompts in +# PermissionsChecker, `tccutil` in SigningIdentity, `KeyInjector.activate` and the +# two `CGEvent.post` calls, and two log-only paths that need a real +# URLSessionTaskMetrics. None can run in a CI test process, so treat ~91% as the +# practical ceiling and don't chase the last points by faking the OS — the useful +# move is extracting pure logic out of a syscall wrapper so it becomes testable +# (`MicCapture+Meter`, `FocusCapture.isBrowserBundleID` / `.isElectronBundle`, +# `KeyInjector.cmdVEvents`), which raises this floor as a side effect. +# +# Past that point this number stops being the interesting one: it says a line ran, +# not that anything asserted its result. `scripts/mutate.sh` (opt-in, not run here) +# answers the second question. +MIN_COVERAGE=88 export OS_ACTIVITY_MODE=disable diff --git a/scripts/mutate.sh b/scripts/mutate.sh new file mode 100755 index 00000000..b9424f7c --- /dev/null +++ b/scripts/mutate.sh @@ -0,0 +1,462 @@ +#!/bin/bash +# Mutation testing for the engine's pure-logic files: change one operator in the +# source, re-run the suite, and record whether any test noticed. +# +# Why this exists, given `check.sh` already gates line coverage: coverage answers +# "did this line run?", which the engine is now near the ceiling of (~89%, and the +# rest is Accessibility/TCC/CGEvent code no CI process can exercise). The question +# it cannot answer is "is this line *asserted*?" — a line can execute, and its +# value be wrong, and every test still pass. A surviving mutant is exactly that: a +# behaviour change no test objected to. +# +# So the default target list below is deliberately the files already at or near +# 100% line coverage. Those are the ones where the coverage number has nothing +# left to say, and where a survivor is a real finding rather than a restatement of +# "nothing covers this file." +# +# Deliberately NOT part of `check.sh`. A full run is minutes, not seconds, and +# survivors need judgement (some are equivalent mutants — a change that genuinely +# cannot alter behaviour, which no test can be expected to catch). A required gate +# that reports unactionable failures is a gate people learn to skip. Run this by +# hand, and treat survivors as a to-do list. +# +# Usage: +# scripts/mutate.sh # all mutants in the default target set +# scripts/mutate.sh --max 40 # stop after 40 mutants +# scripts/mutate.sh --files "a.swift b.swift" +# scripts/mutate.sh --list # enumerate mutants, run nothing +# +# A line ending `// mutate-ok: ` is exempt — for equivalent mutants only +# (a change that provably cannot alter behaviour), matching check-portability.sh. +# +# Safety: this edits tracked source files in place. Every target is copied to a +# temp directory up front and restored by an EXIT/INT/TERM trap, then verified +# byte-for-byte against its backup before the script returns — so a Ctrl-C mid-run +# leaves the tree as it found it. It does not need (or want) a clean git tree. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$REPO_ROOT" + +# The pure-logic engine files: no syscalls, no Accessibility, no network, so every +# line is reachable from a unit test and a survivor means a missing assertion +# rather than a missing environment. Keep this list to files whose logic is +# decision-making — adding a syscall wrapper here produces guaranteed survivors +# (nothing exercises those lines) that drown the real signal. +DEFAULT_TARGETS=( + "Sources/BlurtEngine/Config/APIKeyDisplay.swift" + "Sources/BlurtEngine/Config/APIKeyValidator.swift" + "Sources/BlurtEngine/FocusCapture/FocusCapture+Pure.swift" + "Sources/BlurtEngine/Hotkey/DictationKeyGate.swift" + "Sources/BlurtEngine/Hotkey/DictationKeyRouter.swift" + "Sources/BlurtEngine/Hotkey/TriggerKey.swift" + "Sources/BlurtEngine/Injection/KeyInjector+Separator.swift" + "Sources/BlurtEngine/Pipeline/MeterBarGeometry.swift" + "Sources/BlurtEngine/Pipeline/OverlayPlacement.swift" + "Sources/BlurtEngine/Pipeline/PipelinePhase.swift" + "Sources/BlurtEngine/Pipeline/RecordingCueGate.swift" + "Sources/BlurtEngine/STT/ConversationContext.swift" + "Sources/BlurtEngine/STT/KeytermsBoost.swift" + "Sources/BlurtEngine/STT/SyncSTTLimits.swift" + "Sources/BlurtEngine/StringNormalization.swift" + "Sources/BlurtEngine/Update/AutomaticUpdateCheck.swift" + "Sources/BlurtEngine/Update/SemanticVersion.swift" + "Sources/BlurtEngine/Update/UpdateAlertContent.swift" +) + +MAX_MUTANTS=0 # 0 = no cap +LIST_ONLY=0 +TARGETS=() + +while [ $# -gt 0 ]; do + case "$1" in + --max) + MAX_MUTANTS="${2:?--max needs a count}" + shift 2 + ;; + --files) + # shellcheck disable=SC2206 # deliberate word-split of a space-separated list + TARGETS=(${2:?--files needs a space-separated list}) + shift 2 + ;; + --list) + LIST_ONLY=1 + shift + ;; + -h | --help) + # The whole header block is the help text; keep this range in step with it. + sed -n '2,35p' "$0" + exit 0 + ;; + *) + echo "error: unknown argument '$1' (see --help)" >&2 + exit 2 + ;; + esac +done + +[ "${#TARGETS[@]}" -gt 0 ] || TARGETS=("${DEFAULT_TARGETS[@]}") + +for f in "${TARGETS[@]}"; do + [ -f "$f" ] || { + echo "error: target not found: $f" >&2 + exit 1 + } +done + +command -v python3 >/dev/null 2>&1 || { + echo "error: python3 not found — needed to enumerate and apply mutants" >&2 + exit 1 +} + +WORK="$(mktemp -d)" +BACKUP="$WORK/backup" +mkdir -p "$BACKUP" + +# Restore before anything else can look at the tree, and make it idempotent so the +# explicit call at the end and the trap can't fight. `cp` by flattened name because +# two targets can share a basename only if their paths differ, which the encoding +# below preserves. +restore_all() { + for file in "${TARGETS[@]}"; do + saved="$BACKUP/$(printf '%s' "$file" | tr '/' '_')" + [ -f "$saved" ] && cp "$saved" "$file" + done +} +# The group kill is necessary but not sufficient: `xctest` puts itself in a *new* +# process group (observed with PGID == its own PID and PPID 1 after a timeout), so +# signalling `swift-test`'s group cannot reach it, and the orphan keeps holding the +# build directory — which makes the *next* mutant fail for an unrelated reason and +# report as killed. Sweep it by the absolute path of this checkout's test bundle, so +# the match can't reach another clone, another project, or another user's xctest. +# +# The one thing this would catch unfairly is a `swift test` the developer is running +# by hand in this same repo — but that shares `.build` with this script anyway, so +# the two cannot run concurrently regardless. +reap_orphaned_xctest() { + local pids + pids="$(pgrep -f "xctest .*$REPO_ROOT/\.build/" 2>/dev/null || true)" + [ -n "$pids" ] || return 0 + # shellcheck disable=SC2086 # deliberate word-split: pgrep emits one pid per line + kill -TERM $pids 2>/dev/null || true + sleep 1 + # shellcheck disable=SC2086 + kill -KILL $pids 2>/dev/null || true +} + +# Ctrl-C during a hung suite has to sweep too, or the orphan outlives the script. +trap 'restore_all; reap_orphaned_xctest; rm -rf "$WORK"' EXIT INT TERM + +for file in "${TARGETS[@]}"; do + cp "$file" "$BACKUP/$(printf '%s' "$file" | tr '/' '_')" +done + +# --------------------------------------------------------------------------- +# Enumeration +# +# In python rather than sed/awk because the operators are the characters those +# tools treat as special (`&` in a sed replacement means the whole match, `|` and +# `.` are regex syntax), so a shell implementation would spend its complexity on +# escaping instead of on the part that matters: knowing which characters on a line +# are *code*. This repo is comment-dense and uses multi-line string literals for +# log messages, and a mutant planted in prose is guaranteed to survive — it would +# report as a finding while proving nothing. So the scanner tracks string and +# comment state and emits offsets into code only. +# +# Emits one TSV row per mutant: path, 1-based line, 0-based column, from, to. +# --------------------------------------------------------------------------- +enumerate() { + python3 - "$@" <<'PY' +import sys + +# from -> to, tried in this order. Longest-first within a prefix family (">=" before +# ">") is not needed because the bare "<" / ">" swaps are deliberately absent: Swift +# spells returns as "->" and generics as "", so mutating a lone angle bracket +# mostly yields code that doesn't compile — an invalid mutant costs a full build to +# learn nothing. Same reasoning excludes "+"/"-" (unary, string concat, and Duration +# arithmetic all overload them). +OPERATORS = [ + ("&&", "||"), + ("||", "&&"), + ("==", "!="), + ("!=", "=="), + (">=", "<"), + ("<=", ">"), + ("min(", "max("), + ("max(", "min("), + (".first", ".last"), + (".last", ".first"), + ("true", "false"), + ("false", "true"), +] + +# Word-ish operators must not match inside an identifier (`trueValue`, `isFalse`). +IDENT = set("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789_") +WORDY = {"true", "false"} + + +def code_spans(lines): + """Yield (line_index, start, end) spans that are code — not comment, not string. + + Handles line comments, block comments, single-quoted strings with escapes, and + triple-quoted multi-line strings. (Spelling that last delimiter out here would + close this docstring, which is how the first draft of this scanner failed.) Raw + strings are absent from this codebase; if that changes, a mutant landing inside + one surfaces as an unkillable survivor rather than as silence, which is the + failure mode to prefer. + """ + in_multiline = False + in_block_comment = False + for i, line in enumerate(lines): + j, span_start = 0, None + n = len(line) + while j < n: + if in_multiline: + if line.startswith('"""', j): + in_multiline = False + j += 3 + else: + j += 1 + continue + if in_block_comment: + if line.startswith("*/", j): + in_block_comment = False + j += 2 + else: + j += 1 + continue + if line.startswith('"""', j): + if span_start is not None: + yield (i, span_start, j) + span_start = None + in_multiline = True + j += 3 + continue + if line.startswith("//", j): + break # rest of the line is comment + if line.startswith("/*", j): + if span_start is not None: + yield (i, span_start, j) + span_start = None + in_block_comment = True + j += 2 + continue + if line[j] == '"': + if span_start is not None: + yield (i, span_start, j) + span_start = None + j += 1 + while j < n: + if line[j] == "\\": + j += 2 + continue + if line[j] == '"': + j += 1 + break + j += 1 + continue + if span_start is None: + span_start = j + j += 1 + if span_start is not None: + yield (i, span_start, j) + + +for path in sys.argv[1:]: + with open(path, encoding="utf-8") as handle: + lines = handle.read().split("\n") + for idx, start, end in code_spans(lines): + # Escape hatch, spelled the same way as check-portability.sh's + # `# portable-ok:`. It exists for *equivalent* mutants: a change that provably + # cannot alter behaviour, so no test can be written to catch it. Without a way + # to retire those, they resurface as survivors on every run and train the + # reader to skim a list whose whole value is that every entry is actionable. + if "// mutate-ok:" in lines[idx]: + continue + segment = lines[idx][start:end] + for frm, to in OPERATORS: + pos = 0 + while True: + hit = segment.find(frm, pos) + if hit < 0: + break + pos = hit + 1 + if frm in WORDY: + before = segment[hit - 1] if hit > 0 else "" + after_at = hit + len(frm) + after = segment[after_at] if after_at < len(segment) else "" + if before in IDENT or after in IDENT: + continue + print(f"{path}\t{idx + 1}\t{start + hit}\t{frm}\t{to}") +PY +} + +# Replace exactly one occurrence, located by line and column, so a line carrying +# two `==` yields two distinct mutants instead of one compound edit. +apply_mutant() { + python3 - "$1" "$2" "$3" "$4" "$5" <<'PY' +import sys + +path, line_no, col, frm, to = sys.argv[1], int(sys.argv[2]), int(sys.argv[3]), sys.argv[4], sys.argv[5] +with open(path, encoding="utf-8") as handle: + lines = handle.read().split("\n") +line = lines[line_no - 1] +assert line[col:col + len(frm)] == frm, f"{path}:{line_no}:{col} no longer holds {frm!r}" +lines[line_no - 1] = line[:col] + to + line[col + len(frm):] +with open(path, "w", encoding="utf-8") as handle: + handle.write("\n".join(lines)) +PY +} + +# Bound every build and suite run. A mutant can make a test await a condition that +# is now never satisfied — flipping a `PipelinePhase` predicate does exactly that — +# and `swift test` has no timeout of its own, so an unbounded harness wedges +# indefinitely on the first such mutant. (The first draft of this script sat on one +# for 35 minutes against a suite that normally finishes in ~1.2 s.) +# +# GNU `timeout` is not present on macOS — `check-portability.sh` flags it for that +# reason — so this polls a backgrounded job instead. `set -m` puts that job in its +# own process group so the kill reaches the `xctest` grandchild: signalling only +# `swift-test` leaves the hung test binary orphaned, still holding the build +# directory, and the next mutant then fails for an unrelated reason. +TEST_DEADLINE_SECONDS=60 +BUILD_DEADLINE_SECONDS=300 + +# Returns the command's status, or 124 if it hit the deadline (mirroring what +# `timeout` would report, so the caller reads the same way). +run_bounded() { + local deadline=$1 + shift + local pid waited=0 + set -m + "$@" >/dev/null 2>&1 & + pid=$! + set +m + while kill -0 "$pid" 2>/dev/null; do + if [ "$waited" -ge "$deadline" ]; then + kill -TERM -- "-$pid" 2>/dev/null || true + sleep 1 + kill -KILL -- "-$pid" 2>/dev/null || true + wait "$pid" 2>/dev/null || true + reap_orphaned_xctest + return 124 + fi + sleep 1 + waited=$((waited + 1)) + done + local status=0 + wait "$pid" || status=$? + return "$status" +} + +LIST="$WORK/mutants.tsv" +enumerate "${TARGETS[@]}" >"$LIST" +TOTAL="$(wc -l <"$LIST" | tr -d ' ')" + +if [ "$MAX_MUTANTS" -gt 0 ] && [ "$TOTAL" -gt "$MAX_MUTANTS" ]; then + head -n "$MAX_MUTANTS" "$LIST" >"$LIST.capped" + mv "$LIST.capped" "$LIST" + echo "note: capping at $MAX_MUTANTS of $TOTAL mutants (--max) — the score below" + echo " describes the sample, not the target set" + TOTAL="$MAX_MUTANTS" +fi + +if [ "$LIST_ONLY" -eq 1 ]; then + awk -F'\t' '{ printf "%s:%s %s -> %s\n", $1, $2, $4, $5 }' "$LIST" + echo "$TOTAL mutants across ${#TARGETS[@]} files" + exit 0 +fi + +echo "==> baseline (the suite must be green before a mutant means anything)" +# Without this, a suite that is already red reports every mutant as killed — a +# perfect score that measures nothing. +if ! swift build --build-tests >/dev/null 2>&1; then + echo "error: baseline build failed — fix that first" >&2 + exit 1 +fi +if ! swift test >/dev/null 2>&1; then + echo "error: baseline suite is red — fix that first, or every mutant reports killed" >&2 + exit 1 +fi +echo "baseline green" + +echo "==> $TOTAL mutants (~7s each, so roughly $(((TOTAL * 7 + 59) / 60)) min)" + +KILLED=0 +SURVIVED=0 +INVALID=0 +TIMED_OUT=0 +SURVIVORS="$WORK/survivors.txt" +: >"$SURVIVORS" + +INDEX=0 +# fd 3 so `swift test` inheriting stdin can't consume the mutant list. +while IFS=$'\t' read -r file line col frm to <&3; do + INDEX=$((INDEX + 1)) + printf '[%d/%d] %s:%s %s -> %s ' "$INDEX" "$TOTAL" "$file" "$line" "$frm" "$to" + + apply_mutant "$file" "$line" "$col" "$frm" "$to" + + if ! run_bounded "$BUILD_DEADLINE_SECONDS" swift build --build-tests; then + # The mutant doesn't compile. Counted separately, never as killed: a build + # error is the compiler objecting, not a test, and folding it into the kill + # count is how a mutation score flatters itself. + INVALID=$((INVALID + 1)) + echo "invalid (does not compile)" + else + TEST_STATUS=0 + run_bounded "$TEST_DEADLINE_SECONDS" swift test || TEST_STATUS=$? + case "$TEST_STATUS" in + 0) + SURVIVED=$((SURVIVED + 1)) + printf '%s:%s\t%s -> %s\n' "$file" "$line" "$frm" "$to" >>"$SURVIVORS" + echo "SURVIVED" + ;; + 124) + # A mutant that hangs the suite changed behaviour observably, so it counts + # as killed — but by a test's *liveness* rather than by an assertion, which + # is worth telling apart when reading the score. + KILLED=$((KILLED + 1)) + TIMED_OUT=$((TIMED_OUT + 1)) + echo "killed (timed out after ${TEST_DEADLINE_SECONDS}s)" + ;; + *) + KILLED=$((KILLED + 1)) + echo "killed" + ;; + esac + fi + + restore_all +done 3<"$LIST" + +restore_all +for file in "${TARGETS[@]}"; do + saved="$BACKUP/$(printf '%s' "$file" | tr '/' '_')" + cmp -s "$saved" "$file" || { + echo "error: $file did not restore cleanly — compare against $saved before committing" >&2 + exit 1 + } +done + +echo +echo "==> results" +echo "killed: $KILLED (of which $TIMED_OUT by hanging the suite, not by an assertion)" +echo "survived: $SURVIVED" +echo "invalid: $INVALID (did not compile — excluded from the score)" +SCORED=$((KILLED + SURVIVED)) +if [ "$SCORED" -gt 0 ]; then + echo "score: $((KILLED * 100 / SCORED))% ($KILLED/$SCORED viable mutants killed)" +fi + +if [ "$SURVIVED" -gt 0 ]; then + echo + echo "==> survivors — each is a behaviour change no test objected to" + echo " (some will be equivalent mutants that cannot change behaviour; that" + echo " judgement is yours, which is why this isn't a gate)" + sort "$SURVIVORS" | awk -F'\t' '{ printf " %-58s %s\n", $1, $2 }' +fi + +# Exit 0 regardless of survivors: this is a report, not a gate. `check.sh` owns +# green/not-green, and it does not call this script.