fix(rust): thread subc.connection_file to the transform transport - #376
Conversation
`subc.connection_file` is parsed and schema-validated, and gates whether
`transform_mode: "rust"` may activate — but its value never reached the
transform-lane transport. Both construction sites called
`new SubcModuleTransport()` with no argument, so the constructor's
`connectionFile ?? getDefaultConnectionFile()` fallback always won and the
transport looked at `<data-dir>/cortexkit/run/subc-connection.json`
regardless of configuration.
On a host whose daemon publishes elsewhere (e.g. a systemd
RuntimeDirectory at /run/user/<uid>/), rust mode therefore could not
connect, and the one setting that would fix it was inert:
ENOENT: no such file or directory, statx
'<data-dir>/cortexkit/run/subc-connection.json'
rust transform failed; attempting LKG replay
lkg_miss
rust pass: decision=error served_from=raw module=0.0 ms
`module=0.0 ms` shows the module is never invoked, so the daemon reports
the module perfectly healthy throughout while every pass degrades to raw
and materializes nothing — an empty sidebar rather than a config error.
The same key is already threaded correctly on the embedding lane
(embedding-routing.ts:87), so one config value could reach the Synapse
socket while the transform lane looked somewhere else.
Adds `subc?: { connection_file: string }` to the session-hook deps config
type so the recovery-arm site can thread it too. Defaults are unchanged
when the key is absent.
| const rustModeModuleClient: RustModeModuleClient | undefined = | ||
| pluginConfig.transform_mode === "rust" ? new SubcModuleTransport() : undefined; | ||
| pluginConfig.transform_mode === "rust" | ||
| ? new SubcModuleTransport(pluginConfig.subc?.connection_file) |
There was a problem hiding this comment.
Wake-plane path remains inconsistent
With a non-default subc.connection_file, the changed transform transports reach the configured daemon while wakePlaneStatus still probes the default path, so the daemon's wake-plane ownership is not recognized and smart-note checks continue running locally.
Knowledge Base Used: OpenCode plugin runtime
There was a problem hiding this comment.
Verified from source — this is real, and it's a third site I missed. wake-plane.ts:27-29:
function connectionFile(): string {
return join(getDataDir(), "cortexkit", "run", "subc-connection.json");
}Same hardcoded default the transport had, and unlike the transport it takes no override parameter at all. So with a non-default subc.connection_file the probe hits a path that doesn't exist, probeWakePlaneCatalog fails, and wakePlaneStatus() returns unknown → fail-open → standalone smart-note evaluation keeps running even when the fleet's wake plane owns it. Exactly as described.
I have not folded it into this PR, and the reason is a design call that belongs to you rather than scope-avoidance: wakePlaneStatus() takes no arguments and has three callers — tools/ctx-note/tools.ts:295, smart-notes/runner.ts:54, dreamer/evaluate-smart-notes.ts:100 — none of which carry plugin config today. Options:
- Module-level injection — an exported setter called once at plugin init where
pluginConfigis in scope, mirroring the existing swappablecatalogProbe/nowseams in__wakePlaneTest. ~6 lines, fits the module's shape, adds mutable module state. - Thread config to all three callers — no new module state, wider diff, touches one tool and two dreamer paths.
- Shared resolver — one
resolveSubcConnectionFile(config)consumed by both the transport and the wake plane.
I'd lean (3) as the durable fix: the root problem is that the default path is spelled out in two places and the config value has no single resolution point, which is exactly how this recurs a fourth time. But that's a wider blast radius than a fix PR should decide on its own.
Happy to do any of them — say which and I'll push it here, or open a separate PR if you'd rather keep this one narrow.
Severity note for triage: this one is fail-open (smart notes keep evaluating locally — duplicated work, not lost work), whereas the transform-lane bug this PR fixes was effectively fail-shut: zero materializations, empty sidebar. Same defect class, different urgency.
There was a problem hiding this comment.
Thanks for verifying the third site. I agree this is real, but it should be a separate PR to keep this fix focused. For that PR, use option 3: introduce a shared resolveSubcConnectionFile(config) (or equivalent) and make both the transport and wake-plane paths consume it. Thread the resolved value through the three wake-plane callers rather than adding module-level mutable state. This removes the duplicated default-path logic while keeping the current transform fix narrowly scoped.
Ready-to-post review comment for PR #376Approved. I traced the user-only I also ran the targeted plugin tests (141 passing) and the plugin typecheck. There is no dedicated call-site wiring regression test yet, but I found no blocker in the implementation. The independently default-path-only wake-plane capability probe is separate from the Rust transform transport and would be a follow-up rather than a reason to widen this patch. |
Fixes #375.
The bug
SubcModuleTransportaccepts a connection-file path, but both transform-lane construction sites call it with no argument:so
connectionFile ?? getDefaultConnectionFile()always resolves to the default. Meanwhilesubc.connection_fileis parsed and validated — and is read exactly once, purely as a boolean gate (config/index.ts:470-476):resolveTransformModerequires that to be true before returning"rust". Net effect: setting the key unlocks rust mode, then the path you set is discarded.The embedding lane already threads the same key correctly (
plugin/embedding-routing.ts:87), so on one host a single config value could send Synapse embeddings to the configured socket while the transform lane looked somewhere else entirely.Observed failure
Daemon 0.10.0 publishing to
/run/user/1000/subc-connection.json; module registered, healthy, restarts 0/3.{ "transform_mode": "rust", "subc": { "connection_file": "/run/user/1000/subc-connection.json" } }Over ~40 passes:
defer 36 · error 4 · parked 2 · execute 1, withmaterialized = 0on every row. Zero materializations means m[0]/m[1] are never built, so the sidebar renders empty and the session shows "reconnecting".Worth noting for triage:
module=0.0 msand the daemon's ownhealth ok / in_flight=0 / consecutive_error_count 0are both correct. Nothing ever crosses the wire, so both endpoints look healthy and the failure is invisible from the module side.The change
Thread the configured value at both sites, defaulting exactly as before when the key is absent:
plus
subc?: { connection_file: string }on the session-hook deps config type so the second site can reach it.2 files, +8/−2. No behaviour change when
subc.connection_fileis unset.Verification
The 1 test failure and 6 lint errors reproduce on clean
master(classify.ts,execute-status.ts,storage-db.test.ts,rust-mode-transform.test.ts— none in this diff).Verified end-to-end on a live daemon: with the fix, the transport targets the configured path instead of the default.
On test coverage — stated plainly
I have not added a unit test, and I'd rather say so than bury it. The existing
module-transport.test.tscovers the constructor, which was already correct — the defect is the two call sites, which construct lazily inside a plugin-factory and a closure, and nothing currently exercises that wiring. The obvious harness would needmock.module, which is process-global in Bun and has caused cross-file bleed in this repo before (#279).So the evidence here is the live reproduction above rather than a regression test. If you'd like the wiring locked down, I'm happy to add a harness in whatever shape you prefer — I didn't want to invent one unprompted in a fix this small.
Two adjacent observations (not changed here)
hasUserTierSubcConfigvalidates that a usable path exists, while the transport then uses a different path — so the activation gate doesn't gate on the thing it validates. This PR closes that for free.configured subc.connection_file not found at <path>) would turn a repeating soft failure into one actionable line. Happy to add separately if wanted.Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Fixes rust transform mode so the configured
subc.connection_fileis passed to the module transport instead of always falling back to the default path; the embedding lane already used this key, so the two lanes could point at different sockets.Bug Fixes
subc?: { connection_file: string }to the session-hook deps config type.Written for commit 50c7424. Summary will update on new commits.
Greptile Summary
The PR threads the resolved
subc.connection_filesetting into both Rust transform transport construction paths, preserving the existing default when unset.Confidence Score: 4/5
The PR appears safe to merge, with a non-blocking inconsistency where wake-plane capability discovery still ignores a configured non-default connection file.
Both changed transform transports correctly use the configured daemon path, but the sibling wake-plane probe remains tied to the default location and therefore continues local smart-note evaluation for non-default daemon deployments.
Files Needing Attention: packages/plugin/src/index.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD C[subc.connection_file] --> T[Transform transport] C --> R[Recovery transport] T --> D[Configured subc daemon] R --> D W[Wake-plane probe] --> P[Default connection file] P --> U[Unknown when only configured file exists] U --> L[Local smart-note evaluation]Reviews (1): Last reviewed commit: "fix(rust): thread subc.connection_file t..." | Re-trigger Greptile
Context used: