Add ChronoPostgresDatastore implementation by tjamescouch · Pull Request #59 · neofinancial/chrono

tjamescouch · 2026-01-27T09:05:55Z

Description

Implement PostgreSQL datastore using TypeORM with raw SQL for atomic task claiming
Use FOR UPDATE SKIP LOCKED pattern for concurrent task claiming
Add PGLite-based tests that run without external database dependency
Fix tsdown config across packages to use hash: false for clean output filenames

Tested in service:

https://github.com/neofinancial/core-ledger-service/pull/712

- Implement PostgreSQL datastore using TypeORM with raw SQL for atomic task claiming - Use FOR UPDATE SKIP LOCKED pattern for concurrent task claiming - Add PGLite-based tests that run without external database dependency - Fix tsdown config across packages to use hash: false for clean output filenames Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

wiz-a38520980d · 2026-01-27T09:06:14Z

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	2 5 4
Sensitive Data	-
Secrets	-
IaC Misconfigurations	1
SAST Findings	-

Total	2 5 4 1

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

tjamescouch · 2026-01-27T09:07:02Z

packages/chrono-postgres-datastore/test/unit/chrono-postgres-datastore.test.ts

@@ -0,0 +1,550 @@
+import { PGlite } from '@electric-sql/pglite';


New dev dependency

- Replace all string interpolation SQL with QueryBuilder methods - Use Brackets for complex OR conditions in claim query - Use transaction with pessimistic_write lock for atomic claiming - Update test mock to support QueryBuilder API Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

Move mock QueryBuilder and DataSource infrastructure from test file to test/helpers/mock-datasource.ts to improve test file readability. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

Split the dense mock-datasource.ts into focused, well-documented modules: - sql-utils.ts: SQL string manipulation (snake_case, params) - entity-mapper.ts: Database row to entity conversion - where-clause-builder.ts: WHERE clause construction - mock-query-builder.ts: TypeORM QueryBuilder simulation - mock-entity-manager.ts: TypeORM EntityManager simulation - mock-datasource.ts: Main DataSource entry point - index.ts: Re-exports with architecture diagram Each file has clear documentation and single responsibility. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

tjamescouch · 2026-01-27T10:22:51Z

packages/chrono-postgres-datastore/test/helpers/entity-mapper.ts

@@ -0,0 +1,64 @@
+/**


I'm using PGLite for in memory postgres testing.
AI generated test magic ahead. This is significantly more complicated due to supporting the ORMs query builder. The other option is allowing string interpolation in the source, pick your poison, this seems better to me.

Ditched PGLite. Doesn't really work with typeORM

packages/chrono-core/tsdown.config.ts

- Remove PGlite and mock helpers in favor of real Postgres integration tests - Fix RETURNING clause mapping: raw PostgreSQL rows use snake_case columns, added fromRaw() helper to convert to camelCase entity properties - Add CI workflow with Postgres 15 service container - Add testAccessor Symbol for testing query generation - Fix setLock syntax from array-based to method chaining Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

tjamescouch · 2026-01-28T02:08:45Z

.github/workflows/ci.yml

@@ -13,6 +13,21 @@ jobs:
    runs-on: ubuntu-latest
    timeout-minutes: 10



I looked into alternatives to testing with real postgres. I tried PGLite but it was not good with TypeORM and defeated the purpose of the tests.

Query generation tests are no longer needed - actual cleanup behavior is already verified by the integration tests. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

New tests: - Concurrent claim safety (SKIP LOCKED verification) - Deferred initialization waiting behavior - EntityManager transaction participation - Claim skips COMPLETED/FAILED tasks - Claim skips non-stale claimed tasks - Retry clears claimedAt - Delete by kind+idempotencyKey edge cases (non-existent, with force) - Cleanup preserves PENDING and FAILED tasks Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

This fixes TypeORM version mismatch issues when the package is used as a dependency. The consumer's TypeORM version will be used instead of bundling a separate copy. Bump to 0.5.2-alpha.0 for testing. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

…t silent failures Add Promise.race with configurable timeout to getDataSource() method. If the datastore is not initialized within the timeout period (default 10s), operations will throw an error instead of hanging indefinitely. This prevents silent failures when multiple datastore instances are created and some are never initialized (e.g., due to DI misconfiguration). Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

- Replace non-null assertions with optional chaining in tests - Pass DataSource as parameter to cleanup methods to avoid non-null assertions - Apply biome formatting fixes Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

darrenpicard25

looks good. just some discussion points

darrenpicard25 · 2026-01-29T16:53:39Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    const entity = manager.create(ChronoTaskEntity, {
+      kind: String(input.kind),
+      status: TaskStatus.PENDING,
+      data: input.data as Record<string, unknown>,


booo to cast despite mongo package doing same thing

darrenpicard25 · 2026-01-29T16:55:27Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    }
+
+    const result = await qb.execute();
+    const [rawRow] = result.raw as Record<string, unknown>[];


packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

darrenpicard25 · 2026-01-29T17:07:42Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    });
+
+    // Opportunistic cleanup runs after claim completes, outside the transaction
+    this.maybeCleanupCompletedTasks();


i get it but slightly dislike this clean up logic being apart of claim flow

can we solve this with postgres built in pub/sub model

Opportunistic cleanup is simpler. LISTEN/NOTIFY adds complexity for marginal benefit.

The AI agrees with me here, but we can discuss more with the group.

yeah lets discuss today

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

packages/chrono-postgres-datastore/package.json

packages/chrono-postgres-datastore/tsconfig.json

packages/chrono-mongo-datastore/tsdown.config.ts

packages/chrono-postgres-datastore/package.json

packages/chrono-postgres-datastore/src/chrono-task.entity.ts

lastExecutedAt should only be set when task execution finishes (complete/fail), not when scheduling a retry. This aligns with the mongo datastore behavior. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

BREAKING CHANGE: Replace TypeORM DataSource with pg Pool interface - Replace initialize(dataSource: DataSource) with initialize(pool: Pool) - Replace datastoreOptions.entityManager with datastoreOptions.client (PoolClient) - Remove ChronoTaskEntity, add ChronoTaskRow type - Export migration SQL (MIGRATION_UP_SQL, MIGRATION_DOWN_SQL) and helpers - Use prepared statements for all queries - Remove typeorm peer dependency, add pg as direct dependency This simplifies the implementation and removes the heavy TypeORM dependency in favor of the lightweight pg library with explicit SQL queries. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

Combine SELECT+UPDATE and SELECT+DELETE into single atomic statements: - claim(): Single UPDATE with subquery replaces transaction with separate SELECT FOR UPDATE SKIP LOCKED + UPDATE - cleanup(): Single DELETE with subquery replaces SELECT IDs + DELETE This reduces round trips and removes explicit transaction management for claim operations. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

- Add test/tsconfig.json to enable typechecking test files - Update package.json typecheck script to include test directory - Fix TypeScript strict mode errors in tests Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

tjamescouch · 2026-01-30T13:59:17Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    const timeoutPromise = new Promise<never>((_, reject) => {
+      setTimeout(() => {
+        reject(
+          new Error(
+            `ChronoPostgresDatastore initialization timeout: datastore was not initialized within ${this.config.initializationTimeoutMs}ms. ` +
+              'Ensure initialize() is called before performing operations.',
+          ),
+        );
+      }, this.config.initializationTimeoutMs);
+    });
+
+    return Promise.race([initPromise, timeoutPromise]);
+  }


@darrenpicard25 As we are doing something similar in chrono-mongo-datastore, is there a race condition between the initialization promise resolving and the first messages that the service starts to publish? Because in ALWAYS mode this would mean losing messages right?

not really a race problem. just more a problem with the datastore never being initialized

darrenpicard25

looking good. few minor things and we will see what people thing in todays meeting

darrenpicard25 · 2026-02-02T17:59:18Z

packages/chrono-postgres-datastore/test/tsconfig.json

@@ -0,0 +1,8 @@
+{
+  "extends": "../../../tsconfig.base.json",


would recommend just extending the chrono-postgres-datastore tsconfig not the base

darrenpicard25 · 2026-02-02T17:59:47Z

packages/chrono-postgres-datastore/package.json

+      },
+      "require": {
+        "types": "./build/index.d.ts",
+        "default": "./build/index.js"


these seem wrong

darrenpicard25 · 2026-02-02T18:00:28Z

packages/chrono-postgres-datastore/package.json

+    "@types/pg": "^8.11.0"
+  },
+  "peerDependencies": {
+    "@neofinancial/chrono": ">=0.5.0"


Suggested change

"@neofinancial/chrono": ">=0.5.0"

"@neofinancial/chrono": "workspace:*"

for now.

darrenpicard25 · 2026-02-02T18:01:03Z

tsconfig.base.json

    "isolatedModules": true,
    "verbatimModuleSyntax": false,
+    "declaration": true,
+    "declarationMap": true,


darrenpicard25 · 2026-02-02T18:02:32Z

packages/chrono-postgres-datastore/src/index.ts

+  type PostgresDatastoreOptions,
+} from './chrono-postgres-datastore';
+export { MIGRATION_DOWN_SQL, MIGRATION_UP_SQL, migrateDown, migrateUp } from './migration';
+export type { ChronoTaskRow } from './types';


should this be exported? maybe. dont actually know. but this would be the equivalent of mongo-datastore exporting the Document type

darrenpicard25 · 2026-02-02T18:05:28Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    input: ScheduleInput<TaskKind, TaskMapping[TaskKind], PostgresDatastoreOptions>,
+  ): Promise<Task<TaskKind, TaskMapping[TaskKind]>> {
+    await this.getPool();
+    const queryable = this.getQueryable(input.datastoreOptions);


huh? we are doing the same things twice but with different behaviour

darrenpicard25 · 2026-02-02T18:07:52Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+        query = DELETE_BY_KEY_QUERY;
+        values = [String(key.kind), key.idempotencyKey, TaskStatus.PENDING];
+      }
+    }


would prefer to see in some sort of "getDeleteTaskQuery" private method over if blocks with let variables

darrenpicard25 · 2026-02-02T19:23:20Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    });
+
+    // Opportunistic cleanup runs after claim completes, outside the transaction
+    this.maybeCleanupCompletedTasks();


yeah lets discuss today

darrenpicard25 · 2026-02-02T19:24:24Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+      id: row.id,
+      kind: row.kind as TaskKind,
+      status: row.status as TaskStatus,
+      data: row.data as TaskMapping[TaskKind],


booo to casting. Feel like this should not be needed based on everything ive seen

darrenpicard25 · 2026-02-02T19:29:23Z

packages/chrono-postgres-datastore/src/chrono-postgres-datastore.ts

+    ];
+
+    try {
+      const result = await queryable.query<ChronoTaskRow>(SCHEDULE_QUERY, values);


not a huge deal but would be nice to enforce that values aligns with the expected types for the query. might not be possible but would be nice

- Made getQueryable async so it handles initialization internally - Updated all public methods (schedule, delete, claim, retry, complete, fail) to use getQueryable consistently instead of mixing getPool() + pool.query() - Simplified schedule() by removing redundant getPool() call - getQueryable now properly awaits pool initialization via getPool() Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

Address PR review feedback to use a more current postgres version in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

wiz-a38520980d · 2026-02-24T20:32:01Z

pnpm-lock.yaml

Vulnerability Finding on line 0

More Details

Vulnerabilities [minimatch:10.0.1]

Name Severity Source Fixed version CVSS score CVSS exploitability score Has public exploit Has CISA KEV exploit

CVE-2026-26996 GHSA-3ppc-4f35-3m26 10.2.1 8.7 3.9 true false

To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason

If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).

To get more details on how to remediate this issue using AI, reply to this conversation with #wiz remediate

wiz-a38520980d · 2026-02-24T20:32:02Z

pnpm-lock.yaml

Vulnerability Finding on line 0

More Details

Vulnerabilities [glob:11.0.1]

Name Severity Source Fixed version CVSS score CVSS exploitability score Has public exploit Has CISA KEV exploit

CVE-2025-64756 GHSA-5j98-mcp5-4vw2 11.1.0 7.5 1.6 true false

To ignore this finding as an exception, reply to this conversation with #wiz_ignore reason

If you'd like to ignore this finding in all future scans, add an exception in the .wiz file (learn more) or create an Ignore Rule (learn more).

To get more details on how to remediate this issue using AI, reply to this conversation with #wiz remediate

tjamescouch commented Jan 27, 2026

View reviewed changes

tjamescouch and others added 4 commits January 27, 2026 02:38

prettify

03eda55

Extract mock DataSource helpers to separate file

1d73f8d

Move mock QueryBuilder and DataSource infrastructure from test file to test/helpers/mock-datasource.ts to improve test file readability. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

tjamescouch commented Jan 27, 2026

View reviewed changes

tjamescouch commented Jan 28, 2026

View reviewed changes

packages/chrono-core/tsdown.config.ts Show resolved Hide resolved

add cleanup handling

47c6f0a

tjamescouch force-pushed the postgres-datastore branch from e56683f to b5d361f Compare January 28, 2026 01:54

tjamescouch force-pushed the postgres-datastore branch from b5d361f to c7b8ca4 Compare January 28, 2026 02:01

tjamescouch commented Jan 28, 2026

View reviewed changes

tjamescouch and others added 7 commits January 27, 2026 19:09

Remove testAccessor hack now that we have real Postgres tests

ac3ddfc

Query generation tests are no longer needed - actual cleanup behavior is already verified by the integration tests. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

Simplify claimedAt reset to use null instead of raw SQL

51931fb

Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

update package json

52d37d9

tjamescouch marked this pull request as ready for review January 28, 2026 12:17

tjamescouch requested review from darrenpicard25, maiahneo and neo-luca as code owners January 28, 2026 12:17

darrenpicard25 requested changes Jan 29, 2026

View reviewed changes

tjamescouch commented Jan 29, 2026

View reviewed changes

packages/chrono-postgres-datastore/src/chrono-task.entity.ts Outdated Show resolved Hide resolved

tjamescouch and others added 3 commits January 30, 2026 06:04

fix(chrono-postgres-datastore): remove lastExecutedAt from retry()

66cd962

lastExecutedAt should only be set when task execution finishes (complete/fail), not when scheduling a retry. This aligns with the mongo datastore behavior. Co-Authored-By: Claude (claude-4-opus) <noreply@anthropic.com>

declaration setting moved up

30d0a62

tjamescouch and others added 2 commits January 30, 2026 06:52

tjamescouch commented Jan 30, 2026

View reviewed changes

darrenpicard25 requested changes Feb 2, 2026

View reviewed changes

tjamescouch and others added 2 commits February 2, 2026 14:20

chore: update CI postgres image to 17 and bump alpha version

b9c11fd

Address PR review feedback to use a more current postgres version in CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

wiz-a38520980d bot reviewed Feb 24, 2026

View reviewed changes

		@@ -0,0 +1,550 @@
		import { PGlite } from '@electric-sql/pglite';

		@@ -13,6 +13,21 @@ jobs:
		runs-on: ubuntu-latest
		timeout-minutes: 10

	"@neofinancial/chrono": ">=0.5.0"
	"@neofinancial/chrono": "workspace:*"

Conversation

tjamescouch commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

wiz-a38520980d bot commented Jan 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Wiz Scan Summary

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!

Choose a reason for hiding this comment

Uh oh!

darrenpicard25 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!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

darrenpicard25 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wiz-a38520980d bot Feb 24, 2026

Choose a reason for hiding this comment

Vulnerabilities [minimatch:10.0.1]

Uh oh!

wiz-a38520980d bot Feb 24, 2026

Choose a reason for hiding this comment

Vulnerabilities [glob:11.0.1]

Uh oh!

Reviewers

Assignees

Labels

Projects

tjamescouch commented Jan 27, 2026 •

edited

Loading

wiz-a38520980d bot commented Jan 27, 2026 •

edited

Loading