Skip to content

Switch to isolated-vm and event based automation scripts - Attempt 3 and final one#44

Merged
heavyrubberslave merged 4 commits into
mainfrom
feature/isolated-vm-attempt-3-event-based
Jun 1, 2026
Merged

Switch to isolated-vm and event based automation scripts - Attempt 3 and final one#44
heavyrubberslave merged 4 commits into
mainfrom
feature/isolated-vm-attempt-3-event-based

Conversation

@heavyrubberslave

@heavyrubberslave heavyrubberslave commented Jun 1, 2026

Copy link
Copy Markdown
Member

Summary by CodeRabbit

  • Refactor
    • Updated default automation script template with callback-based event handlers (onStart, onStop, onEvent) for improved code structure and event handling patterns.
    • Reorganized internal type definitions for device and event management.

@coderabbitai

coderabbitai Bot commented Jun 1, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@heavyrubberslave, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 50 minutes and 51 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 6718b612-27f6-4c68-968b-85298cc66d57

📥 Commits

Reviewing files that changed from the base of the PR and between fcf004c and fd90cec.

📒 Files selected for processing (2)
  • src/components/automation/MonacoEditor.vue
  • src/stores/automation.ts
📝 Walkthrough

Walkthrough

Monaco editor type declarations and the default automation script template are updated together to introduce callback-based lifecycle hooks (onStart, onStop, onEvent) and async device methods, replacing the prior event-driven approach with structured function callbacks.

Changes

Automation Script API Modernization

Layer / File(s) Summary
Monaco type declarations for device and lifecycle API
src/components/automation/MonacoEditor.vue
DeviceEventType, Device interface with async getAttribute/setAttribute methods, DeviceEvent structure, and lifecycle hook functions (onEvent, onStart, onStop) are declared to replace prior enum and repository-based type definitions.
Default script template and usage examples
src/stores/automation.ts
defaultCode template skeleton transitions from IIFE-style event handling to callback-based lifecycle (onStart, onStop, onEvent), with updated inline comments and device/event reference examples aligned to the new API.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • SlvCtrlPlus/slvctrlplus-frontend#41: Both PRs update src/components/automation/MonacoEditor.vue's injected Monaco libSource typings around async/promise-based device APIs (e.g., getAttribute/setAttribute), and also adjust src/stores/automation.ts's defaultCode template to match the newer automation-script execution shape.

Suggested labels

patch

Poem

🐰 From events to callbacks, the magic unfolds,
Lifecycle hooks now grip what the script holds,
Monaco whispers types through the editor's dream,
And templates show users the newfangled scheme!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Switch to isolated-vm and event based automation scripts - Attempt 3 and final one' clearly describes the main change: transitioning automation scripts to use event-based callbacks (onStart, onStop, onEvent) instead of the prior IIFE-style approach, which is reflected in both updated files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/isolated-vm-attempt-3-event-based

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🧹 Nitpick comments (1)
src/components/automation/MonacoEditor.vue (1)

73-74: 💤 Low value

Consider renaming properties to follow conventional naming.

The readonly properties getDeviceId and getDeviceName use a "get" prefix, which is unconventional for properties (even those backed by getters). Standard TypeScript convention would be deviceId and deviceName.

♻️ Suggested property names
 declare interface Device {
-    readonly getDeviceId: string;
-    readonly getDeviceName: string;
+    readonly deviceId: string;
+    readonly deviceName: string;
     getAttribute(name: string): Promise<DeviceAttribute | undefined>;
     setAttribute(name: string, value: DeviceAttributeValue): Promise<void>;
 }

Based on learnings, these are correctly modeled as properties (not methods) if the backend exposes getters.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/components/automation/MonacoEditor.vue` around lines 73 - 74, Rename the
readonly properties getDeviceId and getDeviceName to deviceId and deviceName in
MonacoEditor.vue and update all references (template bindings, script imports,
computed/getter definitions, props or emitted payloads) to match the new names;
locate the getter-backed properties named getDeviceId/getDeviceName and rename
their identifiers to deviceId/deviceName and adjust any call sites so they are
used as properties (no parentheses) to keep TypeScript and Vue naming
conventions consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/components/automation/MonacoEditor.vue`:
- Around line 93-95: The global helpers getAttribute(deviceId, attributeName),
setAttribute(deviceId, attributeName, value) and getDevices duplicate the Device
interface methods and have incompatible types; either remove the globals or make
their signatures match Device methods to preserve type safety. Update or delete
the global declarations: either remove getAttribute/setAttribute/getDevices and
update call sites to use devices.getById(id)?.getAttribute(name) and
Device.setAttribute(value) and Device.getAttribute return types, or change the
global declarations to return Promise<DeviceAttribute | undefined> and accept
DeviceAttributeValue for setAttribute (and getDevices to return Array<{ id:
string; name: string }> consistent with Device store) so both APIs are
consistent; ensure you update all usages to the chosen API and keep one
authoritative implementation (prefer devices.getById / Device methods).

In `@src/stores/automation.ts`:
- Line 24: The comment shows an invalid example devices.getById(`{device
uuid}`); update it to a valid JavaScript example by removing the placeholder
braces and using a plain string or a proper template literal (e.g., a quoted
UUID or a backticked identifier without braces) in the comment so readers see a
correct call to devices.getById; update the comment text near the current
devices.getById mention.

---

Nitpick comments:
In `@src/components/automation/MonacoEditor.vue`:
- Around line 73-74: Rename the readonly properties getDeviceId and
getDeviceName to deviceId and deviceName in MonacoEditor.vue and update all
references (template bindings, script imports, computed/getter definitions,
props or emitted payloads) to match the new names; locate the getter-backed
properties named getDeviceId/getDeviceName and rename their identifiers to
deviceId/deviceName and adjust any call sites so they are used as properties (no
parentheses) to keep TypeScript and Vue naming conventions consistent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0322471-e8e3-412d-9a15-ac6195eaea1b

📥 Commits

Reviewing files that changed from the base of the PR and between 6b29ce4 and fcf004c.

📒 Files selected for processing (2)
  • src/components/automation/MonacoEditor.vue
  • src/stores/automation.ts

Comment thread src/components/automation/MonacoEditor.vue Outdated
Comment thread src/stores/automation.ts Outdated
@heavyrubberslave heavyrubberslave added the minor Creates a new minor release if merged label Jun 1, 2026
@heavyrubberslave heavyrubberslave merged commit e109f77 into main Jun 1, 2026
2 checks passed
@heavyrubberslave heavyrubberslave deleted the feature/isolated-vm-attempt-3-event-based branch June 1, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

minor Creates a new minor release if merged

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant