Skip to content

Commit 875be34

Browse files
anandgupta42claude
anandgupta42
and
claude
committed
fix: [#838] address coderabbit nit — Matomo test name + pin "exact-name, not prefix" contract
Reviewer flagged that the test title "strips Piwik/Matomo tracking params (pk_*, piwik_*)" overstates the contract — TRACKING_PARAMS does exact-name matching, NOT pk_* / piwik_* prefix stripping. The title was documenting behavior that isn't guaranteed. Renamed to enumerate the named params we DO strip and added two counter-tests verifying that pk_unknown / piwik_unknown ARE preserved. This pins the "exact-name, not prefix" contract so a future refactor can't silently flip to prefix-stripping without breaking these. Restraint rationale documented inline: prefix-stripping `pk_` risks false positives on functional params from unrelated services that happen to start with the same two letters. 29/29 tests pass; marker check clean. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
1 parent c37af0e commit 875be34

1 file changed

Lines changed: 16 additions & 1 deletion

File tree

‎packages/opencode/test/tool/webfetch-cache.test.ts‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,13 @@ describe("normalizeUrlForCache", () => {
191191
).toBe("https://example.com/page")
192192
})
193193

194-
test("strips Piwik/Matomo tracking params (pk_*, piwik_*)", () => {
194+
test("strips enumerated Piwik/Matomo tracking params (pk_campaign, pk_kwd, pk_source, pk_medium, pk_content, piwik_campaign, piwik_keyword)", () => {
195+
// NOTE: this is an exact-name match against TRACKING_PARAMS, NOT a
196+
// pk_* / piwik_* prefix match. New Matomo params (e.g. a hypothetical
197+
// `pk_term`) would NOT be stripped until added explicitly. The
198+
// restraint is deliberate — prefix-stripping `pk_` would risk false
199+
// positives on functional params from unrelated services that happen
200+
// to start with the same two letters.
195201
expect(
196202
normalizeUrlForCache(
197203
"https://example.com/page?pk_campaign=spring&pk_kwd=hello&pk_source=email&pk_medium=newsletter&pk_content=banner",
@@ -202,6 +208,15 @@ describe("normalizeUrlForCache", () => {
202208
"https://example.com/page?piwik_campaign=summer&piwik_keyword=ai",
203209
),
204210
).toBe("https://example.com/page")
211+
// Non-enumerated pk_* / piwik_* are preserved — pins the "exact-name,
212+
// not prefix" contract so a future refactor can't silently flip to
213+
// prefix-stripping.
214+
expect(normalizeUrlForCache("https://example.com/page?pk_unknown=x")).toBe(
215+
"https://example.com/page?pk_unknown=x",
216+
)
217+
expect(normalizeUrlForCache("https://example.com/page?piwik_unknown=x")).toBe(
218+
"https://example.com/page?piwik_unknown=x",
219+
)
205220
})
206221

207222
test("real-world: HubSpot/Marketo email link variations collapse to one cache key", () => {

0 commit comments

Comments
 (0)