This Codebase Smells! [2026-06-14] #577

2026-06-14T14:49:51Z

github-actions[bot]
Bot Jun 14, 2026

This Codebase Smells!

This week’s bouquet is copy-paste flambé with top notes of stale agent metadata, a heart of surprise JSON behavior, and a lingering finish of recursive filesystem tourism. 🤮 The repo is trying very hard — there are tests everywhere — but it keeps cashing that goodwill on architecture that looks like it was assembled during a caffeine-fueled hostage negotiation.

I picked the highest-payoff fixes: the stuff that buys back maintainability, robustness, performance, and developer sanity without requiring an exorcist. 💀

Agent Metadata Is Everywhere

🪦 Most agent behavior is plain metadata, but the repo stores that metadata as classes, constants, registries, and docs like duplication is a lifestyle choice.

ClaudeAgent is basically a few strings wearing an inheritance trench coat, while allAgents manually instantiates the whole zoo and README.md keeps a second support matrix alive by hand. That is not elegant abstraction; that is distributed bookkeeping with extra ceremony.
Every new agent or path tweak now wants synchronized edits across runtime code, constants, and documentation. Drift is not a risk here — it is the default future.

Files

README.md
src/agents/index.ts
src/agents/ClaudeAgent.ts
src/constants.ts

Code

Support matrix duplicated in docs:

README.md lines 57–62: https://github.com/intellectronica/ruler/blob/main/README.md#L57-L62

| Agent                  | Rules File(s)                                  | MCP Configuration / Notes                        | Skills Support / Location | Subagents Support / Location |
| ---------------------- | ---------------------------------------------- | ------------------------------------------------ | ------------------------- | ---------------------------- |
| AGENTS.md              | `AGENTS.md`                                    | (pseudo-agent ensuring root `AGENTS.md` exists)  | -                         | -                            |
| GitHub Copilot         | `AGENTS.md`                                    | `.vscode/mcp.json`                               | `.claude/skills/`         | `.github/agents/`            |
| Claude Code            | `CLAUDE.md`                                    | `.mcp.json`                                      | `.claude/skills/`         | `.claude/agents/`            |
| OpenAI Codex CLI       | `AGENTS.md`                                    | `.codex/config.toml`                             | `.codex/skills/`          | `.codex/agents/` (`.toml`)   |

Manual runtime registry:

src/agents/index.ts lines 38–46: https://github.com/intellectronica/ruler/blob/main/src/agents/index.ts#L38-L46

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

Trivial adapter class:

src/agents/ClaudeAgent.ts lines 7–18: https://github.com/intellectronica/ruler/blob/main/src/agents/ClaudeAgent.ts#L7-L18

export class ClaudeAgent extends AbstractAgent {
  getIdentifier(): string {
    return 'claude';
  }

  getName(): string {
    return 'Claude Code';
  }

  getDefaultOutputPath(projectRoot: string): string {
    return path.join(projectRoot, 'CLAUDE.md');
  }

Another path matrix living elsewhere:

src/constants.ts lines 57–64: https://github.com/intellectronica/ruler/blob/main/src/constants.ts#L57-L64

export const RULER_SKILLS_PATH = '.ruler/skills';
export const CLAUDE_SKILLS_PATH = '.claude/skills';
export const CODEX_SKILLS_PATH = '.codex/skills';
export const OPENCODE_SKILLS_PATH = '.opencode/skills';
export const PI_SKILLS_PATH = '.pi/skills';
export const GOOSE_SKILLS_PATH = '.agents/skills';
export const VIBE_SKILLS_PATH = '.vibe/skills';
export const ROO_SKILLS_PATH = '.roo/skills';

Suggestions

Define one AgentDefinition registry with identifier, display name, rules path, MCP path/key, skills/subagent support, and optional custom transformers.
Generate allAgents, CLI help, and as much of the README support table as possible from that registry.
Keep real classes only for genuinely weird integrations; let boring agents be boring data.

Propagation Logic Is Copy Paste Exhaust

🤢 Skills and subagent propagation are built as clone armies even though both processors already contain target maps begging for a shared driver.

SkillsProcessor has a path map, then still hand-writes a parade of propagateSkillsFor* functions that all do the same access-check / dry-run / temp-dir / remove / replace dance. Incredible. A standing ovation for avoidable maintenance debt. 💩
SubagentsProcessor pulls the same stunt again. So every fix to atomic writes, cleanup, dry-run output, or logging now has to be copied into multiple near-identical branches and hopefully not forgotten in one of them.

Files

src/core/SkillsProcessor.ts
src/core/SubagentsProcessor.ts

Code

The skills target map already exists:

src/core/SkillsProcessor.ts lines 81–85: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L81-L85

const targetPaths: Record<SkillTarget, string> = {
  claude: CLAUDE_SKILLS_PATH,
  codex: CODEX_SKILLS_PATH,
  opencode: OPENCODE_SKILLS_PATH,
  pi: PI_SKILLS_PATH,

Then the first clone shows up:

src/core/SkillsProcessor.ts lines 503–520: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L503-L520

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);

