Keep clamshell override active after failed force clear#3
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9bc338fbf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
# Conflicts: # Sources/PowerNAPPlatform/Power/ClamshellOverride.swift
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e19e4fe8b4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aea989f28
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let clamshellWasActive = try store.clamshellIsActive() | ||
| let forceClearSucceeded = clamshell.forceClearIgnoreErrors() |
There was a problem hiding this comment.
Clear clamshell before reading local state
When the local restore path is used because the daemon is unavailable, any SQLite error from clamshellIsActive() now aborts before forceClearIgnoreErrors() runs. In the same corrupt/locked/inaccessible state-store scenarios where a safety restore is most needed, this leaves the OS clamshell override uncleared; the previous ordering attempted the IOKit clear before touching the store, so restore could still recover the machine even if state cleanup failed.
Useful? React with 👍 / 👎.
| let clamshellWasActive = (try? store.clamshellIsActive()) ?? false | ||
| let forceClearSucceeded = clamshell.forceClearIgnoreErrors() | ||
| if clamshellWasActive && !forceClearSucceeded { |
There was a problem hiding this comment.
Honor failed in-process clamshell release
If the daemon has an in-memory active clamshell override but the persisted clamshell_state is false or unreadable (for example because the earlier try? store.setClamshellActive(true, ...) failed), leaseManager.forceRelease can return false after failing to disable the override, but this restore path ignores that result and only errors when the store says it was active. In that case the daemon can ACK restore and clear/release state while its own ClamshellOverride still reports active, so callers are told recovery succeeded even though the OS override may still be enabled.
Useful? React with 👍 / 👎.
| if recovery.forceClearIgnoreErrors() { | ||
| try store.setClamshellActive(false, pid: nil) | ||
| } else { | ||
| logger.error("startup: clamshell override clear failed; keeping clamshell_state active") |
There was a problem hiding this comment.
Retry startup clear failures while daemon stays alive
When this startup failure branch runs, the daemon continues to start and write fresh heartbeats while leaving clamshell_state active. The only automatic retry path in WatchdogRuntime.performCheck is guarded by heartbeatAge > staleThresholdSeconds || !daemonAlive, so with this daemon alive and heartbeating the watchdog skips the retry even though the state remains active; a transient IOKit failure at startup can therefore leave the OS override enabled until manual restore or daemon death.
Useful? React with 👍 / 👎.
Summary
Validation