Skip to content

[superlog] Resolve websiteId in annotation tools to fix domain-name mismatch #474

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
superlog-app wants to merge 1 commit into
base: staging
Choose a base branch
from
Open

[superlog] Resolve websiteId in annotation tools to fix domain-name mismatch #474

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions packages/ai/src/ai/tools/annotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { tool } from "ai";
import dayjs from "dayjs";
import { z } from "zod";
import { callRPCProcedure, createToolLogger, getAppContext } from "./utils";
import {
callRPCProcedure,
createToolLogger,
getAppContext,
resolveToolWebsite,
} from "./utils";

const logger = createToolLogger("Annotations Tools");

Expand Down Expand Up @@ -95,10 +100,11 @@ export function createAnnotationTools() {
execute: async ({ websiteId, chartType, chartContext }, options) => {
const context = getAppContext(options);
try {
const resolved = resolveToolWebsite(context, websiteId);
const result = await callRPCProcedure(
"annotations",
"list",
{ websiteId, chartType, chartContext },
{ websiteId: resolved.websiteId, chartType, chartContext },
context
);
return {
Expand All @@ -107,7 +113,7 @@ export function createAnnotationTools() {
};
} catch (error) {
logger.error("Failed to list annotations", {
websiteId,
websiteId: websiteId,
chartType,
error,
});
Expand Down Expand Up @@ -140,6 +146,7 @@ export function createAnnotationTools() {
options
) => {
const context = getAppContext(options);
const resolved = resolveToolWebsite(context, websiteId);
try {
Comment on lines 148 to 150

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Resolution error escapes the catch handler in createAnnotationTool. resolveToolWebsite is called before the try block (unlike listAnnotationsTool where it is inside), so if resolution throws (e.g. domain not in workspace), the error propagates without being logged via logger.error. Moving the call into the try block would make error-handling consistent with listAnnotationsTool.

if (!confirmed) {
const dateRangePreview = `${chartContext.dateRange.start_date} to ${chartContext.dateRange.end_date} (${chartContext.dateRange.granularity})`;
Expand All @@ -149,7 +156,7 @@ export function createAnnotationTools() {
message:
"Please review the annotation details below and confirm if you want to create it:",
annotation: {
websiteId,
websiteId: resolved.websiteId,
chartType,
dateRange: dateRangePreview,
annotationType,
Expand All @@ -170,7 +177,7 @@ export function createAnnotationTools() {
"annotations",
"create",
{
websiteId,
websiteId: resolved.websiteId,
chartType,
chartContext,
annotationType,
Expand Down
42 changes: 42 additions & 0 deletions packages/ai/src/ai/tools/utils/context.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,46 @@ describe("resolveToolWebsite", () => {

expect(() => resolveToolWebsite(ctx)).toThrow(/multiple websites/);
});

it("resolves a domain name matching ctx.websiteDomain to the context websiteId", () => {
// Insights agent passes domain names (e.g. "databuddy.cc") because LLM confuses
// domain with websiteId after seeing domain-keyed SQL queries.
const ctx = makeCtx({
websiteId: "internal-id-xyz",
websiteDomain: "databuddy.cc",
});

expect(resolveToolWebsite(ctx, "databuddy.cc")).toEqual({
websiteId: "internal-id-xyz",
domain: "databuddy.cc",
});
});

it("resolves a domain name matching an accessible website's domain", () => {
const ctx = makeCtx({
accessibleWebsites: [
{ id: "web_a", domain: "a.com", name: null, isPublic: null, createdAt: null },
{ id: "web_b", domain: "b.com", name: null, isPublic: null, createdAt: null },
],
});

expect(resolveToolWebsite(ctx, "b.com")).toEqual({
websiteId: "web_b",
domain: "b.com",
});
});

it("still rejects a domain that is not ctx.websiteDomain and not in accessible list", () => {
const ctx = makeCtx({
websiteId: "internal-id-xyz",
websiteDomain: "databuddy.cc",
accessibleWebsites: [
{ id: "web_a", domain: "a.com", name: null, isPublic: null, createdAt: null },
],
});

expect(() => resolveToolWebsite(ctx, "unknown.com")).toThrow(
/not in this workspace/
);
});
});
26 changes: 20 additions & 6 deletions packages/ai/src/ai/tools/utils/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,29 @@ export function resolveToolWebsite(
(id === ctx.websiteId ? ctx.websiteDomain : undefined);

if (inputWebsiteId) {
const isAccessible =
// Try exact ID match
const isAccessibleById =
accessible.some((w) => w.id === inputWebsiteId) ||
inputWebsiteId === ctx.websiteId;
if (!isAccessible) {
throw new Error(
`Website "${inputWebsiteId}" is not in this workspace. Call list_websites to see available websites.`
);
if (isAccessibleById) {
return { websiteId: inputWebsiteId, domain: domainFor(inputWebsiteId) };
}
return { websiteId: inputWebsiteId, domain: domainFor(inputWebsiteId) };

// Try domain name match (model may pass a domain instead of an ID)
const byDomain = accessible.find((w) => w.domain === inputWebsiteId);
if (byDomain) {
return { websiteId: byDomain.id, domain: byDomain.domain ?? undefined };
}
if (ctx.websiteDomain === inputWebsiteId) {
const fallbackId = ctx.defaultWebsiteId ?? ctx.websiteId;
if (fallbackId) {
return { websiteId: fallbackId, domain: ctx.websiteDomain };
}
}
Comment on lines +43 to +48

@greptile-apps greptile-apps Bot Jun 12, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Wrong ID priority when resolving by ctx.websiteDomain. ctx.websiteDomain is specifically the domain of ctx.websiteId, not of ctx.defaultWebsiteId. If a user's current context is website A (domain "databuddy.cc") but their default website is website B, passing "databuddy.cc" will incorrectly resolve to B while labelling it with A's domain — a silent ID/domain mismatch. Prefer ctx.websiteId over ctx.defaultWebsiteId in this branch.

Suggested change
if (ctx.websiteDomain === inputWebsiteId) {
const fallbackId = ctx.defaultWebsiteId ?? ctx.websiteId;
if (fallbackId) {
return { websiteId: fallbackId, domain: ctx.websiteDomain };
}
}
if (ctx.websiteDomain === inputWebsiteId) {
const fallbackId = ctx.websiteId ?? ctx.defaultWebsiteId;
if (fallbackId) {
return { websiteId: fallbackId, domain: ctx.websiteDomain };
}
}


throw new Error(
`Website "${inputWebsiteId}" is not in this workspace. Call list_websites to see available websites.`
);
}

const fallbackId = ctx.defaultWebsiteId ?? ctx.websiteId;
Expand Down
Loading