feat(ai): finalize IBackendsTransport — add list/uninstall, widen opts, tighten JSDoc (PR #526 follow-up)#527
Merged
sroussey merged 4 commits intoMay 22, 2026
Conversation
…s, tighten JSDoc
Adds list() and uninstall(backend) to IBackendsTransport so the libs interface
matches what builder's MessagePortBackendsTransport already implements and the
renderer UI requires.
Widens IEnsureRunningRequest.opts from { ctx: number } to
Readonly<Record<string, unknown>> so non-llamacpp backends (sd-cpp, future
MLX/whisper) don't have to lie with ctx: 0. Updates StableDiffusionCppProvider
to pass opts: {} accordingly.
Tightens JSDoc on release() (resolves on post, not broker-ack) and
subscribeStatus() (idempotent unsubscribe, dedup of double-subscribe).
@workglow/cli
@workglow/ai
@workglow/browser-control
@workglow/indexeddb
@workglow/javascript
@workglow/job-queue
@workglow/knowledge-base
@workglow/mcp
@workglow/storage
@workglow/task-graph
@workglow/tasks
@workglow/util
workglow
@workglow/anthropic
@workglow/bun-webview
@workglow/chrome-ai
@workglow/electron
@workglow/google-gemini
@workglow/huggingface-inference
@workglow/huggingface-transformers
@workglow/llamacpp-server
@workglow/mlx
@workglow/node-llama-cpp
@workglow/ollama
@workglow/openai
@workglow/playwright
@workglow/postgres
@workglow/sqlite
@workglow/stable-diffusion-cpp
@workglow/supabase
@workglow/tf-mediapipe
commit: |
Contributor
There was a problem hiding this comment.
Pull request overview
Follow-up to PR #526 to reduce interface drift between provider packages and the builder-side backends broker transport, while widening backend runtime options to support non-llamacpp backends.
Changes:
- Extended
IBackendsTransportwithlist()anduninstall(backend)and updated related JSDoc semantics. - Widened
IEnsureRunningRequest.optstoReadonly<Record<string, unknown>>and updated Stable Diffusion CPP provider call site to pass an empty opts object. - Added a new “type-only conformance” Vitest file intended to assert compile-time compatibility of the transport interface.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| providers/stable-diffusion-cpp/src/ai/StableDiffusionCppProvider.ts | Updates ensureRunning() call to pass opts: {} under the widened opts type. |
| packages/ai/tests/provider-utils/IBackendsTransport.types.test.ts | Adds a new conformance test file (currently not picked up by the repo’s main test harness). |
| packages/ai/src/provider-utils/IBackendsTransport.ts | Updates transport interface: widens opts, adds list()/uninstall(), and tightens JSDoc semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+28
to
+29
| * llamacpp uses `{ ctx: number }`; sd-cpp omits it; future backends define | ||
| * their own schema. |
Comment on lines
+7
to
+54
| // ──────────────────────────────────────────────────────────────────────────── | ||
| // Compile-time conformance tests for IBackendsTransport. | ||
| // | ||
| // These tests run via the test runner but their value is in `tsc` accepting | ||
| // (or rejecting) the declarations below. No runtime assertions: if the file | ||
| // compiles, the contract holds. | ||
| // ──────────────────────────────────────────────────────────────────────────── | ||
|
|
||
| import type { IBackendsTransport, IEnsureRunningRequest } from "../../src/provider-utils"; | ||
|
|
||
| // opts is now open — accepts the historic llamacpp shape … | ||
| const _checkOptsWithCtx: IEnsureRunningRequest["opts"] = { ctx: 4096 }; | ||
| // … the empty shape for backends like sd-cpp that have no per-run opts … | ||
| const _checkOptsEmpty: IEnsureRunningRequest["opts"] = {}; | ||
| // … and arbitrary shapes for future backends (MLX, whisper, etc.). | ||
| const _checkOptsArbitrary: IEnsureRunningRequest["opts"] = { foo: "bar", n: 42 }; | ||
|
|
||
| // Interface exposes `list` and `uninstall` alongside the existing methods. | ||
| type _Methods = keyof IBackendsTransport; | ||
| const _hasList: _Methods = "list"; | ||
| const _hasUninstall: _Methods = "uninstall"; | ||
|
|
||
| // Structural-conformance dummy — any breaking change to the interface will | ||
| // fail typecheck on this assignment. | ||
| const _conforms: IBackendsTransport = { | ||
| ensureRunning: async () => ({ url: "http://localhost", release: async () => {} }), | ||
| subscribeStatus: () => () => {}, | ||
| install: async () => {}, | ||
| list: async () => {}, | ||
| uninstall: async () => {}, | ||
| }; | ||
|
|
||
| // Reference each binding so eslint's `no-unused-vars` and `noUnusedLocals` | ||
| // don't trip even though these assertions are purely compile-time. | ||
| void _checkOptsWithCtx; | ||
| void _checkOptsEmpty; | ||
| void _checkOptsArbitrary; | ||
| void _hasList; | ||
| void _hasUninstall; | ||
| void _conforms; | ||
|
|
||
| // Provide a single trivial runtime assertion so test runners that require at | ||
| // least one `test`/`it` block don't choke on this file. | ||
| import { describe, it, expect } from "vitest"; | ||
| describe("IBackendsTransport (type-only conformance)", () => { | ||
| it("compiles", () => { | ||
| expect(true).toBe(true); | ||
| }); |
Comment on lines
+32
to
+36
| ensureRunning: async () => ({ url: "http://localhost", release: async () => {} }), | ||
| subscribeStatus: () => () => {}, | ||
| install: async () => {}, | ||
| list: async () => {}, | ||
| uninstall: async () => {}, |
…iscovery The repo's test runner (scripts/test.ts) only collects Vitest files from packages/test/src/test, so this file was never executed in CI. Removing the old location; the file is recreated under packages/test/src/test/ai in the following commit (also strengthens the structural conformance assertion with explicit parameter signatures).
…ures Recreates the type-conformance test at packages/test/src/test/ai/ (the location scripts/test.ts discovers) and replaces the zero-arg dummy implementations with explicit parameter signatures drawn from the interface. TypeScript happily assigns `() => X` to `(arg) => X`, so the previous form did not catch parameter-list drift; the new form fails typecheck on any rename, type change, or return-type change to a method of `IBackendsTransport`. Imports come from `@workglow/ai/provider-utils` (the public subpath export) instead of the previous relative path into packages/ai/src.
Coverage Report
File CoverageNo changed files found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes 2 HIGH-priority interface-drift findings on top of #526.
Summary
list()anduninstall(backend)toIBackendsTransportso the libs interface matches what builder PR Webhook Notification Tasks #241'sMessagePortBackendsTransportalready implements and the renderer UI requires.IEnsureRunningRequest.optsfrom{ ctx: number }toReadonly<Record<string, unknown>>so non-llamacpp backends (sd-cpp, future MLX/whisper) don't have to lie withctx: 0.release()(resolves on post, not broker-ack) andsubscribeStatus()(idempotent unsubscribe, dedup of double-subscribe).Coordination
builder PR #241 has a paired follow-up PR (
claude/peaceful-albattani-NSph2-interface-drift) that deletes its local copy ofIBackendsTransportand imports from@workglow/ai/provider-utils. That PR's typecheck depends on this one — merge order is libs → builder.Test plan
tests/provider-utils/IBackendsTransport.types.test.ts— compile-time conformance.StableDiffusionCppProviderupdated to passopts: {}).Generated by Claude Code