This Codebase Smells! [2026-01-04]

[github-actions[bot]]

This Codebase Smells!

Welcome to the Ruler carnival of “it works on my machine” engineering 🎪. This repo is a buffet of questionable decisions: recursive file hunts spelunking into node_modules, deletion routines with “force” like they’re breaking up with your data, and logging that argues with its own comments. Buckle up; we’re gonna scrub this thing until it squeaks… or at least stops screaming. 🤮

Table of Contents

• Overzealous Revert Deletes Real Files
• Recursive Scans Dive Into node_modules
• Verbose Logging Misfires To stderr
• Silent Error Swallowing During Directory Walks
• “any” Types Sneak Into Configs
• Forceful Recursive Deletes Without Manifests
• Global Config Writes Are Footguns

Overzealous Revert Deletes Real Files

⛈️ Revert includes a generic config.toml in its “cleanup” list, risking deletion of legitimate project configs.

• The revert engine hard-codes config.toml for deletion. Combined with agent logic that targets root config.toml, this invites data loss outside Ruler’s domain. 💣
• If backups aren’t present, this unlinks real files—your app config becomes “demo collateral.” Fabulous.

Files

• src/core/revert-engine.ts
• src/paths/mcp.ts

Code

• Hard-coded deletion list:
  • Lines 311–318: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L311-L318

    const additionalFiles = [
      '.gemini/settings.json',
      '.mcp.json',
      '.vscode/mcp.json',
      '.cursor/mcp.json',
      '.kilocode/mcp.json',
      'config.toml',
    ];

• OpenHands targets root config.toml:
  • Lines 33–36: https://github.com/intellectronica/ruler/blob/main/src/paths/mcp.ts#L33-L36

    // For Open Hands, we target the main config file, not a separate mcp.json
    candidates.push(path.join(projectRoot, 'config.toml'));

Suggestions

• Gate deletion of “generic” files behind:
  • Presence of a .bak, or a manifest recorded during apply.
  • Provenance markers embedded when Ruler creates files.
• Add --force-extra-cleanup and explicit warnings for non-ruler files.
• In removeAdditionalAgentFiles, only remove config.toml if it contains known OpenHands MCP sections.

Recursive Scans Dive Into node_modules

🕳️ The repo-wide search walks every directory that’s not dot-prefixed, happily spelunking into node_modules, dist, and any other time-sink.

• Downward search skips only directories starting with “.”, not common heavy dirs like node_modules. Enjoy your O(n^everything) traversal. 🐌
• Performance, developer experience, and CI times suffer; also risks touching files you should ignore.

Files

• src/core/FileSystemUtils.ts

Code

• Recursive search with weak filtering:
  • Lines 217–231: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L217-L231

    if (entry.isDirectory()) {
      if (entry.name === '.ruler') {
        rulerDirs.push(fullPath);
      } else {
        // Recursively search subdirectories (but skip hidden directories like .git)
        if (!entry.name.startsWith('.')) {
          await findRulerDirs(fullPath);
        }
      }
    }

Suggestions

• Add an ignore set: ['node_modules','dist','build','coverage','out','.next','.turbo'].
• Prefer targeted globbing over recursive readdir walks.
• Add a max-depth or cap the number of scanned entries; log when limits are hit.

Verbose Logging Misfires To stderr

🔊 The logging comment says verbose/info go to stdout, but logVerbose uses console.error. The code argues with itself.

• Mixed streams make piping and CI parsing miserable; verbose should be opt-in and consistent. 🔧
• Conflicting utilities (logVerbose vs logVerboseInfo) cause inconsistent output behavior.

Files

• src/constants.ts

Code

• Comment vs implementation:
  • Lines 25–27: https://github.com/intellectronica/ruler/blob/main/src/constants.ts#L25-L27

    // - info/verbose go to stdout (user-visible progress)
    // - warn/error go to stderr (problems)

  • Lines 17–21: https://github.com/intellectronica/ruler/blob/main/src/constants.ts#L17-L21

    export function logVerbose(message: string, isVerbose: boolean): void {
      if (isVerbose) {
        console.error(`[ruler:verbose] ${message}`);
      }
    }

Suggestions

• Route all verbose/info to stdout; warn/error to stderr only.
• Deprecate logVerbose or align it to logVerboseInfo.
• Add a unit test that asserts stream usage per level.

