Skip to content

fix(mcp): stop classifying devices as offline in list_devices #3299

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

Merged
saddlepaddle merged 1 commit into from
Apr 9, 2026
Merged

fix(mcp): stop classifying devices as offline in list_devices #3299

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
7 changes: 2 additions & 5 deletions apps/api/MCP_TOOLS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,10 @@ const listTaskStatusesOutput = z.object({
These tools write to `agent_commands` table and poll for results.
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Apr 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

P2: listDevicesOutput schema in MCP_TOOLS.md is still out of sync with the implementation after this refresh: deviceName and ownerName should be .nullable(), and ownerEmail: z.string() is missing entirely. The implementation in list-devices.ts and the type annotation in run-agent.ts both confirm these three differences.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/MCP_TOOLS.md, line 167:

<comment>`listDevicesOutput` schema in MCP_TOOLS.md is still out of sync with the implementation after this refresh: `deviceName` and `ownerName` should be `.nullable()`, and `ownerEmail: z.string()` is missing entirely. The implementation in `list-devices.ts` and the type annotation in `run-agent.ts` both confirm these three differences.</comment>

<file context>
@@ -161,12 +161,10 @@ const listTaskStatusesOutput = z.object({
-const listDevicesInput = z.object({
-  includeOffline: z.boolean().default(false).describe("Include recently offline devices"),
-});
+const listDevicesInput = z.object({});
 
 const listDevicesOutput = z.object({
</file context>
Fix with Cubic


#### `list_devices`
List online devices in the organization.
List registered devices in the organization.

```typescript
const listDevicesInput = z.object({
includeOffline: z.boolean().default(false).describe("Include recently offline devices"),
});
const listDevicesInput = z.object({});

const listDevicesOutput = z.object({
devices: z.array(z.object({
Expand All @@ -176,7 +174,6 @@ const listDevicesOutput = z.object({
ownerId: z.string().uuid().describe("User who owns this device"),
ownerName: z.string().describe("Name of device owner"),
lastSeenAt: z.string().datetime(),
isOnline: z.boolean(),
})),
});
```
Comment on lines 168 to 179
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot Apr 9, 2026

Choose a reason for hiding this comment

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

P2 listDevicesOutput schema is out of sync with the implementation

The schema documented here no longer matches the actual Zod schema in packages/mcp/src/tools/devices/list-devices/list-devices.ts. Three differences:

  1. deviceName is z.string().nullable() in the implementation, but shown as non-nullable here.
  2. ownerName is z.string().nullable() in the implementation, but shown as non-nullable here.
  3. ownerEmail: z.string() is present in the implementation but is missing entirely from this schema block.

The PR description says this file was refreshed, but the schema block was not updated to match.

Suggested change
const listDevicesOutput = z.object({
devices: z.array(z.object({
deviceId: z.string(),
deviceName: z.string().nullable(),
deviceType: z.enum(["desktop", "mobile", "web"]),
lastSeenAt: z.string().datetime(),
ownerId: z.string(),
ownerName: z.string().nullable(),
ownerEmail: z.string(),
})),
});

Expand Down
5 changes: 2 additions & 3 deletions apps/api/src/app/api/integrations/slack/events/utils/run-agent/run-agent.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ async function fetchAgentContext({
mcpClient.callTool({ name: "list_task_statuses", arguments: {} }),
mcpClient.callTool({
name: "list_devices",
arguments: { includeOffline: true },
arguments: {},
}),
]);

Expand Down Expand Up @@ -409,13 +409,12 @@ async function fetchAgentContext({
deviceName: string | null;
ownerName: string | null;
ownerEmail: string;
isOnline: boolean;
}[];
} | null;
if (devicesData?.devices?.length) {
const lines = devicesData.devices.map(
(d) =>
`- ${d.deviceName ?? "Unknown"} (id: ${d.deviceId}, owner: ${d.ownerName ?? d.ownerEmail}, status: ${d.isOnline ? "online" : "offline"})`,
`- ${d.deviceName ?? "Unknown"} (id: ${d.deviceId}, owner: ${d.ownerName ?? d.ownerEmail})`,
);
sections.push(`Devices:\n${lines.join("\n")}`);
}
Expand Down
2 changes: 1 addition & 1 deletion apps/docs/content/docs/mcp.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ API keys grant full access to your organization. Keep them secret and never comm

| Tool | Description |
|------|-------------|
| `list_devices` | List online devices in your organization |
| `list_devices` | List registered devices in your organization |
| `list_projects` | List all projects on a device |
| `get_app_context` | Get current app state (active workspace, pathname) |
| `list_members` | List organization members |
Expand Down
7 changes: 2 additions & 5 deletions packages/cli/src/commands/devices/list/command.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import { boolean, CLIError, command, table } from "@superset/cli-framework";
import { CLIError, command, table } from "@superset/cli-framework";

export default command({
description: "List all devices in the org",
options: {
includeOffline: boolean().desc("Include offline devices"),
},
options: {},
display: (data) =>
table(data as Record<string, unknown>[], [
"deviceName",
"deviceType",
"status",
"lastSeen",
]),
run: async () => {
Expand Down
58 changes: 22 additions & 36 deletions packages/mcp/src/tools/devices/list-devices/list-devices.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const getMcpContextMock = mock(() => ({ organizationId: "org-1" }));

let fetchedDevices = [
{
deviceId: "device-online",
deviceId: "device-a",
deviceName: "Ada's MacBook",
deviceType: "desktop",
lastSeenAt: new Date(Date.now() - 30_000),
Expand All @@ -14,7 +14,7 @@ let fetchedDevices = [
ownerEmail: "ada@example.com",
},
{
deviceId: "device-offline",
deviceId: "device-b",
deviceName: "Grace's iPhone",
deviceType: "mobile",
lastSeenAt: new Date(Date.now() - 120_000),
Expand All @@ -40,7 +40,12 @@ mock.module("@superset/db/client", () => ({
},
}));

// mock.module is process-global in bun test. Preserve every real export
// (notably executeOnDevice, which start-agent-session.test.ts imports after
// this file mounts its mocks) so sibling test files don't break.
const realUtils = await import("../../utils");
mock.module("../../utils", () => ({
...realUtils,
getMcpContext: getMcpContextMock,
}));

Expand All @@ -61,7 +66,6 @@ type RegisteredToolHandler = (
ownerId: string;
ownerName: string | null;
ownerEmail: string;
isOnline: boolean;
}>;
};
}>;
Expand Down Expand Up @@ -102,7 +106,7 @@ describe("list_devices MCP tool", () => {
beforeEach(() => {
fetchedDevices = [
{
deviceId: "device-online",
deviceId: "device-a",
deviceName: "Ada's MacBook",
deviceType: "desktop",
lastSeenAt: new Date(Date.now() - 30_000),
Expand All @@ -111,7 +115,7 @@ describe("list_devices MCP tool", () => {
ownerEmail: "ada@example.com",
},
{
deviceId: "device-offline",
deviceId: "device-b",
deviceName: "Grace's iPhone",
deviceType: "mobile",
lastSeenAt: new Date(Date.now() - 120_000),
Expand All @@ -124,22 +128,16 @@ describe("list_devices MCP tool", () => {
selectMock.mockClear();
});

it("registers input and output schemas that validate includeOffline and isOnline", async () => {
it("registers an output schema that validates the device list", async () => {
const { config, handler } = createTool();
const inputSchema = z.object(config.inputSchema);
const outputSchema = z.object(config.outputSchema);

expect(inputSchema.parse({})).toEqual({ includeOffline: false });
expect(inputSchema.parse({ includeOffline: true })).toEqual({
includeOffline: true,
});

const result = await handler({ includeOffline: true }, {});
const result = await handler({}, {});

expect(() => outputSchema.parse(result.structuredContent)).not.toThrow();
});

it("returns only online devices by default", async () => {
it("returns every registered device regardless of lastSeenAt", async () => {
const { handler } = createTool();

const result = await handler({}, {});
Expand All @@ -148,35 +146,23 @@ describe("list_devices MCP tool", () => {
expect(selectMock).toHaveBeenCalledTimes(1);
expect(result.structuredContent?.devices).toEqual([
{
deviceId: "device-online",
deviceId: "device-a",
deviceName: "Ada's MacBook",
deviceType: "desktop",
lastSeenAt: expect.any(String),
ownerId: "user-1",
ownerName: "Ada",
ownerEmail: "ada@example.com",
isOnline: true,
},
{
deviceId: "device-b",
deviceName: "Grace's iPhone",
deviceType: "mobile",
lastSeenAt: expect.any(String),
ownerId: "user-2",
ownerName: "Grace",
ownerEmail: "grace@example.com",
},
]);
});

it("includes offline devices when requested and marks them offline", async () => {
const { handler } = createTool();

const result = await handler({ includeOffline: true }, {});

expect(result.structuredContent?.devices).toHaveLength(2);
expect(result.structuredContent?.devices).toEqual(
expect.arrayContaining([
expect.objectContaining({
deviceId: "device-online",
isOnline: true,
}),
expect.objectContaining({
deviceId: "device-offline",
isOnline: false,
}),
]),
);
});
});
32 changes: 7 additions & 25 deletions packages/mcp/src/tools/devices/list-devices/list-devices.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,12 @@ import { desc, eq } from "drizzle-orm";
import { z } from "zod";
import { getMcpContext } from "../../utils";

const DEVICE_ONLINE_WINDOW_MS = 60_000;

export function register(server: McpServer) {
server.registerTool(
"list_devices",
{
description:
"List devices in the organization. By default, only devices seen within the last 60 seconds are returned.",
inputSchema: {
includeOffline: z
.boolean()
.default(false)
.describe("Include devices that have not checked in recently"),
},
description: "List registered devices in the organization.",
inputSchema: {},
outputSchema: {
devices: z.array(
z.object({
Expand All @@ -29,15 +21,12 @@ export function register(server: McpServer) {
ownerId: z.string(),
ownerName: z.string().nullable(),
ownerEmail: z.string(),
isOnline: z.boolean(),
}),
),
},
},
async (args, extra) => {
async (_args, extra) => {
const ctx = getMcpContext(extra);
const includeOffline = args.includeOffline === true;
const onlineCutoff = Date.now() - DEVICE_ONLINE_WINDOW_MS;

const devices = await db
.select({
Expand All @@ -54,17 +43,10 @@ export function register(server: McpServer) {
.where(eq(devicePresence.organizationId, ctx.organizationId))
.orderBy(desc(devicePresence.lastSeenAt));

const result = devices
.map((d) => {
const isOnline = d.lastSeenAt.getTime() >= onlineCutoff;

return {
...d,
lastSeenAt: d.lastSeenAt.toISOString(),
isOnline,
};
})
.filter((device) => includeOffline || device.isOnline);
const result = devices.map((d) => ({
...d,
lastSeenAt: d.lastSeenAt.toISOString(),
}));

return {
structuredContent: { devices: result },
Expand Down
Loading