fix: automation platform guard - setup and config set no longer crash on Linux#32
Open
Priyanshubhartistm wants to merge 4 commits into
Open
Conversation
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.
Summary
codealmanac config set automation.*,config apply, andsetup(which reconciles automation internally) unconditionally wireLaunchdSchedulerAdapter, which shells out tolaunchctl. On any non-macOS platform this crashed with a rawlaunchctl bootstrap failed: No such file or directoryerror, after already writing config, and left an orphaned.plistfile in~/Library/LaunchAgents/that nothing would ever read. Even disabling automation crashed, sinceuninstall()'s new launchctl-not-found path doesn't match the "service not found" markers that would otherwise let it no-op.unavailable_reason()to theSchedulerAdapterport (mirroring the existingHarnessReadinessreadiness-query pattern).AutomationService.reconcile_task()/remove_all()check it before touching the scheduler at all, so no plist is ever written and nolaunchctlcall is attempted on an unsupported platform.Related issues #31
Why
AutomationService's constructor insrc/codealmanac/app.pyhas no platform check, and the recent config-first automation redesign (docs/plans/2026-07-10-user-toml-automation-preferences.md) funnels every automation-enabling path —config set,config apply, andsetup's internal reconciliation — through one method,AutomationService.reconcile_task(). All three inherited the same unguarded crash.The fix is asymmetric by design, matching how this codebase's own tests already treat these two call sites differently:
config set/config apply: explicit, single-purpose user commands. They keep propagating scheduler failures as a cleaExecutionFailed(existing tests liketest_schtoml_for_retryalready assert this is thecontract for any scheduler error) — now that failure is immediate and writes nothing, instead of a raw traceback after a half-written plist.setup's internalConfigService.update()(confirmed via grep to beSetupService's only caller — no CLI command callsit): a bulk onboarding operation. A structural pdn't abort instruction/harness config thatalready succeeded, so it catches the same error and reports it via a new
automation_errorfield instead of crashing setup.setupstill exits 0 with everything else instaRejected alternative
Considered putting the platform guard directly ir.install()
/uninstall(). Rejected after itbroke 4 existing adapter-level unit tests (test_launchd_adapter_writes_structured_plistand others) that mocksubprocess.rundirectly and run on Linux CI (/launchctl mechanics independent of host OS —those tests have nothing to do with whether the real host is macOS. Moved the platform fact to a separate, non-raisingunavailable_reason()query instead, consulted utomationService`), keeping the adapter's ownmocked-subprocess tests platform-agnostic.Tests
tests/test_config_service.py:config.set()/config.apply()raiseExecutionFailedcleanly with no scheduler calls whenunavailable (
UnavailableScheduler);config.upreturningautomation_error` while stillpersisting other config values.tests/test_setup_service.py: end-to-end testService/ConfigServicestack (only the bottomSchedulerAdapterfaked) confirmingSetupService.run()` doesn't raise and reports the reason.tests/test_cli.py:codealmanac setup --yesnsupported"/"macOS-only" step and noLibrary/LaunchAgentsdirectory;codealmanac config set automation.sync.every 8hexits 1 with a clean message and writesnothing;
codealmanac uninstall --yesstill sucnavailable.Verification
All pass (471 tests). Also manually verified agaerAdapter
on a Linux machine:configset/config applyfail cleanly with no orphaned plist,automation status` still works read-only.Docs and wiki
.almanac/wiki updated if an implementatiant, or gotcha changed.Wiki prose is written through
ingest/gardenlifecycle operations per this repo's convention, not hand-edited in a featurePR; the existing wiki pages describing the macOSurate.
Notes for reviewers
AutomationService.status()needed no guard —tus()/inspect()already degrade gracefully whenlaunchctlis absent (catches theOSError, returns not-loaded), soautomation status` continues to work as a safe read-only command everywhere.FakeSchedulerAdapter,FakeScheduler,CliSchedulerAdapter) were updated to implement the extended protocol (unavailable_reason()rell prior test behavior.