Silent Error Swallowing During Directory Walks

🤫 Catch-all with empty catch {} suppresses filesystem errors, making diagnosis impossible.

• Silent failure hides permission issues or broken paths; developers get “it did nothing” vibes. 👻
• At least log at debug level; ideally propagate or aggregate warnings.

Files

• src/core/FileSystemUtils.ts

Code

• Silent swallow:
  • Lines 234–235: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L234-L235

    } catch {
      // ignore errors when reading directories
    }

Suggestions

• Log with logVerboseInfo including path and error code when verbose is enabled.
• Accumulate a warnings array returned from the search to surface non-fatal issues.
• Only suppress specific expected errors (ENOENT), rethrow others.

“any” Types Sneak Into Configs

🧪 Liberal any usage around MCP/TOML serialization weakens type safety and invites runtime surprises.

• any masks schema drift and makes refactors dangerous; TOML stringification of loosely-typed objects is a roulette wheel. 🎰
• You already have interfaces—use them and extend safely.

Files

• src/agents/MistralVibeAgent.ts
• src/agents/CodexCliAgent.ts

Code

• Vibe config catch-all:
  • Lines 28–29: https://github.com/intellectronica/ruler/blob/main/src/agents/MistralVibeAgent.ts#L28-L29

    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    [key: string]: any;

• Stringify with any:
  • Lines 176–177: https://github.com/intellectronica/ruler/blob/main/src/agents/MistralVibeAgent.ts#L176-L177

    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    const tomlContent = stringify(updatedConfig as any);

• Codex config catch-all:
  • Lines 14–15: https://github.com/intellectronica/ruler/blob/main/src/agents/CodexCliAgent.ts#L14-L15

    // eslint-disable-next-line @typescript-eslint/no-explicit-any
    [key: string]: any; // Allow additional properties from transformation

Suggestions

• Replace [key: string]: any with Record<string, unknown> and narrow via discriminated unions.
• Define TOML schema types; parse/validate with zod or io-ts before stringify.
• Add type-safe merge helpers to avoid as any casts.

Forceful Recursive Deletes Without Manifests

🗑️ Aggressive fs.rm({ recursive: true, force: true }) appears throughout skills cleanup; it’s a bulldozer with no safety tape.

• Without a manifest or provenance check, this can delete directories someone created manually. Yikes. 💥
• Dry-run exists, but defaults matter—don’t trust users to read flags when their files are on the chopping block.

Files

• src/core/SkillsProcessor.ts

Code

• Forceful remove:
  • Lines 129–131: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L129-L131

    await fs.rm(claudeSkillsPath, { recursive: true, force: true });
    logVerboseInfo(
      `Removed ${CLAUDE_SKILLS_PATH} (skills disabled)`,

Suggestions

• Maintain a write manifest file in .ruler recording created paths; only delete recorded paths.
• Require --force or interactive confirm for directories not in manifest.
• On delete, prefer restore from backup or move to .ruler/.trash before permanent removal.

Global Config Writes Are Footguns

🏠 Writing to ~/.config/ruler by default can surprise users and contaminate global state.

• “Global” operations should be explicit opt-in, with recheck and warnings; falling back to home makes stateful bugs hard to track. 🧨
• CI may not have XDG set; you’ll end up writing to random places.

Files

• src/core/FileSystemUtils.ts
• src/cli/handlers.ts

Code

• XDG fallback:
  • Lines 9–11: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L9-L11

    return process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config');

• Global path derivation:
  • Lines 138–144: https://github.com/intellectronica/ruler/blob/main/src/cli/handlers.ts#L138-L144

    const rulerDir = isGlobal
      ? path.join(
          process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config'),
          'ruler',
        )
      : path.join(projectRoot, '.ruler');

Suggestions

• Require --global flag with a confirmation prompt and echo the resolved path.
• Detect CI and disable global writes unless explicitly allowed.
• Log a prominent notice when writing global files and include a “how to revert” message.

---

If you want more dirt: I can keep digging, but the main sewer pipes are these. Fix the scanning filters, stop deleting strangers’ stuff, make logging consistent, and please… stop inviting any to your types party. Your future self will send you chocolates. Or at least stop swearing at your commit history. 💩