Skip to content

refactor(frontend): sleep/schedule/wake actions on actors#5092

Merged
jog1t merged 1 commit into
mainfrom
05-21-refactor_frontend_sleep_schedule_wake_actions_on_actors
May 21, 2026
Merged

refactor(frontend): sleep/schedule/wake actions on actors#5092
jog1t merged 1 commit into
mainfrom
05-21-refactor_frontend_sleep_schedule_wake_actions_on_actors

Conversation

@jog1t
Copy link
Copy Markdown
Contributor

@jog1t jog1t commented May 21, 2026

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

Copy link
Copy Markdown
Contributor Author

jog1t commented May 21, 2026

Merge activity

  • May 21, 8:51 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 21, 9:13 PM UTC: Graphite rebased this pull request as part of a merge.
  • May 21, 9:13 PM UTC: @jog1t merged this pull request with Graphite.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 21, 2026

PR Review: refactor(frontend): sleep/schedule/wake actions on actors

Overview

This PR refactors the actor action buttons: moves the Destroy button out of the "Danger Zone" section into the main header bar alongside Sleep and Reschedule, upgrades Sleep and Reschedule from icon-only tooltip buttons to labeled buttons with icons, and introduces an auto-wake-suppression module to prevent the frontend from immediately re-waking an actor the user just put to sleep or rescheduled.

Correctness

canDeleteActors / destroyedAt guards are preserved. ActorStopButton has both checks internally, so removing the outer DangerZone wrapper doesn't regress feature-gating or post-destroy visibility. Good.

ActorRescheduleButton behavioral change is correct. Gating on destroyedAt instead of status === "sleeping" makes reschedule available for running/pending/crashing actors, which matches the comment's explanation that reschedule reallocates regardless of status. The comment is clear and accurate.

AutoWakeUpController fixes a real bug. Moving the wake query out of the sleeping overlay and into the .otherwise() branch means auto-wake fires on the metadata tab too, not only on tabs that mount the guard overlay. Good catch.

Rising-edge suppression clear is correct. Using prevStatusRef to detect the undefined/non-running → running transition avoids prematurely clearing suppression when the invalidation triggered by the sleep mutation returns a running status that was already set before the suppression was written.

Issues

subscribe is recreated on every render in useAutoWakeSuppression — this is the main code quality concern:

// auto-wake-suppression.ts
return useSyncExternalStore(
  (onChange) => {          // new arrow fn every render
    listeners.add(onChange);
    return () => listeners.delete(onChange);
  },
  () => suppressed.get(actorId),
);

React compares subscribe by reference and resubscribes on every render when it changes. Each render removes the old listener and adds a new one — harmless, but wasteful. The fix is to extract the stable subscribe function outside the hook:

function subscribe(onChange: () => void) {
  listeners.add(onChange);
  return () => listeners.delete(onChange);
}

export function useAutoWakeSuppression(actorId: ActorId) {
  return useSyncExternalStore(subscribe, () => suppressed.get(actorId));
}

AutoWakeUpRunner is a null-rendering component used only for its side-effect — it runs the wake query and returns null. React components that render nothing and exist only for useQuery side effects can be confusing. A custom useAutoWakeUpQuery(actorId) hook called inside AutoWakeUpController would make the intent clearer and skip an unnecessary React reconciliation layer. Low priority, but worth considering for consistency.

Suppression entry is never cleaned up after actor destruction. If a user sleeps or reschedules an actor and it is then destroyed (before transitioning to running), the entry stays in the suppressed Map until page reload. The practical impact is minimal since destroyed actors are rare targets and the Map stays tiny, but it is worth noting.

Minor Observations

  • Button layout on narrow viewports: Three labeled buttons (Sleep, Reschedule, Destroy) in the header action bar could overflow on narrow panels. Worth a quick visual check at the narrowest actor panel width.
  • Comment style: Multi-line block comments in auto-wake-suppression.ts and guard-connectable-inspector.tsx explain genuinely non-obvious invariants, which is appropriate per project conventions. One nit — the inline comment in actor-stop-button.tsx wraps mid-sentence across lines; a single-line comment or prose reflow would read more cleanly.

Summary

Solid refactor. The auto-wake suppression design is correct, the DangerZone guards are preserved inside ActorStopButton, and the AutoWakeUpController split is a real bug fix. The one actionable fix is the unstable subscribe reference in useAutoWakeSuppression — a one-line extract resolves it. Everything else is minor.

@jog1t jog1t changed the base branch from 05-21-refactor_frontend_bring_back_the_billing_gauge to graphite-base/5092 May 21, 2026 21:10
@jog1t jog1t changed the base branch from graphite-base/5092 to main May 21, 2026 21:11
@jog1t jog1t force-pushed the 05-21-refactor_frontend_sleep_schedule_wake_actions_on_actors branch from cf0ba28 to ebd825f Compare May 21, 2026 21:12
@jog1t jog1t merged commit 8953a85 into main May 21, 2026
8 of 11 checks passed
@jog1t jog1t deleted the 05-21-refactor_frontend_sleep_schedule_wake_actions_on_actors branch May 21, 2026 21:13
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