feat: add Deno-powered extensions runtime (Phase 1)#1938
Conversation
- Add extensions-runtime crate wrapping deno_core for JS/TS execution - Add tauri-plugin-extensions for extension lifecycle management - Add hello-world example extension to validate architecture - Add extension tab type to frontend schema The extensions runtime uses a channel-based async architecture with: - Custom Deno ops for Hyprnote APIs (hypr.log) - IIFE wrapping for extension code to capture exports - v8 Global handles for function references across async boundaries This is Phase 1 of the Deno-powered plugin system. Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughAdds an extensions system: workspace dependencies, a new Rust extensions-runtime crate (deno_core-based), a Tauri plugin exposing extension commands, desktop UI support for extension tabs and a UI registry, and a Hello World JS extension with manifest and UI. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Desktop UI
participant Plugin as Tauri Plugin
participant Host as ExtensionsRuntime (controller)
participant JS as Deno/JS Runtime
UI->>Plugin: load_extension(path)
Plugin->>Host: LoadExtension { extension, responder }
activate Host
Host->>Host: read extension.json & entry file
Host->>JS: execute extension entry (register exports)
Host->>Host: discover exported functions
deactivate Host
Plugin-->>UI: success
UI->>Plugin: call_function(ext_id, fn, args)
Plugin->>Host: CallFunction { ext_id, fn, args, responder }
activate Host
Host->>JS: invoke JS function with converted args
Host->>Host: convert result to JSON Value
deactivate Host
Plugin-->>UI: JSON result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
Comment |
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
crates/extensions-runtime/src/ops.rs (1)
5-8: Consider returning()or aResultinstead of allocating"ok"on every call.Allocating a new
Stringon every log call is unnecessary. If the caller doesn't need a return value, use-> (). If error propagation is desired, useResult<(), Error>.Apply this diff to return
()instead:-pub fn op_hypr_log(#[string] message: String) -> String { +pub fn op_hypr_log(#[string] message: String) { tracing::info!(target: "extension", "{}", message); - "ok".to_string() }plugins/extensions/src/commands.rs (1)
36-42: Stub implementation returns empty list.
list_extensionsalways returns an empty vector. Consider implementing this by querying the runtime state, or add a TODO comment to track this.#[tauri::command] #[specta::specta] pub async fn list_extensions<R: tauri::Runtime>( - _app: tauri::AppHandle<R>, + app: tauri::AppHandle<R>, ) -> Result<Vec<String>, Error> { - Ok(vec![]) + // TODO: Implement by querying loaded extensions from runtime state + app.list_extensions().await }crates/extensions-runtime/src/runtime.rs (2)
32-50: Consider implementingDropfor graceful shutdown.
ExtensionsRuntimespawns a background thread but doesn't implementDrop. If the runtime is dropped without callingshutdown(), the background thread continues running orphaned.+impl Drop for ExtensionsRuntime { + fn drop(&mut self) { + // Best-effort shutdown - ignore errors since we're dropping + let _ = self.sender.try_send(RuntimeRequest::Shutdown); + } +}
40-47: Handle runtime creation failure gracefully.Line 44 uses
.unwrap()which can panic if the Tokio runtime fails to build (e.g., resource exhaustion). Consider propagating this error or using a fallible constructor.-impl ExtensionsRuntime { - pub fn new() -> Self { +impl ExtensionsRuntime { + pub fn new() -> Result<Self> { let (tx, rx) = mpsc::channel(100); std::thread::spawn(move || { let rt = tokio::runtime::Builder::new_current_thread() .enable_all() .build() - .unwrap(); + .expect("Failed to create tokio runtime for extensions"); rt.block_on(runtime_loop(rx)); }); - Self { sender: tx } + Ok(Self { sender: tx }) }Note: If this change is adopted, update
Defaultimpl and callers accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
Cargo.lockis excluded by!**/*.lockplugins/extensions/permissions/autogenerated/commands/call_function.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/autogenerated/commands/execute_code.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/autogenerated/commands/list_extensions.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/autogenerated/commands/load_extension.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/extensions/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**
📒 Files selected for processing (16)
Cargo.toml(2 hunks)apps/desktop/src/store/zustand/tabs/schema.ts(4 hunks)crates/extensions-runtime/Cargo.toml(1 hunks)crates/extensions-runtime/src/error.rs(1 hunks)crates/extensions-runtime/src/lib.rs(1 hunks)crates/extensions-runtime/src/manifest.rs(1 hunks)crates/extensions-runtime/src/ops.rs(1 hunks)crates/extensions-runtime/src/runtime.rs(1 hunks)extensions/hello-world/extension.json(1 hunks)extensions/hello-world/main.js(1 hunks)plugins/extensions/Cargo.toml(1 hunks)plugins/extensions/build.rs(1 hunks)plugins/extensions/src/commands.rs(1 hunks)plugins/extensions/src/error.rs(1 hunks)plugins/extensions/src/ext.rs(1 hunks)plugins/extensions/src/lib.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/store/zustand/tabs/schema.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/store/zustand/tabs/schema.ts
🧬 Code graph analysis (6)
plugins/extensions/build.rs (1)
crates/extensions-runtime/src/runtime.rs (1)
new(37-50)
crates/extensions-runtime/src/error.rs (1)
plugins/extensions/src/error.rs (1)
from(16-30)
plugins/extensions/src/commands.rs (3)
crates/extensions-runtime/src/runtime.rs (3)
load_extension(52-63)call_function(65-83)execute_code(85-97)plugins/extensions/src/ext.rs (6)
load_extension(8-11)load_extension(28-36)call_function(13-18)call_function(38-56)execute_code(20-24)execute_code(58-69)plugins/extensions/src/error.rs (1)
from(16-30)
plugins/extensions/src/lib.rs (3)
crates/extensions-runtime/src/runtime.rs (5)
new(37-50)load_extension(52-63)call_function(65-83)execute_code(85-97)default(109-111)plugins/extensions/src/commands.rs (4)
load_extension(7-12)call_function(16-24)execute_code(28-34)list_extensions(38-42)plugins/extensions/src/ext.rs (9)
load_extension(8-11)load_extension(28-36)call_function(13-18)call_function(38-56)execute_code(20-24)execute_code(58-69)state(29-29)state(44-44)state(63-63)
plugins/extensions/src/ext.rs (3)
crates/extensions-runtime/src/runtime.rs (4)
load_extension(52-63)call_function(65-83)execute_code(85-97)args(277-282)plugins/extensions/src/commands.rs (3)
load_extension(7-12)call_function(16-24)execute_code(28-34)crates/extensions-runtime/src/manifest.rs (1)
load(32-38)
crates/extensions-runtime/src/runtime.rs (3)
crates/extensions-runtime/src/ops.rs (1)
op_hypr_log(5-8)plugins/extensions/src/ext.rs (6)
load_extension(8-11)load_extension(28-36)call_function(13-18)call_function(38-56)execute_code(20-24)execute_code(58-69)crates/extensions-runtime/src/manifest.rs (1)
entry_path(40-42)
🪛 GitHub Actions: .github/workflows/desktop_ci.yaml
apps/desktop/src/store/zustand/tabs/schema.ts
[error] 78-78: TypeScript error TS2554: Expected 2-3 arguments, but got 1.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
🔇 Additional comments (14)
extensions/hello-world/main.js (1)
1-18: LGTM! Simple and effective example extension.The implementation demonstrates the core extension API patterns clearly. For a hello-world example, the lack of input validation and error handling is acceptable.
crates/extensions-runtime/src/error.rs (1)
1-25: LGTM! Error types cover the essential failure modes.The error variants appropriately capture extension lifecycle issues. The conversion to plugin-local errors (as shown in
plugins/extensions/src/error.rs) demonstrates good error boundary design.extensions/hello-world/extension.json (1)
1-8: LGTM! Clean and minimal extension manifest.The manifest correctly defines the extension metadata and entry point. Empty permissions are appropriate for this demonstration extension.
plugins/extensions/build.rs (1)
1-10: LGTM! Standard Tauri plugin build configuration.The build script correctly registers the four extension commands using the standard Tauri plugin builder pattern.
crates/extensions-runtime/Cargo.toml (2)
1-13: LGTM! Dependencies are appropriate for the extensions runtime.The dependency set correctly includes Deno core, async runtime (tokio), serialization (serde), and logging (tracing). The tokio features align with the runtime loop pattern shown in the relevant code snippet.
7-7: Updatedeno_coreto a current stable version; 0.338 is significantly outdated and falls within the period of disclosed security advisories.The pinned version 0.338 is 30 minor versions behind the latest (0.368.0 as of November 2025). More critically, multiple security advisories were published for
deno_corein 2024:
- CVE-2024-27934 (March 2024): Use-after-free vulnerability
- RUSTSEC-2024-0403 / RUSTSEC-2024-0405 (mid–late 2024): Denial-of-service issues via op_panic sandbox exposure
Given the timing and version lag,
0.338likely falls within affected ranges. Upgrade to0.368or the latest stable version to incorporate security fixes and performance improvements.Cargo.toml (1)
43-43: LGTM! Workspace dependencies correctly added.The new workspace members for the extensions runtime and plugin follow the established patterns and correctly reference their paths.
Also applies to: 121-121
crates/extensions-runtime/src/lib.rs (1)
1-8: LGTM!Clean module structure with appropriate public re-exports. The
opsmodule is correctly kept internal since it contains Deno operation implementations.plugins/extensions/src/lib.rs (1)
1-45: LGTM!The plugin initialization is well-structured with proper state management using
Arc<Mutex<State>>. Thetauri::Wryhardcoding incollect_commands!is a known limitation of the specta/tauri integration for type generation.plugins/extensions/src/error.rs (1)
1-31: LGTM!Comprehensive error handling with proper conversion from the runtime error type. All error variants are appropriately mapped with descriptive messages for channel errors.
crates/extensions-runtime/src/runtime.rs (2)
119-135: LGTM for runtime loop initialization.The runtime loop is properly structured with a dedicated JsRuntime and hypr global initialization. The
hypr.logbridge totracing::info!is a good pattern for debugging extensions.
197-200: I'll verify the concerns raised in this review comment aboutBox::leakmemory leaks and the lifetime requirements of deno_core'sexecute_scriptmethod.
<function_calls>
deno_core JsRuntime execute_script lifetime requirements 'static
</function_calls>Let me also search the codebase to understand how
execute_scriptis being used and check the deno_core version being used:
<function_calls>
#!/bin/bashFind and check Cargo.toml for deno_core version
find . -name "Cargo.toml" -type f | head -5 | xargs -I {} sh -c 'echo "=== {} ===" && grep -A2 "deno_core" {}'
</function_calls><function_calls>
#!/bin/bashCheck the actual execute_script usage in the runtime.rs file
cat -n crates/extensions-runtime/src/runtime.rs | sed -n '190,210p'
</function_calls><function_calls>
#!/bin/bashSearch for all uses of execute_script in the codebase to understand patterns
rg -n "execute_script" --type rust -B2 -A2
</function_calls>plugins/extensions/Cargo.toml (1)
19-19: I'll help you verify whether thetestfeature ontauriis needed in the runtime dependencies. Let me start by examining the Cargo.toml file context and checking patterns across other plugins.
<function_calls>
#!/bin/bashFirst, let's see the context around line 19 in plugins/extensions/Cargo.toml
echo "=== Context around line 19 in plugins/extensions/Cargo.toml ==="
cat -n plugins/extensions/Cargo.toml | head -30echo ""
echo "=== Checking tauri dependency patterns in other plugins ==="
rg -n 'tauri\s*=.features' plugins//Cargo.toml
</function_calls>crates/extensions-runtime/src/manifest.rs (1)
15-23: Permissions are defined but not enforced in the runtime.
ExtensionPermissionsdefinesdb,network, andfilesystempermission vectors, but these are never validated during extension loading or execution. TheExtensionsRuntimeaccepts extensions without checking these permissions, and there is no enforcement logic in the codebase. Consider implementing permission validation inload_extension()orcall_function()methods, or document this as a planned feature.
- Add hello-world extension UI component using @hypr/ui (Card, Button) - Add extension tab rendering support in Body component (TabItemExtension, TabContentExtension) - Add extension registry with import.meta.glob for dynamic component loading - Wire extensions plugin into desktop app (lib.rs) - Add Vite alias @extensions for extensions directory - Add TypeScript path alias and type declarations for extensions - Add extensions package.json and tsconfig.json for proper TypeScript support Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
1 similar comment
|
Devin is archived and cannot be woken up. Please unarchive Devin if you want to continue using it. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
apps/desktop/src/types/extensions.d.ts (1)
3-6: Consolidate duplicatedExtensionViewPropsinterface.This interface is duplicated in
extensions/hello-world/ui.tsx(lines 12-15). Consider exporting it from a single shared location (e.g., this declaration file) and importing it in the extension UI files to avoid drift.apps/desktop/src/components/main/body/extensions/registry.ts (2)
14-18: Add defensive check for missing default export.If an extension module doesn't have a default export (perhaps due to a developer error), accessing
mod.defaultwill beundefined, leading to silent failures when rendering.for (const path in extensionModules) { const mod = extensionModules[path]; const parts = path.split("/"); const extensionId = parts[parts.length - 2]; - extensionComponents[extensionId] = mod.default; + if (mod.default) { + extensionComponents[extensionId] = mod.default; + } else { + console.warn(`Extension "${extensionId}" is missing a default export in ui.tsx`); + } }
5-7: Consider supporting both.tsxand.tsextensions.The glob pattern only matches
ui.tsx. If an extension author createsui.ts(no JSX), it won't be discovered.const extensionModules = import.meta.glob<{ default: ComponentType<ExtensionViewProps>; -}>("@extensions/*/ui.tsx", { eager: true }); +}>("@extensions/*/ui.{ts,tsx}", { eager: true });extensions/hello-world/ui.tsx (1)
13-16: Consider importingExtensionViewPropsfrom the shared type declaration.As noted earlier, this interface is duplicated. Once consolidated, import it here:
-export interface ExtensionViewProps { - extensionId: string; - state?: Record<string, unknown>; -} +import type { ExtensionViewProps } from "@/types/extensions"; +export type { ExtensionViewProps };apps/desktop/src/components/main/body/extensions/index.tsx (1)
58-62: Use explicit undefined check fortabIndex.The current truthy check
{tabIndex && ...}would hide the index iftabIndexis0. While the current usage starts indices at 1, an explicit check is more defensive.- {tabIndex && ( + {tabIndex !== undefined && ( <span className="text-xs text-neutral-400 shrink-0"> {tabIndex} </span> )}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (11)
apps/desktop/src-tauri/Cargo.toml(1 hunks)apps/desktop/src-tauri/src/lib.rs(1 hunks)apps/desktop/src/components/main/body/extensions/index.tsx(1 hunks)apps/desktop/src/components/main/body/extensions/registry.ts(1 hunks)apps/desktop/src/components/main/body/index.tsx(3 hunks)apps/desktop/src/types/extensions.d.ts(1 hunks)apps/desktop/tsconfig.json(1 hunks)apps/desktop/vite.config.ts(2 hunks)extensions/hello-world/ui.tsx(1 hunks)extensions/package.json(1 hunks)extensions/tsconfig.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/vite.config.tsapps/desktop/src/types/extensions.d.tsapps/desktop/src/components/main/body/extensions/registry.ts
**/*.config.{ts,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Agent configuration should be centralized and externalized from implementation logic
Files:
apps/desktop/vite.config.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/vite.config.tsapps/desktop/src/types/extensions.d.tsapps/desktop/src/components/main/body/index.tsxapps/desktop/src/components/main/body/extensions/index.tsxapps/desktop/src/components/main/body/extensions/registry.tsextensions/hello-world/ui.tsx
🧠 Learnings (4)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: After some amount of TypeScript changes, run `pnpm -r typecheck`.
Applied to files:
extensions/tsconfig.json
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Use `motion/react` instead of `framer-motion`.
Applied to files:
extensions/tsconfig.json
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Applied to files:
extensions/tsconfig.json
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Applied to files:
extensions/tsconfig.json
🧬 Code graph analysis (3)
apps/desktop/src/types/extensions.d.ts (1)
extensions/hello-world/ui.tsx (1)
ExtensionViewProps(13-16)
apps/desktop/src/components/main/body/extensions/registry.ts (2)
apps/desktop/src/types/extensions.d.ts (2)
ExtensionViewProps(3-6)ExtensionViewProps(11-11)extensions/hello-world/ui.tsx (1)
ExtensionViewProps(13-16)
extensions/hello-world/ui.tsx (1)
apps/desktop/src/types/extensions.d.ts (2)
ExtensionViewProps(3-6)ExtensionViewProps(11-11)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
🔇 Additional comments (12)
extensions/package.json (2)
1-13: Verify whether React should be a runtime vs. dev-only dependency.The package configuration looks solid overall. However, clarify the intended usage: if extension authors will write React components that run at runtime,
reactand@types/reactshould be in"dependencies"rather than"devDependencies". Currently they're only available to the package's development tooling.If extensions can use React at runtime, apply this diff:
{ "name": "@hypr/extensions", "version": "0.0.0", "private": true, "type": "module", + "dependencies": { + "@hypr/ui": "workspace:^", + "react": "^19.2.0" + }, "devDependencies": { - "@hypr/ui": "workspace:^", "@hypr/utils": "workspace:^", "@types/react": "^19.2.7", - "react": "^19.2.0", "typescript": "~5.6.3" } }
6-12: Workspace dependencies and version pinning look appropriate.Using
"workspace:^"for monorepo packages and pinning TypeScript to ~5.6.3 aligns with managing cross-package consistency. The React 19.2.0 with @types/react ^19.2.7 is compatible.extensions/tsconfig.json (1)
1-25: LGTM!The TypeScript configuration is well-structured with appropriate strict mode settings, bundler-friendly module resolution, and correct path aliases for the monorepo's shared packages.
apps/desktop/tsconfig.json (1)
21-27: LGTM!The path alias configuration correctly maps
@extensions/*to the extensions workspace, enabling clean imports of extension modules from the desktop app.apps/desktop/vite.config.ts (1)
15-23: LGTM!The alias configuration is cleanly refactored to support both the development-only updater mock and the new
@extensionsalias. Usingpath.resolveensures correct resolution across platforms.apps/desktop/src/components/main/body/index.tsx (3)
27-27: LGTM!The import follows the established pattern for tab type components.
289-300: LGTM!The extension tab handling in
TabItemcorrectly follows the established pattern used by other tab types, with consistent prop passing.
327-329: LGTM!The extension tab handling in
ContentWrapperis consistent with other tab type handlers.extensions/hello-world/ui.tsx (1)
18-62: LGTM!The component is a clean, idiomatic React implementation using
@hypr/uicomponents. Good demonstration of the extension UI pattern with local state management.apps/desktop/src/components/main/body/extensions/index.tsx (2)
21-88: LGTM!
TabItemExtensionis well-implemented with proper drag controls, context menu integration, and consistent styling usingcn. The pattern follows the established conventions for tab items.
90-113: LGTM!
TabContentExtensionhandles both the missing extension case with a helpful fallback UI and the happy path cleanly. Good defensive programming.apps/desktop/src-tauri/Cargo.toml (1)
34-34: Workspace dependency is properly defined and correctly configured.All verification checks pass:
tauri-plugin-extensionsis defined in rootCargo.tomlatplugins/extensions/- The desktop crate correctly references it with
{ workspace = true }- The extensions plugin depends on
hypr-extensions-runtime(workspace reference)hypr-extensions-runtimeatcrates/extensions-runtime/specifiesdeno_core = "0.338"as required- Alphabetical ordering is correct (between
tauri-plugin-dialogandtauri-plugin-fs)The dependency chain is complete, versions match the PR objectives, and the configuration follows workspace best practices.
- Remove Box::leak in execute_code_impl by using static literal script name - Add comment explaining why Box::leak is necessary in load_extension_impl - Handle serde_v8::to_v8 errors properly instead of unwrap - Map JSON decode errors to InvalidManifest for better error distinction - Validate manifest entry path to prevent directory escape attacks - Implement Clone for ExtensionsRuntime to fix mutex held across await - Remove redundant TypeScript declare module block (TS doesn't support multi-segment wildcards) - Add doc comment about permissions not being enforced yet Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
plugins/extensions/src/ext.rs (1)
7-25: Consider usingasync_traitfor cleaner trait definition.The current approach using
impl Futurein return position works but is verbose and requires Rust 1.75+. If backwards compatibility is needed or for cleaner syntax, considerasync_trait.+use async_trait::async_trait; + +#[async_trait] pub trait ExtensionsPluginExt<R: Runtime> { - fn load_extension( + async fn load_extension( &self, path: PathBuf, - ) -> impl std::future::Future<Output = Result<(), crate::Error>>; + ) -> Result<(), crate::Error>; - fn call_function( + async fn call_function( &self, extension_id: String, function_name: String, args_json: String, - ) -> impl std::future::Future<Output = Result<String, crate::Error>>; + ) -> Result<String, crate::Error>; - fn execute_code( + async fn execute_code( &self, extension_id: String, code: String, - ) -> impl std::future::Future<Output = Result<String, crate::Error>>; + ) -> Result<String, crate::Error>; }crates/extensions-runtime/src/runtime.rs (1)
198-205:Box::leaktrades memory for lifetime satisfaction.The comment explains the rationale, but this is a memory leak. For a small number of extensions loaded once, this is acceptable. However, if extensions can be reloaded or this code path is hit repeatedly, memory will grow unbounded.
Consider alternatives:
- Store script names in a
Vec<Box<str>>on the runtime state (cleaned up on shutdown)- Use a static string interner like
string-internerorlassocrate+// In runtime_loop or a state struct: +let mut script_names: Vec<Box<str>> = Vec::new(); // In load_extension_impl: -let script_name: &'static str = Box::leak(extension.manifest.id.clone().into_boxed_str()); +let boxed_name: Box<str> = extension.manifest.id.clone().into_boxed_str(); +script_names.push(boxed_name); +let script_name: &str = script_names.last().unwrap();Note: This requires passing
script_namestoload_extension_implor restructuring.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/types/extensions.d.ts(1 hunks)crates/extensions-runtime/src/manifest.rs(1 hunks)crates/extensions-runtime/src/runtime.rs(1 hunks)plugins/extensions/src/ext.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/types/extensions.d.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/types/extensions.d.ts
🧠 Learnings (2)
📚 Learning: 2025-11-24T16:32:19.706Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:19.706Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props. Just inline them.
Applied to files:
apps/desktop/src/types/extensions.d.ts
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Applied to files:
apps/desktop/src/types/extensions.d.ts
🧬 Code graph analysis (3)
apps/desktop/src/types/extensions.d.ts (1)
extensions/hello-world/ui.tsx (1)
ExtensionViewProps(13-16)
plugins/extensions/src/ext.rs (3)
crates/extensions-runtime/src/runtime.rs (3)
load_extension(53-64)call_function(66-84)execute_code(86-98)plugins/extensions/src/commands.rs (3)
load_extension(7-12)call_function(16-24)execute_code(28-34)crates/extensions-runtime/src/manifest.rs (1)
load(35-51)
crates/extensions-runtime/src/runtime.rs (3)
crates/extensions-runtime/src/ops.rs (1)
op_hypr_log(5-8)plugins/extensions/src/ext.rs (6)
load_extension(8-11)load_extension(28-39)call_function(13-18)call_function(41-61)execute_code(20-24)execute_code(63-77)crates/extensions-runtime/src/manifest.rs (1)
entry_path(53-55)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: fmt
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
🔇 Additional comments (9)
apps/desktop/src/types/extensions.d.ts (1)
3-6: SharedExtensionViewPropscontract is clear and consistentThe minimal props shape (
extensionIdplus optionalstate) is reasonable for extension UIs and matches the example extension’s definition, giving you a single, predictable contract for the registry and components to share. No issues from a typing or design perspective here.crates/extensions-runtime/src/manifest.rs (2)
34-51: Path validation correctly prevents directory traversal attacks.The canonicalization approach properly validates that the entry file stays within the extension directory. However, there's a subtle TOCTOU (time-of-check-to-time-of-use) consideration: the file is validated here but read later in
runtime.rs.For Phase 1 this is acceptable since extensions are trusted, but for production consider:
- Reading the file content here and passing it along, or
- Re-validating at read time in the runtime
53-55: LGTM!The
entry_pathhelper is simple and correctly joins the base path with the manifest entry.plugins/extensions/src/ext.rs (3)
28-39: Lock pattern correctly avoids holding mutex across await.The pattern of cloning the runtime handle before awaiting properly addresses the concurrency concern from the previous review.
41-61: Consistent lock pattern and proper error handling.The implementation correctly:
- Parses JSON args before acquiring the lock
- Clones runtime and releases lock before await
- Maps errors appropriately
63-77: LGTM!Consistent with the other methods. The lock-clone-release pattern is applied correctly.
crates/extensions-runtime/src/runtime.rs (3)
259-297: Function invocation implementation looks correct.The implementation properly:
- Looks up extension state and function by name
- Converts args with proper error handling (addressing previous review)
- Calls the function and deserializes the result
139-176: Runtime loop handles all request types appropriately.The message loop correctly dispatches to implementation functions and sends results via oneshot channels. The
Shutdownvariant cleanly breaks the loop.
246-257: Based on my investigation, I can now provide the rewritten review comment:
execute_code_implignores extension context and executes code in global runtime scope.The function receives
extension_idthrough the public API (line 86), but the runtime handler deliberately discards it with an underscore pattern (line 165: extension_id: _,), then callsexecute_code_implwithout passing any extension context. In contrast,call_function_impl(line 259) accepts and usesextension_idto isolate calls to a specific extension's state.This creates asymmetric behavior: all extensions share the same global JavaScript runtime state during code execution, while function calls are properly isolated per extension.
No Phase 1 requirements or design documentation were found in the codebase to clarify if this is intentional. Verify with your team whether global execution scope is the intended design or if extension isolation should be added.
V8 (used by deno_core) is compiled with a TLS model incompatible with shared libraries on Linux. Since mobile support is not needed (only macOS/Linux/Windows desktop), removing cdylib from crate-type fixes the linker error without losing any required functionality. Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Summary
This PR adds the foundation for a Deno-powered plugin/extension system for Hyprnote. It introduces:
crates/extensions-runtime- A Rust crate wrappingdeno_core(v0.338) that provides JavaScript execution capabilities. Uses a channel-based async architecture with the V8 runtime running in a dedicated thread.plugins/extensions- A Tauri plugin exposing extension lifecycle management to the frontend via commands:load_extension,call_function,execute_code,list_extensions.extensions/hello-world- A minimal example extension withextension.jsonmanifest,main.jsentry point for Deno runtime, andui.tsxReact component using@hypr/ui.Frontend schema update - Added
extensiontab type to the tab schema inapps/desktop/src/store/zustand/tabs/schema.ts.Extension tab rendering - Added
TabItemExtensionandTabContentExtensioncomponents with a registry that usesimport.meta.globto discover extension UI components at build time.The runtime wraps extension code in an IIFE to capture exports and uses v8 Global handles for function references across async boundaries. Extensions have access to a
hypr.log()API for logging.Updates since last revision
Linux build fix:
cdylibfromcrate-typeinapps/desktop/src-tauri/Cargo.tomlto fix V8 TLS linker error on Linuxcdylibfixes the build without losing functionalityCode review fixes:
serde_v8::to_v8error handling (no longer panics on conversion failure)Box::leakinexecute_code_implby using static literal script nameBox::leakis necessary inload_extension_impl(deno_core requires'staticlifetime)CloneforExtensionsRuntimeto fix mutex held across await pointsExtension::loadto prevent directory escape attacksInvalidManifestfor better error distinctiondeclare moduleblockReview & Testing Checklist for Human
tauri devandtauri buildwork correctly on macOS, Linux, and Windows after removingcdylib.Test Plan
ONBOARDING=0 pnpm -F desktop tauri devextensions/hello-world/greet("World")and verify it returns"Hello, World!"hypr.logoutputhello-worldand verify the React UI rendersNotes
list_extensionscurrently returns an empty vec (placeholder)import.meta.globwhich is build-time only (extensions must be present at build time)Box::leakis still used once per extension load (bounded leak) because deno_core'sexecute_scriptrequires a'staticscript nameLink to Devin run: https://app.devin.ai/sessions/6b7331a4976e430f80ef5e48bc468ea6
Requested by: yujonglee (@yujonglee)