This Codebase Smells! [2026-06-07]

[github-actions[bot]]

This Codebase Smells!

This week’s aroma profile is copy-paste flambé, stale abstraction, and a lingering finish of “future me can mop that up” 🤮. The repo is not beyond saving, but it absolutely has that special enterprise musk where things mostly work while quietly breeding maintenance debt in the walls 💩.

The good news: I did not need to invent problems to fill this report. The bad news: that’s because the real ones were already standing in line.

Table of Contents

• Copy-Paste Propagation Pipeline
• Agent Integrations as a Hard-Coded Zoo
• Two Config Loaders, Zero Peace
• Backup Failures Get Silently Swallowed
• Nested Discovery Scans the Whole Damn Repo
• Test Code Lives Outside the Static Gate

Copy-Paste Propagation Pipeline

💩 SkillsProcessor.ts already has target metadata, yet still duplicates the same propagation logic per target like it’s being paid by the line.

The file clearly knows what a skill target is, but instead of iterating over that definition it hand-wires an if ladder and a herd of propagateSkillsForX functions. So you get the maintenance cost of a metadata layer and the maintenance cost of copy-paste. Remarkably inefficient. Truly artisanal nonsense.

This is the kind of file that feels “fine” at 3 targets and turns into a haunted spreadsheet at 13. Every new integration now means more branch glue, more duplicated filesystem choreography, and more chances for one path to drift while the others keep pretending they’re cousins.

Files

• src/core/SkillsProcessor.ts

Code

Target metadata already exists — Lines 140–153: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L140-L153

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

But dispatch is still handwritten — Lines 377–385: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L377-L385

if (selectedTargets.has('claude')) {
  ...
  await propagateSkillsForClaude(projectRoot, { dryRun });
}

Claude propagation — Lines 523–542: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L523-L542

const tempDir = await createTempSkillsDir(claudeDir);
...
await fs.rm(claudeSkillsPath, { recursive: true, force: true });
...
await replaceSkillsDirectory(tempDir, claudeSkillsPath);

Codex propagation, same story with different nouns — Lines 581–600: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L581-L600

const tempDir = await createTempSkillsDir(codexDir);
...
await fs.rm(codexSkillsPath, { recursive: true, force: true });
...
await replaceSkillsDirectory(tempDir, codexSkillsPath);

Suggestions

• Replace propagateSkillsForX with one generic directory-propagation helper driven by target metadata.
• Store destination path, label, and agent identifiers in a single table and iterate it for dispatch.
• Keep truly target-specific behavior behind optional hooks instead of cloning the entire temp-dir / copy / replace sequence.

Agent Integrations as a Hard-Coded Zoo

🤮 Most agent integrations are tiny piles of metadata wrapped in full classes, so adding a new one still requires a ceremonial copy-paste parade.

The registry is a hand-maintained wall of imports and new WhateverAgent() calls. That makes consistency a matter of human memory instead of design, which is always adorable right up until someone forgets one of the five places a new integration needs to be wired.

A few adapters absolutely deserve custom code. Most do not. When the common case is just “identifier, display name, path, capabilities,” forcing that through separate files and registry edits is not abstraction — it’s paperwork wearing a trench coat.

Files

• src/agents/index.ts
• src/agents/ClaudeAgent.ts
• src/agents/CursorAgent.ts
• src/agents/JunieAgent.ts

Code

Manual registry wiring — Lines 38–56: https://github.com/intellectronica/ruler/blob/main/src/agents/index.ts#L38-L56

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

Plain metadata disguised as a class — Lines 7–18: https://github.com/intellectronica/ruler/blob/main/src/agents/ClaudeAgent.ts#L7-L18

export class ClaudeAgent extends AbstractAgent {
  getIdentifier(): string {
    return 'claude';
  }

