From 27a9f83825bb5abae2fbe0cc8f91db396f12326d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=89mile=20R=C3=A9?= Date: Tue, 26 May 2026 16:01:01 +0200 Subject: [PATCH] Promote on detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Émile Ré --- .../trackers/CookieBannerTrackersPage.tsx | 2 +- .../_components/TrackerPatternRow.tsx | 6 +- pkg/cookiebanner/pattern_analysis_worker.go | 18 +++ .../pattern_analysis_worker_process_test.go | 150 ++++++++++++++++++ pkg/cookiebanner/service.go | 20 +++ pkg/coredata/tracker_pattern_promote_test.go | 2 + 6 files changed, 194 insertions(+), 4 deletions(-) diff --git a/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/CookieBannerTrackersPage.tsx b/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/CookieBannerTrackersPage.tsx index 8fe92694b..7d947db39 100644 --- a/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/CookieBannerTrackersPage.tsx +++ b/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/CookieBannerTrackersPage.tsx @@ -232,8 +232,8 @@ export default function CookieBannerTrackersPage({ > - {__("Name")} {__("Type")} + {__("Name")} {__("Source")} {__("Category")} {__("Last Matched")} diff --git a/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/_components/TrackerPatternRow.tsx b/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/_components/TrackerPatternRow.tsx index b618fd959..3adf35ec2 100644 --- a/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/_components/TrackerPatternRow.tsx +++ b/apps/console/src/pages/organizations/cookie-banners/configuration/trackers/_components/TrackerPatternRow.tsx @@ -301,6 +301,9 @@ export function TrackerPatternRow({ patternKey, connectionId }: TrackerPatternRo return ( + + {typeBadge.label} +
{pattern.displayName} @@ -311,9 +314,6 @@ export function TrackerPatternRow({ patternKey, connectionId }: TrackerPatternRo )}
- - {typeBadge.label} - {srcBadge ? {srcBadge.label} diff --git a/pkg/cookiebanner/pattern_analysis_worker.go b/pkg/cookiebanner/pattern_analysis_worker.go index b7141f990..979d03a04 100644 --- a/pkg/cookiebanner/pattern_analysis_worker.go +++ b/pkg/cookiebanner/pattern_analysis_worker.go @@ -776,6 +776,24 @@ func (h *patternAnalysisHandler) adoptUncategorisedPatterns( return false, fmt.Errorf("cannot relink detected trackers from pattern %q: %w", ep.Pattern, err) } + // Promote the glob's source if this exact carries a + // stronger signal. The merge loop already handles + // promotion when the glob shares the exact's category, + // but adoption is the only path that can lift a glob + // the user (or an earlier pass) has placed in a + // non-uncategorised category. Without this, a + // PRE_EXISTING glob never advances to SCRIPT/EXTENSION + // even though new SDK-observed exacts confirm the + // stronger signal. PromoteSource mutates match.Source + // in place, so subsequent adoptions against the same + // glob ratchet correctly (PRE_EXISTING → EXTENSION → + // SCRIPT) without redundant writes. + if shouldPromoteSource(match.Source, ep.Source) { + if err := match.PromoteSource(ctx, tx, scope, *ep.Source, time.Now()); err != nil { + return false, fmt.Errorf("cannot promote source on glob pattern %q: %w", match.Pattern, err) + } + } + if err := ep.Delete(ctx, tx, scope); err != nil { return false, fmt.Errorf("cannot delete adopted exact pattern %q: %w", ep.Pattern, err) } diff --git a/pkg/cookiebanner/pattern_analysis_worker_process_test.go b/pkg/cookiebanner/pattern_analysis_worker_process_test.go index 4de5a1abe..bcfa0d695 100644 --- a/pkg/cookiebanner/pattern_analysis_worker_process_test.go +++ b/pkg/cookiebanner/pattern_analysis_worker_process_test.go @@ -435,6 +435,156 @@ func TestPatternAnalysisWorker_AdoptionTriggersDraftVersion(t *testing.T) { assert.Equal(t, coredata.CookieBannerVersionStateDraft, latest.State, "adoption must trigger a draft version") } +// TestPatternAnalysisWorker_AdoptionPromotesSourceCrossCategory +// seeds a banner with a PRE_EXISTING `ph_phc_*_posthog` glob already +// placed in a user-set category (analytics) and a single SCRIPT-source +// exact `ph_phc__posthog` in uncategorised. The merge loop hits +// the cross-category skip branch (the existing slot belongs to a +// recategorised glob), so promotion can only happen via the +// adoption path. This guards against the gap where last_matched_at +// was refreshed on the glob but its source stayed at PRE_EXISTING +// despite the new SDK-observed signal. +func TestPatternAnalysisWorker_AdoptionPromotesSourceCrossCategory(t *testing.T) { + t.Parallel() + + client := newTestPgClient(t) + ctx := context.Background() + fx := seedWorkerFixture(t, ctx, client) + + maxAge := 365 * 24 * 3600 + + existingGlob := newGlobInCategory( + fx, + "ph_phc_*_posthog", + fx.normalCategoryID, + coredata.CookieSourcePreExisting, + &maxAge, + ) + uncategorisedExact := newExactPattern( + fx, + "ph_phc_XBwJ2pHAf0MoYgh3TNZK32Qk7zLlTldhk4p9llGtZMN_posthog", + fx.uncategorisedID, + coredata.CookieSourceScript, + &maxAge, + ) + + require.NoError(t, client.WithTx(ctx, func(ctx context.Context, tx pg.Tx) error { + if err := existingGlob.Insert(ctx, tx, fx.scope); err != nil { + return err + } + + return uncategorisedExact.Insert(ctx, tx, fx.scope) + })) + + h := newTestHandler(client) + require.NoError(t, h.Process(ctx, fx.banner)) + + loaded := &coredata.TrackerPattern{} + + require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { + return loaded.LoadByBannerIDTypeAndPattern( + ctx, + conn, + fx.scope, + fx.banner.ID, + coredata.TrackerTypeCookie, + "ph_phc_*_posthog", + &maxAge, + ) + })) + + require.NotNil(t, loaded.Source) + assert.Equal(t, coredata.CookieSourceScript, *loaded.Source, "adoption must promote PRE_EXISTING glob to SCRIPT when a stronger exact is absorbed") + assert.Equal(t, existingGlob.ID, loaded.ID, "the existing glob row must be reused, not replaced") + assert.Equal(t, fx.normalCategoryID, loaded.CookieCategoryID, "user-set category must not be overwritten by the worker") + + var remainingExacts coredata.TrackerPatterns + + require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { + return remainingExacts.LoadAllByCookieBannerID( + ctx, + conn, + fx.scope, + fx.banner.ID, + coredata.NewTrackerPatternFilter(new(coredata.TrackerPatternMatchTypeExact), nil, new(false)), + nil, + ) + })) + assert.Empty(t, remainingExacts, "the uncategorised exact must be adopted into the existing glob") +} + +// TestReportDetectedTrackers_PromotesSourceOnExistingGlob seeds a +// banner with a PRE_EXISTING `ph_phc_*_posthog` glob in +// uncategorised, then reports a SCRIPT-source cookie whose name +// globMatches the existing pattern (e.g. a posthog instance ID). +// FindMatchingPattern in reportDetectedTracker links the +// detected_tracker straight to the glob, so no new exact is +// created and the merge/adoption loops in +// patternAnalysisHandler.Process never see the new signal. Without +// the in-line PromoteSource call in reportDetectedTracker, only +// last_matched_at would advance — source would stay stuck at +// PRE_EXISTING despite the new SDK-observed evidence. This test +// pins the same-category promotion path; the cross-category gap +// is covered by TestPatternAnalysisWorker_AdoptionPromotesSourceCrossCategory. +func TestReportDetectedTrackers_PromotesSourceOnExistingGlob(t *testing.T) { + t.Parallel() + + client := newTestPgClient(t) + ctx := context.Background() + fx := seedWorkerFixture(t, ctx, client) + + maxAge := 365 * 24 * 3600 + + existingGlob := newGlobInCategory( + fx, + "ph_phc_*_posthog", + fx.uncategorisedID, + coredata.CookieSourcePreExisting, + &maxAge, + ) + + require.NoError(t, client.WithTx(ctx, func(ctx context.Context, tx pg.Tx) error { + return existingGlob.Insert(ctx, tx, fx.scope) + })) + + svc := NewService(client, false) + + require.NoError(t, svc.ReportDetectedTrackers(ctx, fx.banner.ID, ReportDetectedTrackersRequest{ + Cookies: []DetectedCookie{ + { + Name: "ph_phc_XBwJ2pHAf0MoYgh3TNZK32Qk7zLlTldhk4p9llGtZMN_posthog", + MaxAgeSeconds: &maxAge, + Source: coredata.CookieSourceScript, + }, + }, + })) + + loaded := &coredata.TrackerPattern{} + + require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { + return loaded.LoadByID(ctx, conn, fx.scope, existingGlob.ID) + })) + + require.NotNil(t, loaded.Source) + assert.Equal(t, coredata.CookieSourceScript, *loaded.Source, "reportDetectedTracker must promote a PRE_EXISTING glob to SCRIPT when a stronger detection matches it") + assert.NotNil(t, loaded.LastMatchedAt, "matched detections must bump last_matched_at") + assert.Equal(t, fx.uncategorisedID, loaded.CookieCategoryID, "category must not move") + + var exacts coredata.TrackerPatterns + + require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { + return exacts.LoadAllByCookieBannerID( + ctx, + conn, + fx.scope, + fx.banner.ID, + coredata.NewTrackerPatternFilter(new(coredata.TrackerPatternMatchTypeExact), nil, new(false)), + nil, + ) + })) + assert.Empty(t, exacts, "the detected cookie globMatches the existing glob; no exact pattern must be created") +} + // TestPatternAnalysisWorker_MergeWithoutAdoptionSkipsDraftVersion // asserts the inverse: when the worker only consolidates exacts into // a glob in their own category (no consent transition), no draft diff --git a/pkg/cookiebanner/service.go b/pkg/cookiebanner/service.go index 0aa6d5164..badcadaa8 100644 --- a/pkg/cookiebanner/service.go +++ b/pkg/cookiebanner/service.go @@ -2207,6 +2207,26 @@ func (s *Service) reportDetectedTracker( if err == nil { patternID = &matchedPattern.ID *matchedPatternIDs = append(*matchedPatternIDs, matchedPattern.ID) + + // A glob (or exact) pattern already covers this + // identifier, so no new exact pattern will be created + // and the merge/adoption loops in + // patternAnalysisHandler.Process will never see this + // detection. Promote the matched pattern's source here + // if the incoming detection carries a stronger signal, + // otherwise a pattern that started life as PRE_EXISTING + // (or EXTENSION) never advances even when subsequent + // SCRIPT-source detections confirm it as a real page + // tracker — last_matched_at would move forward but + // source would stay stale. shouldPromoteSource is a + // no-op when info.Source is nil or weaker, so storage + // items without a source and weaker re-detections cost + // nothing. + if shouldPromoteSource(matchedPattern.Source, info.Source) { + if err := matchedPattern.PromoteSource(ctx, tx, scope, *info.Source, now); err != nil { + return fmt.Errorf("cannot promote source on matched tracker pattern %q: %w", matchedPattern.Pattern, err) + } + } } else { newPattern := &coredata.TrackerPattern{ ID: gid.New(scope.GetTenantID(), coredata.TrackerPatternEntityType), diff --git a/pkg/coredata/tracker_pattern_promote_test.go b/pkg/coredata/tracker_pattern_promote_test.go index 711083809..152aae604 100644 --- a/pkg/coredata/tracker_pattern_promote_test.go +++ b/pkg/coredata/tracker_pattern_promote_test.go @@ -190,6 +190,7 @@ func TestTrackerPattern_PromoteSource_OverwritesSource(t *testing.T) { assert.True(t, tp.UpdatedAt.Equal(bumpedAt), "receiver must reflect the new updated_at") loaded := &coredata.TrackerPattern{} + require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { return loaded.LoadByID(ctx, conn, fx.scope, tp.ID) })) @@ -229,6 +230,7 @@ func TestTrackerPattern_PromoteSource_OnlyTouchesSourceAndUpdatedAt(t *testing.T })) loaded := &coredata.TrackerPattern{} + require.NoError(t, client.WithConn(ctx, func(ctx context.Context, conn pg.Querier) error { return loaded.LoadByID(ctx, conn, fx.scope, tp.ID) }))