This Codebase Smells! [2026-05-17]

[github-actions[bot]]

This Codebase Smells!

This week’s aroma profile is burnt abstractions, stale config handling, and a top note of “just one more special case, bro.” 🤮 The repo is not beyond saving, but it is doing that charming thing where every new feature appears to have been installed with a shovel instead of a design. Delightful.

Table of Contents

• Corrupt Configs Are Quietly “Handled”
• Skills Propagation Is a Copy-Paste Swamp
• Agent Metadata Is Smeared Everywhere
• Nested Discovery Brute-Forces the Repo
• Test Tooling Is Babysitting Dead Artifacts

Corrupt Configs Are Quietly “Handled”

💩 Broken config files get treated like optional fan fiction, which risks silently overwriting real user state with empty defaults.

When ruler.toml, native MCP JSON, or firebender.json is malformed, the code often shrugs, returns {} or { rules: [] }, and carries on. That is spectacularly forgiving in the worst possible way: a user typo can turn into “merge with empty config and write it back,” which is how software politely vandalizes files while pretending to help.

This is a robustness problem and a trust problem. Missing files (ENOENT) are fine; invalid files are not. Right now those cases are blurred together like someone spilled error handling into the carpet and called it resilience.

Files

• src/core/ConfigLoader.ts
• src/paths/mcp.ts
• src/agents/FirebenderAgent.ts
• src/core/apply-engine.ts

Code

• src/core/ConfigLoader.ts — lines 196-220: https://github.com/intellectronica/ruler/blob/main/src/core/ConfigLoader.ts#L196-L220

  } catch (err) {
    if (err instanceof Error && (err as ErrnoException).code !== 'ENOENT') {
      ...
      console.warn(
        `[ruler] Warning: could not read config file at ${configFile}: ${err.message}`,
      );
    }
    raw = {};
  }

• src/paths/mcp.ts — lines 80-89: https://github.com/intellectronica/ruler/blob/main/src/paths/mcp.ts#L80-L89

  export async function readNativeMcp(...) {
    try {
      const text = await fs.readFile(filePath, 'utf8');
      return JSON.parse(text) as Record<string, unknown>;
    } catch {
      return {};
    }
  }

• src/agents/FirebenderAgent.ts — lines 98-118: https://github.com/intellectronica/ruler/blob/main/src/agents/FirebenderAgent.ts#L98-L118

  } catch (error: unknown) {
    ...
    console.warn(`Failed to read/parse existing firebender.json: ${error}`);
    return { rules: [] };
  }

• src/core/apply-engine.ts — lines 900-914: https://github.com/intellectronica/ruler/blob/main/src/core/apply-engine.ts#L900-L914

  let existing = await readNativeMcp(dest);
  ...
  let merged = mergeMcp(existing, mcpToMerge, strategy, serverKey);

Suggestions

• Fail hard on invalid/parsing errors; only treat ENOENT as “missing, start fresh.”
• Return structured diagnostics instead of {} fallbacks, and surface them at the CLI boundary.
• Refuse to write merged config when the existing file is unreadable unless the user explicitly opts into overwrite.
• Centralize config-read semantics so every agent doesn’t invent its own adorable little disaster.

Skills Propagation Is a Copy-Paste Swamp

🤮 SkillsProcessor.ts is a giant repetition factory where every new agent demands another sacrificial function.

This file is doing the same job over and over with slightly different constants: cleanup logic repeated per target, propagation logic repeated per target, error cleanup repeated per target. It is the software equivalent of photocopying the same bug thirteen times so nobody gets lonely.

The bad news: this is hard to maintain. The worse news: there is already a sibling subsystem (SubagentsProcessor) doing its own table-driven-ish version, so the repo contains two competing patterns for nearly the same problem. Chef’s kiss of inconsistency. 🫠

Files

• src/core/SkillsProcessor.ts
• src/core/SubagentsProcessor.ts

Code

