This Codebase Smells! [2026-02-15]

[github-actions[bot]]

This Codebase Smells!

Welcome to another thrilling episode of “Who Hurt This Repo?” 🎭 This week’s special: hard-coded nukes, recursive spelunking into node_modules, and a passionate affair with any that would make TypeScript cry in strict mode. If the goal was “it works on my machine,” congratulations, you’ve achieved avant-garde chaos. 💥

Table of Contents

• Overzealous Revert Deletes Real Files
• Recursive Scans Dive Into node_modules
• Global Config in Home Directories Is a Footgun
• Type Safety: “any”-pocalypse
• Secrets Smuggled Into Config Files
• Gitignore Mutation: Path Games and Collateral Damage
• Logging: Centralized… Except When It Isn’t
• If You Can’t Find Enough Issues

Overzealous Revert Deletes Real Files

⛈️ Revert tries to “clean up” config.toml, risking deletion of legitimate project files.

• There’s a hard-coded deletion list including config.toml. That’s not “cleanup,” that’s a landmine for real projects. 💣
• OpenHands targets config.toml explicitly, so your “revert” can yoink unrelated configs if backups aren’t present. 🤦

Files

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

Code

• Deletion list includes config.toml:
  • 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 uses root config.toml (perfect conflict bait):
  • 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'));

• Codex expects MCP under TOML:
  • Lines 927–929: https://github.com/intellectronica/ruler/blob/main/src/core/apply-engine.ts#L927-L929

    // Codex CLI expects MCP servers under mcp_servers in config.toml.
    mcp_servers: servers ?? {},

Suggestions

• Gate removal of “generic” files behind detection: only remove if a .bak exists or if file contains Ruler provenance markers.
• Replace hard-coded list with a manifest emitted on apply; revert reads and removes only manifest-recorded outputs.
• Add --force-extra-cleanup to opt-in to risky generic deletions and print a giant “ARE YOU SURE” warning.

Recursive Scans Dive Into node_modules

🕳️ Directory crawlers only skip dot-folders; they lovingly traverse node_modules and your entire monorepo like a golden retriever on espresso.

• The downward scan avoids .git by boundary rules and skips directories starting with ., but not node_modules, dist, .next, etc. RIP performance. 🪦
• In large repos, this burns CPU and developer goodwill.

Files

• src/core/FileSystemUtils.ts

Code

• Only skip directories starting with “.” and avoid crossing nested .git repos:
  • Lines 240–256: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L240-L256

    // Recursively search subdirectories (but skip hidden directories like .git)
    if (!entry.name.startsWith('.')) {
      // Do not cross git repository boundaries (except the starting root)
      const gitDir = path.join(fullPath, '.git');
      // ...
      await findRulerDirs(fullPath);
    }

Suggestions

• Add an explicit exclusion list: node_modules, dist, build, coverage, .next, .turbo, .yarn, out, vendor.
• Support a --ignore flag and a config key to extend the skip set.
• Short-circuit traversal when directory size explodes; consider a max-depth or fast stat heuristic.

Global Config in Home Directories Is a Footgun

🏠 Writing and reading global config from ~/.config/ruler without robust guards invites confusion, permission errors, and shared-environment chaos.

• The XDG path fallback is fine, but error logging for a missing global directory leaks noise and can mislead users. 📢
• Global writes from CLI can step on multi-user systems or CI caches.

Files

• src/core/FileSystemUtils.ts
• src/core/ConfigLoader.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');

• Loading global config when local missing:
  • Lines 144–147: https://github.com/intellectronica/ruler/blob/main/src/core/ConfigLoader.ts#L144-L147

    const xdgConfigDir =
      process.env.XDG_CONFIG_HOME || path.join(os.homedir(), '.config');
    configFile = path.join(xdgConfigDir, 'ruler', 'ruler.toml');

• init --global writes into XDG config:
  • Lines 138–147: https://github.com/intellectronica/ruler/blob/main/src/cli/handlers.ts#L138-L147

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

Suggestions

• Gate global reads/writes behind explicit flags; never implicitly fall back to global config without a loud, user-facing message.
• Add --no-global-fallback and default it on in CI; log only when verbose.
• Store a per-install marker (with version) in ~/.config/ruler and validate provenance before reading.

Type Safety: “any”-pocalypse

🔫 TypeScript is here… but you brought any and silenced ESLint. Bold strategy, Cotton.

• Multiple eslint-disable on no-explicit-any and liberal casting betrays type safety and future maintainability. 🤮
• iarna-toml types are declared as any, bleeding dynamic blobs everywhere.

Files

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

Code

• Global disable and any signatures:
  • 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;

• Casting to any to stringify:
  • Lines 176–179: https://github.com/intellectronica/ruler/blob/main/src/agents/MistralVibeAgent.ts#L176-L179

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

• Index signatures of doom:
  • Lines 14–19: https://github.com/intellectronica/ruler/blob/main/src/agents/CodexCliAgent.ts#L14-L19

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

Suggestions

• Replace custom @iarna/toml d.ts with the official @types/iarna__toml (already in devDeps!) and remove the any blanket.
• Introduce strict interfaces for MCP configs and agent config shapes; use zod-derived types where you already validate.
• Ban eslint-disable @typescript-eslint/no-explicit-any; enforce unknown + proper narrowing.

