feat(workflow-executor): allow custom DB schema via DATABASE_SCHEMA env var by nbouliol · Pull Request #1754 · ForestAdmin/agent-nodejs

nbouliol · 2026-07-13T09:57:46Z

Summary

The standalone workflow-executor CLI was locked to the Postgres forest schema. It read no schema env var and never set database.schema, so resolveSchema always fell back to DEFAULT_SCHEMA. Library consumers could already pass database.schema — this closes the CLI gap.

Changes

cli-core.ts: new DATABASE_SCHEMA env var (mirrors DATABASE_SSL) — read into CliConfig, injected into the database options, surfaced in --help and the startup log. Blank/unset resolves to the forest default.
Tests covering env read (value / unset / blank), forwarding to buildDatabase, and the startup-log schema.
README: documented DATABASE_SCHEMA in the Database connection table + note.

No store/build changes: the downstream plumbing and forest fallback already existed.

Test plan

yarn workspace @forestadmin/workflow-executor test -- test/cli.test.ts (108 passed)
yarn workspace @forestadmin/workflow-executor lint (0 errors)
yarn workspace @forestadmin/workflow-executor build

Fixes PRD-773

🤖 Generated with Claude Code

Note

Add `DATABASE_SCHEMA` env var support to workflow-executor for custom Postgres schema

Adds a DATABASE_SCHEMA environment variable to cli-core.ts that sets the Postgres schema used by the executor, defaulting to "forest".
Validates the value as a valid Postgres identifier (max 63 chars, enforced regex); blank/whitespace values are treated as unset and invalid values throw a ConfigurationError on startup.
Forwards the resolved schema to DatabaseExecutorOptions when set, and includes it in structured startup logs.
Risk: the executor requires CREATE privilege on the database at boot time due to CREATE SCHEMA IF NOT EXISTS.

Changes since #1754 opened

Modified runMigrations in schema-migrations.ts to probe pg_namespace for schema existence before attempting schema creation [b6c7707]
Added tests in database-store-schema.test.ts to verify schema existence probing behavior [b6c7707]
Updated documentation in CLAUDE.md and README.md to document schema existence probing and reduced privilege requirements [b6c7707]
Added validation to resolveSchema function in schema-migrations module to throw an error when a configured schema is not a valid Postgres identifier on non-sqlite dialects, while sqlite dialects return undefined without validation [f249390]
Introduced shared Postgres identifier validation utilities in schema-migrations module including POSTGRES_IDENTIFIER regex constant, MAX_IDENTIFIER_LENGTH constant set to 63, and isValidPostgresIdentifier function [f249390]
Refactored parseSchemaEnv function in cli-core module to use shared validation utilities from schema-migrations module instead of locally defined constants and regex [f249390]
Added tests verifying schema validation behavior including acceptance of 63-character schemas in cli.test.ts, Postgres-specific validation with pg_namespace probe queries in database-store-schema.test.ts, sqlite dialect ignoring invalid schemas, and programmatic API validation for invalid values and excessive length [f249390]

^{Macroscope summarized f5efb65.}

…nv var The standalone CLI was locked to the `forest` schema: it read no schema env var and never set `database.schema`, so resolveSchema always fell back to DEFAULT_SCHEMA. Add a DATABASE_SCHEMA env var (mirroring DATABASE_SSL) so the CLI can target another schema; blank/unset keeps the `forest` default. Library consumers could already pass database.schema — this closes the CLI gap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

linear-code · 2026-07-13T09:57:50Z

PRD-773

…up log Replace the bare 'forest' literal in logStartup with the existing DEFAULT_SCHEMA constant, removing the duplicated default, and drop the redundant what/where comment on the DATABASE_SCHEMA read. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

qltysh · 2026-07-13T10:19:28Z

Coverage Impact

⬆️ Merging this pull request will increase total coverage on main by 0.02%.

Modified Files with Diff Coverage (2)

Rating	File	% Diff	Uncovered Line #s
	packages/workflow-executor/src/stores/schema-migrations.ts	100.0%
	packages/workflow-executor/src/cli-core.ts	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.
Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.
File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)
- Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

matthv · 2026-07-13T10:20:18Z

Nice — clean change and the DEFAULT_SCHEMA refactor addresses the duplicated-default nit. Two points still worth considering before merge:

1. Validate `DATABASE_SCHEMA` (fail-fast)

