Skip to content

Latest commit

 

History

History
415 lines (310 loc) · 15.8 KB

AGENTS.md

File metadata and controls

415 lines (310 loc) · 15.8 KB

Project Overview

  • Runtime: Bun (version pinned in .bun-version)
  • Language: TypeScript (strict mode, ESM)
  • Key deps: zod (validation), pino (logging)
  • Setup: bun install

Bun Runtime

  • Documentation index: https://bun.com/llm.txt
  • When you need to look up any Bun API, feature, or usage pattern, fetch the above URL to find the relevant doc page, then read the specific page for details.

Development Standards

  • After each code modification, you must execute: bun run lint:fix bun run ts-check bun run knip
  • For any change that affects commands or CLI behavior, you must check whether documentation under docs/ needs to be updated and update it when necessary
  • Documentation under docs/commands*.md should describe the user-facing CLI contract only: command purpose, arguments, options, stable output shapes, and externally observable behavior. Do not document internal implementation details such as validator order, AJV usage, schema patching, or other internal lint mechanics unless the user explicitly asks for that level of detail.
  • Comments must be in English
  • When generating UUIDs, you must use v7 and must use bun's randomUUIDv7 function
  • Avoid using regular expressions when possible

Bundled Skill Markdown

  • When editing bundled SKILL.md files or related Markdown under contrib/skills/shared, use agentic-markdown directives for agent-specific content instead of creating separate agent copies.
  • Prefer capability-based conditions over long agent-name lists. If a behavior depends on a feature or tool availability, expose that as a render variable and check the variable instead of writing conditions such as agent=claude|hermes|codebuddy|qoderwork|workbuddy.
  • Use presence checks for optional capability variables. With agentic-markdown 0.0.4+, a missing variable in agentic:if is treated as absent instead of throwing:
<!-- agentic:if skillSelectionPromptTool -->
Use <!-- agentic:var skillSelectionPromptTool --> for short multiple-choice prompts.
<!-- agentic:endif -->
  • Use agent-name conditions only when the content truly belongs to specific agents rather than a shared capability:
<!-- agentic:if agent=claude|gemini -->
Claude and Gemini content.
<!-- agentic:endif -->
  • Use variables for agent-specific values:
oo skills preflight --agent <!-- agentic:var agent -->

Command Telemetry

  • Every new or changed user-facing CLI command must make an explicit telemetry decision during implementation and review.
  • The generic executeCli path already emits the command event, so do not add manual command-level emit calls.
  • For useful command-specific dimensions, call context.telemetry?.recordProperties(...) from the command handler with only low-cardinality, privacy-safe enums, booleans, counts, or buckets. Prefer helpers in src/application/telemetry/buckets.ts; for skill-related command properties, use src/application/commands/skills/telemetry.ts.
  • Never record raw user input, paths, cwd, filenames, URL hosts, account IDs/names/emails, usernames, hostnames, error messages, stack traces, tokens, secrets, or free-form option values. Record bucketed values or stable enums instead.
  • Commands that must not be reported, such as oo telemetry * and the internal telemetry flush command, must set excludeFromTelemetry: true or call context.telemetry?.suppressCurrentInvocation(); do not rely on handler branching alone.
  • Every registered command path must have an explicit entry in src/application/commands/telemetry-decisions.test.ts; add or update that manifest whenever adding, renaming, removing, or changing telemetry behavior for a command. This architecture test is the hard guard that makes forgotten telemetry decisions fail in bun run test.
  • The same architecture test rejects forbidden/private property names, common sensitive suffixes, and base telemetry property names declared in the command telemetry decision manifest; extend that guard before allowing any new sensitive key pattern.
  • Do not pass base telemetry property names such as command_full, distinct_id, schema_version, or privacy control properties through recordProperties(...); both the architecture test and the payload builder reject command-specific properties that attempt to override base telemetry fields.
  • When adding command-specific telemetry properties, add or update tests that assert the expected safe property shape and absence of raw/private values. For command behavior changes, still update docs/commands*.md only for the user-facing CLI contract, not internal telemetry details.

Code Quality Rules

These rules are extracted from past refactoring sessions to prevent recurring code smells.

No Trivial Wrappers

Never create single-line functions that merely delegate to another function without adding logic, validation, or semantic value.

// BAD - wrapper adds nothing
function createAuthColors(context: CliExecutionContext): TerminalColors {
    return createWriterColors(context.stdout);
}
function serializeCacheValue<Value>(value: Value): string {
    return JSON.stringify(value);
}

// GOOD - use the real function directly
const colors = createWriterColors(context.stdout);
const serialized = JSON.stringify(value);

The same applies to constant aliases (const OO_BRAND_NAME = APP_NAME) and identity type-check functions (function define<T>(d: T): T { return d; } — use satisfies instead).

DRY — Extract Shared Utilities

