Repository navigation
fix(plugin): initialize file logging from the ESM bridge entry - #2465
Open
akushonkamen wants to merge 1 commit into
Open
akushonkamen wants to merge 1 commit into
akushonkamen wants to merge 1 commit into
Conversation
Fixes MemTensor#2399 ## Summary - pass `initLogging: true` from the ESM entry (`bridge.mts`) to `bootstrapMemoryCoreFull`, aligning it with the legacy CJS entry (`bridge.cts`), which already passes the flag - add an integration regression asserting that a bridge started from `dist/bridge.mjs` creates the `config.logging` file sinks (`logs/memos.log`, `logs/error.log`) under the runtime home ## Why `bootstrapMemoryCoreFull` only initializes the global logger when `options.initLogging` is set — the option's documented contract is that standalone bridge entries own their stdio and must opt in, while embedded hosts leave it false. `bridge.cts` passes the flag; `bridge.mts` never did. Because `bridge_client.py` resolves the entry as `dist/bridge.mjs` > `dist/bridge.cjs` > `bridge.mts` > `bridge.cts`, every bridge launched from a current build took the ESM path and skipped `initLogger` entirely: `config.logging` (timezone, level, channels, and the file sinks under `<home>/logs`) was silently inert — no file sink was ever created and nothing warned about it. ## Validation - new integration case in `apps/memos-local-plugin/tests/integration/bridge-esm-entry.test.ts`: red on unfixed `main` (no `logs/memos.log` after startup), green after the fix - build-level check (`node dist/bridge.mjs --agent=hermes --no-viewer --home=<tmp>`): before the fix `<tmp>/logs` was never created; after the fix it contains all six sinks (`memos.log`, `error.log`, `audit.log`, `events.jsonl`, `llm.jsonl`, `perf.jsonl`) - `npx vitest run tests/unit/bridge-status.test.ts` — 3/3 - `npm run test:unit` — 181 files, 1589 passed, 2 skipped - `npm run lint` (`tsc -p tsconfig.json --noEmit`) — clean The embedded-host path (`adapters/deepseek-harness`, which intentionally passes `initLogging: false`) is untouched.
4 of 5 tasks
Collaborator
🤖 Open Code ReviewTarget: PR #2465 ✅ OpenCodeReview: Review skipped: no items were selected. Generated by cloud-assistant via Open Code Review. |
Collaborator
|
Author
|
Heads-up on CI: the autotest bot reports the same ENV ISSUE here as on #2464 and #2466:
|
Memtensor-AI
pushed a commit
to Memtensor-AI/MemOS
that referenced
this pull request
Oct 8, 2026
…or#2470) `_rebuild_if_stale` in the Hermes adapter compared TypeScript source timestamps against `dist/bridge.cjs`, but both spawn paths prefer `dist/bridge.mjs` since the ESM migration (MemTensor#1736 / MemTensor#1998). A stale `bridge.mjs` next to a fresh `bridge.cjs` therefore slipped past the guard and the daemon kept booting the stale ESM artifact without any warning — exactly the failure mode MemTensor#2399 / MemTensor#2465 triggered. Mirror `_bridge_script()`'s compiled-entry precedence and apply the stricter variant: rebuild when any compiled entry on disk is older than the newest `*.ts` source, treat a missing `dist/` as stale, and name the stale artifacts in the log line. Covered by 9 new unittest cases in `tests/python/test_rebuild_if_stale.py`, including the headline "stale .mjs, fresh .cjs, newer source" scenario the issue describes. Full plugin python suite (141 tests) and ruff check / format pass.
Open
4 of 17 tasks
This was referenced Oct 8, 2026
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 #2399
Summary
The two hand-copied bridge entries drifted apart when the standalone-logging
flag (
initLogging) landed indev-v2.0.25: the legacy CJS entry(
bridge.cts) passesinitLogging: truetobootstrapMemoryCoreFull, but theESM entry (
bridge.mts) — the entrybridge_client.pyactually prefers —does not. Result: every bridge started from a current build never constructs
the logger, so
config.loggingis silently inert end to end (console channel,timezone, level, channels, and all file sinks under
<home>/logs).This PR adds the missing
initLogging: true(plus the explanatory commentalready present in
bridge.cts) and an integration regression that spawnsdist/bridge.mjsand asserts the file sinks appear.Root cause
bootstrapMemoryCoreFullgates its onlyinitLoggercall on the option(
core/pipeline/memory-core.ts):bridge.ctshonors the contract (and comments it);bridge.mtswas neverupdated when the flag was introduced, and the option is optional/falsy-skip, so
nothing warns. The file transports (
core/logger/index.ts→FileRotatingTransport) are the only thing that creates<home>/logs, andthey are constructed eagerly inside
initLogger— hence "no logs directory,ever" from the packaged entry, exactly as reported.
Behavior-change notes
For existing users this restores the documented behavior rather than adding
overhead: after this change a packaged bridge actually produces the sinks that
config.loggingalready promises (defaultlogging.file.enabled: true—memos.log,error.log,audit.log,events.jsonl,llm.jsonl,perf.jsonlunder<home>/logs). Nothing else about logging changes:bridge.cts— already correct, untouched.adapters/deepseek-harness/index.ts,initLogging: false) is intentionally untouched — embedded hosts keeping control oflogging is the documented design.
daemon-start.logmentioned in the issue is a separate codepath outside
initLoggerand is out of scope here.Tests
New regression case in
apps/memos-local-plugin/tests/integration/bridge-esm-entry.test.ts(theexisting #1736 harness: spawn
dist/bridge.mjs, temporary home, grace-periodSIGTERM): after startup,
logs/memos.logandlogs/error.logmust exist underthe runtime home. The transports open their files eagerly during
initLogger,so the assertion is deterministic. As in the #1736 case, an environment-level
missing-native-binding failure is tolerated (unrelated to this issue).
Red→green evidence:
main@a7367d0, built, run by hand and via the newtest:
Validation (all offline, no LLM key / Docker / Neo4j / Qdrant):
npx vitest run tests/integration/bridge-esm-entry.test.ts— 2/2npx vitest run tests/unit/bridge-status.test.ts— 3/3npm run test:unit— 181 files, 1589 passed, 2 skipped (zero regressions)npm run lint(tsc -p tsconfig.json --noEmit) — clean