Repository navigation
feat(plugins): add OMNIROUTE_PLUGINS_DIR to override the plugin scan directory (#11827) - #11906
Conversation
|
CI note, so the red mark isn't mistaken for this PR's doing:
Because and it was red on the source change alone (
|
|
Second red mark, same story: The job's error is not a lint violation — it is ESLint's suppressions bookkeeping: Byte-identical failure on #11875 (different author, different subsystem, five hours earlier This diff cannot be the cause: neither
Happy to rebase once either gate is unblocked on |
|
Full CI picture now that every job has reported — the check matrix on this PR is identical,
The unit-shard failures are the same doc/asset/dependency gates, reproduced on a pristine plus, in shard 4, Not one of them names a file this PR touches. What this PR's own change is answerable for is
No action requested; flagging it so the red column isn't read as this diff's doing. I'll rebase |
…directory (cheaper-inference#11827) The runtime plugin scanner resolved its directory only from HOME/USERPROFILE, with a silent fallback to /tmp/.omniroute/plugins. A container image that does not export HOME therefore scans a path nobody mounted, and the only trace is an info-level scanner.dir_not_found that never names HOME as the cause, so the bind-mounted plugin tree is simply invisible. getDefaultPluginDir() now consults OMNIROUTE_PLUGINS_DIR first and logs the directory it settled on (scanner.dir_resolved) together with the input that won: OMNIROUTE_PLUGINS_DIR, home, or no-home-fallback. Behaviour is unchanged when the variable is unset, and a blank value counts as unset so an empty compose entry cannot point the scanner at a nameless path. The same directory is the root PluginManager installs into, so an override moves discovery and installation together. This is not OMNIROUTE_PLUGIN_PATH: that variable is read only by the CLI command-plugin loader (bin/cli/plugins.mjs) to find omniroute-cmd-* packages and has never reached the server-side scanner. Both docs now say so explicitly. tests/unit/plugins-scanner.test.ts gains six cases: override wins over HOME, override with no HOME at all, unchanged home-derived default, blank override ignored, /tmp fallback, and an end-to-end scan of a directory named only by the variable. The three asserting the new behaviour fail on the parent commit.
65db405 to
728f3a6
Compare
2471a0d
into
cheaper-inference:release/v3.8.51
…directory (cheaper-inference#11827) (cheaper-inference#11906) Boarded with 8 other PRs in one combined worktree: typecheck:core, check:file-size, check:changelog-integrity, check:complexity, check:cognitive-complexity, check:cycles, check-native-deps all green; 75/75 focused tests pass. Clean, well-scoped env-override with correct blank-value handling and a startup log naming the resolution source. Thanks.
Summary
Adds
OMNIROUTE_PLUGINS_DIR, an explicit override for the directory the runtime pluginscanner reads, consulted before the home-derived default (
src/lib/plugins/scanner.ts→getDefaultPluginDir()).Today that directory comes only from
HOME/USERPROFILE, with a silent fallback to/tmp/.omniroute/plugins. An image that never exportsHOME— the reporter's case, and thedefault for several base images — therefore scans a path nobody mounted, and the only trace is
an info-level
scanner.dir_not_foundthat never namesHOMEas the cause. The bind-mountedplugin tree is simply invisible, and the workaround (setting
HOMEin compose to match themount) moves every home-relative behaviour to fix one of them.
OMNIROUTE_PLUGINS_DIR=/opt/omniroute/plugins→ the scanner reads that path, whateverHOMEsays.<home>/.omniroute/plugins,/tmp/...with no home).OMNIROUTE_PLUGINS_DIR=in a composefile cannot point the scanner at a nameless path.
scanner.dir_resolved, naming the inputthat won (
OMNIROUTE_PLUGINS_DIR/home/no-home-fallback) — the reporter's "bonus" item,and what turns this failure class from silent into obvious.
PluginManagerinstalls into (manager.ts:111takes itspluginDirfromthe same function), so an override moves discovery and installation together rather than
splitting them across two directories.
Not a duplicate of
OMNIROUTE_PLUGIN_PATHThe repo already has a similarly-named variable, and it is a different subsystem — worth stating
explicitly so this doesn't read as a redundant knob:
OMNIROUTE_PLUGIN_PATH(existing)OMNIROUTE_PLUGINS_DIR(this PR)bin/cli/plugins.mjs→getPluginDirs()src/lib/plugins/scanner.ts→getDefaultPluginDir()omniroute-cmd-*npm packages that addomniroutesubcommands (package.json,register())plugin.jsonplugins run inside the proxy (manifest, hooks, DB registration)POST /api/plugins/scan,pluginManager.loadAll()/install()~/.omniroute/plugins— an extra search pathSetting one has never affected the other, and after this PR it still doesn't.
.env.example,docs/reference/ENVIRONMENT.md,docs/frameworks/PLUGINS.mdanddocs/frameworks/PLUGIN_MARKETPLACE.mdnow cross-reference each other so the next reader doesn'thave to rediscover the split.
Related Issues
Validation
npm run lint(eslint on the touched TS files: clean;prettier --checkclean; lint-staged ran on commit)release/v3.8.51at13afbfa, the branch tip at the time of writingchangelog.d/features/11906-omniroute-plugins-dir.md(check:changelog-integrityOK — no base bullets lost vsorigin/release/v3.8.51)Failing-then-passing (Hard Rule #18) — same committed test file, source toggled:
Wider plugin surface, all 46
tests/unit/*plugin*.test.tsfiles: 486 pass / 0 fail.Gates that depend on this change:
npm run check:env-doc-sync— the mandatory one for a new env var. It was red on the sourcechange alone (
In code but missing from .env.example: OMNIROUTE_PLUGINS_DIR) and is greenwith
.env.example+docs/reference/ENVIRONMENT.mdupdated:✓ Env / docs contract is in sync.npm run check:doc-links— PASS (859 internal links).check:docs-sync,check:any-budget:t11,check:tracked-artifacts— PASS (pre-commit hook).check:docs-symbols,check:test-discovery,check:file-size— PASS.tsc -pover the two changed TS files — clean.Gates already red on
release/v3.8.51before this branch (verified by running each on apristine checkout of
13afbfa— identical output there and here, and none of them names a filethis PR touches):
check:docs-counts— 11 strict drifts: provider counts (351 vs 357 inREADME.md,AGENTS.md,llm.txt,package.json, 4 SVGs) and migration counts (165 vs 160).check:fabricated-docs --strict— 1 drift:docs/providers/CHATGPT_WEB.md:129referencestests/unit/migration-163-retire-chatgpt-web.test.ts, which does not exist.check:mutation-test-coverage --strict— 5 covering tests missing fromstryker.conf.jsonacross
open-sse/utils/error.tsand the*-web-runtime-blockmodules.scanner.tsis not inthe mutate set.
typecheck:core— pre-existingTS2345inopen-sse/executors/antigravity/executeAttempt.ts:392.Tests Added Or Updated
tests/unit/plugins-scanner.test.ts— six new cases in thegetDefaultPluginDirblock, eachpinning
OMNIROUTE_PLUGINS_DIR/HOME/USERPROFILEthrough a save-and-restore helper so nocase depends on (or leaks into) the runner's ambient environment:
HOME;HOMEat all (the Docker case);/tmp/.omniroute/pluginsfallback with neither input set;scanPluginDir(getDefaultPluginDir())discovers a plugin in a directory namedonly by the variable.
The pre-existing "returns a string path" case is kept as-is.
Coverage Notes
src/lib/plugins/scanner.tsis the only production file changed, and every branch of the newresolution (env / home / no-home fallback / blank-override) is asserted by the tests above —
coverage on the touched function goes up, not down. Docs and
.env.examplecarry no coverage.Reviewer Notes
identical to today's, which is what case 3 pins.
resolve()d). The manager'sassertWithinPluginDir()/assertEntryPointWithinDest()guards alreadyresolve()both sides before comparing, so arelative or non-normalised override cannot weaken the CRITICAL-2/-3 containment checks.
docker-compose.ymlneeds no change: it usesenv_file: .env, so the variable reaches thecontainer as soon as an operator sets it.
docs/guides/DOCKER_GUIDE.mdgains one row inCritical Environment Variables (width-matched to the existing table so prettier leaves the
other rows untouched).
docs/reference/ENVIRONMENT.mdis in.prettierignore(manually aligned); the new row followsthe surrounding column padding of section 2.
(override honoured regardless of
HOME, unchanged when unset, resolved directory logged atstartup, Docker docs mention it, unit coverage on both branches).