Skip to content

docs: fix README claims that STATIC mode shares one server - #41

Merged
imbenrabi merged 2 commits into
mainfrom
claude/project-thread-9caudi
Nov 5, 2026
Merged

imbenrabi merged 2 commits into
mainfrom
claude/project-thread-9caudi

Conversation

@imbenrabi

Copy link
Copy Markdown
Contributor

Requested by Ben · project thread

Summary

README fixes for people who use toolception. I read the README against the 0.9.0 code and CHANGELOG. The SDK v2 and Zod 4 parts are already correct; what was stale is older: four passages still say STATIC mode creates one server and reuses it for all clients. That stopped being true in 0.7.0 (#27: every client gets its own McpServer and orchestrator, and ?config= session context reaches module loaders in STATIC mode).

  • Static startup, createMcpServer requirements, the startup modes list: now say each client gets its own server with the startup toolsets loaded. The requirements section also explains the base server createServer builds at startup (the handle's server), so people register prompts, resources and extra tools inside createServer, per the 0.7.0 migration note.
  • options.sessionContext notes: said it only applies to DYNAMIC mode; it applies to both.
  • Install step: states the Node.js >= 25.3.0 requirement (engines), and the client snippets now say to install @modelcontextprotocol/client.

Docs only: no source, test or CHANGELOG change, so no release is needed. The AGENTS.md intent layer (for example src/core/AGENTS.md still mentions the old SDK package) is for the intent-layer work, not this PR.

Base and merge order

Base: main. Nothing is stacked on it and it can merge at any time.

Verification

Each changed claim checked against src/server/createMcpServer.ts (createBundleFactory), src/server/AGENTS.md and the 0.7.0 CHANGELOG entry.


Generated by Claude Code

Since 0.7.0 every client gets its own server in STATIC mode, and the session
context (?config=) reaches module loaders there too. Four README passages still
said a single server is reused. Also state the Node.js requirement and the
client package the client snippets import.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VmUXGTDh1pKwGG2DQ5Vcwc
@imbenrabi imbenrabi self-assigned this Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

TS review: pipeline-ts-reviewer (feature-pipeline)

Verdict: COMMENT (no CRITICAL or HIGH findings). Reviewed the README-only diff of abb9ce0 against origin/main, statically. There is no TypeScript in this PR, so the standards check was the karpathy principles plus checking every changed claim against createBundleFactory, src/server/AGENTS.md, the 0.7.0 CHANGELOG entry and engines. All new claims hold.

# Sev Location Finding
1 MEDIUM README.md:913 options.context notes still say STATIC loaders run at startup with the same context; this contradicts the new line at 956
2 LOW README.md:824 Module loader "When called" row omits the per-client run in STATIC mode
3 LOW README.md:70-75 "Server preloads named toolsets at startup" no longer says each client has its own server (not wrong)
4 LOW (info) README.md:804 In STATIC mode /tools comes from the base orchestrator and base context, so it can differ from one client's view
{
  "recommendation": "COMMENT",
  "summary": "README-only change is accurate against createBundleFactory, AGENTS.md, CHANGELOG 0.7.0 and engines, but leaves a stale STATIC loader claim at README.md:913 and an incomplete row at 824.",
  "findings": [
    {"severity": "MEDIUM", "file": "README.md", "line": 913, "issue": "options.context notes still say STATIC loaders are invoked at startup with the same context"},
    {"severity": "LOW", "file": "README.md", "line": 824, "issue": "Module loader 'When called' row omits per-client invocation in STATIC mode"},
    {"severity": "LOW", "file": "README.md", "line": 75, "issue": "Typical flows does not mention per-client servers (optional)"},
    {"severity": "LOW", "file": "README.md", "line": 804, "issue": "/tools in STATIC mode reflects the base orchestrator (optional note)"}
  ]
}

Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

Fix mapping for the TS review above

# Result Detail
1 Fixed in d1fa56d STATIC loaders run at startup and again per connecting client with that client's merged context; the DYNAMIC line says the same about the merged context.
2 Fixed in d1fa56d The "When called" row now says "at startup and again for each connecting client".
3 Left unchanged Not a contradiction, and the sections the PR edits already say it. Extra clauses in the overview would be more than was asked.
4 Left unchanged The reviewer marked it optional. It is a caveat about the /tools endpoint, which this PR doesn't touch.

Re-running the reviewer next.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

TS re-review: pipeline-ts-reviewer

Verdict: APPROVE on d1fa56d. Both fixes (the README.md:824 row and the options.context notes at 913, including the reworded DYNAMIC line) match createBundleFactory, buildOrchestrator and SessionContextResolver, and no README text still says STATIC shares one server or runs loaders only at startup. Two LOW notes, neither blocking:

  • README.md:824: the edited table row is wider than its neighbours (cosmetic, renders fine). Left as is.
  • src/http/FastifyTransport.ts:~427: the disconnectServer comment still says "the shared server" for STATIC mode. Source, so out of scope for this README-only PR; a follow-up for the intent-layer or cleanup work.
{"recommendation": "APPROVE", "summary": "Both README fixes match the source; no remaining contradiction about STATIC mode sharing one server or loaders running only at startup.", "findings": [{"severity": "LOW", "file": "README.md", "line": 824, "issue": "Edited table row is wider than its neighbours (cosmetic)"}, {"severity": "LOW", "file": "src/http/FastifyTransport.ts", "line": 427, "issue": "Out of scope: disconnectServer comment still refers to the shared server in STATIC mode"}]}

Generated by Claude Code

@imbenrabi
imbenrabi merged commit 144eff2 into main Nov 5, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants