Skip to content

Keep clamshell override active after failed force clear#3

Open
mstallone wants to merge 3 commits into
mainfrom
buttery-jar
Open

Keep clamshell override active after failed force clear#3
mstallone wants to merge 3 commits into
mainfrom
buttery-jar

Conversation

@mstallone

@mstallone mstallone commented May 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Return success from clamshell force-clear attempts.
  • Keep clamshell state and leases active when force-clear fails so recovery can retry.
  • Cover force-clear success and failure in tests.

Validation

  • swift test

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/PowerNAPPlatform/Power/ClamshellOverride.swift
# Conflicts:
#	Sources/PowerNAPPlatform/Power/ClamshellOverride.swift

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread Sources/PowerNAPPlatform/Power/ClamshellOverride.swift

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment on lines +45 to +46
let clamshellWasActive = try store.clamshellIsActive()
let forceClearSucceeded = clamshell.forceClearIgnoreErrors()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment on lines +346 to +348
let clamshellWasActive = (try? store.clamshellIsActive()) ?? false
let forceClearSucceeded = clamshell.forceClearIgnoreErrors()
if clamshellWasActive && !forceClearSucceeded {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant