Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation for the OmniRoute plugin system, adding guides for plugin development, the marketplace, and updating the SDK reference. The feedback highlights several inconsistencies between the newly added documentation and the actual codebase implementation. Specifically, the dev mode documentation includes a cross-platform path separator fix that is missing in the source code, and the marketplace documentation fails to list the theme-manager plugin, which is already part of the verified seed registry in the implementation.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
|
||
| watcher = watch(pluginDir, { recursive: true }, (_eventType, filename) => { | ||
| if (!filename) return; | ||
| const pluginName = filename.replace(/\\/g, "/").split("/")[0]; |
There was a problem hiding this comment.
The documented code here includes a cross-platform path separator fix (filename.replace(/\\/g, "/")), but the actual implementation in src/lib/plugins/devMode.ts (line 30) still uses filename.split("/")[0]. The actual source file should be updated to include this fix so that dev mode works correctly on Windows.
| The local seed registry ships with **3 verified plugins** maintained by the OmniRoute team: | ||
|
|
||
| | Plugin | Tags | Description | | ||
| |--------|------|-------------| | ||
| | `request-logger` | logging, debugging | Logs all requests and responses with timing | | ||
| | `rate-limiter` | rate-limit, security | Per-model rate limiting with sliding window | | ||
| | `cost-tracker` | analytics, cost | Track token costs per request and per model | | ||
|
|
||
| All three are `verified: true` and `license: MIT`. |
There was a problem hiding this comment.
The documentation states that the local seed registry ships with 3 verified plugins, but the actual implementation in src/lib/plugins/marketplace.ts contains 4 verified plugins, including theme-manager. Please update the documentation to include theme-manager.
| The local seed registry ships with **3 verified plugins** maintained by the OmniRoute team: | |
| | Plugin | Tags | Description | | |
| |--------|------|-------------| | |
| | `request-logger` | logging, debugging | Logs all requests and responses with timing | | |
| | `rate-limiter` | rate-limit, security | Per-model rate limiting with sliding window | | |
| | `cost-tracker` | analytics, cost | Track token costs per request and per model | | |
| All three are `verified: true` and `license: MIT`. | |
| The local seed registry ships with **4 verified plugins** maintained by the OmniRoute team: | |
| | Plugin | Tags | Description | | |
| |--------|------|-------------| | |
| | `request-logger` | logging, debugging | Logs all requests and responses with timing | | |
| | `rate-limiter` | rate-limit, security | Per-model rate limiting with sliding window | | |
| | `cost-tracker` | analytics, cost | Track token costs per request and per model | | |
| | `theme-manager` | theme, ui, css, customization | Dynamic UI theme management via CSS variable injection | | |
| All four are `verified: true` and `license: MIT`. |
|
|
||
| // List all seed plugins | ||
| const allPlugins = await listMarketplacePlugins(); | ||
| console.log(allPlugins); // [request-logger, rate-limiter, cost-tracker] |
There was a problem hiding this comment.
| | Plugin | Author | Verified | License | Description | | ||
| |--------|--------|----------|---------|-------------| | ||
| | `request-logger` | omniroute | ✅ | MIT | Logs all requests and responses with timing | | ||
| | `rate-limiter` | omniroute | ✅ | MIT | Per-model rate limiting with sliding window | | ||
| | `cost-tracker` | omniroute | ✅ | MIT | Track token costs per request and per model | |
There was a problem hiding this comment.
Please update the verified plugins table to include the theme-manager plugin, which is present in the actual SEED_REGISTRY in src/lib/plugins/marketplace.ts.
| | Plugin | Author | Verified | License | Description | | |
| |--------|--------|----------|---------|-------------| | |
| | `request-logger` | omniroute | ✅ | MIT | Logs all requests and responses with timing | | |
| | `rate-limiter` | omniroute | ✅ | MIT | Per-model rate limiting with sliding window | | |
| | `cost-tracker` | omniroute | ✅ | MIT | Track token costs per request and per model | | |
| | Plugin | Author | Verified | License | Description | | |
| |--------|--------|----------|---------|-------------| | |
| | `request-logger` | omniroute | ✅ | MIT | Logs all requests and responses with timing | | |
| | `rate-limiter` | omniroute | ✅ | MIT | Per-model rate limiting with sliding window | | |
| | `cost-tracker` | omniroute | ✅ | MIT | Track token costs per request and per model | | |
| | `theme-manager` | omniroute | ✅ | MIT | Dynamic UI theme management via CSS variable injection | |
|
|
||
| - Seed plugins have initial ratings set by the OmniRoute team: `request-logger: 5`, `rate-limiter: 5`, `cost-tracker: 4` | ||
| - No user ratings yet | ||
| - All plugins are `verified: true` by definition |
There was a problem hiding this comment.
Please update the seed plugins list to include the initial rating for the theme-manager plugin (which is 5 in src/lib/plugins/marketplace.ts).
| - All plugins are `verified: true` by definition | |
| - Seed plugins have initial ratings set by the OmniRoute team: `request-logger: 5`, `rate-limiter: 5`, `cost-tracker: 4`, `theme-manager: 5` |
|
Thanks @oyi77 — and thank you for fixing several things from #3452: the I still can't merge it, though, because two claims don't match the source — and one is a regression from #3452: 1. Dev mode is documented as a working feature, but it isn't wired up. The "Starting Dev Mode" section presents
2. Everything else (core hooks list, |
|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
|
All the requested changes have been addressed in the recent commits:
Ready for merge! 🚀 |
…xpansion (cheaper-inference#3624) v3.8.21 broadened package.json files to include open-sse/ and src/*/* directories for TypeScript-first imports, but pack-artifact-policy.ts was not updated. Add the new directories to PACK_ARTIFACT_ROOT_ALLOWED_PATH_PREFIXES. Also increase execFileSync maxBuffer to 64 MB to fix ENOBUFS on large packs.
* chore(release): open v3.8.22 development cycle * refactor(dashboard): extract ProviderDetailPageClient — cheaper-inference#3501 Phase 0 (cheaper-inference#3633) cheaper-inference#3501 Phase 0: extract ProviderDetailPageClient + smoke test. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * refactor(dashboard): extract auth-import modals — cheaper-inference#3501 Phase 1a (cheaper-inference#3634) cheaper-inference#3501 Phase 1a: extract 3 auth-import modal clusters. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * fix(db): reclassify localDb unexported modules as intentionally-internal (cheaper-inference#3499) (cheaper-inference#3635) Closes cheaper-inference#3499 — reclassify localDb unexported modules as intentionally-internal (audit + honest gate framing). * refactor(db): move call_logs aggregations into callLogStats db module (cheaper-inference#3500) (cheaper-inference#3636) cheaper-inference#3500 slice 1: call_logs aggregations → src/lib/db/callLogStats.ts (Rule #5). Byte-identical queries; TDD 6/6. * refactor(dashboard): extract EditCompatibleNodeModal — cheaper-inference#3501 Phase 1b (cheaper-inference#3638) cheaper-inference#3501 Phase 1b: extract EditCompatibleNodeModal (cycle-safe via leaf constants module). Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * refactor(db): move community_servers SQL into gamification db module (cheaper-inference#3500 slice 3) (cheaper-inference#3639) cheaper-inference#3500 slice 3: community_servers SQL → gamification db module. * refactor(db): move usage_history SQL into usageAnalytics module (cheaper-inference#3500 slice 2) (cheaper-inference#3644) cheaper-inference#3500 slice 2: usage_history/daily_usage_summary SQL → usageAnalytics db module. * refactor(db): move skills UPDATE + db-backups SQL into db modules (cheaper-inference#3500 slice 5) (cheaper-inference#3647) cheaper-inference#3500 slice 5: skills UPDATE (allowlist) + db-backups SQL → db modules. * refactor(db): move usage_logs/semantic_cache/proxy_logs SQL into db modules (cheaper-inference#3500 slice 4) (cheaper-inference#3648) cheaper-inference#3500 slice 4: usage_logs/semantic_cache/proxy_logs SQL → db modules. All internal routes done (2 external by-design remain). * chore(db-gate): reclassify external-DB reads, fully close cheaper-inference#3500 (cheaper-inference#3649) Closes cheaper-inference#3500: reclassify external-DB reads; all internal raw-SQL migrated to db/ modules. * refactor(dashboard): extract pure helpers to providerPageHelpers — cheaper-inference#3501 Phase 2 (cheaper-inference#3653) cheaper-inference#3501 Phase 2: extract pure helpers to providerPageHelpers (leaf, cycle-safe). Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * refactor(dashboard): extract remaining shared helpers to providerPageHelpers — cheaper-inference#3501 Phase 2b (cheaper-inference#3658) cheaper-inference#3501 Phase 2b: extract remaining shared helpers to providerPageHelpers (leaf, cycle-safe). Heavy modals unblocked. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * fix(reasoning): replay reasoning_content on plain DeepSeek turns (cheaper-inference#1682) (cheaper-inference#3632) Integrated into release/v3.8.22 * fix(kiro): route enterprise IAM Identity Center accounts to their regional endpoint (cheaper-inference#3631) Integrated into release/v3.8.22 * refactor: small code cleanup (cheaper-inference#3523) Integrated into release/v3.8.22 * fix(combo): skip same-provider targets on 408/500/502/503/504/524 errors (cheaper-inference#3637) Integrated into release/v3.8.22 — circuit-breaker guard added in review (#1731v2) * feat(providers): add MiMoCode free-tier provider with bootstrap JWT auth (cheaper-inference#3659) Integrated into release/v3.8.22 — page.tsx conflict resolved + NoAuthAccountCard re-applied to ProviderDetailPageClient in review. MiMoCode endpoint validated live. * Log Responses WebSocket calls in history (cheaper-inference#3616) Integrated into release/v3.8.22 — Codex Responses WebSocket call history logging. * Add Claude Code routing preference for unprefixed Claude models (cheaper-inference#3540) Integrated into release/v3.8.22 — page.tsx conflict resolved (re-applied toggle to ProviderDetailPageClient) + disable-test updated for catalog drift in review. * docs(changelog): credit cheaper-inference#3632/cheaper-inference#3631/cheaper-inference#3637/cheaper-inference#3659/cheaper-inference#3540/cheaper-inference#3616/cheaper-inference#3523 (v3.8.22 targeted review round) * fix(mimocode): add required authHeader:"none" to registry entry (cheaper-inference#3659 follow-up) The mimocode RegistryEntry omitted the required authHeader field, which broke typecheck:core (TS2741). Match the no-auth convention (authType:"none" + authHeader:"none") used by veoaifree-web and other free providers. Follow-up to cheaper-inference#3659 (@pizzav-xyz). * fix(responses): detect stream readiness for tool-call-only and object-less chunks (cheaper-inference#3612) (cheaper-inference#3661) Closes cheaper-inference#3612 * fix(mitm): remove duplicated 'Command failed:' error prefix (cheaper-inference#3641) (cheaper-inference#3662) Closes cheaper-inference#3641 * fix(cli): honor HERMES_HOME for Hermes Agent config path (cheaper-inference#3628) (cheaper-inference#3663) Closes cheaper-inference#3628 * fix(api): fetch live OpenCode model catalog for no-auth model picker (cheaper-inference#3611) (cheaper-inference#3664) Closes cheaper-inference#3611 * fix(api): flag provider topology error state by current status, not stale history (cheaper-inference#3619) (cheaper-inference#3666) Closes cheaper-inference#3619 * fix(electron): launch peer-stamping server-ws.mjs entrypoint to avoid 403 LOCAL_ONLY (cheaper-inference#3386) (cheaper-inference#3665) Closes cheaper-inference#3386 * fix(dashboard): restore home topology live in-flight pulse (cheaper-inference#3507) (cheaper-inference#3667) Closes cheaper-inference#3507 * fix(oauth): name Kiro/AWS auto-imported accounts and dedupe by profileArn (cheaper-inference#3615) (cheaper-inference#3671) Closes cheaper-inference#3615 * fix(resilience): clear stale transient connection cooldowns on startup (cheaper-inference#3625) (cheaper-inference#3672) Closes cheaper-inference#3625 * fix(i18n): use logical CSS direction utilities for sidebar and key overlays (RTL cheaper-inference#3541) (cheaper-inference#3670) Closes cheaper-inference#3541 * fix(dashboard): honor auto-hide and switch to visible filter on passthrough Test-all (cheaper-inference#3610) (cheaper-inference#3669) Closes cheaper-inference#3610 * refactor(dashboard): extract AddApiKeyModal + EditConnectionModal — cheaper-inference#3501 Phase 1c (cheaper-inference#3674) cheaper-inference#3501 Phase 1c: extract AddApiKeyModal, EditConnectionModal, WebSessionCredentialGuide into components/; god-component 10,166->8,092 LOC. Reconciles the v3.8.22 file-size drift for this file. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * docs(changelog): reconcile v3.8.22 — credit cheaper-inference#3621/cheaper-inference#3622 + MiMoCode follow-up roll-up * refactor(dashboard): extract ConnectionRow + ModelCompatPopover + SiliconFlowEndpointModal — cheaper-inference#3501 Phase 1d (cheaper-inference#3676) cheaper-inference#3501 Phase 1d: god-component 8,092->6,838 LOC. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * feat(obsidian): add WebDAV config route + encrypt creds at rest (cheaper-inference#3485 part 1) (cheaper-inference#3677) Part 1 of cheaper-inference#3485. Adds /api/settings/obsidian/webdav (GET/POST/DELETE) wiring the ready obsidianSync lib, encrypts webdav password + obsidian token at rest, removes the duplicate UI block, drops the KNOWN_MISSING entry. WebDAV file server is part 2. * feat(obsidian): add /api/v1/webdav file server for Obsidian vault sync (cheaper-inference#3485 part 2) (cheaper-inference#3678) Part 2 of cheaper-inference#3485. WebDAV server (PROPFIND/GET/PUT/DELETE/MKCOL/MOVE/OPTIONS) handled in the custom server layer (standalone-server-ws.mjs) since the App Router cannot export WebDAV methods. Basic-Auth (constant-time), path-traversal hardened, password decrypt ported from encryption.ts (parity-tested), DATA_DIR resolution parity-tested against dataPaths.ts. End-to-end Obsidian-over-Tailscale validation is a live VPS step (Rule #18). * fix(combo): stop premature context compaction — real auto-combo windows + per-target compression limit (cheaper-inference#3680) Integrated into release/v3.8.22 * feat(dashboard): deactivate/activate accounts from the quota overview (cheaper-inference#3675) Integrated into release/v3.8.22 * fix(dashboard): close review gaps in bulk provider connection actions (cheaper-inference#3271 follow-up) (cheaper-inference#3673) Integrated into release/v3.8.22 — page.tsx conflict (god-component split cheaper-inference#3501) resolved by re-applying the bulk-action deltas to ProviderDetailPageClient.tsx * refactor(dashboard): extract useModelCompatState hook + model sections — cheaper-inference#3501 Phase 1e (cheaper-inference#3683) cheaper-inference#3501 Phase 1e: extract useModelCompatState hook (unblocks the model sections) + ModelRow/PassthroughModelsSection/PassthroughModelRow/CustomModelsSection/CompatibleModelsSection. god-component 6,838->4,921 LOC. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * refactor(dashboard): extract useProviderConnections/Settings/Models hooks — cheaper-inference#3501 Phase 1f (cheaper-inference#3684) cheaper-inference#3501 Phase 1f: god-component 4,948->4,062 LOC. Connection state+handlers, settings, and model metadata moved into hooks/. Co-authored-by: oyi77 <oyi77@users.noreply.github.com> * chore(release): v3.8.22 CHANGELOG + env-doc sync - Set release date in CHANGELOG [3.8.22] to 2026-06-11 - Add HERMES_HOME to .env.example (from cheaper-inference#3628/cheaper-inference#3663) - Add HERMES_HOME + OMNIROUTE_PREFER_CLAUDE_CODE_FOR_UNPREFIXED_CLAUDE_MODELS to ENVIRONMENT.md (cheaper-inference#3628/cheaper-inference#3540) * docs(changelog): credit cheaper-inference#3673 + cheaper-inference#3675 — leninejunior bulk-actions + quota-toggle --------- Co-authored-by: oyi77 <oyi77@users.noreply.github.com> Co-authored-by: Abhishek Divekar <adivekar@utexas.edu> Co-authored-by: NOXX - Commiter <artur1992123@mail.ru> Co-authored-by: Nicolas Lorin <androw95220@gmail.com> Co-authored-by: Hernan Javier Ardila Sanchez <hjasgr@gmail.com> Co-authored-by: PizzaV <103120356+pizzav-xyz@users.noreply.github.com> Co-authored-by: kkkayye <98376609+kkkayye@users.noreply.github.com> Co-authored-by: Witroch4 <witalo_rocha@hotmail.com> Co-authored-by: Lenine Júnior <lenine@engrene.com.br>
…ence#3687) server-ws.mjs imports ./webdav-handler.mjs but the assembleStandalone pipeline did not copy it from scripts/dev/, causing a startup crash on any fresh install of v3.8.22 (ERR_MODULE_NOT_FOUND). Fix: add the copy entry to assembleStandalone.mjs, add it to APP_STAGING_ALLOWED_EXACT_PATHS and PACK_ARTIFACT_REQUIRED_PATHS in pack-artifact-policy.ts, and add a regression test. Hotfix validated live on VPS (192.168.0.15): server-ws.mjs resolves the module and the process starts healthy after the file was deployed.
…aper-inference#3691) Integrated into release/v3.8.23
…ce#3690) Integrated into release/v3.8.23
…heaper-inference#3689) Integrated into release/v3.8.23
…ution (cheaper-inference#3692) Integrated into release/v3.8.23
…e#3698) Integrated into release/v3.8.23
Integrated into release/v3.8.23
…per-inference#3685) (cheaper-inference#3702) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…per-inference#3696) (cheaper-inference#3703) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…er-inference#3660) Integrated into release/v3.8.23
…API (cheaper-inference#3712) Integrated into release/v3.8.23. Vertex dynamic model discovery — surfaces image models (imagen-*, gemini-*-image), embeddings and audio from the live Generative Language catalog, with cached→static fallback and the shared parseGeminiModelsList helper. Validated: parser test 5/5, typecheck:core clean.
Integrated into release/v3.8.23. Makes the cheaper-inference#3588 reasoning token buffer safe and configurable: only inflates max_tokens when the model has a known, non-default output cap and the buffered value fits inside it; otherwise preserves/clamps the client limit. Adds the reasoningTokenBufferEnabled kill switch (default ON). Validated: combo-routing-engine 81/81, combo-config 25/25, combo-quality-validator-reasoning 12/12, phase1f 10/10, typecheck:core clean.
…2553→1376 LOC (-1177) (cheaper-inference#3725) Phase 1n-1s of cheaper-inference#3501: client 2553→1376 LOC. Pure extraction (ConnectionsListPanel, ConnectionsHeaderToolbar, ZedImportCard, BatchTestResultsModal, AdaptaTutorialModal, useApiKeySave + helpers). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…cation, and success-decay recovery (cheaper-inference#3629) Integrated into release/v3.8.23
…6→781 LOC (≤800 TARGET REACHED ✅) (cheaper-inference#3727) Phase 1t of cheaper-inference#3501: client 1376→781 LOC (≤800 reached). Original god-component 12,882→781 (−94%). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…opencode' CLI command (cheaper-inference#3726) Integrated into release/v3.8.23
…erence#3724) Integrated into release/v3.8.23
…eaper-inference#3288) (cheaper-inference#3723) Integrated into release/v3.8.23
… every chatHelpers import cheaper-inference#3692 added a lazy 'await import(proxyEgress)' for egress-IP visibility inside safeLogEvents, which is a sync function — an ES syntax error. It went unnoticed because typecheck:core does not cover src/sse and no test in the merge gates loaded chatHelpers via tsx; any consumer that did (chat-context-relay and chat-route-coverage suites, integration harnesses) failed at module load with 'await can only be used inside an async function'. safeLogEvents is fire-and-forget logging with an outer try/catch, so making it async (and 'void'-ing the single chat.ts call site) preserves behavior exactly. Validation: tests/unit/chat-context-relay.test.ts + chat-route-coverage.test.ts went from failing-at-load to green (+14 tests destravados).
… + combo/proxy audit fixes (cheaper-inference#3699) Integrated into release/v3.8.23
…ops 403ing (cheaper-inference#3728) Integrated into release/v3.8.23
… auto-hide (cheaper-inference#3729) Integrated into release/v3.8.23
…place, lifecycle) Closes documentation gaps identified in the post-cheaper-inference#3438 audit of plugin/proxy/skills/memory/rtk/compression coverage. Plugin docs were the highest-priority gap (PARTIAL coverage; SDK doc only 242 lines vs. 17 implementation files in src/lib/plugins/). ## Changes ### New: docs/plugins/PLUGIN_DEVELOPMENT.md (~500 lines) Covers the day-to-day workflow for plugin authors: - Two plugin systems overview (SDK hooks vs CLI commands) - 5-stage lifecycle diagram (Install → Activate → Run → Deactivate → Uninstall) - New lifecycle hooks: onInstall, onActivate, onDeactivate, onUninstall - Dev mode (hot reload) — file watcher, 500ms debounce, reload cycle - Testing plugins — testRunner with mock context, hook-by-hook results - Plugin doctor — 5 health checks (directory, manifest, entry point, spawn, DB) - Plugin signing — SHA-256 + Ed25519 verification, registry workflow - CLI plugin system summary (full ref: docs/dev/plugins.md) - Best practices: structure, versioning, error handling, performance, permissions - Troubleshooting guide ### New: docs/plugins/PLUGIN_MARKETPLACE.md (~400 lines) Documents the marketplace registry and publishing flow: - Architecture (Phase 1 local seed vs Phase 2 remote registry) - API reference: listMarketplacePlugins, searchMarketplace, getMarketplaceEntry, isMarketplaceAvailable - MarketplaceEntry data model (12 fields, all documented) - CLI commands: search, info, install - Discovering plugins by tag (10 common tags) - Phase 2 publishing flow (prepare, sign, submit, version, rate) - Verified plugins, trust tiers, quality signals - Migration plan (Phase 1 → Phase 2 forward-compatible) ### Updated: docs/plugins/PLUGIN_SDK.md (+30 lines) - Added 'Related guides' cross-link block at top - Added 'Two Plugin Systems' section clarifying SDK vs CLI plugins - Added 'Lifecycle Hooks (v3.8.16+)' section with 4-row table linking to PLUGIN_DEVELOPMENT.md for full details ## Verification - prettier --check: all 3 files pass - npm run check:doc-links: PASS (553 internal links, 0 broken) - Branch: docs/plugin-system-overhaul (based on upstream/main v3.8.16) ## Source Coverage - 17 plugin source files (src/lib/plugins/) now documented across 3 files - 100% lifecycle coverage: install, activate, deactivate, uninstall - 100% marketplace coverage: list, search, get, availability - 100% signing coverage: SHA-256, Ed25519, key generation, registry workflow ## Related - Follow-up to PR cheaper-inference#3438 (docs: close critical documentation gaps) - Addresses audit findings for plugin documentation
The lifecycle hooks (onInstall, onActivate, onDeactivate, onUninstall) were fabricated - they don't exist in BUILTIN_EVENTS or the Plugin interface. Source of truth: src/lib/plugins/hooks.ts BUILTIN_EVENTS array.
…, method) and fix code examples
PLUGIN_DEVELOPMENT.md: - Replace fabricated 'Lifecycle Hooks' section (onInstall/onActivate/onDeactivate/onUninstall) with 'Built-in Events' that references the real hooks from src/lib/plugins/hooks.ts. The real hooks are: onRequest, onResponse, onError, onModelSelect, onComboResolve, onRateLimit, onQuotaExhaust, onProviderError, onStreamStart, onStreamEnd. See PLUGIN_SDK.md#built-in-events for complete reference. - Clarify that runPluginDoctor is a programmatic API only; remove the claim that 'omniroute plugin doctor' CLI command exists. There is no CLI wiring for this in bin/cli/commands/. Updated troubleshooting section accordingly. PLUGIN_MARKETPLACE.md: - Correct seed plugin ratings: cost-tracker is rating: 4, not 5. Updated from 'All 3 seed plugins have rating: 5' to explicit list: request-logger: 5, rate-limiter: 5, cost-tracker: 4. - Clarify that 'omniroute plugin search' (CLI command) queries npm for omniroute-cmd-* CLI plugins, separate from the SDK marketplace registry. Updated 'CLI Commands' section to show programmatic marketplace API instead. Added note distinguishing the two separate plugin systems. Fixes: cheaper-inference#3452 (comment 4654628898)
…3452 review) - Normalize path separators in devMode code snippet for Windows compatibility - Remove invalid JSON comment from permissions example
…nce#3452 - Fix comment 1: Change 'Core lifecycle events' to 'Core request-handling events' and add note clarifying that onInstall/onActivate/onDeactivate/ onUninstall are manager-fired lifecycle events, not registerable via definePlugin() - Fix comment 2: Add lifecycle events to Built-in Events table in SDK doc with note that they are declared in manifest hooks field and fired by PluginManager, not registerable via definePlugin(). Add onActivate/ onDeactivate to manifest example. - Comments 3 and 4 were already fixed in prior commits (Windows path separator normalization, JSON comment removal)
…publish` subcommand - Restore `⚠️ Not yet wired up` caveat in Dev Mode section: `PLUGIN_DEV_MODE` is only a logger name (`src/lib/plugins/devMode.ts:12`), `OMNIROUTE_PLUGIN_DEV` is never read, and `startDevMode()` has no call sites. - Replace fabricated `omniroute plugin publish --tarball/--signature/--public-key` example with: real CLI subcommands (`list|install|remove|info|search|update|scaffold`) and the actual distribution path (npm or `git+https` install). Refs cheaper-inference#3555 review comments from @diegosouzapw.
… separator in source devMode - Added the theme-manager plugin to the verified list and Ratings description in PLUGIN_MARKETPLACE.md to match implementation. - Fixed cross-platform path separator handling on Windows in devMode.ts.
8f02160 to
2be9e7b
Compare
| ─────── ─────────────── | ||
| Local seed Remote registry | ||
| registry with ratings, | ||
| (3 entries) downloads, signed |
There was a problem hiding this comment.
SUGGESTION: Architecture diagram still says the seed registry has 3 entries after the text/table were updated to 4.
This contradicts src/lib/plugins/marketplace.ts, which currently has request-logger, rate-limiter, cost-tracker, and theme-manager.
| (3 entries) downloads, signed | |
| (4 entries) downloads, signed |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| import { listMarketplacePlugins, searchMarketplace } from "omniroute/plugins/marketplace"; | ||
|
|
||
| // List all seed plugins | ||
| const allPlugins = await listMarketplacePlugins(); |
There was a problem hiding this comment.
WARNING: listMarketplacePlugins() and searchMarketplace() are synchronous, but this example awaits them.
The actual API in src/lib/plugins/marketplace.ts returns arrays directly, so the copied example would still work in normal JS but is misleading and inconsistent with the earlier examples.
| const allPlugins = await listMarketplacePlugins(); | |
| const allPlugins = listMarketplacePlugins(); |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| @@ -91,7 +111,9 @@ onRequest: (ctx) => { | |||
| "hooks": { | |||
| "onRequest": { "enabled": true, "priority": 50 }, | |||
There was a problem hiding this comment.
WARNING: The manifest example uses an object shape for onRequest, but the current schema only accepts booleans for hooks.
src/lib/plugins/manifest.ts defines HooksSchema as z.boolean().optional() for onRequest, onResponse, and onError, so this documented example would fail validation.
| "onRequest": { "enabled": true, "priority": 50 }, | |
| "onRequest": true, |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| │ │ │ 7. Verify signature | ||
| ``` | ||
|
|
||
| > **Note**: The remote registry with signature verification is planned for **Phase 2** of the marketplace. For now, signing is **advisory** — OmniRoute does not block installation of unsigned plugins, but it surfaces a warning. |
There was a problem hiding this comment.
WARNING: The signing note claims unsigned plugins surface a warning, but the current implementation does not show that behavior.
src/lib/plugins/signing.ts only exports verification helpers, and the loader enforces integrity only when that manifest field is present. There is no warning path for unsigned packages in the source I checked.
| > **Note**: The remote registry with signature verification is planned for **Phase 2** of the marketplace. For now, signing is **advisory** — OmniRoute does not block installation of unsigned plugins, but it surfaces a warning. | |
| > **Note**: The remote registry with signature verification is planned for **Phase 2** of the marketplace. For now, signing is **advisory** — OmniRoute does not block installation of unsigned plugins, and integrity is enforced only when an `integrity` manifest field is present. |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| ### "Hook not firing" | ||
|
|
||
| 1. Check the plugin is **activated** (not just installed): `omniroute plugin list` |
There was a problem hiding this comment.
WARNING: The troubleshooting step documents a manifest hook object, but the current manifest schema only accepts booleans.
src/lib/plugins/manifest.ts validates hooks as booleans only, so "onRequest": { "enabled": true, "priority": 50 } is not a valid plugin manifest.
| 1. Check the plugin is **activated** (not just installed): `omniroute plugin list` | |
| 2. Check the **event** in your manifest: `onRequest: true` |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| import { getProviderBaseUrl, buildOptionalBearerHeaders, normalizeOpenAiLikeModelsResponse } from "../utils.ts"; | ||
| import { GET } from "../route.ts"; | ||
|
|
||
| export async function handleAzureAiModels(ctx: ModelsRequestContext): Promise<any> { |
There was a problem hiding this comment.
WARNING: This handler is added as dead code because route.ts does not import or call it.
A grep of the models route shows provider dispatch still happens inline for azure-ai, oci, and sap; there is no reference to handleAzureAiModels, handleOciModels, or handleSapModels. If this is meant to be a refactor, the route needs to wire these handlers in; otherwise the added files will only increase maintenance burden.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| import { getProviderBaseUrl, buildOptionalBearerHeaders, normalizeOpenAiLikeModelsResponse } from "../utils.ts"; | ||
| import { GET } from "../route.ts"; | ||
|
|
||
| export async function handleOciModels(ctx: ModelsRequestContext): Promise<any> { |
There was a problem hiding this comment.
WARNING: This handler is added as dead code because route.ts does not import or call it.
A grep of the models route shows provider dispatch still happens inline for azure-ai, oci, and sap; there is no reference to handleAzureAiModels, handleOciModels, or handleSapModels. If this is meant to be a refactor, the route needs to wire these handlers in; otherwise the added files will only increase maintenance burden.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| import { asRecord, toNonEmptyString, getProviderBaseUrl, buildOptionalBearerHeaders, normalizeOpenAiLikeModelsResponse } from "../utils.ts"; | ||
| import { GET } from "../route.ts"; | ||
|
|
||
| export async function handleSapModels(ctx: ModelsRequestContext): Promise<any> { |
There was a problem hiding this comment.
WARNING: This handler is added as dead code because route.ts does not import or call it.
A grep of the models route shows provider dispatch still happens inline for azure-ai, oci, and sap; there is no reference to handleAzureAiModels, handleOciModels, or handleSapModels. If this is meant to be a refactor, the route needs to wire these handlers in; otherwise the added files will only increase maintenance burden.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 8 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)CRITICAL
WARNING
SUGGESTION
Other Observations (not in diff)No additional issues found outside the reviewed diff. Files Reviewed (7 files)
Fix these issues in Kilo Cloud Reviewed by nex-n2-pro:free · 4,508,431 tokens |
|
Thanks @oyi77. Heads up that this PR can't be reviewed or merged as a docs PR in its current shape: although the title says "docs(plugins)", the diff is +41k/−15k across ~95 files and is dominated by source-code changes — Merging this would revert a large amount of code to an old branch state. Could you please split out just the plugin documentation ( |
|
Superseded by #3787 — clean docs-only re-cut on |
|
Superseded by #3787 |
Summary
Re-targets the original plugin documentation PR #3452 to the latest release branch (
release/v3.8.20) with the docs fixes from the original review feedback applied. The original PR #3452 was blocked because:PLUGIN_DEV_MODE,OMNIROUTE_PLUGIN_DEV) that didn't actually workChanges in this PR
All 6 original doc fixes from PR #3452 (commits cherry-picked from
docs/plugin-system-overhaul):PLUGIN_DEVELOPMENT.mdto correctly mark lifecycle hooks as "not currently exposed" on thePlugininterfacePLUGIN_SDK.mdto accurately describe which events are available vs. plannedfilename.replace(/\\/g, "/"))PluginContextfields (headers, timestamp, method)Why a new PR?
PR #3452 is based on
release/v3.8.19which is now stale. This re-targets torelease/v3.8.20(latest) so the docs are based on the actual current code state.Verification
Files (3)
Closes #3452 (supersedes the original branch)
Related