env.DATABASE_SCHEMA?.trim() flows unescaped all the way into raw DDL in schema-migrations.ts:

sequelize.query(`CREATE SCHEMA IF NOT EXISTS "${schema}"`, { transaction });
// and quoted references: `"${schema}"."${tableName}"`

Before this PR the value was always the forest constant, so there was no surface. Now it's operator-controlled — a malformed value (typo, embedded ", etc.) fails at boot with a cryptic SQL error instead of a clear message. Suggest validating at the boundary in readEnvConfig, reusing the existing ConfigurationError pattern:

const schema = env.DATABASE_SCHEMA?.trim();
if (schema && !/^[a-zA-Z_][a-zA-Z0-9_$]*$/.test(schema)) {
  throw new ConfigurationError('DATABASE_SCHEMA must be a valid PostgreSQL identifier');
}

(The regex follows PG's unquoted-identifier rules; can be relaxed to only reject "/NUL if exotic names ever need supporting.)

2. Document the `CREATE` privilege requirement (README)

On boot the executor runs CREATE SCHEMA IF NOT EXISTS, which requires the CREATE privilege on the database when the schema doesn't exist — and in Postgres the ACL check happens before the IF NOT EXISTS short-circuit, so pre-creating the schema doesn't remove that requirement. This is exactly the constraint behind PRD-773, so a one-liner near the DATABASE_SCHEMA note would help:

The executor creates the schema on boot if it doesn't exist, which requires the CREATE privilege on the database. To avoid granting it, pre-create the schema and grant USAGE, CREATE on it to the executor's DB user.

Neither is functionally blocking; (1) is the hardening I'd recommend since this PR is what makes the schema configurable.

… privilege The schema now flows into raw DDL, so validate it as a Postgres identifier at the boundary (fail fast with ConfigurationError instead of a cryptic SQL error), and reject values past the 63-char limit Postgres would silently truncate. Document in the README that boot runs CREATE SCHEMA IF NOT EXISTS, which needs CREATE on the database even when the schema already exists. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

nbouliol · 2026-07-13T10:27:20Z

Both addressed in f5efb65.

1. Validation — DATABASE_SCHEMA is now validated at the boundary in readEnvConfig via a parseSchemaEnv helper that throws ConfigurationError for non-identifiers (used your regex ^[a-zA-Z_][a-zA-Z0-9_$]*$), plus a 63-char cap since Postgres silently truncates longer identifiers and would target a different schema. Covered by tests (rejects leading digit, dash, space, embedded ", dot, and >63 chars).

2. README — documented. I verified the privilege behavior empirically on PG 15 rather than trust my assumption: with a role lacking CREATE on the database, CREATE SCHEMA IF NOT EXISTS analytics fails with permission denied for database even when analytics already exists. So you are right — the ACL check fires before the IF NOT EXISTS short-circuit, and pre-creating does not remove the requirement. I reworded the note accordingly (GRANT CREATE ON DATABASE), and dropped the misleading "pre-create to avoid it" workaround.

Worth noting this requirement is not new to this PR: the boot-time CREATE SCHEMA IF NOT EXISTS already ran for the default forest schema, so the executor has always needed CREATE on the database. This PR only makes the schema name configurable.

matthv

LGTM

…xists Postgres checks the database-level CREATE privilege for CREATE SCHEMA IF NOT EXISTS even when the schema is present, so an operator who pre-creates the schema still needed database-level CREATE just to boot. Probe pg_namespace first (world-readable, no privilege) inside the migration lock and skip the CREATE when the schema exists, so a DB user with only schema-level CREATE on a pre-created schema can run the executor. Document both grant paths in the README. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Scra3

Automated review (code / tests / comments). No blocking issues — the change is well-constructed: SQL injection is closed at the CLI boundary, the pg_namespace probe is correct and stays inside the advisory lock, and comments are why-only. Inline notes below are all non-blocking.

Two findings couldn't be anchored to diff lines:

src/build-workflow-executor.ts:313 — new DatabaseStore({ schema: mergedOptions.schema }) forwards the schema to raw DDL unvalidated; only the CLI path validates. Defense-in-depth (operator-supplied config, same trust as uri), not exploitable — worth centralizing validation in DatabaseStore/runMigrations if you want the programmatic API guarded too.
src/stores/schema-migrations.ts:65 — pre-existing doc comment still says the forest schema is created; it's now configurable, reword to "the configured schema (default forest)".

Scra3 · 2026-07-21T08:04:31Z

-        sequelize.query(`CREATE SCHEMA IF NOT EXISTS "${schema}"`, { transaction }),
-      );
+      await withMigrationLock(sequelize, async transaction => {
+        if (!(await schemaExists(sequelize, schema, transaction))) {


praise: clean probe-then-skip — gating CREATE SCHEMA on a world-readable pg_namespace lookup so a pre-created schema boots without database-level CREATE is the right call, and keeping it inside the lock preserves serialization.

Also hardened the programmatic path in the same commit: the identifier check moved to isValidPostgresIdentifier and is now enforced in resolveSchema, so buildDatabaseExecutor({ database: { schema } }) is validated too, not only the CLI env boundary.

Scra3 · 2026-07-21T08:04:31Z

+    },
+  );
+
+  it('rejects a DATABASE_SCHEMA longer than 63 characters', () => {


suggestion: only the 64-char rejection is tested; add a case asserting a 63-char identifier is accepted so a >→>= regression on the boundary gets caught.

Fixed in f249390 — added accepts a DATABASE_SCHEMA of exactly 63 characters asserting the value round-trips, so a >→>= regression on the boundary now fails a test.

Scra3 · 2026-07-21T08:04:31Z

      );
    });

+    it('probes pg_namespace for the schema before creating it', async () => {


suggestion: the ordering test doesn't record the probe, so moving it outside the lock — breaking the documented serialization — would still pass; add probe to the recorded calls sequence.

Fixed in f249390 — the ordering mock now records the pg_namespace probe and the assertion is ['guard','lock','probe','schema','guard','lock','migrate'], so moving the probe outside the lock breaks the test.

Scra3 · 2026-07-21T08:04:31Z

+      );
+    });
+
+    it('skips CREATE SCHEMA when the schema already exists', async () => {


nit: this asserts CREATE SCHEMA was skipped but not that the probe ran, so a silent no-op would pass too — assert the pg_namespace probe was called to make the skip causally explicit.

Fixed in f249390 — the skip test now also asserts the pg_namespace probe was called, making the skip causally explicit (not a silent no-op).

…API too Address review findings on PR #1754: - centralize the Postgres-identifier check in `isValidPostgresIdentifier` and enforce it in `resolveSchema`, so `buildDatabaseExecutor({ database: { schema } })` is guarded, not only the CLI env boundary - test the 63-char boundary is accepted (only the 64-char rejection was covered) - record the pg_namespace probe in the migration-ordering assertion so moving it outside the lock is caught - assert the probe ran in the skip-CREATE test (skip is caused by the probe, not a no-op) - fix stale comments that still described the schema as fixed `forest` Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

# @forestadmin/workflow-executor [1.19.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/workflow-executor@1.18.0...@forestadmin/workflow-executor@1.19.0) (2026-07-21) ### Features * **workflow-executor:** allow custom DB schema via DATABASE_SCHEMA env var ([#1754](#1754)) ([5114744](5114744))

matthv approved these changes Jul 13, 2026

View reviewed changes

Scra3 approved these changes Jul 21, 2026

View reviewed changes

Scra3 reviewed Jul 21, 2026

View reviewed changes

Scra3 merged commit 5114744 into main Jul 21, 2026
37 checks passed

Scra3 deleted the feature/prd-773-workflow-executor-allow-custom-postgres-schema-via branch July 21, 2026 08:36

Uh oh!

Conversation

nbouliol commented Jul 13, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Changes

Test plan

Add DATABASE_SCHEMA env var support to workflow-executor for custom Postgres schema

Changes since #1754 opened

Uh oh!

linear-code Bot commented Jul 13, 2026

Uh oh!

qltysh Bot commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthv commented Jul 13, 2026

1. Validate DATABASE_SCHEMA (fail-fast)

2. Document the CREATE privilege requirement (README)

Uh oh!

nbouliol commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

matthv left a comment

Choose a reason for hiding this comment

Uh oh!

Scra3 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nbouliol commented Jul 13, 2026 •

edited by macroscopeapp Bot

Loading

Add `DATABASE_SCHEMA` env var support to workflow-executor for custom Postgres schema

qltysh Bot commented Jul 13, 2026 •

edited

Loading

1. Validate `DATABASE_SCHEMA` (fail-fast)

2. Document the `CREATE` privilege requirement (README)

nbouliol commented Jul 13, 2026 •

edited

Loading