When identical logic appears in 2+ files, extract it to a shared module. Common candidates:

  • Auth account checking → shared/auth-utils.ts
  • Output helpers → shared/output.ts
  • Database open/close → sqlite-utils.ts
  • File error checks → application/shared/fs-errors.ts

Parameterize the varying parts (error message keys, config values) rather than duplicating the whole function.

// BAD - same pattern in 4 files with different error keys
async function requireCurrentPackageInfoAccount(ctx) { /* ... */ }
async function requireCurrentSkillsSearchAccount(ctx) { /* ... */ }

// GOOD - one shared function, parameterized
export async function requireCurrentAccount(
    context: CliExecutionContext,
    authRequiredKey: string,
    accountMissingKey: string,
): Promise<AuthAccount> { /* ... */ }

Trust the Type System

Do not re-parse or re-validate data whose type is already guaranteed by the function signature. Internal functions should trust their callers.

// BAD - authFile is already typed as AuthFile
function renderAuthFile(authFile: AuthFile): string {
    const parsed = authFileSchema.parse(authFile); // redundant
}

// GOOD
function renderAuthFile(authFile: AuthFile): string {
    const lines = [renderTomlLine("id", authFile.id)]; // direct use
}

Use Modern, Idiomatic APIs

  • str.replaceAll(a, b) instead of str.split(a).join(b)
  • for (const char of str) for Unicode-aware iteration instead of manual index + surrogate pair handling
  • Bun.sleep(ms) instead of new Promise(r => setTimeout(r, ms)) in Bun environment
  • satisfies for type-checking object literals instead of identity wrapper functions

Cross-Platform Paths

This CLI is cross-platform. Never assume POSIX path separators in code, tests, snapshots, or assertions.

  • Use node:path helpers such as join(), resolve(), and relative() for path construction instead of manual string concatenation.
  • Do not assert raw paths with hardcoded "/" or "\\" separators.
  • Prefer comparing paths by composing the expected value with join() or by comparing path relationships via relative(), instead of inventing ad-hoc normalization helpers in tests.
// BAD - fails on Windows
expect(filePath.includes("/contrib/skills/codex/oo/")).toBeTrue();

// GOOD - compose the expected path with node:path
expect(filePath.includes(join("contrib", "skills", "codex", "oo"))).toBeTrue();

Guard Clauses — Fail First

Check error conditions at the top; let the success path be the default flow. Don't wrap success logic inside if (valid).

// BAD
function validate(id: string): void {
    if (id !== "") { return; }
    throw new TypeError("id required");
}

// GOOD
function validate(id: string): void {
    if (id === "") { throw new TypeError("id required"); }
}

No Fake Async

Never mark a function async if it contains no await. Remove async when all code paths are synchronous.

// BAD
async function listGitTags(): Promise<string[]> {
    return Bun.spawnSync(/* ... */); // synchronous
}

// GOOD
function listGitTags(): string[] {
    return Bun.spawnSync(/* ... */);
}

Minimize Export Surface

Only export symbols that are used by other modules. Internal helpers, types, and constants should remain module-private. Smaller public API = more refactoring freedom.

filter+map over flatMap-as-Filter

Use filter() for filtering and map() for transformation. Do not abuse flatMap() with empty arrays as a filtering mechanism — it obscures intent.

// BAD
entries.flatMap(e => e.isFile() ? [e.name] : []);

// GOOD
entries.filter(e => e.isFile()).map(e => e.name);

No Redundant Intermediate Variables

Don't create variables that pass through a value unchanged. If no transformation occurs, use the original directly.

// BAD
const selectedNames = request.names.flatMap((n) => { validate(n); return n; });
doWork(selectedNames);

// GOOD
for (const n of request.names) { validate(n); }
doWork(request.names);

Eliminate Dead Code

  • Remove unused functions, parameters, type fields, and unreachable branches
  • Remove if (error instanceof X) { throw error; } inside catch — restructure control flow instead
  • Remove JSON.stringify() === undefined checks — JSON.stringify never returns undefined

Parallel Async Operations

Use Promise.all() for independent async operations instead of sequential await.

// BAD
await removePath(pathA);
await removePath(pathB);

// GOOD
await Promise.all([removePath(pathA), removePath(pathB)]);

EAFP over LBYL for File Operations

Attempt the operation and catch the error, rather than pre-checking existence then reading. Reduces filesystem calls and avoids TOCTOU races.

// BAD - 3 filesystem calls
if (!(await directoryExists(p)))
    return undefined;
if (!(await fileExists(metaPath)))
    return undefined;
return await readMetadata(p);

// GOOD - 1 filesystem call
try { return await readFile(metaPath, "utf8"); }
catch (e) {
    if (isNodeNotFoundError(e))
        return undefined; throw e;
}

Data-Driven over Parallel Mappings