Secrets Smuggled Into Config Files

🔐 Headers with Bearer tokens get extracted and written as api_key into TOML. That’s a “no” from security.

• Transforming headers into persisted API keys violates least-privilege and makes creds file-resident and diffable. 💀
• Mixed array normalization ensures everything becomes an object, encouraging credential persistence.

Files

• src/mcp/propagateOpenHandsMcp.ts

Code

• Extracting Bearer:
  • Lines 40–61: https://github.com/intellectronica/ruler/blob/main/src/mcp/propagateOpenHandsMcp.ts#L40-L61

    const authHeader = headers.Authorization || headers.authorization;
    // ...
    if ((hasOnlyAuthHeader || hasOnlyStandardHeaders) && authHeader.startsWith('Bearer ')) {
      return authHeader.substring('Bearer '.length);
    }

• Writing api_key to config:
  • Lines 70–74: https://github.com/intellectronica/ruler/blob/main/src/mcp/propagateOpenHandsMcp.ts#L70-L74

    const apiKey = extractApiKey(headers);
    if (apiKey) {
      return { url, api_key: apiKey };
    }

Suggestions

• Never persist secrets to disk; prefer environment variables or OS keychain integration.
• Strip Authorization headers entirely; if required, store a redacted marker and prompt at runtime.
• Add a --no-secrets hard default; require --allow-secrets to be set for testing-only workflows.

Gitignore Mutation: Path Games and Collateral Damage

🧩 Ruler rewrites .gitignore paths, normalizing by project basename and injecting leading slashes—can desync user-managed patterns and create weird path anchoring behaviors.

• The basename heuristic can miscompute relative paths in nested repos, causing path mismatches. 🌀
• Replacement behavior risks clobbering nuanced ignore rules outside the Ruler block.

Files

• src/core/GitignoreUtils.ts

Code

• Basename-stripping trickery:
  • Lines 41–49: https://github.com/intellectronica/ruler/blob/main/src/core/GitignoreUtils.ts#L41-L49

    const projectBasename = path.basename(normalizedProjectRoot);
    if (normalizedPath.startsWith(projectBasename + path.sep)) {
      relative = normalizedPath.substring(projectBasename.length + 1);
    } else {
      relative = normalizedPath;
    }

• Force leading slash + filter out .ruler:
  • Lines 54–60: https://github.com/intellectronica/ruler/blob/main/src/core/GitignoreUtils.ts#L54-L60

    return !p.includes('/.ruler/') && !p.startsWith('.ruler/');
    // Always write full repository-relative paths (prefix with leading /)
    return p.startsWith('/') ? p : `/${p}`;

• Replacement write:
  • Lines 62–76: https://github.com/intellectronica/ruler/blob/main/src/core/GitignoreUtils.ts#L62-L76

    const newPaths = relativePaths.filter((p) => !existingPaths.includes(p));
    const newContent = updateGitignoreContent(existingContent, allRulerPaths);
    await fs.writeFile(gitignorePath, newContent);

Suggestions

• Use gitignore parser libs to preserve structure; append without basing path normalization on project basename.
• Record and diff Ruler-managed entries only; don’t re-normalize user entries or anchors.
• Add dry-run previews with unified diffs before writing.

Logging: Centralized… Except When It Isn’t

🗣️ You have nice centralized logging helpers, and then you bypass them with raw console.log in CLI handlers. Pick a lane.

• Mixing direct console calls and helpers makes verbosity flags and dry-run prefixes inconsistent. 😵‍💫
• Centralization exists—use it.

Files

• src/constants.ts
• src/cli/handlers.ts

Code

• Centralized logging helpers:
  • Lines 29–41: https://github.com/intellectronica/ruler/blob/main/src/constants.ts#L29-L41

    export function logInfo(...){ console.log(`${prefix} ${message}`); }
    export function logWarn(...){ console.warn(`${prefix} ${message}`); }
    export function logError(...){ console.error(`${prefix} ${message}`); }

• Raw console usage in handlers:
  • Lines 200–208: https://github.com/intellectronica/ruler/blob/main/src/cli/handlers.ts#L200-L208

    console.log(`[ruler] Created ${instructionsPath}`);
    console.log(`[ruler] ${DEFAULT_RULES_FILENAME} already exists, skipping`);
    console.log(`[ruler] Created ${tomlPath}`);
    console.log(`[ruler] ruler.toml already exists, skipping`);

Suggestions

• Replace all direct console.* with logInfo/logWarn/logError/logVerboseInfo for consistent prefixes and dry-run handling.
• Add a --json mode that emits structured logs; helpful for CI and tooling.

If You Can’t Find Enough Issues

Good news: tests are numerous and thoughtful. Bad news: they won’t save you from the above footguns. Keep them, expand coverage for “dangerous cleanup,” secrets handling, and traversal exclusions.

Suggestions

• Add tests explicitly asserting:
  • Revert never deletes generic config.toml without a .bak or Ruler marker.
  • Directory traversal skips node_modules et al.
  • Secrets are not persisted in any path when propagation runs.