feat(extensions): implement tab rendering from extension #1949
Conversation
…fecycle hooks - Update manifest schema with api_version, panels array, and PanelDeclaration - Add logging ops (op_hypr_log_error, op_hypr_log_warn) for structured logging - Implement activate() lifecycle hook called on extension load with context - Update hypr global API with structured logging (hypr.log.info/error/warn) - Update Tauri plugin to expose PanelInfo with entry paths - Update hello-world extension to demonstrate Pattern A with activate lifecycle Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
📝 WalkthroughWalkthroughThis PR refactors the extension system to introduce API versioning and a panel-based UI architecture, replacing the single Changes
Sequence DiagramsequenceDiagram
actor User
participant Desktop as Desktop App
participant Registry as Panel Registry
participant Tauri as Tauri Plugin
participant Runtime as Extension Runtime
participant Deno as Deno VM
User->>Desktop: Load app
Desktop->>Registry: loadExtensionPanels()
Registry->>Tauri: list_extensions()
Tauri->>Runtime: iterate installed extensions
Runtime-->>Tauri: return Extension[] with panels & api_version
Tauri-->>Registry: ExtensionInfo[] with PanelInfo[]
Registry->>Registry: cache panels by id & extension
Registry-->>Desktop: panels loaded
User->>Desktop: Activate extension
Desktop->>Registry: getPanelInfoByExtensionId(id)
Registry-->>Desktop: PanelInfo (id, title, entry, path)
Desktop->>Runtime: load extension module
Runtime->>Deno: instantiate with __hypr_context
Deno->>Deno: execute activation hook<br/>with context metadata
Deno-->>Runtime: activate() logs context
Runtime-->>Desktop: extension ready
User->>Desktop: Interact with extension UI
Desktop->>Deno: call __hypr_extension methods
Deno->>Deno: use hypr.log.info/error/warn
Deno-->>Desktop: return results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–75 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
🤖 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
crates/extensions-runtime/src/manifest.rs (1)
72-88: Consider logging when panel path resolution fails.The
panel_pathmethod silently returnsNonewhen canonicalization fails or path traversal is detected. While returningNoneis a safe default, the caller has no way to distinguish between "panel not found" and "panel path is invalid/escapes directory". This could make debugging extension issues harder.Consider returning a
Resultor at minimum logging a warning:.and_then(|panel| { let canonical_root = self.path.canonicalize().ok()?; let joined = self.path.join(&panel.entry); - let canonical_panel = joined.canonicalize().ok()?; + let canonical_panel = match joined.canonicalize() { + Ok(p) => p, + Err(e) => { + tracing::warn!("Failed to resolve panel path for {}: {}", panel_id, e); + return None; + } + }; if canonical_panel.starts_with(&canonical_root) { Some(canonical_panel) } else { + tracing::warn!("Panel path escapes extension directory: {}", panel_id); None } })crates/extensions-runtime/src/runtime.rs (1)
233-233: Memory leak viaBox::leakfor script name.
Box::leakintentionally leaks the string to satisfy the'staticlifetime requirement for V8 stack traces. This is a known pattern, but each loaded extension permanently allocates memory for its ID.If extensions are reloaded frequently or many extensions are loaded over the application lifetime, this could accumulate. Consider documenting this tradeoff or exploring alternatives if extension hot-reloading becomes a use case.
plugins/extensions/src/commands.rs (1)
53-75: Consider extracting shared panel mapping logic.The panel mapping and
ExtensionInfoconstruction logic is duplicated betweenlist_extensions(lines 53-75) andget_extension(lines 114-136). This violates DRY principles and makes future updates more error-prone.Consider extracting a helper function:
fn ext_to_info(ext: hypr_extensions_runtime::Extension) -> ExtensionInfo { let panels = ext .panels() .iter() .map(|p| PanelInfo { id: p.id.clone(), title: p.title.clone(), entry: p.entry.clone(), entry_path: ext .panel_path(&p.id) .map(|p| p.to_string_lossy().to_string()), }) .collect(); ExtensionInfo { id: ext.manifest.id.clone(), name: ext.manifest.name.clone(), version: ext.manifest.version.clone(), api_version: ext.manifest.api_version.clone(), description: ext.manifest.description.clone(), path: ext.path.to_string_lossy().to_string(), panels, } }Then simplify both functions:
pub async fn list_extensions<R: tauri::Runtime>( app: tauri::AppHandle<R>, ) -> Result<Vec<ExtensionInfo>, Error> { // ... existing code ... - Ok(extensions - .into_iter() - .map(|ext| { - let panels = ext - .panels() - .iter() - .map(|p| PanelInfo { /* ... */ }) - .collect(); - ExtensionInfo { /* ... */ } - }) - .collect()) + Ok(extensions.into_iter().map(ext_to_info).collect()) }pub async fn get_extension<R: tauri::Runtime>( app: tauri::AppHandle<R>, extension_id: String, ) -> Result<ExtensionInfo, Error> { // ... existing code ... extensions .into_iter() .find(|ext| ext.manifest.id == extension_id) - .map(|ext| { - let panels = ext - .panels() - .iter() - .map(|p| PanelInfo { /* ... */ }) - .collect(); - ExtensionInfo { /* ... */ } - }) + .map(ext_to_info) .ok_or_else(|| Error::ExtensionNotFound(extension_id)) }Also applies to: 114-136
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
crates/extensions-runtime/src/manifest.rs(2 hunks)crates/extensions-runtime/src/ops.rs(1 hunks)crates/extensions-runtime/src/runtime.rs(4 hunks)extensions/hello-world/extension.json(1 hunks)extensions/hello-world/main.js(1 hunks)plugins/extensions/src/commands.rs(3 hunks)plugins/extensions/src/lib.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
crates/extensions-runtime/src/runtime.rs (1)
crates/extensions-runtime/src/ops.rs (3)
op_hypr_log(5-8)op_hypr_log_error(12-15)op_hypr_log_warn(19-22)
plugins/extensions/src/commands.rs (1)
crates/extensions-runtime/src/manifest.rs (1)
panels(90-92)
⏰ 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: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: fmt
🔇 Additional comments (14)
crates/extensions-runtime/src/ops.rs (1)
9-22: LGTM!The new
op_hypr_log_errorandop_hypr_log_warnops follow the same pattern as the existingop_hypr_logand correctly use the appropriate tracing log levels.crates/extensions-runtime/src/manifest.rs (3)
4-5: LGTM!Good approach to introduce API versioning with a constant. This will help with backward compatibility checks in the future.
22-31: LGTM!The
PanelDeclarationstruct is well-designed with appropriate derives. Thedefault_api_versionfunction provides a sensible default for backward compatibility.
90-92: LGTM!Simple accessor for panels is clean.
crates/extensions-runtime/src/runtime.rs (4)
11-14: LGTM!The extension macro correctly registers the new logging ops alongside the existing one.
134-149: Good backward compatibility approach.The shim maintains the callable
hypr.log(msg)signature while also exposinghypr.log.info/error/warnmethods. This ensures existing extensions continue to work.
268-273: LGTM!The log message now includes the API version, which aids debugging and visibility into which API version each extension uses.
216-231: Acknowledge the format injection risk, but clarify that extensions are application-bundled, not user-provided.The format! macro does interpolate extension code directly into the JavaScript wrapper. However, this pattern is lower risk than initially suggested because extensions are loaded from the filesystem (
./extensions/directory) as part of the application, not from user input or remote sources. Extensions undergo path validation (line 57-62 in manifest.rs) to prevent directory traversal.That said, the review comment's suggestion is still valid: deno_core 0.338 provides module evaluation APIs (
load_*_es_module) that would eliminate the format string interpolation pattern entirely and provide better defense-in-depth. If the application plans to support user-provided or remotely-sourced extensions in the future, this refactoring should be prioritized.Consider using
JsRuntime::load_main_es_module_from_code()followed bymod_evaluate()instead of wrapping code in a format string and callingexecute_script().extensions/hello-world/main.js (3)
13-22: LGTM!The functions correctly use
hypr.log.info()which aligns with the new structured logging API while maintaining backward compatibility through the runtime's shim.
24-30: LGTM!The description update reflects the Pattern A architecture change.
1-11: Reviewer's concern is confirmed:deactivateis not invoked by the runtime.The runtime code shows that
activateis called automatically during extension loading (lines 223-225 ofload_extension_impl), but there is no corresponding call todeactivate. When the runtime shuts down (lines 189-191), it simply breaks the event loop without notifying any loaded extensions. There is also no mechanism to unload individual extensions. The extension lifecycle is incomplete.plugins/extensions/src/lib.rs (2)
26-32: LGTM on PanelInfo structure.The
PanelInfostruct appropriately models panel metadata withid,title,entry, and an optionalentry_path. The optionalentry_pathcorrectly handles cases where the path resolution might fail.
20-24: Add API version validation during extension loading to prevent incompatible versions from silently failing at runtime.The
api_versionfield has implicit backwards compatibility via serde default (defaulting to "0.1" for old manifests without the field), so migration is handled. However, no validation exists: extensions specifying unsupported or futureapi_versionvalues load successfully but may fail during runtime execution.Consider adding a check in
Extension::load()(crates/extensions-runtime/src/manifest.rs) to validate thatapi_versionis supported, e.g., comparing againstCURRENT_API_VERSIONand returning an error if incompatible.plugins/extensions/src/commands.rs (1)
5-5: LGTM on import addition.The
PanelInfoimport is correctly added to support the new panel-based architecture.
…xtension panels Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
77-82: Consider adding guards and feedback to component registration.The
registerExtensionComponentfunction silently overwrites existing components without warning, which could make debugging difficult if an extension is accidentally registered twice.Consider adding validation and logging:
export function registerExtensionComponent( extensionId: string, component: ComponentType<ExtensionViewProps>, ): void { + if (dynamicExtensionComponents[extensionId]) { + console.warn(`Extension component "${extensionId}" is being overwritten`); + } dynamicExtensionComponents[extensionId] = component; + console.log(`Registered extension component: ${extensionId}`); }extensions/build.mjs (2)
18-20: Refine entry path transformation logic for robustness.The string replacement approach (
replace("dist/", "").replace(".js", ".tsx")) is fragile:
- Replaces all occurrences, not just prefix/suffix
- Doesn't handle edge cases like missing "dist/", ".mjs` files, or paths like "test.js.backup"
- Assumes a specific directory structure
Consider using path manipulation utilities for more robust transformation:
- const entryFile = panel.entry.replace("dist/", "").replace(".js", ".tsx"); + // Remove dist/ prefix if present, change extension to .tsx + let entryFile = panel.entry; + if (entryFile.startsWith("dist/")) { + entryFile = entryFile.slice(5); // Remove "dist/" + } + // Replace .js or .mjs extension with .tsx + entryFile = entryFile.replace(/\.(m?js)$/, ".tsx"); const entryPath = path.join(extensionDir, entryFile);Or more explicitly:
- const entryFile = panel.entry.replace("dist/", "").replace(".js", ".tsx"); + const parsed = path.parse(panel.entry); + const dirWithoutDist = parsed.dir.replace(/^dist\/?/, ""); + const entryFile = path.join(dirWithoutDist, parsed.name + ".tsx"); const entryPath = path.join(extensionDir, entryFile);
22-25: Consider adding a strict mode for builds.The script currently warns and continues when a panel entry file is missing. While this is reasonable for development, you might want a strict mode that fails the build in CI/production.
Consider adding an environment variable check:
if (!fs.existsSync(entryPath)) { console.warn(`Panel entry not found: ${entryPath}, skipping...`); + if (process.env.STRICT_BUILD === "true") { + throw new Error(`Missing required panel entry: ${entryPath}`); + } continue; }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
plugins/extensions/js/bindings.gen.tsis excluded by!**/*.gen.ts
📒 Files selected for processing (5)
apps/desktop/src/components/main/body/extensions/index.tsx(3 hunks)apps/desktop/src/components/main/body/extensions/registry.ts(3 hunks)extensions/.gitignore(1 hunks)extensions/build.mjs(1 hunks)extensions/package.json(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- extensions/.gitignore
🧰 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/components/main/body/extensions/registry.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/components/main/body/extensions/registry.tsapps/desktop/src/components/main/body/extensions/index.tsx
🧬 Code graph analysis (2)
apps/desktop/src/components/main/body/extensions/registry.ts (3)
packages/tiptap/src/shared/extensions/index.ts (1)
extensions(188-188)apps/desktop/src/types/extensions.d.ts (1)
ExtensionViewProps(3-6)extensions/hello-world/ui.tsx (1)
ExtensionViewProps(13-16)
apps/desktop/src/components/main/body/extensions/index.tsx (1)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
getPanelInfo(59-61)
⏰ 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: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: fmt
🔇 Additional comments (1)
apps/desktop/src/components/main/body/extensions/index.tsx (1)
92-92: I'll help you verify the review comment about the ID mapping issue betweentab.extensionIdand panel ID. Let me start by exploring the codebase to understand the structure.
<function_calls>
Search for getPanelInfo definition, tab structure, and panel registry
#!/bin/bashecho "=== Looking for getPanelInfo definition ==="
rg -n "getPanelInfo" --type ts --type tsx -B2 -A5echo -e "\n=== Looking for panel registry and loadedPanels ==="
rg -n "loadedPanels" --type ts --type tsx -B2 -A2echo -e "\n=== Looking for tab type definition ==="
rg -n "type.*Tab|interface.*Tab" --type ts --type tsx -B2 -A5<function_calls>
Get the context around line 92 in the target file to understand the usage
#!/bin/bashFirst verify the file exists and check its size
if [ -f "apps/desktop/src/components/main/body/extensions/index.tsx" ]; then
wc -l "apps/desktop/src/components/main/body/extensions/index.tsx"
echo "=== Context around line 92 ==="
sed -n '80,120p' "apps/desktop/src/components/main/body/extensions/index.tsx"
else
echo "File not found, searching for it..."
fd -e tsx -e ts "index.tsx" | grep extensions
fi
</function_calls>
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (4)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
63-86: EnsureloadExtensionPanels()is invoked so panel helpers ever return data.The helper functions (
getPanelInfo,getLoadedPanels,registerExtensionComponent) look good, but without callingloadExtensionPanels()during app startup,loadedPanelsstays empty and these APIs effectively become no-ops. A previous review already flagged the missing call sites; please re-check after your latest changes.To verify current usage, you can run:
#!/bin/bash # Search for any callers of loadExtensionPanels rg -n "loadExtensionPanels" --type ts --type tsx -C3extensions/build.mjs (3)
74-78: Use top‑levelawait(or an asyncmain()) so build completion and failures are explicit.Calling the async functions without awaiting works but makes success/failure handling depend on unhandled‑rejection behavior; awaiting here clarifies control flow and exit codes.
-if (extensionName) { - buildExtension(extensionName); -} else { - buildAll(); -} +if (extensionName) { + await buildExtension(extensionName); +} else { + await buildAll(); +}(If your Node/tooling doesn’t like top‑level
await, you can instead wrap this in anasync function main() { ... } main().catch(err => { console.error(err); process.exit(1); });.)
11-16: Guard manifest read/parse with try/catch so one bad extension doesn’t kill the build.
JSON.parse(fs.readFileSync(...))will currently throw on IO or syntax errors and abort the whole script; better to log and skip that extension instead.- const manifest = JSON.parse(fs.readFileSync(manifestPath, "utf-8")); + let manifest; + try { + manifest = JSON.parse(fs.readFileSync(manifestPath, "utf-8")); + } catch (error) { + console.error( + `Failed to read or parse manifest for ${name} at ${manifestPath}:`, + error, + ); + return; + }Based on learnings, manifest error handling should cover common failure scenarios rather than hard-crashing the build.
34-47: Wrapesbuild.buildin try/catch and fail with a clear, contextual error.Right now a bundling failure surfaces as a generic unhandled rejection; you lose which extension/panel failed and may not get a clean non‑zero exit code.
- console.log(`Building ${name}/${panel.id}: ${entryFile} -> ${panel.entry}`); - - await esbuild.build({ - entryPoints: [entryPath], - bundle: true, - outfile, - format: "esm", - platform: "browser", - target: "es2020", - jsx: "automatic", - external: ["react", "react-dom", "@hypr/ui", "@hypr/utils"], - minify: false, - sourcemap: true, - }); + console.log(`Building ${name}/${panel.id}: ${entryFile} -> ${panel.entry}`); + + try { + await esbuild.build({ + entryPoints: [entryPath], + bundle: true, + outfile, + format: "esm", + platform: "browser", + target: "es2020", + jsx: "automatic", + external: ["react", "react-dom", "@hypr/ui", "@hypr/utils"], + minify: false, + sourcemap: true, + }); + } catch (error) { + console.error( + `Failed to build panel ${name}/${panel.id}: ${entryPath} -> ${panel.entry}`, + error, + ); + process.exit(1); + }Please double‑check this against your esbuild version’s error semantics and recommended CLI patterns.
🧹 Nitpick comments (1)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
21-21: Consider Map lifecycle if extensions can be reloaded/uninstalled.
loadedPanelsonly ever grows; ifloadExtensionPanels()is called again after extensions change, stale entries will remain in the cache. If re-initialization is expected, clear the map first:export async function loadExtensionPanels(): Promise<void> { + loadedPanels.clear(); + const extensions = await listInstalledExtensions(); for (const ext of extensions) { for (const panel of ext.panels) { loadedPanels.set(panel.id, panel); } } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/desktop/src/components/main/body/extensions/registry.ts(3 hunks)extensions/build.mjs(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/components/main/body/extensions/registry.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/components/main/body/extensions/registry.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:13.593Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: packages/nango/.cursor/rules/nango.mdc:0-0
Timestamp: 2025-11-24T16:32:13.593Z
Learning: Applies to packages/nango/**/*.ts : Error handling must cover common failure scenarios
Applied to files:
extensions/build.mjs
⏰ 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). (3)
- 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 (2)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
3-7: Imports updated for panel metadata look correct.Using
PanelInfofrom@hypr/plugin-extensionskeeps this registry aligned with the backend API; no issues here.extensions/build.mjs (1)
53-72:buildAll()directory scan and sequencing look solid.Filtering dot directories/
node_modulesand building only whenextension.jsonexists is reasonable, and sequentialawait buildExtension(entry.name)keeps the pipeline simple and predictable.
yujonglee
changed the title
- Wire extensions build into turbo.json and pnpm-workspace.yaml - Fix property mismatch in index.tsx (check panelInfo?.entry instead of entry_path) - Call loadExtensionPanels() on app startup in Body component - Add getPanelInfoByExtensionId() to fix panel lookup by extension ID - Add guard against duplicate initialization in loadExtensionPanels() - Add error handling for fs.readFileSync/JSON.parse in build.mjs - Add error handling for esbuild.build call in build.mjs - Add top-level await for buildExtension() and buildAll() calls Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/components/main/body/index.tsx (1)
28-44: Trigger a re‑render after panels finish loading to avoid stale panel infoThe startup
useEffectcorrectly initializesloadExtensionPanels(), but because that function only mutates module‑level Maps, components likeTabContentExtensionwon’t re‑readpanelInfowhen the async load completes. For extension tabs that are already open on first render (e.g., from persisted tab state) this can leave them stuck showing “Extension not found”.One lightweight fix is to drive a dummy piece of state off the load:
import { useCallback, useEffect, useRef } from "react"; +import { useState } from "react"; @@ export function Body() { + const [, setPanelsLoadedVersion] = useState(0); @@ - useEffect(() => { - loadExtensionPanels(); - }, []); + useEffect(() => { + void (async () => { + await loadExtensionPanels(); + // Bump a version so descendants (including TabContentExtension) + // re-read panel info after panels are loaded. + setPanelsLoadedVersion((v) => v + 1); + })(); + }, []);You don’t need to use the
panelsLoadedVersionvalue anywhere; just updating it once ensures any already‑rendered extension tabs will re-render and see the populated panel registry.
🧹 Nitpick comments (2)
extensions/build.mjs (1)
16-23: Consider propagating manifest read/parse failures via a non‑zero exit codeRight now, if
extension.jsonis missing or invalid,buildExtension()logs and returns but the script still exits with code 0, even when invoked for a single extension. That makes CI/dev scripts think the build succeeded when it silently skipped that extension.A minimal pattern is to have
buildExtension/buildAllreturn a success flag and use it at the top level:-async function buildExtension(name) { +async function buildExtension(name): Promise<boolean> { const extensionDir = path.join(process.cwd(), name); const manifestPath = path.join(extensionDir, "extension.json"); if (!fs.existsSync(manifestPath)) { console.error(`Extension manifest not found: ${manifestPath}`); - return; + return false; } let manifest; try { const raw = fs.readFileSync(manifestPath, "utf-8"); manifest = JSON.parse(raw); } catch (err) { console.error(`Failed to read/parse manifest ${manifestPath}:`, err); - return; + return false; } for (const panel of manifest.panels || []) { // ... } console.log(`Built extension: ${name}`); -} + return true; +} -async function buildAll() { +async function buildAll(): Promise<boolean> { const entries = fs.readdirSync(process.cwd(), { withFileTypes: true }); + let hadFailures = false; for (const entry of entries) { // ... if (fs.existsSync(manifestPath)) { - await buildExtension(entry.name); + const ok = await buildExtension(entry.name); + if (!ok) { + hadFailures = true; + } } } } + + return !hadFailures; } -if (extensionName) { - await buildExtension(extensionName); -} else { - await buildAll(); -} +if (extensionName) { + const ok = await buildExtension(extensionName); + process.exit(ok ? 0 : 1); +} else { + const ok = await buildAll(); + process.exit(ok ? 0 : 1); +}This keeps individual manifest failures from aborting other builds, but still makes the overall script fail visibly when anything went wrong.
Also applies to: 99-103
apps/desktop/src/components/main/body/extensions/registry.ts (1)
21-23: Panel registry design is solid; consider future multi‑panel and refresh needsThe in‑memory registry (
loadedPanels,extensionPanels,panelsLoaded) plus:
getPanelInfo(panelId)getPanelInfoByExtensionId(extensionId)(first panel)loadExtensionPanels()with an idempotent guardgetLoadedPanels()gives a clean, minimal API for the current “one primary panel per extension” model.
Two things to keep in mind as this evolves (no change required now):
Multiple panels per extension
getPanelInfoByExtensionIdreturningpanels?.[0]is fine today, but once tabs become panel‑aware you’ll likely want either a(extensionId, panelId)lookup or agetPanelsByExtensionIdhelper to avoid overloading “first panel” semantics.Runtime extension changes / refresh
WithpanelsLoadedas a simple boolean, newly installed or removed extensions during a session won’t be visible until restart. If hot‑reloading extensions becomes a goal, a futurereloadExtensionPanels()that clears the Maps and resetspanelsLoaded(or accepts an invalidate flag) would make this component more flexible.Overall, the structure fits the current requirements and is a good base to grow from.
Also applies to: 65-101
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src/components/main/body/extensions/index.tsx(3 hunks)apps/desktop/src/components/main/body/extensions/registry.ts(3 hunks)apps/desktop/src/components/main/body/index.tsx(2 hunks)extensions/build.mjs(1 hunks)pnpm-workspace.yaml(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{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/components/main/body/index.tsxapps/desktop/src/components/main/body/extensions/registry.tsapps/desktop/src/components/main/body/extensions/index.tsx
**/*.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/components/main/body/extensions/registry.ts
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:13.593Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: packages/nango/.cursor/rules/nango.mdc:0-0
Timestamp: 2025-11-24T16:32:13.593Z
Learning: Applies to packages/nango/**/*.ts : Error handling must cover common failure scenarios
Applied to files:
extensions/build.mjs
🧬 Code graph analysis (3)
apps/desktop/src/components/main/body/index.tsx (1)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
loadExtensionPanels(76-97)
apps/desktop/src/components/main/body/extensions/registry.ts (4)
crates/extensions-runtime/src/manifest.rs (1)
panels(90-92)packages/tiptap/src/shared/extensions/index.ts (1)
extensions(188-188)apps/desktop/src/types/extensions.d.ts (1)
ExtensionViewProps(3-6)extensions/hello-world/ui.tsx (1)
ExtensionViewProps(13-16)
apps/desktop/src/components/main/body/extensions/index.tsx (1)
apps/desktop/src/components/main/body/extensions/registry.ts (1)
getPanelInfoByExtensionId(69-74)
🔇 Additional comments (3)
pnpm-workspace.yaml (1)
5-5: The review comment is incorrect and should be resolved.The workspace configuration is correct as written. While the
extensions/directory contains a subdirectory (hello-world/), it does not have its ownpackage.jsonfile. Thehello-worlddirectory is a subdirectory within the extensions package itself, not a separate workspace package. Therefore, using the exact path"extensions"is the correct configuration—matching the single workspace package at that location. The glob pattern"extensions/*"would only be appropriate if subdirectories contained their ownpackage.jsonfiles (as they do underapps/,packages/, andplugins/).turbo.json (1)
14-14: Build outputs glob change looks goodTracking
*/dist/**alongsidestorybook-static/**is consistent with the new per‑package build artifacts and keeps Turbo’s caching behavior aligned with the actual outputs.apps/desktop/src/components/main/body/extensions/index.tsx (1)
17-18: TabContentExtension messaging and panelInfo usage look consistentUsing
getPanelInfoByExtensionId(tab.extensionId)to distinguish between:
- a known panel without a registered UI component, and
- a missing extension ID,
and then showing
panelInfo.entrywhen available, matches the registry’s current shape and provides clearer debugging info.Once the Body
useEffecttriggers a re-render afterloadExtensionPanels()completes (see comment inapps/desktop/src/components/main/body/index.tsx), this UI will correctly update for tabs hydrated on startup as well.Also applies to: 90-108
|
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: 0
🧹 Nitpick comments (1)
apps/desktop/src/routes/app/devtool.tsx (1)
17-17: Extensions devtool wiring looks good; consider de-hardcoding the target extension.The
useTabsintegration andopenNew({ type: "extension", extensionId: "hello-world" })call are consistent with the tab store usage and are fine as a smoke test. Longer term, you may want to either (a) derive the extension(s) listed here from the extension/Panel registry, or (b) at least centralize"hello-world"as a shared constant so the devtool doesn’t drift if the manifest id changes.Also applies to: 106-106, 198-218
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
plugins/extensions/permissions/autogenerated/commands/get_extension.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/autogenerated/commands/get_extensions_dir.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/autogenerated/reference.mdis excluded by!plugins/**/permissions/**plugins/extensions/permissions/default.tomlis excluded by!plugins/**/permissions/**plugins/extensions/permissions/schemas/schema.jsonis excluded by!plugins/**/permissions/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (4)
apps/desktop/src-tauri/capabilities/default.json(1 hunks)apps/desktop/src/routes/app/devtool.tsx(3 hunks)plugins/AGENTS.md(1 hunks)plugins/extensions/build.rs(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- plugins/AGENTS.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/routes/app/devtool.tsx
🧬 Code graph analysis (1)
apps/desktop/src/routes/app/devtool.tsx (1)
apps/desktop/src/store/zustand/tabs/index.ts (1)
useTabs(29-38)
⏰ 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 (3)
apps/desktop/src-tauri/capabilities/default.json (2)
72-72: The review comment is based on incorrect assumptions about the repository structure.Verification shows that only one capability definition file exists in the repository:
apps/desktop/src-tauri/capabilities/default.json. There are no role-specific, feature-gated, or security-sensitive capability sets to review. Theextensions:defaultpermission at line 72 is appropriately placed in the sole capability file, and no other files require this permission.
72-72:extensions:defaultcapability is properly integrated; no action needed.The capability is correctly added to the permission array in
apps/desktop/src-tauri/capabilities/default.jsonand follows Tauri's plugin capability naming convention (<plugin_name>:default). The extensions plugin is fully implemented atplugins/extensions/with all commands defined, and the capability file is the only location where plugin capabilities are declared in this project. The schema is auto-generated from these capability definitions, so no additional configuration is required.plugins/extensions/build.rs (1)
6-7: Commands are properly implemented and integrated; no frontend consumption yet (expected for new API).Both
get_extensions_dirandget_extensioncommands are fully implemented incommands.rswith complete error handling and logic, correctly registered inlib.rs, and properly declared inbuild.rs. The implementations follow the Tauri plugin pattern and integrate with the extension discovery system.Frontend usage was not detected, but this is expected since these are newly added APIs. The backend infrastructure is complete and ready for consumption.
…ofile section Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
162-173: Extension menu wiring looks good; consider avoiding hard‑coded demo values
handleClickExtensioncorrectly mirrors the existing patterns (invokesopenNew({ type: "extension", extensionId: "hello-world" })thencloseMenu), and the"Hello World"menu item is properly hooked up withPuzzleIcon.For long‑term UX, consider:
- Driving the label/icon from extension metadata instead of hard‑coding.
- Hiding/guarding this item if the
hello-worldextension (or a default panel for it) is not installed/loaded, so users don’t hit a dead tab in production.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/components/main/sidebar/profile/index.tsx(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{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/components/main/sidebar/profile/index.tsx
⏰ 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: ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: ci (macos, depot-macos-14)
- GitHub Check: ci (linux, depot-ubuntu-24.04-8)
- GitHub Check: fmt
- GitHub Check: Pages changed - hyprnote
🔇 Additional comments (1)
apps/desktop/src/components/main/sidebar/profile/index.tsx (1)
1-10: NewPuzzleIconimport is correctly wired
PuzzleIconis imported fromlucide-reactand used inmenuItems, so there’s no unused-import or typing concern here.
feat(extensions): implement Pattern A architecture with panels and lifecycle hooks
Summary
This PR implements the foundation for a VS Code-style extension system (Pattern A) where extensions define UI panels that render as tabs. Key changes:
ui: Option<String>withpanels: Vec<PanelDeclaration>and addedapi_versionfield for future compatibility checkingop_hypr_log_errorandop_hypr_log_warnops, exposed ashypr.log.info/error/warn()with backwards compatibility forhypr.log(msg)activate(context)which is called on load with extension metadata (id, path, manifest info)ExtensionInfonow includesapi_versionandpanels: Vec<PanelInfo>instead ofui_pathTab System Integration
loadExtensionPanels(),getPanelInfo(),getPanelInfoByExtensionId(),getLoadedPanels(), andregisterExtensionComponent()functions inregistry.tsTabContentExtensionnow shows panel info when a panel is registered but UI not loadedbuild.mjswith esbuild to bundle extension panel UIs (runpnpm -F @hypr/extensions build)PanelInfotypeUpdates since last revision
Addressed review comments:
turbo.jsonandpnpm-workspace.yamlsodist/ui.jsis produced during normal buildindex.tsx(condition now checkspanelInfo?.entryto match displayed value)loadExtensionPanels()call on app startup inBodycomponent withuseEffectgetPanelInfoByExtensionId()to fix panel lookup (tabs use extension ID, not panel ID)loadExtensionPanels()build.mjswith contextual error messagesesbuild.build()with proper exit codes on failurebuild.mjsopenNewthere didn't affect the main window's tabsReview & Testing Checklist for Human
"ui": "ui.tsx"to"panels": [...]. Verify no production extensions exist using the old formatregisterExtensionComponent()exists but nothing actually loads the bundled JS and registers it - end-to-end UI rendering is incompletereact,react-dom,@hypr/ui,@hypr/utilsas external - verify these are available at runtime when extension UI loadsactivate()but never callsdeactivate()- incomplete lifecycle supportSuggested test plan:
activate()is called (check logs for "Activating Hello World")pnpm -F @hypr/extensions build:hello-worldand verifydist/ui.jsis createdlist_extensionsand verify the newpanelsarray structure withentry_pathpopulatedNotes
This is foundational work for the Pattern A extension architecture. The infrastructure is in place but the end-to-end flow is not complete. Follow-up work needed:
registerExtensionComponent()@hypr/extension-panel-sdk) for UI-to-host communicationLink to Devin run: https://app.devin.ai/sessions/167d232223d84b18b68c889e733412f8
Requested by: yujonglee (yujonglee.dev@gmail.com) / @yujonglee