  try {
    await fs.access(skillsDir);
  } catch {
    return [];
  }

  if (options.dryRun) {
    return [`Copy skills from ${RULER_SKILLS_PATH} to ${CLAUDE_SKILLS_PATH}`];
  }

And the second clone says the same thing with a fake moustache:

src/core/SkillsProcessor.ts lines 561–579: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L561-L579

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);

  try {
    await fs.access(skillsDir);
  } catch {
    return [];
  }

  if (options.dryRun) {
    return [`Copy skills from ${RULER_SKILLS_PATH} to ${CODEX_SKILLS_PATH}`];
  }

Subagents already have a target map too:

src/core/SubagentsProcessor.ts lines 82–86: https://github.com/intellectronica/ruler/blob/main/src/core/SubagentsProcessor.ts#L82-L86

const SUBAGENT_TARGET_PATHS: Record<SubagentTarget, string> = {
  claude: CLAUDE_SUBAGENTS_PATH,
  cursor: CURSOR_SUBAGENTS_PATH,
  codex: CODEX_SUBAGENTS_PATH,
  copilot: COPILOT_SUBAGENTS_PATH,

Then comes another hand-written per-target writer:

src/core/SubagentsProcessor.ts lines 324–342: https://github.com/intellectronica/ruler/blob/main/src/core/SubagentsProcessor.ts#L324-L342

export async function propagateSubagentsForClaude(
  projectRoot: string,
  subagents: SubagentInfo[],
  options: PropagateOptions,
): Promise<string[]> {
  if (subagents.length === 0) return [];
  const targetDir = path.join(projectRoot, CLAUDE_SUBAGENTS_PATH);
  if (options.dryRun) {
    return subagents.map(
      (s) =>
        `Write ${path.join(CLAUDE_SUBAGENTS_PATH, getSourceRelativeMdPath(s))}`,
    );
  }

Suggestions

Replace per-target functions with a shared propagateToTarget(definition, items, options) helper for skills and another for subagents.
Store path, extension, serializer, and dry-run formatter in the target definition map.
Convert tests to table-driven target matrices so adding an integration means adding data, not another 40-line ritual.

Config Parsing Is a JSON Roulette Wheel

🎰 Malformed config files get silently replaced in some agents, hard-failed in others, and barely validated anywhere, which is a splendid way to make bugs feel random.

OpenCodeAgent and CrushAgent swallow parse failures and rebuild config; QwenCodeAgent rethrows non-ENOENT errors. Same class of bad input, totally different outcome depending on which adapter touched the file first. Love that for support tickets.
The repo already has shared validation primitives, but agent adapters mostly freelance with Record<string, unknown> and vibes. That is not robustness; that is config fan fiction.

Files

src/agents/OpenCodeAgent.ts
src/agents/CrushAgent.ts
src/agents/QwenCodeAgent.ts
src/mcp/validate.ts

Code

OpenCode silently recovers by overwriting:

src/agents/OpenCodeAgent.ts lines 74–78: https://github.com/intellectronica/ruler/blob/main/src/agents/OpenCodeAgent.ts#L74-L78

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

Crush does the same trick:

src/agents/CrushAgent.ts lines 112–119: https://github.com/intellectronica/ruler/blob/main/src/agents/CrushAgent.ts#L112-L119

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

Qwen instead rethrows bad JSON:

src/agents/QwenCodeAgent.ts lines 44–50: https://github.com/intellectronica/ruler/blob/main/src/agents/QwenCodeAgent.ts#L44-L50

try {
  const raw = await fs.readFile(settingsPath, 'utf8');
  existingSettings = JSON.parse(raw);
} catch (err: unknown) {
  if ((err as NodeJS.ErrnoException).code !== 'ENOENT') {
    throw err;
  }
}

A shared validator exists and mostly gets ignored:

src/mcp/validate.ts lines 7–23: https://github.com/intellectronica/ruler/blob/main/src/mcp/validate.ts#L7-L23

export function validateMcp(data: unknown): void {
  const mcpServers =
    data && typeof data === 'object'
      ? (data as Record<string, unknown>).mcpServers
      : undefined;

  if (
    !data ||
    typeof data !== 'object' ||
    !('mcpServers' in data) ||
    !mcpServers ||
    typeof mcpServers !== 'object' ||
    Array.isArray(mcpServers)
  ) {
    throw new Error(
      '[ruler] Invalid MCP config: must contain an object property "mcpServers" (Ruler style)',
    );
  }
}

Suggestions

Add one shared loadJsonConfig/loadTomlConfig utility that distinguishes missing-file from parse/shape errors and returns typed data.
Validate adapter inputs with Zod or the existing MCP validator before merge logic; do not treat malformed user config as a blank slate unless repair mode is explicit.
Centralize merge semantics so "merge" vs "overwrite" and error handling stop changing by adapter mood swing.

Nested Discovery Walks the Whole Damn Repo

🥴 Nested mode does multiple recursive filesystem tours and quietly swallows directory-read failures, because apparently the fastest code is the one that explores everything and explains nothing.

In nested mode, apply first crawls downward to find every .ruler, then recursively walks each discovered directory again to read Markdown. On a large repo, that turns a simple config command into a scenic monorepo backpacking trip.
The ignore list is hard-coded and short, and unreadable directories disappear into catch {} oblivion. So when performance or correctness gets interesting, diagnostics get worse. Beautifully cursed. 🤡

Files

src/core/apply-engine.ts
src/core/FileSystemUtils.ts

Code

Nested apply does discovery and then a second per-dir walk:

src/core/apply-engine.ts lines 61–80: https://github.com/intellectronica/ruler/blob/main/src/core/apply-engine.ts#L61-L80

const { dirs: rulerDirs } = await findRulerDirectories(
  projectRoot,
  localOnly,
  true,
);
...
for (const rulerDir of rulerDirs) {
  const config = await loadConfigForRulerDir(...);
  const files = await FileSystemUtils.readMarkdownFiles(rulerDir, {
    includeAgents: shouldIncludeAgentsInRules(config),
  });
}

Ignore list is hand-maintained and incomplete:

src/core/FileSystemUtils.ts lines 10–21: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L10-L21

const DEFAULT_NESTED_DISCOVERY_IGNORES = new Set([
  '__fixtures__',
  '__generated__',
  'build',
  'coverage',
  'dist',
  'fixtures',
  'generated',
  'node_modules',
  'temp',
  'tmp',
]);

Discovery recurses and hides read failures:

src/core/FileSystemUtils.ts lines 417–445: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L417-L445

async function findRulerDirs(dir: string) {
  try {
    const entries = await fs.readdir(dir, { withFileTypes: true });
    ...
    await findRulerDirs(fullPath);
  } catch {
    // ignore errors when reading directories
  }
}

Then markdown loading recurses again:

src/core/FileSystemUtils.ts lines 213–239: https://github.com/intellectronica/ruler/blob/main/src/core/FileSystemUtils.ts#L213-L239

if (isDir) {
  const relativeFromRoot = path.relative(rulerDir, fullPath);
  ...
  await walk(fullPath);
} else if (isFile && entry.name.endsWith('.md')) {
  const content = await fs.readFile(fullPath, 'utf8');
  mdFiles.push({ path: fullPath, content });
}

Suggestions

Replace the two-pass recursion with one traversal that discovers .ruler dirs and collects their markdown manifests together.
Make ignores configurable and/or .gitignore-aware, and add a depth/entry budget so huge repos do not trigger surprise filesystem safaris.
Surface skipped directories as warnings with path + errno instead of silent disappearance.

Warning State Leaks Into Production

🤡 “Warn once” is implemented with module-global booleans, and the tests import reset levers from production code to keep the whole circus running.

Logging behavior depends on process history instead of explicit call context. Same input, different output after the first call. That is cute for a shell script; it is a lousy contract for reusable core code.
The exported _reset...ForTests() hooks are the dead giveaway: tests need secret buttons because the design hides state in modules. That is not a test helper. That is an architectural cry for help. 🚨

Files

src/core/SkillsProcessor.ts
src/core/SubagentsProcessor.ts
src/core/ConfigLoader.ts
tests/subagents-rules-concatenation.test.ts

Code

Skills warning cache lives for the whole process:

src/core/SkillsProcessor.ts lines 108–118: https://github.com/intellectronica/ruler/blob/main/src/core/SkillsProcessor.ts#L108-L118

let hasWarnedExperimental = false;

function warnOnceExperimental(verbose: boolean, dryRun: boolean): void {
  if (hasWarnedExperimental) {
    return;
  }
  hasWarnedExperimental = true;
  logWarn(

Subagents exports a reset backdoor:

src/core/SubagentsProcessor.ts lines 136–151: https://github.com/intellectronica/ruler/blob/main/src/core/SubagentsProcessor.ts#L136-L151

let hasWarnedExperimental = false;

function warnOnceExperimental(dryRun: boolean): void {
  if (hasWarnedExperimental) return;
  hasWarnedExperimental = true;
  logWarn(
    'Subagents support is experimental and behavior may change in future releases.',
    dryRun,
  );
}

export function _resetExperimentalWarningForTests(): void {
  hasWarnedExperimental = false;
}

ConfigLoader exports another one:

src/core/ConfigLoader.ts lines 20–33: https://github.com/intellectronica/ruler/blob/main/src/core/ConfigLoader.ts#L20-L33

let _legacySubagentsWarned = false;

function warnLegacySubagentsSection(): void {
  if (_legacySubagentsWarned) return;
  _legacySubagentsWarned = true;
  logWarn(
    '`[subagents]` is deprecated; rename it to `[agents]` in your ruler.toml. ' +
      'The legacy section is honored for now and will be removed in a future release.',
  );
}

export function _resetLegacySubagentsWarningForTests(): void {
  _legacySubagentsWarned = false;
}

The tests have to call the escape hatches:

tests/subagents-rules-concatenation.test.ts lines 74–77: https://github.com/intellectronica/ruler/blob/main/tests/subagents-rules-concatenation.test.ts#L74-L77

beforeEach(async () => {
  tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), 'ruler-subagents-rules-'));
  _resetExperimentalWarningForTests();
  _resetLegacySubagentsWarningForTests();
});

Suggestions

Move warning deduplication into a per-run context object passed through apply, or into a logger instance with scoped state.
Keep production modules free of test-only exports; tests should construct fresh state, not poke module internals with secret reset wands.
If the CLI wants once-per-process warning suppression, keep that policy at the CLI boundary, not in reusable core modules.

Navigation Menu

This Codebase Smells! [2026-06-14] #577

Uh oh!

{{title}}

Uh oh!

Replies: 0 comments

Select a reply

Uh oh!

This Codebase Smells! [2026-06-14] #577

Uh oh!

github-actions[bot] Bot Jun 14, 2026

This Codebase Smells!

Table of Contents

Agent Metadata Is Everywhere

Files

Code

Suggestions

Propagation Logic Is Copy Paste Exhaust

Files

Code

Suggestions

Config Parsing Is a JSON Roulette Wheel

Files

Code

Suggestions

Nested Discovery Walks the Whole Damn Repo

Files

Code

Suggestions

Warning State Leaks Into Production

Files

Code

Suggestions

Replies: 0 comments

github-actions[bot]
Bot Jun 14, 2026