refactor: 2026 modernization — ARC, modern Hunspell, drop wrapper classes, fix installer security regression #1
Loading…
Reference in a new issue
No description provided.
Delete branch "refactor/2026-modernization"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
3-commit PR. Comprehensive code review + 2026 modernization on top of WARP-LAB/Pareizrakstiba 2020.01.
Commit 1 (
a615b19) — refactor: 2026 modernization+289 / −891 across 21 files.
Security / correctness:
enable-unsigned.shransudo spctl --master-disable, which permanently allowed apps from anywhere on every machine that ran the installer. Replaced with a targetedxattr -rd com.apple.quarantineon the installed.servicebundle (remove-quarantine.sh).ps aux | grep console | grep -v grep | cut -d' ' -f1matched users liketomconsoleadminand unrelated processes — replaced withstat -f "%Su" /dev/console.${SYSVER#*.}then%%.*on15.4.1produced4, so all[ $SYSVER_MIN -gt "5" ]branches took the wrong path on every macOS released since Big Sur (~2020).wordCountNULL-pointer semantics fixed —if (*wordCount) *wordCount = ...would crash on NULL or skip-write when value was zero. Nowif (wordCount != NULL).new Hunspell(NULL, NULL)was a latent crash ifpathForResourcereturned nil.Modernization:
LastUpgradeCheck = 1150(kept; see commit 3).char**API → modernstd::vector<std::string>API.PareizrakstibaSpellServer(emptyNSSpellServersubclass) andPareizrakstibaDelegate(1:1 forwarding wrapper) deleted. Spell checker conforms toNSSpellServerDelegatedirectly.MAC_OS_X_VERSION_MAX_ALLOWED <= MAC_OS_X_VERSION_10_4branches and dead#if DICTENCODEbranches (~80 lines of CFStringEncoding mapping table with two duplicate entries — the lv_LV dictionary is UTF-8).whitespaceAndNewlineCharacterSet(was ASCII space only).WARPDEBUG-prefixedNSLogspam behind#if DEBUG.Portability + hygiene:
.gitmodules: hunspell submodule URL → HTTPS (wasgit@github.com:SSH form, broke fresh--recursiveclones).build-{debug,release}.sh: corrected shebang to#!/bin/bash; xcodebuild paths and"$@"quoted.set -euo pipefail, paths quoted.Info.plist: bumped to2026.01-fork.1, droppedCFBundleSignature ????(creator codes deprecated since 10.6), updatedNSHumanReadableCopyright.Commit 2 (
dd764ed) — simplify: address /simplify review findings+48 / −72 across 10 files.
enumerateSubstringsInRange:options:NSStringEnumerationByWords(single pass, Unicode-correct).hasPrefix:instead of manual length+compare:options:NSLiteralSearch:range:insuggestCompletionsForPartialWordRange:.didLearnWord:anddidForgetWord:(optional inNSSpellServerDelegate; both bodies were#if DEBUG NSLogonly). Behavior change vs commit 1: the macOS spell server checks viarespondsToSelector:and skips the call — equivalent net behavior.Dictionaries/lv_LV-1.4.0andlv_LVto file-scopestatic NSString * const.runAsset(name, asAdmin)handler withquoted form of— fixed latent bug where install paths containing a space would have broken all 6do shell scriptcalls.uninstall.shandInstallPareizrakstiba.shto strictset -euo pipefail; dropped ~30 redundant|| truefromrm -rflines.KEEP IN SYNC WITHmarkers above duplicated CheckSpell/cocoAspell removal blocks in both installers.# Fork: 2026 modernizationheader comments and 2 stale "Replaces the previous spctl..." block comments. Note: this turned out to violate GPL-2.0 §2(a) per-file modification notice requirement; restored in commit 3.Commit 3 (
c06395a) — review: address /review findings+13 / −4 across 10 files.
Modifications © 2026 Ojārs Kapteinis (refactor: 2026 modernization).) on all 9 modified .mm/.h/.sh files. The README and Info.plistNSHumanReadableCopyrightdon't satisfy §2(a) — that section is specifically about per-modified-file notices.LastUpgradeCheckfrom1600back to1150. The bumped value claimed "I have already accepted Xcode 16's recommended-settings upgrade" but I never actually ran the upgrade flow (Command Line Tools only on dev machine, no Xcode.app).MACOSX_DEPLOYMENT_TARGETfrom10.13to10.15(matches the README's tested floor; aligns with current Xcode SDK expectations).remove-quarantine.shheader to credit it as a new file added in this PR rather than carrying the upstream-style copyright header.Test plan
plutil -lintpasses onInfo.plistandPareizrakstiba.xcodeproj/project.pbxprojclang -fsyntax-only -fobjc-arc -std=gnu++17 -mmacosx-version-min=10.15passes onPareizrakstibaSpellChecker.mmandmain.mmbash -npasses on all 8 shell scriptsosacompilepasses on the AppleScriptgrepconfirms no stale references to deleted classes.servicebundle on a clean macOS Sequoia VM, verify Latvian spellcheck works in TextEditxattr -rd com.apple.quarantineis sufficient on Sequoia — if not, may also needxattr -rd com.apple.provenanceNotes for review
#issuecomment-7190) covering all findings and risks.🤖 Generated with Claude Code
Code: - Enable ARC; bump C++ standard to gnu++17; bundle version 2026.01-fork.1 - PareizrakstibaSpellChecker.mm rewrite: - Migrate to modern Hunspell std::vector<std::string> / bool spell() API (was legacy char** with manual free_list) - Conform directly to NSSpellServerDelegate (drops wrapper classes below) - Fix wordCount NULL-pointer semantics (was `if (*wordCount)` — would crash on NULL or skip-write when value was 0; now `if (wordCount != NULL)`) - Word counting via whitespaceAndNewlineCharacterSet (was ASCII space only) - Null-check pathForResource → bail with NSLog instead of crashing on NULL paths to Hunspell constructor - Drop dead macOS 10.4 conditional branches; drop dead DICTENCODE branches and the 80-line CFStringEncoding mapping table (had two duplicate entries) - main.mm slimmed to use NSSpellServer + PareizrakstibaSpellChecker directly - Gate WARPDEBUG-prefixed NSLog spam behind #if DEBUG - Delete PareizrakstibaSpellServer (empty subclass of NSSpellServer) - Delete PareizrakstibaDelegate (1:1 forwarding wrapper, "for future grammar" that never landed) Installer (security + portability): - Replace `spctl --master-disable` with targeted `xattr -rd com.apple.quarantine` on the installed bundle. The previous approach permanently disabled Gatekeeper system-wide for the sake of one unsigned service. - Console-user detection: `stat -f "%Su" /dev/console` (was `ps aux | grep console | grep -v grep | cut -d' ' -f1`, which matched users like "tomconsoleadmin" and unrelated processes containing "console") - macOS-version parser: removed entirely. The old SYSVER_MIN parser (${SYSVER#*.} then %%.*) silently broke on macOS 11+ — on Sequoia 15.4.1 it produced "4" instead of "15", so all `[ $SYSVER_MIN -gt "5" ]` branches were taking the wrong path for ~5 years. Conditional gating dropped (deployment target is 10.13+ so all the gated calls are always-on now). - Quote all paths and variables; remove dead `killall Pareizrakstiba.service` (.service is a bundle, not a process); set strict `set -euo pipefail` Build: - build-{debug,release}.sh: shebang corrected to #!/bin/bash (uses bash-only ${BASH_SOURCE[0]}); xcodebuild args quoted Other: - .gitmodules: hunspell submodule URL → HTTPS (was SSH form, broke --recursive clones for users without GitHub SSH keys) - Drop CFBundleSignature ???? (creator codes deprecated since 10.6) - Update NSHumanReadableCopyright to credit upstream + fork - README: document fork modernization changes Build-tested: clang -fsyntax-only -fobjc-arc -std=gnu++17 passes on the .mm files; plutil -lint passes on Info.plist + project.pbxproj. A full Xcode build couldn't be run on this machine (Command Line Tools only, not full Xcode.app — `xcodebuild` is missing). Recommend opening the project in Xcode 15/16 and verifying the .service bundle loads in TextEdit before release. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>- PareizrakstibaSpellChecker.mm: - Replace componentsSeparatedByCharactersInSet+filter with enumerateSubstringsInRange:options:NSStringEnumerationByWords — single pass, no allocation, respects Unicode word boundaries - Replace manual length+compare:options:NSLiteralSearch:range: in suggestCompletionsForPartialWordRange with hasPrefix: (semantically identical, simpler) - Delete didLearnWord: and didForgetWord: (optional in NSSpellServerDelegate; both bodies were #if DEBUG NSLog only) - Lift "Dictionaries/lv_LV-1.4.0" + "lv_LV" to file-scope const so dictionary version bumps touch one place - Installer.applescript: extract runAsset(name, asAdmin) handler; use `quoted form of` (fixes latent bug — install path with a space would have broken all 6 do-shell-script calls) - Installer scripts: drop redundant `|| true` from `sudo rm -rf` (already returns 0 on missing paths); switch uninstall.sh and InstallPareizrakstiba.sh to strict `set -euo pipefail`; keep `|| true` only on commands that legitimately may fail (defaults delete/write, killall, xattr -rd) - uninstall.sh: drop redundant unprivileged `killall Pareizrakstiba` (script is invoked with administrator privileges, so the second sudo killall already runs as root) - remove-quarantine.sh: drop redundant `[ -d ... ]` guard (the `2>/dev/null || true` already handles missing path) - Add `KEEP IN SYNC WITH <other path>` markers above the duplicated CheckSpell + cocoAspell removal blocks in uninstall.sh and InstallPareizrakstiba.sh (extracting to a sourced helper would add payload-bundle complexity not worth the gain for a list that changes ~once a decade) - Drop 9 stale `# Fork: 2026 modernization by Ojārs Kapteinis` header comments and 2 stale "Replaces the previous spctl --master-disable" block comments — historical narration belongs in the PR description (which already covers it), not in source Build-tested: clang -fsyntax-only -fobjc-arc -std=gnu++17 passes; bash -n passes on all 8 shell scripts; osacompile passes on the AppleScript. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>PR self-review (post-/simplify pass)
🔴 Critical — possible GPL-2.0 §2(a) violation
The
/simplifypass had deleted the per-file// Fork: 2026 modernization by Ojārs Kapteinisheaders from 9 source files. GPL-2.0 §2(a) requires:NSHumanReadableCopyrightinInfo.plistand the README fork-changes block don't satisfy §2(a) — that section is specifically about per-modified-file notices.Status: FIXED in commit
c06395a— concise notice (Modifications © 2026 Ojārs Kapteinis (refactor: 2026 modernization).) restored on all 9 modified .mm/.h/.sh files plus a corrected header on the newremove-quarantine.sh.🟠 High —
LastUpgradeCheck = 1600was unverifiedproject.pbxprojwas bumped to 1600 (Xcode 16) but I never actually ran the Xcode 16 upgrade flow (this Mac has Command Line Tools only). The key signals "I have already accepted Xcode 16's recommended-settings upgrade" — but several settings still look pre-2020 (GCC_C_LANGUAGE_STANDARD = gnu99,compatibilityVersion = "Xcode 3.2").Status: FIXED in commit
c06395a— reverted to1150. When Ojārs opens in Xcode 16, he gets the prompt and can accept + commit the upgrade separately.🟠 High —
MACOSX_DEPLOYMENT_TARGET = 10.13may not build on Xcode 16The README says "Tested on 10.15.5. and 10.13.6", and 10.13 builds in Xcode 16 may produce libc++ warnings or fail outright.
Status: FIXED in commit
c06395a— bumped to10.15matching the README's tested floor.🟡 Medium —
xattr -rdquarantine fix not validated end-to-endOn macOS Sequoia, the unsigned-bundle policy goes beyond
com.apple.quarantine(alsocom.apple.provenance, plus additional notarization probes). The README's priorspctl --master-disablelikely worked because it disabled all those checks at once.Status: NOT YET ADDRESSED. Test plan must include "fresh-download → installer → TextEdit shows Latvian (Pareizrakstiba)" on a clean macOS Sequoia VM. If
xattr -rd com.apple.quarantinealone is insufficient, fall back to alsoxattr -rd com.apple.provenanceand document any remaining manual step.🔵 Low — minor
PareizrakstibaSpellChecker.mm:46—new Hunspell(...)isnoexcept(false). Hunspell may throw on OOM / malformed dict; the spell server then crashes. Wrap in try/catch + NSLog if defensive behaviour is desired (judgment call — original code did not wrap either).PareizrakstibaSpellChecker.mm:101—caseSensitive:YESmatches upstream. Latvian capitalization is significant for proper nouns but not for sentence-initial words. Worth a// TODOif user complaints arise.Installer.applescript— the newrunAssethandler shadowsassetsPathas both a global (line 2) and a parameter. AppleScript scoping is forgiving but a future maintainer might find it confusing.bash -n+clang -fsyntax-onlyForgejo Actions workflow would be cheap regression insurance.What's well done
wordCountNULL semantics,SYSVER_MINparser broken since Big Sur,ps | grep consoleconsole-user matching.spctl --master-disable) properly remediated.__bridgecasts.enumerateSubstringsByWords+hasPrefix:simplifications are textbook Foundation idiom.Risks
clang -fsyntax-onlyvalidates syntax, not link, not bundle resources, not codesigning. The first time someone opens this in Xcode could surface .pbxproj surgery problems my edits didn't catch.WARP-LAB/Pareizrakstibafor at least the security + correctness items.Recommendation
Gating test before merge: open project in Xcode 15/16, accept the recommended-settings prompt, build, install in TextEdit, type a misspelled Latvian word.
🤖 Generated with Claude Code