Repository navigation
Writing: release hardening (pause leak, stream hang, poll, log cap, double hash, orphan helper) - #1929
Merged
Conversation
- Pause for 1 hour is decided when a key is typed. The keyboard no longer captures while paused (it breaks the history segment), and the capture permit refuses during a pause, so text typed in a pause can't reach Save my writing or Personal History when the pause ends before delivery. WritingPausableIngest stays as the second layer. - The completion stream's response waiter always wakes: finish() decides the waiter's outcome under the lock, and a response after finish is refused. The network side sits behind LlamaStreamNetwork so tests can interleave cancel and the response callback deterministically. - The frontmost-window poll (1 Hz CGWindowListCopyWindowInfo) runs only with Autocomplete and Screen Memory on; the app-activation observer only with Autocomplete. Save-only users run neither. - writing-diagnostics.log rolls to .1 at 4 MB. - ModelManager records the launch check's fingerprint, taken from the same locked descriptor before and after the hash, so the first helper handoff skips a second full hash of the model. - Launch reaps an orphaned llama helper once setup is done, whatever the Autocomplete switch says, off the main thread. Same rule as before: only our own binary, re-parented to launchd, sole listener.
Owner
Author
|
Independent review (Fable reviewer, full diff): APPROVE. All six fixes verified (pause refused at key time via permit, same defaults suite across IME/app; stream outcome decided under lock; poll gating; atomic log roll; fingerprint from the hashed handle; reap only our ppid-1 helper, off main). Nits for later: resumed-partial path still re-hashes once; race test leaves a 30 s sleeper. CI runs on the integration PR #1934 (this PR's own run was cancelled to free the queue). |
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
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.
Six confirmed Writing issues fixed before release. Each one has a test, and each test goes red when its fix is reverted (checked by hand, one mutation per fix).
Fixes
1. (P1, privacy) Pause for 1 hour no longer leaks typed text.
The keyboard captured keys while paused. The app dropped them only if it was still paused when the debounced batch arrived, so pause → type → resume before delivery saved the paused text to Save my writing and Personal History.
PersonalHistoryCapture.permitalso refuses during a pause, which covers every capture path (typed, backspace, accepted).WritingPausableIngeststays as the app-side second layer. Secure input, consent/deletion generation and exclusion gates are unchanged.GhostPausedUntiltoUserDefaults(suiteName: keyboardSuiteName), which is the keyboard's bundle id (com.justinbetker.draft.inputmethod.Transcripted). The keyboard is outside the App Sandbox (config/entitlements/keyboard.plistis empty), so its.standarddomain is that same plist. The key now lives in one place,PersonalHistorySettingsContract.pausedUntilKey.PersonalHistoryCaptureTests"Text typed during a pause is never kept, even if the pause ends before delivery", "With suggestions off and no pause, typing is still captured", "An expired pause no longer blocks capture".TildeSettingsTests"The app's pause lands under the key and suite the keyboard reads".2. (P1) The completion stream's response waiter can't be dropped.
finish(throwing:cancelTask:)took the continuation under the lock but re-readreceivedStatusCodeoutside it. A response callback landing in that gap meant nobody resumed, sowaitForResponse()hung and leaked the socket, session and task..canceldisposition) and can't bring the operation back.LlamaStreamNetworkseam, and the delegate callbacks forward to plainreceive(statusCode:)/receive(data:)/complete(error:)methods. The public transport API is unchanged.LlamaStreamResponseRaceTests. It fires the response callback from inside cancel's teardown, which is exactly the old window. With the fix reverted, the test fails as.hungafter a 30 s watchdog. It doesn't stall the run.3. (P2) Save-only users no longer run the window poll or app observer.
CGWindowListCopyWindowInfopoll runs only with Autocomplete and Screen Memory both on. ThedidActivateApplicationobserver runs only while Autocomplete runs.applyAutocompleteToRuntime. Adeferthere also picks up a Screen Memory change on its own.CGWindowListcall.WritingSetupStateTests"Save-only Writing watches no windows; the 1 Hz poll needs Autocomplete and Screen Memory". This is theWritingFrontWindowWatchpolicy.4. (P2)
writing-diagnostics.logis capped.It rolls to
writing-diagnostics.log.1at 4 MB, checked on each open. The size comes from the opened descriptor, andrename(2)never follows a symlink. It keeps one old generation.docs/storage-paths.mdis updated.DiagnosticsLogRollTests.5. (P2) The model is hashed once per launch, not twice.
run()now records the fingerprint from its successful verify, and the first handoff takes the shape-check path. The fingerprint comes from the same locked descriptor the hash read. It's taken before and after the hash and recorded only if the two match and the result is.valid. Any later write or replacement still forces a full hash. The post-download verify records it too.ModelManagerTests"A launch with an installed model hashes it once, not again at the first handoff", "A model edited between the launch check and the handoff is hashed again and refused", "A freshly downloaded model is not hashed again at the first handoff". The existing cache test is updated to the new count.6. (P2) An orphaned llama-server is reaped at launch.
Once Writing setup is completed, launch runs the existing reap once, off the main thread, whatever the Autocomplete switch says. It follows the same rule as before: only this app's helper binary, only when it's re-parented to launchd, and only when it's the port's sole listener. That rule is now a pure function,
orphanToReap.LlamaOrphanReapTestscovers ownership and parent rules.WritingSetupStateTests"Launch reaps an orphaned Writing helper once setup is done, whatever the switches say".Checks run
swift test --filter TranscriptedWritingTests: 939 tests passed.bash run-tests.sh --filter Writing: 513 passed.bash check.sh, one full run on this code before the docs line was added.build.sh --no-open,run-tests.sh,swift testand the repo checks passed. The run reported only "source changed during proof", because the docs edit landed mid-run. A clean rerun was stopped for release speed, so CI is the full-suite proof for this head.check-test-shape.py,check-source-pins.py --changed-only,check-telemetry-keys.py: pass.Not proven here
🤖 Generated with Claude Code