feat(a1): implement plugin system with 16 built-in plugins by Nafania · Pull Request #29 · Nafania/agent-zero

Nafania · 2026-03-29T15:49:18Z

Summary

Phase 2 of the upstream backport project — implements the full plugin system architecture adapted from upstream.

Plugin infrastructure: discovery, toggle system (.toggle-0/.toggle-1), config management, lifecycle hooks, asset serving
@extensible decorator: automatically generates _start/_end extension points around functions
16 built-in plugins: memory, code_execution, browser, search, scheduler, skills, vision, document_query, a2a, notifications, error_retry, infection_check, text_editor, chat_branching, plugin_installer, plugin_scan
Backward compatibility: 25 helper shims + 18 API shims preserve all existing import paths and route URLs
Fork customizations preserved: Cognee persistence, CDP monkeypatch, skills marketplace

Key new files

helpers/cache.py, helpers/yaml.py, helpers/plugins.py — core plugin infrastructure
helpers/extension.py — rewritten with @extensible, async/sync split, webui extensions
api/plugins.py — plugin management API
plugins/*/plugin.yaml — 16 plugin manifests

Changes

216 files changed, 11292 insertions, 9433 deletions
~45 files moved from root directories into plugins/ subdirectories
~50 test files updated with new import paths

Part of

Upstream backport project (Phase 2) — see docs/specs/2026-03-28-upstream-backport-design.md and docs/plans/2026-03-29-a1-plugin-system.md.

Test plan

All 2556 unit tests pass (0 failures, 2 skipped)
All backward-compat shims resolve correctly
Plugin discovery finds all 16 built-in plugins
CI path triggers updated to include plugins/**
AGENTS.md architecture docs updated
Docker integration test
Browser smoke test of core functions

Made with Cursor

@extensible

Phase 2 of the upstream backport project. Implements the full plugin system architecture adapted from upstream, with fork customizations preserved (Cognee persistence, CDP monkeypatch, skills marketplace). Infrastructure: - helpers/cache.py: thread-safe in-memory cache with pattern invalidation - helpers/yaml.py: thin PyYAML wrapper - helpers/plugins.py: plugin discovery, toggle, config, hooks, assets - helpers/extension.py: @extensible decorator, async/sync call_extensions, webui extensions, plugin path integration - helpers/subagents.py: include_plugins param, plugin agent merging - run_ui.py: plugin asset serving, plugin API handler registration - api/plugins.py: plugin management API endpoint Plugin migrations (12 existing modules → plugin directories): - memory (5 tools, 6 helpers, 6 extensions, 5 API handlers) - code_execution (2 tools, 4 helpers) - browser (1 tool, 3 helpers) - search (1 tool, 3 helpers) - scheduler (1 tool, 2 helpers, 6 API handlers) - skills (1 tool, 3 helpers, 3 API handlers, 3 extensions) - vision, document_query, a2a, notifications New plugins from upstream: - error_retry, infection_check (extension-only) - text_editor (file read/write/patch tool) - chat_branching (branch-from-message API) - plugin_installer (ZIP/Git install) - plugin_scan (stub) Backward compatibility: - 25 helper shims re-export from plugin locations - 18 API shims preserve original route paths - All 2556 tests pass (0 failures) Made-with: Cursor

@extensible

… example_agent plugin Closes all identified gaps between our plugin system and upstream: 1. @extensible decorator applied to: - agent.py: 32 methods (AgentContext, LoopData, Agent) - initialize.py: 6 functions - run_ui.py: 5 functions Fixed _get_agent() to handle mocked Agent classes (TypeError) and spec'd mock instances (hasattr guard). 2. api/plugins.py: added 6 upstream actions (list_configs, delete_config, delete_plugin, get_doc, run_init_script, get_init_exec) + toggle_plugin alias. Enhanced get_toggle_status to match upstream response format. 3. plugins/example_agent: reference plugin from upstream. 4. Updated plan doc with upstream verification checklist and marked all gaps as resolved. All 2556 tests pass (0 failures, 2 skipped). Docker smoke test: app starts and serves correctly. Made-with: Cursor

`from ... import *` skips _-prefixed names by convention, so `_get_cognee` was not available through the backward-compat shim. Added explicit imports and switched plugin-internal code to use the direct module path. Made-with: Cursor

Nafania

Code Review: feat(a1) — Plugin System

Overall Assessment

Solid implementation of a complex plugin architecture. The backward-compatibility layer is thorough, the @extensible decorator design is clean, and the upstream API surface compatibility is impressive (verified via the plan's checklist). Test count maintained at 2556 with 0 failures — great discipline.

Below are findings grouped by severity.

Important (should fix before merge)

1. call_plugin_hook() uses asyncio.run() inside an already running event loop
helpers/plugins.py:654 — If a hook is async, asyncio.run() will raise RuntimeError: This event loop is already running when invoked from Flask/uvicorn async context (which is the normal runtime path). nest_asyncio is applied in agent.py, but plugins.py doesn't import it, so the monkey-patch may not have been applied yet if plugins.py is used before agent.py is imported. Either:

Use the existing DeferredTask pattern, or
Add a sync/async split similar to call_extensions_sync/call_extensions_async, or
Import and apply nest_asyncio in plugins.py

2. Error retry plugin won't trigger
plugins/error_retry/extensions/python/message_loop_end/_80_error_retry.py:21 reads kwargs.get("exception"), but the call site in agent.py (call_extensions("message_loop_end", loop_data=self.loop_data)) never passes exception. The extension will silently do nothing. If this is intentional (stub for Phase 3/A3 rework), document it; otherwise wire up the exception kwarg.

3. Infection check extension reads wrong kwarg name
plugins/infection_check/extensions/python/tool_execute_after/_80_infection_check.py:22 reads kwargs.get("result"), but the call site in agent.py:1009 passes response=response, tool_name=tool_name — not result. Additionally, response is a Response object, not a string, so the isinstance(result, str) guard at line 23 would also fail. Net effect: this extension is inert. Fix the kwarg name to response and extract the message string from the Response object.

4. delete_config uses raw os.remove() instead of files.delete_file()
api/plugins.py:209 — All other file operations in the plugin system consistently use helpers.files wrappers. This breaks the abstraction and could behave differently on some platforms. Replace with files.delete_file(path).

Minor (can fix in follow-up)

5. get_enhanced_plugins_list() doesn't deduplicate user vs. builtin
helpers/plugins.py:123 — If a user plugin overrides a built-in (same name in usr/plugins/ and plugins/), both entries will appear in the returned list. get_plugins_list() correctly deduplicates via seen_names, but get_enhanced_plugins_list() doesn't share this logic.

6. toggle_area() in cache.py writes to _enabled_areas without lock
helpers/cache.py:17 — While Python's GIL makes simple dict writes effectively atomic, this is inconsistent with the rest of the module which acquires _lock for all mutations. Worth adding the lock for correctness.

7. chat_branching plugin has fragile history access
plugins/chat_branching/api/branch_chat.py:28 — agent.history[: message_index + 1] applies list slicing to a History object. If History doesn't implement __getitem__ with slice support, this will fail. Also new_ctx.agent0.history = list(history) replaces the typed History instance with a plain list, losing class behavior (e.g., new_topic(), output()).

8. install_from_git() doesn't validate URL protocol
plugins/plugin_installer/helpers/installer.py:46 — While using subprocess.run with a list prevents shell injection, there's no validation that repo_url is an HTTPS URL. A file://, ssh://, or crafted protocol could be used. Consider validating the scheme.

9. Plugin API handler registration in run_ui.py misses the @extensible decorator
run_ui.py:570-603 — The plugin API handler registration loop doesn't use the register_api_handler() helper defined at line 543. Instead it duplicates the auth/csrf wrapping logic inline. The duplication means any future changes to register_api_handler() won't apply to plugin APIs.

Looks Good

@extensible decorator — clean async/sync split, proper _get_agent extraction with TypeError/hasattr guards for mocked objects
helpers/cache.py — simple, thread-safe, pattern invalidation via fnmatch, toggle_area/toggle_global for testing
Backward-compat shims — underscore-prefixed re-exports for _get_cognee etc. is a nice catch (commit 3)
Plugin asset serving — proper is_in_dir path traversal prevention
helpers/yaml.py — thin wrapper, no over-engineering
17 plugin manifests discovered, 2556 tests passing

Questions

Is there a plan to add plugin.yaml JSON Schema validation (beyond Pydantic's model_validate)? Could help catch manifest typos early.
For plugin_scan — it's currently a manifest-only stub. Is this expected to be fleshed out in a later phase, or should it have at least a no-op scanner?

Nafania · 2026-03-29T16:48:51Z

+        return None
+
+    if asyncio.iscoroutinefunction(hook):
+        return asyncio.run(hook(*args, **kwargs))


asyncio.run() will raise RuntimeError: This event loop is already running when called from an async Flask/uvicorn route (which is the normal runtime). nest_asyncio is applied in agent.py but not guaranteed to be imported before plugins.py runs.

Consider an async/sync split similar to call_extensions_async/call_extensions_sync in extension.py, or at minimum import nest_asyncio here.

Nafania · 2026-03-29T16:49:04Z

+        if not retry_on_critical:
+            return
+
+        last_exception = kwargs.get("exception")


This extension reads kwargs.get("exception"), but the call site in agent.py monologue loop:

await self.call_extensions("message_loop_end", loop_data=self.loop_data)

never passes an exception kwarg. This means last_exception will always be None and the retry logic will never trigger.

If this is a Phase 3 (A3) stub that will be wired up once @extensible reworked the monologue loop to pass exception context, add a # TODO(a3) comment. Otherwise, wire the exception through.

Nafania · 2026-03-29T16:49:13Z

+            return
+
+        result = kwargs.get("result", "")
+        if not result or not isinstance(result, str):


Wrong kwarg name: this reads kwargs.get("result") but the call site in agent.py:1009 passes response=response, tool_name=tool_name. The kwarg is named response, not result. Also, response is a Response object (not a string), so the isinstance(result, str) check on line 23 would fail anyway.

Fix: change to kwargs.get("response") and extract the message string: result = getattr(response_obj, "message", "") or similar.

Nafania · 2026-03-29T16:49:20Z

+                return {"ok": True}
+            try:
+                os.remove(path)
+            except Exception as e:


Use files.delete_file(path) here instead of raw os.remove(path) for consistency with the rest of the plugin system. All other file operations go through the helpers.files abstraction.

Nafania · 2026-03-29T16:49:41Z

+            new_ctx = AgentContext(config=initialize_agent(), set_current=False)
+            agent = ctx.agent0
+
+            history = agent.history[: message_index + 1] if agent else []


agent.history[:message_index + 1] applies list slicing to a History object. If History does not implement __getitem__ with slice support, this will fail at runtime. Also, new_ctx.agent0.history = list(history) replaces the typed History instance with a plain list, losing class behavior (new_topic(), output(), etc.).

Consider using a proper History clone/copy method, or at minimum verify History.__getitem__ supports slices.

1. call_plugin_hook(): detect running event loop, apply nest_asyncio before asyncio.run() to prevent RuntimeError in async context 2. error_retry: add TODO(a3) — extension reads exception kwarg that the call site does not pass yet 3. infection_check: fix kwarg name (result → response), extract message string from Response object 4. delete_config: use files.delete_file() via deabsolute_path instead of raw os.remove() 5. get_enhanced_plugins_list(): add seen_names dedup like get_plugins_list() 6. cache.toggle_area/toggle_global: acquire _lock for consistency 7. chat_branching: use history.output() + hist_add_message() instead of unsupported History slicing 8. install_from_git(): validate URL scheme is HTTP(S) 9. plugin API registration: unify into register_api_handler() with route_prefix parameter, eliminating duplicated auth/csrf logic Made-with: Cursor

Nafania · 2026-03-29T17:45:12Z

Code Review — Second Pass

Important (should fix before merge)

5. send_frontend_reload_notification() will crash at runtime
helpers/plugins.py:615-628 — Calls NotificationManager.send_notification(..., id="plugins_frontend_reload"), but send_notification() does NOT accept an id parameter:

# Actual signature (notification.py:72):
def send_notification(type, priority, message, title="", detail="", display_time=3, group="") -> NotificationItem:

This will raise TypeError: send_notification() got an unexpected keyword argument 'id' whenever a plugin with webui extensions is toggled. Either add id (and **kwargs) to send_notification and pass it through to add_notification/NotificationItem, or remove the id= kwarg from the call.

6. Plugin code imports from backward-compat shims instead of direct paths
All 18 plugin tools/helpers import from helpers.* shims rather than from plugins.*.helpers.*. Examples:

Plugin file	Imports from (shim)	Should import from
`plugins/scheduler/tools/scheduler.py`	`helpers.task_scheduler`	`plugins.scheduler.helpers.task_scheduler`
`plugins/code_execution/tools/code_execution_tool.py`	`helpers.shell_local`, `helpers.shell_ssh`, `helpers.docker`	`plugins.code_execution.helpers.*`
`plugins/notifications/tools/notify_user.py`	`helpers.notification`	`plugins.notifications.helpers.notification`
`plugins/skills/helpers/skills_import.py`	`helpers.skills`	`plugins.skills.helpers.skills`
`plugins/scheduler/helpers/job_loop.py`	`helpers.task_scheduler`	`plugins.scheduler.helpers.task_scheduler`
(18 more files...)

This means:

Shims are load-bearing, not just backward-compat — plugins themselves depend on them
Removing shims in a future cleanup would break all plugins
Plugins have unnecessary import indirection through shims

Only one file in the entire plugins/ directory uses a direct import (infection_check/extensions/.../_80_infection_check.py → from plugins.infection_check.helpers.checker import ...).

Recommendation: Update all plugin-internal imports to use direct paths. The shims should only serve external consumers. This can be a follow-up commit.

Minor

7. @extensible async path has edge-case exception loss
helpers/extension.py:108-113 — If _call_original(data) catches a sync exception and returns _UNSET, data["result"] is still _UNSET. The next line (data["result"] = await data["result"]) tries to await _Unset, raising TypeError, which overwrites the original exception in data["exception"].

In practice this rarely fires (async function creation almost never throws synchronously), but if a _start extension corrupts data["args"], the original error is silently lost. Fix:

if _call_original(data) is not _UNSET:
    try:
        data["result"] = await data["result"]
    except Exception as e:
        data["exception"] = e

8. initialize_cognee() missing @extensible decorator
initialize.py:172 — All 5 other initialize_* functions (initialize_agent, initialize_chats, initialize_mcp, initialize_job_loop, initialize_preload, initialize_migration) are decorated with @extensible, but initialize_cognee() is not. This looks like an oversight.

First-pass findings still open

Items 1-4 from the first review still apply (asyncio.run in call_plugin_hook, inert error_retry/infection_check extensions, os.remove in delete_config). The send_notification TypeError (item 5 above) is particularly important since it affects the core plugin toggle flow.

Nafania · 2026-03-29T17:45:21Z

+        )
+
+    DeferredTask().start_task(_send_later)
+


Bug: send_notification() does not accept an id parameter. This will raise TypeError: send_notification() got an unexpected keyword argument 'id' at runtime when a plugin with webui extensions is toggled.

Either add id: str = "" (+ **kwargs) to send_notification() and pass it through to add_notification/NotificationItem, or remove the id= kwarg here.

send_notification() does not accept an id parameter — passing it would raise TypeError at runtime when toggling plugins with webui extensions. Made-with: Cursor

@extensible

6. Plugin imports: switch 26 imports from backward-compat shims to direct plugin module paths (plugins.memory.helpers.*, plugins. scheduler.helpers.*, plugins.a2a.helpers.*). Shims are now truly backward-compat only, not load-bearing for plugin internals. 7. @extensible _run_async: skip `await data["result"]` when _call_original already set data["exception"] — prevents TypeError from `await _UNSET` overwriting the original exception. 8. Add @extensible to initialize_cognee() for consistency with all other initialize_* functions. Made-with: Cursor

Nafania · 2026-03-29T18:12:43Z

+    """Extract a ZIP file into usr/plugins/ and return the plugin name."""
+    with tempfile.TemporaryDirectory() as tmp:
+        with zipfile.ZipFile(zip_path, "r") as zf:
+            zf.extractall(tmp)


Security: ZipSlip vulnerability. zf.extractall(tmp) does not validate that ZIP entry paths are confined to the target directory. A malicious ZIP could contain entries like ../../etc/crontab that escape the temp directory. Python docs explicitly warn: "It is possible that files are created outside of path, e.g. members that have absolute filenames starting with / or filenames with two dots ..."

Recommended fix: iterate members with zf.infolist() and validate each path before extraction, or use a safe-extract wrapper.

Nafania · 2026-03-29T18:12:55Z

+    if parsed.scheme not in ("https", "http"):
+        raise ValueError(f"Only HTTP(S) URLs are supported, got: {parsed.scheme!r}")
+
+    plugin_name = repo_url.rstrip("/").split("/")[-1].removesuffix(".git")


Bug: plugin_name can be empty or a path-traversal component. If repo_url ends with /.git (e.g. https://example.com/.git), then plugin_name becomes "" after removesuffix(".git"). This makes dest = usr/plugins/ — the plugins root. Then shutil.rmtree(dest) on line 49 would delete the entire user plugins directory.

Similarly, a URL like https://evil.com/.. would produce plugin_name = "..", making dest = usr/plugins/.. = usr/, and rmtree would wipe the user data directory.

Add a validation like:

if not plugin_name or plugin_name in (".", "..") or "/" in plugin_name: raise ValueError(f"Invalid plugin name derived from URL: {plugin_name!r}")

Nafania

Third Review Pass

Verified fixes from previous rounds — all 6 inline-commented issues from passes 1-2 have been addressed (nice work!). This pass focuses on areas not covered before.

New Findings

1. Security — ZipSlip vulnerability (plugins/plugin_installer/helpers/installer.py:13)
zf.extractall(tmp) does not validate that ZIP entry paths are confined to the target directory. A malicious ZIP could write files anywhere on the filesystem. See inline comment.

2. Security — Unsafe plugin_name from URL (plugins/plugin_installer/helpers/installer.py:45)
plugin_name derived from the URL can be empty ("") or "..", causing shutil.rmtree to delete the plugins root or usr/ directory. See inline comment.

3. Bug — _load_agent_data_from_dir() missing agent.yaml support (helpers/subagents.py:176)
_get_agents_list_from_dir() (line 83) checks agent.yaml first, then agent.json. But _load_agent_data_from_dir() (line 176) only tries agent.json and falls back to _context.md — it never reads agent.yaml. Agents distributed with agent.yaml (like the example_agent plugin) will show up in listings but lose their title/description/context when actually loaded.

4. Bug — get_enabled_plugins() ignores always_enabled flag (helpers/plugins.py:257)
get_enabled_plugins() evaluates toggle files but never checks the plugin metadata always_enabled flag. Meanwhile, get_toggle_state() (line 300) correctly short-circuits for always_enabled plugins. Result: UI shows "enabled" but the runtime may actually disable the plugin if a .toggle-0 file exists.

5. Architecture — Plugin-internal shim imports
Several plugins import from helpers/ shims (which re-export from plugins.*.helpers.*) instead of importing directly from their own plugin module paths:

plugins/search/tools/search_engine.py:7 → from helpers.searxng import ...
plugins/notifications/tools/notify_user.py:3 → from helpers.notification import ...
plugins/code_execution/tools/code_execution_tool.py:10 → from helpers.docker import ...
plugins/browser/tools/browser_agent.py:10 → from helpers.browser_use import ...
plugins/skills/helpers/skills_import.py:13 → from helpers.skills import ...

Shims should only be consumed by external code for backward compatibility. When plugins themselves depend on them, the shims become load-bearing and can never be removed.

6. Nit — Typo (helpers/subagents.py:293)
Function name get_default_promp_file_names → should be get_default_prompt_file_names.

1. Security: ZipSlip — validate ZIP member paths before extraction 2. Security: validate plugin_name from URL (reject empty, "..", "/") 3. Bug: _load_agent_data_from_dir() now reads agent.yaml (not just agent.json), matching _get_agents_list_from_dir() behavior 4. Bug: get_enabled_plugins() short-circuits for always_enabled plugins, matching get_toggle_state() behavior 5. Plugin imports: switch 6 more plugin-internal imports from shims to direct paths (search, browser, skills, code_exec, notifications) 6. Typo: get_default_promp_file_names → get_default_prompt_file_names Made-with: Cursor

@extensible

- Restructure api/plugins.py: monolithic process() → per-method @extensible dispatch (13 methods), matching upstream structure - Rename API actions: run_init_script → run_execute_script, get_init_exec → get_execute_record (upstream naming) - Script/record filenames: initialize.py → execute.py, init_exec.json → execute_record.json - Add `default` param to call_plugin_hook() — upstream signature compat - Add try/except error handling in call_plugin_hook (safe_call substitute) - Fix clear_plugin_cache(plugin_names=) — add parameter, expand areas - Fix after_plugin_change(python_change=) — add parameter, TODO(a3) for refresh_plugin_modules - Add @extensible handle_exception() to AgentContext and Agent - Update _process_chain to use extensible handle_exception hook - Fix last 4 intra-plugin shim imports (code_execution, document_query) Made-with: Cursor

Made-with: Cursor

Nafania · 2026-03-30T08:26:04Z

Code Review — Fourth Pass

Verified all fixes from rounds 1-3 — nicely addressed. This pass focuses on areas not covered before.

Strengths

Backward compatibility layer is thorough. The 25 helper shims + 18 API shims are minimal-overhead and well-structured. The explicit re-export of underscore-prefixed names (_get_cognee) shows attention to edge cases.
@extensible decorator is well-engineered. The async/sync split, _UNSET sentinel for short-circuit detection, and _start/_end extension points with mutable args/kwargs/result/exception are a clean design.
Thread-safe cache module. helpers/cache.py uses RLock consistently, supports glob-based pattern clearing, and has per-area toggle for testing.
Security hardening from previous rounds — ZipSlip, URL validation, is_in_dir for assets, _validate_plugin_name in installer.
2556 tests passing across 223 file changes is strong evidence of migration correctness.

New Findings

Important (Should Fix)

1. find_plugin_dir() — no input validation for path traversal

helpers/plugins.py:194 — plugin_name comes from user input (API POST body in api/plugins.py) and is used directly in os.path.join. A value like ../../etc would construct:

get_abs_path("usr", "plugins", "../../etc", "plugin.yaml")
→ /project/usr/plugins/../../etc/plugin.yaml
→ /project/etc/plugin.yaml

This escapes the plugins directory. While the impact is limited by auth and the requirement that plugin.yaml exist at the traversed path, _run_execute_script could execute arbitrary Python files at a traversed location if a plugin.yaml happens to exist there.

The _validate_plugin_name function already exists in plugins/plugin_installer/helpers/installer.py but is only used during installation, not in the core lookup.

Fix: Add validation at the top of find_plugin_dir:

def find_plugin_dir(plugin_name: str) -> str | None:
    if not plugin_name or "/" in plugin_name or "\\" in plugin_name or plugin_name in (".", ".."):
        return None

Or better yet, move _validate_plugin_name into this module and call it here — defense in depth for all current and future callers.

2. _get_config() crashes with TypeError on non-existent plugin

api/plugins.py:87-88 — When find_plugin_assets(...) returns [] (no matching config entries), execution falls to the else branch. Here, plugins.find_plugin_dir(plugin_name) returns None for an unknown plugin, and files.get_abs_path(None, ...) raises:

TypeError: expected str, bytes or os.PathLike object, not NoneType

The outer handle_request catches it as a 500, but this should be a clean 404.

Fix:

        else:
            settings = plugins.get_plugin_config(plugin_name, agent=None) or {}
            plugin_dir = plugins.find_plugin_dir(plugin_name)
            if plugin_dir:
                default_path = files.get_abs_path(plugin_dir, plugins.CONFIG_DEFAULT_FILE_NAME)
                path = default_path if files.exists(default_path) else ""
            else:
                path = ""

3. 8 remaining plugin-internal shim imports

Several plugin files still import from shim helpers instead of directly from their own or sibling plugin:

plugins/search/tools/search_engine.py:3 → from helpers import memory, perplexity_search, duckduckgo_search
plugins/code_execution/helpers/shell_local.py:7 → from helpers import tty_session
plugins/skills/* — multiple files import skills or skills_cli via shims

These are plugin-internal files that should use direct imports. The shims should only serve external consumers for backward compatibility. These make the shims load-bearing and can never be removed while plugins depend on them.

Fix: Change to direct imports:

# Instead of: from helpers import tty_session
from plugins.code_execution.helpers import tty_session

Minor (Nice to Have)

4. No tests for new core infrastructure

The most critical new modules have zero direct test coverage:

Module	Lines	Tests
`helpers/plugins.py`	684	0
`helpers/cache.py`	73	0
`api/plugins.py`	358	0
`plugins/plugin_installer/helpers/installer.py`	76	0

Also, tests/helpers/test_extension.py doesn't cover the new @extensible decorator — only the pre-existing Extension class. As the new foundation, these are the highest-risk untested modules. Acceptable as a follow-up, but should be prioritized.

5. call_plugin_hook silently swallows all exceptions

helpers/plugins.py:682 — bare except Exception: return default can hide TypeError, ImportError, and programming errors in hook implementations. Already marked TODO(a3) for functions.safe_call, just flagging for visibility.

Assessment

Ready to merge: Yes, with fixes for #1 and #2.

Core architecture is sound, backward compat is solid, and three prior rounds caught the biggest issues. The find_plugin_dir validation (#1) is defense-in-depth behind auth but worth hardening before merge. The _get_config TypeError (#2) is a straightforward runtime bug with a quick fix. The remaining shim imports (#3) and test gaps (#4) are acceptable as follow-up tasks.

1. find_plugin_dir(): reject plugin_name with /, \, ".", ".." to prevent path traversal via crafted API input 2. _get_config(): handle find_plugin_dir() returning None instead of passing it to get_abs_path() (was TypeError, now returns empty path) 3. search_engine.py: remove dead shim imports (memory, perplexity_search, duckduckgo_search) — none were used in the file 4. shell_local.py: import tty_session from direct plugin path instead of backward-compat shim Made-with: Cursor

plugins/skills/api/skills.py, plugins/skills/tools/skills_tool.py, plugins/skills/api/skill_install.py — import skills and skills_cli from plugins.skills.helpers.* instead of backward-compat shims. Made-with: Cursor

helpers/projects.py exposes get_project_meta_folder(), not get_project_meta(). Three call sites in find_plugin_assets() and determine_plugin_asset_path() used the wrong name, causing AttributeError at runtime when plugin toggle/config was queried with a project context. Made-with: Cursor

Nafania added 3 commits March 29, 2026 18:48

Nafania commented Mar 29, 2026

View reviewed changes

Nafania added 2 commits March 29, 2026 20:57

fix(a1): remove unsupported id= kwarg from send_notification() call

75bd49b

send_notification() does not accept an id parameter — passing it would raise TypeError at runtime when toggling plugins with webui extensions. Made-with: Cursor

Nafania commented Mar 29, 2026

View reviewed changes

Nafania added 3 commits March 29, 2026 21:20

docs(a1): update plan with API compat fix results and A3 deferral list

a13bc01

Made-with: Cursor

Nafania added 3 commits March 30, 2026 11:33

Nafania merged commit 6b7de1c into main Mar 30, 2026
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(a1): implement plugin system with 16 built-in plugins#29

feat(a1): implement plugin system with 16 built-in plugins#29
Nafania merged 12 commits intomainfrom
upstream/a1-plugin-system

Nafania commented Mar 29, 2026

Uh oh!

Nafania left a comment

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania commented Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania Mar 29, 2026

Uh oh!

Nafania left a comment

Uh oh!

Nafania commented Mar 30, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Nafania commented Mar 29, 2026

Summary

Key new files

Changes

Part of

Test plan

Uh oh!

Nafania left a comment

Choose a reason for hiding this comment

Code Review: feat(a1) — Plugin System

Overall Assessment

Important (should fix before merge)

Minor (can fix in follow-up)

Looks Good

Questions

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nafania commented Mar 29, 2026

Code Review — Second Pass

Important (should fix before merge)

Minor

First-pass findings still open

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nafania left a comment

Choose a reason for hiding this comment

Third Review Pass

New Findings

Uh oh!

Nafania commented Mar 30, 2026

Code Review — Fourth Pass

Strengths

New Findings

Important (Should Fix)

Minor (Nice to Have)

Assessment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant