Skip to content

fix(workflows): resolve the kokoro dependency to the tts service - #2495

Open
Hoang130203 wants to merge 1 commit into
Osmantic:mainfrom
Hoang130203:fix/workflow-kokoro-dependency-alias
Open

fix(workflows): resolve the kokoro dependency to the tts service#2495
Hoang130203 wants to merge 1 commit into
Osmantic:mainfrom
Hoang130203:fix/workflow-kokoro-dependency-alias

Conversation

@Hoang130203

Copy link
Copy Markdown

Summary

check_workflow_dependencies() reports a dependency it cannot resolve as
satisfied, without probing anything:

_DEP_ALIASES = {"ollama": "llama-server"}
...
for dep in deps:
    resolved = _DEP_ALIASES.get(dep, dep)
    if resolved in health_cache:
        results[dep] = health_cache[resolved]
    elif resolved in SERVICES:
        status = await check_service_health(resolved, SERVICES[resolved])
        ...
    else:
        results[dep] = True          # unknown name -> "ready"

config/n8n/catalog.json gives voice-to-voice the dependencies
["whisper", "llama-server", "kokoro"]. There is no kokoro service —
Kokoro is the model the tts service serves (ghcr.io/remsky/kokoro-fastapi-cpu),
and the service id is tts:

$ ls extensions/services/ | grep -c '^kokoro$'
0
$ python3 -c "import yaml;print(yaml.safe_load(open('extensions/services/tts/manifest.yaml'))['service']['id'])"
tts

So kokoro misses both the alias map and SERVICES, falls through the else,
and the Workflows page shows voice-to-voice with its dependencies met whether
or not TTS is installed or running. all_deps_met is computed from exactly
these values:

dep_status = await check_workflow_dependencies(wf.get("dependencies", []), health_cache)
all_deps_met = all(dep_status.values())

The user sees a ready card, imports the workflow, and it fails at the speech
node. Of the three declared dependencies, whisper and llama-server are checked
honestly; the one that is silently assumed is the one most likely to be absent,
since tts is an optional extension.

Fix

_DEP_ALIASES exists for precisely this case — "ollama" is already mapped to
"llama-server". Add "kokoro""tts" so the tts health probe actually
runs.

I chose the alias over renaming the dependency in the catalog because kokoro
is the name a user recognises from the workflow's description, and the alias
map is where the codebase already handles this mismatch.

I deliberately did not change the fail-open else branch to fail closed.
It is load-bearing for dependencies that are not ODS services at all, and
flipping it is a behaviour change on the dashboard that deserves its own
discussion. The contract test below makes the typo case impossible instead.

Test

Two additions to tests/test_workflows.py: one asserting kokoro and ollama
both resolve to a probed service, one pinning the fail-open behaviour for a
genuinely unknown name so a future change to it is deliberate.

Plus tests/test-n8n-catalog-contract.py, which reads _DEP_ALIASES back out
of the router source (no FastAPI import needed) and asserts, for the whole
catalog:

  • every dependency resolves to a directory under extensions/services/;
  • every entry names a file that exists, in a declared category, with required
    fields and unique ids;
  • no workflow file on disk is missing from the catalog;
  • every workflow's connections reference nodes that exist.

229 assertions today. Wired into make test and the Linux CI job.

AI Assistance

AI assisted with tracing the fail-open branch, drafting the contract test, and
wording this description. I read the full diff, confirmed there is no kokoro
service directory and that tts is the id, and ran both suites against
origin/main and the patch.

Release Lane

  • Stable hotfix targeting release/2.6.x
  • Mainline change targeting main
  • Next-minor work targeting the next feature/minor release
  • Not sure; reviewer should help classify

Stable hotfix reason:

n/a

Changed Surface

  • Docs only
  • Tests only
  • Dashboard UI
  • Dashboard API / host agent
  • Installer / bootstrap / lifecycle
  • Docker Compose / service manifests
  • Model routing / Hermes / capabilities
  • Network exposure / auth / proxy
  • Dependencies / runtime wiring

(One entry in a lookup table in routers/workflows.py, plus tests. CI config
and the Makefile test target are also touched.)

