This Codebase Smells! [2026-02-08]

[github-actions[bot]]

This Codebase Smells!

Welcome to another episode of “Why does this repo hurt my soul?” 🎭 This week’s fragrance: Eau de Side‑Effect, with notes of “oops we deleted your real config” and a bouquet of “logs to stderr for fun.” Grab a clothespin for your nose and a helmet for your sanity. 💥🦨

Table of Contents

• Nuclear Revert Deletes Real Config Files
• Dry‑Run Prefix Comedy: Calling a function by not calling it
• Verbose Logs to stderr: Mixed‑up streams
• CLI Rage‑Quits with process.exit
• Console.log Party: Bypass centralized logger
• Orphaned JS Tests: Jest pretends they don’t exist
• MCP Path Footgun: Inventing files that aren’t there
• TypeScript Any‑fluenza: The “any” pandemic

Nuclear Revert Deletes Real Config Files

⛏️ Revert gleefully deletes generic project files like config.toml even when they’re legitimate and unrelated to Ruler.

• The revert engine’s “additional files” list includes a raw config.toml — real projects use that, you know, for actual configuration. 💣
• If there’s no .bak, it just unlinks the file. Surprise data loss is a vibe, I guess. 🤡

Files

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

Code

• Hard‑coded extra deletions:
  • 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',
    ];

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

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

Suggestions

• Only delete files with a Ruler provenance marker, or if a corresponding .bak exists.
• Gate config.toml deletion behind OpenHands‑specific content detection (known keys/sections) or a --force-extra-cleanup flag.
• Record outputs during apply and delete strictly from that manifest. No manifest? No delete.

Dry‑Run Prefix Comedy: Calling a function by not calling it

🃏 Logging in the VSCode settings cleanup prints ${actionPrefix} — the function reference, not the prefix. Dry‑run messages become performance art.

• The code calculates prefix = actionPrefix(dryRun)… then logs with ${actionPrefix} (the function), which is not what anyone ordered. 🍝

Files

• src/core/revert-engine.ts

Code

• Lines 366–369: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L366-L369

  logVerbose(
    `${actionPrefix} Restored VSCode settings from backup`,
    verbose,
  );

• Lines 379–381: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L379-L381

  `${actionPrefix} Would remove empty VSCode settings file`,

• Lines 383–386: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L383-L386

  `${actionPrefix} Would remove augment.advanced section from ${settingsPath}`,

• Lines 399–401: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L399-L401

  `${actionPrefix} Removed empty VSCode settings file`,

• Lines 405–407: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L405-L407

  `${actionPrefix} Removed augment.advanced section from VSCode settings`,

Suggestions

• Replace ${actionPrefix} with the computed prefix or call actionPrefix(dryRun) inline.
• Add a unit test asserting dry‑run log content so this doesn’t regress.

Verbose Logs to stderr: Mixed‑up streams

📣 The comment says verbose logs go to stdout, yet logVerbose writes to stderr. So… which is it? Schrödinger’s stream. 🐱

Files

• src/constants.ts

Code

• 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

• Change console.error to console.log for verbose; keep warnings/errors on stderr.
• Audit callers to ensure “user‑visible progress” vs “problems” follow the contract.

CLI Rage‑Quits with process.exit

🛑 The CLI loves process.exit(1) like it’s still 2009. This torpedoes composability, testability, and proper error propagation from libraries. 🤮

Files

• src/cli/handlers.ts

Code

• Lines 44–48: https://github.com/intellectronica/ruler/blob/main/src/cli/handlers.ts#L44-L48

  console.error(`${ERROR_PREFIX} ...`);
  process.exit(1);

• Lines 125–128: https://github.com/intellectronica/ruler/blob/main/src/cli/handlers.ts#L125-L128
• Lines 239–241: https://github.com/intellectronica/ruler/blob/main/src/cli/handlers.ts#L239-L241

Suggestions

• Throw errors from handlers; let the CLI entry point map exceptions to exit codes.
• Standardize an runCli() wrapper that returns status; only main file calls process.exit.

Console.log Party: Bypass centralized logger

🎺 Code prints directly with console.log instead of using the centralized logging helpers. Enjoy inconsistent prefixes and streams! 💃

Files

• src/revert.ts
• src/constants.ts (has logging helpers you ignore)

Code

• Lines 117: https://github.com/intellectronica/ruler/blob/main/src/revert.ts#L117

  console.log(`${prefix} Reverting ${agent.getName()}...`);

• Lines 153–171: https://github.com/intellectronica/ruler/blob/main/src/revert.ts#L153-L171

Suggestions

• Replace direct console.log with logInfo/logWarn consistently.
• Enforce via ESLint rule: ban console.* outside constants.ts.

Orphaned JS Tests: Jest pretends they don’t exist

🧪 You’ve got .js tests chilling like NPCs while Jest looks only for .ts. Either run them or ship them to the great bit bucket in the sky. 🗑️

Files

• jest.config.js
• test_toml_format.js
• test_toml_options.js
• test_manual_toml.js

Code

• Jest test match excludes .js:
  • Lines 5–6: https://github.com/intellectronica/ruler/blob/main/jest.config.js#L5-L6

    testMatch: ['**/tests/**/*.{test,spec}.ts', '**/*.test.ts'],
    moduleFileExtensions: ['ts', 'js', 'json', 'node'],

Suggestions

• Convert .js tests to .ts, or update testMatch to include .js and fix any TS/Jest setup conflicts.
• If they’re legacy, delete them to reduce confusion and CI drift.

MCP Path Footgun: Inventing files that aren’t there

🔫 getNativeMcpPath returns the first candidate even when none exist, effectively “choosing” a path to create/modify. Accidental file creation, anyone? 🧞

Files

• src/paths/mcp.ts

Code

• Lines 73–75: https://github.com/intellectronica/ruler/blob/main/src/paths/mcp.ts#L73-L75

  // default to first candidate if none exist
  return candidates.length > 0 ? candidates[0] : null;

Suggestions

• Return null if none exist; only create new MCP configs in apply paths, not path discovery.
• Add an explicit “create if missing” strategy flag; make revert read‑only.

TypeScript Any‑fluenza: The “any” pandemic

🦠 The amount of any (plus eslint-disable) is… aspirational. If types are optional, bugs are mandatory. 💩

Files

• src/agents/MistralVibeAgent.ts
• src/agents/CodexCliAgent.ts
• src/types/iarna-toml.d.ts

Code

• Mistral Vibe “any” and disable:
  • 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;

  • Lines 177–178: https://github.com/intellectronica/ruler/blob/main/src/agents/MistralVibeAgent.ts#L177-L178

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

• Codex CLI “any” welcome mat:
  • Lines 18–19: https://github.com/intellectronica/ruler/blob/main/src/agents/CodexCliAgent.ts#L18-L19

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

• TOML typings punt entirely:
  • Lines 1–6: https://github.com/intellectronica/ruler/blob/main/src/types/iarna-toml.d.ts#L1-L6

    /* eslint-disable @typescript-eslint/no-explicit-any */
    export function parse(input: string): any;
    export function stringify(input: any): string;

Suggestions

• Define narrow interfaces for TOML shapes per agent (e.g., Vibe, Codex) and replace any with structured types.
• Remove eslint-disable by fixing types; where unavoidable, isolate any behind well‑documented conversion points.

---

If you expected more issues, congratulations: the bar is low. But fix these and the repo will smell less like a burning server rack and more like… software. 🧼🚀