  getName(): string {
    return 'Claude Code';
  }

Even a mild special case gets another wrapper — Lines 18–35: https://github.com/intellectronica/ruler/blob/main/src/agents/CursorAgent.ts#L18-L35

async applyRulerConfig(...) {
  await super.applyRulerConfig(
    concatenatedRules,
    projectRoot,
    null,
    ...
  );
}

Suggestions

• Introduce a single AgentDefinition manifest for standard metadata: id, name, defaultOutput, capabilities, MCP keys, skill/subagent support.
• Generate allAgents, CLI help, target maps, and docs from that source of truth.
• Keep bespoke classes only for genuinely weird adapters, not for every agent with a pulse.

Two Config Loaders, Zero Peace

🤦 This repo parses ruler.toml in two different subsystems, which is a classy way to schedule config drift for later.

ConfigLoader.ts has an actual schema and validates input like a responsible adult. UnifiedConfigLoader.ts then rereads the same file and manually peels fields off tomlRaw like it’s trying to avoid making eye contact with the first loader.

That split already covers overlapping concerns like default_agents, nested, skills, and MCP parsing. So every new config field now has two places to be updated and one future bug report where everybody swears their parser is the real one. Magnificent.

Files

• src/core/ConfigLoader.ts
• src/core/UnifiedConfigLoader.ts

Code

Schema-based validation exists — Lines 68–77: https://github.com/intellectronica/ruler/blob/main/src/core/ConfigLoader.ts#L68-L77

const rulerConfigSchema = z.object({
  default_agents: z.array(z.string()).optional(),
  agents: z
    .object({ ... })
    .catchall(agentConfigSchema)
    .optional(),

And it is actually enforced — Lines 532–542: https://github.com/intellectronica/ruler/blob/main/src/core/ConfigLoader.ts#L532-L542

const validationResult = rulerConfigSchema.safeParse(raw);
if (!validationResult.success) {
  throw createRulerError(...);
}

Meanwhile the other loader reparses the same TOML — Lines 64–72: https://github.com/intellectronica/ruler/blob/main/src/core/UnifiedConfigLoader.ts#L64-L72

const text = await fs.readFile(tomlFile, 'utf8');
tomlRaw = text.trim() ? parseTOML(text) : {};
meta.configFile = tomlFile;

Then manually extracts overlapping fields — Lines 90–119: https://github.com/intellectronica/ruler/blob/main/src/core/UnifiedConfigLoader.ts#L90-L119

let defaultAgents: string[] | undefined;
...
let nested = false;
...
let skillsConfig;

And rolls its own MCP parsing too — Lines 193–206: https://github.com/intellectronica/ruler/blob/main/src/core/UnifiedConfigLoader.ts#L193-L206

const tomlMcpServers: Record<string, McpServerDef> = {};
...
const server: McpServerDef = copyAdditionalMcpServerFields(serverDef);

Suggestions

• Parse TOML once into a canonical config DTO/AST and let both consumers project from that.
• Centralize validation, defaults, and deprecation handling in one place; keep diagnostics as a layer on top, not a separate parser.
• Add equivalence tests that assert both public loaders interpret the same fixture configs the same way.

Backup Failures Get Silently Swallowed

☠️ When backups are enabled, a failed backup can disappear quietly and the overwrite still happens anyway, which is a lovely little betrayal.

backupFile suppresses every copyFile failure even though the comment only talks about a missing file. So EACCES, ENOSPC, EROFS, and friends get the exact same shrug as ENOENT. Very reassuring. Very safety-oriented. Very clown shoes.

Then the main write path carries on and writes the generated file regardless. If backup is turned on, failure to create it should stop the write, not politely step aside and let the damage continue.

Files

• src/core/FileSystemUtils.ts
• src/agents/AbstractAgent.ts

Code

Backup swallows all copy errors — Lines 266–279: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L266-L279

try {
  await fs.copyFile(filePath, backupPath);
} catch {
  // ignore if file does not exist
}

Callers write immediately after the backup attempt — Lines 44–51: https://github.com/intellectronica/ruler/blob/main/src/agents/AbstractAgent.ts#L44-L51

if (backup) {
  await backupFile(absolutePath);
}
await writeGeneratedFile(absolutePath, concatenatedRules);

Suggestions

• Only suppress ENOENT; surface any other backup failure with context.
• Make backupFile return an explicit status or throw, and refuse to overwrite when backup === true but backup creation failed.
• Add tests for permission and disk-failure scenarios so the “safety” feature stops being decorative.

Nested Discovery Scans the Whole Damn Repo

🐢 Nested .ruler discovery is a brute-force filesystem crawl with an ignore list so short it feels sarcastic.

Yes, it skips dot directories and nested git repos, so it’s not completely feral. But everything else is still fair game, which means large monorepos can pay a silly I/O tax wandering through packages/, apps/, vendor/, target/, out/, and whatever other directory somebody forgot to add to the sacred set.

This kind of discovery ages like milk. Every new “big junk directory” becomes a performance bug until someone remembers to stuff another magic string into a constant. Delightful little maintenance slot machine 🎰.

Files

• src/core/FileSystemUtils.ts

Code

Tiny built-in ignore list — Lines 9–20: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L9-L20

const DEFAULT_NESTED_DISCOVERY_IGNORES = new Set([
  '__fixtures__',
  '__generated__',
  'build',
  'coverage',
  'dist',
  ...
]);

Full subtree walk — Lines 315–338: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L315-L338

const entries = await fs.readdir(dir, { withFileTypes: true });
...
if (!shouldSkipNestedDiscoveryDir(entry.name)) {
  ...
  await findRulerDirs(fullPath);
}

Suggestions

• Respect .gitignore/.ignore where practical, or allow ignore patterns in ruler.toml.
• Expand the default skip set for common heavyweight directories (packages, apps, vendor, target, out, etc.).
• Cache discovery results per run and consider bounded-depth or explicit-root options for nested mode.

Test Code Lives Outside the Static Gate

🧪 The test suite is invited to Jest, but not to lint, and only half-invited to type-checking.

The lint script only targets src, so test code gets zero ESLint coverage. For a repo that clearly leans hard on tests, that is a weird amount of trust to place in vibes and good intentions. Dead imports? Unsafe casts? Sketchy setup helpers? Enjoy the surprise party.

tsconfig.json excludes tests, which is fine for build output, but it means test typing piggybacks on ts-jest during test execution instead of having a dedicated static gate. That fuses runtime execution to type validation — slower, noisier, and much worse at saying “your test code is rotting” before the whole circus starts.

Files

• package.json
• tsconfig.json
• jest.config.js

Code

Lint/build scripts stop at src — Lines 7–15: https://github.com/intellectronica/ruler/blob/main/package.json#L7-L15

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

The compiler excludes tests — Lines 113–114: https://github.com/intellectronica/ruler/blob/main/tsconfig.json#L113-L114

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

Tests are TypeScript and only enter through ts-jest — Lines 1–5: https://github.com/intellectronica/ruler/blob/main/jest.config.js#L1-L5

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

Suggestions

• Extend ESLint coverage to tests/**/*.{ts,tsx} and selectively exclude only ugly fixtures.
• Add a dedicated tsc --noEmit pass for tests via tsconfig.tests.json, or otherwise give test code its own static type-check step.
• Keep runtime test execution and static validation separate so failures are faster, clearer, and less annoying.