Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
170 changes: 170 additions & 0 deletions assistant/eslint-rules/__tests__/cli-no-daemon-internals.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
import tsParser from "@typescript-eslint/parser";
import { RuleTester } from "eslint";

import rule from "../cli-no-daemon-internals.js";

const tester = new RuleTester({
languageOptions: {
ecmaVersion: 2022,
sourceType: "module",
parser: tsParser,
},
});

tester.run("cli/no-daemon-internals", rule, {
valid: [
// ipc-tagged file importing only allowed sources
{
code: `
import type { Command } from "commander";
import { cliIpcCall } from "../../ipc/cli-client.js";
import { log } from "../logger.js";
import { printTable } from "../output.js";

registerCommand(program, {
name: "example",
transport: "ipc",
build: () => {},
});
`,
},
// local-tagged file importing allowed sources
{
code: `
import type { Command } from "commander";
import { loadRawConfig } from "../../config/loader.js";
import { getWorkspaceDir } from "../../util/platform.js";

registerCommand(program, {
name: "local-example",
transport: "local",
build: () => {},
});
`,
},
// bootstrap-tagged file importing allowed sources
{
code: `
import type { Command } from "commander";
import { AssistantConfigSchema } from "../../config/schema.js";

registerCommand(program, {
name: "bootstrap-example",
transport: "bootstrap",
build: () => {},
});
`,
},
// ipc-tagged file importing from ../lib/ prefix (shared lib)
{
code: `
import type { Command } from "commander";
import { cliIpcCall } from "../../ipc/cli-client.js";
import { readFileSync } from "../lib/daemon-credential-client.js";

registerCommand(program, {
name: "lib-example",
transport: "ipc",
build: () => {},
});
`,
},
// File with zero imports and no registerCommand — utility file
{
code: `
function utilHelper() {
return 42;
}
export { utilHelper };
`,
},
],

invalid: [
// File with imports but no registerCommand call
{
code: `
import type { Command } from "commander";
import { cliIpcCall } from "../../ipc/cli-client.js";

export function registerMyCommand(program) {
// forgot to call registerCommand
}
`,
errors: [
{
messageId: "missingTransport",
},
],
},
// ipc-tagged file importing a forbidden runtime route
{
code: `
import type { Command } from "commander";
import { cliIpcCall } from "../../ipc/cli-client.js";
import { healthRoutes } from "../../runtime/routes/health-routes.js";

registerCommand(program, {
name: "bad-ipc",
transport: "ipc",
build: () => {},
});
`,
errors: [
{
messageId: "forbiddenImport",
data: {
transport: "ipc",
source: "../../runtime/routes/health-routes.js",
},
},
],
},
// ipc-tagged file importing a skills catalog module
{
code: `
import type { Command } from "commander";
import { cliIpcCall } from "../../ipc/cli-client.js";
import { SkillsCatalog } from "../../skills/catalog.js";

registerCommand(program, {
name: "bad-ipc-skills",
transport: "ipc",
build: () => {},
});
`,
errors: [
{
messageId: "forbiddenImport",
data: {
transport: "ipc",
source: "../../skills/catalog.js",
},
},
],
},
// local-tagged file importing a forbidden runtime route
{
code: `
import type { Command } from "commander";
import { loadRawConfig } from "../../config/loader.js";
import { healthRoutes } from "../../runtime/routes/health-routes.js";

registerCommand(program, {
name: "bad-local",
transport: "local",
build: () => {},
});
`,
errors: [
{
messageId: "forbiddenImport",
data: {
transport: "local",
source: "../../runtime/routes/health-routes.js",
},
},
],
},
],
});
176 changes: 176 additions & 0 deletions assistant/eslint-rules/cli-no-daemon-internals.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,176 @@
const ALLOWED_PREFIXES = {
ipc: [
"node:",
"bun:",
"commander",
"../../ipc/cli-client",
"../logger",
"../output",
"../lib/",
],
local: [
"node:",
"bun:",
"commander",
"../../config/loader",
"../../config/schema",
"../../util/platform",
"../lib/",
],
bootstrap: [
"node:",
"bun:",
"commander",
"../../config/loader",
"../../config/schema",
"../../util/platform",
"../lib/",
],
};

function findTransport(program) {
const worklist = [...program.body];
const seen = new WeakSet();

while (worklist.length > 0) {
const node = worklist.pop();

if (!node || typeof node !== "object" || seen.has(node)) {
continue;
}
seen.add(node);

if (
node.type === "CallExpression" &&
node.callee.type === "Identifier" &&
node.callee.name === "registerCommand"
) {
for (const arg of node.arguments) {
if (arg.type === "ObjectExpression") {
for (const prop of arg.properties) {
if (
prop.type === "Property" &&
prop.key.type === "Identifier" &&
prop.key.name === "transport" &&
prop.value.type === "Literal" &&
typeof prop.value.value === "string"
) {
return prop.value.value;
}
}
}
}
}

switch (node.type) {
case "ExpressionStatement":
worklist.push(node.expression);
break;
case "CallExpression":
for (const arg of node.arguments) {
worklist.push(arg);
}
break;
case "FunctionDeclaration":
case "FunctionExpression":
case "ArrowFunctionExpression":
if (node.body) worklist.push(node.body);
break;
case "BlockStatement":
for (const stmt of node.body) {
worklist.push(stmt);
}
break;
case "ReturnStatement":
if (node.argument) worklist.push(node.argument);
break;
case "ExportNamedDeclaration":
case "ExportDefaultDeclaration":
if (node.declaration) worklist.push(node.declaration);
break;
case "VariableDeclaration":
for (const decl of node.declarations) {
if (decl.init) worklist.push(decl.init);
}
break;
case "ObjectExpression":
for (const prop of node.properties) {
if (prop.type === "Property") {
worklist.push(prop.value);
}
}
break;
default:
break;
}
}

return null;
}

const rule = {
meta: {
type: "suggestion",
docs: {
description:
"Enforce import allowlists for CLI commands by transport class",
},
messages: {
missingTransport:
"CLI command file must call registerCommand({ transport: ... }) to declare its transport class.",
forbiddenImport:
"'{{transport}}'-tagged CLI command imports forbidden module '{{source}}'. See DESIGN.md §3.1 for allowed imports.",
},
schema: [],
},

create(context) {
const importNodes = [];

return {
ImportDeclaration(node) {
importNodes.push(node);
},

"Program:exit"(program) {
if (importNodes.length === 0) {
return;
}

const transport = findTransport(program);

if (transport === null) {
context.report({
node: program,
messageId: "missingTransport",
});
return;
}

const allowedPrefixes = ALLOWED_PREFIXES[transport];
if (!allowedPrefixes) {
return;
}

for (const importNode of importNodes) {
const source = importNode.source.value;
const allowed = allowedPrefixes.some((prefix) =>
source.startsWith(prefix),
);
if (!allowed) {
context.report({
node: importNode,
messageId: "forbiddenImport",
data: {
transport,
source,
},
});
}
}
},
};
},
};

export default rule;
8 changes: 8 additions & 0 deletions assistant/eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { defineConfig, globalIgnores } from "eslint/config";
import simpleImportSort from "eslint-plugin-simple-import-sort";
import tseslint from "typescript-eslint";

import cliNoDaemonInternals from "./eslint-rules/cli-no-daemon-internals.js";

const eslintConfig = defineConfig([
...tseslint.configs.recommended,
globalIgnores(["dist/**", "drizzle/**"]),
Expand Down Expand Up @@ -43,6 +45,12 @@ const eslintConfig = defineConfig([
"@typescript-eslint/no-explicit-any": "off",
},
},
{
files: ["src/cli/commands/**/*.ts"],
ignores: ["src/cli/commands/**/__tests__/**"],
plugins: { cli: { rules: { "no-daemon-internals": cliNoDaemonInternals } } },
rules: { "cli/no-daemon-internals": "warn" },
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.

🚩 Rule will fire missingTransport on ~38 existing command files

The rule fires missingTransport on any file in src/cli/commands/**/*.ts that has imports but no registerCommand() call. There are approximately 38 such files (e.g., conversations.ts, credentials.ts, all oauth/*.ts helpers, platform/*.ts). Since the rule is set to "warn" at assistant/eslint.config.mjs:52, this won't break CI, but it will produce significant lint noise. This is presumably intentional — the rule is being introduced incrementally to guide migration — but worth confirming that the volume of warnings is expected and won't cause developers to ignore lint output.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

},
]);

export default eslintConfig;
Loading