• src/core/SkillsProcessor.ts — lines 177-206: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L177-L206

  const claudeSkillsPath = path.join(projectRoot, CLAUDE_SKILLS_PATH);
  const codexSkillsPath = path.join(projectRoot, CODEX_SKILLS_PATH);
  ...
  await fs.access(claudeSkillsPath);
  ...
  await fs.rm(claudeSkillsPath, { recursive: true, force: true });

• src/core/SkillsProcessor.ts — lines 683-716: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L683-L716

  export async function propagateSkillsForClaude(...) {
    ...
    await copySkillsDirectory(skillsDir, tempDir);
    ...
    await fs.rm(claudeSkillsPath, { recursive: true, force: true });
  }

• src/core/SubagentsProcessor.ts — lines 58-73: https://github.com/intellectronica/ruler/blob/main/src/core/SubagentsProcessor.ts#L58-L73

  const SUBAGENT_TARGET_TO_IDENTIFIERS = new Map([...]);
  const SUBAGENT_TARGET_PATHS: Record<SubagentTarget, string> = {
    claude: CLAUDE_SUBAGENTS_PATH,
    cursor: CURSOR_SUBAGENTS_PATH,
    codex: CODEX_SUBAGENTS_PATH,
    copilot: COPILOT_SUBAGENTS_PATH,
  };

Suggestions

• Replace per-agent functions with a single table-driven pipeline:
  [{ target, path, identifiers, copyMode, transform? }].
• Extract shared helpers for:
  1. existence check,
  2. temp-dir atomic swap,
  3. cleanup,
  4. dry-run message generation.
• Make skills and subagents use the same propagation architecture so fixes land once instead of in twelve miserable places.

Agent Metadata Is Smeared Everywhere

🧟 Adding one agent currently means updating a small committee of registries, switches, maps, and documentation tables like it’s 2009.

The source of truth is not a source of truth; it’s a scavenger hunt. The supported agents list lives in the README, instantiated objects live in allAgents, MCP locations live in a switch, skills routing lives in a map, and subagent routing lives in a different map. This is how drift sneaks in wearing a fake mustache.

The code works today, mostly because someone has been manually herding cats with discipline and caffeine. But every new agent increases the chance of forgetting one surface and shipping a half-supported feature with a confident README. Very enterprise. 💀

Files

• README.md
• src/agents/index.ts
• src/paths/mcp.ts
• src/core/SkillsProcessor.ts
• src/core/SubagentsProcessor.ts

Code

• README.md — lines 57-76: https://github.com/intellectronica/ruler/blob/main/README.md#L57-L76

  | Agent            | Rules File(s) | MCP Configuration / Notes | Skills ... | Subagents ... |
  | GitHub Copilot   | `AGENTS.md`   | `.vscode/mcp.json`        | ...        | ...           |
  | Claude Code      | `CLAUDE.md`   | `.mcp.json`               | ...        | ...           |

• src/agents/index.ts — lines 38-70: https://github.com/intellectronica/ruler/blob/main/src/agents/index.ts#L38-L70

  export const allAgents: IAgent[] = [
    new CopilotAgent(),
    new ClaudeAgent(),
    new CodexCliAgent(),
    ...
  ];

• src/paths/mcp.ts — lines 10-35: https://github.com/intellectronica/ruler/blob/main/src/paths/mcp.ts#L10-L35

  switch (adapterName) {
    case 'GitHub Copilot':
      candidates.push(path.join(projectRoot, '.vscode', 'mcp.json'));
      break;
    ...
    case 'Open Hands':
      candidates.push(path.join(projectRoot, 'config.toml'));
  }

• src/core/SkillsProcessor.ts — lines 140-154: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L140-L154

  const SKILL_TARGET_TO_IDENTIFIERS = new Map<SkillTarget, readonly string[]>([
    ['claude', ['claude', 'copilot', 'kilocode']],
    ['codex', ['codex']],
    ...
  ]);

Suggestions

• Create a single agent manifest describing identifier, display name, rule path(s), MCP path(s), skill target, subagent target, and capabilities.
• Generate allAgents, CLI help, MCP routing, and README tables from that manifest.
• Move per-agent quirks behind typed adapters, not scattered switch statements and side maps.
• Add a test that asserts every documented agent exists in the registry and vice versa.

Nested Discovery Brute-Forces the Repo

🐢 Nested .ruler discovery walks the whole tree and only skips hidden directories, which is a hilariously bad bargain in large repos.

The code recursively descends every non-hidden directory, which means it will happily spelunk through node_modules, build output, vendored junk, giant monorepo packages, or whatever other industrial landfill the repo contains. The comment says “skip hidden directories like .git,” which is cute, because the really expensive directories are usually not hidden.

This is a straight-up performance footgun. The feature most useful in big repos is implemented in a way that gets slower exactly where it matters most. Outstanding symmetry. 🥴

Files

• src/core/FileSystemUtils.ts

Code

• src/core/FileSystemUtils.ts — lines 264-304: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L264-L304

  async function findRulerDirs(dir: string) {
    const entries = await fs.readdir(dir, { withFileTypes: true });
    ...
    if (!entry.name.startsWith('.')) {
      ...
      await findRulerDirs(fullPath);
    }
  }

Suggestions

• Skip common heavy directories explicitly: node_modules, dist, build, coverage, vendor, .next, etc.
• Consider a fast walker with ignore rules instead of handwritten recursive readdir.
• Let nested mode accept include/exclude globs or a max-depth knob.
• Cache discovery results per run so follow-up operations don’t re-tour the entire museum of regrets.

Test Tooling Is Babysitting Dead Artifacts

🪦 The repo contains compiled .js test corpses while lint/typecheck mostly ignore tests, which is peak “works on my machine” feng shui.

Jest only runs .ts tests, ESLint only lints src/**/*.{ts,tsx}, and TypeScript excludes tests entirely—yet committed compiled .js tests are sitting in the tree like haunted leftovers. So the repo gets the worst of both worlds: extra noise in version control and weaker quality gates over the actual test suite.

This is a developer-experience tax. Dead files confuse diffs, weak test linting lets bad test code rot, and the setup encourages accidental divergence between what developers edit and what the repo stores. Gorgeous little mess. ✨💩

Files

• package.json
• jest.config.js
• eslint.config.mjs
• tsconfig.json
• tests/unit/core/FileSystemUtils.test.js
• tests/unit/core/RuleProcessor.test.js

Code

• package.json — lines 6-13: https://github.com/intellectronica/ruler/blob/main/package.json#L6-L13

  "scripts": {
    "lint": "eslint \"src/**/*.{ts,tsx}\"",
    "test": "jest",
    "build": "tsc"
  }

• jest.config.js — lines 1-6: https://github.com/intellectronica/ruler/blob/main/jest.config.js#L1-L6

  module.exports = {
    preset: 'ts-jest',
    testMatch: ['**/tests/**/*.{test,spec}.ts', '**/*.test.ts'],
  }

• eslint.config.mjs — lines 20-22: https://github.com/intellectronica/ruler/blob/main/eslint.config.mjs#L20-L22

  {
    ignores: ['dist/**', 'node_modules/**', '*.js', '*.cjs', '*.mjs'],
  }

• tsconfig.json — lines 113-114: https://github.com/intellectronica/ruler/blob/main/tsconfig.json#L113-L114

  "include": ["src"],
  "exclude": ["node_modules", "dist", "tests"]

• tests/unit/core/FileSystemUtils.test.js — lines 1-4: https://github.com/intellectronica/ruler/blob/main/tests/unit/core/FileSystemUtils.test.js#L1-L4

  "use strict";
  var __createBinding = ...
  var __setModuleDefault = ...

Suggestions

• Delete committed transpiled test artifacts and ignore them permanently.
• Lint test files too; if needed, add a dedicated TS config for tests instead of pretending they don’t exist.
• Keep Jest on TypeScript sources only, but make that a clean choice rather than an archaeological dig.
• Add a CI check that fails if generated .js files appear under tests/.