Skip to content

Add macOS defaults compatibility checker#3

Closed
hadees wants to merge 3 commits into
masterfrom
codex/verify-settings-for-current-osx-version
Closed

Add macOS defaults compatibility checker#3
hadees wants to merge 3 commits into
masterfrom
codex/verify-settings-for-current-osx-version

Conversation

@hadees

@hadees hadees commented Jul 4, 2025

Copy link
Copy Markdown
Owner

Summary

  • add bin/check-macos-defaults.py to verify .macos commands against yannbertrand/macos-defaults docs

Testing

  • bats tests
  • ./bin/check-macos-defaults.py .macos Sonoma | head -n 20

https://chatgpt.com/codex/tasks/task_e_68674b906f1c832ea6078cbf35944462

@icaevan

icaevan commented Jun 11, 2026

Copy link
Copy Markdown

Review

Overview

Adds bin/check-macos-defaults.py, which parses defaults write commands out of .macos and checks each domain/key against the community docs in yannbertrand/macos-defaults, failing if a key has no doc or doesn't list the target version. A CI step runs it against "Sonoma" on every push.

The idea is genuinely useful — .macos is full of legacy keys and an automated staleness check has value. But as written, this will turn CI permanently red. The failure modes below were verified empirically, not just from reading the code.

Blocking issues

  • CI will fail on every run. .macos contains 203 defaults write commands; the macos-defaults project documents only a fraction of them. Any key without a doc → No doc foundsys.exit(1). The PR's testing note (./bin/check-macos-defaults.py .macos Sonoma | head -n 20) pipes through head, which masks the non-zero exit code — so the script was never observed succeeding. Since the workflow runs on a matrix, it'll fail on both ubuntu-latest and macos-latest.

  • The URL mapping doesn't match how the docs repo is organized. Docs are filed by feature category, not by defaults domain. Confirmed with live checks:

    • docs/screencapture/disable-shadow.md404; the real path is docs/screenshots/disable-shadow.md → 200
    • docs/nsglobaldomain/applekeyboarduimode.md404 (NSGlobalDomain keys are scattered across category folders)

    So domain_to_folder() produces false "No doc found" results even for keys that are documented.

  • "Version not listed" is the wrong failure signal. The docs' "Tested on macOS" lists are community-contributed and chronically incomplete — a key tested on Ventura almost always still works on Sequoia. Treating absence as failure makes the check noise, not signal. (The list format itself parses fine — the - **Tested on macOS**: / - Sequoia structure matches the parser.)

Correctness issues

  • The regex silently skips defaults -currentHost write.macos has 4 of these. A checker that silently under-reports is worse than one that says what it skipped.
  • (\S+) for the domain captures a leading " for quoted domains, and ([\w.-]+) can't match keys containing spaces — both produce garbage lookups that surface as "No doc found".
  • requests.get(url) has no timeout — a single hung connection stalls CI until the job-level timeout. ~200 sequential unauthenticated requests to raw.githubusercontent.com per run is also slow and flirts with throttling.
  • The version is hardcoded to Sonoma, but this repo was audited for Sequoia in cf4a466 — the check is stale on arrival, and will silently rot again with each macOS release.

Suggestions

  1. Make it advisory, not a gate. Print a coverage report ("17 of 203 keys documented; 3 missing the target version") and exit 0 by default, with a --strict flag if a gate is ever wanted. Or move it to a workflow_dispatch/scheduled workflow instead of every push.
  2. Drop the requests dependency. One GET per URL is exactly what stdlib urllib.request does; that deletes the pip3 install --break-system-packages step entirely (--break-system-packages is a flag worth avoiding on principle — if a dep were truly needed, uv run --with requests or a venv is cleaner).
  3. Fix discovery instead of guessing URLs: fetch the repo tree once (https://api.github.com/repos/yannbertrand/macos-defaults/git/trees/main?recursive=1, a single request) and match keys against actual paths. That solves the category-folder mismatch and cuts ~200 requests to 1.
  4. Add a bats test consistent with this repo's conventions — e.g. a --parse-only mode tested against .macos offline, asserting it finds the expected count including -currentHost lines. Right now the only "test" is the CI step itself, which fails.
  5. Minor Python style: split the one-line import argparse, re, requests, sys; the trailing sys.exit(0) is redundant; add a timeout=10 to any HTTP call that survives.

Verdict

Request changes. The concept is worth keeping, but merging as-is means a permanently red CI: the gate's two failure conditions ("no doc found", "version not listed") both fire on the majority of .macos's 203 keys, partly due to incomplete third-party docs and partly due to the broken domain→folder URL mapping. Restructure it as an advisory report (or fix discovery via the git tree API) before gating anything on it.

🤖 Generated with Claude Code

@icaevan

icaevan commented Jun 11, 2026

Copy link
Copy Markdown

Closing: the checker gates CI on a curated third-party docs site that only covers a small subset of defaults keys, so ~40 of ~70 settings fail as 'No doc found' and the build can never go green. The underlying audit was done by hand on master (cf4a466, 50e3ec3) using consumer-binary evidence instead. See review discussion for details.

@hadees

hadees commented Jun 11, 2026

Copy link
Copy Markdown
Owner Author

Closing: the checker gates CI on a curated third-party docs site that only covers a small subset of defaults keys, so ~40 of ~70 settings fail as 'No doc found' and the build can never go green. The underlying audit was done by hand on master (cf4a466, 50e3ec3) using consumer-binary evidence instead.

@hadees hadees closed this Jun 11, 2026
@hadees hadees deleted the codex/verify-settings-for-current-osx-version branch June 11, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants