Skip to content

feat(workflow-executor): add RunStore implementations (InMemoryStore + DatabaseStore)#1506

Merged
matthv merged 11 commits intofeat/prd-214-setup-workflow-executor-packagefrom
feature/prd-221-implementer-le-runstore-avec-sequelize
Mar 25, 2026
Merged

feat(workflow-executor): add RunStore implementations (InMemoryStore + DatabaseStore)#1506
matthv merged 11 commits intofeat/prd-214-setup-workflow-executor-packagefrom
feature/prd-221-implementer-le-runstore-avec-sequelize

Conversation

@matthv
Copy link
Copy Markdown
Member

@matthv matthv commented Mar 24, 2026

Summary

  • InMemoryStore — Simple Map-based RunStore implementation for unit tests
  • DatabaseStore — Sequelize-based RunStore with auto-migration via umzug
    • Single table workflow_step_executions with JSON data column for flexible schema
    • Indexed by run_id for fast lookups, unique constraint on (run_id, step_index)
    • Dialect-agnostic upsert (delete + insert) — works across SQLite, Postgres, MySQL, etc.
    • Migration applied automatically at startup via store.migrate() (idempotent)
    • No manual migrations needed at the client — DB adapts on its own

Test plan

  • InMemoryStore: empty get, save/retrieve, multi-step, upsert, run isolation (6 tests)
  • DatabaseStore (SQLite): empty get, save/retrieve, multi-step, ordering, upsert, run isolation, nested JSON, idempotent migration (8 tests)
  • Lint clean
  • 13 tests pass

fixes PRD-221

🤖 Generated with Claude Code

Note

Add InMemoryStore and DatabaseStore implementations of RunStore

  • Adds InMemoryStore (in-memory-store.ts) using nested Maps with upsert-by-stepIndex and sorted retrieval, and DatabaseStore (database-store.ts) backed by Sequelize with Umzug-managed migrations and a transactional delete-then-insert upsert.
  • Adds buildInMemoryRunStore and buildDatabaseRunStore factory functions that initialize the store before returning it; buildDatabaseRunStore closes the Sequelize connection if initialization fails.
  • Extends the RunStore interface with init(): Promise<void> and close(): Promise<void> lifecycle methods; Runner.start() now calls runStore.init() and Runner.stop() calls runStore.close() via Promise.allSettled.
  • Risk: Runner.stop() now uses Promise.allSettled for shutdown, so a failure in aiClient.closeConnections() no longer prevents runStore.close() from being called (and vice versa) — previously the first rejection would short-circuit the other.

Changes since #1506 opened

  • Replaced custom DatabaseConfig interface with native sequelize Options interface for buildDatabaseRunStore function [cab9e38]
  • Modified RunStore interface to accept optional Logger parameter in init and close methods, updated Runner to pass its logger instance to runStore.init and runStore.close, and enhanced DatabaseStore implementation to log migration failures during init and database connection closure failures during close using the provided logger [75510d5]
  • Removed duplicate init and close mock definitions from mock RunStore implementations in test files [75510d5]
  • Removed userConfirmed property from makeContext helper function calls across all step executor test files [153b647]
  • Added userConfirmed boolean field to pendingData objects in test setups and assertions [bb168ff]
  • Changed no-pending-data test cases to expect 'awaiting-input' status instead of error outcomes [bb168ff]

Macroscope summarized a36fce9.

@linear
Copy link
Copy Markdown

linear bot commented Mar 24, 2026

@qltysh
Copy link
Copy Markdown

qltysh bot commented Mar 24, 2026

Qlty

Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (5)

RatingFile% DiffUncovered Line #s
New file Coverage rating: A
packages/workflow-executor/src/runner.ts100.0%
New file Coverage rating: A
packages/workflow-executor/src/stores/in-memory-store.ts100.0%
New file Coverage rating: F
packages/workflow-executor/src/stores/build-run-store.ts33.3%10-21, 25-28
New file Coverage rating: A
packages/workflow-executor/src/index.ts100.0%
New file Coverage rating: B
packages/workflow-executor/src/stores/database-store.ts84.6%69, 81-84, 121
Total75.0%
🤖 Increase coverage with AI coding...

In the `feature/prd-221-implementer-le-runstore-avec-sequelize` branch, add test coverage for this new code:

- `packages/workflow-executor/src/stores/build-run-store.ts` -- Lines 10-21 and 25-28
- `packages/workflow-executor/src/stores/database-store.ts` -- Lines 69, 81-84, and 121

🚦 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
Copy link
Copy Markdown
Member Author

matthv commented Mar 24, 2026

Both Macroscope comments addressed in 29f7cba:

  1. Transaction on upsertsaveStepExecution now wraps DELETE + INSERT in this.sequelize.transaction() to make the operation atomic.
  2. Sort order in InMemoryStoregetStepExecutions now sorts by stepIndex to match the DatabaseStore contract (ORDER BY step_index ASC). Added a dedicated ordering test.

14 tests pass, lint clean.

@matthv matthv force-pushed the feature/prd-221-implementer-le-runstore-avec-sequelize branch from c788c67 to e772931 Compare March 25, 2026 09:32
matthv and others added 7 commits March 25, 2026 11:19
…+ DatabaseStore)

- InMemoryStore: simple Map-based store for unit tests
- DatabaseStore: Sequelize-based store with auto-migration via umzug
  - Single table `workflow_step_executions` with JSON `data` column
  - Indexed by `run_id` for fast lookups, unique on `(run_id, step_index)`
  - Dialect-agnostic upsert (delete + insert)
  - Migration applied automatically at startup via `store.migrate()`
- 13 tests (SQLite for DatabaseStore, pure unit for InMemoryStore)

fixes PRD-221

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Wrap DELETE + INSERT in a transaction to make upsert atomic (Macroscope)
- Sort InMemoryStore results by stepIndex to match DatabaseStore contract (Macroscope)
- Add ordering test for InMemoryStore

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…config

Runner now accepts either:
- runStore: RunStore (injected, as before)
- database: { dialect, uri } (Runner creates DatabaseStore + auto-migrates on start)

DatabaseStore is closed on stop(). Both options are mutually exclusive
via a discriminated union on RunnerConfig.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Add init() and close() to RunStore interface
- InMemoryStore: no-op init/close
- DatabaseStore: rename migrate() → init()
- Remove Sequelize/DatabaseStore knowledge from Runner (no more union type)
- Runner calls runStore.init() on start and runStore.close() on stop
- Add factory functions buildDatabaseRunStore / buildInMemoryRunStore
- RunnerConfig back to simple { runStore: RunStore }

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
If store.init() throws, close the Sequelize connection before
re-throwing to avoid leaking a database connection. (Macroscope)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add tests verifying that Runner.start() calls runStore.init()
and Runner.stop() calls runStore.close().

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…files

RunStore interface now requires init() and close(). Update all
makeMockRunStore/createMockRunStore helpers across test files to
include these methods as jest.fn() no-ops.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matthv matthv force-pushed the feature/prd-221-implementer-le-runstore-avec-sequelize branch from 2f5b4e1 to a36fce9 Compare March 25, 2026 10:20
…atabaseRunStore

Replace custom DatabaseConfig with Sequelize's own Options type.
Callers now pass standard Sequelize options (dialect, storage, host, etc.)
directly. logging: false is applied as default.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment on lines +70 to +73
init: jest.fn().mockResolvedValue(undefined),
close: jest.fn().mockResolvedValue(undefined),
init: jest.fn().mockResolvedValue(undefined),
close: jest.fn().mockResolvedValue(undefined),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👀

- RunStore interface: init() and close() now accept optional Logger
- DatabaseStore: logs migration failures in init(), catches and logs
  close() errors instead of throwing
- Runner passes its logger to runStore.init() and runStore.close()
- Fix duplicate init/close mocks from rebase conflict

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
After PR #1503 moved userConfirmed from ExecutionContext to pendingData,
these tests still passed it in makeContext(). Remove all occurrences.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…tests

After PR #1503 moved userConfirmed from ExecutionContext to pendingData,
the Branch A confirmation tests were missing userConfirmed in their mock
execution pendingData — causing all confirmation flows to return
awaiting-input instead of proceeding.

- Add userConfirmed: true to accepted/error-path tests
- Add userConfirmed: false to rejected tests
- Update "no execution found" tests to expect awaiting-input

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@matthv matthv merged commit 50583e8 into feat/prd-214-setup-workflow-executor-package Mar 25, 2026
29 of 30 checks passed
@matthv matthv deleted the feature/prd-221-implementer-le-runstore-avec-sequelize branch March 25, 2026 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants