This Codebase Smells! [2026-04-05] #409

2026-04-05T14:17:23Z

github-actions[bot]
Bot Apr 5, 2026

This Codebase Smells!

Week’s special: burnt abstraction with a side of filesystem roulette 🤮. This repo is not beyond saving, but it is doing that adorable thing where every feature has three truth sources, four special cases, and a catch {} waiting behind the sofa with a knife.

I focused on the smells most likely to improve maintainability, robustness, security, performance, and developer sanity — because right now the codebase feels like it was assembled by a committee of caffeinated raccoons with write access. 💩

One Registry to Confuse Them All

The supported-agent model is duplicated across docs, runtime registration, MCP path resolution, and skills routing, so adding one agent is a scavenger hunt instead of a change.

There is no canonical source of truth for agent metadata; there are just multiple hand-maintained lists pretending to be friends. That means renames, new agents, or capability changes have to land in several places, and drift is not a risk — it is the default operating mode.
This is how you get docs lying, CLI help aging badly, and one agent mysteriously supporting skills in one subsystem but not another. The repo is basically doing schema design by ritual sacrifice.

Relevant files

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

Quotes

README.md lines 57-63 — link

| Agent                  | Rules File(s)                                  | MCP Configuration / Notes                        | Skills Support / Location |
| AGENTS.md              | `AGENTS.md`                                    | (pseudo-agent ensuring root `AGENTS.md` exists)  | -                         |
| GitHub Copilot         | `AGENTS.md`                                    | `.vscode/mcp.json`                               | `.claude/skills/`         |
| Claude Code            | `CLAUDE.md`                                    | `.mcp.json`                                      | `.claude/skills/`         |

src/agents/index.ts lines 38-45 — link

export const allAgents: IAgent[] = [
  new CopilotAgent(),
  new ClaudeAgent(),
  new CodexCliAgent(),
  new CursorAgent(),
  new WindsurfAgent(),
  new ClineAgent(),
  new AiderAgent(),

src/paths/mcp.ts lines 10-18 — link

switch (adapterName) {
  case 'GitHub Copilot':
    candidates.push(path.join(projectRoot, '.vscode', 'mcp.json'));
    break;
  case 'Visual Studio':
    candidates.push(path.join(projectRoot, '.mcp.json'));
    candidates.push(path.join(projectRoot, '.vs', 'mcp.json'));

src/core/SkillsProcessor.ts lines 140-148 — link

const SKILL_TARGET_TO_IDENTIFIERS = new Map<SkillTarget, readonly string[]>([
  ['claude', ['claude', 'copilot', 'kilocode']],
  ['codex', ['codex']],
  ['opencode', ['opencode']],
  ['pi', ['pi']],

Suggestions

Introduce a single AgentDescriptor registry containing identifier, display name, rule outputs, MCP target, skills target, and capabilities.
Generate CLI help and README support tables from that registry instead of manually curating parallel universes.
Keep adapter classes for behavior only; move metadata out of switches and lists.

Symlink Safari Through the Entire Repo

Rule discovery happily follows symlinks and recursively scans the project tree with almost no guardrails, which is a delightful way to mix security bugs with performance cliffs.

readMarkdownFiles follows symlinked directories and files on purpose. Great: now a .ruler tree can pull content from outside the repo into agent prompts, and a symlink cycle can send traversal into the kind of recursive meltdown usually reserved for bad fantasy novels.
Nested .ruler discovery walks every non-hidden directory. Not “every relevant directory,” not “every safe directory,” just everything that isn’t dot-prefixed. Hello node_modules, build trash, vendored sludge, and accidental latency theater. 🤡

Relevant files

src/core/FileSystemUtils.ts
tests/unit/core/FileSystemUtils.test.ts

Quotes

src/core/FileSystemUtils.ts lines 74-82 — link

let isDir = entry.isDirectory();
let isFile = entry.isFile();
if (entry.isSymbolicLink()) {
  try {
    const stat = await fs.stat(fullPath);
    isDir = stat.isDirectory();
    isFile = stat.isFile();
  } catch {

src/core/FileSystemUtils.ts lines 86-95 — link

if (isDir) {
  const relativeFromRoot = path.relative(rulerDir, fullPath);
  const isSkillsDir =
    relativeFromRoot === SKILLS_DIR ||
    relativeFromRoot.startsWith(`${SKILLS_DIR}${path.sep}`);
  if (isSkillsDir) {
    continue;
  }
  await walk(fullPath);

src/core/FileSystemUtils.ts lines 240-256 — link

// Recursively search subdirectories (but skip hidden directories like .git)
if (!entry.name.startsWith('.')) {
  const gitDir = path.join(fullPath, '.git');
  try {
    const gitStat = await fs.stat(gitDir);
    if (
      gitStat.isDirectory() &&
      path.resolve(fullPath) !== rootPath
    ) {
      continue;
    }
  } catch {
    // no .git boundary, continue traversal
  }
  await findRulerDirs(fullPath);
}

tests/unit/core/FileSystemUtils.test.ts lines 65-78 — link

it('follows symlinked directories', async () => {
  const realDir = path.join(tmpDir, 'external-dir');
  await fs.mkdir(realDir, { recursive: true });
  const realFile = path.join(realDir, 'deep.md');
  await fs.writeFile(realFile, 'deep content');
  const linkPath = path.join(rulerDir, 'linked-dir');
  await fs.symlink(realDir, linkPath);

Suggestions

Do not follow symlinks by default; make that an explicit opt-in if someone truly craves danger.
Track visited realpaths/inodes to break cycles.
Enforce that resolved paths stay under projectRoot before reading anything.
Add ignore rules for heavyweight directories during nested discovery: node_modules, dist, build, coverage, vendor, .next, .turbo, etc.

Backup Guarantees Are Apparently Optional

The adapter contract says “support backups,” the base class implements it, and then several custom agents just freestyle direct writes like consequences are a myth.

The abstraction here is trying so hard to exist. IAgent exposes backup, AbstractAgent does the sane thing, and then custom adapters immediately go off-road with raw fs.writeFile calls and custom merge logic. Consistency has left the building.
That means backup semantics vary by agent, idempotency varies by agent, and revert safety varies by agent. So the user experience depends on which mascot you picked today. Magnificent engineering discipline, no notes. 🙄

Relevant files

src/agents/IAgent.ts
src/agents/AbstractAgent.ts
src/agents/OpenCodeAgent.ts
src/agents/CodexCliAgent.ts
src/agents/GeminiCliAgent.ts
src/agents/QwenCodeAgent.ts
src/agents/ZedAgent.ts

Quotes

src/agents/IAgent.ts lines 37-43 — link

applyRulerConfig(
  concatenatedRules: string,
  projectRoot: string,
  rulerMcpJson: Record<string, unknown> | null,
  agentConfig?: IAgentConfig,
  backup?: boolean,
): Promise<void>;

src/agents/AbstractAgent.ts lines 44-51 — link

const absolutePath = path.resolve(projectRoot, output);
await ensureDirExists(path.dirname(absolutePath));
if (backup) {
  await backupFile(absolutePath);
}
await writeGeneratedFile(absolutePath, concatenatedRules);

src/agents/OpenCodeAgent.ts lines 21-38 — link

async applyRulerConfig(
  concatenatedRules: string,
  projectRoot: string,
  rulerMcpJson: Record<string, unknown> | null,
  agentConfig?: IAgentConfig,
): Promise<void> {
  const outputPaths = this.getDefaultOutputPath(projectRoot);
  const instructionsPath = path.resolve(
    projectRoot,
    agentConfig?.outputPathInstructions ?? outputPaths['instructions'],
  );

  await fs.writeFile(instructionsPath, concatenatedRules);

src/agents/OpenCodeAgent.ts lines 71-72 — link

// Always write the config file, even if MCP is empty
await fs.writeFile(mcpPath, JSON.stringify(finalMcpConfig, null, 2));

src/agents/CodexCliAgent.ts lines 44-50 — link

async applyRulerConfig(
  concatenatedRules: string,
  projectRoot: string,
  rulerMcpJson: RulerMcp | null,
  agentConfig?: IAgentConfig,
  backup = true,
): Promise<void> {

src/agents/CodexCliAgent.ts lines 145-151 — link

const finalConfig = { ...updatedConfig };

// @iarna/toml should handle the formatting properly
const tomlContent = stringify(finalConfig);

await writeGeneratedFile(configPath, tomlContent);

Suggestions

Introduce shared writeManagedTextFile, writeManagedJsonFile, and writeManagedTomlFile helpers that always do ensure-dir, diff, backup, and atomic write.
Push merge logic into helpers too, so adapters provide transforms instead of bespoke file surgery.
Add parameterized adapter-contract tests that verify backup/idempotency/revert behavior for every agent.

Corruption Gets Rewarded With Silent Defaults

When existing config is broken, this codebase repeatedly shrugs, invents an empty object, and marches on toward data loss with enormous confidence.

Missing files and invalid files are treated like the same problem in too many places. They are not. One means “nothing exists yet”; the other means “something exists and you’re about to bulldoze it.”
A config tool should fail loudly on parse corruption, preserve the bad file, and tell the user what to fix. This one often does the software equivalent of whistling while backing over a mailbox.

Relevant files

src/core/ConfigLoader.ts
src/paths/mcp.ts
src/agents/OpenCodeAgent.ts
src/agents/FirebenderAgent.ts
src/agents/CrushAgent.ts

Quotes

src/core/ConfigLoader.ts lines 165-175 — link

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

src/paths/mcp.ts lines 80-89 — link

/** Read native MCP config from disk, or return empty object if missing/invalid. */
export async function readNativeMcp(
  filePath: string,
): Promise<Record<string, unknown>> {
  try {
    const text = await fs.readFile(filePath, 'utf8');
    return JSON.parse(text) as Record<string, unknown>;
  } catch {
    return {};
  }

src/agents/OpenCodeAgent.ts lines 62-68 — link

} catch {
  if (rulerMcpJson) {
    finalMcpConfig = {
      $schema: 'https://opencode.ai/config.json',
      mcp: (rulerMcpJson?.mcpServers ?? {}) as Record<string, unknown>,
    };
  }
}

src/agents/FirebenderAgent.ts lines 107-117 — link

} catch (error: unknown) {
  if (
    error &&
    typeof error === 'object' &&
    'code' in error &&
    (error as { code?: string }).code === 'ENOENT'
  ) {
    return { rules: [] };
  }
  console.warn(`Failed to read/parse existing firebender.json: ${error}`);
  return { rules: [] };
}

src/agents/CrushAgent.ts lines 97-104 — link

} catch {
  if (rulerMcpJson) {
    const transformedServers = this.transformMcpServersForCrush(
      (rulerMcpJson?.mcpServers ?? {}) as Record<string, unknown>,
    );
    finalMcpConfig = {
      mcp: transformedServers,
    };

Suggestions

Treat only ENOENT as “empty”; abort on parse or schema errors.
Preserve invalid files untouched and report exact remediation steps.
Centralize config read/merge/write behavior so parse failure policy is consistent.
Consider a --repair or --force-overwrite path for intentional recovery, instead of stealth resets.

SkillsProcessor Is a Copy Paste Theme Park

SkillsProcessor.ts is a giant copy-paste blob where each new agent gets another near-identical cleanup block and another near-identical propagation function because apparently loops were feeling too mainstream.

This file is doing the same work over and over with different constants jammed into the template. That bloats the code, bloats review burden, and makes “add one skills target” way more dangerous than it should be.
The repeated try/access/rm/copy/rename/catch pattern is exactly the kind of duplication that breeds one-off bugs and inconsistent behavior. It’s not explicitness; it’s clerical work wearing a fake mustache.

Relevant files

src/core/SkillsProcessor.ts

Quotes

src/core/SkillsProcessor.ts lines 177-194 — link

async function cleanupSkillsDirectories(
  projectRoot: string,
  dryRun: boolean,
  verbose: boolean,
): Promise<void> {
  const claudeSkillsPath = path.join(projectRoot, CLAUDE_SKILLS_PATH);
  const codexSkillsPath = path.join(projectRoot, CODEX_SKILLS_PATH);
  const opencodeSkillsPath = path.join(projectRoot, OPENCODE_SKILLS_PATH);
  const piSkillsPath = path.join(projectRoot, PI_SKILLS_PATH);
  const gooseSkillsPath = path.join(projectRoot, GOOSE_SKILLS_PATH);
  const vibeSkillsPath = path.join(projectRoot, VIBE_SKILLS_PATH);

src/core/SkillsProcessor.ts lines 683-722 — link

export async function propagateSkillsForClaude(
  projectRoot: string,
  options: { dryRun: boolean },
): Promise<string[]> {
  const skillsDir = path.join(projectRoot, RULER_SKILLS_PATH);
  const claudeSkillsPath = path.join(projectRoot, CLAUDE_SKILLS_PATH);
  const claudeDir = path.dirname(claudeSkillsPath);
  ...
  await copySkillsDirectory(skillsDir, tempDir);
  ...
  await fs.rename(tempDir, claudeSkillsPath);

src/core/SkillsProcessor.ts lines 741-780 — link

export async function propagateSkillsForCodex(
  projectRoot: string,
  options: { dryRun: boolean },
): Promise<string[]> {
  const skillsDir = path.join(projectRoot, RULER_SKILLS_PATH);
  const codexSkillsPath = path.join(projectRoot, CODEX_SKILLS_PATH);
  const codexDir = path.dirname(codexSkillsPath);
  ...
  await copySkillsDirectory(skillsDir, tempDir);
  ...
  await fs.rename(tempDir, codexSkillsPath);

Suggestions

Replace per-agent skill functions with a SkillTargetDescriptor[] and two generic helpers: one for cleanup, one for propagate.
Keep the atomic replace logic in one place.
Derive skill targets from the same central agent registry instead of hard-coding yet another mapping.
Add table-driven tests over descriptors, not copy-pasted test cases for each target.

Fuzzy Agent Matching Is a CLI Booby Trap

The CLI and config loader accept case-insensitive substrings for agent names, which is cute until half the product line contains the word “code” and your selection logic turns into improv theatre.

The public API documents substring matching, and both selection and config normalization implement it literally. That means ambiguity is baked in as a feature, not treated as an error.
In a repo with Claude Code, Codex, AugmentCode, Kilo Code, OpenCode, Qwen Code, and RooCode, substring matching is not user-friendly — it’s a randomizer with better branding.

Relevant files

src/lib.ts
src/core/agent-selection.ts
src/core/config-utils.ts
tests/unit/core/agent-selection.test.ts
README.md

Quotes

src/lib.ts lines 41-44 — link

 * Applies ruler configurations for selected AI agents.
 * @param projectRoot Root directory of the project
 * @param includedAgents Optional list of agent name filters (case-insensitive substrings)
 */

src/core/agent-selection.ts lines 31-49 — link

const invalidAgents = filters.filter(
  (filter) =>
    !validAgentIdentifiers.has(filter) &&
    ![...validAgentNames].some((name) => name.includes(filter)),
);

selected = allAgents.filter((agent) =>
  filters.some(
    (f) =>
      agent.getIdentifier() === f ||
      agent.getName().toLowerCase().includes(f),
  ),

src/core/config-utils.ts lines 21-31 — link

for (const [key, cfg] of Object.entries(raw)) {
  const lowerKey = key.toLowerCase();
  for (const agent of agents) {
    const identifier = agent.getIdentifier();
    if (
      identifier === lowerKey ||
      agent.getName().toLowerCase().includes(lowerKey)
    ) {
      mappedConfigs[identifier] = cfg;
    }

tests/unit/core/agent-selection.test.ts lines 143-165 — link

it('should handle partial name matches in CLI agents', () => {
  const config: LoadedConfig = {
    cliAgents: ['code'], // Should match "Claude Code"
    agentConfigs: {},
  };
  ...
});

it('should handle partial name matches in default_agents', () => {
  const config: LoadedConfig = {
    defaultAgents: ['code'], // Should match "Claude Code"

README.md lines 77-82 — link

| AugmentCode            | `.augment/rules/ruler_augment_instructions.md` | -                                                | -                         |
| Kilo Code              | `AGENTS.md`                                    | `.kilocode/mcp.json`                             | `.claude/skills/`         |
| OpenCode               | `AGENTS.md`                                    | `opencode.json`                                  | `.opencode/skills/`       |
| Qwen Code              | `AGENTS.md`                                    | `.qwen/settings.json`                            | -                         |
| RooCode                | `AGENTS.md`                                    | `.roo/mcp.json`                                  | `.roo/skills/`            |

Suggestions

Accept exact identifiers by default.
If shorthand is desirable, allow only unique prefixes and error on ambiguity with a candidate list.
Keep backward-compatible substring matching behind a deprecation flag, not as the permanent rules engine for user intent.
Apply the same resolution rules everywhere: CLI parsing, config normalization, help text, and validation.

Navigation Menu

Repository navigation

This Codebase Smells! [2026-04-05] #409

Uh oh!

{{title}}

Uh oh!

Replies: 0 comments

Select a reply

Uh oh!

This Codebase Smells! [2026-04-05] #409

Uh oh!

github-actions[bot] Bot Apr 5, 2026

This Codebase Smells!

Table of Contents

One Registry to Confuse Them All

Symlink Safari Through the Entire Repo

Backup Guarantees Are Apparently Optional

Corruption Gets Rewarded With Silent Defaults

SkillsProcessor Is a Copy Paste Theme Park

Fuzzy Agent Matching Is a CLI Booby Trap

Replies: 0 comments

github-actions[bot]
Bot Apr 5, 2026