Skip to content

chore: remove setPreventSleep#4981

Open
NathanFlurry wants to merge 1 commit intomainfrom
05-05-chore_remove_setpreventsleep
Open

chore: remove setPreventSleep#4981
NathanFlurry wants to merge 1 commit intomainfrom
05-05-chore_remove_setpreventsleep

Conversation

@NathanFlurry
Copy link
Copy Markdown
Member

Description

Please include a summary of the changes and the related issue. Please also include relevant motivation and context.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@railway-app
Copy link
Copy Markdown

railway-app Bot commented May 6, 2026

🚅 Deployed to the rivet-pr-4981 environment in rivet-frontend

Service Status Web Updated (UTC)
website 😴 Sleeping (View Logs) Web May 9, 2026 at 12:32 am
kitchen-sink ❌ Build Failed (View Logs) Web May 6, 2026 at 10:47 pm
frontend-cloud ❌ Build Failed (View Logs) Web May 6, 2026 at 1:30 am
frontend-inspector ❌ Build Failed (View Logs) Web May 6, 2026 at 1:30 am
mcp-hub ✅ Success (View Logs) Web May 6, 2026 at 1:30 am
ladle ❌ Build Failed (View Logs) Web May 6, 2026 at 1:30 am

Copy link
Copy Markdown
Member Author

NathanFlurry commented May 6, 2026

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 6, 2026

PR Review: chore: remove setPreventSleep

This PR completes the removal of the setPreventSleep / preventSleep API that was previously deprecated and converted to a no-op. The actual behavioral change happened in an earlier PR; this one cleans up all the dead code across Rust core, NAPI bindings, TypeScript, agent-OS, tests, and docs.


Code Quality - Positive:

  • Clean layered removal: core to NAPI to TypeScript to agent-OS to tests to docs, all consistent.
  • Removal of waitForPreventSleepClear in mod.ts eliminates a complex Promise.race / while loop that was unreachable since preventSleep was always false.
  • The CanSleep.PreventSleep variant, the preventSleep field, and preventSleepClearedPromise are correctly removed from ActorInstance.
  • The guard test set_prevent_sleep_is_a_deprecated_noop served its purpose and is correctly deleted now that the functions are gone.
  • process.ts gets a real improvement: c.waitUntil(waitPromise) is now wired for spawned processes, which were previously relying on a no-op syncPreventSleep.

Potential Issues:

1. Docs inconsistency in processes.mdx

The line 'Active processes prevent the actor from sleeping' is retained verbatim, but it is no longer accurate. waitUntil runs during the grace window only and does not block the idle-sleep check. An active spawned process will not prevent the actor from entering sleep; it only keeps the grace window open until the process exits. Either update the sentence (e.g. 'Active processes will finish before the actor fully sleeps') or switch processes to keepAwake if the stronger guarantee is intentional.

2. Sessions and shells: no keep-awake

Active sessions (activeSessionIds) and shells (activeShells) no longer have any keep-awake mechanism after syncPreventSleep was removed. They were already broken when setPreventSleep was made a no-op, so this is not a regression from this PR. Worth confirming the intended design: should active LLM sessions or open shells use keepAwake(promise)? The docs update from 'active shells also prevent sleep' to 'close shells when disconnected' is accurate, but callers should understand the actor can sleep mid-session.

3. CLAUDE.md anti-pattern note removed

The removed bullet 'Do not reintroduce a prevent_sleep field, a CanSleep::PreventSleep variant, or branches that read it' served as a regression guard. Consider a short tombstone note: 'prevent_sleep flag-style sleep control was removed in favor of keep_awake(future) regions. Do not reintroduce it.' Minor preference, not blocking.


Test Coverage:

  • Driver test names correctly updated (preventSleep to keepAwake), and test logs confirm all renamed tests pass (3/3 green in each run log).
  • The waitShutdownTasks condition simplification (dropping the dead preventSleep branch) is correct.
  • No new coverage needed for this removal-only change.

Summary:

Clean, mechanical removal of dead code with no new behavioral risk. The one actionable item before merge: processes.mdx retains 'Active processes prevent the actor from sleeping,' which is no longer technically correct with waitUntil semantics.

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