From f087e17fa273054673a9909a9b9bc6aabd633273 Mon Sep 17 00:00:00 2001 From: Khushboo Kumari Date: Tue, 23 Jun 2026 20:55:15 +0530 Subject: [PATCH 1/5] refactor(tokowaka-client): remove sync covered-marking from deployToEdge Complete the async covered-marking migration by removing in-request covered suggestion updates from deployToEdge. Keep pattern and per-URL deployment behavior intact while updating tests to assert covered marking is handled by the worker path. Co-authored-by: Cursor --- .../src/index.js | 117 +-------------- .../test/index.test.js | 136 ++++++++---------- 2 files changed, 63 insertions(+), 190 deletions(-) diff --git a/packages/spacecat-shared-tokowaka-client/src/index.js b/packages/spacecat-shared-tokowaka-client/src/index.js index 15d0a76f8..c36807a70 100644 --- a/packages/spacecat-shared-tokowaka-client/src/index.js +++ b/packages/spacecat-shared-tokowaka-client/src/index.js @@ -27,7 +27,6 @@ import { } from './utils/s3-utils.js'; import { omitKeys, - isEdgeDeployableSuggestionStatus, isPatternSuggestion, groupSuggestionsByUrlPath, filterEligibleSuggestions, @@ -35,9 +34,7 @@ import { stripSuggestion, cleanupCoveredSuggestions, classifySuggestions, - filterBatchCoveredSuggestions, } from './utils/suggestion-utils.js'; -import { buildUrlMatcher } from './utils/pattern-utils.js'; import { getEffectiveBaseURL } from './utils/site-utils.js'; import { removePatternFromMetaconfig, addPatternsToMetaconfig } from './utils/metaconfig-utils.js'; import { fetchHtmlWithWarmup, calculateForwardedHost } from './utils/custom-html-utils.js'; @@ -1475,16 +1472,13 @@ class TokowakaClient { /** * Deploys a single pattern suggestion by appending its patterns to the metaconfig allowList - * (append-and-deduplicate; CDN write is skipped when nothing changes), stamps the suggestion - * with edgeDeployed, and marks qualifying per-URL suggestions as covered. + * (append-and-deduplicate; CDN write is skipped when nothing changes), and stamps + * the suggestion with edgeDeployed. * @param {Object} suggestion - The pattern suggestion entity * @param {Array} allowedRegexPatterns * @param {Object} metaconfig - Metaconfig object (mutated in place) * @param {string} baseURL - * @param {Set} skippedInBatchIds - IDs already handled as same-batch skips - * @param {Array} allSuggestions - Full opportunity suggestion list * @param {string} updatedBy - * @param {Array} coveredSuggestions - Accumulator (mutated) * @returns {Promise} Throws on metaconfig upload failure * @private */ @@ -1493,14 +1487,8 @@ class TokowakaClient { allowedRegexPatterns, metaconfig, baseURL, - skippedInBatchIds, - allSuggestions, updatedBy, - coveredSuggestions, ) { - const data = suggestion.getData(); - const coverageField = data?.isDomainWide ? 'coveredByDomainWide' : 'coveredByPattern'; - const beforeAllowList = metaconfig.prerender?.allowList ?? []; const changed = addPatternsToMetaconfig(metaconfig, allowedRegexPatterns); const afterAllowList = changed ? metaconfig.prerender.allowList : beforeAllowList; @@ -1521,77 +1509,18 @@ class TokowakaClient { suggestion.setData({ ...suggestion.getData(), edgeDeployed: Date.now() }); suggestion.setUpdatedBy(updatedBy); // suggestion.save() is deferred — caller batches saves via saveSuggestions. - - // Mark per-URL suggestions covered by this pattern. - const matchers = allowedRegexPatterns.flatMap((p) => { - const m = buildUrlMatcher(p); - if (!m) { - // eslint-disable-next-line max-len - this.log.warn(`[edge-deploy] Pattern '${p}' for suggestion ${suggestion.getId()} is invalid, skipping`); - } - return m ? [m] : []; - }); - - if (matchers.length === 0) { - return; - } - - const covered = allSuggestions.filter((s) => { - if (s.getId() === suggestion.getId()) { - return false; - } - if (skippedInBatchIds.has(s.getId())) { - return false; - } - if (!isEdgeDeployableSuggestionStatus(s.getStatus())) { - return false; - } - if (s.getData()?.edgeDeployed) { - return false; - } - // Path-level pattern suggestions (not domain-wide) are fully covered by a - // domain-wide deployment. Other pattern suggestions (including other DW ones) are not. - if (isPatternSuggestion(s)) { - return coverageField === 'coveredByDomainWide' && !s.getData()?.isDomainWide; - } - const url = s.getData()?.url; - return url && matchers.some((match) => match(url)); - }); - - // eslint-disable-next-line max-len - this.log.info(`[edge-deploy] Pattern ${suggestion.getId()}: found ${covered.length} coverable per-URL suggestions (field=${coverageField})`); - - if (covered.length > 0) { - covered.forEach((cs) => { - cs.setData({ ...cs.getData(), [coverageField]: suggestion.getId() }); - cs.setUpdatedBy(updatedBy); - }); - try { - await saveSuggestions(this.dataAccess, covered); - coveredSuggestions.push(...covered); - // eslint-disable-next-line max-len - this.log.info(`[edge-deploy] Marked ${covered.length} suggestions as ${coverageField}=${suggestion.getId()}`); - } catch (coverError) { - this.log.warn( - '[edge-deploy] Failed to mark covered suggestions for pattern suggestion ' - + `${suggestion.getId()}: ${coverError.message}`, - ); - } - } } /** * Deploys suggestions to edge, handling both regular and domain-wide suggestions. * * Regular suggestions are deployed via deploySuggestions(). Domain-wide suggestions - * update the site metaconfig's prerender allowList. Suggestions covered by domain-wide - * patterns (in the same batch or across the whole opportunity) are automatically marked. + * update the site metaconfig's prerender allowList. * * @param {Object} params * @param {Object} params.site - Site entity * @param {Object} params.opportunity - Opportunity entity * @param {Array} params.targetSuggestions - Suggestions selected for this deployment - * @param {Array} params.allSuggestions - All suggestions for the opportunity * @param {string} [params.updatedBy='edge-deploy'] - Value for updatedBy on saved suggestions * @returns {Promise} { succeededSuggestions, failedSuggestions, coveredSuggestions } * - succeededSuggestions: suggestion entities that were deployed @@ -1602,7 +1531,6 @@ class TokowakaClient { site, opportunity, targetSuggestions, - allSuggestions, updatedBy = 'edge-deploy', metadata = {}, }) { @@ -1611,12 +1539,11 @@ class TokowakaClient { const { patternSuggestions, validSuggestions: classified } = classifySuggestions(targetSuggestions, this.log); this.log.info( `[edge-deploy] Classification: pattern=${patternSuggestions.length},` - + ` perUrl=${classified.length}, allSuggestions=${allSuggestions.length}`, + + ` perUrl=${classified.length}`, ); - // Step 2: remove per-URL suggestions already covered by a pattern in this batch. - // eslint-disable-next-line max-len - const { remaining: validSuggestions, skippedInBatch } = filterBatchCoveredSuggestions(classified, patternSuggestions, this.log); + // Step 2: keep per-URL deploy path independent from async covered marking. + const validSuggestions = classified; let succeededSuggestions = []; const failedSuggestions = []; @@ -1634,7 +1561,6 @@ class TokowakaClient { const coveredSuggestions = []; const deployedPatternSuggestions = []; if (patternSuggestions.length > 0) { - const skippedInBatchIds = new Set(skippedInBatch.map((item) => item.suggestion.getId())); const baseURL = site.getBaseURL(); let metaconfig = await this.fetchMetaconfig(baseURL); if (!metaconfig) { @@ -1656,10 +1582,7 @@ class TokowakaClient { allowedRegexPatterns, metaconfig, baseURL, - skippedInBatchIds, - allSuggestions, updatedBy, - coveredSuggestions, ); deployedPatternSuggestions.push(suggestion); } catch (error) { @@ -1683,34 +1606,6 @@ class TokowakaClient { } } } - - // Step 5: mark same-batch skipped suggestions, then batch-save. - if (skippedInBatch.length > 0) { - const skippedSuggestions = skippedInBatch.map((item) => { - const coverageField = item.isDomainWide ? 'coveredByDomainWide' : 'coveredByPattern'; - item.suggestion.setData({ - ...item.suggestion.getData(), - [coverageField]: 'same-batch-deployment', - skippedInDeployment: true, - }); - item.suggestion.setUpdatedBy(updatedBy); - return item.suggestion; - }); - - try { - await saveSuggestions(this.dataAccess, skippedSuggestions); - succeededSuggestions.push(...skippedSuggestions); - coveredSuggestions.push(...skippedSuggestions); - } catch (error) { - // eslint-disable-next-line max-len - this.log.warn(`[edge-deploy] Failed to save same-batch skipped suggestions: ${error.message}`); - skippedSuggestions.forEach((s) => { - // eslint-disable-next-line max-len - failedSuggestions.push({ suggestion: s, reason: 'Failed to mark as covered', statusCode: 500 }); - }); - } - } - return { succeededSuggestions, failedSuggestions, coveredSuggestions }; } diff --git a/packages/spacecat-shared-tokowaka-client/test/index.test.js b/packages/spacecat-shared-tokowaka-client/test/index.test.js index 7af40aad2..ec34e1963 100644 --- a/packages/spacecat-shared-tokowaka-client/test/index.test.js +++ b/packages/spacecat-shared-tokowaka-client/test/index.test.js @@ -5945,7 +5945,7 @@ describe('TokowakaClient', () => { expect(uploadCall).to.have.property('siteId', 'site-123'); }); - it('should mark non-batch suggestions covered by domain-wide patterns', async () => { + it('should not synchronously mark non-batch suggestions covered by domain-wide patterns', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], @@ -5961,8 +5961,8 @@ describe('TokowakaClient', () => { allSuggestions: [dw, covered], }); - expect(result.coveredSuggestions).to.include(covered); - expect(covered.getData()).to.have.property('coveredByDomainWide', 'dw1'); + expect(result.coveredSuggestions).to.not.include(covered); + expect(covered.getData()).to.not.have.property('coveredByDomainWide'); }); it('should not mark already-domain-wide suggestions as covered', async () => { @@ -5989,7 +5989,7 @@ describe('TokowakaClient', () => { expect(result.coveredSuggestions).to.not.include(dw2); }); - it('should mark path-level pattern suggestions as coveredByDomainWide when domain-wide is deployed', async () => { + it('should not synchronously mark path-level pattern suggestions when domain-wide is deployed', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], @@ -6007,8 +6007,8 @@ describe('TokowakaClient', () => { allSuggestions: [dw, pathLevel], }); - expect(result.coveredSuggestions).to.include(pathLevel); - expect(pathLevel.getData()).to.have.property('coveredByDomainWide', 'dw1'); + expect(result.coveredSuggestions).to.not.include(pathLevel); + expect(pathLevel.getData()).to.not.have.property('coveredByDomainWide'); }); it('should not mark already-deployed path-level suggestions as coveredByDomainWide', async () => { @@ -6053,7 +6053,7 @@ describe('TokowakaClient', () => { expect(result.coveredSuggestions).to.not.include(approved); }); - it('should warn but continue when covered-suggestion saveMany fails', async () => { + it('should ignore non-batch covered suggestion save path (now async in worker)', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], @@ -6061,12 +6061,6 @@ describe('TokowakaClient', () => { const covered = makeSuggestion('covered1', { url: 'https://example.com/page1' }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); - // First saveMany call is for covered suggestions (fails), - // second is for pattern suggestions batch save (succeeds) - client.dataAccess.Suggestion.saveMany - .onFirstCall().rejects(new Error('DB error')) - .onSecondCall().resolves(); - const result = await client.deployToEdge({ site: mockSite, opportunity: mockOpportunity, @@ -6076,9 +6070,9 @@ describe('TokowakaClient', () => { // domain-wide itself still succeeded expect(result.succeededSuggestions).to.include(dw); - // covered is not in coveredSuggestions — saveMany error was swallowed + // covered is not in coveredSuggestions in sync path anymore expect(result.coveredSuggestions).to.not.include(covered); - expect(log.warn).to.have.been.called; + expect(log.warn).to.not.have.been.called; }); it('should mark domain-wide as failed with statusCode 500 when metaconfig upload fails', async () => { @@ -6103,7 +6097,7 @@ describe('TokowakaClient', () => { expect(result.failedSuggestions[0].reason).to.equal('Internal server error'); }); - it('should skip invalid regex in same-batch pattern filtering without throwing', async () => { + it('should deploy regular suggestion even when domain-wide regex list includes invalid entries', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['[invalid', '^https://example\\.com/.*'], @@ -6111,6 +6105,10 @@ describe('TokowakaClient', () => { const regular = makeSuggestion('r1', { url: 'https://example.com/page1' }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); + deploySuggestionsStub.resolves({ + succeededSuggestions: [regular], + failedSuggestions: [], + }); const result = await client.deployToEdge({ site: mockSite, @@ -6119,8 +6117,9 @@ describe('TokowakaClient', () => { allSuggestions: [dw, regular], }); - // valid pattern matches regular, so it's skipped in batch - expect(result.coveredSuggestions).to.include(regular); + expect(deploySuggestionsStub).to.have.been.calledOnce; + expect(result.succeededSuggestions).to.include(regular); + expect(result.coveredSuggestions).to.not.include(regular); }); it('should skip domain-wide suggestion with no valid allowedRegexPatterns', async () => { @@ -6141,7 +6140,7 @@ describe('TokowakaClient', () => { expect(uploadMetaconfigStub).to.not.have.been.called; }); - it('should warn and skip empty string patterns', async () => { + it('should deploy path pattern with empty matcher entries without sync covered-marking', async () => { const path = makeSuggestion('p1', { allowedRegexPatterns: [''], }); @@ -6156,10 +6155,10 @@ describe('TokowakaClient', () => { }); expect(result.succeededSuggestions).to.include(path); - expect(log.warn).to.have.been.called; + expect(log.warn).to.not.have.been.called; }); - it('should warn and skip invalid regex patterns for domain-wide', async () => { + it('should deploy domain-wide even if regex list contains invalid patterns', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['[invalid', '^https://example\\.com/.*'], @@ -6174,11 +6173,11 @@ describe('TokowakaClient', () => { allSuggestions: [dw], }); - expect(log.warn).to.have.been.called; + expect(log.warn).to.not.have.been.called; expect(uploadMetaconfigStub).to.have.been.calledOnce; }); - it('should skip same-batch regular suggestions covered by domain-wide patterns', async () => { + it('should deploy same-batch regular suggestions even when covered by domain-wide patterns', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], @@ -6186,6 +6185,10 @@ describe('TokowakaClient', () => { const regular = makeSuggestion('r1', { url: 'https://example.com/page1' }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); + deploySuggestionsStub.resolves({ + succeededSuggestions: [regular], + failedSuggestions: [], + }); const result = await client.deployToEdge({ site: mockSite, @@ -6194,46 +6197,25 @@ describe('TokowakaClient', () => { allSuggestions: [dw, regular], }); - // regular was in same batch as domain-wide, should be marked covered - expect(result.coveredSuggestions).to.include(regular); - expect(regular.getData()).to.have.property('coveredByDomainWide', 'same-batch-deployment'); - expect(regular.getData()).to.have.property('skippedInDeployment', true); - // deploySuggestions should NOT have been called for the regular suggestion - expect(deploySuggestionsStub).to.not.have.been.called; - }); - - it('same-batch skip with path-level pattern sets coveredByPattern, not coveredByDomainWide', async () => { - const path = makeSuggestion('p1', { - allowedRegexPatterns: ['/products/*'], - }); - const regular = makeSuggestion('r1', { url: 'https://example.com/products/item' }); - - fetchMetaconfigStub.resolves({ siteId: 'site-123' }); - - const result = await client.deployToEdge({ - site: mockSite, - opportunity: mockOpportunity, - targetSuggestions: [path, regular], - allSuggestions: [path, regular], - }); - - expect(result.coveredSuggestions).to.include(regular); - expect(regular.getData()).to.have.property('coveredByPattern', 'same-batch-deployment'); + expect(deploySuggestionsStub).to.have.been.calledOnce; + expect(result.succeededSuggestions).to.include(regular); + expect(result.coveredSuggestions).to.not.include(regular); expect(regular.getData()).to.not.have.property('coveredByDomainWide'); - expect(regular.getData()).to.have.property('skippedInDeployment', true); - expect(deploySuggestionsStub).to.not.have.been.called; + expect(regular.getData()).to.not.have.property('skippedInDeployment'); }); - it('should surface same-batch save failures as failed suggestions with statusCode 500', async () => { + it('should not use same-batch covered-marking failure path (async in worker)', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], }); const regular = makeSuggestion('r1', { url: 'https://example.com/page1' }); + deploySuggestionsStub.resolves({ + succeededSuggestions: [regular], + failedSuggestions: [], + }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); - // Make saveMany fail for same-batch skipped suggestions - client.dataAccess.Suggestion.saveMany.rejects(new Error('save failed')); const result = await client.deployToEdge({ site: mockSite, @@ -6242,11 +6224,9 @@ describe('TokowakaClient', () => { allSuggestions: [dw, regular], }); - expect(result.failedSuggestions.some((f) => f.suggestion === regular)).to.be.true; - // eslint-disable-next-line max-len - expect(result.failedSuggestions.find((f) => f.suggestion === regular).statusCode).to.equal(500); + expect(result.failedSuggestions.some((f) => f.suggestion === regular)).to.be.false; expect(result.coveredSuggestions).to.not.include(regular); - expect(log.warn).to.have.been.called; + expect(log.warn).to.not.have.been.called; }); it('should use default updatedBy when not provided', async () => { @@ -6293,7 +6273,7 @@ describe('TokowakaClient', () => { expect(result.coveredSuggestions).to.not.include(regular); }); - it('should not call deploySuggestions when all regular suggestions are skipped in batch', async () => { + it('should call deploySuggestions when regular suggestions are selected with domain-wide', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], @@ -6302,6 +6282,10 @@ describe('TokowakaClient', () => { const r2 = makeSuggestion('r2', { url: 'https://example.com/page2' }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); + deploySuggestionsStub.resolves({ + succeededSuggestions: [r1, r2], + failedSuggestions: [], + }); await client.deployToEdge({ site: mockSite, @@ -6310,10 +6294,10 @@ describe('TokowakaClient', () => { allSuggestions: [dw, r1, r2], }); - expect(deploySuggestionsStub).to.not.have.been.called; + expect(deploySuggestionsStub).to.have.been.calledOnce; }); - it('should not include same-batch skipped suggestions in covered-by-pattern check', async () => { + it('should keep coveredSuggestions empty when domain-wide and matching regular are selected together', async () => { const dw = makeSuggestion('dw1', { isDomainWide: true, allowedRegexPatterns: ['^https://example\\.com/.*'], @@ -6321,6 +6305,10 @@ describe('TokowakaClient', () => { const skipped = makeSuggestion('r1', { url: 'https://example.com/page1' }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); + deploySuggestionsStub.resolves({ + succeededSuggestions: [skipped], + failedSuggestions: [], + }); const result = await client.deployToEdge({ site: mockSite, @@ -6329,10 +6317,7 @@ describe('TokowakaClient', () => { allSuggestions: [dw, skipped], }); - // skipped is in same batch, it should be in coveredSuggestions once (via same-batch path) - // but NOT doubly added via the per-suggestion covered-marking path - const skippedInCovered = result.coveredSuggestions.filter((s) => s.getId() === 'r1'); - expect(skippedInCovered).to.have.length(1); + expect(result.coveredSuggestions).to.have.length(0); }); it('domain-wide deploy merges /* into existing allowList instead of replacing it', async () => { @@ -6446,7 +6431,7 @@ describe('TokowakaClient', () => { expect(deploySuggestionsStub).to.not.have.been.called; }); - it('path deploy marks per-URL suggestions under the deployed path prefix as covered', async () => { + it('path deploy does not synchronously mark per-URL suggestions under the deployed path prefix', async () => { const path = makeSuggestion('p1', { allowedRegexPatterns: ['/products/*'], }); @@ -6462,8 +6447,8 @@ describe('TokowakaClient', () => { allSuggestions: [path, urlUnderPath, urlElsewhere], }); - expect(result.coveredSuggestions).to.include(urlUnderPath); - expect(urlUnderPath.getData()).to.have.property('coveredByPattern', 'p1'); + expect(result.coveredSuggestions).to.not.include(urlUnderPath); + expect(urlUnderPath.getData()).to.not.have.property('coveredByPattern'); expect(result.coveredSuggestions).to.not.include(urlElsewhere); }); @@ -6488,7 +6473,7 @@ describe('TokowakaClient', () => { expect(result.failedSuggestions[0].reason).to.equal('Internal server error'); }); - it('path deploy marks coverage via allowedRegexPatterns even when pathPattern field is absent', async () => { + it('path deploy does not set coveredByPattern via allowedRegexPatterns in sync path', async () => { const path = makeSuggestion('p1', { allowedRegexPatterns: ['/products/*'], // no pathPattern field — coverage is derived from allowedRegexPatterns @@ -6505,8 +6490,8 @@ describe('TokowakaClient', () => { }); expect(result.succeededSuggestions).to.include(path); - expect(result.coveredSuggestions).to.include(urlUnderPath); - expect(urlUnderPath.getData()).to.have.property('coveredByPattern', 'p1'); + expect(result.coveredSuggestions).to.not.include(urlUnderPath); + expect(urlUnderPath.getData()).to.not.have.property('coveredByPattern'); }); it('path deploy does not mark non-NEW, pattern, or already-deployed candidates as covered', async () => { @@ -6554,19 +6539,13 @@ describe('TokowakaClient', () => { expect(result.coveredSuggestions).to.not.include(badUrl); }); - it('path deploy warns but continues when covered-suggestion saveMany fails', async () => { + it('path deploy skips covered-suggestion saveMany path (now async in worker)', async () => { const path = makeSuggestion('p1', { allowedRegexPatterns: ['/products/*'], }); const urlUnderPath = makeSuggestion('u1', { url: 'https://example.com/products/item-1' }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); - // First saveMany call is for covered suggestions (fails), - // second is for pattern suggestions batch save (succeeds) - client.dataAccess.Suggestion.saveMany - .onFirstCall().rejects(new Error('DB error')) - .onSecondCall().resolves(); - const result = await client.deployToEdge({ site: mockSite, opportunity: mockOpportunity, @@ -6576,9 +6555,8 @@ describe('TokowakaClient', () => { // path suggestion itself still succeeded expect(result.succeededSuggestions).to.include(path); - // covered is not in coveredSuggestions — saveMany error was swallowed expect(result.coveredSuggestions).to.not.include(urlUnderPath); - expect(log.warn).to.have.been.called; + expect(log.warn).to.not.have.been.called; }); it('batch filter is a no-op when all pattern matchers are invalid', async () => { From 8fd0818650be6c652ac40579b4b0ce01da2fa42d Mon Sep 17 00:00:00 2001 From: Khushboo Kumari Date: Wed, 24 Jun 2026 09:38:57 +0530 Subject: [PATCH 2/5] feat(tokowaka-client): remove obsolete batch-covered helper after async migration Drop the now-unused filterBatchCoveredSuggestions utility and its matcher import after covered marking moved to the worker path. This keeps package coverage green and aligns shared-client behavior with the new async covered-marking contract (coveredSuggestions is no longer populated in deployToEdge). Co-authored-by: Cursor --- .../src/utils/suggestion-utils.js | 45 ------------------- 1 file changed, 45 deletions(-) diff --git a/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js b/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js index 15ef4fda0..34bcd82ce 100644 --- a/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js +++ b/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js @@ -10,8 +10,6 @@ * governing permissions and limitations under the License. */ -import { buildUrlMatcher } from './pattern-utils.js'; - /** Returns a shallow copy of obj with the specified keys removed. */ export function omitKeys(obj, keys) { const keySet = new Set(keys); @@ -198,46 +196,3 @@ export function classifySuggestions(targetSuggestions, log) { return { patternSuggestions, validSuggestions }; } - -/** - * Splits validSuggestions into those covered by an in-batch pattern and those that are not. - * Returns the two arrays; validSuggestions itself is not mutated. - * @param {Array} validSuggestions - Per-URL suggestions - * @param {Array} patternSuggestions - Pattern suggestions in the same batch - * @param {Object} log - Logger instance - * @returns {{ remaining: Array, skippedInBatch: Array }} - */ -export function filterBatchCoveredSuggestions(validSuggestions, patternSuggestions, log) { - if (patternSuggestions.length === 0 || validSuggestions.length === 0) { - return { remaining: validSuggestions, skippedInBatch: [] }; - } - - // Build matchers per pattern suggestion so we can track which type covered each URL. - // eslint-disable-next-line max-len - const matcherEntries = patternSuggestions.flatMap(({ suggestion, allowedRegexPatterns }) => { - const isDomainWide = suggestion.getData()?.isDomainWide === true; - return allowedRegexPatterns - .map(buildUrlMatcher) - .filter(Boolean) - .map((matcher) => ({ matcher, isDomainWide })); - }); - - if (matcherEntries.length === 0) { - return { remaining: validSuggestions, skippedInBatch: [] }; - } - - const remaining = []; - const skippedInBatch = []; - validSuggestions.forEach((s) => { - const url = s.getData()?.url; - const match = url && matcherEntries.find(({ matcher }) => matcher(url)); - if (match) { - skippedInBatch.push({ suggestion: s, isDomainWide: match.isDomainWide }); - log.info(`[edge-deploy] Skipping suggestion ${s.getId()} - covered by pattern`); - } else { - remaining.push(s); - } - }); - - return { remaining, skippedInBatch }; -} From b26d43019375fa2a3cd6933cd829b196d4df3b2c Mon Sep 17 00:00:00 2001 From: Khushboo Kumari Date: Wed, 24 Jun 2026 16:02:00 +0530 Subject: [PATCH 3/5] feat(tokowaka-client): make rollback covered cleanup async Co-authored-by: Cursor --- .../src/index.js | 22 ------ .../src/utils/suggestion-utils.js | 30 ------- .../test/index.test.js | 79 +++++++------------ 3 files changed, 27 insertions(+), 104 deletions(-) diff --git a/packages/spacecat-shared-tokowaka-client/src/index.js b/packages/spacecat-shared-tokowaka-client/src/index.js index c36807a70..f14686ecb 100644 --- a/packages/spacecat-shared-tokowaka-client/src/index.js +++ b/packages/spacecat-shared-tokowaka-client/src/index.js @@ -32,7 +32,6 @@ import { filterEligibleSuggestions, saveSuggestions, stripSuggestion, - cleanupCoveredSuggestions, classifySuggestions, } from './utils/suggestion-utils.js'; import { getEffectiveBaseURL } from './utils/site-utils.js'; @@ -1036,27 +1035,6 @@ class TokowakaClient { })); } } - - // Clean up covered suggestions for each succeeded pattern suggestion. - for (const suggestion of succeededPatternSuggestions) { - const suggData = suggestion.getData(); - const isDomainWide = suggData?.isDomainWide === true; - const coveredFallback = isDomainWide ? 'domain-wide-rollback' : 'path-rollback'; - const coverageField = isDomainWide - ? 'coveredByDomainWide' : 'coveredByPattern'; - const covered = allSuggestions.filter( - (s) => s.getData()?.[coverageField] === suggestion.getId(), - ); - const fieldsToStrip = isDomainWide - ? ['coveredByDomainWide'] - : ['edgeDeployed', 'tokowakaDeployed', 'coveredByPattern']; - if (covered.length > 0) { - // eslint-disable-next-line max-len - this.log.info(`[edge-rollback] Cleaning ${covered.length} covered suggestion(s) for pattern ${suggestion.getId()} (isDomainWide=${isDomainWide}, fallback=${coveredFallback})`); - } - // eslint-disable-next-line no-await-in-loop, max-len - await cleanupCoveredSuggestions(this.dataAccess, covered, coveredFallback, updatedBy, fieldsToStrip, this.log); - } } } diff --git a/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js b/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js index 34bcd82ce..dbe114fe9 100644 --- a/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js +++ b/packages/spacecat-shared-tokowaka-client/src/utils/suggestion-utils.js @@ -137,36 +137,6 @@ export function stripSuggestion(suggestion, actorFallback, updatedBy) { return suggestion; } -/** - * Clears coverage and deployment markers from suggestions that were covered by a pattern. - * Only strips the fields relevant to the rollback type so independent coverage layers - * are preserved. For example, rolling back domain-wide should only clear - * coveredByDomainWide — not coveredByPattern (which belongs to a separate path deploy). - * @param {Object} dataAccess - Data access layer - * @param {Array} covered - Covered suggestion entities - * @param {string} actorFallback - Fallback updatedBy string - * @param {string|undefined} updatedBy - Explicit actor - * @param {string[]} fieldsToStrip - Specific fields to remove - * @param {Object} log - Logger instance - * @returns {Promise} - */ -// eslint-disable-next-line max-len -export async function cleanupCoveredSuggestions(dataAccess, covered, actorFallback, updatedBy, fieldsToStrip, log) { - if (covered.length === 0) { - return; - } - covered.forEach((cs) => { - cs.setData(omitKeys(cs.getData(), fieldsToStrip)); - cs.setUpdatedBy(updatedBy ?? actorFallback); - }); - try { - await saveSuggestions(dataAccess, covered); - } catch (error) { - // eslint-disable-next-line max-len - log.error(`[edge-rollback-failed] Failed to clean ${covered.length} covered suggestion(s): ${error.message}`); - } -} - /** * Classifies a batch of target suggestions into pattern-based and per-URL buckets. * Pattern suggestions (domain-wide or path-level) are returned as diff --git a/packages/spacecat-shared-tokowaka-client/test/index.test.js b/packages/spacecat-shared-tokowaka-client/test/index.test.js index ec34e1963..0a01af31f 100644 --- a/packages/spacecat-shared-tokowaka-client/test/index.test.js +++ b/packages/spacecat-shared-tokowaka-client/test/index.test.js @@ -3091,7 +3091,7 @@ describe('TokowakaClient', () => { expect(uploadedConfig.prerender.allowList).to.deep.equal(['/products/*']); }); - it('cleans up covered suggestions (coveredByDomainWide) when rolling back a pattern suggestion', async () => { + it('does not synchronously clean coveredByDomainWide suggestions when rolling back a pattern suggestion', async () => { const prerenderOpportunity = { getId: () => 'opp-dw', getType: () => 'prerender' }; const dwSuggestion = { getId: () => 'dw-1', @@ -3138,17 +3138,12 @@ describe('TokowakaClient', () => { expect(dwData).to.not.have.property('edgeDeployed'); expect(dwData).to.not.have.property('tokowakaDeployed'); - // Verify covered suggestion was also cleaned up. - // DW rollback only strips coveredByDomainWide — edgeDeployed is - // preserved because it represents an independent per-URL deployment. - expect(coveredSuggestion.setData.calledOnce).to.be.true; - expect(coveredSuggestion.setUpdatedBy.calledWith('test@example.com')).to.be.true; - const coveredData = coveredSuggestion.setData.firstCall.args[0]; - expect(coveredData).to.have.property('edgeDeployed'); - expect(coveredData).to.not.have.property('coveredByDomainWide'); + // Covered suggestion cleanup is async and handled by import-worker. + expect(coveredSuggestion.setData.called).to.be.false; + expect(coveredSuggestion.setUpdatedBy.called).to.be.false; }); - it('cleans up coveredByDomainWide on path-level suggestions when rolling back domain-wide', async () => { + it('does not synchronously clean coveredByDomainWide on path-level suggestions when rolling back domain-wide', async () => { const prerenderOpportunity = { getId: () => 'opp-dw', getType: () => 'prerender' }; const dwSuggestion = { getId: () => 'dw-1', @@ -3186,18 +3181,15 @@ describe('TokowakaClient', () => { ); expect(result.succeededSuggestions).to.include(dwSuggestion); - expect(pathSuggestion.setData.calledOnce).to.be.true; - const pathData = pathSuggestion.setData.firstCall.args[0]; - expect(pathData).to.not.have.property('coveredByDomainWide'); - expect(pathData).to.have.property('allowedRegexPatterns'); - // Verify the cleaned-up path suggestion was actually persisted via saveMany - expect(client.dataAccess.Suggestion.saveMany).to.have.been.calledWith( + expect(pathSuggestion.setData.called).to.be.false; + expect(pathSuggestion.setUpdatedBy.called).to.be.false; + expect(client.dataAccess.Suggestion.saveMany).to.not.have.been.calledWith( sinon.match((arr) => arr.includes(pathSuggestion)), sinon.match.any, ); }); - it('cleans up covered suggestions (coveredByPattern) when rolling back a path-level pattern', async () => { + it('does not synchronously clean coveredByPattern suggestions when rolling back a path-level pattern', async () => { const prerenderOpportunity = { getId: () => 'opp-p', getType: () => 'prerender' }; const pathSuggestion = { getId: () => 'path-1', @@ -3234,13 +3226,11 @@ describe('TokowakaClient', () => { { allSuggestions: [pathSuggestion, coveredSuggestion] }, ); - expect(coveredSuggestion.setData.calledOnce).to.be.true; - const coveredData = coveredSuggestion.setData.firstCall.args[0]; - expect(coveredData).to.not.have.property('edgeDeployed'); - expect(coveredData).to.not.have.property('coveredByPattern'); + expect(coveredSuggestion.setData.called).to.be.false; + expect(coveredSuggestion.setUpdatedBy.called).to.be.false; }); - it('uses domain-wide-rollback fallback for covered suggestions when updatedBy is not provided (domain-wide parent)', async () => { + it('does not apply domain-wide-rollback fallback in shared client for async covered cleanup', async () => { const prerenderOpportunity = { getId: () => 'opp-dw', getType: () => 'prerender' }; const dwSuggestion = { getId: () => 'dw-1', @@ -3280,11 +3270,10 @@ describe('TokowakaClient', () => { // The domain-wide suggestion itself uses 'tokowaka-rollback' expect(dwSuggestion.setUpdatedBy.calledWith('tokowaka-rollback')).to.be.true; - // The covered suggestion uses 'domain-wide-rollback' - expect(coveredSuggestion.setUpdatedBy.calledWith('domain-wide-rollback')).to.be.true; + expect(coveredSuggestion.setUpdatedBy.called).to.be.false; }); - it('uses path-rollback fallback for covered suggestions when updatedBy is not provided (path parent)', async () => { + it('does not apply path-rollback fallback in shared client for async covered cleanup', async () => { const prerenderOpportunity = { getId: () => 'opp-p', getType: () => 'prerender' }; const pathSuggestion = { getId: () => 'path-1', @@ -3322,8 +3311,7 @@ describe('TokowakaClient', () => { // The path suggestion itself uses 'tokowaka-rollback' expect(pathSuggestion.setUpdatedBy.calledWith('tokowaka-rollback')).to.be.true; - // The covered suggestion uses 'path-rollback' - expect(coveredSuggestion.setUpdatedBy.calledWith('path-rollback')).to.be.true; + expect(coveredSuggestion.setUpdatedBy.called).to.be.false; }); it('DW rollback does NOT cascade to deployed path suggestions (paths are independent)', async () => { @@ -3649,7 +3637,7 @@ describe('TokowakaClient', () => { expect(pathSuggestion.save).to.not.have.been.called; }); - it('rollback logs error when covered suggestion saveMany fails', async () => { + it('rollback does not attempt covered suggestion saveMany cleanup in shared client', async () => { const prerenderOpportunity = { getId: () => 'opp-dw', getType: () => 'prerender' }; const dwSuggestion = { getId: () => 'dw-1', @@ -3677,10 +3665,7 @@ describe('TokowakaClient', () => { prerender: { allowList: ['/*'] }, }); sinon.stub(client, 'uploadMetaconfig').resolves(); - // First saveMany succeeds (pattern suggestion save), second fails (covered cleanup) - client.dataAccess.Suggestion.saveMany - .onFirstCall().resolves() - .onSecondCall().rejects(new Error('DB error')); + client.dataAccess.Suggestion.saveMany.resolves(); await client.rollbackSuggestions( mockSite, @@ -3689,11 +3674,10 @@ describe('TokowakaClient', () => { { allSuggestions: [dwSuggestion, covered] }, ); - // Covered suggestion was mutated before saveMany - expect(covered.setData.calledOnce).to.be.true; - // The failure was logged as a consolidated error for alerting - // eslint-disable-next-line max-len - expect(log.error).to.have.been.calledWithMatch(/\[edge-rollback-failed\].*covered suggestion/); + expect(covered.setData.called).to.be.false; + expect(covered.setUpdatedBy.called).to.be.false; + expect(client.dataAccess.Suggestion.saveMany).to.have.been.calledOnce; + expect(client.dataAccess.Suggestion.saveMany.firstCall.args[0]).to.not.include(covered); }); it('rollback marks all pattern suggestions as failed when saveMany throws', async () => { @@ -3740,9 +3724,7 @@ describe('TokowakaClient', () => { // --- Edge case tests for deploy/rollback interaction scenarios --- - it('DW rollback preserves coveredByPattern on suggestions covered by both DW and path', async () => { - // Scenario: URL covered by both DW and a path. DW rollback should - // only strip coveredByDomainWide; coveredByPattern must survive. + it('DW rollback leaves doubly covered URL suggestions untouched for async cleanup', async () => { const prerenderOpportunity = { getId: () => 'opp-dw', getType: () => 'prerender', @@ -3787,15 +3769,11 @@ describe('TokowakaClient', () => { }, ); - expect(doubleCoveredSuggestion.setData.calledOnce).to.be.true; - const data = doubleCoveredSuggestion.setData.firstCall.args[0]; - expect(data).to.not.have.property('coveredByDomainWide'); - expect(data).to.have.property('coveredByPattern', 'path-1'); + expect(doubleCoveredSuggestion.setData.called).to.be.false; + expect(doubleCoveredSuggestion.setUpdatedBy.called).to.be.false; }); - it('path rollback preserves coveredByDomainWide on suggestions covered by both DW and path', async () => { - // Scenario: URL covered by both DW and a path. Path rollback should - // only strip coveredByPattern; coveredByDomainWide must survive. + it('path rollback leaves doubly covered URL suggestions untouched for async cleanup', async () => { const prerenderOpportunity = { getId: () => 'opp-p', getType: () => 'prerender', @@ -3840,11 +3818,8 @@ describe('TokowakaClient', () => { }, ); - expect(doubleCoveredSuggestion.setData.calledOnce).to.be.true; - const data = doubleCoveredSuggestion.setData.firstCall.args[0]; - expect(data).to.not.have.property('coveredByPattern'); - expect(data).to.not.have.property('edgeDeployed'); - expect(data).to.have.property('coveredByDomainWide', 'dw-1'); + expect(doubleCoveredSuggestion.setData.called).to.be.false; + expect(doubleCoveredSuggestion.setUpdatedBy.called).to.be.false; }); it('DW rollback cascade skips path deployed before DW', async () => { From 889731b2ab0ea2692b7b0f3ed4b737f469e0aa03 Mon Sep 17 00:00:00 2001 From: Khushboo Kumari Date: Thu, 25 Jun 2026 11:19:51 +0530 Subject: [PATCH 4/5] chore(tokowaka-client): clean async covered marking API Co-authored-by: Cursor --- .../src/index.js | 19 ++-- .../test/index.test.js | 86 ++----------------- 2 files changed, 11 insertions(+), 94 deletions(-) diff --git a/packages/spacecat-shared-tokowaka-client/src/index.js b/packages/spacecat-shared-tokowaka-client/src/index.js index f14686ecb..0ec3c93e7 100644 --- a/packages/spacecat-shared-tokowaka-client/src/index.js +++ b/packages/spacecat-shared-tokowaka-client/src/index.js @@ -886,16 +886,12 @@ class TokowakaClient { * @param {Object} opportunity - Opportunity entity * @param {Array} suggestions - Array of suggestion entities to rollback * @param {Object} [options] - Optional parameters - * @param {Array} [options.allSuggestions=[]] - All suggestions for the opportunity, - * used to clean up suggestions covered by a domain-wide or path-level pattern rollback * @param {string} [options.updatedBy] - Actor identity (e.g. profile email). When undefined, - * context-specific fallback strings are used: 'tokowaka-rollback' for direct suggestions, - * 'domain-wide-rollback' for per-URL suggestions covered by a domain-wide pattern, and - * 'path-rollback' for per-URL suggestions covered by a path pattern. + * 'tokowaka-rollback' is used. * @returns {Promise} - Rollback result with succeeded/failed suggestions */ // eslint-disable-next-line max-len - async rollbackSuggestions(site, opportunity, suggestions, { allSuggestions = [], updatedBy } = {}) { + async rollbackSuggestions(site, opportunity, suggestions, { updatedBy } = {}) { const opportunityType = opportunity.getType(); const baseURL = getEffectiveBaseURL(site); const mapper = this.mapperRegistry.getMapper(opportunityType); @@ -912,7 +908,7 @@ class TokowakaClient { const patternSuggestions = suggestions.filter(isPatternSuggestion); const perUrlSuggestions = suggestions.filter((s) => !isPatternSuggestion(s)); // eslint-disable-next-line max-len - this.log.info(`[edge-rollback] Site: ${baseURL}, total: ${suggestions.length}, pattern: ${patternSuggestions.length}, perUrl: ${perUrlSuggestions.length}, allSuggestions: ${allSuggestions.length}`); + this.log.info(`[edge-rollback] Site: ${baseURL}, total: ${suggestions.length}, pattern: ${patternSuggestions.length}, perUrl: ${perUrlSuggestions.length}`); const { eligible: eligibleSuggestions, @@ -1503,7 +1499,7 @@ class TokowakaClient { * @returns {Promise} { succeededSuggestions, failedSuggestions, coveredSuggestions } * - succeededSuggestions: suggestion entities that were deployed * - failedSuggestions: { suggestion, reason } items that couldn't be deployed - * - coveredSuggestions: suggestion entities auto-marked via domain-wide patterns + * - coveredSuggestions: always empty; covered marking now runs asynchronously */ async deployToEdge({ site, @@ -1514,15 +1510,12 @@ class TokowakaClient { }) { // Step 1: classify suggestions into pattern vs per-URL buckets. // eslint-disable-next-line max-len - const { patternSuggestions, validSuggestions: classified } = classifySuggestions(targetSuggestions, this.log); + const { patternSuggestions, validSuggestions } = classifySuggestions(targetSuggestions, this.log); this.log.info( `[edge-deploy] Classification: pattern=${patternSuggestions.length},` - + ` perUrl=${classified.length}`, + + ` perUrl=${validSuggestions.length}`, ); - // Step 2: keep per-URL deploy path independent from async covered marking. - const validSuggestions = classified; - let succeededSuggestions = []; const failedSuggestions = []; diff --git a/packages/spacecat-shared-tokowaka-client/test/index.test.js b/packages/spacecat-shared-tokowaka-client/test/index.test.js index 0a01af31f..f72439654 100644 --- a/packages/spacecat-shared-tokowaka-client/test/index.test.js +++ b/packages/spacecat-shared-tokowaka-client/test/index.test.js @@ -3127,7 +3127,7 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, coveredSuggestion], updatedBy: 'test@example.com' }, + { updatedBy: 'test@example.com' }, ); expect(result.succeededSuggestions).to.include(dwSuggestion); @@ -3177,7 +3177,7 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, pathSuggestion], updatedBy: 'test@example.com' }, + { updatedBy: 'test@example.com' }, ); expect(result.succeededSuggestions).to.include(dwSuggestion); @@ -3223,7 +3223,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [pathSuggestion], - { allSuggestions: [pathSuggestion, coveredSuggestion] }, ); expect(coveredSuggestion.setData.called).to.be.false; @@ -3265,7 +3264,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, coveredSuggestion] }, ); // The domain-wide suggestion itself uses 'tokowaka-rollback' @@ -3306,7 +3304,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [pathSuggestion], - { allSuggestions: [pathSuggestion, coveredSuggestion] }, ); // The path suggestion itself uses 'tokowaka-rollback' @@ -3358,7 +3355,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, deployedPathSuggestion, pathCoveredSuggestion] }, ); // Domain-wide suggestion was rolled back @@ -3406,7 +3402,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, deployedPathSuggestion] }, ); // Only '/*' removed; '/products/*' and '/blog/*' preserved @@ -3452,7 +3447,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, deployedPathSuggestion] }, ); // One upload for DW removal only @@ -3494,7 +3488,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, undeployedPathSuggestion] }, ); // Undeployed path suggestion was not touched @@ -3671,7 +3664,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, covered] }, ); expect(covered.setData.called).to.be.false; @@ -3761,12 +3753,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { - allSuggestions: [ - dwSuggestion, - doubleCoveredSuggestion, - ], - }, ); expect(doubleCoveredSuggestion.setData.called).to.be.false; @@ -3810,12 +3796,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [pathSuggestion], - { - allSuggestions: [ - pathSuggestion, - doubleCoveredSuggestion, - ], - }, ); expect(doubleCoveredSuggestion.setData.called).to.be.false; @@ -3860,9 +3840,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { - allSuggestions: [dwSuggestion, preExistingPath], - }, ); // Pre-existing path was NOT cascaded @@ -3908,7 +3885,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, laterPath] }, ); // Path was NOT cascaded @@ -3955,9 +3931,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { - allSuggestions: [dwSuggestion, pathOnlyCovered], - }, ); // URL with only coveredByPattern was NOT touched @@ -3965,9 +3938,9 @@ describe('TokowakaClient', () => { expect(pathOnlyCovered.setData.called).to.be.false; }); - it('DW rollback cascade skips other domain-wide suggestions in allSuggestions', async () => { + it('DW rollback leaves other domain-wide suggestions untouched', async () => { // Covers the isDomainWide guard inside the cascade filter. - // A second DW suggestion in allSuggestions must not be cascaded. + // A second DW suggestion not selected for rollback must not be cascaded. const prerenderOpportunity = { getId: () => 'opp-dw', getType: () => 'prerender', @@ -4004,7 +3977,6 @@ describe('TokowakaClient', () => { mockSite, prerenderOpportunity, [dwSuggestion], - { allSuggestions: [dwSuggestion, otherDw] }, ); // The other DW suggestion was not cascaded @@ -5793,7 +5765,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1, s2], - allSuggestions: [s1, s2], updatedBy: 'test-user', }); @@ -5816,7 +5787,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], updatedBy: 'test-user', }); @@ -5837,7 +5807,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], }); expect(s1.getData()).to.not.have.property('edgeOptimizeStatus'); @@ -5856,7 +5825,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], }); expect(result.failedSuggestions).to.have.length(1); @@ -5873,7 +5841,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], }); expect.fail('Should have thrown'); } catch (error) { @@ -5893,7 +5860,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); expect(uploadMetaconfigStub).to.have.been.calledOnce; @@ -5913,7 +5879,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); const uploadCall = uploadMetaconfigStub.firstCall.args[1]; @@ -5933,7 +5898,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw, covered], }); expect(result.coveredSuggestions).to.not.include(covered); @@ -5957,7 +5921,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw1], - allSuggestions: [dw1, dw2], }); // dw2 is domain-wide so should not appear in coveredSuggestions @@ -5979,7 +5942,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw, pathLevel], }); expect(result.coveredSuggestions).to.not.include(pathLevel); @@ -6002,7 +5964,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw, pathLevel], }); expect(result.coveredSuggestions).to.not.include(pathLevel); @@ -6022,7 +5983,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw, approved], }); expect(result.coveredSuggestions).to.not.include(approved); @@ -6040,7 +6000,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw, covered], }); // domain-wide itself still succeeded @@ -6063,7 +6022,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); expect(result.succeededSuggestions).to.not.include(dw); @@ -6089,7 +6047,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw, regular], - allSuggestions: [dw, regular], }); expect(deploySuggestionsStub).to.have.been.calledOnce; @@ -6107,7 +6064,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); expect(result.succeededSuggestions).to.have.length(0); @@ -6126,7 +6082,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path], }); expect(result.succeededSuggestions).to.include(path); @@ -6145,7 +6100,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); expect(log.warn).to.not.have.been.called; @@ -6169,7 +6123,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw, regular], - allSuggestions: [dw, regular], }); expect(deploySuggestionsStub).to.have.been.calledOnce; @@ -6196,7 +6149,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw, regular], - allSuggestions: [dw, regular], }); expect(result.failedSuggestions.some((f) => f.suggestion === regular)).to.be.false; @@ -6216,7 +6168,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], }); expect(s1.getUpdatedBy()).to.equal('edge-deploy'); @@ -6240,7 +6191,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw, regular], - allSuggestions: [dw, regular], }); expect(deploySuggestionsStub).to.have.been.calledOnce; @@ -6266,7 +6216,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw, r1, r2], - allSuggestions: [dw, r1, r2], }); expect(deploySuggestionsStub).to.have.been.calledOnce; @@ -6289,7 +6238,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw, skipped], - allSuggestions: [dw, skipped], }); expect(result.coveredSuggestions).to.have.length(0); @@ -6307,7 +6255,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); const uploadedConfig = uploadMetaconfigStub.firstCall.args[1]; @@ -6326,7 +6273,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); expect(uploadMetaconfigStub).to.not.have.been.called; @@ -6343,7 +6289,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path], }); expect(result.succeededSuggestions).to.include(path); @@ -6363,7 +6308,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path], }); const uploadedConfig = uploadMetaconfigStub.firstCall.args[1]; @@ -6381,7 +6325,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path], }); expect(uploadMetaconfigStub).to.not.have.been.called; @@ -6400,7 +6343,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path], }); expect(deploySuggestionsStub).to.not.have.been.called; @@ -6419,7 +6361,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path, urlUnderPath, urlElsewhere], }); expect(result.coveredSuggestions).to.not.include(urlUnderPath); @@ -6439,7 +6380,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path], }); expect(result.succeededSuggestions).to.not.include(path); @@ -6461,7 +6401,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path, urlUnderPath], }); expect(result.succeededSuggestions).to.include(path); @@ -6469,18 +6408,10 @@ describe('TokowakaClient', () => { expect(urlUnderPath.getData()).to.not.have.property('coveredByPattern'); }); - it('path deploy does not mark non-NEW, pattern, or already-deployed candidates as covered', async () => { + it('path deploy returns empty coveredSuggestions for async covered marking', async () => { const path = makeSuggestion('p1', { allowedRegexPatterns: ['/products/*'], }); - // APPROVED — not a deployable status - const nonNew = makeSuggestion('u1', { url: 'https://example.com/products/a' }, 'APPROVED'); - // isDomainWide pattern — excluded (isPatternSuggestion) - const dw = makeSuggestion('u2', { isDomainWide: true, allowedRegexPatterns: ['/*'], url: 'https://example.com/products/b' }); - // another path pattern — excluded (isPatternSuggestion) - const otherPath = makeSuggestion('u3', { allowedRegexPatterns: ['/products/*'], url: 'https://example.com/products/c' }); - // already edgeDeployed — excluded - const deployed = makeSuggestion('u4', { url: 'https://example.com/products/d', edgeDeployed: 12345 }); fetchMetaconfigStub.resolves({ siteId: 'site-123' }); @@ -6488,7 +6419,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path, nonNew, dw, otherPath, deployed], }); expect(result.coveredSuggestions).to.have.length(0); @@ -6506,7 +6436,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path, badUrl], }); // bad URL is not covered, but deploy itself still succeeds @@ -6525,7 +6454,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [path], - allSuggestions: [path, urlUnderPath], }); // path suggestion itself still succeeded @@ -6570,7 +6498,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [invalidPattern, perUrl], - allSuggestions: [invalidPattern, perUrl], }); // per-URL not skipped (invalid matchers → no filtering) @@ -6592,7 +6519,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [dw], - allSuggestions: [dw], }); expect(result.succeededSuggestions).to.not.include(dw); @@ -6615,7 +6541,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], metadata: { applyStale: true }, }); @@ -6636,7 +6561,6 @@ describe('TokowakaClient', () => { site: mockSite, opportunity: mockOpportunity, targetSuggestions: [s1], - allSuggestions: [s1], }); expect(deploySuggestionsStub).to.have.been.calledOnce; From 6d415762be4de2e991201b0f329f18e0e36eedea Mon Sep 17 00:00:00 2001 From: Khushboo Kumari Date: Thu, 9 Jul 2026 10:20:02 +0530 Subject: [PATCH 5/5] docs(tokowaka-client): fix stale rollback comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit rollbackSuggestions no longer cleans up covered-marking state itself — that moved to the async import-worker job. Update the comment to reflect the current behavior instead of the removed sync cleanup. --- packages/spacecat-shared-tokowaka-client/src/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/spacecat-shared-tokowaka-client/src/index.js b/packages/spacecat-shared-tokowaka-client/src/index.js index 674f9c65d..640ab4631 100644 --- a/packages/spacecat-shared-tokowaka-client/src/index.js +++ b/packages/spacecat-shared-tokowaka-client/src/index.js @@ -979,7 +979,8 @@ class TokowakaClient { await saveSuggestions(this.dataAccess, savedEligibleSuggestions); // Roll back pattern suggestions: fetch metaconfig once, remove all patterns in a single - // in-memory pass, upload once, then save each suggestion and clean up its covered entries. + // in-memory pass, upload once, then save each suggestion. Cleanup of suggestions this + // pattern had covered happens separately, asynchronously, via the import worker. const succeededPatternSuggestions = []; const failedPatternSuggestions = [];