feat: generic scope-aware module loader with pgpm-based tests#111
Merged
Conversation
- ModuleConfigLoader<T> base class: TTL caching, scope-aware resolution, AmbiguousScopeError / ModuleNotProvisionedError (no fallback defaults) - 5 concrete loaders: function, namespace, secrets, storage, invocation - ModuleLoader facade composing all sub-loaders - Queryable interface (compatible with pg.Pool and PgTestClient) - Integration tests use seed.pgpm() to deploy real metaschema-modules schema - CI workflow with constructiveio/postgres-plus:18
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
- job/compute-service: use ModuleLoader instead of ComputeModuleLoader - job/compute-worker: scope derivation uses entity_type || null - job/worker: graph-complete, compute-meter, storage-meter use ModuleLoader - packages/agentic-server: already using new API (verified) - packages/functions-test: worker helper uses new ModuleLoader - packages/module-loader: graph loader queries graph_execution_module, adds nodeStatesTable, removes all hardcoded defaults - Remove backward-compat shims (UsageLoader, USAGE_DEFAULTS, KNOWN_TABLES)
All 8 integration test files updated to work with the new ModuleLoader API: - Created centralized createModuleMockQuery() helper with MODULE_CONFIGS constants for realistic metaschema resolution - Removed all _resetCache imports (no longer exported) - Updated completeNode/failNode signatures (added databaseId param) - Updated table name filters to use MODULE_CONFIGS instead of hardcoded strings - Fixed param index assertions to match new INSERT structures - Removed dual-INSERT expectations (consolidated into single table) 116 integration tests pass across all 12 suites.
When scope is null and multiple module instances exist for a database, fall back to loading the 'app' scope instance. This matches the design intent: app/platform/database scopes are functionally equivalent for the default (non-entity) case. Affected consumers: - InvocationTracker.resolveInvocationModule() - ComputeLogTracker.log() / .rollup() - FunctionDiscovery.resolve() / .listInvocable() - compute-meter (fire-and-forget) - graph-complete (resolveGraph) - storage-meter (resolveComputeLog) - inference-meter (resolveComputeLog)
The previous fallback tried load(dbId, 'app') which fails when the test infrastructure provisions modules with non-'app' scopes (e.g. 'platform' + 'org'). Using loadAll()[0] picks the first available instance without assuming a specific scope name. Also fixes worker.test.ts: invalidateAll() → invalidate() to match the ModuleLoader API.
loadDefault() picks the platform/app/database-scoped instance when multiple module instances exist. This correctly resolves the functions-test scenario where both 'platform' and 'org' scoped invocation modules are provisioned — the worker should use the platform instance when no entity_type is provided.
The @pgpm/services dependency is already declared in extensions/@pgpm/metaschema-modules/package.json. Adding the extensions to the pnpm workspace means pnpm install resolves it automatically — no need for a separate pgpm install step in CI.
Fixes AmbiguousScopeError crash in compute-service boot prereqs and in billing, graph config, and fn-runtime storage metering. These all just need the platform-level module instance.
- Add graph_execution_module deploy/verify/revert from constructive-db to extensions/@pgpm/metaschema-modules (was missing from local copy) - Fix GRAPH_MODULE_SQL to query real table columns (prefix, node_states_table_name, executions_table_name, outputs_table_name) instead of non-existent complete_node_function/fail_node_function - Derive function names from prefix convention (prefix_complete_node, prefix_fail_node) matching the metaschema generator output - Add executionsTable and outputsTable to GraphModuleConfig type
Add pgpm seed migrations that register the full module chain: 1. seed_graph_execution_tables: register merkle + graph execution tables in metaschema 2. register_graph_execution_module: provision merkle_store_module → graph_module → graph_execution_module This resolves 'Module not provisioned' errors when the compute worker attempts to resolve graph execution config via the ModuleLoader.
graph_module requires graphs_table_id, executions_table_id, outputs_table_id FKs to metaschema_public.table. Also seed platform_function_graphs in the table registry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Generic scope-aware module loader replacing hardcoded table names with dynamic resolution from
metaschema_modules_public.*_moduleconfig tables.ModuleConfigLoader<T>— generic base classload(dbId, scope?)— exact scope match, or unambiguous single-instance whennullloadDefault(dbId)— prefers platform/app/database scopes over entity scopes (fallback forAmbiguousScopeError)databaseId;Queryableinterface works withpg.PoolandPgTestClientWorker scope fix
AmbiguousScopeErrorhandlingAll
.load(dbId, null)sites catch ambiguity →loadDefault()fallback: compute-service boot, billing, graph config, fn-runtime storage meter, invocation tracker, compute log, function discovery.Graph loader:
graph_execution_moduleQueries real table columns (
prefix,node_states_table_name,executions_table_name,outputs_table_name). Derives function names from prefix convention (${prefix}_complete_node,${prefix}_fail_node). Added table DDL to localextensions/@pgpm/metaschema-modules. Companion: constructive-io/pgpm-modules#86Tests
Integration tests via
pgsql-test+seed.pgpm()deploying real metaschema-modules schema. No inline DDL.Link to Devin session: https://app.devin.ai/sessions/671da39a6b554e5ea3710c725acc0696
Requested by: @pyramation