Risk And Validation

  • Risk level: Low
  • Validation run:
    • git diff --check
    • Markdown/link sanity for docs
    • Focused tests listed below
    • Dashboard lint/test/build
    • Extension audit / compose validation
    • Release-grade fleet or scoped hardware validation
    • Stable-lane patch validation, if targeting release/2.6.x

Commands/results:

$ python3 -m pytest tests/test_workflows.py -q     # in dashboard-api
47 passed, 20 warnings in 12.50s
  (45 pre-existing + 2 new; no pre-existing test changed)

# the new dependency test against origin/main's router:
FAILED tests/test_workflows.py::test_dependency_aliases_resolve_to_real_services
1 failed

$ python3 tests/test-n8n-catalog-contract.py
...
Passed: 229  Failed: 0
[PASS] n8n catalog contracts

# the same contract test against origin/main's router:
[FAIL] voice-to-voice: dependency 'kokoro' resolves to a service
       'kokoro' resolves to 'kokoro', which is not a directory under
       extensions/services/. check_workflow_dependencies() reports an
       unresolved dependency as satisfied without probing it, so this
       workflow would advertise itself as ready with the service down.
Passed: 228  Failed: 1

Operational Change Check

check_workflow_dependencies() runs on the dashboard-api /api/workflows
read path. The change makes one more dependency name route to a real health
probe. The only behaviour change: voice-to-voice now shows its TTS
dependency as unmet when the tts service is down or not installed, instead of
always showing it met. No write path, no schema change, no other workflow
affected.

  • This is not an operational change.
  • This is an operational change and validation is recorded above.
  • This is an operational change and validation is intentionally deferred for:

Notes For Reviewers

The bigger thing I found while writing the contract test, and am not fixing
here:
all 18 workflows in config/n8n/ are placeholders. Every one is a
manualTrigger plus a stickyNote reading "This is a template workflow…
Customize the nodes below to match your setup"
, with "connections": {}
nothing is wired to anything:

id                           nodes conns  node types
m4-deterministic-voice           2     0  manualTrigger,stickyNote
document-qa                      2     0  manualTrigger,stickyNote
voice-transcription              2     0  manualTrigger,stickyNote
...  (18 of 18)

Meanwhile the catalog advertises them with real descriptions and
"setupTime": "2 minutes". tests/integration-test.sh passes them because it
only checks that the JSON parses and has a nodes key.

That is a content gap rather than a defect in this code path, and filling it is
ods/CONTRIBUTING.md's "Workflow templates — pre-built n8n workflows that
solve actual problems people have". I am working through implementing them as
separate PRs, starting with the llama-server-only ones. The connection-graph
assertions in this contract test are there to catch a half-wired workflow when
those land.

Tell me if you would rather the catalog mark unimplemented entries explicitly
(e.g. a "status": "template" field the dashboard can badge) — that is a
smaller change than implementing all 18 and would stop the page over-promising
in the meantime.

check_workflow_dependencies() reports a dependency it cannot resolve as
satisfied, without probing anything:

    resolved = _DEP_ALIASES.get(dep, dep)
    if resolved in health_cache:      ...
    elif resolved in SERVICES:        ...probe it...
    else:
        results[dep] = True           # unknown name -> "ready"

config/n8n/catalog.json gives voice-to-voice the dependencies
["whisper", "llama-server", "kokoro"]. There is no `kokoro` service —
Kokoro is the model the `tts` service serves, and the service id is
`tts`. So the name falls through the else branch and the Workflows page
shows voice-to-voice with its dependencies met whether or not TTS is
installed or running. The user imports it and it fails at the speech
node.

_DEP_ALIASES already exists for exactly this ("ollama" -> "llama-server").
Adds "kokoro" -> "tts" so the tts health probe actually runs.

Also adds tests/test-n8n-catalog-contract.py, which reads _DEP_ALIASES
out of the router and asserts every catalog dependency resolves to a
directory under extensions/services/ — plus that every entry names a
file that exists, sits in a declared category, has unique ids, that no
workflow file is missing from the catalog, and that every workflow's
connections reference nodes that exist. Wired into `make test` and the
Linux CI job.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant