This Codebase Smells! [2025-12-14]

[github-actions[bot]]

This Codebase Smells!

Welcome to the weekly ritual where we hold our noses and dive headfirst into your fragrant bouquet of tech debt. This repo is a charcuterie board of good intentions wrapped in duct tape and sprinkled with “it worked on my machine.” Brace yourselves: I brought hand sanitizer, a flamethrower, and a sense of humor. 💣💩

Table of Contents

• Revert Nukes Legit Files Like a Reckless Janitor
• Jest Ignores .js Tests Because Why Not
• ESLint Boldly Ignores All .js Files
• README vs Code: The “Codex TOML” Gaslighting
• TypeScript: SkipLibCheck Is the Coward’s Button
• CLI Dependency Drift and Old Yargs
• Minor: VSCode Settings Revert Uses Wrong Logging Prefix
• If You’re Shocked There Aren’t More Issues, Same

Revert Nukes Legit Files Like a Reckless Janitor

Revert tries to “clean up” config.toml and other files, risking deletion of real project configurations. ⚠️

• You unconditionally include config.toml in a hard-coded deletion list and unlink it if no .bak exists—congrats, you just deleted someone’s real OpenHands or unrelated TOML config. 🤦
• Backups are restored if present, otherwise you swing the axe; there’s zero provenance check. This is the code equivalent of “I saw a file – I deleted it.”

Files

• src/core/revert-engine.ts
• src/paths/mcp.ts
• README.md (documents OpenHands targeting config.toml)

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 config.toml (so it’s very likely to exist and be legit):
  • 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 remove files that:
  • Were created by Ruler (write a provenance marker or manifest).
  • Or have a .bak present. If no backup, warn and skip unless --force-extra-cleanup.
• Add content-based checks for config.toml (e.g., known OpenHands MCP markers) before touching it.
• Gate risky deletions behind a flag: --force-extra-cleanup, with a big red warning.

Jest Ignores .js Tests Because Why Not

Your Jest config runs TypeScript tests but ditches .js tests—several exist. Test coverage, meet trash can. 🗑️

• testMatch only includes TS patterns; .js tests in tests/unit/core/*.test.js are excluded, silently.
• CI stability suffers, and people wonder why regressions slip through. Spoiler: because tests aren’t running.

Files

• jest.config.js
• tests/unit/core/RuleProcessor.test.js
• tests/unit/core/FileSystemUtils.test.js

Code

• Missing JS patterns in Jest:
  • Lines 3–6: https://github.com/intellectronica/ruler/blob/main/jest.config.js#L3-L6

    preset: 'ts-jest',
    testEnvironment: 'node',
    testMatch: ['**/tests/**/*.{test,spec}.ts', '**/*.test.ts'],

• Example JS tests that won’t run:
  • RuleProcessor test file:
    • https://github.com/intellectronica/ruler/blob/main/tests/unit/core/RuleProcessor.test.js
  • FileSystemUtils test file:
    • https://github.com/intellectronica/ruler/blob/main/tests/unit/core/FileSystemUtils.test.js

Suggestions

• Update testMatch to include JS:
  • testMatch: ['**/tests/**/*.{test,spec}.ts', '**/tests/**/*.{test,spec}.js', '**/*.test.ts', '**/*.test.js']
• Alternatively set transform for JS or use testRegex to catch both.
• Run npm test locally and in CI to verify JS tests execute.

ESLint Boldly Ignores All .js Files

Your ESLint config excludes *.js, which is adorable given you have JS test files. Linting? Not today, Satan. 🙃

• The ESLint ignores block includes *.js, so JS is never linted.
• Combined with Jest skipping JS tests, you’re basically begging for entropy.

Files

