Weekly defect review: case-insensitive QR slugs, concurrent-delete races, SDK JSON-parse bug

[DennisAlund]

Weekly defect-hunting review across the app, API, and all three SDKs. Five fixes, each with a regression test that fails before and passes after.

Fixes

1. QR slug lookup was case-sensitive (src/api/qr.ts, src/mcp/server.ts)

• Wrong: slugs are always stored lowercase (link-management.ts normalizes on write), and every other lookup path lowercases before comparing, but handleLinkQr and the get_link_qr MCP tool compared the raw ?slug=/slug argument with ===.
• Impact: a caller passing an uppercase/mixed-case slug value (which passes the route's own permissive [a-zA-Z0-9_-]+ schema) got a spurious 404 for a slug that demonstrably exists on the link.
• Fix: lowercase the requested slug before comparing, matching every other lookup site.
• Test: api-links.test.ts and mcp.test.ts each add a case-mismatch regression test.

2. setSlugPrimary returned 200/null instead of 404 on a concurrent link delete (src/services/link-management.ts)

• Wrong: SlugRepository.setPrimary no-ops silently if the link/slug is gone; the service then force-unwrapped the trailing getById re-read with !.
• Impact: a delete landing between the membership check and the final read produced an HTTP 200 with a null body instead of a 404. Same TOCTOU class already fixed for bundle update/archive/unarchive/delete in a prior commit (551d205); this path was missed.
• Fix: check the re-read for null and return fail(404, "Link not found").
• Test: link-service.test.ts, vi.spyOn(LinkRepository, "getById") simulates the race.

3. getBundleAnalytics returned 200/null instead of 404 on a concurrent bundle delete (src/services/bundle-management.ts)

• Wrong: the service's own getById check passes, then ClickRepository.getBundleStats runs its own independent getById internally and can legitimately return null if the bundle is deleted in between; the result was force-unwrapped with !.
• Impact: HTTP 200 with null body instead of 404, on GET /bundles/:id/analytics and the equivalent MCP tool.
• Fix: check for null and return fail(404, "Bundle not found").
• Test: bundle-service.test.ts, mocks ClickRepository.getBundleStats to simulate the race.

4/5. TS and Dart SDKs threw a raw parse exception instead of ShrtnrError on a non-JSON 2xx body

• Wrong: both HttpClient.request (TS, sdk/typescript/src/internal/http.ts) and requestJson (Dart, sdk/dart/lib/src/base_client.dart) wrap res.json()/jsonDecode in try/catch on the error path but called it bare on the success path.
• Impact: an HTML error page or truncated body from an upstream proxy returning a 200 status threw a raw SyntaxError/FormatException, breaking the documented catch (err) { if (err instanceof ShrtnrError) } contract. Same bug class already fixed in the Python SDK (537a790); TS and Dart had it too.
• Fix: wrap in try/catch, raise ShrtnrError(status, "Invalid JSON response: ..."), mirroring the Python fix exactly.
• Test: client.test.ts (TS) and client_test.dart (Dart) each mock a text/html 2xx body.
• Note: Dart isn't installed in this execution environment, so sdk/dart/test/client_test.dart could not be run locally. The fix was traced manually against jsonDecode's documented FormatException throw, and the test mirrors the exact fixture pattern of the adjacent "throws ShrtnrError on 4xx" test in the same group.

Test suite

• Root: 72 test files / 1051 tests passed.
• TS SDK: 68 tests passed.
• Python SDK: unchanged, not re-run.
• Dart SDK: unchanged aside from the fix above; could not execute (no Dart runtime in this environment).

Flagged for developer judgment (not fixed, no code changes)

• Dart SDK README drift (sdk/dart/README.md): quickstart examples pass range: '7d'/accent: 'green' as raw strings where the real methods require the TimelineRange/BundleAccent enums, so copy-pasted examples fail to compile. The update() examples document an update(id, {field: value}) shape but the real signature is update(Link)/update(Bundle) (whole-object). Exported type names DateClickCount/SlugClickCount don't match the real DateCount/SlugCount. All four are docs-only; no regression test is possible for markdown examples, so left for a developer to fix the README text.
• Python SDK async test-coverage gap: _base.py's parse_json_response fix (537a790) is exercised only by sync tests in test_client.py; test_async_client.py has no equivalent non-JSON-2xx or qr(size=...) test, even though both paths share the same underlying code. Not a live bug today, but a future divergence of the async path would go uncaught.
• Dependency drift: typescript is one major behind (6.x → 7.0.2) in both the root app and TS SDK; @cloudflare/workers-types is one major behind (4.x → 5.x). yarn audit reports 19 (root) / 7 (TS SDK) vulnerabilities, all in transitive dev/test tooling (vite/esbuild via vitest) or bundled into @modelcontextprotocol/sdk's Express transport (fast-uri, ip-address, qs) — none critical, none with a drop-in safe patch identified. Worth a planned major-version bump pass, not an urgent patch.

🤖 Generated with Claude Code

---

Generated by Claude Code

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Updated (UTC)
✅ Deployment successful! View logs	shrtnr	8d6860e	Jul 10 2026, 07:25 PM

[Copilot]

Pull request overview

This PR addresses a set of small but user-visible defects across the REST API, MCP server, and TypeScript/Dart SDKs—primarily around slug case handling, concurrent-delete race behavior returning 200/null, and consistent SDK error wrapping when a 2xx response body is not valid JSON.

Changes:

• Make QR slug selection case-insensitive in both the REST QR handler and the MCP get_link_qr tool.
• Fix concurrent-delete race outcomes by returning 404 (instead of 200 with a null payload) in setSlugPrimary and getBundleAnalytics.
• Wrap JSON parsing failures on 2xx responses in SDK-specific ShrtnrError (TypeScript + Dart) with regression tests.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
src/services/link-management.ts	Returns a proper 404 when a link disappears before the post-update re-read in setSlugPrimary.
src/services/bundle-management.ts	Returns a proper 404 when analytics stats resolve to null due to a concurrent bundle deletion.
src/mcp/server.ts	Lowercases the requested slug when resolving the target slug for get_link_qr.
src/api/qr.ts	Lowercases the ?slug= query parameter before matching against stored slugs.
src/tests/service/link-service.test.ts	Adds regression coverage for setSlugPrimary concurrent-delete returning 404.
src/tests/service/bundle-service.test.ts	Adds regression coverage for getBundleAnalytics returning 404 when stats are null.
src/tests/handler/mcp.test.ts	Adds MCP regression test proving get_link_qr slug matching is case-insensitive.
src/tests/handler/api-links.test.ts	Adds REST regression test proving GET .../qr?slug= matching is case-insensitive.
sdk/typescript/tests/client.test.ts	Adds test ensuring non-JSON 2xx bodies throw ShrtnrError rather than raw SyntaxError.
sdk/typescript/src/internal/http.ts	Wraps res.json() on success path with try/catch and throws ShrtnrError on parse failure.
sdk/dart/test/client_test.dart	Adds test ensuring non-JSON 2xx bodies throw ShrtnrError rather than raw FormatException.
sdk/dart/lib/src/base_client.dart	Wraps jsonDecode on success path with try/catch and throws ShrtnrError on parse failure.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.