From d052c7653eb9571806eee978ed20cb65b13aaadd Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Wed, 8 Apr 2026 11:23:33 -0400 Subject: [PATCH 1/2] CS-10684: Fix test run instances being overwritten across factory iterations The realm search index may not have indexed the TestRun from the previous iteration by the time the next one queries getNextSequenceNumber, causing it to return the same sequence number and overwrite the previous TestRun. Add a lastSequenceNumber floor that callers track across iterations, so Math.max(indexResult, lastSequenceNumber) + 1 always increments even when the search index is stale. Co-Authored-By: Claude Opus 4.6 (1M context) --- .../scripts/lib/factory-implement.ts | 9 +++ .../scripts/lib/factory-tool-builder.ts | 9 +++ .../scripts/lib/test-run-execution.ts | 9 ++- .../scripts/lib/test-run-types.ts | 7 +++ .../tests/factory-test-realm.test.ts | 58 +++++++++++++++++++ 5 files changed, 90 insertions(+), 2 deletions(-) diff --git a/packages/software-factory/scripts/lib/factory-implement.ts b/packages/software-factory/scripts/lib/factory-implement.ts index 39f43847508..dc43df8b5b3 100644 --- a/packages/software-factory/scripts/lib/factory-implement.ts +++ b/packages/software-factory/scripts/lib/factory-implement.ts @@ -463,6 +463,7 @@ function buildTestRunner( toolCallLog: ToolCallEntry[], runConfig: TestRunnerConfig, ): TestRunner { + let lastSequenceNumber = 0; return async (): Promise => { let wroteTestFiles = toolCallLog.some( (entry) => @@ -526,8 +527,16 @@ function buildTestRunner( realmServerUrl: runConfig.realmServerUrl, hostAppUrl: runConfig.hostAppUrl, forceNew: true, + lastSequenceNumber, }); + // Track the sequence number so the next iteration doesn't reuse it + // even if the realm search index hasn't caught up yet. + let seqMatch = handle.testRunId.match(/-(\d+)$/); + if (seqMatch) { + lastSequenceNumber = parseInt(seqMatch[1], 10); + } + let durationMs = Date.now() - start; console.error( `[factory-implement] Test run complete: status=${handle.status} (${durationMs}ms)`, diff --git a/packages/software-factory/scripts/lib/factory-tool-builder.ts b/packages/software-factory/scripts/lib/factory-tool-builder.ts index a990364100b..1b30369ca40 100644 --- a/packages/software-factory/scripts/lib/factory-tool-builder.ts +++ b/packages/software-factory/scripts/lib/factory-tool-builder.ts @@ -423,6 +423,7 @@ function buildCreateCatalogSpecTool(config: ToolBuilderConfig): FactoryTool { } function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { + let lastSequenceNumber = 0; return { name: 'run_tests', description: 'Execute QUnit card tests against the target realm', @@ -467,8 +468,16 @@ function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { projectCardUrl: args.projectCardUrl as string | undefined, realmServerUrl: config.realmServerUrl, forceNew: true, + lastSequenceNumber, }); + // Track the sequence number so subsequent calls don't reuse it + // even if the realm search index hasn't caught up yet. + let seqMatch = result.testRunId.match(/-(\d+)$/); + if (seqMatch) { + lastSequenceNumber = parseInt(seqMatch[1], 10); + } + return result; }, }; diff --git a/packages/software-factory/scripts/lib/test-run-execution.ts b/packages/software-factory/scripts/lib/test-run-execution.ts index b891c76427f..4d11326eae6 100644 --- a/packages/software-factory/scripts/lib/test-run-execution.ts +++ b/packages/software-factory/scripts/lib/test-run-execution.ts @@ -51,7 +51,10 @@ export async function resolveTestRun( }; } - let sequenceNumber = await getNextSequenceNumber(realmOptions); + let sequenceNumber = await getNextSequenceNumber( + realmOptions, + options.lastSequenceNumber, + ); let createResult = await createTestRun(options.slug, options.testNames, { ...realmOptions, @@ -133,6 +136,7 @@ async function findResumableTestRun( async function getNextSequenceNumber( options: TestRunRealmOptions, + minSequenceNumber = 0, ): Promise { let result = await searchRealm( options.testRealmUrl, @@ -151,7 +155,8 @@ async function getNextSequenceNumber( | { attributes?: { sequenceNumber?: number } } | undefined) : undefined; - return (latest?.attributes?.sequenceNumber ?? 0) + 1; + let fromIndex = latest?.attributes?.sequenceNumber ?? 0; + return Math.max(fromIndex, minSequenceNumber) + 1; } // --------------------------------------------------------------------------- diff --git a/packages/software-factory/scripts/lib/test-run-types.ts b/packages/software-factory/scripts/lib/test-run-types.ts index c8aef707a59..23bcf96d882 100644 --- a/packages/software-factory/scripts/lib/test-run-types.ts +++ b/packages/software-factory/scripts/lib/test-run-types.ts @@ -140,4 +140,11 @@ export interface ExecuteTestRunOptions { hostDistDir?: string; /** Log browser console output for debugging. */ debug?: boolean; + /** + * Floor for the next sequence number. When the realm search index is stale + * (hasn't indexed the most recent TestRun yet), getNextSequenceNumber may + * return a number that was already used. Passing the last-used sequence + * number here guarantees the new TestRun gets at least lastSequenceNumber + 1. + */ + lastSequenceNumber?: number; } diff --git a/packages/software-factory/tests/factory-test-realm.test.ts b/packages/software-factory/tests/factory-test-realm.test.ts index 9104140bb80..204e77713f1 100644 --- a/packages/software-factory/tests/factory-test-realm.test.ts +++ b/packages/software-factory/tests/factory-test-realm.test.ts @@ -681,6 +681,64 @@ module('factory-test-realm > resolveTestRun', function () { 'each iteration gets its own TestRun', ); }); + + test('lastSequenceNumber prevents reuse when realm index is stale', async function (assert) { + // Simulates the real-world bug: the realm search index hasn't indexed + // the TestRun created in the previous iteration, so the search returns + // stale data. Without lastSequenceNumber, getNextSequenceNumber would + // return 1 again and overwrite the first TestRun. + let handle1 = await resolveTestRun({ + ...testRealmOptions, + targetRealmUrl: 'https://realms.example.test/user/personal/', + slug: 'my-ticket', + testNames: ['test A'], + forceNew: true, + realmServerUrl: 'https://realms.example.test/', + hostAppUrl: 'https://realms.example.test/', + fetch: buildMockSearchFetch([]), + }); + + assert.strictEqual(handle1.testRunId, 'Test Runs/my-ticket-1'); + + // Second call — search index is STALE (still returns empty), but + // lastSequenceNumber=1 prevents reusing sequence 1. + let handle2 = await resolveTestRun({ + ...testRealmOptions, + targetRealmUrl: 'https://realms.example.test/user/personal/', + slug: 'my-ticket', + testNames: ['test A'], + forceNew: true, + lastSequenceNumber: 1, + realmServerUrl: 'https://realms.example.test/', + hostAppUrl: 'https://realms.example.test/', + fetch: buildMockSearchFetch([]), + }); + + assert.strictEqual( + handle2.testRunId, + 'Test Runs/my-ticket-2', + 'uses lastSequenceNumber as floor even when index returns nothing', + ); + + // Third call — index still stale, lastSequenceNumber=2 + let handle3 = await resolveTestRun({ + ...testRealmOptions, + targetRealmUrl: 'https://realms.example.test/user/personal/', + slug: 'my-ticket', + testNames: ['test A'], + forceNew: true, + lastSequenceNumber: 2, + realmServerUrl: 'https://realms.example.test/', + hostAppUrl: 'https://realms.example.test/', + fetch: buildMockSearchFetch([]), + }); + + assert.strictEqual( + handle3.testRunId, + 'Test Runs/my-ticket-3', + 'continues incrementing from lastSequenceNumber floor', + ); + }); }); // --------------------------------------------------------------------------- From 4acd83a1fe9ac32af868a29eafbccdc42ad170bb Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Wed, 8 Apr 2026 11:33:26 -0400 Subject: [PATCH 2/2] Address PR feedback: return sequenceNumber from TestRunHandle, track per-slug - Add sequenceNumber to TestRunHandle and return it from resolveTestRun and executeTestRunFromRealm, eliminating fragile regex parsing of testRunId - Track lastSequenceNumber per-slug in buildRunTestsTool using a Map, so different slugs get independent sequence floors Co-Authored-By: Claude Opus 4.6 (1M context) --- .../scripts/lib/factory-implement.ts | 5 ++--- .../scripts/lib/factory-tool-builder.ts | 16 ++++++++-------- .../scripts/lib/test-run-execution.ts | 7 ++++++- .../scripts/lib/test-run-types.ts | 2 ++ 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/software-factory/scripts/lib/factory-implement.ts b/packages/software-factory/scripts/lib/factory-implement.ts index dc43df8b5b3..e15f997fdaa 100644 --- a/packages/software-factory/scripts/lib/factory-implement.ts +++ b/packages/software-factory/scripts/lib/factory-implement.ts @@ -532,9 +532,8 @@ function buildTestRunner( // Track the sequence number so the next iteration doesn't reuse it // even if the realm search index hasn't caught up yet. - let seqMatch = handle.testRunId.match(/-(\d+)$/); - if (seqMatch) { - lastSequenceNumber = parseInt(seqMatch[1], 10); + if (handle.sequenceNumber != null) { + lastSequenceNumber = handle.sequenceNumber; } let durationMs = Date.now() - start; diff --git a/packages/software-factory/scripts/lib/factory-tool-builder.ts b/packages/software-factory/scripts/lib/factory-tool-builder.ts index 1b30369ca40..be8f792c523 100644 --- a/packages/software-factory/scripts/lib/factory-tool-builder.ts +++ b/packages/software-factory/scripts/lib/factory-tool-builder.ts @@ -423,7 +423,7 @@ function buildCreateCatalogSpecTool(config: ToolBuilderConfig): FactoryTool { } function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { - let lastSequenceNumber = 0; + let lastSequenceBySlug = new Map(); return { name: 'run_tests', description: 'Execute QUnit card tests against the target realm', @@ -450,6 +450,7 @@ function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { required: ['slug'], }, execute: async (args) => { + let slug = args.slug as string; let targetRealmUrl = config.targetRealmUrl; let authorization = resolveAuthForUrl(config, targetRealmUrl); let testResultsModuleUrl = @@ -460,7 +461,7 @@ function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { let result = await executeFn({ targetRealmUrl, testResultsModuleUrl, - slug: args.slug as string, + slug, hostAppUrl: config.hostAppUrl ?? config.realmServerUrl, testNames: (args.testNames as string[]) ?? [], authorization, @@ -468,14 +469,13 @@ function buildRunTestsTool(config: ToolBuilderConfig): FactoryTool { projectCardUrl: args.projectCardUrl as string | undefined, realmServerUrl: config.realmServerUrl, forceNew: true, - lastSequenceNumber, + lastSequenceNumber: lastSequenceBySlug.get(slug) ?? 0, }); - // Track the sequence number so subsequent calls don't reuse it - // even if the realm search index hasn't caught up yet. - let seqMatch = result.testRunId.match(/-(\d+)$/); - if (seqMatch) { - lastSequenceNumber = parseInt(seqMatch[1], 10); + // Track the sequence number per slug so subsequent calls don't + // reuse it even if the realm search index hasn't caught up yet. + if (result.sequenceNumber != null) { + lastSequenceBySlug.set(slug, result.sequenceNumber); } return result; diff --git a/packages/software-factory/scripts/lib/test-run-execution.ts b/packages/software-factory/scripts/lib/test-run-execution.ts index 4d11326eae6..ff7c356ed86 100644 --- a/packages/software-factory/scripts/lib/test-run-execution.ts +++ b/packages/software-factory/scripts/lib/test-run-execution.ts @@ -45,6 +45,7 @@ export async function resolveTestRun( if (resumeResult) { return { testRunId: resumeResult.testRunId, + sequenceNumber: resumeResult.sequenceNumber, status: 'running', resumed: true, pendingTests: resumeResult.pendingTests, @@ -66,6 +67,7 @@ export async function resolveTestRun( if (!createResult.created) { return { testRunId: createResult.testRunId, + sequenceNumber, status: 'error', errorMessage: `Failed to create TestRun: ${createResult.error}`, resumed: false, @@ -74,6 +76,7 @@ export async function resolveTestRun( return { testRunId: createResult.testRunId, + sequenceNumber, status: 'running', resumed: false, }; @@ -423,6 +426,7 @@ export async function executeTestRunFromRealm( return resolved; } let testRunId = resolved.testRunId; + let sequenceNumber = resolved.sequenceNumber; // Step 2: Serve a custom QUnit test page and navigate Playwright to it. let start = Date.now(); @@ -518,6 +522,7 @@ export async function executeTestRunFromRealm( return { testRunId, + sequenceNumber, status: attrs.status, ...(attrs.errorMessage ? { errorMessage: attrs.errorMessage } : {}), ...(completeResult.error ? { error: completeResult.error } : {}), @@ -544,7 +549,7 @@ export async function executeTestRunFromRealm( } catch { // Best-effort } - return { testRunId, status: 'error', errorMessage }; + return { testRunId, sequenceNumber, status: 'error', errorMessage }; } finally { if (browser) { await browser.close().catch(() => {}); diff --git a/packages/software-factory/scripts/lib/test-run-types.ts b/packages/software-factory/scripts/lib/test-run-types.ts index 23bcf96d882..d5ee6a12f2a 100644 --- a/packages/software-factory/scripts/lib/test-run-types.ts +++ b/packages/software-factory/scripts/lib/test-run-types.ts @@ -51,6 +51,8 @@ export interface TestRunHandle { testRunId: string; status: 'running' | 'passed' | 'failed' | 'error'; errorMessage?: string; + /** The sequence number assigned to this TestRun. */ + sequenceNumber?: number; } /**