Fix clientId usage to follow LiveStore best practices

rgbkrk · rgbkrk · commit 406b9d326db3 · 2025-09-03T07:43:32.000-07:00
- Generate clientId using userId prefix + unique identifier instead of using userId directly
- Add userId to sync payload for proper authorization
- Update authentication pattern to separate client identification from user identification
- clientId now identifies device/app instances, userId identifies users for auth
- Update all tests and schema documentation to reflect new pattern

Fixes the issue where clientId was incorrectly set to userId, which goes against
LiveStore's design where clientId should identify unique client instances.
diff --git a/packages/lib/src/config.ts b/packages/lib/src/config.ts
@@ -36,6 +36,7 @@ export class RuntimeConfig {
   public readonly artifactClient: IArtifactClient;
   public readonly adapter: Adapter | undefined;
   public readonly clientId: string;
+  public readonly userId: string;
 
   constructor(options: RuntimeAgentOptions) {
     this.runtimeId = options.runtimeId;
@@ -48,6 +49,7 @@ export class RuntimeConfig {
       DEFAULT_CONFIG.imageArtifactThresholdBytes;
     this.adapter = options.adapter;
     this.clientId = options.clientId;
+    this.userId = options.userId;
 
     // Use injected artifact client or create default one
     this.artifactClient = options.artifactClient ??
diff --git a/packages/lib/src/runtime-agent.ts b/packages/lib/src/runtime-agent.ts
@@ -98,6 +98,7 @@ export class RuntimeAgent {
           runtimeId: this.config.runtimeId,
           sessionId: this.config.sessionId,
           clientId,
+          userId: this.config.userId,
         },
       });
 
diff --git a/packages/lib/src/types.ts b/packages/lib/src/types.ts
@@ -73,6 +73,8 @@ export interface RuntimeAgentOptions {
   readonly adapter: Adapter;
   /** Client ID for sync payload (must be provided) */
   readonly clientId: string;
+  /** User ID for sync payload authorization (must be provided) */
+  readonly userId: string;
 }
 
 /**
diff --git a/packages/lib/test/integration.test.ts b/packages/lib/test/integration.test.ts
@@ -61,6 +61,7 @@ Deno.test("RuntimeAgent Integration Tests", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "test-integration-token",
       clientId: "test-integration-client",
+      userId: "test-user-id",
       adapter,
 
       capabilities,
@@ -154,6 +155,7 @@ Deno.test("RuntimeAgent Integration Tests", async (t) => {
         syncUrl: "ws://localhost:8787", // Not used with adapter
         authToken: "valid-token",
         clientId: "valid-client",
+        userId: "test-user-id",
         adapter,
         capabilities: capabilities,
       });
@@ -177,6 +179,7 @@ Deno.test("RuntimeAgent Integration Tests", async (t) => {
           syncUrl: "ws://localhost:8787", // Not used with adapter
           authToken: "token",
           clientId: "test-client",
+          userId: "test-user-id",
           adapter,
           capabilities: capabilities,
         });
@@ -252,6 +255,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "test-token",
       clientId: "test-client-3",
+      userId: "test-user-id",
       adapter,
       capabilities: {
         canExecuteCode: true,
@@ -279,6 +283,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "token1",
       clientId: "client1",
+      userId: "test-user-1",
       adapter: adapter1,
       capabilities: {
         canExecuteCode: true,
@@ -296,6 +301,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "token2",
       clientId: "client2",
+      userId: "test-user-2",
       adapter: adapter2,
       capabilities: {
         canExecuteCode: true,
@@ -318,6 +324,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "test-ai-token",
       clientId: "test-client-ai",
+      userId: "test-user-id",
       adapter,
       capabilities: {
         canExecuteCode: true,
diff --git a/packages/lib/test/mod.test.ts b/packages/lib/test/mod.test.ts
@@ -28,6 +28,7 @@ Deno.test("RuntimeConfig validation works", () => {
       authToken: "", // Missing
       notebookId: "", // Missing
       clientId: "test-client",
+      userId: "test-user-id",
       adapter: makeInMemoryAdapter({}),
       capabilities: {
         canExecuteCode: true,
@@ -52,6 +53,7 @@ Deno.test("RuntimeConfig validation works", () => {
     authToken: "test-token",
     notebookId: "test-notebook",
     clientId: "test-client",
+    userId: "test-user-id",
     adapter: makeInMemoryAdapter({}),
     capabilities: {
       canExecuteCode: true,
diff --git a/packages/lib/test/runtime-agent-adapter-injection.test.ts b/packages/lib/test/runtime-agent-adapter-injection.test.ts
@@ -37,6 +37,7 @@ function createTestRuntimeConfig(
       canExecuteAi: false,
     },
     clientId: "test-client",
+    userId: "test-user-id",
     adapter: defaultAdapter,
     ...defaults,
   };
@@ -49,6 +50,7 @@ Deno.test("RuntimeAgent adapter injection", async (t) => {
     () => {
       const config = createTestRuntimeConfig([], {
         clientId: "test-client-backward-compat",
+        userId: "test-user-id",
         notebookId: "test-notebook",
         syncUrl: "ws://fake-url:9999", // Will fail but that's expected
       });
@@ -76,6 +78,7 @@ Deno.test("RuntimeAgent adapter injection", async (t) => {
     const config = createTestRuntimeConfig([], {
       adapter,
       clientId: "test-client-adapter",
+      userId: "test-user-id",
       notebookId: "adapter-test",
       syncUrl: "ws://fake-url:9999",
     });
@@ -109,6 +112,7 @@ Deno.test("RuntimeAgent adapter injection", async (t) => {
       const config = createTestRuntimeConfig([], {
         adapter,
         clientId: "test-client-generated",
+        userId: "test-user-id",
         notebookId: "adapter-test-2",
         authToken: "test-token",
         syncUrl: "ws://fake-url:9999",
@@ -138,6 +142,7 @@ Deno.test("RuntimeAgent adapter injection", async (t) => {
     const config = createTestRuntimeConfig([], {
       adapter,
       runtimeId,
+      userId: "test-user-id",
       notebookId: "clientid-test",
       syncUrl: "ws://fake-url:9999",
     });
@@ -168,6 +173,7 @@ Deno.test("RuntimeAgent adapter injection", async (t) => {
     const config = createTestRuntimeConfig([], {
       adapter,
       clientId: explicitClientId,
+      userId: "test-user-id",
       notebookId: "explicit-clientid-test",
       syncUrl: "ws://fake-url:9999",
     });
diff --git a/packages/lib/test/runtime-agent-artifact.test.ts b/packages/lib/test/runtime-agent-artifact.test.ts
@@ -73,6 +73,7 @@ const mockRuntimeOptions: RuntimeAgentOptions = {
   authToken: "test-token",
   notebookId: "test-notebook",
   clientId: "test-client",
+  userId: "test-user-id",
   imageArtifactThresholdBytes: 6 * 1024, // 6KB threshold
   adapter: makeInMemoryAdapter({}),
 };
diff --git a/packages/lib/test/runtime-agent-text-representations.test.ts b/packages/lib/test/runtime-agent-text-representations.test.ts
@@ -42,11 +42,13 @@ Deno.test("RuntimeAgent Text Representations for Artifacts", async (t) => {
         runtimeId: "test-runtime",
         runtimeType: "test",
         notebookId: "test-notebook",
-        syncUrl: "ws://localhost:8787", // Not used with adapter
+        syncUrl: "ws://localhost:8787",
         authToken: "test-token",
         clientId: "test-client",
+        userId: "test-user-id",
         adapter,
         capabilities: capabilities,
+        imageArtifactThresholdBytes: 1024,
       });
 
       const agent = new RuntimeAgent(config, capabilities);
@@ -158,6 +160,7 @@ Deno.test("RuntimeAgent Text Representations for Artifacts", async (t) => {
         syncUrl: "ws://localhost:8787", // Not used with adapter
         authToken: "test-token",
         clientId: "test-client",
+        userId: "test-user-id",
         adapter,
         capabilities,
 
@@ -273,6 +276,7 @@ Deno.test("RuntimeAgent Text Representations for Artifacts", async (t) => {
         syncUrl: "ws://localhost:8787", // Not used with adapter
         authToken: "test-token",
         clientId: "test-client",
+        userId: "test-user-id",
         adapter,
         capabilities,
 
diff --git a/packages/lib/test/runtime-agent.test.ts b/packages/lib/test/runtime-agent.test.ts
@@ -61,6 +61,7 @@ Deno.test("RuntimeAgent", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "test-token",
       clientId: "test-client",
+      userId: "test-user-id",
       adapter,
       capabilities,
     });
@@ -177,6 +178,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "test-token",
       clientId: "test-client",
+      userId: "test-user-id",
       adapter,
       capabilities: {
         canExecuteCode: true,
@@ -203,6 +205,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "token1",
       clientId: "client1",
+      userId: "test-user-1",
       adapter: adapter1,
       capabilities: {
         canExecuteCode: true,
@@ -220,6 +223,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "token2",
       clientId: "client2",
+      userId: "test-user-2",
       adapter: adapter2,
       capabilities: {
         canExecuteCode: true,
@@ -242,6 +246,7 @@ Deno.test("RuntimeConfig", async (t) => {
       syncUrl: "ws://localhost:8787", // Not used with adapter
       authToken: "test-token",
       clientId: "test-client",
+      userId: "test-user-id",
       adapter,
       capabilities: {
         canExecuteCode: true,
diff --git a/packages/pyodide-runtime-agent/src/auth.ts b/packages/pyodide-runtime-agent/src/auth.ts
@@ -27,18 +27,29 @@ export interface UserInfo {
   name?: string;
 }
 
+/**
+ * Authentication result containing user info and generated client ID
+ */
+export interface AuthenticationResult {
+  userId: string;
+  clientId: string;
+  userInfo: UserInfo;
+}
+
 /**
  * Discover authenticated user identity via /api/me endpoint
  *
  * This should be called before creating runtime agents to get the clientId.
+ * The clientId is generated using the user ID as a prefix plus a unique identifier,
+ * following LiveStore best practices where clientId identifies device/app instances.
  *
  * @param options - Configuration for identity discovery
- * @returns Promise resolving to user ID
+ * @returns Promise resolving to authentication result with userId and generated clientId
  * @throws Error if authentication fails
  */
 export async function discoverUserIdentity(
   options: DiscoverUserIdentityOptions,
-): Promise<string> {
+): Promise<AuthenticationResult> {
   const { authToken, syncUrl, skipInTests = true } = options;
 
   // Skip authentication in test environments if enabled
@@ -55,7 +66,13 @@ export async function discoverUserIdentity(
 
     if (isTestEnvironment) {
       logger.debug("Skipping authentication in test environment");
-      return "test-user-id";
+      const userId = "test-user-id";
+      const clientId = `${userId}-${crypto.randomUUID()}`;
+      return {
+        userId,
+        clientId,
+        userInfo: { id: userId, email: "test@example.com" },
+      };
     }
   }
 
@@ -104,13 +121,23 @@ export async function discoverUserIdentity(
       throw new Error("User ID not found in response");
     }
 
-    logger.debug("User identity discovered", {
-      userId: userInfo.id,
+    // Generate clientId using user ID as prefix plus unique identifier
+    // This follows LiveStore best practices where clientId identifies device/app instances
+    const userId = userInfo.id;
+    const clientId = `${userId}-${crypto.randomUUID()}`;
+
+    logger.debug("User identity discovered and clientId generated", {
+      userId,
+      clientId,
       email: userInfo.email,
       name: userInfo.name,
     });
 
-    return userInfo.id;
+    return {
+      userId,
+      clientId,
+      userInfo,
+    };
   } catch (error) {
     // If we haven't already logged the error above, log it here
     if (!(error instanceof Error && error.message.startsWith("HTTP "))) {
diff --git a/packages/pyodide-runtime-agent/src/mod.ts b/packages/pyodide-runtime-agent/src/mod.ts
@@ -63,13 +63,17 @@ if (import.meta.main) {
     Deno.exit(1);
   }
 
-  // Discover user identity first
-  const clientId = await discoverUserIdentity({
+  // Discover user identity and generate proper clientId
+  const { userId, clientId, userInfo } = await discoverUserIdentity({
     authToken,
     syncUrl,
   });
 
-  logger.info("Authenticated successfully", { clientId });
+  logger.info("Authenticated successfully", {
+    userId,
+    clientId,
+    email: userInfo.email,
+  });
 
   // Create adapter for Node.js environment with Cloudflare sync
   const adapter = makeAdapter({
@@ -80,11 +84,11 @@ if (import.meta.main) {
     },
   });
 
-  // Create agent with discovered clientId and Node.js adapter
+  // Create agent with discovered clientId, userId, and Node.js adapter
   const agent = new PyodideRuntimeAgent(
     Deno.args,
     {}, // pyodide options
-    { clientId, adapter }, // runtime options
+    { clientId, adapter, userId }, // runtime options
   );
 
   logger.info("Starting Agent");
diff --git a/packages/pyodide-runtime-agent/src/pyodide-agent.ts b/packages/pyodide-runtime-agent/src/pyodide-agent.ts
@@ -60,6 +60,7 @@ interface PyodideAgentOptions {
 interface PyodideRuntimeOptions {
   adapter?: Adapter;
   clientId?: string;
+  userId?: string;
 }
 
 /**
@@ -146,6 +147,7 @@ export class PyodideRuntimeAgent extends RuntimeAgent {
         ...options, // Merge options into config
         ...(runtimeOptions.adapter && { adapter: runtimeOptions.adapter }),
         ...(runtimeOptions.clientId && { clientId: runtimeOptions.clientId }),
+        ...(runtimeOptions.userId && { userId: runtimeOptions.userId }),
       });
     } catch (error) {
       // Configuration errors should still go to console for CLI usability
diff --git a/packages/pyodide-runtime-agent/test/config-cli.test.ts b/packages/pyodide-runtime-agent/test/config-cli.test.ts
@@ -22,6 +22,7 @@ function makeBaseConfig(overrides: Partial<Record<string, unknown>> = {}) {
     authToken: "test-token",
     notebookId: "test-nb",
     clientId: "test-client",
+    userId: "test-user-id",
     adapter: makeInMemoryAdapter({}),
     capabilities: {
       canExecuteCode: true,
diff --git a/packages/python-runtime-agent/src/python-runtime-agent.ts b/packages/python-runtime-agent/src/python-runtime-agent.ts
@@ -16,6 +16,7 @@ export class PythonRuntimeAgent extends RuntimeAgent {
         canExecuteAi: false,
       },
       clientId: "dummy",
+      userId: "dummy",
       adapter: makeInMemoryAdapter({}),
     });
 
diff --git a/packages/schema/mod.ts b/packages/schema/mod.ts
@@ -34,10 +34,11 @@ export * from "./queries/index.ts";
  * - ClientId must be non-numeric to prevent user impersonation
  *
  * USER CLIENTS (runtime: false/undefined):
- * - Regular users: clientId = authenticated user ID
- * - Anonymous users: clientId = "anonymous-user"
+ * - Regular users: clientId = userId-{uniqueId} (e.g. "user123-abc-def-ghi")
+ * - Anonymous users: clientId = "anonymous-{uniqueId}" (e.g. "anonymous-abc-def-ghi")
  * - User clients use OIDC tokens for authentication
- * - ClientId must match authenticated user ID
+ * - User ID is passed separately in sync payload for authorization
+ * - ClientId identifies device/app instances following LiveStore best practices
  *
  * PRESENCE DISPLAY:
  * - Runtime agents: Bot icon with runtimeType label

Original file line number	Diff line number	Diff line change
`@@ -73,6 +73,8 @@ export interface RuntimeAgentOptions {`
`73`	`73`	`readonly adapter: Adapter;`
`74`	`74`	`/** Client ID for sync payload (must be provided) */`
`75`	`75`	`readonly clientId: string;`
	`76`	`+ /** User ID for sync payload authorization (must be provided) */`
	`77`	`+ readonly userId: string;`
`76`	`78`	`}`
`77`	`79`
`78`	`80`	`/**`