• eslint.config.mjs
• tests/unit/core/*.js

Code

• ESLint ignores JS globally:
  • Lines 21–22: https://github.com/intellectronica/ruler/blob/main/eslint.config.mjs#L21-L22

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

Suggestions

• Remove *.js from ignores; instead, limit to built files:
  • ignores: ['dist/**', 'node_modules/**']
• Add overrides for tests if needed (e.g., relaxed rules in tests/**).
• Run npm run lint and fix any newly unearthed gremlins.

README vs Code: The “Codex TOML” Gaslighting

README says Codex uses .codex/config.toml; code tries .codex/config.json. Pick one. Preferably the correct one. 🤨

• This mismatch confuses users and breaks expectations; “it’s in TOML!” vs “jk it’s JSON.”
• Configuration loaders and propagators must be consistent or at least support both formats deterministically.

Files

• README.md
• src/paths/mcp.ts

Code

• README (claims TOML path):
  • Lines 62–66 (table row): https://github.com/intellectronica/ruler/blob/main/README.md#L62-L66

    | OpenAI Codex CLI | `AGENTS.md` | `.codex/config.toml` |
    

• Code (targets JSON):
  • Lines 27–29: https://github.com/intellectronica/ruler/blob/main/src/paths/mcp.ts#L27-L29

    case 'OpenAI Codex CLI':
      candidates.push(path.join(projectRoot, '.codex', 'config.json'));

Suggestions

• Align on TOML (as documented) or explicitly support both:
  • Prefer TOML; if JSON exists, load and migrate or merge with precedence rules.
• Update getNativeMcpPath to check .toml then .json with logging.
• Fix README to reflect actual behavior if you decide otherwise.

TypeScript: SkipLibCheck Is the Coward’s Button

skipLibCheck: true hides type issues in dependency types and can conceal real bugs. It’s fine for speed but risky for quality. ⚠️

• With strict mode enabled, skipping lib checks undermines the rigor you’re pretending to have.
• Combined with multiple agent adapters, subtle incompatibilities can slip through.

Files

• tsconfig.json

Code

• Skipping lib checks:
  • Lines 109–115: https://github.com/intellectronica/ruler/blob/main/tsconfig.json#L109-L115

    "skipLibCheck": true

Suggestions

• Set skipLibCheck: false and fix the fallout.
• Keep strict mode, consider enabling noImplicitReturns and noUnusedLocals once clean.
• If performance suffers, use composite projects or targeted excludes rather than global lib suppression.

CLI Dependency Drift and Old Yargs

You’re using yargs v18 with Node 18+, which is… vintage. Not charming-vintage—security-audit-vintage. 🧻

• yargs@^18.0.0 lags behind modern Node behaviors and features; potential bug surface.
• In a CLI that parses flags for critical actions (like deleting files), dependency hygiene matters.

Files

• package.json
• src/cli/*

Code

• Dependency versions:
  • Lines 66–71: https://github.com/intellectronica/ruler/blob/main/package.json#L66-L71

    "dependencies": {
      "@iarna/toml": "^2.2.5",
      "js-yaml": "^4.1.0",
      "yargs": "^18.0.0",
      "zod": "^4.1.12"
    }

Suggestions

• Upgrade yargs to latest v22+.
• Re-run npm audit and fix any flagged issues.
• Validate CLI parsing against README examples post-upgrade.

Minor: VSCode Settings Revert Uses Wrong Logging Prefix

You use actionPrefix without calling it, printing a function reference or wrong content. It’s a small paper cut but it bleeds in logs. 🩹

• In removeAdditionalAgentFiles, you logged with ${actionPrefix} rather than ${actionPrefix(dryRun)} at least once.
• Result: confusing logs and inconsistent dry-run messaging.

Files

• src/core/revert-engine.ts

Code

• Incorrect logging:
  • Lines 365–369: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L365-L369

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

• Correct pattern is used earlier:
  • Lines 321–323: https://github.com/intellectronica/ruler/blob/main/src/core/revert-engine.ts#L321-L323

    const prefix = actionPrefix(dryRun);

Suggestions

• Replace ${actionPrefix} with ${actionPrefix(dryRun)} everywhere.
• Add unit coverage that asserts on dry-run messages for revert paths.

If You’re Shocked There Aren’t More Issues, Same

I expected more tragedy. The rest looks reasonably organized: strict TS, solid test spread, clear README, and robust agent handling. It’s not great, but it’s not on fire. Yet. 🔥

• Keep tightening type checks and test inclusivity.
• Align docs and behavior.
• Stop deleting random user files like a chaotic goblin.

---

Bold fixes summary:

• Make revert safe: provenance markers, backup-only deletions, --force-extra-cleanup.
• Include .js in Jest testMatch; stop ignoring .js in ESLint.
• Align Codex path format (prefer TOML or dual support).
• Disable skipLibCheck and fix resulting type issues.
• Upgrade yargs; audit dependencies.
• Fix actionPrefix logging in revert.

Do these, and your repo will smell less like a landfill and more like a functional engineering project. Maybe. 🤞