fix(ci): make the release guard reject only genuinely untagged pull requests - #105
Conversation
… retention services - Added TrackingModule to encapsulate tracking functionality. - Implemented TrackingIngestService for handling incoming tracking events. - Created TrackingFilingService to manage form submissions and contact filing. - Introduced TrackingRollupService for daily aggregation of tracking data. - Developed TrackingConfigService for managing tracking configurations. - Added TrackingCounterService to handle rate limiting for submissions. - Implemented TrackingController and TrackingRetentionController for API endpoints. - Created necessary contracts and router for tracking operations. - Updated .env.example and AGENTS.md to reflect new tracking features.
…ts for better modularity - Removed TrackingSections component and integrated its functionality directly into the Tracking page. - Added individual components: TrackingScript, VerifyInstallation, TrafficSources, TrackingRules, AllowedDomains, and TrackingCookies. - Updated data fetching logic to handle settings readiness before rendering components.
There was a problem hiding this comment.
4 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/tracking/tracking.controller.ts">
<violation number="1" location="apps/api/src/tracking/tracking.controller.ts:153">
P2: Retention stops after 500,000 rows per nightly run, so a backlog or sustained allowed traffic leaves events older than 90 days indefinitely. Continue deletion asynchronously/add runs until caught up, or explicitly surface and schedule continuation for an incomplete sweep.</violation>
</file>
<file name="AGENTS.md">
<violation number="1" location="AGENTS.md:15">
P3: The new rows points contributors to docs/tracking.md, but that file does not exist in the repo (verified across the whole tree); every other doc referenced in this table exists. Since AGENTS.md's first rule is 'Read the doc for the area you are touching before you touch it', this row sends developers to a missing file and the tracking doc is never actually enforced. Add docs/tracking.md in this PR or drop the row until it exists.</violation>
</file>
<file name="apps/api/src/tracking/tracking-rollup.service.ts">
<violation number="1" location="apps/api/src/tracking/tracking-rollup.service.ts:22">
P1: Daily rows undercount the retention-boundary day because consecutive runs split one calendar day and retain only the larger partial aggregate. Roll and delete whole completed days (or otherwise merge non-overlapping partial slices) so both portions are preserved.</violation>
</file>
<file name="apps/app/app/(app)/[slug]/settings/tracking/traffic-sources.tsx">
<violation number="1" location="apps/app/app/(app)/[slug]/settings/tracking/traffic-sources.tsx:30">
P1: Non-managers visiting a ready tracking settings page receive a forbidden `sources` prefetch and the route render fails; render/prefetch TrafficSources only when `tracking.data.canManage` is true (or make the query available to authorized page viewers).</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…ate UI components - Introduced a new `normalizePath` function to standardize path inputs in the TrackingIngestService. - Updated path handling in event processing to utilize `normalizePath`. - Enhanced the TrafficSources component description for clarity on visitor attribution. - Refactored the WebsiteActivity component to improve layout and detail presentation, including the addition of new properties for better tracking insights. - Simplified the CompanyOverview component by removing unnecessary props and streamlining its structure.
There was a problem hiding this comment.
1 issue found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/db/src/tracking.ts">
<violation number="1" location="packages/db/src/tracking.ts:161">
P3: The `?? "/"` fallback after `raw.split(/[?#]/)[0]` is dead code: `String.prototype.split` always returns an array with at least one element (the original string when there is no match), so `[0]` is always a defined string and the fallback is unreachable. This is harmless but misleading — a reader may assume the fallback guards an empty result. Simplify to `const path = raw.split(/[?#]/)[0];` to keep the intent clear. (The rest of the normalization — query/fragment stripping, leading-slash check, and trailing-slash collapse — correctly implements the one-page-is-one-row goal.)</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
| const raw = input?.trim(); | ||
| if (!raw) return "/"; | ||
|
|
||
| const path = raw.split(/[?#]/)[0] ?? "/"; |
There was a problem hiding this comment.
P3: The ?? "/" fallback after raw.split(/[?#]/)[0] is dead code: String.prototype.split always returns an array with at least one element (the original string when there is no match), so [0] is always a defined string and the fallback is unreachable. This is harmless but misleading — a reader may assume the fallback guards an empty result. Simplify to const path = raw.split(/[?#]/)[0]; to keep the intent clear. (The rest of the normalization — query/fragment stripping, leading-slash check, and trailing-slash collapse — correctly implements the one-page-is-one-row goal.)
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/tracking.ts, line 161:
<comment>The `?? "/"` fallback after `raw.split(/[?#]/)[0]` is dead code: `String.prototype.split` always returns an array with at least one element (the original string when there is no match), so `[0]` is always a defined string and the fallback is unreachable. This is harmless but misleading — a reader may assume the fallback guards an empty result. Simplify to `const path = raw.split(/[?#]/)[0];` to keep the intent clear. (The rest of the normalization — query/fragment stripping, leading-slash check, and trailing-slash collapse — correctly implements the one-page-is-one-row goal.)</comment>
<file context>
@@ -154,6 +154,18 @@ export function normalizeHost(input: string | null | undefined): string | null {
+ const raw = input?.trim();
+ if (!raw) return "/";
+
+ const path = raw.split(/[?#]/)[0] ?? "/";
+ if (!path.startsWith("/")) return "/";
+
</file context>
… session management code - Removed the `shouldDehydrateQuery` option from the query client configuration for cleaner setup. - Deleted the `mksession.ts` file, which contained session management logic that is no longer needed.
- Updated RollupService to track contacts created and cap reasons for submissions. - Refactored TrackingConfigService to manage cache generation for configuration. - Enhanced TrackingCounterService to allow variable increment amounts for event limits. - Improved TrackingFilingService to handle race conditions when creating contacts. - Updated TrackingIngestService to streamline event processing and rate limiting. - Added integration tests for tracking ingestion and filing functionalities. - Enhanced documentation for tracking features and their configurations.
- Updated TrackingConfigService to manage cache generation and prevent stale reads. - Improved TrackingFilingService to handle race conditions when filing submissions. - Enhanced TrackingIngestService to filter events based on type and limit host length. - Refactored TrackingService to optimize database queries for tracked visitors. - Introduced TrackingSections component to modularize tracking settings UI. - Updated documentation to clarify referrer matching rules and cache invalidation behavior. - Added integration tests for filing and ingesting submissions to ensure correct behavior.
- Eliminated unnecessary check for filedAt in the batch delivery test to streamline the integration test logic. - Ensured that the test still verifies the correct behavior of submissions being filed only once.
There was a problem hiding this comment.
3 issues found across 26 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="apps/api/src/tracking/tracking-config.service.ts">
<violation number="1" location="apps/api/src/tracking/tracking-config.service.ts:27">
P1: Tracking changes can be undone in the shared cache when API replicas race: this process-local generation does not protect Redis writes from another replica. Use a shared/atomic config version (or distributed lock/CAS) before accepting a cache write, so paused, rotated, or domain-restricted config cannot be replaced with stale data for the TTL.</violation>
</file>
<file name="packages/db/src/attribution.ts">
<violation number="1" location="packages/db/src/attribution.ts:222">
P2: Referrers such as `google.com.example` are classified as trusted Google organic traffic because any `com.<label>` tail is accepted as a suffix. Use a Public Suffix List-based registrable-domain check (or a verified suffix mapping) so lookalike domains remain referrals.</violation>
</file>
<file name="apps/api/src/tracking/tracking-ingest.service.ts">
<violation number="1" location="apps/api/src/tracking/tracking-ingest.service.ts:212">
P2: Concurrent duplicate deliveries can consume multiple hourly-contact cap slots and race contact creation because both see the unfiled row and call `file()` before either `attach()` claims it. Claim the submission (or add an in-progress state) atomically before filing, while retaining a recoverable retry path for interrupted filings.</violation>
</file>
Tip: instead of fixing issues one by one fix them all with cubic
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| export class TrackingConfigService { | ||
| private readonly logger = new Logger(TrackingConfigService.name); | ||
|
|
||
| private generation = 0; |
There was a problem hiding this comment.
P1: Tracking changes can be undone in the shared cache when API replicas race: this process-local generation does not protect Redis writes from another replica. Use a shared/atomic config version (or distributed lock/CAS) before accepting a cache write, so paused, rotated, or domain-restricted config cannot be replaced with stale data for the TTL.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/tracking/tracking-config.service.ts, line 27:
<comment>Tracking changes can be undone in the shared cache when API replicas race: this process-local generation does not protect Redis writes from another replica. Use a shared/atomic config version (or distributed lock/CAS) before accepting a cache write, so paused, rotated, or domain-restricted config cannot be replaced with stale data for the TTL.</comment>
<file context>
@@ -24,6 +24,8 @@ export interface CompiledConfig {
export class TrackingConfigService {
private readonly logger = new Logger(TrackingConfigService.name);
+ private generation = 0;
+
constructor(
</file context>
| if (tail.length === 1) return true; | ||
| if (tail.length !== 2) return false; | ||
|
|
||
| return SECOND_LEVEL.has(tail[0] ?? ""); |
There was a problem hiding this comment.
P2: Referrers such as google.com.example are classified as trusted Google organic traffic because any com.<label> tail is accepted as a suffix. Use a Public Suffix List-based registrable-domain check (or a verified suffix mapping) so lookalike domains remain referrals.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/db/src/attribution.ts, line 222:
<comment>Referrers such as `google.com.example` are classified as trusted Google organic traffic because any `com.<label>` tail is accepted as a suffix. Use a Public Suffix List-based registrable-domain check (or a verified suffix mapping) so lookalike domains remain referrals.</comment>
<file context>
@@ -177,13 +190,38 @@ function mediumFrom(value: string | null | undefined): Medium {
+ if (tail.length === 1) return true;
+ if (tail.length !== 2) return false;
+
+ return SECOND_LEVEL.has(tail[0] ?? "");
+}
+
</file context>
| }); | ||
|
|
||
| if (!submission) return; | ||
| if (created.count === 0 && !unfiled(submission)) return; |
There was a problem hiding this comment.
P2: Concurrent duplicate deliveries can consume multiple hourly-contact cap slots and race contact creation because both see the unfiled row and call file() before either attach() claims it. Claim the submission (or add an in-progress state) atomically before filing, while retaining a recoverable retry path for interrupted filings.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/tracking/tracking-ingest.service.ts, line 212:
<comment>Concurrent duplicate deliveries can consume multiple hourly-contact cap slots and race contact creation because both see the unfiled row and call `file()` before either `attach()` claims it. Claim the submission (or add an in-progress state) atomically before filing, while retaining a recoverable retry path for interrupted filings.</comment>
<file context>
@@ -164,20 +197,19 @@ export class TrackingIngestService {
});
if (!submission) return;
+ if (created.count === 0 && !unfiled(submission)) return;
const outcome = await this.filing.file({
</file context>
The release guard read `gh pr list --label`, which goes through GitHub's eventually-consistent search index. release-please swaps `autorelease: pending` for `autorelease: tagged` about a second before the guard runs, so every release run flagged the pull request it had just tagged: v1.4.0, v1.5.0 and v1.5.1 were all released correctly and all three runs went red. Confirm each candidate against the REST API, which is strongly consistent, and only fail on one that is still genuinely pending. `turbo run test` ran the packages in parallel against a single database, so `apps/api`, `apps/agent`, `packages/auth` and `packages/telemetry` interleaved their fixtures and their writes to the workspace organization row, the reporting currency and the exchange-rate table. Three runs in four failed, on a different test each time. Serialise the task: six consecutive runs clean, ten seconds slower.
- Updated TrackingConfigService to ensure cache is only set when the current hash matches. - Refactored TrackingCounterService to handle zero or negative amounts gracefully and added a release method for decrementing counter values. - Improved TrackingFilingService to manage contact limits more effectively and removed redundant withinCap check. - Enhanced integration tests to verify correct behavior of counters and filing logic under various conditions.
…ndling - Updated documentation to specify that the last DNS label must be a two-letter country code for accurate referrer classification. - Enhanced explanation of the `SECOND_LEVEL` heuristic to prevent false positives in referral tracking. - Improved handling of contact creation to ensure duplicate submissions do not affect the hourly quota. - Clarified the behavior of the `attach` method to prevent concurrent delivery issues.
…entation - Enhanced documentation to explain the behavior of `CONTACTS_PER_HOUR` and the handling of duplicate submissions. - Clarified the `attach` method's row claiming process to prevent issues with concurrent deliveries. - Improved explanation of cache invalidation mechanisms, emphasizing the importance of per-process generation and shared database hash for consistency.
The titler regenerated the title on every push once it had written one, so a pull request's subject was whatever its *last* commits looked like. #105 carried the whole website-tracking feature and was retitled `fix(ci)` by its final push, squashed onto main under that subject, and released as a patch whose notes mention none of it. A generated title is now left alone unless it stops being a conventional commit or stops covering the branch, and no title — generated or typed — may release less than the commits behind it: `floor_of` takes the strongest bump on the branch and `generate` raises its proposal to meet it. A branch holding a `feat` cannot ship as a `fix`, and one holding a breaking change cannot ship without the `!`. Over-releasing is the safe direction; losing a feature out of the changelog is not.
Opened automatically when
lewis/analytics-trackingwas pushed.The title is written from the diff and rewritten as you push, because this is squashed onto
mainand the title becomes the commit subject and the changelog line. Retitle it yourself and it is yours — the automation stops touching it.Summary by cubic
Adds first‑party website tracking with form capture and source attribution, plus a settings UI and CRM views to see visitor activity. Includes CI and test stability fixes, and hardens tracking with safer config caching, counter release, and better duplicate‑submission handling.
Bug Fixes
CONTACTS_PER_HOUR.Migration
CRON_SECRETand (recommended)REDIS_URLfor shared counters and compiled config.<script src="/t/crm.js" data-site="cmp_XXXXXXXX" defer></script>/t/crm.jsand/t/[site].jsare publicly reachable.Written for commit 0c28e0a. Summary will update on new commits.