fix(llma): redirect skill lookups by UUID to name-based URL

[pauldambra]

Problem

When sharing a link to a skill from the API response (which contains the skill's UUID id field), the URL like /api/.../llma/skills/name/019e35ad-8c0c-752d-9260-1e7a5c303248 returns 404. The canonical lookup key is the skill's slug name, but callers naturally reach for the ID when constructing links.

Changes

• get_by_name now detects when the skill_name path segment is a UUID (via _is_uuid())
• Name lookup runs first — if a skill with that exact name exists (including UUID-shaped slugs), it is returned normally. UUID-by-ID redirect only fires when the name lookup returns no result, preventing ambiguity when a skill is deliberately given a UUID-shaped name.
• If no name match and the segment is a UUID, looks up by primary key and returns a 302 (temporary redirect) with Location pointing to the name-based URL. 302 is used rather than 301 because permanent redirects are cached by browsers indefinitely; if a skill is renamed or deleted after a UUID link is followed, a cached 301 would silently misdirect clients forever.
• Query params (e.g. ?version=) are preserved on the redirect via request.get_full_path().
• If the UUID doesn't match any skill, returns the normal 404.
• 4 parameterized tests covering: UUID redirect, UUID with query string, unknown UUID → 404, UUID-shaped slug name → 200 (no redirect).

How did you test this code?

I'm an agent. Parameterized tests added cover the key paths. Manual verification was not performed.

Publish to changelog?

no

🤖 Agent context

Authored by PostHog Code (Claude Opus 4.7). The user noticed that skill links generated from the MCP's llma-skill-get response (which returns the UUID id field) returned 404 when used as a URL.

Initial implementation used HTTP_301_MOVED_PERMANENTLY. QA swarm (paul-reviewer, xp-reviewer, qa-team) independently flagged this: 301 caches permanently in browsers — if a skill is renamed or deleted, cached redirects would silently misdirect clients. Switched to HTTP_302_FOUND.

QA swarm also found: UUID-shaped skill names could be shadowed (fixed by running name lookup first), str.replace replacing all occurrences (fixed with count=1 via get_full_path()), and missing tests (added 4 parameterized cases).

---

Created with PostHog Code

[pauldambra]

• fix(llma): redirect skill lookups by UUID to name-based URL #58697 👈 (View in Graphite)
• master

This stack of pull requests is managed by Graphite. Learn more about stacking.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1c878fef7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[stamphog]

The author is not on the owning team (@PostHog/team-llm-analytics) and this is a behavioral change to an API endpoint — adding a 301 redirect path to the get_by_name action. Per ownership rules, behavioral API contract changes from non-team members require a human review from the owning team.

[greptile-apps]

Prompt To Fix All With AI

Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
products/llm_analytics/backend/api/skills.py:317
**Unsafe path reconstruction via `str.replace`**

`str.replace(skill_name, skill.name)` replaces ALL occurrences of the UUID string in the path without boundary checking. If the path ever contains the UUID more than once, every occurrence is replaced and the resulting location is wrong. Additionally, `skill.name` is substituted raw into the path before `build_absolute_uri`/`iri_to_uri` runs. `iri_to_uri` leaves `?`, `#`, `&`, and `=` unencoded (they're in its safe set), so a skill name containing any of those characters would corrupt the URL's query-string or fragment boundary. Use `urllib.parse.quote(skill.name, safe="")` when inserting the name into the path, and anchor the replacement to the specific URL segment, e.g. `request.path.replace(f"/name/{skill_name}", f"/name/{urllib.parse.quote(skill.name, safe='')}", 1)`.

### Issue 2 of 3
products/llm_analytics/backend/api/skills.py:313-323
**Redirect drops version context for non-latest skill versions**

`LLMSkill.id` is the primary key of a specific version row — the API exposes it per-version, not per-skill. The redirect uses `skill.name` without appending a `?version=` parameter, so the redirected URL resolves to the _latest_ version via `get_skill_by_name_from_db`. If the UUID the caller provided belongs to a non-latest version, the redirect silently serves a different version than requested. Consider appending `?version_id={skill.id}` (or `?version={skill.version}`) to preserve the exact version the UUID identified, rather than silently upgrading to latest.

### Issue 3 of 3
products/llm_analytics/backend/api/skills.py:321
**301 (permanent) redirect will be cached indefinitely by clients**

HTTP 301 is aggressively cached by browsers, HTTP libraries, and reverse proxies. If a skill is ever renamed, any client that cached the UUID → old-name 301 will continue sending requests to the stale name URL, producing 404s until the cache entry expires or is cleared. A `302 Found` (or `307 Temporary Redirect` to preserve the HTTP method) is safer here.

```suggestion
            response = Response(status=status.HTTP_302_FOUND)
```

_{Reviews (1): Last reviewed commit: "refactor(llma): use DRF Response for the..." | Re-trigger Greptile}

[pauldambra]

Note

🤖 Automated comment by QA Swarm — not written by a human

Multi-perspective review: qa-team (specialists + generalists), paul-reviewer, xp-reviewer

Updated: All findings addressed in b7702a3.

Verdict: ✅ APPROVE

All HIGH findings were fixed before merge. The PR is clean.

Findings & resolution

Severity	Finding	Status
🟠 HIGH · convergent (all 3)	HTTP_301_MOVED_PERMANENTLY caches permanently — browsers would never re-issue the URL after a skill rename/delete	✅ Fixed → HTTP_302_FOUND in b7702a3
🟠 HIGH · convergent (qa-team + xp)	No tests for UUID redirect, unknown UUID → 404, or query-string preservation	✅ Fixed → 4 parameterized tests added in b7702a3
🟠 HIGH · qa-team	UUID-shaped skill names shadowed: _is_uuid() would intercept requests for a skill legitimately named a1b2c3d4-... and redirect to a different skill	✅ Fixed → name lookup first, UUID fallback only on not-found in b7702a3
🟡 MEDIUM · convergent (all 3)	str.replace() replaces all occurrences; request.path loses the query string	✅ Fixed → request.get_full_path().replace(..., 1) in b7702a3

Reviewer summaries

Reviewer	Assessment
🔍 qa-team	Correct approach; all raised issues (301, tests, UUID-name ambiguity, str.replace) addressed in follow-up commit.
👤 paul	Good compact change; 301 vs 302, str.replace fragility, and comment style all resolved.
📐 xp	Functionally sound; missing tests and 301 were the key issues, both fixed.

---

Automated by QA Swarm — not a human review

[greptile-apps]

Security Review

• Potential host-header open redirect (skills.py, get_by_name): request.build_absolute_uri() embeds the request's Host header into the Location response header. If a crafted request with a spoofed Host value were to reach this view (e.g., if proxy configuration doesn't enforce ALLOWED_HOSTS), the redirect could be aimed at an attacker-controlled domain. Using the relative path returned by get_full_path() eliminates this vector entirely.

Prompt To Fix All With AI

Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 1
products/llm_analytics/backend/api/skills.py:323-327
`build_absolute_uri` builds the Location header using `request.META['HTTP_HOST']`, which is caller-controlled. While Django's `ALLOWED_HOSTS` guard normally rejects spoofed Host headers before requests reach a view, using a relative URL here removes the dependency entirely: it's simpler, avoids the theoretical open-redirect vector, and any HTTP client will follow a relative Location just as well as an absolute one.

```suggestion
                response = Response(status=status.HTTP_302_FOUND)
                response["Location"] = request.get_full_path().replace(skill_name, skill_by_id.name, 1)
```

_{Reviews (2): Last reviewed commit: "fix(llma): address qa-swarm review on sk..." | Re-trigger Greptile}

[greptile-apps]

_{Reviews (3): Last reviewed commit: "fix(llma): use relative path in UUID red..." | Re-trigger Greptile}

[stamphog]

The implementation is technically correct and all bot-raised inline concerns are resolved, but the author is not on the owning team (@PostHog/team-llm-analytics) and this is a behavioral change to an API endpoint (adding a 302 redirect path to get_by_name). That combination requires a human review from the owning team before auto-approval.