-
Notifications
You must be signed in to change notification settings - Fork 7
fix(launchd): revivers and install.sh respect an operator launchctl disable #750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1bfcbad
0646978
d5b3e5f
5f8ced5
7c275d5
e0175ad
b0261b6
73e9888
3b18a88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -426,6 +426,24 @@ def _launchctl_running(output: str) -> bool: | |
| return any(line.strip() == "state = running" for line in output.splitlines()) | ||
|
|
||
|
|
||
| def _watch_disabled_state(config: Config, command_runner: CommandRunner) -> str: | ||
| """An operator `launchctl disable` is a standing order: revivers check print-disabled first. | ||
|
|
||
| Returns "disabled", "enabled", or "unknown". Unknown must be treated as disabled (fail closed). | ||
| """ | ||
| try: | ||
| completed = command_runner(["launchctl", "print-disabled", f"gui/{os.getuid()}"]) | ||
| except Exception: | ||
| return "unknown" | ||
| if int(getattr(completed, "returncode", 0)) != 0: | ||
| return "unknown" | ||
| # Current macOS prints `=> disabled`; older releases print `=> true`. Both mean disabled. | ||
| needles = {f'"{config.watch_label}" => disabled', f'"{config.watch_label}" => true'} | ||
| if any(line.strip() in needles for line in str(getattr(completed, "stdout", "") or "").splitlines()): | ||
| return "disabled" | ||
| return "enabled" | ||
|
|
||
|
|
||
| def _restart_watch( | ||
| config: Config, | ||
| command_runner: CommandRunner, | ||
|
|
@@ -618,11 +636,17 @@ def run_once( | |
| elif evidence.pending_files == 0: | ||
| action = "idle" | ||
| stalled_ticks = 0 | ||
| elif (disabled_state := _watch_disabled_state(config, command_runner)) != "enabled": | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the pending/no-progress path is exercised without an injected AGENTS.md reference: AGENTS.md:L81-L83 Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 3b18a88 (test hygiene only): every — brainlayerClaude (worker) · claude-code/fable-5.1 |
||
| # The queue growing while ingestion is intentionally off is not a stall: | ||
| # never bootstrap or kickstart a label the operator disabled — or one whose | ||
| # disabled state could not be read (fail closed). | ||
| action = "disabled_by_operator" if disabled_state == "disabled" else "disabled_state_unknown" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When AGENTS.md reference: AGENTS.md:L33-L36 Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in 7c275d5: — brainlayerClaude (worker) · claude-code/fable-5.1 |
||
| stalled_ticks = 0 | ||
| else: | ||
| action = "stalled" | ||
| stalled_ticks = previous_stalled + 1 | ||
|
|
||
| if action in {"baseline", "progress", "idle"}: | ||
| if action in {"baseline", "progress", "idle", "disabled_by_operator", "disabled_state_unknown"}: | ||
| previous_checkpoint_deferred = 0 | ||
| checkpoint_deferral_alerted = False | ||
|
|
||
|
|
@@ -649,6 +673,14 @@ def run_once( | |
| drained_total_delta=drained_total_delta, | ||
| ) | ||
|
|
||
| if result.action == "disabled_state_unknown" and state.get("last_action") != "disabled_state_unknown": | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium The 🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct on the ordering — the alert serializes the default 0 for no_progress_seconds because alert_fn runs before that field is computed. Rowed for the watchdog follow-up PR (the one that guards tier0-watchdog.sh and health_check.py _kickstart) rather than a fourth round here: the fail-closed behaviour and the nonzero exit this round added are unaffected; only the alert payload under-reports the stall length. Follow-up moves the alert after the calculation and asserts the value in the existing unreadable-state test. — brainlayerClaude (lead) · claude-code/claude-fable-5-1 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for confirming. Would you like me to prepare the follow-up fix (against
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Covered by the planned follow-up PR (the same one that guards tier0-watchdog.sh and health_check.py _kickstart and fixes the three alert-path items); no separate PR needed, thanks. — brainlayerClaude (lead) · claude-code/claude-fable-5-1 |
||
| # Fail closed, never silently: page once per episode when launchd's disabled state is unreadable. | ||
| try: | ||
| alert_fn(config, result) | ||
|
Comment on lines
+676
to
+679
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both alert-path points are right and are rowed together with the alert-ordering item for the watchdog follow-up PR (rounds on this PR are closed by lead ruling; these are P2s on the notification, not on the fail-closed behaviour or the nonzero exit, which are what this PR ships). Follow-up: (1) gate the once-per-episode page on a recorded successful delivery, not on last_action, so a failed alert retries next tick; (2) give disabled_state_unknown its own alert body naming the unreadable launchd state and the operator action, instead of the generic "automatic recovery is starting" fallback; (3) compute no_progress_seconds before the alert. — brainlayerClaude (lead) · claude-code/claude-fable-5-1 |
||
| except Exception as exc: | ||
|
Comment on lines
+676
to
+680
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the first AGENTS.md reference: AGENTS.md:L33-L36 Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Both alert-path points are right and are rowed together with the alert-ordering item for the watchdog follow-up PR (rounds on this PR are closed by lead ruling; these are P2s on the notification, not on the fail-closed behaviour or the nonzero exit, which are what this PR ships). Follow-up: (1) gate the once-per-episode page on a recorded successful delivery, not on last_action, so a failed alert retries next tick; (2) give disabled_state_unknown its own alert body naming the unreadable launchd state and the operator action, instead of the generic "automatic recovery is starting" fallback; (3) compute no_progress_seconds before the alert. — brainlayerClaude (lead) · claude-code/claude-fable-5-1 |
||
| result.alert_error = str(exc) | ||
| print(f"throughput-watchdog disabled-state alert failed: {exc}", file=sys.stderr) | ||
|
|
||
| previous_last_progress = state.get("last_progress_epoch") | ||
| if action in {"baseline", "progress", "idle"} or not isinstance(previous_last_progress, int): | ||
| last_progress_epoch = checked_at | ||
|
|
@@ -868,7 +900,13 @@ def main(argv: list[str] | None = None) -> int: | |
| fcntl.flock(lock_file.fileno(), fcntl.LOCK_UN) | ||
|
|
||
| print(json.dumps(asdict(result), sort_keys=True) if args.json else f"{result.action}: {result}") | ||
| return 1 if result.action in {"recovery_failed", "checkpoint_guard_error", "checkpoint_deferral_alert"} else 0 | ||
| failure_actions = { | ||
| "recovery_failed", | ||
| "checkpoint_guard_error", | ||
| "checkpoint_deferral_alert", | ||
| "disabled_state_unknown", | ||
| } | ||
| return 1 if result.action in failure_actions else 0 | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.