internal/plugin has ~500 LOC and zero test coverage

[jason-shen]

Problem

internal/plugin has no test file:

internal/plugin/manager.go   295 lines
internal/plugin/external.go  211 lines
internal/plugin/types.go

Roughly 500 lines with zero coverage, and it's the package with the widest blast radius in the repo — it discovers plugin directories from disk, parses YAML manifests, spawns Python and Node subprocesses, and speaks a line-delimited protocol to them over stdio. The failure modes are the ones that are hardest to debug in production and easiest to pin down in a test: a malformed manifest, a plugin that writes garbage to stdout, a subprocess that exits mid-call, one that never responds.

Everything else in the repo with comparable weight is tested — internal/pipeline has eleven test files, internal/tts has seven, internal/config, internal/vad, internal/peer, and internal/signaling all have coverage. This package is the outlier, and it's the one that runs untrusted-ish code.

Proposed change

Add internal/plugin/manager_test.go and internal/plugin/external_test.go.

Discovery and manifests (manager.go) — no subprocesses needed

Point NewManager at a t.TempDir() populated with fixture directories and assert on LoadAll:

• A well-formed plugin registers under its declared name.
• Malformed YAML fails that one plugin without aborting the whole load — one bad plugin directory must not take down every other plugin.
• A directory with no manifest is skipped.
• An empty plugin dir, and an empty dir string, both no-op cleanly.
• Two plugins declaring the same name — assert whatever the intended behaviour is (last wins, first wins, or error). If the current behaviour looks accidental, say so on the issue rather than encoding it.
• Concurrent LoadAll and lookups don't race — Manager guards state with a sync.RWMutex, and CI runs go test -race, so this is worth an explicit test.

Subprocess protocol (external.go)

Don't require Python or Node on the test machine — that makes the suite environment-dependent and it will rot. Two options, either is fine:

• Drive the protocol against a fake process (a shell script or a compiled test helper) that emits canned responses, including deliberately broken ones.
• Or extract the message encode/decode from the process management, and unit-test the codec directly. This is the cleaner outcome if the seam is easy to find.

Cases worth covering: a plugin that exits immediately, one that writes non-JSON to stdout, one that never replies (assert the call is bounded and doesn't hang the pipeline), and a normal request/response round trip.

PLUGIN_SDK_DIR is read from the environment in NewManager — use t.Setenv so tests stay isolated.

Acceptance criteria

• Both files added; go test -race ./internal/plugin/... passes.
• No test depends on Python, Node, or network access.
• Fixtures live under internal/plugin/testdata/.
• A malformed plugin is proven not to break loading of a valid sibling.

Note

If a test exposes a real bug — a hang on an unresponsive plugin, say — open a separate issue for the fix and link it. Landing the tests is valuable on its own and shouldn't be blocked on fixing what they find.

Pointers

• internal/plugin/manager.go — NewManager ~L28, LoadAll ~L42
• internal/plugin/external.go — subprocess handling
• internal/tts/http_test.go — house style for table-driven tests with fakes