[superlog] Fix insights agent annotation creation: resolve domain→UUID and add manage:config scope#472
Conversation
…D and add manage:config scope
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
The latest updates on your projects. Learn more about Unkey Deploy
|
Greptile SummaryThis PR fixes two compounding failures that silently dropped every annotation created by the insights agent: the LLM passes domain names where the backend expects UUIDs, and the service auth lacked the
Confidence Score: 3/5The scope and routing fixes are safe; the domain-resolution helper has a logic gap that could silently pass raw domain strings to the RPC layer in non-insights contexts. The packages/ai/src/ai/tools/annotations.ts — the resolveWebsiteId helper has two issues: branch 2 should return Important Files Changed
Sequence DiagramsequenceDiagram
participant LLM as Insights LLM
participant Tool as create_annotation tool
participant Resolve as resolveWebsiteId()
participant RPC as annotations.create RPC
participant WW as withWorkspace()
participant Scope as requiredScopesForResource()
participant DB as Database
LLM->>Tool: "websiteId = "example.com" (domain)"
Tool->>Resolve: resolveWebsiteId(ctx, "example.com")
Resolve-->>Tool: "uuid-abc-123" (matched ctx.websiteDomain → ctx.websiteId)
Tool->>RPC: "{ websiteId: "uuid-abc-123", ... }"
RPC->>WW: "withWorkspace({ websiteId: "uuid-abc-123", permissions: ["update"], scopeResource: "organization" })"
WW->>DB: getWebsiteById("uuid-abc-123")
DB-->>WW: website row ✓
WW->>Scope: requiredScopesForResource("organization", ["update"])
Scope-->>WW: ["manage:config"]
WW-->>WW: hasKeyScope(apiKey, "manage:config") ✓
WW-->>RPC: workspace ✓
RPC->>DB: INSERT annotation
DB-->>RPC: new annotation row
RPC-->>LLM: annotation created ✓
Reviews (1): Last reviewed commit: "[superlog] Fix insights agent annotation..." | Re-trigger Greptile |
| // Input is the context domain → return the context UUID. | ||
| if (ctx.websiteDomain && inputId === ctx.websiteDomain && ctx.websiteId) { | ||
| return ctx.websiteId; |
There was a problem hiding this comment.
Domain→UUID mapping silently misses
defaultWebsiteId-only contexts
Branch 2 returns ctx.websiteId, but the codebase uses defaultWebsiteId ?? websiteId as the canonical primary identifier (see requireWebsiteId in context.ts). When ctx.websiteId is falsy but ctx.defaultWebsiteId holds the correct UUID — a valid AppContext for non-insights chat agents — the guard && ctx.websiteId causes a fallthrough to the accessibleWebsites scan. If that list is also empty (common in lightweight contexts), the raw domain string passes through to the RPC layer, reproducing the same NOT_FOUND error this PR was written to fix. The fix is one line: return ctx.websiteId ?? ctx.defaultWebsiteId instead of just ctx.websiteId.
| if (ctx.websiteDomain && inputId === ctx.websiteDomain && ctx.websiteId) { | ||
| return ctx.websiteId; | ||
| } | ||
| // Input matches an accessible website's domain → return its UUID. | ||
| const byDomain = (ctx.accessibleWebsites ?? []).find( | ||
| (w) => w.domain === inputId | ||
| ); |
There was a problem hiding this comment.
Domain comparisons use strict equality, making them case-sensitive. LLMs occasionally return domains with mixed capitalisation (e.g.
AbcEducation.edu.au vs the stored abceducation.edu.au), which would cause both branch 2 and branch 3 to miss and silently fall through to returning the raw domain string.
| if (ctx.websiteDomain && inputId === ctx.websiteDomain && ctx.websiteId) { | |
| return ctx.websiteId; | |
| } | |
| // Input matches an accessible website's domain → return its UUID. | |
| const byDomain = (ctx.accessibleWebsites ?? []).find( | |
| (w) => w.domain === inputId | |
| ); | |
| if (ctx.websiteDomain && inputId.toLowerCase() === ctx.websiteDomain.toLowerCase() && ctx.websiteId) { | |
| return ctx.websiteId; | |
| } | |
| // Input matches an accessible website's domain → return its UUID. | |
| const byDomain = (ctx.accessibleWebsites ?? []).find( | |
| (w) => w.domain?.toLowerCase() === inputId.toLowerCase() | |
| ); |
Summary
Every scheduled
insights-generate-websitejob that callscreate_annotationfails withNOT_FOUND: website not found. The insight itself is generated and delivered correctly, but the chart timeline annotation is silently dropped.The insights agent's prompt identifies websites by domain name (e.g.,
abeeducation.edu.au), so the LLM passes that domain as thewebsiteIdparameter tocreate_annotation. The backendwithWorkspaceresolves websites by UUID only —db.query.websites.findFirst({ where: { id } })— so a domain name never matches and throwsNOT_FOUND.A second latent issue would surface after fixing the first: the insights service auth had
scopes: ["read:data"], but annotationcreate/update/deletetriggereffectiveResource = "website"insidewithWorkspace(becausewebsiteIdis in the options), and the scope override mapswebsite+update → manage:websites— a scope the service auth doesn't hold. Annotations are a config-level operation, not website management.Changes:
packages/ai/src/ai/tools/annotations.ts— addresolveWebsiteId(ctx, inputId)that maps a domain name back to the UUID from the app context (ctx.websiteDomain → ctx.websiteId, also checksaccessibleWebsitesby domain). Apply it inlist_annotationsandcreate_annotation.packages/rpc/src/procedures/with-workspace.ts— addscopeResource?: stringoption; when set, it overrides the resource type used for API-key scope checks only (user role checks are unaffected), allowing annotation endpoints to requiremanage:configinstead ofmanage:websites.packages/rpc/src/routers/annotations.ts— passscopeResource: "organization"towithWorkspaceincreate,update, anddeletehandlers (organization+update → manage:config).apps/insights/src/generation.ts— extend insights service auth scopes to["read:data", "manage:config"].An alternative remediation would be to include the website UUID explicitly in the insights agent's system prompt (via
formatContextForLLM) so the LLM always has the correct identifier — this would fix Bug 1 without code changes to the annotation tool, but Bug 2 (scope) still needs a fix.Incident on Superlog
Was this PR helpful? Leave feedback — goes straight to the Superlog team.
Summary by cubic
Fixes failed insights annotations by resolving domain names to website UUIDs and using config-level scopes for annotation CRUD. Scheduled insights jobs now create timeline annotations successfully.
websiteIdfrom domain via context inlist_annotationsandcreate_annotation(packages/ai/src/ai/tools/annotations.ts).scopeResource?: stringtowithWorkspaceto override API-key scope checks (packages/rpc/src/procedures/with-workspace.ts); routers passscopeResource: "organization"for annotation create/update/delete (packages/rpc/src/routers/annotations.ts) so they requiremanage:configinstead ofmanage:websites.["read:data", "manage:config"](apps/insights/src/generation.ts).Written for commit 670f607. Summary will update on new commits.