[Tsan1024]

What and why

Closes #93.

When Memory uses the shared HTTP host, closing its server leaves /internal/v1/ routes registered. Requests still reach the closed server, and mounting a replacement fails with a duplicate-prefix error. Retain and invoke the host's unmount callback on close, while leaving the shared listener and other roles running.

The regression test verifies that closed routes return 404, another role remains reachable, a replacement can mount, and repeated closes of the old server do not remove the replacement.

How it was verified

• Focused tests for the changed behavior: Memory server suite, 13 tests passed on Node 26.5.0 / pnpm 11.7.0. The new regression test fails against the baseline (expected 404, received 401).
• Fast checks of every touched app: make check-node-services ran; typecheck and lint passed. The full suite reported 2,537 passing tests, 45 skipped and one failure in tests/document/main.test.ts, "starts without the thread while its port is busy and serves once the port frees". That failure also reproduces on the unchanged baseline. The full gate is not passing locally.
• Contracts regenerated if contracts/openapi/ changed: not applicable; no contract changes.

Boundary

Documented shared-host shutdown ownership once in apps/node-services/AGENTS.md as a rule for all roles: call the unmount function returned by mount() and leave the shared listener to its host. The README remains unchanged. No readiness policy or role-selection changes. The user-message to provider_ready path is unchanged; no network round trip or file walk is added there.

[linger-review]

Approve with nits (1 P3)

Scope: base main at merge-base 860dbfc (branch is up to date with main), 2 commits, 4 files reviewed. Helper skill loaded: ponytail-review (sections 1–3); coderbook and karpathy-guidelines were not loaded and were applied from the review checklist instead. Ran in a separate session.

Findings

P3

• apps/node-services/README.md:5-12: the new "Shared HTTP lifecycle" section describes unmount-on-close as a Memory-only behavior, and apps/node-services/AGENTS.md:17-18 says the same thing again. This is really a host-level rule that every shared-host role already follows: src/llm-wiki/wiki-server.ts:66-69 and src/voice/service.ts:385 both keep the mount() return value and call it on close. Memory was the only role that broke the rule. Suggested fix: state it once, in the AGENTS.md shared-host bullet, as a rule for all roles ("a role's close calls the unmount function mount() returned; it never closes a shared listener"). Then drop the Memory-specific README section, or shorten it to a pointer. Nothing breaks if this is left as is. The cost is two docs that describe a narrower rule than the code actually follows.

Verified as fine

• Fix is at the root cause. startMemoryServer (src/memory/server.ts:409-418) now keeps the unmount function that HttpHost.mount returns and calls it in close(). It does this before closing an owned private listener. When the listener is shared, ownedHost is undefined, so the listener stays open.
• Idempotency and replacement safety. The unmount closure in src/http-host.ts:152-157 removes only its own {prefix, route} entries, matched by object identity, and skips them when indexOf returns -1. So a repeated close() does nothing, and closing a stale instance cannot remove a replacement that mounted the same prefix. The new test covers both cases.
• Consistency. The change matches the existing pattern in the Wiki and Voice roles. It adds no new abstraction.
• Readiness. Unmounting also removes Memory's route from the host's /readyz aggregation (src/http-host.ts:62-68), which is the intended effect.
• Regression test. The test fails without the fix (I reverted server.ts to the merge base locally: expected 401 to be 404) and passes with it.
• PR description. Every claim matches the diff. No contract, env, or migration changes. The user-message → provider_ready path is untouched. The repository is English-only and the change follows that.

Process notes

• No merge commits and no fixup commits. The code commit and the docs commit are separate and each stands on its own.
• Merge state is BLOCKED, presumably because required checks and reviews are missing. See CI below.

CI

• gh pr checks reports no checks on fix/memory-route-cleanup. CI needs to run before merge.
• Run locally: pnpm --filter @linger/node-services exec vitest run tests/memory/server.test.ts gave 13/13 passed. The new test also failed as expected against the baseline server.ts.
• Not run: the full make check-node-services. The author reports one failure in tests/document/main.test.ts that also reproduces on the baseline. This PR does not touch that file.

Automated review by Claude Code

---

Automated review of 5eb7f4a by Claude Code.

[linger-review]

Approve with nits (1 P3)

Scope: follow-up on 5eb7f4a; base main at merge-base 860dbfc (branch is up to date with main), 3 commits, 3 files reviewed (apps/node-services/AGENTS.md, src/memory/server.ts, tests/memory/server.test.ts). Loaded coderbook; ponytail-review and karpathy-guidelines were not loaded in this run, so I applied their criteria from the review checklist instead. Ran in a separate session.

Previous findings

• README framed the generic unmount rule as Memory-only and duplicated AGENTS.md (P3): Resolved. 2b8d5e8 drops the "Shared HTTP lifecycle" section from apps/node-services/README.md and rewrites apps/node-services/AGENTS.md:17-19 as a host-level rule for every role, kept in one place.

Findings

P3

• apps/node-services/AGENTS.md:17: The new rule says "When a role closes, it calls the unmount function returned by mount()", but the Agent role does not do this. src/agent/executor/http/agent-manager-http-server.ts:217 calls httpHost.mount({ prefixes: ["/v1/"], ... }) and discards the returned function, and closeOwnedNetwork (:204-208) only closes the owned host. On a shared host, closing the Agent manager leaves /v1/ routed to a closed app, and mounting a replacement would throw HTTP route prefix already mounted. This is the same bug class as #93 and is out of scope here, but the doc now states an invariant the code doesn't hold. Either phrase the rule as the target for new and changed code, or track the Agent-role fix separately. That fix is one line: keep the unmount and call it in closeOwnedNetwork.

Verified as fine

• src/memory/server.ts:409-417: close() calls unmount() before closing the owned host. With a shared host, the listener stays open. With an owned host, unmounting before closing is harmless.
• Idempotency: http-host.ts:152-157 removes routes by entry identity (routes.indexOf(entry)), so closing an old instance again can't remove a replacement mounted on the same prefix. The new test checks this explicitly: it closes twice, then closes the old instance after the replacement mounts and still gets 401.
• The test covers the three claims in the PR body: closed routes return 404, the other role (/v1/) still answers, and a replacement mounts without the duplicate-prefix error. The PR body says the test fails on the baseline (401 instead of 404), which matches the code.
• This follows the existing pattern in llm-wiki/wiki-server.ts:62 and voice/service.ts:385 and adds no new helper or abstraction.
• No contract, env, migration or provider_ready path changes. All text is English.

Process notes

• No merge commits. The two docs(node-services) commits overlap: the second rewrites what the first added. They could be squashed into one docs commit, but this doesn't block.
• All commits are authored by Tsan1024 <tsan1024@163.com>.

CI

gh pr checks reports no checks on fix/memory-route-cleanup, so CI has not run on this head, and the merge state is BLOCKED. Make sure the required workflows run before merging. I ran no tests locally: the change since the previous review is docs-only, and the code change was already reviewed.

Automated review by Claude Code

---

Automated review of 2b8d5e8 by Claude Code.