When multiple switch/map structures share the same keys, consolidate into a single configuration object.

// BAD - two separate mappings
const translations: Record<Status, string> = { valid: "...", invalid: "..." };
function readTone(s: Status) { switch(s) { case "valid": return "success"; ... } }

// GOOD - single config
const statusConfig = {
    valid: { tone: "success", key: "auth.status.valid" },
    invalid: { tone: "danger", key: "auth.status.invalid" },
} as const;

Precise Type Checks

Use !== undefined instead of truthy checks when empty string "" or 0 are valid values.

// BAD - filters out valid empty strings
if (value) { output(value); }

// GOOD
if (value !== undefined) { output(value); }

No Duplicate Computations

Compute an expression once, store in a variable, reuse. Especially inside switch statements and loops.

// BAD
switch (str.trim().toLowerCase()) {
    case "a": return str.trim().toLowerCase(); // computed twice
}

// GOOD
const normalized = str.trim().toLowerCase();
switch (normalized) {
    case "a": return normalized;
}

Single Source of Truth for Constants

Never duplicate constant values across files. Define once, import or re-export with aliases elsewhere.

Parameterize Common Patterns with Factories

When multiple definitions share the same shape with only one or two varying fields, use a factory function.

// BAD - 3 identical method bodies with different error keys
lang: { createError(v) { return new Err("errors.config.invalidLang", 2, { value: String(v ?? "") }); } },
dir:  { createError(v) { return new Err("errors.config.invalidDir",  2, { value: String(v ?? "") }); } },

// GOOD
function createValueErrorFactory(key: string) {
    return (v: unknown) => new Err(key, 2, { value: String(v ?? "") });
}
lang: { createError: createValueErrorFactory("errors.config.invalidLang") },
dir:  { createError: createValueErrorFactory("errors.config.invalidDir") },

Keep Function Parameters Narrow

Pass only what the function needs, not entire context objects. This improves testability and reduces coupling.

// BAD
function writeLine(context: CliExecutionContext, msg: string) {
    context.stdout.write(`${msg}\n`);
}

// GOOD
function writeLine(stream: Writer, msg: string) {
    stream.write(`${msg}\n`);
}

DRY in Tests — Extract Repeated Setup

When the same mock, stub, or setup object appears in multiple tests within the same file, extract it into a local factory function at the bottom of the file. Copy-pasted test setup is still copy-paste.

// BAD - identical 15-line mock in every test
test("case A", () => {
    const translator = { t: (key) => { switch (key) { case "x": return "X"; /* 10 more */ } } };
    // ...
});
test("case B", () => {
    const translator = { t: (key) => { switch (key) { case "x": return "X"; /* same 10 */ } } };
    // ...
});

// GOOD - one factory, all tests share it
test("case A", () => {
    const translator = createTranslatorStub();
    // ...
});
test("case B", () => {
    const translator = createTranslatorStub();
    // ...
});

function createTranslatorStub() {
    return { t: (key: string) => { switch (key) { case "x": return "X"; /* ... */ default: return key; } } };
}

Complete the Extraction — Propagate to Tests

When extracting a shared utility from production code, also replace any test helpers or inline expressions that duplicate the same logic. An extraction is incomplete if test files still contain local functions or raw inline code doing the same thing under a different name.

// BAD - shared utility extracted, but tests still duplicate it
// skill-metadata.ts (shared module)
export function renderSkillMetadataJson(metadata: object): string {
    return `${JSON.stringify(metadata, null, 2)}\n`;
}

// index.test.ts (local helper duplicates the shared utility)
function formatBundledSkillMetadataContent(version: string): string {
    return `${JSON.stringify({ version }, null, 2)}\n`;
}

// update.test.ts (inline expression duplicates the shared utility)
await Bun.write(path, `${JSON.stringify({ version: "1.0.0" }, null, 2)}\n`);

// GOOD - tests import the shared utility directly
import { renderSkillMetadataJson } from "./skill-metadata.ts";
await Bun.write(path, renderSkillMetadataJson({ version: "1.0.0" }));

try/finally over try/catch/rethrow for Cleanup

When the only purpose of a catch block is to clean up and rethrow, use finally instead. It eliminates the duplicate cleanup call and removes dead code in the success path.

// BAD - cleanup duplicated, catch block only rethrows
try {
    doWork();
} catch (error) {
    cleanup();
    throw error;
}
cleanup();

// GOOD
try {
    doWork();
} finally {
    cleanup();
}

Testing

  • Any modification must include sufficient tests
  • Do not write tests for Markdown files
  • Tests can only be run using bun run test
  • Test titles must be in English
  • The testing framework is bun's built-in framework
  • Test files should be placed in the same directory as the source files
  • If a helper function might be called by other test files, it must be placed in the __tests__/helpers.ts file. Otherwise, the function should be placed in the test file (at the end of that file).