From 728f335042e98b5ccd38c2c06061414f6264ef50 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Mon, 9 Feb 2026 11:52:18 +0000 Subject: [PATCH 1/5] fix: make warehouseId optional when no plugin requires it Adds a `static requires` mechanism to the Plugin class so plugins can declare which shared resources they need (e.g. warehouseId). At startup, createApp() collects requirements from all plugins and passes them to ServiceContext.initialize(), which only resolves warehouseId when a plugin declared it. Apps using only server() no longer need DATABRICKS_WAREHOUSE_ID set. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Pedro Figueiredo --- .../appkit/src/context/execution-context.ts | 5 +++- .../appkit/src/context/service-context.ts | 17 ++++++++++---- packages/appkit/src/context/user-context.ts | 4 ++-- packages/appkit/src/core/appkit.ts | 23 +++++++++++++++---- packages/appkit/src/plugin/plugin.ts | 2 ++ .../appkit/src/plugins/analytics/analytics.ts | 2 ++ packages/shared/src/plugin.ts | 3 +++ 7 files changed, 45 insertions(+), 11 deletions(-) diff --git a/packages/appkit/src/context/execution-context.ts b/packages/appkit/src/context/execution-context.ts index e201fec6..9fbc2523 100644 --- a/packages/appkit/src/context/execution-context.ts +++ b/packages/appkit/src/context/execution-context.ts @@ -62,9 +62,12 @@ export function getWorkspaceClient() { /** * Get the warehouse ID promise. + * Safe to use non-null assertion: if a plugin requires warehouseId, + * ServiceContext.initialize() will have resolved it at startup. */ export function getWarehouseId(): Promise { - return getExecutionContext().warehouseId; + // biome-ignore lint/style/noNonNullAssertion: warehouseId is guaranteed to exist when a plugin declares requires=["warehouseId"] + return getExecutionContext().warehouseId!; } /** diff --git a/packages/appkit/src/context/service-context.ts b/packages/appkit/src/context/service-context.ts index ee4b2453..fd606bd5 100644 --- a/packages/appkit/src/context/service-context.ts +++ b/packages/appkit/src/context/service-context.ts @@ -4,6 +4,7 @@ import { WorkspaceClient, } from "@databricks/sdk-experimental"; import { coerce } from "semver"; +import type { ServiceContextResource } from "shared"; import { name as productName, version as productVersion, @@ -24,8 +25,8 @@ export interface ServiceContextState { client: WorkspaceClient; /** The service principal's user ID */ serviceUserId: string; - /** Promise that resolves to the warehouse ID */ - warehouseId: Promise; + /** Promise that resolves to the warehouse ID (only present when a plugin requires it) */ + warehouseId?: Promise; /** Promise that resolves to the workspace ID */ workspaceId: Promise; } @@ -62,6 +63,7 @@ export class ServiceContext { * of creating one from environment credentials. */ static async initialize( + requiredResources: ServiceContextResource[] = [], client?: WorkspaceClient, ): Promise { if (ServiceContext.instance) { @@ -72,7 +74,10 @@ export class ServiceContext { return ServiceContext.initPromise; } - ServiceContext.initPromise = ServiceContext.createContext(client); + ServiceContext.initPromise = ServiceContext.createContext( + requiredResources, + client, + ); ServiceContext.instance = await ServiceContext.initPromise; return ServiceContext.instance; } @@ -153,11 +158,15 @@ export class ServiceContext { } private static async createContext( + requiredResources: ServiceContextResource[] = [], client?: WorkspaceClient, ): Promise { const wsClient = client ?? new WorkspaceClient({}, getClientOptions()); - const warehouseId = ServiceContext.getWarehouseId(wsClient); + const warehouseId = requiredResources.includes("warehouseId") + ? ServiceContext.getWarehouseId(wsClient) + : undefined; + const workspaceId = ServiceContext.getWorkspaceId(wsClient); const currentUser = await wsClient.currentUser.me(); diff --git a/packages/appkit/src/context/user-context.ts b/packages/appkit/src/context/user-context.ts index e106510e..7b409f9b 100644 --- a/packages/appkit/src/context/user-context.ts +++ b/packages/appkit/src/context/user-context.ts @@ -11,8 +11,8 @@ export interface UserContext { userId: string; /** The user's name (from request headers) */ userName?: string; - /** Promise that resolves to the warehouse ID (inherited from service context) */ - warehouseId: Promise; + /** Promise that resolves to the warehouse ID (inherited from service context, only present when a plugin requires it) */ + warehouseId?: Promise; /** Promise that resolves to the workspace ID (inherited from service context) */ workspaceId: Promise; /** Flag indicating this is a user context */ diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index ed226b36..9dd0fad2 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -7,6 +7,7 @@ import type { PluginConstructor, PluginData, PluginMap, + ServiceContextResource, } from "shared"; import { CacheManager } from "../cache"; import { ServiceContext } from "../context"; @@ -149,11 +150,13 @@ export class AppKit { TelemetryManager.initialize(config?.telemetry); await CacheManager.getInstance(config?.cache); - // Initialize ServiceContext for Databricks client management - // This provides the service principal client and shared resources - await ServiceContext.initialize(config?.client); - + // Collect required resources from all plugins before initializing context const rawPlugins = config.plugins as T; + const requiredResources = AppKit.collectRequiredResources(rawPlugins); + + // Initialize ServiceContext for Databricks client management + // Only resolves resources that plugins actually need + await ServiceContext.initialize(requiredResources, config?.client); const preparedPlugins = AppKit.preparePlugins(rawPlugins); const mergedConfig = { plugins: preparedPlugins, @@ -178,6 +181,18 @@ export class AppKit { } return result; } + + private static collectRequiredResources( + plugins: PluginData[], + ): ServiceContextResource[] { + const resources = new Set(); + for (const { plugin } of plugins) { + for (const req of plugin.requires ?? []) { + resources.add(req); + } + } + return [...resources]; + } } /** diff --git a/packages/appkit/src/plugin/plugin.ts b/packages/appkit/src/plugin/plugin.ts index c5050ca9..bd6cdbbd 100644 --- a/packages/appkit/src/plugin/plugin.ts +++ b/packages/appkit/src/plugin/plugin.ts @@ -8,6 +8,7 @@ import type { PluginExecutionSettings, PluginPhase, RouteConfig, + ServiceContextResource, StreamExecuteHandler, StreamExecutionSettings, } from "shared"; @@ -76,6 +77,7 @@ export abstract class Plugin< private registeredEndpoints: PluginEndpointMap = {}; static phase: PluginPhase = "normal"; + static requires: ServiceContextResource[] = []; name: string; constructor(protected config: TConfig) { diff --git a/packages/appkit/src/plugins/analytics/analytics.ts b/packages/appkit/src/plugins/analytics/analytics.ts index a631a776..e9cb30b8 100644 --- a/packages/appkit/src/plugins/analytics/analytics.ts +++ b/packages/appkit/src/plugins/analytics/analytics.ts @@ -3,6 +3,7 @@ import type express from "express"; import type { IAppRouter, PluginExecuteConfig, + ServiceContextResource, SQLTypeMarker, StreamExecutionSettings, } from "shared"; @@ -25,6 +26,7 @@ import type { const logger = createLogger("analytics"); export class AnalyticsPlugin extends Plugin { + static requires: ServiceContextResource[] = ["warehouseId"]; name = "analytics"; protected envVars: string[] = []; diff --git a/packages/shared/src/plugin.ts b/packages/shared/src/plugin.ts index a30260aa..36dbb2b1 100644 --- a/packages/shared/src/plugin.ts +++ b/packages/shared/src/plugin.ts @@ -46,6 +46,8 @@ export interface PluginConfig { export type PluginPhase = "core" | "normal" | "deferred"; +export type ServiceContextResource = "warehouseId" | "workspaceId"; + export type PluginConstructor< C = BasePluginConfig, I extends BasePlugin = BasePlugin, @@ -54,6 +56,7 @@ export type PluginConstructor< ) => I) & { DEFAULT_CONFIG?: Record; phase?: PluginPhase; + requires?: ServiceContextResource[]; }; export type ConfigFor = T extends { DEFAULT_CONFIG: infer D } From 2ef418d297f6f6fafa5567d3463d4991b86a3a33 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Mon, 9 Feb 2026 11:57:06 +0000 Subject: [PATCH 2/5] fix: warehouseID should be optional --- .serena/.gitignore | 1 + .../appkit-servicecontext-plugin-patterns.md | 262 ++++++++++++++++++ .serena/project.yml | 112 ++++++++ ...x-merge-config-dedup-array-plugins-plan.md | 123 ++++++++ ...al-warehouse-id-in-service-context-plan.md | 195 +++++++++++++ 5 files changed, 693 insertions(+) create mode 100644 .serena/.gitignore create mode 100644 .serena/memories/appkit-servicecontext-plugin-patterns.md create mode 100644 .serena/project.yml create mode 100644 docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md create mode 100644 docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md diff --git a/.serena/.gitignore b/.serena/.gitignore new file mode 100644 index 00000000..14d86ad6 --- /dev/null +++ b/.serena/.gitignore @@ -0,0 +1 @@ +/cache diff --git a/.serena/memories/appkit-servicecontext-plugin-patterns.md b/.serena/memories/appkit-servicecontext-plugin-patterns.md new file mode 100644 index 00000000..ad377a01 --- /dev/null +++ b/.serena/memories/appkit-servicecontext-plugin-patterns.md @@ -0,0 +1,262 @@ +# AppKit ServiceContext & Plugin Architecture Learnings + +## Overview + +This document captures key patterns and learnings about ServiceContext initialization, plugin dependency management, and configuration requirements in the AppKit SDK. + +## Key Patterns + +### 1. ServiceContext Initialization (Singleton Pattern) + +**Location:** `/packages/appkit/src/context/service-context.ts` + +**Pattern:** ServiceContext is a singleton that manages the service principal's WorkspaceClient and shared resources. + +```typescript +// Initialize once at app startup (safe to call multiple times) +await ServiceContext.initialize(); + +// Get the initialized context (throws if not initialized) +const ctx = ServiceContext.get(); + +// Check initialization status +ServiceContext.isInitialized(); +``` + +**Key characteristics:** +- Initialized once at app startup via `createApp()` +- Returns the same instance on multiple calls (idempotent) +- Caches both the instance and the initialization promise +- Throws `InitializationError.notInitialized()` if accessed before initialization + +**What it provides:** +- `client`: WorkspaceClient authenticated as service principal +- `serviceUserId`: The service principal's user ID +- `warehouseId`: Promise resolving to warehouse ID (from env or auto-detected) +- `workspaceId`: Promise resolving to workspace ID (from env or API) + +### 2. Plugin Initialization Sequence (3 Phases) + +**Location:** `/packages/appkit/src/core/appkit.ts` + +Plugins initialize in strict order: + +1. **Core Phase** (first) + - Reserved for framework-level plugins + - Example: Internal system plugins + +2. **Normal Phase** (default) + - Application plugins initialize here + - Can access other normal plugins via `config.plugins` (deferred only) + +3. **Deferred Phase** (last) + - Initializes after all core and normal plugins + - Full access to other plugin instances via `config.plugins` + - Use for plugins that depend on other plugins (e.g., Server Plugin) + +```typescript +class MyPlugin extends Plugin { + static phase: PluginPhase = "deferred"; // Options: "core", "normal", "deferred" + + constructor(config: IMyConfig & { plugins?: Record }) { + super(config); + // Can now access config.plugins to reference other plugins + } +} +``` + +### 3. Warehouse ID Resolution (Lazy Promises) + +**Location:** `/packages/appkit/src/context/service-context.ts` (lines 192-251) + +ServiceContext uses lazy promise resolution for warehouse ID: + +```typescript +// In ServiceContextState interface +warehouseId: Promise; // Not awaited during init, resolved lazily + +// Resolution logic: +// 1. Try DATABRICKS_WAREHOUSE_ID env var (fastest) +// 2. In development: auto-detect first healthy warehouse +// 3. In production: throw error if not configured +``` + +**Key insight:** Warehouse ID is NOT required to initialize ServiceContext, but queries will fail if not available. This enables optional analytics plugin scenarios. + +**Environment variables:** +- `DATABRICKS_WAREHOUSE_ID`: Explicitly set warehouse (required in production if using analytics) +- `DATABRICKS_WORKSPACE_ID`: Workspace ID (auto-fetched if not set) +- `DATABRICKS_HOST`: Workspace URL (required) + +### 4. User Context (asUser Pattern) + +**Location:** `/packages/appkit/src/plugin/plugin.ts` (lines 128-160) + +Plugins support executing operations as the requesting user: + +```typescript +// In route handler +router.post("/my-endpoint", async (req, res) => { + // Execute query as the user + const result = await this.asUser(req).myMethod(); +}); + +// Implementation uses AsyncLocalStorage for context propagation +const userContext = ServiceContext.createUserContext(token, userId); +return runInUserContext(userContext, () => userPlugin.method()); +``` + +**Headers required:** +- `x-forwarded-access-token`: User's OAuth/PAT token (required in production) +- `x-forwarded-user`: User ID (required in production) + +**Development behavior:** Falls back to service principal if token unavailable (logs warning) + +### 5. Execution Context Pattern + +**Location:** `/packages/appkit/src/context/execution-context.ts` + +Two-tier context system: + +```typescript +// Service context (singleton, initialized at startup) +const serviceCtx = ServiceContext.get(); + +// Execution context (current request scope) +const ctx = getExecutionContext(); // Returns user OR service context + +// Helper functions +getCurrentUserId(); // User ID or service principal ID +getWorkspaceClient(); // Appropriate client for current context +getWarehouseId(); // Promise +getWorkspaceId(); // Promise +isInUserContext(); // boolean +``` + +**Pattern:** Execution context is determined at request time via AsyncLocalStorage, enabling seamless user-scoped operations without explicit parameter passing. + +### 6. Plugin Dependencies via Config + +**Location:** `/packages/appkit/src/core/appkit.ts` (lines 46-52) + +Deferred plugins receive other plugin instances: + +```typescript +// During plugin creation, deferred plugins get: +const extraData = { plugins: this.#pluginInstances }; + +// Plugin constructor receives merged config: +constructor(config: IPluginConfig & { plugins?: Record }) { + super(config); + // Can access: this.config.plugins.somePlugin +} +``` + +## Optional Dependencies Pattern + +Based on the codebase analysis, here's the recommended pattern for optional plugins: + +### Pattern: Optional Warehouse + +```typescript +class MyPlugin extends Plugin { + protected envVars = []; // Empty = no required env vars + + constructor(config: IMyConfig) { + super(config); + } + + async myQueryMethod() { + try { + const warehouseId = await getWarehouseId(); + // Use warehouse... + } catch (error) { + if (error instanceof ConfigurationError) { + throw new InitializationError( + "Warehouse not configured. Set DATABRICKS_WAREHOUSE_ID in app.yaml", + { cause: error } + ); + } + throw error; + } + } +} +``` + +### Pattern: Optional Plugin Dependency + +```typescript +class MyPlugin extends Plugin { + static phase: PluginPhase = "deferred"; + + constructor(config: IMyConfig & { plugins?: Record }) { + super(config); + this.analyticsPlugin = config.plugins?.analytics; + } + + async getAnalytics() { + if (!this.analyticsPlugin) { + throw new InitializationError("Analytics plugin not configured"); + } + return this.analyticsPlugin.query("..."); + } +} +``` + +## Configuration Hierarchy + +**Creation flow:** +``` +createApp({ + plugins: [...], + telemetry?: TelemetryConfig, + cache?: CacheConfig +}) + ↓ +TelemetryManager.initialize(config?.telemetry) +CacheManager.getInstance(config?.cache) +ServiceContext.initialize() // ← Fails here if auth misconfigured + ↓ +Plugin instantiation (Core → Normal → Deferred) + ↓ +Plugin.setup() called for each plugin +``` + +## Error Handling Patterns + +**InitializationError:** +- Thrown when accessing services before ready +- Used for setup failures and missing required resources +- Has special factory methods: `notInitialized()`, `setupFailed()`, `migrationFailed()` + +**ConfigurationError:** +- Thrown for missing env vars or environment misconfigurations +- Includes hints for resolution + +## Important Implementation Details + +1. **ServiceContext is eager**: Initialized at app startup before plugins, not lazy +2. **Warehouse ID is lazy**: Promise-based, resolved on first use +3. **Plugin setup() is sequential**: All await in parallel via `Promise.all()` +4. **asUser() uses Proxy**: Wraps method calls to inject user context +5. **EXCLUDED_FROM_PROXY**: Lifecycle methods don't wrap (`setup`, `shutdown`, `validateEnv`, etc.) +6. **Development mode fallback**: asUser() falls back to service principal in dev if token missing + +## Common Mistakes to Avoid + +1. **Accessing ServiceContext before initialize()**: Will throw InitializationError +2. **Missing DATABRICKS_HOST**: ServiceContext.initialize() will fail +3. **Assuming warehouse in production**: Must configure DATABRICKS_WAREHOUSE_ID in app.yaml +4. **Using getWarehouseId() synchronously**: It's always a Promise +5. **Forgetting plugin phase**: Normal plugins can't reliably access other plugins +6. **asUser() without proper headers**: Will throw AuthenticationError in production + +## File References + +Key source files for reference: +- `/packages/appkit/src/context/service-context.ts` - ServiceContext implementation +- `/packages/appkit/src/context/execution-context.ts` - Execution context helpers +- `/packages/appkit/src/core/appkit.ts` - Plugin initialization sequence +- `/packages/appkit/src/plugin/plugin.ts` - Plugin base class with asUser pattern +- `/docs/docs/plugins.md` - Plugin system documentation +- `/docs/docs/configuration.mdx` - Environment variable reference diff --git a/.serena/project.yml b/.serena/project.yml new file mode 100644 index 00000000..d9c557c5 --- /dev/null +++ b/.serena/project.yml @@ -0,0 +1,112 @@ +# the name by which the project can be referenced within Serena +project_name: "appkit" + + +# list of languages for which language servers are started; choose from: +# al bash clojure cpp csharp +# csharp_omnisharp dart elixir elm erlang +# fortran fsharp go groovy haskell +# java julia kotlin lua markdown +# matlab nix pascal perl php +# powershell python python_jedi r rego +# ruby ruby_solargraph rust scala swift +# terraform toml typescript typescript_vts vue +# yaml zig +# (This list may be outdated. For the current list, see values of Language enum here: +# https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py +# For some languages, there are alternative language servers, e.g. csharp_omnisharp, ruby_solargraph.) +# Note: +# - For C, use cpp +# - For JavaScript, use typescript +# - For Free Pascal/Lazarus, use pascal +# Special requirements: +# Some languages require additional setup/installations. +# See here for details: https://oraios.github.io/serena/01-about/020_programming-languages.html#language-servers +# When using multiple languages, the first language server that supports a given file will be used for that file. +# The first language is the default language and the respective language server will be used as a fallback. +# Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored. +languages: +- typescript + +# the encoding used by text files in the project +# For a list of possible encodings, see https://docs.python.org/3.11/library/codecs.html#standard-encodings +encoding: "utf-8" + +# whether to use project's .gitignore files to ignore files +ignore_all_files_in_gitignore: true + +# list of additional paths to ignore in all projects +# same syntax as gitignore, so you can use * and ** +ignored_paths: [] + +# whether the project is in read-only mode +# If set to true, all editing tools will be disabled and attempts to use them will result in an error +# Added on 2025-04-18 +read_only: false + +# list of tool names to exclude. We recommend not excluding any tools, see the readme for more details. +# Below is the complete list of tools for convenience. +# To make sure you have the latest list of tools, and to view their descriptions, +# execute `uv run scripts/print_tool_overview.py`. +# +# * `activate_project`: Activates a project by name. +# * `check_onboarding_performed`: Checks whether project onboarding was already performed. +# * `create_text_file`: Creates/overwrites a file in the project directory. +# * `delete_lines`: Deletes a range of lines within a file. +# * `delete_memory`: Deletes a memory from Serena's project-specific memory store. +# * `execute_shell_command`: Executes a shell command. +# * `find_referencing_code_snippets`: Finds code snippets in which the symbol at the given location is referenced. +# * `find_referencing_symbols`: Finds symbols that reference the symbol at the given location (optionally filtered by type). +# * `find_symbol`: Performs a global (or local) search for symbols with/containing a given name/substring (optionally filtered by type). +# * `get_current_config`: Prints the current configuration of the agent, including the active and available projects, tools, contexts, and modes. +# * `get_symbols_overview`: Gets an overview of the top-level symbols defined in a given file. +# * `initial_instructions`: Gets the initial instructions for the current project. +# Should only be used in settings where the system prompt cannot be set, +# e.g. in clients you have no control over, like Claude Desktop. +# * `insert_after_symbol`: Inserts content after the end of the definition of a given symbol. +# * `insert_at_line`: Inserts content at a given line in a file. +# * `insert_before_symbol`: Inserts content before the beginning of the definition of a given symbol. +# * `list_dir`: Lists files and directories in the given directory (optionally with recursion). +# * `list_memories`: Lists memories in Serena's project-specific memory store. +# * `onboarding`: Performs onboarding (identifying the project structure and essential tasks, e.g. for testing or building). +# * `prepare_for_new_conversation`: Provides instructions for preparing for a new conversation (in order to continue with the necessary context). +# * `read_file`: Reads a file within the project directory. +# * `read_memory`: Reads the memory with the given name from Serena's project-specific memory store. +# * `remove_project`: Removes a project from the Serena configuration. +# * `replace_lines`: Replaces a range of lines within a file with new content. +# * `replace_symbol_body`: Replaces the full definition of a symbol. +# * `restart_language_server`: Restarts the language server, may be necessary when edits not through Serena happen. +# * `search_for_pattern`: Performs a search for a pattern in the project. +# * `summarize_changes`: Provides instructions for summarizing the changes made to the codebase. +# * `switch_modes`: Activates modes by providing a list of their names +# * `think_about_collected_information`: Thinking tool for pondering the completeness of collected information. +# * `think_about_task_adherence`: Thinking tool for determining whether the agent is still on track with the current task. +# * `think_about_whether_you_are_done`: Thinking tool for determining whether the task is truly completed. +# * `write_memory`: Writes a named memory (for future reference) to Serena's project-specific memory store. +excluded_tools: [] + +# list of tools to include that would otherwise be disabled (particularly optional tools that are disabled by default) +included_optional_tools: [] + +# fixed set of tools to use as the base tool set (if non-empty), replacing Serena's default set of tools. +# This cannot be combined with non-empty excluded_tools or included_optional_tools. +fixed_tools: [] + +# list of mode names to that are always to be included in the set of active modes +# The full set of modes to be activated is base_modes + default_modes. +# If the setting is undefined, the base_modes from the global configuration (serena_config.yml) apply. +# Otherwise, this setting overrides the global configuration. +# Set this to [] to disable base modes for this project. +# Set this to a list of mode names to always include the respective modes for this project. +base_modes: + +# list of mode names that are to be activated by default. +# The full set of modes to be activated is base_modes + default_modes. +# If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. +# Otherwise, this overrides the setting from the global configuration (serena_config.yml). +# This setting can, in turn, be overridden by CLI parameters (--mode). +default_modes: + +# initial prompt for the project. It will always be given to the LLM upon activating the project +# (contrary to the memories, which are loaded on demand). +initial_prompt: "" diff --git a/docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md b/docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md new file mode 100644 index 00000000..a7eae957 --- /dev/null +++ b/docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md @@ -0,0 +1,123 @@ +--- +title: "fix: mergeConfigDedup crashes on array-returning Vite plugins" +type: fix +date: 2026-02-06 +--- + +# fix: mergeConfigDedup crashes on array-returning Vite plugins + +## Overview + +`mergeConfigDedup` in `packages/appkit/src/utils/vite-config-merge.ts` assumes every entry in the `plugins` array is a single `Plugin` object with a `.name` property. However, Vite's `PluginOption` type allows plugins to be arrays (nested), falsy values (`false | null | undefined`), or promises. Plugins like `@tailwindcss/vite` return `Plugin[]` — a standard Vite convention called "plugin presets." This causes `mergeConfigDedup` to fail because it accesses `.name` on an array. + +## Problem Statement + +```typescript +// Current code (vite-config-merge.ts:11-17) +merged.plugins = [...base.plugins, ...override.plugins].filter( + (p: Plugin) => { // <-- assumes p is always a Plugin object + const name = p.name; // <-- crashes/undefined when p is an array + if (seen.has(name)) return false; + seen.add(name); + return true; + }, +); +``` + +**When a user's Vite config includes:** +```typescript +// user's vite.config.ts +import tailwindcss from "@tailwindcss/vite"; +export default { plugins: [tailwindcss()] } // tailwindcss() returns Plugin[] +``` + +The spread `...base.plugins` produces `[Plugin[]]` (an array containing an array), and accessing `.name` on the inner array returns `undefined`, breaking deduplication. + +**Vite's own `PluginOption` type for reference:** +```typescript +type PluginOption = Thenable +type FalsyPlugin = false | null | undefined +``` + +## Proposed Solution + +Flatten and filter the plugins array before deduplicating, matching Vite's own behavior: + +1. **Flatten** nested arrays recursively (handles `@tailwindcss/vite` and similar) +2. **Filter out falsy values** (`false`, `null`, `undefined`) — standard Vite convention for conditional plugins +3. **Then deduplicate** by `.name` as before + +### Implementation + +**File:** `packages/appkit/src/utils/vite-config-merge.ts` + +```typescript +import type { Plugin } from "vite"; + +function flattenPlugins(plugins: any[]): Plugin[] { + return plugins.flat(Infinity).filter(Boolean) as Plugin[]; +} + +export function mergeConfigDedup( + base: any, + override: any, + mergeFn: (a: any, b: any) => any, +) { + const merged = mergeFn(base, override); + if (base.plugins && override.plugins) { + const seen = new Set(); + const allPlugins = flattenPlugins([...base.plugins, ...override.plugins]); + merged.plugins = allPlugins.filter((p) => { + const name = p.name; + if (seen.has(name)) return false; + seen.add(name); + return true; + }); + } + return merged; +} +``` + +### Key decisions + +- **`flat(Infinity)`** handles arbitrarily nested arrays (Vite's `PluginOption[]` is recursive) +- **`filter(Boolean)`** removes `false | null | undefined` entries (Vite convention for conditional plugins like `condition && plugin()`) +- Helper is a local function, not exported — only needed here + +## Acceptance Criteria + +- [x] `mergeConfigDedup` handles plugins that return arrays (e.g. `@tailwindcss/vite`) +- [x] Falsy plugin entries (`false`, `null`, `undefined`) are filtered out without errors +- [x] Deeply nested plugin arrays are flattened correctly +- [x] Deduplication by `.name` still works as before for single Plugin objects +- [x] Unit tests cover: single plugins, array plugins, mixed, falsy values, nested arrays, dedup across both + +## Test Plan + +**New test file:** `packages/appkit/src/utils/tests/vite-config-merge.test.ts` + +```typescript +describe("mergeConfigDedup", () => { + // existing behavior + it("deduplicates plugins by name across base and override") + it("preserves base plugin when duplicate exists in override") + it("returns merged config when no plugins on either side") + + // new: array plugin support + it("flattens array-returning plugins (e.g. @tailwindcss/vite)") + it("deduplicates across flattened array plugins and single plugins") + it("handles deeply nested plugin arrays") + + // new: falsy plugin filtering + it("filters out false/null/undefined plugin entries") + it("handles mixed falsy and valid plugins") +}); +``` + +## References + +- **Bug location:** `packages/appkit/src/utils/vite-config-merge.ts:11-17` +- **Call site:** `packages/appkit/src/plugins/server/vite-dev-server.ts:75` +- **Vite PluginOption type:** [vite/src/node/plugin.ts](https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugin.ts) +- **@tailwindcss/vite source:** [tailwindcss/@tailwindcss-vite/src/index.ts](https://github.com/tailwindlabs/tailwindcss/blob/main/packages/@tailwindcss-vite/src/index.ts) — returns 3 plugins in an array +- **Vite plugin preset convention:** [vite.dev/guide/api-plugin](https://vite.dev/guide/api-plugin) diff --git a/docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md b/docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md new file mode 100644 index 00000000..dff9e10a --- /dev/null +++ b/docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md @@ -0,0 +1,195 @@ +--- +title: "fix: Make warehouseId optional in ServiceContext when no plugin requires it" +type: fix +date: 2026-02-09 +--- + +# fix: Make warehouseId optional in ServiceContext when no plugin requires it + +## Overview + +`ServiceContext.createContext()` unconditionally calls `getWarehouseId(client)`, which in production throws if `DATABRICKS_WAREHOUSE_ID` is unset, and in dev mode fires an unnecessary API call. Only `AnalyticsPlugin` uses the warehouse ID. Apps using only `server()` should not require it. + +## Problem Statement + +``` +createApp({ plugins: [server()] }) + -> ServiceContext.initialize() + -> createContext() + -> getWarehouseId(client) // always called, throws in prod if env var missing +``` + +## Proposed Solution + +Add `static requires` to the Plugin class so plugins declare which shared resources they need. At `createApp()` time, collect requirements from all plugins. Pass them to `ServiceContext.initialize()`. Only call `getWarehouseId()` if a plugin declared it. Make `warehouseId` optional on the interfaces. + +### Changes + +#### 1. Add `ServiceContextResource` type and `requires` to `PluginConstructor` (`packages/shared/src/plugin.ts`) + +```typescript +export type ServiceContextResource = "warehouseId" | "workspaceId"; + +export type PluginConstructor< + C = BasePluginConfig, + I extends BasePlugin = BasePlugin, +> = (new (config: C) => I) & { + DEFAULT_CONFIG?: Record; + phase?: PluginPhase; + requires?: ServiceContextResource[]; // NEW +}; +``` + +#### 2. Add default `requires` to `Plugin` base class (`packages/appkit/src/plugin/plugin.ts`) + +```typescript +static requires: ServiceContextResource[] = []; +``` + +#### 3. Set `requires` on `AnalyticsPlugin` (`packages/appkit/src/plugins/analytics/analytics.ts`) + +```typescript +export class AnalyticsPlugin extends Plugin { + static requires: ServiceContextResource[] = ["warehouseId"]; + // ... +} +``` + +#### 4. Update `ServiceContext` (`packages/appkit/src/context/service-context.ts`) + +Make `warehouseId` optional. Only resolve when required. + +```typescript +export interface ServiceContextState { + client: WorkspaceClient; + serviceUserId: string; + warehouseId?: Promise; // optional now + workspaceId: Promise; +} + +static async initialize( + requiredResources: ServiceContextResource[] = [] +): Promise { + if (ServiceContext.instance) return ServiceContext.instance; + if (ServiceContext.initPromise) return ServiceContext.initPromise; + + ServiceContext.initPromise = ServiceContext.createContext(requiredResources); + ServiceContext.instance = await ServiceContext.initPromise; + return ServiceContext.instance; +} + +private static async createContext( + requiredResources: ServiceContextResource[] = [] +): Promise { + const client = new WorkspaceClient({}, getClientOptions()); + + // Only resolve warehouseId if a plugin requires it + const warehouseId = requiredResources.includes("warehouseId") + ? ServiceContext.getWarehouseId(client) + : undefined; + + const workspaceId = ServiceContext.getWorkspaceId(client); + const currentUser = await client.currentUser.me(); + + if (!currentUser.id) { + throw ConfigurationError.resourceNotFound("Service user ID"); + } + + return { client, serviceUserId: currentUser.id, warehouseId, workspaceId }; +} +``` + +#### 5. Update `UserContext` interface (`packages/appkit/src/context/user-context.ts`) + +```typescript +export interface UserContext { + client: ServiceContextState["client"]; + userId: string; + userName?: string; + warehouseId?: Promise; // optional now + workspaceId: Promise; + isUserContext: true; +} +``` + +#### 6. Update `getWarehouseId()` helper (`packages/appkit/src/context/execution-context.ts`) + +Since `warehouseId` is now optional on the interface, use non-null assertion — startup validation already guarantees it exists when a plugin declared `requires = ["warehouseId"]`. + +```typescript +export function getWarehouseId(): Promise { + // Safe: if a plugin requires warehouseId, ServiceContext.initialize() resolved it + return getExecutionContext().warehouseId!; +} +``` + +#### 7. Update `_createApp()` to collect and pass requirements (`packages/appkit/src/core/appkit.ts`) + +```typescript +static async _createApp<...>(config = {}): Promise> { + TelemetryManager.initialize(config?.telemetry); + await CacheManager.getInstance(config?.cache); + + const rawPlugins = config.plugins as T; + const requiredResources = AppKit.collectRequiredResources(rawPlugins); + + await ServiceContext.initialize(requiredResources); + + const preparedPlugins = AppKit.preparePlugins(rawPlugins); + // ...rest unchanged... +} + +private static collectRequiredResources( + plugins: PluginData[] +): ServiceContextResource[] { + const resources = new Set(); + for (const { plugin } of plugins) { + for (const req of plugin.requires ?? []) { + resources.add(req); + } + } + return [...resources]; +} +``` + +#### 8. No changes needed + +- **`createUserContext()`**: passes `serviceCtx.warehouseId` — works as-is (`undefined` or real promise) +- **Type generator / Vite plugin**: reads env var directly, already handles missing +- **CLI generate-types**: reads env var directly, already handles missing + +## Acceptance Criteria + +- [x] `createApp({ plugins: [server()] })` succeeds without `DATABRICKS_WAREHOUSE_ID` in production +- [x] `createApp({ plugins: [server(), analytics()] })` fails fast at startup if `DATABRICKS_WAREHOUSE_ID` missing in production +- [x] `createApp({ plugins: [server()] })` in dev mode does NOT fire warehouse discovery API call +- [x] Custom plugins declaring `static requires = ["warehouseId"]` trigger warehouse resolution +- [x] Existing apps with `DATABRICKS_WAREHOUSE_ID` set work identically (backward compatible) +- [x] `getWarehouseId()` throws clear error when no warehouse-requiring plugin was registered +- [x] All existing tests pass +- [ ] New tests cover: server-only startup, analytics startup, missing warehouse error path + +## Files to Modify + +| File | Change | +|------|--------| +| `packages/shared/src/plugin.ts` | Add `ServiceContextResource` type, add `requires?` to `PluginConstructor` | +| `packages/appkit/src/plugin/plugin.ts` | Add `static requires: ServiceContextResource[] = []` | +| `packages/appkit/src/plugins/analytics/analytics.ts` | Add `static requires = ["warehouseId"]` | +| `packages/appkit/src/context/service-context.ts` | Make `warehouseId` optional, conditionally resolve in `createContext()` | +| `packages/appkit/src/context/user-context.ts` | Make `warehouseId` optional | +| `packages/appkit/src/context/execution-context.ts` | Add `!` assertion in `getWarehouseId()` for optional type | +| `packages/appkit/src/core/appkit.ts` | Add `collectRequiredResources()`, pass to `ServiceContext.initialize()` | +| `tools/test-helpers.ts` | Update mock contexts to handle optional warehouseId | + +## Risks & Mitigations + +| Risk | Mitigation | +|------|------------| +| Custom plugins calling `getWarehouseId()` without `requires` | Returns `undefined` — plugin developer must declare `requires` | +| `PluginConstructor` type change | `requires` is optional — no existing code breaks | +| `ServiceContextState.warehouseId` becoming optional | `getWarehouseId()` uses `!` assertion — safe because startup validation guarantees existence when required | + +## Unresolved Questions + +1. **Should `workspaceId` also be made conditional?** The `ServiceContextResource` type already includes it for future use. Recommend: leave eager for now, can apply the same pattern later. From 78f5a22127419b885c0ebe0c474b72cc044dbea9 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Mon, 9 Feb 2026 12:21:13 +0000 Subject: [PATCH 3/5] fix: rename --- .gitignore | 4 + .serena/.gitignore | 1 - .../appkit-servicecontext-plugin-patterns.md | 262 ------------------ .serena/project.yml | 112 -------- ...x-merge-config-dedup-array-plugins-plan.md | 123 -------- ...al-warehouse-id-in-service-context-plan.md | 195 ------------- .../appkit/src/context/execution-context.ts | 12 +- packages/appkit/src/core/appkit.ts | 2 +- packages/appkit/src/plugin/plugin.ts | 2 +- .../appkit/src/plugins/analytics/analytics.ts | 2 +- packages/shared/src/plugin.ts | 4 +- 11 files changed, 17 insertions(+), 702 deletions(-) delete mode 100644 .serena/.gitignore delete mode 100644 .serena/memories/appkit-servicecontext-plugin-patterns.md delete mode 100644 .serena/project.yml delete mode 100644 docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md delete mode 100644 docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md diff --git a/.gitignore b/.gitignore index 3b6cc969..fdbf6744 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,7 @@ coverage *.tsbuildinfo .turbo + +# AI generated files +.serena +docs/plans/ \ No newline at end of file diff --git a/.serena/.gitignore b/.serena/.gitignore deleted file mode 100644 index 14d86ad6..00000000 --- a/.serena/.gitignore +++ /dev/null @@ -1 +0,0 @@ -/cache diff --git a/.serena/memories/appkit-servicecontext-plugin-patterns.md b/.serena/memories/appkit-servicecontext-plugin-patterns.md deleted file mode 100644 index ad377a01..00000000 --- a/.serena/memories/appkit-servicecontext-plugin-patterns.md +++ /dev/null @@ -1,262 +0,0 @@ -# AppKit ServiceContext & Plugin Architecture Learnings - -## Overview - -This document captures key patterns and learnings about ServiceContext initialization, plugin dependency management, and configuration requirements in the AppKit SDK. - -## Key Patterns - -### 1. ServiceContext Initialization (Singleton Pattern) - -**Location:** `/packages/appkit/src/context/service-context.ts` - -**Pattern:** ServiceContext is a singleton that manages the service principal's WorkspaceClient and shared resources. - -```typescript -// Initialize once at app startup (safe to call multiple times) -await ServiceContext.initialize(); - -// Get the initialized context (throws if not initialized) -const ctx = ServiceContext.get(); - -// Check initialization status -ServiceContext.isInitialized(); -``` - -**Key characteristics:** -- Initialized once at app startup via `createApp()` -- Returns the same instance on multiple calls (idempotent) -- Caches both the instance and the initialization promise -- Throws `InitializationError.notInitialized()` if accessed before initialization - -**What it provides:** -- `client`: WorkspaceClient authenticated as service principal -- `serviceUserId`: The service principal's user ID -- `warehouseId`: Promise resolving to warehouse ID (from env or auto-detected) -- `workspaceId`: Promise resolving to workspace ID (from env or API) - -### 2. Plugin Initialization Sequence (3 Phases) - -**Location:** `/packages/appkit/src/core/appkit.ts` - -Plugins initialize in strict order: - -1. **Core Phase** (first) - - Reserved for framework-level plugins - - Example: Internal system plugins - -2. **Normal Phase** (default) - - Application plugins initialize here - - Can access other normal plugins via `config.plugins` (deferred only) - -3. **Deferred Phase** (last) - - Initializes after all core and normal plugins - - Full access to other plugin instances via `config.plugins` - - Use for plugins that depend on other plugins (e.g., Server Plugin) - -```typescript -class MyPlugin extends Plugin { - static phase: PluginPhase = "deferred"; // Options: "core", "normal", "deferred" - - constructor(config: IMyConfig & { plugins?: Record }) { - super(config); - // Can now access config.plugins to reference other plugins - } -} -``` - -### 3. Warehouse ID Resolution (Lazy Promises) - -**Location:** `/packages/appkit/src/context/service-context.ts` (lines 192-251) - -ServiceContext uses lazy promise resolution for warehouse ID: - -```typescript -// In ServiceContextState interface -warehouseId: Promise; // Not awaited during init, resolved lazily - -// Resolution logic: -// 1. Try DATABRICKS_WAREHOUSE_ID env var (fastest) -// 2. In development: auto-detect first healthy warehouse -// 3. In production: throw error if not configured -``` - -**Key insight:** Warehouse ID is NOT required to initialize ServiceContext, but queries will fail if not available. This enables optional analytics plugin scenarios. - -**Environment variables:** -- `DATABRICKS_WAREHOUSE_ID`: Explicitly set warehouse (required in production if using analytics) -- `DATABRICKS_WORKSPACE_ID`: Workspace ID (auto-fetched if not set) -- `DATABRICKS_HOST`: Workspace URL (required) - -### 4. User Context (asUser Pattern) - -**Location:** `/packages/appkit/src/plugin/plugin.ts` (lines 128-160) - -Plugins support executing operations as the requesting user: - -```typescript -// In route handler -router.post("/my-endpoint", async (req, res) => { - // Execute query as the user - const result = await this.asUser(req).myMethod(); -}); - -// Implementation uses AsyncLocalStorage for context propagation -const userContext = ServiceContext.createUserContext(token, userId); -return runInUserContext(userContext, () => userPlugin.method()); -``` - -**Headers required:** -- `x-forwarded-access-token`: User's OAuth/PAT token (required in production) -- `x-forwarded-user`: User ID (required in production) - -**Development behavior:** Falls back to service principal if token unavailable (logs warning) - -### 5. Execution Context Pattern - -**Location:** `/packages/appkit/src/context/execution-context.ts` - -Two-tier context system: - -```typescript -// Service context (singleton, initialized at startup) -const serviceCtx = ServiceContext.get(); - -// Execution context (current request scope) -const ctx = getExecutionContext(); // Returns user OR service context - -// Helper functions -getCurrentUserId(); // User ID or service principal ID -getWorkspaceClient(); // Appropriate client for current context -getWarehouseId(); // Promise -getWorkspaceId(); // Promise -isInUserContext(); // boolean -``` - -**Pattern:** Execution context is determined at request time via AsyncLocalStorage, enabling seamless user-scoped operations without explicit parameter passing. - -### 6. Plugin Dependencies via Config - -**Location:** `/packages/appkit/src/core/appkit.ts` (lines 46-52) - -Deferred plugins receive other plugin instances: - -```typescript -// During plugin creation, deferred plugins get: -const extraData = { plugins: this.#pluginInstances }; - -// Plugin constructor receives merged config: -constructor(config: IPluginConfig & { plugins?: Record }) { - super(config); - // Can access: this.config.plugins.somePlugin -} -``` - -## Optional Dependencies Pattern - -Based on the codebase analysis, here's the recommended pattern for optional plugins: - -### Pattern: Optional Warehouse - -```typescript -class MyPlugin extends Plugin { - protected envVars = []; // Empty = no required env vars - - constructor(config: IMyConfig) { - super(config); - } - - async myQueryMethod() { - try { - const warehouseId = await getWarehouseId(); - // Use warehouse... - } catch (error) { - if (error instanceof ConfigurationError) { - throw new InitializationError( - "Warehouse not configured. Set DATABRICKS_WAREHOUSE_ID in app.yaml", - { cause: error } - ); - } - throw error; - } - } -} -``` - -### Pattern: Optional Plugin Dependency - -```typescript -class MyPlugin extends Plugin { - static phase: PluginPhase = "deferred"; - - constructor(config: IMyConfig & { plugins?: Record }) { - super(config); - this.analyticsPlugin = config.plugins?.analytics; - } - - async getAnalytics() { - if (!this.analyticsPlugin) { - throw new InitializationError("Analytics plugin not configured"); - } - return this.analyticsPlugin.query("..."); - } -} -``` - -## Configuration Hierarchy - -**Creation flow:** -``` -createApp({ - plugins: [...], - telemetry?: TelemetryConfig, - cache?: CacheConfig -}) - ↓ -TelemetryManager.initialize(config?.telemetry) -CacheManager.getInstance(config?.cache) -ServiceContext.initialize() // ← Fails here if auth misconfigured - ↓ -Plugin instantiation (Core → Normal → Deferred) - ↓ -Plugin.setup() called for each plugin -``` - -## Error Handling Patterns - -**InitializationError:** -- Thrown when accessing services before ready -- Used for setup failures and missing required resources -- Has special factory methods: `notInitialized()`, `setupFailed()`, `migrationFailed()` - -**ConfigurationError:** -- Thrown for missing env vars or environment misconfigurations -- Includes hints for resolution - -## Important Implementation Details - -1. **ServiceContext is eager**: Initialized at app startup before plugins, not lazy -2. **Warehouse ID is lazy**: Promise-based, resolved on first use -3. **Plugin setup() is sequential**: All await in parallel via `Promise.all()` -4. **asUser() uses Proxy**: Wraps method calls to inject user context -5. **EXCLUDED_FROM_PROXY**: Lifecycle methods don't wrap (`setup`, `shutdown`, `validateEnv`, etc.) -6. **Development mode fallback**: asUser() falls back to service principal in dev if token missing - -## Common Mistakes to Avoid - -1. **Accessing ServiceContext before initialize()**: Will throw InitializationError -2. **Missing DATABRICKS_HOST**: ServiceContext.initialize() will fail -3. **Assuming warehouse in production**: Must configure DATABRICKS_WAREHOUSE_ID in app.yaml -4. **Using getWarehouseId() synchronously**: It's always a Promise -5. **Forgetting plugin phase**: Normal plugins can't reliably access other plugins -6. **asUser() without proper headers**: Will throw AuthenticationError in production - -## File References - -Key source files for reference: -- `/packages/appkit/src/context/service-context.ts` - ServiceContext implementation -- `/packages/appkit/src/context/execution-context.ts` - Execution context helpers -- `/packages/appkit/src/core/appkit.ts` - Plugin initialization sequence -- `/packages/appkit/src/plugin/plugin.ts` - Plugin base class with asUser pattern -- `/docs/docs/plugins.md` - Plugin system documentation -- `/docs/docs/configuration.mdx` - Environment variable reference diff --git a/.serena/project.yml b/.serena/project.yml deleted file mode 100644 index d9c557c5..00000000 --- a/.serena/project.yml +++ /dev/null @@ -1,112 +0,0 @@ -# the name by which the project can be referenced within Serena -project_name: "appkit" - - -# list of languages for which language servers are started; choose from: -# al bash clojure cpp csharp -# csharp_omnisharp dart elixir elm erlang -# fortran fsharp go groovy haskell -# java julia kotlin lua markdown -# matlab nix pascal perl php -# powershell python python_jedi r rego -# ruby ruby_solargraph rust scala swift -# terraform toml typescript typescript_vts vue -# yaml zig -# (This list may be outdated. For the current list, see values of Language enum here: -# https://github.com/oraios/serena/blob/main/src/solidlsp/ls_config.py -# For some languages, there are alternative language servers, e.g. csharp_omnisharp, ruby_solargraph.) -# Note: -# - For C, use cpp -# - For JavaScript, use typescript -# - For Free Pascal/Lazarus, use pascal -# Special requirements: -# Some languages require additional setup/installations. -# See here for details: https://oraios.github.io/serena/01-about/020_programming-languages.html#language-servers -# When using multiple languages, the first language server that supports a given file will be used for that file. -# The first language is the default language and the respective language server will be used as a fallback. -# Note that when using the JetBrains backend, language servers are not used and this list is correspondingly ignored. -languages: -- typescript - -# the encoding used by text files in the project -# For a list of possible encodings, see https://docs.python.org/3.11/library/codecs.html#standard-encodings -encoding: "utf-8" - -# whether to use project's .gitignore files to ignore files -ignore_all_files_in_gitignore: true - -# list of additional paths to ignore in all projects -# same syntax as gitignore, so you can use * and ** -ignored_paths: [] - -# whether the project is in read-only mode -# If set to true, all editing tools will be disabled and attempts to use them will result in an error -# Added on 2025-04-18 -read_only: false - -# list of tool names to exclude. We recommend not excluding any tools, see the readme for more details. -# Below is the complete list of tools for convenience. -# To make sure you have the latest list of tools, and to view their descriptions, -# execute `uv run scripts/print_tool_overview.py`. -# -# * `activate_project`: Activates a project by name. -# * `check_onboarding_performed`: Checks whether project onboarding was already performed. -# * `create_text_file`: Creates/overwrites a file in the project directory. -# * `delete_lines`: Deletes a range of lines within a file. -# * `delete_memory`: Deletes a memory from Serena's project-specific memory store. -# * `execute_shell_command`: Executes a shell command. -# * `find_referencing_code_snippets`: Finds code snippets in which the symbol at the given location is referenced. -# * `find_referencing_symbols`: Finds symbols that reference the symbol at the given location (optionally filtered by type). -# * `find_symbol`: Performs a global (or local) search for symbols with/containing a given name/substring (optionally filtered by type). -# * `get_current_config`: Prints the current configuration of the agent, including the active and available projects, tools, contexts, and modes. -# * `get_symbols_overview`: Gets an overview of the top-level symbols defined in a given file. -# * `initial_instructions`: Gets the initial instructions for the current project. -# Should only be used in settings where the system prompt cannot be set, -# e.g. in clients you have no control over, like Claude Desktop. -# * `insert_after_symbol`: Inserts content after the end of the definition of a given symbol. -# * `insert_at_line`: Inserts content at a given line in a file. -# * `insert_before_symbol`: Inserts content before the beginning of the definition of a given symbol. -# * `list_dir`: Lists files and directories in the given directory (optionally with recursion). -# * `list_memories`: Lists memories in Serena's project-specific memory store. -# * `onboarding`: Performs onboarding (identifying the project structure and essential tasks, e.g. for testing or building). -# * `prepare_for_new_conversation`: Provides instructions for preparing for a new conversation (in order to continue with the necessary context). -# * `read_file`: Reads a file within the project directory. -# * `read_memory`: Reads the memory with the given name from Serena's project-specific memory store. -# * `remove_project`: Removes a project from the Serena configuration. -# * `replace_lines`: Replaces a range of lines within a file with new content. -# * `replace_symbol_body`: Replaces the full definition of a symbol. -# * `restart_language_server`: Restarts the language server, may be necessary when edits not through Serena happen. -# * `search_for_pattern`: Performs a search for a pattern in the project. -# * `summarize_changes`: Provides instructions for summarizing the changes made to the codebase. -# * `switch_modes`: Activates modes by providing a list of their names -# * `think_about_collected_information`: Thinking tool for pondering the completeness of collected information. -# * `think_about_task_adherence`: Thinking tool for determining whether the agent is still on track with the current task. -# * `think_about_whether_you_are_done`: Thinking tool for determining whether the task is truly completed. -# * `write_memory`: Writes a named memory (for future reference) to Serena's project-specific memory store. -excluded_tools: [] - -# list of tools to include that would otherwise be disabled (particularly optional tools that are disabled by default) -included_optional_tools: [] - -# fixed set of tools to use as the base tool set (if non-empty), replacing Serena's default set of tools. -# This cannot be combined with non-empty excluded_tools or included_optional_tools. -fixed_tools: [] - -# list of mode names to that are always to be included in the set of active modes -# The full set of modes to be activated is base_modes + default_modes. -# If the setting is undefined, the base_modes from the global configuration (serena_config.yml) apply. -# Otherwise, this setting overrides the global configuration. -# Set this to [] to disable base modes for this project. -# Set this to a list of mode names to always include the respective modes for this project. -base_modes: - -# list of mode names that are to be activated by default. -# The full set of modes to be activated is base_modes + default_modes. -# If the setting is undefined, the default_modes from the global configuration (serena_config.yml) apply. -# Otherwise, this overrides the setting from the global configuration (serena_config.yml). -# This setting can, in turn, be overridden by CLI parameters (--mode). -default_modes: - -# initial prompt for the project. It will always be given to the LLM upon activating the project -# (contrary to the memories, which are loaded on demand). -initial_prompt: "" diff --git a/docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md b/docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md deleted file mode 100644 index a7eae957..00000000 --- a/docs/plans/2026-02-06-fix-merge-config-dedup-array-plugins-plan.md +++ /dev/null @@ -1,123 +0,0 @@ ---- -title: "fix: mergeConfigDedup crashes on array-returning Vite plugins" -type: fix -date: 2026-02-06 ---- - -# fix: mergeConfigDedup crashes on array-returning Vite plugins - -## Overview - -`mergeConfigDedup` in `packages/appkit/src/utils/vite-config-merge.ts` assumes every entry in the `plugins` array is a single `Plugin` object with a `.name` property. However, Vite's `PluginOption` type allows plugins to be arrays (nested), falsy values (`false | null | undefined`), or promises. Plugins like `@tailwindcss/vite` return `Plugin[]` — a standard Vite convention called "plugin presets." This causes `mergeConfigDedup` to fail because it accesses `.name` on an array. - -## Problem Statement - -```typescript -// Current code (vite-config-merge.ts:11-17) -merged.plugins = [...base.plugins, ...override.plugins].filter( - (p: Plugin) => { // <-- assumes p is always a Plugin object - const name = p.name; // <-- crashes/undefined when p is an array - if (seen.has(name)) return false; - seen.add(name); - return true; - }, -); -``` - -**When a user's Vite config includes:** -```typescript -// user's vite.config.ts -import tailwindcss from "@tailwindcss/vite"; -export default { plugins: [tailwindcss()] } // tailwindcss() returns Plugin[] -``` - -The spread `...base.plugins` produces `[Plugin[]]` (an array containing an array), and accessing `.name` on the inner array returns `undefined`, breaking deduplication. - -**Vite's own `PluginOption` type for reference:** -```typescript -type PluginOption = Thenable -type FalsyPlugin = false | null | undefined -``` - -## Proposed Solution - -Flatten and filter the plugins array before deduplicating, matching Vite's own behavior: - -1. **Flatten** nested arrays recursively (handles `@tailwindcss/vite` and similar) -2. **Filter out falsy values** (`false`, `null`, `undefined`) — standard Vite convention for conditional plugins -3. **Then deduplicate** by `.name` as before - -### Implementation - -**File:** `packages/appkit/src/utils/vite-config-merge.ts` - -```typescript -import type { Plugin } from "vite"; - -function flattenPlugins(plugins: any[]): Plugin[] { - return plugins.flat(Infinity).filter(Boolean) as Plugin[]; -} - -export function mergeConfigDedup( - base: any, - override: any, - mergeFn: (a: any, b: any) => any, -) { - const merged = mergeFn(base, override); - if (base.plugins && override.plugins) { - const seen = new Set(); - const allPlugins = flattenPlugins([...base.plugins, ...override.plugins]); - merged.plugins = allPlugins.filter((p) => { - const name = p.name; - if (seen.has(name)) return false; - seen.add(name); - return true; - }); - } - return merged; -} -``` - -### Key decisions - -- **`flat(Infinity)`** handles arbitrarily nested arrays (Vite's `PluginOption[]` is recursive) -- **`filter(Boolean)`** removes `false | null | undefined` entries (Vite convention for conditional plugins like `condition && plugin()`) -- Helper is a local function, not exported — only needed here - -## Acceptance Criteria - -- [x] `mergeConfigDedup` handles plugins that return arrays (e.g. `@tailwindcss/vite`) -- [x] Falsy plugin entries (`false`, `null`, `undefined`) are filtered out without errors -- [x] Deeply nested plugin arrays are flattened correctly -- [x] Deduplication by `.name` still works as before for single Plugin objects -- [x] Unit tests cover: single plugins, array plugins, mixed, falsy values, nested arrays, dedup across both - -## Test Plan - -**New test file:** `packages/appkit/src/utils/tests/vite-config-merge.test.ts` - -```typescript -describe("mergeConfigDedup", () => { - // existing behavior - it("deduplicates plugins by name across base and override") - it("preserves base plugin when duplicate exists in override") - it("returns merged config when no plugins on either side") - - // new: array plugin support - it("flattens array-returning plugins (e.g. @tailwindcss/vite)") - it("deduplicates across flattened array plugins and single plugins") - it("handles deeply nested plugin arrays") - - // new: falsy plugin filtering - it("filters out false/null/undefined plugin entries") - it("handles mixed falsy and valid plugins") -}); -``` - -## References - -- **Bug location:** `packages/appkit/src/utils/vite-config-merge.ts:11-17` -- **Call site:** `packages/appkit/src/plugins/server/vite-dev-server.ts:75` -- **Vite PluginOption type:** [vite/src/node/plugin.ts](https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugin.ts) -- **@tailwindcss/vite source:** [tailwindcss/@tailwindcss-vite/src/index.ts](https://github.com/tailwindlabs/tailwindcss/blob/main/packages/@tailwindcss-vite/src/index.ts) — returns 3 plugins in an array -- **Vite plugin preset convention:** [vite.dev/guide/api-plugin](https://vite.dev/guide/api-plugin) diff --git a/docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md b/docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md deleted file mode 100644 index dff9e10a..00000000 --- a/docs/plans/2026-02-09-fix-optional-warehouse-id-in-service-context-plan.md +++ /dev/null @@ -1,195 +0,0 @@ ---- -title: "fix: Make warehouseId optional in ServiceContext when no plugin requires it" -type: fix -date: 2026-02-09 ---- - -# fix: Make warehouseId optional in ServiceContext when no plugin requires it - -## Overview - -`ServiceContext.createContext()` unconditionally calls `getWarehouseId(client)`, which in production throws if `DATABRICKS_WAREHOUSE_ID` is unset, and in dev mode fires an unnecessary API call. Only `AnalyticsPlugin` uses the warehouse ID. Apps using only `server()` should not require it. - -## Problem Statement - -``` -createApp({ plugins: [server()] }) - -> ServiceContext.initialize() - -> createContext() - -> getWarehouseId(client) // always called, throws in prod if env var missing -``` - -## Proposed Solution - -Add `static requires` to the Plugin class so plugins declare which shared resources they need. At `createApp()` time, collect requirements from all plugins. Pass them to `ServiceContext.initialize()`. Only call `getWarehouseId()` if a plugin declared it. Make `warehouseId` optional on the interfaces. - -### Changes - -#### 1. Add `ServiceContextResource` type and `requires` to `PluginConstructor` (`packages/shared/src/plugin.ts`) - -```typescript -export type ServiceContextResource = "warehouseId" | "workspaceId"; - -export type PluginConstructor< - C = BasePluginConfig, - I extends BasePlugin = BasePlugin, -> = (new (config: C) => I) & { - DEFAULT_CONFIG?: Record; - phase?: PluginPhase; - requires?: ServiceContextResource[]; // NEW -}; -``` - -#### 2. Add default `requires` to `Plugin` base class (`packages/appkit/src/plugin/plugin.ts`) - -```typescript -static requires: ServiceContextResource[] = []; -``` - -#### 3. Set `requires` on `AnalyticsPlugin` (`packages/appkit/src/plugins/analytics/analytics.ts`) - -```typescript -export class AnalyticsPlugin extends Plugin { - static requires: ServiceContextResource[] = ["warehouseId"]; - // ... -} -``` - -#### 4. Update `ServiceContext` (`packages/appkit/src/context/service-context.ts`) - -Make `warehouseId` optional. Only resolve when required. - -```typescript -export interface ServiceContextState { - client: WorkspaceClient; - serviceUserId: string; - warehouseId?: Promise; // optional now - workspaceId: Promise; -} - -static async initialize( - requiredResources: ServiceContextResource[] = [] -): Promise { - if (ServiceContext.instance) return ServiceContext.instance; - if (ServiceContext.initPromise) return ServiceContext.initPromise; - - ServiceContext.initPromise = ServiceContext.createContext(requiredResources); - ServiceContext.instance = await ServiceContext.initPromise; - return ServiceContext.instance; -} - -private static async createContext( - requiredResources: ServiceContextResource[] = [] -): Promise { - const client = new WorkspaceClient({}, getClientOptions()); - - // Only resolve warehouseId if a plugin requires it - const warehouseId = requiredResources.includes("warehouseId") - ? ServiceContext.getWarehouseId(client) - : undefined; - - const workspaceId = ServiceContext.getWorkspaceId(client); - const currentUser = await client.currentUser.me(); - - if (!currentUser.id) { - throw ConfigurationError.resourceNotFound("Service user ID"); - } - - return { client, serviceUserId: currentUser.id, warehouseId, workspaceId }; -} -``` - -#### 5. Update `UserContext` interface (`packages/appkit/src/context/user-context.ts`) - -```typescript -export interface UserContext { - client: ServiceContextState["client"]; - userId: string; - userName?: string; - warehouseId?: Promise; // optional now - workspaceId: Promise; - isUserContext: true; -} -``` - -#### 6. Update `getWarehouseId()` helper (`packages/appkit/src/context/execution-context.ts`) - -Since `warehouseId` is now optional on the interface, use non-null assertion — startup validation already guarantees it exists when a plugin declared `requires = ["warehouseId"]`. - -```typescript -export function getWarehouseId(): Promise { - // Safe: if a plugin requires warehouseId, ServiceContext.initialize() resolved it - return getExecutionContext().warehouseId!; -} -``` - -#### 7. Update `_createApp()` to collect and pass requirements (`packages/appkit/src/core/appkit.ts`) - -```typescript -static async _createApp<...>(config = {}): Promise> { - TelemetryManager.initialize(config?.telemetry); - await CacheManager.getInstance(config?.cache); - - const rawPlugins = config.plugins as T; - const requiredResources = AppKit.collectRequiredResources(rawPlugins); - - await ServiceContext.initialize(requiredResources); - - const preparedPlugins = AppKit.preparePlugins(rawPlugins); - // ...rest unchanged... -} - -private static collectRequiredResources( - plugins: PluginData[] -): ServiceContextResource[] { - const resources = new Set(); - for (const { plugin } of plugins) { - for (const req of plugin.requires ?? []) { - resources.add(req); - } - } - return [...resources]; -} -``` - -#### 8. No changes needed - -- **`createUserContext()`**: passes `serviceCtx.warehouseId` — works as-is (`undefined` or real promise) -- **Type generator / Vite plugin**: reads env var directly, already handles missing -- **CLI generate-types**: reads env var directly, already handles missing - -## Acceptance Criteria - -- [x] `createApp({ plugins: [server()] })` succeeds without `DATABRICKS_WAREHOUSE_ID` in production -- [x] `createApp({ plugins: [server(), analytics()] })` fails fast at startup if `DATABRICKS_WAREHOUSE_ID` missing in production -- [x] `createApp({ plugins: [server()] })` in dev mode does NOT fire warehouse discovery API call -- [x] Custom plugins declaring `static requires = ["warehouseId"]` trigger warehouse resolution -- [x] Existing apps with `DATABRICKS_WAREHOUSE_ID` set work identically (backward compatible) -- [x] `getWarehouseId()` throws clear error when no warehouse-requiring plugin was registered -- [x] All existing tests pass -- [ ] New tests cover: server-only startup, analytics startup, missing warehouse error path - -## Files to Modify - -| File | Change | -|------|--------| -| `packages/shared/src/plugin.ts` | Add `ServiceContextResource` type, add `requires?` to `PluginConstructor` | -| `packages/appkit/src/plugin/plugin.ts` | Add `static requires: ServiceContextResource[] = []` | -| `packages/appkit/src/plugins/analytics/analytics.ts` | Add `static requires = ["warehouseId"]` | -| `packages/appkit/src/context/service-context.ts` | Make `warehouseId` optional, conditionally resolve in `createContext()` | -| `packages/appkit/src/context/user-context.ts` | Make `warehouseId` optional | -| `packages/appkit/src/context/execution-context.ts` | Add `!` assertion in `getWarehouseId()` for optional type | -| `packages/appkit/src/core/appkit.ts` | Add `collectRequiredResources()`, pass to `ServiceContext.initialize()` | -| `tools/test-helpers.ts` | Update mock contexts to handle optional warehouseId | - -## Risks & Mitigations - -| Risk | Mitigation | -|------|------------| -| Custom plugins calling `getWarehouseId()` without `requires` | Returns `undefined` — plugin developer must declare `requires` | -| `PluginConstructor` type change | `requires` is optional — no existing code breaks | -| `ServiceContextState.warehouseId` becoming optional | `getWarehouseId()` uses `!` assertion — safe because startup validation guarantees existence when required | - -## Unresolved Questions - -1. **Should `workspaceId` also be made conditional?** The `ServiceContextResource` type already includes it for future use. Recommend: leave eager for now, can apply the same pattern later. diff --git a/packages/appkit/src/context/execution-context.ts b/packages/appkit/src/context/execution-context.ts index 9fbc2523..459746eb 100644 --- a/packages/appkit/src/context/execution-context.ts +++ b/packages/appkit/src/context/execution-context.ts @@ -1,4 +1,5 @@ import { AsyncLocalStorage } from "node:async_hooks"; +import { ConfigurationError } from "../errors"; import { ServiceContext } from "./service-context"; import { type ExecutionContext, @@ -62,12 +63,15 @@ export function getWorkspaceClient() { /** * Get the warehouse ID promise. - * Safe to use non-null assertion: if a plugin requires warehouseId, - * ServiceContext.initialize() will have resolved it at startup. */ export function getWarehouseId(): Promise { - // biome-ignore lint/style/noNonNullAssertion: warehouseId is guaranteed to exist when a plugin declares requires=["warehouseId"] - return getExecutionContext().warehouseId!; + const ctx = getExecutionContext(); + if (!ctx.warehouseId) { + throw new ConfigurationError( + "warehouseId is not available - this should not happen, ensure the plugin is correctly configured", + ); + } + return ctx.warehouseId; } /** diff --git a/packages/appkit/src/core/appkit.ts b/packages/appkit/src/core/appkit.ts index 9dd0fad2..017966a6 100644 --- a/packages/appkit/src/core/appkit.ts +++ b/packages/appkit/src/core/appkit.ts @@ -187,7 +187,7 @@ export class AppKit { ): ServiceContextResource[] { const resources = new Set(); for (const { plugin } of plugins) { - for (const req of plugin.requires ?? []) { + for (const req of plugin.requiredResources ?? []) { resources.add(req); } } diff --git a/packages/appkit/src/plugin/plugin.ts b/packages/appkit/src/plugin/plugin.ts index bd6cdbbd..3a1e8161 100644 --- a/packages/appkit/src/plugin/plugin.ts +++ b/packages/appkit/src/plugin/plugin.ts @@ -77,7 +77,7 @@ export abstract class Plugin< private registeredEndpoints: PluginEndpointMap = {}; static phase: PluginPhase = "normal"; - static requires: ServiceContextResource[] = []; + static requiredResources: ServiceContextResource[] = []; name: string; constructor(protected config: TConfig) { diff --git a/packages/appkit/src/plugins/analytics/analytics.ts b/packages/appkit/src/plugins/analytics/analytics.ts index e9cb30b8..43b269ca 100644 --- a/packages/appkit/src/plugins/analytics/analytics.ts +++ b/packages/appkit/src/plugins/analytics/analytics.ts @@ -26,7 +26,7 @@ import type { const logger = createLogger("analytics"); export class AnalyticsPlugin extends Plugin { - static requires: ServiceContextResource[] = ["warehouseId"]; + static requiredResources: ServiceContextResource[] = ["warehouseId"]; name = "analytics"; protected envVars: string[] = []; diff --git a/packages/shared/src/plugin.ts b/packages/shared/src/plugin.ts index 36dbb2b1..a596f1db 100644 --- a/packages/shared/src/plugin.ts +++ b/packages/shared/src/plugin.ts @@ -46,7 +46,7 @@ export interface PluginConfig { export type PluginPhase = "core" | "normal" | "deferred"; -export type ServiceContextResource = "warehouseId" | "workspaceId"; +export type ServiceContextResource = "warehouseId"; export type PluginConstructor< C = BasePluginConfig, @@ -56,7 +56,7 @@ export type PluginConstructor< ) => I) & { DEFAULT_CONFIG?: Record; phase?: PluginPhase; - requires?: ServiceContextResource[]; + requiredResources?: ServiceContextResource[]; }; export type ConfigFor = T extends { DEFAULT_CONFIG: infer D } From 034d790fc37f0067b48fdb486996f50592be4061 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Mon, 9 Feb 2026 12:25:40 +0000 Subject: [PATCH 4/5] refactor: better error handling --- packages/appkit/src/context/execution-context.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/appkit/src/context/execution-context.ts b/packages/appkit/src/context/execution-context.ts index 459746eb..af6a6980 100644 --- a/packages/appkit/src/context/execution-context.ts +++ b/packages/appkit/src/context/execution-context.ts @@ -67,8 +67,9 @@ export function getWorkspaceClient() { export function getWarehouseId(): Promise { const ctx = getExecutionContext(); if (!ctx.warehouseId) { - throw new ConfigurationError( - "warehouseId is not available - this should not happen, ensure the plugin is correctly configured", + throw ConfigurationError.resourceNotFound( + "Warehouse ID", + 'Ensure a plugin declares static requires = ["warehouseId"] or set DATABRICKS_WAREHOUSE_ID', ); } return ctx.warehouseId; From f5e252d96772996ffea0600221039c3147342952 Mon Sep 17 00:00:00 2001 From: Pedro Figueiredo Date: Mon, 9 Feb 2026 14:16:56 +0000 Subject: [PATCH 5/5] fix: error message --- packages/appkit/src/context/execution-context.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/appkit/src/context/execution-context.ts b/packages/appkit/src/context/execution-context.ts index af6a6980..663511bb 100644 --- a/packages/appkit/src/context/execution-context.ts +++ b/packages/appkit/src/context/execution-context.ts @@ -69,7 +69,7 @@ export function getWarehouseId(): Promise { if (!ctx.warehouseId) { throw ConfigurationError.resourceNotFound( "Warehouse ID", - 'Ensure a plugin declares static requires = ["warehouseId"] or set DATABRICKS_WAREHOUSE_ID', + 'Ensure a plugin declares static requiredResources = ["warehouseId"] or set DATABRICKS_WAREHOUSE_ID', ); } return ctx.warehouseId;