From e24b75cf3b3faff54531ba32d467d075c3dc57ed Mon Sep 17 00:00:00 2001 From: Brad Groux <3053586+BradGroux@users.noreply.github.com> Date: Sun, 23 Aug 2026 10:51:29 -0500 Subject: [PATCH] fix: make workspace test gate deterministic (#1175) --- .github/workflows/ci.yml | 3 + AGENTS.md | 6 +- CONTRIBUTING.md | 27 ++-- README.md | 2 +- cli/package.json | 1 + mcp/package.json | 1 + package.json | 4 +- playwright.config.ts | 4 +- scripts/run-workspace-unit-tests.mjs | 121 ++++++++++++++++++ scripts/run-workspace-unit-tests.test.mjs | 70 ++++++++++ server/package.json | 2 +- .../__tests__/status-history-service.test.ts | 10 +- .../sqlite-workflow-run-execution.test.ts | 5 + .../__tests__/workflow-run-service.test.ts | 12 ++ server/src/__tests__/worktree-service.test.ts | 3 - server/src/services/status-history-service.ts | 15 ++- server/src/services/workflow-run-service.ts | 24 +++- web/package.json | 2 +- 18 files changed, 274 insertions(+), 38 deletions(-) create mode 100644 scripts/run-workspace-unit-tests.mjs create mode 100644 scripts/run-workspace-unit-tests.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4859d5c8..497bf37e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -371,6 +371,7 @@ jobs: pnpm --filter "$package_filter" exec vitest related \ --run \ + --maxWorkers=4 \ --passWithNoTests \ "${extra_args[@]}" \ "${related_files[@]}" @@ -476,6 +477,8 @@ jobs: echo "- Selection reason: $SELECTION_REASON" echo "- Workflow checkout SHA: \`$GITHUB_SHA\`" echo "- Current job status: \`$CURRENT_JOB_STATUS\`" + echo "- Unit-test workspaces: \`server, web, cli, mcp\`" + echo "- Workspace workers: \`4 maximum per Vitest project\`" } >> "$GITHUB_STEP_SUMMARY" - name: Record full-tier skip diff --git a/AGENTS.md b/AGENTS.md index be8d96d8..9aa2cd30 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -58,9 +58,9 @@ pnpm build pnpm dev # Tests -pnpm test # Vitest across server, web, mcp, cli -pnpm test:unit # Per-workspace tests sequentially -pnpm test:e2e # Playwright end-to-end +pnpm test # Canonical sequential workspace unit gate +pnpm test:unit # Shared build, then server, web, CLI, and MCP +pnpm test:e2e # Playwright end-to-end, zero retries # Type check (builds shared first) pnpm typecheck diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index f620bf5b..b618f774 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -305,12 +305,19 @@ Follow the existing conventions in `.eslintrc.*`, `.prettierrc`, and `tsconfig.j pnpm test ``` + This is the canonical unit gate. It builds the shared package, then runs the + server, web, CLI, and MCP suites sequentially with at most four Vitest workers + per project. The final line reports PASS, FAIL, or NOT RUN for every workspace. + - **End-to-end tests** use [Playwright](https://playwright.dev/): ```bash pnpm test:e2e ``` + Playwright does not retry failures. Screenshots and traces from the first + failure are retained in `test-results/` and uploaded by Scheduled QA. + - **Load smoke tests** use [k6](https://k6.io/): ```bash @@ -329,16 +336,16 @@ Follow the existing conventions in `.eslintrc.*`, `.prettierrc`, and `tsconfig.j ### CI tiers -| Trigger | Stable checks | Scope | -| ----------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | -| Documentation-only pull request or merge | Static gates; unit-test jobs record skip decisions | No workspace unit suite | -| Ordinary code pull request | `Lint & Type Check`, `Changed Tests`, `Build`, `Security Audit` | Vitest `related` coverage for affected server, web, CLI, or MCP workspaces | -| Ordinary code merge to `main` | Default static gates plus `Changed Tests` | Related coverage limited to affected workspaces | -| Pull request with `ci:full`, or a CI selector/workflow control change | Default checks plus `Workspace Unit Tests` | Complete workspace, desktop readiness regressions, and exact dual-storage parity | -| Merge whose reviewed head already passed `Workspace Unit Tests` | Static gates; both unit-test tiers record skip decisions | Reuses exact successful head evidence when that head is an ancestor of the merge commit | -| Nightly 08:00 UTC or manual `CI` dispatch with `test_scope=full` | Static gates, `Workspace Unit Tests`, `Build`, `Security Audit` | Complete authoritative workspace suite | -| Manual `CI` dispatch with `test_scope=focused` and optional `base_sha` | Static gates plus the selected unit-test tier | Classifies `base_sha...HEAD` (or `HEAD^...HEAD`) and stays focused unless CI controls changed | -| Desktop/package/release-workflow pull request, relevant `main` push, or manual `Desktop Artifacts` dispatch | Unsigned macOS, Linux, and Windows artifact jobs | Cross-platform packaging | +| Trigger | Stable checks | Scope | +| ----------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------- | --------------------------------------------------------------------------------------------- | +| Documentation-only pull request or merge | Static gates; unit-test jobs record skip decisions | No workspace unit suite | +| Ordinary code pull request | `Lint & Type Check`, `Changed Tests`, `Build`, `Security Audit` | Vitest `related` coverage for affected server, web, CLI, or MCP workspaces | +| Ordinary code merge to `main` | Default static gates plus `Changed Tests` | Related coverage limited to affected workspaces | +| Pull request with `ci:full`, or a CI selector/workflow control change | Default checks plus `Workspace Unit Tests` | Complete workspace, desktop readiness regressions, and exact dual-storage parity | +| Merge whose reviewed head already passed `Workspace Unit Tests` | Static gates; both unit-test tiers record skip decisions | Reuses exact successful head evidence when that head is an ancestor of the merge commit | +| Nightly 08:00 UTC or manual `CI` dispatch with `test_scope=full` | Static gates, `Workspace Unit Tests`, `Build`, `Security Audit` | Complete authoritative workspace suite | +| Manual `CI` dispatch with `test_scope=focused` and optional `base_sha` | Static gates plus the selected unit-test tier | Classifies `base_sha...HEAD` (or `HEAD^...HEAD`) and stays focused unless CI controls changed | +| Desktop/package/release-workflow pull request, relevant `main` push, or manual `Desktop Artifacts` dispatch | Unsigned macOS, Linux, and Windows artifact jobs | Cross-platform packaging | `Select Test Scope` is the decision record for each run. Its summary names the event, exact base/head range, changed-path count, selected tier, affected diff --git a/README.md b/README.md index 06b5e473..661fea43 100644 --- a/README.md +++ b/README.md @@ -837,7 +837,7 @@ pnpm build # Production build pnpm typecheck # TypeScript strict check pnpm lint # ESLint pnpm lint:budget # ESLint with current warning budget -pnpm test # Unit tests (Vitest) +pnpm test # Canonical unit gate (server, web, CLI, MCP) pnpm test:e2e # E2E tests (Playwright) pnpm test:load:smoke # k6 API smoke test pnpm validate:release # Release readiness checks diff --git a/cli/package.json b/cli/package.json index b6bd345a..5ac7a64e 100644 --- a/cli/package.json +++ b/cli/package.json @@ -9,6 +9,7 @@ "scripts": { "build": "tsc", "dev": "tsx src/index.ts", + "test": "vitest run --maxWorkers=4", "typecheck": "tsc --noEmit" }, "dependencies": { diff --git a/mcp/package.json b/mcp/package.json index 5a9cc706..536abbd3 100644 --- a/mcp/package.json +++ b/mcp/package.json @@ -10,6 +10,7 @@ "scripts": { "build": "tsc", "dev": "tsx src/index.ts", + "test": "vitest run --maxWorkers=4", "start": "node dist/index.js" }, "dependencies": { diff --git a/package.json b/package.json index 25e85e6c..78ef545b 100644 --- a/package.json +++ b/package.json @@ -38,8 +38,8 @@ "check:pnpm-settings": "node scripts/check-pnpm-settings.mjs", "check:security-artifacts": "node scripts/check-security-artifacts.mjs", "typecheck": "pnpm --filter @veritas-kanban/shared build && pnpm -r typecheck", - "test": "vitest run", - "test:unit": "pnpm -r --workspace-concurrency=1 test", + "test": "pnpm test:unit", + "test:unit": "node --test scripts/run-workspace-unit-tests.test.mjs && node scripts/run-workspace-unit-tests.mjs", "test:ci-scope": "node --test scripts/select-ci-test-scope.test.mjs", "test:e2e": "playwright test", "test:e2e:headed": "playwright test --headed", diff --git a/playwright.config.ts b/playwright.config.ts index 313e09bb..63786950 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -33,7 +33,7 @@ export default defineConfig({ testDir: './e2e', fullyParallel: false, // Run sequentially — tests may share board state forbidOnly: !!process.env.CI, - retries: process.env.CI ? 2 : 0, + retries: 0, workers: 1, reporter: process.env.CI && process.env.PLAYWRIGHT_HTML_REPORT === '1' @@ -45,7 +45,7 @@ export default defineConfig({ use: { baseURL: 'http://127.0.0.1:3000', - trace: 'on-first-retry', + trace: 'retain-on-failure', screenshot: 'only-on-failure', // Auth — read admin key from server/.env (loaded via dotenv above) extraHTTPHeaders: { diff --git a/scripts/run-workspace-unit-tests.mjs b/scripts/run-workspace-unit-tests.mjs new file mode 100644 index 00000000..c569c5b9 --- /dev/null +++ b/scripts/run-workspace-unit-tests.mjs @@ -0,0 +1,121 @@ +#!/usr/bin/env node + +import { spawnSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { fileURLToPath } from 'node:url'; +import path from 'node:path'; + +const repositoryRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..'); +const pnpmExecutable = process.platform === 'win32' ? 'pnpm.cmd' : 'pnpm'; + +export const UNIT_TEST_STAGES = Object.freeze([ + { + id: 'shared-build', + label: 'shared prerequisite', + directory: 'shared', + packageName: '@veritas-kanban/shared', + requiredScript: 'build', + }, + { + id: 'server', + label: 'server unit tests', + directory: 'server', + packageName: '@veritas-kanban/server', + requiredScript: 'test', + }, + { + id: 'web', + label: 'web unit tests', + directory: 'web', + packageName: '@veritas-kanban/web', + requiredScript: 'test', + }, + { + id: 'cli', + label: 'CLI unit tests', + directory: 'cli', + packageName: '@veritas-kanban/cli', + requiredScript: 'test', + }, + { + id: 'mcp', + label: 'MCP unit tests', + directory: 'mcp', + packageName: '@veritas-kanban/mcp', + requiredScript: 'test', + }, +]); + +function readWorkspacePackage(stage) { + const packagePath = path.join(repositoryRoot, stage.directory, 'package.json'); + return JSON.parse(readFileSync(packagePath, 'utf8')); +} + +export function validateStage(stage, { readPackageJson = readWorkspacePackage } = {}) { + let packageJson; + try { + packageJson = readPackageJson(stage); + } catch (error) { + return `Cannot read ${stage.directory}/package.json: ${error.message}`; + } + if (packageJson.name !== stage.packageName) { + return `${stage.directory}/package.json must declare name ${stage.packageName}`; + } + if ( + typeof packageJson.scripts?.[stage.requiredScript] !== 'string' || + packageJson.scripts[stage.requiredScript].trim() === '' + ) { + return `${stage.packageName} must define a non-empty ${stage.requiredScript} script`; + } + return undefined; +} + +function executeStage(stage) { + const result = spawnSync(pnpmExecutable, ['--filter', stage.packageName, stage.requiredScript], { + cwd: repositoryRoot, + env: process.env, + stdio: 'inherit', + }); + + if (result.error) { + console.error(`Could not start ${stage.label}: ${result.error.message}`); + return 1; + } + return result.status ?? 1; +} + +export function runWorkspaceUnitTests({ + run = executeStage, + validate = validateStage, + log = console.log, +} = {}) { + const results = new Map(UNIT_TEST_STAGES.map((stage) => [stage.id, 'NOT RUN'])); + let exitCode = 0; + + for (const stage of UNIT_TEST_STAGES) { + log(`\n==> ${stage.label}`); + const validationError = validate(stage); + if (validationError) { + log(`Configuration error: ${validationError}`); + results.set(stage.id, 'FAIL'); + exitCode = 1; + break; + } + const status = run(stage); + results.set(stage.id, status === 0 ? 'PASS' : 'FAIL'); + if (status !== 0) { + exitCode = status > 0 ? status : 1; + break; + } + } + + const workspaceSummary = UNIT_TEST_STAGES.filter((stage) => stage.id !== 'shared-build') + .map((stage) => `${stage.id}: ${results.get(stage.id)}`) + .join(' | '); + log(`\nWorkspace unit-test summary: ${workspaceSummary}`); + return exitCode; +} + +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + process.exitCode = runWorkspaceUnitTests(); +} diff --git a/scripts/run-workspace-unit-tests.test.mjs b/scripts/run-workspace-unit-tests.test.mjs new file mode 100644 index 00000000..02c36fb1 --- /dev/null +++ b/scripts/run-workspace-unit-tests.test.mjs @@ -0,0 +1,70 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; + +import { + UNIT_TEST_STAGES, + runWorkspaceUnitTests, + validateStage, +} from './run-workspace-unit-tests.mjs'; + +test('runs the shared prerequisite and every unit-test workspace in order', () => { + const calls = []; + const lines = []; + + const status = runWorkspaceUnitTests({ + run: (stage) => { + calls.push(stage.id); + return 0; + }, + log: (line) => lines.push(line), + }); + + assert.equal(status, 0); + assert.deepEqual( + calls, + UNIT_TEST_STAGES.map((stage) => stage.id) + ); + assert.match(lines.at(-1), /server: PASS.*web: PASS.*cli: PASS.*mcp: PASS/); +}); + +test('stops after the first failure and reports unexecuted workspaces', () => { + const calls = []; + const lines = []; + + const status = runWorkspaceUnitTests({ + run: (stage) => { + calls.push(stage.id); + return stage.id === 'web' ? 7 : 0; + }, + log: (line) => lines.push(line), + }); + + assert.equal(status, 7); + assert.deepEqual(calls, ['shared-build', 'server', 'web']); + assert.match(lines.at(-1), /server: PASS.*web: FAIL.*cli: NOT RUN.*mcp: NOT RUN/); +}); + +test('fails before execution when a workspace test script is missing', () => { + const calls = []; + const lines = []; + + const status = runWorkspaceUnitTests({ + validate: (stage) => + validateStage(stage, { + readPackageJson: () => ({ + name: stage.packageName, + scripts: { [stage.requiredScript]: stage.id === 'cli' ? '' : 'test-command' }, + }), + }), + run: (stage) => { + calls.push(stage.id); + return 0; + }, + log: (line) => lines.push(line), + }); + + assert.equal(status, 1); + assert.deepEqual(calls, ['shared-build', 'server', 'web']); + assert.match(lines.at(-2), /must define a non-empty test script/); + assert.match(lines.at(-1), /server: PASS.*web: PASS.*cli: FAIL.*mcp: NOT RUN/); +}); diff --git a/server/package.json b/server/package.json index a4f1b797..9d6f10c2 100644 --- a/server/package.json +++ b/server/package.json @@ -10,7 +10,7 @@ "start": "node dist/index.js", "typecheck": "tsc --noEmit", "lint": "eslint src --ext .ts", - "test": "VERITAS_DISABLE_WATCHERS=1 vitest run", + "test": "VERITAS_DISABLE_WATCHERS=1 vitest run --maxWorkers=4", "test:watch": "vitest", "clean": "rm -rf dist", "reset-password": "tsx src/scripts/reset-password.ts" diff --git a/server/src/__tests__/status-history-service.test.ts b/server/src/__tests__/status-history-service.test.ts index 50f23a36..de6e2c88 100644 --- a/server/src/__tests__/status-history-service.test.ts +++ b/server/src/__tests__/status-history-service.test.ts @@ -95,14 +95,12 @@ describe('StatusHistoryService', () => { }); it('should calculate duration from previous entry', async () => { + let nowMs = Date.parse('2026-08-23T15:00:00.000Z'); + service = new StatusHistoryService({ historyFile, now: () => new Date(nowMs) }); await service.logStatusChange('idle', 'working'); - - // Wait a small amount for timestamp difference - await new Promise((r) => setTimeout(r, 50)); - + nowMs += 50; const second = await service.logStatusChange('working', 'idle'); - expect(second.durationMs).toBeDefined(); - expect(second.durationMs!).toBeGreaterThan(0); + expect(second.durationMs).toBe(50); }); it('should include subAgentCount when provided', async () => { diff --git a/server/src/__tests__/storage/sqlite-workflow-run-execution.test.ts b/server/src/__tests__/storage/sqlite-workflow-run-execution.test.ts index 5af16da8..6a02eb3c 100644 --- a/server/src/__tests__/storage/sqlite-workflow-run-execution.test.ts +++ b/server/src/__tests__/storage/sqlite-workflow-run-execution.test.ts @@ -80,10 +80,12 @@ describe('SQLite workflow run execution', () => { let fixture: TestSqliteDatabase; let testRoot: string; let workflowService: WorkflowService; + let runServices: Array<{ dispose(): void }>; beforeEach(async () => { fixture = createTestSqliteDatabase(); fixture.database.open(); + runServices = []; testRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'veritas-sqlite-workflow-execution-')); workflowService = new WorkflowService({ workflowsDir: path.join(testRoot, 'storage', 'workflows'), @@ -95,6 +97,7 @@ describe('SQLite workflow run execution', () => { afterEach(async () => { vi.clearAllMocks(); + for (const runService of runServices) runService.dispose(); fixture.cleanup(); await fs.rm(testRoot, { recursive: true, force: true }); }); @@ -109,6 +112,7 @@ describe('SQLite workflow run execution', () => { sqliteDatabase: fixture.database, workflowService, }); + runServices.push(runService); const counts: Record = {}; await workflowService.saveWorkflow(definition); @@ -183,6 +187,7 @@ describe('SQLite workflow run execution', () => { workflowService, admission, }); + runServices.push(first, second); const recovery: RunRecoveryRecord = { schemaVersion: 'run-recovery/v1', rootRunId: 'run_root', diff --git a/server/src/__tests__/workflow-run-service.test.ts b/server/src/__tests__/workflow-run-service.test.ts index 3b857ff4..f0d065d6 100644 --- a/server/src/__tests__/workflow-run-service.test.ts +++ b/server/src/__tests__/workflow-run-service.test.ts @@ -96,6 +96,7 @@ describe('WorkflowRunService', () => { }); afterEach(async () => { + service.dispose(); vi.clearAllMocks(); await fs.rm(tmpDir, { recursive: true, force: true }); }); @@ -425,6 +426,17 @@ describe('WorkflowRunService', () => { expect(mockExecuteStep).toHaveBeenCalledTimes(1); }); + it('clears owned recovery timers before test storage is removed', () => { + service.scheduleWorkflowRecovery('run_timer_cleanup', 'step-1', { + state: 'scheduled', + notBefore: '2999-01-01T00:00:00.000Z', + }); + + expect(service.scheduledWorkflowRecoveryTimers.size).toBe(1); + service.dispose(); + expect(service.scheduledWorkflowRecoveryTimers.size).toBe(0); + }); + it('blocks a restarted workflow when a launched recovery cannot be proven terminal', async () => { mockLoadWorkflow.mockResolvedValue( makeWorkflow({ diff --git a/server/src/__tests__/worktree-service.test.ts b/server/src/__tests__/worktree-service.test.ts index 1a401c0e..04c11320 100644 --- a/server/src/__tests__/worktree-service.test.ts +++ b/server/src/__tests__/worktree-service.test.ts @@ -151,13 +151,10 @@ describe('WorktreeService transactional lifecycle', () => { it('waits for a timed-out Git child to close before rejecting the operation', async () => { const runner = new DefaultWorktreeGitRunner(100, process.execPath, 50); - const startedAt = Date.now(); await expect( runner.run(root, ['-e', "process.on('SIGTERM', () => {}); setInterval(() => {}, 1000);"]) ).rejects.toThrow(/timed out/i); - - expect(Date.now() - startedAt).toBeGreaterThanOrEqual(140); }); it('resolves and persists the exact remote base commit before creating a unique worktree', async () => { diff --git a/server/src/services/status-history-service.ts b/server/src/services/status-history-service.ts index d4ca318e..e4df032b 100644 --- a/server/src/services/status-history-service.ts +++ b/server/src/services/status-history-service.ts @@ -44,6 +44,7 @@ export interface StatusHistoryServiceOptions { storageType?: 'file' | 'sqlite'; sqliteDatabase?: SqliteDatabase; sqliteConnectionOptions?: SqliteConnectionOptions; + now?: () => Date; } export class StatusHistoryService { @@ -54,8 +55,10 @@ export class StatusHistoryService { private repository: StatusHistoryRepository | null = null; private sqliteDatabase: SqliteDatabase | null = null; private ownsSqliteDatabase = false; + private readonly now: () => Date; constructor(options: StatusHistoryServiceOptions = {}) { + this.now = options.now ?? (() => new Date()); this.historyFile = options.historyFile || join(process.cwd(), '.veritas-kanban', 'status-history.json'); const storageType = @@ -139,7 +142,7 @@ export class StatusHistoryService { await this.initPromise; - const now = new Date(); + const now = this.now(); const timestamp = now.toISOString(); // Calculate duration of previous status @@ -151,7 +154,7 @@ export class StatusHistoryService { } const entry: StatusHistoryEntry = { - id: `status_${Date.now()}_${Math.random().toString(36).substr(2, 9)}`, + id: `status_${now.getTime()}_${Math.random().toString(36).substr(2, 9)}`, timestamp, previousStatus, newStatus, @@ -209,7 +212,7 @@ export class StatusHistoryService { return this.repository.getDailySummary(date); } - const targetDate = date || new Date().toISOString().split('T')[0]; + const targetDate = date || this.now().toISOString().split('T')[0]; const startOfDay = new Date(`${targetDate}T00:00:00.000Z`); const endOfDay = new Date(`${targetDate}T23:59:59.999Z`); @@ -237,7 +240,7 @@ export class StatusHistoryService { endTime = new Date(nextEntry.timestamp); } else { // Last entry - use current time or end of day if analyzing past dates - const now = new Date(); + const now = this.now(); endTime = now < endOfDay ? now : endOfDay; } @@ -277,7 +280,7 @@ export class StatusHistoryService { if (beforeDay.length > 0) { // The most recent entry before this day determines the starting status const lastBeforeDay = beforeDay[0]; - const now = new Date(); + const now = this.now(); const effectiveEnd = now < endOfDay ? now : endOfDay; const durationMs = effectiveEnd.getTime() - startOfDay.getTime(); @@ -318,7 +321,7 @@ export class StatusHistoryService { } const summaries: DailySummary[] = []; - const today = new Date(); + const today = this.now(); for (let i = 0; i < 7; i++) { const date = new Date(today); diff --git a/server/src/services/workflow-run-service.ts b/server/src/services/workflow-run-service.ts index 5cfb84f6..5bca0277 100644 --- a/server/src/services/workflow-run-service.ts +++ b/server/src/services/workflow-run-service.ts @@ -64,9 +64,10 @@ const log = createLogger('workflow-run'); /** Default maximum cross-step reroutes per run before exhaustion policy fires (#780) */ const MAX_REROUTES_DEFAULT = 10; +type WorkflowRecoveryTimer = ReturnType; const scheduledWorkflowRecoveries = new Map< string, - { stepId: string; timer: ReturnType } + { stepId: string; timer: WorkflowRecoveryTimer; ownerTimers: Set } >(); const RUN_ID_PATTERN = /^run_\d{10,}_[a-zA-Z0-9_-]{6,}$/; const WORKFLOW_ADMISSION_ID_PREFIX = 'workflow'; @@ -113,6 +114,7 @@ class WorkflowRunChangedError extends ConflictError { } export class WorkflowRunService { + private readonly scheduledWorkflowRecoveryTimers = new Set(); private runsDir: string; private workflowService: ReturnType; private stepExecutor: WorkflowStepExecutor; @@ -2220,20 +2222,27 @@ export class WorkflowRunService { const delay = Math.max(0, Math.min(2_147_483_647, notBefore - Date.now())); const timer = setTimeout(() => { const scheduled = scheduledWorkflowRecoveries.get(runId); - if (!scheduled || scheduled.stepId !== stepId) return; + if (!scheduled || scheduled.stepId !== stepId || scheduled.timer !== timer) return; scheduledWorkflowRecoveries.delete(runId); + scheduled.ownerTimers.delete(scheduled.timer); void this.resumeScheduledWorkflowRecovery(runId, stepId).catch((error) => { log.error({ err: error, runId, stepId }, 'Scheduled workflow recovery failed'); }); }, delay); timer.unref?.(); - scheduledWorkflowRecoveries.set(runId, { stepId, timer }); + this.scheduledWorkflowRecoveryTimers.add(timer); + scheduledWorkflowRecoveries.set(runId, { + stepId, + timer, + ownerTimers: this.scheduledWorkflowRecoveryTimers, + }); } private clearScheduledWorkflowRecovery(runId: string, expectedStepId?: string): void { const scheduled = scheduledWorkflowRecoveries.get(runId); if (!scheduled || (expectedStepId && scheduled.stepId !== expectedStepId)) return; clearTimeout(scheduled.timer); + scheduled.ownerTimers.delete(scheduled.timer); scheduledWorkflowRecoveries.delete(runId); } @@ -2845,6 +2854,15 @@ export class WorkflowRunService { } dispose(): void { + for (const timer of this.scheduledWorkflowRecoveryTimers) { + clearTimeout(timer); + } + for (const [runId, scheduled] of scheduledWorkflowRecoveries) { + if (scheduled.ownerTimers === this.scheduledWorkflowRecoveryTimers) { + scheduledWorkflowRecoveries.delete(runId); + } + } + this.scheduledWorkflowRecoveryTimers.clear(); if (this.ownsSqliteDatabase) { this.sqliteDatabase?.close(); } diff --git a/web/package.json b/web/package.json index d5e50d47..ec6b991b 100644 --- a/web/package.json +++ b/web/package.json @@ -9,7 +9,7 @@ "preview": "vite preview", "typecheck": "tsc --noEmit", "lint": "eslint src --ext .ts,.tsx", - "test": "vitest run --testTimeout 15000", + "test": "vitest run --testTimeout 15000 --maxWorkers=4", "test:watch": "vitest --testTimeout 15000", "clean": "rm -rf dist" },