Repository navigation
fix(storage): keep non-HTTP transports working in single-user mode #86
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,7 +61,7 @@ import { PubSub } from '../engine/pubsub.js'; | |
| import { registerEventTools } from './events.js'; | ||
| import { AuditLogger } from './audit.js'; | ||
| import { withSession } from './types.js'; | ||
| import { closeDb, campaignDbPath, assertNoLegacyDatabase } from '../storage/index.js'; | ||
| import { closeDb, campaignDbPath, assertNoLegacyDatabase, useSingleUserDatabase } from '../storage/index.js'; | ||
|
|
||
| // Agent runtime | ||
| import { ProviderFactory } from '../agent/provider/factory.js'; | ||
|
|
@@ -183,8 +183,6 @@ function buildServer(pubsub: PubSub, auditLogger: AuditLogger): McpServer { | |
|
|
||
| async function main() { | ||
| setupShutdownHandlers(); | ||
| assertNoLegacyDatabase(); | ||
| console.error(`[Server] Campaign databases: ${campaignDbPath('<campaign-id>')}`); | ||
|
|
||
| // ========================================================================= | ||
| // AGENT RUNTIME: wire LLM providers + repos behind getAgentRuntime() | ||
|
|
@@ -239,6 +237,22 @@ async function main() { | |
| const index = args.indexOf(name); | ||
| return index !== -1 ? args[index + 1] : undefined; | ||
| }; | ||
| // Only the HTTP transport is multi-tenant; it establishes a verified tenant | ||
| // per request. Every other transport serves one local operator, so it opens a | ||
| // single database up front. Without this, getDb() would find no tenant and | ||
| // every storage tool would fail for local users of the npm package, the | ||
| // standalone binaries, and MCP client configs. | ||
| // | ||
| // The legacy-database assertion is likewise HTTP-only: for a hosted server a | ||
| // pre-split file means an incomplete cutover, but in single-user mode that | ||
| // same file is simply the operator's database. | ||
| if (transportType === 'http') { | ||
| assertNoLegacyDatabase(); | ||
| console.error(`[Server] Campaign databases: ${campaignDbPath('<campaign-id>')}`); | ||
| } else { | ||
| useSingleUserDatabase(getArgValue('--db-path')); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When both Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| const networkHost = getArgValue('--host') || '127.0.0.1'; | ||
| const transportToken = getArgValue('--transport-token') || process.env.RPG_MCP_TRANSPORT_TOKEN; | ||
| const maxMessageBytes = parseInt(getArgValue('--max-message-bytes') || '1048576', 10); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -278,6 +278,27 @@ function evictBeyondCap(): void { | |||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * Puts the process in single-user mode against one local database. | ||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||
| * Multi-tenancy is a property of the hosted HTTP server, where every request | ||||||||||||||||||||||||||||||||||||||||||
| * carries a signed tenant context. The other transports — stdio, TCP, unix | ||||||||||||||||||||||||||||||||||||||||||
| * socket, WebSocket — have nowhere to put one: they serve a single operator | ||||||||||||||||||||||||||||||||||||||||||
| * running the engine locally, through the npm package, the standalone | ||||||||||||||||||||||||||||||||||||||||||
| * binaries, or an MCP client config. Without this they would resolve no tenant | ||||||||||||||||||||||||||||||||||||||||||
| * and every storage call would throw, which is a regression rather than a | ||||||||||||||||||||||||||||||||||||||||||
| * boundary: there is no second tenant to isolate from. | ||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||
| export function useSingleUserDatabase(path?: string): Database.Database { | ||||||||||||||||||||||||||||||||||||||||||
| if (!overrideDb) { | ||||||||||||||||||||||||||||||||||||||||||
| const resolvedPath = resolveDbPath(path); | ||||||||||||||||||||||||||||||||||||||||||
| console.error(`[Database] Single-user mode: ${resolvedPath}`); | ||||||||||||||||||||||||||||||||||||||||||
| overrideDb = initDB(resolvedPath); | ||||||||||||||||||||||||||||||||||||||||||
| migrate(overrideDb); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| return overrideDb; | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+293
to
+299
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win Cache the database only after migration succeeds. If Suggested initialization order if (!overrideDb) {
const resolvedPath = resolveDbPath(path);
console.error(`[Database] Single-user mode: ${resolvedPath}`);
- overrideDb = initDB(resolvedPath);
- migrate(overrideDb);
+ const database = initDB(resolvedPath);
+ try {
+ migrate(database);
+ } catch (error) {
+ database.close();
+ throw error;
+ }
+ overrideDb = database;
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||
| * The database for the current request's campaign. | ||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When two local/TCP/WebSocket callers supply different
sessionIdvalues, this branch only installs a database and leaves both requests without tenant context.withSessiontherefore replaces both IDs withunscoped(src/server/types.ts:35-40), while event subscriptions, worlds, and combat engines are keyed byctx.sessionId; callers can consequently share or overwrite each other's in-memory state. Single-user database mode should retain the caller's session identity (or establish an equivalent transport-local context) rather than restoring storage while collapsing every session.Useful? React with 👍 / 👎.